From 0119d6a88b0f2498ff3e325bc5c029bc238e9e9e Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Sat, 26 Oct 2024 22:03:09 +0200 Subject: [PATCH 1/9] fix: allow resources surrogates refs https://github.com/gorhill/uBlock/wiki/Resources-Library refs https://github.com/ghostery/adblocker/issues/4392 fixes https://github.com/ghostery/adblocker/issues/4393 test: allow scriptlets being used as resources fix: allow redirect resources surrogates test: drop outdated tests fix: always query with file extension chore: strictly check content type test: resources surrogate test: drop outdated --- packages/adblocker/src/resources.ts | 20 +++++++++++++++++++- packages/adblocker/test/resources.test.ts | 12 ++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/adblocker/src/resources.ts b/packages/adblocker/src/resources.ts index 2a6b15b1c3..a67c3b3ef4 100644 --- a/packages/adblocker/src/resources.ts +++ b/packages/adblocker/src/resources.ts @@ -311,7 +311,7 @@ export default class Resources { } public getScriptlet(name: string): string | undefined { - const scriptlet = this.getRawScriptlet(name); + const scriptlet = this.getRawScriptlet(name) || this.getResourceAsScriptlet(name); if (scriptlet === undefined) { return undefined; @@ -333,6 +333,24 @@ export default class Resources { return script; } + private getResourceAsScriptlet(name: string): Scriptlet | undefined { + if (name.endsWith('.js') === true) { + return undefined; + } + + const resource = this.resourcesByName.get(name + '.js'); + if (resource === undefined || resource.contentType !== 'application/javascript') { + return undefined; + } + + return { + name: resource.name, + aliases: [], + body: resource.body, + dependencies: [], + }; + } + public getScriptletCanonicalName(name: string): string | undefined { return this.getRawScriptlet(name)?.name; } diff --git a/packages/adblocker/test/resources.test.ts b/packages/adblocker/test/resources.test.ts index 0e2af9017d..babcfb69fb 100644 --- a/packages/adblocker/test/resources.test.ts +++ b/packages/adblocker/test/resources.test.ts @@ -227,6 +227,14 @@ describe('#Resources', function () { requiresTrust: false, }, ], + resources: [ + { + name: 'surrogate.js', + aliases: [], + body: 'function resource() {}', + contentType: 'application/javascript', + }, + ], }); }); @@ -272,6 +280,10 @@ describe('#Resources', function () { it('includes setup for scritplet globals', function () { expect(resources.getScriptlet('a')).to.include('var scriptletGlobals = {};'); }); + + it('allows resources surrogate', function () { + expect(resources.getScriptlet('surrogate')).to.include('resource'); + }); }); context('#getResource', function () { From 309aab384806d857093b0f3758f2495c293d887e Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Mon, 28 Oct 2024 22:35:24 +0900 Subject: [PATCH 2/9] fix: prevent surrogated scriptlet being wrapped --- packages/adblocker/src/resources.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/adblocker/src/resources.ts b/packages/adblocker/src/resources.ts index a67c3b3ef4..b1c9132e20 100644 --- a/packages/adblocker/src/resources.ts +++ b/packages/adblocker/src/resources.ts @@ -311,9 +311,13 @@ export default class Resources { } public getScriptlet(name: string): string | undefined { - const scriptlet = this.getRawScriptlet(name) || this.getResourceAsScriptlet(name); + const scriptlet = this.getRawScriptlet(name); if (scriptlet === undefined) { + const surrogate = this.getResourceAsScriptlet(name); + if (surrogate !== undefined) { + return surrogate.body; + } return undefined; } From 189af7af481bc5e826632ff7188ebf5530b17544 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Mon, 28 Oct 2024 22:55:01 +0900 Subject: [PATCH 3/9] refactor: simplify branches --- packages/adblocker/src/resources.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/adblocker/src/resources.ts b/packages/adblocker/src/resources.ts index b1c9132e20..3b72330371 100644 --- a/packages/adblocker/src/resources.ts +++ b/packages/adblocker/src/resources.ts @@ -314,11 +314,7 @@ export default class Resources { const scriptlet = this.getRawScriptlet(name); if (scriptlet === undefined) { - const surrogate = this.getResourceAsScriptlet(name); - if (surrogate !== undefined) { - return surrogate.body; - } - return undefined; + return this.getResourceAsScriptlet(name)?.body; } let script = this.scriptletsCache.get(scriptlet.name); From f185f6360ca835bb1d7d71d5ffd47eab51343dab Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Mon, 28 Oct 2024 22:56:01 +0900 Subject: [PATCH 4/9] test: do not wrap surrogated scriptlet --- packages/adblocker/test/resources.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/adblocker/test/resources.test.ts b/packages/adblocker/test/resources.test.ts index babcfb69fb..fe0cb72f92 100644 --- a/packages/adblocker/test/resources.test.ts +++ b/packages/adblocker/test/resources.test.ts @@ -282,7 +282,9 @@ describe('#Resources', function () { }); it('allows resources surrogate', function () { - expect(resources.getScriptlet('surrogate')).to.include('resource'); + expect(resources.getScriptlet('surrogate')) + .to.include('resource') + .not.to.include('scriptletGlobals'); }); }); From b5ff5278a6dca73f4b7b08aaeda761e27453a783 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Wed, 30 Oct 2024 10:08:26 +0900 Subject: [PATCH 5/9] refactor: resource surrogate body getter --- packages/adblocker/src/resources.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/packages/adblocker/src/resources.ts b/packages/adblocker/src/resources.ts index 3b72330371..baaeb70e48 100644 --- a/packages/adblocker/src/resources.ts +++ b/packages/adblocker/src/resources.ts @@ -314,7 +314,7 @@ export default class Resources { const scriptlet = this.getRawScriptlet(name); if (scriptlet === undefined) { - return this.getResourceAsScriptlet(name)?.body; + return this.getResourceAsScriptletSurrogate(name); } let script = this.scriptletsCache.get(scriptlet.name); @@ -333,7 +333,7 @@ export default class Resources { return script; } - private getResourceAsScriptlet(name: string): Scriptlet | undefined { + private getResourceAsScriptletSurrogate(name: string): string | undefined { if (name.endsWith('.js') === true) { return undefined; } @@ -343,12 +343,7 @@ export default class Resources { return undefined; } - return { - name: resource.name, - aliases: [], - body: resource.body, - dependencies: [], - }; + return resource.body; } public getScriptletCanonicalName(name: string): string | undefined { From 8e61a8bb7b3d743c38466003d27a1432eac1956d Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Wed, 30 Oct 2024 10:10:23 +0900 Subject: [PATCH 6/9] chore(test): comment surrogate rules --- packages/adblocker/test/resources.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/adblocker/test/resources.test.ts b/packages/adblocker/test/resources.test.ts index fe0cb72f92..3a75e8bdcf 100644 --- a/packages/adblocker/test/resources.test.ts +++ b/packages/adblocker/test/resources.test.ts @@ -231,7 +231,7 @@ describe('#Resources', function () { { name: 'surrogate.js', aliases: [], - body: 'function resource() {}', + body: '(function resource() {})()', contentType: 'application/javascript', }, ], @@ -282,9 +282,9 @@ describe('#Resources', function () { }); it('allows resources surrogate', function () { - expect(resources.getScriptlet('surrogate')) - .to.include('resource') - .not.to.include('scriptletGlobals'); + // Resource surrogates should be called without `.js` suffix or file extension. + // Also, they don't need to be wrapped with scriptlet suppliments like `scriptletGlobals` to avoid possible conflicts. + expect(resources.getScriptlet('surrogate')).to.be.equal('(function resource() {})()'); }); }); From 45ebb0a1e4873c74174ce8887159cddeb537d945 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Wed, 30 Oct 2024 19:19:14 +0900 Subject: [PATCH 7/9] chore: rename to `getSurrogate` --- packages/adblocker/src/resources.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/adblocker/src/resources.ts b/packages/adblocker/src/resources.ts index baaeb70e48..db47af566a 100644 --- a/packages/adblocker/src/resources.ts +++ b/packages/adblocker/src/resources.ts @@ -314,7 +314,7 @@ export default class Resources { const scriptlet = this.getRawScriptlet(name); if (scriptlet === undefined) { - return this.getResourceAsScriptletSurrogate(name); + return this.getSurrogate(name); } let script = this.scriptletsCache.get(scriptlet.name); @@ -333,7 +333,7 @@ export default class Resources { return script; } - private getResourceAsScriptletSurrogate(name: string): string | undefined { + private getSurrogate(name: string): string | undefined { if (name.endsWith('.js') === true) { return undefined; } From 4e92a91509067309951458105bb7fee6d678f054 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Wed, 30 Oct 2024 19:25:24 +0900 Subject: [PATCH 8/9] Update packages/adblocker/src/resources.ts Co-authored-by: Krzysztof Modras <1228153+chrmod@users.noreply.github.com> --- packages/adblocker/src/resources.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/adblocker/src/resources.ts b/packages/adblocker/src/resources.ts index db47af566a..d643c71e72 100644 --- a/packages/adblocker/src/resources.ts +++ b/packages/adblocker/src/resources.ts @@ -334,11 +334,7 @@ export default class Resources { } private getSurrogate(name: string): string | undefined { - if (name.endsWith('.js') === true) { - return undefined; - } - - const resource = this.resourcesByName.get(name + '.js'); + const resource = this.resourcesByName.get(name.endsWith('.js') ? name : `${name}.js`); if (resource === undefined || resource.contentType !== 'application/javascript') { return undefined; } From bddaf2217babdab3de2019fa9356176169186861 Mon Sep 17 00:00:00 2001 From: HoJeong Go Date: Wed, 30 Oct 2024 19:27:17 +0900 Subject: [PATCH 9/9] test: surrogate with `.js` suffix --- packages/adblocker/src/resources.ts | 1 + packages/adblocker/test/resources.test.ts | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/adblocker/src/resources.ts b/packages/adblocker/src/resources.ts index d643c71e72..e7700b54bf 100644 --- a/packages/adblocker/src/resources.ts +++ b/packages/adblocker/src/resources.ts @@ -335,6 +335,7 @@ export default class Resources { private getSurrogate(name: string): string | undefined { const resource = this.resourcesByName.get(name.endsWith('.js') ? name : `${name}.js`); + if (resource === undefined || resource.contentType !== 'application/javascript') { return undefined; } diff --git a/packages/adblocker/test/resources.test.ts b/packages/adblocker/test/resources.test.ts index 3a75e8bdcf..4e995de10e 100644 --- a/packages/adblocker/test/resources.test.ts +++ b/packages/adblocker/test/resources.test.ts @@ -282,9 +282,10 @@ describe('#Resources', function () { }); it('allows resources surrogate', function () { - // Resource surrogates should be called without `.js` suffix or file extension. // Also, they don't need to be wrapped with scriptlet suppliments like `scriptletGlobals` to avoid possible conflicts. expect(resources.getScriptlet('surrogate')).to.be.equal('(function resource() {})()'); + // Allow calling surrogate with `.js`. + expect(resources.getScriptlet('surrogate.js')).to.be.equal('(function resource() {})()'); }); });