8024973: Using a different ScriptContext with a CompiledScript results in ScriptException
authorsundar
Wed, 18 Sep 2013 16:36:25 +0530
changeset 20210 80d652da38ac
parent 20209 92c2787c959a
child 20211 b447d46339ba
8024973: Using a different ScriptContext with a CompiledScript results in ScriptException Reviewed-by: jlaskey, hannesw
nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java
nashorn/src/jdk/nashorn/internal/runtime/Source.java
nashorn/test/script/trusted/JDK-8008305.js
nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java
--- a/nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java	Wed Sep 18 13:06:17 2013 +0530
+++ b/nashorn/src/jdk/nashorn/api/scripting/NashornScriptEngine.java	Wed Sep 18 16:36:25 2013 +0530
@@ -185,21 +185,12 @@
 
     @Override
     public Object eval(final Reader reader, final ScriptContext ctxt) throws ScriptException {
-        try {
-            if (reader instanceof URLReader) {
-                final URL url = ((URLReader)reader).getURL();
-                final Charset cs = ((URLReader)reader).getCharset();
-                return evalImpl(compileImpl(new Source(url.toString(), url, cs), ctxt), ctxt);
-            }
-            return evalImpl(Source.readFully(reader), ctxt);
-        } catch (final IOException e) {
-            throw new ScriptException(e);
-        }
+        return evalImpl(makeSource(reader, ctxt), ctxt);
     }
 
     @Override
     public Object eval(final String script, final ScriptContext ctxt) throws ScriptException {
-        return evalImpl(script.toCharArray(), ctxt);
+        return evalImpl(makeSource(script, ctxt), ctxt);
     }
 
     @Override
@@ -221,16 +212,12 @@
 
     @Override
     public CompiledScript compile(final Reader reader) throws ScriptException {
-        try {
-            return asCompiledScript(compileImpl(Source.readFully(reader), context));
-        } catch (final IOException e) {
-            throw new ScriptException(e);
-        }
+        return asCompiledScript(makeSource(reader, context));
     }
 
     @Override
     public CompiledScript compile(final String str) throws ScriptException {
-        return asCompiledScript(compileImpl(str.toCharArray(), context));
+        return asCompiledScript(makeSource(str, context));
     }
 
     // Invocable methods
@@ -292,6 +279,29 @@
 
     // Implementation only below this point
 
+    private static Source makeSource(final Reader reader, final ScriptContext ctxt) throws ScriptException {
+        try {
+            if (reader instanceof URLReader) {
+                final URL url = ((URLReader)reader).getURL();
+                final Charset cs = ((URLReader)reader).getCharset();
+                return new Source(url.toString(), url, cs);
+            } else {
+                return new Source(getScriptName(ctxt), Source.readFully(reader));
+            }
+        } catch (final IOException ioExp) {
+            throw new ScriptException(ioExp);
+        }
+    }
+
+    private static Source makeSource(final String src, final ScriptContext ctxt) {
+        return new Source(getScriptName(ctxt), src);
+    }
+
+    private static String getScriptName(final ScriptContext ctxt) {
+        final Object val = ctxt.getAttribute(ScriptEngine.FILENAME);
+        return (val != null) ? val.toString() : "<eval>";
+    }
+
     private <T> T getInterfaceInner(final Object thiz, final Class<T> clazz) {
         if (clazz == null || !clazz.isInterface()) {
             throw new IllegalArgumentException(getMessage("interface.class.expected"));
@@ -429,7 +439,7 @@
         // current ScriptContext exposed as "context"
         // "context" is non-writable from script - but script engine still
         // needs to set it and so save the context Property object
-        contextProperty = newGlobal.addOwnProperty("context", NON_ENUMERABLE_CONSTANT, null);
+        contextProperty = newGlobal.addOwnProperty("context", NON_ENUMERABLE_CONSTANT, ctxt);
         // current ScriptEngine instance exposed as "engine". We added @SuppressWarnings("LeakingThisInConstructor") as
         // NetBeans identifies this assignment as such a leak - this is a false positive as we're setting this property
         // in the Global of a Context we just created - both the Context and the Global were just created and can not be
@@ -509,8 +519,8 @@
         throw new IllegalArgumentException(getMessage("interface.on.non.script.object"));
     }
 
-    private Object evalImpl(final char[] buf, final ScriptContext ctxt) throws ScriptException {
-        return evalImpl(compileImpl(buf, ctxt), ctxt);
+    private Object evalImpl(final Source src, final ScriptContext ctxt) throws ScriptException {
+        return evalImpl(compileImpl(src, ctxt), ctxt);
     }
 
     private Object evalImpl(final ScriptFunction script, final ScriptContext ctxt) throws ScriptException {
@@ -561,11 +571,20 @@
         }
     }
 
-    private CompiledScript asCompiledScript(final ScriptFunction script) {
+    private CompiledScript asCompiledScript(final Source source) throws ScriptException {
+        final ScriptFunction func = compileImpl(source, context);
         return new CompiledScript() {
             @Override
             public Object eval(final ScriptContext ctxt) throws ScriptException {
-                return evalImpl(script, ctxt);
+                final ScriptObject global = getNashornGlobalFrom(ctxt);
+                // Are we running the script in the correct global?
+                if (func.getScope() == global) {
+                    return evalImpl(func, ctxt, global);
+                } else {
+                    // ScriptContext with a different global. Compile again!
+                    // Note that we may still hit per-global compilation cache.
+                    return evalImpl(compileImpl(source, ctxt), ctxt, global);
+                }
             }
             @Override
             public ScriptEngine getEngine() {
@@ -574,12 +593,6 @@
         };
     }
 
-    private ScriptFunction compileImpl(final char[] buf, final ScriptContext ctxt) throws ScriptException {
-        final Object val = ctxt.getAttribute(ScriptEngine.FILENAME);
-        final String fileName = (val != null) ? val.toString() : "<eval>";
-        return compileImpl(new Source(fileName, buf), ctxt);
-    }
-
     private ScriptFunction compileImpl(final Source source, final ScriptContext ctxt) throws ScriptException {
         return compileImpl(source, getNashornGlobalFrom(ctxt));
     }
--- a/nashorn/src/jdk/nashorn/internal/runtime/Source.java	Wed Sep 18 13:06:17 2013 +0530
+++ b/nashorn/src/jdk/nashorn/internal/runtime/Source.java	Wed Sep 18 16:36:25 2013 +0530
@@ -169,7 +169,7 @@
 
         final Source src = (Source)obj;
         // Only compare content as a last resort measure
-        return length == src.length && Objects.equals(name, src.name) && Arrays.equals(content, src.content);
+        return length == src.length && Objects.equals(url, src.url) && Objects.equals(name, src.name) && Arrays.equals(content, src.content);
     }
 
     @Override
--- a/nashorn/test/script/trusted/JDK-8008305.js	Wed Sep 18 13:06:17 2013 +0530
+++ b/nashorn/test/script/trusted/JDK-8008305.js	Wed Sep 18 16:36:25 2013 +0530
@@ -54,6 +54,6 @@
     fail("Expected SecurityException from script!");
 } catch (e) {
     if (! (e instanceof SecurityException)) {
-        faile("Expected SecurityException, but got " + e);
+        fail("Expected SecurityException, but got " + e);
     }
 }
--- a/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java	Wed Sep 18 13:06:17 2013 +0530
+++ b/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java	Wed Sep 18 16:36:25 2013 +0530
@@ -37,10 +37,12 @@
 import java.util.concurrent.Callable;
 import javax.script.Compilable;
 import javax.script.CompiledScript;
+import javax.script.ScriptContext;
 import javax.script.ScriptEngine;
 import javax.script.ScriptEngineFactory;
 import javax.script.ScriptEngineManager;
 import javax.script.ScriptException;
+import javax.script.SimpleScriptContext;
 import org.testng.annotations.Test;
 
 /**
@@ -231,6 +233,17 @@
     }
 
     @Test
+    public void compileAndEvalInDiffContextTest() throws ScriptException {
+        final ScriptEngineManager m = new ScriptEngineManager();
+        final ScriptEngine engine = m.getEngineByName("js");
+        final Compilable compilable = (Compilable) engine;
+        final CompiledScript compiledScript = compilable.compile("foo");
+        final ScriptContext ctxt = new SimpleScriptContext();
+        ctxt.setAttribute("foo", "hello", ScriptContext.ENGINE_SCOPE);
+        assertEquals(compiledScript.eval(ctxt), "hello");
+    }
+
+    @Test
     public void accessGlobalTest() {
         final ScriptEngineManager m = new ScriptEngineManager();
         final ScriptEngine e = m.getEngineByName("nashorn");