From 8c4dd7ab05ce11bf242b1a215c06c72be823aa6b Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 13 Aug 2019 11:14:44 +1000 Subject: [PATCH 1/9] Fixed decoration changes for #3804 Fixed bad names in OWB webapp. Don't have the owb jetty-web.xml on by default. Signed-off-by: Greg Wilkins --- .../eclipse/jetty/tests/distribution/CDITests.java | 11 ++++++----- .../test-cdi-common-webapp/src/main/webapp/index.html | 2 +- tests/test-webapps/test-owb-cdi-webapp/pom.xml | 2 +- .../WEB-INF/{jetty-web.xml => jetty-web-owb.xml} | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) rename tests/test-webapps/test-owb-cdi-webapp/src/main/webapp/WEB-INF/{jetty-web.xml => jetty-web-owb.xml} (82%) diff --git a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/CDITests.java b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/CDITests.java index a0e1b988940..79570423ea6 100644 --- a/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/CDITests.java +++ b/tests/test-distribution/src/test/java/org/eclipse/jetty/tests/distribution/CDITests.java @@ -42,12 +42,13 @@ public class CDITests extends AbstractDistributionTest // Tests from here use these parameters public static Stream tests() { - Consumer removeJettyWebXml = d -> + Consumer renameJettyWebOwbXml = d -> { try { + Path jettyWebOwbXml = d.getJettyBase().resolve("webapps/demo/WEB-INF/jetty-web-owb.xml"); Path jettyWebXml = d.getJettyBase().resolve("webapps/demo/WEB-INF/jetty-web.xml"); - Files.deleteIfExists(jettyWebXml); + Files.move(jettyWebOwbXml, jettyWebXml); } catch(IOException e) { @@ -63,8 +64,8 @@ public class CDITests extends AbstractDistributionTest // TODO Arguments.of("weld", "cdi-decorate", null), // Weld >= 3.1.3 // -- Apache OpenWebBeans -- - Arguments.of("owb", "cdi-spi", removeJettyWebXml), - Arguments.of("owb", "cdi2", null) + Arguments.of("owb", "cdi-spi", null), + Arguments.of("owb", "jsp", renameJettyWebOwbXml) // Arguments.of("owb", "decorate", null), // Not supported // Arguments.of("owb", "cdi-decorate", null) // Not supported ); @@ -87,7 +88,7 @@ public class CDITests extends AbstractDistributionTest String[] args1 = { "--create-startd", "--approve-all-licenses", - "--add-to-start=http,deploy,annotations,jsp,"+integration + "--add-to-start=http,deploy,annotations,jsp" + (integration==null?"":(","+integration)) }; try (DistributionTester.Run run1 = distribution.start(args1)) { diff --git a/tests/test-webapps/test-cdi-common-webapp/src/main/webapp/index.html b/tests/test-webapps/test-cdi-common-webapp/src/main/webapp/index.html index 9beca9836d8..a1765080059 100644 --- a/tests/test-webapps/test-cdi-common-webapp/src/main/webapp/index.html +++ b/tests/test-webapps/test-cdi-common-webapp/src/main/webapp/index.html @@ -1,4 +1,4 @@ -

OWB CDI Test Webapp

+

CDI Test Webapp

CDI Info

diff --git a/tests/test-webapps/test-owb-cdi-webapp/pom.xml b/tests/test-webapps/test-owb-cdi-webapp/pom.xml index c81c088d78c..d20b4db280c 100644 --- a/tests/test-webapps/test-owb-cdi-webapp/pom.xml +++ b/tests/test-webapps/test-owb-cdi-webapp/pom.xml @@ -16,7 +16,7 @@ - weld-owb-demo + owb-cdi-demo diff --git a/tests/test-webapps/test-owb-cdi-webapp/src/main/webapp/WEB-INF/jetty-web.xml b/tests/test-webapps/test-owb-cdi-webapp/src/main/webapp/WEB-INF/jetty-web-owb.xml similarity index 82% rename from tests/test-webapps/test-owb-cdi-webapp/src/main/webapp/WEB-INF/jetty-web.xml rename to tests/test-webapps/test-owb-cdi-webapp/src/main/webapp/WEB-INF/jetty-web-owb.xml index 404e1bc09dc..4c5b8dcd533 100644 --- a/tests/test-webapps/test-owb-cdi-webapp/src/main/webapp/WEB-INF/jetty-web.xml +++ b/tests/test-webapps/test-owb-cdi-webapp/src/main/webapp/WEB-INF/jetty-web-owb.xml @@ -1,6 +1,6 @@ - + From 4b17d28cb085b2c33ace9d45f401295d2e200f43 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 13 Aug 2019 07:30:14 -0500 Subject: [PATCH 2/9] Issue #3969 - adding testcase to verify behavior Signed-off-by: Joakim Erdfelt --- .../ForwardedRequestCustomizerTest.java | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) 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 6ee032d2ab3..ffd1f471ec2 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 @@ -627,6 +627,29 @@ public class ForwardedRequestCustomizerTest assertThat("requestURL", _requestURL.get(), is("http://example.com/")); } + @Test + public void testAllXForwarded() throws Exception + { + HttpTester.Response response = HttpTester.parseResponse( + _connector.getResponse( + "GET / HTTP/1.1\n" + + "Host: myhost\n" + + "X-Forwarded-Proto: https\n" + + "X-Forwarded-Host: www.example.com\n" + + "X-Forwarded-Port: 4333\n" + + "X-Forwarded-For: 8.5.4.3:2222\n" + + "X-Forwarded-Server: fw.example.com\n" + + "\n")); + + assertThat("status", response.getStatus(), is(200)); + assertThat("scheme", _scheme.get(), is("https")); + assertThat("serverName", _serverName.get(), is("www.example.com")); + assertThat("serverPort", _serverPort.get(), is(4333)); + assertThat("remoteAddr", _remoteAddr.get(), is("8.5.4.3")); + assertThat("remotePort", _remotePort.get(), is(2222)); + assertThat("requestURL", _requestURL.get(), is("https://www.example.com:4333/")); + } + interface RequestTester { boolean check(HttpServletRequest request, HttpServletResponse response) throws IOException; From 72c05bc8baac40cb975d78cbbaf8e9986bd71ff2 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 13 Aug 2019 12:39:30 -0500 Subject: [PATCH 3/9] Fixes #3969 - Fixing X-Forwarded-Port header setter + Fixing ForwardedRequestCustomizer.getForwardedPortHeader() + Fixing ForwardedRequestCustomizer.setForwardedPortHeader(String) + Refactoring unit tests: + Tests default ForwardedRequestCustomizer behavior on one Connector + Tests header configured ForwardedRequestCustomizer behavior on different Connector Signed-off-by: Joakim Erdfelt --- .../server/ForwardedRequestCustomizer.java | 59 +- .../ForwardedRequestCustomizerTest.java | 1147 +++++++++-------- 2 files changed, 614 insertions(+), 592 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 93586854e62..8937c1286b1 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 @@ -35,8 +35,6 @@ import org.eclipse.jetty.util.ArrayTrie; import org.eclipse.jetty.util.HostPort; import org.eclipse.jetty.util.StringUtil; import org.eclipse.jetty.util.Trie; -import org.eclipse.jetty.util.log.Log; -import org.eclipse.jetty.util.log.Logger; import static java.lang.invoke.MethodType.methodType; @@ -63,8 +61,6 @@ import static java.lang.invoke.MethodType.methodType; */ public class ForwardedRequestCustomizer implements Customizer { - private static final Logger LOG = Log.getLogger(ForwardedRequestCustomizer.class); - private HostPortHttpField _forcedHost; private boolean _proxyAsAuthority = false; private boolean _forwardedPortAsAuthority = true; @@ -236,7 +232,7 @@ public class ForwardedRequestCustomizer implements Customizer public String getForwardedPortHeader() { - return _forwardedHostHeader; + return _forwardedPortHeader; } /** @@ -244,9 +240,9 @@ public class ForwardedRequestCustomizer implements Customizer */ public void setForwardedPortHeader(String forwardedPortHeader) { - if (_forwardedHostHeader == null || !_forwardedHostHeader.equalsIgnoreCase(forwardedPortHeader)) + if (_forwardedPortHeader == null || !_forwardedPortHeader.equalsIgnoreCase(forwardedPortHeader)) { - _forwardedHostHeader = forwardedPortHeader; + _forwardedPortHeader = forwardedPortHeader; updateHandles(); } } @@ -458,7 +454,6 @@ public class ForwardedRequestCustomizer implements Customizer { int size = 0; MethodHandles.Lookup lookup = MethodHandles.lookup(); - MethodType type = methodType(Void.TYPE, HttpField.class); while (true) { @@ -467,23 +462,23 @@ public class ForwardedRequestCustomizer implements Customizer size += 128; _handles = new ArrayTrie<>(size); - if (_forwardedCipherSuiteHeader != null && !_handles.put(_forwardedCipherSuiteHeader, lookup.findVirtual(Forwarded.class, "handleCipherSuite", type))) + if (updateForwardedHandle(lookup, getForwardedCipherSuiteHeader(), "handleCipherSuite")) continue; - if (_forwardedSslSessionIdHeader != null && !_handles.put(_forwardedSslSessionIdHeader, lookup.findVirtual(Forwarded.class, "handleSslSessionId", type))) + if (updateForwardedHandle(lookup, getForwardedSslSessionIdHeader(), "handleSslSessionId")) continue; - if (_forwardedHeader != null && !_handles.put(_forwardedHeader, lookup.findVirtual(Forwarded.class, "handleRFC7239", type))) + if (updateForwardedHandle(lookup, getForwardedHeader(), "handleRFC7239")) continue; - if (_forwardedForHeader != null && !_handles.put(_forwardedForHeader, lookup.findVirtual(Forwarded.class, "handleFor", type))) + if (updateForwardedHandle(lookup, getForwardedForHeader(), "handleFor")) continue; - if (_forwardedPortHeader != null && !_handles.put(_forwardedPortHeader, lookup.findVirtual(Forwarded.class, "handlePort", type))) + if (updateForwardedHandle(lookup, getForwardedPortHeader(), "handlePort")) continue; - if (_forwardedHostHeader != null && !_handles.put(_forwardedHostHeader, lookup.findVirtual(Forwarded.class, "handleHost", type))) + if (updateForwardedHandle(lookup, getForwardedHostHeader(), "handleHost")) continue; - if (_forwardedProtoHeader != null && !_handles.put(_forwardedProtoHeader, lookup.findVirtual(Forwarded.class, "handleProto", type))) + if (updateForwardedHandle(lookup, getForwardedProtoHeader(), "handleProto")) continue; - if (_forwardedHttpsHeader != null && !_handles.put(_forwardedHttpsHeader, lookup.findVirtual(Forwarded.class, "handleHttps", type))) + if (updateForwardedHandle(lookup, getForwardedHttpsHeader(), "handleHttps")) continue; - if (_forwardedServerHeader != null && !_handles.put(_forwardedServerHeader, lookup.findVirtual(Forwarded.class, "handleServer", type))) + if (updateForwardedHandle(lookup, getForwardedServerHeader(), "handleServer")) continue; break; } @@ -494,6 +489,16 @@ public class ForwardedRequestCustomizer implements Customizer } } + private boolean updateForwardedHandle(MethodHandles.Lookup lookup, String headerName, String forwardedMethodName) throws NoSuchMethodException, IllegalAccessException + { + final MethodType type = methodType(Void.TYPE, HttpField.class); + + if (StringUtil.isBlank(headerName)) + return false; + + return !_handles.put(headerName, lookup.findVirtual(Forwarded.class, forwardedMethodName, type)); + } + private static class ForcedHostPort extends HostPort { ForcedHostPort(String authority) @@ -550,6 +555,7 @@ public class ForwardedRequestCustomizer implements Customizer _host = _forcedHost.getHostPort(); } + @SuppressWarnings("unused") public void handleCipherSuite(HttpField field) { _request.setAttribute("javax.servlet.request.cipher_suite", field.getValue()); @@ -560,6 +566,7 @@ public class ForwardedRequestCustomizer implements Customizer } } + @SuppressWarnings("unused") public void handleSslSessionId(HttpField field) { _request.setAttribute("javax.servlet.request.ssl_session_id", field.getValue()); @@ -572,7 +579,7 @@ public class ForwardedRequestCustomizer implements Customizer public void handleHost(HttpField field) { - if (_forwardedPortAsAuthority && !StringUtil.isEmpty(_forwardedPortHeader)) + if (getForwardedPortAsAuthority() && !StringUtil.isEmpty(getForwardedPortHeader())) { if (_host == null) _host = new PossiblyPartialHostPort(getLeftMost(field.getValue())); @@ -585,22 +592,25 @@ public class ForwardedRequestCustomizer implements Customizer } } + @SuppressWarnings("unused") public void handleServer(HttpField field) { - if (_proxyAsAuthority) + if (getProxyAsAuthority()) return; handleHost(field); } + @SuppressWarnings("unused") public void handleProto(HttpField field) { if (_proto == null) _proto = getLeftMost(field.getValue()); } + @SuppressWarnings("unused") public void handleFor(HttpField field) { - if (!_forwardedPortAsAuthority && !StringUtil.isEmpty(_forwardedPortHeader)) + if (!getForwardedPortAsAuthority() && !StringUtil.isEmpty(getForwardedPortHeader())) { if (_for == null) _for = new PossiblyPartialHostPort(getLeftMost(field.getValue())); @@ -613,9 +623,10 @@ public class ForwardedRequestCustomizer implements Customizer } } + @SuppressWarnings("unused") public void handlePort(HttpField field) { - if (!_forwardedPortAsAuthority) + if (!getForwardedPortAsAuthority()) { if (_for == null) _for = new PortSetHostPort(_request.getRemoteHost(), field.getIntValue()); @@ -631,12 +642,14 @@ public class ForwardedRequestCustomizer implements Customizer } } + @SuppressWarnings("unused") public void handleHttps(HttpField field) { if (_proto == null && ("on".equalsIgnoreCase(field.getValue()) || "true".equalsIgnoreCase(field.getValue()))) _proto = HttpScheme.HTTPS.asString(); } + @SuppressWarnings("unused") public void handleRFC7239(HttpField field) { addValue(field.getValue()); @@ -652,11 +665,11 @@ public class ForwardedRequestCustomizer implements Customizer switch (name) { case "by": - if (!_proxyAsAuthority) + if (!getProxyAsAuthority()) break; if (value.startsWith("_") || "unknown".equals(value)) break; - if (_proxyAsAuthority && (_host == null || !(_host instanceof Rfc7239HostPort))) + if (_host == null || !(_host instanceof Rfc7239HostPort)) _host = new Rfc7239HostPort(value); break; case "for": 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 ffd1f471ec2..03f4e9077e6 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 @@ -21,6 +21,9 @@ package org.eclipse.jetty.server; import java.io.IOException; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.stream.Stream; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -29,625 +32,630 @@ import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.server.handler.AbstractHandler; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -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.MethodSource; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; public class ForwardedRequestCustomizerTest { - private Server _server; - private LocalConnector _connector; - private RequestHandler _handler; - final AtomicBoolean _wasSecure = new AtomicBoolean(false); - final AtomicReference _sslSession = new AtomicReference<>(); - final AtomicReference _sslCertificate = new AtomicReference<>(); - final AtomicReference _scheme = new AtomicReference<>(); - final AtomicReference _serverName = new AtomicReference<>(); - final AtomicReference _serverPort = new AtomicReference<>(); - final AtomicReference _remoteAddr = new AtomicReference<>(); - final AtomicReference _remotePort = new AtomicReference<>(); - final AtomicReference _requestURL = new AtomicReference<>(); + private Server server; + private RequestHandler handler; + private LocalConnector connector; + private LocalConnector connectorConfigured; + private ForwardedRequestCustomizer customizer; + private ForwardedRequestCustomizer customizerConfigured; - ForwardedRequestCustomizer _customizer; + private static class Actual + { + final AtomicReference scheme = new AtomicReference<>(); + final AtomicBoolean wasSecure = new AtomicBoolean(false); + final AtomicReference serverName = new AtomicReference<>(); + final AtomicReference serverPort = new AtomicReference<>(); + final AtomicReference requestURL = new AtomicReference<>(); + final AtomicReference remoteAddr = new AtomicReference<>(); + final AtomicReference remotePort = new AtomicReference<>(); + final AtomicReference sslSession = new AtomicReference<>(); + final AtomicReference sslCertificate = new AtomicReference<>(); + } + + private Actual actual; @BeforeEach public void init() throws Exception { - _server = new Server(); + server = new Server(); + + // Default behavior Connector HttpConnectionFactory http = new HttpConnectionFactory(); http.setInputBufferSize(1024); http.getHttpConfiguration().setRequestHeaderSize(512); http.getHttpConfiguration().setResponseHeaderSize(512); http.getHttpConfiguration().setOutputBufferSize(2048); - http.getHttpConfiguration().addCustomizer(_customizer = new ForwardedRequestCustomizer()); - _connector = new LocalConnector(_server, http); - _server.addConnector(_connector); - _handler = new RequestHandler(); - _server.setHandler(_handler); + customizer = new ForwardedRequestCustomizer(); + http.getHttpConfiguration().addCustomizer(customizer); + connector = new LocalConnector(server, http); + server.addConnector(connector); - _handler._checker = (request, response) -> + // Configured behavior Connector + http = new HttpConnectionFactory(); + http.setInputBufferSize(1024); + http.getHttpConfiguration().setRequestHeaderSize(512); + http.getHttpConfiguration().setResponseHeaderSize(512); + http.getHttpConfiguration().setOutputBufferSize(2048); + customizerConfigured = new ForwardedRequestCustomizer(); + customizerConfigured.setForwardedHeader("Jetty-Forwarded"); + customizerConfigured.setForwardedHostHeader("Jetty-Forwarded-Host"); + customizerConfigured.setForwardedServerHeader("Jetty-Forwarded-Server"); + customizerConfigured.setForwardedProtoHeader("Jetty-Forwarded-Proto"); + customizerConfigured.setForwardedForHeader("Jetty-Forwarded-For"); + customizerConfigured.setForwardedPortHeader("Jetty-Forwarded-Port"); + customizerConfigured.setForwardedHttpsHeader("Jetty-Proxied-Https"); + customizerConfigured.setForwardedCipherSuiteHeader("Jetty-Proxy-Auth-Cert"); + customizerConfigured.setForwardedSslSessionIdHeader("Jetty-Proxy-Ssl-Id"); + + http.getHttpConfiguration().addCustomizer(customizerConfigured); + connectorConfigured = new LocalConnector(server, http); + server.addConnector(connectorConfigured); + + handler = new RequestHandler(); + server.setHandler(handler); + + handler.requestTester = (request, response) -> { - _wasSecure.set(request.isSecure()); - _sslSession.set(String.valueOf(request.getAttribute("javax.servlet.request.ssl_session_id"))); - _sslCertificate.set(String.valueOf(request.getAttribute("javax.servlet.request.cipher_suite"))); - _scheme.set(request.getScheme()); - _serverName.set(request.getServerName()); - _serverPort.set(request.getServerPort()); - _remoteAddr.set(request.getRemoteAddr()); - _remotePort.set(request.getRemotePort()); - _requestURL.set(request.getRequestURL().toString()); + actual = new Actual(); + actual.wasSecure.set(request.isSecure()); + actual.sslSession.set(String.valueOf(request.getAttribute("javax.servlet.request.ssl_session_id"))); + actual.sslCertificate.set(String.valueOf(request.getAttribute("javax.servlet.request.cipher_suite"))); + actual.scheme.set(request.getScheme()); + actual.serverName.set(request.getServerName()); + actual.serverPort.set(request.getServerPort()); + actual.remoteAddr.set(request.getRemoteAddr()); + actual.remotePort.set(request.getRemotePort()); + actual.requestURL.set(request.getRequestURL().toString()); return true; }; - _server.start(); + server.start(); } @AfterEach public void destroy() throws Exception { - _server.stop(); + server.stop(); } - @Test - public void testHostIpv4() throws Exception + public static Stream cases() { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: 1.2.3.4:2222\n" + - "\n")); - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("1.2.3.4")); - assertThat("serverPort", _serverPort.get(), is(2222)); - assertThat("requestURL", _requestURL.get(), is("http://1.2.3.4:2222/")); + return Stream.of( + // Host IPv4 + Arguments.of( + new Request("IPv4 Host Only") + .headers( + "GET / HTTP/1.1", + "Host: 1.2.3.4:2222" + ), + new Expectations() + .scheme("http").serverName("1.2.3.4").serverPort(2222) + .requestURL("http://1.2.3.4:2222/") + ), + Arguments.of(new Request("IPv6 Host Only") + .headers( + "GET / HTTP/1.1", + "Host: [::1]:2222" + ), + new Expectations() + .scheme("http").serverName("[::1]").serverPort(2222) + .requestURL("http://[::1]:2222/") + ), + Arguments.of(new Request("IPv4 in Request Line") + .headers( + "GET http://1.2.3.4:2222/ HTTP/1.1", + "Host: wrong" + ), + new Expectations() + .scheme("http").serverName("1.2.3.4").serverPort(2222) + .requestURL("http://1.2.3.4:2222/") + ), + Arguments.of(new Request("IPv6 in Request Line") + .headers( + "GET http://[::1]:2222/ HTTP/1.1", + "Host: wrong" + ), + new Expectations() + .scheme("http").serverName("[::1]").serverPort(2222) + .requestURL("http://[::1]:2222/") + ), + + // ================================================================= + // https://tools.ietf.org/html/rfc7239#section-4 - examples of syntax + Arguments.of(new Request("RFC7239 Examples: Section 4") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "Forwarded: for=\"_gazonk\"", + "Forwarded: For=\"[2001:db8:cafe::17]:4711\"", + "Forwarded: for=192.0.2.60;proto=http;by=203.0.113.43", + "Forwarded: for=192.0.2.43, for=198.51.100.17" + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(80) + .requestURL("http://myhost/") + .remoteAddr("[2001:db8:cafe::17]").remotePort(4711) + ), + + // https://tools.ietf.org/html/rfc7239#section-7 - Examples of syntax with regards to HTTP header fields + Arguments.of(new Request("RFC7239 Examples: Section 7") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "Forwarded: for=192.0.2.43,for=\"[2001:db8:cafe::17]\",for=unknown" + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(80) + .requestURL("http://myhost/") + .remoteAddr("192.0.2.43").remotePort(0) + ), + + // (same as above, but with spaces, as shown in RFC section 7.1) + Arguments.of(new Request("RFC7239 Examples: Section 7 (spaced)") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "Forwarded: for=192.0.2.43, for=\"[2001:db8:cafe::17]\", for=unknown" + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(80) + .requestURL("http://myhost/") + .remoteAddr("192.0.2.43").remotePort(0) + ), + + // (same as above, but as multiple headers, as shown in RFC section 7.1) + Arguments.of(new Request("RFC7239 Examples: Section 7 (multi header)") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "Forwarded: for=192.0.2.43", + "Forwarded: for=\"[2001:db8:cafe::17]\", for=unknown" + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(80) + .requestURL("http://myhost/") + .remoteAddr("192.0.2.43").remotePort(0) + ), + + // https://tools.ietf.org/html/rfc7239#section-7.4 - Transition + Arguments.of(new Request("RFC7239 Examples: Section 7.4 (old syntax)") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "X-Forwarded-For: 192.0.2.43, 2001:db8:cafe::17" + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(80) + .requestURL("http://myhost/") + .remoteAddr("192.0.2.43").remotePort(0) + ), + Arguments.of(new Request("RFC7239 Examples: Section 7.4 (new syntax)") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "Forwarded: for=192.0.2.43, for=\"[2001:db8:cafe::17]\"" + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(80) + .requestURL("http://myhost/") + .remoteAddr("192.0.2.43").remotePort(0) + ), + + // https://tools.ietf.org/html/rfc7239#section-7.5 - Example Usage + Arguments.of(new Request("RFC7239 Examples: Section 7.5") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "Forwarded: for=192.0.2.43,for=198.51.100.17;by=203.0.113.60;proto=http;host=example.com" + ), + new Expectations() + .scheme("http").serverName("example.com").serverPort(80) + .requestURL("http://example.com/") + .remoteAddr("192.0.2.43").remotePort(0) + ), + + // Forwarded, proto only + Arguments.of(new Request("RFC7239: Forwarded proto only") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "Forwarded: proto=https" + ), + new Expectations() + .scheme("https").serverName("myhost").serverPort(443) + .requestURL("https://myhost/") + ), + + // ================================================================= + // X-Forwarded-* usages + Arguments.of(new Request("X-Forwarded-Proto (old syntax)") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "X-Forwarded-Proto: https" + ), + new Expectations() + .scheme("https").serverName("myhost").serverPort(443) + .requestURL("https://myhost/") + ), + Arguments.of(new Request("X-Forwarded-For (multiple headers)") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "X-Forwarded-For: 10.9.8.7,6.5.4.3", + "X-Forwarded-For: 8.9.8.7,7.5.4.3" + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(80) + .requestURL("http://myhost/") + .remoteAddr("10.9.8.7").remotePort(0) + ), + Arguments.of(new Request("X-Forwarded-For (IPv4 with port)") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "X-Forwarded-For: 10.9.8.7:1111,6.5.4.3:2222" + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(80) + .requestURL("http://myhost/") + .remoteAddr("10.9.8.7").remotePort(1111) + ), + Arguments.of(new Request("X-Forwarded-For (IPv6 with port)") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "X-Forwarded-For: [2001:db8:cafe::17]:1111,6.5.4.3:2222" + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(80) + .requestURL("http://myhost/") + .remoteAddr("[2001:db8:cafe::17]").remotePort(1111) + ), + Arguments.of(new Request("X-Forwarded-For and X-Forwarded-Port (once)") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "X-Forwarded-For: 1:2:3:4:5:6:7:8", + "X-Forwarded-Port: 2222" + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(2222) + .requestURL("http://myhost:2222/") + .remoteAddr("[1:2:3:4:5:6:7:8]").remotePort(0) + ), + Arguments.of(new Request("X-Forwarded-For and X-Forwarded-Port (multiple times)") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "X-Forwarded-Port: 2222", // this wins + "X-Forwarded-For: 1:2:3:4:5:6:7:8", // this wins + "X-Forwarded-For: 7:7:7:7:7:7:7:7", // ignored + "X-Forwarded-Port: 3333" // ignored + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(2222) + .requestURL("http://myhost:2222/") + .remoteAddr("[1:2:3:4:5:6:7:8]").remotePort(0) + ), + Arguments.of(new Request("X-Forwarded-Port") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "X-Forwarded-Port: 4444", // resets server port + "X-Forwarded-For: 192.168.1.200" + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(4444) + .requestURL("http://myhost:4444/") + .remoteAddr("192.168.1.200").remotePort(0) + ), + Arguments.of(new Request("X-Forwarded-Port (ForwardedPortAsAuthority==false)") + .configureCustomizer((customizer) -> customizer.setForwardedPortAsAuthority(false)) + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "X-Forwarded-Port: 4444", + "X-Forwarded-For: 192.168.1.200" + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(80) + .requestURL("http://myhost/") + .remoteAddr("192.168.1.200").remotePort(4444) + ), + Arguments.of(new Request("X-Forwarded-Port for Late Host header") + .headers( + "GET / HTTP/1.1", + "X-Forwarded-Port: 4444", // this order is intentional + "X-Forwarded-For: 192.168.1.200", + "Host: myhost" // leave this as last header + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(4444) + .requestURL("http://myhost:4444/") + .remoteAddr("192.168.1.200").remotePort(0) + ), + Arguments.of(new Request("X-Forwarded-* (all headers)") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "X-Forwarded-Proto: https", + "X-Forwarded-Host: www.example.com", + "X-Forwarded-Port: 4333", + "X-Forwarded-For: 8.5.4.3:2222", + "X-Forwarded-Server: fw.example.com" + ), + new Expectations() + .scheme("https").serverName("www.example.com").serverPort(4333) + .requestURL("https://www.example.com:4333/") + .remoteAddr("8.5.4.3").remotePort(2222) + ), + + // ================================================================= + // Mixed Behavior + Arguments.of(new Request("RFC7239 mixed with X-Forwarded-* headers") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "X-Forwarded-For: 11.9.8.7:1111,8.5.4.3:2222", + "X-Forwarded-Port: 3333", + "Forwarded: for=192.0.2.43,for=198.51.100.17;by=203.0.113.60;proto=http;host=example.com", + "X-Forwarded-For: 11.9.8.7:1111,8.5.4.3:2222" + ), + new Expectations() + .scheme("http").serverName("example.com").serverPort(80) + .requestURL("http://example.com/") + .remoteAddr("192.0.2.43").remotePort(0) + ), + + // ================================================================= + // Legacy Headers + Arguments.of(new Request("X-Proxied-Https") + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "X-Proxied-Https: on" + ), + new Expectations() + .scheme("https").serverName("myhost").serverPort(443) + .requestURL("https://myhost/") + .remoteAddr("0.0.0.0").remotePort(0) + ), + Arguments.of(new Request("Proxy-Ssl-Id (setSslIsSecure==false)") + .configureCustomizer((customizer) -> customizer.setSslIsSecure(false)) + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "Proxy-Ssl-Id: Wibble" + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(80) + .requestURL("http://myhost/") + .remoteAddr("0.0.0.0").remotePort(0) + .sslSession("Wibble") + ), + Arguments.of(new Request("Proxy-Ssl-Id (setSslIsSecure==true)") + .configureCustomizer((customizer) -> customizer.setSslIsSecure(true)) + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "Proxy-Ssl-Id: 0123456789abcdef" + ), + new Expectations() + .scheme("https").serverName("myhost").serverPort(443) + .requestURL("https://myhost/") + .remoteAddr("0.0.0.0").remotePort(0) + .sslSession("0123456789abcdef") + ), + Arguments.of(new Request("Proxy-Auth-Cert (setSslIsSecure==false)") + .configureCustomizer((customizer) -> customizer.setSslIsSecure(false)) + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "Proxy-auth-cert: Wibble" + ), + new Expectations() + .scheme("http").serverName("myhost").serverPort(80) + .requestURL("http://myhost/") + .remoteAddr("0.0.0.0").remotePort(0) + .sslCertificate("Wibble") + ), + Arguments.of(new Request("Proxy-Auth-Cert (setSslIsSecure==true)") + .configureCustomizer((customizer) -> customizer.setSslIsSecure(true)) + .headers( + "GET / HTTP/1.1", + "Host: myhost", + "Proxy-auth-cert: 0123456789abcdef" + ), + new Expectations() + .scheme("https").serverName("myhost").serverPort(443) + .requestURL("https://myhost/") + .remoteAddr("0.0.0.0").remotePort(0) + .sslCertificate("0123456789abcdef") + ) + ); } - @Test - public void testHostIpv6() throws Exception + @ParameterizedTest(name = "{0}") + @MethodSource("cases") + @SuppressWarnings("unused") + public void testDefaultBehavior(Request request, Expectations expectations) throws Exception { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: [::1]:2222\n" + - "\n")); + request.configure(customizer); + + String rawRequest = request.getRawRequest((header) -> header); + System.out.println(rawRequest); + + HttpTester.Response response = HttpTester.parseResponse(connector.getResponse(rawRequest)); assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("[::1]")); - assertThat("serverPort", _serverPort.get(), is(2222)); - assertThat("requestURL", _requestURL.get(), is("http://[::1]:2222/")); + + expectations.accept(actual); } - @Test - public void testURIIpv4() throws Exception + @ParameterizedTest(name = "{0}") + @MethodSource("cases") + @SuppressWarnings("unused") + public void testConfiguredBehavior(Request request, Expectations expectations) throws Exception { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET http://1.2.3.4:2222/ HTTP/1.1\n" + - "Host: wrong\n" + - "\n")); + request.configure(customizerConfigured); + + String rawRequest = request.getRawRequest((header) -> header + .replaceFirst("X-Forwarded-", "Jetty-Forwarded-") + .replaceFirst("Forwarded:", "Jetty-Forwarded:") + .replaceFirst("X-Proxied-Https:", "Jetty-Proxied-Https:") + .replaceFirst("Proxy-Ssl-Id:", "Jetty-Proxy-Ssl-Id:") + .replaceFirst("Proxy-auth-cert:", "Jetty-Proxy-Auth-Cert:")); + System.out.println(rawRequest); + + HttpTester.Response response = HttpTester.parseResponse(connectorConfigured.getResponse(rawRequest)); assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("1.2.3.4")); - assertThat("serverPort", _serverPort.get(), is(2222)); - assertThat("requestURL", _requestURL.get(), is("http://1.2.3.4:2222/")); + + expectations.accept(actual); } - @Test - public void testURIIpv6() throws Exception + private static class Request { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET http://[::1]:2222/ HTTP/1.1\n" + - "Host: wrong\n" + - "\n")); - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("[::1]")); - assertThat("serverPort", _serverPort.get(), is(2222)); - assertThat("requestURL", _requestURL.get(), is("http://[::1]:2222/")); + String description; + String[] requestHeaders; + Consumer forwardedRequestCustomizerConsumer; + + public Request(String description) + { + this.description = description; + } + + public Request headers(String... headers) + { + this.requestHeaders = headers; + return this; + } + + public Request configureCustomizer(Consumer forwardedRequestCustomizerConsumer) + { + this.forwardedRequestCustomizerConsumer = forwardedRequestCustomizerConsumer; + return this; + } + + public void configure(ForwardedRequestCustomizer customizer) + { + if (forwardedRequestCustomizerConsumer != null) + { + forwardedRequestCustomizerConsumer.accept(customizer); + } + } + + public String getRawRequest(Function headerManip) + { + StringBuilder request = new StringBuilder(); + for (String header : requestHeaders) + { + request.append(headerManip.apply(header)).append('\n'); + } + request.append('\n'); + return request.toString(); + } + + @Override + public String toString() + { + return this.description; + } } - /** - * RFC 7239: Section 4 - * - * Examples of syntax. - */ - @Test - public void testRFC7239_Examples_4() throws Exception + private static class Expectations implements Consumer { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "Forwarded: for=\"_gazonk\"\n" + - "Forwarded: For=\"[2001:db8:cafe::17]:4711\"\n" + - "Forwarded: for=192.0.2.60;proto=http;by=203.0.113.43\n" + - "Forwarded: for=192.0.2.43, for=198.51.100.17\n" + - "\n")); - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(80)); - assertThat("remoteAddr", _remoteAddr.get(), is("[2001:db8:cafe::17]")); - assertThat("remotePort", _remotePort.get(), is(4711)); - assertThat("requestURL", _requestURL.get(), is("http://myhost/")); - } + String expectedScheme; + String expectedServerName; + int expectedServerPort; + String expectedRequestURL; + String expectedRemoteAddr = "0.0.0.0"; + int expectedRemotePort = 0; + String expectedSslSession; + String expectedSslCertificate; - /** - * RFC 7239: Section 7.1 - * - * Examples of syntax with regards to HTTP header fields - */ - @Test - public void testRFC7239_Examples_7_1() throws Exception - { - // Without spaces - HttpTester.Response response1 = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "Forwarded: for=192.0.2.43,for=\"[2001:db8:cafe::17]\",for=unknown\n" + - "\n")); + @Override + public void accept(Actual actual) + { + assertThat("scheme", actual.scheme.get(), is(expectedScheme)); + if (actual.scheme.equals("https")) + { + assertTrue(actual.wasSecure.get(), "wasSecure"); + } + assertThat("serverName", actual.serverName.get(), is(expectedServerName)); + assertThat("serverPort", actual.serverPort.get(), is(expectedServerPort)); + assertThat("requestURL", actual.requestURL.get(), is(expectedRequestURL)); + if (expectedRemoteAddr != null) + { + assertThat("remoteAddr", actual.remoteAddr.get(), is(expectedRemoteAddr)); + assertThat("remotePort", actual.remotePort.get(), is(expectedRemotePort)); + } + if (expectedSslSession != null) + { + assertThat("sslSession", actual.sslSession.get(), is(expectedSslSession)); + } + if (expectedSslCertificate != null) + { + assertThat("sslCertificate", actual.sslCertificate.get(), is(expectedSslCertificate)); + } + } - assertThat("status", response1.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(80)); - assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("http://myhost/")); + public Expectations scheme(String scheme) + { + this.expectedScheme = scheme; + return this; + } - // With spaces - HttpTester.Response response2 = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "Forwarded: for=192.0.2.43, for=\"[2001:db8:cafe::17]\", for=unknown\n" + - "\n")); - assertThat("status", response2.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(80)); - assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("http://myhost/")); + public Expectations serverName(String name) + { + this.expectedServerName = name; + return this; + } - // As multiple headers - HttpTester.Response response3 = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "Forwarded: for=192.0.2.43\n" + - "Forwarded: for=\"[2001:db8:cafe::17]\", for=unknown\n" + - "\n")); + public Expectations serverPort(int port) + { + this.expectedServerPort = port; + return this; + } - assertThat("status", response3.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(80)); - assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("http://myhost/")); - } + public Expectations requestURL(String requestURL) + { + this.expectedRequestURL = requestURL; + return this; + } - /** - * RFC 7239: Section 7.4 - * - * Transition - */ - @Test - public void testRFC7239_Examples_7_4() throws Exception - { - // Old syntax - HttpTester.Response response1 = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "X-Forwarded-For: 192.0.2.43, 2001:db8:cafe::17\n" + - "\n")); + public Expectations remoteAddr(String remoteAddr) + { + this.expectedRemoteAddr = remoteAddr; + return this; + } - assertThat("status", response1.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(80)); - assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("http://myhost/")); + public Expectations remotePort(int remotePort) + { + this.expectedRemotePort = remotePort; + return this; + } - // New syntax - HttpTester.Response response2 = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "Forwarded: for=192.0.2.43, for=\"[2001:db8:cafe::17]\"\n" + - "\n")); + public Expectations sslSession(String sslSession) + { + this.expectedSslSession = sslSession; + return this; + } - assertThat("status", response2.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(80)); - assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("http://myhost/")); - } - - /** - * RFC 7239: Section 7.5 - * - * Example Usage - */ - @Test - public void testRFC7239_Examples_7_5() throws Exception - { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "Forwarded: for=192.0.2.43,for=198.51.100.17;by=203.0.113.60;proto=http;host=example.com\n" + - "\n")); - - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("example.com")); - assertThat("serverPort", _serverPort.get(), is(80)); - assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("http://example.com/")); - } - - @Test - public void testProto_OldSyntax() throws Exception - { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "X-Forwarded-Proto: https\n" + - "\n")); - - assertTrue(_wasSecure.get(), "wasSecure"); - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("https")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(443)); - assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("https://myhost/")); - } - - @Test - public void testRFC7239_Proto() throws Exception - { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "Forwarded: proto=https\n" + - "\n")); - - assertTrue(_wasSecure.get(), "wasSecure"); - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("https")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(443)); - assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("https://myhost/")); - } - - @Test - public void testFor() throws Exception - { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "X-Forwarded-For: 10.9.8.7,6.5.4.3\n" + - "X-Forwarded-For: 8.9.8.7,7.5.4.3\n" + - "\n")); - - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(80)); - assertThat("remoteAddr", _remoteAddr.get(), is("10.9.8.7")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("http://myhost/")); - } - - @Test - public void testForIpv4WithPort() throws Exception - { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "X-Forwarded-For: 10.9.8.7:1111,6.5.4.3:2222\n" + - "\n")); - - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(80)); - assertThat("remoteAddr", _remoteAddr.get(), is("10.9.8.7")); - assertThat("remotePort", _remotePort.get(), is(1111)); - assertThat("requestURL", _requestURL.get(), is("http://myhost/")); - } - - @Test - public void testForIpv6WithPort() throws Exception - { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "X-Forwarded-For: [2001:db8:cafe::17]:1111,6.5.4.3:2222\n" + - "\n")); - - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(80)); - assertThat("remoteAddr", _remoteAddr.get(), is("[2001:db8:cafe::17]")); - assertThat("remotePort", _remotePort.get(), is(1111)); - assertThat("requestURL", _requestURL.get(), is("http://myhost/")); - } - - @Test - public void testForIpv6AndPort() throws Exception - { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "X-Forwarded-For: 1:2:3:4:5:6:7:8\n" + - "X-Forwarded-Port: 2222\n" + - "\n")); - - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(2222)); - assertThat("remoteAddr", _remoteAddr.get(), is("[1:2:3:4:5:6:7:8]")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("http://myhost:2222/")); - } - - @Test - public void testForIpv6AndPort_MultiField() throws Exception - { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "X-Forwarded-Port: 2222\n" + - "X-Forwarded-For: 1:2:3:4:5:6:7:8\n" + - "X-Forwarded-For: 7:7:7:7:7:7:7:7\n" + - "X-Forwarded-Port: 3333\n" + - "\n")); - - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(2222)); - assertThat("remoteAddr", _remoteAddr.get(), is("[1:2:3:4:5:6:7:8]")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("http://myhost:2222/")); - } - - @Test - public void testLegacyProto() throws Exception - { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "X-Proxied-Https: on\n" + - "\n")); - assertTrue(_wasSecure.get(), "wasSecure"); - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("https")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(443)); - assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("https://myhost/")); - } - - @Test - public void testSslSession() throws Exception - { - _customizer.setSslIsSecure(false); - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "Proxy-Ssl-Id: Wibble\n" + - "\n")); - - assertFalse(_wasSecure.get(), "wasSecure"); - assertThat("sslSession", _sslSession.get(), is("Wibble")); - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(80)); - assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("http://myhost/")); - - _customizer.setSslIsSecure(true); - response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "Proxy-Ssl-Id: 0123456789abcdef\n" + - "\n")); - - assertTrue(_wasSecure.get(), "wasSecure"); - assertThat("sslSession", _sslSession.get(), is("0123456789abcdef")); - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("https")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(443)); - assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("https://myhost/")); - } - - @Test - public void testSslCertificate() throws Exception - { - _customizer.setSslIsSecure(false); - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "Proxy-auth-cert: Wibble\n" + - "\n")); - - assertFalse(_wasSecure.get(), "wasSecure"); - assertThat("sslCertificate", _sslCertificate.get(), is("Wibble")); - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(80)); - assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("http://myhost/")); - - _customizer.setSslIsSecure(true); - response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "Proxy-auth-cert: 0123456789abcdef\n" + - "\n")); - - assertTrue(_wasSecure.get(), "wasSecure"); - assertThat("sslCertificate", _sslCertificate.get(), is("0123456789abcdef")); - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("https")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(443)); - assertThat("remoteAddr", _remoteAddr.get(), is("0.0.0.0")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("https://myhost/")); - } - - /** - * Resetting the server port via a forwarding header - */ - @Test - public void testPort_For() throws Exception - { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "X-Forwarded-Port: 4444\n" + - "X-Forwarded-For: 192.168.1.200\n" + - "\n")); - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(4444)); - assertThat("remoteAddr", _remoteAddr.get(), is("192.168.1.200")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("http://myhost:4444/")); - } - - /** - * Resetting the server port via a forwarding header - */ - @Test - public void testRemote_Port_For() throws Exception - { - _customizer.setForwardedPortAsAuthority(false); - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "X-Forwarded-Port: 4444\n" + - "X-Forwarded-For: 192.168.1.200\n" + - "\n")); - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(80)); - assertThat("remoteAddr", _remoteAddr.get(), is("192.168.1.200")); - assertThat("remotePort", _remotePort.get(), is(4444)); - assertThat("requestURL", _requestURL.get(), is("http://myhost/")); - } - - /** - * Test setting the server Port before the "Host" header has been seen. - */ - @Test - public void testPort_For_LateHost() throws Exception - { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "X-Forwarded-Port: 4444\n" + // this order is intentional - "X-Forwarded-For: 192.168.1.200\n" + - "Host: myhost\n" + // leave this as the last header - "\n")); - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("myhost")); - assertThat("serverPort", _serverPort.get(), is(4444)); - assertThat("remoteAddr", _remoteAddr.get(), is("192.168.1.200")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("http://myhost:4444/")); - } - - @Test - public void testMixed_For_Port_RFC_For() throws Exception - { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "X-Forwarded-For: 11.9.8.7:1111,8.5.4.3:2222\n" + - "X-Forwarded-Port: 3333\n" + - "Forwarded: for=192.0.2.43,for=198.51.100.17;by=203.0.113.60;proto=http;host=example.com\n" + - "X-Forwarded-For: 11.9.8.7:1111,8.5.4.3:2222\n" + - "\n")); - - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("http")); - assertThat("serverName", _serverName.get(), is("example.com")); - assertThat("serverPort", _serverPort.get(), is(80)); - assertThat("remoteAddr", _remoteAddr.get(), is("192.0.2.43")); - assertThat("remotePort", _remotePort.get(), is(0)); - assertThat("requestURL", _requestURL.get(), is("http://example.com/")); - } - - @Test - public void testAllXForwarded() throws Exception - { - HttpTester.Response response = HttpTester.parseResponse( - _connector.getResponse( - "GET / HTTP/1.1\n" + - "Host: myhost\n" + - "X-Forwarded-Proto: https\n" + - "X-Forwarded-Host: www.example.com\n" + - "X-Forwarded-Port: 4333\n" + - "X-Forwarded-For: 8.5.4.3:2222\n" + - "X-Forwarded-Server: fw.example.com\n" + - "\n")); - - assertThat("status", response.getStatus(), is(200)); - assertThat("scheme", _scheme.get(), is("https")); - assertThat("serverName", _serverName.get(), is("www.example.com")); - assertThat("serverPort", _serverPort.get(), is(4333)); - assertThat("remoteAddr", _remoteAddr.get(), is("8.5.4.3")); - assertThat("remotePort", _remotePort.get(), is(2222)); - assertThat("requestURL", _requestURL.get(), is("https://www.example.com:4333/")); + public Expectations sslCertificate(String sslCertificate) + { + this.expectedSslCertificate = sslCertificate; + return this; + } } interface RequestTester @@ -657,14 +665,15 @@ public class ForwardedRequestCustomizerTest private class RequestHandler extends AbstractHandler { - private RequestTester _checker; + private RequestTester requestTester; @Override - public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException + public void handle(String target, org.eclipse.jetty.server.Request baseRequest, + HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException { baseRequest.setHandled(true); - if (_checker != null && _checker.check(request, response)) + if (requestTester != null && requestTester.check(request, response)) response.setStatus(200); else response.sendError(500); From 3940baea9c40ed8524a3fd849dfea79e21ff7e1a Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 13 Aug 2019 15:56:11 -0500 Subject: [PATCH 4/9] Fixes #3969 - Adding comments from PR review Signed-off-by: Joakim Erdfelt --- .../org/eclipse/jetty/server/ForwardedRequestCustomizer.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 8937c1286b1..c94ac4ea4e8 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 @@ -455,11 +455,12 @@ public class ForwardedRequestCustomizer implements Customizer int size = 0; MethodHandles.Lookup lookup = MethodHandles.lookup(); + // Loop to grow capacity of ArrayTrie for all headers while (true) { try { - size += 128; + size += 128; // experimented good baseline size _handles = new ArrayTrie<>(size); if (updateForwardedHandle(lookup, getForwardedCipherSuiteHeader(), "handleCipherSuite")) From fec01a46284dfe8ce6f00bfa8d22ae85d2c602f0 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 13 Aug 2019 16:00:18 -0500 Subject: [PATCH 5/9] Fixes #3969 - Changing TYPE to class from PR review Signed-off-by: Joakim Erdfelt --- .../org/eclipse/jetty/server/ForwardedRequestCustomizer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 c94ac4ea4e8..865381f5581 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 @@ -492,7 +492,7 @@ public class ForwardedRequestCustomizer implements Customizer private boolean updateForwardedHandle(MethodHandles.Lookup lookup, String headerName, String forwardedMethodName) throws NoSuchMethodException, IllegalAccessException { - final MethodType type = methodType(Void.TYPE, HttpField.class); + final MethodType type = methodType(void.class, HttpField.class); if (StringUtil.isBlank(headerName)) return false; From cbe34d9bc21f6f05f1e6c072893a1e13d7af1c6c Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 13 Aug 2019 16:05:35 -0500 Subject: [PATCH 6/9] Revert "Jetty 9.4.x release faster (no need of triggering plugins already triggered) (#3944)" + Breaks the release build. javadoc and source artifacts lack gpg signatures This reverts commit 50aa1cf7864f14e572a3a9e5bf20b4232899db4e. --- pom.xml | 24 +++++++++++++++--------- scripts/release-jetty.sh | 2 +- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/pom.xml b/pom.xml index 21867defb2d..de121a55c88 100644 --- a/pom.xml +++ b/pom.xml @@ -318,7 +318,7 @@ attach-sources process-classes - jar-no-fork + jar @@ -562,11 +562,6 @@ - - org.apache.maven.plugins - maven-gpg-plugin - 1.6 - org.apache.maven.plugins maven-javadoc-plugin @@ -1305,16 +1300,28 @@ org.apache.maven.plugins - maven-javadoc-plugin + maven-source-plugin + attach-sources + + jar + + + + + + org.apache.maven.plugins + maven-javadoc-plugin + + + attach-javadocs jar - diff --git a/scripts/release-jetty.sh b/scripts/release-jetty.sh index a98a762814e..57ae4c9afbd 100755 --- a/scripts/release-jetty.sh +++ b/scripts/release-jetty.sh @@ -167,7 +167,7 @@ if proceedyn "Are you sure you want to release using above? (y/N)" n; then # This is equivalent to 'mvn release:perform' if proceedyn "Build/Deploy from tag $TAG_NAME? (Y/n)" y; then git checkout $TAG_NAME - mvn clean package gpg:sign javadoc:aggregate-jar deploy \ + mvn clean package source:jar javadoc:jar gpg:sign javadoc:aggregate-jar deploy \ -Peclipse-release $DEPLOY_OPTS reportMavenTestFailures git checkout $GIT_BRANCH_ID From 8761b345b5fbf32f10cd5e2f554b87253a0b4524 Mon Sep 17 00:00:00 2001 From: Olivier Lamy Date: Wed, 14 Aug 2019 20:30:54 +1000 Subject: [PATCH 7/9] Jetty 9.4.x timeout to build only do not include time to get node (#3975) * fix timeout to apply on build time not on getting node time Signed-off-by: olivier lamy * fix typo Signed-off-by: olivier lamy --- Jenkinsfile | 101 +++++++++++++++++++++++++++------------------------- 1 file changed, 53 insertions(+), 48 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index dd81c80846a..da0aa068230 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -9,85 +9,90 @@ pipeline { parallel { stage("Build / Test - JDK8") { agent { node { label 'linux' } } - options { timeout(time: 120, unit: 'MINUTES') } steps { - mavenBuild("jdk8", "-Pmongodb install", "maven3", true) - // Collect up the jacoco execution results (only on main build) - jacoco inclusionPattern: '**/org/eclipse/jetty/**/*.class', - exclusionPattern: '' + - // build tools - '**/org/eclipse/jetty/ant/**' + - ',**/org/eclipse/jetty/maven/**' + - ',**/org/eclipse/jetty/jspc/**' + - // example code / documentation - ',**/org/eclipse/jetty/embedded/**' + - ',**/org/eclipse/jetty/asyncrest/**' + - ',**/org/eclipse/jetty/demo/**' + - // special environments / late integrations - ',**/org/eclipse/jetty/gcloud/**' + - ',**/org/eclipse/jetty/infinispan/**' + - ',**/org/eclipse/jetty/osgi/**' + - ',**/org/eclipse/jetty/spring/**' + - ',**/org/eclipse/jetty/http/spi/**' + - // test classes - ',**/org/eclipse/jetty/tests/**' + - ',**/org/eclipse/jetty/test/**', - execPattern: '**/target/jacoco.exec', - classPattern: '**/target/classes', - sourcePattern: '**/src/main/java' - warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']] - junit testResults: '**/target/surefire-reports/*.xml,**/target/invoker-reports/TEST*.xml' + timeout(time: 120, unit: 'MINUTES') { + mavenBuild("jdk8", "-Pmongodb install", "maven3", true) + // Collect up the jacoco execution results (only on main build) + jacoco inclusionPattern: '**/org/eclipse/jetty/**/*.class', + exclusionPattern: '' + + // build tools + '**/org/eclipse/jetty/ant/**' + + ',**/org/eclipse/jetty/maven/**' + + ',**/org/eclipse/jetty/jspc/**' + + // example code / documentation + ',**/org/eclipse/jetty/embedded/**' + + ',**/org/eclipse/jetty/asyncrest/**' + + ',**/org/eclipse/jetty/demo/**' + + // special environments / late integrations + ',**/org/eclipse/jetty/gcloud/**' + + ',**/org/eclipse/jetty/infinispan/**' + + ',**/org/eclipse/jetty/osgi/**' + + ',**/org/eclipse/jetty/spring/**' + + ',**/org/eclipse/jetty/http/spi/**' + + // test classes + ',**/org/eclipse/jetty/tests/**' + + ',**/org/eclipse/jetty/test/**', + execPattern: '**/target/jacoco.exec', + classPattern: '**/target/classes', + sourcePattern: '**/src/main/java' + warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']] + junit testResults: '**/target/surefire-reports/*.xml,**/target/invoker-reports/TEST*.xml' + } } } stage("Build / Test - JDK11") { agent { node { label 'linux' } } - options { timeout(time: 120, unit: 'MINUTES') } steps { - mavenBuild("jdk11", "-Pmongodb install", "maven3", true) - warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']] - junit testResults: '**/target/surefire-reports/*.xml,**/target/invoker-reports/TEST*.xml' + timeout(time: 120, unit: 'MINUTES') { + mavenBuild("jdk11", "-Pmongodb install", "maven3", true) + warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']] + junit testResults: '**/target/surefire-reports/*.xml,**/target/invoker-reports/TEST*.xml' + } } } stage("Build / Test - JDK12") { agent { node { label 'linux' } } - options { timeout(time: 120, unit: 'MINUTES') } steps { - mavenBuild("jdk12", "-Pmongodb install", "maven3", true) - warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']] - junit testResults: '**/target/surefire-reports/*.xml,**/target/invoker-reports/TEST*.xml' + timeout(time: 120, unit: 'MINUTES') { + mavenBuild("jdk12", "-Pmongodb install", "maven3", true) + warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']] + junit testResults: '**/target/surefire-reports/*.xml,**/target/invoker-reports/TEST*.xml' + } } } stage("Build Javadoc") { agent { node { label 'linux' } } - options { timeout(time: 30, unit: 'MINUTES') } steps { - mavenBuild("jdk11", "install javadoc:javadoc javadoc:aggregate-jar -DskipTests", "maven3", true) - warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'JavaDoc'], [parserName: 'Java']] + timeout(time: 30, unit: 'MINUTES') { + mavenBuild("jdk11", "install javadoc:javadoc javadoc:aggregate-jar -DskipTests", "maven3", true) + warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'JavaDoc'], [parserName: 'Java']] + } } } stage("Checkstyle ") { agent { node { label 'linux' } } - options { timeout(time: 30, unit: 'MINUTES') } steps { - mavenBuild("jdk11", "install -f build-resources", "maven3", true) - mavenBuild("jdk11", "install checkstyle:check -DskipTests", "maven3", true) - recordIssues( - enabledForFailure: true, aggregatingResults: true, - tools: [java(), checkStyle(pattern: '**/target/checkstyle-result.xml', reportEncoding: 'UTF-8')] - ) + timeout(time: 30, unit: 'MINUTES') { + mavenBuild("jdk11", "install -f build-resources", "maven3", true) + mavenBuild("jdk11", "install checkstyle:check -DskipTests", "maven3", true) + recordIssues( + enabledForFailure: true, aggregatingResults: true, + tools: [java(), checkStyle(pattern: '**/target/checkstyle-result.xml', reportEncoding: 'UTF-8')]) + } } } stage("Build Compact3") { agent { node { label 'linux' } } - options { timeout(time: 120, unit: 'MINUTES') } steps { - mavenBuild("jdk8", "-Pcompact3 install -DskipTests", "maven3", true) - warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']] + timeout(time: 30, unit: 'MINUTES') { + mavenBuild("jdk8", "-Pcompact3 install -DskipTests", "maven3", true) + warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']] + } } } } From 2a109dccbcd907d8680d0ec47293402665ec5f8f Mon Sep 17 00:00:00 2001 From: Lachlan Date: Wed, 14 Aug 2019 21:28:35 +1000 Subject: [PATCH 8/9] Issue #3968 - prevent ReadPending and ISE from AbstractWebSocketConnection (#3979) * Issue #3968 - websocket suspend fix and cleanups Signed-off-by: Lachlan Roberts * Issue #3968 - fixed race conditions when using websocket ReadState combine the previous ReadMode into ReadState by using ReadState.Action which is returned from ReadState.getAction(ByteBuffer) where an atomic decision is made of what action to do Signed-off-by: Lachlan Roberts --- .../io/AbstractWebSocketConnection.java | 231 +++++++----------- .../jetty/websocket/common/io/ReadState.java | 140 +++++++---- .../websocket/common/io/ReadStateTest.java | 28 +-- 3 files changed, 185 insertions(+), 214 deletions(-) diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/io/AbstractWebSocketConnection.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/io/AbstractWebSocketConnection.java index 2e573f8ddef..fa100fe8ef3 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/io/AbstractWebSocketConnection.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/io/AbstractWebSocketConnection.java @@ -121,13 +121,6 @@ public abstract class AbstractWebSocketConnection extends AbstractConnection imp } } - private enum ReadMode - { - PARSE, - DISCARD, - EOF - } - private static final Logger LOG = Log.getLogger(AbstractWebSocketConnection.class); private static final AtomicLong ID_GEN = new AtomicLong(0); @@ -148,7 +141,6 @@ public abstract class AbstractWebSocketConnection extends AbstractConnection imp private WebSocketSession session; private List extensions = new ArrayList<>(); private ByteBuffer prefillBuffer; - private ReadMode readMode = ReadMode.PARSE; private Stats stats = new Stats(); private CloseInfo fatalCloseInfo; @@ -420,10 +412,11 @@ public abstract class AbstractWebSocketConnection extends AbstractConnection imp public void onFillable() { if (LOG.isDebugEnabled()) - { LOG.debug("{} onFillable()", policy.getBehavior()); - } + stats.countOnFillableEvents.incrementAndGet(); + if (readState.getBuffer() != null) + throw new IllegalStateException(); ByteBuffer buffer = bufferPool.acquire(getInputBufferSize(), true); onFillable(buffer); } @@ -431,39 +424,93 @@ public abstract class AbstractWebSocketConnection extends AbstractConnection imp private void onFillable(ByteBuffer buffer) { if (LOG.isDebugEnabled()) - { LOG.debug("{} onFillable(ByteBuffer): {}", policy.getBehavior(), buffer); - } - try + while (true) { - if (readMode == ReadMode.PARSE) - readMode = readParse(buffer); - else - readMode = readDiscard(buffer); - } - catch (Throwable t) - { - bufferPool.release(buffer); - throw t; - } + ReadState.Action action = readState.getAction(buffer); + if (LOG.isDebugEnabled()) + LOG.debug("ReadState Action: {}", action); - if (readMode == ReadMode.EOF) - { - bufferPool.release(buffer); - readState.eof(); + switch (action) + { + case PARSE: + try + { + parser.parseSingleFrame(buffer); + } + catch (Throwable t) + { + close(t); + readState.discard(); + } + break; - // Handle case where the remote connection was abruptly terminated without a close frame - CloseInfo close = new CloseInfo(StatusCode.SHUTDOWN); - close(close, new DisconnectCallback(this)); - } - else if (!readState.suspend()) - { - bufferPool.release(buffer); - fillInterested(); + case FILL: + try + { + int filled = getEndPoint().fill(buffer); + if (filled < 0) + { + readState.eof(); + break; + } + if (filled == 0) + { + // Done reading, wait for next onFillable + bufferPool.release(buffer); + fillInterested(); + return; + } + + if (LOG.isDebugEnabled()) + LOG.debug("Filled {} bytes - {}", filled, BufferUtil.toDetailString(buffer)); + } + catch (IOException e) + { + close(e); + readState.eof(); + } + break; + + case DISCARD: + if (LOG.isDebugEnabled()) + LOG.debug("Discarded buffer - {}", BufferUtil.toDetailString(buffer)); + buffer.clear(); + break; + + case SUSPEND: + return; + + case EOF: + bufferPool.release(buffer); + + // Handle case where the remote connection was abruptly terminated without a close frame + CloseInfo close = new CloseInfo(StatusCode.SHUTDOWN); + close(close, new DisconnectCallback(this)); + return; + + default: + throw new IllegalStateException(action.name()); + } } } + @Override + public void resume() + { + ByteBuffer resume = readState.resume(); + if (resume != null) + onFillable(resume); + } + + @Override + public SuspendToken suspend() + { + readState.suspending(); + return this; + } + @Override protected void onFillInterestedFailed(Throwable cause) { @@ -517,120 +564,6 @@ public abstract class AbstractWebSocketConnection extends AbstractConnection imp } } - private ReadMode readDiscard(ByteBuffer buffer) - { - EndPoint endPoint = getEndPoint(); - try - { - while (true) - { - int filled = endPoint.fill(buffer); - if (filled == 0) - { - return ReadMode.DISCARD; - } - else if (filled < 0) - { - if (LOG.isDebugEnabled()) - { - LOG.debug("read - EOF Reached (remote: {})", getRemoteAddress()); - } - return ReadMode.EOF; - } - else - { - if (LOG.isDebugEnabled()) - { - LOG.debug("Discarded {} bytes - {}", filled, BufferUtil.toDetailString(buffer)); - } - } - } - } - catch (IOException e) - { - LOG.ignore(e); - return ReadMode.EOF; - } - catch (Throwable t) - { - LOG.ignore(t); - return ReadMode.DISCARD; - } - } - - private ReadMode readParse(ByteBuffer buffer) - { - EndPoint endPoint = getEndPoint(); - try - { - // Process the content from the Endpoint next - while (true) - { - // We may start with a non empty buffer, consume before filling - while (buffer.hasRemaining()) - { - if (readState.suspendParse(buffer)) - { - if (LOG.isDebugEnabled()) - { - LOG.debug("suspending parse {}", buffer); - } - - return ReadMode.PARSE; - } - else - parser.parseSingleFrame(buffer); - } - - int filled = endPoint.fill(buffer); - if (filled < 0) - { - if (LOG.isDebugEnabled()) - { - LOG.debug("read - EOF Reached (remote: {})", getRemoteAddress()); - } - return ReadMode.EOF; - } - else if (filled == 0) - { - // Done reading, wait for next onFillable - return ReadMode.PARSE; - } - - if (LOG.isDebugEnabled()) - { - LOG.debug("Filled {} bytes - {}", filled, BufferUtil.toDetailString(buffer)); - } - } - } - catch (Throwable t) - { - close(t); - return ReadMode.DISCARD; - } - } - - @Override - public void resume() - { - ByteBuffer resume = readState.resume(); - if (resume == null) - { - fillInterested(); - } - else if (resume != ReadState.NO_ACTION) - { - onFillable(resume); - } - } - - @Override - public SuspendToken suspend() - { - readState.suspending(); - return this; - } - /** * Get the list of extensions in use. *

diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/io/ReadState.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/io/ReadState.java index 7f2bb7fa9bc..5b7bcf9e4e9 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/io/ReadState.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/io/ReadState.java @@ -21,14 +21,26 @@ package org.eclipse.jetty.websocket.common.io; import java.nio.ByteBuffer; import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.log.Log; +import org.eclipse.jetty.util.log.Logger; class ReadState { + private static final Logger LOG = Log.getLogger(ReadState.class); + public static final ByteBuffer NO_ACTION = BufferUtil.EMPTY_BUFFER; private State state = State.READING; private ByteBuffer buffer; + public ByteBuffer getBuffer() + { + synchronized (this) + { + return buffer; + } + } + boolean isReading() { synchronized (this) @@ -45,8 +57,38 @@ class ReadState } } + public Action getAction(ByteBuffer buffer) + { + synchronized (this) + { + if (LOG.isDebugEnabled()) + LOG.debug("{} getAction({})", this, BufferUtil.toDetailString(buffer)); + + switch (state) + { + case READING: + return buffer.hasRemaining() ? Action.PARSE : Action.FILL; + + case SUSPENDING: + this.buffer = buffer; + this.state = State.SUSPENDED; + return Action.SUSPEND; + + case EOF: + return Action.EOF; + + case DISCARDING: + return buffer.hasRemaining() ? Action.DISCARD : Action.FILL; + + case SUSPENDED: + default: + throw new IllegalStateException(toString(state)); + } + } + } + /** - * Requests that reads from the connection be suspended when {@link #suspend()} is called. + * Requests that reads from the connection be suspended. * * @return whether the suspending was successful */ @@ -54,6 +96,9 @@ class ReadState { synchronized (this) { + if (LOG.isDebugEnabled()) + LOG.debug("suspending {}", state); + switch (state) { case READING: @@ -67,52 +112,6 @@ class ReadState } } - public boolean suspendParse(ByteBuffer buffer) - { - synchronized (this) - { - switch (state) - { - case READING: - return false; - case SUSPENDING: - this.buffer = buffer; - this.state = State.SUSPENDED; - return true; - default: - throw new IllegalStateException(toString(state)); - } - } - } - - /** - * Suspends reads from the connection if {@link #suspending()} was called. - * - * @return whether reads from the connection should be suspended - */ - boolean suspend() - { - synchronized (this) - { - switch (state) - { - case READING: - return false; - case SUSPENDING: - state = State.SUSPENDED; - return true; - case SUSPENDED: - if (buffer == null) - throw new IllegalStateException(); - return true; - case EOF: - return true; - default: - throw new IllegalStateException(toString(state)); - } - } - } - /** * @return a ByteBuffer to finish processing, or null if we should register fillInterested * If return value is {@link BufferUtil#EMPTY_BUFFER} no action should be taken. @@ -121,18 +120,21 @@ class ReadState { synchronized (this) { + if (LOG.isDebugEnabled()) + LOG.debug("resuming {}", state); + switch (state) { case SUSPENDING: state = State.READING; - return NO_ACTION; + return null; case SUSPENDED: state = State.READING; ByteBuffer bb = buffer; buffer = null; return bb; case EOF: - return NO_ACTION; + return null; default: throw new IllegalStateException(toString(state)); } @@ -143,10 +145,36 @@ class ReadState { synchronized (this) { + if (LOG.isDebugEnabled()) + LOG.debug("eof {}", state); + state = State.EOF; } } + public void discard() + { + synchronized (this) + { + if (LOG.isDebugEnabled()) + LOG.debug("discard {}", state); + + switch (state) + { + case READING: + case SUSPENDED: + case SUSPENDING: + state = State.DISCARDING; + break; + + case DISCARDING: + case EOF: + default: + throw new IllegalStateException(toString(state)); + } + } + } + private String toString(State state) { return String.format("%s@%x[%s]", getClass().getSimpleName(), hashCode(), state); @@ -161,6 +189,15 @@ class ReadState } } + public enum Action + { + FILL, + PARSE, + DISCARD, + SUSPEND, + EOF + } + private enum State { /** @@ -178,6 +215,11 @@ class ReadState */ SUSPENDED, + /** + * Reading from connection and discarding bytes until EOF. + */ + DISCARDING, + /** * Won't read from the connection (terminal state). */ diff --git a/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/io/ReadStateTest.java b/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/io/ReadStateTest.java index d7f93ee1128..5fdc72e7e77 100644 --- a/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/io/ReadStateTest.java +++ b/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/io/ReadStateTest.java @@ -18,6 +18,9 @@ package org.eclipse.jetty.websocket.common.io; +import java.nio.ByteBuffer; + +import org.eclipse.jetty.util.BufferUtil; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; @@ -34,7 +37,7 @@ public class ReadStateTest ReadState readState = new ReadState(); assertThat("Initially reading", readState.isReading(), is(true)); - assertThat("No prior suspending", readState.suspend(), is(false)); + assertThat("Action is reading", readState.getAction(BufferUtil.toBuffer("content")), is(ReadState.Action.PARSE)); assertThat("No prior suspending", readState.isSuspended(), is(false)); assertThrows(IllegalStateException.class, readState::resume, "No suspending to resume"); @@ -50,10 +53,8 @@ public class ReadStateTest assertTrue(readState.suspending()); assertThat("Suspending doesn't take effect immediately", readState.isSuspended(), is(false)); - assertThat("Resume from suspending requires no followup", readState.resume(), is(ReadState.NO_ACTION)); - assertThat("Resume from suspending requires no followup", readState.isSuspended(), is(false)); - - assertThat("Suspending was discarded", readState.suspend(), is(false)); + assertNull(readState.resume()); + assertThat("Action is reading", readState.getAction(BufferUtil.toBuffer("content")), is(ReadState.Action.PARSE)); assertThat("Suspending was discarded", readState.isSuspended(), is(false)); } @@ -66,10 +67,11 @@ public class ReadStateTest assertThat(readState.suspending(), is(true)); assertThat("Suspending doesn't take effect immediately", readState.isSuspended(), is(false)); - assertThat("Suspended", readState.suspend(), is(true)); + ByteBuffer content = BufferUtil.toBuffer("content"); + assertThat(readState.getAction(content), is(ReadState.Action.SUSPEND)); assertThat("Suspended", readState.isSuspended(), is(true)); - assertNull(readState.resume(), "Resumed"); + assertThat(readState.resume(), is(content)); assertThat("Resumed", readState.isSuspended(), is(false)); } @@ -77,19 +79,13 @@ public class ReadStateTest public void testEof() { ReadState readState = new ReadState(); + ByteBuffer content = BufferUtil.toBuffer("content"); readState.eof(); assertThat(readState.isReading(), is(false)); assertThat(readState.isSuspended(), is(true)); - assertThat(readState.suspend(), is(true)); - assertThat(readState.suspending(), is(false)); - assertThat(readState.isSuspended(), is(true)); - - assertThat(readState.suspend(), is(true)); - assertThat(readState.isSuspended(), is(true)); - - assertThat(readState.resume(), is(ReadState.NO_ACTION)); - assertThat(readState.isSuspended(), is(true)); + assertThat(readState.getAction(content), is(ReadState.Action.EOF)); + assertNull(readState.resume()); } } From 47759b3f9bfee8ef87ada59f1cd0bfc0f5cdab33 Mon Sep 17 00:00:00 2001 From: Chris Walker Date: Wed, 14 Aug 2019 10:23:22 -0400 Subject: [PATCH 9/9] Updated security documentation with latest CVEs. Resolves #3980 --- .../reference/troubleshooting/security-reports.adoc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/jetty-documentation/src/main/asciidoc/reference/troubleshooting/security-reports.adoc b/jetty-documentation/src/main/asciidoc/reference/troubleshooting/security-reports.adoc index 658bfd190a5..34eed7ee5e7 100644 --- a/jetty-documentation/src/main/asciidoc/reference/troubleshooting/security-reports.adoc +++ b/jetty-documentation/src/main/asciidoc/reference/troubleshooting/security-reports.adoc @@ -28,6 +28,15 @@ If you would like to report a security issue please follow these link:#security- |======================================================================= |yyyy/mm/dd |ID |Exploitable |Severity |Affects |Fixed Version |Comment +|2019/04/11 |CVE-2019-10247 |Med |Med |< = 9.4.16 |9.2.28, 9.3.27, 9.4.17 +|https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-10247[If no webapp was mounted to the root namespace and a 404 was encountered, an HTML page would be generated displaying the fully qualified base resource location for each context.] + +|2019/04/11 |CVE-2019-10246 |High |High |< = 9.4.16 |9.2.28, 9.3.27, 9.4.17 +|https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-10246[Use of `DefaultServlet` or `ResourceHandler` with indexing was vulnerable to XSS behaviors to expose the directory listing on Windows operating systems.] + +|2019/04/11 |CVE-2019-10241 |High |High |< = 9.4.15 |9.2.27, 9.3.26, 9.4.16 +|https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-10241[Use of `DefaultServlet` or `ResourceHandler` with indexing was vulnerable to XSS behaviors to expose the directory listing.] + |2018/06/25 |CVE-2018-12538 |High |High |>= 9.4.0, < = 9.4.8 |9.4.9 |https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-12538[`HttpSessions` present specifically in the FileSystem’s storage could be hijacked/accessed by an unauthorized user.]