diff --git a/rhino/src/main/java/org/mozilla/javascript/AbstractEcmaObjectOperations.java b/rhino/src/main/java/org/mozilla/javascript/AbstractEcmaObjectOperations.java index 2dc3cfdb10..c89d41e354 100644 --- a/rhino/src/main/java/org/mozilla/javascript/AbstractEcmaObjectOperations.java +++ b/rhino/src/main/java/org/mozilla/javascript/AbstractEcmaObjectOperations.java @@ -122,9 +122,15 @@ static boolean setIntegrityLevel(Context cx, Object o, INTEGRITY_LEVEL level) { 7. both conditions under which false would be returned cannot occur here 8. both conditions under which false would be returned cannot occur here */ + // TODO check .preventExtensions() return value once implemented and act accordingly to spec + + if (o instanceof NativeJavaObject) { + ((NativeJavaObject) o).freezeObject(); + return true; + } + ScriptableObject obj = ScriptableObject.ensureScriptableObject(o); - // TODO check .preventExtensions() return value once implemented and act accordingly to spec obj.preventExtensions(); for (Object key : obj.getIds(true, true)) { diff --git a/rhino/src/main/java/org/mozilla/javascript/NativeJavaArray.java b/rhino/src/main/java/org/mozilla/javascript/NativeJavaArray.java index 2253ba9dc6..9753463549 100644 --- a/rhino/src/main/java/org/mozilla/javascript/NativeJavaArray.java +++ b/rhino/src/main/java/org/mozilla/javascript/NativeJavaArray.java @@ -99,6 +99,9 @@ public void put(String id, Scriptable start, Object value) { @Override public void put(int index, Scriptable start, Object value) { if (0 <= index && index < length) { + if (super.checkFrozen(Integer.toString(index)) == FrozenCheckResult.SILENTLY_IGNORE) + return; + Array.set(array, index, Context.jsToJava(value, cls)); } else { throw Context.reportRuntimeErrorById( diff --git a/rhino/src/main/java/org/mozilla/javascript/NativeJavaClass.java b/rhino/src/main/java/org/mozilla/javascript/NativeJavaClass.java index 7f41b07243..818417a914 100644 --- a/rhino/src/main/java/org/mozilla/javascript/NativeJavaClass.java +++ b/rhino/src/main/java/org/mozilla/javascript/NativeJavaClass.java @@ -94,6 +94,7 @@ public Object get(String name, Scriptable start) { @Override public void put(String name, Scriptable start, Object value) { + if (super.checkFrozen(name) == FrozenCheckResult.SILENTLY_IGNORE) return; members.put(this, name, javaObject, value, true); } diff --git a/rhino/src/main/java/org/mozilla/javascript/NativeJavaList.java b/rhino/src/main/java/org/mozilla/javascript/NativeJavaList.java index f292a6f8d7..e36db91f80 100644 --- a/rhino/src/main/java/org/mozilla/javascript/NativeJavaList.java +++ b/rhino/src/main/java/org/mozilla/javascript/NativeJavaList.java @@ -124,6 +124,9 @@ public Object get(Symbol key, Scriptable start) { @Override public void put(int index, Scriptable start, Object value) { if (index >= 0) { + if (super.checkFrozen(Integer.toString(index)) == FrozenCheckResult.SILENTLY_IGNORE) + return; + Object javaValue = Context.jsToJava(value, Object.class); if (index == list.size()) { list.add(javaValue); // use "add" at the end of list. @@ -139,6 +142,8 @@ public void put(int index, Scriptable start, Object value) { @Override public void put(String name, Scriptable start, Object value) { if (list != null && "length".equals(name)) { + if (super.checkFrozen(name) == FrozenCheckResult.SILENTLY_IGNORE) return; + setLength(value); return; } diff --git a/rhino/src/main/java/org/mozilla/javascript/NativeJavaMap.java b/rhino/src/main/java/org/mozilla/javascript/NativeJavaMap.java index 752f2c0edf..7a26ee9eef 100644 --- a/rhino/src/main/java/org/mozilla/javascript/NativeJavaMap.java +++ b/rhino/src/main/java/org/mozilla/javascript/NativeJavaMap.java @@ -109,6 +109,7 @@ public Object get(Symbol key, Scriptable start) { public void put(String name, Scriptable start, Object value) { Context cx = Context.getCurrentContext(); if (cx != null && cx.hasFeature(Context.FEATURE_ENABLE_JAVA_MAP_ACCESS)) { + if (super.checkFrozen(name) == FrozenCheckResult.SILENTLY_IGNORE) return; map.put(name, Context.jsToJava(value, Object.class)); } else { super.put(name, start, value); @@ -119,6 +120,8 @@ public void put(String name, Scriptable start, Object value) { public void put(int index, Scriptable start, Object value) { Context cx = Context.getContext(); if (cx != null && cx.hasFeature(Context.FEATURE_ENABLE_JAVA_MAP_ACCESS)) { + if (super.checkFrozen(Integer.toString(index)) == FrozenCheckResult.SILENTLY_IGNORE) + return; map.put(Integer.valueOf(index), Context.jsToJava(value, Object.class)); } else { super.put(index, start, value); diff --git a/rhino/src/main/java/org/mozilla/javascript/NativeJavaObject.java b/rhino/src/main/java/org/mozilla/javascript/NativeJavaObject.java index 033737b68f..6cc2b3aa28 100644 --- a/rhino/src/main/java/org/mozilla/javascript/NativeJavaObject.java +++ b/rhino/src/main/java/org/mozilla/javascript/NativeJavaObject.java @@ -115,9 +115,13 @@ public void put(String name, Scriptable start, Object value) { // We could be asked to modify the value of a property in the // prototype. Since we can't add a property to a Java object, // we modify it in the prototype rather than copy it down. - if (prototype == null || members.has(name, false)) + if (prototype == null || members.has(name, false)) { + if (checkFrozen(name) == FrozenCheckResult.SILENTLY_IGNORE) return; + members.put(this, name, javaObject, value, false); - else prototype.put(name, prototype, value); + } else { + prototype.put(name, prototype, value); + } } @Override @@ -127,6 +131,8 @@ public void put(Symbol symbol, Scriptable start, Object value) { // we modify it in the prototype rather than copy it down. String name = symbol.toString(); if (prototype == null || members.has(name, false)) { + if (checkFrozen(symbol.toString()) == FrozenCheckResult.SILENTLY_IGNORE) return; + members.put(this, name, javaObject, value, false); } else if (prototype instanceof SymbolScriptable) { ((SymbolScriptable) prototype).put(symbol, prototype, value); @@ -249,6 +255,38 @@ public Object getDefaultValue(Class hint) { return value; } + /** + * Freeze this object, preventing changes to the values of existing properties. It is already + * not possible to defined new properties on this class, so we do not need to distinguish like + * {@link ScriptableObject#isExtensible} and {@link ScriptableObject#isSealed}. + */ + public void freezeObject() { + this.isFrozen = true; + } + + /** + * Checks whether the object is frozen or not. Can either throw an exception (in strict mode) or + * return whether we should perform or silently ignore the assignment. + */ + protected FrozenCheckResult checkFrozen(String id) { + if (isFrozen) { + // In strict mode, we throw an error. In non-strict mode, we silently ignore the + // assignment + if (Context.isCurrentContextStrict()) { + String msg = "Assignment to \"" + id + "\" on frozen object in strict mode"; + throw ScriptRuntime.constructError("ReferenceError", msg); + } else return FrozenCheckResult.SILENTLY_IGNORE; + } + + // Go ahead! + return FrozenCheckResult.PERFORM_ASSIGNMENT; + } + + protected enum FrozenCheckResult { + SILENTLY_IGNORE, + PERFORM_ASSIGNMENT, + } + /** * Determine whether we can/should convert between the given type and the desired one. This * should be superceded by a conversion-cost calculation function, but for now I'll hide behind @@ -958,6 +996,7 @@ protected String getTag() { protected Scriptable parent; protected transient Object javaObject; + protected transient boolean isFrozen = false; protected transient Class staticType; protected transient JavaMembers members; diff --git a/rhino/src/main/java/org/mozilla/javascript/NativeObject.java b/rhino/src/main/java/org/mozilla/javascript/NativeObject.java index c3aea11948..ba7aa22e3a 100644 --- a/rhino/src/main/java/org/mozilla/javascript/NativeObject.java +++ b/rhino/src/main/java/org/mozilla/javascript/NativeObject.java @@ -633,7 +633,8 @@ public Object execIdCall( { Object arg = args.length < 1 ? Undefined.instance : args[0]; if (cx.getLanguageVersion() >= Context.VERSION_ES6 - && !(arg instanceof ScriptableObject)) { + && !(arg instanceof ScriptableObject) + && !(arg instanceof NativeJavaObject)) { return arg; } diff --git a/tests/src/test/java/org/mozilla/javascript/tests/NativeJavaObjectFreezingTest.java b/tests/src/test/java/org/mozilla/javascript/tests/NativeJavaObjectFreezingTest.java new file mode 100644 index 0000000000..c5178fd651 --- /dev/null +++ b/tests/src/test/java/org/mozilla/javascript/tests/NativeJavaObjectFreezingTest.java @@ -0,0 +1,474 @@ +/* -*- Mode: java; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. + */ +package org.mozilla.javascript.tests; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; +import org.mozilla.javascript.Context; +import org.mozilla.javascript.ContextFactory; +import org.mozilla.javascript.EcmaError; +import org.mozilla.javascript.EvaluatorException; +import org.mozilla.javascript.NativeJavaObject; +import org.mozilla.javascript.Scriptable; +import org.mozilla.javascript.ScriptableObject; + +public class NativeJavaObjectFreezingTest { + // NativeJavaObject + + @Test + public void cannotAddNewPropertiesToNativeJavaObjects() { + runTest( + (cx, scope) -> { + TestBean bean = new TestBean("abc"); + NativeJavaObject object = new NativeJavaObject(scope, bean, TestBean.class); + + EvaluatorException error = + assertThrows( + EvaluatorException.class, () -> object.put("age", object, 40)); + assertTrue( + error.toString() + .contains( + "has no public instance field or method named \"age\"")); + }); + } + + @Test + public void cannotAddNewPropertiesToNativeJavaObjectsViaJs() { + runTest( + (cx, scope) -> { + String source = + "var o = new Packages." + + TestBean.class.getName() + + "('abc');\n" + + "o.age = 40;"; + EvaluatorException error = + assertThrows( + EvaluatorException.class, + () -> cx.evaluateString(scope, source, "source", 1, null)); + assertTrue( + error.toString() + .contains( + "has no public instance field or method named \"age\"")); + }); + } + + @Test + public void cannotDeletePropertiesOfNativeJavaObjects() { + runTest( + (cx, scope) -> { + TestBean bean = new TestBean("abc"); + NativeJavaObject object = new NativeJavaObject(scope, bean, TestBean.class); + object.delete("name"); + + Object propertyValue = object.get("name", object); + assertTrue(propertyValue instanceof NativeJavaObject); + assertEquals( + "Property still exists", + "abc", + ((NativeJavaObject) propertyValue).unwrap()); + }); + } + + @Test + public void cannotDeletePropertiesOfNativeJavaObjectsViaJs() { + runTest( + (cx, scope) -> { + String source = + "var o = new Packages." + + TestBean.class.getName() + + "('abc');\n" + + "delete o.name;\n" + + "o.name"; + Object result = cx.evaluateString(scope, source, "source", 1, null); + assertTrue(result instanceof NativeJavaObject); + assertEquals( + "Property still exists", "abc", ((NativeJavaObject) result).unwrap()); + }); + } + + @Test + public void nativeJavaObjectCanBeFrozen() { + runTest( + (cx, scope) -> { + TestBean bean = new TestBean("abc"); + NativeJavaObject object = new NativeJavaObject(scope, bean, TestBean.class); + object.freezeObject(); + object.put("name", object, "def"); + + Object propertyValue = object.get("name", object); + assertTrue(propertyValue instanceof NativeJavaObject); + assertEquals( + "Property was not changed", + "abc", + ((NativeJavaObject) propertyValue).unwrap()); + assertEquals("Wrapped object's field was not changed", "abc", bean.name); + }); + } + + @Test + public void nativeJavaObjectCanBeFrozenViaJsNonStrictMode() { + runTest( + (cx, scope) -> { + String source = + "var o = new Packages." + + TestBean.class.getName() + + "('abc');\n" + + "Object.freeze(o);\n" + + "o.name = 'def';\n" + + "o.name"; + Object result = cx.evaluateString(scope, source, "source", 1, null); + assertTrue(result instanceof NativeJavaObject); + assertEquals( + "Property was not changed", + "abc", + ((NativeJavaObject) result).unwrap()); + }); + } + + @Test + public void nativeJavaObjectCanBeFrozenViaJsStrictMode() { + runTest( + (cx, scope) -> { + String source = + "'use strict';\n" + + "var o = new Packages." + + TestBean.class.getName() + + "('abc');\n" + + "Object.freeze(o);\n" + + "o.name = 'def';"; + EcmaError error = + assertThrows( + EcmaError.class, + () -> cx.evaluateString(scope, source, "source", 1, null)); + assertTrue( + "error is: " + error, + error.toString() + .contains( + "ReferenceError: Assignment to \"name\" on frozen object in strict mode")); + }); + } + + @Test + public void cannotUseDefinePropertyOnNativeJavaObject() { + runTest( + (cx, scope) -> { + String source = + "var o = new Packages." + + TestBean.class.getName() + + "('abc');\n" + + "Object.defineProperty(o, 'name', {value: 'def'});\n"; + EcmaError error = + assertThrows( + EcmaError.class, + () -> cx.evaluateString(scope, source, "source", 1, null)); + assertTrue(error.toString().contains("Expected argument of type object")); + }); + } + + // Subclasses - NativeJavaClass + + @Test + public void canFreezeNativeJavaClassViaJsNonStrictMode() { + runTest( + (cx, scope) -> { + String source = + "var c = Packages." + + TestBean.class.getName() + + ";\n" + + "Object.freeze(c);\n" + + "c.aStatic = 'def';\n" + + "c.aStatic"; + Object result = cx.evaluateString(scope, source, "source", 1, null); + assertTrue(result instanceof NativeJavaObject); + assertEquals( + "Property was not changed", + "abc", + ((NativeJavaObject) result).unwrap()); + }); + } + + @Test + public void canFreezeNativeJavaClassViaJsStrictMode() { + runTest( + (cx, scope) -> { + String source = + "'use strict';\n" + + "var c = Packages." + + TestBean.class.getName() + + ";\n" + + "Object.freeze(c);\n" + + "c.aStatic = 'def';\n" + + "c.aStatic"; + EcmaError error = + assertThrows( + EcmaError.class, + () -> cx.evaluateString(scope, source, "source", 1, null)); + assertTrue( + "error is: " + error, + error.toString() + .contains( + "ReferenceError: Assignment to \"aStatic\" on frozen object in strict mode")); + }); + } + + // Subclasses - NativeJavaMap + + @Test + public void canFreezeNativeJavaMapStringKeysViaJsNonStrictMode() { + runTest( + (cx, scope) -> { + String source = + "var m = new Packages.java.util.HashMap();\n" + + "m.name = 'abc';\n" + + "Object.freeze(m);\n" + + "m.name = 'def';\n" + + "m.name"; + Object result = cx.evaluateString(scope, source, "source", 1, null); + assertTrue(result instanceof NativeJavaObject); + assertEquals( + "Property was not created", + "abc", + ((NativeJavaObject) result).unwrap()); + }); + } + + @Test + public void canFreezeNativeJavaMapStringKeysViaJsStrictMode() { + runTest( + (cx, scope) -> { + String source = + "'use strict';\n" + + "var m = new Packages.java.util.HashMap();\n" + + "m.name = 'abc';\n" + + "Object.freeze(m);\n" + + "m.name = 'def';\n" + + "m.name"; + EcmaError error = + assertThrows( + EcmaError.class, + () -> cx.evaluateString(scope, source, "source", 1, null)); + assertTrue( + "error is: " + error, + error.toString() + .contains( + "ReferenceError: Assignment to \"name\" on frozen object in strict mode")); + }); + } + + @Test + public void canFreezeNativeJavaMapIntegerKeysViaJsNonStrictMode() { + runTest( + (cx, scope) -> { + String source = + "var m = new Packages.java.util.HashMap();\n" + + "m[42] = 'abc';\n" + + "Object.freeze(m);\n" + + "m[42] = 'def';\n" + + "m[42]"; + Object result = cx.evaluateString(scope, source, "source", 1, null); + assertTrue(result instanceof NativeJavaObject); + assertEquals( + "Property was not created", + "abc", + ((NativeJavaObject) result).unwrap()); + }); + } + + @Test + public void canFreezeNativeJavaMapIntegerKeysViaJsStrictMode() { + runTest( + (cx, scope) -> { + String source = + "'use strict';\n" + + "var m = new Packages.java.util.HashMap();\n" + + "m[42] = 'abc';\n" + + "Object.freeze(m);\n" + + "m[42] = 'def';\n" + + "m[42]"; + EcmaError error = + assertThrows( + EcmaError.class, + () -> cx.evaluateString(scope, source, "source", 1, null)); + assertTrue( + "error is: " + error, + error.toString() + .contains( + "ReferenceError: Assignment to \"42\" on frozen object in strict mode")); + }); + } + + // Subclasses - NativeJavaList + + @Test + public void canFreezeNativeJavaListViaJsNonStrictMode() { + runTest( + (cx, scope) -> { + String source = + "var l = new Packages.java.util.ArrayList();\n" + + "l.add('abc');" + + "Object.freeze(l);\n" + + "l[0] = 'def';\n" + + "l[0]"; + Object result = cx.evaluateString(scope, source, "source", 1, null); + assertTrue(result instanceof NativeJavaObject); + assertEquals( + "Property was not changed", + "abc", + ((NativeJavaObject) result).unwrap()); + }); + } + + @Test + public void canFreezeNativeJavaListViaJsStrictMode() { + runTest( + (cx, scope) -> { + String source = + "'use strict';\n" + + "var l = new Packages.java.util.ArrayList();\n" + + "l.add('abc');" + + "Object.freeze(l);\n" + + "l[0] = 'def';\n" + + "l[0]"; + EcmaError error = + assertThrows( + EcmaError.class, + () -> cx.evaluateString(scope, source, "source", 1, null)); + assertTrue( + "error is: " + error, + error.toString() + .contains( + "ReferenceError: Assignment to \"0\" on frozen object in strict mode")); + }); + } + + @Test + public void canFreezeNativeJavaListLengthViaJsNonStrictMode() { + runTest( + (cx, scope) -> { + String source = + "var l = new Packages.java.util.ArrayList();\n" + + "l.length = 1;" + + "Object.freeze(l);\n" + + "l.length = 2;\n" + + "l.length"; + Object result = cx.evaluateString(scope, source, "source", 1, null); + assertTrue(result instanceof Number); + assertEquals("Property was not changed", 1, ((Number) result).intValue()); + }); + } + + @Test + public void canFreezeNativeJavaListLengthViaJsStrictMode() { + runTest( + (cx, scope) -> { + String source = + "'use strict';\n" + + "var l = new Packages.java.util.ArrayList();\n" + + "l.length = 1;" + + "Object.freeze(l);\n" + + "l.length = 2;\n" + + "l.length"; + EcmaError error = + assertThrows( + EcmaError.class, + () -> cx.evaluateString(scope, source, "source", 1, null)); + assertTrue( + "error is: " + error, + error.toString() + .contains( + "ReferenceError: Assignment to \"length\" on frozen object in strict mode")); + }); + } + + // Subclasses - NativeJavaArray + + @Test + public void canFreezeNativeJavaArrayViaJsNonStrictMode() { + runTest( + (cx, scope) -> { + String source = + "var l = new Packages.java.util.ArrayList();\n" + + "l.add('abc');\n" + + "var a = l.toArray();\n" + + "Object.freeze(a);\n" + + "a[0] = 'def';\n" + + "a[0]"; + Object result = cx.evaluateString(scope, source, "source", 1, null); + assertTrue(result instanceof NativeJavaObject); + assertEquals( + "Property was not changed", + "abc", + ((NativeJavaObject) result).unwrap()); + }); + } + + @Test + public void canFreezeNativeJavaArrayViaJsStrictMode() { + runTest( + (cx, scope) -> { + String source = + "'use strict';\n" + + "var l = new Packages.java.util.ArrayList();\n" + + "l.add('abc');\n" + + "var a = l.toArray();\n" + + "Object.freeze(a);\n" + + "a[0] = 'def';\n" + + "a[0]"; + EcmaError error = + assertThrows( + EcmaError.class, + () -> cx.evaluateString(scope, source, "source", 1, null)); + assertTrue( + "error is: " + error, + error.toString() + .contains( + "ReferenceError: Assignment to \"0\" on frozen object in strict mode")); + }); + } + + // Trivial class to check property mutation + + public static class TestBean { + public static String aStatic = "abc"; + public String name; + + public TestBean(String name) { + this.name = name; + } + } + + // Factory and helper method + + private final ContextFactory contextFactoryWithMapAccess = + new ContextFactory() { + @Override + protected boolean hasFeature(Context cx, int featureIndex) { + if (featureIndex == Context.FEATURE_ENABLE_JAVA_MAP_ACCESS) { + return true; + } + return super.hasFeature(cx, featureIndex); + } + }; + + @FunctionalInterface + private interface TestRunner { + void run(Context cx, Scriptable scope); + } + + private void runTest(TestRunner testRunner) { + contextFactoryWithMapAccess.call( + cx -> { + cx.setLanguageVersion(Context.VERSION_ES6); + ScriptableObject scope = cx.initStandardObjects(); + testRunner.run(cx, scope); + return null; + }); + } +}