Merge branch '6.1.x'

spring-expression/src/main/java/org/springframework/expression/spel/ast/VariableReference.java

@@ -29,7 +29,8 @@ import org.springframework.expression.spel.SpelEvaluationException;
 import org.springframework.lang.Nullable;
 
 /**
- * Represents a variable reference — for example, {@code #someVar}.
+ * Represents a variable reference — for example, {@code #root}, {@code #this},
+ * {@code #someVar}, etc.
  *
  * @author Andy Clement
  * @author Sam Brannen
@@ -37,10 +38,11 @@ import org.springframework.lang.Nullable;
  */
 public class VariableReference extends SpelNodeImpl {
 
-	// Well known variables:
-	private static final String THIS = "this";  // currently active context object
+	/** Currently active context object. */
+	private static final String THIS = "this";
 
-	private static final String ROOT = "root";  // root context object
+	/** Root context object. */
+	private static final String ROOT = "root";
 
 
 	private final String name;
@@ -54,41 +56,56 @@ public class VariableReference extends SpelNodeImpl {
 
 	@Override
 	public ValueRef getValueRef(ExpressionState state) throws SpelEvaluationException {
-		if (this.name.equals(THIS)) {
+		if (THIS.equals(this.name)) {
 			return new ValueRef.TypedValueHolderValueRef(state.getActiveContextObject(), this);
 		}
-		if (this.name.equals(ROOT)) {
+		if (ROOT.equals(this.name)) {
 			return new ValueRef.TypedValueHolderValueRef(state.getRootContextObject(), this);
 		}
 		TypedValue result = state.lookupVariable(this.name);
-		// a null value will mean either the value was null or the variable was not found
+		// A null value in the returned VariableRef will mean either the value was
+		// null or the variable was not found.
 		return new VariableRef(this.name, result, state.getEvaluationContext());
 	}
 
 	@Override
 	public TypedValue getValueInternal(ExpressionState state) throws SpelEvaluationException {
-		if (this.name.equals(THIS)) {
+		if (THIS.equals(this.name)) {
 			return state.getActiveContextObject();
 		}
-		if (this.name.equals(ROOT)) {
+		if (ROOT.equals(this.name)) {
 			TypedValue result = state.getRootContextObject();
 			this.exitTypeDescriptor = CodeFlow.toDescriptorFromObject(result.getValue());
 			return result;
 		}
+
 		TypedValue result = state.lookupVariable(this.name);
-		Object value = result.getValue();
+		setExitTypeDescriptor(result.getValue());
+
+		// A null value in the returned TypedValue will mean either the value was
+		// null or the variable was not found.
+		return result;
+	}
+
+	/**
+	 * Set the exit type descriptor for the supplied value.
+	 * <p>If the value is {@code null}, we set the exit type descriptor to
+	 * {@link Object}.
+	 * <p>If the value's type is not public, {@link #generateCode} would insert
+	 * a checkcast to the non-public type in the generated byte code which would
+	 * result in an {@link IllegalAccessError} when the compiled byte code is
+	 * invoked. Thus, as a preventative measure, we set the exit type descriptor
+	 * to {@code Object} in such cases. If resorting to {@code Object} is not
+	 * sufficient, we could consider traversing the hierarchy to find the first
+	 * public type.
+	 */
+	private void setExitTypeDescriptor(@Nullable Object value) {
 		if (value == null || !Modifier.isPublic(value.getClass().getModifiers())) {
-			// If the type is not public then when generateCode produces a checkcast to it
-			// then an IllegalAccessError will occur.
-			// If resorting to Object isn't sufficient, the hierarchy could be traversed for
-			// the first public type.
 			this.exitTypeDescriptor = "Ljava/lang/Object";
 		}
 		else {
 			this.exitTypeDescriptor = CodeFlow.toDescriptorFromObject(value);
 		}
-		// a null value will mean either the value was null or the variable was not found
-		return result;
 	}
 
 	@Override
@@ -105,7 +122,7 @@ public class VariableReference extends SpelNodeImpl {
 
 	@Override
 	public boolean isWritable(ExpressionState expressionState) throws SpelEvaluationException {
-		return !(this.name.equals(THIS) || this.name.equals(ROOT));
+		return !(THIS.equals(this.name) || ROOT.equals(this.name));
 	}
 
 	@Override
@@ -115,13 +132,14 @@ public class VariableReference extends SpelNodeImpl {
 
 	@Override
 	public void generateCode(MethodVisitor mv, CodeFlow cf) {
-		if (this.name.equals(ROOT)) {
-			mv.visitVarInsn(ALOAD,1);
+		if (ROOT.equals(this.name)) {
+			mv.visitVarInsn(ALOAD, 1);
 		}
 		else {
 			mv.visitVarInsn(ALOAD, 2);
 			mv.visitLdcInsn(this.name);
-			mv.visitMethodInsn(INVOKEINTERFACE, "org/springframework/expression/EvaluationContext", "lookupVariable", "(Ljava/lang/String;)Ljava/lang/Object;",true);
+			mv.visitMethodInsn(INVOKEINTERFACE, "org/springframework/expression/EvaluationContext",
+					"lookupVariable", "(Ljava/lang/String;)Ljava/lang/Object;", true);
 		}
 		CodeFlow.insertCheckCast(mv, this.exitTypeDescriptor);
 		cf.pushDescriptor(this.exitTypeDescriptor);

spring-expression/src/test/java/org/springframework/expression/spel/CompilableMapAccessor.java

@@ -0,0 +1,117 @@
+/*
+ * Copyright 2002-2024 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.expression.spel;
+
+import java.util.Map;
+
+import org.springframework.asm.MethodVisitor;
+import org.springframework.expression.AccessException;
+import org.springframework.expression.EvaluationContext;
+import org.springframework.expression.TypedValue;
+import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
+
+/**
+ * This is a local COPY of {@link org.springframework.context.expression.MapAccessor}.
+ *
+ * @author Juergen Hoeller
+ * @author Andy Clement
+ * @since 4.1
+ */
+class CompilableMapAccessor implements CompilablePropertyAccessor {
+
+	@Override
+	public Class<?>[] getSpecificTargetClasses() {
+		return new Class<?>[] {Map.class};
+	}
+
+	@Override
+	public boolean canRead(EvaluationContext context, @Nullable Object target, String name) throws AccessException {
+		return (target instanceof Map<?, ?> map && map.containsKey(name));
+	}
+
+	@Override
+	public TypedValue read(EvaluationContext context, @Nullable Object target, String name) throws AccessException {
+		Assert.state(target instanceof Map, "Target must be of type Map");
+		Map<?, ?> map = (Map<?, ?>) target;
+		Object value = map.get(name);
+		if (value == null && !map.containsKey(name)) {
+			throw new MapAccessException(name);
+		}
+		return new TypedValue(value);
+	}
+
+	@Override
+	public boolean canWrite(EvaluationContext context, @Nullable Object target, String name) throws AccessException {
+		return true;
+	}
+
+	@Override
+	@SuppressWarnings("unchecked")
+	public void write(EvaluationContext context, @Nullable Object target, String name, @Nullable Object newValue)
+			throws AccessException {
+
+		Assert.state(target instanceof Map, "Target must be a Map");
+		Map<Object, Object> map = (Map<Object, Object>) target;
+		map.put(name, newValue);
+	}
+
+	@Override
+	public boolean isCompilable() {
+		return true;
+	}
+
+	@Override
+	public Class<?> getPropertyType() {
+		return Object.class;
+	}
+
+	@Override
+	public void generateCode(String propertyName, MethodVisitor mv, CodeFlow cf) {
+		String descriptor = cf.lastDescriptor();
+		if (descriptor == null || !descriptor.equals("Ljava/util/Map")) {
+			if (descriptor == null) {
+				cf.loadTarget(mv);
+			}
+			CodeFlow.insertCheckCast(mv, "Ljava/util/Map");
+		}
+		mv.visitLdcInsn(propertyName);
+		mv.visitMethodInsn(INVOKEINTERFACE, "java/util/Map", "get","(Ljava/lang/Object;)Ljava/lang/Object;",true);
+	}
+
+
+	/**
+	 * Exception thrown from {@code read} in order to reset a cached
+	 * PropertyAccessor, allowing other accessors to have a try.
+	 */
+	@SuppressWarnings("serial")
+	private static class MapAccessException extends AccessException {
+
+		private final String key;
+
+		public MapAccessException(String key) {
+			super("");
+			this.key = key;
+		}
+
+		@Override
+		public String getMessage() {
+			return "Map does not contain a value for key '" + this.key + "'";
+		}
+	}
+
+}

spring-expression/src/test/java/org/springframework/expression/spel/standard/SpelCompilerTests.java

@@ -36,6 +36,7 @@ import static org.springframework.expression.spel.standard.SpelExpressionTestUti
  * @author Sam Brannen
  * @author Andy Clement
  * @since 5.1.14
+ * @see org.springframework.expression.spel.SpelCompilationCoverageTests
  */
 class SpelCompilerTests {