Bug 1323228 - throw an exception if using the storage API without an ID, r?kmag
This protection is in ext-storage.js rather than
ExtensionStorageSync.jsm because storage.local may one day be migrated
to work using the same code as ExtensionStorageSync, but without any
syncing event loop. When this happens, we still want to raise but only
in the storage.sync case.
MozReview-Commit-ID: Jwu9FA5DZA6
--- a/toolkit/components/extensions/ext-storage.js
+++ b/toolkit/components/extensions/ext-storage.js
@@ -1,22 +1,35 @@
"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 will not work 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) {
return ExtensionStorage.get(extension.id, spec);
},
@@ -28,25 +41,29 @@ function storageApiFactory(context) {
},
clear: function() {
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/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,39 @@ 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.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