Skip to content

Commit

Permalink
AG-27573 Fix set-constant - set proxy trap only once #380
Browse files Browse the repository at this point in the history
Squashed commit of the following:

commit d325bd7
Merge: 95fded6 f5c5921
Author: Adam Wróblewski <[email protected]>
Date:   Mon Nov 13 13:16:59 2023 +0100

    Merge branch 'master' into fix/AG-27573

commit 95fded6
Author: Slava Leleka <[email protected]>
Date:   Fri Nov 10 20:27:54 2023 +0300

    Update test

commit b30f892
Author: Slava Leleka <[email protected]>
Date:   Fri Nov 10 20:27:42 2023 +0300

    Update test

commit f76eaf3
Author: Adam Wróblewski <[email protected]>
Date:   Fri Nov 10 17:57:42 2023 +0100

    Update test

commit f5543e8
Author: Adam Wróblewski <[email protected]>
Date:   Fri Nov 10 15:58:22 2023 +0100

    Fix set-constant - set proxy trap only once
  • Loading branch information
AdamWr committed Nov 13, 2023
1 parent f5c5921 commit b153be6
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 14 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- issue with setting proxy trap every time when property is accessed in `set-constant` scriptlet
[#380](https://github.com/AdguardTeam/Scriptlets/issues/380)
- issue with `stack` in `evaldata-prune` scriptlet [#378](https://github.com/AdguardTeam/Scriptlets/issues/378)
- issue with setting values to wrong properties in `set-constant` scriptlet
[#373](https://github.com/AdguardTeam/Scriptlets/issues/373)
Expand Down
33 changes: 19 additions & 14 deletions src/scriptlets/set-constant.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ export function setConstant(source, property, value, stack = '', valueWrapper =
return;
}

let isProxyTrapSet = false;

const emptyArr = noopArray();
const emptyObj = noopObject();

Expand Down Expand Up @@ -297,20 +299,23 @@ export function setConstant(source, property, value, stack = '', valueWrapper =
// Get properties which should be checked and remove first one
// because it's current object
const propertiesToCheck = property.split('.').slice(1);
a = new Proxy(a, {
get: (target, propertyKey, val) => {
// Check if object contains required property, if so
// check if current value is equal to constantValue, if not, set it to constantValue
propertiesToCheck.reduce((object, currentProp, index, array) => {
const currentObj = object?.[currentProp];
if (currentObj && index === array.length - 1 && currentObj !== constantValue) {
object[currentProp] = constantValue;
}
return currentObj || object;
}, target);
return Reflect.get(target, propertyKey, val);
},
});
if (!isProxyTrapSet) {
isProxyTrapSet = true;
a = new Proxy(a, {
get: (target, propertyKey, val) => {
// Check if object contains required property, if so
// check if current value is equal to constantValue, if not, set it to constantValue
propertiesToCheck.reduce((object, currentProp, index, array) => {
const currentObj = object?.[currentProp];
if (currentObj && index === array.length - 1 && currentObj !== constantValue) {
object[currentProp] = constantValue;
}
return currentObj || object;
}, target);
return Reflect.get(target, propertyKey, val);
},
});
}
}
handler.set(a);
},
Expand Down
35 changes: 35 additions & 0 deletions tests/scriptlets/set-constant.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -795,4 +795,39 @@ if (!isSupported) {
assert.strictEqual(window.something.start, undefined, 'something.start was not set');
clearGlobalProps('something');
});

test('Check if proxy was not set many times', (assert) => {
runScriptletFromTag('proxy.test.abc', 'trueFunc');

// Expected number of calls to getOwnPropertyDescriptor
const EXPECTED_NUMBER = 4;

let number = 0;
window.proxy = window.proxy || {};

const handler = {
getOwnPropertyDescriptor(target, prop) {
number += 1;
return Reflect.getOwnPropertyDescriptor(target, prop);
},
};

window.proxy = new Proxy(window.proxy, handler);

// Access object 100 times, if getOwnPropertyDescriptor was called more than 4 times
// it means that proxy was set more than once
for (let index = 0; index < 100; index += 1) {
if (number > EXPECTED_NUMBER) {
break;
}
window.proxy = window.proxy || {};
}
window.proxy.test = {
abc: () => false,
};
const result = window.proxy.test.abc();
assert.strictEqual(result, true, 'proxy.test.abc set to true by scriptlet');
assert.strictEqual(number, EXPECTED_NUMBER, `getOwnPropertyDescriptor was called ${EXPECTED_NUMBER} times`);
clearGlobalProps('proxy');
});
}

0 comments on commit b153be6

Please sign in to comment.