From d80de043ce20bd2362f79a04b97d1cf230673e0c Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 29 Jan 2025 16:06:19 +0100 Subject: [PATCH] Fix filtered HTTP headers in data binding 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 --- .../ExtendedWebExchangeDataBinder.java | 7 ++--- .../InitBinderBindingContextTests.java | 2 +- .../ExtendedServletRequestDataBinder.java | 9 ++++--- ...ExtendedServletRequestDataBinderTests.java | 26 ++++++++++++++++++- 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ExtendedWebExchangeDataBinder.java b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ExtendedWebExchangeDataBinder.java index 8ef5f3021e..c807adaaa9 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ExtendedWebExchangeDataBinder.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/annotation/ExtendedWebExchangeDataBinder.java @@ -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; @@ -43,11 +44,11 @@ import org.springframework.web.server.ServerWebExchange; */ public class ExtendedWebExchangeDataBinder extends WebExchangeDataBinder { - private static final Set FILTERED_HEADER_NAMES = Set.of("Accept", "Authorization", "Connection", - "Cookie", "From", "Host", "Origin", "Priority", "Range", "Referer", "Upgrade"); + private static final Set FILTERED_HEADER_NAMES = Set.of("accept", "authorization", "connection", + "cookie", "from", "host", "origin", "priority", "range", "referer", "upgrade"); - private Predicate headerPredicate = name -> !FILTERED_HEADER_NAMES.contains(name); + private Predicate headerPredicate = name -> !FILTERED_HEADER_NAMES.contains(name.toLowerCase(Locale.ROOT)); public ExtendedWebExchangeDataBinder(@Nullable Object target, String objectName) { 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 6e2ed7b811..0492b75859 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 @@ -224,7 +224,7 @@ class InitBinderBindingContextTests { @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") 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 b0acd640ae..568ed2d4d5 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 @@ -19,6 +19,7 @@ package org.springframework.web.servlet.mvc.method.annotation; 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; @@ -39,7 +40,7 @@ import org.springframework.web.servlet.HandlerMapping; * *

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 + * 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 @@ -53,11 +54,11 @@ import org.springframework.web.servlet.HandlerMapping; */ public class ExtendedServletRequestDataBinder extends ServletRequestDataBinder { - private static final Set FILTERED_HEADER_NAMES = Set.of("Accept", "Authorization", "Connection", - "Cookie", "From", "Host", "Origin", "Priority", "Range", "Referer", "Upgrade"); + private static final Set FILTERED_HEADER_NAMES = Set.of("accept", "authorization", "connection", + "cookie", "from", "host", "origin", "priority", "range", "referer", "upgrade"); - private Predicate headerPredicate = name -> !FILTERED_HEADER_NAMES.contains(name); + private Predicate headerPredicate = name -> !FILTERED_HEADER_NAMES.contains(name.toLowerCase(Locale.ROOT)); /** diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinderTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinderTests.java index 3164fba945..e049a5ca76 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinderTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExtendedServletRequestDataBinderTests.java @@ -27,6 +27,7 @@ import org.junit.jupiter.params.provider.ValueSource; 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; @@ -36,7 +37,7 @@ import org.springframework.web.testfixture.servlet.MockHttpServletRequest; import static org.assertj.core.api.Assertions.assertThat; /** - * Test fixture for {@link ExtendedServletRequestDataBinder}. + * Tests for {@link ExtendedServletRequestDataBinder}. * * @author Rossen Stoyanchev */ @@ -136,6 +137,19 @@ class ExtendedServletRequestDataBinderTests { 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(); @@ -179,4 +193,14 @@ class ExtendedServletRequestDataBinderTests { } } + static class TestTarget { + + final String priority; + + public TestTarget(String priority) { + this.priority = priority; + } + + } + }