From bf8a0476b39bb9499577dec23f0aa57d68182ad2 Mon Sep 17 00:00:00 2001 From: Maxim Topciu Date: Fri, 29 Mar 2024 20:07:45 +0300 Subject: [PATCH] AG-31530 do not block should collapse with popup rules Merge in ADGUARD-FILTERS/tsurlfilter from fix/AG-31530 to master Squashed commit of the following: commit 5a4dd60cccfc6e6c22f9f4ef37df60e3cbe7d308 Author: Slava Leleka Date: Fri Mar 29 19:44:24 2024 +0300 packages/tswebextension/CHANGELOG.md edited online with Bitbucket commit c5916122d3adb806c0fce79a452956b2063504cf Author: Slava Leleka Date: Fri Mar 29 19:43:20 2024 +0300 packages/tsurlfilter/CHANGELOG.md edited online with Bitbucket commit d6e5adad9b6f44260ff977f05a804989569a4b8d Author: Maxim Topciu Date: Fri Mar 29 18:34:21 2024 +0200 AG-31530 add comment commit ac4b7fa35922c9e86aa6a925db22be11c8a4411a Author: Maxim Topciu Date: Fri Mar 29 18:29:24 2024 +0200 AG-31530 add changelog commit cb3d9d94480838ac1e2c87876b9c95d725bb02a8 Author: Maxim Topciu Date: Fri Mar 29 18:02:10 2024 +0200 AG-31530 do not block should collapse with popup rules --- packages/tsurlfilter/CHANGELOG.md | 6 ++++++ .../tsurlfilter/src/engine/matching-result.ts | 9 ++++++++- packages/tsurlfilter/test/engine/engine.test.ts | 16 ++++++++-------- packages/tswebextension/CHANGELOG.md | 6 ++++++ .../background/request/request-blocking-api.ts | 12 ++++++++++-- .../src/lib/mv2/background/web-request-api.ts | 2 +- .../request/request-blocking-api.test.ts | 10 +++++++++- 7 files changed, 48 insertions(+), 13 deletions(-) diff --git a/packages/tsurlfilter/CHANGELOG.md b/packages/tsurlfilter/CHANGELOG.md index ccb481769..951db33cb 100644 --- a/packages/tsurlfilter/CHANGELOG.md +++ b/packages/tsurlfilter/CHANGELOG.md @@ -8,6 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 +## [2.2.18] - 2024-03-29 + +### Fixed + +- Do not block "Should collapse" mechanism with `$popup` rules + ## [2.2.17] - 2024-03-28 ### Changed diff --git a/packages/tsurlfilter/src/engine/matching-result.ts b/packages/tsurlfilter/src/engine/matching-result.ts index d5f239dc8..b378c448a 100644 --- a/packages/tsurlfilter/src/engine/matching-result.ts +++ b/packages/tsurlfilter/src/engine/matching-result.ts @@ -79,7 +79,7 @@ export class MatchingResult { * has an intersection by use cases with $all. * https://kb.adguard.com/en/general/how-to-create-your-own-ad-filters#popup-modifier */ - public popupRule: NetworkRule | null; + private popupRule: NetworkRule | null; /** * Creates an instance of the MatchingResult struct and fills it with the rules. @@ -191,6 +191,13 @@ export class MatchingResult { } } + /** + * Returns popup rule + */ + public getPopupRule(): NetworkRule | null { + return this.popupRule; + } + /** * GetBasicResult returns a rule that should be applied to the web request. * Possible outcomes are: diff --git a/packages/tsurlfilter/test/engine/engine.test.ts b/packages/tsurlfilter/test/engine/engine.test.ts index 79bbca5dd..3ccac2b38 100644 --- a/packages/tsurlfilter/test/engine/engine.test.ts +++ b/packages/tsurlfilter/test/engine/engine.test.ts @@ -418,28 +418,28 @@ describe('TestEngineMatchRequest - popup modifier', () => { let result = engine.matchRequest(request); expect(result.getBasicResult()).not.toBeNull(); expect(result.getBasicResult()!.getText()).toBe(blockingRuleText); - expect(result.popupRule!.getText()).toEqual(popupBlockingRuleText); + expect(result.getPopupRule()!.getText()).toEqual(popupBlockingRuleText); // Tests matching a script request; expects to match the basic blocking rule request = new Request('http://example.org/', 'http://example.com/', RequestType.Script); result = engine.matchRequest(request); expect(result.getBasicResult()).not.toBeNull(); expect(result.getBasicResult()!.getText()).toBe(blockingRuleText); - expect(result.popupRule!.getText()).toEqual(popupBlockingRuleText); + expect(result.getPopupRule()!.getText()).toEqual(popupBlockingRuleText); // Tests matching an image request; expects to match the basic blocking rule request = new Request('http://example.org/', 'http://example.com/', RequestType.Image); result = engine.matchRequest(request); expect(result.getBasicResult()).not.toBeNull(); expect(result.getBasicResult()!.getText()).toBe(blockingRuleText); - expect(result.popupRule!.getText()).toEqual(popupBlockingRuleText); + expect(result.getPopupRule()!.getText()).toEqual(popupBlockingRuleText); // Tests matching a document request; expects to match the popup blocking rule request = new Request('http://example.org/', 'http://example.com/', RequestType.Document); result = engine.matchRequest(request); expect(result.getBasicResult()).not.toBeNull(); expect(result.getBasicResult()!.getText()).toBe(blockingRuleText); - expect(result.popupRule!.getText()).toBe(popupBlockingRuleText); + expect(result.getPopupRule()!.getText()).toBe(popupBlockingRuleText); }); it('match requests against all and popup blocking rules', () => { @@ -457,28 +457,28 @@ describe('TestEngineMatchRequest - popup modifier', () => { let result = engine.matchRequest(request); expect(result.getBasicResult()).not.toBeNull(); expect(result.getBasicResult()!.getText()).toBe(blockingAllRuleText); - expect(result.popupRule!.getText()).toEqual(popupBlockingRuleText); + expect(result.getPopupRule()!.getText()).toEqual(popupBlockingRuleText); // Tests matching a script request; expects to match the all-encompassing blocking rule request = new Request('http://example.org/', 'http://example.com/', RequestType.Script); result = engine.matchRequest(request); expect(result.getBasicResult()).not.toBeNull(); expect(result.getBasicResult()!.getText()).toBe(blockingAllRuleText); - expect(result.popupRule!.getText()).toEqual(popupBlockingRuleText); + expect(result.getPopupRule()!.getText()).toEqual(popupBlockingRuleText); // Tests matching an image request; expects to match the all-encompassing blocking rule request = new Request('http://example.org/', 'http://example.com/', RequestType.Image); result = engine.matchRequest(request); expect(result.getBasicResult()).not.toBeNull(); expect(result.getBasicResult()!.getText()).toBe(blockingAllRuleText); - expect(result.popupRule!.getText()).toEqual(popupBlockingRuleText); + expect(result.getPopupRule()!.getText()).toEqual(popupBlockingRuleText); // Tests matching a document request; expects to match the popup blocking rule request = new Request('http://example.org/', 'http://example.com/', RequestType.Document); result = engine.matchRequest(request); expect(result.getBasicResult()).not.toBeNull(); expect(result.getBasicResult()!.getText()).toBe(blockingAllRuleText); - expect(result.popupRule!.getText()).toBe(popupBlockingRuleText); + expect(result.getPopupRule()!.getText()).toBe(popupBlockingRuleText); }); }); diff --git a/packages/tswebextension/CHANGELOG.md b/packages/tswebextension/CHANGELOG.md index 6d6ea1131..d79c32e60 100644 --- a/packages/tswebextension/CHANGELOG.md +++ b/packages/tswebextension/CHANGELOG.md @@ -8,6 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 +## [1.0.21] - 2024-03-29 + +### Fixed + +- Do not block "Should collapse" mechanism with `$popup` rules + ## [1.0.20] - 2024-03-28 ### Changed diff --git a/packages/tswebextension/src/lib/mv2/background/request/request-blocking-api.ts b/packages/tswebextension/src/lib/mv2/background/request/request-blocking-api.ts index 9abc5c306..4aa60fb08 100644 --- a/packages/tswebextension/src/lib/mv2/background/request/request-blocking-api.ts +++ b/packages/tswebextension/src/lib/mv2/background/request/request-blocking-api.ts @@ -73,6 +73,14 @@ export class RequestBlockingApi { return false; } + const basicRule = result.getBasicResult(); + const popupRule = result.getPopupRule(); + + // we do not want to block the main page if the rule has only $popup modifier + if (basicRule === popupRule) { + return false; + } + return RequestBlockingApi.isRequestBlockedByRule(result.getBasicResult()); } @@ -133,7 +141,7 @@ export class RequestBlockingApi { } // popup rule will be handled in the condition with requesttype === document below - if (popupRule?.getText() === rule.getText() && requestType !== RequestType.Document) { + if (popupRule === rule && requestType !== RequestType.Document) { return undefined; } @@ -171,7 +179,7 @@ export class RequestBlockingApi { } // we do not want to block the main page if rule has only $popup modifier - if (rule.getText() === popupRule?.getText() && !tabsApi.isNewPopupTab(tabId)) { + if (rule === popupRule && !tabsApi.isNewPopupTab(tabId)) { return undefined; } diff --git a/packages/tswebextension/src/lib/mv2/background/web-request-api.ts b/packages/tswebextension/src/lib/mv2/background/web-request-api.ts index 84a7eff6a..90a2e6526 100644 --- a/packages/tswebextension/src/lib/mv2/background/web-request-api.ts +++ b/packages/tswebextension/src/lib/mv2/background/web-request-api.ts @@ -359,7 +359,7 @@ export class WebRequestApi { // the response in order to actually apply $replace rules to it. const response = RequestBlockingApi.getBlockingResponse({ rule: basicResult, - popupRule: result.popupRule, + popupRule: result.getPopupRule(), eventId, requestUrl, referrerUrl, diff --git a/packages/tswebextension/test/lib/mv2/background/request/request-blocking-api.test.ts b/packages/tswebextension/test/lib/mv2/background/request/request-blocking-api.test.ts index 96510340d..3d8a03ff1 100644 --- a/packages/tswebextension/test/lib/mv2/background/request/request-blocking-api.test.ts +++ b/packages/tswebextension/test/lib/mv2/background/request/request-blocking-api.test.ts @@ -36,7 +36,7 @@ const getGetBlockingResponseParamsData = ( tabId: 1, eventId: '1', rule: result.getBasicResult(), - popupRule: result.popupRule, + popupRule: result.getPopupRule(), referrerUrl: '', requestUrl, requestType, @@ -64,6 +64,14 @@ describe('Request Blocking Api - shouldCollapseElement', () => { ).toBe(true); }); + it('iframe should not be collapsed by popup rule', () => { + mockMatchingResult('$popup,third-party,domain=example.org'); + + expect( + RequestBlockingApi.shouldCollapseElement(1, 'https://example.com', 'https://example.org', RequestType.SubDocument), + ).toBe(false); + }); + it('element without rule match shouldn`t be collapsed', () => { mockMatchingResult();