From 4591a67641c87f8cccbce19bafac6cccf8727ce3 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Mon, 10 Feb 2025 17:41:05 +0000 Subject: [PATCH 1/3] Handle [] leniently in constructor binding See gh-34305 --- .../validation/DataBinder.java | 39 +++++++++++++------ .../validation/DataBinderConstructTests.java | 11 ++++++ 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/validation/DataBinder.java b/spring-context/src/main/java/org/springframework/validation/DataBinder.java index efce2b9d5f..2d00655f4c 100644 --- a/spring-context/src/main/java/org/springframework/validation/DataBinder.java +++ b/spring-context/src/main/java/org/springframework/validation/DataBinder.java @@ -142,6 +142,10 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter { */ protected static final Log logger = LogFactory.getLog(DataBinder.class); + /** Internal constant for constructor binding via "[]". */ + private static final int NO_INDEX = -1; + + @Nullable private Object target; @@ -1056,15 +1060,17 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter { return null; } - int size = (indexes.last() < this.autoGrowCollectionLimit ? indexes.last() + 1 : 0); + int lastIndex = Math.max(indexes.last(), 0); + int size = (lastIndex < this.autoGrowCollectionLimit ? lastIndex + 1 : 0); List list = (List) CollectionFactory.createCollection(paramType, size); for (int i = 0; i < size; i++) { list.add(null); } for (int index : indexes) { - String indexedPath = paramPath + "[" + index + "]"; - list.set(index, createIndexedValue(paramPath, paramType, elementType, indexedPath, valueResolver)); + String indexedPath = paramPath + "[" + (index != NO_INDEX ? index : "") + "]"; + list.set(Math.max(index, 0), + createIndexedValue(paramPath, paramType, elementType, indexedPath, valueResolver)); } return list; @@ -1108,12 +1114,14 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter { return null; } - int size = (indexes.last() < this.autoGrowCollectionLimit ? indexes.last() + 1: 0); + int lastIndex = Math.max(indexes.last(), 0); + int size = (lastIndex < this.autoGrowCollectionLimit ? lastIndex + 1: 0); V[] array = (V[]) Array.newInstance(elementType.resolve(), size); for (int index : indexes) { - String indexedPath = paramPath + "[" + index + "]"; - array[index] = createIndexedValue(paramPath, paramType, elementType, indexedPath, valueResolver); + String indexedPath = paramPath + "[" + (index != NO_INDEX ? index : "") + "]"; + array[Math.max(index, 0)] = + createIndexedValue(paramPath, paramType, elementType, indexedPath, valueResolver); } return array; @@ -1124,13 +1132,20 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter { SortedSet indexes = null; for (String name : valueResolver.getNames()) { if (name.startsWith(paramPath + "[")) { - int endIndex = name.indexOf(']', paramPath.length() + 2); - String rawIndex = name.substring(paramPath.length() + 1, endIndex); - if (StringUtils.hasLength(rawIndex)) { - int index = Integer.parseInt(rawIndex); - indexes = (indexes != null ? indexes : new TreeSet<>()); - indexes.add(index); + int index; + if (paramPath.length() + 2 == name.length()) { + if (!name.endsWith("[]")) { + continue; + } + index = NO_INDEX; } + else { + int endIndex = name.indexOf(']', paramPath.length() + 2); + String indexValue = name.substring(paramPath.length() + 1, endIndex); + index = Integer.parseInt(indexValue); + } + indexes = (indexes != null ? indexes : new TreeSet<>()); + indexes.add(index); } } return indexes; diff --git a/spring-context/src/test/java/org/springframework/validation/DataBinderConstructTests.java b/spring-context/src/test/java/org/springframework/validation/DataBinderConstructTests.java index 7b0fdb72ee..8bae5d4c4d 100644 --- a/spring-context/src/test/java/org/springframework/validation/DataBinderConstructTests.java +++ b/spring-context/src/test/java/org/springframework/validation/DataBinderConstructTests.java @@ -188,6 +188,17 @@ class DataBinderConstructTests { assertThat(target.integerList()).containsExactly(1, 2); } + @Test + void simpleListBindingEmptyBrackets() { + MapValueResolver valueResolver = new MapValueResolver(Map.of("integerList[]", "1")); + + DataBinder binder = initDataBinder(IntegerListRecord.class); + binder.construct(valueResolver); + + IntegerListRecord target = getTarget(binder); + assertThat(target.integerList()).containsExactly(1); + } + @Test void simpleMapBinding() { MapValueResolver valueResolver = new MapValueResolver(Map.of("integerMap[a]", "1", "integerMap[b]", "2")); From 9f55296049172081d2f32b0ebd847b7d05a4f008 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 11 Feb 2025 10:11:39 +0000 Subject: [PATCH 2/3] Nested list/map/array with constructor binding Closes gh-34305 --- .../validation/DataBinder.java | 33 +++++++++++------ .../validation/DataBinderConstructTests.java | 36 +++++++++++++++++++ 2 files changed, 59 insertions(+), 10 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/validation/DataBinder.java b/spring-context/src/main/java/org/springframework/validation/DataBinder.java index 2d00655f4c..3433af095b 100644 --- a/spring-context/src/main/java/org/springframework/validation/DataBinder.java +++ b/spring-context/src/main/java/org/springframework/validation/DataBinder.java @@ -1154,23 +1154,36 @@ public class DataBinder implements PropertyEditorRegistry, TypeConverter { @SuppressWarnings("unchecked") @Nullable private V createIndexedValue( - String paramPath, Class paramType, ResolvableType elementType, + String paramPath, Class containerType, ResolvableType elementType, String indexedPath, ValueResolver valueResolver) { Object value = null; Class elementClass = elementType.resolve(Object.class); - Object rawValue = valueResolver.resolveValue(indexedPath, elementClass); - if (rawValue != null) { - try { - value = convertIfNecessary(rawValue, elementClass); - } - catch (TypeMismatchException ex) { - handleTypeMismatchException(ex, paramPath, paramType, rawValue); - } + + if (List.class.isAssignableFrom(elementClass)) { + value = createList(indexedPath, elementClass, elementType, valueResolver); + } + else if (Map.class.isAssignableFrom(elementClass)) { + value = createMap(indexedPath, elementClass, elementType, valueResolver); + } + else if (elementClass.isArray()) { + value = createArray(indexedPath, elementClass, elementType, valueResolver); } else { - value = createObject(elementType, indexedPath + ".", valueResolver); + Object rawValue = valueResolver.resolveValue(indexedPath, elementClass); + if (rawValue != null) { + try { + value = convertIfNecessary(rawValue, elementClass); + } + catch (TypeMismatchException ex) { + handleTypeMismatchException(ex, paramPath, containerType, rawValue); + } + } + else { + value = createObject(elementType, indexedPath + ".", valueResolver); + } } + return (V) value; } diff --git a/spring-context/src/test/java/org/springframework/validation/DataBinderConstructTests.java b/spring-context/src/test/java/org/springframework/validation/DataBinderConstructTests.java index 8bae5d4c4d..4c81ca65cc 100644 --- a/spring-context/src/test/java/org/springframework/validation/DataBinderConstructTests.java +++ b/spring-context/src/test/java/org/springframework/validation/DataBinderConstructTests.java @@ -221,6 +221,34 @@ class DataBinderConstructTests { assertThat(target.integerArray()).containsExactly(1, 2); } + @Test + void nestedListWithinMap() { + MapValueResolver valueResolver = new MapValueResolver(Map.of( + "integerListMap[a][0]", "1", "integerListMap[a][1]", "2", + "integerListMap[b][0]", "3", "integerListMap[b][1]", "4")); + + DataBinder binder = initDataBinder(IntegerListMapRecord.class); + binder.construct(valueResolver); + + IntegerListMapRecord target = getTarget(binder); + assertThat(target.integerListMap().get("a")).containsExactly(1, 2); + assertThat(target.integerListMap().get("b")).containsExactly(3, 4); + } + + @Test + void nestedMapWithinList() { + MapValueResolver valueResolver = new MapValueResolver(Map.of( + "integerMapList[0][a]", "1", "integerMapList[0][b]", "2", + "integerMapList[1][a]", "3", "integerMapList[1][b]", "4")); + + DataBinder binder = initDataBinder(IntegerMapListRecord.class); + binder.construct(valueResolver); + + IntegerMapListRecord target = getTarget(binder); + assertThat(target.integerMapList().get(0)).containsOnly(Map.entry("a", 1), Map.entry("b", 2)); + assertThat(target.integerMapList().get(1)).containsOnly(Map.entry("a", 3), Map.entry("b", 4)); + } + @SuppressWarnings("SameParameterValue") private static DataBinder initDataBinder(Class targetType) { @@ -317,6 +345,14 @@ class DataBinderConstructTests { } + private record IntegerMapListRecord(List> integerMapList) { + } + + + private record IntegerListMapRecord(Map> integerListMap) { + } + + private record MapValueResolver(Map map) implements DataBinder.ValueResolver { @Override From d04883f8391dff7ecbcd1e336a0fa53e8fdae759 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Tue, 11 Feb 2025 11:08:00 +0000 Subject: [PATCH 3/3] HTTP Interface client handles query param with ":" Closes gh-34364 --- .../web/service/invoker/HttpRequestValues.java | 2 +- .../invoker/HttpRequestValuesTests.java | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java b/spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java index e2c95915dc..309aa60faf 100644 --- a/spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java +++ b/spring-web/src/main/java/org/springframework/web/service/invoker/HttpRequestValues.java @@ -493,7 +493,7 @@ public class HttpRequestValues { UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUriString(uriTemplate); for (Map.Entry> entry : requestParams.entrySet()) { - String nameVar = entry.getKey(); + String nameVar = entry.getKey().replace(":", "%3A"); // suppress treatment as regex uriVars.put(nameVar, entry.getKey()); for (int j = 0; j < entry.getValue().size(); j++) { String valueVar = nameVar + "[" + j + "]"; diff --git a/spring-web/src/test/java/org/springframework/web/service/invoker/HttpRequestValuesTests.java b/spring-web/src/test/java/org/springframework/web/service/invoker/HttpRequestValuesTests.java index f929e0c72b..5f026afe65 100644 --- a/spring-web/src/test/java/org/springframework/web/service/invoker/HttpRequestValuesTests.java +++ b/spring-web/src/test/java/org/springframework/web/service/invoker/HttpRequestValuesTests.java @@ -99,6 +99,24 @@ class HttpRequestValuesTests { .isEqualTo("/path?param1=1st%20value¶m2=2nd%20value%20A¶m2=2nd%20value%20B"); } + @Test // gh-34364 + void queryParamWithSemicolon() { + HttpRequestValues requestValues = HttpRequestValues.builder().setHttpMethod(HttpMethod.POST) + .setUriTemplate("/path") + .addRequestParameter("userId:eq", "test value") + .build(); + + String uriTemplate = requestValues.getUriTemplate(); + assertThat(uriTemplate).isEqualTo("/path?{userId%3Aeq}={userId%3Aeq[0]}"); + + URI uri = UriComponentsBuilder.fromUriString(uriTemplate) + .encode() + .build(requestValues.getUriVariables()); + + assertThat(uri.toString()) + .isEqualTo("/path?userId%3Aeq=test%20value"); + } + @Test void queryParamsWithPreparedUri() {