Skip to content

Commit

Permalink
Fix filtered HTTP headers in data binding
Browse files Browse the repository at this point in the history
Prior to this commit, several common HTTP headers were ignored from the
data binding process when collecting property values, in gh-34039 and
gh-34182.

This commit completes the initial enhancement by ensuring that the
default header predicate is also considering cases where constructor
binding is applied and the Java type has a lowercase variant of the HTTP
header name to filter.

Fixes gh-34292
  • Loading branch information
bclozel committed Jan 29, 2025
1 parent 7c5b6f1 commit d80de04
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.springframework.web.reactive.result.method.annotation;

import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.function.Predicate;
Expand All @@ -43,11 +44,11 @@
*/
public class ExtendedWebExchangeDataBinder extends WebExchangeDataBinder {

private static final Set<String> FILTERED_HEADER_NAMES = Set.of("Accept", "Authorization", "Connection",
"Cookie", "From", "Host", "Origin", "Priority", "Range", "Referer", "Upgrade");
private static final Set<String> FILTERED_HEADER_NAMES = Set.of("accept", "authorization", "connection",
"cookie", "from", "host", "origin", "priority", "range", "referer", "upgrade");


private Predicate<String> headerPredicate = name -> !FILTERED_HEADER_NAMES.contains(name);
private Predicate<String> headerPredicate = name -> !FILTERED_HEADER_NAMES.contains(name.toLowerCase(Locale.ROOT));


public ExtendedWebExchangeDataBinder(@Nullable Object target, String objectName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ void headerPredicate() throws Exception {

@ParameterizedTest
@ValueSource(strings = {"Accept", "Authorization", "Connection",
"Cookie", "From", "Host", "Origin", "Priority", "Range", "Referer", "Upgrade"})
"Cookie", "From", "Host", "Origin", "Priority", "Range", "Referer", "Upgrade", "priority"})
void filteredHeaders(String headerName) throws Exception {
MockServerHttpRequest request = MockServerHttpRequest.get("/path")
.header(headerName, "u1")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.function.Predicate;
Expand All @@ -39,7 +40,7 @@
*
* <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
* 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
Expand All @@ -53,11 +54,11 @@
*/
public class ExtendedServletRequestDataBinder extends ServletRequestDataBinder {

private static final Set<String> FILTERED_HEADER_NAMES = Set.of("Accept", "Authorization", "Connection",
"Cookie", "From", "Host", "Origin", "Priority", "Range", "Referer", "Upgrade");
private static final Set<String> FILTERED_HEADER_NAMES = Set.of("accept", "authorization", "connection",
"cookie", "from", "host", "origin", "priority", "range", "referer", "upgrade");


private Predicate<String> headerPredicate = name -> !FILTERED_HEADER_NAMES.contains(name);
private Predicate<String> headerPredicate = name -> !FILTERED_HEADER_NAMES.contains(name.toLowerCase(Locale.ROOT));


/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.springframework.beans.MutablePropertyValues;
import org.springframework.beans.testfixture.beans.TestBean;
import org.springframework.core.ResolvableType;
import org.springframework.validation.BindingResult;
import org.springframework.web.bind.ServletRequestDataBinder;
import org.springframework.web.bind.annotation.BindParam;
import org.springframework.web.bind.support.BindParamNameResolver;
Expand All @@ -36,7 +37,7 @@
import static org.assertj.core.api.Assertions.assertThat;

/**
* Test fixture for {@link ExtendedServletRequestDataBinder}.
* Tests for {@link ExtendedServletRequestDataBinder}.
*
* @author Rossen Stoyanchev
*/
Expand Down Expand Up @@ -136,6 +137,19 @@ void headerPredicateWithConstructorArgs() {
assertThat(bean.someIntArray()).isNull();
}

@Test
void filteredPriorityHeaderForConstructorBinding() {
TestBinder binder = new TestBinder();
binder.setTargetType(ResolvableType.forClass(TestTarget.class));
request.addHeader("Priority", "u1");

binder.construct(request);
BindingResult result = binder.getBindingResult();
TestTarget target = (TestTarget) result.getTarget();

assertThat(target.priority).isNull();
}

@Test
void headerPredicate() {
TestBinder binder = new TestBinder();
Expand Down Expand Up @@ -179,4 +193,14 @@ public void addBindValues(MutablePropertyValues mpvs, ServletRequest request) {
}
}

static class TestTarget {

final String priority;

public TestTarget(String priority) {
this.priority = priority;
}

}

}

0 comments on commit d80de04

Please sign in to comment.