From e4ca15336460a84c1e1b33caeb47e541e8bd710c Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 2 Sep 2020 09:34:16 -0500 Subject: [PATCH 1/3] Issue #5224 - Test to replicate reported issue Signed-off-by: Joakim Erdfelt --- .../server/ForwardedRequestCustomizerTest.java | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java index 6c477eb10ac1..b40b17b52ac3 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java @@ -497,7 +497,21 @@ public static Stream cases() .requestURL("http://fw.example.com:4333/") .remoteAddr("8.5.4.3").remotePort(2222) ), - + Arguments.of(new Request("X-Forwarded-* ( Multiple Ports )") + .headers( + "GET / HTTP/1.1", + "Host: myhost:10001", + "X-Forwarded-For: 127.0.0.1:8888,127.0.0.2:9999", + "X-Forwarded-Port: 10002", + "X-Forwarded-Proto: https", + "X-Forwarded-Host: sub1.example.com:10003", + "X-Forwarded-Server: sub2.example.com" + ), + new Expectations() + .scheme("https").serverName("sub1.example.com").serverPort(10002) // Jetty 9.4.18 serverPort is 10003 + .requestURL("https://sub.example.com:10002/") + .remoteAddr("127.0.0.1").remotePort(8888) + ), // ================================================================= // Mixed Behavior Arguments.of(new Request("RFC7239 mixed with X-Forwarded-* headers") From e2134b13d794f341dfd4f8e629a0dfeef4eab6cf Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 2 Sep 2020 10:27:33 -0500 Subject: [PATCH 2/3] Issue #5224 - X-Forwarded-Host support for port + More test cases + Allowing X-Forwarded-Host to parse port (if present properly) Signed-off-by: Joakim Erdfelt --- .../server/ForwardedRequestCustomizer.java | 8 +-- .../ForwardedRequestCustomizerTest.java | 52 +++++++++++++++++-- 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java index bf79bd61e720..5b515db51ac9 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java @@ -598,16 +598,18 @@ public void handleSslSessionId(HttpField field) @SuppressWarnings("unused") public void handleHost(HttpField field) { + HostPort hp = new HostPort(getLeftMost(field.getValue())); + if (getForwardedPortAsAuthority() && !StringUtil.isEmpty(getForwardedPortHeader())) { if (_host == null) - _host = new PossiblyPartialHostPort(getLeftMost(field.getValue())); + _host = new PossiblyPartialHostPort(hp.getHost(), hp.getPort()); else if (_host instanceof PortSetHostPort) - _host = new HostPort(HostPort.normalizeHost(getLeftMost(field.getValue())), _host.getPort()); + _host = new HostPort(hp.getHost(), _host.getPort()); } else if (_host == null) { - _host = new HostPort(getLeftMost(field.getValue())); + _host = hp; } } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java index b40b17b52ac3..bcb6317d03f6 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java @@ -497,7 +497,7 @@ public static Stream cases() .requestURL("http://fw.example.com:4333/") .remoteAddr("8.5.4.3").remotePort(2222) ), - Arguments.of(new Request("X-Forwarded-* ( Multiple Ports )") + Arguments.of(new Request("X-Forwarded-* (Multiple Ports)") .headers( "GET / HTTP/1.1", "Host: myhost:10001", @@ -508,8 +508,54 @@ public static Stream cases() "X-Forwarded-Server: sub2.example.com" ), new Expectations() - .scheme("https").serverName("sub1.example.com").serverPort(10002) // Jetty 9.4.18 serverPort is 10003 - .requestURL("https://sub.example.com:10002/") + .scheme("https").serverName("sub1.example.com").serverPort(10002) + .requestURL("https://sub1.example.com:10002/") + .remoteAddr("127.0.0.1").remotePort(8888) + ), + Arguments.of(new Request("X-Forwarded-* (Multiple Ports - Server First)") + .headers( + "GET / HTTP/1.1", + "X-Forwarded-Server: sub2.example.com:10007", + "Host: myhost:10001", + "X-Forwarded-For: 127.0.0.1:8888,127.0.0.2:9999", + "X-Forwarded-Proto: https", + "X-Forwarded-Port: 10002", + "X-Forwarded-Host: sub1.example.com:10003" + ), + new Expectations() + .scheme("https").serverName("sub1.example.com").serverPort(10002) + .requestURL("https://sub1.example.com:10002/") + .remoteAddr("127.0.0.1").remotePort(8888) + ), + Arguments.of(new Request("X-Forwarded-* (Multiple Ports - setForwardedPortAsAuthority = false)") + .configureCustomizer((customizer) -> customizer.setForwardedPortAsAuthority(false)) + .headers( + "GET / HTTP/1.1", + "Host: myhost:10001", + "X-Forwarded-For: 127.0.0.1:8888,127.0.0.2:9999", + "X-Forwarded-Port: 10002", + "X-Forwarded-Proto: https", + "X-Forwarded-Host: sub1.example.com:10003", + "X-Forwarded-Server: sub2.example.com" + ), + new Expectations() + .scheme("https").serverName("sub1.example.com").serverPort(10003) + .requestURL("https://sub1.example.com:10003/") + .remoteAddr("127.0.0.1").remotePort(8888) + ), + Arguments.of(new Request("X-Forwarded-* (Multiple Ports Alt Order)") + .headers( + "GET / HTTP/1.1", + "Host: myhost:10001", + "X-Forwarded-For: 127.0.0.1:8888,127.0.0.2:9999", + "X-Forwarded-Proto: https", + "X-Forwarded-Host: sub1.example.com:10003", + "X-Forwarded-Port: 10002", + "X-Forwarded-Server: sub2.example.com" + ), + new Expectations() + .scheme("https").serverName("sub1.example.com").serverPort(10003) + .requestURL("https://sub1.example.com:10003/") .remoteAddr("127.0.0.1").remotePort(8888) ), // ================================================================= From 2896ed31d6df6173dde11596ef9d2809f4a7cd7d Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 8 Sep 2020 12:34:53 -0500 Subject: [PATCH 3/3] Issue #5224 - Updating ForwardedRequestCustomizerTest expectations Signed-off-by: Joakim Erdfelt --- .../jetty/server/ForwardedRequestCustomizer.java | 8 ++++---- .../jetty/server/ForwardedRequestCustomizerTest.java | 10 ++++------ 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java b/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java index 5b515db51ac9..2e4a38fd4277 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/ForwardedRequestCustomizer.java @@ -598,18 +598,18 @@ public void handleSslSessionId(HttpField field) @SuppressWarnings("unused") public void handleHost(HttpField field) { - HostPort hp = new HostPort(getLeftMost(field.getValue())); + HostPort hostField = new HostPort(getLeftMost(field.getValue())); if (getForwardedPortAsAuthority() && !StringUtil.isEmpty(getForwardedPortHeader())) { if (_host == null) - _host = new PossiblyPartialHostPort(hp.getHost(), hp.getPort()); + _host = new PossiblyPartialHostPort(hostField.getHost(), hostField.getPort()); else if (_host instanceof PortSetHostPort) - _host = new HostPort(hp.getHost(), _host.getPort()); + _host = new HostPort(hostField.getHost(), hostField.getPort() > 0 ? hostField.getPort() : _host.getPort()); } else if (_host == null) { - _host = hp; + _host = hostField; } } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java index bcb6317d03f6..63c0d42ead55 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java @@ -508,8 +508,8 @@ public static Stream cases() "X-Forwarded-Server: sub2.example.com" ), new Expectations() - .scheme("https").serverName("sub1.example.com").serverPort(10002) - .requestURL("https://sub1.example.com:10002/") + .scheme("https").serverName("sub1.example.com").serverPort(10003) + .requestURL("https://sub1.example.com:10003/") .remoteAddr("127.0.0.1").remotePort(8888) ), Arguments.of(new Request("X-Forwarded-* (Multiple Ports - Server First)") @@ -523,8 +523,8 @@ public static Stream cases() "X-Forwarded-Host: sub1.example.com:10003" ), new Expectations() - .scheme("https").serverName("sub1.example.com").serverPort(10002) - .requestURL("https://sub1.example.com:10002/") + .scheme("https").serverName("sub1.example.com").serverPort(10003) + .requestURL("https://sub1.example.com:10003/") .remoteAddr("127.0.0.1").remotePort(8888) ), Arguments.of(new Request("X-Forwarded-* (Multiple Ports - setForwardedPortAsAuthority = false)") @@ -645,7 +645,6 @@ public static Stream cases() @ParameterizedTest(name = "{0}") @MethodSource("cases") - @SuppressWarnings("unused") public void testDefaultBehavior(Request request, Expectations expectations) throws Exception { request.configure(customizer); @@ -661,7 +660,6 @@ public void testDefaultBehavior(Request request, Expectations expectations) thro @ParameterizedTest(name = "{0}") @MethodSource("cases") - @SuppressWarnings("unused") public void testConfiguredBehavior(Request request, Expectations expectations) throws Exception { request.configure(customizerConfigured);