Skip to content

Commit 3bbe7e5

Browse files
gilles-duboscqgilles-duboscq
committed
[GR-45354] Fix globalThis in vm context.
PullRequest: js/2773
2 parents a819274 + 023fbed commit 3bbe7e5

File tree

3 files changed

+104
-34
lines changed

3 files changed

+104
-34
lines changed

‎graal-nodejs/mx.graal-nodejs/com.oracle.truffle.trufflenode/src/com/oracle/truffle/trufflenode/GraalJSAccess.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,8 @@ private GraalJSAccess(String[] args) throws Exception {
392392
contextBuilder.option(JSContextOptions.CONSOLE_NAME, "false");
393393
// Node.js does not have global arguments property
394394
contextBuilder.option(JSContextOptions.GLOBAL_ARGUMENTS_NAME, "false");
395+
// Node.js context does not have a predefined global 'global' property.
396+
contextBuilder.option(JSContextOptions.GLOBAL_PROPERTY_NAME, "false");
395397
contextBuilder.useSystemExit(true);
396398

397399
exposeGC = options.isGCExposed();
@@ -2836,7 +2838,10 @@ public Object contextNew(Object templateObj) {
28362838
if (templateObj != null) {
28372839
ObjectTemplate template = (ObjectTemplate) templateObj;
28382840
if (template.hasPropertyHandler()) {
2839-
global = propertyHandlerInstantiate(context, realm, template, global, true);
2841+
var globalProxy = propertyHandlerInstantiate(context, realm, template, global, true);
2842+
assert JSObject.hasOwnProperty(global, Strings.GLOBAL_THIS) && !JSObject.hasOwnProperty(global, Strings.GLOBAL);
2843+
JSObject.set(global, Strings.GLOBAL_THIS, globalProxy);
2844+
global = globalProxy;
28402845
realm.setGlobalObject(global);
28412846
} else {
28422847
JSObject prototype = JSOrdinary.create(context, realm);

‎graal-nodejs/mx.graal-nodejs/com.oracle.truffle.trufflenode/src/com/oracle/truffle/trufflenode/node/ExecuteNativePropertyHandlerNode.java‎

Lines changed: 38 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2018, 2023, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -41,9 +41,10 @@
4141
package com.oracle.truffle.trufflenode.node;
4242

4343
import java.util.ArrayList;
44-
import java.util.Arrays;
4544
import java.util.List;
4645

46+
import org.graalvm.collections.EconomicSet;
47+
4748
import com.oracle.truffle.api.CompilerDirectives;
4849
import com.oracle.truffle.api.frame.VirtualFrame;
4950
import com.oracle.truffle.api.object.HiddenKey;
@@ -55,8 +56,10 @@
5556
import com.oracle.truffle.js.runtime.JSRuntime;
5657
import com.oracle.truffle.js.runtime.JavaScriptRootNode;
5758
import com.oracle.truffle.js.runtime.Symbol;
59+
import com.oracle.truffle.js.runtime.array.ScriptArray;
5860
import com.oracle.truffle.js.runtime.builtins.JSAbstractArray;
5961
import com.oracle.truffle.js.runtime.builtins.JSArray;
62+
import com.oracle.truffle.js.runtime.builtins.JSArrayObject;
6063
import com.oracle.truffle.js.runtime.objects.JSDynamicObject;
6164
import com.oracle.truffle.js.runtime.objects.JSObject;
6265
import com.oracle.truffle.js.runtime.objects.PropertyDescriptor;
@@ -346,45 +349,47 @@ private Object executeOwnKeys(ObjectTemplate template, Object holder, Object[] a
346349
PropertyHandler indexedHandler = template.getIndexedPropertyHandler();
347350
PropertyHandler namedHandler = template.getNamedPropertyHandler();
348351
JSDynamicObject proxy = (JSDynamicObject) holder;
352+
JSDynamicObject target = (JSDynamicObject) arguments[2];
349353

350-
Object[] nativeCallArgs = JSArguments.create(proxy, arguments[1], arguments[2]);
351-
JSDynamicObject ownKeys = null;
352-
if (namedHandler != null && namedHandler.getEnumerator() != 0) {
353-
ownKeys = (JSDynamicObject) NativeAccess.executePropertyHandlerEnumerator(namedHandler.getEnumerator(), holder, nativeCallArgs, namedHandler.getData());
354-
}
354+
Object[] nativeCallArgs = JSArguments.create(proxy, arguments[1], target);
355+
List<Object> ownKeys = new ArrayList<>();
356+
EconomicSet<Object> ownKeysSet = EconomicSet.create();
357+
358+
// Note: the indexed and the named handler may both return the same result.
359+
// This is currently the case for the global object template of ContextifyContext.
355360
if (indexedHandler != null && indexedHandler.getEnumerator() != 0) {
356-
JSDynamicObject ownKeys2 = (JSDynamicObject) NativeAccess.executePropertyHandlerEnumerator(indexedHandler.getEnumerator(), holder, nativeCallArgs, indexedHandler.getData());
357-
if (JSArray.isJSArray(ownKeys2)) {
358-
// indexed handler returns array of numbers but we need an array of property keys
359-
long length = JSAbstractArray.arrayGetLength(ownKeys2);
360-
for (long i = 0; i < length; i++) {
361-
Object key = JSObject.get(ownKeys2, i);
362-
JSObject.set(ownKeys2, i, JSRuntime.toString(key));
363-
}
364-
ownKeys = concatArrays(ownKeys, ownKeys2);
361+
Object handlerResult = NativeAccess.executePropertyHandlerEnumerator(indexedHandler.getEnumerator(), holder, nativeCallArgs, indexedHandler.getData());
362+
if (handlerResult instanceof JSArrayObject resultArray) {
363+
addKeysFromHandlerResultArray(resultArray, ownKeys, ownKeysSet);
365364
}
366365
}
367-
if (!JSArray.isJSArray(ownKeys)) {
368-
ownKeys = JSArray.createEmpty(context, getRealm(), 0);
366+
367+
if (namedHandler != null && namedHandler.getEnumerator() != 0) {
368+
Object handlerResult = NativeAccess.executePropertyHandlerEnumerator(namedHandler.getEnumerator(), holder, nativeCallArgs, namedHandler.getData());
369+
if (handlerResult instanceof JSArrayObject resultArray) {
370+
addKeysFromHandlerResultArray(resultArray, ownKeys, ownKeysSet);
371+
}
369372
}
370-
return ownKeys;
373+
374+
List<Object> targetList = JSObject.ownPropertyKeys(target);
375+
for (Object propertyKey : targetList) {
376+
if (ownKeysSet.add(propertyKey)) {
377+
ownKeys.add(propertyKey);
378+
}
379+
}
380+
381+
return JSArray.createConstant(context, getRealm(), ownKeys.toArray(ScriptArray.EMPTY_OBJECT_ARRAY));
371382
}
372383

373-
private JSDynamicObject concatArrays(JSDynamicObject array1, JSDynamicObject array2) {
374-
if (JSArray.isJSArray(array1)) {
375-
if (JSArray.isJSArray(array2)) {
376-
List<Object> keys = new ArrayList<>(Arrays.asList(JSArray.toArray(array1)));
377-
for (Object key : JSArray.toArray(array2)) {
378-
if (!keys.contains(key)) {
379-
keys.add(key);
380-
}
381-
}
382-
return JSArray.createConstant(context, getRealm(), keys.toArray());
383-
} else {
384-
return array1;
384+
private static void addKeysFromHandlerResultArray(JSArrayObject resultArray, List<Object> keys, EconomicSet<Object> keysSet) {
385+
long length = JSAbstractArray.arrayGetLength(resultArray);
386+
for (long i = 0; i < length; i++) {
387+
Object key = JSObject.get(resultArray, i);
388+
// handler returns numeric keys as numbers but we need an array of property keys
389+
Object propertyKey = JSRuntime.isPropertyKey(key) ? key : JSRuntime.toString(key);
390+
if (keysSet.add(propertyKey)) {
391+
keys.add(propertyKey);
385392
}
386-
} else {
387-
return array2;
388393
}
389394
}
390395

‎graal-nodejs/test/graal/unit/vm.js‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,4 +96,64 @@ describe('vm', function () {
9696
'abc'
9797
);
9898
});
99+
it('should have correct globalThis', function () {
100+
var sandbox = {};
101+
var fooValue = 42;
102+
Object.defineProperty(sandbox, 'foo', {value: fooValue});
103+
Object.defineProperty(sandbox, 'setTimeout', {value: setTimeout});
104+
var context = vm.createContext(sandbox);
105+
assert.strictEqual(vm.runInContext('foo', context), fooValue);
106+
assert.strictEqual(vm.runInContext('globalThis.foo', context), fooValue);
107+
var desc = vm.runInContext('Reflect.getOwnPropertyDescriptor(globalThis, "setTimeout")', context);
108+
assert.ok(desc);
109+
assert.strictEqual(desc.value, setTimeout);
110+
assert.strictEqual(desc.writable, false);
111+
assert.strictEqual(desc.enumerable, false);
112+
assert.strictEqual(desc.configurable, false);
113+
});
114+
it('should allow overriding globalThis', function () {
115+
var sandbox = {};
116+
var fooValue = 42;
117+
var globalThisOverride = {foo: 43};
118+
Object.defineProperty(sandbox, 'foo', {value: fooValue});
119+
Object.defineProperty(sandbox, 'globalThis', {value: globalThisOverride, configurable: true, writable: true});
120+
var context = vm.createContext(sandbox);
121+
assert.strictEqual(vm.runInContext('foo', context), fooValue);
122+
assert.strictEqual(vm.runInContext('globalThis', context), globalThisOverride);
123+
assert.strictEqual(vm.runInContext('globalThis.foo', context), globalThisOverride.foo);
124+
assert.strictEqual(sandbox.globalThis, globalThisOverride);
125+
});
126+
it('should not have "global" property', function () {
127+
var context = vm.createContext();
128+
assert.strictEqual(vm.runInContext('typeof global', context), 'undefined');
129+
assert.strictEqual(vm.runInContext('typeof globalThis.global', context), 'undefined');
130+
});
131+
it('should include global own keys', function () {
132+
var emptyContext = vm.createContext();
133+
var context = vm.createContext({1: 'one', 3: 'three', extra: 42, [Symbol.unscopables]: 43});
134+
135+
for (let query of ['Object.getOwnPropertyNames(globalThis)', 'Reflect.ownKeys(globalThis)']) {
136+
var globalBuiltins = vm.runInContext(query, emptyContext);
137+
assert(globalBuiltins.includes('Object'), globalBuiltins);
138+
139+
var globalPropertyNames = vm.runInContext('globalThis[0] = "zero"; globalThis[2] = "two"; ' + query, context);
140+
assert(globalPropertyNames.includes('Object'), globalPropertyNames);
141+
142+
// Make sure properties are in the correct order, too.
143+
assert.strictEqual(globalPropertyNames[0], '0');
144+
assert.strictEqual(globalPropertyNames[1], '1');
145+
assert.strictEqual(globalPropertyNames[2], '2');
146+
assert.strictEqual(globalPropertyNames[3], '3');
147+
assert.strictEqual(globalPropertyNames[4], 'extra');
148+
assert.strictEqual(globalPropertyNames[5], globalBuiltins[0]);
149+
150+
// Symbols of the context object are not included.
151+
assert(!globalPropertyNames.includes(Symbol.unscopables), globalPropertyNames);
152+
}
153+
154+
var globalPropertySymbols = vm.runInContext('Object.getOwnPropertySymbols(globalThis)', emptyContext);
155+
// Symbols of the context object are not included.
156+
assert(!globalPropertySymbols.includes(Symbol.unscopables), globalPropertySymbols);
157+
assert.strictEqual(43, vm.runInContext('globalThis[Symbol.unscopables]', context));
158+
});
99159
});

0 commit comments

Comments
 (0)