Bug 1257565 - Perform HEAD request on XML blocklist when using JSON r=mossop draft
authorMathieu Leplatre <mathieu@mozilla.com>
Thu, 15 Jun 2017 16:19:18 +0200
changeset 655877 1f143c049ecc6dc49e83350b10a8b877e5822675
parent 655876 ec694449eb74d5b785647ef23af52df7ac4ceb86
child 655878 b8da5aea5ef120f37cfc3824f395591c51e8faed
push id76975
push usermleplatre@mozilla.com
push dateWed, 30 Aug 2017 12:43:51 +0000
reviewersmossop
bugs1257565
milestone57.0a1
Bug 1257565 - Perform HEAD request on XML blocklist when using JSON r=mossop When switching to JSON, we still want to track Active Daily Users metrics. We use a HEAD request for that. MozReview-Commit-ID: B1J3oSciu0w
toolkit/mozapps/extensions/nsBlocklistService.js
toolkit/mozapps/extensions/test/xpcshell/test_blocklist_json.js
--- a/toolkit/mozapps/extensions/nsBlocklistService.js
+++ b/toolkit/mozapps/extensions/nsBlocklistService.js
@@ -198,16 +198,76 @@ function getPref(func, preference, defau
  * @returns The nsIURI constructed.
  */
 function newURI(spec) {
   var ioServ = Cc["@mozilla.org/network/io-service;1"].
                getService(Ci.nsIIOService);
   return ioServ.newURI(spec);
 }
 
+/**
+ * Moved legacy code into promised-based helper to download the blocklist XML.
+ * @param   uri
+ *          The URI to fetch
+ * @param   options.method Request HTTP method (default: "GET")
+ * @returns A Promise resolved on load or rejected on error.
+ */
+function fetchXML(uri, options = {}) {
+  return new Promise((resolve, reject) => {
+    const { method = "GET" } = options;
+    LOG(`Blocklist::fetchXML: Requesting ${method} ${uri}`);
+    const request = new ServiceRequest();
+    request.open(method, uri, true);
+    request.channel.notificationCallbacks = new gCertUtils.BadCertHandler();
+    request.overrideMimeType("text/xml");
+    request.setRequestHeader("Cache-Control", "no-cache");
+    request.QueryInterface(Components.interfaces.nsIJSXMLHttpRequest);
+    request.addEventListener("load", () => {
+      try {
+        gCertUtils.checkCert(request.channel);
+      } catch (e) {
+        reject(e);
+      }
+      // Reject if not success status.
+      if (request.status != 200 && request.status != 0) {
+        reject(new Error(`Error response: ${request.status}`));
+      }
+      if (method != "HEAD") {
+        const responseXML = request.responseXML;
+        if (!responseXML || responseXML.documentElement.namespaceURI == XMLURI_PARSE_ERROR) {
+          reject(new Error("Invalid XML"));
+        }
+      }
+      resolve(request);
+    });
+    request.addEventListener("error", () => {
+      let requestResponse, status;
+      try {
+        requestResponse = request;
+        // the following may throw (e.g. a local file or timeout)
+        status = request.status;
+      } catch (e) {
+        requestResponse = request.channel.QueryInterface(Ci.nsIRequest);
+        status = request.status;
+      }
+      var statusText = "nsIXMLHttpRequest channel unavailable";
+      // When status is 0 we don't have a valid channel.
+      if (status != 0) {
+        try {
+          statusText = requestResponse.statusText;
+        } catch (e) {
+        }
+      }
+      reject(new Error(statusText));
+    });
+
+    request.send(null);
+  });
+}
+
 // Restarts the application checking in with observers first
 function restartApp() {
   // Notify all windows that an application quit has been requested.
   var os = Cc["@mozilla.org/observer-service;1"].
            getService(Ci.nsIObserverService);
   var cancelQuit = Cc["@mozilla.org/supports-PRBool;1"].
                    createInstance(Ci.nsISupportsPRBool);
   os.notifyObservers(cancelQuit, "quit-application-requested");
@@ -551,22 +611,16 @@ Blocklist.prototype = {
 
     return url;
   },
 
   notify(aTimer) {
     if (!gBlocklistEnabled)
       return;
 
-    // If blocklist does not rely on XML, do not download the file.
-    // Updates are performed in services/common/kinto-updater.js
-    if (!gBlocklistFromXML) {
-      return;
-    }
-
     try {
       var dsURI = gPref.getCharPref(PREF_BLOCKLIST_URL);
     } catch (e) {
       LOG("Blocklist::notify: The " + PREF_BLOCKLIST_URL + " preference" +
           " is missing!");
       return;
     }
 
@@ -647,93 +701,67 @@ Blocklist.prototype = {
     try {
       var uri = newURI(dsURI);
     } catch (e) {
       LOG("Blocklist::notify: There was an error creating the blocklist URI\r\n" +
           "for: " + dsURI + ", error: " + e);
       return;
     }
 
-    LOG("Blocklist::notify: Requesting " + uri.spec);
-    let request = new ServiceRequest();
-    request.open("GET", uri.spec, true);
-    request.channel.notificationCallbacks = new gCertUtils.BadCertHandler();
-    request.overrideMimeType("text/xml");
-    request.setRequestHeader("Cache-Control", "no-cache");
-
-    request.addEventListener("error", event => this.onXMLError(event));
-    request.addEventListener("load", event => this.onXMLLoad(event));
-    request.send(null);
+    // If blocklist does not rely on XML, do not download the file:
+    // updates are performed in services/common/kinto-updater.js.
+    // We still do a HEAD request, since ADU metrics rely on this blocklist.
+    if (gBlocklistFromXML) {
+      fetchXML(uri.spec)
+        .then(request => this.onXMLLoad(request))
+        .catch(error => {
+          LOG("Blocklist::notify: There was an error fetching the blocklist\r\n" +
+              error.message);
+        });
+    } else {
+      fetchXML(uri.spec, {method: "HEAD"})
+        .catch(error => {
+          LOG("Blocklist::notify: There was an error sending HEAD request\r\n" +
+              error.message);
+        });
+    }
 
     // When the blocklist loads we need to compare it to the current copy so
     // make sure we have loaded it.
     if (!this._isBlocklistLoaded())
       this._loadBlocklist();
 
     // If blocklist update via Kinto is enabled, poll for changes and sync.
     // Currently certificates blocklist relies on it by default.
     if (gPref.getBoolPref(PREF_BLOCKLIST_UPDATE_ENABLED)) {
       BlocklistUpdater.checkVersions().catch(() => {
         // Bug 1254099 - Telemetry (success or errors) will be collected during this process.
       });
     }
   },
 
-  async onXMLLoad(aEvent) {
-    let request = aEvent.target;
-    try {
-      gCertUtils.checkCert(request.channel);
-    } catch (e) {
-      LOG("Blocklist::onXMLLoad: " + e);
-      return;
-    }
-    let responseXML = request.responseXML;
-    if (!responseXML || responseXML.documentElement.namespaceURI == XMLURI_PARSE_ERROR ||
-        (request.status != 200 && request.status != 0)) {
-      LOG("Blocklist::onXMLLoad: there was an error during load");
-      return;
-    }
+  async onXMLLoad(request) {
+    const rawXML = request.responseText;
 
-    var oldAddonEntries = this._addonEntries;
-    var oldPluginEntries = this._pluginEntries;
+    const oldAddonEntries = this._addonEntries;
+    const oldPluginEntries = this._pluginEntries;
 
-    this._loadBlocklistFromXMLString(request.responseText);
+    this._loadBlocklistFromXMLString(rawXML);
     // We don't inform the users when the graphics blocklist changed at runtime.
     // However addons and plugins blocking status is refreshed.
     this._blocklistUpdated(oldAddonEntries, oldPluginEntries);
 
     try {
       let path = OS.Path.join(OS.Constants.Path.profileDir, FILE_BLOCKLIST);
-      await OS.File.writeAtomic(path, request.responseText, {tmpPath: path + ".tmp"});
+      await OS.File.writeAtomic(path, rawXML, {tmpPath: path + ".tmp"});
     } catch (e) {
       LOG("Blocklist::onXMLLoad: " + e);
     }
   },
 
-  onXMLError(aEvent) {
-    try {
-      var request = aEvent.target;
-      // the following may throw (e.g. a local file or timeout)
-      var status = request.status;
-    } catch (e) {
-      request = aEvent.target.channel.QueryInterface(Ci.nsIRequest);
-      status = request.status;
-    }
-    var statusText = "nsIXMLHttpRequest channel unavailable";
-    // When status is 0 we don't have a valid channel.
-    if (status != 0) {
-      try {
-        statusText = request.statusText;
-      } catch (e) {
-      }
-    }
-    LOG("Blocklist:onError: There was an error loading the blocklist file\r\n" +
-        statusText);
-  },
-
   /**
    * Finds the newest blocklist file from the application and the profile and
    * load it or does nothing if neither exist.
    */
   _loadBlocklist() {
     if (!gBlocklistEnabled) {
       LOG("Blocklist::_loadBlocklistFromFile: blocklist is disabled");
       return;
--- a/toolkit/mozapps/extensions/test/xpcshell/test_blocklist_json.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_blocklist_json.js
@@ -9,17 +9,16 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 XPCOMUtils.defineLazyModuleGetter(this, "OS",
                                   "resource://gre/modules/osfile.jsm");
 
 const PREF_BLOCKLIST_VIA_AMO = "security.blocklist.via.amo";
 const KEY_APPDIR             = "XCurProcD";
 const KEY_PROFILEDIR         = "ProfD";
 const TEST_APP_ID            = "xpcshell@tests.mozilla.org";
 
-
 const SAMPLE_FILE = do_get_file("data/test_blocklist_json/sample.json");
 
 const SAMPLE_ADDON_RECORD = {
   "prefs": [],
   "blockID": "i446",
   "last_modified": 1457434834683,
   "versionRange": [{
     "targetApplication": [{
@@ -62,16 +61,19 @@ const SAMPLE_GFX_RECORD = {
   "feature": "DIRECT3D_9_LAYERS",
   "devices": ["0x0a6c"],
   "featureStatus": "BLOCKED_DRIVER_VERSION",
   "last_modified": 1458035931837,
   "os": "WINNT 6.1",
   "id": "3f947f16-37c2-4e96-d356-78b26363729b"
 };
 
+// Instantiate a test server to for the HEAD request instead of the XML download.
+const testserver = createHttpServer();
+gPort = testserver.identity.primaryPort;
 
 function clearProfile(name) {
   let filename = name + ".json";
   let blocklist = FileUtils.getFile(KEY_PROFILEDIR, ["blocklists", filename]);
   if (blocklist.exists())
     blocklist.remove(true);
 }
 
@@ -94,16 +96,19 @@ function Blocklist() {
 }
 
 
 function run_test() {
   // Some blocklist code rely on gApp.ID.
   createAppInfo(TEST_APP_ID, "XPCShell", "1", "1");
   // Disable blocklist via AMO.
   Services.prefs.setBoolPref(PREF_BLOCKLIST_VIA_AMO, false);
+  // But still define a URL for the HEAD request.
+  Services.prefs.setCharPref("extensions.blocklist.url",
+                             "http://localhost:" + gPort + "/blocklist.xml");
 
   // Starts addons manager.
   startupManager();
 
   // Clean-up for profile data.
   do_register_cleanup(function() {
     for (let filename of ["addons.json", "plugins.json"]) {
       const file = FileUtils.getFile(KEY_PROFILEDIR, ["blocklists", filename]);
@@ -258,22 +263,32 @@ add_task(function* test_is_loaded_synchr
   const blocklist = Blocklist();
   strictEqual(blocklist._isBlocklistLoaded(), false);
   // Calls synchronous method from Interface.
   blocklist.isAddonBlocklisted("addon", "appVersion", "toolkitVersion");
   strictEqual(blocklist._isBlocklistLoaded(), true);
 });
 
 
-add_task(function* test_notify_does_not_download_xml_file() {
+add_task(function* test_notify_sends_a_HEAD_request_to_server() {
+  // When managed with Kinto (default), blocklist.xml is reached with a HEAD request.
+  const waitForHead = new Promise((resolve) => {
+    testserver.registerPathHandler("/blocklist.xml", (request) => {
+      if (request.method != "HEAD") {
+        do_throw(`unexpected ${request.method} request for ${request.path}`);
+      }
+      resolve();
+    });
+  });
+
   const blocklist = Blocklist();
   strictEqual(blocklist._isBlocklistLoaded(), false);
-  // When managed with Kinto, nothing is loaded/downloaded on notify.
   blocklist.notify(null);
-  strictEqual(blocklist._isBlocklistLoaded(), false);
+  // Wait for the request to be received.
+  yield waitForHead;
 });
 
 
 add_task(function* preload_json_async() {
   const blocklist = Blocklist();
 
   yield blocklist._preloadBlocklist();