spring/spring-framework
1
0
Fork 0
mirror of https://github.com/spring-projects/spring-framework.git synced 2026-09-17 16:39:29 +00:00
Code Issues Projects Releases Wiki Activity

Use verified property names when constructing Property instances in SpEL

Browse Source 
ReflectivePropertyAccessor's canRead(), read(), and canWrite() methods
previously constructed org.springframework.core.convert.Property
instances without an explicit name, forcing Property#resolveName() to
re-derive the property name from the accessor method via prefix
matching. That heuristic incorrectly resolves record-style and other
prefix-less accessor methods whose names embed or start with "get"/"is"
(for example, budget(), issue(), or island()), and it also normalizes
acronym-style JavaBean properties inconsistently (for example, getURL()
resolves to "uRL" rather than "URL").

By the time these three methods construct a Property, they have already
located the accessor method by searching for exactly the requested
property name, so the resolved name is already known and verified. This
commit passes that name through explicitly via the 4-arg Property
constructor, bypassing Property#resolveName() entirely at these call
sites.

This commit also introduces tests in PropertyAccessTests to cover the
following scenarios:

- A genuine record accessor whose component name embeds or starts with
  a "get"/"is" prefix
- The same scenario on a hand-written, non-record "data class"
- The read() call site exercised directly, since it is otherwise
  unreachable once canRead() has warmed the cache
- A boolean isXxx() getter, as a plain regression check
- An acronym-style property with a decoy field to prove that the
  correct field (and its annotations) is now resolved for both reads
  and writes

See gh-36911
Closes gh-37123
This commit is contained in:
Sam Brannen
2026-08-19 17:45:47 +02:00
parent ba13cae6cc
commit fd95ab16ba
2 changed files with 209 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
spring-expression/src/main/java/org/springframework/expression/spel/support/ReflectivePropertyAccessor.java
+3 -3
View File
@@ -149,7 +149,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
if (method != null) {
// Treat it like a property...
// The readerCache will only contain gettable properties (let's not worry about setters for now).
Property property = new Property(type, method, null);
Property property = new Property(type, method, null, name);
TypeDescriptor typeDescriptor = new TypeDescriptor(property);
Method methodToInvoke = ClassUtils.getPubliclyAccessibleMethodIfPossible(method, type);
this.readerCache.put(cacheKey, new InvokerPair(methodToInvoke, typeDescriptor));
@@ -193,7 +193,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
if (method != null) {
// Treat it like a property...
// The readerCache will only contain gettable properties (let's not worry about setters for now).
Property property = new Property(type, method, null);
Property property = new Property(type, method, null, name);
TypeDescriptor typeDescriptor = new TypeDescriptor(property);
methodToInvoke = ClassUtils.getPubliclyAccessibleMethodIfPossible(method, type);
invoker = new InvokerPair(methodToInvoke, typeDescriptor);
@@ -251,7 +251,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
Method method = findSetterForProperty(name, type, target);
if (method != null) {
// Treat it like a property
Property property = new Property(type, null, method);
Property property = new Property(type, null, method, name);
TypeDescriptor typeDescriptor = new TypeDescriptor(property);
method = ClassUtils.getPubliclyAccessibleMethodIfPossible(method, type);
this.writerCache.put(cacheKey, method);
spring-expression/src/test/java/org/springframework/expression/spel/PropertyAccessTests.java
+206
View File
@@ -16,17 +16,25 @@
package org.springframework.expression.spel;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.lang.reflect.Method;
import java.time.ZoneId;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.assertj.core.api.ThrowableTypeAssert;
import org.jspecify.annotations.Nullable;
import org.junit.jupiter.api.Test;
import org.springframework.core.convert.TypeDescriptor;
import org.springframework.core.convert.converter.ConditionalGenericConverter;
import org.springframework.core.convert.support.GenericConversionService;
import org.springframework.expression.AccessException;
import org.springframework.expression.EvaluationContext;
import org.springframework.expression.EvaluationException;
@@ -35,8 +43,10 @@ import org.springframework.expression.PropertyAccessor;
import org.springframework.expression.TypedValue;
import org.springframework.expression.spel.standard.SpelExpression;
import org.springframework.expression.spel.standard.SpelExpressionParser;
import org.springframework.expression.spel.support.ReflectivePropertyAccessor;
import org.springframework.expression.spel.support.SimpleEvaluationContext;
import org.springframework.expression.spel.support.StandardEvaluationContext;
import org.springframework.expression.spel.support.StandardTypeConverter;
import org.springframework.expression.spel.testresources.Inventor;
import org.springframework.expression.spel.testresources.Person;
import org.springframework.expression.spel.testresources.RecordPerson;
@@ -238,6 +248,109 @@ class PropertyAccessTests extends AbstractExpressionTests {
.extracting(SpelEvaluationException::getMessageCode).isEqualTo(SpelMessage.NOT_ASSIGNABLE);
}
@Test // gh-37123
void propertyReadOnlyWithGenuineRecordAccessorEmbeddingPrefix() {
EvaluationContext context = SimpleEvaluationContext.forReadOnlyDataBinding().build();
Deal deal = new Deal("100", "urgent");
Expression budgetExpr = parser.parseExpression("budget");
Expression issueExpr = parser.parseExpression("issue");
assertThat(budgetExpr.getValue(context, deal)).isEqualTo("100");
assertThat(issueExpr.getValue(context, deal)).isEqualTo("urgent");
// The property name used to look up the record component's backing field (and thus
// its field-only annotations) must exactly match the component name: "budget", not ""
// from Property#resolveName() stripping a "get" that merely appears embedded in the
// name; "issue", not "sue" from stripping an "is" that merely happens to start it.
assertThat(budgetExpr.getValueTypeDescriptor(context, deal).getAnnotation(Marker.class).value())
.isEqualTo("budget");
assertThat(issueExpr.getValueTypeDescriptor(context, deal).getAnnotation(Marker.class).value())
.isEqualTo("issue");
}
@Test // gh-37123
void propertyReadOnlyWithNonRecordDataClassAccessorEmbeddingPrefix() {
EvaluationContext context = SimpleEvaluationContext.forReadOnlyDataBinding().build();
IslandDataClass target = new IslandDataClass("Bali", "gizmo");
Expression islandExpr = parser.parseExpression("island");
Expression widgetExpr = parser.parseExpression("widget");
assertThat(islandExpr.getValue(context, target)).isEqualTo("Bali");
assertThat(widgetExpr.getValue(context, target)).isEqualTo("gizmo");
// Same guarantee as for a genuine java.lang.Record (see above): "island", not "land"
// from stripping an "is" that merely happens to start it; "widget", not "" from
// stripping a "get" that merely appears embedded in the name. But here on a plain,
// hand-written "data class" that is not a record and thus cannot be special-cased
// by type in Property#resolveName().
assertThat(islandExpr.getValueTypeDescriptor(context, target).getAnnotation(Marker.class).value())
.isEqualTo("island");
assertThat(widgetExpr.getValueTypeDescriptor(context, target).getAnnotation(Marker.class).value())
.isEqualTo("widget");
}
@Test // gh-37123
void readCallSiteWithoutPriorCanReadResolvesCorrectAnnotation() throws AccessException {
// ReflectivePropertyAccessor#read() has its own Property construction call site,
// distinct from the one in canRead(). In the standard SpEL evaluation flow, that
// site is bypassed once canRead() has warmed the cache (and once an optimal
// accessor takes over), so we exercise it directly here with a fresh accessor/cache.
ReflectivePropertyAccessor accessor = new ReflectivePropertyAccessor();
EvaluationContext context = new StandardEvaluationContext();
IslandDataClass target = new IslandDataClass("Bali", "gizmo");
TypedValue result = accessor.read(context, target, "island");
assertThat(result.getValue()).isEqualTo("Bali");
assertThat(result.getTypeDescriptor().getAnnotation(Marker.class).value()).isEqualTo("island");
}
@Test // gh-37123
void propertyReadWriteWithBooleanJavaBeanGetter() {
EvaluationContext context = SimpleEvaluationContext.forReadWriteDataBinding().build();
Flag target = new Flag();
Expression activeExpr = parser.parseExpression("active");
assertThat(activeExpr.getValue(context, target)).isEqualTo(false);
activeExpr.setValue(context, target, true);
assertThat(target.isActive()).isTrue();
assertThat(activeExpr.getValue(context, target)).isEqualTo(true);
}
@Test // gh-37123
void propertyReadResolvesCorrectFieldForAcronymPropertyDespiteSimilarlyNamedField() {
EvaluationContext context = SimpleEvaluationContext.forReadOnlyDataBinding().build();
UrlHolder target = new UrlHolder();
target.setURL("https://spring.io");
Expression urlExpr = parser.parseExpression("URL");
assertThat(urlExpr.getValue(context, target)).isEqualTo("https://spring.io");
// Property#resolveName() would derive "uRL" (plain uncapitalize of "URL") for this
// getter, which happens to be the exact name of a decoy field on UrlHolder; passing
// the already-verified SpEL identifier "URL" through directly avoids that mismatch.
assertThat(urlExpr.getValueTypeDescriptor(context, target).getAnnotation(Marker.class).value())
.isEqualTo("right");
}
@Test // gh-37123
void propertyWriteAppliesAnnotationDrivenConversionOnlyForCorrectlyResolvedAcronymField() {
GenericConversionService conversionService = new GenericConversionService();
conversionService.addConverter(new MarkerAwareStringConverter());
StandardEvaluationContext context = new StandardEvaluationContext();
context.setTypeConverter(new StandardTypeConverter(conversionService));
UrlHolder target = new UrlHolder();
parser.parseExpression("URL").setValue(context, target, "https://spring.io");
// Only observable if canWrite()'s Property resolves the "URL" field (not the "uRL"
// decoy field) and thus picks up the conversion-triggering @Marker("right") annotation.
assertThat(target.getURL()).isEqualTo("[https://spring.io]");
}
@Test
void propertyReadWrite() {
EvaluationContext context = SimpleEvaluationContext.forReadWriteDataBinding().build();
@@ -401,4 +514,97 @@ class PropertyAccessTests extends AbstractExpressionTests {
}
}
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.FIELD)
private @interface Marker {
String value();
}
// Intentionally contains a "budget" property with a "get" prefix in its name
// and an "issue" property with an "is" prefix in its name.
private record Deal(@Marker("budget") String budget, @Marker("issue") String issue) {
}
// Intentionally contains a "widget" property with a "get" prefix in its name
// and an "island" property with an "is" prefix in its name.
static class IslandDataClass {
@Marker("island")
private final String island;
@Marker("widget")
private final String widget;
IslandDataClass(String island, String widget) {
this.island = island;
this.widget = widget;
}
// Intentionally begins with an "is" prefix, so to speak.
public String island() {
return this.island;
}
// Intentionally embeds a "get" prefix, so to speak.
public String widget() {
return this.widget;
}
}
static class Flag {
private boolean active;
public boolean isActive() {
return this.active;
}
public void setActive(boolean active) {
this.active = active;
}
}
private static class UrlHolder {
@Marker("wrong")
private String uRL = "decoy";
@Marker("right")
private String URL;
public String getURL() {
return this.URL;
}
public void setURL(String URL) {
this.URL = URL;
}
}
private static class MarkerAwareStringConverter implements ConditionalGenericConverter {
@Override
public Set<ConvertiblePair> getConvertibleTypes() {
return Set.of(new ConvertiblePair(String.class, String.class));
}
@Override
public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
Marker marker = targetType.getAnnotation(Marker.class);
return (marker != null && marker.value().equals("right"));
}
@Override
public @Nullable Object convert(@Nullable Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
return "[" + source + "]";
}
}
}