mirror of
https://github.com/spring-projects/spring-framework.git
synced 2026-09-17 16:39:29 +00:00
Do not reuse cached PropertyAccessor in Indexer without checking EvaluationContext
Prior to this commit, the SpEL Indexer's PropertyAccessorValueRef could reuse a cached PropertyAccessor for reads and writes even after that accessor had been removed from the current EvaluationContext, leading to stale property access if the EvaluationContext changes between evaluations of the same expression. This commit aligns PropertyAccessorValueRef with the analogous logic in PropertyOrFieldReference and Indexer's IndexAccessorValueRef by verifying that the cached PropertyAccessor is still registered in the current EvaluationContext before reusing it in getValue() and setValue(). Closes gh-36986
This commit is contained in:
+6
-4
@@ -755,9 +755,10 @@ public class Indexer extends SpelNodeImpl {
|
||||
if (cachedPropertyReadState != null) {
|
||||
String cachedPropertyName = cachedPropertyReadState.name;
|
||||
Class<?> cachedTargetType = cachedPropertyReadState.targetType;
|
||||
PropertyAccessor accessor = cachedPropertyReadState.accessor;
|
||||
// Is it OK to use the cached accessor?
|
||||
if (cachedPropertyName.equals(this.name) && cachedTargetType.equals(targetType)) {
|
||||
PropertyAccessor accessor = cachedPropertyReadState.accessor;
|
||||
if (cachedPropertyName.equals(this.name) && cachedTargetType.equals(targetType) &&
|
||||
this.evaluationContext.getPropertyAccessors().contains(accessor)) {
|
||||
this.expressionState.trackOperation();
|
||||
return accessor.read(this.evaluationContext, this.targetObject, this.name);
|
||||
}
|
||||
@@ -799,9 +800,10 @@ public class Indexer extends SpelNodeImpl {
|
||||
if (cachedPropertyWriteState != null) {
|
||||
String cachedPropertyName = cachedPropertyWriteState.name;
|
||||
Class<?> cachedTargetType = cachedPropertyWriteState.targetType;
|
||||
PropertyAccessor accessor = cachedPropertyWriteState.accessor;
|
||||
// Is it OK to use the cached accessor?
|
||||
if (cachedPropertyName.equals(this.name) && cachedTargetType.equals(targetType)) {
|
||||
PropertyAccessor accessor = cachedPropertyWriteState.accessor;
|
||||
if (cachedPropertyName.equals(this.name) && cachedTargetType.equals(targetType) &&
|
||||
this.evaluationContext.getPropertyAccessors().contains(accessor)) {
|
||||
this.expressionState.trackOperation();
|
||||
accessor.write(this.evaluationContext, this.targetObject, this.name, newValue);
|
||||
return;
|
||||
|
||||
+76
@@ -849,6 +849,82 @@ class IndexingTests {
|
||||
}
|
||||
}
|
||||
|
||||
@Nested
|
||||
class PropertyAccessorValueRefTests { // gh-36986
|
||||
|
||||
private final StandardEvaluationContext context = new StandardEvaluationContext();
|
||||
|
||||
private final SpelExpressionParser parser = new SpelExpressionParser();
|
||||
|
||||
@Test
|
||||
void readIndexDoesNotUseRemovedPropertyAccessor() {
|
||||
Person person = new Person("Jane");
|
||||
this.context.setVariable("person", person);
|
||||
PropertyAccessor accessor = new UppercasingPropertyAccessor();
|
||||
this.context.addPropertyAccessor(accessor);
|
||||
|
||||
Expression expression = this.parser.parseExpression("#person['name']");
|
||||
|
||||
// The first evaluation resolves and caches the custom accessor.
|
||||
assertThat(expression.getValue(this.context)).isEqualTo("JANE");
|
||||
|
||||
// Simulate an application reconfiguring the context at runtime.
|
||||
this.context.removePropertyAccessor(accessor);
|
||||
|
||||
// The removed accessor must not be reused for subsequent evaluations.
|
||||
assertThat(expression.getValue(this.context)).isEqualTo("Jane");
|
||||
}
|
||||
|
||||
@Test
|
||||
void writeIndexDoesNotUseRemovedPropertyAccessor() {
|
||||
Person person = new Person("Jane");
|
||||
this.context.setVariable("person", person);
|
||||
PropertyAccessor accessor = new UppercasingPropertyAccessor();
|
||||
this.context.addPropertyAccessor(accessor);
|
||||
|
||||
Expression expression = this.parser.parseExpression("#person['name']");
|
||||
|
||||
// The first write resolves and caches the custom accessor.
|
||||
expression.setValue(this.context, "Alice");
|
||||
assertThat(person.getName()).isEqualTo("custom:Alice");
|
||||
|
||||
// Simulate an application reconfiguring the context at runtime.
|
||||
this.context.removePropertyAccessor(accessor);
|
||||
|
||||
// The removed accessor must not be reused for subsequent writes.
|
||||
expression.setValue(this.context, "Bob");
|
||||
assertThat(person.getName()).isEqualTo("Bob");
|
||||
}
|
||||
|
||||
private static class UppercasingPropertyAccessor implements PropertyAccessor {
|
||||
|
||||
@Override
|
||||
public Class<?>[] getSpecificTargetClasses() {
|
||||
return new Class<?>[] {Person.class};
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean canRead(EvaluationContext context, @Nullable Object target, String name) {
|
||||
return "name".equals(name);
|
||||
}
|
||||
|
||||
@Override
|
||||
public TypedValue read(EvaluationContext context, @Nullable Object target, String name) {
|
||||
return new TypedValue(((Person) target).getName().toUpperCase());
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean canWrite(EvaluationContext context, @Nullable Object target, String name) {
|
||||
return "name".equals(name);
|
||||
}
|
||||
|
||||
@Override
|
||||
public void write(EvaluationContext context, @Nullable Object target, String name, @Nullable Object newValue) {
|
||||
((Person) target).setName("custom:" + newValue);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@Target({ElementType.FIELD})
|
||||
@Retention(RetentionPolicy.RUNTIME)
|
||||
|
||||
Reference in New Issue
Block a user