8044000: Access to undefined property yields "null" instead of "undefined"

Reviewed-by: lagergren, jlaskey

4 files changed, 72 insertions, 5 deletions

diff --git a/src/jdk/nashorn/internal/runtime/linker/Bootstrap.java b/src/jdk/nashorn/internal/runtime/linker/Bootstrap.java
index c8f39fcf..48821036 100644
--- a/src/jdk/nashorn/internal/runtime/linker/Bootstrap.java
+++ b/src/jdk/nashorn/internal/runtime/linker/Bootstrap.java
@@ -61,9 +61,17 @@ public final class Bootstrap {
     private static final DynamicLinker dynamicLinker;
     static {
         final DynamicLinkerFactory factory = new DynamicLinkerFactory();
-        factory.setPrioritizedLinkers(new NashornLinker(), new NashornPrimitiveLinker(), new NashornStaticClassLinker(),
-                new BoundDynamicMethodLinker(), new JavaSuperAdapterLinker(), new JSObjectLinker(), new ReflectionCheckLinker());
-        factory.setFallbackLinkers(new NashornBeansLinker(), new NashornBottomLinker());
+        final NashornBeansLinker nashornBeansLinker = new NashornBeansLinker();
+        final JSObjectLinker jsObjectLinker = new JSObjectLinker(nashornBeansLinker);
+        factory.setPrioritizedLinkers(
+            new NashornLinker(),
+            new NashornPrimitiveLinker(),
+            new NashornStaticClassLinker(),
+            new BoundDynamicMethodLinker(),
+            new JavaSuperAdapterLinker(),
+            jsObjectLinker,
+            new ReflectionCheckLinker());
+        factory.setFallbackLinkers(nashornBeansLinker, new NashornBottomLinker());
         factory.setSyncOnRelink(true);
         final int relinkThreshold = Options.getIntProperty("nashorn.unstable.relink.threshold", -1);
         if (relinkThreshold > -1) {
diff --git a/src/jdk/nashorn/internal/runtime/linker/JSObjectLinker.java b/src/jdk/nashorn/internal/runtime/linker/JSObjectLinker.java
index f3c8284b..52fb46bc 100644
--- a/src/jdk/nashorn/internal/runtime/linker/JSObjectLinker.java
+++ b/src/jdk/nashorn/internal/runtime/linker/JSObjectLinker.java
@@ -30,6 +30,7 @@ import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodType;
 import java.util.HashMap;
 import java.util.Map;
+import javax.script.Bindings;
 import jdk.internal.dynalink.CallSiteDescriptor;
 import jdk.internal.dynalink.linker.GuardedInvocation;
 import jdk.internal.dynalink.linker.GuardedTypeConversion;
@@ -48,14 +49,23 @@ import jdk.nashorn.internal.runtime.JSType;
  * as ScriptObjects from other Nashorn contexts.
  */
 final class JSObjectLinker implements TypeBasedGuardingDynamicLinker, GuardingTypeConverterFactory {
+    private final NashornBeansLinker nashornBeansLinker;
+
+    JSObjectLinker(final NashornBeansLinker nashornBeansLinker) {
+        this.nashornBeansLinker = nashornBeansLinker;
+    }
+
     @Override
     public boolean canLinkType(final Class<?> type) {
         return canLinkTypeStatic(type);
     }
 
     static boolean canLinkTypeStatic(final Class<?> type) {
-        // can link JSObject
-        return JSObject.class.isAssignableFrom(type);
+        // can link JSObject also handles Map, Bindings to make
+        // sure those are not JSObjects.
+        return Map.class.isAssignableFrom(type) ||
+               Bindings.class.isAssignableFrom(type) ||
+               JSObject.class.isAssignableFrom(type);
     }
 
     @Override
@@ -72,6 +82,11 @@ final class JSObjectLinker implements TypeBasedGuardingDynamicLinker, GuardingTy
         final GuardedInvocation inv;
         if (self instanceof JSObject) {
             inv = lookup(desc);
+        } else if (self instanceof Map || self instanceof Bindings) {
+            // guard to make sure the Map or Bindings does not turn into JSObject later!
+            final GuardedInvocation beanInv = nashornBeansLinker.getGuardedInvocation(request, linkerServices);
+            inv = new GuardedInvocation(beanInv.getInvocation(),
+                NashornGuards.combineGuards(beanInv.getGuard(), NashornGuards.getNotJSObjectGuard()));
         } else {
             throw new AssertionError(); // Should never reach here.
         }
diff --git a/src/jdk/nashorn/internal/runtime/linker/NashornGuards.java b/src/jdk/nashorn/internal/runtime/linker/NashornGuards.java
index ca3e10c4..77c5e93c 100644
--- a/src/jdk/nashorn/internal/runtime/linker/NashornGuards.java
+++ b/src/jdk/nashorn/internal/runtime/linker/NashornGuards.java
@@ -31,6 +31,7 @@ import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.ref.WeakReference;
 import jdk.internal.dynalink.CallSiteDescriptor;
+import jdk.nashorn.api.scripting.JSObject;
 import jdk.nashorn.internal.codegen.ObjectClassGenerator;
 import jdk.nashorn.internal.objects.Global;
 import jdk.nashorn.internal.runtime.Property;
@@ -43,6 +44,7 @@ import jdk.nashorn.internal.runtime.ScriptObject;
  */
 public final class NashornGuards {
     private static final MethodHandle IS_SCRIPTOBJECT   = findOwnMH("isScriptObject", boolean.class, Object.class);
+    private static final MethodHandle IS_NOT_JSOBJECT   = findOwnMH("isNotJSObject", boolean.class, Object.class);
     private static final MethodHandle IS_SCRIPTFUNCTION = findOwnMH("isScriptFunction", boolean.class, Object.class);
     private static final MethodHandle IS_MAP            = findOwnMH("isMap", boolean.class, Object.class, PropertyMap.class);
     private static final MethodHandle SAME_OBJECT       = findOwnMH("sameObject", boolean.class, Object.class, WeakReference.class);
@@ -61,6 +63,14 @@ public final class NashornGuards {
     }
 
     /**
+     * Get the guard that checks if an item is not a {@code JSObject}
+     * @return method handle for guard
+     */
+    public static MethodHandle getNotJSObjectGuard() {
+        return IS_NOT_JSOBJECT;
+    }
+
+    /**
      * Get the guard that checks if an item is a {@code ScriptFunction}
      * @return method handle for guard
      */
@@ -157,6 +167,11 @@ public final class NashornGuards {
     }
 
     @SuppressWarnings("unused")
+    private static boolean isNotJSObject(final Object self) {
+        return !(self instanceof JSObject);
+    }
+
+    @SuppressWarnings("unused")
     private static boolean isScriptFunction(final Object self) {
         return self instanceof ScriptFunction;
     }
diff --git a/test/src/jdk/nashorn/api/scripting/ScriptObjectMirrorTest.java b/test/src/jdk/nashorn/api/scripting/ScriptObjectMirrorTest.java
index 241f22c3..30eaefca 100644
--- a/test/src/jdk/nashorn/api/scripting/ScriptObjectMirrorTest.java
+++ b/test/src/jdk/nashorn/api/scripting/ScriptObjectMirrorTest.java
@@ -29,6 +29,8 @@ import java.nio.ByteBuffer;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import javax.script.Bindings;
+import javax.script.ScriptContext;
 import javax.script.ScriptEngine;
 import javax.script.ScriptEngineManager;
 import javax.script.ScriptException;
@@ -276,4 +278,31 @@ public class ScriptObjectMirrorTest {
             "({ toString: function() { return 'foo' } })");
         assertEquals("foo", obj.to(String.class));
     }
+
+    // @bug 8044000: Access to undefined property yields "null" instead of "undefined"
+    @Test
+    public void mapScriptObjectMirrorCallsiteTest() throws ScriptException {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine engine = m.getEngineByName("nashorn");
+        final String TEST_SCRIPT = "typeof obj.foo";
+
+        final Bindings global = engine.getContext().getBindings(ScriptContext.ENGINE_SCOPE);
+        engine.eval("var obj = java.util.Collections.emptyMap()");
+        // this will drive callsite "obj.foo" of TEST_SCRIPT
+        // to use "obj instanceof Map" as it's guard
+        engine.eval(TEST_SCRIPT, global);
+        // redefine 'obj' to be a script object
+        engine.eval("obj = {}");
+
+        final Bindings newGlobal = engine.createBindings();
+        // transfer 'obj' from default global to new global
+        // new global will get a ScriptObjectMirror wrapping 'obj'
+        newGlobal.put("obj", global.get("obj"));
+
+        // Every ScriptObjectMirror is a Map! If callsite "obj.foo"
+        // does not see the new 'obj' is a ScriptObjectMirror, it'll
+        // continue to use Map's get("obj.foo") instead of ScriptObjectMirror's
+        // getMember("obj.foo") - thereby getting null instead of undefined
+        assertEquals("undefined", engine.eval(TEST_SCRIPT, newGlobal));
+    }
 }