From 409158c919e26bcd9a09aad914ed4e3fb52d470f Mon Sep 17 00:00:00 2001 From: LiviaMedeiros Date: Wed, 13 Jul 2022 01:56:08 +0800 Subject: [PATCH] test: add `common.mustNotMutateObjectDeep()` This function returns a Proxy object that throws on attempt to mutate it Functions and primitives are returned directly PR-URL: https://github.com/nodejs/node/pull/43196 Reviewed-By: Darshan Sen Reviewed-By: Antoine du Hamel --- test/common/README.md | 35 +++ test/common/index.js | 47 ++++ test/common/index.mjs | 2 + ...est-common-must-not-mutate-object-deep.mjs | 225 ++++++++++++++++++ 4 files changed, 309 insertions(+) create mode 100644 test/parallel/test-common-must-not-mutate-object-deep.mjs diff --git a/test/common/README.md b/test/common/README.md index 60462345abf46c..517fce47cd2bd4 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -299,6 +299,40 @@ If `fn` is not provided, an empty function will be used. Returns a function that triggers an `AssertionError` if it is invoked. `msg` is used as the error message for the `AssertionError`. +### `mustNotMutateObjectDeep([target])` + +* `target` [\][] default = `undefined` +* return [\][] + +If `target` is an Object, returns a proxy object that triggers +an `AssertionError` on mutation attempt, including mutation of deeply nested +Objects. Otherwise, it returns `target` directly. + +Use of this function is encouraged for relevant regression tests. + +```mjs +import { open } from 'node:fs/promises'; +import { mustNotMutateObjectDeep } from '../common/index.mjs'; + +const _mutableOptions = { length: 4, position: 8 }; +const options = mustNotMutateObjectDeep(_mutableOptions); + +// In filehandle.read or filehandle.write, attempt to mutate options will throw +// In the test code, options can still be mutated via _mutableOptions +const fh = await open('/path/to/file', 'r+'); +const { buffer } = await fh.read(options); +_mutableOptions.position = 4; +await fh.write(buffer, options); + +// Inline usage +const stats = await fh.stat(mustNotMutateObjectDeep({ bigint: true })); +console.log(stats.size); +``` + +Caveats: built-in objects that make use of their internal slots (for example, +`Map`s and `Set`s) might not work with this function. It returns Functions +directly, not preventing their mutation. + ### `mustSucceed([fn])` * `fn` [\][] default = () => {} @@ -1024,6 +1058,7 @@ See [the WPT tests README][] for details. []: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function []: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object []: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp +[]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Data_types []: https://github.com/tc39/proposal-bigint []: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Boolean_type []: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Number_type diff --git a/test/common/index.js b/test/common/index.js index 2f056c96e8eb85..1b62c9610893e5 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -498,6 +498,52 @@ function mustNotCall(msg) { }; } +const _mustNotMutateObjectDeepProxies = new WeakMap(); + +function mustNotMutateObjectDeep(original) { + // Return primitives and functions directly. Primitives are immutable, and + // proxied functions are impossible to compare against originals, e.g. with + // `assert.deepEqual()`. + if (original === null || typeof original !== 'object') { + return original; + } + + const cachedProxy = _mustNotMutateObjectDeepProxies.get(original); + if (cachedProxy) { + return cachedProxy; + } + + const _mustNotMutateObjectDeepHandler = { + __proto__: null, + defineProperty(target, property, descriptor) { + assert.fail(`Expected no side effects, got ${inspect(property)} ` + + 'defined'); + }, + deleteProperty(target, property) { + assert.fail(`Expected no side effects, got ${inspect(property)} ` + + 'deleted'); + }, + get(target, prop, receiver) { + return mustNotMutateObjectDeep(Reflect.get(target, prop, receiver)); + }, + preventExtensions(target) { + assert.fail('Expected no side effects, got extensions prevented on ' + + inspect(target)); + }, + set(target, property, value, receiver) { + assert.fail(`Expected no side effects, got ${inspect(value)} ` + + `assigned to ${inspect(property)}`); + }, + setPrototypeOf(target, prototype) { + assert.fail(`Expected no side effects, got set prototype to ${prototype}`); + } + }; + + const proxy = new Proxy(original, _mustNotMutateObjectDeepHandler); + _mustNotMutateObjectDeepProxies.set(original, proxy); + return proxy; +} + function printSkipMessage(msg) { console.log(`1..0 # Skipped: ${msg}`); } @@ -806,6 +852,7 @@ const common = { mustCall, mustCallAtLeast, mustNotCall, + mustNotMutateObjectDeep, mustSucceed, nodeProcessAborted, PIPE, diff --git a/test/common/index.mjs b/test/common/index.mjs index ec181dcacb4d72..a3a34ae7f04435 100644 --- a/test/common/index.mjs +++ b/test/common/index.mjs @@ -35,6 +35,7 @@ const { canCreateSymLink, getCallSite, mustNotCall, + mustNotMutateObjectDeep, printSkipMessage, skip, nodeProcessAborted, @@ -81,6 +82,7 @@ export { canCreateSymLink, getCallSite, mustNotCall, + mustNotMutateObjectDeep, printSkipMessage, skip, nodeProcessAborted, diff --git a/test/parallel/test-common-must-not-mutate-object-deep.mjs b/test/parallel/test-common-must-not-mutate-object-deep.mjs new file mode 100644 index 00000000000000..76a7ad67641bbc --- /dev/null +++ b/test/parallel/test-common-must-not-mutate-object-deep.mjs @@ -0,0 +1,225 @@ +import { mustNotMutateObjectDeep } from '../common/index.mjs'; +import assert from 'node:assert'; +import { promisify } from 'node:util'; + +// Test common.mustNotMutateObjectDeep() + +const original = { + foo: { bar: 'baz' }, + qux: null, + quux: [ + 'quuz', + { corge: 'grault' }, + ], +}; + +// Make a copy to make sure original doesn't get altered by the function itself. +const backup = JSON.parse(JSON.stringify(original)); + +// Wrapper for convenience: +const obj = () => mustNotMutateObjectDeep(original); + +function testOriginal(root) { + assert.deepStrictEqual(root, backup); + return root.foo.bar === 'baz' && root.quux[1].corge.length === 6; +} + +function definePropertyOnRoot(root) { + Object.defineProperty(root, 'xyzzy', {}); +} + +function definePropertyOnFoo(root) { + Object.defineProperty(root.foo, 'xyzzy', {}); +} + +function deletePropertyOnRoot(root) { + delete root.foo; +} + +function deletePropertyOnFoo(root) { + delete root.foo.bar; +} + +function preventExtensionsOnRoot(root) { + Object.preventExtensions(root); +} + +function preventExtensionsOnFoo(root) { + Object.preventExtensions(root.foo); +} + +function preventExtensionsOnRootViaSeal(root) { + Object.seal(root); +} + +function preventExtensionsOnFooViaSeal(root) { + Object.seal(root.foo); +} + +function preventExtensionsOnRootViaFreeze(root) { + Object.freeze(root); +} + +function preventExtensionsOnFooViaFreeze(root) { + Object.freeze(root.foo); +} + +function setOnRoot(root) { + root.xyzzy = 'gwak'; +} + +function setOnFoo(root) { + root.foo.xyzzy = 'gwak'; +} + +function setQux(root) { + root.qux = 'gwak'; +} + +function setQuux(root) { + root.quux.push('gwak'); +} + +function setQuuxItem(root) { + root.quux[0] = 'gwak'; +} + +function setQuuxProperty(root) { + root.quux[1].corge = 'gwak'; +} + +function setPrototypeOfRoot(root) { + Object.setPrototypeOf(root, Array); +} + +function setPrototypeOfFoo(root) { + Object.setPrototypeOf(root.foo, Array); +} + +function setPrototypeOfQuux(root) { + Object.setPrototypeOf(root.quux, Array); +} + + +{ + assert.ok(testOriginal(obj())); + + assert.throws( + () => definePropertyOnRoot(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => definePropertyOnFoo(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => deletePropertyOnRoot(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => deletePropertyOnFoo(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => preventExtensionsOnRoot(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => preventExtensionsOnFoo(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => preventExtensionsOnRootViaSeal(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => preventExtensionsOnFooViaSeal(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => preventExtensionsOnRootViaFreeze(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => preventExtensionsOnFooViaFreeze(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => setOnRoot(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => setOnFoo(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => setQux(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => setQuux(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => setQuux(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => setQuuxItem(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => setQuuxProperty(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => setPrototypeOfRoot(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => setPrototypeOfFoo(obj()), + { code: 'ERR_ASSERTION' } + ); + assert.throws( + () => setPrototypeOfQuux(obj()), + { code: 'ERR_ASSERTION' } + ); + + // Test that no mutation happened: + assert.ok(testOriginal(obj())); +} + +// Test various supported types, directly and nested: +[ + undefined, null, false, true, 42, 42n, Symbol('42'), NaN, Infinity, {}, [], + () => {}, async () => {}, Promise.resolve(), Math, Object.create(null), +].forEach((target) => { + assert.deepStrictEqual(mustNotMutateObjectDeep(target), target); + assert.deepStrictEqual(mustNotMutateObjectDeep({ target }), { target }); + assert.deepStrictEqual(mustNotMutateObjectDeep([ target ]), [ target ]); +}); + +// Test that passed functions keep working correctly: +{ + const fn = () => 'blep'; + fn.foo = {}; + const fnImmutableView = mustNotMutateObjectDeep(fn); + assert.deepStrictEqual(fnImmutableView, fn); + + // Test that the function still works: + assert.strictEqual(fn(), 'blep'); + assert.strictEqual(fnImmutableView(), 'blep'); + + // Test that the original function is not deeply frozen: + fn.foo.bar = 'baz'; + assert.strictEqual(fn.foo.bar, 'baz'); + assert.strictEqual(fnImmutableView.foo.bar, 'baz'); + + // Test the original function is not frozen: + fn.qux = 'quux'; + assert.strictEqual(fn.qux, 'quux'); + assert.strictEqual(fnImmutableView.qux, 'quux'); + + // Redefining util.promisify.custom also works: + promisify(mustNotMutateObjectDeep(promisify(fn))); +}