Issue #8578 - Changes from review

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
jetty · Sep 12, 2022 · 5944ff4 · 5944ff4
1 parent 48c16de
commit 5944ff4
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 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
@@ -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
@@ -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
@@ -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<String> resultRequestURL = new AtomicReference<>();
         final AtomicReference<String> 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<String> resultRequestURL = new AtomicReference<>();
+        final AtomicReference<String> 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