Handle delete super.x by throwing an error

andreabergia · 0xe · andreabergia · commit 0874113d3cae · 2024-11-27T10:20:40.000+01:00
See spec 13.5.1.2 at https://tc39.es/ecma262/#sec-delete-operator-runtime-semantics-evaluation Co-authored-by: Satish Srinivasan <satish.srinivasan@servicenow.com>
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -1864,6 +1864,9 @@ private static Node createUnary(int nodeType, Node child) {`
`1864`	`1864`	`// Always evaluate delete operand, see ES5 11.4.1 & bug #726121`
`1865`	`1865`	`n = new Node(nodeType, new Node(Token.TRUE), child);`
`1866`	`1866`	`}`
	`1867`	`+ if (child.getIntProp(Node.SUPER_PROPERTY_ACCESS, 0) == 1) {`
	`1868`	`+ n.putIntProp(Node.SUPER_PROPERTY_ACCESS, 1);`
	`1869`	`+ }`
`1867`	`1870`	`return n;`
`1868`	`1871`	`}`
`1869`	`1872`	`case Token.TYPEOF:`
Original file line number	Diff line number	Diff line change
`@@ -1643,6 +1643,11 @@ private static Object interpretLoop(Context cx, CallFrame frame, Object throwabl`
`1643`	`1643`	`stackTop = doDelName(cx, frame, op, stack, sDbl, stackTop);`
`1644`	`1644`	`continue Loop;`
`1645`	`1645`	`}`
	`1646`	`+ case Icode_DELPROP_SUPER:`
	`1647`	`+ stackTop -= 1;`
	`1648`	`+ stack[stackTop] = Boolean.FALSE;`
	`1649`	`+ ScriptRuntime.throwDeleteOnSuperPropertyNotAllowed();`
	`1650`	`+ continue Loop;`
`1646`	`1651`	`case Token.GETPROPNOWARN:`
`1647`	`1652`	`{`
`1648`	`1653`	`Object lhs = stack[stackTop];`