From 370f36818f5673d0d935d7c1540f9a6d92b5d941 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Tue, 9 Jan 2024 15:56:35 +0900 Subject: [PATCH 1/4] feat: modifier replacer for NetworkFilter.toString To make minimal performance impact on current codes using adblocker library, I chose to use Array.prototype.map function instead of putting `if` statements or using empty proxying function: `(str: text) => str` as default value. refs https://github.com/ghostery/adblocker/issues/3693 --- packages/adblocker/src/filters/network.ts | 8 ++++++-- packages/adblocker/test/parsing.test.ts | 19 +++++++++++++++++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/packages/adblocker/src/filters/network.ts b/packages/adblocker/src/filters/network.ts index 147ec85c30..aad1826677 100644 --- a/packages/adblocker/src/filters/network.ts +++ b/packages/adblocker/src/filters/network.ts @@ -1086,7 +1086,7 @@ export default class NetworkFilter implements IFilter { * there are things which cannot be recovered though, like domains options * of which only hashes are stored. */ - public toString() { + public toString(modifierReplacer?: (modifier: string) => string) { if (this.rawLine !== undefined) { return this.rawLine; } @@ -1200,7 +1200,11 @@ export default class NetworkFilter implements IFilter { } if (options.length > 0) { - filter += `$${options.join(',')}`; + if (typeof modifierReplacer === 'function') { + filter += `$${options.map(modifierReplacer).join(',')}`; + } else { + filter += `$${options.join(',')}`; + } } return filter; diff --git a/packages/adblocker/test/parsing.test.ts b/packages/adblocker/test/parsing.test.ts index ac2d22df9b..09c86b9468 100644 --- a/packages/adblocker/test/parsing.test.ts +++ b/packages/adblocker/test/parsing.test.ts @@ -117,11 +117,16 @@ const DEFAULT_NETWORK_FILTER = { describe('Network filters', () => { describe('toString', () => { - const checkToString = (line: string, expected: string, debug: boolean = false) => { + const checkToString = ( + line: string, + expected: string, + debug: boolean = false, + modifierReplacer = (modifier: string) => modifier, + ) => { const parsed = NetworkFilter.parse(line, debug); expect(parsed).not.to.be.null; if (parsed !== null) { - expect(parsed.toString()).to.equal(expected); + expect(parsed.toString(modifierReplacer)).to.equal(expected); } }; @@ -177,6 +182,16 @@ describe('Network filters', () => { true, ); }); + + it('pprint longer form of modifiers', () => { + checkToString('||foo.com^$3p', '||foo.com^$third-party', false, (modifier) => { + if (modifier === '3p') { + return 'third-party'; + } + + return modifier; + }); + }); }); it('parses pattern', () => { From 5a86a0bf6c388d28f0e8f11548418aa6b7cf471c Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Tue, 9 Jan 2024 15:58:51 +0900 Subject: [PATCH 2/4] chore: fix types use `as const` keyword --- packages/adblocker/test/parsing.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/adblocker/test/parsing.test.ts b/packages/adblocker/test/parsing.test.ts index 09c86b9468..231e733926 100644 --- a/packages/adblocker/test/parsing.test.ts +++ b/packages/adblocker/test/parsing.test.ts @@ -1377,9 +1377,9 @@ describe('Network filters', () => { [new Uint32Array([NORMALIZED_TYPE_TOKEN.document])], ], ['@@/wp-content/themes/$script', [hashStrings(['content'])]], - ]) { + ] as const) { it(`get tokens for ${filter}`, () => { - const parsed = NetworkFilter.parse(filter as string, true); + const parsed = NetworkFilter.parse(filter, true); expect(parsed).not.to.be.null; if (parsed !== null) { expect(parsed.getTokens()).to.eql(regexTokens); From f82f0a1f0f0ff2580b12830fe8a2f095e7d7c6b6 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Tue, 9 Jan 2024 16:10:45 +0900 Subject: [PATCH 3/4] chore(test): fix test output refs https://github.com/ghostery/adblocker/pull/3681 --- packages/adblocker/test/parsing.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/adblocker/test/parsing.test.ts b/packages/adblocker/test/parsing.test.ts index 231e733926..a8f9dd26f2 100644 --- a/packages/adblocker/test/parsing.test.ts +++ b/packages/adblocker/test/parsing.test.ts @@ -184,7 +184,7 @@ describe('Network filters', () => { }); it('pprint longer form of modifiers', () => { - checkToString('||foo.com^$3p', '||foo.com^$third-party', false, (modifier) => { + checkToString('||foo.com^$3p', '||foo.com^|$third-party', false, (modifier) => { if (modifier === '3p') { return 'third-party'; } From bb4dd02fb585be23eca69a5e22ee84f7bcd1a162 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Wed, 10 Jan 2024 11:19:41 +0900 Subject: [PATCH 4/4] fix(test): test expectation to match latest commit refs #3681 --- packages/adblocker/test/parsing.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/adblocker/test/parsing.test.ts b/packages/adblocker/test/parsing.test.ts index fd0e7ee916..79fc46726f 100644 --- a/packages/adblocker/test/parsing.test.ts +++ b/packages/adblocker/test/parsing.test.ts @@ -185,7 +185,7 @@ describe('Network filters', () => { }); it('pprint longer form of modifiers', () => { - checkToString('||foo.com^$3p', '||foo.com^|$third-party', false, (modifier) => { + checkToString('||foo.com^$3p', '||foo.com^$third-party', false, (modifier) => { if (modifier === '3p') { return 'third-party'; }