diff --git a/spring-beans/src/main/java/org/springframework/beans/PropertyAccessor.java b/spring-beans/src/main/java/org/springframework/beans/PropertyAccessor.java index 3a417aadcd37..03201a89d0d7 100644 --- a/spring-beans/src/main/java/org/springframework/beans/PropertyAccessor.java +++ b/spring-beans/src/main/java/org/springframework/beans/PropertyAccessor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,8 +23,9 @@ /** * Common interface for classes that can access named properties - * (such as bean properties of an object or fields in an object) - * Serves as base interface for {@link BeanWrapper}. + * (such as bean properties of an object or fields in an object). + * + *

Serves as base interface for {@link BeanWrapper}. * * @author Juergen Hoeller * @since 1.1 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 612dfc5622a2..8ee5e43b02fe 100644 --- a/spring-context/src/main/java/org/springframework/validation/DataBinder.java +++ b/spring-context/src/main/java/org/springframework/validation/DataBinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -51,18 +51,20 @@ import org.springframework.util.StringUtils; /** - * Binder that allows for setting property values onto a target object, - * including support for validation and binding result analysis. - * The binding process can be customized through specifying allowed fields, + * Binder that allows for setting property values on a target object, including + * support for validation and binding result analysis. + * + *

The binding process can be customized by specifying allowed field patterns, * required fields, custom editors, etc. * - *

Note that there are potential security implications in failing to set an array - * of allowed fields. In the case of HTTP form POST data for example, malicious clients - * can attempt to subvert an application by supplying values for fields or properties - * that do not exist on the form. In some cases this could lead to illegal data being - * set on command objects or their nested objects. For this reason, it is - * highly recommended to specify the {@link #setAllowedFields allowedFields} property - * on the DataBinder. + *

WARNING: Data binding can lead to security issues by exposing + * parts of the object graph that are not meant to be accessed or modified by + * external clients. Therefore the design and use of data binding should be considered + * carefully with regard to security. For more details, please refer to the dedicated + * sections on data binding for + * Spring Web MVC and + * Spring WebFlux + * in the reference manual. * *

The binding results can be examined via the {@link BindingResult} interface, * extending the {@link Errors} interface: see the {@link #getBindingResult()} method. @@ -96,6 +98,7 @@ * @author Rob Harrop * @author Stephane Nicoll * @author Kazuki Shimizu + * @author Sam Brannen * @see #setAllowedFields * @see #setRequiredFields * @see #registerCustomEditor @@ -418,15 +421,21 @@ public boolean isIgnoreInvalidFields() { } /** - * Register fields that should be allowed for binding. Default is all fields. - * Restrict this for example to avoid unwanted modifications by malicious + * Register field patterns that should be allowed for binding. + *

Default is all fields. + *

Restrict this for example to avoid unwanted modifications by malicious * users when binding HTTP request parameters. - *

Supports "xxx*", "*xxx", "*xxx*" and "xxx*yyy" matches (with an - * arbitrary number of pattern parts), as well as direct equality. More - * sophisticated matching can be implemented by overriding the - * {@code isAllowed} method. - *

Alternatively, specify a list of disallowed fields. - * @param allowedFields array of field names + *

Supports {@code "xxx*"}, {@code "*xxx"}, {@code "*xxx*"}, and + * {@code "xxx*yyy"} matches (with an arbitrary number of pattern parts), as + * well as direct equality. + *

The default implementation of this method stores allowed field patterns + * in {@linkplain PropertyAccessorUtils#canonicalPropertyName(String) canonical} + * form. Subclasses which override this method must therefore take this into + * account. + *

More sophisticated matching can be implemented by overriding the + * {@link #isAllowed} method. + *

Alternatively, specify a list of disallowed field patterns. + * @param allowedFields array of allowed field patterns * @see #setDisallowedFields * @see #isAllowed(String) */ @@ -435,8 +444,9 @@ public void setAllowedFields(@Nullable String... allowedFields) { } /** - * Return the fields that should be allowed for binding. - * @return array of field names + * Return the field patterns that should be allowed for binding. + * @return array of allowed field patterns + * @see #setAllowedFields(String...) */ @Nullable public String[] getAllowedFields() { @@ -444,25 +454,44 @@ public String[] getAllowedFields() { } /** - * Register fields that should not be allowed for binding. Default - * is none. Mark fields as disallowed for example to avoid unwanted + * Register field patterns that should not be allowed for binding. + *

Default is none. + *

Mark fields as disallowed, for example to avoid unwanted * modifications by malicious users when binding HTTP request parameters. - *

Supports "xxx*", "*xxx", "*xxx*" and "xxx*yyy" matches (with an - * arbitrary number of pattern parts), as well as direct equality. - * More sophisticated matching can be implemented by overriding the - * {@code isAllowed} method. - *

Alternatively, specify a list of allowed fields. - * @param disallowedFields array of field names + *

Supports {@code "xxx*"}, {@code "*xxx"}, {@code "*xxx*"}, and + * {@code "xxx*yyy"} matches (with an arbitrary number of pattern parts), as + * well as direct equality. + *

The default implementation of this method stores disallowed field patterns + * in {@linkplain PropertyAccessorUtils#canonicalPropertyName(String) canonical} + * form. As of Spring Framework 5.2.21, the default implementation also transforms + * disallowed field patterns to {@linkplain String#toLowerCase() lowercase} to + * support case-insensitive pattern matching in {@link #isAllowed}. Subclasses + * which override this method must therefore take both of these transformations + * into account. + *

More sophisticated matching can be implemented by overriding the + * {@link #isAllowed} method. + *

Alternatively, specify a list of allowed field patterns. + * @param disallowedFields array of disallowed field patterns * @see #setAllowedFields * @see #isAllowed(String) */ public void setDisallowedFields(@Nullable String... disallowedFields) { - this.disallowedFields = PropertyAccessorUtils.canonicalPropertyNames(disallowedFields); + if (disallowedFields == null) { + this.disallowedFields = null; + } + else { + String[] fieldPatterns = new String[disallowedFields.length]; + for (int i = 0; i < fieldPatterns.length; i++) { + fieldPatterns[i] = PropertyAccessorUtils.canonicalPropertyName(disallowedFields[i]).toLowerCase(); + } + this.disallowedFields = fieldPatterns; + } } /** - * Return the fields that should not be allowed for binding. - * @return array of field names + * Return the field patterns that should not be allowed for binding. + * @return array of disallowed field patterns + * @see #setDisallowedFields(String...) */ @Nullable public String[] getDisallowedFields() { @@ -774,16 +803,20 @@ protected void checkAllowedFields(MutablePropertyValues mpvs) { } /** - * Return if the given field is allowed for binding. - * Invoked for each passed-in property value. - *

The default implementation checks for "xxx*", "*xxx", "*xxx*" and "xxx*yyy" - * matches (with an arbitrary number of pattern parts), as well as direct equality, - * in the specified lists of allowed fields and disallowed fields. A field matching - * a disallowed pattern will not be accepted even if it also happens to match a - * pattern in the allowed list. - *

Can be overridden in subclasses. + * Determine if the given field is allowed for binding. + *

Invoked for each passed-in property value. + *

Checks for {@code "xxx*"}, {@code "*xxx"}, {@code "*xxx*"}, and + * {@code "xxx*yyy"} matches (with an arbitrary number of pattern parts), as + * well as direct equality, in the configured lists of allowed field patterns + * and disallowed field patterns. + *

Matching against allowed field patterns is case-sensitive; whereas, + * matching against disallowed field patterns is case-insensitive. + *

A field matching a disallowed pattern will not be accepted even if it + * also happens to match a pattern in the allowed list. + *

Can be overridden in subclasses, but care must be taken to honor the + * aforementioned contract. * @param field the field to check - * @return if the field is allowed + * @return {@code true} if the field is allowed * @see #setAllowedFields * @see #setDisallowedFields * @see org.springframework.util.PatternMatchUtils#simpleMatch(String, String) @@ -792,7 +825,7 @@ protected boolean isAllowed(String field) { String[] allowed = getAllowedFields(); String[] disallowed = getDisallowedFields(); return ((ObjectUtils.isEmpty(allowed) || PatternMatchUtils.simpleMatch(allowed, field)) && - (ObjectUtils.isEmpty(disallowed) || !PatternMatchUtils.simpleMatch(disallowed, field))); + (ObjectUtils.isEmpty(disallowed) || !PatternMatchUtils.simpleMatch(disallowed, field.toLowerCase()))); } /** diff --git a/spring-context/src/test/java/org/springframework/validation/DataBinderTests.java b/spring-context/src/test/java/org/springframework/validation/DataBinderTests.java index 3d39e2ecd309..546c599c01f7 100644 --- a/spring-context/src/test/java/org/springframework/validation/DataBinderTests.java +++ b/spring-context/src/test/java/org/springframework/validation/DataBinderTests.java @@ -64,23 +64,26 @@ import org.springframework.format.support.FormattingConversionService; import org.springframework.lang.Nullable; import org.springframework.tests.sample.beans.BeanWithObjectProperty; -import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.assertj.core.api.Assertions.entry; /** + * Unit tests for {@link DataBinder}. + * * @author Rod Johnson * @author Juergen Hoeller * @author Rob Harrop * @author Kazuki Shimizu + * @author Sam Brannen */ class DataBinderTests { @Test - void testBindingNoErrors() throws BindException { + void bindingNoErrors() throws BindException { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod, "person"); assertThat(binder.isIgnoreUnknownFields()).isTrue(); @@ -110,12 +113,11 @@ void testBindingNoErrors() throws BindException { assertThat(ex).isEqualTo(binder.getBindingResult()); other.reject("xxx"); - boolean condition = !other.equals(binder.getBindingResult()); - assertThat(condition).isTrue(); + assertThat(other).isNotEqualTo(binder.getBindingResult()); } @Test - void testBindingWithDefaultConversionNoErrors() throws BindException { + void bindingWithDefaultConversionNoErrors() throws BindException { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod, "person"); assertThat(binder.isIgnoreUnknownFields()).isTrue(); @@ -131,7 +133,7 @@ void testBindingWithDefaultConversionNoErrors() throws BindException { } @Test - void testNestedBindingWithDefaultConversionNoErrors() throws BindException { + void nestedBindingWithDefaultConversionNoErrors() throws BindException { TestBean rod = new TestBean(new TestBean()); DataBinder binder = new DataBinder(rod, "person"); assertThat(binder.isIgnoreUnknownFields()).isTrue(); @@ -147,7 +149,7 @@ void testNestedBindingWithDefaultConversionNoErrors() throws BindException { } @Test - void testBindingNoErrorsNotIgnoreUnknown() { + void bindingNoErrorsNotIgnoreUnknown() { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod, "person"); binder.setIgnoreUnknownFields(false); @@ -160,7 +162,7 @@ void testBindingNoErrorsNotIgnoreUnknown() { } @Test - void testBindingNoErrorsWithInvalidField() { + void bindingNoErrorsWithInvalidField() { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod, "person"); MutablePropertyValues pvs = new MutablePropertyValues(); @@ -171,7 +173,7 @@ void testBindingNoErrorsWithInvalidField() { } @Test - void testBindingNoErrorsWithIgnoreInvalid() { + void bindingNoErrorsWithIgnoreInvalid() throws BindException { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod, "person"); binder.setIgnoreInvalidFields(true); @@ -180,10 +182,14 @@ void testBindingNoErrorsWithIgnoreInvalid() { pvs.add("spouse.age", 32); binder.bind(pvs); + binder.close(); + + assertThat(rod.getName()).isEqualTo("Rod"); + assertThat(rod.getSpouse()).isNull(); } @Test - void testBindingWithErrors() { + void bindingWithErrors() { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod, "person"); MutablePropertyValues pvs = new MutablePropertyValues(); @@ -245,7 +251,7 @@ void testBindingWithErrors() { } @Test - void testBindingWithSystemFieldError() { + void bindingWithSystemFieldError() { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod, "person"); MutablePropertyValues pvs = new MutablePropertyValues(); @@ -257,7 +263,7 @@ void testBindingWithSystemFieldError() { } @Test - void testBindingWithErrorsAndCustomEditors() { + void bindingWithErrorsAndCustomEditors() { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod, "person"); binder.registerCustomEditor(String.class, "touchy", new PropertyEditorSupport() { @@ -325,7 +331,7 @@ public String getAsText() { } @Test - void testBindingWithCustomEditorOnObjectField() { + void bindingWithCustomEditorOnObjectField() { BeanWithObjectProperty tb = new BeanWithObjectProperty(); DataBinder binder = new DataBinder(tb); binder.registerCustomEditor(Integer.class, "object", new CustomNumberEditor(Integer.class, true)); @@ -336,7 +342,7 @@ void testBindingWithCustomEditorOnObjectField() { } @Test - void testBindingWithFormatter() { + void bindingWithFormatter() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb); FormattingConversionService conversionService = new FormattingConversionService(); @@ -368,7 +374,7 @@ void testBindingWithFormatter() { } @Test - void testBindingErrorWithFormatter() { + void bindingErrorWithFormatter() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb); FormattingConversionService conversionService = new FormattingConversionService(); @@ -391,7 +397,7 @@ void testBindingErrorWithFormatter() { } @Test - void testBindingErrorWithParseExceptionFromFormatter() { + void bindingErrorWithParseExceptionFromFormatter() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb); FormattingConversionService conversionService = new FormattingConversionService(); @@ -419,7 +425,7 @@ public String print(String object, Locale locale) { } @Test - void testBindingErrorWithRuntimeExceptionFromFormatter() { + void bindingErrorWithRuntimeExceptionFromFormatter() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb); FormattingConversionService conversionService = new FormattingConversionService(); @@ -447,7 +453,7 @@ public String print(String object, Locale locale) { } @Test - void testBindingWithFormatterAgainstList() { + void bindingWithFormatterAgainstList() { BeanWithIntegerList tb = new BeanWithIntegerList(); DataBinder binder = new DataBinder(tb); FormattingConversionService conversionService = new FormattingConversionService(); @@ -469,7 +475,7 @@ void testBindingWithFormatterAgainstList() { } @Test - void testBindingErrorWithFormatterAgainstList() { + void bindingErrorWithFormatterAgainstList() { BeanWithIntegerList tb = new BeanWithIntegerList(); DataBinder binder = new DataBinder(tb); FormattingConversionService conversionService = new FormattingConversionService(); @@ -492,7 +498,7 @@ void testBindingErrorWithFormatterAgainstList() { } @Test - void testBindingWithFormatterAgainstFields() { + void bindingWithFormatterAgainstFields() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb); FormattingConversionService conversionService = new FormattingConversionService(); @@ -525,7 +531,7 @@ void testBindingWithFormatterAgainstFields() { } @Test - void testBindingErrorWithFormatterAgainstFields() { + void bindingErrorWithFormatterAgainstFields() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb); binder.initDirectFieldAccess(); @@ -549,7 +555,7 @@ void testBindingErrorWithFormatterAgainstFields() { } @Test - void testBindingWithCustomFormatter() { + void bindingWithCustomFormatter() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb); binder.addCustomFormatter(new NumberStyleFormatter(), Float.class); @@ -578,7 +584,7 @@ void testBindingWithCustomFormatter() { } @Test - void testBindingErrorWithCustomFormatter() { + void bindingErrorWithCustomFormatter() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb); binder.addCustomFormatter(new NumberStyleFormatter()); @@ -599,7 +605,7 @@ void testBindingErrorWithCustomFormatter() { } @Test - void testBindingErrorWithParseExceptionFromCustomFormatter() { + void bindingErrorWithParseExceptionFromCustomFormatter() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb); @@ -624,7 +630,7 @@ public String print(String object, Locale locale) { } @Test - void testBindingErrorWithRuntimeExceptionFromCustomFormatter() { + void bindingErrorWithRuntimeExceptionFromCustomFormatter() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb); @@ -649,7 +655,7 @@ public String print(String object, Locale locale) { } @Test - void testConversionWithInappropriateStringEditor() { + void conversionWithInappropriateStringEditor() { DataBinder dataBinder = new DataBinder(null); DefaultFormattingConversionService conversionService = new DefaultFormattingConversionService(); dataBinder.setConversionService(conversionService); @@ -662,7 +668,7 @@ void testConversionWithInappropriateStringEditor() { } @Test - void testBindingWithAllowedFields() throws BindException { + void bindingWithAllowedFields() throws BindException { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod); binder.setAllowedFields("name", "myparam"); @@ -672,30 +678,32 @@ void testBindingWithAllowedFields() throws BindException { binder.bind(pvs); binder.close(); - assertThat(rod.getName().equals("Rod")).as("changed name correctly").isTrue(); - assertThat(rod.getAge() == 0).as("did not change age").isTrue(); + + assertThat(rod.getName()).as("changed name correctly").isEqualTo("Rod"); + assertThat(rod.getAge()).as("did not change age").isZero(); } @Test - void testBindingWithDisallowedFields() throws BindException { + void bindingWithDisallowedFields() throws BindException { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod); - binder.setDisallowedFields("age"); + binder.setDisallowedFields(" ", "\t", "favouriteColour", null, "age"); MutablePropertyValues pvs = new MutablePropertyValues(); pvs.add("name", "Rod"); pvs.add("age", "32x"); + pvs.add("favouriteColour", "BLUE"); binder.bind(pvs); binder.close(); - assertThat(rod.getName().equals("Rod")).as("changed name correctly").isTrue(); - assertThat(rod.getAge() == 0).as("did not change age").isTrue(); - String[] disallowedFields = binder.getBindingResult().getSuppressedFields(); - assertThat(disallowedFields.length).isEqualTo(1); - assertThat(disallowedFields[0]).isEqualTo("age"); + + assertThat(rod.getName()).as("changed name correctly").isEqualTo("Rod"); + assertThat(rod.getAge()).as("did not change age").isZero(); + assertThat(rod.getFavouriteColour()).as("did not change favourite colour").isNull(); + assertThat(binder.getBindingResult().getSuppressedFields()).containsExactlyInAnyOrder("age", "favouriteColour"); } @Test - void testBindingWithAllowedAndDisallowedFields() throws BindException { + void bindingWithAllowedAndDisallowedFields() throws BindException { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod); binder.setAllowedFields("name", "myparam"); @@ -706,34 +714,32 @@ void testBindingWithAllowedAndDisallowedFields() throws BindException { binder.bind(pvs); binder.close(); - assertThat(rod.getName().equals("Rod")).as("changed name correctly").isTrue(); - assertThat(rod.getAge() == 0).as("did not change age").isTrue(); - String[] disallowedFields = binder.getBindingResult().getSuppressedFields(); - assertThat(disallowedFields).hasSize(1); - assertThat(disallowedFields[0]).isEqualTo("age"); + + assertThat(rod.getName()).as("changed name correctly").isEqualTo("Rod"); + assertThat(rod.getAge()).as("did not change age").isZero(); + assertThat(binder.getBindingResult().getSuppressedFields()).containsExactly("age"); } @Test - void testBindingWithOverlappingAllowedAndDisallowedFields() throws BindException { + void bindingWithOverlappingAllowedAndDisallowedFields() throws BindException { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod); binder.setAllowedFields("name", "age"); - binder.setDisallowedFields("age"); + binder.setDisallowedFields("AGE"); MutablePropertyValues pvs = new MutablePropertyValues(); pvs.add("name", "Rod"); pvs.add("age", "32x"); binder.bind(pvs); binder.close(); - assertThat(rod.getName().equals("Rod")).as("changed name correctly").isTrue(); - assertThat(rod.getAge() == 0).as("did not change age").isTrue(); - String[] disallowedFields = binder.getBindingResult().getSuppressedFields(); - assertThat(disallowedFields).hasSize(1); - assertThat(disallowedFields[0]).isEqualTo("age"); + + assertThat(rod.getName()).as("changed name correctly").isEqualTo("Rod"); + assertThat(rod.getAge()).as("did not change age").isZero(); + assertThat(binder.getBindingResult().getSuppressedFields()).containsExactly("age"); } @Test - void testBindingWithAllowedFieldsUsingAsterisks() throws BindException { + void bindingWithAllowedFieldsUsingAsterisks() throws BindException { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod, "person"); binder.setAllowedFields("nam*", "*ouchy"); @@ -760,11 +766,11 @@ void testBindingWithAllowedFieldsUsingAsterisks() throws BindException { } @Test - void testBindingWithAllowedAndDisallowedMapFields() throws BindException { + void bindingWithAllowedAndDisallowedMapFields() throws BindException { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod); binder.setAllowedFields("someMap[key1]", "someMap[key2]"); - binder.setDisallowedFields("someMap['key3']", "someMap[key4]"); + binder.setDisallowedFields("someMap['KEY3']", "SomeMap[key4]"); MutablePropertyValues pvs = new MutablePropertyValues(); pvs.add("someMap[key1]", "value1"); @@ -774,21 +780,18 @@ void testBindingWithAllowedAndDisallowedMapFields() throws BindException { binder.bind(pvs); binder.close(); - assertThat(rod.getSomeMap().get("key1")).isEqualTo("value1"); - assertThat(rod.getSomeMap().get("key2")).isEqualTo("value2"); - assertThat(rod.getSomeMap().get("key3")).isNull(); - assertThat(rod.getSomeMap().get("key4")).isNull(); - String[] disallowedFields = binder.getBindingResult().getSuppressedFields(); - assertThat(disallowedFields).hasSize(2); - assertThat(ObjectUtils.containsElement(disallowedFields, "someMap[key3]")).isTrue(); - assertThat(ObjectUtils.containsElement(disallowedFields, "someMap[key4]")).isTrue(); + + @SuppressWarnings("unchecked") + Map someMap = (Map) rod.getSomeMap(); + assertThat(someMap).containsOnly(entry("key1", "value1"), entry("key2", "value2")); + assertThat(binder.getBindingResult().getSuppressedFields()).containsExactly("someMap[key3]", "someMap[key4]"); } /** * Tests for required field, both null, non-existing and empty strings. */ @Test - void testBindingWithRequiredFields() { + void bindingWithRequiredFields() { TestBean tb = new TestBean(); tb.setSpouse(new TestBean()); @@ -819,7 +822,7 @@ void testBindingWithRequiredFields() { } @Test - void testBindingWithRequiredMapFields() { + void bindingWithRequiredMapFields() { TestBean tb = new TestBean(); tb.setSpouse(new TestBean()); @@ -839,7 +842,7 @@ void testBindingWithRequiredMapFields() { } @Test - void testBindingWithNestedObjectCreation() { + void bindingWithNestedObjectCreation() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb, "person"); @@ -860,7 +863,7 @@ public void setAsText(String text) throws IllegalArgumentException { } @Test - void testCustomEditorWithOldValueAccess() { + void customEditorWithOldValueAccess() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb, "tb"); @@ -885,7 +888,7 @@ public void setAsText(String text) throws IllegalArgumentException { } @Test - void testCustomEditorForSingleProperty() { + void customEditorForSingleProperty() { TestBean tb = new TestBean(); tb.setSpouse(new TestBean()); DataBinder binder = new DataBinder(tb, "tb"); @@ -925,7 +928,7 @@ public String getAsText() { } @Test - void testCustomEditorForPrimitiveProperty() { + void customEditorForPrimitiveProperty() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb, "tb"); @@ -949,7 +952,7 @@ public String getAsText() { } @Test - void testCustomEditorForAllStringProperties() { + void customEditorForAllStringProperties() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb, "tb"); @@ -981,7 +984,7 @@ public String getAsText() { } @Test - void testCustomFormatterForSingleProperty() { + void customFormatterForSingleProperty() { TestBean tb = new TestBean(); tb.setSpouse(new TestBean()); DataBinder binder = new DataBinder(tb, "tb"); @@ -1021,7 +1024,7 @@ public String print(String object, Locale locale) { } @Test - void testCustomFormatterForPrimitiveProperty() { + void customFormatterForPrimitiveProperty() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb, "tb"); @@ -1045,7 +1048,7 @@ public String print(Integer object, Locale locale) { } @Test - void testCustomFormatterForAllStringProperties() { + void customFormatterForAllStringProperties() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb, "tb"); @@ -1077,7 +1080,7 @@ public String print(String object, Locale locale) { } @Test - void testJavaBeanPropertyConventions() { + void javaBeanPropertyConventions() { Book book = new Book(); DataBinder binder = new DataBinder(book); @@ -1101,7 +1104,7 @@ void testJavaBeanPropertyConventions() { } @Test - void testOptionalProperty() { + void optionalProperty() { OptionalHolder bean = new OptionalHolder(); DataBinder binder = new DataBinder(bean); binder.setConversionService(new DefaultConversionService()); @@ -1122,7 +1125,7 @@ void testOptionalProperty() { } @Test - void testValidatorNoErrors() throws Exception { + void validatorNoErrors() throws Exception { TestBean tb = new TestBean(); tb.setAge(33); tb.setName("Rod"); @@ -1175,15 +1178,13 @@ void testValidatorNoErrors() throws Exception { assertThat(errors.getNestedPath()).isEqualTo("spouse."); assertThat(errors.getErrorCount()).isEqualTo(1); - boolean condition1 = !errors.hasGlobalErrors(); - assertThat(condition1).isTrue(); + assertThat(errors.hasGlobalErrors()).isFalse(); assertThat(errors.getFieldErrorCount("age")).isEqualTo(1); - boolean condition = !errors.hasFieldErrors("name"); - assertThat(condition).isTrue(); + assertThat(errors.hasFieldErrors("name")).isFalse(); } @Test - void testValidatorWithErrors() { + void validatorWithErrors() { TestBean tb = new TestBean(); tb.setSpouse(new TestBean()); @@ -1252,7 +1253,7 @@ void testValidatorWithErrors() { } @Test - void testValidatorWithErrorsAndCodesPrefix() { + void validatorWithErrorsAndCodesPrefix() { TestBean tb = new TestBean(); tb.setSpouse(new TestBean()); @@ -1324,7 +1325,7 @@ void testValidatorWithErrorsAndCodesPrefix() { } @Test - void testValidatorWithNestedObjectNull() { + void validatorWithNestedObjectNull() { TestBean tb = new TestBean(); Errors errors = new BeanPropertyBindingResult(tb, "tb"); Validator testValidator = new TestBeanValidator(); @@ -1343,7 +1344,7 @@ void testValidatorWithNestedObjectNull() { } @Test - void testNestedValidatorWithoutNestedPath() { + void nestedValidatorWithoutNestedPath() { TestBean tb = new TestBean(); tb.setName("XXX"); Errors errors = new BeanPropertyBindingResult(tb, "tb"); @@ -1357,7 +1358,8 @@ void testNestedValidatorWithoutNestedPath() { } @Test - void testBindingStringArrayToIntegerSet() { + @SuppressWarnings("unchecked") + void bindingStringArrayToIntegerSet() { IndexedTestBean tb = new IndexedTestBean(); DataBinder binder = new DataBinder(tb, "tb"); binder.registerCustomEditor(Set.class, new CustomCollectionEditor(TreeSet.class) { @@ -1371,12 +1373,8 @@ protected Object convertElement(Object element) { binder.bind(pvs); assertThat(binder.getBindingResult().getFieldValue("set")).isEqualTo(tb.getSet()); - boolean condition = tb.getSet() instanceof TreeSet; - assertThat(condition).isTrue(); - assertThat(tb.getSet().size()).isEqualTo(3); - assertThat(tb.getSet().contains(10)).isTrue(); - assertThat(tb.getSet().contains(20)).isTrue(); - assertThat(tb.getSet().contains(30)).isTrue(); + assertThat(tb.getSet()).isInstanceOf(TreeSet.class); + assertThat((Set) tb.getSet()).containsExactly(10, 20, 30); pvs = new MutablePropertyValues(); pvs.add("set", null); @@ -1386,7 +1384,7 @@ protected Object convertElement(Object element) { } @Test - void testBindingNullToEmptyCollection() { + void bindingNullToEmptyCollection() { IndexedTestBean tb = new IndexedTestBean(); DataBinder binder = new DataBinder(tb, "tb"); binder.registerCustomEditor(Set.class, new CustomCollectionEditor(TreeSet.class, true)); @@ -1394,13 +1392,12 @@ void testBindingNullToEmptyCollection() { pvs.add("set", null); binder.bind(pvs); - boolean condition = tb.getSet() instanceof TreeSet; - assertThat(condition).isTrue(); - assertThat(tb.getSet().isEmpty()).isTrue(); + assertThat(tb.getSet()).isInstanceOf(TreeSet.class); + assertThat(tb.getSet()).isEmpty(); } @Test - void testBindingToIndexedField() { + void bindingToIndexedField() { IndexedTestBean tb = new IndexedTestBean(); DataBinder binder = new DataBinder(tb, "tb"); binder.registerCustomEditor(String.class, "array.name", new PropertyEditorSupport() { @@ -1439,7 +1436,7 @@ public void setAsText(String text) throws IllegalArgumentException { } @Test - void testBindingToNestedIndexedField() { + void bindingToNestedIndexedField() { IndexedTestBean tb = new IndexedTestBean(); tb.getArray()[0].setNestedIndexedBean(new IndexedTestBean()); tb.getArray()[1].setNestedIndexedBean(new IndexedTestBean()); @@ -1470,7 +1467,7 @@ public void setAsText(String text) throws IllegalArgumentException { } @Test - void testEditorForNestedIndexedField() { + void editorForNestedIndexedField() { IndexedTestBean tb = new IndexedTestBean(); tb.getArray()[0].setNestedIndexedBean(new IndexedTestBean()); tb.getArray()[1].setNestedIndexedBean(new IndexedTestBean()); @@ -1496,7 +1493,7 @@ public String getAsText() { } @Test - void testSpecificEditorForNestedIndexedField() { + void specificEditorForNestedIndexedField() { IndexedTestBean tb = new IndexedTestBean(); tb.getArray()[0].setNestedIndexedBean(new IndexedTestBean()); tb.getArray()[1].setNestedIndexedBean(new IndexedTestBean()); @@ -1522,7 +1519,7 @@ public String getAsText() { } @Test - void testInnerSpecificEditorForNestedIndexedField() { + void innerSpecificEditorForNestedIndexedField() { IndexedTestBean tb = new IndexedTestBean(); tb.getArray()[0].setNestedIndexedBean(new IndexedTestBean()); tb.getArray()[1].setNestedIndexedBean(new IndexedTestBean()); @@ -1548,7 +1545,7 @@ public String getAsText() { } @Test - void testDirectBindingToIndexedField() { + void directBindingToIndexedField() { IndexedTestBean tb = new IndexedTestBean(); DataBinder binder = new DataBinder(tb, "tb"); binder.registerCustomEditor(TestBean.class, "array", new PropertyEditorSupport() { @@ -1601,7 +1598,7 @@ public String getAsText() { } @Test - void testDirectBindingToEmptyIndexedFieldWithRegisteredSpecificEditor() { + void directBindingToEmptyIndexedFieldWithRegisteredSpecificEditor() { IndexedTestBean tb = new IndexedTestBean(); DataBinder binder = new DataBinder(tb, "tb"); binder.registerCustomEditor(TestBean.class, "map[key0]", new PropertyEditorSupport() { @@ -1632,7 +1629,7 @@ public String getAsText() { } @Test - void testDirectBindingToEmptyIndexedFieldWithRegisteredGenericEditor() { + void directBindingToEmptyIndexedFieldWithRegisteredGenericEditor() { IndexedTestBean tb = new IndexedTestBean(); DataBinder binder = new DataBinder(tb, "tb"); binder.registerCustomEditor(TestBean.class, "map", new PropertyEditorSupport() { @@ -1663,7 +1660,7 @@ public String getAsText() { } @Test - void testCustomEditorWithSubclass() { + void customEditorWithSubclass() { IndexedTestBean tb = new IndexedTestBean(); DataBinder binder = new DataBinder(tb, "tb"); binder.registerCustomEditor(TestBean.class, new PropertyEditorSupport() { @@ -1697,7 +1694,7 @@ public String getAsText() { } @Test - void testBindToStringArrayWithArrayEditor() { + void bindToStringArrayWithArrayEditor() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb, "tb"); binder.registerCustomEditor(String[].class, "stringArray", new PropertyEditorSupport() { @@ -1709,15 +1706,12 @@ public void setAsText(String text) throws IllegalArgumentException { MutablePropertyValues pvs = new MutablePropertyValues(); pvs.add("stringArray", "a1-b2"); binder.bind(pvs); - boolean condition = !binder.getBindingResult().hasErrors(); - assertThat(condition).isTrue(); - assertThat(tb.getStringArray().length).isEqualTo(2); - assertThat(tb.getStringArray()[0]).isEqualTo("a1"); - assertThat(tb.getStringArray()[1]).isEqualTo("b2"); + assertThat(binder.getBindingResult().hasErrors()).isFalse(); + assertThat(tb.getStringArray()).containsExactly("a1", "b2"); } @Test - void testBindToStringArrayWithComponentEditor() { + void bindToStringArrayWithComponentEditor() { TestBean tb = new TestBean(); DataBinder binder = new DataBinder(tb, "tb"); binder.registerCustomEditor(String.class, "stringArray", new PropertyEditorSupport() { @@ -1729,15 +1723,14 @@ public void setAsText(String text) throws IllegalArgumentException { MutablePropertyValues pvs = new MutablePropertyValues(); pvs.add("stringArray", new String[] {"a1", "b2"}); binder.bind(pvs); - boolean condition = !binder.getBindingResult().hasErrors(); - assertThat(condition).isTrue(); + assertThat(binder.getBindingResult().hasErrors()).isFalse(); assertThat(tb.getStringArray().length).isEqualTo(2); assertThat(tb.getStringArray()[0]).isEqualTo("Xa1"); assertThat(tb.getStringArray()[1]).isEqualTo("Xb2"); } @Test - void testBindingErrors() { + void bindingErrors() { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod, "person"); MutablePropertyValues pvs = new MutablePropertyValues(); @@ -1764,7 +1757,7 @@ void testBindingErrors() { } @Test - void testAddAllErrors() { + void addAllErrors() { TestBean rod = new TestBean(); DataBinder binder = new DataBinder(rod, "person"); MutablePropertyValues pvs = new MutablePropertyValues(); @@ -1784,7 +1777,7 @@ void testAddAllErrors() { @Test @SuppressWarnings("unchecked") - void testBindingWithResortedList() { + void bindingWithResortedList() { IndexedTestBean tb = new IndexedTestBean(); DataBinder binder = new DataBinder(tb, "tb"); MutablePropertyValues pvs = new MutablePropertyValues(); @@ -1802,7 +1795,7 @@ void testBindingWithResortedList() { } @Test - void testRejectWithoutDefaultMessage() { + void rejectWithoutDefaultMessage() { TestBean tb = new TestBean(); tb.setName("myName"); tb.setAge(99); @@ -1820,7 +1813,7 @@ void testRejectWithoutDefaultMessage() { } @Test - void testBindExceptionSerializable() throws Exception { + void bindExceptionSerializable() throws Exception { SerializablePerson tb = new SerializablePerson(); tb.setName("myName"); tb.setAge(99); @@ -1849,27 +1842,27 @@ void testBindExceptionSerializable() throws Exception { } @Test - void testTrackDisallowedFields() { + void trackDisallowedFields() { TestBean testBean = new TestBean(); DataBinder binder = new DataBinder(testBean, "testBean"); binder.setAllowedFields("name", "age"); String name = "Rob Harrop"; - String beanName = "foobar"; + int age = 42; MutablePropertyValues mpvs = new MutablePropertyValues(); mpvs.add("name", name); - mpvs.add("beanName", beanName); + mpvs.add("age", age); + mpvs.add("beanName", "foobar"); binder.bind(mpvs); assertThat(testBean.getName()).isEqualTo(name); - String[] disallowedFields = binder.getBindingResult().getSuppressedFields(); - assertThat(disallowedFields).hasSize(1); - assertThat(disallowedFields[0]).isEqualTo("beanName"); + assertThat(testBean.getAge()).isEqualTo(age); + assertThat(binder.getBindingResult().getSuppressedFields()).containsExactly("beanName"); } @Test - void testAutoGrowWithinDefaultLimit() { + void autoGrowWithinDefaultLimit() { TestBean testBean = new TestBean(); DataBinder binder = new DataBinder(testBean, "testBean"); @@ -1881,7 +1874,7 @@ void testAutoGrowWithinDefaultLimit() { } @Test - void testAutoGrowBeyondDefaultLimit() { + void autoGrowBeyondDefaultLimit() { TestBean testBean = new TestBean(); DataBinder binder = new DataBinder(testBean, "testBean"); @@ -1894,7 +1887,7 @@ void testAutoGrowBeyondDefaultLimit() { } @Test - void testAutoGrowWithinCustomLimit() { + void autoGrowWithinCustomLimit() { TestBean testBean = new TestBean(); DataBinder binder = new DataBinder(testBean, "testBean"); binder.setAutoGrowCollectionLimit(10); @@ -1907,7 +1900,7 @@ void testAutoGrowWithinCustomLimit() { } @Test - void testAutoGrowBeyondCustomLimit() { + void autoGrowBeyondCustomLimit() { TestBean testBean = new TestBean(); DataBinder binder = new DataBinder(testBean, "testBean"); binder.setAutoGrowCollectionLimit(10); @@ -1921,7 +1914,7 @@ void testAutoGrowBeyondCustomLimit() { } @Test - void testNestedGrowingList() { + void nestedGrowingList() { Form form = new Form(); DataBinder binder = new DataBinder(form, "form"); MutablePropertyValues mpv = new MutablePropertyValues(); @@ -1937,7 +1930,7 @@ void testNestedGrowingList() { } @Test - void testFieldErrorAccessVariations() { + void fieldErrorAccessVariations() { TestBean testBean = new TestBean(); DataBinder binder = new DataBinder(testBean, "testBean"); assertThat(binder.getBindingResult().getGlobalError()).isNull(); @@ -1958,7 +1951,7 @@ void testFieldErrorAccessVariations() { } @Test // SPR-14888 - void testSetAutoGrowCollectionLimit() { + void setAutoGrowCollectionLimit() { BeanWithIntegerList tb = new BeanWithIntegerList(); DataBinder binder = new DataBinder(tb); binder.setAutoGrowCollectionLimit(257); @@ -1972,7 +1965,7 @@ void testSetAutoGrowCollectionLimit() { } @Test // SPR-14888 - void testSetAutoGrowCollectionLimitAfterInitialization() { + void setAutoGrowCollectionLimitAfterInitialization() { DataBinder binder = new DataBinder(new BeanWithIntegerList()); binder.registerCustomEditor(String.class, new StringTrimmerEditor(true)); assertThatIllegalStateException().isThrownBy(() -> @@ -1981,7 +1974,7 @@ void testSetAutoGrowCollectionLimitAfterInitialization() { } @Test // SPR-15009 - void testSetCustomMessageCodesResolverBeforeInitializeBindingResultForBeanPropertyAccess() { + void setCustomMessageCodesResolverBeforeInitializeBindingResultForBeanPropertyAccess() { TestBean testBean = new TestBean(); DataBinder binder = new DataBinder(testBean, "testBean"); DefaultMessageCodesResolver messageCodesResolver = new DefaultMessageCodesResolver(); @@ -1998,7 +1991,7 @@ void testSetCustomMessageCodesResolverBeforeInitializeBindingResultForBeanProper } @Test // SPR-15009 - void testSetCustomMessageCodesResolverBeforeInitializeBindingResultForDirectFieldAccess() { + void setCustomMessageCodesResolverBeforeInitializeBindingResultForDirectFieldAccess() { TestBean testBean = new TestBean(); DataBinder binder = new DataBinder(testBean, "testBean"); DefaultMessageCodesResolver messageCodesResolver = new DefaultMessageCodesResolver(); @@ -2013,7 +2006,7 @@ void testSetCustomMessageCodesResolverBeforeInitializeBindingResultForDirectFiel } @Test // SPR-15009 - void testSetCustomMessageCodesResolverAfterInitializeBindingResult() { + void setCustomMessageCodesResolverAfterInitializeBindingResult() { TestBean testBean = new TestBean(); DataBinder binder = new DataBinder(testBean, "testBean"); binder.initBeanPropertyAccess(); @@ -2028,7 +2021,7 @@ void testSetCustomMessageCodesResolverAfterInitializeBindingResult() { } @Test // SPR-15009 - void testSetMessageCodesResolverIsNullAfterInitializeBindingResult() { + void setMessageCodesResolverIsNullAfterInitializeBindingResult() { TestBean testBean = new TestBean(); DataBinder binder = new DataBinder(testBean, "testBean"); binder.initBeanPropertyAccess(); @@ -2042,8 +2035,7 @@ void testSetMessageCodesResolverIsNullAfterInitializeBindingResult() { } @Test // SPR-15009 - void testCallSetMessageCodesResolverTwice() { - + void callSetMessageCodesResolverTwice() { TestBean testBean = new TestBean(); DataBinder binder = new DataBinder(testBean, "testBean"); binder.setMessageCodesResolver(new DefaultMessageCodesResolver()); 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 16bf30ee01fc..864e21843984 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -33,6 +33,15 @@ * Special {@link org.springframework.validation.DataBinder} to perform data binding * from servlet request parameters to JavaBeans, including support for multipart files. * + *

WARNING: Data binding can lead to security issues by exposing + * parts of the object graph that are not meant to be accessed or modified by + * external clients. Therefore the design and use of data binding should be considered + * carefully with regard to security. For more details, please refer to the dedicated + * sections on data binding for + * Spring Web MVC and + * Spring WebFlux + * in the reference manual. + * *

See the DataBinder/WebDataBinder superclasses for customization options, * which include specifying allowed/required fields, and registering custom * property editors. diff --git a/spring-web/src/main/java/org/springframework/web/bind/WebDataBinder.java b/spring-web/src/main/java/org/springframework/web/bind/WebDataBinder.java index a1cd50ad7443..754f72ac5493 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/WebDataBinder.java +++ b/spring-web/src/main/java/org/springframework/web/bind/WebDataBinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -34,6 +34,15 @@ * the Servlet API; serves as base class for more specific DataBinder variants, * such as {@link org.springframework.web.bind.ServletRequestDataBinder}. * + *

WARNING: Data binding can lead to security issues by exposing + * parts of the object graph that are not meant to be accessed or modified by + * external clients. Therefore the design and use of data binding should be considered + * carefully with regard to security. For more details, please refer to the dedicated + * sections on data binding for + * Spring Web MVC and + * Spring WebFlux + * in the reference manual. + * *

Includes support for field markers which address a common problem with * HTML checkboxes and select options: detecting that a field was part of * the form, but did not generate a request parameter because it was empty. diff --git a/spring-web/src/main/java/org/springframework/web/bind/annotation/ExceptionHandler.java b/spring-web/src/main/java/org/springframework/web/bind/annotation/ExceptionHandler.java index 3f48fa431185..467d9853ef2e 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/annotation/ExceptionHandler.java +++ b/spring-web/src/main/java/org/springframework/web/bind/annotation/ExceptionHandler.java @@ -100,6 +100,7 @@ * @author Arjen Poutsma * @author Juergen Hoeller * @since 3.0 + * @see ControllerAdvice * @see org.springframework.web.context.request.WebRequest */ @Target(ElementType.METHOD) diff --git a/spring-web/src/main/java/org/springframework/web/bind/annotation/InitBinder.java b/spring-web/src/main/java/org/springframework/web/bind/annotation/InitBinder.java index 5fc5d6bcc279..370b2f2801e0 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/annotation/InitBinder.java +++ b/spring-web/src/main/java/org/springframework/web/bind/annotation/InitBinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -23,15 +23,24 @@ import java.lang.annotation.Target; /** - * Annotation that identifies methods which initialize the + * Annotation that identifies methods that initialize the * {@link org.springframework.web.bind.WebDataBinder} which * will be used for populating command and form object arguments * of annotated handler methods. * - *

Such init-binder methods support all arguments that {@link RequestMapping} - * supports, except for command/form objects and corresponding validation result - * objects. Init-binder methods must not have a return value; they are usually - * declared as {@code void}. + *

WARNING: Data binding can lead to security issues by exposing + * parts of the object graph that are not meant to be accessed or modified by + * external clients. Therefore the design and use of data binding should be considered + * carefully with regard to security. For more details, please refer to the dedicated + * sections on data binding for + * Spring Web MVC and + * Spring WebFlux + * in the reference manual. + * + *

{@code @InitBinder} methods support all arguments that + * {@link RequestMapping @RequestMapping} methods support, except for command/form + * objects and corresponding validation result objects. {@code @InitBinder} methods + * must not have a return value; they are usually declared as {@code void}. * *

Typical arguments are {@link org.springframework.web.bind.WebDataBinder} * in combination with {@link org.springframework.web.context.request.WebRequest} @@ -39,6 +48,7 @@ * * @author Juergen Hoeller * @since 2.5 + * @see ControllerAdvice * @see org.springframework.web.bind.WebDataBinder * @see org.springframework.web.context.request.WebRequest */ diff --git a/spring-web/src/main/java/org/springframework/web/bind/annotation/ModelAttribute.java b/spring-web/src/main/java/org/springframework/web/bind/annotation/ModelAttribute.java index 717a6d0106ef..3316065a0760 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/annotation/ModelAttribute.java +++ b/spring-web/src/main/java/org/springframework/web/bind/annotation/ModelAttribute.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -31,18 +31,27 @@ * for controller classes with {@link RequestMapping @RequestMapping} * methods. * - *

Can be used to expose command objects to a web view, using - * specific attribute names, through annotating corresponding - * parameters of an {@link RequestMapping @RequestMapping} method. + *

WARNING: Data binding can lead to security issues by exposing + * parts of the object graph that are not meant to be accessed or modified by + * external clients. Therefore the design and use of data binding should be considered + * carefully with regard to security. For more details, please refer to the dedicated + * sections on data binding for + * Spring Web MVC and + * Spring WebFlux + * in the reference manual. * - *

Can also be used to expose reference data to a web view - * through annotating accessor methods in a controller class with + *

{@code @ModelAttribute} can be used to expose command objects to a web view, + * using specific attribute names, by annotating corresponding parameters of an + * {@link RequestMapping @RequestMapping} method. + * + *

{@code @ModelAttribute} can also be used to expose reference data to a web + * view by annotating accessor methods in a controller class with * {@link RequestMapping @RequestMapping} methods. Such accessor * methods are allowed to have any arguments that * {@link RequestMapping @RequestMapping} methods support, returning * the model attribute value to expose. * - *

Note however that reference data and all other model content is + *

Note however that reference data and all other model content are * not available to web views when request processing results in an * {@code Exception} since the exception could be raised at any time * making the content of the model unreliable. For this reason @@ -52,6 +61,7 @@ * @author Juergen Hoeller * @author Rossen Stoyanchev * @since 2.5 + * @see ControllerAdvice */ @Target({ElementType.PARAMETER, ElementType.METHOD}) @Retention(RetentionPolicy.RUNTIME) @@ -77,7 +87,7 @@ String name() default ""; /** - * Allows declaring data binding disabled directly on an {@code @ModelAttribute} + * Allows data binding to be disabled directly on an {@code @ModelAttribute} * method parameter or on the attribute returned from an {@code @ModelAttribute} * method, both of which would prevent data binding for that attribute. *

By default this is set to {@code true} in which case data binding applies. diff --git a/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java b/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java index ed7855e79097..b4957970a5d2 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java +++ b/spring-web/src/main/java/org/springframework/web/bind/support/WebExchangeDataBinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,6 +36,15 @@ * Specialized {@link org.springframework.validation.DataBinder} to perform data * binding from URL query parameters or form data in the request data to Java objects. * + *

WARNING: Data binding can lead to security issues by exposing + * parts of the object graph that are not meant to be accessed or modified by + * external clients. Therefore the design and use of data binding should be considered + * carefully with regard to security. For more details, please refer to the dedicated + * sections on data binding for + * Spring Web MVC and + * Spring WebFlux + * in the reference manual. + * * @author Rossen Stoyanchev * @author Juergen Hoeller * @since 5.0 diff --git a/spring-web/src/main/java/org/springframework/web/bind/support/WebRequestDataBinder.java b/spring-web/src/main/java/org/springframework/web/bind/support/WebRequestDataBinder.java index 76ea4abddab4..805667f69b87 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/support/WebRequestDataBinder.java +++ b/spring-web/src/main/java/org/springframework/web/bind/support/WebRequestDataBinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -35,6 +35,15 @@ * Special {@link org.springframework.validation.DataBinder} to perform data binding * from web request parameters to JavaBeans, including support for multipart files. * + *

WARNING: Data binding can lead to security issues by exposing + * parts of the object graph that are not meant to be accessed or modified by + * external clients. Therefore the design and use of data binding should be considered + * carefully with regard to security. For more details, please refer to the dedicated + * sections on data binding for + * Spring Web MVC and + * Spring WebFlux + * in the reference manual. + * *

See the DataBinder/WebDataBinder superclasses for customization options, * which include specifying allowed/required fields, and registering custom * property editors. diff --git a/spring-web/src/test/java/org/springframework/web/method/annotation/InitBinderDataBinderFactoryTests.java b/spring-web/src/test/java/org/springframework/web/method/annotation/InitBinderDataBinderFactoryTests.java index 1126d2bf2516..ce284f935e71 100644 --- a/spring-web/src/test/java/org/springframework/web/method/annotation/InitBinderDataBinderFactoryTests.java +++ b/spring-web/src/test/java/org/springframework/web/method/annotation/InitBinderDataBinderFactoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -116,7 +116,7 @@ public void createBinderTypeConversion() throws Exception { WebDataBinder dataBinder = factory.createBinder(this.webRequest, null, "foo"); assertThat(dataBinder.getDisallowedFields()).isNotNull(); - assertThat(dataBinder.getDisallowedFields()[0]).isEqualTo("requestParam-22"); + assertThat(dataBinder.getDisallowedFields()[0]).isEqualToIgnoringCase("requestParam-22"); } private WebDataBinderFactory createFactory(String methodName, Class... parameterTypes) diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/InitBinderBindingContextTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/InitBinderBindingContextTests.java index 56ba84873cca..d695a3f750c6 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/InitBinderBindingContextTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/InitBinderBindingContextTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -121,7 +121,7 @@ public void createBinderTypeConversion() throws Exception { WebDataBinder dataBinder = context.createDataBinder(exchange, null, "foo"); assertThat(dataBinder.getDisallowedFields()).isNotNull(); - assertThat(dataBinder.getDisallowedFields()[0]).isEqualTo("requestParam-22"); + assertThat(dataBinder.getDisallowedFields()[0]).isEqualToIgnoringCase("requestParam-22"); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinder.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinder.java index 2a7489b98176..662f1e722991 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinder.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,6 +29,15 @@ * Subclass of {@link ServletRequestDataBinder} that adds URI template variables * to the values used for data binding. * + *

WARNING: Data binding can lead to security issues by exposing + * parts of the object graph that are not meant to be accessed or modified by + * external clients. Therefore the design and use of data binding should be considered + * carefully with regard to security. For more details, please refer to the dedicated + * sections on data binding for + * Spring Web MVC and + * Spring WebFlux + * in the reference manual. + * * @author Rossen Stoyanchev * @since 3.1 * @see ServletRequestDataBinder diff --git a/src/docs/asciidoc/web/web-data-binding-model-design.adoc b/src/docs/asciidoc/web/web-data-binding-model-design.adoc new file mode 100644 index 000000000000..352e63d3c6f3 --- /dev/null +++ b/src/docs/asciidoc/web/web-data-binding-model-design.adoc @@ -0,0 +1,95 @@ +In the context of web applications, _data binding_ involves the binding of HTTP request +parameters (that is, form data or query parameters) to properties in a model object and +its nested objects. + +Only `public` properties following the +https://www.oracle.com/java/technologies/javase/javabeans-spec.html[JavaBeans naming conventions] +are exposed for data binding — for example, `public String getFirstName()` and +`public void setFirstName(String)` methods for a `firstName` property. + +TIP: The model object, and its nested object graph, is also sometimes referred to as a +_command object_, _form-backing object_, or _POJO_ (Plain Old Java Object). + +By default, Spring permits binding to all public properties in the model object graph. +This means you need to carefully consider what public properties the model has, since a +client could target any public property path, even some that are not expected to be +targeted for a given use case. + +For example, given an HTTP form data endpoint, a malicious client could supply values for +properties that exist in the model object graph but are not part of the HTML form +presented in the browser. This could lead to data being set on the model object and any +of its nested objects, that is not expected to be updated. + +The recommended approach is to use a _dedicated model object_ that exposes only +properties that are relevant for the form submission. For example, on a form for changing +a user's email address, the model object should declare a minimum set of properties such +as in the following `ChangeEmailForm`. + +[source,java,indent=0,subs="verbatim,quotes"] +---- + public class ChangeEmailForm { + + private String oldEmailAddress; + private String newEmailAddress; + + public void setOldEmailAddress(String oldEmailAddress) { + this.oldEmailAddress = oldEmailAddress; + } + + public String getOldEmailAddress() { + return this.oldEmailAddress; + } + + public void setNewEmailAddress(String newEmailAddress) { + this.newEmailAddress = newEmailAddress; + } + + public String getNewEmailAddress() { + return this.newEmailAddress; + } + + } +---- + +If you cannot or do not want to use a _dedicated model object_ for each data +binding use case, you **must** limit the properties that are allowed for data binding. +Ideally, you can achieve this by registering _allowed field patterns_ via the +`setAllowedFields()` method on `WebDataBinder`. + +For example, to register allowed field patterns in your application, you can implement an +`@InitBinder` method in a `@Controller` or `@ControllerAdvice` component as shown below: + +[source,java,indent=0,subs="verbatim,quotes"] +---- + @Controller + public class ChangeEmailController { + + @InitBinder + void initBinder(WebDataBinder binder) { + binder.setAllowedFields("oldEmailAddress", "newEmailAddress"); + } + + // @RequestMapping methods, etc. + + } +---- + +In addition to registering allowed patterns, it is also possible to register _disallowed +field patterns_ via the `setDisallowedFields()` method in `DataBinder` and its subclasses. +Please note, however, that an "allow list" is safer than a "deny list". Consequently, +`setAllowedFields()` should be favored over `setDisallowedFields()`. + +Note that matching against allowed field patterns is case-sensitive; whereas, matching +against disallowed field patterns is case-insensitive. In addition, a field matching a +disallowed pattern will not be accepted even if it also happens to match a pattern in the +allowed list. + +[WARNING] +==== +It is extremely important to properly configure allowed and disallowed field patterns +when exposing your domain model directly for data binding purposes. Otherwise, it is a +big security risk. + +Furthermore, it is strongly recommended that you do **not** use types from your domain +model such as JPA or Hibernate entities as the model object in data binding scenarios. +==== diff --git a/src/docs/asciidoc/web/webflux.adoc b/src/docs/asciidoc/web/webflux.adoc index 111980c64616..2276f15aadf9 100644 --- a/src/docs/asciidoc/web/webflux.adoc +++ b/src/docs/asciidoc/web/webflux.adoc @@ -3319,6 +3319,11 @@ controller-specific `Formatter` instances, as the following example shows: ---- <1> Adding a custom formatter (a `DateFormatter`, in this case). +[[webflux-ann-initbinder-model-design]] +==== Model Design +[.small]#<># + +include::web-data-binding-model-design.adoc[] [[webflux-ann-controller-exceptions]] diff --git a/src/docs/asciidoc/web/webmvc.adoc b/src/docs/asciidoc/web/webmvc.adoc index 705c33a5d2a7..30a3867b00b6 100644 --- a/src/docs/asciidoc/web/webmvc.adoc +++ b/src/docs/asciidoc/web/webmvc.adoc @@ -3751,6 +3751,13 @@ controller-specific `Formatter` implementations, as the following example shows: ---- <1> Defining an `@InitBinder` method on a custom formatter. +[[mvc-ann-initbinder-model-design]] +==== Model Design +[.small]#<># + +include::web-data-binding-model-design.adoc[] + + [[mvc-ann-exceptionhandler]] === Exceptions [.small]#<>#