Repository: mvel/mvel
Pull request #372 (2024-08-15 07:59:28)

[mvel-370] NullSafe may cause resolution of the next step to involve variables instead of the next property
Issue: https://github.com/mvel/mvel/issues/370

Alternative solution of https://github.com/mvel/mvel/pull/371



Patch:
diff --git a/src/main/java/org/mvel2/optimizers/AccessorOptimizer.java b/src/main/java/org/mvel2/optimizers/AccessorOptimizer.java
index 869d4faf..0c5a7134 100644
--- a/src/main/java/org/mvel2/optimizers/AccessorOptimizer.java
+++ b/src/main/java/org/mvel2/optimizers/AccessorOptimizer.java
@@ -28,6 +28,15 @@ public interface AccessorOptimizer {
   public Accessor optimizeAccessor(ParserContext pCtx, char[] property, int start, int offset, Object ctx, Object thisRef,
                                    VariableResolverFactory factory, boolean rootThisRef, Class ingressType);
 
+  /*
+    * This method is to optimize the accessor which is potentially in the middle of the bean property chain.
+    * If 'first' is true, it starts with the top level.
+   */
+  default Accessor optimizeAccessor(ParserContext pCtx, char[] property, int start, int offset, Object ctx, Object thisRef,
+                                    VariableResolverFactory factory, boolean rootThisRef, Class ingressType, boolean first) {
+    throw new UnsupportedOperationException();
+  }
+
   public Accessor optimizeSetAccessor(ParserContext pCtx, char[] property, int start, int offset, Object ctx, Object thisRef,
                                       VariableResolverFactory factory, boolean rootThisRef, Object value, Class ingressType);
 
diff --git a/src/main/java/org/mvel2/optimizers/impl/refl/ReflectiveAccessorOptimizer.java b/src/main/java/org/mvel2/optimizers/impl/refl/ReflectiveAccessorOptimizer.java
index 1042958d..5fd0d9e8 100644
--- a/src/main/java/org/mvel2/optimizers/impl/refl/ReflectiveAccessorOptimizer.java
+++ b/src/main/java/org/mvel2/optimizers/impl/refl/ReflectiveAccessorOptimizer.java
@@ -149,6 +149,12 @@ public class ReflectiveAccessorOptimizer extends AbstractOptimizer implements Ac
 
   public Accessor optimizeAccessor(ParserContext pCtx, char[] property, int start, int offset, Object ctx, Object thisRef,
                                    VariableResolverFactory factory, boolean root, Class ingressType) {
+    return optimizeAccessor(pCtx, property, start, offset, ctx, thisRef, factory, root, ingressType, true);
+  }
+
+  @Override
+  public Accessor optimizeAccessor(ParserContext pCtx, char[] property, int start, int offset, Object ctx, Object thisRef,
+                                   VariableResolverFactory factory, boolean root, Class ingressType, boolean first) {
     this.rootNode = this.currNode = null;
     this.expr = property;
     this.start = start;
@@ -156,7 +162,7 @@ public class ReflectiveAccessorOptimizer extends AbstractOptimizer implements Ac
     this.length = end - start;
 
 
-    this.first = true;
+    this.first = first;
     this.ctx = ctx;
     this.thisRef = thisRef;
     this.variableFactory = factory;
@@ -376,16 +382,16 @@ public class ReflectiveAccessorOptimizer extends AbstractOptimizer implements Ac
               break;
           }
 
-          first = false;
           if (curr != null) returnType = curr.getClass();
           if (cursor < end) {
             if (nullSafe) {
               int os = expr[cursor] == '.' ? 1 : 0;
-              addAccessorNode(new NullSafe(expr, cursor + os, end - cursor - os, pCtx));
+              addAccessorNode(new NullSafe(expr, cursor + os, end - cursor - os, pCtx, first));
               if (curr == null) break;
             }
             if (curr == null) throw new NullPointerException();
           }
+          first = false;
           staticAccess = false;
         }
 
@@ -409,16 +415,16 @@ public class ReflectiveAccessorOptimizer extends AbstractOptimizer implements Ac
               break;
           }
 
-          first = false;
           if (curr != null) returnType = curr.getClass();
           if (cursor < end) {
             if (nullSafe) {
               int os = expr[cursor] == '.' ? 1 : 0;
-              addAccessorNode(new NullSafe(expr, cursor + os, end - cursor - os, pCtx));
+              addAccessorNode(new NullSafe(expr, cursor + os, end - cursor - os, pCtx, first));
               if (curr == null) break;
             }
             if (curr == null) throw new NullPointerException();
           }
+          first = false;
           staticAccess = false;
         }
       }
diff --git a/src/main/java/org/mvel2/optimizers/impl/refl/nodes/NullSafe.java b/src/main/java/org/mvel2/optimizers/impl/refl/nodes/NullSafe.java
index dce330c5..7633362b 100644
--- a/src/main/java/org/mvel2/optimizers/impl/refl/nodes/NullSafe.java
+++ b/src/main/java/org/mvel2/optimizers/impl/refl/nodes/NullSafe.java
@@ -12,19 +12,22 @@ public class NullSafe implements AccessorNode {
   private int start;
   private int offset;
   private ParserContext pCtx;
+  private boolean topLevel;
 
-  public NullSafe(char[] expr, int start, int offset, ParserContext pCtx) {
+  public NullSafe(char[] expr, int start, int offset, ParserContext pCtx, boolean topLevel) {
     this.expr = expr;
     this.start = start;
     this.offset = offset;
     this.pCtx = pCtx;
+    this.topLevel = topLevel;
   }
 
   public Object getValue(Object ctx, Object elCtx, VariableResolverFactory variableFactory) {
     if (ctx == null) return null;
     if (nextNode == null) {
+      // If NullSafe is not a top level element, optimizer doesn't use variableFactory to resolve a bean property.
       final Accessor a = OptimizerFactory.getAccessorCompiler(OptimizerFactory.SAFE_REFLECTIVE)
-          .optimizeAccessor(pCtx, expr, start, offset, ctx, elCtx, variableFactory, true, ctx.getClass());
+              .optimizeAccessor(pCtx, expr, start, offset, ctx, elCtx, variableFactory, true, ctx.getClass(), topLevel);
 
       nextNode = new AccessorNode() {
         public AccessorNode getNextNode() {


Test patch:
diff --git a/src/test/java/org/mvel2/compiler/MvelCompileExpNullSafeTest.java b/src/test/java/org/mvel2/compiler/MvelCompileExpNullSafeTest.java
index 79f9dabf..341351d2 100644
--- a/src/test/java/org/mvel2/compiler/MvelCompileExpNullSafeTest.java
+++ b/src/test/java/org/mvel2/compiler/MvelCompileExpNullSafeTest.java
@@ -1,11 +1,13 @@
 package org.mvel2.compiler;
 
+import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import org.junit.Assert;
 import org.junit.Test;
 import org.mvel2.MVEL;
 import org.mvel2.compiler.CompiledExpression;
@@ -120,6 +122,75 @@ public class MvelCompileExpNullSafeTest extends BaseMvelTest
 
     }
 
+	/**
+	 * Tests NullSafe with a segment which shadows a variable.
+	 */
+	@Test
+	public void testNullSafeVariableOverlap() {
+		String expression = "foo.?bar.baz"; // "baz" shadows the variable named "baz".
+
+		final String expectedResult = "You got me!";
+
+		Map<String, Object> foo = new HashMap<>();
+		Map<String, Object> ctx = Collections.singletonMap("foo", foo);
+		Map<String, Object> bar = Collections.singletonMap("baz", expectedResult);
+		foo.put("bar", null); // start with null, as this will cause NullSafe to be added to the AST.
+
+		// This variable overlaps the segment to be resolved next after NullSafe.
+		Map<String, Object> vars = Collections.singletonMap("baz", "no, over here!");
+
+		Serializable compiledExpr = MVEL.compileExpression(expression);
+		Assert.assertNull(MVEL.executeExpression(compiledExpr, ctx, vars));
+
+		foo.put("bar", bar);
+		Assert.assertEquals(expectedResult, MVEL.executeExpression(compiledExpr, ctx, vars));
+	}
+
+	@Test
+	public void testNullSafeWithVariableAsMapKey() {
+		String expression = "foo.?bar[myvar]";
+
+		final String expectedResult = "You got me!";
+
+		Map<String, Object> foo = new HashMap<>();
+		Map<String, Object> ctx = Collections.singletonMap("foo", foo);
+		Map<String, Object> bar = Collections.singletonMap("baz", expectedResult);
+		foo.put("bar", null); // start with null, as this will cause NullSafe to be added to the AST.
+
+		Map<String, Object> vars = new HashMap<>();
+		vars.put("myvar", "baz");
+
+		Serializable compiledExpr = MVEL.compileExpression(expression);
+		Assert.assertNull(MVEL.executeExpression(compiledExpr, ctx, vars));
+
+		foo.put("bar", bar);
+		Assert.assertEquals(expectedResult, MVEL.executeExpression(compiledExpr, ctx, vars));
+	}
+
+	@Test
+	public void testNullSafeWithVariableAsMethodArg() {
+		String expression = "foo.?bar.convert(myvar)";
+
+		Map<String, Object> foo = new HashMap<>();
+		Map<String, Object> ctx = Collections.singletonMap("foo", foo);
+		foo.put("bar", null); // start with null, as this will cause NullSafe to be added to the AST.
+
+		Map<String, Object> vars = new HashMap<>();
+		vars.put("myvar", "hello");
+
+		Serializable compiledExpr = MVEL.compileExpression(expression);
+		Assert.assertNull(MVEL.executeExpression(compiledExpr, ctx, vars));
+
+		foo.put("bar", new StringConverter());
+		Assert.assertEquals("HELLO", MVEL.executeExpression(compiledExpr, ctx, vars));
+	}
+
+	public class StringConverter {
+		public String convert(String value) {
+			return value.toUpperCase();
+		}
+	}
+
 	public class Child {
 		private String firstName;
 		private String lastName;