spring/spring-framework
1
0
Fork 0
mirror of https://github.com/spring-projects/spring-framework.git synced 2026-09-21 05:09:13 +00:00
Code Issues Projects Releases Wiki Activity

Revise "Skip binding entirely when field is not allowed"

Browse Source 
This commit reverts the changes made to WebDataBinder's doBind()
implementation in e4d03f6625 and instead implements the skipping logic
directly in checkFieldDefaults(), checkFieldMarkers(), and
adaptEmptyArrayIndices() by preemptively checking if the corresponding
field is allowed.

This commit also improves the Javadoc and adds missing tests.

Fixes gh-36625
This commit is contained in:
Sam Brannen
2026-04-16 14:33:50 +02:00
parent cb320468db
commit 68c575ab14
2 changed files with 137 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files
spring-web/src/test/java/org/springframework/web/bind/support/WebRequestDataBinderTests.java
+106 -28
View File
@@ -23,7 +23,12 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.Parameter;
import org.junit.jupiter.params.ParameterizedClass;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.springframework.beans.PropertyValue;
import org.springframework.beans.PropertyValues;
@@ -37,9 +42,15 @@ import org.springframework.web.testfixture.servlet.MockMultipartFile;
import org.springframework.web.testfixture.servlet.MockMultipartHttpServletRequest;
import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.web.bind.WebDataBinder.DEFAULT_FIELD_DEFAULT_PREFIX;
import static org.springframework.web.bind.WebDataBinder.DEFAULT_FIELD_MARKER_PREFIX;
/**
* Tests for {@link WebRequestDataBinder}.
*
* @author Juergen Hoeller
* @author Brian Clozel
* @author Sam Brannen
*/
class WebRequestDataBinderTests {
@@ -79,10 +90,13 @@ class WebRequestDataBinderTests {
assertThat(tb.getSpouse().getName()).isEqualTo("test");
}
@Test
void fieldPrefixCausesFieldReset() {
@ParameterizedTest
@ValueSource(booleans = { true, false })
void markerPrefixCausesFieldReset(boolean ignoreUnknownFields) {
TestBean target = new TestBean();
WebRequestDataBinder binder = new WebRequestDataBinder(target);
binder.setIgnoreUnknownFields(ignoreUnknownFields);
MockHttpServletRequest request = new MockHttpServletRequest();
request.addParameter("_postProcessed", "visible");
@@ -96,23 +110,30 @@ class WebRequestDataBinderTests {
}
@Test
void fieldPrefixCausesFieldResetWithIgnoreUnknownFields() {
void fieldWithArrayIndices() {
TestBean target = new TestBean();
WebRequestDataBinder binder = new WebRequestDataBinder(target);
binder.setIgnoreUnknownFields(false);
MockHttpServletRequest request = new MockHttpServletRequest();
request.addParameter("_postProcessed", "visible");
request.addParameter("postProcessed", "on");
request.addParameter("stringArray[0]", "ONE");
request.addParameter("stringArray[1]", "TWO");
binder.bind(new ServletWebRequest(request));
assertThat(target.isPostProcessed()).isTrue();
request.removeParameter("postProcessed");
binder.bind(new ServletWebRequest(request));
assertThat(target.isPostProcessed()).isFalse();
assertThat(target.getStringArray()).containsExactly("ONE", "TWO");
}
@Test // gh-25836
@Test
void fieldWithMissingArrayIndex() {
TestBean target = new TestBean();
WebRequestDataBinder binder = new WebRequestDataBinder(target);
MockHttpServletRequest request = new MockHttpServletRequest();
request.addParameter("stringArray", "ONE");
request.addParameter("stringArray", "TWO");
binder.bind(new ServletWebRequest(request));
assertThat(target.getStringArray()).containsExactly("ONE", "TWO");
}
@Test // gh-25836
void fieldWithEmptyArrayIndex() {
TestBean target = new TestBean();
WebRequestDataBinder binder = new WebRequestDataBinder(target);
@@ -140,8 +161,7 @@ class WebRequestDataBinderTests {
assertThat(target.isPostProcessed()).isFalse();
}
// SPR-13502
@Test
@Test // SPR-13502
void collectionFieldsDefault() {
TestBean target = new TestBean();
target.setSomeSet(null);
@@ -332,7 +352,7 @@ class WebRequestDataBinderTests {
for (PropertyValue pv : pvArray) {
Object val = m.get(pv.getName());
assertThat(val).as("Can't have unexpected value").isNotNull();
assertThat(val).as("Val i string").isInstanceOf(String.class);
assertThat(val).as("Val is string").isInstanceOf(String.class);
assertThat(val).as("val matches expected").isEqualTo(pv.getValue());
m.remove(pv.getName());
}
@@ -359,21 +379,80 @@ class WebRequestDataBinderTests {
assertThat(Arrays.asList(original)).as("Correct values").isEqualTo(Arrays.asList(values));
}
@Test
void defaultArgumentShouldNotTriggerAutoGrowWhenDisallowed() {
TestBean tb = new TestBean();
tb.setSomeMap(null);
WebRequestDataBinder binder = new WebRequestDataBinder(tb, "person");
binder.setAllowedFields("name");
@ParameterizedClass // gh-36625
@ValueSource(strings = { DEFAULT_FIELD_DEFAULT_PREFIX, DEFAULT_FIELD_MARKER_PREFIX })
@Nested
class DefaultAndMarkerPrefixTests {
MockHttpServletRequest request = new MockHttpServletRequest();
request.addParameter("name", "spring");
request.addParameter("!someMap[key1]", "test");
binder.bind(new ServletWebRequest(request));
@Parameter
String prefix;
assertThat(tb.getName()).isEqualTo("spring");
assertThat(tb.getSomeMap()).isNull();
@Test
void shouldNotTriggerBindingWhenFieldIsNotAllowed() {
TestBean tb = new TestBean();
WebRequestDataBinder binder = new WebRequestDataBinder(tb, "person");
binder.setAllowedFields("name");
MockHttpServletRequest request = new MockHttpServletRequest();
request.addParameter("name", "spring");
request.addParameter(prefix + "country", "test");
binder.bind(new ServletWebRequest(request));
assertThat(tb.getName()).isEqualTo("spring");
assertThat(tb.getCountry()).isNull();
}
@Test
void shouldNotTriggerBindingWhenFieldIsDisallowed() {
TestBean tb = new TestBean();
WebRequestDataBinder binder = new WebRequestDataBinder(tb, "person");
binder.setDisallowedFields("country");
MockHttpServletRequest request = new MockHttpServletRequest();
request.addParameter("name", "spring");
request.addParameter(prefix + "country", "test");
binder.bind(new ServletWebRequest(request));
assertThat(tb.getName()).isEqualTo("spring");
assertThat(tb.getCountry()).isNull();
}
@Test
void shouldNotTriggerAutoGrowWhenFieldIsNotAllowed() {
TestBean tb = new TestBean();
tb.setSomeMap(null);
WebRequestDataBinder binder = new WebRequestDataBinder(tb, "person");
binder.setAllowedFields("name");
MockHttpServletRequest request = new MockHttpServletRequest();
request.addParameter("name", "spring");
request.addParameter(prefix + "someMap[key1]", "test");
binder.bind(new ServletWebRequest(request));
assertThat(tb.getName()).isEqualTo("spring");
assertThat(tb.getSomeMap()).isNull();
}
@Test
void shouldNotTriggerAutoGrowWhenFieldIsDisallowed() {
TestBean tb = new TestBean();
tb.setSomeMap(null);
WebRequestDataBinder binder = new WebRequestDataBinder(tb, "person");
binder.setDisallowedFields("someMap[key1]");
MockHttpServletRequest request = new MockHttpServletRequest();
request.addParameter("name", "spring");
request.addParameter(prefix + "someMap[key1]", "test");
binder.bind(new ServletWebRequest(request));
assertThat(tb.getName()).isEqualTo("spring");
assertThat(tb.getSomeMap()).isNull();
}
}
@@ -404,5 +483,4 @@ class WebRequestDataBinderTests {
}
}
}