Skip to content

Commit

Permalink
Improve documentation and matching algorithm in data binders
Browse files Browse the repository at this point in the history
  • Loading branch information
@sbrannen
sbrannen committed Apr 13, 2022
1 parent 0cf7f7b commit a7cf19c
Show file tree
Hide file tree
Showing 16 changed files with 402 additions and 203 deletions.
7 changes: 4 additions & 3 deletions spring-beans/src/main/java/org/springframework/beans/PropertyAccessor.java
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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).
*
* <p>Serves as base interface for {@link BeanWrapper}.
*
* @author Juergen Hoeller
* @since 1.1
Expand Down
117 changes: 75 additions & 42 deletions spring-context/src/main/java/org/springframework/validation/DataBinder.java
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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.
*
* <p>The binding process can be customized by specifying allowed field patterns,
* required fields, custom editors, etc.
*
* <p>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 <i>or their nested objects</i>. For this reason, it is
* <b>highly recommended to specify the {@link #setAllowedFields allowedFields} property</b>
* on the DataBinder.
* <p><strong>WARNING</strong>: 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
* <a href="https://docs.spring.io/spring-framework/docs/current/reference/html/web.html#mvc-ann-initbinder-model-design">Spring Web MVC</a> and
* <a href="https://docs.spring.io/spring-framework/docs/current/reference/html/web-reactive.html#webflux-ann-initbinder-model-design">Spring WebFlux</a>
* in the reference manual.
*
* <p>The binding results can be examined via the {@link BindingResult} interface,
* extending the {@link Errors} interface: see the {@link #getBindingResult()} method.
Expand Down Expand Up @@ -96,6 +98,7 @@
* @author Rob Harrop
* @author Stephane Nicoll
* @author Kazuki Shimizu
* @author Sam Brannen
* @see #setAllowedFields
* @see #setRequiredFields
* @see #registerCustomEditor
Expand Down Expand Up @@ -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.
* <p>Default is all fields.
* <p>Restrict this for example to avoid unwanted modifications by malicious
* users when binding HTTP request parameters.
* <p>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.
* <p>Alternatively, specify a list of <i>disallowed</i> fields.
* @param allowedFields array of field names
* <p>Supports {@code "xxx*"}, {@code "*xxx"}, {@code "*xxx*"}, and
* {@code "xxx*yyy"} matches (with an arbitrary number of pattern parts), as
* well as direct equality.
* <p>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.
* <p>More sophisticated matching can be implemented by overriding the
* {@link #isAllowed} method.
* <p>Alternatively, specify a list of <i>disallowed</i> field patterns.
* @param allowedFields array of allowed field patterns
* @see #setDisallowedFields
* @see #isAllowed(String)
*/
Expand All @@ -435,34 +444,54 @@ 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() {
return this.allowedFields;
}

/**
* Register fields that should <i>not</i> be allowed for binding. Default
* is none. Mark fields as disallowed for example to avoid unwanted
* Register field patterns that should <i>not</i> be allowed for binding.
* <p>Default is none.
* <p>Mark fields as disallowed, for example to avoid unwanted
* modifications by malicious users when binding HTTP request parameters.
* <p>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.
* <p>Alternatively, specify a list of <i>allowed</i> fields.
* @param disallowedFields array of field names
* <p>Supports {@code "xxx*"}, {@code "*xxx"}, {@code "*xxx*"}, and
* {@code "xxx*yyy"} matches (with an arbitrary number of pattern parts), as
* well as direct equality.
* <p>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.
* <p>More sophisticated matching can be implemented by overriding the
* {@link #isAllowed} method.
* <p>Alternatively, specify a list of <i>allowed</i> 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 <i>not</i> be allowed for binding.
* @return array of field names
* Return the field patterns that should <i>not</i> be allowed for binding.
* @return array of disallowed field patterns
* @see #setDisallowedFields(String...)
*/
@Nullable
public String[] getDisallowedFields() {
Expand Down Expand Up @@ -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.
* <p>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.
* <p>Can be overridden in subclasses.
* Determine if the given field is allowed for binding.
* <p>Invoked for each passed-in property value.
* <p>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.
* <p>Matching against allowed field patterns is case-sensitive; whereas,
* matching against disallowed field patterns is case-insensitive.
* <p>A field matching a disallowed pattern will not be accepted even if it
* also happens to match a pattern in the allowed list.
* <p>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)
Expand All @@ -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())));
}

/**
Expand Down
Loading

0 comments on commit a7cf19c

Please sign in to comment.