8008103: Source object should maintain URL of the script source as a private field

Reviewed-by: lagergren, jlaskey

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

@@ -453,7 +453,7 @@ public final class NashornScriptEngine extends AbstractScriptEngine implements C
                 setNashornGlobal(ctxtGlobal);
             }
 
-            return nashornContext.compileScript(source, ctxtGlobal, nashornContext._strict);
+            return nashornContext.compileScript(source, ctxtGlobal);
         } catch (final Exception e) {
             throwAsScriptException(e);
             throw new AssertionError("should not reach here");

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

@@ -492,29 +492,11 @@ public final class Context {
      *
      * @param source the source
      * @param scope  the scope
-     * @param strict are we in strict mode
      *
      * @return top level function for script
      */
-    public ScriptFunction compileScript(final Source source, final ScriptObject scope, final boolean strict) {
+    public ScriptFunction compileScript(final Source source, final ScriptObject scope) {
-        return compileScript(source, scope, this.errors, strict);
+        return compileScript(source, scope, this.errors);
-    }
-
-    /**
-     * Compile a top level script - no Source given, but an URL to
-     * load it from
-     *
-     * @param name    name of script/source
-     * @param url     URL to source
-     * @param scope   the scope
-     * @param strict  are we in strict mode
-     *
-     * @return top level function for the script
-     *
-     * @throws IOException if URL cannot be resolved
-     */
-    public ScriptFunction compileScript(final String name, final URL url, final ScriptObject scope, final boolean strict) throws IOException {
-        return compileScript(name, url, scope, this.errors, strict);
     }
 
     /**
@@ -591,26 +573,25 @@ public final class Context {
      * expression
      *
      * @param scope  the scope
-     * @param source source expression for script
+     * @param from   source expression for script
      *
      * @return return value for load call (undefined)
      *
      * @throws IOException if source cannot be found or loaded
      */
-    public Object load(final ScriptObject scope, final Object source) throws IOException {
+    public Object load(final ScriptObject scope, final Object from) throws IOException {
-        Object src = source;
+        Object src = (from instanceof ConsString)?  from.toString() : from;
-        URL url = null;
+        Source source = null;
-        String srcName = null;
 
-        if (src instanceof ConsString) {
+        // load accepts a String (which could be a URL or a file name), a File, a URL
-            src = src.toString();
+        // or a ScriptObject that has "name" and "source" (string valued) properties.
-        }
         if (src instanceof String) {
-            srcName = (String)src;
+            String srcStr = (String)src;
             final File file = new File((String)src);
-            if (srcName.indexOf(':') != -1) {
+            if (srcStr.indexOf(':') != -1) {
                 try {
-                    url = new URL((String)src);
+                    final URL url = new URL((String)src);
+                    source = new Source(url.toString(), url);
                 } catch (final MalformedURLException e) {
                     // fallback URL - nashorn:foo.js - check under jdk/nashorn/internal/runtime/resources
                     final String str = (String)src;
@@ -618,7 +599,7 @@ public final class Context {
                         final String resource = "resources/" + str.substring("nashorn:".length());
                         // NOTE: even sandbox scripts should be able to load scripts in nashorn: scheme
                         // These scripts are always available and are loaded from nashorn.jar's resources.
-                        final Source code = AccessController.doPrivileged(
+                        source = AccessController.doPrivileged(
                                 new PrivilegedAction<Source>() {
                                     @Override
                                     public Source run() {
@@ -630,45 +611,32 @@ public final class Context {
                                         }
                                     }
                                 });
-                        if (code == null) {
-                            throw e;
-                        }
-                        return evaluateSource(code, scope, scope);
                     } else {
                         throw e;
                     }
                 }
             } else if (file.isFile()) {
-                url = file.toURI().toURL();
+                source = new Source(srcStr, file);
             }
-            src = url;
+        } else if (src instanceof File && ((File)src).isFile()) {
-        }
-
-        if (src instanceof File && ((File)src).isFile()) {
             final File file = (File)src;
-            url = file.toURI().toURL();
+            source = new Source(file.getName(), file);
-            if (srcName == null) {
-                srcName = file.getCanonicalPath();
-            }
         } else if (src instanceof URL) {
-            url = (URL)src;
+            final URL url = (URL)src;
-            if (srcName == null) {
+            source = new Source(url.toString(), url);
-                srcName = url.toString();
-            }
-        }
-
-        if (url != null) {
-            assert srcName != null : "srcName null here!";
-            return evaluateSource(srcName, url, scope, scope);
         } else if (src instanceof ScriptObject) {
             final ScriptObject sobj = (ScriptObject)src;
             if (sobj.has("script") && sobj.has("name")) {
                 final String script = JSType.toString(sobj.get("script"));
                 final String name   = JSType.toString(sobj.get("name"));
-                return evaluateSource(new Source(name, script), scope, scope);
+                source = new Source(name, script);
             }
         }
 
+        if (source != null) {
+            return evaluateSource(source, scope, scope);
+        }
+
         typeError("cant.load.script", ScriptRuntime.safeToString(source));
 
         return UNDEFINED;
@@ -850,23 +818,11 @@ public final class Context {
         return (context != null) ? context : Context.getContextTrusted();
     }
 
-    private Object evaluateSource(final String name, final URL url, final ScriptObject scope, final ScriptObject thiz) throws IOException {
-        ScriptFunction script = null;
-
-        try {
-            script = compileScript(name, url, scope, new Context.ThrowErrorManager(), _strict);
-        } catch (final ParserException e) {
-            e.throwAsEcmaException();
-        }
-
-        return ScriptRuntime.apply(script, thiz);
-    }
-
     private Object evaluateSource(final Source source, final ScriptObject scope, final ScriptObject thiz) {
         ScriptFunction script = null;
 
         try {
-            script = compileScript(source, scope, new Context.ThrowErrorManager(), _strict);
+            script = compileScript(source, scope, new Context.ThrowErrorManager());
         } catch (final ParserException e) {
             e.throwAsEcmaException();
         }
@@ -902,19 +858,11 @@ public final class Context {
         return ((GlobalObject)Context.getGlobalTrusted()).newScriptFunction(RUN_SCRIPT.tag(), runMethodHandle, scope, strict);
     }
 
-    private ScriptFunction compileScript(final String name, final URL url, final ScriptObject scope, final ErrorManager errMan, final boolean strict) throws IOException {
+    private ScriptFunction compileScript(final Source source, final ScriptObject scope, final ErrorManager errMan) {
-        return getRunScriptFunction(compile(new Source(name, url), url, errMan, strict), scope);
+        return getRunScriptFunction(compile(source, errMan, this._strict), scope);
     }
 
-    private ScriptFunction compileScript(final Source source, final ScriptObject scope, final ErrorManager errMan, final boolean strict) {
+    private synchronized Class<?> compile(final Source source, final ErrorManager errMan, final boolean strict) {
-        return getRunScriptFunction(compile(source, null, errMan, strict), scope);
-    }
-
-    private Class<?> compile(final Source source, final ErrorManager errMan, final boolean strict) {
-        return compile(source, null, errMan, strict);
-    }
-
-   private synchronized Class<?> compile(final Source source, final URL url, final ErrorManager errMan, final boolean strict) {
         // start with no errors, no warnings.
         errMan.reset();
 
@@ -935,6 +883,7 @@ public final class Context {
             return null;
         }
 
+        final URL url = source.getURL();
         final ScriptLoader loader = _loader_per_compile ? createNewLoader() : scriptLoader;
         final CodeSource   cs     = url == null ? null : new CodeSource(url, (CodeSigner[])null);
 

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

@@ -71,13 +71,20 @@ public final class Source {
     /** Cached hash code */
     private int hash;
 
+    /** Source URL if available */
+    private final URL url;
+
     private static final int BUFSIZE = 8 * 1024;
 
-    private Source(final String name, final String base, final char[] content) {
+    // Do *not* make this public ever! Trusts the URL and content. So has to be called
+    // from other public constructors. Note that this can not be some init method as
+    // we initialize final fields from here.
+    private Source(final String name, final String base, final char[] content, final URL url) {
         this.name    = name;
         this.base    = base;
         this.content = content;
         this.length  = content.length;
+        this.url     = url;
     }
 
     /**
@@ -87,7 +94,7 @@ public final class Source {
      * @param content contents as char array
      */
     public Source(final String name, final char[] content) {
-        this(name, baseName(name, null), content);
+        this(name, baseName(name, null), content, null);
     }
 
     /**
@@ -109,7 +116,7 @@ public final class Source {
      * @throws IOException if source cannot be loaded
      */
     public Source(final String name, final URL url) throws IOException {
-        this(name, baseURL(url, null), readFully(url.openStream()));
+        this(name, baseURL(url, null), readFully(url.openStream()), url);
     }
 
     /**
@@ -121,7 +128,7 @@ public final class Source {
      * @throws IOException if source cannot be loaded
      */
     public Source(final String name, final File file) throws IOException {
-        this(name, dirName(file, null), readFully(file));
+        this(name, dirName(file, null), readFully(file), getURLFromFile(file));
     }
 
     @Override
@@ -205,6 +212,16 @@ public final class Source {
         return new String(content, start, end - start);
     }
 
+    /**
+     * Returns the source URL of this script Source. Can be null if Source
+     * was created from a String or a char[].
+     *
+     * @return URL source or null
+     */
+    public URL getURL() {
+        return url;
+    }
+
     /**
      * Find the beginning of the line containing position.
      * @param position Index to offending token.
@@ -288,7 +305,7 @@ public final class Source {
      * @return content
      */
     public char[] getContent() {
-        return content;
+        return content.clone();
     }
 
     /**
@@ -433,4 +450,12 @@ public final class Source {
     public String toString() {
         return getName();
     }
+
+    private static URL getURLFromFile(final File file) {
+        try {
+            return file.toURI().toURL();
+        } catch (final SecurityException | MalformedURLException ignored) {
+            return null;
+        }
+    }
 }

nashorn/src/jdk/nashorn/tools/Shell.java

@@ -282,7 +282,7 @@ public class Shell {
             // For each file on the command line.
             for (final String fileName : files) {
                 final File file = new File(fileName);
-                ScriptFunction script = context.compileScript(fileName, file.toURI().toURL(), global, context._strict);
+                ScriptFunction script = context.compileScript(new Source(fileName, file.toURI().toURL()), global);
                 if (script == null || errors.getNumberOfErrors() != 0) {
                     return COMPILATION_ERROR;
                 }

nashorn/test/src/jdk/nashorn/internal/codegen/CompilerTest.java

@@ -159,7 +159,7 @@ public class CompilerTest {
                 Context.setGlobal(global);
             }
             final Source source = new Source(file.getAbsolutePath(), buffer);
-            final ScriptFunction script = context.compileScript(source, global, context._strict);
+            final ScriptFunction script = context.compileScript(source, global);
             if (script == null || context.getErrorManager().getNumberOfErrors() > 0) {
                 log("Compile failed: " + file.getAbsolutePath());
                 failed++;

nashorn/test/src/jdk/nashorn/internal/runtime/ContextTest.java

@@ -111,7 +111,7 @@ public class ContextTest {
     private Object eval(final Context cx, final String name, final String code) {
         final Source source = new Source(name, code);
         final ScriptObject global = Context.getGlobal();
-        final ScriptFunction func = cx.compileScript(source, global, cx._strict);
+        final ScriptFunction func = cx.compileScript(source, global);
         return func != null ? ScriptRuntime.apply(func, global) : null;
     }
 

nashorn/test/src/jdk/nashorn/internal/test/framework/SharedContextEvaluator.java

@@ -39,6 +39,7 @@ import jdk.nashorn.internal.runtime.ErrorManager;
 import jdk.nashorn.internal.runtime.ScriptFunction;
 import jdk.nashorn.internal.runtime.ScriptObject;
 import jdk.nashorn.internal.runtime.ScriptRuntime;
+import jdk.nashorn.internal.runtime.Source;
 import jdk.nashorn.internal.runtime.options.Options;
 
 /**
@@ -124,7 +125,7 @@ public final class SharedContextEvaluator implements ScriptEvaluator {
                     continue;
                 }
                 final File file = new File(fileName);
-                ScriptFunction script = context.compileScript(fileName, file.toURI().toURL(), global, context._strict);
+                ScriptFunction script = context.compileScript(new Source(fileName, file.toURI().toURL()), global);
                 if (script == null || errors.getNumberOfErrors() != 0) {
                     return COMPILATION_ERROR;
                 }