author | Mathieu Leplatre <mathieu@mozilla.com> |
Tue, 08 May 2018 16:30:40 +0200 | |
changeset 417835 | cfba5652a29cc056bf3ce25e765a922abc7581a4 |
parent 417834 | ce79538855b7e2f5b0f378865fc7878885e27c37 |
child 417836 | 1591f55d3e32fc488133d3ea9e7a2a3163066663 |
push id | 33980 |
push user | ebalazs@mozilla.com |
push date | Fri, 11 May 2018 09:35:12 +0000 |
treeherder | mozilla-central@8e9a4a323f0c [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | Gijs, mgoodwin |
bugs | 1458920 |
milestone | 62.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/services/common/docs/RemoteSettings.rst +++ b/services/common/docs/RemoteSettings.rst @@ -63,16 +63,19 @@ The ``sync`` event allows to be notified RemoteSettings("a-key").on("sync", event => { const { data: { current } } = event; for(const entry of current) { // Do something with entry... // await InternalAPI.reload(entry.id, entry.label, entry.weight); } }); +.. important:: + If one of the event handler fails, the others handlers for the same remote settings collection won't be executed. + .. note:: Currently, the update of remote settings is triggered by the `nsBlocklistService <https://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/nsBlocklistService.js>`_ (~ every 24H). File attachments ---------------- When an entry has a file attached to it, it has an ``attachment`` attribute, which contains the file related information (url, hash, size, mimetype, etc.). Remote files are not downloaded automatically.
--- a/services/common/remote-settings.js +++ b/services/common/remote-settings.js @@ -342,32 +342,44 @@ class RemoteSettingsClient { } else if (/Backoff/.test(e.message)) { reportStatus = UptakeTelemetry.STATUS.BACKOFF; } else { reportStatus = UptakeTelemetry.STATUS.SYNC_ERROR; } throw e; } } - // Read local collection of records. - const { data: current } = await collection.list(); - // Handle the obtained records (ie. apply locally). - try { - // Execute callbacks in order and sequentially. + // Handle the obtained records (ie. apply locally through events). + // Build the event data list. It should be filtered (ie. by application target) + const { created: allCreated, updated: allUpdated, deleted: allDeleted } = syncResult; + const [created, deleted, updatedFiltered] = await Promise.all( + [allCreated, allDeleted, allUpdated.map(e => e.new)].map(this._filterEntries.bind(this)) + ); + // For updates, keep entries whose updated form is matches the target. + const updatedFilteredIds = new Set(updatedFiltered.map(e => e.id)); + const updated = allUpdated.filter(({ new: { id } }) => updatedFilteredIds.has(id)); + + // If every changed entry is filtered, we don't even fire the event. + if (created.length || updated.length || deleted.length) { + // Read local collection of records (also filtered). + const { data: allData } = await collection.list(); + const current = await this._filterEntries(allData); + // Fire the event: execute callbacks in order and sequentially. // If one fails everything fails. - const { created, updated, deleted } = syncResult; const event = { data: { current, created, updated, deleted } }; const callbacks = this._callbacks.get("sync"); - for (const cb of callbacks) { - await cb(event); + try { + for (const cb of callbacks) { + await cb(event); + } + } catch (e) { + reportStatus = UptakeTelemetry.STATUS.APPLY_ERROR; + throw e; } - } catch (e) { - reportStatus = UptakeTelemetry.STATUS.APPLY_ERROR; - throw e; } // Track last update. this._updateLastCheck(serverTime); } catch (e) { // No specific error was tracked, mark it as unknown. if (reportStatus === null) {
--- a/services/common/tests/unit/test_blocklist_clients.js +++ b/services/common/tests/unit/test_blocklist_clients.js @@ -170,16 +170,55 @@ add_task(async function test_sends_reloa client.maybeSync(2000, Date.now() - 1000, {loadDump: false}); }); equal(received.data.filename, client.filename); } }); add_task(clear_state); +add_task(async function test_sync_event_data_is_filtered_for_target() { + // Here we will synchronize 4 times, the first two to initialize the local DB and + // the last two about event filtered data. + const timestamp1 = 2000; + const timestamp2 = 3000; + const timestamp3 = 4000; + const timestamp4 = 5000; + // Fake a date value obtained from server (used to store a pref, useless here). + const fakeServerTime = Date.now(); + + for (let {client} of gBlocklistClients) { + // Initialize the collection with some data (local is empty, thus no ?_since) + await client.maybeSync(timestamp1, fakeServerTime - 30, {loadDump: false}); + // This will pick the data with ?_since=3000. + await client.maybeSync(timestamp2 + 1, fakeServerTime - 20); + + // In ?_since=4000 entries, no target matches. The sync event is not called. + let called = false; + client.on("sync", e => called = true); + await client.maybeSync(timestamp3 + 1, fakeServerTime - 10); + equal(called, false, `no sync event for ${client.collectionName}`); + + // In ?_since=5000 entries, only one entry matches. + let syncEventData; + client.on("sync", e => syncEventData = e.data); + await client.maybeSync(timestamp4 + 1, fakeServerTime); + const { current, created, updated, deleted } = syncEventData; + equal(created.length + updated.length + deleted.length, 1, `event filtered data for ${client.collectionName}`); + + // Since we had entries whose target does not match, the internal storage list + // and the event current data should differ. + const collection = await client.openCollection(); + const { data: internalData } = await collection.list(); + ok(internalData.length > current.length, `event current data for ${client.collectionName}`); + } +}); +add_task(clear_state); + + // get a response for a given request from sample data function getSampleResponse(req, port) { const responses = { "OPTIONS": { "sampleHeaders": [ "Access-Control-Allow-Headers: Content-Length,Expires,Backoff,Retry-After,Last-Modified,Total-Records,ETag,Pragma,Cache-Control,authorization,content-type,if-none-match,Alert,Next-Page", "Access-Control-Allow-Methods: GET,HEAD,OPTIONS,POST,DELETE,OPTIONS", "Access-Control-Allow-Origin: *", @@ -317,31 +356,30 @@ function getSampleResponse(req, port) { "responseBody": JSON.stringify({"data": [{ "infoURL": "https://get.adobe.com/flashplayer/", "blockID": "p1044", "matchFilename": "libflashplayer\\.so", "last_modified": 4000, "versionRange": [{ "targetApplication": [], "minVersion": "11.2.202.509", - "maxVersion": "11.2.202.539", + "maxVersion": "*", "severity": "0", "vulnerabilityStatus": "1" }], "os": "Linux", "id": "aabad965-e556-ffe7-4191-074f5dee3df3" }, { "matchFilename": "npViewpoint.dll", "blockID": "p32", "id": "1f48af42-c508-b8ef-b8d5-609d48e4f6c9", "last_modified": 3500, "versionRange": [{ "targetApplication": [{ "minVersion": "3.0", - "guid": "{ec8030f7-c20a-464f-9b0e-13a3a9e97384}", "maxVersion": "*" }] }] }]}) }, "GET:/v1/buckets/blocklists/collections/gfx/records?_sort=-last_modified&_since=3000": { "sampleHeaders": [ "Access-Control-Allow-Origin: *", @@ -365,15 +403,172 @@ function getSampleResponse(req, port) { "blockID": "g200", "feature": "WEBGL_MSAA", "devices": [], "id": "c3a15ba9-e0e2-421f-e399-c995e5b8d14e", "last_modified": 3500, "os": "Darwin 11", "featureStatus": "BLOCKED_DEVICE" }]}) + }, + "GET:/v1/buckets/blocklists/collections/addons/records?_sort=-last_modified&_since=4000": { + "sampleHeaders": [ + "Access-Control-Allow-Origin: *", + "Access-Control-Expose-Headers: Retry-After, Content-Length, Alert, Backoff", + "Content-Type: application/json; charset=UTF-8", + "Server: waitress", + "Etag: \"5000\"" + ], + "status": {status: 200, statusText: "OK"}, + "responseBody": JSON.stringify({"data": [{ + "last_modified": 4001, + "versionRange": [{ + "targetApplication": [{ + "guid": "some-guid" + }], + }], + "id": "8f03b264-57b7-4263-9b15-ad91b033a034" + }]}) + }, + "GET:/v1/buckets/blocklists/collections/plugins/records?_sort=-last_modified&_since=4000": { + "sampleHeaders": [ + "Access-Control-Allow-Origin: *", + "Access-Control-Expose-Headers: Retry-After, Content-Length, Alert, Backoff", + "Content-Type: application/json; charset=UTF-8", + "Server: waitress", + "Etag: \"5000\"" + ], + "status": {status: 200, statusText: "OK"}, + "responseBody": JSON.stringify({"data": [{ + "last_modified": 4001, + "versionRange": [{ + "targetApplication": [{ + "guid": "xpcshell@tests.mozilla.org", + "minVersion": "0", + "maxVersion": "57.*" + }] + }], + "id": "cd3ea0b2-1ba8-4fb6-b242-976a87626116" + }]}) + }, + "GET:/v1/buckets/blocklists/collections/gfx/records?_sort=-last_modified&_since=4000": { + "sampleHeaders": [ + "Access-Control-Allow-Origin: *", + "Access-Control-Expose-Headers: Retry-After, Content-Length, Alert, Backoff", + "Content-Type: application/json; charset=UTF-8", + "Server: waitress", + "Etag: \"5000\"" + ], + "status": {status: 200, statusText: "OK"}, + "responseBody": JSON.stringify({"data": [{ + "last_modified": 4001, + "versionRange": [{ + "targetApplication": [{ + "guid": "xpcshell@tests.mozilla.org", + "minVersion": "99999" + }], + }], + "id": "86771771-e803-4006-95e9-c9275d58b3d1" + }]}) + }, + "GET:/v1/buckets/blocklists/collections/addons/records?_sort=-last_modified&_since=5000": { + "sampleHeaders": [ + "Access-Control-Allow-Origin: *", + "Access-Control-Expose-Headers: Retry-After, Content-Length, Alert, Backoff", + "Content-Type: application/json; charset=UTF-8", + "Server: waitress", + "Etag: \"6000\"" + ], + "status": {status: 200, statusText: "OK"}, + "responseBody": JSON.stringify({"data": [{ + // delete an entry with non matching target (see above) + "last_modified": 5001, + "deleted": true, + "id": "8f03b264-57b7-4263-9b15-ad91b033a034" + }, { + // delete entry with matching target (see above) + "last_modified": 5002, + "deleted": true, + "id": "9ccfac91-e463-c30c-f0bd-14143794a8dd" + }, { + // create an extra non matching + "last_modified": 5003, + "id": "75b36589-435a-48d4-8ee4-bacee3fb6119", + "versionRange": [{ + "targetApplication": [{ + "guid": "xpcshell@tests.mozilla.org", + "minVersion": "0", + "maxVersion": "57.*" + }] + }], + }]}) + }, + "GET:/v1/buckets/blocklists/collections/plugins/records?_sort=-last_modified&_since=5000": { + "sampleHeaders": [ + "Access-Control-Allow-Origin: *", + "Access-Control-Expose-Headers: Retry-After, Content-Length, Alert, Backoff", + "Content-Type: application/json; charset=UTF-8", + "Server: waitress", + "Etag: \"6000\"" + ], + "status": {status: 200, statusText: "OK"}, + "responseBody": JSON.stringify({"data": [{ + // entry with non matching target (see above) + "newAttribute": 42, + "versionRange": [{ + "targetApplication": [{ + "guid": "xpcshell@tests.mozilla.org", + "minVersion": "0", + "maxVersion": "57.*" + }] + }], + "id": "cd3ea0b2-1ba8-4fb6-b242-976a87626116" + }, { + // entry with matching target (see above) + "newAttribute": 42, + "matchFilename": "npViewpoint.dll", + "blockID": "p32", + "id": "1f48af42-c508-b8ef-b8d5-609d48e4f6c9", + "last_modified": 3500, + "versionRange": [{ + "targetApplication": [{ + "guid": "xpcshell@tests.mozilla.org", + "minVersion": "3.0", + "maxVersion": "*" + }] + }] + }]}) + }, + "GET:/v1/buckets/blocklists/collections/gfx/records?_sort=-last_modified&_since=5000": { + "sampleHeaders": [ + "Access-Control-Allow-Origin: *", + "Access-Control-Expose-Headers: Retry-After, Content-Length, Alert, Backoff", + "Content-Type: application/json; charset=UTF-8", + "Server: waitress", + "Etag: \"6000\"" + ], + "status": {status: 200, statusText: "OK"}, + "responseBody": JSON.stringify({"data": [{ + "versionRange": [{ + "targetApplication": [{ + "guid": "xpcshell@tests.mozilla.org", + "minVersion": "0", + "maxVersion": "*" + }] + }], + "id": "43031a81-5f36-4eef-9b35-52f0bbeba363" + }, { + "versionRange": [{ + "targetApplication": [{ + "guid": "xpcshell@tests.mozilla.org", + "minVersion": "0", + "maxVersion": "3" + }] + }], + "id": "75a06bd3-f906-427d-a448-02092ee589fc" + }]}) } }; return responses[`${req.method}:${req.path}?${req.queryString}`] || responses[`${req.method}:${req.path}`] || responses[req.method]; }
--- a/services/common/tests/unit/test_blocklist_signatures.js +++ b/services/common/tests/unit/test_blocklist_signatures.js @@ -444,29 +444,26 @@ add_task(async function test_check_signa "GET:/v1/buckets/blocklists/collections/certificates/records?_sort=id": [RESPONSE_COMPLETE_INITIAL_SORTED_BY_ID] }; registerHandlers(badSigGoodSigResponses); startHistogram = getUptakeTelemetrySnapshot(TELEMETRY_HISTOGRAM_KEY); - let retrySyncData; - OneCRLBlocklistClient.on("sync", ({ data }) => { retrySyncData = data; }); + let syncEventSent = false; + OneCRLBlocklistClient.on("sync", ({ data }) => { syncEventSent = true; }); await OneCRLBlocklistClient.maybeSync(5000, startTime); endHistogram = getUptakeTelemetrySnapshot(TELEMETRY_HISTOGRAM_KEY); - // since we only fixed the signature, and no data was changed, the sync result - // will be called with empty lists of created/updated/deleted. - equal(retrySyncData.current.length, 2); - equal(retrySyncData.created.length, 0); - equal(retrySyncData.updated.length, 0); - equal(retrySyncData.deleted.length, 0); + // since we only fixed the signature, and no data was changed, the sync event + // was not sent. + equal(syncEventSent, false); // ensure that the failure count is incremented for a succesful sync with an // (initial) bad signature - only SERVICES_SETTINGS_SYNC_SIG_FAIL should // increment. expectedIncrements = {[UptakeTelemetry.STATUS.SIGNATURE_ERROR]: 1}; checkUptakeTelemetry(startHistogram, endHistogram, expectedIncrements); @@ -488,27 +485,24 @@ add_task(async function test_check_signa [RESPONSE_EMPTY_INITIAL], }; // ensure our collection hasn't been replaced with an older, empty one await checkRecordCount(OneCRLBlocklistClient, 2); registerHandlers(badSigGoodOldResponses); - let oldChangesData; - OneCRLBlocklistClient.on("sync", ({ data }) => { oldChangesData = data; }); + syncEventSent = false; + OneCRLBlocklistClient.on("sync", ({ data }) => { syncEventSent = true; }); await OneCRLBlocklistClient.maybeSync(5000, startTime); - // Local data was unchanged, since it was never than the one returned by the server. - equal(oldChangesData.current.length, 2); - equal(oldChangesData.created.length, 0); - equal(oldChangesData.updated.length, 0); - equal(oldChangesData.deleted.length, 0); - + // Local data was unchanged, since it was never than the one returned by the server, + // thus the sync event is not sent. + equal(syncEventSent, false); const badLocalContentGoodSigResponses = { // In this test, we deliberately serve a bad signature initially. The // subsequent signature returned is a valid one for the three item // collection. "GET:/v1/buckets/blocklists/collections/certificates?": [RESPONSE_META_BAD_SIG, RESPONSE_META_THREE_ITEMS_SIG], // The next request is for the full collection. This will be checked