changeset 87:8c72a2bec1be

8008197: Cross script engine function calls do not work as expected Reviewed-by: lagergren, hannesw
author sundar
date Thu, 14 Feb 2013 14:16:58 +0530
parents 222b9f32b674
children 43e32b36153c
files src/jdk/nashorn/api/scripting/NashornScriptEngine.java src/jdk/nashorn/api/scripting/ScriptObjectMirror.java src/jdk/nashorn/internal/runtime/ScriptFunction.java src/jdk/nashorn/internal/runtime/ScriptRuntime.java test/script/basic/JDK-8008197.js test/script/basic/jquery.js
diffstat 6 files changed, 138 insertions(+), 50 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk/nashorn/api/scripting/NashornScriptEngine.java	Thu Feb 14 09:14:31 2013 +0530
+++ b/src/jdk/nashorn/api/scripting/NashornScriptEngine.java	Thu Feb 14 14:16:58 2013 +0530
@@ -329,7 +329,7 @@
         final ScriptObject ctxtGlobal    = getNashornGlobalFrom(context);
         final boolean globalChanged = (oldGlobal != ctxtGlobal);
 
-        Object self = selfObject;
+        Object self = globalChanged? ScriptObjectMirror.wrap(selfObject, oldGlobal) : selfObject;
 
         try {
             if (globalChanged) {
@@ -354,7 +354,8 @@
             if (value instanceof ScriptFunction) {
                 final Object res;
                 try {
-                    res = ScriptRuntime.apply((ScriptFunction)value, self, ScriptObjectMirror.unwrapArray(args, ctxtGlobal));
+                    final Object[] modArgs = globalChanged? ScriptObjectMirror.wrapArray(args, oldGlobal) : args;
+                    res = ScriptRuntime.checkAndApply((ScriptFunction)value, self, ScriptObjectMirror.unwrapArray(modArgs, ctxtGlobal));
                 } catch (final Exception e) {
                     throwAsScriptException(e);
                     throw new AssertionError("should not reach here");
--- a/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java	Thu Feb 14 09:14:31 2013 +0530
+++ b/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java	Thu Feb 14 14:16:58 2013 +0530
@@ -104,38 +104,30 @@
     // JSObject methods
     @Override
     public Object call(final String methodName, final Object args[]) {
-        final Object val = sobj.get(methodName);
         final ScriptObject oldGlobal = NashornScriptEngine.getNashornGlobal();
         final boolean globalChanged = (oldGlobal != global);
 
-        if (val instanceof ScriptFunction) {
-            final Object[] modifiedArgs = unwrapArray(args, global);
-            if (modifiedArgs != null) {
-                for (int i = 0; i < modifiedArgs.length; i++) {
-                    final Object arg = modifiedArgs[i];
-                    if (arg instanceof ScriptObject) {
-                        modifiedArgs[i] = wrap(arg, oldGlobal);
-                    }
-                }
+        try {
+            if (globalChanged) {
+                NashornScriptEngine.setNashornGlobal(global);
+            }
+
+            final Object val = sobj.get(methodName);
+            if (! (val instanceof ScriptFunction)) {
+                throw new RuntimeException("No such method: " + methodName);
             }
 
-            try {
-                if (globalChanged) {
-                    NashornScriptEngine.setNashornGlobal(global);
-                }
-                return wrap(((ScriptFunction)val).invoke(sobj, modifiedArgs), global);
-            } catch (final RuntimeException | Error e) {
-                throw e;
-            } catch (final Throwable t) {
-                throw new RuntimeException(t);
-            } finally {
-                if (globalChanged) {
-                    NashornScriptEngine.setNashornGlobal(oldGlobal);
-                }
+            final Object[] modArgs = globalChanged? wrapArray(args, oldGlobal) : args;
+            return wrap(ScriptRuntime.checkAndApply((ScriptFunction)val, sobj, unwrapArray(modArgs, global)), global);
+        } catch (final RuntimeException | Error e) {
+            throw e;
+        } catch (final Throwable t) {
+            throw new RuntimeException(t);
+        } finally {
+            if (globalChanged) {
+                NashornScriptEngine.setNashornGlobal(oldGlobal);
             }
-       }
-
-       throw new RuntimeException("No such method: " + methodName);
+        }
     }
 
     @Override
@@ -180,7 +172,7 @@
 
     @Override
     public void setMember(final String name, final Object value) {
-        put(name, wrap(value, NashornScriptEngine.getNashornGlobal()));
+        put(name, value);
     }
 
     @Override
@@ -275,20 +267,27 @@
 
     @Override
     public Object put(final String key, final Object value) {
+        final ScriptObject oldGlobal = NashornScriptEngine.getNashornGlobal();
+        final boolean globalChanged = (oldGlobal != global);
         return inGlobal(new Callable<Object>() {
             @Override public Object call() {
-                return sobj.put(key, unwrap(value, global));
+                final Object modValue = globalChanged? wrap(value, oldGlobal) : value;
+                return translateUndefined(wrap(sobj.put(key, unwrap(modValue, global)), global));
             }
         });
     }
 
     @Override
     public void putAll(final Map<? extends String, ? extends Object> map) {
+        final ScriptObject oldGlobal = NashornScriptEngine.getNashornGlobal();
+        final boolean globalChanged = (oldGlobal != global);
         final boolean strict = sobj.isStrictContext();
         inGlobal(new Callable<Object>() {
             @Override public Object call() {
                 for (final Map.Entry<? extends String, ? extends Object> entry : map.entrySet()) {
-                    sobj.set(entry.getKey(), unwrap(entry.getValue(), global), strict);
+                    final Object value = entry.getValue();
+                    final Object modValue = globalChanged? wrap(value, oldGlobal) : value;
+                    sobj.set(entry.getKey(), unwrap(modValue, global), strict);
                 }
                 return null;
             }
@@ -321,7 +320,7 @@
                 final Iterator<Object> iter   = sobj.valueIterator();
 
                 while (iter.hasNext()) {
-                    values.add(wrap(iter.next(), global));
+                    values.add(translateUndefined(wrap(iter.next(), global)));
                 }
 
                 return Collections.unmodifiableList(values);
--- a/src/jdk/nashorn/internal/runtime/ScriptFunction.java	Thu Feb 14 09:14:31 2013 +0530
+++ b/src/jdk/nashorn/internal/runtime/ScriptFunction.java	Thu Feb 14 14:16:58 2013 +0530
@@ -192,7 +192,7 @@
      * @return ScriptFunction result.
      * @throws Throwable if there is an exception/error with the invocation or thrown from it
      */
-    public Object invoke(final Object self, final Object... arguments) throws Throwable {
+    Object invoke(final Object self, final Object... arguments) throws Throwable {
         if (Context.DEBUG) {
             invokes++;
         }
--- a/src/jdk/nashorn/internal/runtime/ScriptRuntime.java	Thu Feb 14 09:14:31 2013 +0530
+++ b/src/jdk/nashorn/internal/runtime/ScriptRuntime.java	Thu Feb 14 14:16:58 2013 +0530
@@ -302,6 +302,37 @@
     }
 
     /**
+     * Check that the target function is associated with current Context. And also make sure that 'self', if
+     * ScriptObject, is from current context.
+     *
+     * Call a function given self and args. If the number of the arguments is known in advance, you can likely achieve
+     * better performance by {@link Bootstrap#createDynamicInvoker(String, Class, Class...) creating a dynamic invoker}
+     * for operation {@code "dyn:call"}, then using its {@link MethodHandle#invokeExact(Object...)} method instead.
+     *
+     * @param target ScriptFunction object.
+     * @param self   Receiver in call.
+     * @param args   Call arguments.
+     * @return Call result.
+     */
+    public static Object checkAndApply(final ScriptFunction target, final Object self, final Object... args) {
+        final ScriptObject global = Context.getGlobalTrusted();
+        if (! (global instanceof GlobalObject)) {
+            throw new IllegalStateException("No current global set");
+        }
+
+        if (target.getContext() != global.getContext()) {
+            throw new IllegalArgumentException("'target' function is not from current Context");
+        }
+
+        if (self instanceof ScriptObject && ((ScriptObject)self).getContext() != global.getContext()) {
+            throw new IllegalArgumentException("'self' object is not from current Context");
+        }
+
+        // all in order - call real 'apply'
+        return apply(target, self, args);
+    }
+
+    /**
      * Call a function given self and args. If the number of the arguments is known in advance, you can likely achieve
      * better performance by {@link Bootstrap#createDynamicInvoker(String, Class, Class...) creating a dynamic invoker}
      * for operation {@code "dyn:call"}, then using its {@link MethodHandle#invokeExact(Object...)} method instead.
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/script/basic/JDK-8008197.js	Thu Feb 14 14:16:58 2013 +0530
@@ -0,0 +1,69 @@
+/*
+ * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ * 
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ * 
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ * 
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ * 
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/**
+ * JDK-8008197: Cross script engine function calls do not work as expected
+ *
+ * @test
+ * @run
+ */
+
+
+var m = new javax.script.ScriptEngineManager();
+var e = m.getEngineByName("nashorn");
+
+var obj = {
+    func: function(str) { 
+        return /hello/.exec(str);
+    }
+};
+
+// set our object as object to the engine created
+e.put("obj", obj);
+
+// try to invoke method from the other engine
+if (e.eval("obj.func('hello')") == null) {
+    fail("FAILED: #1 obj.func('hello') returns null");
+}
+
+// define an object in the engine
+ e.eval("var foo = { callMe: function(callback) { return callback() } }");
+
+// try to invoke a script method from here but callback is from this engine
+var res = e.invokeMethod(e.get("foo"), "callMe", function() {
+    return /nashorn/;
+});
+
+if (! res.exec("nashorn")) {
+    fail("FAILED: #2 /nashorn/ does not match 'nashorn'");
+}
+
+// invoke a script method from here with callback from this engine.
+// This uses JSObject.call interface
+res = e.get("foo").callMe(function() {
+    return /ecmascript/;
+});
+
+if (! res.exec("ecmascript")) {
+    fail("FAILED: #3 /ecmascript/ does not match 'ecmascript'");
+}
--- a/test/script/basic/jquery.js	Thu Feb 14 09:14:31 2013 +0530
+++ b/test/script/basic/jquery.js	Thu Feb 14 14:16:58 2013 +0530
@@ -60,23 +60,13 @@
     var name;
     
     try {    
+	var split = url.split('/');
+	name = split[split.length - 1];
+	var path  = __DIR__ + "../external/jquery/" + name;
 	try {
-	    var split = url.split('/');
-	    name = split[split.length - 1];
-	    var path  = __DIR__ + "../external/jquery/" + name;
-	    try {
-		load(path);
-	    } catch (e) {
-		checkWindow(e);
-	    }
+	    load(path);
 	} catch (e) {
-	    // try to load it from the internet, if we for some licensing reason
-	    // aren't allowed to ship third party code under MIT license
-	    try {
-		load(url);
-	    } catch (e) {
-		checkWindow(e);
-	    }
+	    checkWindow(e);
 	}
     } catch (e) {
 	print("Unexpected exception " + e);
@@ -85,8 +75,6 @@
     print("done " + name);
 }
 
-
-
 for each (url in urls) {
     test_jquery(url);
 }