Bug 1412083 - Avoid syncing preferences that refer to moz-extension or blob urls. r?eoger
MozReview-Commit-ID: 9Ipq5z1dykr
--- a/services/sync/modules/engines/prefs.js
+++ b/services/sync/modules/engines/prefs.js
@@ -67,16 +67,28 @@ PrefsEngine.prototype = {
if (this.justWiped) {
this.justWiped = false;
return true;
}
return SyncEngine.prototype._reconcile.call(this, item);
}
};
+// We don't use services.sync.engine.tabs.filteredUrls since it includes
+// about: pages and the like, which we want to be syncable in preferences.
+// Blob and moz-extension uris are never safe to sync, so we limit our check
+// to those.
+const UNSYNCABLE_URL_REGEXP = /^(moz-extension|blob):/i;
+function isUnsyncableURLPref(prefName) {
+ if (Services.prefs.getPrefType(prefName) != Ci.nsIPrefBranch.PREF_STRING) {
+ return false;
+ }
+ const prefValue = Services.prefs.getStringPref(prefName, "");
+ return UNSYNCABLE_URL_REGEXP.test(prefValue);
+}
function PrefStore(name, engine) {
Store.call(this, name, engine);
Svc.Obs.add("profile-before-change", function() {
this.__prefs = null;
}, this);
}
PrefStore.prototype = {
@@ -87,31 +99,35 @@ PrefStore.prototype = {
if (!this.__prefs) {
this.__prefs = new Preferences();
}
return this.__prefs;
},
_getSyncPrefs() {
let syncPrefs = Services.prefs.getBranch(PREF_SYNC_PREFS_PREFIX)
- .getChildList("", {});
+ .getChildList("", {})
+ .filter(pref => !isUnsyncableURLPref(pref));
// Also sync preferences that determine which prefs get synced.
let controlPrefs = syncPrefs.map(pref => PREF_SYNC_PREFS_PREFIX + pref);
return controlPrefs.concat(syncPrefs);
},
_isSynced(pref) {
return pref.startsWith(PREF_SYNC_PREFS_PREFIX) ||
this._prefs.get(PREF_SYNC_PREFS_PREFIX + pref, false);
},
_getAllPrefs() {
let values = {};
for (let pref of this._getSyncPrefs()) {
- if (this._isSynced(pref)) {
+ // Note: _isSynced doesn't call isUnsyncableURLPref since it would cause
+ // us not to apply (syncable) changes to preferences that are set locally
+ // which have unsyncable urls.
+ if (this._isSynced(pref) && !isUnsyncableURLPref(pref)) {
// Missing and default prefs get the null value.
values[pref] = this._prefs.isSet(pref) ? this._prefs.get(pref, null) : null;
}
}
return values;
},
_updateLightWeightTheme(themeID) {
@@ -131,16 +147,20 @@ PrefStore.prototype = {
// _isSynced returns false when 'foo.pref' doesn't exist (e.g., on a new device).
let prefs = Object.keys(values).sort(a => -a.indexOf(PREF_SYNC_PREFS_PREFIX));
for (let pref of prefs) {
if (!this._isSynced(pref)) {
continue;
}
let value = values[pref];
+ if (typeof value == "string" && UNSYNCABLE_URL_REGEXP.test(value)) {
+ this._log.trace(`Skipping incoming unsyncable url for pref: ${pref}`);
+ continue;
+ }
switch (pref) {
// Some special prefs we don't want to set directly.
case selectedThemeIDPref:
selectedThemeIDAfter = value;
break;
// default is to just set the pref
--- a/services/sync/services-sync.js
+++ b/services/sync/services-sync.js
@@ -25,17 +25,17 @@ pref("services.sync.errorhandler.network
pref("services.sync.engine.addons", true);
pref("services.sync.engine.addresses", false);
pref("services.sync.engine.bookmarks", true);
pref("services.sync.engine.creditcards", false);
pref("services.sync.engine.history", true);
pref("services.sync.engine.passwords", true);
pref("services.sync.engine.prefs", true);
pref("services.sync.engine.tabs", true);
-pref("services.sync.engine.tabs.filteredUrls", "^(about:.*|resource:.*|chrome:.*|wyciwyg:.*|file:.*|blob:.*)$");
+pref("services.sync.engine.tabs.filteredUrls", "^(about:.*|resource:.*|chrome:.*|wyciwyg:.*|file:.*|blob:.*|moz-extension:.*)$");
// The addresses and CC engines might not actually be available at all.
pref("services.sync.engine.addresses.available", false);
pref("services.sync.engine.creditcards.available", 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.
--- a/services/sync/tests/unit/prefs_test_prefs_store.js
+++ b/services/sync/tests/unit/prefs_test_prefs_store.js
@@ -7,21 +7,30 @@
pref("services.sync.prefs.sync.testing.int", true);
pref("services.sync.prefs.sync.testing.string", true);
pref("services.sync.prefs.sync.testing.bool", true);
pref("services.sync.prefs.sync.testing.dont.change", true);
// this one is a user pref, so it *will* sync.
user_pref("services.sync.prefs.sync.testing.turned.off", false);
pref("services.sync.prefs.sync.testing.nonexistent", true);
pref("services.sync.prefs.sync.testing.default", true);
+pref("services.sync.prefs.sync.testing.synced.url", true);
+// We shouldn't sync the URL, or the flag that says we should sync the pref
+// (otherwise some other client might overwrite our local value).
+user_pref("services.sync.prefs.sync.testing.unsynced.url", true);
// The preference values - these are all user_prefs, otherwise their value
// will not be synced.
user_pref("testing.int", 123);
user_pref("testing.string", "ohai");
user_pref("testing.bool", true);
user_pref("testing.dont.change", "Please don't change me.");
user_pref("testing.turned.off", "I won't get synced.");
user_pref("testing.not.turned.on", "I won't get synced either!");
+// Some url we don't want to sync
+user_pref("testing.unsynced.url", "moz-extension://d5d31b00-b944-4afb-bd3d-d0326551a0ae");
+user_pref("testing.synced.url", "https://www.example.com");
// A pref that exists but still has the default value - will be synced with
// null as the value.
pref("testing.default", "I'm the default value");
+
+// A pref that shouldn't be synced
--- a/services/sync/tests/unit/test_prefs_store.js
+++ b/services/sync/tests/unit/test_prefs_store.js
@@ -52,45 +52,60 @@ add_task(async function run_test() {
Assert.equal(record.value["testing.string"], "ohai");
Assert.equal(record.value["testing.bool"], true);
// non-existing prefs get null as the value
Assert.equal(record.value["testing.nonexistent"], null);
// as do prefs that have a default value.
Assert.equal(record.value["testing.default"], null);
Assert.equal(false, "testing.turned.off" in record.value);
Assert.equal(false, "testing.not.turned.on" in record.value);
+ // Prefs we consider unsyncable (since they are URLs that won't be stable on
+ // another firefox) shouldn't be included
+ Assert.equal(record.value["testing.unsynced.url"], null);
+ // Other URLs with user prefs should be synced, though.
+ Assert.equal(record.value["testing.synced.url"], "https://www.example.com");
_("Prefs record contains non-default pref sync prefs too.");
Assert.equal(record.value["services.sync.prefs.sync.testing.int"], null);
Assert.equal(record.value["services.sync.prefs.sync.testing.string"], null);
Assert.equal(record.value["services.sync.prefs.sync.testing.bool"], null);
Assert.equal(record.value["services.sync.prefs.sync.testing.dont.change"], null);
+ Assert.equal(record.value["services.sync.prefs.sync.testing.synced.url"], null);
// but this one is a user_pref so *will* be synced.
Assert.equal(record.value["services.sync.prefs.sync.testing.turned.off"], false);
Assert.equal(record.value["services.sync.prefs.sync.testing.nonexistent"], null);
Assert.equal(record.value["services.sync.prefs.sync.testing.default"], null);
+ // This is a user pref, but shouldn't be synced since it refers to an invalid url
+ Assert.equal(record.value["services.sync.prefs.sync.testing.unsynced.url"], null);
_("Update some prefs, including one that's to be reset/deleted.");
Svc.Prefs.set("testing.deleteme", "I'm going to be deleted!");
record = new PrefRec("prefs", PREFS_GUID);
record.value = {
"testing.int": 42,
"testing.string": "im in ur prefs",
"testing.bool": false,
"testing.deleteme": null,
"testing.somepref": "im a new pref from other device",
- "services.sync.prefs.sync.testing.somepref": true
+ "services.sync.prefs.sync.testing.somepref": true,
+ // Pretend some a stale remote client is overwriting it with a value
+ // we consider unsyncable.
+ "testing.synced.url": "blob:ebeb707a-502e-40c6-97a5-dd4bda901463",
+ // Make sure we can replace the unsynced URL with a valid URL.
+ "testing.unsynced.url": "https://www.example.com/2",
};
await store.update(record);
Assert.equal(prefs.get("testing.int"), 42);
Assert.equal(prefs.get("testing.string"), "im in ur prefs");
Assert.equal(prefs.get("testing.bool"), false);
Assert.equal(prefs.get("testing.deleteme"), undefined);
Assert.equal(prefs.get("testing.dont.change"), "Please don't change me.");
Assert.equal(prefs.get("testing.somepref"), "im a new pref from other device");
+ Assert.equal(prefs.get("testing.synced.url"), "https://www.example.com");
+ Assert.equal(prefs.get("testing.unsynced.url"), "https://www.example.com/2");
Assert.equal(Svc.Prefs.get("prefs.sync.testing.somepref"), true);
_("Enable persona");
// Ensure we don't go to the network to fetch personas and end up leaking
// stuff.
Services.io.offline = true;
Assert.ok(!prefs.get("lightweightThemes.selectedThemeID"));
Assert.equal(LightweightThemeManager.currentTheme, null);