Skip to content

Commit

Permalink
HttpURI.Mutable path changes should clear out any existing path viola…
Browse files Browse the repository at this point in the history 
…tions before parsing the new path.

Fixes: #11298
  • Loading branch information
@joakime
joakime committed Sep 24, 2024
1 parent 1d9f108 commit 814024f
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 12 deletions.
19 changes: 19 additions & 0 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -954,6 +954,21 @@ public Collection<Violation> getViolations()
return _violations == null ? Collections.emptySet() : Collections.unmodifiableCollection(_violations);
}

/**
* Clear, from the violations list, any path related violations.
*/
private void clearPathViolations()
{
if (_violations == null)
return;
_violations.removeIf((violation) ->
(violation == Violation.AMBIGUOUS_PATH_PARAMETER) ||
(violation == Violation.AMBIGUOUS_PATH_SEGMENT) ||
(violation == Violation.AMBIGUOUS_PATH_SEPARATOR) ||
(violation == Violation.AMBIGUOUS_PATH_ENCODING) ||
(violation == Violation.AMBIGUOUS_EMPTY_SEGMENT));
}

public Mutable normalize()
{
HttpScheme scheme = _scheme == null ? null : HttpScheme.CACHE.get(_scheme);
Expand Down Expand Up @@ -994,6 +1009,8 @@ public Mutable path(String path)
throw new IllegalArgumentException("Relative path with authority");
if (!URIUtil.isPathValid(path))
throw new IllegalArgumentException("Path not correctly encoded: " + path);
// since we are resetting the path, lets clear out the path specific violations.
clearPathViolations();
_uri = null;
_path = null;
_canonicalPath = null;
Expand All @@ -1016,6 +1033,8 @@ public Mutable pathQuery(String pathQuery)
{
if (hasAuthority() && !isPathValidForAuthority(pathQuery))
throw new IllegalArgumentException("Relative path with authority");
// since we are resetting the path, lets clear out the path specific violations.
clearPathViolations();
_uri = null;
_path = null;
_canonicalPath = null;
Expand Down
78 changes: 66 additions & 12 deletions ...re/jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/CompactPathRuleTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,18 @@

package org.eclipse.jetty.rewrite.handler;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.ByteBuffer;
import java.util.Objects;
import java.util.Properties;
import java.util.stream.Stream;

import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.http.HttpURI;
import org.eclipse.jetty.http.UriCompliance;
import org.eclipse.jetty.io.Content;
import org.eclipse.jetty.server.Handler;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.server.Response;
Expand All @@ -36,14 +41,20 @@ public static Stream<Arguments> scenarios()
{
return Stream.of(
// shouldn't change anything
Arguments.of("/foo", null, "/foo", null, "/foo"),
Arguments.of("/", null, "/", null, "/"),
Arguments.of("/foo", null, "/foo", "", "/foo"),
Arguments.of("/", null, "/", "", "/"),
// simple compact path
Arguments.of("////foo", null, "/foo", null, "/foo"),
Arguments.of("////foo", null, "/foo", "", "/foo"),
// with simple query
Arguments.of("//foo//bar", "a=b", "/foo/bar", "a=b", "/foo/bar?a=b"),
// with query that has double slashes (should preserve slashes in query)
Arguments.of("//foo//bar", "a=b//c", "/foo/bar", "a=b//c", "/foo/bar?a=b//c")
Arguments.of("//foo//bar", "a=b//c", "/foo/bar", "a=b//c", "/foo/bar?a=b//c"),
// with ambiguous path parameter
Arguments.of("//foo/..;/bar", "a=b//c", "/bar", "a=b//c", "/bar?a=b//c"),
// with ambiguous path separator (not changed)
Arguments.of("//foo/b%2far", "a=b//c", "/foo/b%2Far", "a=b//c", "/foo/b%2Far?a=b//c"),
// with ambiguous path encoding (not changed)
Arguments.of("//foo/%2562ar", "a=b//c", "/foo/%2562ar", "a=b//c", "/foo/%2562ar?a=b//c")
);
}

Expand All @@ -59,24 +70,67 @@ public void testCompactPathRule(String inputPath, String inputQuery, String expe
@Override
public boolean handle(Request request, Response response, Callback callback)
{
Content.Sink.write(response, true, request.getHttpURI().getPathQuery(), callback);
Properties props = new Properties();
HttpURI httpURI = request.getHttpURI();
props.setProperty("uri.path", of(httpURI.getPath()));
props.setProperty("uri.query", of(httpURI.getQuery()));
props.setProperty("uri.pathQuery", of(httpURI.getPathQuery()));
props.setProperty("uri.hasViolations", of(httpURI.hasViolations()));
props.setProperty("uri.isAmbiguous", of(httpURI.isAmbiguous()));
props.setProperty("uri.hasAmbiguousEmptySegment", of(httpURI.hasAmbiguousEmptySegment()));
props.setProperty("uri.hasAmbiguousEncoding", of(httpURI.hasAmbiguousEncoding()));
props.setProperty("uri.hasAmbiguousParameter", of(httpURI.hasAmbiguousParameter()));
props.setProperty("uri.hasAmbiguousSeparator", of(httpURI.hasAmbiguousSeparator()));
props.setProperty("uri.hasAmbiguousSegment", of(httpURI.hasAmbiguousSegment()));
try (ByteArrayOutputStream out = new ByteArrayOutputStream())
{
props.store(out, "HttpURI State");
response.write(true, ByteBuffer.wrap(out.toByteArray()), callback);
}
catch (IOException e)
{
callback.failed(e);
}
return true;
}

private String of(Object obj)
{
if (obj == null)
return "";
if (obj instanceof Boolean)
return Boolean.toString((Boolean)obj);
return Objects.toString(obj);
}
});


String request = """
GET %s HTTP/1.1
Host: localhost
""".formatted(HttpURI.build().path(inputPath).query(inputQuery));

HttpTester.Response response = HttpTester.parseResponse(_connector.getResponse(request));
System.err.println(response.getReason());
assertEquals(HttpStatus.OK_200, response.getStatus());
HttpURI.Mutable result = HttpURI.build(response.getContent());
assertEquals(expectedPath, result.getPath());
assertEquals(expectedQuery, result.getQuery());
assertEquals(expectedPathQuery, result.getPathQuery());
Properties props = new Properties();
try (ByteArrayInputStream in = new ByteArrayInputStream(response.getContentBytes()))
{
props.load(in);
assertEquals(expectedPath, props.getProperty("uri.path"));
assertEquals(expectedQuery, props.getProperty("uri.query"));
assertEquals(expectedPathQuery, props.getProperty("uri.pathQuery"));

boolean ambiguousPathSep = inputPath.contains("%2f");
boolean ambiguousPathEncoding = inputPath.contains("%25");

assertEquals(Boolean.toString(ambiguousPathSep || ambiguousPathEncoding), props.getProperty("uri.isAmbiguous"));
assertEquals(Boolean.toString(ambiguousPathSep || ambiguousPathEncoding), props.getProperty("uri.hasViolations"));
assertEquals("false", props.getProperty("uri.hasAmbiguousEmptySegment"));
assertEquals(Boolean.toString(ambiguousPathEncoding), props.getProperty("uri.hasAmbiguousEncoding"));
assertEquals("false", props.getProperty("uri.hasAmbiguousParameter"));
assertEquals(Boolean.toString(ambiguousPathSep), props.getProperty("uri.hasAmbiguousSeparator"));
assertEquals("false", props.getProperty("uri.hasAmbiguousSegment"));
}
}
}

0 comments on commit 814024f

Please sign in to comment.