Skip to content

Commit 833e750

Browse files
committed
Improve documentation and matching algorithm in data binders
1 parent d70054d commit 833e750

File tree

16 files changed

+406
-202
lines changed

16 files changed

+406
-202
lines changed

spring-beans/src/main/java/org/springframework/beans/PropertyAccessor.java

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2022 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -23,8 +23,9 @@
2323

2424
/**
2525
* Common interface for classes that can access named properties
26-
* (such as bean properties of an object or fields in an object)
27-
* Serves as base interface for {@link BeanWrapper}.
26+
* (such as bean properties of an object or fields in an object).
27+
*
28+
* <p>Serves as base interface for {@link BeanWrapper}.
2829
*
2930
* @author Juergen Hoeller
3031
* @since 1.1

spring-context/src/main/java/org/springframework/validation/DataBinder.java

+77-39
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2022 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -51,18 +51,20 @@
5151
import org.springframework.util.StringUtils;
5252

5353
/**
54-
* Binder that allows for setting property values onto a target object,
55-
* including support for validation and binding result analysis.
56-
* The binding process can be customized through specifying allowed fields,
54+
* Binder that allows for setting property values on a target object, including
55+
* support for validation and binding result analysis.
56+
*
57+
* <p>The binding process can be customized by specifying allowed field patterns,
5758
* required fields, custom editors, etc.
5859
*
59-
* <p>Note that there are potential security implications in failing to set an array
60-
* of allowed fields. In the case of HTTP form POST data for example, malicious clients
61-
* can attempt to subvert an application by supplying values for fields or properties
62-
* that do not exist on the form. In some cases this could lead to illegal data being
63-
* set on command objects <i>or their nested objects</i>. For this reason, it is
64-
* <b>highly recommended to specify the {@link #setAllowedFields allowedFields} property</b>
65-
* on the DataBinder.
60+
* <p><strong>WARNING</strong>: Data binding can lead to security issues by exposing
61+
* parts of the object graph that are not meant to be accessed or modified by
62+
* external clients. Therefore the design and use of data binding should be considered
63+
* carefully with regard to security. For more details, please refer to the dedicated
64+
* sections on data binding for
65+
* <a href="https://docs.spring.io/spring-framework/docs/current/reference/html/web.html#mvc-ann-initbinder-model-design">Spring Web MVC</a> and
66+
* <a href="https://docs.spring.io/spring-framework/docs/current/reference/html/web-reactive.html#webflux-ann-initbinder-model-design">Spring WebFlux</a>
67+
* in the reference manual.
6668
*
6769
* <p>The binding results can be examined via the {@link BindingResult} interface,
6870
* extending the {@link Errors} interface: see the {@link #getBindingResult()} method.
@@ -96,6 +98,7 @@
9698
* @author Rob Harrop
9799
* @author Stephane Nicoll
98100
* @author Kazuki Shimizu
101+
* @author Sam Brannen
99102
* @see #setAllowedFields
100103
* @see #setRequiredFields
101104
* @see #registerCustomEditor
@@ -415,13 +418,21 @@ public boolean isIgnoreInvalidFields() {
415418
}
416419

417420
/**
418-
* Register fields that should be allowed for binding. Default is all
419-
* fields. Restrict this for example to avoid unwanted modifications
420-
* by malicious users when binding HTTP request parameters.
421-
* <p>Supports "xxx*", "*xxx" and "*xxx*" patterns. More sophisticated matching
422-
* can be implemented by overriding the {@code isAllowed} method.
423-
* <p>Alternatively, specify a list of <i>disallowed</i> fields.
424-
* @param allowedFields array of field names
421+
* Register field patterns that should be allowed for binding.
422+
* <p>Default is all fields.
423+
* <p>Restrict this for example to avoid unwanted modifications by malicious
424+
* users when binding HTTP request parameters.
425+
* <p>Supports {@code "xxx*"}, {@code "*xxx"}, {@code "*xxx*"}, and
426+
* {@code "xxx*yyy"} matches (with an arbitrary number of pattern parts), as
427+
* well as direct equality.
428+
* <p>The default implementation of this method stores allowed field patterns
429+
* in {@linkplain PropertyAccessorUtils#canonicalPropertyName(String) canonical}
430+
* form. Subclasses which override this method must therefore take this into
431+
* account.
432+
* <p>More sophisticated matching can be implemented by overriding the
433+
* {@link #isAllowed} method.
434+
* <p>Alternatively, specify a list of <i>disallowed</i> field patterns.
435+
* @param allowedFields array of allowed field patterns
425436
* @see #setDisallowedFields
426437
* @see #isAllowed(String)
427438
*/
@@ -430,32 +441,54 @@ public void setAllowedFields(@Nullable String... allowedFields) {
430441
}
431442

432443
/**
433-
* Return the fields that should be allowed for binding.
434-
* @return array of field names
444+
* Return the field patterns that should be allowed for binding.
445+
* @return array of allowed field patterns
446+
* @see #setAllowedFields(String...)
435447
*/
436448
@Nullable
437449
public String[] getAllowedFields() {
438450
return this.allowedFields;
439451
}
440452

441453
/**
442-
* Register fields that should <i>not</i> be allowed for binding. Default is none.
443-
* Mark fields as disallowed for example to avoid unwanted modifications
444-
* by malicious users when binding HTTP request parameters.
445-
* <p>Supports "xxx*", "*xxx" and "*xxx*" patterns. More sophisticated matching
446-
* can be implemented by overriding the {@code isAllowed} method.
447-
* <p>Alternatively, specify a list of <i>allowed</i> fields.
448-
* @param disallowedFields array of field names
454+
* Register field patterns that should <i>not</i> be allowed for binding.
455+
* <p>Default is none.
456+
* <p>Mark fields as disallowed, for example to avoid unwanted
457+
* modifications by malicious users when binding HTTP request parameters.
458+
* <p>Supports {@code "xxx*"}, {@code "*xxx"}, {@code "*xxx*"}, and
459+
* {@code "xxx*yyy"} matches (with an arbitrary number of pattern parts), as
460+
* well as direct equality.
461+
* <p>The default implementation of this method stores disallowed field patterns
462+
* in {@linkplain PropertyAccessorUtils#canonicalPropertyName(String) canonical}
463+
* form. As of Spring Framework 5.2.21, the default implementation also transforms
464+
* disallowed field patterns to {@linkplain String#toLowerCase() lowercase} to
465+
* support case-insensitive pattern matching in {@link #isAllowed}. Subclasses
466+
* which override this method must therefore take both of these transformations
467+
* into account.
468+
* <p>More sophisticated matching can be implemented by overriding the
469+
* {@link #isAllowed} method.
470+
* <p>Alternatively, specify a list of <i>allowed</i> field patterns.
471+
* @param disallowedFields array of disallowed field patterns
449472
* @see #setAllowedFields
450473
* @see #isAllowed(String)
451474
*/
452475
public void setDisallowedFields(@Nullable String... disallowedFields) {
453-
this.disallowedFields = PropertyAccessorUtils.canonicalPropertyNames(disallowedFields);
476+
if (disallowedFields == null) {
477+
this.disallowedFields = null;
478+
}
479+
else {
480+
String[] fieldPatterns = new String[disallowedFields.length];
481+
for (int i = 0; i < fieldPatterns.length; i++) {
482+
fieldPatterns[i] = PropertyAccessorUtils.canonicalPropertyName(disallowedFields[i]).toLowerCase();
483+
}
484+
this.disallowedFields = fieldPatterns;
485+
}
454486
}
455487

456488
/**
457-
* Return the fields that should <i>not</i> be allowed for binding.
458-
* @return array of field names
489+
* Return the field patterns that should <i>not</i> be allowed for binding.
490+
* @return array of disallowed field patterns
491+
* @see #setDisallowedFields(String...)
459492
*/
460493
@Nullable
461494
public String[] getDisallowedFields() {
@@ -767,15 +800,20 @@ protected void checkAllowedFields(MutablePropertyValues mpvs) {
767800
}
768801

769802
/**
770-
* Return if the given field is allowed for binding.
771-
* Invoked for each passed-in property value.
772-
* <p>The default implementation checks for "xxx*", "*xxx" and "*xxx*" matches,
773-
* as well as direct equality, in the specified lists of allowed fields and
774-
* disallowed fields. A field matching a disallowed pattern will not be accepted
775-
* even if it also happens to match a pattern in the allowed list.
776-
* <p>Can be overridden in subclasses.
803+
* Determine if the given field is allowed for binding.
804+
* <p>Invoked for each passed-in property value.
805+
* <p>Checks for {@code "xxx*"}, {@code "*xxx"}, {@code "*xxx*"}, and
806+
* {@code "xxx*yyy"} matches (with an arbitrary number of pattern parts), as
807+
* well as direct equality, in the configured lists of allowed field patterns
808+
* and disallowed field patterns.
809+
* <p>Matching against allowed field patterns is case-sensitive; whereas,
810+
* matching against disallowed field patterns is case-insensitive.
811+
* <p>A field matching a disallowed pattern will not be accepted even if it
812+
* also happens to match a pattern in the allowed list.
813+
* <p>Can be overridden in subclasses, but care must be taken to honor the
814+
* aforementioned contract.
777815
* @param field the field to check
778-
* @return if the field is allowed
816+
* @return {@code true} if the field is allowed
779817
* @see #setAllowedFields
780818
* @see #setDisallowedFields
781819
* @see org.springframework.util.PatternMatchUtils#simpleMatch(String, String)
@@ -784,7 +822,7 @@ protected boolean isAllowed(String field) {
784822
String[] allowed = getAllowedFields();
785823
String[] disallowed = getDisallowedFields();
786824
return ((ObjectUtils.isEmpty(allowed) || PatternMatchUtils.simpleMatch(allowed, field)) &&
787-
(ObjectUtils.isEmpty(disallowed) || !PatternMatchUtils.simpleMatch(disallowed, field)));
825+
(ObjectUtils.isEmpty(disallowed) || !PatternMatchUtils.simpleMatch(disallowed, field.toLowerCase())));
788826
}
789827

790828
/**

0 commit comments

Comments
 (0)