From 70de19e2ccdd6e39a1b4417d7c614d32d742fdd3 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Wed, 4 Sep 2024 11:58:49 +0100 Subject: [PATCH] vm: add vm proto property lookup test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add more test coverage on vm prototype properties lookup with `in` operator and property access. PR-URL: https://github.com/nodejs/node/pull/54606 Refs: https://github.com/nodejs/node/issues/54436 Reviewed-By: Michaël Zasso Reviewed-By: Joyee Cheung Reviewed-By: James M Snell --- .../test-vm-global-property-enumerator.js | 14 ++ .../test-vm-global-property-prototype.js | 224 +++++++++++++++--- 2 files changed, 207 insertions(+), 31 deletions(-) diff --git a/test/parallel/test-vm-global-property-enumerator.js b/test/parallel/test-vm-global-property-enumerator.js index 7443c2c27d6..140baf692c3 100644 --- a/test/parallel/test-vm-global-property-enumerator.js +++ b/test/parallel/test-vm-global-property-enumerator.js @@ -8,6 +8,9 @@ const assert = require('assert'); const cases = [ { + get 1() { + return 'value'; + }, get key() { return 'value'; }, @@ -16,15 +19,21 @@ const cases = [ // Intentionally single setter. // eslint-disable-next-line accessor-pairs set key(value) {}, + // eslint-disable-next-line accessor-pairs + set 1(value) {}, }, {}, { key: 'value', + 1: 'value', }, (new class GetterObject { get key() { return 'value'; } + get 1() { + return 'value'; + } }()), (new class SetterObject { // Intentionally single setter. @@ -32,12 +41,17 @@ const cases = [ set key(value) { // noop } + // eslint-disable-next-line accessor-pairs + set 1(value) { + // noop + } }()), [], [['key', 'value']], { __proto__: { key: 'value', + 1: 'value', }, }, (() => { diff --git a/test/parallel/test-vm-global-property-prototype.js b/test/parallel/test-vm-global-property-prototype.js index fe8abc8be45..0a854c43a8d 100644 --- a/test/parallel/test-vm-global-property-prototype.js +++ b/test/parallel/test-vm-global-property-prototype.js @@ -3,48 +3,110 @@ require('../common'); const assert = require('assert'); const vm = require('vm'); +const outerProto = { + onOuterProto: 'onOuterProto', + bothProto: 'onOuterProto', +}; +function onOuterProtoGetter() { + return 'onOuterProtoGetter'; +} +Object.defineProperties(outerProto, { + onOuterProtoGetter: { + get: onOuterProtoGetter, + }, + bothProtoGetter: { + get: onOuterProtoGetter, + }, + // outer proto indexed + 0: { + value: 'onOuterProtoIndexed', + writable: false, + enumerable: false, + configurable: true, + }, + // both proto indexed + 3: { + value: 'onOuterProtoIndexed', + writable: false, + enumerable: false, + configurable: true, + }, +}); + +// Creating a new intermediate proto to mimic the +// window -> Window.prototype -> EventTarget.prototype chain in JSDom. +const sandboxProto = { + __proto__: outerProto, +}; + const sandbox = { + __proto__: sandboxProto, onSelf: 'onSelf', }; function onSelfGetter() { return 'onSelfGetter'; } - -Object.defineProperty(sandbox, 'onSelfGetter', { - get: onSelfGetter, -}); - -Object.defineProperty(sandbox, 1, { - value: 'onSelfIndexed', - writable: false, - enumerable: false, - configurable: true, +Object.defineProperties(sandbox, { + onSelfGetter: { + get: onSelfGetter, + }, + 1: { + value: 'onSelfIndexed', + writable: false, + enumerable: false, + configurable: true, + } }); const ctx = vm.createContext(sandbox); const result = vm.runInContext(` -Object.prototype.onProto = 'onProto'; -Object.defineProperty(Object.prototype, 'onProtoGetter', { - get() { - return 'onProtoGetter'; +Object.prototype.onInnerProto = 'onInnerProto'; +Object.defineProperties(Object.prototype, { + onInnerProtoGetter: { + get() { + return 'onInnerProtoGetter'; + }, + }, + 2: { + value: 'onInnerProtoIndexed', + writable: false, + enumerable: false, + configurable: true, }, }); -Object.defineProperty(Object.prototype, 2, { - value: 'onProtoIndexed', - writable: false, - enumerable: false, - configurable: true, + +// Override outer proto properties +Object.prototype.bothProto = 'onInnerProto'; +Object.defineProperties(Object.prototype, { + bothProtoGetter: { + get() { + return 'onInnerProtoGetter'; + }, + }, + // outer proto indexed + 3: { + value: 'onInnerProtoIndexed', + writable: false, + enumerable: false, + configurable: true, + }, }); const resultHasOwn = { onSelf: Object.hasOwn(this, 'onSelf'), onSelfGetter: Object.hasOwn(this, 'onSelfGetter'), onSelfIndexed: Object.hasOwn(this, 1), - onProto: Object.hasOwn(this, 'onProto'), - onProtoGetter: Object.hasOwn(this, 'onProtoGetter'), - onProtoIndexed: Object.hasOwn(this, 2), + onOuterProto: Object.hasOwn(this, 'onOuterProto'), + onOuterProtoGetter: Object.hasOwn(this, 'onOuterProtoGetter'), + onOuterProtoIndexed: Object.hasOwn(this, 0), + onInnerProto: Object.hasOwn(this, 'onInnerProto'), + onInnerProtoGetter: Object.hasOwn(this, 'onInnerProtoGetter'), + onInnerProtoIndexed: Object.hasOwn(this, 2), + bothProto: Object.hasOwn(this, 'bothProto'), + bothProtoGetter: Object.hasOwn(this, 'bothProtoGetter'), + bothProtoIndexed: Object.hasOwn(this, 3), }; const getDesc = (prop) => Object.getOwnPropertyDescriptor(this, prop); @@ -52,13 +114,49 @@ const resultDesc = { onSelf: getDesc('onSelf'), onSelfGetter: getDesc('onSelfGetter'), onSelfIndexed: getDesc(1), - onProto: getDesc('onProto'), - onProtoGetter: getDesc('onProtoGetter'), - onProtoIndexed: getDesc(2), + onOuterProto: getDesc('onOuterProto'), + onOuterProtoGetter: getDesc('onOuterProtoGetter'), + onOuterProtoIndexed: getDesc(0), + onInnerProto: getDesc('onInnerProto'), + onInnerProtoGetter: getDesc('onInnerProtoGetter'), + onInnerProtoIndexed: getDesc(2), + bothProto: getDesc('bothProto'), + bothProtoGetter: getDesc('bothProtoGetter'), + bothProtoIndexed: getDesc(3), +}; +const resultIn = { + onSelf: 'onSelf' in this, + onSelfGetter: 'onSelfGetter' in this, + onSelfIndexed: 1 in this, + onOuterProto: 'onOuterProto' in this, + onOuterProtoGetter: 'onOuterProtoGetter' in this, + onOuterProtoIndexed: 0 in this, + onInnerProto: 'onInnerProto' in this, + onInnerProtoGetter: 'onInnerProtoGetter' in this, + onInnerProtoIndexed: 2 in this, + bothProto: 'bothProto' in this, + bothProtoGetter: 'bothProtoGetter' in this, + bothProtoIndexed: 3 in this, +}; +const resultValue = { + onSelf: this.onSelf, + onSelfGetter: this.onSelfGetter, + onSelfIndexed: this[1], + onOuterProto: this.onOuterProto, + onOuterProtoGetter: this.onOuterProtoGetter, + onOuterProtoIndexed: this[0], + onInnerProto: this.onInnerProto, + onInnerProtoGetter: this.onInnerProtoGetter, + onInnerProtoIndexed: this[2], + bothProto: this.bothProto, + bothProtoGetter: this.bothProtoGetter, + bothProtoIndexed: this[3], }; ({ resultHasOwn, resultDesc, + resultIn, + resultValue, }); `, ctx); @@ -68,16 +166,80 @@ assert.deepEqual(result, { onSelf: true, onSelfGetter: true, onSelfIndexed: true, - onProto: false, - onProtoGetter: false, - onProtoIndexed: false, + + // All prototype properties are not own properties. + onOuterProto: false, + onOuterProtoGetter: false, + onOuterProtoIndexed: false, + onInnerProto: false, + onInnerProtoGetter: false, + onInnerProtoIndexed: false, + bothProto: false, + bothProtoGetter: false, + bothProtoIndexed: false, }, resultDesc: { onSelf: { value: 'onSelf', writable: true, enumerable: true, configurable: true }, onSelfGetter: { get: onSelfGetter, set: undefined, enumerable: false, configurable: false }, onSelfIndexed: { value: 'onSelfIndexed', writable: false, enumerable: false, configurable: true }, - onProto: undefined, - onProtoGetter: undefined, - onProtoIndexed: undefined, + + // All prototype properties are not own properties. + onOuterProto: undefined, + onOuterProtoGetter: undefined, + onOuterProtoIndexed: undefined, + onInnerProto: undefined, + onInnerProtoGetter: undefined, + onInnerProtoIndexed: undefined, + bothProto: undefined, + bothProtoGetter: undefined, + bothProtoIndexed: undefined, + }, + resultIn: { + onSelf: true, + onSelfGetter: true, + onSelfIndexed: true, + + // Only properties exist on inner prototype chain will be looked up + // on `in` operator. In the VM Context, the prototype chain will be like: + // ``` + // Object + // ^ + // | prototype + // InnerPrototype + // ^ + // | prototype + // globalThis + // ``` + // Outer prototype is not in the inner global object prototype chain and it + // will not be looked up on `in` operator. + onOuterProto: false, + onOuterProtoGetter: false, + onOuterProtoIndexed: false, + onInnerProto: true, + onInnerProtoGetter: true, + onInnerProtoIndexed: true, + bothProto: true, + bothProtoGetter: true, + bothProtoIndexed: true, + }, + resultValue: { + onSelf: 'onSelf', + onSelfGetter: 'onSelfGetter', + onSelfIndexed: 'onSelfIndexed', + + // FIXME(legendecas): The outer prototype is not observable from the inner + // vm. Allowing property getter on the outer prototype can be confusing + // comparing to the normal JavaScript objects. + // Additionally, this may expose unexpected properties on the outer + // prototype chain, like polyfills, to the vm context. + onOuterProto: 'onOuterProto', + onOuterProtoGetter: 'onOuterProtoGetter', + onOuterProtoIndexed: 'onOuterProtoIndexed', + onInnerProto: 'onInnerProto', + onInnerProtoGetter: 'onInnerProtoGetter', + onInnerProtoIndexed: 'onInnerProtoIndexed', + bothProto: 'onOuterProto', + bothProtoGetter: 'onOuterProtoGetter', + bothProtoIndexed: 'onOuterProtoIndexed', }, });