ClassCastException when compiling malformed destructuring expression

[rohanpadhye]

Input to Rhino 1.7.8:

(1 ? ({x: (y)}) : (490)) = 1

Expected: ReferenceError (Invalid left-hand side in assignment)
Instead, I get:

java.lang.ClassCastException: org.mozilla.javascript.Node cannot be cast to org.mozilla.javascript.ast.ObjectLiteral

	at org.mozilla.javascript.Parser.destructuringAssignmentHelper(Parser.java:3932)
	at org.mozilla.javascript.Parser.createDestructuringAssignment(Parser.java:3902)
	at org.mozilla.javascript.IRFactory.createAssignment(IRFactory.java:2235)
	at org.mozilla.javascript.IRFactory.transformAssignment(IRFactory.java:432)
	at org.mozilla.javascript.IRFactory.transform(IRFactory.java:212)
	at org.mozilla.javascript.IRFactory.transformExprStmt(IRFactory.java:516)
	at org.mozilla.javascript.IRFactory.transform(IRFactory.java:209)
	at org.mozilla.javascript.IRFactory.transformScript(IRFactory.java:1042)
	at org.mozilla.javascript.IRFactory.transform(IRFactory.java:192)
	at org.mozilla.javascript.IRFactory.transformTree(IRFactory.java:117)
	at org.mozilla.javascript.Context.compileImpl(Context.java:2540)
	at org.mozilla.javascript.Context.compileString(Context.java:1507)
	at org.mozilla.javascript.Context.compileString(Context.java:1496)

Found using JQF.

[rohanpadhye]

Hi, it's been more than one year since this was first reported. Are there any updates?

[tuchida]

This code will reproduce it.

// Node{ type: Token.OBJECTLIT }
(1 ? {} : 490) = 1

// Node{ type: Token.ARRAYLIT }
(1 ? [] : 490) = 1

This will fix it.

diff --git a/rhino/src/main/java/org/mozilla/javascript/Parser.java b/rhino/src/main/java/org/mozilla/javascript/Parser.java
index 754dc43..a7df6dd 100644
--- a/rhino/src/main/java/org/mozilla/javascript/Parser.java
+++ b/rhino/src/main/java/org/mozilla/javascript/Parser.java
@@ -4032,37 +4032,28 @@ public class Parser {
         result.addChildToBack(comma);
         List<String> destructuringNames = new ArrayList<>();
         boolean empty = true;
-        switch (left.getType()) {
-            case Token.ARRAYLIT:
-                empty =
-                        destructuringArray(
-                                (ArrayLiteral) left,
-                                variableType,
-                                tempName,
-                                comma,
-                                destructuringNames);
-                break;
-            case Token.OBJECTLIT:
-                empty =
-                        destructuringObject(
-                                (ObjectLiteral) left,
-                                variableType,
-                                tempName,
-                                comma,
-                                destructuringNames);
-                break;
-            case Token.GETPROP:
-            case Token.GETELEM:
-                switch (variableType) {
-                    case Token.CONST:
-                    case Token.LET:
-                    case Token.VAR:
-                        reportError("msg.bad.assign.left");
-                }
-                comma.addChildToBack(simpleAssignment(left, createName(tempName)));
-                break;
-            default:
-                reportError("msg.bad.assign.left");
+        if (left instanceof ArrayLiteral) {
+            empty =
+                    destructuringArray(
+                            (ArrayLiteral) left, variableType, tempName, comma, destructuringNames);
+        } else if (left instanceof ObjectLiteral) {
+            empty =
+                    destructuringObject(
+                            (ObjectLiteral) left,
+                            variableType,
+                            tempName,
+                            comma,
+                            destructuringNames);
+        } else if (left.getType() == Token.GETPROP || left.getType() == Token.GETELEM) {
+            switch (variableType) {
+                case Token.CONST:
+                case Token.LET:
+                case Token.VAR:
+                    reportError("msg.bad.assign.left");
+            }
+            comma.addChildToBack(simpleAssignment(left, createName(tempName)));
+        } else {
+            reportError("msg.bad.assign.left");
         }
         if (empty) {
             // Don't want a COMMA node with no children. Just add a zero.

This modification is close to #1187. But is this the right relationship between IRFactory&Node and Parser&AstNode?