diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index 8f45c419651f..482c00d6d1cf 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -4204,11 +4204,7 @@ function openHomeDialog(aURL) { ); if (pressedVal == 0) { - try { - HomePage.set(aURL); - } catch (ex) { - dump("Failed to set the home page.\n" + ex + "\n"); - } + HomePage.set(aURL).catch(Cu.reportError); } } diff --git a/browser/components/BrowserGlue.jsm b/browser/components/BrowserGlue.jsm index 7b3c84389b25..9a1dfb8c4428 100644 --- a/browser/components/BrowserGlue.jsm +++ b/browser/components/BrowserGlue.jsm @@ -3033,7 +3033,7 @@ BrowserGlue.prototype = { HomePage.reset(); } else { value = updated; - HomePage.set(value); + HomePage.safeSet(value); } } } diff --git a/browser/components/extensions/parent/ext-chrome-settings-overrides.js b/browser/components/extensions/parent/ext-chrome-settings-overrides.js index b34a9943813e..dc8064200e23 100644 --- a/browser/components/extensions/parent/ext-chrome-settings-overrides.js +++ b/browser/components/extensions/parent/ext-chrome-settings-overrides.js @@ -21,6 +21,11 @@ ChromeUtils.defineModuleGetter( "ExtensionControlledPopup", "resource:///modules/ExtensionControlledPopup.jsm" ); +ChromeUtils.defineModuleGetter( + this, + "HomePage", + "resource:///modules/HomePage.jsm" +); const DEFAULT_SEARCH_STORE_TYPE = "default_search"; const DEFAULT_SEARCH_SETTING_NAME = "defaultSearch"; @@ -226,8 +231,11 @@ this.chrome_settings_overrides = class extends ExtensionAPI { await ExtensionSettingsStore.initialize(); let homepageUrl = manifest.chrome_settings_overrides.homepage; + const ignoreHomePageUrl = + homepageUrl && (await HomePage.shouldIgnore(homepageUrl)); - if (homepageUrl) { + // If this is a page we ignore, just skip the homepage setting completely. + if (homepageUrl && !ignoreHomePageUrl) { let inControl; if ( extension.startupReason == "ADDON_INSTALL" || diff --git a/browser/components/extensions/test/browser/browser_ext_chrome_settings_overrides_home.js b/browser/components/extensions/test/browser/browser_ext_chrome_settings_overrides_home.js index 6d596bf3e5b7..1fe24e8423c6 100644 --- a/browser/components/extensions/test/browser/browser_ext_chrome_settings_overrides_home.js +++ b/browser/components/extensions/test/browser/browser_ext_chrome_settings_overrides_home.js @@ -13,6 +13,11 @@ ChromeUtils.defineModuleGetter( "ExtensionSettingsStore", "resource://gre/modules/ExtensionSettingsStore.jsm" ); +ChromeUtils.defineModuleGetter( + this, + "HomePage", + "resource:///modules/HomePage.jsm" +); // Named this way so they correspond to the extensions const HOME_URI_2 = "http://example.com/"; @@ -24,6 +29,8 @@ const CONTROLLED_BY_OTHER = "controlled_by_other_extensions"; const NOT_CONTROLLABLE = "not_controllable"; const HOMEPAGE_URL_PREF = "browser.startup.homepage"; +const HOMEPAGE_EXTENSION_CONTROLLED = + "browser.startup.homepage_override.extensionControlled"; const getHomePageURL = () => { return Services.prefs.getStringPref(HOMEPAGE_URL_PREF); @@ -610,3 +617,31 @@ add_task(async function test_overriding_home_page_incognito_external() { await extension.unload(); await BrowserTestUtils.closeWindow(win); }); + +add_task(async function test_overriding_with_ignored_url() { + // Manually poke into the ignore list a value to be ignored. + HomePage._ignoreList.push("ignore=me"); + Services.prefs.setBoolPref(HOMEPAGE_EXTENSION_CONTROLLED, false); + + let extension = ExtensionTestUtils.loadExtension({ + manifest: { + chrome_settings_overrides: { homepage: "https://example.com/?ignore=me" }, + name: "extension", + }, + useAddonManager: "temporary", + }); + + await extension.startup(); + + ok(HomePage.isDefault, "Should still have the default homepage"); + is( + Services.prefs.getBoolPref( + "browser.startup.homepage_override.extensionControlled" + ), + false, + "Should not be extension controlled." + ); + + await extension.unload(); + HomePage._ignoreList.pop(); +}); diff --git a/browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js b/browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js index 892570536522..0b9363fd4e61 100644 --- a/browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js +++ b/browser/components/extensions/test/xpcshell/test_ext_chrome_settings_overrides_update.js @@ -5,7 +5,11 @@ const { AddonTestUtils } = ChromeUtils.import( "resource://testing-common/AddonTestUtils.jsm" ); -const { HomePage } = ChromeUtils.import("resource:///modules/HomePage.jsm"); +XPCOMUtils.defineLazyModuleGetters(this, { + HomePage: "resource:///modules/HomePage.jsm", + RemoteSettings: "resource://services-settings/remote-settings.js", + sinon: "resource://testing-common/Sinon.jsm", +}); AddonTestUtils.init(this); AddonTestUtils.overrideCertDB(); @@ -17,8 +21,20 @@ AddonTestUtils.createAppInfo( "42" ); +async function setupRemoteSettings() { + const settings = await RemoteSettings("hijack-blocklists"); + sinon.stub(settings, "get").returns([ + { + id: "homepage-urls", + matches: ["ignore=me"], + _status: "synced", + }, + ]); +} + add_task(async function setup() { await AddonTestUtils.promiseStartupManager(); + await setupRemoteSettings(); }); add_task(async function test_overrides_update_removal() { diff --git a/browser/components/preferences/in-content/home.js b/browser/components/preferences/in-content/home.js index f8b8036334c1..6fb9c880d357 100644 --- a/browser/components/preferences/in-content/home.js +++ b/browser/components/preferences/in-content/home.js @@ -314,7 +314,7 @@ var gHomePane = { break; case this.HOME_MODE_BLANK: if (HomePage.get() !== "about:blank") { - HomePage.set("about:blank"); + HomePage.safeSet("about:blank"); } else { this._renderCustomSettings({ shouldShow: false }); } @@ -370,7 +370,7 @@ var gHomePane = { // FIXME Bug 244192: using dangerous "|" joiner! if (tabs.length) { - HomePage.set(tabs.map(getTabURI).join("|")); + HomePage.set(tabs.map(getTabURI).join("|")).catch(Cu.reportError); } Services.telemetry.scalarAdd("preferences.use_current_page", 1); @@ -382,7 +382,7 @@ var gHomePane = { } if (rv.urls && rv.names) { // XXX still using dangerous "|" joiner! - HomePage.set(rv.urls.join("|")); + HomePage.set(rv.urls.join("|")).catch(Cu.reportError); } }, @@ -429,7 +429,7 @@ var gHomePane = { onCustomHomePageChange(event) { const value = event.target.value || HomePage.getDefault(); - HomePage.set(value); + HomePage.set(value).catch(Cu.reportError); }, /** diff --git a/browser/components/preferences/in-content/main.js b/browser/components/preferences/in-content/main.js index 33d8d9ab1d5e..2804490c81c0 100644 --- a/browser/components/preferences/in-content/main.js +++ b/browser/components/preferences/in-content/main.js @@ -1077,7 +1077,7 @@ var gMainPane = { if (value) { // We need to restore the blank homepage setting in our other pref if (startupPref.value === this.STARTUP_PREF_BLANK) { - HomePage.set("about:blank"); + HomePage.safeSet("about:blank"); } newValue = this.STARTUP_PREF_RESTORE_SESSION; let warnOnQuitPref = Preferences.get("browser.sessionstore.warnOnQuit"); diff --git a/browser/components/preferences/in-content/tests/browser_homepages_filter_aboutpreferences.js b/browser/components/preferences/in-content/tests/browser_homepages_filter_aboutpreferences.js index 56e577cfde97..62788ca03b47 100644 --- a/browser/components/preferences/in-content/tests/browser_homepages_filter_aboutpreferences.js +++ b/browser/components/preferences/in-content/tests/browser_homepages_filter_aboutpreferences.js @@ -20,13 +20,16 @@ add_task(async function testSetHomepageUseCurrent() { useCurrent.click(); is(gBrowser.tabs.length, 3, "Three tabs should be open"); + await TestUtils.waitForCondition( + () => HomePage.get() == "about:blank|about:home" + ); is( HomePage.get(), "about:blank|about:home", "about:blank and about:home should be the only homepages set" ); - HomePage.set(oldHomepage); + HomePage.safeSet(oldHomepage); BrowserTestUtils.removeTab(gBrowser.selectedTab); BrowserTestUtils.removeTab(gBrowser.selectedTab); }); diff --git a/browser/components/preferences/in-content/tests/browser_homepages_use_bookmark.js b/browser/components/preferences/in-content/tests/browser_homepages_use_bookmark.js index 940fa7c28c16..8d3570feacbb 100644 --- a/browser/components/preferences/in-content/tests/browser_homepages_use_bookmark.js +++ b/browser/components/preferences/in-content/tests/browser_homepages_use_bookmark.js @@ -44,8 +44,10 @@ add_task(async function testSetHomepageFromBookmark() { dialog.document.getElementById("bookmarks").selectItems([bm.guid]); dialog.document.documentElement.getButton("accept").click(); + await TestUtils.waitForCondition(() => HomePage.get() == TEST_URL1); + Assert.equal( - Services.prefs.getCharPref("browser.startup.homepage"), + HomePage.get(), TEST_URL1, "Should have set the homepage to the same as the bookmark." ); @@ -74,8 +76,12 @@ add_task(async function testSetHomepageFromTopLevelFolder() { .selectItems([PlacesUtils.bookmarks.menuGuid]); dialog.document.documentElement.getButton("accept").click(); + await TestUtils.waitForCondition( + () => HomePage.get() == `${TEST_URL1}|${TEST_URL2}` + ); + Assert.equal( - Services.prefs.getCharPref("browser.startup.homepage"), + HomePage.get(), `${TEST_URL1}|${TEST_URL2}`, "Should have set the homepage to the same as the bookmark." ); diff --git a/browser/modules/HomePage.jsm b/browser/modules/HomePage.jsm index ffe255ae9b9a..d0e0fe94104b 100644 --- a/browser/modules/HomePage.jsm +++ b/browser/modules/HomePage.jsm @@ -167,7 +167,30 @@ let HomePage = { * The new value to set the preference to. This could be a single url, or a * `|` separated list of URLs. */ - set(value) { + async set(value) { + await this.init(); + + if (await this.shouldIgnore(value)) { + Cu.reportError( + `Ignoring homepage setting for ${value} as it is on the ignore list.` + ); + return false; + } + Services.prefs.setStringPref(kPrefName, value); + return true; + }, + + /** + * Sets the homepage preference to a new page. This is an synchronous version + * that should only be used when we know the source is safe as it bypasses the + * ignore list, e.g. when setting directly to about:blank or a value not + * supplied externally. + * + * @param {string} value + * The new value to set the preference to. This could be a single url, or a + * `|` separated list of URLs. + */ + safeSet(value) { Services.prefs.setStringPref(kPrefName, value); }, @@ -186,6 +209,21 @@ let HomePage = { Services.prefs.setStringPref(kPrefName, kDefaultHomePage); }, + /** + * Determines if a url should be ignored according to the ignore list. + * + * @param {string} url + * A string that is the url or urls to be ignored. + * @returns {boolean} + * True if the url should be ignored. + */ + async shouldIgnore(url) { + await this.init(); + + const lowerURL = url.toLowerCase(); + return this._ignoreList.some(code => lowerURL.includes(code)); + }, + /** * Handles updates of the ignore list, checking the existing preference and * correcting it as necessary. @@ -193,7 +231,7 @@ let HomePage = { * @param {Object} eventData * The event data as received from RemoteSettings. */ - _handleIgnoreListUpdated({ data: { current } }) { + async _handleIgnoreListUpdated({ data: { current } }) { for (const entry of current) { if (entry.id == kHomePageIgnoreListId) { this._ignoreList = [...entry.matches]; diff --git a/browser/modules/test/unit/test_HomePage.js b/browser/modules/test/unit/test_HomePage.js index 49efc71a5f30..cbe736233bb6 100644 --- a/browser/modules/test/unit/test_HomePage.js +++ b/browser/modules/test/unit/test_HomePage.js @@ -42,7 +42,7 @@ add_task(function test_HomePage() { "Homepage should not be overriden by default." ); let newvalue = "about:blank|about:newtab"; - HomePage.set(newvalue); + HomePage.safeSet(newvalue); Assert.ok(HomePage.overridden, "Homepage should be overriden after set()"); Assert.equal(HomePage.get(), newvalue, "Homepage should be ${newvalue}"); Assert.notEqual( @@ -89,49 +89,3 @@ add_task(function test_recoverEmptyHomepage() { Assert.equal(HomePage.get(), HomePage.getDefault(), "Recover is default"); Assert.ok(!HomePage.overridden, "Recover should have set default"); }); - -add_task(async function test_initWithIgnoredPageCausesReset() { - HomePage.set("http://bad/?ignore=me"); - Assert.ok(HomePage.overridden, "Should have overriden the homepage"); - - await HomePage.init(); - - Assert.ok( - !HomePage.overridden, - "Should no longer be overriding the homepage." - ); - Assert.equal( - HomePage.get(), - HomePage.getDefault(), - "Should have reset to the default preference" - ); -}); - -add_task(async function test_updateIgnoreListCausesReset() { - HomePage.set("http://bad/?new=ignore"); - Assert.ok(HomePage.overridden, "Should have overriden the homepage"); - - // Simulate an ignore list update. - await RemoteSettings("hijack-blocklists").emit("sync", { - data: { - current: [ - { - id: HOMEPAGE_IGNORELIST, - schema: 1553857697843, - last_modified: 1553859483588, - matches: ["ignore=me", "new=ignore"], - }, - ], - }, - }); - - Assert.ok( - !HomePage.overridden, - "Should no longer be overriding the homepage." - ); - Assert.equal( - HomePage.get(), - HomePage.getDefault(), - "Should have reset to the default preference" - ); -}); diff --git a/browser/modules/test/unit/test_HomePage_ignore.js b/browser/modules/test/unit/test_HomePage_ignore.js new file mode 100644 index 000000000000..de22058f4c06 --- /dev/null +++ b/browser/modules/test/unit/test_HomePage_ignore.js @@ -0,0 +1,104 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ + */ + +"use strict"; + +const { XPCOMUtils } = ChromeUtils.import( + "resource://gre/modules/XPCOMUtils.jsm" +); + +XPCOMUtils.defineLazyModuleGetters(this, { + HomePage: "resource:///modules/HomePage.jsm", + Services: "resource://gre/modules/Services.jsm", + RemoteSettings: "resource://services-settings/remote-settings.js", + sinon: "resource://testing-common/Sinon.jsm", +}); + +const HOMEPAGE_IGNORELIST = "homepage-urls"; + +/** + * Provides a basic set of remote settings for use in tests. + */ +async function setupRemoteSettings() { + const settings = await RemoteSettings("hijack-blocklists"); + sinon.stub(settings, "get").returns([ + { + id: HOMEPAGE_IGNORELIST, + matches: ["ignore=me"], + _status: "synced", + }, + ]); +} + +add_task(async function setup() { + await setupRemoteSettings(); +}); + +add_task(async function test_initWithIgnoredPageCausesReset() { + // Set the preference direct as the set() would block us. + Services.prefs.setStringPref( + "browser.startup.homepage", + "http://bad/?ignore=me" + ); + Assert.ok(HomePage.overridden, "Should have overriden the homepage"); + + await HomePage.init(); + + Assert.ok( + !HomePage.overridden, + "Should no longer be overriding the homepage." + ); + Assert.equal( + HomePage.get(), + HomePage.getDefault(), + "Should have reset to the default preference" + ); +}); + +add_task(async function test_updateIgnoreListCausesReset() { + Services.prefs.setStringPref( + "browser.startup.homepage", + "http://bad/?new=ignore" + ); + Assert.ok(HomePage.overridden, "Should have overriden the homepage"); + + // Simulate an ignore list update. + await RemoteSettings("hijack-blocklists").emit("sync", { + data: { + current: [ + { + id: HOMEPAGE_IGNORELIST, + schema: 1553857697843, + last_modified: 1553859483588, + matches: ["ignore=me", "new=ignore"], + }, + ], + }, + }); + + Assert.ok( + !HomePage.overridden, + "Should no longer be overriding the homepage." + ); + Assert.equal( + HomePage.get(), + HomePage.getDefault(), + "Should have reset to the default preference" + ); +}); + +add_task(async function test_setIgnoredUrl() { + Assert.ok(!HomePage.overriden, "Should not be overriding the homepage"); + + await HomePage.set("http://bad/?ignore=me"); + + Assert.equal( + HomePage.get(), + HomePage.getDefault(), + "Should still have the default homepage." + ); + Assert.ok(!HomePage.overriden, "Should not be overriding the homepage."); +}); + +// Also need an integration mochitest with an extension (if we can). diff --git a/browser/modules/test/unit/xpcshell.ini b/browser/modules/test/unit/xpcshell.ini index 75957a037d1c..4ba599fb4112 100644 --- a/browser/modules/test/unit/xpcshell.ini +++ b/browser/modules/test/unit/xpcshell.ini @@ -5,6 +5,7 @@ skip-if = toolkit == 'android' [test_E10SUtils_nested_URIs.js] [test_HomePage.js] +[test_HomePage_ignore.js] [test_LiveBookmarkMigrator.js] [test_Sanitizer_interrupted.js] [test_SitePermissions.js]