From a8a1fc6de5be2307eb56844ac1df1a09168f2ad9 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 3 Aug 2018 14:41:33 +0300 Subject: [PATCH] Earlier processing of forwarded headers Forwarded headers are now processed before ServerWebExchange is created through ForwardedHeaderTransformer which has the same logic as the ForwardedHeaderFilter but works on the request only. ForwardedHeaderFilter is deprecated as of 5.1 but if registered it is removed from the list of filters and ForwardedHeaderTransformer is used instead. Issue: SPR-17072 --- .../reactive/ForwardedHeaderFilter.java | 105 ++---------- .../adapter/ForwardedHeaderTransformer.java | 131 ++++++++++++++ .../server/adapter/HttpWebHandlerAdapter.java | 28 +++ .../server/adapter/WebHttpHandlerBuilder.java | 70 ++++++++ .../reactive/ForwardedHeaderFilterTests.java | 161 ------------------ .../ForwardedHeaderTransformerTests.java | 112 ++++++++++++ .../adapter/WebHttpHandlerBuilderTests.java | 27 +++ .../RequestMappingIntegrationTests.java | 37 +++- src/docs/asciidoc/web/webflux.adoc | 67 +++++--- src/docs/asciidoc/web/webmvc.adoc | 37 ++-- 10 files changed, 475 insertions(+), 300 deletions(-) create mode 100644 spring-web/src/main/java/org/springframework/web/server/adapter/ForwardedHeaderTransformer.java delete mode 100644 spring-web/src/test/java/org/springframework/web/filter/reactive/ForwardedHeaderFilterTests.java create mode 100644 spring-web/src/test/java/org/springframework/web/server/adapter/ForwardedHeaderTransformerTests.java diff --git a/spring-web/src/main/java/org/springframework/web/filter/reactive/ForwardedHeaderFilter.java b/spring-web/src/main/java/org/springframework/web/filter/reactive/ForwardedHeaderFilter.java index 43c8b32f76..4416d1f2e8 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/reactive/ForwardedHeaderFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/reactive/ForwardedHeaderFilter.java @@ -16,115 +16,42 @@ package org.springframework.web.filter.reactive; -import java.net.URI; -import java.util.LinkedHashSet; -import java.util.Set; - import reactor.core.publisher.Mono; -import org.springframework.http.HttpHeaders; import org.springframework.http.server.reactive.ServerHttpRequest; -import org.springframework.lang.Nullable; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebFilter; import org.springframework.web.server.WebFilterChain; -import org.springframework.web.util.UriComponentsBuilder; +import org.springframework.web.server.adapter.ForwardedHeaderTransformer; /** - * Extract values from "Forwarded" and "X-Forwarded-*" headers, and use them to - * override {@link ServerHttpRequest#getURI()} to reflect the client-originated - * protocol and address. + * Extract values from "Forwarded" and "X-Forwarded-*" headers to override the + * request URI (i.e. {@link ServerHttpRequest#getURI()}) so it reflects the + * client-originated protocol and address. * - *

This filter can also be used in a {@link #setRemoveOnly removeOnly} mode - * where "Forwarded" and "X-Forwarded-*" headers are eliminated, and not used. + *

Alternatively if {@link #setRemoveOnly removeOnly} is set to "true", then + * "Forwarded" and "X-Forwarded-*" headers are only removed, and not used. * * @author Arjen Poutsma * @author Rossen Stoyanchev + * @deprecated as of 5.1 this filter is deprecated in favor of using + * {@link ForwardedHeaderTransformer} which can be declared as a bean with the + * name "forwardedHeaderTransformer" or registered explicitly in + * {@link org.springframework.web.server.adapter.WebHttpHandlerBuilder + * WebHttpHandlerBuilder}. * @since 5.0 * @see https://tools.ietf.org/html/rfc7239 */ -public class ForwardedHeaderFilter implements WebFilter { - - static final Set FORWARDED_HEADER_NAMES = new LinkedHashSet<>(5); - - static { - FORWARDED_HEADER_NAMES.add("Forwarded"); - FORWARDED_HEADER_NAMES.add("X-Forwarded-Host"); - FORWARDED_HEADER_NAMES.add("X-Forwarded-Port"); - FORWARDED_HEADER_NAMES.add("X-Forwarded-Proto"); - FORWARDED_HEADER_NAMES.add("X-Forwarded-Prefix"); - FORWARDED_HEADER_NAMES.add("X-Forwarded-Ssl"); - } - - - private boolean removeOnly; - - - /** - * Enables mode in which any "Forwarded" or "X-Forwarded-*" headers are - * removed only and the information in them ignored. - * @param removeOnly whether to discard and ignore forwarded headers - */ - public void setRemoveOnly(boolean removeOnly) { - this.removeOnly = removeOnly; - } - +@Deprecated +public class ForwardedHeaderFilter extends ForwardedHeaderTransformer implements WebFilter { @Override public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { ServerHttpRequest request = exchange.getRequest(); - if (!hasForwardedHeaders(request)) { - return chain.filter(exchange); - } - - ServerWebExchange mutatedExchange; - if (this.removeOnly) { - mutatedExchange = exchange.mutate().request(this::removeForwardedHeaders).build(); - } - else { - mutatedExchange = exchange.mutate() - .request(builder -> { - URI uri = UriComponentsBuilder.fromHttpRequest(request).build().toUri(); - builder.uri(uri); - String prefix = getForwardedPrefix(request); - if (prefix != null) { - builder.path(prefix + uri.getPath()); - builder.contextPath(prefix); - } - removeForwardedHeaders(builder); - }) - .build(); - } - - return chain.filter(mutatedExchange); - } - - private boolean hasForwardedHeaders(ServerHttpRequest request) { - HttpHeaders headers = request.getHeaders(); - for (String headerName : FORWARDED_HEADER_NAMES) { - if (headers.containsKey(headerName)) { - return true; - } + if (hasForwardedHeaders(request)) { + exchange = exchange.mutate().request(apply(request)).build(); } - return false; - } - - @Nullable - private static String getForwardedPrefix(ServerHttpRequest request) { - HttpHeaders headers = request.getHeaders(); - String prefix = headers.getFirst("X-Forwarded-Prefix"); - if (prefix != null) { - int endIndex = prefix.length(); - while (endIndex > 1 && prefix.charAt(endIndex - 1) == '/') { - endIndex--; - } - prefix = (endIndex != prefix.length() ? prefix.substring(0, endIndex) : prefix); - } - return prefix; - } - - private ServerHttpRequest.Builder removeForwardedHeaders(ServerHttpRequest.Builder builder) { - return builder.headers(map -> FORWARDED_HEADER_NAMES.forEach(map::remove)); + return chain.filter(exchange); } } diff --git a/spring-web/src/main/java/org/springframework/web/server/adapter/ForwardedHeaderTransformer.java b/spring-web/src/main/java/org/springframework/web/server/adapter/ForwardedHeaderTransformer.java new file mode 100644 index 0000000000..174545ecd4 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/web/server/adapter/ForwardedHeaderTransformer.java @@ -0,0 +1,131 @@ +/* + * Copyright 2002-2018 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.web.server.adapter; + +import java.net.URI; +import java.util.LinkedHashSet; +import java.util.Set; +import java.util.function.Function; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.lang.Nullable; +import org.springframework.web.util.UriComponentsBuilder; + +/** + * Extract values from "Forwarded" and "X-Forwarded-*" headers to override the + * request URI (i.e. {@link ServerHttpRequest#getURI()}) so it reflects the + * client-originated protocol and address. + * + *

Alternatively if {@link #setRemoveOnly removeOnly} is set to "true", then + * "Forwarded" and "X-Forwarded-*" headers are only removed, and not used. + * + * @author Rossen Stoyanchev + * @since 5.1 + * @see https://tools.ietf.org/html/rfc7239 + */ +public class ForwardedHeaderTransformer implements Function { + + static final Set FORWARDED_HEADER_NAMES = new LinkedHashSet<>(5); + + static { + FORWARDED_HEADER_NAMES.add("Forwarded"); + FORWARDED_HEADER_NAMES.add("X-Forwarded-Host"); + FORWARDED_HEADER_NAMES.add("X-Forwarded-Port"); + FORWARDED_HEADER_NAMES.add("X-Forwarded-Proto"); + FORWARDED_HEADER_NAMES.add("X-Forwarded-Prefix"); + FORWARDED_HEADER_NAMES.add("X-Forwarded-Ssl"); + } + + + private boolean removeOnly; + + + /** + * Enables mode in which any "Forwarded" or "X-Forwarded-*" headers are + * removed only and the information in them ignored. + * @param removeOnly whether to discard and ignore forwarded headers + */ + public void setRemoveOnly(boolean removeOnly) { + this.removeOnly = removeOnly; + } + + /** + * Whether the "remove only" mode is on. + */ + public boolean isRemoveOnly() { + return this.removeOnly; + } + + + /** + * Apply and remove, or remove Forwarded type headers. + * @param request the request + */ + @Override + public ServerHttpRequest apply(ServerHttpRequest request) { + + if (hasForwardedHeaders(request)) { + ServerHttpRequest.Builder builder = request.mutate(); + if (!this.removeOnly) { + URI uri = UriComponentsBuilder.fromHttpRequest(request).build().toUri(); + builder.uri(uri); + String prefix = getForwardedPrefix(request); + if (prefix != null) { + builder.path(prefix + uri.getPath()); + builder.contextPath(prefix); + } + } + removeForwardedHeaders(builder); + request = builder.build(); + } + + return request; + } + + /** + * Whether the request has any Forwarded headers. + * @param request the request + */ + protected boolean hasForwardedHeaders(ServerHttpRequest request) { + HttpHeaders headers = request.getHeaders(); + for (String headerName : FORWARDED_HEADER_NAMES) { + if (headers.containsKey(headerName)) { + return true; + } + } + return false; + } + + private void removeForwardedHeaders(ServerHttpRequest.Builder builder) { + builder.headers(map -> FORWARDED_HEADER_NAMES.forEach(map::remove)); + } + + @Nullable + private static String getForwardedPrefix(ServerHttpRequest request) { + HttpHeaders headers = request.getHeaders(); + String prefix = headers.getFirst("X-Forwarded-Prefix"); + if (prefix != null) { + int endIndex = prefix.length(); + while (endIndex > 1 && prefix.charAt(endIndex - 1) == '/') { + endIndex--; + } + prefix = (endIndex != prefix.length() ? prefix.substring(0, endIndex) : prefix); + } + return prefix; + } + +} diff --git a/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java b/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java index bd6bfd2d53..8495bb3e1c 100644 --- a/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java +++ b/spring-web/src/main/java/org/springframework/web/server/adapter/HttpWebHandlerAdapter.java @@ -92,6 +92,9 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa private LocaleContextResolver localeContextResolver = new AcceptHeaderLocaleContextResolver(); + @Nullable + private ForwardedHeaderTransformer forwardedHeaderTransformer; + @Nullable private ApplicationContext applicationContext; @@ -169,6 +172,27 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa return this.localeContextResolver; } + /** + * Enable processing of forwarded headers, either extracting and removing, + * or remove only. + *

By default this is not set. + * @param transformer the transformer to use + * @since 5.1 + */ + public void setForwardedHeaderTransformer(ForwardedHeaderTransformer transformer) { + Assert.notNull(transformer, "ForwardedHeaderTransformer is required"); + this.forwardedHeaderTransformer = transformer; + } + + /** + * Return the configured {@link ForwardedHeaderTransformer}. + * @since 5.1 + */ + @Nullable + public ForwardedHeaderTransformer getForwardedHeaderTransformer() { + return this.forwardedHeaderTransformer; + } + /** * Configure the {@code ApplicationContext} associated with the web application, * if it was initialized with one via @@ -208,6 +232,10 @@ public class HttpWebHandlerAdapter extends WebHandlerDecorator implements HttpHa @Override public Mono handle(ServerHttpRequest request, ServerHttpResponse response) { + if (this.forwardedHeaderTransformer != null) { + request = this.forwardedHeaderTransformer.apply(request); + } + ServerWebExchange exchange = createExchange(request, response); logExchange(exchange); diff --git a/spring-web/src/main/java/org/springframework/web/server/adapter/WebHttpHandlerBuilder.java b/spring-web/src/main/java/org/springframework/web/server/adapter/WebHttpHandlerBuilder.java index 95d3f67940..68786ea928 100644 --- a/spring-web/src/main/java/org/springframework/web/server/adapter/WebHttpHandlerBuilder.java +++ b/spring-web/src/main/java/org/springframework/web/server/adapter/WebHttpHandlerBuilder.java @@ -21,6 +21,7 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.function.Consumer; +import java.util.stream.Collectors; import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.annotation.Autowired; @@ -74,6 +75,9 @@ public final class WebHttpHandlerBuilder { /** Well-known name for the LocaleContextResolver in the bean factory. */ public static final String LOCALE_CONTEXT_RESOLVER_BEAN_NAME = "localeContextResolver"; + /** Well-known name for the ForwardedHeaderTransformer in the bean factory. */ + public static final String FORWARDED_HEADER_TRANSFORMER_BEAN_NAME = "forwardedHeaderTransformer"; + private final WebHandler webHandler; @@ -93,6 +97,9 @@ public final class WebHttpHandlerBuilder { @Nullable private LocaleContextResolver localeContextResolver; + @Nullable + private ForwardedHeaderTransformer forwardedHeaderTransformer; + /** * Private constructor to use when initialized from an ApplicationContext. @@ -114,6 +121,7 @@ public final class WebHttpHandlerBuilder { this.sessionManager = other.sessionManager; this.codecConfigurer = other.codecConfigurer; this.localeContextResolver = other.localeContextResolver; + this.forwardedHeaderTransformer = other.forwardedHeaderTransformer; } @@ -181,6 +189,22 @@ public final class WebHttpHandlerBuilder { // Fall back on default } + try { + builder.localeContextResolver( + context.getBean(LOCALE_CONTEXT_RESOLVER_BEAN_NAME, LocaleContextResolver.class)); + } + catch (NoSuchBeanDefinitionException ex) { + // Fall back on default + } + + try { + builder.forwardedHeaderTransformer( + context.getBean(FORWARDED_HEADER_TRANSFORMER_BEAN_NAME, ForwardedHeaderTransformer.class)); + } + catch (NoSuchBeanDefinitionException ex) { + // Fall back on default + } + return builder; } @@ -192,6 +216,7 @@ public final class WebHttpHandlerBuilder { public WebHttpHandlerBuilder filter(WebFilter... filters) { if (!ObjectUtils.isEmpty(filters)) { this.filters.addAll(Arrays.asList(filters)); + updateFilters(); } return this; } @@ -202,9 +227,29 @@ public final class WebHttpHandlerBuilder { */ public WebHttpHandlerBuilder filters(Consumer> consumer) { consumer.accept(this.filters); + updateFilters(); return this; } + private void updateFilters() { + + if (this.filters.isEmpty()) { + return; + } + + List filtersToUse = this.filters.stream() + .peek(filter -> { + if (filter instanceof ForwardedHeaderTransformer && this.forwardedHeaderTransformer == null) { + this.forwardedHeaderTransformer = (ForwardedHeaderTransformer) filter; + } + }) + .filter(filter -> !(filter instanceof ForwardedHeaderTransformer)) + .collect(Collectors.toList()); + + this.filters.clear(); + this.filters.addAll(filtersToUse); + } + /** * Add the given exception handler(s). * @param handlers the exception handler(s) @@ -284,11 +329,33 @@ public final class WebHttpHandlerBuilder { return (this.localeContextResolver != null); } + /** + * Configure the {@link ForwardedHeaderTransformer} for extracting and/or + * removing forwarded headers. + * @param transformer the transformer + * @since 5.1 + */ + public WebHttpHandlerBuilder forwardedHeaderTransformer(ForwardedHeaderTransformer transformer) { + this.forwardedHeaderTransformer = transformer; + return this; + } + + /** + * Whether a {@code ForwardedHeaderTransformer} is configured or not, either + * detected from an {@code ApplicationContext} or explicitly configured via + * {@link #forwardedHeaderTransformer(ForwardedHeaderTransformer)}. + * @since 5.1 + */ + public boolean hasForwardedHeaderTransformer() { + return (this.forwardedHeaderTransformer != null); + } + /** * Build the {@link HttpHandler}. */ public HttpHandler build() { + WebHandler decorated = new FilteringWebHandler(this.webHandler, this.filters); decorated = new ExceptionHandlingWebHandler(decorated, this.exceptionHandlers); @@ -302,6 +369,9 @@ public final class WebHttpHandlerBuilder { if (this.localeContextResolver != null) { adapted.setLocaleContextResolver(this.localeContextResolver); } + if (this.forwardedHeaderTransformer != null) { + adapted.setForwardedHeaderTransformer(this.forwardedHeaderTransformer); + } if (this.applicationContext != null) { adapted.setApplicationContext(this.applicationContext); } diff --git a/spring-web/src/test/java/org/springframework/web/filter/reactive/ForwardedHeaderFilterTests.java b/spring-web/src/test/java/org/springframework/web/filter/reactive/ForwardedHeaderFilterTests.java deleted file mode 100644 index 3079337168..0000000000 --- a/spring-web/src/test/java/org/springframework/web/filter/reactive/ForwardedHeaderFilterTests.java +++ /dev/null @@ -1,161 +0,0 @@ -/* - * Copyright 2002-2018 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. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.web.filter.reactive; - -import java.net.URI; -import java.time.Duration; - -import org.junit.Test; -import reactor.core.publisher.Mono; - -import org.springframework.http.HttpHeaders; -import org.springframework.http.server.reactive.ServerHttpRequest; -import org.springframework.lang.Nullable; -import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; -import org.springframework.mock.web.test.server.MockServerWebExchange; -import org.springframework.web.server.ServerWebExchange; -import org.springframework.web.server.WebFilterChain; - -import static org.junit.Assert.*; - -/** - * Unit tests for {@link ForwardedHeaderFilter}. - * @author Arjen Poutsma - * @author Rossen Stoyanchev - */ -public class ForwardedHeaderFilterTests { - - private static final String BASE_URL = "http://example.com/path"; - - - private final ForwardedHeaderFilter filter = new ForwardedHeaderFilter(); - - private final TestWebFilterChain filterChain = new TestWebFilterChain(); - - - @Test - public void removeOnly() { - - this.filter.setRemoveOnly(true); - - HttpHeaders headers = new HttpHeaders(); - headers.add("Forwarded", "for=192.0.2.60;proto=http;by=203.0.113.43"); - headers.add("X-Forwarded-Host", "example.com"); - headers.add("X-Forwarded-Port", "8080"); - headers.add("X-Forwarded-Proto", "http"); - headers.add("X-Forwarded-Prefix", "prefix"); - headers.add("X-Forwarded-Ssl", "on"); - this.filter.filter(getExchange(headers), this.filterChain).block(Duration.ZERO); - - this.filterChain.assertForwardedHeadersRemoved(); - } - - @Test - public void xForwardedHeaders() throws Exception { - HttpHeaders headers = new HttpHeaders(); - headers.add("X-Forwarded-Host", "84.198.58.199"); - headers.add("X-Forwarded-Port", "443"); - headers.add("X-Forwarded-Proto", "https"); - headers.add("foo", "bar"); - this.filter.filter(getExchange(headers), this.filterChain).block(Duration.ZERO); - - assertEquals(new URI("https://84.198.58.199/path"), this.filterChain.uri); - this.filterChain.assertForwardedHeadersRemoved(); - } - - @Test - public void forwardedHeader() throws Exception { - HttpHeaders headers = new HttpHeaders(); - headers.add("Forwarded", "host=84.198.58.199;proto=https"); - this.filter.filter(getExchange(headers), this.filterChain).block(Duration.ZERO); - - assertEquals(new URI("https://84.198.58.199/path"), this.filterChain.uri); - this.filterChain.assertForwardedHeadersRemoved(); - } - - @Test - public void xForwardedPrefix() throws Exception { - HttpHeaders headers = new HttpHeaders(); - headers.add("X-Forwarded-Prefix", "/prefix"); - this.filter.filter(getExchange(headers), this.filterChain).block(Duration.ZERO); - - assertEquals(new URI("http://example.com/prefix/path"), this.filterChain.uri); - assertEquals("/prefix/path", this.filterChain.requestPathValue); - this.filterChain.assertForwardedHeadersRemoved(); - } - - @Test - public void xForwardedPrefixTrailingSlash() throws Exception { - HttpHeaders headers = new HttpHeaders(); - headers.add("X-Forwarded-Prefix", "/prefix////"); - this.filter.filter(getExchange(headers), this.filterChain).block(Duration.ZERO); - - assertEquals(new URI("http://example.com/prefix/path"), this.filterChain.uri); - assertEquals("/prefix/path", this.filterChain.requestPathValue); - this.filterChain.assertForwardedHeadersRemoved(); - } - - private MockServerWebExchange getExchange(HttpHeaders headers) { - MockServerHttpRequest request = MockServerHttpRequest.get(BASE_URL).headers(headers).build(); - return MockServerWebExchange.from(request); - } - - - private static class TestWebFilterChain implements WebFilterChain { - - @Nullable - private HttpHeaders headers; - - @Nullable - private URI uri; - - @Nullable String requestPathValue; - - - @Nullable - public HttpHeaders getHeaders() { - return this.headers; - } - - @Nullable - public String getHeader(String name) { - assertNotNull(this.headers); - return this.headers.getFirst(name); - } - - public void assertForwardedHeadersRemoved() { - assertNotNull(this.headers); - ForwardedHeaderFilter.FORWARDED_HEADER_NAMES - .forEach(name -> assertFalse(this.headers.containsKey(name))); - } - - @Nullable - public URI getUri() { - return this.uri; - } - - @Override - public Mono filter(ServerWebExchange exchange) { - ServerHttpRequest request = exchange.getRequest(); - this.headers = request.getHeaders(); - this.uri = request.getURI(); - this.requestPathValue = request.getPath().value(); - return Mono.empty(); - } - } - -} diff --git a/spring-web/src/test/java/org/springframework/web/server/adapter/ForwardedHeaderTransformerTests.java b/spring-web/src/test/java/org/springframework/web/server/adapter/ForwardedHeaderTransformerTests.java new file mode 100644 index 0000000000..fd5a7e52c8 --- /dev/null +++ b/spring-web/src/test/java/org/springframework/web/server/adapter/ForwardedHeaderTransformerTests.java @@ -0,0 +1,112 @@ +/* + * Copyright 2002-2018 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.web.server.adapter; + +import java.net.URI; + +import org.junit.Test; + +import org.springframework.http.HttpHeaders; +import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; + +import static org.junit.Assert.*; + +/** + * Unit tests for {@link ForwardedHeaderTransformer}. + * @author Rossen Stoyanchev + */ +public class ForwardedHeaderTransformerTests { + + private static final String BASE_URL = "http://example.com/path"; + + + private final ForwardedHeaderTransformer requestMutator = new ForwardedHeaderTransformer(); + + + @Test + public void removeOnly() { + + this.requestMutator.setRemoveOnly(true); + + HttpHeaders headers = new HttpHeaders(); + headers.add("Forwarded", "for=192.0.2.60;proto=http;by=203.0.113.43"); + headers.add("X-Forwarded-Host", "example.com"); + headers.add("X-Forwarded-Port", "8080"); + headers.add("X-Forwarded-Proto", "http"); + headers.add("X-Forwarded-Prefix", "prefix"); + headers.add("X-Forwarded-Ssl", "on"); + ServerHttpRequest request = this.requestMutator.apply(getRequest(headers)); + + assertForwardedHeadersRemoved(request); + } + + @Test + public void xForwardedHeaders() throws Exception { + HttpHeaders headers = new HttpHeaders(); + headers.add("X-Forwarded-Host", "84.198.58.199"); + headers.add("X-Forwarded-Port", "443"); + headers.add("X-Forwarded-Proto", "https"); + headers.add("foo", "bar"); + ServerHttpRequest request = this.requestMutator.apply(getRequest(headers)); + + assertEquals(new URI("https://84.198.58.199/path"), request.getURI()); + assertForwardedHeadersRemoved(request); + } + + @Test + public void forwardedHeader() throws Exception { + HttpHeaders headers = new HttpHeaders(); + headers.add("Forwarded", "host=84.198.58.199;proto=https"); + ServerHttpRequest request = this.requestMutator.apply(getRequest(headers)); + + assertEquals(new URI("https://84.198.58.199/path"), request.getURI()); + assertForwardedHeadersRemoved(request); + } + + @Test + public void xForwardedPrefix() throws Exception { + HttpHeaders headers = new HttpHeaders(); + headers.add("X-Forwarded-Prefix", "/prefix"); + ServerHttpRequest request = this.requestMutator.apply(getRequest(headers)); + + assertEquals(new URI("http://example.com/prefix/path"), request.getURI()); + assertEquals("/prefix/path", request.getPath().value()); + assertForwardedHeadersRemoved(request); + } + + @Test + public void xForwardedPrefixTrailingSlash() throws Exception { + HttpHeaders headers = new HttpHeaders(); + headers.add("X-Forwarded-Prefix", "/prefix////"); + ServerHttpRequest request = this.requestMutator.apply(getRequest(headers)); + + assertEquals(new URI("http://example.com/prefix/path"), request.getURI()); + assertEquals("/prefix/path", request.getPath().value()); + assertForwardedHeadersRemoved(request); + } + + private MockServerHttpRequest getRequest(HttpHeaders headers) { + return MockServerHttpRequest.get(BASE_URL).headers(headers).build(); + } + + private void assertForwardedHeadersRemoved(ServerHttpRequest request) { + ForwardedHeaderTransformer.FORWARDED_HEADER_NAMES + .forEach(name -> assertFalse(request.getHeaders().containsKey(name))); + } + +} diff --git a/spring-web/src/test/java/org/springframework/web/server/adapter/WebHttpHandlerBuilderTests.java b/spring-web/src/test/java/org/springframework/web/server/adapter/WebHttpHandlerBuilderTests.java index 31eb78d4d3..e261647859 100644 --- a/spring-web/src/test/java/org/springframework/web/server/adapter/WebHttpHandlerBuilderTests.java +++ b/spring-web/src/test/java/org/springframework/web/server/adapter/WebHttpHandlerBuilderTests.java @@ -17,6 +17,7 @@ package org.springframework.web.server.adapter; import java.nio.charset.StandardCharsets; +import java.util.Collections; import org.junit.Test; import reactor.core.publisher.Flux; @@ -31,6 +32,7 @@ import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.http.server.reactive.HttpHandler; import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest; import org.springframework.mock.http.server.reactive.test.MockServerHttpResponse; +import org.springframework.web.filter.reactive.ForwardedHeaderFilter; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebExceptionHandler; import org.springframework.web.server.WebFilter; @@ -62,6 +64,17 @@ public class WebHttpHandlerBuilderTests { assertEquals("FilterB::FilterA", response.getBodyAsString().block(ofMillis(5000))); } + @Test + public void forwardedHeaderFilter() { + AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); + context.register(ForwardedHeaderFilterConfig.class); + context.refresh(); + + WebHttpHandlerBuilder builder = WebHttpHandlerBuilder.applicationContext(context); + builder.filters(filters -> assertEquals(Collections.emptyList(), filters)); + assertTrue(builder.hasForwardedHeaderTransformer()); + } + @Test // SPR-15074 public void orderedWebExceptionHandlerBeans() { AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(); @@ -166,6 +179,20 @@ public class WebHttpHandlerBuilderTests { } } + @Configuration + @SuppressWarnings({"unused", "deprecation"}) + static class ForwardedHeaderFilterConfig { + + @Bean + public ForwardedHeaderFilter forwardedHeaderFilter() { + return new ForwardedHeaderFilter(); + } + + @Bean + public WebHandler webHandler() { + return exchange -> Mono.error(new Exception()); + } + } @Configuration @SuppressWarnings("unused") diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingIntegrationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingIntegrationTests.java index b325ee0e08..58ae4d7361 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingIntegrationTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/RequestMappingIntegrationTests.java @@ -16,6 +16,7 @@ package org.springframework.web.reactive.result.method.annotation; +import java.net.URI; import java.time.Duration; import org.junit.Test; @@ -23,11 +24,16 @@ import org.reactivestreams.Publisher; import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpHeaders; +import org.springframework.http.RequestEntity; +import org.springframework.http.ResponseEntity; +import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.RestController; import org.springframework.web.reactive.config.EnableWebFlux; +import org.springframework.web.server.adapter.ForwardedHeaderTransformer; import static org.junit.Assert.*; @@ -46,7 +52,7 @@ public class RequestMappingIntegrationTests extends AbstractRequestMappingIntegr @Override protected ApplicationContext initApplicationContext() { AnnotationConfigApplicationContext wac = new AnnotationConfigApplicationContext(); - wac.register(WebConfig.class, TestRestController.class); + wac.register(WebConfig.class, TestRestController.class, LocalConfig.class); wac.refresh(); return wac; } @@ -62,6 +68,20 @@ public class RequestMappingIntegrationTests extends AbstractRequestMappingIntegr assertEquals(3, headers.getContentLength()); } + @Test + public void forwardedHeaders() { + + // One integration test to verify triggering of Forwarded header support. + // More fine-grained tests in ForwardedHeaderTransformerTests. + + RequestEntity request = RequestEntity + .get(URI.create("http://localhost:" + this.port + "/uri")) + .header("Forwarded", "host=84.198.58.199;proto=https") + .build(); + ResponseEntity entity = getRestTemplate().exchange(request, String.class); + assertEquals("https://84.198.58.199/uri", entity.getBody()); + } + @Test public void stream() throws Exception { String[] expected = {"0", "1", "2", "3", "4"}; @@ -84,10 +104,25 @@ public class RequestMappingIntegrationTests extends AbstractRequestMappingIntegr return "Foo"; } + @GetMapping("/uri") + public String uri(ServerHttpRequest request) { + return request.getURI().toString(); + } + @GetMapping("/stream") public Publisher stream() { return testInterval(Duration.ofMillis(50), 5); } } + + @Configuration + static class LocalConfig { + + @Bean + public ForwardedHeaderTransformer forwardedHeaderTransformer() { + return new ForwardedHeaderTransformer(); + } + } + } diff --git a/src/docs/asciidoc/web/webflux.adoc b/src/docs/asciidoc/web/webflux.adoc index 4860aa355d..a896253cc9 100644 --- a/src/docs/asciidoc/web/webflux.adoc +++ b/src/docs/asciidoc/web/webflux.adoc @@ -508,6 +508,12 @@ The table below lists the components that `WebHttpHandlerBuilder` detects: | 0..1 | The resolver for `LocaleContext` exposed through a method on `ServerWebExchange`. `AcceptHeaderLocaleContextResolver` by default. + +| "forwardedHeaderTransformer" +| `ForwardedHeaderTransformer` +| 0..1 +| For processing Forwarded type headers, either extracting and removing, or removing them only. + Not used by default. |=== @@ -553,6 +559,39 @@ parsing multipart data in full. By contrast `@RequestBody` can be used to decode content to `Flux` without collecting to a `MultiValueMap`. +[[webflux-forwarded-headers]] +==== Forwarded Headers +[.small]#<># + +As a request goes through proxies such as load balancers the host, port, and +scheme may change and that makes it a challenge to create links that point to the correct +host, port, and scheme from a client perspective. + +https://tools.ietf.org/html/rfc7239[RFC 7239] defines the "Forwarded" HTTP header +that proxies can use to provide information about the original request. There are other +non-standard headers too including "X-Forwarded-Host", "X-Forwarded-Port", +"X-Forwarded-Proto", "X-Forwarded-Ssl", and "X-Forwarded-Prefix". + +`ForwardedHeaderTransformer` is a component that modifies the host, port, and scheme of +the request, based on Forwarded headers, and then removes those headers. Simply declare +it as a bean with the name "forwardedHeaderTransformer" and it will be +<> and used. + +There are security considerations for forwarded headers since an application can't know +if the headers were added by a proxy as intended, or with a malicious client. This is why +a proxy at the boundary of trust should be configured to remove untrusted Forwarded coming +from the outside. You can also configure the `ForwardedHeaderTransformer` with +`removeOnly=true` in which case it will remove but not use the headers. + +[NOTE] +==== +In 5.1 `ForwardedHeaderFilter` was deprecated and superceded by +`ForwardedHeaderTransformer` so forwarded headers can be processed earlier, before the +exchange is created. If the filter is configured anyway, it is taken out of the list of +filters, and `ForwardedHeaderTransformer` is used instead. +==== + + [[webflux-filters]] === Filters @@ -567,34 +606,6 @@ the bean declaration or by implementing `Ordered`. The following describe the available `WebFilter` implementations: -[[webflux-filters-forwarded-headers]] -==== Forwarded Headers -[.small]#<># - -As a request goes through proxies such as load balancers the host, port, and -scheme may change presenting a challenge for applications that need to create links -to resources since the links should reflect the host, port, and scheme of the -original request as seen from a client perspective. - -https://tools.ietf.org/html/rfc7239[RFC 7239] defines the "Forwarded" HTTP header -for proxies to use to provide information about the original request. There are also -other non-standard headers in use such as "X-Forwarded-Host", "X-Forwarded-Port", -and "X-Forwarded-Proto". - -`ForwardedHeaderFilter` detects, extracts, and uses information from the "Forwarded" -header, or from "X-Forwarded-Host", "X-Forwarded-Port", and "X-Forwarded-Proto". -It wraps the request in order to overlay its host, port, and scheme and also "hides" -the forwarded headers for subsequent processing. - -Note that there are security considerations when using forwarded headers as explained -in Section 8 of RFC 7239. At the application level it is difficult to determine whether -forwarded headers can be trusted or not. This is why the network upstream should be -configured correctly to filter out untrusted forwarded headers from the outside. - -Applications that don't have a proxy and don't need to use forwarded headers can -configure the `ForwardedHeaderFilter` to remove and ignore such headers. - - [[webflux-filters-cors]] ==== CORS [.small]#<># diff --git a/src/docs/asciidoc/web/webmvc.adoc b/src/docs/asciidoc/web/webmvc.adoc index 86cdfe3213..019306fe67 100644 --- a/src/docs/asciidoc/web/webmvc.adoc +++ b/src/docs/asciidoc/web/webmvc.adoc @@ -1116,32 +1116,27 @@ available through the `ServletRequest.getParameter{asterisk}()` family of method -[[webflux-filters-forwarded-headers]] +[[filters-forwarded-headers]] === Forwarded Headers -[.small]#<># +[.small]#<># As a request goes through proxies such as load balancers the host, port, and -scheme may change presenting a challenge for applications that need to create links -to resources since the links should reflect the host, port, and scheme of the -original request as seen from a client perspective. +scheme may change and that makes it a challenge to create links that point to the correct +host, port, and scheme from a client perspective. https://tools.ietf.org/html/rfc7239[RFC 7239] defines the "Forwarded" HTTP header -for proxies to use to provide information about the original request. There are also -other non-standard headers in use such as "X-Forwarded-Host", "X-Forwarded-Port", -and "X-Forwarded-Proto". - -`ForwardedHeaderFilter` detects, extracts, and uses information from the "Forwarded" -header, or from "X-Forwarded-Host", "X-Forwarded-Port", and "X-Forwarded-Proto". -It wraps the request in order to overlay its host, port, and scheme and also "hides" -the forwarded headers for subsequent processing. - -Note that there are security considerations when using forwarded headers as explained -in Section 8 of RFC 7239. At the application level it is difficult to determine whether -forwarded headers can be trusted or not. This is why the network upstream should be -configured correctly to filter out untrusted forwarded headers from the outside. - -Applications that don't have a proxy and don't need to use forwarded headers can -configure the `ForwardedHeaderFilter` to remove and ignore such headers. +that proxies can use to provide information about the original request. There are other +non-standard headers too including "X-Forwarded-Host", "X-Forwarded-Port", +"X-Forwarded-Proto", "X-Forwarded-Ssl", and "X-Forwarded-Prefix". + +`ForwardedHeaderFilter` is a Servlet filter that modifies the host, port, and scheme of +the request, based on Forwarded headers, and then removes those headers. + +There are security considerations for forwarded headers since an application can't know +if the headers were added by a proxy as intended, or with a malicious client. This is why +a proxy at the boundary of trust should be configured to remove untrusted Forwarded coming +from the outside. You can also configure the `ForwardedHeaderFilter` with +`removeOnly=true` in which case it will remove but not use the headers. -- GitLab