Polish Forwarded header support

spring-web/src/main/java/org/springframework/web/filter/ForwardedHeaderFilter.java

@@ -42,19 +42,19 @@ import org.springframework.web.util.UriComponentsBuilder;
 import org.springframework.web.util.UrlPathHelper;
 
 /**
- * Extract values from "Forwarded" and "X-Forwarded-*" headers in order to wrap
- * and override the following from the request and response:
- * {@link HttpServletRequest#getServerName() getServerName()},
- * {@link HttpServletRequest#getServerPort() getServerPort()},
- * {@link HttpServletRequest#getScheme() getScheme()},
- * {@link HttpServletRequest#isSecure() isSecure()}, and
- * {@link HttpServletResponse#sendRedirect(String) sendRedirect(String)}.
- * In effect the wrapped request and response reflect the client-originated
- * protocol and address.
+ * Extract values from "Forwarded" and "X-Forwarded-*" headers, wrap the request
+ * and response, and make they reflect the client-originated protocol and
+ * address in the following methods:
+ * <ul>
+ * <li>{@link HttpServletRequest#getServerName() getServerName()}
+ * <li>{@link HttpServletRequest#getServerPort() getServerPort()}
+ * <li>{@link HttpServletRequest#getScheme() getScheme()}
+ * <li>{@link HttpServletRequest#isSecure() isSecure()}
+ * <li>{@link HttpServletResponse#sendRedirect(String) sendRedirect(String)}.
+ * </ul>
  *
- * <p><strong>Note:</strong> This filter can also be used in a
- * {@link #setRemoveOnly removeOnly} mode where "Forwarded" and "X-Forwarded-*"
- * headers are only eliminated without being used.
+ * <p>This filter can also be used in a {@link #setRemoveOnly removeOnly} mode
+ * where "Forwarded" and "X-Forwarded-*" headers are eliminated, and not used.
  *
  * @author Rossen Stoyanchev
  * @author Eddú Meléndez
@@ -117,7 +117,7 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter {
 
 
 	@Override
-	protected boolean shouldNotFilter(HttpServletRequest request) throws ServletException {
+	protected boolean shouldNotFilter(HttpServletRequest request) {
 		for (String headerName : FORWARDED_HEADER_NAMES) {
 			if (request.getHeader(headerName) != null) {
 				return false;
@@ -141,15 +141,18 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter {
 			FilterChain filterChain) throws ServletException, IOException {
 
 		if (this.removeOnly) {
-			ForwardedHeaderRemovingRequest theRequest = new ForwardedHeaderRemovingRequest(request);
-			filterChain.doFilter(theRequest, response);
+			ForwardedHeaderRemovingRequest wrappedRequest = new ForwardedHeaderRemovingRequest(request);
+			filterChain.doFilter(wrappedRequest, response);
 		}
 		else {
-			HttpServletRequest theRequest = new ForwardedHeaderExtractingRequest(request, this.pathHelper);
-			HttpServletResponse theResponse = (this.relativeRedirects ?
+			HttpServletRequest wrappedRequest =
+					new ForwardedHeaderExtractingRequest(request, this.pathHelper);
+
+			HttpServletResponse wrappedResponse = this.relativeRedirects ?
 					RelativeRedirectResponseWrapper.wrapIfNecessary(response, HttpStatus.SEE_OTHER) :
-					new ForwardedHeaderExtractingResponse(response, theRequest));
-			filterChain.doFilter(theRequest, theResponse);
+					new ForwardedHeaderExtractingResponse(response, wrappedRequest);
+
+			filterChain.doFilter(wrappedRequest, wrappedResponse);
 		}
 	}
 
@@ -221,7 +224,8 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter {
 
 		private final String requestUrl;
 
-		public ForwardedHeaderExtractingRequest(HttpServletRequest request, UrlPathHelper pathHelper) {
+
+		ForwardedHeaderExtractingRequest(HttpServletRequest request, UrlPathHelper pathHelper) {
 			super(request);
 
 			HttpRequest httpRequest = new ServletServerHttpRequest(request);
@@ -257,6 +261,7 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter {
 			return prefix;
 		}
 
+
 		@Override
 		@Nullable
 		public String getScheme() {
@@ -302,11 +307,13 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter {
 
 		private final HttpServletRequest request;
 
-		public ForwardedHeaderExtractingResponse(HttpServletResponse response, HttpServletRequest request) {
+
+		ForwardedHeaderExtractingResponse(HttpServletResponse response, HttpServletRequest request) {
 			super(response);
 			this.request = request;
 		}
 
+
 		@Override
 		public void sendRedirect(String location) throws IOException {
 

spring-web/src/main/java/org/springframework/web/filter/reactive/ForwardedHeaderFilter.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 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.
@@ -31,17 +31,16 @@ import org.springframework.web.server.WebFilterChain;
 import org.springframework.web.util.UriComponentsBuilder;
 
 /**
- * Extract values from "Forwarded" and "X-Forwarded-*" headers in order to change
- * and override {@link ServerHttpRequest#getURI()}.
- * In effect the request URI will reflect the client-originated
+ * Extract values from "Forwarded" and "X-Forwarded-*" headers, and use them to
+ * override {@link ServerHttpRequest#getURI()} to reflect the client-originated
  * protocol and address.
  *
- * <p><strong>Note:</strong> This filter can also be used in a
- * {@link #setRemoveOnly removeOnly} mode where "Forwarded" and "X-Forwarded-*"
- * headers are only eliminated without being used.
+ * <p>This filter can also be used in a {@link #setRemoveOnly removeOnly} mode
+ * where "Forwarded" and "X-Forwarded-*" headers are eliminated, and not used.
+ *
  * @author Arjen Poutsma
- * @see <a href="https://tools.ietf.org/html/rfc7239">https://tools.ietf.org/html/rfc7239</a>
  * @since 5.0
+ * @see <a href="https://tools.ietf.org/html/rfc7239">https://tools.ietf.org/html/rfc7239</a>
  */
 public class ForwardedHeaderFilter implements WebFilter {
 
@@ -55,8 +54,10 @@ public class ForwardedHeaderFilter implements WebFilter {
 		FORWARDED_HEADER_NAMES.add("X-Forwarded-Prefix");
 	}
 
+
 	private boolean removeOnly;
 
+
 	/**
 	 * Enables mode in which any "Forwarded" or "X-Forwarded-*" headers are
 	 * removed only and the information in them ignored.
@@ -66,6 +67,7 @@ public class ForwardedHeaderFilter implements WebFilter {
 		this.removeOnly = removeOnly;
 	}
 
+
 	@Override
 	public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
 
@@ -73,31 +75,29 @@ public class ForwardedHeaderFilter implements WebFilter {
 			return chain.filter(exchange);
 		}
 
+		ServerWebExchange mutatedExchange;
+
 		if (this.removeOnly) {
-			ServerWebExchange withoutForwardHeaders = exchange.mutate()
-					.request(builder -> builder.headers(
-							headers -> {
-								for (String headerName : FORWARDED_HEADER_NAMES) {
-									headers.remove(headerName);
-								}
-							})).build();
-			return chain.filter(withoutForwardHeaders);
+			mutatedExchange = exchange.mutate().request(builder ->
+					builder.headers(headers -> {
+						FORWARDED_HEADER_NAMES.forEach(headers::remove);
+					}))
+					.build();
 		}
 		else {
 			URI uri = UriComponentsBuilder.fromHttpRequest(exchange.getRequest()).build().toUri();
 			String prefix = getForwardedPrefix(exchange.getRequest().getHeaders());
 
-			ServerWebExchange withChangedUri = exchange.mutate()
-					.request(builder -> {
-						builder.uri(uri);
-						if (prefix != null) {
-							builder.path(prefix + uri.getPath());
-							builder.contextPath(prefix);
-						}
-					}).build();
-			return chain.filter(withChangedUri);
+			mutatedExchange = exchange.mutate().request(builder -> {
+				builder.uri(uri);
+				if (prefix != null) {
+					builder.path(prefix + uri.getPath());
+					builder.contextPath(prefix);
+				}
+			}).build();
 		}
 
+		return chain.filter(mutatedExchange);
 	}
 
 	private boolean shouldNotFilter(ServerHttpRequest request) {

spring-web/src/test/java/org/springframework/web/filter/ForwardedHeaderFilterTests.java

@@ -103,6 +103,17 @@ public class ForwardedHeaderFilterTests {
 		assertEquals("/prefix", actual);
 	}
 
+	private String filterAndGetContextPath() throws ServletException, IOException {
+		return filterAndGetWrappedRequest().getContextPath();
+	}
+
+	private HttpServletRequest filterAndGetWrappedRequest() throws ServletException, IOException {
+		MockHttpServletResponse response = new MockHttpServletResponse();
+		this.filter.doFilterInternal(this.request, response, this.filterChain);
+		return (HttpServletRequest) this.filterChain.getRequest();
+	}
+
+
 	@Test
 	public void contextPathPreserveEncoding() throws Exception {
 		this.request.setContextPath("/app%20");
@@ -183,8 +194,8 @@ public class ForwardedHeaderFilterTests {
 	@Test
 	public void caseInsensitiveForwardedPrefix() throws Exception {
 		this.request = new MockHttpServletRequest() {
-			// Make it case-sensitive (SPR-14372)
-			@Override
+
+			@Override // SPR-14372: make it case-sensitive
 			public String getHeader(String header) {
 				Enumeration<String> names = getHeaderNames();
 				while (names.hasMoreElements()) {
@@ -204,15 +215,21 @@ public class ForwardedHeaderFilterTests {
 	}
 
 	@Test
-	public void shouldFilter() throws Exception {
+	public void shouldFilter() {
 		testShouldFilter("Forwarded");
 		testShouldFilter(X_FORWARDED_HOST);
 		testShouldFilter(X_FORWARDED_PORT);
 		testShouldFilter(X_FORWARDED_PROTO);
 	}
 
+	private void testShouldFilter(String headerName) {
+		MockHttpServletRequest request = new MockHttpServletRequest();
+		request.addHeader(headerName, "1");
+		assertFalse(this.filter.shouldNotFilter(request));
+	}
+
 	@Test
-	public void shouldNotFilter() throws Exception {
+	public void shouldNotFilter() {
 		assertTrue(this.filter.shouldNotFilter(new MockHttpServletRequest()));
 	}
 
@@ -417,7 +434,6 @@ public class ForwardedHeaderFilterTests {
 		this.request.addHeader(X_FORWARDED_HOST, "example.com");
 		this.request.addHeader(X_FORWARDED_PORT, "443");
 		this.filter.setRelativeRedirects(true);
-
 		String location = sendRedirect("/a");
 
 		assertEquals("/a", location);
@@ -426,7 +442,6 @@ public class ForwardedHeaderFilterTests {
 	@Test
 	public void sendRedirectWhenRequestOnlyAndNoXForwardedThenUsesRelativeRedirects() throws Exception {
 		this.filter.setRelativeRedirects(true);
-
 		String location = sendRedirect("/a");
 
 		assertEquals("/a", location);
@@ -441,34 +456,12 @@ public class ForwardedHeaderFilterTests {
 				res.sendRedirect(location);
 			}
 		};
-		MockHttpServletResponse response = doWithFiltersAndGetResponse(this.filter, filter);
-		return response.getRedirectedUrl();
-	}
-
-	@SuppressWarnings("serial")
-	private MockHttpServletResponse doWithFiltersAndGetResponse(Filter... filters)
-			throws ServletException, IOException {
 
 		MockHttpServletResponse response = new MockHttpServletResponse();
-		FilterChain filterChain = new MockFilterChain(new HttpServlet() {}, filters);
+		FilterChain filterChain = new MockFilterChain(new HttpServlet() {}, this.filter, filter);
 		filterChain.doFilter(request, response);
-		return response;
-	}
 
-	private String filterAndGetContextPath() throws ServletException, IOException {
-		return filterAndGetWrappedRequest().getContextPath();
-	}
-
-	private HttpServletRequest filterAndGetWrappedRequest() throws ServletException, IOException {
-		MockHttpServletResponse response = new MockHttpServletResponse();
-		this.filter.doFilterInternal(this.request, response, this.filterChain);
-		return (HttpServletRequest) this.filterChain.getRequest();
-	}
-
-	private void testShouldFilter(String headerName) throws ServletException {
-		MockHttpServletRequest request = new MockHttpServletRequest();
-		request.addHeader(headerName, "1");
-		assertFalse(this.filter.shouldNotFilter(request));
+		return response.getRedirectedUrl();
 	}
 
 }

spring-web/src/test/java/org/springframework/web/filter/reactive/ForwardedHeaderFilterTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 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.
@@ -24,18 +24,21 @@ import reactor.core.publisher.Mono;
 
 import org.springframework.http.HttpHeaders;
 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.*;
+import static org.springframework.mock.http.server.reactive.test.MockServerHttpRequest.*;
 
 /**
  * @author Arjen Poutsma
  */
 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();
@@ -43,8 +46,7 @@ public class ForwardedHeaderFilterTests {
 
 	@Test
 	public void removeOnly() {
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest
-				.get("/")
+		ServerWebExchange exchange = MockServerWebExchange.from(get(BASE_URL)
 				.header("Forwarded", "for=192.0.2.60;proto=http;by=203.0.113.43")
 				.header("X-Forwarded-Host", "example.com")
 				.header("X-Forwarded-Port", "8080")
@@ -65,66 +67,57 @@ public class ForwardedHeaderFilterTests {
 
 	@Test
 	public void xForwardedRequest() throws Exception {
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest
-				.get("http://example.com/path")
+		ServerWebExchange exchange = MockServerWebExchange.from(get(BASE_URL)
 				.header("X-Forwarded-Host", "84.198.58.199")
 				.header("X-Forwarded-Port", "443")
 				.header("X-Forwarded-Proto", "https"));
 
-		this.filter.filter(exchange, this.filterChain).block(Duration.ZERO);
-
-		URI uri = this.filterChain.uri;
-		assertEquals(new URI("https://84.198.58.199/path"), uri);
+		assertEquals(new URI("https://84.198.58.199/path"), filterAndGetUri(exchange));
 	}
 
 	@Test
 	public void forwardedRequest() throws Exception {
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest
-				.get("http://example.com/path")
+		ServerWebExchange exchange = MockServerWebExchange.from(get(BASE_URL)
 				.header("Forwarded", "host=84.198.58.199;proto=https"));
 
-		this.filter.filter(exchange, this.filterChain).block(Duration.ZERO);
-
-		URI uri = this.filterChain.uri;
-		assertEquals(new URI("https://84.198.58.199/path"), uri);
+		assertEquals(new URI("https://84.198.58.199/path"), filterAndGetUri(exchange));
 	}
 
 	@Test
 	public void requestUriWithForwardedPrefix() throws Exception {
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest
-				.get("http://example.com/path")
+		ServerWebExchange exchange = MockServerWebExchange.from(get(BASE_URL)
 				.header("X-Forwarded-Prefix", "/prefix"));
 
-		this.filter.filter(exchange, this.filterChain).block(Duration.ZERO);
-
-		URI uri = this.filterChain.uri;
-		assertEquals(new URI("http://example.com/prefix/path"), uri);
+		assertEquals(new URI("http://example.com/prefix/path"), filterAndGetUri(exchange));
 	}
 
 	@Test
 	public void requestUriWithForwardedPrefixTrailingSlash() throws Exception {
-		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest
-				.get("http://example.com/path")
+		ServerWebExchange exchange = MockServerWebExchange.from(get(BASE_URL)
 				.header("X-Forwarded-Prefix", "/prefix/"));
 
-		this.filter.filter(exchange, this.filterChain).block(Duration.ZERO);
+		assertEquals(new URI("http://example.com/prefix/path"), filterAndGetUri(exchange));
+	}
 
-		URI uri = this.filterChain.uri;
-		assertEquals(new URI("http://example.com/prefix/path"), uri);
+	@Nullable
+	private URI filterAndGetUri(ServerWebExchange exchange) {
+		this.filter.filter(exchange, this.filterChain).block(Duration.ZERO);
+		return this.filterChain.uri;
 	}
 
 
 	private static class TestWebFilterChain implements WebFilterChain {
 
 		@Nullable
-		private HttpHeaders httpHeaders;
+		private HttpHeaders headers;
 
 		@Nullable
 		private URI uri;
 
+
 		@Nullable
 		public HttpHeaders getHeaders() {
-			return this.httpHeaders;
+			return this.headers;
 		}
 
 		@Nullable
@@ -134,12 +127,10 @@ public class ForwardedHeaderFilterTests {
 
 		@Override
 		public Mono<Void> filter(ServerWebExchange exchange) {
-			this.httpHeaders = exchange.getRequest().getHeaders();
+			this.headers = exchange.getRequest().getHeaders();
 			this.uri = exchange.getRequest().getURI();
 			return Mono.empty();
 		}
 	}
 
-
-
 }