Bug 1323228 - throw an exception if using the storage API without an ID, r?kmag draft
authorEthan Glasser-Camp <eglassercamp@mozilla.com>
Mon, 19 Dec 2016 18:06:05 -0500
changeset 452498 5d942e2015b3cf74c1742edb68f08bf5f4464ab6
parent 451111 d4b3146a5567a7ddbcdfa5244945db55616cb8d1
child 540239 833d82d5cb87bdc9cba6e6d8320e06db3b7e0050
push id39417
push usereglassercamp@mozilla.com
push dateWed, 21 Dec 2016 20:58:31 +0000
reviewerskmag
bugs1323228
milestone53.0a1
Bug 1323228 - throw an exception if using the storage API without an ID, r?kmag MozReview-Commit-ID: Jwu9FA5DZA6
toolkit/components/extensions/ext-storage.js
toolkit/components/extensions/test/mochitest/test_ext_listener_proxies.html
toolkit/components/extensions/test/xpcshell/test_ext_storage.js
toolkit/mozapps/extensions/AddonManager.jsm
toolkit/mozapps/extensions/internal/XPIProvider.jsm
--- a/toolkit/components/extensions/ext-storage.js
+++ b/toolkit/components/extensions/ext-storage.js
@@ -1,52 +1,73 @@
 "use strict";
 
 var {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 XPCOMUtils.defineLazyModuleGetter(this, "ExtensionStorage",
                                   "resource://gre/modules/ExtensionStorage.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ExtensionStorageSync",
                                   "resource://gre/modules/ExtensionStorageSync.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "AddonManagerPrivate",
+                                  "resource://gre/modules/AddonManager.jsm");
 
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 var {
   EventManager,
+  ExtensionError,
 } = ExtensionUtils;
 
+function enforceNoTemporaryAddon(extensionId) {
+  const EXCEPTION_MESSAGE =
+        "The storage API is not available with a temporary addon ID. " +
+        "Please add an explicit addon ID to your manifest. " +
+        "For more information see https://bugzil.la/1323228.";
+  if (AddonManagerPrivate.isTemporaryInstallID(extensionId)) {
+    throw new ExtensionError(EXCEPTION_MESSAGE);
+  }
+}
+
 function storageApiFactory(context) {
   let {extension} = context;
   return {
     storage: {
       local: {
         get: function(spec) {
+          enforceNoTemporaryAddon(extension.id);
           return ExtensionStorage.get(extension.id, spec);
         },
         set: function(items) {
+          enforceNoTemporaryAddon(extension.id);
           return ExtensionStorage.set(extension.id, items, context);
         },
         remove: function(keys) {
+          enforceNoTemporaryAddon(extension.id);
           return ExtensionStorage.remove(extension.id, keys);
         },
         clear: function() {
+          enforceNoTemporaryAddon(extension.id);
           return ExtensionStorage.clear(extension.id);
         },
       },
 
       sync: {
         get: function(spec) {
+          enforceNoTemporaryAddon(extension.id);
           return ExtensionStorageSync.get(extension, spec, context);
         },
         set: function(items) {
+          enforceNoTemporaryAddon(extension.id);
           return ExtensionStorageSync.set(extension, items, context);
         },
         remove: function(keys) {
+          enforceNoTemporaryAddon(extension.id);
           return ExtensionStorageSync.remove(extension, keys, context);
         },
         clear: function() {
+          enforceNoTemporaryAddon(extension.id);
           return ExtensionStorageSync.clear(extension, context);
         },
       },
 
       onChanged: new EventManager(context, "storage.onChanged", fire => {
         let listenerLocal = changes => {
           fire(changes, "local");
         };
--- a/toolkit/components/extensions/test/mochitest/test_ext_listener_proxies.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_listener_proxies.html
@@ -10,18 +10,16 @@
 </head>
 <body>
 
 <script type="text/javascript">
 "use strict";
 
 add_task(function* test_listener_proxies() {
   let extension = ExtensionTestUtils.loadExtension({
-    useAddonManager: "temporary",
-
     manifest: {
       "permissions": ["storage"],
     },
 
     async background() {
       // Test that adding multiple listeners for the same event works as
       // expected.
 
--- a/toolkit/components/extensions/test/xpcshell/test_ext_storage.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_storage.js
@@ -1,15 +1,19 @@
 /* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
 /* vim: set sts=2 sw=2 et tw=80: */
 "use strict";
 
 const STORAGE_SYNC_PREF = "webextensions.storage.sync.enabled";
 Cu.import("resource://gre/modules/Preferences.jsm");
 
+add_task(function* setup() {
+  yield ExtensionTestUtils.startAddonManager();
+});
+
 /**
  * Utility function to ensure that all supported APIs for getting are
  * tested.
  *
  * @param {string} areaName
  *        either "local" or "sync" according to what we want to test
  * @param {string} prop
  *        "key" to look up using the storage API
@@ -327,8 +331,41 @@ add_task(function* test_backgroundScript
   yield extension.awaitMessage("test-finished");
 
   extension.sendMessage("test-sync");
   yield extension.awaitMessage("test-finished");
 
   Preferences.reset(STORAGE_SYNC_PREF);
   yield extension.unload();
 });
+
+add_task(function* test_storage_requires_real_id() {
+  async function backgroundScript() {
+    const EXCEPTION_MESSAGE =
+          "The storage API is not available with a temporary addon ID. " +
+          "Please add an explicit addon ID to your manifest. " +
+          "For more information see https://bugzil.la/1323228.";
+
+    await browser.test.assertRejects(browser.storage.local.set({"foo": "bar"}),
+                                     EXCEPTION_MESSAGE);
+    await browser.test.assertRejects(browser.storage.sync.set({"foo": "bar"}),
+                                     EXCEPTION_MESSAGE);
+
+    browser.test.notifyPass("exception correct");
+  }
+
+  let extensionData = {
+    background: `(${backgroundScript})(${checkGetImpl})`,
+    manifest: {
+      permissions: ["storage"],
+    },
+    useAddonManager: "temporary",
+  };
+
+  Preferences.set(STORAGE_SYNC_PREF, true);
+
+  let extension = ExtensionTestUtils.loadExtension(extensionData);
+  yield extension.startup();
+  yield extension.awaitFinish("exception correct");
+
+  Preferences.reset(STORAGE_SYNC_PREF);
+  yield extension.unload();
+});
--- a/toolkit/mozapps/extensions/AddonManager.jsm
+++ b/toolkit/mozapps/extensions/AddonManager.jsm
@@ -3066,16 +3066,33 @@ this.AddonManagerPrivate = {
 
   hasUpgradeListener: function(aId) {
     return AddonManagerInternal.upgradeListeners.has(aId);
   },
 
   getUpgradeListener: function(aId) {
     return AddonManagerInternal.upgradeListeners.get(aId);
   },
+
+  /**
+   * Predicate that returns true if we think the given extension ID
+   * might have been generated by XPIProvider.
+   */
+  isTemporaryInstallID: function(extensionId) {
+     if (!gStarted)
+       throw Components.Exception("AddonManager is not initialized",
+                                  Cr.NS_ERROR_NOT_INITIALIZED);
+
+     if (!extensionId || typeof extensionId != "string")
+       throw Components.Exception("extensionId must be a string",
+                                  Cr.NS_ERROR_INVALID_ARG);
+
+    return AddonManagerInternal._getProviderByName("XPIProvider")
+                               .isTemporaryInstallID(extensionId);
+  },
 };
 
 /**
  * This is the public API that UI and developers should be calling. All methods
  * just forward to AddonManagerInternal.
  */
 this.AddonManager = {
   // Constants for the AddonInstall.state property
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -1359,27 +1359,29 @@ function defineSyncGUID(aAddon) {
       delete aAddon.syncGUID;
       aAddon.syncGUID = val;
     },
     configurable: true,
     enumerable: true,
   });
 }
 
+const TEMPORARY_ADDON_SUFFIX = "@temporary-addon";
+
 // Generate a unique ID based on the path to this temporary add-on location.
 function generateTemporaryInstallID(aFile) {
   const hasher = Cc["@mozilla.org/security/hash;1"]
         .createInstance(Ci.nsICryptoHash);
   hasher.init(hasher.SHA1);
   const data = new TextEncoder().encode(aFile.path);
   // Make it so this ID cannot be guessed.
   const sess = TEMP_INSTALL_ID_GEN_SESSION;
   hasher.update(sess, sess.length);
   hasher.update(data, data.length);
-  let id = `${getHashStringForCrypto(hasher)}@temporary-addon`;
+  let id = `${getHashStringForCrypto(hasher)}${TEMPORARY_ADDON_SUFFIX}`;
   logger.info(`Generated temp id ${id} (${sess.join("")}) for ${aFile.path}`);
   return id;
 }
 
 /**
  * Loads an AddonInternal object from an add-on extracted in a directory.
  *
  * @param  aDir
@@ -3963,16 +3965,21 @@ this.XPIProvider = {
     let requireSecureOrigin = Preferences.get(PREF_INSTALL_REQUIRESECUREORIGIN, true);
     let safeSchemes = ["https", "chrome", "file"];
     if (requireSecureOrigin && safeSchemes.indexOf(uri.scheme) == -1)
       return false;
 
     return true;
   },
 
+  // Identify temporary install IDs.
+  isTemporaryInstallID: function(id) {
+    return id.endsWith(TEMPORARY_ADDON_SUFFIX);
+  },
+
   /**
    * Called to get an AddonInstall to download and install an add-on from a URL.
    *
    * @param  aUrl
    *         The URL to be installed
    * @param  aHash
    *         A hash for the install
    * @param  aName