diff --git a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java index 1515986a26f8..405dc919eeb5 100644 --- a/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java +++ b/jetty-client/src/main/java/org/eclipse/jetty/client/HttpRequest.java @@ -195,6 +195,8 @@ public Request path(String path) String rawPath = uri.getRawPath(); if (rawPath == null) rawPath = ""; + if (!rawPath.startsWith("/")) + rawPath = "/" + rawPath; this.path = rawPath; String query = uri.getRawQuery(); if (query != null) @@ -949,14 +951,14 @@ private URI buildURI(boolean withQuery) return result; } - private URI newURI(String uri) + private URI newURI(String path) { try { // Handle specially the "OPTIONS *" case, since it is possible to create a URI from "*" (!). - if ("*".equals(uri)) + if ("*".equals(path)) return null; - URI result = new URI(uri); + URI result = new URI(path); return result.isOpaque() ? null : result; } catch (URISyntaxException x) diff --git a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientURITest.java b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientURITest.java index ebca9e2bc403..b1cbb937d655 100644 --- a/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientURITest.java +++ b/jetty-client/src/test/java/org/eclipse/jetty/client/HttpClientURITest.java @@ -24,8 +24,10 @@ import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.Locale; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -36,6 +38,8 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpMethod; import org.eclipse.jetty.http.HttpStatus; +import org.eclipse.jetty.http.UriCompliance; +import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.toolchain.test.Net; import org.eclipse.jetty.util.Fields; @@ -77,6 +81,52 @@ public void testIPv6Host(Scenario scenario) throws Exception assertEquals(HttpStatus.OK_200, request.send().getStatus()); } + @ParameterizedTest + @ArgumentsSource(ScenarioProvider.class) + public void testPathWithPathParameter(Scenario scenario) throws Exception + { + AtomicReference serverLatchRef = new AtomicReference<>(); + start(scenario, new EmptyServerHandler() + { + @Override + protected void service(String target, org.eclipse.jetty.server.Request jettyRequest, HttpServletRequest request, HttpServletResponse response) + { + if (jettyRequest.getHttpURI().hasAmbiguousEmptySegment()) + response.setStatus(400); + serverLatchRef.get().countDown(); + } + }); + // Allow empty segments to test them. + connector.getContainedBeans(HttpConfiguration.class) + .forEach(httpConfig -> httpConfig.setUriCompliance(UriCompliance.from("DEFAULT,AMBIGUOUS_EMPTY_SEGMENT"))); + + serverLatchRef.set(new CountDownLatch(1)); + ContentResponse response1 = client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .path("/url;p=v") + .send(); + assertEquals(HttpStatus.OK_200, response1.getStatus()); + assertTrue(serverLatchRef.get().await(5, TimeUnit.SECONDS)); + + // Ambiguous empty segment. + serverLatchRef.set(new CountDownLatch(1)); + ContentResponse response2 = client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .path(";p=v/url") + .send(); + assertEquals(HttpStatus.BAD_REQUEST_400, response2.getStatus()); + assertTrue(serverLatchRef.get().await(5, TimeUnit.SECONDS)); + + // Ambiguous empty segment. + serverLatchRef.set(new CountDownLatch(1)); + ContentResponse response3 = client.newRequest("localhost", connector.getLocalPort()) + .scheme(scenario.getScheme()) + .path(";@host.org/url") + .send(); + assertEquals(HttpStatus.BAD_REQUEST_400, response3.getStatus()); + assertTrue(serverLatchRef.get().await(5, TimeUnit.SECONDS)); + } + @ParameterizedTest @ArgumentsSource(ScenarioProvider.class) public void testIDNHost(Scenario scenario) throws Exception diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java index 9423e66db9d2..12e121aea8d3 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java @@ -119,7 +119,7 @@ static Immutable from(String uri) static Immutable from(String method, String uri) { if (HttpMethod.CONNECT.is(method)) - return new Immutable(uri); + return HttpURI.build().uri(method, uri).asImmutable(); if (uri.startsWith("/")) return HttpURI.build().pathQuery(uri).asImmutable(); return HttpURI.from(uri); @@ -628,6 +628,8 @@ public String asString() */ public Mutable authority(String host, int port) { + if (host != null && !isPathValidForAuthority(_path)) + throw new IllegalArgumentException("Relative path with authority"); _user = null; _host = host; _port = port; @@ -636,12 +638,14 @@ public Mutable authority(String host, int port) } /** - * @param hostport the host and port combined + * @param hostPort the host and port combined * @return this mutable */ - public Mutable authority(String hostport) + public Mutable authority(String hostPort) { - HostPort hp = new HostPort(hostport); + if (hostPort != null && !isPathValidForAuthority(_path)) + throw new IllegalArgumentException("Relative path with authority"); + HostPort hp = new HostPort(hostPort); _user = null; _host = hp.getHost(); _port = hp.getPort(); @@ -649,6 +653,15 @@ public Mutable authority(String hostport) return this; } + private boolean isPathValidForAuthority(String path) + { + if (path == null) + return true; + if (path.isEmpty() || "*".equals(path)) + return true; + return path.startsWith("/"); + } + public Mutable clear() { _scheme = null; @@ -775,6 +788,8 @@ public int hashCode() public Mutable host(String host) { + if (host != null && !isPathValidForAuthority(_path)) + throw new IllegalArgumentException("Relative path with authority"); _host = host; _uri = null; return this; @@ -834,10 +849,12 @@ public Mutable param(String param) /** * @param path the path - * @return this Mutuble + * @return this Mutable */ public Mutable path(String path) { + if (hasAuthority() && !isPathValidForAuthority(path)) + throw new IllegalArgumentException("Relative path with authority"); _uri = null; _path = path; _decodedPath = null; @@ -846,6 +863,8 @@ public Mutable path(String path) public Mutable pathQuery(String pathQuery) { + if (hasAuthority() && !isPathValidForAuthority(pathQuery)) + throw new IllegalArgumentException("Relative path with authority"); _uri = null; _path = null; _decodedPath = null; @@ -911,10 +930,7 @@ public Mutable uri(HttpURI uri) _query = uri.getQuery(); _uri = null; _decodedPath = uri.getDecodedPath(); - if (uri.hasAmbiguousSeparator()) - _violations.add(Violation.AMBIGUOUS_PATH_SEPARATOR); - if (uri.hasAmbiguousSegment()) - _violations.add(Violation.AMBIGUOUS_PATH_SEGMENT); + _violations.addAll(uri.getViolations()); return this; } @@ -931,8 +947,7 @@ public Mutable uri(String method, String uri) if (HttpMethod.CONNECT.is(method)) { clear(); - _uri = uri; - _path = uri; + parse(State.HOST, uri); } else if (uri.startsWith("/")) { diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java index f3161dd0a4bb..e5f09b3badb4 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java @@ -177,6 +177,32 @@ public void testParseRequestTarget() assertThat(uri.getPath(), is("/bar")); } + @Test + public void testCONNECT() + { + HttpURI uri; + + uri = HttpURI.from("CONNECT", "host:80"); + assertThat(uri.getHost(), is("host")); + assertThat(uri.getPort(), is(80)); + assertThat(uri.getPath(), nullValue()); + + uri = HttpURI.from("CONNECT", "host"); + assertThat(uri.getHost(), is("host")); + assertThat(uri.getPort(), is(-1)); + assertThat(uri.getPath(), nullValue()); + + uri = HttpURI.from("CONNECT", "192.168.0.1:8080"); + assertThat(uri.getHost(), is("192.168.0.1")); + assertThat(uri.getPort(), is(8080)); + assertThat(uri.getPath(), nullValue()); + + uri = HttpURI.from("CONNECT", "[::1]:8080"); + assertThat(uri.getHost(), is("[::1]")); + assertThat(uri.getPort(), is(8080)); + assertThat(uri.getPath(), nullValue()); + } + @Test public void testAt() { @@ -824,4 +850,46 @@ public void testEncodedQuery(String input, String expectedQuery) HttpURI httpURI = HttpURI.build(input); assertThat("[" + input + "] .query", httpURI.getQuery(), is(expectedQuery)); } + + @Test + public void testRelativePathWithAuthority() + { + assertThrows(IllegalArgumentException.class, () -> HttpURI.build() + .authority("host") + .path("path")); + assertThrows(IllegalArgumentException.class, () -> HttpURI.build() + .authority("host", 8080) + .path(";p=v/url")); + assertThrows(IllegalArgumentException.class, () -> HttpURI.build() + .host("host") + .path(";")); + + assertThrows(IllegalArgumentException.class, () -> HttpURI.build() + .path("path") + .authority("host")); + assertThrows(IllegalArgumentException.class, () -> HttpURI.build() + .path(";p=v/url") + .authority("host", 8080)); + assertThrows(IllegalArgumentException.class, () -> HttpURI.build() + .path(";") + .host("host")); + + HttpURI.Mutable uri = HttpURI.build() + .path("*") + .authority("host"); + assertEquals("//host*", uri.asString()); + uri = HttpURI.build() + .authority("host") + .path("*"); + assertEquals("//host*", uri.asString()); + + uri = HttpURI.build() + .path("") + .authority("host"); + assertEquals("//host", uri.asString()); + uri = HttpURI.build() + .authority("host") + .path(""); + assertEquals("//host", uri.asString()); + } } diff --git a/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ConnectHandler.java b/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ConnectHandler.java index 7975f1e79284..b69e64b93616 100644 --- a/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ConnectHandler.java +++ b/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/ConnectHandler.java @@ -33,8 +33,6 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeaderValue; import org.eclipse.jetty.http.HttpMethod; -import org.eclipse.jetty.http.HttpURI; -import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.io.EndPoint; @@ -193,12 +191,7 @@ public void handle(String target, Request jettyRequest, HttpServletRequest reque String tunnelProtocol = jettyRequest.getMetaData().getProtocol(); if (HttpMethod.CONNECT.is(request.getMethod()) && tunnelProtocol == null) { - String serverAddress = target; - if (HttpVersion.HTTP_2.is(request.getProtocol())) - { - HttpURI httpURI = jettyRequest.getHttpURI(); - serverAddress = httpURI.getHost() + ":" + httpURI.getPort(); - } + String serverAddress = jettyRequest.getHttpURI().getAuthority(); if (LOG.isDebugEnabled()) LOG.debug("CONNECT request for {}", serverAddress); handleConnect(jettyRequest, request, response, serverAddress); 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 126766c4c0c9..01af35813b76 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 @@ -1750,6 +1750,7 @@ public void setMetaData(MetaData.Request request) if (field instanceof HostPortHttpField) { HostPortHttpField authority = (HostPortHttpField)field; + builder.host(authority.getHost()).port(authority.getPort()); } else diff --git a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java index 12d36084afaf..0785020d1299 100644 --- a/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java +++ b/tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/HttpClientLoadTest.java @@ -118,8 +118,7 @@ public void testIterative(Transport transport) throws Exception public void testConcurrent(Transport transport) throws Exception { // TODO: cannot run HTTP/3 (or UDP) in Jenkins. - if ("ci".equals(System.getProperty("env"))) - Assumptions.assumeTrue(transport != Transport.H3); + Assumptions.assumeTrue(transport != Transport.H3); init(transport); scenario.start(new LoadHandler(), client ->