Skip to content

Commit

Permalink
Fix ServletRequestDataBinder ctor binding with []-indexed query params
Browse files Browse the repository at this point in the history
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
  • Loading branch information
simonbasle committed Dec 27, 2024
1 parent 3505c4b commit 0f38c28
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
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;
Expand Down Expand Up @@ -57,7 +56,6 @@ void postArray(WebApplicationContext wac) throws Exception {
.andExpect(content().string("valueB"));
}

@Disabled("see gh-34121")
@Test // gh-34121
void postArrayWithEmptyIndex(WebApplicationContext wac) throws Exception {
MockMvc mockMvc = webAppContextSetup(wac).build();
Expand Down Expand Up @@ -88,7 +86,6 @@ void postList(WebApplicationContext wac) throws Exception {
.andExpect(content().string("valueB"));
}

@Disabled("see gh-34121")
@Test // gh-34121
void postListWithEmptyIndex(WebApplicationContext wac) throws Exception {
MockMvc mockMvc = webAppContextSetup(wac).build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,10 @@ public final Object resolveValue(String name, Class<?> paramType) {
@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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,32 @@ void testFieldPrefixCausesFieldResetWithIgnoreUnknownFields() {
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();
Expand Down

0 comments on commit 0f38c28

Please sign in to comment.