8020463: Input argument array wrapping in loadWithNewGlobal is wrong

Fri, 12 Jul 2013 20:06:41 +0530

author
sundar
date
Fri, 12 Jul 2013 20:06:41 +0530
changeset 437
5cdf4352ee0b
parent 435
e27ebcfed6fa
child 438
cbfeffbcd3f2
child 439
973d78ee0728

8020463: Input argument array wrapping in loadWithNewGlobal is wrong
Reviewed-by: attila, jlaskey

src/jdk/nashorn/api/scripting/ScriptObjectMirror.java file | annotate | diff | comparison | revisions
src/jdk/nashorn/internal/runtime/Context.java file | annotate | diff | comparison | revisions
test/script/basic/JDK-8020463.js file | annotate | diff | comparison | revisions
test/src/jdk/nashorn/api/scripting/ScriptEngineSecurityTest.java file | annotate | diff | comparison | revisions
test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java file | annotate | diff | comparison | revisions
     1.1 --- a/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java	Fri Jul 12 11:58:42 2013 +0200
     1.2 +++ b/src/jdk/nashorn/api/scripting/ScriptObjectMirror.java	Fri Jul 12 20:06:41 2013 +0530
     1.3 @@ -52,11 +52,6 @@
     1.4      private final ScriptObject sobj;
     1.5      private final ScriptObject global;
     1.6  
     1.7 -    ScriptObjectMirror(final ScriptObject sobj, final ScriptObject global) {
     1.8 -        this.sobj = sobj;
     1.9 -        this.global = global;
    1.10 -    }
    1.11 -
    1.12      @Override
    1.13      public boolean equals(final Object other) {
    1.14          if (other instanceof ScriptObjectMirror) {
    1.15 @@ -81,25 +76,6 @@
    1.16          });
    1.17      }
    1.18  
    1.19 -    private <V> V inGlobal(final Callable<V> callable) {
    1.20 -        final ScriptObject oldGlobal = NashornScriptEngine.getNashornGlobal();
    1.21 -        final boolean globalChanged = (oldGlobal != global);
    1.22 -        if (globalChanged) {
    1.23 -            NashornScriptEngine.setNashornGlobal(global);
    1.24 -        }
    1.25 -        try {
    1.26 -            return callable.call();
    1.27 -        } catch (final RuntimeException e) {
    1.28 -            throw e;
    1.29 -        } catch (final Exception e) {
    1.30 -            throw new AssertionError("Cannot happen", e);
    1.31 -        } finally {
    1.32 -            if (globalChanged) {
    1.33 -                NashornScriptEngine.setNashornGlobal(oldGlobal);
    1.34 -            }
    1.35 -        }
    1.36 -    }
    1.37 -
    1.38      // JSObject methods
    1.39      @Override
    1.40      public Object call(final String functionName, final Object... args) {
    1.41 @@ -212,6 +188,8 @@
    1.42          });
    1.43      }
    1.44  
    1.45 +    // javax.script.Bindings methods
    1.46 +
    1.47      @Override
    1.48      public void clear() {
    1.49          inGlobal(new Callable<Object>() {
    1.50 @@ -379,7 +357,7 @@
    1.51      public Object getProto() {
    1.52          return inGlobal(new Callable<Object>() {
    1.53              @Override public Object call() {
    1.54 -                return wrap(getScriptObject().getProto(), global);
    1.55 +                return wrap(sobj.getProto(), global);
    1.56              }
    1.57          });
    1.58      }
    1.59 @@ -395,7 +373,7 @@
    1.60      public Object getOwnPropertyDescriptor(final String key) {
    1.61          return inGlobal(new Callable<Object>() {
    1.62              @Override public Object call() {
    1.63 -                return wrap(getScriptObject().getOwnPropertyDescriptor(key), global);
    1.64 +                return wrap(sobj.getOwnPropertyDescriptor(key), global);
    1.65              }
    1.66          });
    1.67      }
    1.68 @@ -409,7 +387,7 @@
    1.69      public String[] getOwnKeys(final boolean all) {
    1.70          return inGlobal(new Callable<String[]>() {
    1.71              @Override public String[] call() {
    1.72 -                return getScriptObject().getOwnKeys(all);
    1.73 +                return sobj.getOwnKeys(all);
    1.74              }
    1.75          });
    1.76      }
    1.77 @@ -422,7 +400,7 @@
    1.78      public ScriptObjectMirror preventExtensions() {
    1.79          return inGlobal(new Callable<ScriptObjectMirror>() {
    1.80              @Override public ScriptObjectMirror call() {
    1.81 -                getScriptObject().preventExtensions();
    1.82 +                sobj.preventExtensions();
    1.83                  return ScriptObjectMirror.this;
    1.84              }
    1.85          });
    1.86 @@ -435,7 +413,7 @@
    1.87      public boolean isExtensible() {
    1.88          return inGlobal(new Callable<Boolean>() {
    1.89              @Override public Boolean call() {
    1.90 -                return getScriptObject().isExtensible();
    1.91 +                return sobj.isExtensible();
    1.92              }
    1.93          });
    1.94      }
    1.95 @@ -447,7 +425,7 @@
    1.96      public ScriptObjectMirror seal() {
    1.97          return inGlobal(new Callable<ScriptObjectMirror>() {
    1.98              @Override public ScriptObjectMirror call() {
    1.99 -                getScriptObject().seal();
   1.100 +                sobj.seal();
   1.101                  return ScriptObjectMirror.this;
   1.102              }
   1.103          });
   1.104 @@ -460,7 +438,7 @@
   1.105      public boolean isSealed() {
   1.106          return inGlobal(new Callable<Boolean>() {
   1.107              @Override public Boolean call() {
   1.108 -                return getScriptObject().isSealed();
   1.109 +                return sobj.isSealed();
   1.110              }
   1.111          });
   1.112      }
   1.113 @@ -472,7 +450,7 @@
   1.114      public ScriptObjectMirror freeze() {
   1.115          return inGlobal(new Callable<ScriptObjectMirror>() {
   1.116              @Override public ScriptObjectMirror call() {
   1.117 -                getScriptObject().freeze();
   1.118 +                sobj.freeze();
   1.119                  return ScriptObjectMirror.this;
   1.120              }
   1.121          });
   1.122 @@ -485,7 +463,7 @@
   1.123      public boolean isFrozen() {
   1.124          return inGlobal(new Callable<Boolean>() {
   1.125              @Override public Boolean call() {
   1.126 -                return getScriptObject().isFrozen();
   1.127 +                return sobj.isFrozen();
   1.128              }
   1.129          });
   1.130      }
   1.131 @@ -507,12 +485,39 @@
   1.132  
   1.133          return inGlobal(new Callable<Boolean>() {
   1.134              @Override public Boolean call() {
   1.135 -                return getScriptObject().isInstance(instance.getScriptObject());
   1.136 +                return sobj.isInstance(instance.sobj);
   1.137              }
   1.138          });
   1.139      }
   1.140  
   1.141      /**
   1.142 +     * is this a function object?
   1.143 +     *
   1.144 +     * @return if this mirror wraps a ECMAScript function instance
   1.145 +     */
   1.146 +    public boolean isFunction() {
   1.147 +        return sobj instanceof ScriptFunction;
   1.148 +    }
   1.149 +
   1.150 +    /**
   1.151 +     * is this a 'use strict' function object?
   1.152 +     *
   1.153 +     * @return true if this mirror represents a ECMAScript 'use strict' function
   1.154 +     */
   1.155 +    public boolean isStrictFunction() {
   1.156 +        return isFunction() && ((ScriptFunction)sobj).isStrict();
   1.157 +    }
   1.158 +
   1.159 +    /**
   1.160 +     * is this an array object?
   1.161 +     *
   1.162 +     * @return if this mirror wraps a ECMAScript array object
   1.163 +     */
   1.164 +    public boolean isArray() {
   1.165 +        return sobj.isArray();
   1.166 +    }
   1.167 +
   1.168 +    /**
   1.169       * Utility to check if given object is ECMAScript undefined value
   1.170       *
   1.171       * @param obj object to check
   1.172 @@ -523,35 +528,6 @@
   1.173      }
   1.174  
   1.175      /**
   1.176 -     * is this a function object?
   1.177 -     *
   1.178 -     * @return if this mirror wraps a ECMAScript function instance
   1.179 -     */
   1.180 -    public boolean isFunction() {
   1.181 -        return getScriptObject() instanceof ScriptFunction;
   1.182 -    }
   1.183 -
   1.184 -    /**
   1.185 -     * is this a 'use strict' function object?
   1.186 -     *
   1.187 -     * @return true if this mirror represents a ECMAScript 'use strict' function
   1.188 -     */
   1.189 -    public boolean isStrictFunction() {
   1.190 -        return isFunction() && ((ScriptFunction)getScriptObject()).isStrict();
   1.191 -    }
   1.192 -
   1.193 -    /**
   1.194 -     * is this an array object?
   1.195 -     *
   1.196 -     * @return if this mirror wraps a ECMAScript array object
   1.197 -     */
   1.198 -    public boolean isArray() {
   1.199 -        return getScriptObject().isArray();
   1.200 -    }
   1.201 -
   1.202 -    // These are public only so that Context can access these.
   1.203 -
   1.204 -    /**
   1.205       * Make a script object mirror on given object if needed.
   1.206       *
   1.207       * @param obj object to be wrapped
   1.208 @@ -621,6 +597,12 @@
   1.209      }
   1.210  
   1.211      // package-privates below this.
   1.212 +
   1.213 +    ScriptObjectMirror(final ScriptObject sobj, final ScriptObject global) {
   1.214 +        this.sobj = sobj;
   1.215 +        this.global = global;
   1.216 +    }
   1.217 +
   1.218      ScriptObject getScriptObject() {
   1.219          return sobj;
   1.220      }
   1.221 @@ -628,4 +610,25 @@
   1.222      static Object translateUndefined(Object obj) {
   1.223          return (obj == ScriptRuntime.UNDEFINED)? null : obj;
   1.224      }
   1.225 +
   1.226 +    // internals only below this.
   1.227 +    private <V> V inGlobal(final Callable<V> callable) {
   1.228 +        final ScriptObject oldGlobal = NashornScriptEngine.getNashornGlobal();
   1.229 +        final boolean globalChanged = (oldGlobal != global);
   1.230 +        if (globalChanged) {
   1.231 +            NashornScriptEngine.setNashornGlobal(global);
   1.232 +        }
   1.233 +        try {
   1.234 +            return callable.call();
   1.235 +        } catch (final RuntimeException e) {
   1.236 +            throw e;
   1.237 +        } catch (final Exception e) {
   1.238 +            throw new AssertionError("Cannot happen", e);
   1.239 +        } finally {
   1.240 +            if (globalChanged) {
   1.241 +                NashornScriptEngine.setNashornGlobal(oldGlobal);
   1.242 +            }
   1.243 +        }
   1.244 +    }
   1.245 +
   1.246  }
     2.1 --- a/src/jdk/nashorn/internal/runtime/Context.java	Fri Jul 12 11:58:42 2013 +0200
     2.2 +++ b/src/jdk/nashorn/internal/runtime/Context.java	Fri Jul 12 20:06:41 2013 +0530
     2.3 @@ -526,11 +526,13 @@
     2.4          });
     2.5          setGlobalTrusted(newGlobal);
     2.6  
     2.7 -        final Object[] wrapped = args == null? ScriptRuntime.EMPTY_ARRAY :  ScriptObjectMirror.wrapArray(args, newGlobal);
     2.8 +        final Object[] wrapped = args == null? ScriptRuntime.EMPTY_ARRAY :  ScriptObjectMirror.wrapArray(args, oldGlobal);
     2.9          newGlobal.put("arguments", ((GlobalObject)newGlobal).wrapAsObject(wrapped));
    2.10  
    2.11          try {
    2.12 -            return ScriptObjectMirror.wrap(load(newGlobal, from), newGlobal);
    2.13 +            // wrap objects from newGlobal's world as mirrors - but if result
    2.14 +            // is from oldGlobal's world, unwrap it!
    2.15 +            return ScriptObjectMirror.unwrap(ScriptObjectMirror.wrap(load(newGlobal, from), newGlobal), oldGlobal);
    2.16          } finally {
    2.17              setGlobalTrusted(oldGlobal);
    2.18          }
     3.1 --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
     3.2 +++ b/test/script/basic/JDK-8020463.js	Fri Jul 12 20:06:41 2013 +0530
     3.3 @@ -0,0 +1,54 @@
     3.4 +/*
     3.5 + * Copyright (c) 2010, 2013, Oracle and/or its affiliates. All rights reserved.
     3.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
     3.7 + * 
     3.8 + * This code is free software; you can redistribute it and/or modify it
     3.9 + * under the terms of the GNU General Public License version 2 only, as
    3.10 + * published by the Free Software Foundation.
    3.11 + * 
    3.12 + * This code is distributed in the hope that it will be useful, but WITHOUT
    3.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
    3.14 + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
    3.15 + * version 2 for more details (a copy is included in the LICENSE file that
    3.16 + * accompanied this code).
    3.17 + * 
    3.18 + * You should have received a copy of the GNU General Public License version
    3.19 + * 2 along with this work; if not, write to the Free Software Foundation,
    3.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
    3.21 + * 
    3.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
    3.23 + * or visit www.oracle.com if you need additional information or have any
    3.24 + * questions.
    3.25 + */
    3.26 +
    3.27 +/**
    3.28 + * JDK-8020463: Input argument array wrapping in loadWithNewGlobal is wrong
    3.29 + *
    3.30 + * @test
    3.31 + * @run
    3.32 + */
    3.33 +
    3.34 +loadWithNewGlobal({
    3.35 +    name: "test",
    3.36 +    script: "arguments[0]();"
    3.37 +}, func);
    3.38 +
    3.39 +function func() {
    3.40 +    try {
    3.41 +        foo;
    3.42 +    } catch (e) {
    3.43 +        if (! (e instanceof ReferenceError)) {
    3.44 +            fail("FAILED: expected ReferenceError!");
    3.45 +        }
    3.46 +    }
    3.47 +}
    3.48 +
    3.49 +// identity
    3.50 +var result = loadWithNewGlobal({
    3.51 +    name: "test2",
    3.52 +    script: "arguments[0]"
    3.53 +}, this);
    3.54 +
    3.55 +if (result !== this) {
    3.56 +    fail("FAILED: expected global to be returned 'as is'");
    3.57 +}
     4.1 --- a/test/src/jdk/nashorn/api/scripting/ScriptEngineSecurityTest.java	Fri Jul 12 11:58:42 2013 +0200
     4.2 +++ b/test/src/jdk/nashorn/api/scripting/ScriptEngineSecurityTest.java	Fri Jul 12 20:06:41 2013 +0530
     4.3 @@ -27,7 +27,10 @@
     4.4  
     4.5  import static org.testng.Assert.fail;
     4.6  
     4.7 +import java.util.Objects;
     4.8 +import javax.script.Invocable;
     4.9  import javax.script.ScriptEngine;
    4.10 +import javax.script.ScriptException;
    4.11  import javax.script.ScriptEngineManager;
    4.12  import org.testng.annotations.Test;
    4.13  
    4.14 @@ -44,6 +47,7 @@
    4.15      public void securityPackagesTest() {
    4.16          if (System.getSecurityManager() == null) {
    4.17              // pass vacuously
    4.18 +            return;
    4.19          }
    4.20  
    4.21          final ScriptEngineManager m = new ScriptEngineManager();
    4.22 @@ -64,6 +68,7 @@
    4.23      public void securityJavaTypeTest() {
    4.24          if (System.getSecurityManager() == null) {
    4.25              // pass vacuously
    4.26 +            return;
    4.27          }
    4.28  
    4.29          final ScriptEngineManager m = new ScriptEngineManager();
    4.30 @@ -84,6 +89,7 @@
    4.31      public void securityClassForNameTest() {
    4.32          if (System.getSecurityManager() == null) {
    4.33              // pass vacuously
    4.34 +            return;
    4.35          }
    4.36  
    4.37          final ScriptEngineManager m = new ScriptEngineManager();
    4.38 @@ -104,6 +110,7 @@
    4.39      public void securitySystemExit() {
    4.40          if (System.getSecurityManager() == null) {
    4.41              // pass vacuously
    4.42 +            return;
    4.43          }
    4.44  
    4.45          final ScriptEngineManager m = new ScriptEngineManager();
    4.46 @@ -124,6 +131,7 @@
    4.47      public void securitySystemLoadLibrary() {
    4.48          if (System.getSecurityManager() == null) {
    4.49              // pass vacuously
    4.50 +            return;
    4.51          }
    4.52  
    4.53          final ScriptEngineManager m = new ScriptEngineManager();
    4.54 @@ -139,4 +147,40 @@
    4.55              }
    4.56          }
    4.57      }
    4.58 +
    4.59 +    @Test
    4.60 +    /**
    4.61 +     * Check that script can't implement sensitive package interfaces.
    4.62 +     */
    4.63 +    public void checkSensitiveInterfaceImplTest() throws ScriptException {
    4.64 +        if (System.getSecurityManager() == null) {
    4.65 +            // pass vacuously
    4.66 +            return;
    4.67 +        }
    4.68 +
    4.69 +        final ScriptEngineManager m = new ScriptEngineManager();
    4.70 +        final ScriptEngine e = m.getEngineByName("nashorn");
    4.71 +        final Object[] holder = new Object[1];
    4.72 +        e.put("holder", holder);
    4.73 +        // put an empty script object into array
    4.74 +        e.eval("holder[0] = {}");
    4.75 +        // holder[0] is an object of some subclass of ScriptObject
    4.76 +        Class ScriptObjectClass = holder[0].getClass().getSuperclass();
    4.77 +        Class PropertyAccessClass = ScriptObjectClass.getInterfaces()[0];
    4.78 +        // implementation methods for PropertyAccess class
    4.79 +        e.eval("function set() {}; function get() {}; function getInt(){} " +
    4.80 +               "function getDouble(){}; function getLong() {}; " +
    4.81 +               "this.delete = function () {}; function has() {}; " +
    4.82 +               "function hasOwnProperty() {}");
    4.83 +
    4.84 +        // get implementation of a restricted package interface
    4.85 +        try {
    4.86 +            log(Objects.toString(((Invocable)e).getInterface((Class<?>)PropertyAccessClass)));
    4.87 +            fail("should have thrown SecurityException");
    4.88 +        } catch (final Exception exp) {
    4.89 +            if (! (exp instanceof SecurityException)) {
    4.90 +                fail("SecurityException expected, got " + exp);
    4.91 +            }
    4.92 +        }
    4.93 +    }
    4.94  }
     5.1 --- a/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java	Fri Jul 12 11:58:42 2013 +0200
     5.2 +++ b/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java	Fri Jul 12 20:06:41 2013 +0530
     5.3 @@ -945,35 +945,4 @@
     5.4          Assert.assertEquals(itf.test1(42, "a", "b"), "i == 42, strings instanceof java.lang.String[] == true, strings == [a, b]");
     5.5          Assert.assertEquals(itf.test2(44, "c", "d", "e"), "arguments[0] == 44, arguments[1] instanceof java.lang.String[] == true, arguments[1] == [c, d, e]");
     5.6      }
     5.7 -
     5.8 -    @Test
     5.9 -    /**
    5.10 -     * Check that script can't implement sensitive package interfaces.
    5.11 -     */
    5.12 -    public void checkSensitiveInterfaceImplTest() throws ScriptException {
    5.13 -        final ScriptEngineManager m = new ScriptEngineManager();
    5.14 -        final ScriptEngine e = m.getEngineByName("nashorn");
    5.15 -        final Object[] holder = new Object[1];
    5.16 -        e.put("holder", holder);
    5.17 -        // put an empty script object into array
    5.18 -        e.eval("holder[0] = {}");
    5.19 -        // holder[0] is an object of some subclass of ScriptObject
    5.20 -        Class ScriptObjectClass = holder[0].getClass().getSuperclass();
    5.21 -        Class PropertyAccessClass = ScriptObjectClass.getInterfaces()[0];
    5.22 -        // implementation methods for PropertyAccess class
    5.23 -        e.eval("function set() {}; function get() {}; function getInt(){} " +
    5.24 -               "function getDouble(){}; function getLong() {}; " +
    5.25 -               "this.delete = function () {}; function has() {}; " +
    5.26 -               "function hasOwnProperty() {}");
    5.27 -
    5.28 -        // get implementation of a restricted package interface
    5.29 -        try {
    5.30 -            log(Objects.toString(((Invocable)e).getInterface((Class<?>)PropertyAccessClass)));
    5.31 -            fail("should have thrown SecurityException");
    5.32 -        } catch (final Exception exp) {
    5.33 -            if (! (exp instanceof SecurityException)) {
    5.34 -                fail("SecurityException expected, got " + exp);
    5.35 -            }
    5.36 -        }
    5.37 -    }
    5.38  }

mercurial