Skip to content

Commit 5915fd4

Browse files
legendecaslouwers
authored andcommitted
vm: add vm proto property lookup test
Add more test coverage on vm prototype properties lookup with `in` operator and property access. PR-URL: nodejs#54606 Refs: nodejs#54436 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent b911093 commit 5915fd4

File tree

2 files changed

+207
-31
lines changed

2 files changed

+207
-31
lines changed

test/parallel/test-vm-global-property-enumerator.js

+14
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ const assert = require('assert');
88

99
const cases = [
1010
{
11+
get 1() {
12+
return 'value';
13+
},
1114
get key() {
1215
return 'value';
1316
},
@@ -16,28 +19,39 @@ const cases = [
1619
// Intentionally single setter.
1720
// eslint-disable-next-line accessor-pairs
1821
set key(value) {},
22+
// eslint-disable-next-line accessor-pairs
23+
set 1(value) {},
1924
},
2025
{},
2126
{
2227
key: 'value',
28+
1: 'value',
2329
},
2430
(new class GetterObject {
2531
get key() {
2632
return 'value';
2733
}
34+
get 1() {
35+
return 'value';
36+
}
2837
}()),
2938
(new class SetterObject {
3039
// Intentionally single setter.
3140
// eslint-disable-next-line accessor-pairs
3241
set key(value) {
3342
// noop
3443
}
44+
// eslint-disable-next-line accessor-pairs
45+
set 1(value) {
46+
// noop
47+
}
3548
}()),
3649
[],
3750
[['key', 'value']],
3851
{
3952
__proto__: {
4053
key: 'value',
54+
1: 'value',
4155
},
4256
},
4357
(() => {

test/parallel/test-vm-global-property-prototype.js

+193-31
Original file line numberDiff line numberDiff line change
@@ -3,62 +3,160 @@ require('../common');
33
const assert = require('assert');
44
const vm = require('vm');
55

6+
const outerProto = {
7+
onOuterProto: 'onOuterProto',
8+
bothProto: 'onOuterProto',
9+
};
10+
function onOuterProtoGetter() {
11+
return 'onOuterProtoGetter';
12+
}
13+
Object.defineProperties(outerProto, {
14+
onOuterProtoGetter: {
15+
get: onOuterProtoGetter,
16+
},
17+
bothProtoGetter: {
18+
get: onOuterProtoGetter,
19+
},
20+
// outer proto indexed
21+
0: {
22+
value: 'onOuterProtoIndexed',
23+
writable: false,
24+
enumerable: false,
25+
configurable: true,
26+
},
27+
// both proto indexed
28+
3: {
29+
value: 'onOuterProtoIndexed',
30+
writable: false,
31+
enumerable: false,
32+
configurable: true,
33+
},
34+
});
35+
36+
// Creating a new intermediate proto to mimic the
37+
// window -> Window.prototype -> EventTarget.prototype chain in JSDom.
38+
const sandboxProto = {
39+
__proto__: outerProto,
40+
};
41+
642
const sandbox = {
43+
__proto__: sandboxProto,
744
onSelf: 'onSelf',
845
};
946

1047
function onSelfGetter() {
1148
return 'onSelfGetter';
1249
}
13-
14-
Object.defineProperty(sandbox, 'onSelfGetter', {
15-
get: onSelfGetter,
16-
});
17-
18-
Object.defineProperty(sandbox, 1, {
19-
value: 'onSelfIndexed',
20-
writable: false,
21-
enumerable: false,
22-
configurable: true,
50+
Object.defineProperties(sandbox, {
51+
onSelfGetter: {
52+
get: onSelfGetter,
53+
},
54+
1: {
55+
value: 'onSelfIndexed',
56+
writable: false,
57+
enumerable: false,
58+
configurable: true,
59+
}
2360
});
2461

2562
const ctx = vm.createContext(sandbox);
2663

2764
const result = vm.runInContext(`
28-
Object.prototype.onProto = 'onProto';
29-
Object.defineProperty(Object.prototype, 'onProtoGetter', {
30-
get() {
31-
return 'onProtoGetter';
65+
Object.prototype.onInnerProto = 'onInnerProto';
66+
Object.defineProperties(Object.prototype, {
67+
onInnerProtoGetter: {
68+
get() {
69+
return 'onInnerProtoGetter';
70+
},
71+
},
72+
2: {
73+
value: 'onInnerProtoIndexed',
74+
writable: false,
75+
enumerable: false,
76+
configurable: true,
3277
},
3378
});
34-
Object.defineProperty(Object.prototype, 2, {
35-
value: 'onProtoIndexed',
36-
writable: false,
37-
enumerable: false,
38-
configurable: true,
79+
80+
// Override outer proto properties
81+
Object.prototype.bothProto = 'onInnerProto';
82+
Object.defineProperties(Object.prototype, {
83+
bothProtoGetter: {
84+
get() {
85+
return 'onInnerProtoGetter';
86+
},
87+
},
88+
// outer proto indexed
89+
3: {
90+
value: 'onInnerProtoIndexed',
91+
writable: false,
92+
enumerable: false,
93+
configurable: true,
94+
},
3995
});
4096
4197
const resultHasOwn = {
4298
onSelf: Object.hasOwn(this, 'onSelf'),
4399
onSelfGetter: Object.hasOwn(this, 'onSelfGetter'),
44100
onSelfIndexed: Object.hasOwn(this, 1),
45-
onProto: Object.hasOwn(this, 'onProto'),
46-
onProtoGetter: Object.hasOwn(this, 'onProtoGetter'),
47-
onProtoIndexed: Object.hasOwn(this, 2),
101+
onOuterProto: Object.hasOwn(this, 'onOuterProto'),
102+
onOuterProtoGetter: Object.hasOwn(this, 'onOuterProtoGetter'),
103+
onOuterProtoIndexed: Object.hasOwn(this, 0),
104+
onInnerProto: Object.hasOwn(this, 'onInnerProto'),
105+
onInnerProtoGetter: Object.hasOwn(this, 'onInnerProtoGetter'),
106+
onInnerProtoIndexed: Object.hasOwn(this, 2),
107+
bothProto: Object.hasOwn(this, 'bothProto'),
108+
bothProtoGetter: Object.hasOwn(this, 'bothProtoGetter'),
109+
bothProtoIndexed: Object.hasOwn(this, 3),
48110
};
49111
50112
const getDesc = (prop) => Object.getOwnPropertyDescriptor(this, prop);
51113
const resultDesc = {
52114
onSelf: getDesc('onSelf'),
53115
onSelfGetter: getDesc('onSelfGetter'),
54116
onSelfIndexed: getDesc(1),
55-
onProto: getDesc('onProto'),
56-
onProtoGetter: getDesc('onProtoGetter'),
57-
onProtoIndexed: getDesc(2),
117+
onOuterProto: getDesc('onOuterProto'),
118+
onOuterProtoGetter: getDesc('onOuterProtoGetter'),
119+
onOuterProtoIndexed: getDesc(0),
120+
onInnerProto: getDesc('onInnerProto'),
121+
onInnerProtoGetter: getDesc('onInnerProtoGetter'),
122+
onInnerProtoIndexed: getDesc(2),
123+
bothProto: getDesc('bothProto'),
124+
bothProtoGetter: getDesc('bothProtoGetter'),
125+
bothProtoIndexed: getDesc(3),
126+
};
127+
const resultIn = {
128+
onSelf: 'onSelf' in this,
129+
onSelfGetter: 'onSelfGetter' in this,
130+
onSelfIndexed: 1 in this,
131+
onOuterProto: 'onOuterProto' in this,
132+
onOuterProtoGetter: 'onOuterProtoGetter' in this,
133+
onOuterProtoIndexed: 0 in this,
134+
onInnerProto: 'onInnerProto' in this,
135+
onInnerProtoGetter: 'onInnerProtoGetter' in this,
136+
onInnerProtoIndexed: 2 in this,
137+
bothProto: 'bothProto' in this,
138+
bothProtoGetter: 'bothProtoGetter' in this,
139+
bothProtoIndexed: 3 in this,
140+
};
141+
const resultValue = {
142+
onSelf: this.onSelf,
143+
onSelfGetter: this.onSelfGetter,
144+
onSelfIndexed: this[1],
145+
onOuterProto: this.onOuterProto,
146+
onOuterProtoGetter: this.onOuterProtoGetter,
147+
onOuterProtoIndexed: this[0],
148+
onInnerProto: this.onInnerProto,
149+
onInnerProtoGetter: this.onInnerProtoGetter,
150+
onInnerProtoIndexed: this[2],
151+
bothProto: this.bothProto,
152+
bothProtoGetter: this.bothProtoGetter,
153+
bothProtoIndexed: this[3],
58154
};
59155
({
60156
resultHasOwn,
61157
resultDesc,
158+
resultIn,
159+
resultValue,
62160
});
63161
`, ctx);
64162

@@ -68,16 +166,80 @@ assert.deepEqual(result, {
68166
onSelf: true,
69167
onSelfGetter: true,
70168
onSelfIndexed: true,
71-
onProto: false,
72-
onProtoGetter: false,
73-
onProtoIndexed: false,
169+
170+
// All prototype properties are not own properties.
171+
onOuterProto: false,
172+
onOuterProtoGetter: false,
173+
onOuterProtoIndexed: false,
174+
onInnerProto: false,
175+
onInnerProtoGetter: false,
176+
onInnerProtoIndexed: false,
177+
bothProto: false,
178+
bothProtoGetter: false,
179+
bothProtoIndexed: false,
74180
},
75181
resultDesc: {
76182
onSelf: { value: 'onSelf', writable: true, enumerable: true, configurable: true },
77183
onSelfGetter: { get: onSelfGetter, set: undefined, enumerable: false, configurable: false },
78184
onSelfIndexed: { value: 'onSelfIndexed', writable: false, enumerable: false, configurable: true },
79-
onProto: undefined,
80-
onProtoGetter: undefined,
81-
onProtoIndexed: undefined,
185+
186+
// All prototype properties are not own properties.
187+
onOuterProto: undefined,
188+
onOuterProtoGetter: undefined,
189+
onOuterProtoIndexed: undefined,
190+
onInnerProto: undefined,
191+
onInnerProtoGetter: undefined,
192+
onInnerProtoIndexed: undefined,
193+
bothProto: undefined,
194+
bothProtoGetter: undefined,
195+
bothProtoIndexed: undefined,
196+
},
197+
resultIn: {
198+
onSelf: true,
199+
onSelfGetter: true,
200+
onSelfIndexed: true,
201+
202+
// Only properties exist on inner prototype chain will be looked up
203+
// on `in` operator. In the VM Context, the prototype chain will be like:
204+
// ```
205+
// Object
206+
// ^
207+
// | prototype
208+
// InnerPrototype
209+
// ^
210+
// | prototype
211+
// globalThis
212+
// ```
213+
// Outer prototype is not in the inner global object prototype chain and it
214+
// will not be looked up on `in` operator.
215+
onOuterProto: false,
216+
onOuterProtoGetter: false,
217+
onOuterProtoIndexed: false,
218+
onInnerProto: true,
219+
onInnerProtoGetter: true,
220+
onInnerProtoIndexed: true,
221+
bothProto: true,
222+
bothProtoGetter: true,
223+
bothProtoIndexed: true,
224+
},
225+
resultValue: {
226+
onSelf: 'onSelf',
227+
onSelfGetter: 'onSelfGetter',
228+
onSelfIndexed: 'onSelfIndexed',
229+
230+
// FIXME(legendecas): The outer prototype is not observable from the inner
231+
// vm. Allowing property getter on the outer prototype can be confusing
232+
// comparing to the normal JavaScript objects.
233+
// Additionally, this may expose unexpected properties on the outer
234+
// prototype chain, like polyfills, to the vm context.
235+
onOuterProto: 'onOuterProto',
236+
onOuterProtoGetter: 'onOuterProtoGetter',
237+
onOuterProtoIndexed: 'onOuterProtoIndexed',
238+
onInnerProto: 'onInnerProto',
239+
onInnerProtoGetter: 'onInnerProtoGetter',
240+
onInnerProtoIndexed: 'onInnerProtoIndexed',
241+
bothProto: 'onOuterProto',
242+
bothProtoGetter: 'onOuterProtoGetter',
243+
bothProtoIndexed: 'onOuterProtoIndexed',
82244
},
83245
});

0 commit comments

Comments
 (0)