avoid infinite loop on ASM optimizer

Caused when accessing the element of an element of a null collection. See the new unit tests testInfiniteLoopWithASMOptimizer() and testInfiniteLoopWithASMOptimizerAO() for details
karibe · Oct 14, 2015 · a7b64ed · a7b64ed
1 parent 430bc8a
commit a7b64ed
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 8 deletions.
diff --git a/src/main/java/org/mvel2/optimizers/impl/asm/ASMAccessorOptimizer.java b/src/main/java/org/mvel2/optimizers/impl/asm/ASMAccessorOptimizer.java
@@ -1357,15 +1357,9 @@ private Object getCollectionProperty(Object ctx, String prop)
     }
 
     currType = null;
-    if (ctx == null) return null;
 
     assert debug("\n  **  ENTER -> {collection:<<" + prop + ">>; ctx=" + ctx + "}");
 
-    if (first) {
-      assert debug("ALOAD 1");
-      mv.visitVarInsn(ALOAD, 1);
-    }
-
     int start = ++cursor;
 
     skipWhitespace();
@@ -1380,6 +1374,12 @@ private Object getCollectionProperty(Object ctx, String prop)
 
     assert debug("{collection token: [" + tk + "]}");
 
+    if (ctx == null) return null;
+    if (first) {
+      assert debug("ALOAD 1");
+      mv.visitVarInsn(ALOAD, 1);
+    }
+
     ExecutableStatement compiled = (ExecutableStatement) subCompileExpression(tk.toCharArray(), pCtx);
     Object item = compiled.getValue(ctx, variableFactory);
 
@@ -1514,7 +1514,6 @@ private Object getCollectionPropertyAO(Object ctx, String prop)
     }
 
     currType = null;
-    if (ctx == null) return null;
 
     assert debug("\n  **  ENTER -> {collection:<<" + prop + ">>; ctx=" + ctx + "}");
 
@@ -1532,6 +1531,8 @@ private Object getCollectionPropertyAO(Object ctx, String prop)
 
     assert debug("{collection token:<<" + tk + ">>}");
 
+    if (ctx == null) return null;
+
     ExecutableStatement compiled = (ExecutableStatement) subCompileExpression(tk.toCharArray());
     Object item = compiled.getValue(ctx, variableFactory);
 

diff --git a/src/test/java/org/mvel2/tests/core/PropertyAccessTests.java b/src/test/java/org/mvel2/tests/core/PropertyAccessTests.java
@@ -1,5 +1,6 @@
 package org.mvel2.tests.core;
 
+import org.mvel2.CompileException;
 import org.mvel2.MVEL;
 import org.mvel2.ParserContext;
 import org.mvel2.integration.PropertyHandler;
@@ -18,6 +19,13 @@
 
 
 public class PropertyAccessTests extends AbstractTest {
+  protected void setUp() throws Exception {
+    super.setUp();
+    // Ensure MVEL settings are not leaked across tests
+    MVEL.COMPILER_OPT_ALLOW_OVERRIDE_ALL_PROPHANDLING = false;
+    OptimizerFactory.setDefaultOptimizer(OptimizerFactory.DYNAMIC);
+  }
+
   public void testSingleProperty() {
     assertEquals(false, test("fun"));
   }
@@ -358,6 +366,7 @@ public Map<String, Object> getMap() {
   }
 
   public void testMVEL226() {
+    MVEL.COMPILER_OPT_ALLOW_OVERRIDE_ALL_PROPHANDLING = true;
     A226 a = new A226();
     Map m = Collections.singletonMap("a", a);
     Map<String, Object> nestMap = Collections.<String, Object>singletonMap("foo", "bar");
@@ -380,6 +389,27 @@ public void testMVEL226() {
     a.map = nestMap;
     assertEquals("bar", MVEL.executeExpression(s, m));
   }
+
+  private void infiniteLoop() {
+    try {
+      Serializable compiled = MVEL.compileExpression("a['b']['c']");
+      Map<String, Object> vars = Collections.singletonMap("a", (Object)Collections.emptyMap());
+      MVEL.executeExpression(compiled, vars);
+      fail("expected exception");
+    } catch(CompileException t) {
+    }
+  }
+
+  public void testInfiniteLoopWithASMOptimizer() {
+    OptimizerFactory.setDefaultOptimizer("ASM");
+    infiniteLoop();
+  }
+
+  public void testInfiniteLoopWithASMOptimizerAO() {
+    MVEL.COMPILER_OPT_ALLOW_OVERRIDE_ALL_PROPHANDLING = true;
+    OptimizerFactory.setDefaultOptimizer("ASM");
+    infiniteLoop();
+  }
 
   public void testInfiniteLoop() {
     A226 a = new A226();
@@ -393,7 +423,12 @@ public void testInfiniteLoop() {
       // ignore
     }
   }
-
+
+  public void testInfiniteLoopAO() {
+    MVEL.COMPILER_OPT_ALLOW_OVERRIDE_ALL_PROPHANDLING = true;
+    testInfiniteLoop();
+  }
+
   public void testNonHashMapImplMapPutMVEL302() {
     test("map=new java.util.Hashtable();map.foo='bar'");
   }