8021252: invokeMethod throws NoSuchMethodException when script object is from different script context
authorsundar
Thu, 25 Jul 2013 14:05:03 +0530
changeset 19099 30230d3febb8
parent 19098 473dfe87bb7b
child 19100 62d400be5b44
8021252: invokeMethod throws NoSuchMethodException when script object is from different script context Reviewed-by: lagergren, hannesw
nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java
nashorn/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java
nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java
--- a/nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java	Wed Jul 24 21:01:22 2013 +0530
+++ b/nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java	Thu Jul 25 14:05:03 2013 +0530
@@ -357,50 +357,39 @@
     }
 
     private Object invokeImpl(final Object selfObject, final String name, final Object... args) throws ScriptException, NoSuchMethodException {
-        final ScriptObject oldGlobal     = Context.getGlobal();
-        final ScriptObject ctxtGlobal    = getNashornGlobalFrom(context);
-        final boolean globalChanged = (oldGlobal != ctxtGlobal);
-
-        Object self = globalChanged? ScriptObjectMirror.wrap(selfObject, oldGlobal) : selfObject;
-
-        try {
-            if (globalChanged) {
-                Context.setGlobal(ctxtGlobal);
-            }
-
-            ScriptObject sobj;
-            Object       value = null;
-
-            self = ScriptObjectMirror.unwrap(self, ctxtGlobal);
+        name.getClass(); // null check
 
-            // FIXME: should convert when self is not ScriptObject
-            if (self instanceof ScriptObject) {
-                sobj = (ScriptObject)self;
-                value = sobj.get(name);
-            } else if (self == null) {
-                self  = ctxtGlobal;
-                sobj  = ctxtGlobal;
-                value = sobj.get(name);
+        ScriptObjectMirror selfMirror = null;
+        if (selfObject instanceof ScriptObjectMirror) {
+            selfMirror = (ScriptObjectMirror)selfObject;
+        } else if (selfObject instanceof ScriptObject) {
+            // invokeMethod called from script code - in which case we may get 'naked' ScriptObject
+            // Wrap it with oldGlobal to make a ScriptObjectMirror for the same.
+            final ScriptObject oldGlobal = Context.getGlobal();
+            if (oldGlobal != null) {
+                selfMirror = (ScriptObjectMirror)ScriptObjectMirror.wrap(selfObject, oldGlobal);
             }
+        } else if (selfObject == null) {
+            // selfObject is null => global function call
+            final ScriptObject ctxtGlobal = getNashornGlobalFrom(context);
+            selfMirror = (ScriptObjectMirror)ScriptObjectMirror.wrap(ctxtGlobal, ctxtGlobal);
+        }
 
-            if (value instanceof ScriptFunction) {
-                final Object res;
-                try {
-                    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");
+        if (selfMirror != null) {
+            try {
+                return ScriptObjectMirror.translateUndefined(selfMirror.call(name, args));
+            } catch (final Exception e) {
+                final Throwable cause = e.getCause();
+                if (cause instanceof NoSuchMethodException) {
+                    throw (NoSuchMethodException)cause;
                 }
-                return ScriptObjectMirror.translateUndefined(ScriptObjectMirror.wrap(res, ctxtGlobal));
-            }
-
-            throw new NoSuchMethodException(name);
-        } finally {
-            if (globalChanged) {
-                Context.setGlobal(oldGlobal);
+                throwAsScriptException(e);
+                throw new AssertionError("should not reach here");
             }
         }
+
+        // Non-script object passed as selfObject
+        throw new IllegalArgumentException("can not call invokeMethod on non-script objects");
     }
 
     private Object evalImpl(final char[] buf, final ScriptContext ctxt) throws ScriptException {
--- a/nashorn/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java	Wed Jul 24 21:01:22 2013 +0530
+++ b/nashorn/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java	Thu Jul 25 14:05:03 2013 +0530
@@ -89,7 +89,7 @@
 
             final Object val = functionName == null? sobj : sobj.get(functionName);
             if (! (val instanceof ScriptFunction)) {
-                throw new RuntimeException("No such function " + ((functionName != null)? functionName : ""));
+                throw new NoSuchMethodException("No such function " + ((functionName != null)? functionName : ""));
             }
 
             final Object[] modArgs = globalChanged? wrapArray(args, oldGlobal) : args;
@@ -630,5 +630,4 @@
             }
         }
     }
-
 }
--- a/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java	Wed Jul 24 21:01:22 2013 +0530
+++ b/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java	Thu Jul 25 14:05:03 2013 +0530
@@ -649,6 +649,91 @@
     }
 
     @Test
+    /**
+     * Check that we can call invokeMethod on an object that we got by evaluating
+     * script with different Context set.
+     */
+    public void invokeMethodDifferentContextTest() {
+       ScriptEngineManager m = new ScriptEngineManager();
+       ScriptEngine e = m.getEngineByName("nashorn");
+
+       try {
+           // define an object with method on it
+           Object obj = e.eval("({ hello: function() { return 'Hello World!'; } })");
+
+           final ScriptContext ctxt = new SimpleScriptContext();
+           ctxt.setBindings(e.createBindings(), ScriptContext.ENGINE_SCOPE);
+           e.setContext(ctxt);
+
+           // invoke 'func' on obj - but with current script context changed
+           final Object res = ((Invocable)e).invokeMethod(obj, "hello");
+           assertEquals(res, "Hello World!");
+       } catch (final Exception exp) {
+           exp.printStackTrace();
+           fail(exp.getMessage());
+       }
+    }
+
+    @Test
+    /**
+     * Check that invokeMethod throws NPE on null method name.
+     */
+    public void invokeMethodNullNameTest() {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine e = m.getEngineByName("nashorn");
+
+        try {
+            final Object obj = e.eval("({})");
+            final Object res = ((Invocable)e).invokeMethod(obj, null);
+            fail("should have thrown NPE");
+        } catch (final Exception exp) {
+            if (! (exp instanceof NullPointerException)) {
+                exp.printStackTrace();
+                fail(exp.getMessage());
+            }
+        }
+    }
+
+    @Test
+    /**
+     * Check that invokeMethod throws NoSuchMethodException on missing method.
+     */
+    public void invokeMethodMissingTest() {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine e = m.getEngineByName("nashorn");
+
+        try {
+            final Object obj = e.eval("({})");
+            final Object res = ((Invocable)e).invokeMethod(obj, "nonExistentMethod");
+            fail("should have thrown NoSuchMethodException");
+        } catch (final Exception exp) {
+            if (! (exp instanceof NoSuchMethodException)) {
+                exp.printStackTrace();
+                fail(exp.getMessage());
+            }
+        }
+    }
+
+    @Test
+    /**
+     * Check that calling method on non-script object 'thiz' results in IllegalArgumentException.
+     */
+    public void invokeMethodNonScriptObjectThizTest() {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine e = m.getEngineByName("nashorn");
+
+        try {
+            ((Invocable)e).invokeMethod(new Object(), "toString");
+            fail("should have thrown IllegalArgumentException");
+        } catch (final Exception exp) {
+            if (! (exp instanceof IllegalArgumentException)) {
+                exp.printStackTrace();
+                fail(exp.getMessage());
+            }
+        }
+    }
+
+    @Test
     public void noEnumerablePropertiesTest() {
         final ScriptEngineManager m = new ScriptEngineManager();
         final ScriptEngine e = m.getEngineByName("nashorn");
@@ -767,6 +852,70 @@
     }
 
     @Test
+    /**
+     * check that null function name results in NPE.
+     */
+    public void invokeFunctionNullNameTest() {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine e = m.getEngineByName("nashorn");
+
+        try {
+            final Object res = ((Invocable)e).invokeFunction(null);
+            fail("should have thrown NPE");
+        } catch (final Exception exp) {
+            if (! (exp instanceof NullPointerException)) {
+                exp.printStackTrace();
+                fail(exp.getMessage());
+            }
+        }
+    }
+
+    @Test
+    /**
+     * Check that attempt to call missing function results in NoSuchMethodException.
+     */
+    public void invokeFunctionMissingTest() {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine e = m.getEngineByName("nashorn");
+
+        try {
+            final Object res = ((Invocable)e).invokeFunction("NonExistentFunc");
+            fail("should have thrown NoSuchMethodException");
+        } catch (final Exception exp) {
+            if (! (exp instanceof NoSuchMethodException)) {
+                exp.printStackTrace();
+                fail(exp.getMessage());
+            }
+        }
+    }
+
+    @Test
+    /**
+     * Check that invokeFunction calls functions only from current context's Bindings.
+     */
+    public void invokeFunctionDifferentContextTest() {
+        ScriptEngineManager m = new ScriptEngineManager();
+        ScriptEngine e = m.getEngineByName("nashorn");
+
+        try {
+            // define an object with method on it
+            Object obj = e.eval("function hello() { return 'Hello World!'; }");
+            final ScriptContext ctxt = new SimpleScriptContext();
+            ctxt.setBindings(e.createBindings(), ScriptContext.ENGINE_SCOPE);
+            // change engine's current context
+            e.setContext(ctxt);
+
+            ((Invocable)e).invokeFunction("hello"); // no 'hello' in new context!
+            fail("should have thrown NoSuchMethodException");
+        } catch (final Exception exp) {
+            if (! (exp instanceof NoSuchMethodException)) {
+                exp.printStackTrace();
+                fail(exp.getMessage());
+            }
+        }
+    }
+
+    @Test
     public void invokeFunctionExceptionTest() {
         final ScriptEngineManager m = new ScriptEngineManager();
         final ScriptEngine e = m.getEngineByName("nashorn");