8068603: ScriptObjectMirror should reject null/empty string/non-string parameters in Bindings methods
Reviewed-by: hannesw, sundar
--- a/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/api/scripting/ScriptObjectMirror.java Mon Jan 19 16:07:16 2015 +0100
+++ b/nashorn/src/jdk.scripting.nashorn/share/classes/jdk/nashorn/api/scripting/ScriptObjectMirror.java Tue Jan 20 12:34:21 2015 +0100
@@ -340,9 +340,10 @@
@Override
public boolean containsKey(final Object key) {
+ checkKey(key);
return inGlobal(new Callable<Boolean>() {
@Override public Boolean call() {
- return sobj.containsKey(unwrap(key, global));
+ return sobj.containsKey(key);
}
});
}
@@ -376,6 +377,7 @@
@Override
public Object get(final Object key) {
+ checkKey(key);
return inGlobal(new Callable<Object>() {
@Override public Object call() {
return translateUndefined(wrap(sobj.get(key), global));
@@ -410,6 +412,7 @@
@Override
public Object put(final String key, final Object value) {
+ checkKey(key);
final ScriptObject oldGlobal = Context.getGlobal();
final boolean globalChanged = (oldGlobal != global);
return inGlobal(new Callable<Object>() {
@@ -422,6 +425,9 @@
@Override
public void putAll(final Map<? extends String, ? extends Object> map) {
+ if (map == null) {
+ throw new NullPointerException("map is null");
+ }
final ScriptObject oldGlobal = Context.getGlobal();
final boolean globalChanged = (oldGlobal != global);
inGlobal(new Callable<Object>() {
@@ -429,7 +435,9 @@
for (final Map.Entry<? extends String, ? extends Object> entry : map.entrySet()) {
final Object value = entry.getValue();
final Object modValue = globalChanged? wrap(value, oldGlobal) : value;
- sobj.set(entry.getKey(), unwrap(modValue, global), getCallSiteFlags());
+ final String key = entry.getKey();
+ checkKey(key);
+ sobj.set(key, unwrap(modValue, global), getCallSiteFlags());
}
return null;
}
@@ -438,9 +446,10 @@
@Override
public Object remove(final Object key) {
+ checkKey(key);
return inGlobal(new Callable<Object>() {
@Override public Object call() {
- return wrap(sobj.remove(unwrap(key, global), strict), global);
+ return wrap(sobj.remove(key, strict), global);
}
});
}
@@ -629,7 +638,7 @@
}
/**
- * Utilitity to convert this script object to the given type.
+ * Utility to convert this script object to the given type.
*
* @param <T> destination type to convert to
* @param type destination type to convert to
@@ -786,6 +795,24 @@
}
}
+ /**
+ * Ensures the key is not null, empty string, or a non-String object. The contract of the {@link Bindings}
+ * interface requires that these are not accepted as keys.
+ * @param key the key to check
+ * @throws NullPointerException if key is null
+ * @throws ClassCastException if key is not a String
+ * @throws IllegalArgumentException if key is empty string
+ */
+ private static void checkKey(final Object key) {
+ if (key == null) {
+ throw new NullPointerException("key can not be null");
+ } else if (!(key instanceof String)) {
+ throw new ClassCastException("key should be a String. It is " + key.getClass().getName() + " instead.");
+ } else if (((String)key).length() == 0) {
+ throw new IllegalArgumentException("key can not be empty");
+ }
+ }
+
@Override
public double toNumber() {
return inGlobal(new Callable<Double>() {
--- a/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java Mon Jan 19 16:07:16 2015 +0100
+++ b/nashorn/test/src/jdk/nashorn/api/scripting/ScriptEngineTest.java Tue Jan 20 12:34:21 2015 +0100
@@ -36,10 +36,12 @@
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
+import java.util.Collections;
import java.util.concurrent.Callable;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
import java.util.function.Function;
+import javax.script.Bindings;
import javax.script.Compilable;
import javax.script.CompiledScript;
import javax.script.Invocable;
@@ -719,6 +721,128 @@
assertTrue(invoked.get());
}
+ // @bug JDK-8068603: NashornScriptEngine.put/get() impls don't conform to NPE, IAE spec assertions
+ @Test
+ public void illegalBindingsValuesTest() throws Exception {
+ final ScriptEngineManager manager = new ScriptEngineManager();
+ final ScriptEngine e = manager.getEngineByName("nashorn");
+
+ try {
+ e.put(null, "null-value");
+ fail();
+ } catch (NullPointerException x) {
+ // expected
+ }
+
+ try {
+ e.put("", "empty-value");
+ fail();
+ } catch (IllegalArgumentException x) {
+ // expected
+ }
+
+ final Bindings b = e.getBindings(ScriptContext.ENGINE_SCOPE);
+ assertTrue(b instanceof ScriptObjectMirror);
+
+ try {
+ b.put(null, "null-value");
+ fail();
+ } catch (NullPointerException x) {
+ // expected
+ }
+
+ try {
+ b.put("", "empty-value");
+ fail();
+ } catch (IllegalArgumentException x) {
+ // expected
+ }
+
+ try {
+ b.get(null);
+ fail();
+ } catch (NullPointerException x) {
+ // expected
+ }
+
+ try {
+ b.get("");
+ fail();
+ } catch (IllegalArgumentException x) {
+ // expected
+ }
+
+ try {
+ b.get(1);
+ fail();
+ } catch (ClassCastException x) {
+ // expected
+ }
+
+ try {
+ b.remove(null);
+ fail();
+ } catch (NullPointerException x) {
+ // expected
+ }
+
+ try {
+ b.remove("");
+ fail();
+ } catch (IllegalArgumentException x) {
+ // expected
+ }
+
+ try {
+ b.remove(1);
+ fail();
+ } catch (ClassCastException x) {
+ // expected
+ }
+
+ try {
+ b.containsKey(null);
+ fail();
+ } catch (NullPointerException x) {
+ // expected
+ }
+
+ try {
+ b.containsKey("");
+ fail();
+ } catch (IllegalArgumentException x) {
+ // expected
+ }
+
+ try {
+ b.containsKey(1);
+ fail();
+ } catch (ClassCastException x) {
+ // expected
+ }
+
+ try {
+ b.putAll(null);
+ fail();
+ } catch (NullPointerException x) {
+ // expected
+ }
+
+ try {
+ b.putAll(Collections.singletonMap((String)null, "null-value"));
+ fail();
+ } catch (NullPointerException x) {
+ // expected
+ }
+
+ try {
+ b.putAll(Collections.singletonMap("", "empty-value"));
+ fail();
+ } catch (IllegalArgumentException x) {
+ // expected
+ }
+ }
+
private static void checkProperty(final ScriptEngine e, final String name)
throws ScriptException {
final String value = System.getProperty(name);