Bug 1277695 Add sanity checks for addons with no id r?kmag
The patch for
bug 1277965 (enable ADDON_SIGNING by default) fixes
the actual problem here but this patch adds a series of sanity
checks in the add-on manager and the test case, so any similar
problem in the future should be easier to troubleshoot.
MozReview-Commit-ID: JGQk9yCVFkQ
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -428,16 +428,22 @@ SafeInstallOperation.prototype = {
logger.error("Failed to " + (aCopy ? "copy" : "move") + " file " + aFile.path +
" to " + aTargetDirectory.path, e);
throw e;
}
this._installedFiles.push({ oldFile: oldFile, newFile: newFile });
},
_installDirectory: function(aDirectory, aTargetDirectory, aCopy) {
+ if (aDirectory.contains(aTargetDirectory)) {
+ let err = new Error(`Not installing ${aDirectory} into its own descendent ${aTargetDirectory}`);
+ logger.error(err);
+ throw err;
+ }
+
let newDir = aTargetDirectory.clone();
newDir.append(aDirectory.leafName);
try {
newDir.create(Ci.nsIFile.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
}
catch (e) {
logger.error("Failed to create directory " + newDir.path, e);
throw e;
@@ -5617,16 +5623,23 @@ AddonInstall.prototype = {
// loadManifestFromZipReader performs the certificate verification for us
this.addon = yield loadManifestFromZipReader(zipreader, this.installLocation);
}
catch (e) {
zipreader.close();
return Promise.reject([AddonManager.ERROR_CORRUPT_FILE, e]);
}
+ // A multi-package XPI is a container, the add-ons it holds each
+ // have their own id. Everything else had better have an id here.
+ if (!this.addon.id && this.addon.type != "multipackage") {
+ let err = new Error(`Cannot find id for addon ${file.path}`);
+ return Promise.reject([AddonManager.ERROR_CORRUPT_FILE, err]);
+ }
+
if (mustSign(this.addon.type)) {
if (this.addon.signedState <= AddonManager.SIGNEDSTATE_MISSING) {
// This add-on isn't properly signed by a signature that chains to the
// trusted root.
let state = this.addon.signedState;
this.addon = null;
zipreader.close();
--- a/toolkit/mozapps/extensions/test/xpcshell/test_webextension_install.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_webextension_install.js
@@ -1,44 +1,58 @@
+
+const {ADDON_SIGNING} = AM_Cu.import("resource://gre/modules/addons/AddonConstants.jsm", {});
+
function run_test() {
run_next_test();
}
let profileDir;
add_task(function* setup() {
profileDir = gProfD.clone();
profileDir.append("extensions");
+ if (!profileDir.exists())
+ profileDir.create(AM_Ci.nsIFile.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
+
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() {
+ // This test needs to read the xpi certificate which only works
+ // if signing is enabled.
+ ok(ADDON_SIGNING, "Addon signing is enabled");
+
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() {
+ // This test needs to read the xpi certificate which only works
+ // if signing is enabled.
+ ok(ADDON_SIGNING, "Addon signing is enabled");
+
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);