Skip to content

Commit ca1b6e3

Browse files
committed
AG-55222 Fix 'isMatchingSuspended' in 'trusted-json-set' and 'trusted-replace-argument' scriptlets. #565
Squashed commit of the following: commit cd1b6cc Merge: 938d3d2 a7a8050 Author: Adam Wróblewski <adam@adguard.com> Date: Tue Jun 16 16:22:32 2026 +0200 Merge branch 'master' into fix/AG-55222 commit 938d3d2 Author: Adam Wróblewski <adam@adguard.com> Date: Thu Jun 11 12:52:34 2026 +0200 Wrapped `applyWrapper` and `constructWrapper` in `try/catch` in `trusted-replace-argument` So that if an error occurs while `isMatchingSuspended` is `true`, the flag is reset and the scriptlet remains functional for subsequent calls. Previously, an internal error would permanently disable the scriptlet. commit ad2eff3 Author: Adam Wróblewski <adam@adguard.com> Date: Thu Jun 11 12:13:33 2026 +0200 Fix infinite loop in 'trusted-json-set' and 'trusted-replace-argument' when used with internally used methods
1 parent a7a8050 commit ca1b6e3

5 files changed

Lines changed: 162 additions & 71 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,15 @@ The format is based on [Keep a Changelog], and this project adheres to [Semantic
2828

2929
### Fixed
3030

31+
- Issue with `Maximum call stack size exceeded` error when
32+
`trusted-json-set` or `trusted-replace-argument` is called with method used inside scriptlet [#565].
3133
- Infinite loop in `getRecursiveCandidates` in `json-path-utils` [#563].
3234
- Added the missing `google.ima.dai.api.ui`, `google.ima.dai.api.customUi` surfaces and the
3335
default `StreamRequest.ui` container in the `google-ima3-dai` redirect.
3436
- Added the missing `google.ima.dai.api.StreamRequest.StreamFormat` object to the `google-ima3-dai` redirect.
3537

3638
[Unreleased]: https://github.com/AdguardTeam/Scriptlets/compare/v2.4.2...HEAD
39+
[#565]: https://github.com/AdguardTeam/Scriptlets/issues/565
3740
[#563]: https://github.com/AdguardTeam/Scriptlets/issues/563
3841
[#560]: https://github.com/AdguardTeam/Scriptlets/issues/560
3942
[#529]: https://github.com/AdguardTeam/Scriptlets/issues/529

src/scriptlets/trusted-json-set.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1013,7 +1013,6 @@ export function trustedJsonSet(
10131013
) => {
10141014
try {
10151015
if (isMatchingSuspended) {
1016-
isMatchingSuspended = false;
10171016
return Reflect.apply(target, thisArg, args);
10181017
}
10191018
isMatchingSuspended = true;

src/scriptlets/trusted-replace-argument.ts

Lines changed: 80 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -328,99 +328,109 @@ export function trustedReplaceArgument(
328328
let isMatchingSuspended = false;
329329

330330
const applyWrapper = (target: Function, thisArg: any, argumentsList: unknown[]) => {
331-
if (isMatchingSuspended) {
332-
isMatchingSuspended = false;
333-
return Reflect.apply(target, thisArg, argumentsList);
334-
}
335-
isMatchingSuspended = true;
331+
try {
332+
if (isMatchingSuspended) {
333+
return Reflect.apply(target, thisArg, argumentsList);
334+
}
335+
isMatchingSuspended = true;
336336

337-
// Log the original arguments before modification
338-
if (verbose === 'true') {
339-
const formattedMessage = createFormattedMessage(argumentsList);
340-
logMessage(source, formattedMessage);
341-
}
337+
// Log the original arguments before modification
338+
if (verbose === 'true') {
339+
const formattedMessage = createFormattedMessage(argumentsList);
340+
logMessage(source, formattedMessage);
341+
}
342342

343-
// If we only need to log the arguments, skip further processing
344-
if (SHOULD_LOG_ONLY) {
345-
isMatchingSuspended = false;
346-
return Reflect.apply(target, thisArg, argumentsList);
347-
}
343+
// If we only need to log the arguments, skip further processing
344+
if (SHOULD_LOG_ONLY) {
345+
isMatchingSuspended = false;
346+
return Reflect.apply(target, thisArg, argumentsList);
347+
}
348348

349-
const argumentToReplace = argumentsList[Number(argumentIndex)];
349+
const argumentToReplace = argumentsList[Number(argumentIndex)];
350350

351-
const shouldSetArgument = checkArgument(argumentToReplace);
351+
const shouldSetArgument = checkArgument(argumentToReplace);
352352

353-
if (!shouldSetArgument) {
354-
isMatchingSuspended = false;
355-
return Reflect.apply(target, thisArg, argumentsList);
356-
}
353+
if (!shouldSetArgument) {
354+
isMatchingSuspended = false;
355+
return Reflect.apply(target, thisArg, argumentsList);
356+
}
357357

358-
if (typeof argumentToReplace === 'string' && shouldReplaceArgument) {
359-
argumentsList[Number(argumentIndex)] = argumentToReplace
360-
.replace(replaceRegexValue, constantValue as string);
361-
} else {
362-
argumentsList[Number(argumentIndex)] = constantValue;
363-
}
358+
if (typeof argumentToReplace === 'string' && shouldReplaceArgument) {
359+
argumentsList[Number(argumentIndex)] = argumentToReplace
360+
.replace(replaceRegexValue, constantValue as string);
361+
} else {
362+
argumentsList[Number(argumentIndex)] = constantValue;
363+
}
364364

365-
// Log the modified arguments after replacement
366-
if (verbose === 'true') {
367-
const formattedMessage = createFormattedMessage(argumentsList, 'modified');
368-
logMessage(source, formattedMessage);
369-
}
365+
// Log the modified arguments after replacement
366+
if (verbose === 'true') {
367+
const formattedMessage = createFormattedMessage(argumentsList, 'modified');
368+
logMessage(source, formattedMessage);
369+
}
370370

371-
hit(source);
371+
hit(source);
372372

373-
isMatchingSuspended = false;
373+
isMatchingSuspended = false;
374374

375-
return Reflect.apply(target, thisArg, argumentsList);
375+
return Reflect.apply(target, thisArg, argumentsList);
376+
} catch (error) {
377+
isMatchingSuspended = false;
378+
logMessage(source, `Unexpected error during argument replacement: ${(error as Error).message}`);
379+
return Reflect.apply(target, thisArg, argumentsList);
380+
}
376381
};
377382

378383
const constructWrapper = (target: Function, argumentsList: unknown[], newTarget: any) => {
379-
if (isMatchingSuspended) {
380-
isMatchingSuspended = false;
381-
return Reflect.construct(target, argumentsList, newTarget);
382-
}
383-
isMatchingSuspended = true;
384+
try {
385+
if (isMatchingSuspended) {
386+
return Reflect.construct(target, argumentsList, newTarget);
387+
}
388+
isMatchingSuspended = true;
384389

385-
// Log the original arguments before modification
386-
if (verbose === 'true') {
387-
const formattedMessage = createFormattedMessage(argumentsList);
388-
logMessage(source, formattedMessage);
389-
}
390+
// Log the original arguments before modification
391+
if (verbose === 'true') {
392+
const formattedMessage = createFormattedMessage(argumentsList);
393+
logMessage(source, formattedMessage);
394+
}
390395

391-
// If we only need to log the arguments, skip further processing
392-
if (SHOULD_LOG_ONLY) {
393-
isMatchingSuspended = false;
394-
return Reflect.construct(target, argumentsList, newTarget);
395-
}
396+
// If we only need to log the arguments, skip further processing
397+
if (SHOULD_LOG_ONLY) {
398+
isMatchingSuspended = false;
399+
return Reflect.construct(target, argumentsList, newTarget);
400+
}
396401

397-
const argumentToReplace = argumentsList[Number(argumentIndex)];
402+
const argumentToReplace = argumentsList[Number(argumentIndex)];
398403

399-
const shouldSetArgument = checkArgument(argumentToReplace);
404+
const shouldSetArgument = checkArgument(argumentToReplace);
400405

401-
if (!shouldSetArgument) {
402-
isMatchingSuspended = false;
403-
return Reflect.construct(target, argumentsList, newTarget);
404-
}
406+
if (!shouldSetArgument) {
407+
isMatchingSuspended = false;
408+
return Reflect.construct(target, argumentsList, newTarget);
409+
}
405410

406-
if (typeof argumentToReplace === 'string' && shouldReplaceArgument) {
407-
argumentsList[Number(argumentIndex)] = argumentToReplace
408-
.replace(replaceRegexValue, constantValue as string);
409-
} else {
410-
argumentsList[Number(argumentIndex)] = constantValue;
411-
}
411+
if (typeof argumentToReplace === 'string' && shouldReplaceArgument) {
412+
argumentsList[Number(argumentIndex)] = argumentToReplace
413+
.replace(replaceRegexValue, constantValue as string);
414+
} else {
415+
argumentsList[Number(argumentIndex)] = constantValue;
416+
}
412417

413-
// Log the modified arguments after replacement
414-
if (verbose === 'true') {
415-
const formattedMessage = createFormattedMessage(argumentsList, 'modified');
416-
logMessage(source, formattedMessage);
417-
}
418+
// Log the modified arguments after replacement
419+
if (verbose === 'true') {
420+
const formattedMessage = createFormattedMessage(argumentsList, 'modified');
421+
logMessage(source, formattedMessage);
422+
}
418423

419-
hit(source);
424+
hit(source);
420425

421-
isMatchingSuspended = false;
426+
isMatchingSuspended = false;
422427

423-
return Reflect.construct(target, argumentsList, newTarget);
428+
return Reflect.construct(target, argumentsList, newTarget);
429+
} catch (error) {
430+
isMatchingSuspended = false;
431+
logMessage(source, `Unexpected error during argument replacement: ${(error as Error).message}`);
432+
return Reflect.construct(target, argumentsList, newTarget);
433+
}
424434
};
425435

426436
const getWrapper = (target: Function, propName: string, receiver: any) => {

tests/scriptlets/trusted-json-set.test.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1126,3 +1126,34 @@ test('creates missing nested path in JSON.parse result after earlier parse error
11261126
);
11271127
assert.strictEqual(window.hit, 'FIRED', 'hit function fired');
11281128
});
1129+
1130+
test('modifies thisArg when jsonSource is this and uses property used inside scriptlet', (assert) => {
1131+
runScriptlet(name, ['Object.prototype.hasOwnProperty', '$.*.foo.bar', '$remove$', '', 'this']);
1132+
1133+
const obj = {
1134+
test: {
1135+
foo: {
1136+
bar: 1,
1137+
baz: 1,
1138+
},
1139+
},
1140+
qwerty: 1,
1141+
};
1142+
1143+
// To test if "Maximum call stack size exceeded" error is avoided
1144+
// when the scriptlet uses a property that is also used inside the scriptlet code
1145+
Object.prototype.hasOwnProperty.call(window, 'whatever');
1146+
1147+
// eslint-disable-next-line no-prototype-builtins
1148+
const result = obj.hasOwnProperty('test');
1149+
assert.strictEqual(result, true, 'should check if the object has the property');
1150+
assert.deepEqual(obj, {
1151+
test: {
1152+
foo: {
1153+
baz: 1,
1154+
},
1155+
},
1156+
qwerty: 1,
1157+
}, 'should modify the thisArg object by removing the specified property');
1158+
assert.strictEqual(window.hit, 'FIRED', 'hit function fired');
1159+
});

tests/scriptlets/trusted-replace-argument.test.js

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const nativeDocumentQuerySelector = window.document.querySelector;
1212
const nativeDocumentQuerySelectorAll = window.document.querySelectorAll;
1313
const nativeMutationObserver = window.MutationObserver;
1414
const nativeObjectDefineProperty = window.Object.defineProperty;
15+
const nativeStringReplace = String.prototype.replace;
1516

1617
const beforeEach = () => {
1718
window.__debug = () => {
@@ -29,6 +30,7 @@ const afterEach = () => {
2930
window.document.querySelectorAll = nativeDocumentQuerySelectorAll;
3031
window.MutationObserver = nativeMutationObserver;
3132
window.Object.defineProperty = nativeObjectDefineProperty;
33+
window.String.prototype.replace = nativeStringReplace;
3234
};
3335

3436
module(name, { beforeEach, afterEach });
@@ -242,3 +244,49 @@ test('Replace argument in Object.defineProperty if pattern matches, test for "js
242244
assert.strictEqual(objectToReplace.adblock, expected, `"The property 'adblock' should be '${expected}'`);
243245
assert.strictEqual(window.hit, 'FIRED', 'hit function fired');
244246
});
247+
248+
test('Replace argument in JSON.parse after earlier error inside the scriptlet', (assert) => {
249+
// Test to check if the scriptlet works correctly
250+
// after an error occurred inside the scriptlet code while matching was suspended
251+
runScriptlet(name, ['JSON.parse', '0', 'replace:/ads/no_ads/g', 'ads']);
252+
253+
try {
254+
// String conversion of the argument throws,
255+
// so the error occurs inside the scriptlet during pattern matching
256+
const throwingArgument = () => {};
257+
throwingArgument.toString = () => {
258+
throw new Error('test error');
259+
};
260+
JSON.parse(throwingArgument);
261+
} catch (error) {
262+
console.error('An error occurred:', error);
263+
}
264+
265+
const jsonString = '{ "ads1": 1, "content": "fooBar" }';
266+
const result = JSON.parse(jsonString);
267+
268+
assert.deepEqual(result, { no_ads1: 1, content: 'fooBar' }, 'Replaced "ads" with "no_ads" after the error');
269+
assert.strictEqual(
270+
JSON.parse.toString(),
271+
nativeJSONParse.toString(),
272+
'JSON.parse.toString() returns the original value',
273+
);
274+
assert.strictEqual(window.hit, 'FIRED', 'hit function fired');
275+
});
276+
277+
test('Replace argument in String.prototype.replace to "test" if pattern matches', (assert) => {
278+
// Test to check if "Maximum call stack size exceeded" error is avoided
279+
// when the scriptlet uses a property that is also used inside the scriptlet code
280+
runScriptlet(name, ['String.prototype.replace', '1', 'test', 'foo']);
281+
282+
const string = 'regex';
283+
const result = string.replace(/regex/, 'foo bar');
284+
285+
assert.strictEqual(result, 'test', 'The scriptlet should change the method result');
286+
assert.strictEqual(
287+
String.prototype.replace.toString(),
288+
nativeStringReplace.toString(),
289+
'String.prototype.replace.toString() returns the original value',
290+
);
291+
assert.strictEqual(window.hit, 'FIRED', 'hit function fired');
292+
});

0 commit comments

Comments
 (0)