Bug 1420775: Mark detected add-ons initially disabled. r?rhelmer draft
authorKris Maglione <maglione.k@gmail.com>
Wed, 17 Jan 2018 11:58:22 -0800
changeset 721693 90f491f3b80699008a315612fd822f55240ce951
parent 720736 06f766e7640a3cd180f7da7c092823e500a3d674
child 746425 78a288bf68684a575dcdb535558e91e945140b57
push id95936
push usermaglione.k@gmail.com
push dateWed, 17 Jan 2018 19:58:57 +0000
reviewersrhelmer
bugs1420775
milestone59.0a1
Bug 1420775: Mark detected add-ons initially disabled. r?rhelmer When we detect a new add-on during a directory scan, we initially add a skeleton XPIStates entry for it, and rely on the database reconciliation to fill in the details later in startup. In the case of extensions with invalid install.rdf files, though, this doesn't happen, and the entry remains as we initially created it. Since those entries are currently marked enabled by default, and the XPIStates entries determine what non-bootstrapped chrome manifests we load at startup, that leads to us mistakenly loading resources that we shouldn't. This change marks newly-detected entries disabled by default, and leaves it to later startup stages to enable them if necessary. We still leave the XPIStates entry in place, since removing it would cause us to rebuild the database on every subsequent startup, when we re-detect it. MozReview-Commit-ID: AvBmj79Ro2W
toolkit/mozapps/extensions/AddonManager.jsm
toolkit/mozapps/extensions/internal/XPIProvider.jsm
toolkit/mozapps/extensions/test/xpcshell/head_addons.js
toolkit/mozapps/extensions/test/xpcshell/test_invalid_install_rdf.js
toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini
--- a/toolkit/mozapps/extensions/AddonManager.jsm
+++ b/toolkit/mozapps/extensions/AddonManager.jsm
@@ -2218,17 +2218,17 @@ var AddonManagerInternal = {
                                 Cr.NS_ERROR_INVALID_ARG);
 
   if (!aCallback || typeof aCallback != "function")
     throw Components.Exception("aCallback must be a function",
                                Cr.NS_ERROR_INVALID_ARG);
 
    this.getAddonByInstanceID(aInstanceID).then(wrapper => {
      if (!wrapper) {
-       throw Error(`No addon matching instanceID: ${aInstanceID}`);
+       throw Error(`No addon matching instanceID: ${String(aInstanceID)}`);
      }
      let addonId = wrapper.id;
      logger.debug(`Registering upgrade listener for ${addonId}`);
      this.upgradeListeners.set(addonId, aCallback);
    });
   },
 
   /**
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -1420,17 +1420,17 @@ class XPIStateLocation extends Map {
    *
    * @param {string} addonId
    *        The ID of the add-on represented by the given file.
    * @param {nsIFile} file
    *        The local file or directory containing the add-on.
    * @returns {XPIState}
    */
   addFile(addonId, file) {
-    let xpiState = this._addState(addonId, {enabled: true, file: file.clone()});
+    let xpiState = this._addState(addonId, {enabled: false, file: file.clone()});
     xpiState.getModTime(xpiState.file, addonId);
     return xpiState;
   }
 
   /**
    * Migrates saved state data for the given add-on from the values
    * stored in xpiState and bootstrappedAddons preferences, and adds it to
    * the DB.
@@ -1570,17 +1570,17 @@ this.XPIStates = {
     let changed = false;
     let oldLocations = new Set(Object.keys(oldState));
 
     for (let location of XPIProvider.installLocations) {
       oldLocations.delete(location.name);
 
       // The results of scanning this location.
       let loc = this.getLocation(location.name, location.path || null,
-                                 oldState[location.name]);
+                                 oldState[location.name] || undefined);
       changed = changed || loc.changed;
 
       // Don't bother checking scopes where we don't accept side-loads.
       if (ignoreSideloads && !(location.scope & gStartupScanScopes)) {
         continue;
       }
 
       if (location.name == KEY_APP_TEMPORARY) {
--- a/toolkit/mozapps/extensions/test/xpcshell/head_addons.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/head_addons.js
@@ -673,26 +673,31 @@ function check_startup_changes(aType, aI
  *          The directory to add the install.rdf to
  * @param   aId
  *          An optional string to override the default installation aId
  * @param   aExtraFile
  *          An optional dummy file to create in the directory
  * @return  An nsIFile for the directory in which the add-on is installed.
  */
 function writeInstallRDFToDir(aData, aDir, aId = aData.id, aExtraFile = null) {
+  return awaitPromise(promiseWriteInstallRDFToDir(aData, aDir, aId, aExtraFile));
+}
+async function promiseWriteInstallRDFToDir(aData, aDir, aId = aData.id, aExtraFile = null) {
   let files = {
     "install.rdf": AddonTestUtils.createInstallRDF(aData),
   };
-  if (aExtraFile)
+  if (typeof aExtraFile === "object")
+    Object.assign(files, aExtraFile);
+  else
     files[aExtraFile] = "";
 
   let dir = aDir.clone();
   dir.append(aId);
 
-  awaitPromise(AddonTestUtils.promiseWriteFilesToDir(dir.path, files));
+  await AddonTestUtils.promiseWriteFilesToDir(dir.path, files);
   return dir;
 }
 
 /**
  * Writes an install.rdf manifest into a packed extension using the properties passed
  * in a JS object. The objects should contain a property for each property to
  * appear in the RDF. The object may contain an array of objects with id,
  * minVersion and maxVersion in the targetApplications property to give target
@@ -707,29 +712,35 @@ function writeInstallRDFToDir(aData, aDi
  * @param   aExtraFile
  *          An optional dummy file to create in the extension
  * @return  A file pointing to where the extension was installed
  */
 function writeInstallRDFToXPI(aData, aDir, aId = aData.id, aExtraFile = null) {
   let files = {
     "install.rdf": AddonTestUtils.createInstallRDF(aData),
   };
+  if (typeof aExtraFile === "object")
+    Object.assign(files, aExtraFile);
+  else
   if (aExtraFile)
     files[aExtraFile] = "";
 
   if (!aDir.exists())
     aDir.create(AM_Ci.nsIFile.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
 
   var file = aDir.clone();
   file.append(`${aId}.xpi`);
 
   AddonTestUtils.writeFilesToZip(file.path, files);
 
   return file;
 }
+async function promiseWriteInstallRDFToXPI(aData, aDir, aId = aData.id, aExtraFile = null) {
+  return writeInstallRDFToXPI(aData, aDir, aId, aExtraFile);
+}
 
 /**
  * Writes an install.rdf manifest into an extension using the properties passed
  * in a JS object. The objects should contain a property for each property to
  * appear in the RDF. The object may contain an array of objects with id,
  * minVersion and maxVersion in the targetApplications property to give target
  * application compatibility.
  *
new file mode 100644
--- /dev/null
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_invalid_install_rdf.js
@@ -0,0 +1,82 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+// Test that side-loaded extensions with invalid install.rdf files are
+// not initialized at startup.
+
+const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components;
+
+const APP_ID = "xpcshell@tests.mozilla.org";
+
+Services.prefs.setIntPref("extensions.enabledScopes", AddonManager.SCOPE_USER);
+
+createAppInfo(APP_ID, "XPCShell", "1", "1.9.2");
+
+const userAppDir = AddonTestUtils.profileDir.clone();
+userAppDir.append("app-extensions");
+userAppDir.create(Ci.nsIFile.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
+AddonTestUtils.registerDirectory("XREUSysExt", userAppDir);
+
+const userExtensions = userAppDir.clone();
+userExtensions.append(APP_ID);
+userExtensions.create(Ci.nsIFile.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
+
+add_task(async function() {
+  await promiseWriteInstallRDFToXPI({
+    id: "langpack-foo@addons.mozilla.org",
+    version: "1.0",
+    type: 8,
+    targetApplications: [{
+      id: "xpcshell@tests.mozilla.org",
+      minVersion: "1",
+      maxVersion: "1"
+    }],
+    name: "Invalid install.rdf extension",
+  }, userExtensions, undefined, {
+    "chrome.manifest": `
+      resource foo-langpack ./
+    `,
+  });
+
+  await promiseWriteInstallRDFToXPI({
+    id: "foo@addons.mozilla.org",
+    version: "1.0",
+    targetApplications: [{
+      id: "xpcshell@tests.mozilla.org",
+      minVersion: "1",
+      maxVersion: "1"
+    }],
+    name: "Invalid install.rdf extension",
+  }, userExtensions, undefined, {
+    "chrome.manifest": `
+      resource foo ./
+    `,
+  });
+
+  const resProto = Services.io.getProtocolHandler("resource");
+  resProto.QueryInterface(Ci.nsIResProtocolHandler);
+
+  equal(resProto.hasSubstitution("foo-langpack"), false,
+        "Should not foo-langpack resource before AOM startup");
+  equal(resProto.hasSubstitution("foo"), false,
+        "Should not foo resource before AOM startup");
+
+  await promiseStartupManager();
+
+  equal(resProto.hasSubstitution("foo-langpack"), false,
+        "Should not have registered chrome manifest for invalid extension");
+  equal(resProto.hasSubstitution("foo"), true,
+        "Should have registered chrome manifest for valid extension");
+
+  await promiseRestartManager();
+
+  equal(resProto.hasSubstitution("foo-langpack"), false,
+        "Should still not have registered chrome manifest for invalid extension after restart");
+  equal(resProto.hasSubstitution("foo"), true,
+        "Should still have registered chrome manifest for valid extension after restart");
+
+  await promiseShutdownManager();
+
+  userAppDir.remove(true);
+});
--- a/toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini
+++ b/toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini
@@ -15,16 +15,17 @@ tags = blocklist
 [test_blocklist_gfx.js]
 tags = blocklist
 [test_cache_certdb.js]
 run-if = addon_signing
 [test_cacheflush.js]
 [test_gmpProvider.js]
 skip-if = appname != "firefox"
 [test_hotfix_cert.js]
+[test_invalid_install_rdf.js]
 [test_isReady.js]
 [test_pluginInfoURL.js]
 tags = blocklist
 [test_provider_markSafe.js]
 [test_provider_shutdown.js]
 [test_provider_unsafe_access_shutdown.js]
 [test_provider_unsafe_access_startup.js]
 [test_ProductAddonChecker.js]