The existence of a field defaults indicates that the specified - * value should be used if the field is otherwise not present. + * Check the given property values for fields that start with the field default + * prefix. + *
The existence of a field default indicates that the specified value should + * be used if the field is allowed and is otherwise not present. * @param mpvs the property values to be bound (can be modified) - * @see #getFieldDefaultPrefix + * @see #getFieldDefaultPrefix() + * @see #isAllowed(String) */ protected void checkFieldDefaults(MutablePropertyValues mpvs) { String fieldDefaultPrefix = getFieldDefaultPrefix(); @@ -252,7 +253,7 @@ public class WebDataBinder extends DataBinder { for (PropertyValue pv : pvArray) { if (pv.getName().startsWith(fieldDefaultPrefix)) { String field = pv.getName().substring(fieldDefaultPrefix.length()); - if (getPropertyAccessor().isWritableProperty(field) && !mpvs.contains(field)) { + if (isAllowed(field) && getPropertyAccessor().isWritableProperty(field) && !mpvs.contains(field)) { mpvs.add(field, pv.getValue()); } mpvs.removePropertyValue(pv); @@ -262,14 +263,15 @@ public class WebDataBinder extends DataBinder { } /** - * Check the given property values for field markers, - * i.e. for fields that start with the field marker prefix. - *
The existence of a field marker indicates that the specified - * field existed in the form. If the property values do not contain - * a corresponding field value, the field will be considered as empty - * and will be reset appropriately. + * Check the given property values for fields that start with the field marker + * prefix. + *
The existence of a field marker indicates that the specified field existed + * in the form. + *
If the field is allowed and the property values do not contain a corresponding + * field value, the field will be considered as empty and will be reset appropriately. * @param mpvs the property values to be bound (can be modified) - * @see #getFieldMarkerPrefix + * @see #getFieldMarkerPrefix() + * @see #isAllowed(String) * @see #getEmptyValue(String, Class) */ protected void checkFieldMarkers(MutablePropertyValues mpvs) { @@ -279,7 +281,7 @@ public class WebDataBinder extends DataBinder { for (PropertyValue pv : pvArray) { if (pv.getName().startsWith(fieldMarkerPrefix)) { String field = pv.getName().substring(fieldMarkerPrefix.length()); - if (getPropertyAccessor().isWritableProperty(field) && !mpvs.contains(field)) { + if (isAllowed(field) && getPropertyAccessor().isWritableProperty(field) && !mpvs.contains(field)) { Class fieldType = getPropertyAccessor().getPropertyType(field); mpvs.add(field, getEmptyValue(field, fieldType)); } @@ -290,19 +292,22 @@ public class WebDataBinder extends DataBinder { } /** - * Check for property values with names that end on {@code "[]"}. This is - * used by some clients for array syntax without an explicit index value. - * If such values are found, drop the brackets to adapt to the expected way - * of expressing the same for data binding purposes. + * Check the given property values for fields that end with empty brackets + * ({@code []}). + *
This is used by some clients for array syntax without an explicit index + * value. + *
If such a field is allowed, the brackets will be removed in order to adapt + * to the expected format for expressing the same for data binding purposes. * @param mpvs the property values to be bound (can be modified) * @since 5.3 + * @see #isAllowed(String) */ protected void adaptEmptyArrayIndices(MutablePropertyValues mpvs) { for (PropertyValue pv : mpvs.getPropertyValues()) { String name = pv.getName(); if (name.endsWith("[]")) { String field = name.substring(0, name.length() - 2); - if (getPropertyAccessor().isWritableProperty(field) && !mpvs.contains(field)) { + if (isAllowed(field) && getPropertyAccessor().isWritableProperty(field) && !mpvs.contains(field)) { mpvs.add(field, pv.getValue()); } mpvs.removePropertyValue(pv); diff --git a/spring-web/src/test/java/org/springframework/web/bind/support/WebRequestDataBinderTests.java b/spring-web/src/test/java/org/springframework/web/bind/support/WebRequestDataBinderTests.java index 2a7c4b826cb..bd1d341d6fc 100644 --- a/spring-web/src/test/java/org/springframework/web/bind/support/WebRequestDataBinderTests.java +++ b/spring-web/src/test/java/org/springframework/web/bind/support/WebRequestDataBinderTests.java @@ -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 { } } - }