From 5a3d4e23304f23afce42eaa0d005f9bdfb4af131 Mon Sep 17 00:00:00 2001 From: "andrea.bergia" Date: Tue, 26 Nov 2024 12:02:26 +0100 Subject: [PATCH] Handle `delete super.x` by throwing an error See spec 13.5.1.2 at https://tc39.es/ecma262/#sec-delete-operator-runtime-semantics-evaluation Co-authored-by: Satish Srinivasan --- .../org/mozilla/javascript/CodeGenerator.java | 4 +- .../org/mozilla/javascript/IRFactory.java | 3 ++ .../java/org/mozilla/javascript/Icode.java | 7 ++- .../org/mozilla/javascript/Interpreter.java | 5 ++ .../org/mozilla/javascript/ScriptRuntime.java | 9 ++++ .../javascript/optimizer/BodyCodegen.java | 29 ++++++++---- .../javascript/resources/Messages.properties | 3 ++ .../org/mozilla/javascript/SuperTest.java | 47 ++++++++++++++++++- 8 files changed, 96 insertions(+), 11 deletions(-) diff --git a/rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java b/rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java index cc6e9907ea..0cab5e156c 100644 --- a/rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java +++ b/rhino/src/main/java/org/mozilla/javascript/CodeGenerator.java @@ -737,7 +737,9 @@ private void visitExpression(Node node, int contextFlags) { visitExpression(child, 0); child = child.getNext(); visitExpression(child, 0); - if (isName) { + if (node.getIntProp(Node.SUPER_PROPERTY_ACCESS, 0) == 1) { + addIcode(Icode_DELPROP_SUPER); + } else if (isName) { // special handling for delete name addIcode(Icode_DELNAME); } else { diff --git a/rhino/src/main/java/org/mozilla/javascript/IRFactory.java b/rhino/src/main/java/org/mozilla/javascript/IRFactory.java index 1627a7751c..9a9b77b3ad 100644 --- a/rhino/src/main/java/org/mozilla/javascript/IRFactory.java +++ b/rhino/src/main/java/org/mozilla/javascript/IRFactory.java @@ -1864,6 +1864,9 @@ private static Node createUnary(int nodeType, Node child) { // Always evaluate delete operand, see ES5 11.4.1 & bug #726121 n = new Node(nodeType, new Node(Token.TRUE), child); } + if (child.getIntProp(Node.SUPER_PROPERTY_ACCESS, 0) == 1) { + n.putIntProp(Node.SUPER_PROPERTY_ACCESS, 1); + } return n; } case Token.TYPEOF: diff --git a/rhino/src/main/java/org/mozilla/javascript/Icode.java b/rhino/src/main/java/org/mozilla/javascript/Icode.java index 8e87db775f..3ec1c777ab 100644 --- a/rhino/src/main/java/org/mozilla/javascript/Icode.java +++ b/rhino/src/main/java/org/mozilla/javascript/Icode.java @@ -156,8 +156,11 @@ abstract class Icode { // Call a method on the super object, i.e. super.foo() Icode_CALL_ON_SUPER = Icode_IF_NOT_NULL_UNDEF - 1, + // delete super.prop + Icode_DELPROP_SUPER = Icode_CALL_ON_SUPER - 1, + // Last icode - MIN_ICODE = Icode_CALL_ON_SUPER; + MIN_ICODE = Icode_DELPROP_SUPER; static String bytecodeName(int bytecode) { if (!validBytecode(bytecode)) { @@ -343,6 +346,8 @@ static String bytecodeName(int bytecode) { return "IF_NOT_NULL_UNDEF"; case Icode_CALL_ON_SUPER: return "CALL_ON_SUPER"; + case Icode_DELPROP_SUPER: + return "DELPROP_SUPER"; } // icode without name diff --git a/rhino/src/main/java/org/mozilla/javascript/Interpreter.java b/rhino/src/main/java/org/mozilla/javascript/Interpreter.java index 25ac7db4d8..3ecc91e103 100644 --- a/rhino/src/main/java/org/mozilla/javascript/Interpreter.java +++ b/rhino/src/main/java/org/mozilla/javascript/Interpreter.java @@ -1643,6 +1643,11 @@ private static Object interpretLoop(Context cx, CallFrame frame, Object throwabl stackTop = doDelName(cx, frame, op, stack, sDbl, stackTop); continue Loop; } + case Icode_DELPROP_SUPER: + stackTop -= 1; + stack[stackTop] = Boolean.FALSE; + ScriptRuntime.throwDeleteOnSuperPropertyNotAllowed(); + continue Loop; case Token.GETPROPNOWARN: { Object lhs = stack[stackTop]; diff --git a/rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java b/rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java index 0c15dbb405..072410f136 100644 --- a/rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java +++ b/rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java @@ -5364,6 +5364,10 @@ public static EcmaError syntaxErrorById(String messageId, Object... args) { return syntaxError(msg); } + public static EcmaError referenceError(String message) { + return constructError("ReferenceError", message); + } + private static void warnAboutNonJSObject(Object nonJSObject) { final String omitParam = ScriptRuntime.getMessageById("params.omit.non.js.object.warning"); if (!"true".equals(omitParam)) { @@ -5592,6 +5596,11 @@ public static JavaScriptException throwCustomError( return new JavaScriptException(error, filename, linep[0]); } + /** Throws a ReferenceError "cannot delete a super property". See ECMAScript spec 13.5.1.2 */ + public static void throwDeleteOnSuperPropertyNotAllowed() { + throw referenceError("msg.delete.super"); + } + public static final Object[] emptyArgs = new Object[0]; public static final String[] emptyStrings = new String[0]; } diff --git a/rhino/src/main/java/org/mozilla/javascript/optimizer/BodyCodegen.java b/rhino/src/main/java/org/mozilla/javascript/optimizer/BodyCodegen.java index cb40e79865..d67cc6434f 100644 --- a/rhino/src/main/java/org/mozilla/javascript/optimizer/BodyCodegen.java +++ b/rhino/src/main/java/org/mozilla/javascript/optimizer/BodyCodegen.java @@ -1555,14 +1555,27 @@ private void generateExpression(Node node, Node parent) { generateExpression(child, node); child = child.getNext(); generateExpression(child, node); - cfw.addALoad(contextLocal); - cfw.addPush(isName); - addScriptRuntimeInvoke( - "delete", - "(Ljava/lang/Object;" - + "Ljava/lang/Object;" - + "Lorg/mozilla/javascript/Context;" - + "Z)Ljava/lang/Object;"); + if (node.getIntProp(Node.SUPER_PROPERTY_ACCESS, 0) == 1) { + // We have pushed `super` and the expression, but we need to remove them because + // we actually are just going to throw an error. However, delete is supposed to + // put a boolean on the stack and the class file writer would complain if we + // don't have only popped here. So we pop and the push 0 (false). Anyway, this + // is code that will always fail, so honestly no one will ever write something + // like this (delete super[foo]), so... even if this is not the most efficient + // bytecode, it's fine. + cfw.add(ByteCode.POP2); + cfw.addLoadConstant(0); + addScriptRuntimeInvoke("throwDeleteOnSuperPropertyNotAllowed", "()V"); + } else { + cfw.addALoad(contextLocal); + cfw.addPush(isName); + addScriptRuntimeInvoke( + "delete", + "(Ljava/lang/Object;" + + "Ljava/lang/Object;" + + "Lorg/mozilla/javascript/Context;" + + "Z)Ljava/lang/Object;"); + } break; case Token.BINDNAME: diff --git a/rhino/src/main/resources/org/mozilla/javascript/resources/Messages.properties b/rhino/src/main/resources/org/mozilla/javascript/resources/Messages.properties index b829843ce8..c183115283 100644 --- a/rhino/src/main/resources/org/mozilla/javascript/resources/Messages.properties +++ b/rhino/src/main/resources/org/mozilla/javascript/resources/Messages.properties @@ -459,6 +459,9 @@ msg.super.shorthand.function =\ msg.optional.super =\ super is not allowed in an optional chaining expression +msg.super.delete =\ + cannot delete a super property + msg.no.paren.catch =\ missing ( before catch-block condition diff --git a/rhino/src/test/java/org/mozilla/javascript/SuperTest.java b/rhino/src/test/java/org/mozilla/javascript/SuperTest.java index 411de94d2e..5927d707ea 100644 --- a/rhino/src/test/java/org/mozilla/javascript/SuperTest.java +++ b/rhino/src/test/java/org/mozilla/javascript/SuperTest.java @@ -414,7 +414,7 @@ void prototypeIsNull() { } @Nested - class PropertyWrite { + class PropertyMutate { @Test void byName() { String script = @@ -748,6 +748,51 @@ void modifyOperatorByName() { + "object.x + ':' + proto.x"; Utils.assertWithAllOptimizationLevelsES6("proto1:proto", script); } + + @Test + void deleteNotAllowed() { + String script = + "" + + "var catchHit = false;\n" + + "var getterCalled = false;\n" + + "var proto = { get x() { getterCalled = true; } };" + + "var object = {\n" + + " f() {\n" + + " try {\n" + + " delete super.x;\n" + + " } catch (err) {\n" + + " catchHit = err instanceof ReferenceError;" + + " }\n" + + " }\n" + + "};\n" + + "Object.setPrototypeOf(object, proto);\n" + + "object.f();\n" + + "catchHit + ':' + getterCalled"; + Utils.assertWithAllOptimizationLevelsES6("true:false", script); + } + + @Test + void deleteSuperFirstEvaluatesPropertyKey() { + String script = + "" + + "var catchHit = false;\n" + + "var gCalled = false;\n" + + "var proto = { x: 1 };\n" + + "function g() { gCalled = true; return 'x'; }\n" + + " object = {\n" + + " f() {\n" + + " try {\n" + + " delete super[g()];\n" + + " } catch (err) {\n" + + " catchHit = err instanceof ReferenceError;" + + " }\n" + + " }\n" + + "};\n" + + "Object.setPrototypeOf(object, proto);\n" + + "object.f();\n" + + "catchHit + ':' + gCalled"; + Utils.assertWithAllOptimizationLevelsES6("true:true", script); + } } @Nested