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] 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); }