Skip to content

Commit 942fbf3

Browse files
bclozelbclozel
committed
Polishing contribution
Closes gh-34683
1 parent 7b3c4e5 commit 942fbf3

File tree

9 files changed

+51
-107
lines changed

9 files changed

+51
-107
lines changed

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -343,10 +343,6 @@ the request, based on forwarded headers, and then removes those headers. If you
343343
it as a bean with the name `forwardedHeaderTransformer`, it will be
344344
xref:web/webflux/reactive-spring.adoc#webflux-web-handler-api-special-beans[detected] and used.
345345

346-
NOTE: In 5.1 `ForwardedHeaderFilter` was deprecated and superseded by
347-
`ForwardedHeaderTransformer` so forwarded headers can be processed earlier, before the
348-
exchange is created. If the filter is configured anyway, it is taken out of the list of
349-
filters, and `ForwardedHeaderTransformer` is used instead.
350346

351347
[[webflux-forwarded-headers-security]]
352348
=== Security Considerations

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ that proxies can use to provide information about the original request.
99
=== Non-standard Headers
1010

1111
There are other non-standard headers, too, including `X-Forwarded-Host`, `X-Forwarded-Port`,
12-
`X-Forwarded-Proto`, `X-Forwarded-Ssl`, and `X-Forwarded-Prefix`.
12+
`X-Forwarded-Proto`, `X-Forwarded-Ssl`, `X-Forwarded-Prefix`, and `X-Forwarded-For`.
1313

1414
[[x-forwarded-host]]
1515
==== X-Forwarded-Host
@@ -113,3 +113,12 @@ https://example.com/api/app1/{path} -> http://localhost:8080/app1/{path}
113113
In this case, the proxy has a prefix of `/api/app1` and the server has a prefix of
114114
`/app1`. The proxy can send `X-Forwarded-Prefix: /api/app1` to have the original prefix
115115
`/api/app1` override the server prefix `/app1`.
116+
117+
[[x-forwarded-for]]
118+
==== X-Forwarded-For
119+
120+
https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/X-Forwarded-For[`X-Forwarded-For: <address>`]
121+
is a de-facto standard header that is used to communicate the original `InetSocketAddress` of the client to a
122+
downstream server. For example, if a request is sent by a client at `[fd00:fefe:1::4]` to a proxy at
123+
`192.168.0.1`, the "remote address" information contained in the HTTP request will reflect the actual address of the
124+
client, not the proxy.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ interface Builder {
186186

187187
/**
188188
* Set the address of the local client.
189-
* @since 7.x
189+
* @since 7.0
190190
*/
191191
Builder localAddress(InetSocketAddress localAddress);
192192

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,6 @@ public class ForwardedHeaderFilter extends OncePerRequestFilter {
9393
FORWARDED_HEADER_NAMES.add("X-Forwarded-Prefix");
9494
FORWARDED_HEADER_NAMES.add("X-Forwarded-Ssl");
9595
FORWARDED_HEADER_NAMES.add("X-Forwarded-For");
96-
FORWARDED_HEADER_NAMES.add("X-Forwarded-By");
9796
}
9897

9998

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,6 @@ public class ForwardedHeaderTransformer implements Function<ServerHttpRequest, S
7373
FORWARDED_HEADER_NAMES.add("X-Forwarded-Prefix");
7474
FORWARDED_HEADER_NAMES.add("X-Forwarded-Ssl");
7575
FORWARDED_HEADER_NAMES.add("X-Forwarded-For");
76-
FORWARDED_HEADER_NAMES.add("X-Forwarded-By");
7776
}
7877

7978

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

Lines changed: 24 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -159,23 +159,7 @@ private static void adaptForwardedHost(UriComponentsBuilder uriComponentsBuilder
159159
Matcher matcher = FORWARDED_FOR_PATTERN.matcher(forwardedToUse);
160160
if (matcher.find()) {
161161
String value = matcher.group(1).trim();
162-
String host = value;
163-
int portSeparatorIdx = value.lastIndexOf(':');
164-
int squareBracketIdx = value.lastIndexOf(']');
165-
if (portSeparatorIdx > squareBracketIdx) {
166-
if (squareBracketIdx == -1 && value.indexOf(':') != portSeparatorIdx) {
167-
throw new IllegalArgumentException("Invalid IPv4 address: " + value);
168-
}
169-
host = value.substring(0, portSeparatorIdx);
170-
try {
171-
port = Integer.parseInt(value, portSeparatorIdx + 1, value.length(), 10);
172-
}
173-
catch (NumberFormatException ex) {
174-
throw new IllegalArgumentException(
175-
"Failed to parse a port from \"forwarded\"-type header value: " + value);
176-
}
177-
}
178-
return InetSocketAddress.createUnresolved(host, port);
162+
return parseInetSocketAddress(value, port);
179163
}
180164
}
181165

@@ -191,13 +175,14 @@ private static void adaptForwardedHost(UriComponentsBuilder uriComponentsBuilder
191175
}
192176

193177
/**
194-
* Parse the first "Forwarded: by=..." or "X-Forwarded-By" header value to
178+
* Parse the first "Forwarded: by=..." header value to
195179
* an {@code InetSocketAddress} representing the address of the server.
196180
* @param uri the request {@code URI}
197181
* @param headers the request headers that may contain forwarded headers
198182
* @param localAddress the current local address
199183
* @return an {@code InetSocketAddress} with the extracted host and port, or
200184
* {@code null} if the headers are not present
185+
* @since 7.0
201186
* @see <a href="https://tools.ietf.org/html/rfc7239#section-5.1">RFC 7239, Section 5.1</a>
202187
*/
203188
public static @Nullable InetSocketAddress parseForwardedBy(
@@ -212,35 +197,31 @@ private static void adaptForwardedHost(UriComponentsBuilder uriComponentsBuilder
212197
Matcher matcher = FORWARDED_BY_PATTERN.matcher(forwardedToUse);
213198
if (matcher.find()) {
214199
String value = matcher.group(1).trim();
215-
String host = value;
216-
int portSeparatorIdx = value.lastIndexOf(':');
217-
int squareBracketIdx = value.lastIndexOf(']');
218-
if (portSeparatorIdx > squareBracketIdx) {
219-
if (squareBracketIdx == -1 && value.indexOf(':') != portSeparatorIdx) {
220-
throw new IllegalArgumentException("Invalid IPv4 address: " + value);
221-
}
222-
host = value.substring(0, portSeparatorIdx);
223-
try {
224-
port = Integer.parseInt(value, portSeparatorIdx + 1, value.length(), 10);
225-
}
226-
catch (NumberFormatException ex) {
227-
throw new IllegalArgumentException(
228-
"Failed to parse a port from \"forwarded\"-type header value: " + value);
229-
}
230-
}
231-
return InetSocketAddress.createUnresolved(host, port);
200+
return parseInetSocketAddress(value, port);
232201
}
233202
}
234203

235-
String byHeader = headers.getFirst("X-Forwarded-By");
236-
if (StringUtils.hasText(byHeader)) {
237-
String host = StringUtils.tokenizeToStringArray(byHeader, ",")[0];
238-
boolean ipv6 = (host.indexOf(':') != -1);
239-
host = (ipv6 && !host.startsWith("[") && !host.endsWith("]") ? "[" + host + "]" : host);
240-
return InetSocketAddress.createUnresolved(host, port);
241-
}
242-
243204
return null;
244205
}
245206

207+
private static InetSocketAddress parseInetSocketAddress(String value, int port) {
208+
String host = value;
209+
int portSeparatorIdx = value.lastIndexOf(':');
210+
int squareBracketIdx = value.lastIndexOf(']');
211+
if (portSeparatorIdx > squareBracketIdx) {
212+
if (squareBracketIdx == -1 && value.indexOf(':') != portSeparatorIdx) {
213+
throw new IllegalArgumentException("Invalid IPv4 address: " + value);
214+
}
215+
host = value.substring(0, portSeparatorIdx);
216+
try {
217+
port = Integer.parseInt(value, portSeparatorIdx + 1, value.length(), 10);
218+
}
219+
catch (NumberFormatException ex) {
220+
throw new IllegalArgumentException(
221+
"Failed to parse a port from \"forwarded\"-type header value: " + value);
222+
}
223+
}
224+
return InetSocketAddress.createUnresolved(host, port);
225+
}
226+
246227
}

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

Lines changed: 5 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,6 @@ class ForwardedHeaderFilterTests {
6767

6868
private static final String X_FORWARDED_FOR = "x-forwarded-for";
6969

70-
private static final String X_FORWARDED_BY = "x-forwarded-by";
71-
7270

7371
private final ForwardedHeaderFilter filter = new ForwardedHeaderFilter();
7472

@@ -96,7 +94,6 @@ void shouldFilter() {
9694
testShouldFilter(X_FORWARDED_SSL);
9795
testShouldFilter(X_FORWARDED_PREFIX);
9896
testShouldFilter(X_FORWARDED_FOR);
99-
testShouldFilter(X_FORWARDED_BY);
10097
}
10198

10299
private void testShouldFilter(String headerName) {
@@ -119,7 +116,6 @@ void forwardedRequest(String protocol) throws Exception {
119116
this.request.addHeader(X_FORWARDED_PORT, "443");
120117
this.request.addHeader("foo", "bar");
121118
this.request.addHeader(X_FORWARDED_FOR, "[203.0.113.195]");
122-
this.request.addHeader(X_FORWARDED_BY, "[203.0.113.196]");
123119

124120
this.filter.doFilter(this.request, new MockHttpServletResponse(), this.filterChain);
125121
HttpServletRequest actual = (HttpServletRequest) this.filterChain.getRequest();
@@ -131,13 +127,11 @@ void forwardedRequest(String protocol) throws Exception {
131127
assertThat(actual.getServerPort()).isEqualTo(443);
132128
assertThat(actual.isSecure()).isTrue();
133129
assertThat(actual.getRemoteAddr()).isEqualTo(actual.getRemoteHost()).isEqualTo("[203.0.113.195]");
134-
assertThat(actual.getLocalAddr()).isEqualTo(actual.getLocalAddr()).isEqualTo("[203.0.113.196]");
135130

136131
assertThat(actual.getHeader(X_FORWARDED_PROTO)).isNull();
137132
assertThat(actual.getHeader(X_FORWARDED_HOST)).isNull();
138133
assertThat(actual.getHeader(X_FORWARDED_PORT)).isNull();
139134
assertThat(actual.getHeader(X_FORWARDED_FOR)).isNull();
140-
assertThat(actual.getHeader(X_FORWARDED_BY)).isNull();
141135
assertThat(actual.getHeader("foo")).isEqualTo("bar");
142136
}
143137

@@ -150,7 +144,6 @@ void forwardedRequestInRemoveOnlyMode() throws Exception {
150144
this.request.addHeader(X_FORWARDED_SSL, "on");
151145
this.request.addHeader("foo", "bar");
152146
this.request.addHeader(X_FORWARDED_FOR, "203.0.113.195");
153-
this.request.addHeader(X_FORWARDED_BY, "203.0.113.196");
154147

155148
this.filter.setRemoveOnly(true);
156149
this.filter.doFilter(this.request, new MockHttpServletResponse(), this.filterChain);
@@ -171,7 +164,6 @@ void forwardedRequestInRemoveOnlyMode() throws Exception {
171164
assertThat(actual.getHeader(X_FORWARDED_PORT)).isNull();
172165
assertThat(actual.getHeader(X_FORWARDED_SSL)).isNull();
173166
assertThat(actual.getHeader(X_FORWARDED_FOR)).isNull();
174-
assertThat(actual.getHeader(X_FORWARDED_BY)).isNull();
175167
assertThat(actual.getHeader("foo")).isEqualTo("bar");
176168
}
177169

@@ -555,34 +547,7 @@ void forwardedForMultipleIdentifiers() throws Exception {
555547
class ForwardedBy {
556548

557549
@Test
558-
void xForwardedForEmpty() throws Exception {
559-
request.addHeader(X_FORWARDED_BY, "");
560-
HttpServletRequest actual = filterAndGetWrappedRequest();
561-
562-
assertThat(actual.getLocalAddr()).isEqualTo(MockHttpServletRequest.DEFAULT_SERVER_ADDR);
563-
assertThat(actual.getLocalPort()).isEqualTo(MockHttpServletRequest.DEFAULT_SERVER_PORT);
564-
}
565-
566-
@Test
567-
void xForwardedForSingleIdentifier() throws Exception {
568-
request.addHeader(X_FORWARDED_BY, "203.0.113.195");
569-
HttpServletRequest actual = filterAndGetWrappedRequest();
570-
571-
assertThat(actual.getLocalAddr()).isEqualTo(actual.getLocalAddr()).isEqualTo("203.0.113.195");
572-
assertThat(actual.getLocalPort()).isEqualTo(MockHttpServletRequest.DEFAULT_SERVER_PORT);
573-
}
574-
575-
@Test
576-
void xForwardedForMultipleIdentifiers() throws Exception {
577-
request.addHeader(X_FORWARDED_BY, "203.0.113.195, 70.41.3.18, 150.172.238.178");
578-
HttpServletRequest actual = filterAndGetWrappedRequest();
579-
580-
assertThat(actual.getLocalAddr()).isEqualTo(actual.getLocalAddr()).isEqualTo("203.0.113.195");
581-
assertThat(actual.getLocalPort()).isEqualTo(MockHttpServletRequest.DEFAULT_SERVER_PORT);
582-
}
583-
584-
@Test
585-
void forwardedForIpV4Identifier() throws Exception {
550+
void forwardedByIpV4Identifier() throws Exception {
586551
request.addHeader(FORWARDED, "By=203.0.113.195");
587552
HttpServletRequest actual = filterAndGetWrappedRequest();
588553

@@ -591,7 +556,7 @@ void forwardedForIpV4Identifier() throws Exception {
591556
}
592557

593558
@Test
594-
void forwardedForIpV6Identifier() throws Exception {
559+
void forwardedByIpV6Identifier() throws Exception {
595560
request.addHeader(FORWARDED, "By=\"[2001:db8:cafe::17]\"");
596561
HttpServletRequest actual = filterAndGetWrappedRequest();
597562

@@ -600,7 +565,7 @@ void forwardedForIpV6Identifier() throws Exception {
600565
}
601566

602567
@Test
603-
void forwardedForIpV4IdentifierWithPort() throws Exception {
568+
void forwardedByIpV4IdentifierWithPort() throws Exception {
604569
request.addHeader(FORWARDED, "By=\"203.0.113.195:47011\"");
605570
HttpServletRequest actual = filterAndGetWrappedRequest();
606571

@@ -609,7 +574,7 @@ void forwardedForIpV4IdentifierWithPort() throws Exception {
609574
}
610575

611576
@Test
612-
void forwardedForIpV6IdentifierWithPort() throws Exception {
577+
void forwardedByIpV6IdentifierWithPort() throws Exception {
613578
request.addHeader(FORWARDED, "By=\"[2001:db8:cafe::17]:47011\"");
614579
HttpServletRequest actual = filterAndGetWrappedRequest();
615580

@@ -618,7 +583,7 @@ void forwardedForIpV6IdentifierWithPort() throws Exception {
618583
}
619584

620585
@Test
621-
void forwardedForMultipleIdentifiers() throws Exception {
586+
void forwardedByMultipleIdentifiers() throws Exception {
622587
request.addHeader(FORWARDED, "by=203.0.113.195;proto=http, by=\"[2001:db8:cafe::17]\", by=unknown");
623588
HttpServletRequest actual = filterAndGetWrappedRequest();
624589

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

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ void removeOnly() {
5353
headers.add("X-Forwarded-Prefix", "prefix");
5454
headers.add("X-Forwarded-Ssl", "on");
5555
headers.add("X-Forwarded-For", "203.0.113.195");
56-
headers.add("X-Forwarded-By", "203.0.113.196");
5756
ServerHttpRequest request = this.requestMutator.apply(getRequest(headers));
5857

5958
assertForwardedHeadersRemoved(request);
@@ -254,21 +253,6 @@ void forwardedBy() {
254253
assertThat(request.getLocalAddress().getPort()).isEqualTo(4711);
255254
}
256255

257-
@Test
258-
void xForwardedBy() {
259-
HttpHeaders headers = new HttpHeaders();
260-
headers.add("x-forwarded-by", "203.0.113.195, 70.41.3.18, 150.172.238.178");
261-
262-
ServerHttpRequest request = MockServerHttpRequest
263-
.method(HttpMethod.GET, URI.create("https://example.com/a%20b?q=a%2Bb"))
264-
.headers(headers)
265-
.build();
266-
267-
request = this.requestMutator.apply(request);
268-
assertThat(request.getLocalAddress()).isNotNull();
269-
assertThat(request.getLocalAddress().getHostName()).isEqualTo("203.0.113.195");
270-
}
271-
272256

273257
private MockServerHttpRequest getRequest(HttpHeaders headers) {
274258
return MockServerHttpRequest.get(BASE_URL).headers(headers).build();

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,4 +551,15 @@ void fromHttpRequestXForwardedHeaderForIpv6Formatting() {
551551
assertThat(address.getHostName()).isEqualTo("[fd00:fefe:1::4]");
552552
}
553553

554+
@Test
555+
void parseForwardedByHeader() {
556+
HttpHeaders headers = new HttpHeaders();
557+
headers.add("Forwarded", "by=[fd00:fefe:1::4], 192.168.0.1");
558+
559+
InetSocketAddress address =
560+
ForwardedHeaderUtils.parseForwardedBy(URI.create("https://example.com"), headers, null);
561+
562+
assertThat(address.getHostName()).isEqualTo("[fd00:fefe:1::4]");
563+
}
564+
554565
}

0 commit comments

Comments
 (0)