From d3c7ee3d71c57a32336481df7246c49ff51282b1 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 9 Sep 2022 14:58:51 -0500 Subject: [PATCH 1/3] Issue #8578 - restore backward compat of getRequestURL and getRequestURI when working with CONNECT method Signed-off-by: Joakim Erdfelt --- .../org/eclipse/jetty/server/Request.java | 16 ++++++++++++-- .../org/eclipse/jetty/server/RequestTest.java | 21 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index dd67bf9bc7e1..b612dde55659 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -1341,7 +1341,17 @@ public String getRequestedSessionId() public String getRequestURI() { MetaData.Request metadata = _metaData; - return (metadata == null) ? null : metadata.getURI().getPath(); + if (metadata == null) + return null; + HttpURI uri = metadata.getURI(); + if (uri == null) + return null; + // maintain backward compat for CONNECT method + if (HttpMethod.CONNECT.is(getMethod())) + return uri.getAuthority(); + else + // spec compliant path + return uri.getPath(); } /* @@ -1352,7 +1362,9 @@ public StringBuffer getRequestURL() { final StringBuffer url = new StringBuffer(128); URIUtil.appendSchemeHostPort(url, getScheme(), getServerName(), getServerPort()); - url.append(getRequestURI()); + // only add RequestURI if not a CONNECT method + if (!HttpMethod.CONNECT.is(getMethod())) + url.append(getRequestURI()); return url; } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java index 9cbd7801264f..3c79473aedb0 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -55,6 +55,7 @@ import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpComplianceSection; +import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.http.MimeTypes; import org.eclipse.jetty.io.Connection; @@ -872,6 +873,26 @@ public boolean check(HttpServletRequest request, HttpServletResponse response) assertEquals(" x=z; ", results.get(i++)); } + @Test + public void testConnectRequestURL() throws Exception + { + final AtomicReference result = new AtomicReference<>(); + _handler._checker = (request, response) -> + { + result.set("" + request.getRequestURL()); + return true; + }; + + String rawResponse = _connector.getResponse( + "CONNECT myhost:9999 HTTP/1.1\n" + + "Host: myhost:9999\n" + + "Connection: close\n" + + "\n"); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + assertThat(result.get(), is("http://myhost:9999")); + } + @Test public void testHostPort() throws Exception { From 48c16deb21efd67d369675a9126e68459fdc9408 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 9 Sep 2022 15:11:24 -0500 Subject: [PATCH 2/3] Issue #8578 - test both request URL/URI results Signed-off-by: Joakim Erdfelt --- .../test/java/org/eclipse/jetty/server/RequestTest.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java index 3c79473aedb0..6e2719dcef56 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -876,10 +876,12 @@ public boolean check(HttpServletRequest request, HttpServletResponse response) @Test public void testConnectRequestURL() throws Exception { - final AtomicReference result = new AtomicReference<>(); + final AtomicReference resultRequestURL = new AtomicReference<>(); + final AtomicReference resultRequestURI = new AtomicReference<>(); _handler._checker = (request, response) -> { - result.set("" + request.getRequestURL()); + resultRequestURL.set("" + request.getRequestURL()); + resultRequestURI.set("" + request.getRequestURI()); return true; }; @@ -890,7 +892,8 @@ public void testConnectRequestURL() throws Exception "\n"); HttpTester.Response response = HttpTester.parseResponse(rawResponse); assertThat(response.getStatus(), is(HttpStatus.OK_200)); - assertThat(result.get(), is("http://myhost:9999")); + assertThat(resultRequestURL.get(), is("http://myhost:9999")); + assertThat(resultRequestURI.get(), is("myhost:9999")); } @Test From 5944ff4b3a0aa0b9c2a5ad4048fd497e6d7a23cf Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 12 Sep 2022 09:37:49 -0500 Subject: [PATCH 3/3] Issue #8578 - Changes from review Signed-off-by: Joakim Erdfelt --- .../jetty/server/HttpChannelOverHttp.java | 6 ++-- .../org/eclipse/jetty/server/Request.java | 18 +++------- .../org/eclipse/jetty/server/RequestTest.java | 34 ++++++++++++++++--- 3 files changed, 36 insertions(+), 22 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java index 758ad5d406d7..7aca86b74dd6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java @@ -102,10 +102,10 @@ public boolean isExpecting102Processing() } @Override - public boolean startRequest(String method, String uri, HttpVersion version) + public boolean startRequest(String method, String requestTarget, HttpVersion version) { _metadata.setMethod(method); - _metadata.getURI().parseRequestTarget(method, uri); + _metadata.getURI().parseRequestTarget(method, requestTarget); _metadata.setHttpVersion(version); _unknownExpectation = false; _expect100Continue = false; @@ -127,7 +127,7 @@ public void parsedHeader(HttpField field) break; case HOST: - if (!_metadata.getURI().isAbsolute() && field instanceof HostPortHttpField) + if (!HttpMethod.CONNECT.is(_metadata.getMethod()) && !_metadata.getURI().isAbsolute() && field instanceof HostPortHttpField) { HostPortHttpField hp = (HostPortHttpField)field; _metadata.getURI().setAuthority(hp.getHost(), hp.getPort()); diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index b612dde55659..4d66f75eb8f7 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -1341,17 +1341,7 @@ public String getRequestedSessionId() public String getRequestURI() { MetaData.Request metadata = _metaData; - if (metadata == null) - return null; - HttpURI uri = metadata.getURI(); - if (uri == null) - return null; - // maintain backward compat for CONNECT method - if (HttpMethod.CONNECT.is(getMethod())) - return uri.getAuthority(); - else - // spec compliant path - return uri.getPath(); + return (metadata == null) ? null : metadata.getURI().getPath(); } /* @@ -1362,9 +1352,9 @@ public StringBuffer getRequestURL() { final StringBuffer url = new StringBuffer(128); URIUtil.appendSchemeHostPort(url, getScheme(), getServerName(), getServerPort()); - // only add RequestURI if not a CONNECT method - if (!HttpMethod.CONNECT.is(getMethod())) - url.append(getRequestURI()); + String path = getRequestURI(); + if (path != null) + url.append(path); return url; } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java index 6e2719dcef56..f77f39b63018 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -84,6 +84,7 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.Matchers.startsWith; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -874,14 +875,14 @@ public boolean check(HttpServletRequest request, HttpServletResponse response) } @Test - public void testConnectRequestURL() throws Exception + public void testConnectRequestURLSameAsHost() throws Exception { final AtomicReference resultRequestURL = new AtomicReference<>(); final AtomicReference resultRequestURI = new AtomicReference<>(); _handler._checker = (request, response) -> { - resultRequestURL.set("" + request.getRequestURL()); - resultRequestURI.set("" + request.getRequestURI()); + resultRequestURL.set(request.getRequestURL().toString()); + resultRequestURI.set(request.getRequestURI()); return true; }; @@ -892,8 +893,31 @@ public void testConnectRequestURL() throws Exception "\n"); HttpTester.Response response = HttpTester.parseResponse(rawResponse); assertThat(response.getStatus(), is(HttpStatus.OK_200)); - assertThat(resultRequestURL.get(), is("http://myhost:9999")); - assertThat(resultRequestURI.get(), is("myhost:9999")); + assertThat("request.getRequestURL", resultRequestURL.get(), is("http://myhost:9999")); + assertThat("request.getRequestURI", resultRequestURI.get(), is(nullValue())); + } + + @Test + public void testConnectRequestURLDifferentThanHost() throws Exception + { + final AtomicReference resultRequestURL = new AtomicReference<>(); + final AtomicReference resultRequestURI = new AtomicReference<>(); + _handler._checker = (request, response) -> + { + resultRequestURL.set(request.getRequestURL().toString()); + resultRequestURI.set(request.getRequestURI()); + return true; + }; + + String rawResponse = _connector.getResponse( + "CONNECT myhost:9999 HTTP/1.1\n" + + "Host: otherhost:8888\n" + // per spec, this is ignored if request-target is authority-form + "Connection: close\n" + + "\n"); + HttpTester.Response response = HttpTester.parseResponse(rawResponse); + assertThat(response.getStatus(), is(HttpStatus.OK_200)); + assertThat("request.getRequestURL", resultRequestURL.get(), is("http://myhost:9999")); + assertThat("request.getRequestURI", resultRequestURI.get(), is(nullValue())); } @Test