Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue #6302 - Treat empty path segments as ambiguous. #6304

Merged
merged 13 commits into from Jun 10, 2021
Merged

Issue #6302 - Treat empty path segments as ambiguous. #6304

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
36332ab
Issue #6302 - Treat empty path segments are ambiguous.
lachlan-roberts May 20, 2021
c30875e
Fix false empty segments being reported + changes from review.
lachlan-roberts May 21, 2021
3206dab
Fix broken test + improve comments.
lachlan-roberts May 23, 2021
3aec963
Add HttpUriTests for the empty segment as ambiguous
lachlan-roberts May 25, 2021
0cae7bf
Issue #63002 - Only call checkSegment() for real empty segments.
lachlan-roberts May 25, 2021
2649182
Issue #63002 - Empty segments are only ambiguous if followed by a '#'…
lachlan-roberts May 25, 2021
2792217
If only the last segment is empty it is not ambiguous.
lachlan-roberts May 27, 2021
3dd5460
improve testing for path HttpUri pathQuery()
lachlan-roberts May 28, 2021
93eaffd
Add javadoc for HttpURI.Ambiguous
lachlan-roberts May 31, 2021
918f377
Add decodedPath to testPathQuery() expectations.
lachlan-roberts May 31, 2021
339e245
Change comments, add some extra tests.
lachlan-roberts Jun 8, 2021
26d1646
Update jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
gregw Jun 10, 2021
ceccab9
Update jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
gregw Jun 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 19 additions & 0 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
View file
Open in desktop
Expand Up @@ -50,10 +50,29 @@ public interface HttpURI
{
enum Ambiguous
{
/**
* URI contains ambiguous path segments e.g. <code>/foo/%2e%2e/bar</code>
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
*/
SEGMENT,

/**
* URI contains ambiguous empty segments e.g. <code>//</code>
*/
EMPTY,

/**
* URI contains ambiguous path separator within a URI segment e.g. <code>/foo/b%2fr</code>
*/
SEPARATOR,

/**
* URI contains ambiguous path encoding within a URI segment e.g. <code>/%2557EB-INF</code>
*/
ENCODING,

/**
* URI contains ambiguous path parameters within a URI segment e.g. <code>/foo/..;/bar</code>
*/
PARAM
}

Expand Down
85 changes: 80 additions & 5 deletions jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java
View file
Open in desktop
Expand Up @@ -28,6 +28,7 @@
import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

Expand Down Expand Up @@ -429,11 +430,41 @@ public void testDecodedPath(String input, String decodedPath, EnumSet<Ambiguous>
}
}

public static Stream<Arguments> emptySegmentTests()
public static Stream<Arguments> pathFromQueryTests()
{
return Arrays.stream(new Object[][]
{
// Empty segment tests.
// Simple path example
{"/path/info", EnumSet.noneOf(Ambiguous.class)},

// legal non ambiguous relative paths
{"/path/../info", EnumSet.noneOf(Ambiguous.class)},
{"/path/./info", EnumSet.noneOf(Ambiguous.class)},
{"path/../info", EnumSet.noneOf(Ambiguous.class)},
{"path/./info", EnumSet.noneOf(Ambiguous.class)},

// illegal paths
{"/../path/info", null},
{"../path/info", null},
{"/path/%XX/info", null},
{"/path/%2/F/info", null},

// ambiguous dot encodings
{"/path/%2e/info", EnumSet.of(Ambiguous.SEGMENT)},
{"path/%2e/info/", EnumSet.of(Ambiguous.SEGMENT)},
{"/path/%2e%2e/info", EnumSet.of(Ambiguous.SEGMENT)},
{"/path/%2e%2e;/info", EnumSet.of(Ambiguous.SEGMENT)},
{"/path/%2e%2e;param/info", EnumSet.of(Ambiguous.SEGMENT)},
{"/path/%2e%2e;param;other/info;other", EnumSet.of(Ambiguous.SEGMENT)},
{"%2e/info", EnumSet.of(Ambiguous.SEGMENT)},
{"%2e%2e/info", EnumSet.of(Ambiguous.SEGMENT)},
{"%2e%2e;/info", EnumSet.of(Ambiguous.SEGMENT)},
{"%2e", EnumSet.of(Ambiguous.SEGMENT)},
{"%2e.", EnumSet.of(Ambiguous.SEGMENT)},
{".%2e", EnumSet.of(Ambiguous.SEGMENT)},
{"%2e%2e", EnumSet.of(Ambiguous.SEGMENT)},

// empty segment treated as ambiguous
{"/", EnumSet.noneOf(Ambiguous.class)},
{"/#", EnumSet.noneOf(Ambiguous.class)},
{"/path", EnumSet.noneOf(Ambiguous.class)},
Expand All @@ -449,14 +480,58 @@ public static Stream<Arguments> emptySegmentTests()
{"/foo/#bar", EnumSet.noneOf(Ambiguous.class)},
{"/foo/;param", EnumSet.noneOf(Ambiguous.class)},
{"/foo/;param/bar", EnumSet.of(Ambiguous.EMPTY)},
{"/foo//bar", EnumSet.of(Ambiguous.EMPTY)},
{"/foo//../bar", EnumSet.of(Ambiguous.EMPTY)},
{"/foo///../../../bar", EnumSet.of(Ambiguous.EMPTY)},
{"/foo/./../bar", EnumSet.noneOf(Ambiguous.class)},
{"/foo//./bar", EnumSet.of(Ambiguous.EMPTY)},
{"foo/bar", EnumSet.noneOf(Ambiguous.class)},
{"foo;/bar", EnumSet.noneOf(Ambiguous.class)},
{";/bar", EnumSet.of(Ambiguous.EMPTY)},
{";?n=v", EnumSet.of(Ambiguous.EMPTY)},
{"?n=v", EnumSet.noneOf(Ambiguous.class)},
{"#n=v", EnumSet.noneOf(Ambiguous.class)},
{"", EnumSet.noneOf(Ambiguous.class)},

// ambiguous parameter inclusions
{"/path/.;/info", EnumSet.of(Ambiguous.PARAM)},
{"/path/.;param/info", EnumSet.of(Ambiguous.PARAM)},
{"/path/..;/info", EnumSet.of(Ambiguous.PARAM)},
{"/path/..;param/info", EnumSet.of(Ambiguous.PARAM)},
{".;/info", EnumSet.of(Ambiguous.PARAM)},
{".;param/info", EnumSet.of(Ambiguous.PARAM)},
{"..;/info", EnumSet.of(Ambiguous.PARAM)},
{"..;param/info", EnumSet.of(Ambiguous.PARAM)},

// ambiguous segment separators
{"/path/%2f/info", EnumSet.of(Ambiguous.SEPARATOR)},
{"%2f/info", EnumSet.of(Ambiguous.SEPARATOR)},
{"%2F/info", EnumSet.of(Ambiguous.SEPARATOR)},
{"/path/%2f../info", EnumSet.of(Ambiguous.SEPARATOR)},

// ambiguous encoding
{"/path/%25/info", EnumSet.of(Ambiguous.ENCODING)},
{"%25/info", EnumSet.of(Ambiguous.ENCODING)},
{"/path/%25../info", EnumSet.of(Ambiguous.ENCODING)},

// combinations
{"/path/%2f/..;/info", EnumSet.of(Ambiguous.SEPARATOR, Ambiguous.PARAM)},
{"/path/%2f/..;/%2e/info", EnumSet.of(Ambiguous.SEPARATOR, Ambiguous.PARAM, Ambiguous.SEGMENT)},
}).map(Arguments::of);
}

@ParameterizedTest
@MethodSource("emptySegmentTests")
public void testEmptySegment(String input, EnumSet<Ambiguous> expected)
@MethodSource("pathFromQueryTests")
public void testPathFromQuery(String input, EnumSet<Ambiguous> expected)
lachlan-roberts marked this conversation as resolved.
Show resolved Hide resolved
{
HttpURI uri = HttpURI.from("GET", input);
// If expected is null then it is a bad URI and should throw.
if (expected == null)
{
assertThrows(Throwable.class, () -> HttpURI.build().pathQuery(input));
return;
}

HttpURI uri = HttpURI.build().pathQuery(input);
assertThat(uri.isAmbiguous(), is(!expected.isEmpty()));
assertThat(uri.hasAmbiguousEmptySegment(), is(expected.contains(Ambiguous.EMPTY)));
assertThat(uri.hasAmbiguousSegment(), is(expected.contains(Ambiguous.SEGMENT)));
Expand Down