From 957375a4a1c65e1e446afc08933651f91d659370 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dariusz=20J=C4=99drzejczyk?= Date: Wed, 18 Sep 2024 10:34:50 +0200 Subject: [PATCH 1/6] Parse X-Forwarded-Prefix request header The X-Forwarded-Prefix can be obtained via `HttpServerRequest#forwardedPrefix()`. Resolves #3432 --- .../netty/http/server/ConnectionInfo.java | 31 +++++++++++++- .../DefaultHttpForwardedHeaderHandler.java | 8 +++- .../http/server/HttpServerOperations.java | 5 +++ .../netty/http/server/HttpServerRequest.java | 9 +++- .../http/server/ConnectionInfoTests.java | 41 ++++++++++++++++++- .../CustomXForwardedHeadersHandler.java | 9 +++- 6 files changed, 98 insertions(+), 5 deletions(-) diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/ConnectionInfo.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/ConnectionInfo.java index 04e3134f98..d7bbe0979d 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/server/ConnectionInfo.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/server/ConnectionInfo.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2023 VMware, Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2018-2024 VMware, Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -57,6 +57,8 @@ public final class ConnectionInfo { final boolean isInetAddress; + final String forwardedPrefix; + static ConnectionInfo from(Channel channel, HttpRequest request, boolean secured, SocketAddress remoteAddress, @Nullable BiFunction forwardedHeaderHandler) { String hostName = DEFAULT_HOST_NAME; @@ -94,12 +96,18 @@ static ConnectionInfo from(Channel channel, HttpRequest request, boolean secured ConnectionInfo(SocketAddress hostAddress, String hostName, int hostPort, SocketAddress remoteAddress, String scheme, boolean isInetAddress) { + this(hostAddress, hostName, hostPort, remoteAddress, scheme, isInetAddress, ""); + } + + ConnectionInfo(SocketAddress hostAddress, String hostName, int hostPort, + SocketAddress remoteAddress, String scheme, boolean isInetAddress, String forwardedPrefix) { this.hostAddress = hostAddress; this.hostName = hostName; this.hostPort = hostPort; this.isInetAddress = isInetAddress; this.remoteAddress = remoteAddress; this.scheme = scheme; + this.forwardedPrefix = forwardedPrefix; } /** @@ -173,6 +181,18 @@ public ConnectionInfo withScheme(String scheme) { return new ConnectionInfo(this.hostAddress, this.hostName, this.hostPort, this.remoteAddress, scheme, this.isInetAddress); } + /** + * Return a new {@link ConnectionInfo} with the forwardedPrefix set. + * @param forwardedPrefix the prefix provided via X-Forwarded-Prefix header + * @return a new {@link ConnectionInfo} + * @since 1.1.23 + */ + public ConnectionInfo withForwardedPrefix(String forwardedPrefix) { + requireNonNull(forwardedPrefix, "forwardedPrefix"); + return new ConnectionInfo(this.hostAddress, this.hostName, this.hostPort, this.remoteAddress, this.scheme, + this.isInetAddress, forwardedPrefix); + } + /** * Returns the connection host name. * @return the connection host name @@ -191,6 +211,15 @@ public int getHostPort() { return hostPort != -1 ? hostPort : getDefaultHostPort(scheme); } + /** + * Returns the X-Forwarded-Prefix if it was part of the request headers. + * @return the X-Forwarded-Prefix + * @since 1.1.23 + */ + public String getForwardedPrefix() { + return forwardedPrefix; + } + /** * Returns the default host port number based on scheme. * @param scheme a connection scheme like "http", "https", or "wss" diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java index 4d923eb26b..f2cc90b8bc 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2020-2023 VMware, Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2020-2024 VMware, Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -39,6 +39,7 @@ final class DefaultHttpForwardedHeaderHandler implements BiFunction source) { diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerRequest.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerRequest.java index 77b5e9d61c..82e3a7e518 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerRequest.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerRequest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011-2023 VMware, Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2011-2024 VMware, Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -158,4 +158,11 @@ default Flux receiveContent() { * @since 1.0.28 */ ZonedDateTime timestamp(); + + /** + * Returns the X-Forwarded-Prefix if it was part of the request headers. + * @return the X-Forwarded-Prefix + * @since 1.1.23 + */ + String forwardedPrefix(); } diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/server/ConnectionInfoTests.java b/reactor-netty-http/src/test/java/reactor/netty/http/server/ConnectionInfoTests.java index 17d42f2da2..682c907fb8 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/server/ConnectionInfoTests.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/server/ConnectionInfoTests.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2023 VMware, Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2018-2024 VMware, Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -218,6 +218,7 @@ void xForwardedFor(boolean useCustomForwardedHandler) { serverRequest -> { Assertions.assertThat(serverRequest.remoteAddress().getHostString()).isEqualTo("1abc:2abc:3abc:0:0:0:5abc:6abc"); Assertions.assertThat(serverRequest.remoteAddress().getPort()).isEqualTo(8080); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); }, useCustomForwardedHandler); } @@ -234,6 +235,7 @@ void xForwardedHost(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(port); Assertions.assertThat(serverRequest.hostName()).isEqualTo("1abc:2abc:3abc:0:0:0:5abc:6abc"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(port); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); }, useCustomForwardedHandler); } @@ -250,6 +252,7 @@ void xForwardedHostEmptyHostHeader(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(port); Assertions.assertThat(serverRequest.hostName()).isEqualTo("1abc:2abc:3abc:0:0:0:5abc:6abc"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(port); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); }, useCustomForwardedHandler); } @@ -265,6 +268,7 @@ void xForwardedHostPortIncluded(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(9090); Assertions.assertThat(serverRequest.hostName()).isEqualTo("1abc:2abc:3abc:0:0:0:5abc:6abc"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(9090); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); }, useCustomForwardedHandler); } @@ -282,6 +286,7 @@ void xForwardedHostAndPort(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(8080); Assertions.assertThat(serverRequest.hostName()).isEqualTo("192.168.0.1"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8080); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); }, useCustomForwardedHandler); } @@ -299,6 +304,31 @@ void xForwardedHostPortIncludedAndXForwardedPort(boolean useCustomForwardedHandl Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(8080); Assertions.assertThat(serverRequest.hostName()).isEqualTo("192.168.0.1"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8080); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); + }, + useCustomForwardedHandler); + } + + @ParameterizedTest(name = "{displayName}({arguments})") + @ValueSource(booleans = {true, false}) + void xForwardedPrefix(boolean useCustomForwardedHandler) { + testClientRequest( + clientRequestHeaders -> { + clientRequestHeaders.add("X-Forwarded-Prefix", "test-prefix"); + }, + serverRequest -> { + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo("test-prefix"); + }, + useCustomForwardedHandler); + } + + @ParameterizedTest(name = "{displayName}({arguments})") + @ValueSource(booleans = {true, false}) + void xForwardedPrefixEmpty(boolean useCustomForwardedHandler) { + testClientRequest( + clientRequestHeaders -> {}, + serverRequest -> { + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); }, useCustomForwardedHandler); } @@ -314,6 +344,7 @@ void xForwardedMultipleHeaders(boolean useCustomForwardedHandler) { clientRequestHeaders.add("X-Forwarded-Port", "8081"); clientRequestHeaders.add("X-Forwarded-Proto", "http"); clientRequestHeaders.add("X-Forwarded-Proto", "https"); + clientRequestHeaders.add("X-Forwarded-Prefix", "test-prefix"); }, serverRequest -> { Assertions.assertThat(serverRequest.hostAddress().getHostString()).isEqualTo("192.168.0.1"); @@ -321,6 +352,7 @@ void xForwardedMultipleHeaders(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostName()).isEqualTo("192.168.0.1"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8080); Assertions.assertThat(serverRequest.scheme()).isEqualTo("http"); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo("test-prefix"); }, useCustomForwardedHandler); } @@ -339,6 +371,7 @@ void xForwardedHostAndEmptyPort(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(port); Assertions.assertThat(serverRequest.hostName()).isEqualTo("192.168.0.1"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(port); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); }, getForwardedHandler(useCustomForwardedHandler), httpClient -> httpClient, @@ -359,6 +392,7 @@ void xForwardedHostAndNonNumericPort(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(8080); Assertions.assertThat(serverRequest.hostName()).isEqualTo("192.168.0.1"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8080); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); }, getForwardedHandler(useCustomForwardedHandler), httpClient -> httpClient, @@ -382,6 +416,7 @@ void xForwardedForHostAndPort(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(8080); Assertions.assertThat(serverRequest.hostName()).isEqualTo("a.example.com"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8080); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); }, useCustomForwardedHandler); } @@ -403,6 +438,7 @@ void xForwardedForHostAndPortAndProto(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostName()).isEqualTo("a.example.com"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8080); Assertions.assertThat(serverRequest.scheme()).isEqualTo("http"); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); }, useCustomForwardedHandler); } @@ -424,6 +460,7 @@ void xForwardedForMultipleHostAndPortAndProto(boolean useCustomForwardedHandler) Assertions.assertThat(serverRequest.hostName()).isEqualTo("a.example.com"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8080); Assertions.assertThat(serverRequest.scheme()).isEqualTo("http"); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); }, useCustomForwardedHandler); } @@ -449,6 +486,7 @@ void xForwardedForAndPortOnly(boolean useCustomForwardedHandler) throws SSLExcep Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8443); Assertions.assertThat(serverRequest.hostName()).isEqualTo("a.example.com"); Assertions.assertThat(serverRequest.scheme()).isEqualTo("https"); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); }, getForwardedHandler(useCustomForwardedHandler), httpClient -> httpClient.secure(ssl -> ssl.sslContext(clientSslContext)), @@ -471,6 +509,7 @@ void xForwardedProtoOnly(String protocol, boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostName()).isEqualTo("a.example.com"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(getDefaultHostPort(protocol)); Assertions.assertThat(serverRequest.scheme()).isEqualTo(protocol); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); }, useCustomForwardedHandler); } diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/server/forwardheaderhandler/CustomXForwardedHeadersHandler.java b/reactor-netty-http/src/test/java/reactor/netty/http/server/forwardheaderhandler/CustomXForwardedHeadersHandler.java index b7adb0ac2d..81d894ea93 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/server/forwardheaderhandler/CustomXForwardedHeadersHandler.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/server/forwardheaderhandler/CustomXForwardedHeadersHandler.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2023 VMware, Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2023-2024 VMware, Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -36,6 +36,7 @@ public final class CustomXForwardedHeadersHandler { static final String X_FORWARDED_HOST_HEADER = "X-Forwarded-Host"; static final String X_FORWARDED_PORT_HEADER = "X-Forwarded-Port"; static final String X_FORWARDED_PROTO_HEADER = "X-Forwarded-Proto"; + static final String X_FORWARDED_PREFIX_HEADER = "X-Forwarded-Prefix"; private CustomXForwardedHeadersHandler() { } @@ -74,6 +75,12 @@ private ConnectionInfo parseXForwardedInfo(ConnectionInfo connectionInfo, HttpRe throw new IllegalArgumentException("Failed to parse a port from " + portHeader); } } + + String prefixHeader = request.headers().get(X_FORWARDED_PREFIX_HEADER); + if (prefixHeader != null && !prefixHeader.isEmpty()) { + connectionInfo = connectionInfo.withForwardedPrefix(prefixHeader); + } + return connectionInfo; } } From 78350beb3bdc5766909eaf87bc9e64bbe2a1a03e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dariusz=20J=C4=99drzejczyk?= Date: Wed, 18 Sep 2024 10:38:44 +0200 Subject: [PATCH 2/6] Exclude new method from JAPICMP checks --- reactor-netty-http/build.gradle | 1 + 1 file changed, 1 insertion(+) diff --git a/reactor-netty-http/build.gradle b/reactor-netty-http/build.gradle index 19b93d4312..c3bcd133a3 100644 --- a/reactor-netty-http/build.gradle +++ b/reactor-netty-http/build.gradle @@ -249,6 +249,7 @@ task japicmp(type: JapicmpTask) { compatibilityChangeExcludes = [ "METHOD_NEW_DEFAULT" ] methodExcludes = [ + "reactor.netty.http.server.HttpServerRequest#forwardedPrefix()" ] } From 3ca54a85397a66e1cfc5353ccbeddd3815c73198 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dariusz=20J=C4=99drzejczyk?= Date: Mon, 23 Sep 2024 16:09:13 +0200 Subject: [PATCH 3/6] Distinguish missing header from empty, handle multiple components --- .../netty/http/server/ConnectionInfo.java | 6 +- .../DefaultHttpForwardedHeaderHandler.java | 35 ++++++++++- .../netty/http/server/HttpServerRequest.java | 1 + .../http/server/ConnectionInfoTests.java | 60 ++++++++++++++----- .../CustomXForwardedHeadersHandler.java | 2 +- 5 files changed, 85 insertions(+), 19 deletions(-) diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/ConnectionInfo.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/ConnectionInfo.java index d7bbe0979d..496c93a6ea 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/server/ConnectionInfo.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/server/ConnectionInfo.java @@ -57,6 +57,7 @@ public final class ConnectionInfo { final boolean isInetAddress; + @Nullable final String forwardedPrefix; static ConnectionInfo from(Channel channel, HttpRequest request, boolean secured, SocketAddress remoteAddress, @@ -96,11 +97,11 @@ static ConnectionInfo from(Channel channel, HttpRequest request, boolean secured ConnectionInfo(SocketAddress hostAddress, String hostName, int hostPort, SocketAddress remoteAddress, String scheme, boolean isInetAddress) { - this(hostAddress, hostName, hostPort, remoteAddress, scheme, isInetAddress, ""); + this(hostAddress, hostName, hostPort, remoteAddress, scheme, isInetAddress, null); } ConnectionInfo(SocketAddress hostAddress, String hostName, int hostPort, - SocketAddress remoteAddress, String scheme, boolean isInetAddress, String forwardedPrefix) { + SocketAddress remoteAddress, String scheme, boolean isInetAddress, @Nullable String forwardedPrefix) { this.hostAddress = hostAddress; this.hostName = hostName; this.hostPort = hostPort; @@ -216,6 +217,7 @@ public int getHostPort() { * @return the X-Forwarded-Prefix * @since 1.1.23 */ + @Nullable public String getForwardedPrefix() { return forwardedPrefix; } diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java index f2cc90b8bc..b47fb397f7 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java @@ -15,6 +15,10 @@ */ package reactor.netty.http.server; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.StringTokenizer; import java.util.function.BiFunction; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -45,6 +49,8 @@ final class DefaultHttpForwardedHeaderHandler implements BiFunction 1 && rawPrefix.charAt(endIndex - 1) == '/') { + endIndex--; + } + prefix.append((endIndex != rawPrefix.length() ? rawPrefix.substring(0, endIndex) : rawPrefix)); + } + return prefix.toString(); + } + + private static String[] tokenizeToStringArray(String str) { + StringTokenizer st = new StringTokenizer(str, ","); + ArrayList tokens = new ArrayList<>(); + while (st.hasMoreTokens()) { + String token = st.nextToken().trim(); + if (!token.isEmpty()) { + tokens.add(token); + } + } + return !tokens.isEmpty() ? tokens.toArray(EMPTY_STRING_ARRAY) : EMPTY_STRING_ARRAY; + } } diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerRequest.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerRequest.java index 82e3a7e518..a65118e8c6 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerRequest.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/server/HttpServerRequest.java @@ -164,5 +164,6 @@ default Flux receiveContent() { * @return the X-Forwarded-Prefix * @since 1.1.23 */ + @Nullable String forwardedPrefix(); } diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/server/ConnectionInfoTests.java b/reactor-netty-http/src/test/java/reactor/netty/http/server/ConnectionInfoTests.java index 682c907fb8..8de955518f 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/server/ConnectionInfoTests.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/server/ConnectionInfoTests.java @@ -42,6 +42,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; import reactor.core.publisher.Mono; @@ -218,7 +219,7 @@ void xForwardedFor(boolean useCustomForwardedHandler) { serverRequest -> { Assertions.assertThat(serverRequest.remoteAddress().getHostString()).isEqualTo("1abc:2abc:3abc:0:0:0:5abc:6abc"); Assertions.assertThat(serverRequest.remoteAddress().getPort()).isEqualTo(8080); - Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); + Assertions.assertThat(serverRequest.forwardedPrefix()).isNull(); }, useCustomForwardedHandler); } @@ -235,7 +236,7 @@ void xForwardedHost(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(port); Assertions.assertThat(serverRequest.hostName()).isEqualTo("1abc:2abc:3abc:0:0:0:5abc:6abc"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(port); - Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); + Assertions.assertThat(serverRequest.forwardedPrefix()).isNull(); }, useCustomForwardedHandler); } @@ -252,7 +253,7 @@ void xForwardedHostEmptyHostHeader(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(port); Assertions.assertThat(serverRequest.hostName()).isEqualTo("1abc:2abc:3abc:0:0:0:5abc:6abc"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(port); - Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); + Assertions.assertThat(serverRequest.forwardedPrefix()).isNull(); }, useCustomForwardedHandler); } @@ -268,7 +269,7 @@ void xForwardedHostPortIncluded(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(9090); Assertions.assertThat(serverRequest.hostName()).isEqualTo("1abc:2abc:3abc:0:0:0:5abc:6abc"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(9090); - Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); + Assertions.assertThat(serverRequest.forwardedPrefix()).isNull(); }, useCustomForwardedHandler); } @@ -286,7 +287,7 @@ void xForwardedHostAndPort(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(8080); Assertions.assertThat(serverRequest.hostName()).isEqualTo("192.168.0.1"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8080); - Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); + Assertions.assertThat(serverRequest.forwardedPrefix()).isNull(); }, useCustomForwardedHandler); } @@ -304,7 +305,7 @@ void xForwardedHostPortIncludedAndXForwardedPort(boolean useCustomForwardedHandl Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(8080); Assertions.assertThat(serverRequest.hostName()).isEqualTo("192.168.0.1"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8080); - Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); + Assertions.assertThat(serverRequest.forwardedPrefix()).isNull(); }, useCustomForwardedHandler); } @@ -322,17 +323,48 @@ void xForwardedPrefix(boolean useCustomForwardedHandler) { useCustomForwardedHandler); } + @ParameterizedTest + @CsvSource(value = { + "/first,/second | /first/second", + "/first,/second/ | /first/second", + "/first/,/second/ | /first/second", + "/first/,/second// | /first/second" + }, delimiter = '|') + void xForwardedPrefixDelimited(String input, String output) { + testClientRequest( + clientRequestHeaders -> { + clientRequestHeaders.add("X-Forwarded-Prefix", input); + }, + serverRequest -> { + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(output); + }, + false); + } + @ParameterizedTest(name = "{displayName}({arguments})") @ValueSource(booleans = {true, false}) void xForwardedPrefixEmpty(boolean useCustomForwardedHandler) { testClientRequest( - clientRequestHeaders -> {}, + clientRequestHeaders -> { + clientRequestHeaders.add("X-Forwarded-Prefix", ""); + }, serverRequest -> { Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); }, useCustomForwardedHandler); } + @ParameterizedTest(name = "{displayName}({arguments})") + @ValueSource(booleans = {true, false}) + void xForwardedPrefixMissing(boolean useCustomForwardedHandler) { + testClientRequest( + clientRequestHeaders -> {}, + serverRequest -> { + Assertions.assertThat(serverRequest.forwardedPrefix()).isNull(); + }, + useCustomForwardedHandler); + } + @ParameterizedTest(name = "{displayName}({arguments})") @ValueSource(booleans = {true, false}) void xForwardedMultipleHeaders(boolean useCustomForwardedHandler) { @@ -371,7 +403,7 @@ void xForwardedHostAndEmptyPort(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(port); Assertions.assertThat(serverRequest.hostName()).isEqualTo("192.168.0.1"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(port); - Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); + Assertions.assertThat(serverRequest.forwardedPrefix()).isNull(); }, getForwardedHandler(useCustomForwardedHandler), httpClient -> httpClient, @@ -392,7 +424,7 @@ void xForwardedHostAndNonNumericPort(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(8080); Assertions.assertThat(serverRequest.hostName()).isEqualTo("192.168.0.1"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8080); - Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); + Assertions.assertThat(serverRequest.forwardedPrefix()).isNull(); }, getForwardedHandler(useCustomForwardedHandler), httpClient -> httpClient, @@ -416,7 +448,7 @@ void xForwardedForHostAndPort(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostAddress().getPort()).isEqualTo(8080); Assertions.assertThat(serverRequest.hostName()).isEqualTo("a.example.com"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8080); - Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); + Assertions.assertThat(serverRequest.forwardedPrefix()).isNull(); }, useCustomForwardedHandler); } @@ -438,7 +470,7 @@ void xForwardedForHostAndPortAndProto(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostName()).isEqualTo("a.example.com"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8080); Assertions.assertThat(serverRequest.scheme()).isEqualTo("http"); - Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); + Assertions.assertThat(serverRequest.forwardedPrefix()).isNull(); }, useCustomForwardedHandler); } @@ -460,7 +492,7 @@ void xForwardedForMultipleHostAndPortAndProto(boolean useCustomForwardedHandler) Assertions.assertThat(serverRequest.hostName()).isEqualTo("a.example.com"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8080); Assertions.assertThat(serverRequest.scheme()).isEqualTo("http"); - Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); + Assertions.assertThat(serverRequest.forwardedPrefix()).isNull(); }, useCustomForwardedHandler); } @@ -486,7 +518,7 @@ void xForwardedForAndPortOnly(boolean useCustomForwardedHandler) throws SSLExcep Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8443); Assertions.assertThat(serverRequest.hostName()).isEqualTo("a.example.com"); Assertions.assertThat(serverRequest.scheme()).isEqualTo("https"); - Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); + Assertions.assertThat(serverRequest.forwardedPrefix()).isNull(); }, getForwardedHandler(useCustomForwardedHandler), httpClient -> httpClient.secure(ssl -> ssl.sslContext(clientSslContext)), @@ -509,7 +541,7 @@ void xForwardedProtoOnly(String protocol, boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostName()).isEqualTo("a.example.com"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(getDefaultHostPort(protocol)); Assertions.assertThat(serverRequest.scheme()).isEqualTo(protocol); - Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo(""); + Assertions.assertThat(serverRequest.forwardedPrefix()).isNull(); }, useCustomForwardedHandler); } diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/server/forwardheaderhandler/CustomXForwardedHeadersHandler.java b/reactor-netty-http/src/test/java/reactor/netty/http/server/forwardheaderhandler/CustomXForwardedHeadersHandler.java index 81d894ea93..c0bb688988 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/server/forwardheaderhandler/CustomXForwardedHeadersHandler.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/server/forwardheaderhandler/CustomXForwardedHeadersHandler.java @@ -77,7 +77,7 @@ private ConnectionInfo parseXForwardedInfo(ConnectionInfo connectionInfo, HttpRe } String prefixHeader = request.headers().get(X_FORWARDED_PREFIX_HEADER); - if (prefixHeader != null && !prefixHeader.isEmpty()) { + if (prefixHeader != null) { connectionInfo = connectionInfo.withForwardedPrefix(prefixHeader); } From 83c78c26cff304e56e35d4038e9071b286b01ae7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dariusz=20J=C4=99drzejczyk?= Date: Tue, 24 Sep 2024 11:02:06 +0200 Subject: [PATCH 4/6] Removed unused imports --- .../netty/http/server/DefaultHttpForwardedHeaderHandler.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java index b47fb397f7..b9fdb4afeb 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java @@ -16,8 +16,6 @@ package reactor.netty.http.server; import java.util.ArrayList; -import java.util.Collection; -import java.util.List; import java.util.StringTokenizer; import java.util.function.BiFunction; import java.util.regex.Matcher; From 89c35127563a3cb34210fa051c425d1bb04684d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dariusz=20J=C4=99drzejczyk?= Date: Tue, 1 Oct 2024 11:40:05 +0200 Subject: [PATCH 5/6] Validating X-Forwarded-Prefix contains a slash --- .../DefaultHttpForwardedHeaderHandler.java | 6 ++++- .../http/server/ConnectionInfoTests.java | 24 +++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java index b9fdb4afeb..259e7a9804 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java @@ -138,7 +138,11 @@ private static String parseForwardedPrefix(String prefixHeader) { } prefix.append((endIndex != rawPrefix.length() ? rawPrefix.substring(0, endIndex) : rawPrefix)); } - return prefix.toString(); + String parsedPrefix = prefix.toString(); + if (!parsedPrefix.isEmpty() && DEFAULT_FORWARDED_HEADER_VALIDATION && !parsedPrefix.startsWith("/")) { + throw new IllegalArgumentException("X-Forwarded-Prefix did not start with a slash (\"/\"): " + prefixHeader); + } + return parsedPrefix; } private static String[] tokenizeToStringArray(String str) { diff --git a/reactor-netty-http/src/test/java/reactor/netty/http/server/ConnectionInfoTests.java b/reactor-netty-http/src/test/java/reactor/netty/http/server/ConnectionInfoTests.java index 8de955518f..0bc45f72c4 100644 --- a/reactor-netty-http/src/test/java/reactor/netty/http/server/ConnectionInfoTests.java +++ b/reactor-netty-http/src/test/java/reactor/netty/http/server/ConnectionInfoTests.java @@ -315,14 +315,30 @@ void xForwardedHostPortIncludedAndXForwardedPort(boolean useCustomForwardedHandl void xForwardedPrefix(boolean useCustomForwardedHandler) { testClientRequest( clientRequestHeaders -> { - clientRequestHeaders.add("X-Forwarded-Prefix", "test-prefix"); + clientRequestHeaders.add("X-Forwarded-Prefix", "/test-prefix"); }, serverRequest -> { - Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo("test-prefix"); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo("/test-prefix"); }, useCustomForwardedHandler); } + @Test + void xForwardedPrefixWithoutForwardSlash() { + testClientRequest( + clientRequestHeaders -> { + clientRequestHeaders.add("X-Forwarded-Prefix", "forward-slash-missing"); + }, + serverRequest -> { + + }, + null, + httpClient -> httpClient, + httpServer -> httpServer.port(8080), + false, + true); + } + @ParameterizedTest @CsvSource(value = { "/first,/second | /first/second", @@ -376,7 +392,7 @@ void xForwardedMultipleHeaders(boolean useCustomForwardedHandler) { clientRequestHeaders.add("X-Forwarded-Port", "8081"); clientRequestHeaders.add("X-Forwarded-Proto", "http"); clientRequestHeaders.add("X-Forwarded-Proto", "https"); - clientRequestHeaders.add("X-Forwarded-Prefix", "test-prefix"); + clientRequestHeaders.add("X-Forwarded-Prefix", "/test-prefix"); }, serverRequest -> { Assertions.assertThat(serverRequest.hostAddress().getHostString()).isEqualTo("192.168.0.1"); @@ -384,7 +400,7 @@ void xForwardedMultipleHeaders(boolean useCustomForwardedHandler) { Assertions.assertThat(serverRequest.hostName()).isEqualTo("192.168.0.1"); Assertions.assertThat(serverRequest.hostPort()).isEqualTo(8080); Assertions.assertThat(serverRequest.scheme()).isEqualTo("http"); - Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo("test-prefix"); + Assertions.assertThat(serverRequest.forwardedPrefix()).isEqualTo("/test-prefix"); }, useCustomForwardedHandler); } From ef1bb0da294a7c33f764e3a8ad228f1c9e7cb3c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dariusz=20J=C4=99drzejczyk?= Date: Tue, 1 Oct 2024 13:23:28 +0200 Subject: [PATCH 6/6] Replace `String#startsWith` with `String#charAt` --- .../netty/http/server/DefaultHttpForwardedHeaderHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java b/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java index 259e7a9804..3ba3d1fcd9 100644 --- a/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java +++ b/reactor-netty-http/src/main/java/reactor/netty/http/server/DefaultHttpForwardedHeaderHandler.java @@ -139,7 +139,7 @@ private static String parseForwardedPrefix(String prefixHeader) { prefix.append((endIndex != rawPrefix.length() ? rawPrefix.substring(0, endIndex) : rawPrefix)); } String parsedPrefix = prefix.toString(); - if (!parsedPrefix.isEmpty() && DEFAULT_FORWARDED_HEADER_VALIDATION && !parsedPrefix.startsWith("/")) { + if (!parsedPrefix.isEmpty() && DEFAULT_FORWARDED_HEADER_VALIDATION && parsedPrefix.charAt(0) != '/') { throw new IllegalArgumentException("X-Forwarded-Prefix did not start with a slash (\"/\"): " + prefixHeader); } return parsedPrefix;