From 2c10348ee70d5e6a91830e0bff5f077fd62688cd Mon Sep 17 00:00:00 2001 From: Stanislav Atroschenko Date: Tue, 20 Dec 2022 17:09:28 +0300 Subject: [PATCH] AG-17905 fix closure bug for prevent-xhr and trusted-replace-xhr-response #261 Merge in ADGUARD-FILTERS/scriptlets from fix/AG-17905 to release/v1.8 Squashed commit of the following: commit 09626a31665014ce514ce5c347d053ab4b14645c Merge: 25c9820 f0fbd11 Author: Stanislav A Date: Tue Dec 20 16:53:59 2022 +0300 Merge branch 'release/v1.8' into fix/AG-17905 commit 25c9820494ae8d1e1d35dddfac7dfb2e8f4b2547 Author: Stanislav A Date: Tue Dec 20 13:29:55 2022 +0300 update changelog and fix typos in test commit 6973376ccdb7e4c730a9ad76bf2658d98b5f2a80 Author: Stanislav A Date: Mon Dec 19 22:10:18 2022 +0300 add tests commit 492da8f85ccb0da99f023c042163f79d9879ffbd Author: Stanislav A Date: Fri Dec 16 21:40:20 2022 +0300 fix closure bug for trusted-replace-xhr-response commit 4f1d59b783b558a23e5f02766186ab2de02bc2f6 Author: Stanislav A Date: Fri Dec 16 21:24:30 2022 +0300 fix closure bug for prevent-xhr and add testcase --- CHANGELOG.md | 53 +++++++++++-------- src/scriptlets/prevent-xhr.js | 7 ++- .../trusted-replace-xhr-response.js | 17 +++--- tests/scriptlets/prevent-xhr.test.js | 52 +++++++++++++++--- .../trusted-replace-xhr-response.test.js | 41 ++++++++++++++ 5 files changed, 128 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f84875010..757985df2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,41 +1,48 @@ # Scriptlets and Redirect Resources Changelog +## 1.8.0 + +### Fixed + +- `prevent-xhr` and `trusted-replace-xhr-response` closure bug on multiple requests [#261](https://github.com/AdguardTeam/Scriptlets/issues/261) + + ## v1.7.10 ### Added -* new scriptlet `trusted-set-cookie-reload` +- new scriptlet `trusted-set-cookie-reload` ### Fixed -* `set-cookie-reload` infinite page reloading [#265](https://github.com/AdguardTeam/Scriptlets/issues/265) -* breakage of `prevent-element-src-loading` due to `window` getting into `apply` wrapper [#264](https://github.com/AdguardTeam/Scriptlets/issues/264) -* spread of args bug at `getXhrData` call for `trusted-replace-xhr-response` -* request properties array not being served to `getRequestData` and `parseMatchProps` helpers +- `set-cookie-reload` infinite page reloading [#265](https://github.com/AdguardTeam/Scriptlets/issues/265) +- breakage of `prevent-element-src-loading` due to `window` getting into `apply` wrapper [#264](https://github.com/AdguardTeam/Scriptlets/issues/264) +- spread of args bug at `getXhrData` call for `trusted-replace-xhr-response` +- request properties array not being served to `getRequestData` and `parseMatchProps` helpers ## v1.7.3 ### Added -* [Trusted scriptlets](./README.md#trusted-scriptlets) with extended capabilities: - * trusted-click-element [#23](https://github.com/AdguardTeam/Scriptlets/issues/23) - * trusted-replace-xhr-response [#202](https://github.com/AdguardTeam/Scriptlets/issues/202) - * trusted-replace-fetch-response - * trusted-set-local-storage-item - * trusted-set-cookie +- [Trusted scriptlets](./README.md#trusted-scriptlets) with extended capabilities: + - trusted-click-element [#23](https://github.com/AdguardTeam/Scriptlets/issues/23) + - trusted-replace-xhr-response [#202](https://github.com/AdguardTeam/Scriptlets/issues/202) + - trusted-replace-fetch-response + - trusted-set-local-storage-item + - trusted-set-cookie -* Scriptlets: - * xml-prune [#249](https://github.com/AdguardTeam/Scriptlets/issues/249) +- Scriptlets: + - xml-prune [#249](https://github.com/AdguardTeam/Scriptlets/issues/249) ### Improved -* Scriptlets: - * prevent-element-src-loading [#228](https://github.com/AdguardTeam/Scriptlets/issues/228) - * prevent-fetch [#216](https://github.com/AdguardTeam/Scriptlets/issues/216) - * abort-on-stack-trace [#201](https://github.com/AdguardTeam/Scriptlets/issues/201) - * abort-current-inline-script [#251](https://github.com/AdguardTeam/Scriptlets/issues/251) - * set-cookie & set-cookie-reload -* Redirects: - * google-ima3 [#255](https://github.com/AdguardTeam/Scriptlets/issues/255) - * metrika-yandex-tag [#254](https://github.com/AdguardTeam/Scriptlets/issues/254) - * googlesyndication-adsbygoogle [#252](https://github.com/AdguardTeam/Scriptlets/issues/252) +- Scriptlets: + - prevent-element-src-loading [#228](https://github.com/AdguardTeam/Scriptlets/issues/228) + - prevent-fetch [#216](https://github.com/AdguardTeam/Scriptlets/issues/216) + - abort-on-stack-trace [#201](https://github.com/AdguardTeam/Scriptlets/issues/201) + - abort-current-inline-script [#251](https://github.com/AdguardTeam/Scriptlets/issues/251) + - set-cookie & set-cookie-reload +- Redirects: + - google-ima3 [#255](https://github.com/AdguardTeam/Scriptlets/issues/255) + - metrika-yandex-tag [#254](https://github.com/AdguardTeam/Scriptlets/issues/254) + - googlesyndication-adsbygoogle [#252](https://github.com/AdguardTeam/Scriptlets/issues/252) diff --git a/src/scriptlets/prevent-xhr.js b/src/scriptlets/prevent-xhr.js index 4147da97e..39e115c17 100644 --- a/src/scriptlets/prevent-xhr.js +++ b/src/scriptlets/prevent-xhr.js @@ -96,7 +96,6 @@ export function preventXHR(source, propsToMatch, customResponseText) { return; } - let shouldPrevent = false; let response = ''; let responseText = ''; let responseUrl; @@ -111,15 +110,15 @@ export function preventXHR(source, propsToMatch, customResponseText) { // Log if no propsToMatch given logMessage(source, `xhr( ${objectToString(xhrData)} )`, true); hit(source); - } else { - shouldPrevent = matchRequestProps(source, propsToMatch, xhrData); + } else if (matchRequestProps(source, propsToMatch, xhrData)) { + thisArg.shouldBePrevented = true; } return Reflect.apply(target, thisArg, args); }; const sendWrapper = (target, thisArg, args) => { - if (!shouldPrevent) { + if (!thisArg.shouldBePrevented) { return Reflect.apply(target, thisArg, args); } diff --git a/src/scriptlets/trusted-replace-xhr-response.js b/src/scriptlets/trusted-replace-xhr-response.js index 5279cead9..9eaccd88a 100644 --- a/src/scriptlets/trusted-replace-xhr-response.js +++ b/src/scriptlets/trusted-replace-xhr-response.js @@ -90,9 +90,7 @@ export function trustedReplaceXhrResponse(source, pattern = '', replacement = '' const nativeOpen = window.XMLHttpRequest.prototype.open; const nativeSend = window.XMLHttpRequest.prototype.send; - let shouldReplace = false; let xhrData; - let requestHeaders = []; const openWrapper = (target, thisArg, args) => { // eslint-disable-next-line prefer-spread @@ -106,13 +104,16 @@ export function trustedReplaceXhrResponse(source, pattern = '', replacement = '' return Reflect.apply(target, thisArg, args); } - shouldReplace = matchRequestProps(source, propsToMatch, xhrData); + if (matchRequestProps(source, propsToMatch, xhrData)) { + thisArg.shouldBePrevented = true; + } // Trap setRequestHeader of target xhr object to mimic request headers later - if (shouldReplace) { + if (thisArg.shouldBePrevented) { + thisArg.collectedHeaders = []; const setRequestHeaderWrapper = (target, thisArg, args) => { // Collect headers - requestHeaders.push(args); + thisArg.collectedHeaders.push(args); return Reflect.apply(target, thisArg, args); }; @@ -129,7 +130,7 @@ export function trustedReplaceXhrResponse(source, pattern = '', replacement = '' }; const sendWrapper = (target, thisArg, args) => { - if (!shouldReplace) { + if (!thisArg.shouldBePrevented) { return Reflect.apply(target, thisArg, args); } @@ -197,13 +198,13 @@ export function trustedReplaceXhrResponse(source, pattern = '', replacement = '' // Mimic request headers before sending // setRequestHeader can only be called on open request objects - requestHeaders.forEach((header) => { + thisArg.collectedHeaders.forEach((header) => { const name = header[0]; const value = header[1]; forgedRequest.setRequestHeader(name, value); }); - requestHeaders = []; + thisArg.collectedHeaders = []; try { nativeSend.call(forgedRequest, args); diff --git a/tests/scriptlets/prevent-xhr.test.js b/tests/scriptlets/prevent-xhr.test.js index 3672ad50e..91dc1585e 100644 --- a/tests/scriptlets/prevent-xhr.test.js +++ b/tests/scriptlets/prevent-xhr.test.js @@ -159,7 +159,7 @@ if (isSupported) { xhr.send(); }); - test('Args, method matched, randomize response text, rangeMin equal to rangeMax (length:100-100)', async (assert) => { + test('Args, method matched, randomize response text, rangeMin === rangeMax (length:100-100)', async (assert) => { const METHOD = 'GET'; const URL = `${FETCH_OBJECTS_PATH}/test01.json`; const MATCH_DATA = ['method:GET', 'length:100-100']; @@ -203,7 +203,7 @@ if (isSupported) { xhr.send(); }); - test('Empty arg, prevent all, do not randomize response text - limit range (rangeMin + rangeMax - length:8888888888888888-99999999999999999999999)', async (assert) => { + test('Empty arg, prevent all, dont randomize response - limit range (rangeMin+rangeMax-length)', async (assert) => { const METHOD = 'GET'; const URL = `${FETCH_OBJECTS_PATH}/test01.json`; const MATCH_DATA = ['', 'length:8888888888888888-99999999999999999999999']; @@ -224,7 +224,7 @@ if (isSupported) { xhr.send(); }); - test('Empty arg, prevent all, do not randomize response text - limit range (rangeMax - length:10000-600000)', async (assert) => { + test('Empty arg, prevent all, dont randomize response text - limit range (rangeMax - length)', async (assert) => { const METHOD = 'GET'; const URL = `${FETCH_OBJECTS_PATH}/test01.json`; const MATCH_DATA = ['', 'length:10000-600000']; @@ -286,7 +286,7 @@ if (isSupported) { xhr.send(); }); - test('Empty arg, prevent all, do not randomize response text - invalid argument (length:test-30000)', async (assert) => { + test('Empty arg, prevent all, dont randomize response - invalid argument (length:test-30000)', async (assert) => { const METHOD = 'GET'; const URL = `${FETCH_OBJECTS_PATH}/test01.json`; const MATCH_DATA = ['', 'length:test-30000']; @@ -346,7 +346,7 @@ if (isSupported) { xhr.send(); }); - test('Empty arg, prevent all, do not randomize response text - invalid argument (length:123-345-450)', async (assert) => { + test('Empty arg, prevent all, dont randomize response - invalid argument (length:123-345-450)', async (assert) => { const METHOD = 'GET'; const URL = `${FETCH_OBJECTS_PATH}/test01.json`; const MATCH_DATA = ['', 'length:123-345-450']; @@ -366,7 +366,7 @@ if (isSupported) { xhr.send(); }); - test('Empty arg, prevent all, do not randomize response text - invalid argument (length:123---450)', async (assert) => { + test('Empty arg, prevent all, dont randomize response - invalid argument (length:123---450)', async (assert) => { const METHOD = 'GET'; const URL = `${FETCH_OBJECTS_PATH}/test01.json`; const MATCH_DATA = ['', 'length:123---450']; @@ -386,7 +386,7 @@ if (isSupported) { xhr.send(); }); - test('Empty arg, prevent all, do not randomize response text - invalid argument (length::123-450)', async (assert) => { + test('Empty arg, prevent all, dont randomize response - invalid argument (length::123-450)', async (assert) => { const METHOD = 'GET'; const URL = `${FETCH_OBJECTS_PATH}/test01.json`; const MATCH_DATA = ['', 'length::123-450']; @@ -588,6 +588,44 @@ if (isSupported) { }; xhr.send(); }); + + // https://github.com/AdguardTeam/Scriptlets/issues/261 + test('Works correctly with different parallel XHR requests', async (assert) => { + const METHOD = 'GET'; + const URL_TO_BLOCK = `${FETCH_OBJECTS_PATH}/test01.json`; + const URL_TO_PASS = `${FETCH_OBJECTS_PATH}/test02.json`; + const MATCH_DATA = ['test01.json']; + + runScriptlet(name, MATCH_DATA); + + const done1 = assert.async(); + const done2 = assert.async(); + + const xhr1 = new XMLHttpRequest(); + const xhr2 = new XMLHttpRequest(); + + xhr1.open(METHOD, URL_TO_PASS); + xhr2.open(METHOD, URL_TO_BLOCK); + + xhr1.onload = () => { + assert.strictEqual(xhr1.readyState, 4, 'Response done'); + assert.ok(xhr1.response, 'Response data exists'); + assert.strictEqual(window.hit, undefined, 'hit should not fire'); + done1(); + }; + + xhr2.onload = () => { + assert.strictEqual(xhr2.readyState, 4, 'Response done'); + assert.strictEqual(typeof xhr2.responseText, 'string', 'Response text mocked'); + assert.strictEqual(window.hit, 'FIRED', 'hit function fired'); + clearGlobalProps('hit'); + done2(); + }; + + xhr1.send(); + // use timeout to avoid hit collisions + setTimeout(() => xhr2.send(), 1); + }); } else { test('unsupported', (assert) => { assert.ok(true, 'Browser does not support it'); diff --git a/tests/scriptlets/trusted-replace-xhr-response.test.js b/tests/scriptlets/trusted-replace-xhr-response.test.js index 47ebdb38e..92ff21717 100644 --- a/tests/scriptlets/trusted-replace-xhr-response.test.js +++ b/tests/scriptlets/trusted-replace-xhr-response.test.js @@ -194,6 +194,47 @@ if (isSupported) { done3(); }); }); + + test('Works correctly with different parallel XHR requests', async (assert) => { + const URL_TO_PASS = `${FETCH_OBJECTS_PATH}/test02.json`; + const INTACT_RESPONSE_PART = 'test'; + + const METHOD = 'GET'; + const URL_TO_BLOCK = `${FETCH_OBJECTS_PATH}/test01.json`; + const PATTERN = '*'; + const REPLACEMENT = ''; + const MATCH_DATA = [PATTERN, REPLACEMENT, 'test01']; + + runScriptlet(name, MATCH_DATA); + + const done1 = assert.async(); + const done2 = assert.async(); + + const xhr1 = new XMLHttpRequest(); + const xhr2 = new XMLHttpRequest(); + + xhr1.open(METHOD, URL_TO_PASS); + xhr2.open(METHOD, URL_TO_BLOCK); + + xhr1.onload = () => { + assert.strictEqual(xhr1.readyState, 4, 'Response done'); + assert.ok(xhr1.response.includes(INTACT_RESPONSE_PART), 'Response is intact'); + assert.strictEqual(window.hit, undefined, 'hit should not fire'); + done1(); + }; + + xhr2.onload = () => { + assert.strictEqual(xhr2.readyState, 4, 'Response done'); + assert.ok(xhr2.response === '', 'Response has been removed'); + + assert.strictEqual(window.hit, 'FIRED', 'hit function fired'); + done2(); + }; + + xhr1.send(); + // use timeout to avoid hit collisions + setTimeout(() => xhr2.send(), 1); + }); } else { test('unsupported', (assert) => { assert.ok(true, 'Browser does not support it');