root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 6 Packages Projects Releases Wiki Activity

Support SpEL compilation of #root or #this with non-public type 

Prior to this commit, if a Spring Expression Language (SpEL) expression
referenced the root context object via the #root or #this variable, we
inserted a checkcast in the generated byte code that cast the object to
its concrete type. However if the root context object's type was
non-public, that resulted in an IllegalAccessError when the compiled
byte code was executed.

VariableReference.getValueInternal() already contains a solution for
global variables which inserts a checkcast to Object in the generated
byte code instead of to the object's concrete non-public type.

This commit therefore applies the same logic to #root (or #this when
used to reference the root context object) that is already applied to
global variables.

Closes gh-32356
This commit is contained in:
Sam Brannen 2024-03-03 14:35:04 +01:00
parent 5cba32df32
commit 380184e85a
3 changed files with 154 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
framework-docs/modules/ROOT/pages/core/expressions/language-ref/variables.adoc
View File

@ -20,6 +20,15 @@ characters.
* dollar sign: `$`
====
[TIP]
====
When setting a variable or root context object in the `EvaluationContext`, it is advised
that the type of the variable or root context object be `public`.
Otherwise, certain types of SpEL expressions involving a variable or root context object
with a non-public type may fail to evaluate or compile.
====
[WARNING]
====
Since variables share a common namespace with

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

@ -70,16 +70,23 @@ public class VariableReference extends SpelNodeImpl {
@Override
public TypedValue getValueInternal(ExpressionState state) throws SpelEvaluationException {
TypedValue result;
if (THIS.equals(this.name)) {
return state.getActiveContextObject();
result = state.getActiveContextObject();
// If the active context object (#this) is not the root context object (#root),
// that means that #this is being evaluated within a nested scope (for example,
// collection selection or collection project), which is not a compilable
// expression, so we return the result without setting the exit type descriptor.
if (result != state.getRootContextObject()) {
return result;
}
}
if (ROOT.equals(this.name)) {
TypedValue result = state.getRootContextObject();
this.exitTypeDescriptor = CodeFlow.toDescriptorFromObject(result.getValue());
return result;
else if (ROOT.equals(this.name)) {
result = state.getRootContextObject();
}
else {
result = state.lookupVariable(this.name);
}
TypedValue result = state.lookupVariable(this.name);
setExitTypeDescriptor(result.getValue());
// A null value in the returned TypedValue will mean either the value was
@ -132,7 +139,7 @@ public class VariableReference extends SpelNodeImpl {
@Override
public void generateCode(MethodVisitor mv, CodeFlow cf) {
if (ROOT.equals(this.name)) {
if (THIS.equals(this.name) || ROOT.equals(this.name)) {
mv.visitVarInsn(ALOAD, 1);
}
else {

200
spring-expression/src/test/java/org/springframework/expression/spel/SpelCompilationCoverageTests.java
View File

@ -19,6 +19,7 @@ package org.springframework.expression.spel;
import java.io.IOException;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@ -52,7 +53,6 @@ import org.springframework.expression.spel.testdata.PersonInOtherPackage;
import static java.util.stream.Collectors.joining;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatException;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.within;
import static org.assertj.core.api.InstanceOfAssertFactories.BOOLEAN;
@ -124,7 +124,7 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
* Further TODOs for compilation:
*
* - OpMinus with a single literal operand could be treated as a negative literal. Will save a
* pointless loading of 0 and then a subtract instruction in code geneneration.
* pointless loading of 0 and then a subtract instruction in code generation.
*
* - allow other accessors/resolvers to participate in compilation and create their own code.
*
@ -143,6 +143,84 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
private SpelNodeImpl ast;
@Nested
class VariableReferenceTests {
@ParameterizedTest // gh-32356
@ValueSource(strings = { "#root", "#this" })
void rootVariableWithPublicType(String spel) {
String string = "hello";
expression = parser.parseExpression(spel);
Object result = expression.getValue(string, String.class);
assertThat(result).isEqualTo(string);
assertCanCompile(expression);
result = expression.getValue(string, String.class);
assertThat(result).isEqualTo(string);
Integer number = 42;
expression = parser.parseExpression(spel);
result = expression.getValue(number, Integer.class);
assertThat(result).isEqualTo(number);
assertCanCompile(expression);
result = expression.getValue(number, Integer.class);
assertThat(result).isEqualTo(number);
}
@ParameterizedTest // gh-32356
@ValueSource(strings = {
"#root.empty ? 0 : #root.size",
"#this.empty ? 0 : #this.size"
})
void rootVariableWithNonPublicType(String spel) {
Map<String, Integer> map = Map.of("a", 13, "b", 42);
// Prerequisite: root type must not be public for this use case.
assertThat(Modifier.isPublic(map.getClass().getModifiers())).isFalse();
expression = parser.parseExpression(spel);
Integer result = expression.getValue(map, Integer.class);
assertThat(result).isEqualTo(2);
assertCanCompile(expression);
result = expression.getValue(map, Integer.class);
assertThat(result).isEqualTo(2);
}
@Test
void userDefinedVariable() {
EvaluationContext ctx = new StandardEvaluationContext();
ctx.setVariable("target", "abc");
expression = parser.parseExpression("#target");
assertThat(expression.getValue(ctx)).isEqualTo("abc");
assertCanCompile(expression);
assertThat(expression.getValue(ctx)).isEqualTo("abc");
ctx.setVariable("target", "123");
assertThat(expression.getValue(ctx)).isEqualTo("123");
// Changing the variable type from String to Integer results in a
// ClassCastException in the compiled code.
ctx.setVariable("target", 42);
assertThatExceptionOfType(SpelEvaluationException.class)
.isThrownBy(() -> expression.getValue(ctx))
.withCauseInstanceOf(ClassCastException.class);
ctx.setVariable("target", "abc");
expression = parser.parseExpression("#target.charAt(0)");
assertThat(expression.getValue(ctx)).isEqualTo('a');
assertCanCompile(expression);
assertThat(expression.getValue(ctx)).isEqualTo('a');
ctx.setVariable("target", "1");
assertThat(expression.getValue(ctx)).isEqualTo('1');
// Changing the variable type from String to Integer results in a
// ClassCastException in the compiled code.
ctx.setVariable("target", 42);
assertThatExceptionOfType(SpelEvaluationException.class)
.isThrownBy(() -> expression.getValue(ctx))
.withCauseInstanceOf(ClassCastException.class);
}
}
@Nested
class IndexingTests {
@ -466,10 +544,13 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Object");
}
@Test
@Test // gh-32356
void indexIntoMapOfPrimitiveIntArray() {
Map<String, int[]> map = Map.of("foo", new int[] { 1, 2, 3 });
// Prerequisite: root type must not be public for this use case.
assertThat(Modifier.isPublic(map.getClass().getModifiers())).isFalse();
// map key access
expression = parser.parseExpression("['foo']");
@ -479,22 +560,37 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertThat(stringify(expression.getValue(map))).isEqualTo("1 2 3");
assertThat(getAst().getExitDescriptor()).isEqualTo("Ljava/lang/Object");
// map key access & array index
// map key access via implicit #root & array index
expression = parser.parseExpression("['foo'][1]");
assertThat(expression.getValue(map)).isEqualTo(2);
assertCanCompile(expression);
assertThat(expression.getValue(map)).isEqualTo(2);
// map key access via explicit #root & array index
expression = parser.parseExpression("#root['foo'][1]");
assertThat(expression.getValue(map)).isEqualTo(2);
assertCanCompile(expression);
assertThat(expression.getValue(map)).isEqualTo(2);
// map key access via explicit #this & array index
expression = parser.parseExpression("#this['foo'][1]");
assertThat(expression.getValue(map)).isEqualTo(2);
assertCanCompile(expression);
assertThat(expression.getValue(map)).isEqualTo(2);
}
@Test
@Test // gh-32356
void indexIntoMapOfPrimitiveIntArrayWithCompilableMapAccessor() {
StandardEvaluationContext context = new StandardEvaluationContext();
context.addPropertyAccessor(new CompilableMapAccessor());
// Map<String, int[]> map = Map.of("foo", new int[] { 1, 2, 3 });
Map<String, int[]> map = new HashMap<>();
map.put("foo", new int[] { 1, 2, 3 });
Map<String, int[]> map = Map.of("foo", new int[] { 1, 2, 3 });
// Prerequisite: root type must not be public for this use case.
assertThat(Modifier.isPublic(map.getClass().getModifiers())).isFalse();
// map key access
expression = parser.parseExpression("['foo']");
@ -516,12 +612,13 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertThat(expression.getValue(context, map)).isEqualTo(2);
assertCanCompile(expression);
// TODO If map is created via Map.of(), the following fails with an IllegalAccessError.
//
// IllegalAccessError: failed to access class java.util.ImmutableCollections$Map1 from class
// spel.Ex2774 (java.util.ImmutableCollections$Map1 is in module java.base of loader 'bootstrap';
// spel.Ex2774 is in unnamed module of loader
// org.springframework.expression.spel.standard.SpelCompiler$ChildClassLoader @359b650b)
assertThat(expression.getValue(context, map)).isEqualTo(2);
// custom CompilableMapAccessor via explicit #this & array index
expression = parser.parseExpression("#this.foo[1]");
assertThat(expression.getValue(context, map)).isEqualTo(2);
assertCanCompile(expression);
assertThat(expression.getValue(context, map)).isEqualTo(2);
// map key access & array index
@ -644,7 +741,9 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertThat(expression.getValue()).isEqualTo(boolean.class);
expression = parse("T(Missing)");
assertGetValueFail(expression);
assertThatExceptionOfType(SpelEvaluationException.class)
.isThrownBy(expression::getValue)
.withMessageEndingWith("Type cannot be found 'Missing'");
assertCantCompile(expression);
}
@ -915,16 +1014,20 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
// Code gen is different for -1 .. 6 because there are bytecode instructions specifically for those values
// Not an int literal but an opminus with one operand:
// expression = parser.parseExpression("-1");
// assertCanCompile(expression);
// assertEquals(-1, expression.getValue());
// Not an int literal but an opMinus with one operand:
expression = parser.parseExpression("-1");
expression.getValue(Integer.class);
assertCanCompile(expression);
assertThat(expression.getValue()).isEqualTo(-1);
expression = parser.parseExpression("0");
assertCanCompile(expression);
assertThat(expression.getValue()).isEqualTo(0);
expression = parser.parseExpression("2");
assertCanCompile(expression);
assertThat(expression.getValue()).isEqualTo(2);
expression = parser.parseExpression("7");
assertCanCompile(expression);
assertThat(expression.getValue()).isEqualTo(7);
@ -1374,23 +1477,6 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertCanCompile(expression);
}
@Test
void variableReference_root() {
String s = "hello";
Expression expression = parser.parseExpression("#root");
String resultI = expression.getValue(s, String.class);
assertCanCompile(expression);
String resultC = expression.getValue(s, String.class);
assertThat(resultI).isEqualTo(s);
assertThat(resultC).isEqualTo(s);
expression = parser.parseExpression("#root");
int i = (Integer) expression.getValue(42);
assertThat(i).isEqualTo(42);
assertCanCompile(expression);
i = (Integer) expression.getValue(42);
assertThat(i).isEqualTo(42);
}
public static String concat(String a, String b) {
return a+b;
@ -1776,34 +1862,6 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
assertThat(expression.getValue(ctx)).isEqualTo("abc");
}
@Test
void variableReference_userDefined() {
EvaluationContext ctx = new StandardEvaluationContext();
ctx.setVariable("target", "abc");
expression = parser.parseExpression("#target");
assertThat(expression.getValue(ctx)).isEqualTo("abc");
assertCanCompile(expression);
assertThat(expression.getValue(ctx)).isEqualTo("abc");
ctx.setVariable("target", "123");
assertThat(expression.getValue(ctx)).isEqualTo("123");
ctx.setVariable("target", 42);
assertThatExceptionOfType(SpelEvaluationException.class)
.isThrownBy(() -> expression.getValue(ctx))
.withCauseInstanceOf(ClassCastException.class);
ctx.setVariable("target", "abc");
expression = parser.parseExpression("#target.charAt(0)");
assertThat(expression.getValue(ctx)).isEqualTo('a');
assertCanCompile(expression);
assertThat(expression.getValue(ctx)).isEqualTo('a');
ctx.setVariable("target", "1");
assertThat(expression.getValue(ctx)).isEqualTo('1');
ctx.setVariable("target", 42);
assertThatExceptionOfType(SpelEvaluationException.class)
.isThrownBy(() -> expression.getValue(ctx))
.withCauseInstanceOf(ClassCastException.class);
}
@Test
void opLt() {
expression = parse("3.0d < 4.0d");
@ -5402,21 +5460,23 @@ public class SpelCompilationCoverageTests extends AbstractExpressionTests {
}
private void assertCanCompile(Expression expression) {
assertThat(SpelCompiler.compile(expression)).isTrue();
assertThat(SpelCompiler.compile(expression))
.as(() -> "Expression <%s> should be compilable"
.formatted(((SpelExpression) expression).toStringAST()))
.isTrue();
}
private void assertCantCompile(Expression expression) {
assertThat(SpelCompiler.compile(expression)).isFalse();
assertThat(SpelCompiler.compile(expression))
.as(() -> "Expression <%s> should not be compilable"
.formatted(((SpelExpression) expression).toStringAST()))
.isFalse();
}
private Expression parse(String expression) {
return parser.parseExpression(expression);
}
private void assertGetValueFail(Expression expression) {
assertThatException().isThrownBy(expression::getValue);
}
// Nested types