8053910: ScriptObjectMirror causing havoc with Invocation interface

Reviewed-by: jlaskey, attila, hannesw

nashorn/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java

@@ -715,6 +715,23 @@ public final class ScriptObjectMirror extends AbstractJSObject implements Bindin
         return newArgs;
     }
 
+    /**
+     * Are the given objects mirrors to same underlying object?
+     *
+     * @param obj1 first object
+     * @param obj2 second object
+     * @return true if obj1 and obj2 are identical script objects or mirrors of it.
+     */
+    public static boolean identical(final Object obj1, final Object obj2) {
+        final Object o1 = (obj1 instanceof ScriptObjectMirror)?
+            ((ScriptObjectMirror)obj1).sobj : obj1;
+
+        final Object o2 = (obj2 instanceof ScriptObjectMirror)?
+            ((ScriptObjectMirror)obj2).sobj : obj2;
+
+        return o1 == o2;
+    }
+
     // package-privates below this.
 
     ScriptObjectMirror(final ScriptObject sobj, final Global global) {

nashorn/src/jdk/nashorn/internal/runtime/ScriptRuntime.java

@@ -702,6 +702,9 @@ public final class ScriptRuntime {
         if (x instanceof ScriptObject && y instanceof ScriptObject) {
             return x == y;
         }
+        if (x instanceof ScriptObjectMirror || y instanceof ScriptObjectMirror) {
+            return ScriptObjectMirror.identical(x, y);
+        }
         return equalValues(x, y);
     }
 

nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterBytecodeGenerator.java

@@ -60,6 +60,7 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import jdk.nashorn.api.scripting.ScriptUtils;
 import jdk.internal.org.objectweb.asm.ClassWriter;
 import jdk.internal.org.objectweb.asm.Handle;
 import jdk.internal.org.objectweb.asm.Label;
@@ -134,10 +135,12 @@ import sun.reflect.CallerSensitive;
  * implemented securely.
  */
 final class JavaAdapterBytecodeGenerator {
+    private static final Type SCRIPTUTILS_TYPE = Type.getType(ScriptUtils.class);
     private static final Type OBJECT_TYPE = Type.getType(Object.class);
     private static final Type CLASS_TYPE  = Type.getType(Class.class);
 
     static final String OBJECT_TYPE_NAME  = OBJECT_TYPE.getInternalName();
+    static final String SCRIPTUTILS_TYPE_NAME  = SCRIPTUTILS_TYPE.getInternalName();
 
     static final String INIT = "<init>";
 
@@ -172,6 +175,7 @@ final class JavaAdapterBytecodeGenerator {
     private static final String GET_GLOBAL_METHOD_DESCRIPTOR = Type.getMethodDescriptor(OBJECT_TYPE);
     private static final String GET_CLASS_METHOD_DESCRIPTOR = Type.getMethodDescriptor(CLASS_TYPE);
     private static final String EXPORT_RETURN_VALUE_METHOD_DESCRIPTOR = Type.getMethodDescriptor(OBJECT_TYPE, OBJECT_TYPE);
+    private static final String UNWRAP_METHOD_DESCRIPTOR = Type.getMethodDescriptor(OBJECT_TYPE, OBJECT_TYPE);
     private static final String GET_CONVERTER_METHOD_DESCRIPTOR = Type.getMethodDescriptor(METHOD_HANDLE_TYPE, CLASS_TYPE);
     private static final String TO_CHAR_PRIMITIVE_METHOD_DESCRIPTOR = Type.getMethodDescriptor(Type.CHAR_TYPE, OBJECT_TYPE);
     private static final String TO_STRING_METHOD_DESCRIPTOR = Type.getMethodDescriptor(STRING_TYPE, OBJECT_TYPE);
@@ -927,10 +931,14 @@ final class JavaAdapterBytecodeGenerator {
             invokeValueOf(mv, "Double", 'D');
             break;
         case Type.ARRAY:
-        case Type.OBJECT:
         case Type.METHOD:
             // Already boxed
             break;
+        case Type.OBJECT:
+            if(t.equals(OBJECT_TYPE)) {
+                mv.invokestatic(SCRIPTUTILS_TYPE_NAME, "unwrap", UNWRAP_METHOD_DESCRIPTOR, false);
+            }
+            break;
         default:
             // Not expecting anything else (e.g. VOID)
             assert false;

nashorn/src/jdk/nashorn/internal/runtime/linker/JavaAdapterServices.java

@@ -47,6 +47,8 @@ import jdk.internal.org.objectweb.asm.ClassWriter;
 import jdk.internal.org.objectweb.asm.Opcodes;
 import jdk.internal.org.objectweb.asm.Type;
 import jdk.internal.org.objectweb.asm.commons.InstructionAdapter;
+import jdk.nashorn.api.scripting.ScriptUtils;
+import jdk.nashorn.api.scripting.ScriptObjectMirror;
 import jdk.nashorn.internal.runtime.Context;
 import jdk.nashorn.internal.runtime.ScriptFunction;
 import jdk.nashorn.internal.runtime.ScriptObject;
@@ -214,12 +216,12 @@ public final class JavaAdapterServices {
 
     /**
      * Invoked when returning Object from an adapted method to filter out internal Nashorn objects that must not be seen
-     * by the callers. Currently only transforms {@code ConsString} into {@code String}.
+     * by the callers. Currently only transforms {@code ConsString} into {@code String} and transforms {@code ScriptObject} into {@code ScriptObjectMirror}.
      * @param obj the return value
      * @return the filtered return value.
      */
     public static Object exportReturnValue(final Object obj) {
-        return NashornBeansLinker.exportArgument(obj);
+        return ScriptUtils.wrap(NashornBeansLinker.exportArgument(obj));
     }
 
     /**

nashorn/test/src/jdk/nashorn/api/scripting/ScriptObjectMirrorTest.java

@@ -31,10 +31,12 @@ import static org.testng.Assert.assertTrue;
 import static org.testng.Assert.fail;
 
 import java.nio.ByteBuffer;
+import java.util.function.Function;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import javax.script.Bindings;
+import javax.script.Invocable;
 import javax.script.ScriptContext;
 import javax.script.ScriptEngine;
 import javax.script.ScriptEngineManager;
@@ -306,4 +308,57 @@ public class ScriptObjectMirrorTest {
         // getMember("obj.foo") - thereby getting null instead of undefined
         assertEquals("undefined", engine.eval(TEST_SCRIPT, newGlobal));
     }
+
+    public interface MirrorCheckExample {
+        Object test1(Object arg);
+        Object test2(Object arg);
+        boolean compare(Object o1, Object o2);
+    }
+
+    // @bug 8053910: ScriptObjectMirror causing havoc with Invocation interface
+    @Test
+    public void checkMirrorToObject() throws Exception {
+        final ScriptEngineManager engineManager = new ScriptEngineManager();
+        final ScriptEngine engine = engineManager.getEngineByName("nashorn");
+        final Invocable invocable = (Invocable)engine;
+
+        engine.eval("function test1(arg) { return { arg: arg }; }");
+        engine.eval("function test2(arg) { return arg; }");
+        engine.eval("function compare(arg1, arg2) { return arg1 == arg2; }");
+
+        final Map<String, Object> map = new HashMap<>();
+        map.put("option", true);
+
+        final MirrorCheckExample example = invocable.getInterface(MirrorCheckExample.class);
+
+        final Object value1 = invocable.invokeFunction("test1", map);
+        final Object value2 = example.test1(map);
+        final Object value3 = invocable.invokeFunction("test2", value2);
+        final Object value4 = example.test2(value2);
+
+        // check that Object type argument receives a ScriptObjectMirror
+        // when ScriptObject is passed
+        assertEquals(ScriptObjectMirror.class, value1.getClass());
+        assertEquals(ScriptObjectMirror.class, value2.getClass());
+        assertEquals(ScriptObjectMirror.class, value3.getClass());
+        assertEquals(ScriptObjectMirror.class, value4.getClass());
+        assertTrue((boolean)invocable.invokeFunction("compare", value1, value1));
+        assertTrue((boolean)example.compare(value1, value1));
+        assertTrue((boolean)invocable.invokeFunction("compare", value3, value4));
+        assertTrue((boolean)example.compare(value3, value4));
+    }
+
+    // @bug 8053910: ScriptObjectMirror causing havoc with Invocation interface
+    @Test
+    @SuppressWarnings("unchecked")
+    public void mirrorUnwrapInterfaceMethod() throws Exception {
+        final ScriptEngineManager engineManager = new ScriptEngineManager();
+        final ScriptEngine engine = engineManager.getEngineByName("nashorn");
+        final Invocable invocable = (Invocable)engine;
+        engine.eval("function apply(obj) { " +
+            " return obj instanceof Packages.jdk.nashorn.api.scripting.ScriptObjectMirror; " +
+            "}");
+        Function<Object,Object> func = invocable.getInterface(Function.class);
+        assertFalse((boolean)func.apply(engine.eval("({ x: 2 })")));
+    }
 }