Bug 1262005 - Rework how WebExtensions IDs are determined. r=rhelmer

MozReview-Commit-ID: 37EujfhGh0U

--HG--
extra : rebase_source : 3ac03fc02f618e149ffecc26678ac25914bb8358

toolkit/components/extensions/schemas/manifest.json

@@ -15,28 +15,22 @@
 
           "applications": {
             "type": "object",
+            "optional": true,
             "properties": {
               "gecko": {
-                "type": "object",
-                "properties": {
-                  "id": { "$ref": "ExtensionID" },
+                "$ref": "FirefoxSpecificProperties",
+                "optional": true
+              }
+            }
+          },
 
-                  "update_url": {
-                    "type": "string",
-                    "format": "url",
-                    "optional": true
-                  },
-
-                  "strict_min_version": {
-                    "type": "string",
-                    "optional": true
-                  },
-
-                  "strict_max_version": {
-                    "type": "string",
-                    "optional": true
-                  }
-                }
+          "browser_specific_settings": {
+            "type": "object",
+            "optional": true,
+            "properties": {
+              "gecko": {
+                "$ref": "FirefoxSpecificProperties",
+                "optional": true
               }
             }
           },
@@ -205,6 +199,32 @@
           }
         ]
       },
+      {
+        "id": "FirefoxSpecificProperties",
+        "type": "object",
+        "properties": {
+          "id": {
+            "$ref": "ExtensionID",
+            "optional": true
+          },
+
+          "update_url": {
+            "type": "string",
+            "format": "url",
+            "optional": true
+          },
+
+          "strict_min_version": {
+            "type": "string",
+            "optional": true
+          },
+
+          "strict_max_version": {
+            "type": "string",
+            "optional": true
+          }
+        }
+      },
       {
         "id": "MatchPattern",
         "choices": [

toolkit/mozapps/extensions/internal/XPIProvider.jsm

@@ -880,8 +880,14 @@ var loadManifestFromWebManifest = Task.async(function*(aUri) {
   if (extension.errors.length)
     throw new Error("Extension is invalid");
 
+  let bss = (manifest.browser_specific_settings && manifest.browser_specific_settings.gecko)
+      || (manifest.applications && manifest.applications.gecko) || {};
+  if (manifest.browser_specific_settings && manifest.applications) {
+    logger.warn("Ignoring applications property in manifest");
+  }
+
   let addon = new AddonInternal();
-  addon.id = manifest.applications.gecko.id;
+  addon.id = bss.id;
   addon.version = manifest.version;
   addon.type = "webextension";
   addon.unpack = false;
@@ -890,7 +896,7 @@ var loadManifestFromWebManifest = Task.async(function*(aUri) {
   addon.hasBinaryComponents = false;
   addon.multiprocessCompatible = true;
   addon.internalName = null;
-  addon.updateURL = manifest.applications.gecko.update_url;
+  addon.updateURL = bss.update_url;
   addon.updateKey = null;
   addon.optionsURL = null;
   addon.optionsType = null;
@@ -937,9 +943,9 @@ var loadManifestFromWebManifest = Task.async(function*(aUri) {
 
   addon.targetApplications = [{
     id: TOOLKIT_ID,
-    minVersion: (manifest.applications.gecko.strict_min_version ||
+    minVersion: (bss.strict_min_version ||
                  AddonManagerPrivate.webExtensionsMinPlatformVersion),
-    maxVersion: manifest.applications.gecko.strict_max_version || "*",
+    maxVersion: bss.strict_max_version || "*",
   }];
 
   addon.targetPlatforms = [];
@@ -1314,14 +1320,29 @@ var loadManifestFromDir = Task.async(function*(aDir, aInstallLocation) {
 
   let uri = Services.io.newFileURI(file).QueryInterface(Ci.nsIFileURL);
 
-  let addon = file.leafName == FILE_WEB_MANIFEST ?
-              yield loadManifestFromWebManifest(uri) :
-              loadFromRDF(uri);
+  let addon;
+  if (file.leafName == FILE_WEB_MANIFEST) {
+    addon = yield loadManifestFromWebManifest(uri);
+    if (!addon.id) {
+      if (aInstallLocation == TemporaryInstallLocation) {
+        let id = Cc["@mozilla.org/uuid-generator;1"]
+            .getService(Ci.nsIUUIDGenerator)
+            .generateUUID().toString();
+        logger.info(`Generated temporary id ${id} for ${aDir.path}`);
+        addon.id = id;
+      } else {
+        addon.id = aDir.leafName;
+      }
+    }
+  } else {
+    addon = loadFromRDF(uri);
+  }
 
   addon._sourceBundle = aDir.clone();
   addon._installLocation = aInstallLocation;
   addon.size = getFileSize(aDir);
-  addon.signedState = yield verifyDirSignedState(aDir, addon);
+  addon.signedState = yield verifyDirSignedState(aDir, addon)
+    .then(({signedState}) => signedState);
   addon.appDisabled = !isUsableAddon(addon);
 
   defineSyncGUID(addon);
@@ -1380,7 +1401,9 @@ var loadManifestFromZipReader = Task.async(function*(aZipReader, aInstallLocatio
 
   let uri = buildJarURI(aZipReader.file, entry);
 
-  let addon = entry == FILE_WEB_MANIFEST ?
+  let isWebExtension = (entry == FILE_WEB_MANIFEST);
+
+  let addon = isWebExtension ?
               yield loadManifestFromWebManifest(uri) :
               loadFromRDF(uri);
 
@@ -1392,7 +1415,11 @@ var loadManifestFromZipReader = Task.async(function*(aZipReader, aInstallLocatio
   while (entries.hasMore())
     addon.size += aZipReader.getEntry(entries.getNext()).realSize;
 
-  addon.signedState = yield verifyZipSignedState(aZipReader.file, addon);
+  let {signedState, cert} = yield verifyZipSignedState(aZipReader.file, addon);
+  addon.signedState = signedState;
+  if (isWebExtension && !addon.id && cert) {
+    addon.id = cert.commonName;
+  }
   addon.appDisabled = !isUsableAddon(addon);
 
   defineSyncGUID(addon);
@@ -1571,7 +1598,7 @@ function verifyZipSigning(aZip, aCertificate) {
  */
 function getSignedStatus(aRv, aCert, aAddonID) {
   let expectedCommonName = aAddonID;
-  if (aAddonID.length > 64) {
+  if (aAddonID && aAddonID.length > 64) {
     let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"].
                     createInstance(Ci.nsIScriptableUnicodeConverter);
     converter.charset = "UTF-8";
@@ -1586,7 +1613,7 @@ function getSignedStatus(aRv, aCert, aAddonID) {
 
   switch (aRv) {
     case Cr.NS_OK:
-      if (expectedCommonName != aCert.commonName)
+      if (expectedCommonName && expectedCommonName != aCert.commonName)
         return AddonManager.SIGNEDSTATE_BROKEN;
 
       let hotfixID = Preferences.get(PREF_EM_HOTFIX_ID, undefined);
@@ -1659,11 +1686,16 @@ let gCertDB = Cc["@mozilla.org/security/x509certdb;1"]
  *         the xpi file to check
  * @param  aAddon
  *         the add-on object to verify
- * @return a Promise that resolves to an AddonManager.SIGNEDSTATE_* constant.
+ * @return a Promise that resolves to an object with properties:
+ *         signedState: an AddonManager.SIGNEDSTATE_* constant
+ *         cert: an nsIX509Cert
  */
 function verifyZipSignedState(aFile, aAddon) {
   if (!shouldVerifySignedState(aAddon))
-    return Promise.resolve(AddonManager.SIGNEDSTATE_NOT_REQUIRED);
+    return Promise.resolve({
+      signedState: AddonManager.SIGNEDSTATE_NOT_REQUIRED,
+      cert: null
+    });
 
   let root = Ci.nsIX509CertDB.AddonsPublicRoot;
   if (!REQUIRE_SIGNING && Preferences.get(PREF_XPI_SIGNATURES_DEV_ROOT, false))
@@ -1674,7 +1706,10 @@ function verifyZipSignedState(aFile, aAddon) {
       openSignedAppFileFinished: function(aRv, aZipReader, aCert) {
         if (aZipReader)
           aZipReader.close();
-        resolve(getSignedStatus(aRv, aCert, aAddon.id));
+        resolve({
+          signedState: getSignedStatus(aRv, aCert, aAddon.id),
+          cert: aCert
+        });
       }
     };
     // This allows the certificate DB to get the raw JS callback object so the
@@ -1693,11 +1728,16 @@ function verifyZipSignedState(aFile, aAddon) {
  *         the directory to check
  * @param  aAddon
  *         the add-on object to verify
- * @return a Promise that resolves to an AddonManager.SIGNEDSTATE_* constant.
+ * @return a Promise that resolves to an object with properties:
+ *         signedState: an AddonManager.SIGNEDSTATE_* constant
+ *         cert: an nsIX509Cert
  */
 function verifyDirSignedState(aDir, aAddon) {
   if (!shouldVerifySignedState(aAddon))
-    return Promise.resolve(AddonManager.SIGNEDSTATE_NOT_REQUIRED);
+    return Promise.resolve({
+      signedState: AddonManager.SIGNEDSTATE_NOT_REQUIRED,
+      cert: null,
+    });
 
   let root = Ci.nsIX509CertDB.AddonsPublicRoot;
   if (!REQUIRE_SIGNING && Preferences.get(PREF_XPI_SIGNATURES_DEV_ROOT, false))
@@ -1706,7 +1746,10 @@ function verifyDirSignedState(aDir, aAddon) {
   return new Promise(resolve => {
     let callback = {
       verifySignedDirectoryFinished: function(aRv, aCert) {
-        resolve(getSignedStatus(aRv, aCert, aAddon.id));
+        resolve({
+          signedState: getSignedStatus(aRv, aCert, aAddon.id),
+          cert: null,
+        });
       }
     };
     // This allows the certificate DB to get the raw JS callback object so the
@@ -1728,9 +1771,9 @@ function verifyDirSignedState(aDir, aAddon) {
  * @return a Promise that resolves to an AddonManager.SIGNEDSTATE_* constant.
  */
 function verifyBundleSignedState(aBundle, aAddon) {
-  if (aBundle.isFile())
-    return verifyZipSignedState(aBundle, aAddon);
-  return verifyDirSignedState(aBundle, aAddon);
+  let promise = aBundle.isFile() ? verifyZipSignedState(aBundle, aAddon)
+      : verifyDirSignedState(aBundle, aAddon);
+  return promise.then(({signedState}) => signedState);
 }
 
 /**

toolkit/mozapps/extensions/test/xpcshell/test_webextension.js

@@ -190,25 +190,6 @@ add_task(function* test_manifest_localization() {
   addon.uninstall();
 });
 
-// Missing ID should cause a failure
-add_task(function*() {
-  writeWebManifestForExtension({
-    name: "Web Extension Name",
-    version: "1.0",
-    manifest_version: 2,
-  }, profileDir, ID);
-
-  yield promiseRestartManager();
-
-  let addon = yield promiseAddonByID(ID);
-  do_check_eq(addon, null);
-
-  let file = getFileForAddon(profileDir, ID);
-  do_check_false(file.exists());
-
-  yield promiseRestartManager();
-});
-
 // Missing version should cause a failure
 add_task(function*() {
   writeWebManifestForExtension({

toolkit/mozapps/extensions/test/xpcshell/test_webextension_install.js

@@ -0,0 +1,112 @@
+function run_test() {
+  run_next_test();
+}
+
+let profileDir;
+add_task(function* setup() {
+  profileDir = gProfD.clone();
+  profileDir.append("extensions");
+
+  createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
+  startupManager();
+});
+
+const IMPLICIT_ID_XPI = "data/webext-implicit-id.xpi";
+const IMPLICIT_ID_ID = "webext_implicit_id@tests.mozilla.org";
+
+// webext-implicit-id.xpi has a minimal manifest with no
+// applications or browser_specific_settings, so its id comes
+// from its signature, which should be the ID constant defined below.
+add_task(function* test_implicit_id() {
+  let addon = yield promiseAddonByID(IMPLICIT_ID_ID);
+  do_check_eq(addon, null);
+
+  let xpifile = do_get_file(IMPLICIT_ID_XPI);
+  yield promiseInstallAllFiles([xpifile]);
+
+  addon = yield promiseAddonByID(IMPLICIT_ID_ID);
+  do_check_neq(addon, null);
+
+  addon.uninstall();
+});
+
+// We should also be able to install webext-implicit-id.xpi temporarily
+// and it should look just like the regular install (ie, the ID should
+// come from the signature)
+add_task(function* test_implicit_id_temp() {
+  let addon = yield promiseAddonByID(IMPLICIT_ID_ID);
+  do_check_eq(addon, null);
+
+  let xpifile = do_get_file(IMPLICIT_ID_XPI);
+  yield AddonManager.installTemporaryAddon(xpifile);
+
+  addon = yield promiseAddonByID(IMPLICIT_ID_ID);
+  do_check_neq(addon, null);
+
+  addon.uninstall();
+});
+
+// Test that we can get the ID from browser_specific_settings
+add_task(function* test_bss_id() {
+  const ID = "webext_bss_id@tests.mozilla.org";
+
+  let manifest = {
+    name: "bss test",
+    description: "test that ID may be in browser_specific_settings",
+    manifest_version: 2,
+    version: "1.0",
+
+    browser_specific_settings: {
+      gecko: {
+        id: ID
+      }
+    }
+  };
+
+  let addon = yield promiseAddonByID(ID);
+  do_check_eq(addon, null);
+
+  writeWebManifestForExtension(manifest, profileDir, ID);
+  yield promiseRestartManager();
+
+  addon = yield promiseAddonByID(ID);
+  do_check_neq(addon, null);
+
+  addon.uninstall();
+});
+
+// Test that if we have IDs in both browser_specific_settings and applications,
+// that we prefer the ID in browser_specific_settings.
+add_task(function* test_two_ids() {
+  const GOOD_ID = "two_ids@tests.mozilla.org";
+  const BAD_ID = "i_am_obsolete@tests.mozilla.org";
+
+  let manifest = {
+    name: "two id test",
+    description: "test a web extension with ids in both applications and browser_specific_settings",
+    manifest_version: 2,
+    version: "1.0",
+
+    applications: {
+      gecko: {
+        id: BAD_ID
+      }
+    },
+
+    browser_specific_settings: {
+      gecko: {
+        id: GOOD_ID
+      }
+    }
+  }
+
+  writeWebManifestForExtension(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();
+});

toolkit/mozapps/extensions/test/xpcshell/test_webextension_paths.js

@@ -0,0 +1,55 @@
+function run_test() {
+  run_next_test();
+}
+
+let profileDir;
+add_task(function* setup() {
+  profileDir = gProfD.clone();
+  profileDir.append("extensions");
+
+  createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
+  startupManager();
+});
+
+// When installing an unpacked addon we derive the ID from the
+// directory name.  Make sure that if the directoy name is not a valid
+// addon ID that we reject it.
+add_task(function* test_bad_unpacked_path() {
+  let MANIFEST_ID = "webext_bad_path@tests.mozilla.org";
+
+  let manifest = {
+    name: "path test",
+    description: "test of a bad directory name",
+    manifest_version: 2,
+    version: "1.0",
+
+    browser_specific_settings: {
+      gecko: {
+        id: MANIFEST_ID
+      }
+    }
+  };
+
+  const directories = [
+    "not a valid ID",
+    '"quotes"@tests.mozilla.org',
+  ];
+
+  for (let dir of directories) {
+    try {
+      writeWebManifestForExtension(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();
+
+    let addon = yield promiseAddonByID(dir);
+    do_check_eq(addon, null);
+    addon = yield promiseAddonByID(MANIFEST_ID);
+    do_check_eq(addon, null);
+  }
+});
+

toolkit/mozapps/extensions/test/xpcshell/xpcshell-shared.ini

@@ -305,6 +305,7 @@ run-sequentially = Uses global XCurProcD dir.
 [test_sourceURI.js]
 [test_webextension_icons.js]
 [test_webextension.js]
+[test_webextension_install.js]
 [test_bootstrap_globals.js]
 [test_bug1180901_2.js]
 skip-if = os != "win"

toolkit/mozapps/extensions/test/xpcshell/xpcshell-unpack.ini

@@ -6,4 +6,6 @@ skip-if = toolkit == 'android' || toolkit == 'gonk'
 dupe-manifest =
 tags = addons
 
+[test_webextension_paths.js]
+
 [include:xpcshell-shared.ini]