AG-17905 fix closure bug for prevent-xhr and trusted-replace-xhr-resp…

…onse #261 Merge in ADGUARD-FILTERS/scriptlets from fix/AG-17905 to release/v1.8 Squashed commit of the following: commit 09626a3 Merge: 25c9820 f0fbd11 Author: Stanislav A <s.atroschenko@adguard.com> Date: Tue Dec 20 16:53:59 2022 +0300 Merge branch 'release/v1.8' into fix/AG-17905 commit 25c9820 Author: Stanislav A <s.atroschenko@adguard.com> Date: Tue Dec 20 13:29:55 2022 +0300 update changelog and fix typos in test commit 6973376 Author: Stanislav A <s.atroschenko@adguard.com> Date: Mon Dec 19 22:10:18 2022 +0300 add tests commit 492da8f Author: Stanislav A <s.atroschenko@adguard.com> Date: Fri Dec 16 21:40:20 2022 +0300 fix closure bug for trusted-replace-xhr-response commit 4f1d59b Author: Stanislav A <s.atroschenko@adguard.com> Date: Fri Dec 16 21:24:30 2022 +0300 fix closure bug for prevent-xhr and add testcase
AdguardTeam · Dec 20, 2022 · 2c10348 · 2c10348
1 parent f0fbd11
commit 2c10348
Show file tree

Hide file tree

Showing 5 changed files with 128 additions and 42 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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');