Bug 1281884 - strict_min_version and strict_max_version aren't respected in "Load Temporary Add-on", r?aswan draft
authorBob Silverberg <bsilverberg@mozilla.com>
Mon, 22 Aug 2016 13:28:17 -0400
changeset 404573 f85413ba5a9c9da223e76efc2c4c819005670dc3
parent 403970 194fe275b4e60ded2af6b25173eec421f0dba8ad
child 529218 5be787774942ccb30ca3634dfc4c85cce16070ce
push id27245
push userbmo:bob.silverberg@gmail.com
push dateTue, 23 Aug 2016 19:55:44 +0000
reviewersaswan
bugs1281884
milestone51.0a1
Bug 1281884 - strict_min_version and strict_max_version aren't respected in "Load Temporary Add-on", r?aswan Replace test_disabled_addon_can_be_enabled_after_reload in test_reload.js with test_reload_to_invalid_version_fails. MozReview-Commit-ID: 9OEBnbwNplC
toolkit/mozapps/extensions/internal/XPIProvider.jsm
toolkit/mozapps/extensions/test/xpcshell/head_addons.js
toolkit/mozapps/extensions/test/xpcshell/test_reload.js
toolkit/mozapps/extensions/test/xpcshell/test_webextension.js
toolkit/mozapps/extensions/test/xpcshell/test_webextension_icons.js
toolkit/mozapps/extensions/test/xpcshell/test_webextension_install.js
toolkit/mozapps/extensions/test/xpcshell/test_webextension_paths.js
toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -984,19 +984,18 @@ var loadManifestFromWebManifest = Task.a
 
   addon.defaultLocale = getLocale(extension.defaultLocale);
   addon.locales = Array.from(locales.keys(), getLocale);
 
   delete addon.defaultLocale.locales;
 
   addon.targetApplications = [{
     id: TOOLKIT_ID,
-    minVersion: (bss.strict_min_version ||
-                 AddonManagerPrivate.webExtensionsMinPlatformVersion),
-    maxVersion: bss.strict_max_version || "*",
+    minVersion: bss.strict_min_version,
+    maxVersion: bss.strict_max_version,
   }];
 
   addon.targetPlatforms = [];
   addon.userDisabled = false;
   addon.softDisabled = addon.blocklistState == Blocklist.STATE_SOFTBLOCKED;
 
   return addon;
 });
@@ -3989,16 +3988,23 @@ this.XPIProvider = {
    *         same ID is already temporarily installed
    */
   installTemporaryAddon: Task.async(function*(aFile) {
     if (aFile.exists() && aFile.isFile()) {
       flushJarCache(aFile);
     }
     let addon = yield loadManifestFromFile(aFile, TemporaryInstallLocation);
 
+    if (!addon.isCompatible) {
+      let app = addon.matchingTargetApplication;
+      throw new Error(`Add-on ${addon.id} is not compatible with application version. ` +
+                      `add-on minVersion: ${app.minVersion}, ` +
+                      `add-on maxVersion: ${app.maxVersion}`);
+    }
+
     if (!addon.bootstrap) {
       throw new Error("Only restartless (bootstrap) add-ons"
                     + " can be temporarily installed:", addon.id);
     }
     let installReason = BOOTSTRAP_REASONS.ADDON_INSTALL;
     let oldAddon = yield new Promise(
                    resolve => XPIDatabase.getVisibleAddonForID(addon.id, resolve));
     if (oldAddon) {
@@ -7009,16 +7015,20 @@ AddonInternal.prototype = {
     return matchedOS && !needsABI;
   },
 
   isCompatibleWith: function(aAppVersion, aPlatformVersion) {
     let app = this.matchingTargetApplication;
     if (!app)
       return false;
 
+    // set reasonable defaults for minVersion and maxVersion
+    let minVersion = app.minVersion || "0";
+    let maxVersion = app.maxVersion || "*";
+
     if (!aAppVersion)
       aAppVersion = Services.appinfo.version;
     if (!aPlatformVersion)
       aPlatformVersion = Services.appinfo.platformVersion;
 
     let version;
     if (app.id == Services.appinfo.ID)
       version = aAppVersion;
@@ -7045,24 +7055,24 @@ AddonInternal.prototype = {
       // Extremely old extensions should not be compatible by default.
       let minCompatVersion;
       if (app.id == Services.appinfo.ID)
         minCompatVersion = XPIProvider.minCompatibleAppVersion;
       else if (app.id == TOOLKIT_ID)
         minCompatVersion = XPIProvider.minCompatiblePlatformVersion;
 
       if (minCompatVersion &&
-          Services.vc.compare(minCompatVersion, app.maxVersion) > 0)
+          Services.vc.compare(minCompatVersion, maxVersion) > 0)
         return false;
 
-      return Services.vc.compare(version, app.minVersion) >= 0;
-    }
-
-    return (Services.vc.compare(version, app.minVersion) >= 0) &&
-           (Services.vc.compare(version, app.maxVersion) <= 0)
+      return Services.vc.compare(version, minVersion) >= 0;
+    }
+
+    return (Services.vc.compare(version, minVersion) >= 0) &&
+           (Services.vc.compare(version, maxVersion) <= 0)
   },
 
   get matchingTargetApplication() {
     let app = null;
     for (let targetApp of this.targetApplications) {
       if (targetApp.id == Services.appinfo.ID)
         return targetApp;
       if (targetApp.id == TOOLKIT_ID)
--- a/toolkit/mozapps/extensions/test/xpcshell/head_addons.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/head_addons.js
@@ -712,23 +712,21 @@ function writeInstallRDFForExtension(aDa
  * @param   aManifest
  *          The data to write
  * @param   aDir
  *          The install directory to add the extension to
  * @param   aId
  *          An optional string to override the default installation aId
  * @return  A file pointing to where the extension was installed
  */
-function writeWebManifestForExtension(aData, aDir, aId = aData.applications.gecko.id) {
+function promiseWriteWebManifestForExtension(aData, aDir, aId = aData.applications.gecko.id) {
   let files = {
     "manifest.json": JSON.stringify(aData),
   }
-  let promise = AddonTestUtils.promiseWriteFilesToExtension(aDir.path, aId, files);
-
-  return awaitPromise(promise);
+  return AddonTestUtils.promiseWriteFilesToExtension(aDir.path, aId, files);
 }
 
 var {writeFilesToZip} = AddonTestUtils;
 
 /**
  * Creates an XPI file for some manifest data in the temporary directory and
  * returns the nsIFile for it. The file will be deleted when the test completes.
  *
--- a/toolkit/mozapps/extensions/test/xpcshell/test_reload.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_reload.js
@@ -15,16 +15,29 @@ const manifestSample = {
   bootstrap: true,
   targetApplications: [{
     id: "xpcshell@tests.mozilla.org",
     minVersion: "1",
     maxVersion: "1"
   }],
 };
 
+const { Management } = Components.utils.import("resource://gre/modules/Extension.jsm", {});
+
+function promiseAddonStartup() {
+  return new Promise(resolve => {
+    let listener = (extension) => {
+      Management.off("startup", listener);
+      resolve(extension);
+    };
+
+    Management.on("startup", listener);
+  });
+}
+
 function* installAddon(fixtureName, addonID) {
   yield promiseInstallAllFiles([do_get_addon(fixtureName)]);
   return promiseAddonByID(addonID);
 }
 
 function* tearDownAddon(addon) {
   addon.uninstall();
   yield promiseShutdownManager();
@@ -105,48 +118,68 @@ add_task(function* test_can_reload_perma
 
   notEqual(addon, null);
   equal(addon.appDisabled, false);
   equal(addon.userDisabled, false);
 
   yield tearDownAddon(addon);
 });
 
-add_task(function* test_disabled_addon_can_be_enabled_after_reload() {
+add_task(function* test_reload_to_invalid_version_fails() {
   yield promiseRestartManager();
   let tempdir = gTmpD.clone();
 
-  // Create an add-on with strictCompatibility which should cause it
-  // to be appDisabled.
-  const unpackedAddon = writeInstallRDFToDir(
-    Object.assign({}, manifestSample, {
-      strictCompatibility: true,
-      targetApplications: [{
-        id: "xpcshell@tests.mozilla.org",
-        minVersion: "0.1",
-        maxVersion: "0.1"
-      }],
-    }), tempdir, manifestSample.id, "bootstrap.js");
+  // The initial version of the add-on will be compatible, and will therefore load
+  const addonId = "invalid_version_cannot_be_reloaded@tests.mozilla.org";
+  let manifest = {
+    name: "invalid_version_cannot_be_reloaded",
+    description: "test invalid_version_cannot_be_reloaded",
+    manifest_version: 2,
+    version: "1.0",
+    applications: {
+      gecko: {
+        id: addonId,
+      }
+    },
+  };
+
+  let addonDir = yield promiseWriteWebManifestForExtension(manifest, tempdir, "invalid_version");
+  yield AddonManager.installTemporaryAddon(addonDir);
+  yield promiseAddonStartup();
 
-  yield AddonManager.installTemporaryAddon(unpackedAddon);
-  const addon = yield promiseAddonByID(manifestSample.id);
+  let addon = yield promiseAddonByID(addonId);
   notEqual(addon, null);
-  equal(addon.appDisabled, true);
+  equal(addon.id, addonId);
+  equal(addon.version, "1.0");
+  equal(addon.appDisabled, false);
+  equal(addon.userDisabled, false);
+  addonDir.remove(true);
+
+  // update the manifest to make the add-on version incompatible, so the reload will reject
+  manifest.applications.gecko.strict_min_version = "1";
+  manifest.applications.gecko.strict_max_version = "1";
+  manifest.version = "2.0";
 
-  // Remove strictCompatibility from the manifest.
-  writeInstallRDFToDir(manifestSample, tempdir, manifestSample.id);
+  addonDir = yield promiseWriteWebManifestForExtension(manifest, tempdir, "invalid_version", false);
+  let expectedMsg = new RegExp("Add-on invalid_version_cannot_be_reloaded@tests.mozilla.org is not compatible with application version. " +
+                               "add-on minVersion: 1, add-on maxVersion: 1");
 
-  yield addon.reload();
+  yield Assert.rejects(addon.reload(),
+                       expectedMsg,
+                       "Reload rejects when application version does not fall between minVersion and maxVersion");
 
-  const reloadedAddon = yield promiseAddonByID(manifestSample.id);
+  let reloadedAddon = yield promiseAddonByID(addonId);
   notEqual(reloadedAddon, null);
+  equal(reloadedAddon.id, addonId);
+  equal(reloadedAddon.version, "1.0");
   equal(reloadedAddon.appDisabled, false);
+  equal(reloadedAddon.userDisabled, false);
 
   yield tearDownAddon(reloadedAddon);
-  unpackedAddon.remove(true);
+  addonDir.remove(true);
 });
 
 add_task(function* test_manifest_changes_are_refreshed() {
   yield promiseRestartManager();
   let tempdir = gTmpD.clone();
 
   const unpackedAddon = writeInstallRDFToDir(
     Object.assign({}, manifestSample, {
--- a/toolkit/mozapps/extensions/test/xpcshell/test_webextension.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_webextension.js
@@ -119,17 +119,17 @@ add_task(function*() {
   equal(GlobalManager.extensionMap.size, 0);
   do_check_false(GlobalManager.extensionMap.has(ID));
 
   yield promiseShutdownManager();
 });
 
 // Writing the manifest direct to the profile should work
 add_task(function*() {
-  writeWebManifestForExtension({
+  yield promiseWriteWebManifestForExtension({
     name: "Web Extension Name",
     version: "1.0",
     manifest_version: 2,
     applications: {
       gecko: {
         id: ID
       }
     }
@@ -185,17 +185,17 @@ add_task(function* test_manifest_localiz
   equal(addon.name, "Web Extensiøn foo ☹");
   equal(addon.description, "Descriptïon bar ☹ of add-on");
 
   addon.uninstall();
 });
 
 // Missing version should cause a failure
 add_task(function*() {
-  writeWebManifestForExtension({
+  yield promiseWriteWebManifestForExtension({
     name: "Web Extension Name",
     manifest_version: 2,
     applications: {
       gecko: {
         id: ID
       }
     }
   }, profileDir);
@@ -208,17 +208,17 @@ add_task(function*() {
   let file = getFileForAddon(profileDir, ID);
   do_check_false(file.exists());
 
   yield promiseRestartManager();
 });
 
 // Incorrect manifest version should cause a failure
 add_task(function*() {
-  writeWebManifestForExtension({
+  yield promiseWriteWebManifestForExtension({
     name: "Web Extension Name",
     version: "1.0",
     manifest_version: 1,
     applications: {
       gecko: {
         id: ID
       }
     }
--- a/toolkit/mozapps/extensions/test/xpcshell/test_webextension_icons.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_webextension_icons.js
@@ -21,17 +21,17 @@ function promiseAddonStartup() {
     };
 
     Management.on("startup", listener);
   });
 }
 
 // Test simple icon set parsing
 add_task(function*() {
-  writeWebManifestForExtension({
+  yield promiseWriteWebManifestForExtension({
     name: "Web Extension Name",
     version: "1.0",
     manifest_version: 2,
     applications: {
       gecko: {
         id: ID
       }
     },
@@ -85,17 +85,17 @@ add_task(function*() {
 
   addon.uninstall();
 
   yield promiseRestartManager();
 });
 
 // Test AddonManager.getPreferredIconURL for retina screen sizes
 add_task(function*() {
-  writeWebManifestForExtension({
+  yield promiseWriteWebManifestForExtension({
     name: "Web Extension Name",
     version: "1.0",
     manifest_version: 2,
     applications: {
       gecko: {
         id: ID
       }
     },
@@ -131,17 +131,17 @@ add_task(function*() {
 
   addon.uninstall();
 
   yield promiseRestartManager();
 });
 
 // Handles no icons gracefully
 add_task(function*() {
-  writeWebManifestForExtension({
+  yield promiseWriteWebManifestForExtension({
     name: "Web Extension Name",
     version: "1.0",
     manifest_version: 2,
     applications: {
       gecko: {
         id: ID
       }
     }
--- a/toolkit/mozapps/extensions/test/xpcshell/test_webextension_install.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_webextension_install.js
@@ -74,17 +74,17 @@ add_task(function* test_unsigned_no_id_t
   }
   const manifest = {
     name: "no ID",
     description: "extension without an ID",
     manifest_version: 2,
     version: "1.0"
   };
 
-  const addonDir = writeWebManifestForExtension(manifest, gTmpD,
+  const addonDir = yield promiseWriteWebManifestForExtension(manifest, gTmpD,
                                                 "the-addon-sub-dir");
   const addon = yield AddonManager.installTemporaryAddon(addonDir);
   ok(addon.id, "ID should have been auto-generated");
 
   // The sourceURI of a temporary installed addon should be equal to the
   // file url of the installed source dir.
   do_check_eq(addon.sourceURI && addon.sourceURI.spec,
               Services.io.newFileURI(addonDir).spec);
@@ -107,19 +107,19 @@ add_task(function* test_multiple_no_id_e
   }
   const manifest = {
     name: "no ID",
     description: "extension without an ID",
     manifest_version: 2,
     version: "1.0"
   };
 
-  const firstAddonDir = writeWebManifestForExtension(manifest, gTmpD,
+  const firstAddonDir = yield promiseWriteWebManifestForExtension(manifest, gTmpD,
                                                      "addon-sub-dir-one");
-  const secondAddonDir = writeWebManifestForExtension(manifest, gTmpD,
+  const secondAddonDir = yield promiseWriteWebManifestForExtension(manifest, gTmpD,
                                                       "addon-sub-dir-two");
   const [firstAddon, secondAddon] = yield Promise.all([
     AddonManager.installTemporaryAddon(firstAddonDir),
     AddonManager.installTemporaryAddon(secondAddonDir)
   ]);
 
   const allAddons = yield new Promise(resolve => {
     AddonManager.getAllAddons(addons => resolve(addons));
@@ -150,17 +150,17 @@ add_task(function* test_bss_id() {
         id: ID
       }
     }
   };
 
   let addon = yield promiseAddonByID(ID);
   do_check_eq(addon, null);
 
-  writeWebManifestForExtension(manifest, profileDir, ID);
+  yield promiseWriteWebManifestForExtension(manifest, profileDir, ID);
   yield promiseRestartManager();
 
   addon = yield promiseAddonByID(ID);
   do_check_neq(addon, null);
 
   addon.uninstall();
 });
 
@@ -184,18 +184,183 @@ add_task(function* test_two_ids() {
 
     browser_specific_settings: {
       gecko: {
         id: GOOD_ID
       }
     }
   }
 
-  writeWebManifestForExtension(manifest, profileDir, GOOD_ID);
+  yield promiseWriteWebManifestForExtension(manifest, profileDir, GOOD_ID);
   yield promiseRestartManager();
 
   let addon = yield promiseAddonByID(BAD_ID);
   do_check_eq(addon, null);
   addon = yield promiseAddonByID(GOOD_ID);
   do_check_neq(addon, null);
 
   addon.uninstall();
 });
+
+// Test that strict_min_version and strict_max_version are enforced for
+// loading temporary extension.
+add_task(function* test_strict_min_max() {
+  // the app version being compared to is 1.9.2
+  const addonId = "strict_min_max@tests.mozilla.org";
+  const MANIFEST = {
+    name: "strict min max test",
+    description: "test strict min and max with temporary loading",
+    manifest_version: 2,
+    version: "1.0",
+  };
+
+  function flushAndRemove(file) {
+    // flush JAR cache and remove the file
+    Services.obs.notifyObservers(file, "flush-cache-entry", null);
+    file.remove(true);
+  }
+
+  // bad max good min
+  let apps = {
+    applications: {
+      gecko: {
+        id: addonId,
+        strict_min_version: "1",
+        strict_max_version: "1"
+      },
+    },
+  }
+  let testManifest = Object.assign(apps, MANIFEST);
+
+  let addonDir = yield promiseWriteWebManifestForExtension(testManifest, gTmpD,
+                                              "the-addon-sub-dir");
+
+  let expectedMsg = new RegExp("Add-on strict_min_max@tests.mozilla.org is not compatible with application version. " +
+                               "add-on minVersion: 1, add-on maxVersion: 1");
+  yield Assert.rejects(AddonManager.installTemporaryAddon(addonDir),
+                       expectedMsg,
+                       "Install rejects when specified maxVersion is not valid");
+
+  let addon = yield promiseAddonByID(addonId);
+  do_check_eq(addon, null);
+  flushAndRemove(addonDir);
+
+  // bad min good max
+  apps = {
+    applications: {
+      gecko: {
+        id: addonId,
+        strict_min_version: "2",
+        strict_max_version: "2"
+      },
+    },
+  }
+  testManifest = Object.assign(apps, MANIFEST);
+
+  addonDir = yield promiseWriteWebManifestForExtension(testManifest, gTmpD,
+                                          "the-addon-sub-dir");
+
+  expectedMsg = new RegExp("Add-on strict_min_max@tests.mozilla.org is not compatible with application version. " +
+                               "add-on minVersion: 2, add-on maxVersion: 2");
+  yield Assert.rejects(AddonManager.installTemporaryAddon(addonDir),
+                       expectedMsg,
+                       "Install rejects when specified minVersion is not valid");
+
+  addon = yield promiseAddonByID(addonId);
+  do_check_eq(addon, null);
+  flushAndRemove(addonDir);
+
+  // bad both
+  apps = {
+    applications: {
+      gecko: {
+        id: addonId,
+        strict_min_version: "2",
+        strict_max_version: "1"
+      },
+    },
+  }
+  testManifest = Object.assign(apps, MANIFEST);
+
+  addonDir = yield promiseWriteWebManifestForExtension(testManifest, gTmpD,
+                                          "the-addon-sub-dir");
+
+  expectedMsg = new RegExp("Add-on strict_min_max@tests.mozilla.org is not compatible with application version. " +
+                               "add-on minVersion: 2, add-on maxVersion: 1");
+  yield Assert.rejects(AddonManager.installTemporaryAddon(addonDir),
+                       expectedMsg,
+                       "Install rejects when specified minVersion and maxVersion are not valid");
+
+  addon = yield promiseAddonByID(addonId);
+  do_check_eq(addon, null);
+  flushAndRemove(addonDir);
+
+  // good both
+  apps = {
+    applications: {
+      gecko: {
+        id: addonId,
+        strict_min_version: "1",
+        strict_max_version: "2"
+      },
+    },
+  }
+  testManifest = Object.assign(apps, MANIFEST);
+
+  addonDir = yield promiseWriteWebManifestForExtension(testManifest, gTmpD,
+                                          "strict_min_max");
+
+  yield AddonManager.installTemporaryAddon(addonDir);
+  addon = yield promiseAddonByID(addonId);
+
+  do_check_neq(addon, null);
+  do_check_eq(addon.id, addonId);
+  addon.uninstall();
+  flushAndRemove(addonDir);
+
+  // good only min
+  let newId = "strict_min_only@tests.mozilla.org";
+  apps = {
+    applications: {
+      gecko: {
+        id: newId,
+        strict_min_version: "1",
+      },
+    },
+  }
+  testManifest = Object.assign(apps, MANIFEST);
+
+  addonDir = yield promiseWriteWebManifestForExtension(testManifest, gTmpD,
+                                          "strict_min_only");
+
+  yield AddonManager.installTemporaryAddon(addonDir);
+  addon = yield promiseAddonByID(newId);
+
+  do_check_neq(addon, null);
+  do_check_eq(addon.id, newId);
+
+  addon.uninstall();
+  flushAndRemove(addonDir);
+
+  // good only max
+  newId = "strict_max_only@tests.mozilla.org";
+  apps = {
+    applications: {
+      gecko: {
+        id: newId,
+        strict_max_version: "2",
+      },
+    },
+  }
+  testManifest = Object.assign(apps, MANIFEST);
+
+  addonDir = yield promiseWriteWebManifestForExtension(testManifest, gTmpD,
+                                          "strict_max_only");
+
+  yield AddonManager.installTemporaryAddon(addonDir);
+  addon = yield promiseAddonByID(newId);
+
+  do_check_neq(addon, null);
+  do_check_eq(addon.id, newId);
+
+  addon.uninstall();
+  flushAndRemove(addonDir);
+});
--- a/toolkit/mozapps/extensions/test/xpcshell/test_webextension_paths.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_webextension_paths.js
@@ -32,17 +32,17 @@ add_task(function* test_bad_unpacked_pat
 
   const directories = [
     "not a valid ID",
     '"quotes"@tests.mozilla.org',
   ];
 
   for (let dir of directories) {
     try {
-      writeWebManifestForExtension(manifest, profileDir, dir);
+      yield promiseWriteWebManifestForExtension(manifest, profileDir, dir);
     } catch (ex) {
       // This can fail if the underlying filesystem (looking at you windows)
       // doesn't handle some of the characters in the ID.  In that case,
       // just ignore this test on this platform.
       continue;
     }
     yield promiseRestartManager();
 
--- a/toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini
+++ b/toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini
@@ -1,17 +1,18 @@
 # The file is shared between the two main xpcshell manifest files.
 [DEFAULT]
 skip-if = toolkit == 'android' || toolkit == 'gonk'
 tags = addons
 
 [test_AddonRepository.js]
 [test_reload.js]
 # Bug 676992: test consistently hangs on Android
-skip-if = os == "android"
+# There's a problem removing a temp file without manually clearing the cache on Windows
+skip-if = os == "android" || os == "win"
 tags = webextensions
 [test_AddonRepository_cache.js]
 # Bug 676992: test consistently hangs on Android
 # Bug 1026805: frequent hangs on OSX 10.8
 skip-if = os == "android" || os == "mac"
 run-sequentially = Uses hardcoded ports in xpi files.
 [test_AddonRepository_compatmode.js]
 # Bug 676992: test consistently hangs on Android