From a6a386a6cc4bd32d5117ff740ff3657ea646ffac Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Fri, 9 Sep 2022 15:11:46 -0500 Subject: [PATCH 1/2] 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 | 15 ++++++++++-- .../org/eclipse/jetty/server/RequestTest.java | 24 +++++++++++++++++++ 2 files changed, 37 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 55f9b297aa3d..082d05e73397 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 @@ -1278,7 +1278,12 @@ public String getRequestedSessionId() @Override public String getRequestURI() { - return _uri == null ? null : _uri.getPath(); + if (_uri == null) + return null; + if (HttpMethod.CONNECT.is(getMethod())) + return _uri.getAuthority(); + else + return _uri.getPath(); } @Override @@ -1286,7 +1291,13 @@ 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())) + { + String requestURI = getRequestURI(); + if (requestURI != null) + url.append(requestURI); + } 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 f2b58d9395b9..71b226bfac59 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 @@ -57,6 +57,7 @@ import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; +import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.HttpVersion; @@ -856,6 +857,29 @@ public void testContentTypeEncoding() throws Exception assertEquals(" x=z; ", results.get(i)); } + @Test + public void testConnectRequestURL() throws Exception + { + final AtomicReference resultRequestURL = new AtomicReference<>(); + final AtomicReference resultRequestURI = new AtomicReference<>(); + _handler._checker = (request, response) -> + { + resultRequestURL.set("" + request.getRequestURL()); + resultRequestURI.set("" + request.getRequestURI()); + 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(resultRequestURL.get(), is("http://myhost:9999")); + assertThat(resultRequestURI.get(), is("myhost:9999")); + } + @Test public void testHostPort() throws Exception { From 8eba1c9924cbb3578f7c6600afecd26864c8f3eb Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Mon, 12 Sep 2022 09:55:14 -0500 Subject: [PATCH 2/2] Issue #8578 - Changes from review Signed-off-by: Joakim Erdfelt --- .../org/eclipse/jetty/server/Request.java | 17 +++------- .../org/eclipse/jetty/server/RequestTest.java | 33 ++++++++++++++++--- 2 files changed, 32 insertions(+), 18 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 082d05e73397..e0929a2a191b 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 @@ -1278,12 +1278,7 @@ public String getRequestedSessionId() @Override public String getRequestURI() { - if (_uri == null) - return null; - if (HttpMethod.CONNECT.is(getMethod())) - return _uri.getAuthority(); - else - return _uri.getPath(); + return _uri == null ? null : _uri.getPath(); } @Override @@ -1291,13 +1286,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())) - { - String requestURI = getRequestURI(); - if (requestURI != null) - url.append(requestURI); - } + 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 71b226bfac59..e9658d62198e 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 @@ -858,14 +858,14 @@ public void testContentTypeEncoding() throws Exception } @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; }; @@ -876,8 +876,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("/")); + } + + @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("/")); } @Test