changeset 473:f22ca0f9b6ee

8021361: ClassCastException:.ScriptObjectMirror -> ScriptObject when getInterface called on object from different ScriptContext Reviewed-by: jlaskey, attila
author sundar
date Thu, 25 Jul 2013 20:10:48 +0530
parents f74faac51bfb
children d55856f82352 f6588f168d79
files src/jdk/nashorn/api/scripting/NashornScriptEngine.java src/jdk/nashorn/api/scripting/ScriptObjectMirror.java src/jdk/nashorn/api/scripting/resources/Messages.properties src/jdk/nashorn/internal/runtime/ScriptObject.java test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java
diffstat 5 files changed, 254 insertions(+), 25 deletions(-) [+]
line wrap: on
line diff
--- a/src/jdk/nashorn/api/scripting/NashornScriptEngine.java	Thu Jul 25 11:56:12 2013 +0200
+++ b/src/jdk/nashorn/api/scripting/NashornScriptEngine.java	Thu Jul 25 20:10:48 2013 +0530
@@ -40,6 +40,9 @@
 import java.security.PrivilegedAction;
 import java.security.PrivilegedActionException;
 import java.security.PrivilegedExceptionAction;
+import java.text.MessageFormat;
+import java.util.Locale;
+import java.util.ResourceBundle;
 import javax.script.AbstractScriptEngine;
 import javax.script.Bindings;
 import javax.script.Compilable;
@@ -79,6 +82,28 @@
     // default options passed to Nashorn Options object
     private static final String[] DEFAULT_OPTIONS = new String[] { "-scripting", "-doe" };
 
+    private static final String MESSAGES_RESOURCE = "jdk.nashorn.api.scripting.resources.Messages";
+
+    // Without do privileged, under security manager messages can not be loaded.
+    private static final ResourceBundle MESSAGES_BUNDLE;
+    static {
+        MESSAGES_BUNDLE = AccessController.doPrivileged(
+        new PrivilegedAction<ResourceBundle>() {
+            @Override
+            public ResourceBundle run() {
+                return ResourceBundle.getBundle(MESSAGES_RESOURCE, Locale.getDefault());
+            }
+        });
+    }
+
+    private static String getMessage(final String msgId, final String... args) {
+        try {
+            return new MessageFormat(MESSAGES_BUNDLE.getString(msgId)).format(args);
+        } catch (final java.util.MissingResourceException e) {
+            throw new RuntimeException("no message resource found for message id: "+ msgId);
+        }
+    }
+
     NashornScriptEngine(final NashornScriptEngineFactory factory, final ClassLoader appLoader) {
         this(factory, DEFAULT_OPTIONS, appLoader);
     }
@@ -176,43 +201,63 @@
     }
 
     @Override
-    public Object invokeMethod(final Object self, final String name, final Object... args)
+    public Object invokeMethod(final Object thiz, final String name, final Object... args)
             throws ScriptException, NoSuchMethodException {
-        if (self == null) {
-            throw new IllegalArgumentException("script object can not be null");
+        if (thiz == null) {
+            throw new IllegalArgumentException(getMessage("thiz.cannot.be.null"));
         }
-        return invokeImpl(self, name, args);
+        return invokeImpl(thiz, name, args);
     }
 
-    private <T> T getInterfaceInner(final Object self, final Class<T> clazz) {
+    private <T> T getInterfaceInner(final Object thiz, final Class<T> clazz) {
         if (clazz == null || !clazz.isInterface()) {
-            throw new IllegalArgumentException("interface Class expected");
+            throw new IllegalArgumentException(getMessage("interface.class.expected"));
         }
 
         // perform security access check as early as possible
         final SecurityManager sm = System.getSecurityManager();
         if (sm != null) {
             if (! Modifier.isPublic(clazz.getModifiers())) {
-                throw new SecurityException("attempt to implement non-public interfce: " + clazz);
+                throw new SecurityException(getMessage("implementing.non.public.interface", clazz.getName()));
             }
             Context.checkPackageAccess(clazz.getName());
         }
 
-        final ScriptObject realSelf;
-        final ScriptObject ctxtGlobal = getNashornGlobalFrom(context);
-        if(self == null) {
-            realSelf = ctxtGlobal;
-        } else if (!(self instanceof ScriptObject)) {
-            realSelf = (ScriptObject)ScriptObjectMirror.unwrap(self, ctxtGlobal);
-        } else {
-            realSelf = (ScriptObject)self;
+        ScriptObject realSelf = null;
+        ScriptObject realGlobal = null;
+        if(thiz == null) {
+            // making interface out of global functions
+            realSelf = realGlobal = getNashornGlobalFrom(context);
+        } else if (thiz instanceof ScriptObjectMirror) {
+            final ScriptObjectMirror mirror = (ScriptObjectMirror)thiz;
+            realSelf = mirror.getScriptObject();
+            realGlobal = mirror.getHomeGlobal();
+            if (! realGlobal.isOfContext(nashornContext)) {
+                throw new IllegalArgumentException(getMessage("script.object.from.another.engine"));
+            }
+        } else if (thiz instanceof ScriptObject) {
+            // called from script code.
+            realSelf = (ScriptObject)thiz;
+            realGlobal = Context.getGlobal();
+            if (realGlobal == null) {
+                throw new IllegalArgumentException(getMessage("no.current.nashorn.global"));
+            }
+
+            if (! realGlobal.isOfContext(nashornContext)) {
+                throw new IllegalArgumentException(getMessage("script.object.from.another.engine"));
+            }
+        }
+
+        if (realSelf == null) {
+            throw new IllegalArgumentException(getMessage("interface.on.non.script.object"));
         }
 
         try {
             final ScriptObject oldGlobal = Context.getGlobal();
+            final boolean globalChanged = (oldGlobal != realGlobal);
             try {
-                if(oldGlobal != ctxtGlobal) {
-                    Context.setGlobal(ctxtGlobal);
+                if (globalChanged) {
+                    Context.setGlobal(realGlobal);
                 }
 
                 if (! isInterfaceImplemented(clazz, realSelf)) {
@@ -220,7 +265,7 @@
                 }
                 return clazz.cast(JavaAdapterFactory.getConstructor(realSelf.getClass(), clazz).invoke(realSelf));
             } finally {
-                if(oldGlobal != ctxtGlobal) {
+                if (globalChanged) {
                     Context.setGlobal(oldGlobal);
                 }
             }
@@ -237,11 +282,11 @@
     }
 
     @Override
-    public <T> T getInterface(final Object self, final Class<T> clazz) {
-        if (self == null) {
-            throw new IllegalArgumentException("script object can not be null");
+    public <T> T getInterface(final Object thiz, final Class<T> clazz) {
+        if (thiz == null) {
+            throw new IllegalArgumentException(getMessage("thiz.cannot.be.null"));
         }
-        return getInterfaceInner(self, clazz);
+        return getInterfaceInner(thiz, clazz);
     }
 
     // These are called from the "engine.js" script
@@ -362,13 +407,22 @@
         ScriptObjectMirror selfMirror = null;
         if (selfObject instanceof ScriptObjectMirror) {
             selfMirror = (ScriptObjectMirror)selfObject;
+            if (! selfMirror.getHomeGlobal().isOfContext(nashornContext)) {
+                throw new IllegalArgumentException(getMessage("script.object.from.another.engine"));
+            }
         } 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);
+            if (oldGlobal == null) {
+                throw new IllegalArgumentException(getMessage("no.current.nashorn.global"));
             }
+
+            if (! oldGlobal.isOfContext(nashornContext)) {
+                throw new IllegalArgumentException(getMessage("script.object.from.another.engine"));
+            }
+
+            selfMirror = (ScriptObjectMirror)ScriptObjectMirror.wrap(selfObject, oldGlobal);
         } else if (selfObject == null) {
             // selfObject is null => global function call
             final ScriptObject ctxtGlobal = getNashornGlobalFrom(context);
@@ -389,7 +443,7 @@
         }
 
         // Non-script object passed as selfObject
-        throw new IllegalArgumentException("can not call invokeMethod on non-script objects");
+        throw new IllegalArgumentException(getMessage("interface.on.non.script.object"));
     }
 
     private Object evalImpl(final char[] buf, final ScriptContext ctxt) throws ScriptException {
--- a/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java	Thu Jul 25 11:56:12 2013 +0200
+++ b/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java	Thu Jul 25 20:10:48 2013 +0530
@@ -599,14 +599,22 @@
     // package-privates below this.
 
     ScriptObjectMirror(final ScriptObject sobj, final ScriptObject global) {
+        assert sobj != null : "ScriptObjectMirror on null!";
+        assert global != null : "null global for ScriptObjectMirror!";
+
         this.sobj = sobj;
         this.global = global;
     }
 
+    // accessors for script engine
     ScriptObject getScriptObject() {
         return sobj;
     }
 
+    ScriptObject getHomeGlobal() {
+        return global;
+    }
+
     static Object translateUndefined(Object obj) {
         return (obj == ScriptRuntime.UNDEFINED)? null : obj;
     }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/src/jdk/nashorn/api/scripting/resources/Messages.properties	Thu Jul 25 20:10:48 2013 +0530
@@ -0,0 +1,32 @@
+#
+# 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.  Oracle designates this
+# particular file as subject to the "Classpath" exception as provided
+# by Oracle in the LICENSE file that accompanied this code.
+#
+# 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.
+#
+
+thiz.cannot.be.null=script object 'this' for getMethod, getInterface calls can not be null
+interface.class.expected=interface Class expected in getInterface
+interface.on.non.script.object=getInterface cannot be called on non-script object
+no.current.nashorn.global=no current Global instance for nashorn
+implementing.non.public.interface=Cannot implement non-public interface: {0}
+script.object.from.another.engine=Script object belongs to another script engine
+
--- a/src/jdk/nashorn/internal/runtime/ScriptObject.java	Thu Jul 25 11:56:12 2013 +0200
+++ b/src/jdk/nashorn/internal/runtime/ScriptObject.java	Thu Jul 25 20:10:48 2013 +0530
@@ -1046,6 +1046,15 @@
     }
 
     /**
+     * Checks if this object belongs to the given context
+     * @param ctx context to check against
+     * @return true if this object belongs to the given context
+     */
+    public final boolean isOfContext(final Context ctx) {
+        return context == ctx;
+    }
+
+    /**
      * Return the current context from the object's map.
      * @return Current context.
      */
--- a/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java	Thu Jul 25 11:56:12 2013 +0200
+++ b/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java	Thu Jul 25 20:10:48 2013 +0530
@@ -363,6 +363,90 @@
     }
 
     @Test
+    /**
+     * Check that we can get interface out of a script object even after
+     * switching to use different ScriptContext.
+     */
+    public void getInterfaceDifferentContext() {
+       ScriptEngineManager m = new ScriptEngineManager();
+       ScriptEngine e = m.getEngineByName("nashorn");
+       try {
+           Object obj = e.eval("({ run: function() { } })");
+
+           // change script context
+           ScriptContext ctxt = new SimpleScriptContext();
+           ctxt.setBindings(e.createBindings(), ScriptContext.ENGINE_SCOPE);
+           e.setContext(ctxt);
+
+           Runnable r = ((Invocable)e).getInterface(obj, Runnable.class);
+           r.run();
+       }catch (final Exception exp) {
+            exp.printStackTrace();
+            fail(exp.getMessage());
+       }
+    }
+
+    @Test
+    /**
+     * Check that getInterface on non-script object 'thiz' results in IllegalArgumentException.
+     */
+    public void getInterfaceNonScriptObjectThizTest() {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine e = m.getEngineByName("nashorn");
+
+        try {
+            ((Invocable)e).getInterface(new Object(), Runnable.class);
+            fail("should have thrown IllegalArgumentException");
+        } catch (final Exception exp) {
+            if (! (exp instanceof IllegalArgumentException)) {
+                exp.printStackTrace();
+                fail(exp.getMessage());
+            }
+        }
+    }
+
+    @Test
+    /**
+     * Check that getInterface on null 'thiz' results in IllegalArgumentException.
+     */
+    public void getInterfaceNullThizTest() {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine e = m.getEngineByName("nashorn");
+
+        try {
+            ((Invocable)e).getInterface(null, Runnable.class);
+            fail("should have thrown IllegalArgumentException");
+        } catch (final Exception exp) {
+            if (! (exp instanceof IllegalArgumentException)) {
+                exp.printStackTrace();
+                fail(exp.getMessage());
+            }
+        }
+    }
+
+    @Test
+    /**
+     * Check that calling getInterface on mirror created by another engine results in IllegalArgumentException.
+     */
+    public void getInterfaceMixEnginesTest() {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine engine1 = m.getEngineByName("nashorn");
+        final ScriptEngine engine2 = m.getEngineByName("nashorn");
+
+        try {
+            Object obj = engine1.eval("({ run: function() {} })");
+            // pass object from engine1 to engine2 as 'thiz' for getInterface
+            ((Invocable)engine2).getInterface(obj, Runnable.class);
+            fail("should have thrown IllegalArgumentException");
+        } catch (final Exception exp) {
+            if (! (exp instanceof IllegalArgumentException)) {
+                exp.printStackTrace();
+                fail(exp.getMessage());
+            }
+        }
+    }
+
+    @Test
     public void accessGlobalTest() {
         final ScriptEngineManager m = new ScriptEngineManager();
         final ScriptEngine e = m.getEngineByName("nashorn");
@@ -734,6 +818,48 @@
     }
 
     @Test
+    /**
+     * Check that calling method on null 'thiz' results in IllegalArgumentException.
+     */
+    public void invokeMethodNullThizTest() {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine e = m.getEngineByName("nashorn");
+
+        try {
+            ((Invocable)e).invokeMethod(null, "toString");
+            fail("should have thrown IllegalArgumentException");
+        } catch (final Exception exp) {
+            if (! (exp instanceof IllegalArgumentException)) {
+                exp.printStackTrace();
+                fail(exp.getMessage());
+            }
+        }
+    }
+
+
+    @Test
+    /**
+     * Check that calling method on mirror created by another engine results in IllegalArgumentException.
+     */
+    public void invokeMethodMixEnginesTest() {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine engine1 = m.getEngineByName("nashorn");
+        final ScriptEngine engine2 = m.getEngineByName("nashorn");
+
+        try {
+            Object obj = engine1.eval("({ run: function() {} })");
+            // pass object from engine1 to engine2 as 'thiz' for invokeMethod
+            ((Invocable)engine2).invokeMethod(obj, "run");
+            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");