From ae4214aa9589854c9d9e10e54bcee04c822b94db Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Sun, 26 Jul 2026 10:46:30 +0300 Subject: [PATCH] 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 --- .../expression/spel/ast/Indexer.java | 10 ++- .../expression/spel/IndexingTests.java | 76 +++++++++++++++++++ 2 files changed, 82 insertions(+), 4 deletions(-) diff --git a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java index eb0ca5d20d7..3db7ad6e8d5 100644 --- a/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java +++ b/spring-expression/src/main/java/org/springframework/expression/spel/ast/Indexer.java @@ -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; diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java index 4d2aacf34ba..618e3450506 100644 --- a/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java +++ b/spring-expression/src/test/java/org/springframework/expression/spel/IndexingTests.java @@ -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)