From 0f38c28e9155e0190cf3ffb5478aca3e9282b6e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Fri, 27 Dec 2024 10:30:35 +0100 Subject: [PATCH] Fix ServletRequestDataBinder ctor binding with `[]`-indexed query params This change ensures that a request containing query parameters in the array format `someArray[]=value` can be bound into a simple array in constructors, even for cases where the array values don't have nested properties. The value resolver is directly called in the constructor case, before any mutable properties are considered or even cleared (see `WebDataBinder#adaptEmptyArrayIndices` method). As a result, we need to accommodate the possibility that the request stores array elements under the `name[]` key rather than `name`. This change attempts a secondary lookup with the `[]` suffix if the type is a list or array, and the key doesn't include an index. Closes gh-34121 --- ...vletRequestDataBinderIntegrationTests.java | 3 --- .../web/bind/ServletRequestDataBinder.java | 4 +++ .../bind/ServletRequestDataBinderTests.java | 26 +++++++++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/spring-test/src/test/java/org/springframework/test/web/servlet/samples/spr/ServletRequestDataBinderIntegrationTests.java b/spring-test/src/test/java/org/springframework/test/web/servlet/samples/spr/ServletRequestDataBinderIntegrationTests.java index 1750d4a8950..64d7d5f434b 100644 --- a/spring-test/src/test/java/org/springframework/test/web/servlet/samples/spr/ServletRequestDataBinderIntegrationTests.java +++ b/spring-test/src/test/java/org/springframework/test/web/servlet/samples/spr/ServletRequestDataBinderIntegrationTests.java @@ -19,7 +19,6 @@ package org.springframework.test.web.servlet.samples.spr; import java.util.List; import java.util.Map; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.test.context.junit.jupiter.web.SpringJUnitWebConfig; @@ -57,7 +56,6 @@ class ServletRequestDataBinderIntegrationTests { .andExpect(content().string("valueB")); } - @Disabled("see gh-34121") @Test // gh-34121 void postArrayWithEmptyIndex(WebApplicationContext wac) throws Exception { MockMvc mockMvc = webAppContextSetup(wac).build(); @@ -88,7 +86,6 @@ class ServletRequestDataBinderIntegrationTests { .andExpect(content().string("valueB")); } - @Disabled("see gh-34121") @Test // gh-34121 void postListWithEmptyIndex(WebApplicationContext wac) throws Exception { MockMvc mockMvc = webAppContextSetup(wac).build(); diff --git a/spring-web/src/main/java/org/springframework/web/bind/ServletRequestDataBinder.java b/spring-web/src/main/java/org/springframework/web/bind/ServletRequestDataBinder.java index 07ae3555666..6add6a2dbb9 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/ServletRequestDataBinder.java +++ b/spring-web/src/main/java/org/springframework/web/bind/ServletRequestDataBinder.java @@ -244,6 +244,10 @@ public class ServletRequestDataBinder extends WebDataBinder { @Nullable protected Object getRequestParameter(String name, Class type) { Object value = this.request.getParameterValues(name); + if (value == null && !name.endsWith ("[]") && + (List.class.isAssignableFrom(type) || type.isArray())) { + value = this.request.getParameterValues(name + "[]"); + } return (ObjectUtils.isArray(value) && Array.getLength(value) == 1 ? Array.get(value, 0) : value); } diff --git a/spring-web/src/test/java/org/springframework/web/bind/ServletRequestDataBinderTests.java b/spring-web/src/test/java/org/springframework/web/bind/ServletRequestDataBinderTests.java index adbae1ddc62..fa10f23a045 100644 --- a/spring-web/src/test/java/org/springframework/web/bind/ServletRequestDataBinderTests.java +++ b/spring-web/src/test/java/org/springframework/web/bind/ServletRequestDataBinderTests.java @@ -93,6 +93,32 @@ class ServletRequestDataBinderTests { assertThat(target.isPostProcessed()).isFalse(); } + @Test + public void testFieldWithArrayIndex() { + TestBean target = new TestBean(); + ServletRequestDataBinder binder = new ServletRequestDataBinder(target); + binder.setIgnoreUnknownFields(false); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addParameter("stringArray[0]", "ONE"); + request.addParameter("stringArray[1]", "TWO"); + binder.bind(request); + assertThat(target.getStringArray()).containsExactly("ONE", "TWO"); + } + + @Test + public void testFieldWithEmptyArrayIndex() { + TestBean target = new TestBean(); + ServletRequestDataBinder binder = new ServletRequestDataBinder(target); + binder.setIgnoreUnknownFields(false); + + MockHttpServletRequest request = new MockHttpServletRequest(); + request.addParameter("stringArray[]", "ONE"); + request.addParameter("stringArray[]", "TWO"); + binder.bind(request); + assertThat(target.getStringArray()).containsExactly("ONE", "TWO"); + } + @Test void testFieldDefault() { TestBean target = new TestBean();