From 0262f93456ee32dbd96af7a988e14b7dd6550bf7 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Thu, 12 Dec 2024 15:33:57 +0900 Subject: [PATCH 01/13] feat: introduce `$to` in `NetworkFilters` --- packages/adblocker/src/filters/network.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/packages/adblocker/src/filters/network.ts b/packages/adblocker/src/filters/network.ts index 16e487f5b3..cdec3caad2 100644 --- a/packages/adblocker/src/filters/network.ts +++ b/packages/adblocker/src/filters/network.ts @@ -153,6 +153,7 @@ export const enum NETWORK_FILTER_MASK { isHostnameAnchor = 1 << 28, isRedirectRule = 1 << 29, isRedirect = 1 << 30, + isDenyallowNegated = 1 << 31, // IMPORTANT: the mask is now full, no more options can be added // Consider creating a separate fitler type for isReplace if a new // network filter option is needed. @@ -731,7 +732,14 @@ export default class NetworkFilter implements IFilter { const value = rawOption[1]; switch (option) { + case 'to': case 'denyallow': { + // Always override with an option declared later + if (option === 'to') { + mask = setBit(mask, NETWORK_FILTER_MASK.isDenyallowNegated); + } else { + mask = clearBit(mask, NETWORK_FILTER_MASK.isDenyallowNegated); + } denyallow = Domains.parse(value.split('|'), debug); break; } @@ -1818,6 +1826,10 @@ export default class NetworkFilter implements IFilter { return getBit(this.mask, NETWORK_FILTER_MASK.isBadFilter); } + public isDenyallowNegated() { + return getBit(this.mask, NETWORK_FILTER_MASK.isDenyallowNegated); + } + public isUnicode() { return getBit(this.mask, NETWORK_FILTER_MASK.isUnicode); } @@ -2108,7 +2120,9 @@ function checkOptions(filter: NetworkFilter, request: Request): boolean { // If `hostname` is matched by `denyallow` then the request should be allowed. if ( filter.denyallow !== undefined && - filter.denyallow.match(request.getHostnameHashes(), request.getEntityHashes()) === true + filter.denyallow.match(request.getHostnameHashes(), request.getEntityHashes()) === + // In case of `$denyallow`, this should work as an exception so `true` is required. + !filter.isDenyallowNegated() ) { return false; } From 556f66758d0d84c2d635245eac4fa1574bf02cb5 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Thu, 12 Dec 2024 15:35:29 +0900 Subject: [PATCH 02/13] chore: allow only one option --- packages/adblocker/src/filters/network.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/adblocker/src/filters/network.ts b/packages/adblocker/src/filters/network.ts index cdec3caad2..7e9b834593 100644 --- a/packages/adblocker/src/filters/network.ts +++ b/packages/adblocker/src/filters/network.ts @@ -734,11 +734,11 @@ export default class NetworkFilter implements IFilter { switch (option) { case 'to': case 'denyallow': { - // Always override with an option declared later + if (denyallow !== undefined) { + return null; + } if (option === 'to') { mask = setBit(mask, NETWORK_FILTER_MASK.isDenyallowNegated); - } else { - mask = clearBit(mask, NETWORK_FILTER_MASK.isDenyallowNegated); } denyallow = Domains.parse(value.split('|'), debug); break; From c7e0411522e768d88316851224fb85e1b56eaf63 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Thu, 12 Dec 2024 15:45:11 +0900 Subject: [PATCH 03/13] test: `$to` --- packages/adblocker/test/matching.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/adblocker/test/matching.test.ts b/packages/adblocker/test/matching.test.ts index 5d5c283ff9..73e709bbfc 100644 --- a/packages/adblocker/test/matching.test.ts +++ b/packages/adblocker/test/matching.test.ts @@ -396,6 +396,23 @@ describe('#NetworkFilter.match', () => { url: 'https://sub.y.com/bar', type: 'script', }); + + // to + expect(f`*$3p,from=a.com,to=b.com`).to.matchRequest({ + sourceUrl: 'https://a.com', + url: 'https://b.com/bar', + type: 'script', + }); + expect(f`*$frame,3p,from=a.com|b.com,to=~c.com`).to.not.matchRequest({ + sourceUrl: 'https://a.com', + url: 'https://c.com/bar', + type: 'sub_frame', + }); + expect(f`$frame,csp=non-relevant,to=~safe.com,from=c.com|d.com`).to.not.matchRequest({ + sourceUrl: 'https://c.com', + url: 'https://safe.com/foo', + type: 'sub_frame', + }); }); }); From a69b5ea288b0890d6d36c02dfc5e32eea2c430b7 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Thu, 12 Dec 2024 16:00:20 +0900 Subject: [PATCH 04/13] feat: `to` as a negated alias of `denyallow` --- packages/adblocker/src/filters/network.ts | 26 ++++++++++++++--------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/packages/adblocker/src/filters/network.ts b/packages/adblocker/src/filters/network.ts index 7e9b834593..6cae716f33 100644 --- a/packages/adblocker/src/filters/network.ts +++ b/packages/adblocker/src/filters/network.ts @@ -153,7 +153,7 @@ export const enum NETWORK_FILTER_MASK { isHostnameAnchor = 1 << 28, isRedirectRule = 1 << 29, isRedirect = 1 << 30, - isDenyallowNegated = 1 << 31, + isTo = 1 << 31, // IMPORTANT: the mask is now full, no more options can be added // Consider creating a separate fitler type for isReplace if a new // network filter option is needed. @@ -737,10 +737,14 @@ export default class NetworkFilter implements IFilter { if (denyallow !== undefined) { return null; } + let parts = value.split('|'); if (option === 'to') { - mask = setBit(mask, NETWORK_FILTER_MASK.isDenyallowNegated); + mask = setBit(mask, NETWORK_FILTER_MASK.isTo); + parts = parts.map((part) => + part.charCodeAt(0) === 126 /* '~' */ ? part.slice(1) : `~${part}`, + ); } - denyallow = Domains.parse(value.split('|'), debug); + denyallow = Domains.parse(parts, debug); break; } case 'domain': @@ -1532,10 +1536,14 @@ export default class NetworkFilter implements IFilter { } if (this.denyallow !== undefined) { + const optionName = this.isTo() ? 'to' : 'denyallow'; if (this.denyallow.parts !== undefined) { - options.push(`denyallow=${this.denyallow.parts}`); + const parts = this.denyallow.parts.split(','); + options.push( + `${optionName}=${this.isTo() ? parts.map((part) => (part.charCodeAt(0) === 126 /* '~' */ ? part.slice(1) : `~${part}`)).join('|') : parts.join('|')}`, + ); } else { - options.push('denyallow='); + options.push(`${optionName}=`); } } @@ -1826,8 +1834,8 @@ export default class NetworkFilter implements IFilter { return getBit(this.mask, NETWORK_FILTER_MASK.isBadFilter); } - public isDenyallowNegated() { - return getBit(this.mask, NETWORK_FILTER_MASK.isDenyallowNegated); + public isTo() { + return getBit(this.mask, NETWORK_FILTER_MASK.isTo); } public isUnicode() { @@ -2120,9 +2128,7 @@ function checkOptions(filter: NetworkFilter, request: Request): boolean { // If `hostname` is matched by `denyallow` then the request should be allowed. if ( filter.denyallow !== undefined && - filter.denyallow.match(request.getHostnameHashes(), request.getEntityHashes()) === - // In case of `$denyallow`, this should work as an exception so `true` is required. - !filter.isDenyallowNegated() + filter.denyallow.match(request.getHostnameHashes(), request.getEntityHashes()) === true ) { return false; } From f1062278f6ae04f6e4ff55ccb5f2680e7752f78c Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Thu, 12 Dec 2024 16:00:29 +0900 Subject: [PATCH 05/13] test: `$to` under `$denyallow` --- packages/adblocker/test/parsing.test.ts | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/packages/adblocker/test/parsing.test.ts b/packages/adblocker/test/parsing.test.ts index 1b9da5b8b5..f06664900a 100644 --- a/packages/adblocker/test/parsing.test.ts +++ b/packages/adblocker/test/parsing.test.ts @@ -803,6 +803,25 @@ describe('Network filters', () => { denyallow: undefined, }); }); + + context('to', () => { + it('fails when both denyallow and to used', () => { + network('||foo.com$denyallow=foo.com,to=bar.com', null); + network('||foo.com$to=foo.com,denyallow=bar.com', null); + }); + + it('reverses domains condition', () => { + network('||foo.com$to=bar.com|baz.com', { + denyallow: { + hostnames: undefined, + entities: undefined, + notHostnames: h(['bar.com', 'baz.com']), + notEntities: undefined, + parts: undefined, + }, + }); + }); + }); }); describe('redirect', () => { From 35c36cc093fd3775d85d9b91cd44e0deebd8b5c1 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Fri, 13 Dec 2024 15:20:25 +0900 Subject: [PATCH 06/13] feat: merge denyallow and to --- packages/adblocker/src/filters/network.ts | 21 +++++---------------- packages/adblocker/test/parsing.test.ts | 14 +++++++++----- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/packages/adblocker/src/filters/network.ts b/packages/adblocker/src/filters/network.ts index 6cae716f33..035e151940 100644 --- a/packages/adblocker/src/filters/network.ts +++ b/packages/adblocker/src/filters/network.ts @@ -153,7 +153,6 @@ export const enum NETWORK_FILTER_MASK { isHostnameAnchor = 1 << 28, isRedirectRule = 1 << 29, isRedirect = 1 << 30, - isTo = 1 << 31, // IMPORTANT: the mask is now full, no more options can be added // Consider creating a separate fitler type for isReplace if a new // network filter option is needed. @@ -726,6 +725,7 @@ export default class NetworkFilter implements IFilter { // --------------------------------------------------------------------- // // parseOptions // --------------------------------------------------------------------- // + const denyallowEntities: Set = new Set(); for (const rawOption of getFilterOptions(line, optionsIndex + 1, line.length)) { const negation = rawOption[0].charCodeAt(0) === 126; /* '~' */ const option = negation === true ? rawOption[0].slice(1) : rawOption[0]; @@ -734,17 +734,14 @@ export default class NetworkFilter implements IFilter { switch (option) { case 'to': case 'denyallow': { - if (denyallow !== undefined) { - return null; - } let parts = value.split('|'); if (option === 'to') { - mask = setBit(mask, NETWORK_FILTER_MASK.isTo); parts = parts.map((part) => part.charCodeAt(0) === 126 /* '~' */ ? part.slice(1) : `~${part}`, ); } - denyallow = Domains.parse(parts, debug); + parts.forEach((part) => denyallowEntities.add(part)); + denyallow = Domains.parse(Array.from(denyallowEntities), debug); break; } case 'domain': @@ -1536,14 +1533,10 @@ export default class NetworkFilter implements IFilter { } if (this.denyallow !== undefined) { - const optionName = this.isTo() ? 'to' : 'denyallow'; if (this.denyallow.parts !== undefined) { - const parts = this.denyallow.parts.split(','); - options.push( - `${optionName}=${this.isTo() ? parts.map((part) => (part.charCodeAt(0) === 126 /* '~' */ ? part.slice(1) : `~${part}`)).join('|') : parts.join('|')}`, - ); + options.push(`denyallow=${this.denyallow.parts.replace(/,/g, '|')}`); } else { - options.push(`${optionName}=`); + options.push('denyallow='); } } @@ -1834,10 +1827,6 @@ export default class NetworkFilter implements IFilter { return getBit(this.mask, NETWORK_FILTER_MASK.isBadFilter); } - public isTo() { - return getBit(this.mask, NETWORK_FILTER_MASK.isTo); - } - public isUnicode() { return getBit(this.mask, NETWORK_FILTER_MASK.isUnicode); } diff --git a/packages/adblocker/test/parsing.test.ts b/packages/adblocker/test/parsing.test.ts index f06664900a..5149de76c7 100644 --- a/packages/adblocker/test/parsing.test.ts +++ b/packages/adblocker/test/parsing.test.ts @@ -805,12 +805,16 @@ describe('Network filters', () => { }); context('to', () => { - it('fails when both denyallow and to used', () => { - network('||foo.com$denyallow=foo.com,to=bar.com', null); - network('||foo.com$to=foo.com,denyallow=bar.com', null); - }); - it('reverses domains condition', () => { + network('||foo.com$to=foo.com|~bar.com,denyallow=bar.com|~foo.com', { + denyallow: { + hostnames: h(['bar.com']), + entities: undefined, + notHostnames: h(['foo.com']), + notEntities: undefined, + parts: undefined, + }, + }); network('||foo.com$to=bar.com|baz.com', { denyallow: { hostnames: undefined, From 86e5d84d14ab6b3bad55c158ae1f5df6ffbd1fb0 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Wed, 25 Dec 2024 12:52:04 +0900 Subject: [PATCH 07/13] chore: revert fix for parts literal fixes https://github.com/ghostery/adblocker/pull/4524#discussion_r1895397346 --- packages/adblocker/src/filters/network.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/adblocker/src/filters/network.ts b/packages/adblocker/src/filters/network.ts index 035e151940..4e892a98f2 100644 --- a/packages/adblocker/src/filters/network.ts +++ b/packages/adblocker/src/filters/network.ts @@ -1534,7 +1534,7 @@ export default class NetworkFilter implements IFilter { if (this.denyallow !== undefined) { if (this.denyallow.parts !== undefined) { - options.push(`denyallow=${this.denyallow.parts.replace(/,/g, '|')}`); + options.push(`denyallow=${this.denyallow.parts}`); } else { options.push('denyallow='); } From b9004e50ae696803b3a39ef4c42f660caf6560cc Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Wed, 25 Dec 2024 12:55:07 +0900 Subject: [PATCH 08/13] refactor: use `for...of` loop fixes https://github.com/ghostery/adblocker/pull/4524#discussion_r1895398135 fixes https://github.com/ghostery/adblocker/pull/4524#discussion_r1895401820 --- packages/adblocker/src/filters/network.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/adblocker/src/filters/network.ts b/packages/adblocker/src/filters/network.ts index 4e892a98f2..deb9cd6e6a 100644 --- a/packages/adblocker/src/filters/network.ts +++ b/packages/adblocker/src/filters/network.ts @@ -734,13 +734,17 @@ export default class NetworkFilter implements IFilter { switch (option) { case 'to': case 'denyallow': { - let parts = value.split('|'); - if (option === 'to') { - parts = parts.map((part) => - part.charCodeAt(0) === 126 /* '~' */ ? part.slice(1) : `~${part}`, - ); + for (const part of value.split('|')) { + if (option === 'to') { + if (part.charCodeAt(0) === 126 /* '~' */) { + denyallowEntities.add(part.slice(1)); + } else { + denyallowEntities.add(`~${part}`); + } + } else { + denyallowEntities.add(part); + } } - parts.forEach((part) => denyallowEntities.add(part)); denyallow = Domains.parse(Array.from(denyallowEntities), debug); break; } From c65e50e3bdbd276dcd6d4b5a208d8caff3cdff04 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Thu, 23 Jan 2025 18:00:11 +0900 Subject: [PATCH 09/13] fix: invalid use of `Domains.parse` --- packages/adblocker/src/filters/network.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/adblocker/src/filters/network.ts b/packages/adblocker/src/filters/network.ts index ebd851a076..16d20f0ba4 100644 --- a/packages/adblocker/src/filters/network.ts +++ b/packages/adblocker/src/filters/network.ts @@ -745,7 +745,10 @@ export default class NetworkFilter implements IFilter { denyallowEntities.add(part); } } - denyallow = Domains.parse(Array.from(denyallowEntities), { delimiter: '|', debug }); + denyallow = Domains.parse(Array.from(denyallowEntities).join('|'), { + delimiter: '|', + debug, + }); if (denyallow === undefined) { return null; } From 69fb0b7dac8e59cd7e54bb8017b2c0a78505e920 Mon Sep 17 00:00:00 2001 From: Krzysztof Modras Date: Thu, 23 Jan 2025 16:31:20 +0100 Subject: [PATCH 10/13] Move negation logic to Domains.parse --- packages/adblocker/src/engine/domains.ts | 21 +++++++++++++++++++-- packages/adblocker/src/filters/network.ts | 15 ++------------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/packages/adblocker/src/engine/domains.ts b/packages/adblocker/src/engine/domains.ts index f207d7c0c7..aa08c68623 100644 --- a/packages/adblocker/src/engine/domains.ts +++ b/packages/adblocker/src/engine/domains.ts @@ -14,7 +14,11 @@ import { binLookup, hasUnicode, HASH_INTERNAL_MULT } from '../utils.js'; export class Domains { public static parse( value: string, - { delimiter = ',', debug = false }: { delimiter?: string; debug?: boolean } = {}, + { + delimiter = ',', + debug = false, + negate = false, + }: { delimiter?: string; debug?: boolean; negate?: boolean } = {}, ): Domains | undefined { const parts = value.split(delimiter); @@ -32,8 +36,21 @@ export class Domains { const notEntities: number[] = []; const hostnames: number[] = []; const notHostnames: number[] = []; + const rawParts: string[] = []; for (let hostname of parts) { + if (negate) { + if (hostname.charCodeAt(0) === 126 /* '~' */) { + hostname = hostname.slice(1); + } else { + hostname = `~${hostname}`; + } + } + + if (debug === true) { + rawParts.push(hostname); + } + if (hasUnicode(hostname)) { hostname = toASCII(hostname); } @@ -70,7 +87,7 @@ export class Domains { hostnames: hostnames.length !== 0 ? new Uint32Array(hostnames).sort() : undefined, notEntities: notEntities.length !== 0 ? new Uint32Array(notEntities).sort() : undefined, notHostnames: notHostnames.length !== 0 ? new Uint32Array(notHostnames).sort() : undefined, - parts: debug === true ? parts.join(delimiter) : undefined, + parts: debug === true ? rawParts.join(delimiter) : undefined, }); } diff --git a/packages/adblocker/src/filters/network.ts b/packages/adblocker/src/filters/network.ts index 16d20f0ba4..f1a31188f2 100644 --- a/packages/adblocker/src/filters/network.ts +++ b/packages/adblocker/src/filters/network.ts @@ -725,7 +725,6 @@ export default class NetworkFilter implements IFilter { // --------------------------------------------------------------------- // // parseOptions // --------------------------------------------------------------------- // - const denyallowEntities: Set = new Set(); for (const rawOption of getFilterOptions(line, optionsIndex + 1, line.length)) { const negation = rawOption[0].charCodeAt(0) === 126; /* '~' */ const option = negation === true ? rawOption[0].slice(1) : rawOption[0]; @@ -734,20 +733,10 @@ export default class NetworkFilter implements IFilter { switch (option) { case 'to': case 'denyallow': { - for (const part of value.split('|')) { - if (option === 'to') { - if (part.charCodeAt(0) === 126 /* '~' */) { - denyallowEntities.add(part.slice(1)); - } else { - denyallowEntities.add(`~${part}`); - } - } else { - denyallowEntities.add(part); - } - } - denyallow = Domains.parse(Array.from(denyallowEntities).join('|'), { + denyallow = Domains.parse(value, { delimiter: '|', debug, + negate: option === 'to', }); if (denyallow === undefined) { return null; From 8081ba568ac36bcf09047f398b37233137c648be Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Fri, 24 Jan 2025 16:26:00 +0900 Subject: [PATCH 11/13] refactor: optimize runtime negation fixes https://github.com/ghostery/adblocker/pull/4524/files/69fb0b7dac8e59cd7e54bb8017b2c0a78505e920#r1927688982 --- packages/adblocker/src/engine/domains.ts | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/adblocker/src/engine/domains.ts b/packages/adblocker/src/engine/domains.ts index aa08c68623..1da16458ce 100644 --- a/packages/adblocker/src/engine/domains.ts +++ b/packages/adblocker/src/engine/domains.ts @@ -39,14 +39,6 @@ export class Domains { const rawParts: string[] = []; for (let hostname of parts) { - if (negate) { - if (hostname.charCodeAt(0) === 126 /* '~' */) { - hostname = hostname.slice(1); - } else { - hostname = `~${hostname}`; - } - } - if (debug === true) { rawParts.push(hostname); } @@ -67,7 +59,7 @@ export class Domains { negation === true || entity === true ? hostname.slice(start, end) : hostname, ); - if (negation) { + if (negate ? !negation : negation) { if (entity) { notEntities.push(hash); } else { From 7b868fe28b7eeaffb184a685aa28ebb3f49c6d04 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Fri, 24 Jan 2025 16:42:19 +0900 Subject: [PATCH 12/13] chore: preserve unprocessed `hostname` on debug --- packages/adblocker/src/engine/domains.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/adblocker/src/engine/domains.ts b/packages/adblocker/src/engine/domains.ts index 1da16458ce..6f7c493616 100644 --- a/packages/adblocker/src/engine/domains.ts +++ b/packages/adblocker/src/engine/domains.ts @@ -38,10 +38,8 @@ export class Domains { const notHostnames: number[] = []; const rawParts: string[] = []; - for (let hostname of parts) { - if (debug === true) { - rawParts.push(hostname); - } + for (const rawHostname of parts) { + let hostname = rawHostname; if (hasUnicode(hostname)) { hostname = toASCII(hostname); @@ -59,18 +57,25 @@ export class Domains { negation === true || entity === true ? hostname.slice(start, end) : hostname, ); - if (negate ? !negation : negation) { + // If conditionally negated value of `negation` by `negate` is `true` + if ((+negation ^ +negate) === 1) { if (entity) { notEntities.push(hash); } else { notHostnames.push(hash); } + if (debug) { + rawParts.push(negation ? rawHostname : `~${rawHostname}`); + } } else { if (entity) { entities.push(hash); } else { hostnames.push(hash); } + if (debug) { + rawParts.push(negation ? rawHostname.slice(1) : rawHostname); + } } } From 3d7bf42425a0332086cb67528cf1aa8e144cabfc Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Fri, 24 Jan 2025 16:50:42 +0900 Subject: [PATCH 13/13] chore: drop unnecessary evaluation --- packages/adblocker/src/engine/domains.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/adblocker/src/engine/domains.ts b/packages/adblocker/src/engine/domains.ts index 6f7c493616..54617a3ccb 100644 --- a/packages/adblocker/src/engine/domains.ts +++ b/packages/adblocker/src/engine/domains.ts @@ -57,8 +57,8 @@ export class Domains { negation === true || entity === true ? hostname.slice(start, end) : hostname, ); - // If conditionally negated value of `negation` by `negate` is `true` - if ((+negation ^ +negate) === 1) { + // If conditionally negated value of `negation` by `negate` is `1` + if (+negation ^ +negate) { if (entity) { notEntities.push(hash); } else {