Bug 1236377: Ignore invalid file descriptors when loading an add-ons database written by a different OS. r?rhelmer draft
authorDave Townsend <dtownsend@oxymoronical.com>
Tue, 02 Feb 2016 13:17:10 -0800
changeset 329198 faa21ebc1d0b6b91bc81f79072e6d5fb9697bd18
parent 328315 673d0e96112b42c82db31a0c307be5cc794e4c05
child 513928 2a2f3d796516fd455f6bb027f94c0b0b6b2c1d0c
push id10491
push userdtownsend@mozilla.com
push dateFri, 05 Feb 2016 19:12:26 +0000
reviewersrhelmer
bugs1236377
milestone47.0a1
Bug 1236377: Ignore invalid file descriptors when loading an add-ons database written by a different OS. r?rhelmer nsIFile descriptors use OS specific formats so when trying to read them we have to catch any failure if the database was written by a different OS. This leaves the _sourceBundle undefined in only one case which is guaranteed to be during startup since xpistate.descriptor for the add-on will also be incorrect and so the full add-on scan will be triggered. That will spot the mismatch and update the add-on in the database with the correct descriptor.
toolkit/mozapps/extensions/internal/XPIProviderUtils.js
toolkit/mozapps/extensions/test/xpcshell/test_switch_os.js
toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini
toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini
--- a/toolkit/mozapps/extensions/internal/XPIProviderUtils.js
+++ b/toolkit/mozapps/extensions/internal/XPIProviderUtils.js
@@ -328,27 +328,22 @@ function DBAddonInternal(aLoaded) {
     this.location = aLoaded._installLocation.name;
   }
   else if (aLoaded.location) {
     this._installLocation = XPIProvider.installLocationsByName[this.location];
   }
 
   this._key = this.location + ":" + this.id;
 
-  if (aLoaded._sourceBundle) {
-    this._sourceBundle = aLoaded._sourceBundle;
-  }
-  else if (aLoaded.descriptor) {
-    this._sourceBundle = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
-    this._sourceBundle.persistentDescriptor = aLoaded.descriptor;
-  }
-  else {
+  if (!aLoaded._sourceBundle) {
     throw new Error("Expected passed argument to contain a descriptor");
   }
 
+  this._sourceBundle = aLoaded._sourceBundle;
+
   XPCOMUtils.defineLazyGetter(this, "pendingUpgrade", function() {
       for (let install of XPIProvider.installs) {
         if (install.state == AddonManager.STATE_INSTALLED &&
             !(install.addon.inDatabase) &&
             install.addon.id == this.id &&
             install.installLocation == this._installLocation) {
           delete this.pendingUpgrade;
           return this.pendingUpgrade = install.addon;
@@ -654,16 +649,26 @@ this.XPIDatabase = {
         // When we rev the schema of the JSON database, we need to make sure we
         // force the DB to save so that the DB_SCHEMA value in the JSON file and
         // the preference are updated.
       }
       // If we got here, we probably have good data
       // Make AddonInternal instances from the loaded data and save them
       let addonDB = new Map();
       for (let loadedAddon of inputAddons.addons) {
+        loadedAddon._sourceBundle = Cc["@mozilla.org/file/local;1"].createInstance(Ci.nsIFile);
+        try {
+          loadedAddon._sourceBundle.persistentDescriptor = loadedAddon.descriptor;
+        }
+        catch (e) {
+          // We can fail here when the descriptor is invalid, usually from the
+          // wrong OS
+          logger.warn("Could not find source bundle for add-on " + loadedAddon.id, e);
+        }
+
         let newAddon = new DBAddonInternal(loadedAddon);
         addonDB.set(newAddon._key, newAddon);
       };
       parseTimer.done();
       this.addonDB = addonDB;
       logger.debug("Successfully read XPI database");
       this.initialized = true;
     }
@@ -1878,16 +1883,29 @@ this.XPIDatabaseReconcile = {
     let loadedManifest = (aInstallLocation, aId) => {
       if (!(aInstallLocation.name in aManifests))
         return null;
       if (!(aId in aManifests[aInstallLocation.name]))
         return null;
       return aManifests[aInstallLocation.name][aId];
     };
 
+    // Add-ons loaded from the database can have an uninitialized _sourceBundle
+    // if the descriptor was invalid. Swallow that error and say they don't exist.
+    let exists = (aAddon) => {
+      try {
+        return aAddon._sourceBundle.exists();
+      }
+      catch (e) {
+        if (e.result == Cr.NS_ERROR_NOT_INITIALIZED)
+          return false;
+        throw e;
+      }
+    };
+
     // Get the previous add-ons from the database and put them into maps by location
     let previousAddons = new Map();
     for (let a of XPIDatabase.getAddons()) {
       let locationAddonMap = previousAddons.get(a.location);
       if (!locationAddonMap) {
         locationAddonMap = new Map();
         previousAddons.set(a.location, locationAddonMap);
       }
@@ -2075,17 +2093,17 @@ this.XPIDatabaseReconcile = {
 
           let installReason = Services.vc.compare(previousAddon.version, currentAddon.version) < 0 ?
                               BOOTSTRAP_REASONS.ADDON_UPGRADE :
                               BOOTSTRAP_REASONS.ADDON_DOWNGRADE;
 
           // If the previous add-on was in a different path, bootstrapped
           // and still exists then call its uninstall method.
           if (previousAddon.bootstrap && previousAddon._installLocation &&
-              previousAddon._sourceBundle.exists() &&
+              exists(previousAddon) &&
               currentAddon._sourceBundle.path != previousAddon._sourceBundle.path) {
 
             XPIProvider.callBootstrapMethod(previousAddon, previousAddon._sourceBundle,
                                             "uninstall", installReason,
                                             { newVersion: currentAddon.version });
             XPIProvider.unloadBootstrapScope(previousAddon.id);
           }
 
@@ -2134,17 +2152,17 @@ this.XPIDatabaseReconcile = {
     for (let [id, previousAddon] of previousVisible) {
       if (currentVisible.has(id))
         continue;
 
       // This add-on vanished
 
       // If the previous add-on was bootstrapped and still exists then call its
       // uninstall method.
-      if (previousAddon.bootstrap && previousAddon._sourceBundle.exists()) {
+      if (previousAddon.bootstrap && exists(previousAddon)) {
         XPIProvider.callBootstrapMethod(previousAddon, previousAddon._sourceBundle,
                                         "uninstall", BOOTSTRAP_REASONS.ADDON_UNINSTALL);
         XPIProvider.unloadBootstrapScope(previousAddon.id);
       }
       AddonManagerPrivate.addStartupChange(AddonManager.STARTUP_CHANGE_UNINSTALLED, id);
 
       // Make sure to flush the cache when an old add-on has gone away
       flushStartupCache();
new file mode 100644
--- /dev/null
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_switch_os.js
@@ -0,0 +1,52 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+Components.utils.import("resource://gre/modules/AppConstants.jsm");
+
+const ID = "bootstrap1@tests.mozilla.org";
+
+BootstrapMonitor.init();
+
+const profileDir = gProfD.clone();
+profileDir.append("extensions");
+
+createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
+
+add_task(function*() {
+  startupManager();
+
+  let install = yield new Promise(resolve => AddonManager.getInstallForFile(do_get_addon("test_bootstrap1_1"), resolve));
+  yield promiseCompleteAllInstalls([install]);
+
+  let addon = yield promiseAddonByID(ID);
+  do_check_neq(addon, null);
+
+  BootstrapMonitor.checkAddonStarted(ID);
+  do_check_false(addon.userDisabled);
+  do_check_true(addon.isActive);
+
+  yield promiseShutdownManager();
+
+  BootstrapMonitor.checkAddonNotStarted(ID);
+
+  let jData = loadJSON(gExtensionsJSON);
+
+  for (let addon of jData.addons) {
+    if (addon.id == ID) {
+      // Set to something that would be an invalid descriptor for this platform
+      addon.descriptor = AppConstants.platform == "win" ? "/foo/bar" : "C:\\foo\\bar";
+    }
+  }
+
+  saveJSON(jData, gExtensionsJSON);
+
+  startupManager();
+
+  addon = yield promiseAddonByID(ID);
+  do_check_neq(addon, null);
+
+  BootstrapMonitor.checkAddonStarted(ID);
+  do_check_false(addon.userDisabled);
+  do_check_true(addon.isActive);
+});
--- a/toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini
+++ b/toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini
@@ -300,8 +300,11 @@ run-sequentially = Uses global XCurProcD
 [test_webextension_icons.js]
 [test_webextension.js]
 [test_bootstrap_globals.js]
 [test_bug1180901_2.js]
 skip-if = os != "win"
 [test_bug1180901.js]
 skip-if = os != "win"
 [test_e10s_restartless.js]
+[test_switch_os.js]
+# Bug 1246231
+skip-if = os == "mac" && debug
--- a/toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini
+++ b/toolkit/mozapps/extensions/test/xpcshell/xpcshell.ini
@@ -4,17 +4,16 @@ tags = addons
 head = head_addons.js
 tail =
 firefox-appdir = browser
 dupe-manifest =
 support-files =
   data/**
   xpcshell-shared.ini
 
-
 [test_addon_path_service.js]
 [test_asyncBlocklistLoad.js]
 [test_cacheflush.js]
 [test_DeferredSave.js]
 [test_gmpProvider.js]
 skip-if = appname != "firefox"
 [test_hotfix_cert.js]
 [test_isReady.js]