mirror of
https://github.com/spring-projects/spring-framework.git
synced 2026-10-08 16:19:48 +00:00
Use PropertyPath instead of utility methods
Prior to this commit, `DataBinder` and the property accessor hierarchy relied on `PropertyAccessorUtils` and several independent scanners for property paths. This commit migrates all of them to `PropertyPath`, so there is exactly one parser deciding what a well-formed property path is, used identically for policy checks and for actual navigation. This removes long standing protected methods like A`getPropertyAccessorForPropertyPathi` and `getFinalPath` from `bstractNestablePropertyAccessor`. The path is now parsed exactly once per public entry point, via the new `resolvePropertyPath`, which returns a `ResolvedProperty`. Then, property navigation walks the parsed segment list rather than re-scanning partial strings. Any subclass overriding the removed method will need to adapt. This removal initially conflicted with gh-37252 (maxNestedPathDepth support). The public configuration remains but the actual behavior changed; it is replaced with `PropertyPath.Options` which enforces limit right after parsing, before property navigation begins. The exception thrown changes from `InvalidPropertyException` to `InvalidPropertyPathException`. This commmit also reverts the `map[']` / `map["]` quoting behavior from gh-36765 as it is incompatible with the new grammar. See gh-37275
This commit is contained in:
+3
-3
@@ -22,8 +22,8 @@ import org.jspecify.annotations.Nullable;
|
||||
|
||||
import org.springframework.beans.BeanUtils;
|
||||
import org.springframework.beans.ConfigurablePropertyAccessor;
|
||||
import org.springframework.beans.PropertyAccessorUtils;
|
||||
import org.springframework.beans.PropertyEditorRegistry;
|
||||
import org.springframework.beans.PropertyPath;
|
||||
import org.springframework.core.convert.ConversionService;
|
||||
import org.springframework.core.convert.TypeDescriptor;
|
||||
import org.springframework.core.convert.support.ConvertingPropertyEditorAdapter;
|
||||
@@ -76,11 +76,11 @@ public abstract class AbstractPropertyBindingResult extends AbstractBindingResul
|
||||
|
||||
/**
|
||||
* Returns the canonical property name.
|
||||
* @see org.springframework.beans.PropertyAccessorUtils#canonicalPropertyName
|
||||
* @see PropertyPath#canonicalNameOrOriginal(String)
|
||||
*/
|
||||
@Override
|
||||
protected String canonicalFieldName(String field) {
|
||||
return PropertyAccessorUtils.canonicalPropertyName(field);
|
||||
return PropertyPath.canonicalNameOrOriginal(field);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -42,12 +42,13 @@ import org.springframework.beans.BeanInstantiationException;
|
||||
import org.springframework.beans.BeanUtils;
|
||||
import org.springframework.beans.ConfigurablePropertyAccessor;
|
||||
import org.springframework.beans.InvalidPropertyException;
|
||||
import org.springframework.beans.InvalidPropertyPathException;
|
||||
import org.springframework.beans.MutablePropertyValues;
|
||||
import org.springframework.beans.PropertyAccessException;
|
||||
import org.springframework.beans.PropertyAccessorUtils;
|
||||
import org.springframework.beans.PropertyBatchUpdateException;
|
||||
import org.springframework.beans.PropertyEditorRegistrar;
|
||||
import org.springframework.beans.PropertyEditorRegistry;
|
||||
import org.springframework.beans.PropertyPath;
|
||||
import org.springframework.beans.PropertyValue;
|
||||
import org.springframework.beans.PropertyValues;
|
||||
import org.springframework.beans.SimpleTypeConverter;
|
||||
@@ -540,9 +541,8 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
|
||||
* {@code "xxx*yyy"} matches (with an arbitrary number of pattern parts), as
|
||||
* well as direct equality.
|
||||
* <p>The default implementation of this method stores allowed field patterns
|
||||
* in {@linkplain PropertyAccessorUtils#canonicalPropertyName(String) canonical}
|
||||
* form. Subclasses which override this method must therefore take this into
|
||||
* account.
|
||||
* in {@linkplain PropertyPath#canonicalName() canonical} form. Subclasses
|
||||
* which override this method must therefore take this into account.
|
||||
* <p>More sophisticated matching can be implemented by overriding the
|
||||
* {@link #isAllowed} method.
|
||||
* <p>Used for binding to fields with {@link #bind(PropertyValues)}, and not
|
||||
@@ -552,7 +552,7 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
|
||||
* @see #isAllowed(String)
|
||||
*/
|
||||
public void setAllowedFields(String @Nullable ... allowedFields) {
|
||||
this.allowedFields = PropertyAccessorUtils.canonicalPropertyNames(allowedFields);
|
||||
this.allowedFields = canonicalPropertyNames(allowedFields);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -573,9 +573,9 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
|
||||
* {@code "xxx*yyy"} matches (with an arbitrary number of pattern parts),
|
||||
* as well as direct equality.
|
||||
* <p>The default implementation of this method stores disallowed field
|
||||
* patterns in {@linkplain PropertyAccessorUtils#canonicalPropertyName(String)
|
||||
* canonical} form, and subsequently pattern matching in {@link #isAllowed}
|
||||
* is case-insensitive. Subclasses that override this method must therefore
|
||||
* patterns in {@linkplain PropertyPath#canonicalName() canonical} form,
|
||||
* and subsequently pattern matching in {@link #isAllowed} is
|
||||
* case-insensitive. Subclasses that override this method must therefore
|
||||
* take this transformation into account.
|
||||
* <p>More sophisticated matching can be implemented by overriding the
|
||||
* {@link #isAllowed} method.
|
||||
@@ -592,16 +592,7 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
|
||||
*/
|
||||
@Deprecated(since = "7.1", forRemoval = true)
|
||||
public void setDisallowedFields(String @Nullable ... disallowedFields) {
|
||||
if (disallowedFields == null) {
|
||||
this.disallowedFields = null;
|
||||
}
|
||||
else {
|
||||
String[] fieldPatterns = new String[disallowedFields.length];
|
||||
for (int i = 0; i < fieldPatterns.length; i++) {
|
||||
fieldPatterns[i] = PropertyAccessorUtils.canonicalPropertyName(disallowedFields[i]);
|
||||
}
|
||||
this.disallowedFields = fieldPatterns;
|
||||
}
|
||||
this.disallowedFields = canonicalPropertyNames(disallowedFields);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -628,7 +619,7 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
|
||||
* @see DefaultBindingErrorProcessor#MISSING_FIELD_ERROR_CODE
|
||||
*/
|
||||
public void setRequiredFields(String @Nullable ... requiredFields) {
|
||||
this.requiredFields = PropertyAccessorUtils.canonicalPropertyNames(requiredFields);
|
||||
this.requiredFields = canonicalPropertyNames(requiredFields);
|
||||
if (logger.isDebugEnabled()) {
|
||||
logger.debug("DataBinder requires binding of required fields [" +
|
||||
StringUtils.arrayToCommaDelimitedString(requiredFields) + "]");
|
||||
@@ -1294,6 +1285,17 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
|
||||
applyPropertyValues(mpvs);
|
||||
}
|
||||
|
||||
private static String @Nullable [] canonicalPropertyNames(String @Nullable [] fields) {
|
||||
if (fields == null) {
|
||||
return null;
|
||||
}
|
||||
String[] result = new String[fields.length];
|
||||
for (int i = 0; i < fields.length; i++) {
|
||||
result[i] = PropertyPath.canonicalNameOrOriginal(fields[i]);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check the given property values against the allowed fields,
|
||||
* removing values for fields that are not allowed.
|
||||
@@ -1303,11 +1305,23 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
|
||||
*/
|
||||
protected void checkAllowedFields(MutablePropertyValues mpvs) {
|
||||
PropertyValue[] pvs = mpvs.getPropertyValues();
|
||||
PropertyPath.Options options = PropertyPath.Options.withMaxNestedPathDepth(getMaxNestedPathDepth());
|
||||
for (PropertyValue pv : pvs) {
|
||||
String field = PropertyAccessorUtils.canonicalPropertyName(pv.getName());
|
||||
if (!isAllowed(field)) {
|
||||
PropertyPath field;
|
||||
try {
|
||||
field = PropertyPath.parse(pv.getName(), options);
|
||||
}
|
||||
catch (InvalidPropertyPathException ex) {
|
||||
mpvs.removePropertyValue(pv);
|
||||
getBindingResult().recordSuppressedField(field);
|
||||
Object target = getTarget();
|
||||
getBindingErrorProcessor().processPropertyAccessException(
|
||||
new InvalidPropertyPathException(target != null ? target : this, pv.getName(), pv.getValue(), ex),
|
||||
getInternalBindingResult());
|
||||
continue;
|
||||
}
|
||||
if (!isAllowed(field.canonicalName())) {
|
||||
mpvs.removePropertyValue(pv);
|
||||
getBindingResult().recordSuppressedField(field.canonicalName());
|
||||
if (logger.isDebugEnabled()) {
|
||||
logger.debug("Field [" + field + "] has been removed from PropertyValues " +
|
||||
"and will not be bound, because it has not been found in the list of allowed fields");
|
||||
@@ -1327,12 +1341,14 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
|
||||
* matching against disallowed field patterns is case-insensitive.
|
||||
* <p>A field matching a disallowed pattern will not be accepted even if it
|
||||
* also happens to match a pattern in the allowed list.
|
||||
* <p>{@code field} is matched as-is, but it must already have been validated
|
||||
* and canonicalized via {@link PropertyPath} by the caller first.
|
||||
* <p>Can be overridden in subclasses, but care must be taken to honor the
|
||||
* aforementioned contract.
|
||||
* @param field the field to check
|
||||
* @param field the field to check, expected to already be in canonical form
|
||||
* @return {@code true} if the field is allowed
|
||||
* @see #setAllowedFields
|
||||
* @see org.springframework.util.PatternMatchUtils#simpleMatch(String, String)
|
||||
* @see org.springframework.util.PatternMatchUtils#simpleMatch(String[], String)
|
||||
*/
|
||||
protected boolean isAllowed(String field) {
|
||||
String[] allowed = getAllowedFields();
|
||||
@@ -1361,7 +1377,7 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter {
|
||||
Map<String, PropertyValue> propertyValues = new HashMap<>();
|
||||
PropertyValue[] pvs = mpvs.getPropertyValues();
|
||||
for (PropertyValue pv : pvs) {
|
||||
String canonicalName = PropertyAccessorUtils.canonicalPropertyName(pv.getName());
|
||||
String canonicalName = PropertyPath.canonicalNameOrOriginal(pv.getName());
|
||||
propertyValues.put(canonicalName, pv);
|
||||
}
|
||||
for (String field : requiredFields) {
|
||||
|
||||
+7
-4
@@ -23,6 +23,7 @@ import java.util.Map;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import org.springframework.beans.InvalidPropertyException;
|
||||
import org.springframework.beans.InvalidPropertyPathException;
|
||||
import org.springframework.beans.MutablePropertyValues;
|
||||
import org.springframework.beans.NotWritablePropertyException;
|
||||
import org.springframework.beans.NullValueInNestedPathException;
|
||||
@@ -162,7 +163,7 @@ class DataBinderFieldAccessTests {
|
||||
rod.setSpouse(kerry);
|
||||
kerry.setSpouse(rod);
|
||||
|
||||
DataBinder binder = new DataBinder(rod);
|
||||
DataBinder binder = new DataBinder(rod, "rod");
|
||||
binder.setMaxNestedPathDepth(2);
|
||||
binder.initDirectFieldAccess();
|
||||
|
||||
@@ -173,9 +174,11 @@ class DataBinderFieldAccessTests {
|
||||
|
||||
MutablePropertyValues tooDeep = new MutablePropertyValues();
|
||||
tooDeep.add("spouse.spouse.spouse.name", "Joe");
|
||||
assertThatExceptionOfType(InvalidPropertyException.class)
|
||||
.isThrownBy(() -> binder.bind(tooDeep))
|
||||
.withMessageEndingWith("Nesting depth of property path exceeds the maximum of 2");
|
||||
binder.bind(tooDeep);
|
||||
assertThat(binder.getBindingResult().getFieldErrors("spouse.spouse.spouse.name")).singleElement().satisfies(error -> {
|
||||
assertThat(error.getCode()).isEqualTo(InvalidPropertyPathException.ERROR_CODE);
|
||||
assertThat(error.getRejectedValue()).isEqualTo("Joe");
|
||||
});
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -42,6 +42,7 @@ import org.junit.jupiter.api.Test;
|
||||
import org.springframework.beans.BeanWrapper;
|
||||
import org.springframework.beans.ConfigurablePropertyAccessor;
|
||||
import org.springframework.beans.InvalidPropertyException;
|
||||
import org.springframework.beans.InvalidPropertyPathException;
|
||||
import org.springframework.beans.MethodInvocationException;
|
||||
import org.springframework.beans.MutablePropertyValues;
|
||||
import org.springframework.beans.NotWritablePropertyException;
|
||||
@@ -735,6 +736,56 @@ class DataBinderTests {
|
||||
assertThat(binder.getBindingResult().getSuppressedFields()).containsExactlyInAnyOrder("age", "favoriteColor");
|
||||
}
|
||||
|
||||
@Test // gh-37275
|
||||
void bindingWithMalformedFieldNameSurfacesAsFieldErrorRatherThanBeingSilentlyDropped() throws BindException {
|
||||
TestBean rod = new TestBean();
|
||||
DataBinder binder = new DataBinder(rod, "rod");
|
||||
MutablePropertyValues pvs = new MutablePropertyValues();
|
||||
pvs.add("name", "Rod");
|
||||
pvs.add("map[key1]other", "value"); // trailing garbage after a key: not a well-formed property path
|
||||
|
||||
binder.bind(pvs);
|
||||
|
||||
assertThat(rod.getName()).as("well-formed fields still bind").isEqualTo("Rod");
|
||||
assertThat(binder.getBindingResult().getFieldErrors("map[key1]other")).singleElement().satisfies(error -> {
|
||||
assertThat(error.getCode()).isEqualTo(InvalidPropertyPathException.ERROR_CODE);
|
||||
assertThat(error.getRejectedValue()).isEqualTo("value");
|
||||
});
|
||||
assertThatExceptionOfType(BindException.class).isThrownBy(binder::close);
|
||||
}
|
||||
|
||||
@Test // gh-37275
|
||||
void bindingWithMalformedFieldNameIsNotSuppressedEvenWhenAllowedFieldsAreConfigured() throws BindException {
|
||||
TestBean rod = new TestBean();
|
||||
DataBinder binder = new DataBinder(rod, "rod");
|
||||
binder.setAllowedFields("name");
|
||||
MutablePropertyValues pvs = new MutablePropertyValues();
|
||||
pvs.add("name", "Rod");
|
||||
pvs.add("map[key1]other", "value");
|
||||
|
||||
binder.bind(pvs);
|
||||
|
||||
assertThat(binder.getBindingResult().getSuppressedFields()).isEmpty();
|
||||
assertThat(binder.getBindingResult().getFieldErrors("map[key1]other")).hasSize(1);
|
||||
}
|
||||
|
||||
@Test // gh-37275
|
||||
void isAllowedNeverThrowsForMalformedField() {
|
||||
class ExposingBinder extends DataBinder {
|
||||
ExposingBinder(Object target) {
|
||||
super(target, "target");
|
||||
}
|
||||
boolean callIsAllowed(String field) {
|
||||
return isAllowed(field);
|
||||
}
|
||||
}
|
||||
|
||||
ExposingBinder binder = new ExposingBinder(new TestBean());
|
||||
binder.setAllowedFields("name");
|
||||
|
||||
assertThat(binder.callIsAllowed("map[key1]other")).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
@SuppressWarnings("removal")
|
||||
void bindingWithAllowedAndDisallowedFields() throws BindException {
|
||||
@@ -2080,7 +2131,7 @@ class DataBinderTests {
|
||||
rod.setSpouse(kerry);
|
||||
kerry.setSpouse(rod);
|
||||
|
||||
DataBinder binder = new DataBinder(rod);
|
||||
DataBinder binder = new DataBinder(rod, "rod");
|
||||
binder.setMaxNestedPathDepth(2);
|
||||
|
||||
MutablePropertyValues pvs = new MutablePropertyValues();
|
||||
@@ -2090,9 +2141,11 @@ class DataBinderTests {
|
||||
|
||||
MutablePropertyValues tooDeep = new MutablePropertyValues();
|
||||
tooDeep.add("spouse.spouse.spouse.name", "Joe");
|
||||
assertThatExceptionOfType(InvalidPropertyException.class)
|
||||
.isThrownBy(() -> binder.bind(tooDeep))
|
||||
.withMessageEndingWith("Nesting depth of property path exceeds the maximum of 2");
|
||||
binder.bind(tooDeep);
|
||||
assertThat(binder.getBindingResult().getFieldErrors("spouse.spouse.spouse.name")).singleElement().satisfies(error -> {
|
||||
assertThat(error.getCode()).isEqualTo(InvalidPropertyPathException.ERROR_CODE);
|
||||
assertThat(error.getRejectedValue()).isEqualTo("Joe");
|
||||
});
|
||||
}
|
||||
|
||||
@Test // gh-37252
|
||||
|
||||
Reference in New Issue
Block a user