Polishing contribution

Closes gh-34683

framework-docs/modules/ROOT/pages/web/webflux/reactive-spring.adoc

@@ -343,10 +343,6 @@ the request, based on forwarded headers, and then removes those headers. If you
 it as a bean with the name `forwardedHeaderTransformer`, it will be
 xref:web/webflux/reactive-spring.adoc#webflux-web-handler-api-special-beans[detected] and used.
 
-NOTE: In 5.1 `ForwardedHeaderFilter` was deprecated and superseded 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-forwarded-headers-security]]
 === Security Considerations

framework-docs/modules/ROOT/partials/web/forwarded-headers.adoc

@@ -9,7 +9,7 @@ that proxies can use to provide information about the original request.
 === Non-standard Headers
 
 There are other non-standard headers, too, including `X-Forwarded-Host`, `X-Forwarded-Port`,
-`X-Forwarded-Proto`, `X-Forwarded-Ssl`, and `X-Forwarded-Prefix`.
+`X-Forwarded-Proto`, `X-Forwarded-Ssl`, `X-Forwarded-Prefix`, and `X-Forwarded-For`.
 
 [[x-forwarded-host]]
 ==== X-Forwarded-Host
@@ -113,3 +113,12 @@ https://example.com/api/app1/{path} -> http://localhost:8080/app1/{path}
 In this case, the proxy has a prefix of `/api/app1` and the server has a prefix of
 `/app1`. The proxy can send `X-Forwarded-Prefix: /api/app1` to have the original prefix
 `/api/app1` override the server prefix `/app1`.
+
+[[x-forwarded-for]]
+==== X-Forwarded-For
+
+https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/X-Forwarded-For[`X-Forwarded-For: <address>`]
+is a de-facto standard header that is used to communicate the original `InetSocketAddress` of the client to a
+downstream server. For example, if a request is sent by a client at `[fd00:fefe:1::4]` to a proxy at
+`192.168.0.1`, the "remote address" information contained in the HTTP request will reflect the actual address of the
+client, not the proxy.

spring-web/src/main/java/org/springframework/http/server/reactive/ServerHttpRequest.java

@@ -186,7 +186,7 @@ public interface ServerHttpRequest extends HttpRequest, ReactiveHttpInputMessage
 
 		/**
 		 * Set the address of the local client.
-		 * @since 7.x
+		 * @since 7.0
 		 */
 		Builder localAddress(InetSocketAddress localAddress);
 

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

@@ -93,7 +93,6 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter {
 		FORWARDED_HEADER_NAMES.add("X-Forwarded-Prefix");
 		FORWARDED_HEADER_NAMES.add("X-Forwarded-Ssl");
 		FORWARDED_HEADER_NAMES.add("X-Forwarded-For");
-		FORWARDED_HEADER_NAMES.add("X-Forwarded-By");
 	}
 
 

spring-web/src/main/java/org/springframework/web/server/adapter/ForwardedHeaderTransformer.java

@@ -73,7 +73,6 @@ public class ForwardedHeaderTransformer implements Function<ServerHttpRequest, S
 		FORWARDED_HEADER_NAMES.add("X-Forwarded-Prefix");
 		FORWARDED_HEADER_NAMES.add("X-Forwarded-Ssl");
 		FORWARDED_HEADER_NAMES.add("X-Forwarded-For");
-		FORWARDED_HEADER_NAMES.add("X-Forwarded-By");
 	}
 
 

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

@@ -159,23 +159,7 @@ public abstract class ForwardedHeaderUtils {
 			Matcher matcher = FORWARDED_FOR_PATTERN.matcher(forwardedToUse);
 			if (matcher.find()) {
 				String value = matcher.group(1).trim();
-				String host = value;
-				int portSeparatorIdx = value.lastIndexOf(':');
-				int squareBracketIdx = value.lastIndexOf(']');
-				if (portSeparatorIdx > squareBracketIdx) {
-					if (squareBracketIdx == -1 && value.indexOf(':') != portSeparatorIdx) {
-						throw new IllegalArgumentException("Invalid IPv4 address: " + value);
-					}
-					host = value.substring(0, portSeparatorIdx);
-					try {
-						port = Integer.parseInt(value, portSeparatorIdx + 1, value.length(), 10);
-					}
-					catch (NumberFormatException ex) {
-						throw new IllegalArgumentException(
-								"Failed to parse a port from \"forwarded\"-type header value: " + value);
-					}
-				}
-				return InetSocketAddress.createUnresolved(host, port);
+				return parseInetSocketAddress(value, port);
 			}
 		}
 
@@ -191,13 +175,14 @@ public abstract class ForwardedHeaderUtils {
 	}
 
 	/**
-	 * Parse the first "Forwarded: by=..." or "X-Forwarded-By" header value to
+	 * Parse the first "Forwarded: by=..." header value to
 	 * an {@code InetSocketAddress} representing the address of the server.
 	 * @param uri the request {@code URI}
 	 * @param headers the request headers that may contain forwarded headers
 	 * @param localAddress the current local address
 	 * @return an {@code InetSocketAddress} with the extracted host and port, or
 	 * {@code null} if the headers are not present
+	 * @since 7.0
 	 * @see <a href="https://tools.ietf.org/html/rfc7239#section-5.1">RFC 7239, Section 5.1</a>
 	 */
 	public static @Nullable InetSocketAddress parseForwardedBy(
@@ -212,35 +197,31 @@ public abstract class ForwardedHeaderUtils {
 			Matcher matcher = FORWARDED_BY_PATTERN.matcher(forwardedToUse);
 			if (matcher.find()) {
 				String value = matcher.group(1).trim();
-				String host = value;
-				int portSeparatorIdx = value.lastIndexOf(':');
-				int squareBracketIdx = value.lastIndexOf(']');
-				if (portSeparatorIdx > squareBracketIdx) {
-					if (squareBracketIdx == -1 && value.indexOf(':') != portSeparatorIdx) {
-						throw new IllegalArgumentException("Invalid IPv4 address: " + value);
-					}
-					host = value.substring(0, portSeparatorIdx);
-					try {
-						port = Integer.parseInt(value, portSeparatorIdx + 1, value.length(), 10);
-					}
-					catch (NumberFormatException ex) {
-						throw new IllegalArgumentException(
-								"Failed to parse a port from \"forwarded\"-type header value: " + value);
-					}
-				}
-				return InetSocketAddress.createUnresolved(host, port);
+				return parseInetSocketAddress(value, port);
 			}
 		}
 
-		String byHeader = headers.getFirst("X-Forwarded-By");
-		if (StringUtils.hasText(byHeader)) {
-			String host = StringUtils.tokenizeToStringArray(byHeader, ",")[0];
-			boolean ipv6 = (host.indexOf(':') != -1);
-			host = (ipv6 && !host.startsWith("[") && !host.endsWith("]") ? "[" + host + "]" : host);
-			return InetSocketAddress.createUnresolved(host, port);
-		}
-
 		return null;
 	}
 
+	private static InetSocketAddress parseInetSocketAddress(String value, int port) {
+		String host = value;
+		int portSeparatorIdx = value.lastIndexOf(':');
+		int squareBracketIdx = value.lastIndexOf(']');
+		if (portSeparatorIdx > squareBracketIdx) {
+			if (squareBracketIdx == -1 && value.indexOf(':') != portSeparatorIdx) {
+				throw new IllegalArgumentException("Invalid IPv4 address: " + value);
+			}
+			host = value.substring(0, portSeparatorIdx);
+			try {
+				port = Integer.parseInt(value, portSeparatorIdx + 1, value.length(), 10);
+			}
+			catch (NumberFormatException ex) {
+				throw new IllegalArgumentException(
+						"Failed to parse a port from \"forwarded\"-type header value: " + value);
+			}
+		}
+		return InetSocketAddress.createUnresolved(host, port);
+	}
+
 }

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

@@ -67,8 +67,6 @@ class ForwardedHeaderFilterTests {
 
 	private static final String X_FORWARDED_FOR = "x-forwarded-for";
 
-	private static final String X_FORWARDED_BY = "x-forwarded-by";
-
 
 	private final ForwardedHeaderFilter filter = new ForwardedHeaderFilter();
 
@@ -96,7 +94,6 @@ class ForwardedHeaderFilterTests {
 		testShouldFilter(X_FORWARDED_SSL);
 		testShouldFilter(X_FORWARDED_PREFIX);
 		testShouldFilter(X_FORWARDED_FOR);
-		testShouldFilter(X_FORWARDED_BY);
 	}
 
 	private void testShouldFilter(String headerName) {
@@ -119,7 +116,6 @@ class ForwardedHeaderFilterTests {
 		this.request.addHeader(X_FORWARDED_PORT, "443");
 		this.request.addHeader("foo", "bar");
 		this.request.addHeader(X_FORWARDED_FOR, "[203.0.113.195]");
-		this.request.addHeader(X_FORWARDED_BY, "[203.0.113.196]");
 
 		this.filter.doFilter(this.request, new MockHttpServletResponse(), this.filterChain);
 		HttpServletRequest actual = (HttpServletRequest) this.filterChain.getRequest();
@@ -131,13 +127,11 @@ class ForwardedHeaderFilterTests {
 		assertThat(actual.getServerPort()).isEqualTo(443);
 		assertThat(actual.isSecure()).isTrue();
 		assertThat(actual.getRemoteAddr()).isEqualTo(actual.getRemoteHost()).isEqualTo("[203.0.113.195]");
-		assertThat(actual.getLocalAddr()).isEqualTo(actual.getLocalAddr()).isEqualTo("[203.0.113.196]");
 
 		assertThat(actual.getHeader(X_FORWARDED_PROTO)).isNull();
 		assertThat(actual.getHeader(X_FORWARDED_HOST)).isNull();
 		assertThat(actual.getHeader(X_FORWARDED_PORT)).isNull();
 		assertThat(actual.getHeader(X_FORWARDED_FOR)).isNull();
-		assertThat(actual.getHeader(X_FORWARDED_BY)).isNull();
 		assertThat(actual.getHeader("foo")).isEqualTo("bar");
 	}
 
@@ -150,7 +144,6 @@ class ForwardedHeaderFilterTests {
 		this.request.addHeader(X_FORWARDED_SSL, "on");
 		this.request.addHeader("foo", "bar");
 		this.request.addHeader(X_FORWARDED_FOR, "203.0.113.195");
-		this.request.addHeader(X_FORWARDED_BY, "203.0.113.196");
 
 		this.filter.setRemoveOnly(true);
 		this.filter.doFilter(this.request, new MockHttpServletResponse(), this.filterChain);
@@ -171,7 +164,6 @@ class ForwardedHeaderFilterTests {
 		assertThat(actual.getHeader(X_FORWARDED_PORT)).isNull();
 		assertThat(actual.getHeader(X_FORWARDED_SSL)).isNull();
 		assertThat(actual.getHeader(X_FORWARDED_FOR)).isNull();
-		assertThat(actual.getHeader(X_FORWARDED_BY)).isNull();
 		assertThat(actual.getHeader("foo")).isEqualTo("bar");
 	}
 
@@ -555,34 +547,7 @@ class ForwardedHeaderFilterTests {
 	class ForwardedBy {
 
 		@Test
-		void xForwardedForEmpty() throws Exception {
-			request.addHeader(X_FORWARDED_BY, "");
-			HttpServletRequest actual = filterAndGetWrappedRequest();
-
-			assertThat(actual.getLocalAddr()).isEqualTo(MockHttpServletRequest.DEFAULT_SERVER_ADDR);
-			assertThat(actual.getLocalPort()).isEqualTo(MockHttpServletRequest.DEFAULT_SERVER_PORT);
-		}
-
-		@Test
-		void xForwardedForSingleIdentifier() throws Exception {
-			request.addHeader(X_FORWARDED_BY, "203.0.113.195");
-			HttpServletRequest actual = filterAndGetWrappedRequest();
-
-			assertThat(actual.getLocalAddr()).isEqualTo(actual.getLocalAddr()).isEqualTo("203.0.113.195");
-			assertThat(actual.getLocalPort()).isEqualTo(MockHttpServletRequest.DEFAULT_SERVER_PORT);
-		}
-
-		@Test
-		void xForwardedForMultipleIdentifiers() throws Exception {
-			request.addHeader(X_FORWARDED_BY, "203.0.113.195, 70.41.3.18, 150.172.238.178");
-			HttpServletRequest actual = filterAndGetWrappedRequest();
-
-			assertThat(actual.getLocalAddr()).isEqualTo(actual.getLocalAddr()).isEqualTo("203.0.113.195");
-			assertThat(actual.getLocalPort()).isEqualTo(MockHttpServletRequest.DEFAULT_SERVER_PORT);
-		}
-
-		@Test
-		void forwardedForIpV4Identifier() throws Exception {
+		void forwardedByIpV4Identifier() throws Exception {
 			request.addHeader(FORWARDED, "By=203.0.113.195");
 			HttpServletRequest actual = filterAndGetWrappedRequest();
 
@@ -591,7 +556,7 @@ class ForwardedHeaderFilterTests {
 		}
 
 		@Test
-		void forwardedForIpV6Identifier() throws Exception {
+		void forwardedByIpV6Identifier() throws Exception {
 			request.addHeader(FORWARDED, "By=\"[2001:db8:cafe::17]\"");
 			HttpServletRequest actual = filterAndGetWrappedRequest();
 
@@ -600,7 +565,7 @@ class ForwardedHeaderFilterTests {
 		}
 
 		@Test
-		void forwardedForIpV4IdentifierWithPort() throws Exception {
+		void forwardedByIpV4IdentifierWithPort() throws Exception {
 			request.addHeader(FORWARDED, "By=\"203.0.113.195:47011\"");
 			HttpServletRequest actual = filterAndGetWrappedRequest();
 
@@ -609,7 +574,7 @@ class ForwardedHeaderFilterTests {
 		}
 
 		@Test
-		void forwardedForIpV6IdentifierWithPort() throws Exception {
+		void forwardedByIpV6IdentifierWithPort() throws Exception {
 			request.addHeader(FORWARDED, "By=\"[2001:db8:cafe::17]:47011\"");
 			HttpServletRequest actual = filterAndGetWrappedRequest();
 
@@ -618,7 +583,7 @@ class ForwardedHeaderFilterTests {
 		}
 
 		@Test
-		void forwardedForMultipleIdentifiers() throws Exception {
+		void forwardedByMultipleIdentifiers() throws Exception {
 			request.addHeader(FORWARDED, "by=203.0.113.195;proto=http, by=\"[2001:db8:cafe::17]\", by=unknown");
 			HttpServletRequest actual = filterAndGetWrappedRequest();
 

spring-web/src/test/java/org/springframework/web/server/adapter/ForwardedHeaderTransformerTests.java

@@ -53,7 +53,6 @@ class ForwardedHeaderTransformerTests {
 		headers.add("X-Forwarded-Prefix", "prefix");
 		headers.add("X-Forwarded-Ssl", "on");
 		headers.add("X-Forwarded-For", "203.0.113.195");
-		headers.add("X-Forwarded-By", "203.0.113.196");
 		ServerHttpRequest request = this.requestMutator.apply(getRequest(headers));
 
 		assertForwardedHeadersRemoved(request);
@@ -254,21 +253,6 @@ class ForwardedHeaderTransformerTests {
 		assertThat(request.getLocalAddress().getPort()).isEqualTo(4711);
 	}
 
-	@Test
-	void xForwardedBy() {
-		HttpHeaders headers = new HttpHeaders();
-		headers.add("x-forwarded-by", "203.0.113.195, 70.41.3.18, 150.172.238.178");
-
-		ServerHttpRequest request = MockServerHttpRequest
-				.method(HttpMethod.GET, URI.create("https://example.com/a%20b?q=a%2Bb"))
-				.headers(headers)
-				.build();
-
-		request = this.requestMutator.apply(request);
-		assertThat(request.getLocalAddress()).isNotNull();
-		assertThat(request.getLocalAddress().getHostName()).isEqualTo("203.0.113.195");
-	}
-
 
 	private MockServerHttpRequest getRequest(HttpHeaders headers) {
 		return MockServerHttpRequest.get(BASE_URL).headers(headers).build();

spring-web/src/test/java/org/springframework/web/util/ForwardedHeaderUtilsTests.java

@@ -551,4 +551,15 @@ class ForwardedHeaderUtilsTests {
 		assertThat(address.getHostName()).isEqualTo("[fd00:fefe:1::4]");
 	}
 
+	@Test
+	void parseForwardedByHeader() {
+		HttpHeaders headers = new HttpHeaders();
+		headers.add("Forwarded", "by=[fd00:fefe:1::4], 192.168.0.1");
+
+		InetSocketAddress address =
+				ForwardedHeaderUtils.parseForwardedBy(URI.create("https://example.com"), headers, null);
+
+		assertThat(address.getHostName()).isEqualTo("[fd00:fefe:1::4]");
+	}
+
 }