Skip to content

Commit

Permalink
Fixes #8014 - Review HttpRequest URI construction.
Browse files Browse the repository at this point in the history
Now always adding a "/" before the path, if not already present.
Disabled flakey HTTP/3 test.

Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
  • Loading branch information
sbordet committed May 17, 2022
1 parent 797e1f8 commit c1cb2dc
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 9 deletions.
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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<CountDownLatch> 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
Expand Down
5 changes: 1 addition & 4 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
Expand Up @@ -911,10 +911,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;
}

Expand Down
Expand Up @@ -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 ->
Expand Down

0 comments on commit c1cb2dc

Please sign in to comment.