[GR-38976] Consider properties of foreign targets in JSProxy.getOwnProperty().

PullRequest: js/2472

Author: iamstolis

graal-js/src/com.oracle.truffle.js.test/src/com/oracle/truffle/js/test/regress/GH4613.java

@@ -0,0 +1,74 @@
+/*
+ * Copyright (c) 2022, 2022, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * The Universal Permissive License (UPL), Version 1.0
+ *
+ * Subject to the condition set forth below, permission is hereby granted to any
+ * person obtaining a copy of this software, associated documentation and/or
+ * data (collectively the "Software"), free of charge and under any and all
+ * copyright rights in the Software, and any and all patent rights owned or
+ * freely licensable by each licensor hereunder covering either (i) the
+ * unmodified Software as contributed to or provided by such licensor, or (ii)
+ * the Larger Works (as defined below), to deal in both
+ *
+ * (a) the Software, and
+ *
+ * (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
+ * one is included with the Software each a "Larger Work" to which the Software
+ * is contributed by such licensors),
+ *
+ * without restriction, including without limitation the rights to copy, create
+ * derivative works of, display, perform, and distribute the Software and make,
+ * use, sell, offer for sale, import, export, have made, and have sold the
+ * Software and the Larger Work(s), and to sublicense the foregoing rights on
+ * either these or other terms.
+ *
+ * This license is subject to the following condition:
+ *
+ * The above copyright notice and either this complete permission notice or at a
+ * minimum a reference to the UPL must be included in all copies or substantial
+ * portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+package com.oracle.truffle.js.test.regress;
+
+import com.oracle.truffle.js.lang.JavaScriptLanguage;
+import com.oracle.truffle.js.test.JSTest;
+import java.util.HashMap;
+import java.util.Map;
+import org.graalvm.polyglot.Context;
+import org.graalvm.polyglot.Value;
+import org.graalvm.polyglot.proxy.ProxyObject;
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Tests the reading of a property of JavaScript Proxy with a foreign target.
+ */
+public class GH4613 {
+
+    // Derived from https://github.com/oracle/graal/issues/4613
+    @Test
+    public void test() {
+        try (Context context = JSTest.newContextBuilder().build()) {
+            Map<String, Object> map = new HashMap<>();
+            map.put("key", "value");
+            context.getBindings(JavaScriptLanguage.ID).putMember("target", ProxyObject.fromMap(map));
+
+            Value proxy = context.eval(JavaScriptLanguage.ID, "new Proxy(target, {})");
+            Value value = proxy.getMember("key");
+
+            Assert.assertTrue(value.isString());
+            Assert.assertEquals("value", value.asString());
+        }
+    }
+
+}

graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/builtins/ObjectFunctionBuiltins.java

@@ -360,30 +360,9 @@ protected JSDynamicObject getForeignObject(Object thisObj, Object property,
                         @Cached TruffleString.ReadCharUTF16Node charAtNode) {
             Object propertyKey = toPropertyKeyNode.execute(property);
             if (Strings.isTString(propertyKey)) {
-                try {
-                    String member = Strings.toJavaString((TruffleString) propertyKey);
-                    if (interop.hasMembers(thisObj)) {
-                        if (interop.isMemberExisting(thisObj, member) && interop.isMemberReadable(thisObj, member)) {
-                            PropertyDescriptor desc = PropertyDescriptor.createData(
-                                            toJSType.executeWithTarget(interop.readMember(thisObj, member)),
-                                            !interop.isMemberInternal(thisObj, member),
-                                            interop.isMemberWritable(thisObj, member),
-                                            interop.isMemberRemovable(thisObj, member));
-                            return fromPropertyDescriptorNode.execute(desc, getContext());
-                        }
-                    }
-                    long index = JSRuntime.propertyNameToArrayIndex((TruffleString) propertyKey, charAtNode);
-                    if (JSRuntime.isArrayIndex(index) && interop.hasArrayElements(thisObj)) {
-                        if (interop.isArrayElementExisting(thisObj, index) && interop.isArrayElementReadable(thisObj, index)) {
-                            PropertyDescriptor desc = PropertyDescriptor.createData(
-                                            toJSType.executeWithTarget(interop.readArrayElement(thisObj, index)),
-                                            true,
-                                            interop.isArrayElementWritable(thisObj, index),
-                                            interop.isArrayElementRemovable(thisObj, index));
-                            return fromPropertyDescriptorNode.execute(desc, getContext());
-                        }
-                    }
-                } catch (InteropException iex) {
+                PropertyDescriptor desc = JSInteropUtil.getOwnProperty(thisObj, (TruffleString) propertyKey, interop, toJSType, charAtNode);
+                if (desc != null) {
+                    return fromPropertyDescriptorNode.execute(desc, getContext());
                 }
             }
             return Undefined.instance;
@@ -449,12 +428,8 @@ protected JSDynamicObject getForeignObject(Object thisObj,
                     }
                     for (int i = 0; i < size; i++) {
                         String member = (String) members.readArrayElement(keysObj, i);
-                        if (interop.isMemberReadable(thisObj, member)) {
-                            PropertyDescriptor desc = PropertyDescriptor.createData(
-                                            toJSType.executeWithTarget(interop.readMember(thisObj, member)),
-                                            !interop.isMemberInternal(thisObj, member),
-                                            interop.isMemberWritable(thisObj, member),
-                                            interop.isMemberRemovable(thisObj, member));
+                        PropertyDescriptor desc = JSInteropUtil.getExistingMemberProperty(thisObj, member, interop, toJSType);
+                        if (desc != null) {
                             JSDynamicObject propDesc = fromPropertyDescriptorNode.execute(desc, getContext());
                             Properties.putWithFlags(putPropDescNode, result, Strings.fromJavaString(member), propDesc, JSAttributes.configurableEnumerableWritable());
                         }
@@ -467,12 +442,8 @@ protected JSDynamicObject getForeignObject(Object thisObj,
                         throw Errors.createRangeErrorInvalidArrayLength();
                     }
                     for (long i = 0; i < size; i++) {
-                        if (interop.isArrayElementExisting(thisObj, i) && interop.isArrayElementReadable(thisObj, i)) {
-                            PropertyDescriptor desc = PropertyDescriptor.createData(
-                                            toJSType.executeWithTarget(interop.readArrayElement(thisObj, i)),
-                                            true,
-                                            interop.isArrayElementWritable(thisObj, i),
-                                            interop.isArrayElementRemovable(thisObj, i));
+                        PropertyDescriptor desc = JSInteropUtil.getArrayElementProperty(thisObj, i, interop, toJSType);
+                        if (desc != null) {
                             JSDynamicObject propDesc = fromPropertyDescriptorNode.execute(desc, getContext());
                             Properties.putWithFlags(putPropDescNode, result, Strings.fromLong(i), propDesc, JSAttributes.configurableEnumerableWritable());
                         }

graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/builtins/JSProxy.java

@@ -67,6 +67,7 @@
 import com.oracle.truffle.js.runtime.JSRuntime;
 import com.oracle.truffle.js.runtime.JavaScriptRootNode;
 import com.oracle.truffle.js.runtime.Strings;
+import com.oracle.truffle.js.runtime.Symbol;
 import com.oracle.truffle.js.runtime.ToDisplayStringFormat;
 import com.oracle.truffle.js.runtime.interop.JSInteropUtil;
 import com.oracle.truffle.js.runtime.objects.JSDynamicObject;
@@ -774,7 +775,12 @@ public PropertyDescriptor getOwnProperty(JSDynamicObject thisObj, Object key) {
             if (JSDynamicObject.isJSDynamicObject(target)) {
                 return JSObject.getOwnProperty((JSDynamicObject) target, key);
             } else {
-                return null;
+                if (Strings.isTString(key)) {
+                    return JSInteropUtil.getOwnProperty(target, (TruffleString) key);
+                } else {
+                    assert key instanceof Symbol;
+                    return null; // No symbols in foreign objects
+                }
             }
         }
         Object trapResultObj = checkTrapReturnValue(JSRuntime.call(getOwnPropertyFn, handler, new Object[]{target, key}));

graal-js/src/com.oracle.truffle.js/src/com/oracle/truffle/js/runtime/interop/JSInteropUtil.java

@@ -45,6 +45,7 @@
 
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.interop.ArityException;
+import com.oracle.truffle.api.interop.InteropException;
 import com.oracle.truffle.api.interop.InteropLibrary;
 import com.oracle.truffle.api.interop.InvalidArrayIndexException;
 import com.oracle.truffle.api.interop.UnknownIdentifierException;
@@ -60,6 +61,7 @@
 import com.oracle.truffle.js.runtime.objects.JSObject;
 import com.oracle.truffle.js.runtime.Strings;
 import com.oracle.truffle.js.runtime.objects.Null;
+import com.oracle.truffle.js.runtime.objects.PropertyDescriptor;
 import com.oracle.truffle.js.runtime.objects.Undefined;
 
 /**
@@ -236,4 +238,51 @@ public static Object construct(Object target, Object[] args) {
     public static boolean isBoxedPrimitive(Object receiver, InteropLibrary interop) {
         return interop.isString(receiver) || interop.isNumber(receiver) || interop.isBoolean(receiver);
     }
+
+    public static PropertyDescriptor getOwnProperty(Object object, TruffleString propertyKey) {
+        return getOwnProperty(object, propertyKey, InteropLibrary.getUncached(), ImportValueNode.getUncached(), TruffleString.ReadCharUTF16Node.getUncached());
+    }
+
+    public static PropertyDescriptor getOwnProperty(Object object, TruffleString propertyKey, InteropLibrary interop, ImportValueNode importValueNode, TruffleString.ReadCharUTF16Node charAtNode) {
+        try {
+            String key = Strings.toJavaString(propertyKey);
+            if (interop.hasMembers(object) && interop.isMemberExisting(object, key)) {
+                PropertyDescriptor desc = getExistingMemberProperty(object, key, interop, importValueNode);
+                if (desc != null) {
+                    return desc;
+                }
+            }
+            long index = JSRuntime.propertyNameToArrayIndex(propertyKey, charAtNode);
+            if (JSRuntime.isArrayIndex(index) && interop.hasArrayElements(object)) {
+                return getArrayElementProperty(object, index, interop, importValueNode);
+            }
+        } catch (InteropException iex) {
+        }
+        return null;
+    }
+
+    public static PropertyDescriptor getExistingMemberProperty(Object object, String key, InteropLibrary interop, ImportValueNode importValueNode) throws InteropException {
+        assert interop.hasMembers(object) && interop.isMemberExisting(object, key);
+        if (interop.isMemberReadable(object, key)) {
+            return PropertyDescriptor.createData(
+                            importValueNode.executeWithTarget(interop.readMember(object, key)),
+                            !interop.isMemberInternal(object, key),
+                            interop.isMemberWritable(object, key),
+                            interop.isMemberRemovable(object, key));
+        }
+        return null;
+    }
+
+    public static PropertyDescriptor getArrayElementProperty(Object object, long index, InteropLibrary interop, ImportValueNode importValueNode) throws InteropException {
+        assert interop.hasArrayElements(object) && JSRuntime.isArrayIndex(index);
+        if (interop.isArrayElementExisting(object, index) && interop.isArrayElementReadable(object, index)) {
+            return PropertyDescriptor.createData(
+                            importValueNode.executeWithTarget(interop.readArrayElement(object, index)),
+                            true,
+                            interop.isArrayElementWritable(object, index),
+                            interop.isArrayElementRemovable(object, index));
+        }
+        return null;
+    }
+
 }