mirror of
https://github.com/spring-projects/spring-framework.git
synced 2026-09-17 16:39:29 +00:00
Eagerly reject property paths with unbalanced brackets
As a follow up to 1b56f58999, this commit introduces
hasUnbalancedBrackets() in PropertyAccessorUtils, which
AbstractNestablePropertyAccessor uses to reject property paths with
unbalanced '[' or ']' brackets by throwing a
NotReadablePropertyException with an informative message, thereby
improving diagnostics for users.
See gh-36999
This commit is contained in:
+4
@@ -802,6 +802,10 @@ public abstract class AbstractNestablePropertyAccessor extends AbstractPropertyA
|
||||
* @return a property accessor for the target bean
|
||||
*/
|
||||
protected AbstractNestablePropertyAccessor getPropertyAccessorForPropertyPath(String propertyPath) {
|
||||
if (PropertyAccessorUtils.hasUnbalancedBrackets(propertyPath)) {
|
||||
throw new NotReadablePropertyException(getRootClass(), this.nestedPath + propertyPath,
|
||||
"Property path '" + propertyPath + "' contains unbalanced brackets");
|
||||
}
|
||||
int pos = PropertyAccessorUtils.getFirstNestedPropertySeparatorIndex(propertyPath);
|
||||
// Handle nested properties recursively.
|
||||
if (pos > -1) {
|
||||
|
||||
@@ -225,4 +225,33 @@ public abstract class PropertyAccessorUtils {
|
||||
return -1;
|
||||
}
|
||||
|
||||
/**
|
||||
* Determine whether the given property path contains unbalanced
|
||||
* {@code [} or {@code ]} brackets, by scanning the whole path once
|
||||
* and tracking bracket nesting depth.
|
||||
* <p>A path is considered balanced if the depth never goes negative
|
||||
* (that is, a {@code ]} is never encountered without a corresponding
|
||||
* preceding {@code [}) and returns to {@code 0} by the end of the path
|
||||
* (that is, every {@code [} has a corresponding {@code ]}).
|
||||
* @param propertyPath the property path (or path segment) to check
|
||||
* @return {@code true} if the path contains unbalanced brackets
|
||||
* @since 7.1
|
||||
*/
|
||||
static boolean hasUnbalancedBrackets(String propertyPath) {
|
||||
int depth = 0;
|
||||
int length = propertyPath.length();
|
||||
for (int i = 0; i < length; i++) {
|
||||
switch (propertyPath.charAt(i)) {
|
||||
case PropertyAccessor.PROPERTY_KEY_PREFIX_CHAR -> depth++;
|
||||
case PropertyAccessor.PROPERTY_KEY_SUFFIX_CHAR -> {
|
||||
depth--;
|
||||
if (depth < 0) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return (depth != 0);
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
+28
@@ -36,6 +36,8 @@ import java.util.TreeSet;
|
||||
|
||||
import org.jspecify.annotations.Nullable;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.params.ParameterizedTest;
|
||||
import org.junit.jupiter.params.provider.ValueSource;
|
||||
|
||||
import org.springframework.beans.factory.annotation.Autowire;
|
||||
import org.springframework.beans.propertyeditors.StringArrayPropertyEditor;
|
||||
@@ -67,6 +69,7 @@ import static org.assertj.core.api.Assertions.within;
|
||||
* @author Chris Beams
|
||||
* @author Dave Syer
|
||||
* @author Stephane Nicoll
|
||||
* @author Sam Brannen
|
||||
*/
|
||||
abstract class AbstractPropertyAccessorTests {
|
||||
|
||||
@@ -291,6 +294,18 @@ abstract class AbstractPropertyAccessorTests {
|
||||
accessor.getPropertyValue("address.bar"));
|
||||
}
|
||||
|
||||
@ParameterizedTest // gh-36999
|
||||
@ValueSource(strings = {"address.[.city", "address.].city", "address.[[.city",
|
||||
"address.]].city", "address.][.city", "address.[X.city", "address.X[.city"})
|
||||
void getNestedPropertyWithUnbalancedBracket(String propertyPath) {
|
||||
Person target = createPerson("John", "London", "UK");
|
||||
AbstractPropertyAccessor accessor = createAccessor(target);
|
||||
|
||||
assertThatExceptionOfType(NotReadablePropertyException.class)
|
||||
.isThrownBy(() -> accessor.getPropertyValue(propertyPath))
|
||||
.withMessageEndingWith("contains unbalanced brackets");
|
||||
}
|
||||
|
||||
@Test
|
||||
void setSimpleProperty() {
|
||||
Simple target = new Simple("John", 2);
|
||||
@@ -1362,6 +1377,19 @@ abstract class AbstractPropertyAccessorTests {
|
||||
accessor.setPropertyValue("address.bar", "value"));
|
||||
}
|
||||
|
||||
@ParameterizedTest // gh-36999
|
||||
@ValueSource(strings = {"address.[.city", "address.].city", "address.[[.city",
|
||||
"address.]].city", "address.][.city", "address.[X.city", "address.X[.city"})
|
||||
void setNestedPropertyWithUnbalancedBracket(String propertyPath) {
|
||||
Person target = createPerson("John", "Paris", "FR");
|
||||
AbstractPropertyAccessor accessor = createAccessor(target);
|
||||
|
||||
assertThatExceptionOfType(NotWritablePropertyException.class)
|
||||
.isThrownBy(() -> accessor.setPropertyValue(propertyPath, "Zürich"))
|
||||
.withMessageEndingWith("does not exist");
|
||||
assertThat(target.getAddress().getCity()).isEqualTo("Paris");
|
||||
}
|
||||
|
||||
@Test
|
||||
void setPropertyValuesIgnoresInvalidNestedOnRequest() {
|
||||
ITestBean target = new TestBean();
|
||||
|
||||
@@ -25,6 +25,7 @@ import static org.assertj.core.api.Assertions.assertThat;
|
||||
*
|
||||
* @author Juergen Hoeller
|
||||
* @author Chris Beams
|
||||
* @author Sam Brannen
|
||||
*/
|
||||
class PropertyAccessorUtilsTests {
|
||||
|
||||
@@ -104,6 +105,32 @@ class PropertyAccessorUtilsTests {
|
||||
assertThat(PropertyAccessorUtils.canonicalPropertyName("users['admin[0]']")).isEqualTo("users[admin[0]]");
|
||||
}
|
||||
|
||||
@Test // gh-36999
|
||||
void hasUnbalancedBrackets() {
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("property")).isFalse();
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("array[]")).isFalse();
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("array[0]")).isFalse();
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("map[key1]")).isFalse();
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("map[key1][key2]")).isFalse();
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("map[key1].name")).isFalse();
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("map['key[0]'].name")).isFalse();
|
||||
|
||||
// A lone '[' or ']' anywhere in the path is unbalanced.
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("map.[.name")).isTrue();
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("map.].name")).isTrue();
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("map.[X.name")).isTrue();
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("map.X[.name")).isTrue();
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("[map")).isTrue();
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("map]")).isTrue();
|
||||
|
||||
// Multiple stray brackets are unbalanced.
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("map.[[.name")).isTrue();
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("map.]].name")).isTrue();
|
||||
|
||||
// A ']' before any preceding '[' is unbalanced even if a '[' follows later.
|
||||
assertThat(PropertyAccessorUtils.hasUnbalancedBrackets("map.][.name")).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void canonicalPropertyNames() {
|
||||
assertThat(PropertyAccessorUtils.canonicalPropertyNames(null)).isNull();
|
||||
|
||||
Reference in New Issue
Block a user