From 1911647ab5eedfed16c5f0f49c3a768289d8fc00 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Wed, 2 Sep 2026 14:00:56 +0200 Subject: [PATCH] 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 --- .../AbstractNestablePropertyAccessor.java | 4 +++ .../beans/PropertyAccessorUtils.java | 29 +++++++++++++++++++ .../beans/AbstractPropertyAccessorTests.java | 28 ++++++++++++++++++ .../beans/PropertyAccessorUtilsTests.java | 27 +++++++++++++++++ 4 files changed, 88 insertions(+) diff --git a/spring-beans/src/main/java/org/springframework/beans/AbstractNestablePropertyAccessor.java b/spring-beans/src/main/java/org/springframework/beans/AbstractNestablePropertyAccessor.java index 4f5cce3407c..0f48323807b 100644 --- a/spring-beans/src/main/java/org/springframework/beans/AbstractNestablePropertyAccessor.java +++ b/spring-beans/src/main/java/org/springframework/beans/AbstractNestablePropertyAccessor.java @@ -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) { diff --git a/spring-beans/src/main/java/org/springframework/beans/PropertyAccessorUtils.java b/spring-beans/src/main/java/org/springframework/beans/PropertyAccessorUtils.java index d305ba6bfcf..8dff89e0970 100644 --- a/spring-beans/src/main/java/org/springframework/beans/PropertyAccessorUtils.java +++ b/spring-beans/src/main/java/org/springframework/beans/PropertyAccessorUtils.java @@ -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. + *

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); + } + } diff --git a/spring-beans/src/test/java/org/springframework/beans/AbstractPropertyAccessorTests.java b/spring-beans/src/test/java/org/springframework/beans/AbstractPropertyAccessorTests.java index e6a17ddd2c5..3d04fe0a8b7 100644 --- a/spring-beans/src/test/java/org/springframework/beans/AbstractPropertyAccessorTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/AbstractPropertyAccessorTests.java @@ -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(); diff --git a/spring-beans/src/test/java/org/springframework/beans/PropertyAccessorUtilsTests.java b/spring-beans/src/test/java/org/springframework/beans/PropertyAccessorUtilsTests.java index 2602e95f1e2..e4884ee2121 100644 --- a/spring-beans/src/test/java/org/springframework/beans/PropertyAccessorUtilsTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/PropertyAccessorUtilsTests.java @@ -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();