From 1c8109847b344cf71d945762f19d8b77fac41067 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Thu, 9 Sep 2021 17:25:46 +0700 Subject: [PATCH 1/7] Replace isFirstParty() in content_cosmetic.ts to a native function --- .../renderer/cosmetic_filters_js_handler.cc | 13 +++++++ .../renderer/cosmetic_filters_js_handler.h | 1 + .../resources/data/content_cosmetic.ts | 36 ++----------------- 3 files changed, 17 insertions(+), 33 deletions(-) diff --git a/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.cc b/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.cc index 5d49b55a0e3..6034c4db18c 100644 --- a/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.cc +++ b/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.cc @@ -212,6 +212,15 @@ void CosmeticFiltersJSHandler::HiddenClassIdSelectors( base::Unretained(this))); } +bool CosmeticFiltersJSHandler::OnIsFirstParty(const std::string& url_string) { + const auto url = GURL(url_string); + if (!url.is_valid()) + return false; + + return net::registry_controlled_domains::SameDomainOrHost( + url, url_, net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES); +} + void CosmeticFiltersJSHandler::AddJavaScriptObjectToFrame( v8::Local context) { v8::Isolate* isolate = blink::MainThreadIsolate(); @@ -249,6 +258,10 @@ void CosmeticFiltersJSHandler::BindFunctionsToObject( isolate, javascript_object, "hiddenClassIdSelectors", base::BindRepeating(&CosmeticFiltersJSHandler::HiddenClassIdSelectors, base::Unretained(this))); + BindFunctionToObject( + isolate, javascript_object, "isFirstPartyUrl", + base::BindRepeating(&CosmeticFiltersJSHandler::OnIsFirstParty, + base::Unretained(this))); } template diff --git a/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.h b/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.h index 3441ae33ea0..5f949334ba4 100644 --- a/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.h +++ b/components/cosmetic_filters/renderer/cosmetic_filters_js_handler.h @@ -60,6 +60,7 @@ class CosmeticFiltersJSHandler { void OnUrlCosmeticResources(base::OnceClosure callback, base::Value result); void CSSRulesRoutine(base::DictionaryValue* resources_dict); void OnHiddenClassIdSelectors(base::Value result); + bool OnIsFirstParty(const std::string& url_string); content::RenderFrame* render_frame_; mojo::Remote diff --git a/components/cosmetic_filters/resources/data/content_cosmetic.ts b/components/cosmetic_filters/resources/data/content_cosmetic.ts index e3496faf0f2..24c5af61699 100644 --- a/components/cosmetic_filters/resources/data/content_cosmetic.ts +++ b/components/cosmetic_filters/resources/data/content_cosmetic.ts @@ -9,7 +9,6 @@ // - for cosmetic filters work with CSS and stylesheet. That work itself // could call the script several times. -const { parseDomain, ParseResultType } = require('parse-domain') // Start looking for things to unhide before at most this long after // the backend script is up and connected (eg backgroundReady = true), // or sooner if the thread is idle. @@ -160,43 +159,14 @@ const handleMutations: MutationCallback = (mutations: MutationRecord[]) => { fetchNewClassIdRules() } -const _parseDomainCache = Object.create(null) -const getParsedDomain = (aDomain: string) => { - const cacheResult = _parseDomainCache[aDomain] - if (cacheResult !== undefined) { - return cacheResult - } - - const newResult = parseDomain(aDomain) - _parseDomainCache[aDomain] = newResult - return newResult -} - -const _parsedCurrentDomain = getParsedDomain(window.location.host) const isFirstPartyUrl = (url: string): boolean => { if (isRelativeUrl(url)) { return true } - const parsedTargetDomain = getParsedDomain(url) - - if (parsedTargetDomain.type !== _parsedCurrentDomain.type) { - return false - } - - if (parsedTargetDomain.type === ParseResultType.Listed) { - const isSameEtldP1 = (_parsedCurrentDomain.icann.topLevelDomains === parsedTargetDomain.icann.topLevelDomains && - _parsedCurrentDomain.icann.domain === parsedTargetDomain.icann.domain) - return isSameEtldP1 - } - - const looksLikePrivateOrigin = - [ParseResultType.NotListed, ParseResultType.Ip, ParseResultType.Reserved].includes(parsedTargetDomain.type) - if (looksLikePrivateOrigin) { - return _parsedCurrentDomain.hostname === parsedTargetDomain.hostname - } - - return false + // Callback to c++ renderer process + // @ts-ignore + return cf_worker.isFirstPartyUrl(url) } const stripChildTagsFromText = (elm: HTMLElement, tagName: string, text: string): string => { From a5ce93dde0e1b4cd7afbad0ff719d8ca905f5c34 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Fri, 10 Sep 2021 13:22:30 +0700 Subject: [PATCH 2/7] Enable and improve AdBlockServiceTest.CosmeticFilteringProtect1p --- .../ad_block_service_browsertest.cc | 30 ++++++++++++++----- test/data/cosmetic_filtering.html | 25 +++++++++++++++- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/browser/brave_shields/ad_block_service_browsertest.cc b/browser/brave_shields/ad_block_service_browsertest.cc index 315ee3080f8..4468ce6e100 100644 --- a/browser/brave_shields/ad_block_service_browsertest.cc +++ b/browser/brave_shields/ad_block_service_browsertest.cc @@ -1712,22 +1712,36 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringSimple) { } // Test cosmetic filtering ignores content determined to be 1st party -// This is disabled due to https://github.com/brave/brave-browser/issues/13882 -#define MAYBE_CosmeticFilteringProtect1p DISABLED_CosmeticFilteringProtect1p -IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, MAYBE_CosmeticFilteringProtect1p) { - UpdateAdBlockInstanceWithRules("b.com##.fpsponsored\n"); +IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringProtect1p) { + UpdateAdBlockInstanceWithRules( + "appspot.com##.fpsponsored\n" + "appspot.com##.fpsponsored1\n" + "appspot.com##.fpsponsored2\n" + "appspot.com##.fpsponsored3\n" + "appspot.com##.fpsponsored4\n"); WaitForBraveExtensionShieldsDataReady(); - GURL tab_url = - embedded_test_server()->GetURL("b.com", "/cosmetic_filtering.html"); + GURL tab_url = embedded_test_server()->GetURL("test.lion.appspot.com", + "/cosmetic_filtering.html"); ui_test_utils::NavigateToURL(browser(), tab_url); content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents(); - ASSERT_EQ(true, EvalJs(contents, - "checkSelector('.fpsponsored', 'display', 'block')")); + EXPECT_EQ(true, + EvalJs(contents, + "checkSelector('#relative-url-div', 'display', 'block')")); + EXPECT_EQ(true, + EvalJs(contents, + "checkSelector('#same-origin-div', 'display', 'block')")); + EXPECT_EQ( + true, + EvalJs(contents, "checkSelector('#subdomain-div', 'display', 'block')")); + EXPECT_EQ(true, EvalJs(contents, + "checkSelector('#same-etld', 'display', 'block')")); + EXPECT_EQ(true, EvalJs(contents, + "checkSelector('#another-etld', 'display', 'none')")); } // Test cosmetic filtering bypasses 1st party checks when toggled diff --git a/test/data/cosmetic_filtering.html b/test/data/cosmetic_filtering.html index 89c24d03805..78218b5e0ab 100644 --- a/test/data/cosmetic_filtering.html +++ b/test/data/cosmetic_filtering.html @@ -23,6 +23,8 @@ let didWait = false; function checkSelector(selector, property, expected) { const checkSelectorInner = () => { let elements = [].slice.call(document.querySelectorAll(selector)); + if (elements.length === 0) + return false let result = elements.every(e => { let style = window.getComputedStyle(e); return style[property] === expected; @@ -47,12 +49,33 @@ function checkSelector(selector, property, expected) { -
+ + +
+ A text of 30 chars and 5 words is needed here to consider element significant. + +
+
+ A text of 30 chars and 5 words is needed here to consider element significant. + +
+
+ A text of 30 chars and 5 words is needed here to consider element significant. + +
+
+ A text of 30 chars and 5 words is needed here to consider element significant. + +
+
+ A text of 30 chars and 5 words is needed here to consider element significant. + +
From 7f6f9431b335be2c1bf21e95b95205885c86e340 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Fri, 10 Sep 2021 21:39:12 +0700 Subject: [PATCH 3/7] Add a comment about the test domain --- browser/brave_shields/ad_block_service_browsertest.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/browser/brave_shields/ad_block_service_browsertest.cc b/browser/brave_shields/ad_block_service_browsertest.cc index 4468ce6e100..94fdf75f83d 100644 --- a/browser/brave_shields/ad_block_service_browsertest.cc +++ b/browser/brave_shields/ad_block_service_browsertest.cc @@ -1722,6 +1722,8 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringProtect1p) { WaitForBraveExtensionShieldsDataReady(); + // *.appspot.com is used here to check the eTLD logic. + // Tt's a private suffix from https://publicsuffix.org/list/ GURL tab_url = embedded_test_server()->GetURL("test.lion.appspot.com", "/cosmetic_filtering.html"); ui_test_utils::NavigateToURL(browser(), tab_url); From d30304cac6f69c82f7e0a83d5fb987c5bd4d8abf Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Fri, 10 Sep 2021 22:20:46 +0700 Subject: [PATCH 4/7] Fix comment --- browser/brave_shields/ad_block_service_browsertest.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser/brave_shields/ad_block_service_browsertest.cc b/browser/brave_shields/ad_block_service_browsertest.cc index 94fdf75f83d..e3c66553fed 100644 --- a/browser/brave_shields/ad_block_service_browsertest.cc +++ b/browser/brave_shields/ad_block_service_browsertest.cc @@ -1723,7 +1723,7 @@ IN_PROC_BROWSER_TEST_F(AdBlockServiceTest, CosmeticFilteringProtect1p) { WaitForBraveExtensionShieldsDataReady(); // *.appspot.com is used here to check the eTLD logic. - // Tt's a private suffix from https://publicsuffix.org/list/ + // It's a private suffix from https://publicsuffix.org/list/ GURL tab_url = embedded_test_server()->GetURL("test.lion.appspot.com", "/cosmetic_filtering.html"); ui_test_utils::NavigateToURL(browser(), tab_url); From c24264fa065dcef41447c5b93d7134b80eba2fc8 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Mon, 13 Sep 2021 11:56:21 +0700 Subject: [PATCH 5/7] Remove parse-domain from package-lock.json --- package-lock.json | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/package-lock.json b/package-lock.json index 39144133834..3fe10a282a4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14070,16 +14070,6 @@ "safe-buffer": "^5.1.1" } }, - "parse-domain": { - "version": "3.0.3", - "resolved": "https://registry.npmjs.org/parse-domain/-/parse-domain-3.0.3.tgz", - "integrity": "sha512-KOJR8kEymjWO5xHrt57LFJ4xtncwGfd/Z9+Twm6apKU9NIw3uSnwYTAoRUwC+MflGsn5h6MeyHltz6Fa6KT7cA==", - "requires": { - "is-ip": "^3.1.0", - "node-fetch": "^2.6.0", - "punycode": "^2.1.1" - } - }, "parse-entities": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/parse-entities/-/parse-entities-2.0.0.tgz", From a1fa4a60a2a0098590aa951af3216e79ef8ebf78 Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Mon, 13 Sep 2021 12:00:29 +0700 Subject: [PATCH 6/7] Remove parse-domain from package.json --- package.json | 1 - 1 file changed, 1 deletion(-) diff --git a/package.json b/package.json index 29c74a2f9f6..f4281b7daf3 100644 --- a/package.json +++ b/package.json @@ -322,7 +322,6 @@ "core-js": "^3.9.1", "date-fns": "^2.15.0", "jszip": "^3.2.2", - "parse-domain": "^3.0.2", "prettier-bytes": "^1.0.4", "qr-image": "^3.2.0", "react-sortable-hoc": "^1.10.1", From 1abbb085ee553f6e62ccc8f34310db0353170b2b Mon Sep 17 00:00:00 2001 From: Mikhail Atuchin Date: Mon, 13 Sep 2021 12:21:10 +0700 Subject: [PATCH 7/7] Remove third_party/npm_parse-domain --- third_party/npm_parse-domain/README.chromium | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 third_party/npm_parse-domain/README.chromium diff --git a/third_party/npm_parse-domain/README.chromium b/third_party/npm_parse-domain/README.chromium deleted file mode 100644 index 26cb7c6ee02..00000000000 --- a/third_party/npm_parse-domain/README.chromium +++ /dev/null @@ -1,4 +0,0 @@ -Name: parse-domain -URL: https://github.com/peerigon/parse-domain -License: Unlicense -License File: /brave/node_modules/parse-domain/LICENSE