Bug 1275139 (part 1) - kill ignoreRepositoryChecking pref, replacing it with AddonRepository.cache. r=rhelmer

MozReview-Commit-ID: 4tbctcuoFeB
This commit is contained in:
Mark Hammond 2016-06-03 14:45:27 +10:00
parent 2e8a001926
commit 31b43265c9
8 changed files with 28 additions and 37 deletions

View File

@ -374,6 +374,9 @@ AddonUtilsInternal.prototype = {
// Verify that the source URI uses TLS. We don't allow installs from
// insecure sources for security reasons. The Addon Manager ensures
// that cert validation etc is performed.
// (We should also consider just dropping this entirely and calling
// XPIProvider.isInstallAllowed, but that has additional semantics we might
// need to think through...)
let requireSecureURI = true;
if (options && options.requireSecureURI !== undefined) {
requireSecureURI = options.requireSecureURI;

View File

@ -25,10 +25,13 @@
*
* Synchronization is influenced by the following preferences:
*
* - services.sync.addons.ignoreRepositoryChecking
* - services.sync.addons.ignoreUserEnabledChanges
* - services.sync.addons.trustedSourceHostnames
*
* and also influenced by whether addons have repository caching enabled and
* whether they allow installation of addons from insecure options (both of
* which are themselves influenced by the "extensions." pref branch)
*
* See the documentation in services-sync.js for the behavior of these prefs.
*/
"use strict";
@ -291,7 +294,7 @@ AddonsStore.prototype = {
id: record.addonID,
syncGUID: record.id,
enabled: record.enabled,
requireSecureURI: !Svc.Prefs.get("addons.ignoreRepositoryChecking", false),
requireSecureURI: this._extensionsPrefs.get("install.requireSecureOrigin", true),
}], cb);
// This will throw if there was an error. This will get caught by the sync
@ -566,10 +569,10 @@ AddonsStore.prototype = {
return false;
}
// We provide a back door to skip the repository checking of an add-on.
// This is utilized by the tests to make testing easier. Users could enable
// this, but it would sacrifice security.
if (Svc.Prefs.get("addons.ignoreRepositoryChecking", false)) {
// If the AddonRepository's cache isn't enabled (which it typically isn't
// in tests), getCachedAddonByID always returns null - so skip the check
// in that case.
if (!AddonRepository.cacheEnabled) {
return true;
}

View File

@ -38,9 +38,6 @@ pref("services.sync.jpake.firstMsgMaxTries", 300); // 5 minutes
pref("services.sync.jpake.lastMsgMaxTries", 300); // 5 minutes
pref("services.sync.jpake.maxTries", 10);
// Allow add-ons to be synced from non-trusted sources.
pref("services.sync.addons.ignoreRepositoryChecking", false);
// If true, add-on sync ignores changes to the user-enabled flag. This
// allows people to have the same set of add-ons installed across all
// profiles while maintaining different enabled states.

View File

@ -103,8 +103,6 @@ add_test(function test_source_uri_rewrite() {
// This tests for conformance with bug 708134 so server-side metrics aren't
// skewed.
Svc.Prefs.set("addons.ignoreRepositoryChecking", true);
// We resort to monkeypatching because of the API design.
let oldFunction = AddonUtils.__proto__.installAddonFromSearchResult;
@ -139,6 +137,5 @@ add_test(function test_source_uri_rewrite() {
do_check_true(installCalled);
AddonUtils.__proto__.installAddonFromSearchResult = oldFunction;
Svc.Prefs.reset("addons.ignoreRepositoryChecking");
server.stop(run_next_test);
});

View File

@ -16,6 +16,7 @@ Cu.import("resource://testing-common/services/sync/utils.js");
var prefs = new Preferences();
prefs.set("extensions.getAddons.get.url",
"http://localhost:8888/search/guid:%IDS%");
prefs.set("extensions.install.requireSecureOrigin", false);
loadAddonTestFunctions();
startupManager();
@ -35,8 +36,6 @@ function advance_test() {
reconciler.saveState(null, cb);
cb.wait();
Svc.Prefs.reset("addons.ignoreRepositoryChecking");
run_next_test();
}
@ -104,7 +103,6 @@ add_test(function test_get_changed_ids() {
tracker.clearChangedIDs();
_("Ensure reconciler changes are populated.");
Svc.Prefs.set("addons.ignoreRepositoryChecking", true);
let addon = installAddon("test_bootstrap1_1");
tracker.clearChangedIDs(); // Just in case.
changes = engine.getChangedIDs();
@ -151,9 +149,6 @@ add_test(function test_disabled_install_semantics() {
// This is essentially a test for bug 712542, which snuck into the original
// add-on sync drop. It ensures that when an add-on is installed that the
// disabled state and incoming syncGUID is preserved, even on the next sync.
Svc.Prefs.set("addons.ignoreRepositoryChecking", true);
const USER = "foo";
const PASSWORD = "password";
const PASSPHRASE = "abcdeabcdeabcdeabcdeabcdea";

View File

@ -16,6 +16,8 @@ const HTTP_PORT = 8888;
var prefs = new Preferences();
prefs.set("extensions.getAddons.get.url", "http://localhost:8888/search/guid:%IDS%");
prefs.set("extensions.install.requireSecureOrigin", false);
loadAddonTestFunctions();
startupManager();
@ -194,7 +196,6 @@ add_test(function test_apply_uninstall() {
add_test(function test_addon_syncability() {
_("Ensure isAddonSyncable functions properly.");
Svc.Prefs.set("addons.ignoreRepositoryChecking", true);
Svc.Prefs.set("addons.trustedSourceHostnames",
"addons.mozilla.org,other.example.com");
@ -268,8 +269,6 @@ add_test(function test_addon_syncability() {
add_test(function test_ignore_hotfixes() {
_("Ensure that hotfix extensions are ignored.");
Svc.Prefs.set("addons.ignoreRepositoryChecking", true);
// A hotfix extension is one that has the id the same as the
// extensions.hotfix.id pref.
let prefs = new Preferences("extensions.");
@ -301,7 +300,6 @@ add_test(function test_ignore_hotfixes() {
uninstallAddon(addon);
Svc.Prefs.reset("addons.ignoreRepositoryChecking");
prefs.reset("hotfix.id");
run_next_test();
@ -311,8 +309,6 @@ add_test(function test_ignore_hotfixes() {
add_test(function test_get_all_ids() {
_("Ensures that getAllIDs() returns an appropriate set.");
Svc.Prefs.set("addons.ignoreRepositoryChecking", true);
_("Installing two addons.");
let addon1 = installAddon("test_install1");
let addon2 = installAddon("test_bootstrap1_1");
@ -331,7 +327,6 @@ add_test(function test_get_all_ids() {
addon1.install.cancel();
uninstallAddon(addon2);
Svc.Prefs.reset("addons.ignoreRepositoryChecking");
run_next_test();
});
@ -357,9 +352,6 @@ add_test(function test_change_item_id() {
add_test(function test_create() {
_("Ensure creating/installing an add-on from a record works.");
// Set this so that getInstallFromSearchResult doesn't end up
// failing the install due to an insecure source URI scheme.
Svc.Prefs.set("addons.ignoreRepositoryChecking", true);
let server = createAndStartHTTPServer(HTTP_PORT);
let addon = installAddon("test_bootstrap1_1");
@ -379,7 +371,6 @@ add_test(function test_create() {
uninstallAddon(newAddon);
Svc.Prefs.reset("addons.ignoreRepositoryChecking");
server.stop(run_next_test);
});
@ -416,7 +407,16 @@ add_test(function test_create_bad_install() {
let failed = store.applyIncomingBatch([record]);
// This addon had no source URI so was skipped - but it's not treated as
// failure.
do_check_eq(0, failed.length);
// XXX - this test isn't testing what we thought it was. Previously the addon
// was not being installed due to requireSecureURL checking *before* we'd
// attempted to get the XPI.
// With requireSecureURL disabled we do see a download failure, but the addon
// *does* get added to |failed|.
// FTR: onDownloadFailed() is called with ERROR_NETWORK_FAILURE, so it's going
// to be tricky to distinguish a 404 from other transient network errors
// where we do want the addon to end up in |failed|.
// This is being tracked in bug 1284778.
//do_check_eq(0, failed.length);
let addon = getAddonFromAddonManagerByID(id);
do_check_eq(null, addon);
@ -429,14 +429,11 @@ add_test(function test_wipe() {
let addon1 = installAddon("test_bootstrap1_1");
Svc.Prefs.set("addons.ignoreRepositoryChecking", true);
store.wipe();
let addon = getAddonFromAddonManagerByID(addon1.id);
do_check_eq(null, addon);
Svc.Prefs.reset("addons.ignoreRepositoryChecking");
run_next_test();
});
@ -451,7 +448,6 @@ add_test(function test_wipe_and_install() {
let record = createRecordForThisApp(installed.syncGUID, installed.id, true,
false);
Svc.Prefs.set("addons.ignoreRepositoryChecking", true);
store.wipe();
let deleted = getAddonFromAddonManagerByID(installed.id);
@ -465,7 +461,6 @@ add_test(function test_wipe_and_install() {
let fetched = getAddonFromAddonManagerByID(record.addonID);
do_check_true(!!fetched);
Svc.Prefs.reset("addons.ignoreRepositoryChecking");
server.stop(run_next_test);
});

View File

@ -11,7 +11,6 @@ Cu.import("resource://services-sync/util.js");
loadAddonTestFunctions();
startupManager();
Svc.Prefs.set("addons.ignoreRepositoryChecking", true);
Svc.Prefs.set("engine.addons", true);
Service.engineManager.register(AddonsEngine);

View File

@ -65,10 +65,12 @@ class TPSTestRunner(object):
# Allow installing extensions dropped into the profile folder
'extensions.autoDisableScopes': 10,
'extensions.getAddons.get.url': 'http://127.0.0.1:4567/addons/api/%IDS%.xml',
# Our pretend addons server doesn't support metadata...
'extensions.getAddons.cache.enabled': False,
'extensions.install.requireSecureOrigin': False,
'extensions.update.enabled': False,
# Don't open a dialog to show available add-on updates
'extensions.update.notifyUser': False,
'services.sync.addons.ignoreRepositoryChecking': True,
'services.sync.firstSync': 'notReady',
'services.sync.lastversion': '1.0',
'toolkit.startup.max_resumed_crashes': -1,