From d243106d325ff10cd48b7b7ac99b4cb8273a484e Mon Sep 17 00:00:00 2001 From: Greg Brail Date: Mon, 30 Dec 2024 17:51:00 -0500 Subject: [PATCH] Support more spec-compliant ScriptableObject extensions Add "putOwnProperty" methods, which can return boolean and be used in strict mode, and wire them in so that there is no need to call "putImpl" directly or override it. Also, allow subclasses that don't override the attributes methods in ScriptableObject to work as well. --- .../AbstractEcmaObjectOperations.java | 4 +- .../org/mozilla/javascript/NativeObject.java | 26 +++- .../mozilla/javascript/ScriptableObject.java | 46 ++++-- .../javascript/tests/AssignSubclassTest.java | 134 ++++++++++++++++++ tests/testsrc/test262.properties | 5 - 5 files changed, 194 insertions(+), 21 deletions(-) create mode 100644 rhino/src/test/java/org/mozilla/javascript/tests/AssignSubclassTest.java diff --git a/rhino/src/main/java/org/mozilla/javascript/AbstractEcmaObjectOperations.java b/rhino/src/main/java/org/mozilla/javascript/AbstractEcmaObjectOperations.java index 89534360cc..ab5066e9d7 100644 --- a/rhino/src/main/java/org/mozilla/javascript/AbstractEcmaObjectOperations.java +++ b/rhino/src/main/java/org/mozilla/javascript/AbstractEcmaObjectOperations.java @@ -221,7 +221,7 @@ static void put(Context cx, Scriptable o, String p, Object v, boolean isThrow) { if (base == null) base = o; if (base instanceof ScriptableObject) { - if (((ScriptableObject) base).putImpl(p, 0, o, v, isThrow)) return; + if (((ScriptableObject) base).putOwnProperty(p, o, v, isThrow)) return; o.put(p, o, v); } else { @@ -239,7 +239,7 @@ static void put(Context cx, Scriptable o, int p, Object v, boolean isThrow) { if (base == null) base = o; if (base instanceof ScriptableObject) { - if (((ScriptableObject) base).putImpl(null, p, o, v, isThrow)) return; + if (((ScriptableObject) base).putOwnProperty(p, o, v, isThrow)) return; o.put(p, o, v); } else { diff --git a/rhino/src/main/java/org/mozilla/javascript/NativeObject.java b/rhino/src/main/java/org/mozilla/javascript/NativeObject.java index 5da98dabb8..30730b540c 100644 --- a/rhino/src/main/java/org/mozilla/javascript/NativeObject.java +++ b/rhino/src/main/java/org/mozilla/javascript/NativeObject.java @@ -737,8 +737,14 @@ && isEnumerable(stringId, sourceObj)) { private boolean isEnumerable(int index, Object obj) { if (obj instanceof ScriptableObject) { ScriptableObject so = (ScriptableObject) obj; - int attrs = so.getAttributes(index); - return (attrs & ScriptableObject.DONTENUM) == 0; + try { + int attrs = so.getAttributes(index); + return (attrs & ScriptableObject.DONTENUM) == 0; + } catch (RhinoException re) { + // Not all ScriptableObject implementations implement + // "getAttributes" for all properties + return true; + } } else { return true; } @@ -747,8 +753,12 @@ private boolean isEnumerable(int index, Object obj) { private boolean isEnumerable(String key, Object obj) { if (obj instanceof ScriptableObject) { ScriptableObject so = (ScriptableObject) obj; - int attrs = so.getAttributes(key); - return (attrs & ScriptableObject.DONTENUM) == 0; + try { + int attrs = so.getAttributes(key); + return (attrs & ScriptableObject.DONTENUM) == 0; + } catch (RhinoException re) { + return true; + } } else { return true; } @@ -757,8 +767,12 @@ private boolean isEnumerable(String key, Object obj) { private boolean isEnumerable(Symbol sym, Object obj) { if (obj instanceof ScriptableObject) { ScriptableObject so = (ScriptableObject) obj; - int attrs = so.getAttributes(sym); - return (attrs & ScriptableObject.DONTENUM) == 0; + try { + int attrs = so.getAttributes(sym); + return (attrs & ScriptableObject.DONTENUM) == 0; + } catch (RhinoException re) { + return true; + } } else { return true; } diff --git a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java index f8e1f80fa2..1dacfbf683 100644 --- a/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java +++ b/rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java @@ -294,12 +294,23 @@ public Object get(Symbol key, Scriptable start) { */ @Override public void put(String name, Scriptable start, Object value) { - if (putImpl(name, 0, start, value)) return; + if (putOwnProperty(name, start, value, Context.isCurrentContextStrict())) return; if (start == this) throw Kit.codeBug(); start.put(name, start, value); } + /** + * Set the value of the named property, and return true if the property is actually defined and + * can be set. Subclasses of ScriptableObject should override this method, and not "put," for + * proper strict mode operation in the future. + * + * @param isThrow if true, throw an exception as if in strict mode + */ + protected boolean putOwnProperty(String name, Scriptable start, Object value, boolean isThrow) { + return putImpl(name, 0, start, value, isThrow); + } + /** * Sets the value of the indexed property, creating it if need be. * @@ -326,21 +337,43 @@ public void put(int index, Scriptable start, Object value) { return; } - if (putImpl(null, index, start, value)) return; + if (putOwnProperty(index, start, value, Context.isCurrentContextStrict())) return; if (start == this) throw Kit.codeBug(); start.put(index, start, value); } + /** + * Set the value of the named property, and return true if the property is actually defined and + * can be set. Subclasses of ScriptableObject should override this method, and not "put," for + * proper strict mode operation in the future + * + * @param isThrow if true, throw an exception as if in strict mode + */ + protected boolean putOwnProperty(int index, Scriptable start, Object value, boolean isThrow) { + return putImpl(null, index, start, value, isThrow); + } + /** Implementation of put required by SymbolScriptable objects. */ @Override public void put(Symbol key, Scriptable start, Object value) { - if (putImpl(key, 0, start, value)) return; + if (putOwnProperty(key, start, value, Context.isCurrentContextStrict())) return; if (start == this) throw Kit.codeBug(); ensureSymbolScriptable(start).put(key, start, value); } + /** + * Set the value of the named property, and return true if the property is actually defined and + * can be set. Subclasses of ScriptableObject should override this method, and not "put," for + * proper strict mode operation in the future + * + * @param isThrow if true, throw an exception as if in strict mode + */ + protected boolean putOwnProperty(Symbol key, Scriptable start, Object value, boolean isThrow) { + return putImpl(key, 0, start, value, isThrow); + } + /** * Removes a named property from the object. * @@ -2662,10 +2695,6 @@ public final synchronized Object associateValue(Object key, Object value) { return Kit.initHash(h, key, value); } - private boolean putImpl(Object key, int index, Scriptable start, Object value) { - return putImpl(key, index, start, value, Context.isCurrentContextStrict()); - } - /** * @param key * @param index @@ -2675,7 +2704,8 @@ private boolean putImpl(Object key, int index, Scriptable start, Object value) { * @return false if this != start and no slot was found. true if this == start or this != start * and a READONLY slot was found. */ - boolean putImpl(Object key, int index, Scriptable start, Object value, boolean isThrow) { + private boolean putImpl( + Object key, int index, Scriptable start, Object value, boolean isThrow) { // This method is very hot (basically called on each assignment) // so we inline the extensible/sealed checks below. Slot slot; diff --git a/rhino/src/test/java/org/mozilla/javascript/tests/AssignSubclassTest.java b/rhino/src/test/java/org/mozilla/javascript/tests/AssignSubclassTest.java new file mode 100644 index 0000000000..c6b160dfc1 --- /dev/null +++ b/rhino/src/test/java/org/mozilla/javascript/tests/AssignSubclassTest.java @@ -0,0 +1,134 @@ +package org.mozilla.javascript.tests; + +import static org.junit.jupiter.api.Assertions.*; + +import org.junit.jupiter.api.Test; +import org.mozilla.javascript.ScriptRuntime; +import org.mozilla.javascript.Scriptable; +import org.mozilla.javascript.ScriptableObject; + +public class AssignSubclassTest { + @Test + public void basicOperations() { + Utils.runWithAllModes( + cx -> { + Scriptable scope = cx.initStandardObjects(); + try { + ScriptableObject.defineClass(scope, MySubclass.class); + } catch (Exception e) { + fail(e); + } + Object result = + cx.evaluateString( + scope, + "let o = new MySubclass();" + + "o.foo = 'bar';\n" + + "o.something = 'else';\n" + + "o[0] = 'baz';\n" + + "o[1] = 'frooby';\n" + + "o;\n", + "test", + 1, + null); + assertInstanceOf(Scriptable.class, result); + Scriptable o = (Scriptable) result; + assertEquals("bar", o.get("foo", o)); + assertEquals("else", o.get("something", o)); + assertEquals("baz", o.get(0, o)); + assertEquals("frooby", o.get(1, o)); + return null; + }); + } + + @Test + public void assign() { + Utils.runWithAllModes( + cx -> { + Scriptable scope = cx.initStandardObjects(); + try { + ScriptableObject.defineClass(scope, MySubclass.class); + } catch (Exception e) { + fail(e); + } + Object result = + cx.evaluateString( + scope, + "\n" + + "let s = {foo: 'bar', something: 'else', 0: 'baz', 1: 'frooby'};\n" + + "let o = new MySubclass();" + + "o.foo = 'x';\n" + + "o.something = 'y';\n" + + "o[0] = 'z';\n" + + "o[1] = false;\n" + + "Object.assign(o, s);\n" + + "o;\n", + "test", + 1, + null); + assertInstanceOf(Scriptable.class, result); + Scriptable o = (Scriptable) result; + assertEquals("bar", o.get("foo", o)); + assertEquals("else", o.get("something", o)); + assertEquals("baz", o.get(0, o)); + assertEquals("frooby", o.get(1, o)); + return null; + }); + } + + public static class MySubclass extends ScriptableObject { + private String foo; + private String bar; + + public MySubclass() {} + + @Override + public String getClassName() { + return "MySubclass"; + } + + @Override + public Object get(String name, Scriptable start) { + if ("foo".equals(name)) { + return foo; + } + return super.get(name, start); + } + + @Override + public boolean has(String name, Scriptable start) { + return "foo".equals(name) || super.has(name, start); + } + + @Override + public boolean putOwnProperty( + String name, Scriptable start, Object value, boolean isThrow) { + if ("foo".equals(name)) { + foo = ScriptRuntime.toString(value); + return true; + } + return super.putOwnProperty(name, start, value, isThrow); + } + + @Override + public Object get(int ix, Scriptable start) { + if (ix == 0) { + return bar; + } + return super.get(ix, start); + } + + @Override + public boolean has(int ix, Scriptable start) { + return ix == 0 || super.has(ix, start); + } + + @Override + public boolean putOwnProperty(int ix, Scriptable start, Object value, boolean isThrow) { + if (ix == 0) { + bar = ScriptRuntime.toString(value); + return true; + } + return super.putOwnProperty(ix, start, value, isThrow); + } + } +} diff --git a/tests/testsrc/test262.properties b/tests/testsrc/test262.properties index 516aa6903a..6f9056d4fb 100644 --- a/tests/testsrc/test262.properties +++ b/tests/testsrc/test262.properties @@ -1132,7 +1132,6 @@ built-ins/Object 178/3408 (5.22%) setPrototypeOf/not-a-constructor.js values/not-a-constructor.js values/observable-operations.js - values/order-after-define-property.js property-order.js proto-from-ctor-realm.js subclass-object-arg.js {unsupported: [class]} @@ -1560,10 +1559,7 @@ built-ins/Proxy 76/311 (24.44%) getOwnPropertyDescriptor/resultdesc-is-not-configurable-not-writable-targetdesc-is-writable.js getOwnPropertyDescriptor/resultdesc-is-not-configurable-targetdesc-is-configurable.js getOwnPropertyDescriptor/resultdesc-is-not-configurable-targetdesc-is-undefined.js - getOwnPropertyDescriptor/trap-is-missing-target-is-proxy.js getOwnPropertyDescriptor/trap-is-null-target-is-proxy.js - getOwnPropertyDescriptor/trap-is-undefined.js - getOwnPropertyDescriptor/trap-is-undefined-target-is-proxy.js get/accessor-get-is-undefined-throws.js get/trap-is-undefined-receiver.js has/call-in-prototype.js @@ -3188,7 +3184,6 @@ language/arguments-object 185/263 (70.34%) mapped/nonwritable-nonenumerable-nonconfigurable-descriptors-set-by-param.js non-strict unmapped/via-params-dstr.js non-strict unmapped/via-params-rest.js non-strict - 10.6-11-b-1.js arguments-caller.js async-gen-meth-args-trailing-comma-multiple.js {unsupported: [async-iteration, async]} async-gen-meth-args-trailing-comma-null.js {unsupported: [async-iteration, async]}