Remove individual detection of forwarded headers

This commit removes all places where forwarded headers are checked
implicitly, on an ad-hoc basis.

ForwardedHeaderFilter is expected to be used instead providing
centralized control over using or discarding such headers.

Issue: SPR-16668

Author: rstoyanchev

spring-test/src/main/java/org/springframework/mock/web/reactive/function/server/MockServerRequest.java

@@ -38,7 +38,6 @@
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpRange;
-import org.springframework.http.HttpRequest;
 import org.springframework.http.MediaType;
 import org.springframework.http.codec.HttpMessageReader;
 import org.springframework.http.codec.multipart.Part;
@@ -141,7 +140,7 @@ public URI uri() {
 
 	@Override
 	public UriBuilder uriBuilder() {
-		return UriComponentsBuilder.fromHttpRequest(new ServerRequestAdapter());
+		return UriComponentsBuilder.fromUri(this.uri);
 	}
 
 	@Override
@@ -571,22 +570,4 @@ private OptionalLong toOptionalLong(long value) {
 
 	}
 
-	private final class ServerRequestAdapter implements HttpRequest {
-
-		@Override
-		public String getMethodValue() {
-			return methodName();
-		}
-
-		@Override
-		public URI getURI() {
-			return MockServerRequest.this.uri;
-		}
-
-		@Override
-		public HttpHeaders getHeaders() {
-			return MockServerRequest.this.headers.headers;
-		}
-	}
-
 }

spring-web/src/main/java/org/springframework/web/cors/reactive/CorsUtils.java

@@ -16,6 +16,8 @@
 
 package org.springframework.web.cors.reactive;
 
+import java.net.URI;
+
 import org.springframework.http.HttpHeaders;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.server.reactive.ServerHttpRequest;
@@ -49,28 +51,26 @@ public static boolean isPreFlightRequest(ServerHttpRequest request) {
 	}
 
 	/**
-	 * Check if the request is a same-origin one, based on {@code Origin}, {@code Host},
-	 * {@code Forwarded}, {@code X-Forwarded-Proto}, {@code X-Forwarded-Host} and
-	 * @code X-Forwarded-Port} headers.
+	 * Check if the request is a same-origin one, based on {@code Origin}, and
+	 * {@code Host} headers.
+	 *
+	 * <p><strong>Note:</strong> as of 5.1 this method ignores
+	 * {@code "Forwarded"} and {@code "X-Forwarded-*"} headers that specify the
+	 * client-originated address. Consider using the {@code ForwardedHeaderFilter}
+	 * to extract and use, or to discard such headers.
+	 *
 	 * @return {@code true} if the request is a same-origin one, {@code false} in case
 	 * of a cross-origin request
-	 * <p><strong>Note:</strong> this method uses values from "Forwarded"
-	 * (<a href="http://tools.ietf.org/html/rfc7239">RFC 7239</a>),
-	 * "X-Forwarded-Host", "X-Forwarded-Port", and "X-Forwarded-Proto" headers,
-	 * if present, in order to reflect the client-originated address.
-	 * Consider using the {@code ForwardedHeaderFilter} in order to choose from a
-	 * central place whether to extract and use, or to discard such headers.
-	 * See the Spring Framework reference for more on this filter.
 	 */
 	public static boolean isSameOrigin(ServerHttpRequest request) {
 		String origin = request.getHeaders().getOrigin();
 		if (origin == null) {
 			return true;
 		}
 
-		UriComponents actualUrl = UriComponentsBuilder.fromHttpRequest(request).build();
-		String actualHost = actualUrl.getHost();
-		int actualPort = getPort(actualUrl.getScheme(), actualUrl.getPort());
+		URI uri = request.getURI();
+		String actualHost = uri.getHost();
+		int actualPort = getPort(uri.getScheme(), uri.getPort());
 		Assert.notNull(actualHost, "Actual request host must not be null");
 		Assert.isTrue(actualPort != -1, "Actual request port must not be undefined");
 

spring-web/src/main/java/org/springframework/web/util/WebUtils.java

@@ -18,11 +18,10 @@
 
 import java.io.File;
 import java.io.FileNotFoundException;
+import java.net.URI;
 import java.util.Collection;
 import java.util.Enumeration;
-import java.util.LinkedHashSet;
 import java.util.Map;
-import java.util.Set;
 import java.util.StringTokenizer;
 import java.util.TreeMap;
 import javax.servlet.ServletContext;
@@ -138,16 +137,6 @@ public abstract class WebUtils {
 	/** Key for the mutex session attribute */
 	public static final String SESSION_MUTEX_ATTRIBUTE = WebUtils.class.getName() + ".MUTEX";
 
-	private static final Set<String> 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");
-	}
-
 
 	/**
 	 * Set a system property to the web application root directory.
@@ -677,13 +666,12 @@ public static MultiValueMap<String, String> parseMatrixVariables(String matrixVa
 	 * Check the given request origin against a list of allowed origins.
 	 * A list containing "*" means that all origins are allowed.
 	 * An empty list means only same origin is allowed.
-	 * <p><strong>Note:</strong> this method may use values from "Forwarded"
-	 * (<a href="http://tools.ietf.org/html/rfc7239">RFC 7239</a>),
-	 * "X-Forwarded-Host", "X-Forwarded-Port", and "X-Forwarded-Proto" headers,
-	 * if present, in order to reflect the client-originated address.
-	 * Consider using the {@code ForwardedHeaderFilter} in order to choose from a
-	 * central place whether to extract and use, or to discard such headers.
-	 * See the Spring Framework reference for more on this filter.
+	 *
+	 * <p><strong>Note:</strong> as of 5.1 this method ignores
+	 * {@code "Forwarded"} and {@code "X-Forwarded-*"} headers that specify the
+	 * client-originated address. Consider using the {@code ForwardedHeaderFilter}
+	 * to extract and use, or to discard such headers.
+	 *
 	 * @return {@code true} if the request origin is valid, {@code false} otherwise
 	 * @since 4.1.5
 	 * @see <a href="https://tools.ietf.org/html/rfc6454">RFC 6454: The Web Origin Concept</a>
@@ -708,13 +696,12 @@ else if (CollectionUtils.isEmpty(allowedOrigins)) {
 	 * Check if the request is a same-origin one, based on {@code Origin}, {@code Host},
 	 * {@code Forwarded}, {@code X-Forwarded-Proto}, {@code X-Forwarded-Host} and
 	 * @code X-Forwarded-Port} headers.
-	 * <p><strong>Note:</strong> this method uses values from "Forwarded"
-	 * (<a href="http://tools.ietf.org/html/rfc7239">RFC 7239</a>),
-	 * "X-Forwarded-Host", "X-Forwarded-Port", and "X-Forwarded-Proto" headers,
-	 * if present, in order to reflect the client-originated address.
-	 * Consider using the {@code ForwardedHeaderFilter} in order to choose from a
-	 * central place whether to extract and use, or to discard such headers.
-	 * See the Spring Framework reference for more on this filter.
+	 *
+	 * <p><strong>Note:</strong> as of 5.1 this method ignores
+	 * {@code "Forwarded"} and {@code "X-Forwarded-*"} headers that specify the
+	 * client-originated address. Consider using the {@code ForwardedHeaderFilter}
+	 * to extract and use, or to discard such headers.
+
 	 * @return {@code true} if the request is a same-origin one, {@code false} in case
 	 * of cross-origin request
 	 * @since 4.2
@@ -735,37 +722,19 @@ public static boolean isSameOrigin(HttpRequest request) {
 			scheme = servletRequest.getScheme();
 			host = servletRequest.getServerName();
 			port = servletRequest.getServerPort();
-			if (containsForwardedHeaders(servletRequest)) {
-				UriComponents actualUrl = new UriComponentsBuilder()
-						.scheme(scheme).host(host).port(port)
-						.adaptFromForwardedHeaders(headers)
-						.build();
-				scheme = actualUrl.getScheme();
-				host = actualUrl.getHost();
-				port = actualUrl.getPort();
-			}
 		}
 		else {
-			UriComponents actualUrl = UriComponentsBuilder.fromHttpRequest(request).build();
-			scheme = actualUrl.getScheme();
-			host = actualUrl.getHost();
-			port = actualUrl.getPort();
+			URI uri = request.getURI();
+			scheme = uri.getScheme();
+			host = uri.getHost();
+			port = uri.getPort();
 		}
 
 		UriComponents originUrl = UriComponentsBuilder.fromOriginHeader(origin).build();
 		return (ObjectUtils.nullSafeEquals(host, originUrl.getHost()) &&
 				getPort(scheme, port) == getPort(originUrl.getScheme(), originUrl.getPort()));
 	}
 
-	private static boolean containsForwardedHeaders(HttpServletRequest request) {
-		for (String headerName : FORWARDED_HEADER_NAMES) {
-			if (request.getHeader(headerName) != null) {
-				return true;
-			}
-		}
-		return false;
-	}
-
 	private static int getPort(@Nullable String scheme, int port) {
 		if (port == -1) {
 			if ("http".equals(scheme) || "ws".equals(scheme)) {

spring-web/src/test/java/org/springframework/web/cors/reactive/CorsUtilsTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * 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.
@@ -16,15 +16,19 @@
 
 package org.springframework.web.cors.reactive;
 
+import java.util.concurrent.atomic.AtomicReference;
+
 import org.junit.Test;
+import reactor.core.publisher.Mono;
 
 import org.springframework.http.HttpHeaders;
+import org.springframework.http.server.reactive.ServerHttpRequest;
 import org.springframework.mock.http.server.reactive.test.MockServerHttpRequest;
+import org.springframework.mock.web.test.server.MockServerWebExchange;
+import org.springframework.web.filter.reactive.ForwardedHeaderFilter;
 
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.get;
-import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.options;
+import static org.junit.Assert.*;
+import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.*;
 
 /**
  * Test case for reactive {@link CorsUtils}.
@@ -35,19 +39,19 @@ public class CorsUtilsTests {
 
 	@Test
 	public void isCorsRequest() {
-		MockServerHttpRequest request = get("/").header(HttpHeaders.ORIGIN, "http://domain.com").build();
+		ServerHttpRequest request = get("/").header(HttpHeaders.ORIGIN, "http://domain.com").build();
 		assertTrue(CorsUtils.isCorsRequest(request));
 	}
 
 	@Test
 	public void isNotCorsRequest() {
-		MockServerHttpRequest request = get("/").build();
+		ServerHttpRequest request = get("/").build();
 		assertFalse(CorsUtils.isCorsRequest(request));
 	}
 
 	@Test
 	public void isPreFlightRequest() {
-		MockServerHttpRequest request = options("/")
+		ServerHttpRequest request = options("/")
 				.header(HttpHeaders.ORIGIN, "http://domain.com")
 				.header(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET")
 				.build();
@@ -56,7 +60,7 @@ public void isPreFlightRequest() {
 
 	@Test
 	public void isNotPreFlightRequest() {
-		MockServerHttpRequest request = get("/").build();
+		ServerHttpRequest request = get("/").build();
 		assertFalse(CorsUtils.isPreFlightRequest(request));
 
 		request = options("/").header(HttpHeaders.ORIGIN, "http://domain.com").build();
@@ -68,31 +72,35 @@ public void isNotPreFlightRequest() {
 
 	@Test  // SPR-16262
 	public void isSameOriginWithXForwardedHeaders() {
-		assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", -1, "https", null, -1, "https://mydomain1.com"));
-		assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", 123, "https", null, -1, "https://mydomain1.com"));
-		assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", -1, "https", "mydomain2.com", -1, "https://mydomain2.com"));
-		assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", 123, "https", "mydomain2.com", -1, "https://mydomain2.com"));
-		assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", -1, "https", "mydomain2.com", 456, "https://mydomain2.com:456"));
-		assertTrue(checkSameOriginWithXForwardedHeaders("mydomain1.com", 123, "https", "mydomain2.com", 456, "https://mydomain2.com:456"));
+		String server = "mydomain1.com";
+		testWithXForwardedHeaders(server, -1, "https", null, -1, "https://mydomain1.com");
+		testWithXForwardedHeaders(server, 123, "https", null, -1, "https://mydomain1.com");
+		testWithXForwardedHeaders(server, -1, "https", "mydomain2.com", -1, "https://mydomain2.com");
+		testWithXForwardedHeaders(server, 123, "https", "mydomain2.com", -1, "https://mydomain2.com");
+		testWithXForwardedHeaders(server, -1, "https", "mydomain2.com", 456, "https://mydomain2.com:456");
+		testWithXForwardedHeaders(server, 123, "https", "mydomain2.com", 456, "https://mydomain2.com:456");
 	}
 
 	@Test  // SPR-16262
 	public void isSameOriginWithForwardedHeader() {
-		assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", -1, "proto=https", "https://mydomain1.com"));
-		assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", 123, "proto=https", "https://mydomain1.com"));
-		assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", -1, "proto=https; host=mydomain2.com", "https://mydomain2.com"));
-		assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", 123, "proto=https; host=mydomain2.com", "https://mydomain2.com"));
-		assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", -1, "proto=https; host=mydomain2.com:456", "https://mydomain2.com:456"));
-		assertTrue(checkSameOriginWithForwardedHeader("mydomain1.com", 123, "proto=https; host=mydomain2.com:456", "https://mydomain2.com:456"));
+		String server = "mydomain1.com";
+		testWithForwardedHeader(server, -1, "proto=https", "https://mydomain1.com");
+		testWithForwardedHeader(server, 123, "proto=https", "https://mydomain1.com");
+		testWithForwardedHeader(server, -1, "proto=https; host=mydomain2.com", "https://mydomain2.com");
+		testWithForwardedHeader(server, 123, "proto=https; host=mydomain2.com", "https://mydomain2.com");
+		testWithForwardedHeader(server, -1, "proto=https; host=mydomain2.com:456", "https://mydomain2.com:456");
+		testWithForwardedHeader(server, 123, "proto=https; host=mydomain2.com:456", "https://mydomain2.com:456");
 	}
 
-	private boolean checkSameOriginWithXForwardedHeaders(String serverName, int port, String forwardedProto, String forwardedHost, int forwardedPort, String originHeader) {
+	private void testWithXForwardedHeaders(String serverName, int port,
+			String forwardedProto, String forwardedHost, int forwardedPort, String originHeader) {
+
 		String url = "http://" + serverName;
 		if (port != -1) {
 			url = url + ":" + port;
 		}
-		MockServerHttpRequest.BaseBuilder<?> builder = get(url)
-				.header(HttpHeaders.ORIGIN, originHeader);
+
+		MockServerHttpRequest.BaseBuilder<?> builder = get(url).header(HttpHeaders.ORIGIN, originHeader);
 		if (forwardedProto != null) {
 			builder.header("X-Forwarded-Proto", forwardedProto);
 		}
@@ -102,18 +110,36 @@ private boolean checkSameOriginWithXForwardedHeaders(String serverName, int port
 		if (forwardedPort != -1) {
 			builder.header("X-Forwarded-Port", String.valueOf(forwardedPort));
 		}
-		return CorsUtils.isSameOrigin(builder.build());
+
+		ServerHttpRequest request = adaptFromForwardedHeaders(builder);
+		assertTrue(CorsUtils.isSameOrigin(request));
 	}
 
-	private boolean checkSameOriginWithForwardedHeader(String serverName, int port, String forwardedHeader, String originHeader) {
+	private void testWithForwardedHeader(String serverName, int port,
+			String forwardedHeader, String originHeader) {
+
 		String url = "http://" + serverName;
 		if (port != -1) {
 			url = url + ":" + port;
 		}
+
 		MockServerHttpRequest.BaseBuilder<?> builder = get(url)
 				.header("Forwarded", forwardedHeader)
 				.header(HttpHeaders.ORIGIN, originHeader);
-		return CorsUtils.isSameOrigin(builder.build());
+
+		ServerHttpRequest request = adaptFromForwardedHeaders(builder);
+		assertTrue(CorsUtils.isSameOrigin(request));
+	}
+
+	// SPR-16668
+	private ServerHttpRequest adaptFromForwardedHeaders(MockServerHttpRequest.BaseBuilder<?> builder) {
+		AtomicReference<ServerHttpRequest> requestRef = new AtomicReference<>();
+		MockServerWebExchange exchange = MockServerWebExchange.from(builder);
+		new ForwardedHeaderFilter().filter(exchange, exchange2 -> {
+			requestRef.set(exchange2.getRequest());
+			return Mono.empty();
+		}).block();
+		return requestRef.get();
 	}
 
 }