From 6d6d74aa1448a4750737412817be5f614c5c254e Mon Sep 17 00:00:00 2001 From: Mark Banner Date: Fri, 13 Aug 2021 19:24:20 +0000 Subject: [PATCH] Bug 1724249 - Don't add interactions if they are part of the adult filter list. r=amy Differential Revision: https://phabricator.services.mozilla.com/D121892 --- .../components/newtab/lib/ActivityStream.jsm | 7 -- browser/components/newtab/lib/FilterAdult.jsm | 80 ++++++++++++++-- .../components/newtab/lib/HighlightsFeed.jsm | 6 +- .../components/newtab/lib/TopSitesFeed.jsm | 6 +- .../newtab/test/unit/lib/FilterAdult.test.js | 96 +++++++++++++++---- .../test/unit/lib/HighlightsFeed.test.js | 20 ++-- .../newtab/test/unit/lib/TopSitesFeed.test.js | 20 ++-- .../places/InteractionsBlocklist.jsm | 5 + .../browser_interactions_blocklist.js | 11 ++- 9 files changed, 182 insertions(+), 69 deletions(-) diff --git a/browser/components/newtab/lib/ActivityStream.jsm b/browser/components/newtab/lib/ActivityStream.jsm index 0f9162352d11..31b231191c14 100644 --- a/browser/components/newtab/lib/ActivityStream.jsm +++ b/browser/components/newtab/lib/ActivityStream.jsm @@ -202,13 +202,6 @@ const PREFS_CONFIG = new Map([ }), }, ], - [ - "filterAdult", - { - title: "Remove adult pages from sites, highlights, etc.", - value: true, - }, - ], [ "showSearch", { diff --git a/browser/components/newtab/lib/FilterAdult.jsm b/browser/components/newtab/lib/FilterAdult.jsm index 52a21dfba80b..043dea68cf80 100644 --- a/browser/components/newtab/lib/FilterAdult.jsm +++ b/browser/components/newtab/lib/FilterAdult.jsm @@ -3,12 +3,23 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ "use strict"; +const { XPCOMUtils } = ChromeUtils.import( + "resource://gre/modules/XPCOMUtils.jsm" +); + ChromeUtils.defineModuleGetter( this, "Services", "resource://gre/modules/Services.jsm" ); +XPCOMUtils.defineLazyPreferenceGetter( + this, + "gFilterAdultEnabled", + "browser.newtabpage.activity-stream.filterAdult", + true +); + // Keep a Set of adult base domains for lookup (initialized at end of file) let gAdultSet; @@ -38,21 +49,70 @@ function md5Hash(text) { return gCryptoHash.finish(true); } -/** - * Filter out any link objects that have a url with an adult base domain. - */ -function filterAdult(links) { - return links.filter(({ url }) => { +const FilterAdult = { + /** + * Filter out any link objects that have a url with an adult base domain. + * + * @param {string[]} links + * An array of links to test. + * @returns {string[]} + * A filtered array without adult links. + */ + filter(links) { + if (!gFilterAdultEnabled) { + return links; + } + + return links.filter(({ url }) => { + try { + const uri = Services.io.newURI(url); + return !gAdultSet.has(md5Hash(Services.eTLD.getBaseDomain(uri))); + } catch (ex) { + return true; + } + }); + }, + + /** + * Determine if the supplied url is an adult url or not. + * + * @param {string} url + * The url to test. + * @returns {boolean} + * True if it is an adult url. + */ + isAdultUrl(url) { + if (!gFilterAdultEnabled) { + return false; + } try { const uri = Services.io.newURI(url); - return !gAdultSet.has(md5Hash(Services.eTLD.getBaseDomain(uri))); + return gAdultSet.has(md5Hash(Services.eTLD.getBaseDomain(uri))); } catch (ex) { - return true; + return false; } - }); -} + }, -const EXPORTED_SYMBOLS = ["filterAdult"]; + /** + * For tests, adds a domain to the adult list. + */ + addDomainToList(url) { + gAdultSet.add( + md5Hash(Services.eTLD.getBaseDomain(Services.io.newURI(url))) + ); + }, + + /** + * For tests, removes a domain to the adult list. + */ + removeDomainFromList(url) { + gAdultSet.delete( + md5Hash(Services.eTLD.getBaseDomain(Services.io.newURI(url))) + ); + }, +}; + +const EXPORTED_SYMBOLS = ["FilterAdult"]; // These are md5 hashes of base domains to be filtered out. Originally from: // https://hg.mozilla.org/mozilla-central/log/default/browser/base/content/newtab/newTab.inadjacent.json diff --git a/browser/components/newtab/lib/HighlightsFeed.jsm b/browser/components/newtab/lib/HighlightsFeed.jsm index 54f83c9f29be..1da7ceb865d1 100644 --- a/browser/components/newtab/lib/HighlightsFeed.jsm +++ b/browser/components/newtab/lib/HighlightsFeed.jsm @@ -25,7 +25,7 @@ const { Dedupe } = ChromeUtils.import( ChromeUtils.defineModuleGetter( this, - "filterAdult", + "FilterAdult", "resource://activity-stream/lib/FilterAdult.jsm" ); ChromeUtils.defineModuleGetter( @@ -217,9 +217,7 @@ this.HighlightsFeed = class HighlightsFeed { const orderedPages = this._orderHighlights(manyPages); // Remove adult highlights if we need to - const checkedAdult = this.store.getState().Prefs.values.filterAdult - ? filterAdult(orderedPages) - : orderedPages; + const checkedAdult = FilterAdult.filter(orderedPages); // Remove any Highlights that are in Top Sites already const [, deduped] = this.dedupe.group( diff --git a/browser/components/newtab/lib/TopSitesFeed.jsm b/browser/components/newtab/lib/TopSitesFeed.jsm index 42262df09a5e..eb7815a82c31 100644 --- a/browser/components/newtab/lib/TopSitesFeed.jsm +++ b/browser/components/newtab/lib/TopSitesFeed.jsm @@ -38,7 +38,7 @@ const { ChromeUtils.defineModuleGetter( this, - "filterAdult", + "FilterAdult", "resource://activity-stream/lib/FilterAdult.jsm" ); ChromeUtils.defineModuleGetter( @@ -810,9 +810,7 @@ this.TopSitesFeed = class TopSitesFeed { const dedupedUnpinned = [...dedupedFrecent, ...dedupedDefaults]; // Remove adult sites if we need to - const checkedAdult = prefValues.filterAdult - ? filterAdult(dedupedUnpinned) - : dedupedUnpinned; + const checkedAdult = FilterAdult.filter(dedupedUnpinned); // Insert the original pinned sites into the deduped frecent and defaults. let withPinned = insertPinned(checkedAdult, pinned); diff --git a/browser/components/newtab/test/unit/lib/FilterAdult.test.js b/browser/components/newtab/test/unit/lib/FilterAdult.test.js index 69590de54e23..e5d15a3fb0ef 100644 --- a/browser/components/newtab/test/unit/lib/FilterAdult.test.js +++ b/browser/components/newtab/test/unit/lib/FilterAdult.test.js @@ -1,7 +1,7 @@ -import { filterAdult } from "lib/FilterAdult.jsm"; +import { FilterAdult } from "lib/FilterAdult.jsm"; import { GlobalOverrider } from "test/unit/utils"; -describe("filterAdult", () => { +describe("FilterAdult", () => { let hashStub; let hashValue; let globals; @@ -20,35 +20,93 @@ describe("filterAdult", () => { }, }, }); + globals.set("gFilterAdultEnabled", true); }); afterEach(() => { + hashValue = ""; globals.restore(); }); - it("should default to include on unexpected urls", () => { - const empty = {}; + describe("filter", () => { + it("should default to include on unexpected urls", () => { + const empty = {}; - const result = filterAdult([empty]); + const result = FilterAdult.filter([empty]); - assert.equal(result.length, 1); - assert.equal(result[0], empty); + assert.equal(result.length, 1); + assert.equal(result[0], empty); + }); + it("should not filter out non-adult urls", () => { + const link = { url: "https://mozilla.org/" }; + + const result = FilterAdult.filter([link]); + + assert.equal(result.length, 1); + assert.equal(result[0], link); + }); + it("should filter out adult urls", () => { + // Use a hash value that is in the adult set + hashValue = "+/UCpAhZhz368iGioEO8aQ=="; + const link = { url: "https://some-adult-site/" }; + + const result = FilterAdult.filter([link]); + + assert.equal(result.length, 0); + }); + it("should not filter out adult urls if the preference is turned off", () => { + // Use a hash value that is in the adult set + hashValue = "+/UCpAhZhz368iGioEO8aQ=="; + globals.set("gFilterAdultEnabled", false); + const link = { url: "https://some-adult-site/" }; + + const result = FilterAdult.filter([link]); + + assert.equal(result.length, 1); + assert.equal(result[0], link); + }); }); - it("should not filter out non-adult urls", () => { - const link = { url: "https://mozilla.org/" }; - const result = filterAdult([link]); + describe("isAdultUrl", () => { + it("should default to false on unexpected urls", () => { + const result = FilterAdult.isAdultUrl(""); - assert.equal(result.length, 1); - assert.equal(result[0], link); - }); - it("should filter out adult urls", () => { - // Use a hash value that is in the adult set - hashValue = "+/UCpAhZhz368iGioEO8aQ=="; - const link = { url: "https://some-adult-site/" }; + assert.equal(result, false); + }); + it("should return false for non-adult urls", () => { + const result = FilterAdult.isAdultUrl("https://mozilla.org/"); - const result = filterAdult([link]); + assert.equal(result, false); + }); + it("should return true for adult urls", () => { + // Use a hash value that is in the adult set + hashValue = "+/UCpAhZhz368iGioEO8aQ=="; + const result = FilterAdult.isAdultUrl("https://some-adult-site/"); - assert.equal(result.length, 0); + assert.equal(result, true); + }); + it("should return false for adult urls when the preference is turned off", () => { + // Use a hash value that is in the adult set + hashValue = "+/UCpAhZhz368iGioEO8aQ=="; + globals.set("gFilterAdultEnabled", false); + const result = FilterAdult.isAdultUrl("https://some-adult-site/"); + + assert.equal(result, false); + }); + + describe("test functions", () => { + it("should add and remove a filter in the adult list", () => { + // Use a hash value that is in the adult set + FilterAdult.addDomainToList("https://some-adult-site/"); + let result = FilterAdult.isAdultUrl("https://some-adult-site/"); + + assert.equal(result, true); + + FilterAdult.removeDomainFromList("https://some-adult-site/"); + result = FilterAdult.isAdultUrl("https://some-adult-site/"); + + assert.equal(result, false); + }); + }); }); }); diff --git a/browser/components/newtab/test/unit/lib/HighlightsFeed.test.js b/browser/components/newtab/test/unit/lib/HighlightsFeed.test.js index 2b93e6e008d2..79f29965269f 100644 --- a/browser/components/newtab/test/unit/lib/HighlightsFeed.test.js +++ b/browser/components/newtab/test/unit/lib/HighlightsFeed.test.js @@ -60,7 +60,9 @@ describe("Highlights Feed", () => { maybeCacheScreenshot: Screenshots.maybeCacheScreenshot, _shouldGetScreenshots: sinon.stub().returns(true), }; - filterAdultStub = sinon.stub().returns([]); + filterAdultStub = { + filter: sinon.stub().returnsArg(0), + }; shortURLStub = sinon .stub() .callsFake(site => site.url.match(/\/([^/]+)/)[1]); @@ -71,6 +73,7 @@ describe("Highlights Feed", () => { globals.set("NewTabUtils", fakeNewTabUtils); globals.set("PageThumbs", fakePageThumbs); + globals.set("gFilterAdultEnabled", false); ({ HighlightsFeed, SECTION_ID, @@ -78,7 +81,7 @@ describe("Highlights Feed", () => { BOOKMARKS_RESTORE_SUCCESS_EVENT, BOOKMARKS_RESTORE_FAILED_EVENT, } = injector({ - "lib/FilterAdult.jsm": { filterAdult: filterAdultStub }, + "lib/FilterAdult.jsm": { FilterAdult: filterAdultStub }, "lib/ShortURL.jsm": { shortURL: shortURLStub }, "lib/SectionsManager.jsm": { SectionsManager: sectionsManagerStub }, "lib/Screenshots.jsm": { Screenshots: fakeScreenshot }, @@ -96,7 +99,6 @@ describe("Highlights Feed", () => { state: { Prefs: { values: { - filterAdult: false, "section.highlights.includePocket": false, "section.highlights.includeDownloads": false, }, @@ -570,18 +572,12 @@ describe("Highlights Feed", () => { assert.propertyVal(highlights[0], "type", "history"); }); - it("should not filter out adult pages when pref is false", async () => { - await feed.fetchHighlights(); - - assert.notCalled(filterAdultStub); - }); - it("should filter out adult pages when pref is true", async () => { - feed.store.state.Prefs.values.filterAdult = true; - + it("should filter out adult pages", async () => { + filterAdultStub.filter = sinon.stub().returns([]); const highlights = await fetchHighlights(); // The stub filters out everything - assert.calledOnce(filterAdultStub); + assert.calledOnce(filterAdultStub.filter); assert.equal(highlights.length, 0); }); it("should not expose internal link properties", async () => { diff --git a/browser/components/newtab/test/unit/lib/TopSitesFeed.test.js b/browser/components/newtab/test/unit/lib/TopSitesFeed.test.js index 75cd4a8642bf..1b63eb0e0834 100644 --- a/browser/components/newtab/test/unit/lib/TopSitesFeed.test.js +++ b/browser/components/newtab/test/unit/lib/TopSitesFeed.test.js @@ -92,7 +92,9 @@ describe("Top Sites Feed", () => { maybeCacheScreenshot: sandbox.spy(Screenshots.maybeCacheScreenshot), _shouldGetScreenshots: sinon.stub().returns(true), }; - filterAdultStub = sinon.stub().returns([]); + filterAdultStub = { + filter: sinon.stub().returnsArg(0), + }; shortURLStub = sinon .stub() .callsFake(site => @@ -105,6 +107,7 @@ describe("Top Sites Feed", () => { }; globals.set("PageThumbs", fakePageThumbs); globals.set("NewTabUtils", fakeNewTabUtils); + globals.set("gFilterAdultEnabled", false); sandbox.spy(global.XPCOMUtils, "defineLazyGetter"); FakePrefs.prototype.prefs["default.sites"] = "https://foo.com/"; ({ TopSitesFeed, DEFAULT_TOP_SITES } = injector({ @@ -115,7 +118,7 @@ describe("Top Sites Feed", () => { TOP_SITES_DEFAULT_ROWS, TOP_SITES_MAX_SITES_PER_ROW, }, - "lib/FilterAdult.jsm": { filterAdult: filterAdultStub }, + "lib/FilterAdult.jsm": { FilterAdult: filterAdultStub }, "lib/Screenshots.jsm": { Screenshots: fakeScreenshot }, "lib/TippyTopProvider.jsm": { TippyTopProvider: FakeTippyTopProvider }, "lib/ShortURL.jsm": { shortURL: shortURLStub }, @@ -138,7 +141,7 @@ describe("Top Sites Feed", () => { return this.state; }, state: { - Prefs: { values: { filterAdult: false, topSitesRows: 2 } }, + Prefs: { values: { topSitesRows: 2 } }, TopSites: { rows: Array(12).fill("site") }, }, dbStorage: { getDbTable: sandbox.stub().returns(storage) }, @@ -260,19 +263,14 @@ describe("Top Sites Feed", () => { assert.propertyVal(result[0], "typedBonus", true); }); - it("should not filter out adult sites when pref is false", async () => { - await feed.getLinksWithDefaults(); - - assert.notCalled(filterAdultStub); - }); - it("should filter out non-pinned adult sites when pref is true", async () => { - feed.store.state.Prefs.values.filterAdult = true; + it("should filter out non-pinned adult sites", async () => { + filterAdultStub.filter = sinon.stub().returns([]); fakeNewTabUtils.pinnedLinks.links = [{ url: "https://foo.com/" }]; const result = await feed.getLinksWithDefaults(); // The stub filters out everything - assert.calledOnce(filterAdultStub); + assert.calledOnce(filterAdultStub.filter); assert.equal(result.length, 1); assert.equal(result[0].url, fakeNewTabUtils.pinnedLinks.links[0].url); }); diff --git a/browser/components/places/InteractionsBlocklist.jsm b/browser/components/places/InteractionsBlocklist.jsm index aeac4f7d1b57..5bf338a73dfe 100644 --- a/browser/components/places/InteractionsBlocklist.jsm +++ b/browser/components/places/InteractionsBlocklist.jsm @@ -11,6 +11,7 @@ const { XPCOMUtils } = ChromeUtils.import( ); XPCOMUtils.defineLazyModuleGetters(this, { + FilterAdult: "resource://activity-stream/lib/FilterAdult.jsm", Services: "resource://gre/modules/Services.jsm", UrlbarUtils: "resource:///modules/UrlbarUtils.jsm", }); @@ -120,6 +121,10 @@ class _InteractionsBlocklist { * True if `url` is on a blocklist. False otherwise. */ isUrlBlocklisted(urlToCheck) { + if (FilterAdult.isAdultUrl(urlToCheck)) { + return true; + } + // First, find the URL's base host: the hostname without any subdomains or a // public suffix. let url; diff --git a/browser/components/places/tests/browser/interactions/browser_interactions_blocklist.js b/browser/components/places/tests/browser/interactions/browser_interactions_blocklist.js index 4b18cc2e6d8e..d905522a5a0e 100644 --- a/browser/components/places/tests/browser/interactions/browser_interactions_blocklist.js +++ b/browser/components/places/tests/browser/interactions/browser_interactions_blocklist.js @@ -5,10 +5,11 @@ * Tests that interactions are not recorded for sites on the blocklist. */ -const ALLOWED_TEST_URL = "https://example.com/"; +const ALLOWED_TEST_URL = "http://mochi.test:8888/"; const BLOCKED_TEST_URL = "https://example.com/browser"; XPCOMUtils.defineLazyModuleGetters(this, { + FilterAdult: "resource://activity-stream/lib/FilterAdult.jsm", InteractionsBlocklist: "resource:///modules/InteractionsBlocklist.jsm", }); @@ -75,7 +76,7 @@ async function loadBlockedUrl(expectRecording) { }); } -add_task(async function test() { +add_task(async function test_regexp() { info("Record BLOCKED_TEST_URL because it is not yet blocklisted."); await loadBlockedUrl(true); @@ -96,3 +97,9 @@ add_task(async function test() { ); await loadBlockedUrl(true); }); + +add_task(async function test_adult() { + FilterAdult.addDomainToList("https://example.com/browser"); + await loadBlockedUrl(false); + FilterAdult.removeDomainFromList("https://example.com/browser"); +});