From cc125742f7921ccbb8d60b946610339cf8884d1a Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Sat, 16 Dec 2023 01:53:15 +0000 Subject: [PATCH] Use `__defineSetter__` instead of `toLocaleString` --- lib/init/with.js | 50 ++++++++++++++++++--------------- lib/instrument/visitors/with.js | 6 ++-- test/with.test.js | 26 +++++++++++++---- 3 files changed, 50 insertions(+), 32 deletions(-) diff --git a/lib/init/with.js b/lib/init/with.js index bc4f7d86..5162b32c 100644 --- a/lib/init/with.js +++ b/lib/init/with.js @@ -3,6 +3,8 @@ * Functions to handle `with` statements * ------------------*/ +/* eslint-disable no-restricted-properties, no-extend-native */ + 'use strict'; // Modules @@ -27,7 +29,7 @@ let withState; * * 1. Tracker calls need to get the values of vars outside the `with ()`, * but `with ()` could block access to them. - * 2. `with ()` can block access to Livepack's internal vars (e.g. `livepack_tracker`) + * 2. `with ()` can block access to Livepack's internal vars (e.g. `livepack_tracker`), * causing Livepack's internal mechanisms to malfunction. * * In both cases, it's necessary to "smuggle" the values of variables outside `with ()` to inside, @@ -37,8 +39,8 @@ let withState; * and then retrieve them inside `with ()`. * * To avoid this being visible to user code, repurpose an existing global function - * `Object.prototype.toLocaleString`. `toLocaleString` is chosen because it's fairly obscure, - * but could use any method. + * `Object.prototype.__defineSetter__`. + * `__defineSetter__` is chosen because it's fairly obscure, but could use any method. * * `with (obj) x();` is instrumented as: * @@ -49,11 +51,11 @@ let withState; * (eval, livepack_temp_3) => eval(livepack_temp_3), * () => eval * ) - * ) with ( {}.toLocaleString() ) x(); + * ) with ( {}.__defineSetter__() ) x(); * ``` * - * `wrapWith()` stores the 2 functions in `withState`, and `{}.toLocaleString()` retrieves them again. - * `toLocaleString()` returns a Proxy which allows dynamically overriding the original `with ()` + * `wrapWith()` stores the 2 functions in `withState`, and `{}.__defineSetter__()` retrieves them again. + * `__defineSetter__()` returns a Proxy which allows dynamically overriding the original `with ()` * by using the 2 functions `wrapWith()` was called with to access any var outside the `with ()`. * This Proxy is used as the value for a 2nd `with ()` which is inserted inside the original. * @@ -66,16 +68,16 @@ function addWrapWithFunctionToTracker(tracker, prefixNum) { // Don't do anything if `null` or `undefined`, as it will error if (withValue == null) return withValue; - // Throw an error if user has changed value of `Object.prototype.toLocaleString` - const descriptor = Object.getOwnPropertyDescriptor(Object.prototype, 'toLocaleString'); + // Throw an error if user has changed value of `Object.prototype.__defineSetter__` + const descriptor = Object.getOwnPropertyDescriptor(Object.prototype, '__defineSetter__'); assert( // eslint-disable-next-line no-use-before-define - descriptor?.value === shimmedToLocaleString, - 'Livepack shims `Object.prototype.toLocaleString` to instrument `with` statements.' - + "It has been altered in use code, which prevents Livepack's correct functioning." + descriptor?.value === shimmedDefineSetter, + 'Livepack shims `Object.prototype.__defineSetter__` to instrument `with` statements.' + + "It has been altered in user code, which prevents Livepack's correct functioning." ); - // Store state for shimmed `toLocaleString` to retrieve + // Store state for shimmed `__defineSetter__` to retrieve withState = [prefixNum, runEval, getEval]; // Return original `with` value @@ -83,17 +85,18 @@ function addWrapWithFunctionToTracker(tracker, prefixNum) { }; } -// Shim `Object.prototype.toLocaleString`. +// Shim `Object.prototype.__defineSetter__`. // NB: This code runs before globals are catalogued. // Define replacement as a method, as original does not have a `prototype` property. -const {toLocaleString} = Object.prototype; -const shimmedToLocaleString = { - toLocaleString() { +// 2 function params to maintain original's `.length` property. +const defineSetter = Object.prototype.__defineSetter__; +const shimmedDefineSetter = { + __defineSetter__(_x, _y) { // eslint-disable-line no-unused-vars // If being used normally, defer to original // eslint-disable-next-line prefer-rest-params - if (!withState) return toLocaleString.apply(this, arguments); + if (!withState) return defineSetter.apply(this, arguments); - // Is being used to augment `with`. + // Is being used to smuggle values into `with ()`. // Get state previously stored by `wrapWith()`. const [prefixNum, runEval, getEval] = withState; withState = undefined; @@ -106,7 +109,7 @@ const shimmedToLocaleString = { // 2. Var being accessed is one of Livepack's internal vars. // In these cases, intercept access which would otherwise hit the outer `with ()`, // and instead use the `runEval()` function to get/set the variable outside `with ()`. - // If var being accessed is itself called `eval`, use `getEval()` instead. + // If var being accessed is called `eval`, use `getEval()` instead. return new Proxy(Object.create(null), { has(target, key) { return getIsGettingScope() || key.startsWith(internalVarsPrefix); @@ -115,12 +118,13 @@ const shimmedToLocaleString = { return key === 'eval' ? getEval() : runEval(nativeEval, key); }, set(target, key, value) { - const paramName = key === 'a' ? 'b' : 'a'; - const set = runEval(nativeEval, `${paramName} => ${key} = ${paramName}`); + // Only used for setting internal temp vars, so no need to handle if key is `v` or `eval` + const set = runEval(nativeEval, `v => ${key} = v`); set(value); return true; } }); } -}.toLocaleString; -Object.prototype.toLocaleString = shimmedToLocaleString; // eslint-disable-line no-extend-native +}.__defineSetter__; + +Object.prototype.__defineSetter__ = shimmedDefineSetter; diff --git a/lib/instrument/visitors/with.js b/lib/instrument/visitors/with.js index e5d2cab8..e063634c 100644 --- a/lib/instrument/visitors/with.js +++ b/lib/instrument/visitors/with.js @@ -74,7 +74,7 @@ function instrumentWithObj(node, binding, state) { // (eval, livepack_temp_3) => eval(livepack_temp_3), // () => eval // ) - // ) with ( {}.toLocaleString() ) foo; + // ) with ( {}.__defineSetter__() ) foo; // ``` const tempVarNode = createTempVarNode(state), evalNode = t.identifier('eval'); @@ -96,9 +96,9 @@ function instrumentWithObj(node, binding, state) { ] ); - // `foo;` -> `with ( {}.toLocaleString() ) foo;` + // `foo;` -> `with ( {}.__defineSetter__() ) foo;` node.body = t.withStatement( - t.callExpression(t.memberExpression(t.objectExpression([]), t.identifier('toLocaleString')), []), + t.callExpression(t.memberExpression(t.objectExpression([]), t.identifier('__defineSetter__')), []), node.body ); } diff --git a/test/with.test.js b/test/with.test.js index 282c76eb..fce1d5b2 100644 --- a/test/with.test.js +++ b/test/with.test.js @@ -564,13 +564,27 @@ describe('with statements', () => { }); }); - itSerializesEqual('Shimming `Object.prototype.toLocaleString` does not prevent serializing it', { - in: () => Object.prototype.toLocaleString, - out: 'Object.prototype.toLocaleString', - validate(toLocaleString) { - expect(toLocaleString).toBe(Object.prototype.toLocaleString); - } + /* eslint-disable no-restricted-properties */ + describe('shimming `Object.prototype.__defineSetter__`', () => { + it('does not interfere with its normal functioning', () => { + const callee = spy(); + const obj = {}; + obj.__defineSetter__('foo', callee); + expect(callee).not.toHaveBeenCalled(); + obj.foo = 123; + expect(callee).toHaveBeenCalledOnce(); + expect(callee).toHaveBeenCalledWith(123); + }); + + itSerializesEqual('does not prevent serializing it', { + in: () => Object.prototype.__defineSetter__, + out: 'Object.prototype.__defineSetter__', + validate(defineSetter) { + expect(defineSetter).toBe(Object.prototype.__defineSetter__); + } + }); }); + /* eslint-enable no-restricted-properties */ }); // TODO: Test for access to `eval` - as a global, and as local var.