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 af2dcda012b2..b421a796bcb7 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 @@ -48,7 +48,7 @@ */ public interface HttpURI { - enum Ambiguous + enum Violation { /** * URI contains ambiguous path segments e.g. {@code /foo/%2e%2e/bar} @@ -73,7 +73,12 @@ enum Ambiguous /** * URI contains ambiguous path parameters within a URI segment e.g. {@code /foo/..;/bar} */ - PARAM + PARAM, + + /** + * Contains UTF16 encodings + */ + UTF16 } static Mutable build() @@ -165,6 +170,11 @@ static Immutable from(String scheme, String host, int port, String pathQuery) */ boolean isAmbiguous(); + /** + * @return True if the URI has any Violations. + */ + boolean hasViolations(); + /** * @return True if the URI has a possibly ambiguous segment like '..;' or '%2e%2e' */ @@ -190,6 +200,8 @@ static Immutable from(String scheme, String host, int port, String pathQuery) */ boolean hasAmbiguousEncoding(); + boolean hasUtf16Encoding(); + default URI toURI() { try @@ -215,7 +227,7 @@ class Immutable implements HttpURI private final String _fragment; private String _uri; private String _decodedPath; - private final EnumSet _ambiguous = EnumSet.noneOf(Mutable.Ambiguous.class); + private final EnumSet _violations = EnumSet.noneOf(Violation.class); private Immutable(Mutable builder) { @@ -229,7 +241,7 @@ private Immutable(Mutable builder) _fragment = builder._fragment; _uri = builder._uri; _decodedPath = builder._decodedPath; - _ambiguous.addAll(builder._ambiguous); + _violations.addAll(builder._violations); } private Immutable(String uri) @@ -396,37 +408,49 @@ public boolean isAbsolute() @Override public boolean isAmbiguous() { - return !_ambiguous.isEmpty(); + return !_violations.isEmpty() && !(_violations.size() == 1 && _violations.contains(Violation.UTF16)); + } + + @Override + public boolean hasViolations() + { + return !_violations.isEmpty(); } @Override public boolean hasAmbiguousSegment() { - return _ambiguous.contains(Ambiguous.SEGMENT); + return _violations.contains(Violation.SEGMENT); } @Override public boolean hasAmbiguousEmptySegment() { - return _ambiguous.contains(Ambiguous.EMPTY); + return _violations.contains(Violation.EMPTY); } @Override public boolean hasAmbiguousSeparator() { - return _ambiguous.contains(Ambiguous.SEPARATOR); + return _violations.contains(Violation.SEPARATOR); } @Override public boolean hasAmbiguousParameter() { - return _ambiguous.contains(Ambiguous.PARAM); + return _violations.contains(Violation.PARAM); } @Override public boolean hasAmbiguousEncoding() { - return _ambiguous.contains(Ambiguous.ENCODING); + return _violations.contains(Violation.ENCODING); + } + + @Override + public boolean hasUtf16Encoding() + { + return _violations.contains(Violation.UTF16); } @Override @@ -480,12 +504,18 @@ private enum State */ private static final Index __ambiguousSegments = new Index.Builder() .caseSensitive(false) + .with(".", Boolean.FALSE) .with("%2e", Boolean.TRUE) - .with("%2e%2e", Boolean.TRUE) + .with("%u002e", Boolean.TRUE) + .with("..", Boolean.FALSE) .with(".%2e", Boolean.TRUE) + .with(".%u002e", Boolean.TRUE) .with("%2e.", Boolean.TRUE) - .with("..", Boolean.FALSE) - .with(".", Boolean.FALSE) + .with("%2e%2e", Boolean.TRUE) + .with("%2e%u002e", Boolean.TRUE) + .with("%u002e.", Boolean.TRUE) + .with("%u002e%2e", Boolean.TRUE) + .with("%u002e%u002e", Boolean.TRUE) .build(); private String _scheme; @@ -498,7 +528,7 @@ private enum State private String _fragment; private String _uri; private String _decodedPath; - private final EnumSet _ambiguous = EnumSet.noneOf(Ambiguous.class); + private final EnumSet _violations = EnumSet.noneOf(Violation.class); private boolean _emptySegment; private Mutable() @@ -623,7 +653,7 @@ public Mutable clear() _fragment = null; _uri = null; _decodedPath = null; - _ambiguous.clear(); + _violations.clear(); return this; } @@ -750,37 +780,49 @@ public boolean isAbsolute() @Override public boolean isAmbiguous() { - return !_ambiguous.isEmpty(); + return !_violations.isEmpty() && !(_violations.size() == 1 && _violations.contains(Violation.UTF16)); + } + + @Override + public boolean hasViolations() + { + return !_violations.isEmpty(); } @Override public boolean hasAmbiguousSegment() { - return _ambiguous.contains(Mutable.Ambiguous.SEGMENT); + return _violations.contains(Violation.SEGMENT); } @Override public boolean hasAmbiguousEmptySegment() { - return _ambiguous.contains(Ambiguous.EMPTY); + return _violations.contains(Violation.EMPTY); } @Override public boolean hasAmbiguousSeparator() { - return _ambiguous.contains(Mutable.Ambiguous.SEPARATOR); + return _violations.contains(Violation.SEPARATOR); } @Override public boolean hasAmbiguousParameter() { - return _ambiguous.contains(Ambiguous.PARAM); + return _violations.contains(Violation.PARAM); } @Override public boolean hasAmbiguousEncoding() { - return _ambiguous.contains(Ambiguous.ENCODING); + return _violations.contains(Violation.ENCODING); + } + + @Override + public boolean hasUtf16Encoding() + { + return _violations.contains(Violation.UTF16); } public Mutable normalize() @@ -885,9 +927,9 @@ public Mutable uri(HttpURI uri) _uri = null; _decodedPath = uri.getDecodedPath(); if (uri.hasAmbiguousSeparator()) - _ambiguous.add(Ambiguous.SEPARATOR); + _violations.add(Violation.SEPARATOR); if (uri.hasAmbiguousSegment()) - _ambiguous.add(Ambiguous.SEGMENT); + _violations.add(Violation.SEGMENT); return this; } @@ -938,9 +980,11 @@ private void parse(State state, final String uri) int mark = 0; // the start of the current section being parsed int pathMark = 0; // the start of the path section int segment = 0; // the start of the current segment within the path - boolean encoded = false; // set to true if the path contains % encoded characters - boolean dot = false; // set to true if the path containers . or .. segments - int escapedTwo = 0; // state of parsing a %2 + boolean encodedPath = false; // set to true if the path contains % encoded characters + boolean encodedUtf16 = false; // Is the current encoding for UTF16? + int encodedCharacters = 0; // partial state of parsing a % encoded character + int encodedValue = 0; // the partial encoded value + boolean dot = false; // set to true if the path contains . or .. segments int end = uri.length(); _emptySegment = false; for (int i = 0; i < end; i++) @@ -981,8 +1025,9 @@ private void parse(State state, final String uri) state = State.ASTERISK; break; case '%': - encoded = true; - escapedTwo = 1; + encodedPath = true; + encodedCharacters = 2; + encodedValue = 0; mark = pathMark = segment = i; state = State.PATH; break; @@ -1032,9 +1077,10 @@ private void parse(State state, final String uri) state = State.QUERY; break; case '%': - // must have be in an encoded path - encoded = true; - escapedTwo = 1; + // must have been in an encoded path + encodedPath = true; + encodedCharacters = 2; + encodedValue = 0; state = State.PATH; break; case '#': @@ -1154,55 +1200,71 @@ else if (c == '/') } case PATH: { - switch (c) + if (encodedCharacters > 0) { - case ';': - checkSegment(uri, segment, i, true); - mark = i + 1; - state = State.PARAM; - break; - case '?': - checkSegment(uri, segment, i, false); - _path = uri.substring(pathMark, i); - mark = i + 1; - state = State.QUERY; - break; - case '#': - checkSegment(uri, segment, i, false); - _path = uri.substring(pathMark, i); - mark = i + 1; - state = State.FRAGMENT; - break; - case '/': - // There is no leading segment when parsing only a path that starts with slash. - if (i != 0) + if (encodedCharacters == 2 && c == 'u' && !encodedUtf16) + { + _violations.add(Violation.UTF16); + encodedUtf16 = true; + encodedCharacters = 4; + continue; + } + encodedValue = (encodedValue << 4) + TypeUtil.convertHexDigit(c); + + if (--encodedCharacters == 0) + { + switch (encodedValue) + { + case '/': + _violations.add(Violation.SEPARATOR); + break; + case '%': + _violations.add(Violation.ENCODING); + break; + default: + break; + } + } + } + else + { + switch (c) + { + case ';': + checkSegment(uri, segment, i, true); + mark = i + 1; + state = State.PARAM; + break; + case '?': checkSegment(uri, segment, i, false); - segment = i + 1; - break; - case '.': - dot |= segment == i; - break; - case '%': - encoded = true; - escapedTwo = 1; - break; - case '2': - escapedTwo = escapedTwo == 1 ? 2 : 0; - break; - case 'f': - case 'F': - if (escapedTwo == 2) - _ambiguous.add(Ambiguous.SEPARATOR); - escapedTwo = 0; - break; - case '5': - if (escapedTwo == 2) - _ambiguous.add(Ambiguous.ENCODING); - escapedTwo = 0; - break; - default: - escapedTwo = 0; - break; + _path = uri.substring(pathMark, i); + mark = i + 1; + state = State.QUERY; + break; + case '#': + checkSegment(uri, segment, i, false); + _path = uri.substring(pathMark, i); + mark = i + 1; + state = State.FRAGMENT; + break; + case '/': + // There is no leading segment when parsing only a path that starts with slash. + if (i != 0) + checkSegment(uri, segment, i, false); + segment = i + 1; + break; + case '.': + dot |= segment == i; + break; + case '%': + encodedPath = true; + encodedUtf16 = false; + encodedCharacters = 2; + encodedValue = 0; + break; + default: + break; + } } break; } @@ -1223,7 +1285,7 @@ else if (c == '/') state = State.FRAGMENT; break; case '/': - encoded = true; + encodedPath = true; segment = i + 1; state = State.PATH; break; @@ -1238,11 +1300,26 @@ else if (c == '/') } case QUERY: { - if (c == '#') + switch (c) { - _query = uri.substring(mark, i); - mark = i + 1; - state = State.FRAGMENT; + case '%': + encodedCharacters = 2; + break; + case 'u': + case 'U': + if (encodedCharacters == 1) + _violations.add(Violation.UTF16); + encodedCharacters = 0; + break; + case '#': + _query = uri.substring(mark, i); + mark = i + 1; + state = State.FRAGMENT; + encodedCharacters = 0; + break; + default: + encodedCharacters = 0; + break; } break; } @@ -1300,7 +1377,7 @@ else if (c == '/') throw new IllegalStateException(state.toString()); } - if (!encoded && !dot) + if (!encodedPath && !dot) { if (_param == null) _decodedPath = _path; @@ -1333,7 +1410,7 @@ private void checkSegment(String uri, int segment, int end, boolean param) // Empty segments are only ambiguous if they are not the last segment // So if this method is called for any segment and we have previously seen an empty segment, then it was ambiguous if (_emptySegment) - _ambiguous.add(Ambiguous.EMPTY); + _violations.add(Violation.EMPTY); if (end == segment) { @@ -1344,7 +1421,7 @@ private void checkSegment(String uri, int segment, int end, boolean param) // If this empty segment is the first segment then it is ambiguous. if (segment == 0) { - _ambiguous.add(Ambiguous.EMPTY); + _violations.add(Violation.EMPTY); return; } @@ -1361,12 +1438,12 @@ private void checkSegment(String uri, int segment, int end, boolean param) if (ambiguous == Boolean.TRUE) { // The segment is always ambiguous. - _ambiguous.add(Ambiguous.SEGMENT); + _violations.add(Violation.SEGMENT); } else if (param && ambiguous == Boolean.FALSE) { // The segment is ambiguous only when followed by a parameter. - _ambiguous.add(Ambiguous.PARAM); + _violations.add(Violation.PARAM); } } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java b/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java index 29b51dc9bff1..15120cefd741 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java @@ -69,7 +69,11 @@ public enum Violation implements ComplianceViolation /** * Allow Non canonical ambiguous paths. eg /foo/x%2f%2e%2e%/bar provided to applications as /foo/x/../bar */ - NON_CANONICAL_AMBIGUOUS_PATHS("https://tools.ietf.org/html/rfc3986#section-3.3", "Non canonical ambiguous paths"); + NON_CANONICAL_AMBIGUOUS_PATHS("https://tools.ietf.org/html/rfc3986#section-3.3", "Non canonical ambiguous paths"), + /** + * Allow UTF-16 encoding eg /foo%u2192bar. + */ + UTF16_ENCODINGS("https://www.w3.org/International/iri-edit/draft-duerst-iri.html#anchor29", "UTF16 encoding"); private final String _url; private final String _description; @@ -109,9 +113,15 @@ public String getDescription() /** * LEGACY compliance mode that models Jetty-9.4 behavior by allowing {@link Violation#AMBIGUOUS_PATH_SEGMENT}, - * {@link Violation#AMBIGUOUS_EMPTY_SEGMENT}, {@link Violation#AMBIGUOUS_PATH_SEPARATOR} and {@link Violation#AMBIGUOUS_PATH_ENCODING}. + * {@link Violation#AMBIGUOUS_EMPTY_SEGMENT}, {@link Violation#AMBIGUOUS_PATH_SEPARATOR}, {@link Violation#AMBIGUOUS_PATH_ENCODING} + * and {@link Violation#UTF16_ENCODINGS} */ - public static final UriCompliance LEGACY = new UriCompliance("LEGACY", of(Violation.AMBIGUOUS_PATH_SEGMENT, Violation.AMBIGUOUS_PATH_SEPARATOR, Violation.AMBIGUOUS_PATH_ENCODING, Violation.AMBIGUOUS_EMPTY_SEGMENT)); + public static final UriCompliance LEGACY = new UriCompliance("LEGACY", + of(Violation.AMBIGUOUS_PATH_SEGMENT, + Violation.AMBIGUOUS_PATH_SEPARATOR, + Violation.AMBIGUOUS_PATH_ENCODING, + Violation.AMBIGUOUS_EMPTY_SEGMENT, + Violation.UTF16_ENCODINGS)); /** * Compliance mode that exactly follows RFC3986, including allowing all additional ambiguous URI Violations, diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURIParseTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURIParseTest.java deleted file mode 100644 index b3f88b990c46..000000000000 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURIParseTest.java +++ /dev/null @@ -1,247 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.http; - -import java.net.URI; -import java.net.URISyntaxException; -import java.util.stream.Stream; - -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.notNullValue; -import static org.hamcrest.Matchers.nullValue; -import static org.junit.jupiter.api.Assumptions.assumeTrue; - -public class HttpURIParseTest -{ - public static Stream data() - { - return Stream.of( - - // Nothing but path - Arguments.of("path", null, null, "-1", "path", null, null, null), - Arguments.of("path/path", null, null, "-1", "path/path", null, null, null), - Arguments.of("%65ncoded/path", null, null, "-1", "%65ncoded/path", null, null, null), - - // Basic path reference - Arguments.of("/path/to/context", null, null, "-1", "/path/to/context", null, null, null), - - // Basic with encoded query - Arguments.of("http://example.com/path/to/context;param?query=%22value%22#fragment", "http", "example.com", "-1", "/path/to/context;param", "param", "query=%22value%22", "fragment"), - Arguments.of("http://[::1]/path/to/context;param?query=%22value%22#fragment", "http", "[::1]", "-1", "/path/to/context;param", "param", "query=%22value%22", "fragment"), - - // Basic with parameters and query - Arguments.of("http://example.com:8080/path/to/context;param?query=%22value%22#fragment", "http", "example.com", "8080", "/path/to/context;param", "param", "query=%22value%22", "fragment"), - Arguments.of("http://[::1]:8080/path/to/context;param?query=%22value%22#fragment", "http", "[::1]", "8080", "/path/to/context;param", "param", "query=%22value%22", "fragment"), - - // Path References - Arguments.of("/path/info", null, null, null, "/path/info", null, null, null), - Arguments.of("/path/info#fragment", null, null, null, "/path/info", null, null, "fragment"), - Arguments.of("/path/info?query", null, null, null, "/path/info", null, "query", null), - Arguments.of("/path/info?query#fragment", null, null, null, "/path/info", null, "query", "fragment"), - Arguments.of("/path/info;param", null, null, null, "/path/info;param", "param", null, null), - Arguments.of("/path/info;param#fragment", null, null, null, "/path/info;param", "param", null, "fragment"), - Arguments.of("/path/info;param?query", null, null, null, "/path/info;param", "param", "query", null), - Arguments.of("/path/info;param?query#fragment", null, null, null, "/path/info;param", "param", "query", "fragment"), - Arguments.of("/path/info;a=b/foo;c=d", null, null, null, "/path/info;a=b/foo;c=d", "c=d", null, null), // TODO #405 - - // Protocol Less (aka scheme-less) URIs - Arguments.of("//host/path/info", null, "host", null, "/path/info", null, null, null), - Arguments.of("//user@host/path/info", null, "host", null, "/path/info", null, null, null), - Arguments.of("//user@host:8080/path/info", null, "host", "8080", "/path/info", null, null, null), - Arguments.of("//host:8080/path/info", null, "host", "8080", "/path/info", null, null, null), - - // Host Less - Arguments.of("http:/path/info", "http", null, null, "/path/info", null, null, null), - Arguments.of("http:/path/info#fragment", "http", null, null, "/path/info", null, null, "fragment"), - Arguments.of("http:/path/info?query", "http", null, null, "/path/info", null, "query", null), - Arguments.of("http:/path/info?query#fragment", "http", null, null, "/path/info", null, "query", "fragment"), - Arguments.of("http:/path/info;param", "http", null, null, "/path/info;param", "param", null, null), - Arguments.of("http:/path/info;param#fragment", "http", null, null, "/path/info;param", "param", null, "fragment"), - Arguments.of("http:/path/info;param?query", "http", null, null, "/path/info;param", "param", "query", null), - Arguments.of("http:/path/info;param?query#fragment", "http", null, null, "/path/info;param", "param", "query", "fragment"), - - // Everything and the kitchen sink - Arguments.of("http://user@host:8080/path/info;param?query#fragment", "http", "host", "8080", "/path/info;param", "param", "query", "fragment"), - Arguments.of("xxxxx://user@host:8080/path/info;param?query#fragment", "xxxxx", "host", "8080", "/path/info;param", "param", "query", "fragment"), - - // No host, parameter with no content - Arguments.of("http:///;?#", "http", null, null, "/;", "", "", ""), - - // Path with query that has no value - Arguments.of("/path/info?a=?query", null, null, null, "/path/info", null, "a=?query", null), - - // Path with query alt syntax - Arguments.of("/path/info?a=;query", null, null, null, "/path/info", null, "a=;query", null), - - // URI with host character - Arguments.of("/@path/info", null, null, null, "/@path/info", null, null, null), - Arguments.of("/user@path/info", null, null, null, "/user@path/info", null, null, null), - Arguments.of("//user@host/info", null, "host", null, "/info", null, null, null), - Arguments.of("//@host/info", null, "host", null, "/info", null, null, null), - Arguments.of("@host/info", null, null, null, "@host/info", null, null, null), - - // Scheme-less, with host and port (overlapping with path) - Arguments.of("//host:8080//", null, "host", "8080", "//", null, null, null), - - // File reference - Arguments.of("file:///path/info", "file", null, null, "/path/info", null, null, null), - Arguments.of("file:/path/info", "file", null, null, "/path/info", null, null, null), - - // Bad URI (no scheme, no host, no path) - Arguments.of("//", null, null, null, null, null, null, null), - - // Simple localhost references - Arguments.of("http://localhost/", "http", "localhost", null, "/", null, null, null), - Arguments.of("http://localhost:8080/", "http", "localhost", "8080", "/", null, null, null), - Arguments.of("http://localhost/?x=y", "http", "localhost", null, "/", null, "x=y", null), - - // Simple path with parameter - Arguments.of("/;param", null, null, null, "/;param", "param", null, null), - Arguments.of(";param", null, null, null, ";param", "param", null, null), - - // Simple path with query - Arguments.of("/?x=y", null, null, null, "/", null, "x=y", null), - Arguments.of("/?abc=test", null, null, null, "/", null, "abc=test", null), - - // Simple path with fragment - Arguments.of("/#fragment", null, null, null, "/", null, null, "fragment"), - - // Simple IPv4 host with port (default path) - Arguments.of("http://192.0.0.1:8080/", "http", "192.0.0.1", "8080", "/", null, null, null), - - // Simple IPv6 host with port (default path) - - Arguments.of("http://[2001:db8::1]:8080/", "http", "[2001:db8::1]", "8080", "/", null, null, null), - // IPv6 authenticated host with port (default path) - - Arguments.of("http://user@[2001:db8::1]:8080/", "http", "[2001:db8::1]", "8080", "/", null, null, null), - - // Simple IPv6 host no port (default path) - Arguments.of("http://[2001:db8::1]/", "http", "[2001:db8::1]", null, "/", null, null, null), - - // Scheme-less IPv6, host with port (default path) - Arguments.of("//[2001:db8::1]:8080/", null, "[2001:db8::1]", "8080", "/", null, null, null), - - // Interpreted as relative path of "*" (no host/port/scheme/query/fragment) - Arguments.of("*", null, null, null, "*", null, null, null), - - // Path detection Tests (seen from JSP/JSTL and use) - Arguments.of("http://host:8080/path/info?q1=v1&q2=v2", "http", "host", "8080", "/path/info", null, "q1=v1&q2=v2", null), - Arguments.of("/path/info?q1=v1&q2=v2", null, null, null, "/path/info", null, "q1=v1&q2=v2", null), - Arguments.of("/info?q1=v1&q2=v2", null, null, null, "/info", null, "q1=v1&q2=v2", null), - Arguments.of("info?q1=v1&q2=v2", null, null, null, "info", null, "q1=v1&q2=v2", null), - Arguments.of("info;q1=v1?q2=v2", null, null, null, "info;q1=v1", "q1=v1", "q2=v2", null), - - // Path-less, query only (seen from JSP/JSTL and use) - Arguments.of("?q1=v1&q2=v2", null, null, null, "", null, "q1=v1&q2=v2", null) - ); - } - - @ParameterizedTest - @MethodSource("data") - public void testParseString(String input, String scheme, String host, Integer port, String path, String param, String query, String fragment) throws Exception - { - HttpURI httpUri = HttpURI.from(input); - - try - { - new URI(input); - // URI is valid (per java.net.URI parsing) - - // Test case sanity check - assertThat("[" + input + "] expected path (test case) cannot be null", path, notNullValue()); - - // Assert expectations - assertThat("[" + input + "] .scheme", httpUri.getScheme(), is(scheme)); - assertThat("[" + input + "] .host", httpUri.getHost(), is(host)); - assertThat("[" + input + "] .port", httpUri.getPort(), is(port == null ? -1 : port)); - assertThat("[" + input + "] .path", httpUri.getPath(), is(path)); - assertThat("[" + input + "] .param", httpUri.getParam(), is(param)); - assertThat("[" + input + "] .query", httpUri.getQuery(), is(query)); - assertThat("[" + input + "] .fragment", httpUri.getFragment(), is(fragment)); - assertThat("[" + input + "] .toString", httpUri.toString(), is(input)); - } - catch (URISyntaxException e) - { - // Assert HttpURI values for invalid URI (such as "//") - assertThat("[" + input + "] .scheme", httpUri.getScheme(), is(nullValue())); - assertThat("[" + input + "] .host", httpUri.getHost(), is(nullValue())); - assertThat("[" + input + "] .port", httpUri.getPort(), is(-1)); - assertThat("[" + input + "] .path", httpUri.getPath(), is(nullValue())); - assertThat("[" + input + "] .param", httpUri.getParam(), is(nullValue())); - assertThat("[" + input + "] .query", httpUri.getQuery(), is(nullValue())); - assertThat("[" + input + "] .fragment", httpUri.getFragment(), is(nullValue())); - } - } - - @ParameterizedTest - @MethodSource("data") - public void testParseURI(String input, String scheme, String host, Integer port, String path, String param, String query, String fragment) throws Exception - { - URI javaUri = null; - try - { - javaUri = new URI(input); - } - catch (URISyntaxException ignore) - { - // Ignore, as URI is invalid anyway - } - assumeTrue(javaUri != null, "Skipping, not a valid input URI: " + input); - - HttpURI httpUri = HttpURI.from(javaUri); - - assertThat("[" + input + "] .scheme", httpUri.getScheme(), is(scheme)); - assertThat("[" + input + "] .host", httpUri.getHost(), is(host)); - assertThat("[" + input + "] .port", httpUri.getPort(), is(port == null ? -1 : port)); - assertThat("[" + input + "] .path", httpUri.getPath(), is(path)); - assertThat("[" + input + "] .param", httpUri.getParam(), is(param)); - assertThat("[" + input + "] .query", httpUri.getQuery(), is(query)); - assertThat("[" + input + "] .fragment", httpUri.getFragment(), is(fragment)); - - assertThat("[" + input + "] .toString", httpUri.toString(), is(input)); - } - - @ParameterizedTest - @MethodSource("data") - public void testCompareToJavaNetURI(String input, String scheme, String host, Integer port, String path, String param, String query, String fragment) throws Exception - { - URI javaUri = null; - try - { - javaUri = new URI(input); - } - catch (URISyntaxException ignore) - { - // Ignore, as URI is invalid anyway - } - assumeTrue(javaUri != null, "Skipping, not a valid input URI"); - - HttpURI httpUri = HttpURI.from(javaUri); - - assertThat("[" + input + "] .scheme", httpUri.getScheme(), is(javaUri.getScheme())); - assertThat("[" + input + "] .host", httpUri.getHost(), is(javaUri.getHost())); - assertThat("[" + input + "] .port", httpUri.getPort(), is(javaUri.getPort())); - assertThat("[" + input + "] .path", httpUri.getPath(), is(javaUri.getRawPath())); - // Not Relevant for java.net.URI -- assertThat("["+input+"] .param", httpUri.getParam(), is(param)); - assertThat("[" + input + "] .query", httpUri.getQuery(), is(javaUri.getRawQuery())); - assertThat("[" + input + "] .fragment", httpUri.getFragment(), is(javaUri.getFragment())); - assertThat("[" + input + "] .toString", httpUri.toString(), is(javaUri.toASCIIString())); - } -} 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 09fcc5ff19af..4c199e15d40b 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 @@ -13,11 +13,13 @@ package org.eclipse.jetty.http; +import java.net.URI; +import java.net.URISyntaxException; import java.util.Arrays; import java.util.EnumSet; import java.util.stream.Stream; -import org.eclipse.jetty.http.HttpURI.Ambiguous; +import org.eclipse.jetty.http.HttpURI.Violation; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -25,12 +27,14 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; 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; +import static org.junit.jupiter.api.Assumptions.assumeTrue; public class HttpURITest { @@ -323,109 +327,140 @@ public static Stream decodePathTests() return Arrays.stream(new Object[][] { // Simple path example - {"http://host/path/info", "/path/info", EnumSet.noneOf(Ambiguous.class)}, - {"//host/path/info", "/path/info", EnumSet.noneOf(Ambiguous.class)}, - {"/path/info", "/path/info", EnumSet.noneOf(Ambiguous.class)}, + {"http://host/path/info", "/path/info", EnumSet.noneOf(Violation.class)}, + {"//host/path/info", "/path/info", EnumSet.noneOf(Violation.class)}, + {"/path/info", "/path/info", EnumSet.noneOf(Violation.class)}, + + // Scheme & host containing unusual valid characters + {"ht..tp://host/path/info", "/path/info", EnumSet.noneOf(Violation.class)}, + {"ht1.2+..-3.4tp://127.0.0.1:8080/path/info", "/path/info", EnumSet.noneOf(Violation.class)}, + {"http://h%2est/path/info", "/path/info", EnumSet.noneOf(Violation.class)}, + {"http://h..est/path/info", "/path/info", EnumSet.noneOf(Violation.class)}, // legal non ambiguous relative paths - {"http://host/../path/info", null, EnumSet.noneOf(Ambiguous.class)}, - {"http://host/path/../info", "/info", EnumSet.noneOf(Ambiguous.class)}, - {"http://host/path/./info", "/path/info", EnumSet.noneOf(Ambiguous.class)}, - {"//host/path/../info", "/info", EnumSet.noneOf(Ambiguous.class)}, - {"//host/path/./info", "/path/info", EnumSet.noneOf(Ambiguous.class)}, - {"/path/../info", "/info", EnumSet.noneOf(Ambiguous.class)}, - {"/path/./info", "/path/info", EnumSet.noneOf(Ambiguous.class)}, - {"path/../info", "info", EnumSet.noneOf(Ambiguous.class)}, - {"path/./info", "path/info", EnumSet.noneOf(Ambiguous.class)}, + {"http://host/../path/info", null, EnumSet.noneOf(Violation.class)}, + {"http://host/path/../info", "/info", EnumSet.noneOf(Violation.class)}, + {"http://host/path/./info", "/path/info", EnumSet.noneOf(Violation.class)}, + {"//host/path/../info", "/info", EnumSet.noneOf(Violation.class)}, + {"//host/path/./info", "/path/info", EnumSet.noneOf(Violation.class)}, + {"/path/../info", "/info", EnumSet.noneOf(Violation.class)}, + {"/path/./info", "/path/info", EnumSet.noneOf(Violation.class)}, + {"path/../info", "info", EnumSet.noneOf(Violation.class)}, + {"path/./info", "path/info", EnumSet.noneOf(Violation.class)}, + + // encoded paths + {"/f%6f%6F/bar", "/foo/bar", EnumSet.noneOf(Violation.class)}, + {"/f%u006f%u006F/bar", "/foo/bar", EnumSet.of(Violation.UTF16)}, // illegal paths - {"//host/../path/info", null, EnumSet.noneOf(Ambiguous.class)}, - {"/../path/info", null, EnumSet.noneOf(Ambiguous.class)}, - {"../path/info", null, EnumSet.noneOf(Ambiguous.class)}, - {"/path/%XX/info", null, EnumSet.noneOf(Ambiguous.class)}, - {"/path/%2/F/info", null, EnumSet.noneOf(Ambiguous.class)}, + {"//host/../path/info", null, EnumSet.noneOf(Violation.class)}, + {"/../path/info", null, EnumSet.noneOf(Violation.class)}, + {"../path/info", null, EnumSet.noneOf(Violation.class)}, + {"/path/%XX/info", null, EnumSet.noneOf(Violation.class)}, + {"/path/%2/F/info", null, EnumSet.noneOf(Violation.class)}, + {"/path/%/info", null, EnumSet.noneOf(Violation.class)}, + {"/path/%u000X/info", null, EnumSet.noneOf(Violation.class)}, // ambiguous dot encodings - {"scheme://host/path/%2e/info", "/path/./info", EnumSet.of(Ambiguous.SEGMENT)}, - {"scheme:/path/%2e/info", "/path/./info", EnumSet.of(Ambiguous.SEGMENT)}, - {"/path/%2e/info", "/path/./info", EnumSet.of(Ambiguous.SEGMENT)}, - {"path/%2e/info/", "path/./info/", EnumSet.of(Ambiguous.SEGMENT)}, - {"/path/%2e%2e/info", "/path/../info", EnumSet.of(Ambiguous.SEGMENT)}, - {"/path/%2e%2e;/info", "/path/../info", EnumSet.of(Ambiguous.SEGMENT)}, - {"/path/%2e%2e;param/info", "/path/../info", EnumSet.of(Ambiguous.SEGMENT)}, - {"/path/%2e%2e;param;other/info;other", "/path/../info", EnumSet.of(Ambiguous.SEGMENT)}, - {"%2e/info", "./info", EnumSet.of(Ambiguous.SEGMENT)}, - {"%2e%2e/info", "../info", EnumSet.of(Ambiguous.SEGMENT)}, - {"%2e%2e;/info", "../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)}, + {"scheme://host/path/%2e/info", "/path/./info", EnumSet.of(Violation.SEGMENT)}, + {"scheme:/path/%2e/info", "/path/./info", EnumSet.of(Violation.SEGMENT)}, + {"/path/%2e/info", "/path/./info", EnumSet.of(Violation.SEGMENT)}, + {"path/%2e/info/", "path/./info/", EnumSet.of(Violation.SEGMENT)}, + {"/path/%2e%2e/info", "/path/../info", EnumSet.of(Violation.SEGMENT)}, + {"/path/%2e%2e;/info", "/path/../info", EnumSet.of(Violation.SEGMENT)}, + {"/path/%2e%2e;param/info", "/path/../info", EnumSet.of(Violation.SEGMENT)}, + {"/path/%2e%2e;param;other/info;other", "/path/../info", EnumSet.of(Violation.SEGMENT)}, + {"%2e/info", "./info", EnumSet.of(Violation.SEGMENT)}, + {"%u002e/info", "./info", EnumSet.of(Violation.SEGMENT, Violation.UTF16)}, + {"%2e%2e/info", "../info", EnumSet.of(Violation.SEGMENT)}, + {"%u002e%u002e/info", "../info", EnumSet.of(Violation.SEGMENT, Violation.UTF16)}, + {"%2e%2e;/info", "../info", EnumSet.of(Violation.SEGMENT)}, + {"%u002e%u002e;/info", "../info", EnumSet.of(Violation.SEGMENT, Violation.UTF16)}, + {"%2e", ".", EnumSet.of(Violation.SEGMENT)}, + {"%u002e", ".", EnumSet.of(Violation.SEGMENT, Violation.UTF16)}, + {"%2e.", "..", EnumSet.of(Violation.SEGMENT)}, + {"%u002e.", "..", EnumSet.of(Violation.SEGMENT, Violation.UTF16)}, + {".%2e", "..", EnumSet.of(Violation.SEGMENT)}, + {".%u002e", "..", EnumSet.of(Violation.SEGMENT, Violation.UTF16)}, + {"%2e%2e", "..", EnumSet.of(Violation.SEGMENT)}, + {"%u002e%u002e", "..", EnumSet.of(Violation.SEGMENT, Violation.UTF16)}, + {"%2e%u002e", "..", EnumSet.of(Violation.SEGMENT, Violation.UTF16)}, + {"%u002e%2e", "..", EnumSet.of(Violation.SEGMENT, Violation.UTF16)}, // empty segment treated as ambiguous - {"/foo//bar", "/foo//bar", EnumSet.of(Ambiguous.EMPTY)}, - {"/foo//../bar", "/foo/bar", EnumSet.of(Ambiguous.EMPTY)}, - {"/foo///../../../bar", "/bar", EnumSet.of(Ambiguous.EMPTY)}, - {"/foo/./../bar", "/bar", EnumSet.noneOf(Ambiguous.class)}, - {"/foo//./bar", "/foo//bar", EnumSet.of(Ambiguous.EMPTY)}, - {"foo/bar", "foo/bar", EnumSet.noneOf(Ambiguous.class)}, - {"foo;/bar", "foo/bar", EnumSet.noneOf(Ambiguous.class)}, - {";/bar", "/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)}, - {"http:/foo", "/foo", EnumSet.noneOf(Ambiguous.class)}, + {"/foo//bar", "/foo//bar", EnumSet.of(Violation.EMPTY)}, + {"/foo//../bar", "/foo/bar", EnumSet.of(Violation.EMPTY)}, + {"/foo///../../../bar", "/bar", EnumSet.of(Violation.EMPTY)}, + {"/foo/./../bar", "/bar", EnumSet.noneOf(Violation.class)}, + {"/foo//./bar", "/foo//bar", EnumSet.of(Violation.EMPTY)}, + {"foo/bar", "foo/bar", EnumSet.noneOf(Violation.class)}, + {"foo;/bar", "foo/bar", EnumSet.noneOf(Violation.class)}, + {";/bar", "/bar", EnumSet.of(Violation.EMPTY)}, + {";?n=v", "", EnumSet.of(Violation.EMPTY)}, + {"?n=v", "", EnumSet.noneOf(Violation.class)}, + {"#n=v", "", EnumSet.noneOf(Violation.class)}, + {"", "", EnumSet.noneOf(Violation.class)}, + {"http:/foo", "/foo", EnumSet.noneOf(Violation.class)}, // ambiguous parameter inclusions - {"/path/.;/info", "/path/./info", EnumSet.of(Ambiguous.PARAM)}, - {"/path/.;param/info", "/path/./info", EnumSet.of(Ambiguous.PARAM)}, - {"/path/..;/info", "/path/../info", EnumSet.of(Ambiguous.PARAM)}, - {"/path/..;param/info", "/path/../info", EnumSet.of(Ambiguous.PARAM)}, - {".;/info", "./info", EnumSet.of(Ambiguous.PARAM)}, - {".;param/info", "./info", EnumSet.of(Ambiguous.PARAM)}, - {"..;/info", "../info", EnumSet.of(Ambiguous.PARAM)}, - {"..;param/info", "../info", EnumSet.of(Ambiguous.PARAM)}, + {"/path/.;/info", "/path/./info", EnumSet.of(Violation.PARAM)}, + {"/path/.;param/info", "/path/./info", EnumSet.of(Violation.PARAM)}, + {"/path/..;/info", "/path/../info", EnumSet.of(Violation.PARAM)}, + {"/path/..;param/info", "/path/../info", EnumSet.of(Violation.PARAM)}, + {".;/info", "./info", EnumSet.of(Violation.PARAM)}, + {".;param/info", "./info", EnumSet.of(Violation.PARAM)}, + {"..;/info", "../info", EnumSet.of(Violation.PARAM)}, + {"..;param/info", "../info", EnumSet.of(Violation.PARAM)}, // ambiguous segment separators - {"/path/%2f/info", "/path///info", EnumSet.of(Ambiguous.SEPARATOR)}, - {"%2f/info", "//info", EnumSet.of(Ambiguous.SEPARATOR)}, - {"%2F/info", "//info", EnumSet.of(Ambiguous.SEPARATOR)}, - {"/path/%2f../info", "/path//../info", EnumSet.of(Ambiguous.SEPARATOR)}, + {"/path/%2f/info", "/path///info", EnumSet.of(Violation.SEPARATOR)}, + {"%2f/info", "//info", EnumSet.of(Violation.SEPARATOR)}, + {"%2F/info", "//info", EnumSet.of(Violation.SEPARATOR)}, + {"/path/%2f../info", "/path//../info", EnumSet.of(Violation.SEPARATOR)}, // ambiguous encoding - {"/path/%25/info", "/path/%/info", EnumSet.of(Ambiguous.ENCODING)}, - {"%25/info", "%/info", EnumSet.of(Ambiguous.ENCODING)}, - {"/path/%25../info", "/path/%../info", EnumSet.of(Ambiguous.ENCODING)}, + {"/path/%25/info", "/path/%/info", EnumSet.of(Violation.ENCODING)}, + {"/path/%u0025/info", "/path/%/info", EnumSet.of(Violation.ENCODING, Violation.UTF16)}, + {"%25/info", "%/info", EnumSet.of(Violation.ENCODING)}, + {"/path/%25../info", "/path/%../info", EnumSet.of(Violation.ENCODING)}, + {"/path/%u0025../info", "/path/%../info", EnumSet.of(Violation.ENCODING, Violation.UTF16)}, // combinations - {"/path/%2f/..;/info", "/path///../info", EnumSet.of(Ambiguous.SEPARATOR, Ambiguous.PARAM)}, - {"/path/%2f/..;/%2e/info", "/path///.././info", EnumSet.of(Ambiguous.SEPARATOR, Ambiguous.PARAM, Ambiguous.SEGMENT)}, + {"/path/%2f/..;/info", "/path///../info", EnumSet.of(Violation.SEPARATOR, Violation.PARAM)}, + {"/path/%u002f/..;/info", "/path///../info", EnumSet.of(Violation.SEPARATOR, Violation.PARAM, Violation.UTF16)}, + {"/path/%2f/..;/%2e/info", "/path///.././info", EnumSet.of(Violation.SEPARATOR, Violation.PARAM, Violation.SEGMENT)}, // Non ascii characters // @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck - {"http://localhost:9000/x\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32", "/x\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32", EnumSet.noneOf(Ambiguous.class)}, - {"http://localhost:9000/\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32", "/\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32", EnumSet.noneOf(Ambiguous.class)}, + {"http://localhost:9000/x\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32", "/x\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32", EnumSet.noneOf(Violation.class)}, + {"http://localhost:9000/\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32", "/\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32", EnumSet.noneOf(Violation.class)}, // @checkstyle-enable-check : AvoidEscapedUnicodeCharactersCheck }).map(Arguments::of); } @ParameterizedTest @MethodSource("decodePathTests") - public void testDecodedPath(String input, String decodedPath, EnumSet expected) + public void testDecodedPath(String input, String decodedPath, EnumSet expected) { try { HttpURI uri = HttpURI.from(input); assertThat(uri.getDecodedPath(), is(decodedPath)); - assertThat(uri.isAmbiguous(), is(!expected.isEmpty())); - assertThat(uri.hasAmbiguousSegment(), is(expected.contains(Ambiguous.SEGMENT))); - assertThat(uri.hasAmbiguousSeparator(), is(expected.contains(Ambiguous.SEPARATOR))); - assertThat(uri.hasAmbiguousParameter(), is(expected.contains(Ambiguous.PARAM))); - assertThat(uri.hasAmbiguousEncoding(), is(expected.contains(Ambiguous.ENCODING))); + EnumSet ambiguous = EnumSet.copyOf(expected); + ambiguous.retainAll(EnumSet.complementOf(EnumSet.of(Violation.UTF16))); + + assertThat(uri.isAmbiguous(), is(!ambiguous.isEmpty())); + assertThat(uri.hasAmbiguousSegment(), is(ambiguous.contains(Violation.SEGMENT))); + assertThat(uri.hasAmbiguousSeparator(), is(ambiguous.contains(Violation.SEPARATOR))); + assertThat(uri.hasAmbiguousParameter(), is(ambiguous.contains(Violation.PARAM))); + assertThat(uri.hasAmbiguousEncoding(), is(ambiguous.contains(Violation.ENCODING))); + + assertThat(uri.hasUtf16Encoding(), is(expected.contains(Violation.UTF16))); } catch (Exception e) { + if (decodedPath != null) + e.printStackTrace(); assertThat(decodedPath, nullValue()); } } @@ -435,13 +470,13 @@ public static Stream testPathQueryTests() return Arrays.stream(new Object[][] { // Simple path example - {"/path/info", "/path/info", EnumSet.noneOf(Ambiguous.class)}, + {"/path/info", "/path/info", EnumSet.noneOf(Violation.class)}, // legal non ambiguous relative paths - {"/path/../info", "/info", EnumSet.noneOf(Ambiguous.class)}, - {"/path/./info", "/path/info", EnumSet.noneOf(Ambiguous.class)}, - {"path/../info", "info", EnumSet.noneOf(Ambiguous.class)}, - {"path/./info", "path/info", EnumSet.noneOf(Ambiguous.class)}, + {"/path/../info", "/info", EnumSet.noneOf(Violation.class)}, + {"/path/./info", "/path/info", EnumSet.noneOf(Violation.class)}, + {"path/../info", "info", EnumSet.noneOf(Violation.class)}, + {"path/./info", "path/info", EnumSet.noneOf(Violation.class)}, // illegal paths {"/../path/info", null, null}, @@ -450,82 +485,82 @@ public static Stream testPathQueryTests() {"/path/%2/F/info", null, null}, // ambiguous dot encodings - {"/path/%2e/info", "/path/./info", EnumSet.of(Ambiguous.SEGMENT)}, - {"path/%2e/info/", "path/./info/", EnumSet.of(Ambiguous.SEGMENT)}, - {"/path/%2e%2e/info", "/path/../info", EnumSet.of(Ambiguous.SEGMENT)}, - {"/path/%2e%2e;/info", "/path/../info", EnumSet.of(Ambiguous.SEGMENT)}, - {"/path/%2e%2e;param/info", "/path/../info", EnumSet.of(Ambiguous.SEGMENT)}, - {"/path/%2e%2e;param;other/info;other", "/path/../info", EnumSet.of(Ambiguous.SEGMENT)}, - {"%2e/info", "./info", EnumSet.of(Ambiguous.SEGMENT)}, - {"%2e%2e/info", "../info", EnumSet.of(Ambiguous.SEGMENT)}, - {"%2e%2e;/info", "../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)}, + {"/path/%2e/info", "/path/./info", EnumSet.of(Violation.SEGMENT)}, + {"path/%2e/info/", "path/./info/", EnumSet.of(Violation.SEGMENT)}, + {"/path/%2e%2e/info", "/path/../info", EnumSet.of(Violation.SEGMENT)}, + {"/path/%2e%2e;/info", "/path/../info", EnumSet.of(Violation.SEGMENT)}, + {"/path/%2e%2e;param/info", "/path/../info", EnumSet.of(Violation.SEGMENT)}, + {"/path/%2e%2e;param;other/info;other", "/path/../info", EnumSet.of(Violation.SEGMENT)}, + {"%2e/info", "./info", EnumSet.of(Violation.SEGMENT)}, + {"%2e%2e/info", "../info", EnumSet.of(Violation.SEGMENT)}, + {"%2e%2e;/info", "../info", EnumSet.of(Violation.SEGMENT)}, + {"%2e", ".", EnumSet.of(Violation.SEGMENT)}, + {"%2e.", "..", EnumSet.of(Violation.SEGMENT)}, + {".%2e", "..", EnumSet.of(Violation.SEGMENT)}, + {"%2e%2e", "..", EnumSet.of(Violation.SEGMENT)}, // empty segment treated as ambiguous - {"/", "/", EnumSet.noneOf(Ambiguous.class)}, - {"/#", "/", EnumSet.noneOf(Ambiguous.class)}, - {"/path", "/path", EnumSet.noneOf(Ambiguous.class)}, - {"/path/", "/path/", EnumSet.noneOf(Ambiguous.class)}, - {"//", "//", EnumSet.of(Ambiguous.EMPTY)}, - {"/foo//", "/foo//", EnumSet.of(Ambiguous.EMPTY)}, - {"/foo//bar", "/foo//bar", EnumSet.of(Ambiguous.EMPTY)}, - {"//foo/bar", "//foo/bar", EnumSet.of(Ambiguous.EMPTY)}, - {"/foo?bar", "/foo", EnumSet.noneOf(Ambiguous.class)}, - {"/foo#bar", "/foo", EnumSet.noneOf(Ambiguous.class)}, - {"/foo;bar", "/foo", EnumSet.noneOf(Ambiguous.class)}, - {"/foo/?bar", "/foo/", EnumSet.noneOf(Ambiguous.class)}, - {"/foo/#bar", "/foo/", EnumSet.noneOf(Ambiguous.class)}, - {"/foo/;param", "/foo/", EnumSet.noneOf(Ambiguous.class)}, - {"/foo/;param/bar", "/foo//bar", EnumSet.of(Ambiguous.EMPTY)}, - {"/foo//bar", "/foo//bar", EnumSet.of(Ambiguous.EMPTY)}, - {"/foo//bar//", "/foo//bar//", EnumSet.of(Ambiguous.EMPTY)}, - {"//foo//bar//", "//foo//bar//", EnumSet.of(Ambiguous.EMPTY)}, - {"/foo//../bar", "/foo/bar", EnumSet.of(Ambiguous.EMPTY)}, - {"/foo///../../../bar", "/bar", EnumSet.of(Ambiguous.EMPTY)}, - {"/foo/./../bar", "/bar", EnumSet.noneOf(Ambiguous.class)}, - {"/foo//./bar", "/foo//bar", EnumSet.of(Ambiguous.EMPTY)}, - {"foo/bar", "foo/bar", EnumSet.noneOf(Ambiguous.class)}, - {"foo;/bar", "foo/bar", EnumSet.noneOf(Ambiguous.class)}, - {";/bar", "/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)}, + {"/", "/", EnumSet.noneOf(Violation.class)}, + {"/#", "/", EnumSet.noneOf(Violation.class)}, + {"/path", "/path", EnumSet.noneOf(Violation.class)}, + {"/path/", "/path/", EnumSet.noneOf(Violation.class)}, + {"//", "//", EnumSet.of(Violation.EMPTY)}, + {"/foo//", "/foo//", EnumSet.of(Violation.EMPTY)}, + {"/foo//bar", "/foo//bar", EnumSet.of(Violation.EMPTY)}, + {"//foo/bar", "//foo/bar", EnumSet.of(Violation.EMPTY)}, + {"/foo?bar", "/foo", EnumSet.noneOf(Violation.class)}, + {"/foo#bar", "/foo", EnumSet.noneOf(Violation.class)}, + {"/foo;bar", "/foo", EnumSet.noneOf(Violation.class)}, + {"/foo/?bar", "/foo/", EnumSet.noneOf(Violation.class)}, + {"/foo/#bar", "/foo/", EnumSet.noneOf(Violation.class)}, + {"/foo/;param", "/foo/", EnumSet.noneOf(Violation.class)}, + {"/foo/;param/bar", "/foo//bar", EnumSet.of(Violation.EMPTY)}, + {"/foo//bar", "/foo//bar", EnumSet.of(Violation.EMPTY)}, + {"/foo//bar//", "/foo//bar//", EnumSet.of(Violation.EMPTY)}, + {"//foo//bar//", "//foo//bar//", EnumSet.of(Violation.EMPTY)}, + {"/foo//../bar", "/foo/bar", EnumSet.of(Violation.EMPTY)}, + {"/foo///../../../bar", "/bar", EnumSet.of(Violation.EMPTY)}, + {"/foo/./../bar", "/bar", EnumSet.noneOf(Violation.class)}, + {"/foo//./bar", "/foo//bar", EnumSet.of(Violation.EMPTY)}, + {"foo/bar", "foo/bar", EnumSet.noneOf(Violation.class)}, + {"foo;/bar", "foo/bar", EnumSet.noneOf(Violation.class)}, + {";/bar", "/bar", EnumSet.of(Violation.EMPTY)}, + {";?n=v", "", EnumSet.of(Violation.EMPTY)}, + {"?n=v", "", EnumSet.noneOf(Violation.class)}, + {"#n=v", "", EnumSet.noneOf(Violation.class)}, + {"", "", EnumSet.noneOf(Violation.class)}, // ambiguous parameter inclusions - {"/path/.;/info", "/path/./info", EnumSet.of(Ambiguous.PARAM)}, - {"/path/.;param/info", "/path/./info", EnumSet.of(Ambiguous.PARAM)}, - {"/path/..;/info", "/path/../info", EnumSet.of(Ambiguous.PARAM)}, - {"/path/..;param/info", "/path/../info", EnumSet.of(Ambiguous.PARAM)}, - {".;/info", "./info", EnumSet.of(Ambiguous.PARAM)}, - {".;param/info", "./info", EnumSet.of(Ambiguous.PARAM)}, - {"..;/info", "../info", EnumSet.of(Ambiguous.PARAM)}, - {"..;param/info", "../info", EnumSet.of(Ambiguous.PARAM)}, + {"/path/.;/info", "/path/./info", EnumSet.of(Violation.PARAM)}, + {"/path/.;param/info", "/path/./info", EnumSet.of(Violation.PARAM)}, + {"/path/..;/info", "/path/../info", EnumSet.of(Violation.PARAM)}, + {"/path/..;param/info", "/path/../info", EnumSet.of(Violation.PARAM)}, + {".;/info", "./info", EnumSet.of(Violation.PARAM)}, + {".;param/info", "./info", EnumSet.of(Violation.PARAM)}, + {"..;/info", "../info", EnumSet.of(Violation.PARAM)}, + {"..;param/info", "../info", EnumSet.of(Violation.PARAM)}, // ambiguous segment separators - {"/path/%2f/info", "/path///info", EnumSet.of(Ambiguous.SEPARATOR)}, - {"%2f/info", "//info", EnumSet.of(Ambiguous.SEPARATOR)}, - {"%2F/info", "//info", EnumSet.of(Ambiguous.SEPARATOR)}, - {"/path/%2f../info", "/path//../info", EnumSet.of(Ambiguous.SEPARATOR)}, + {"/path/%2f/info", "/path///info", EnumSet.of(Violation.SEPARATOR)}, + {"%2f/info", "//info", EnumSet.of(Violation.SEPARATOR)}, + {"%2F/info", "//info", EnumSet.of(Violation.SEPARATOR)}, + {"/path/%2f../info", "/path//../info", EnumSet.of(Violation.SEPARATOR)}, // ambiguous encoding - {"/path/%25/info", "/path/%/info", EnumSet.of(Ambiguous.ENCODING)}, - {"%25/info", "%/info", EnumSet.of(Ambiguous.ENCODING)}, - {"/path/%25../info", "/path/%../info", EnumSet.of(Ambiguous.ENCODING)}, + {"/path/%25/info", "/path/%/info", EnumSet.of(Violation.ENCODING)}, + {"%25/info", "%/info", EnumSet.of(Violation.ENCODING)}, + {"/path/%25../info", "/path/%../info", EnumSet.of(Violation.ENCODING)}, // combinations - {"/path/%2f/..;/info", "/path///../info", EnumSet.of(Ambiguous.SEPARATOR, Ambiguous.PARAM)}, - {"/path/%2f/..;/%2e/info", "/path///.././info", EnumSet.of(Ambiguous.SEPARATOR, Ambiguous.PARAM, Ambiguous.SEGMENT)}, - {"/path/%2f/%25/..;/%2e//info", "/path///%/.././/info", EnumSet.of(Ambiguous.SEPARATOR, Ambiguous.PARAM, Ambiguous.SEGMENT, Ambiguous.ENCODING, Ambiguous.EMPTY)}, + {"/path/%2f/..;/info", "/path///../info", EnumSet.of(Violation.SEPARATOR, Violation.PARAM)}, + {"/path/%2f/..;/%2e/info", "/path///.././info", EnumSet.of(Violation.SEPARATOR, Violation.PARAM, Violation.SEGMENT)}, + {"/path/%2f/%25/..;/%2e//info", "/path///%/.././/info", EnumSet.of(Violation.SEPARATOR, Violation.PARAM, Violation.SEGMENT, Violation.ENCODING, Violation.EMPTY)}, }).map(Arguments::of); } @ParameterizedTest @MethodSource("testPathQueryTests") - public void testPathQuery(String input, String decodedPath, EnumSet expected) + public void testPathQuery(String input, String decodedPath, EnumSet expected) { // If expected is null then it is a bad URI and should throw. if (expected == null) @@ -537,10 +572,225 @@ public void testPathQuery(String input, String decodedPath, EnumSet e HttpURI uri = HttpURI.build().pathQuery(input); assertThat(uri.getDecodedPath(), is(decodedPath)); assertThat(uri.isAmbiguous(), is(!expected.isEmpty())); - assertThat(uri.hasAmbiguousEmptySegment(), is(expected.contains(Ambiguous.EMPTY))); - assertThat(uri.hasAmbiguousSegment(), is(expected.contains(Ambiguous.SEGMENT))); - assertThat(uri.hasAmbiguousSeparator(), is(expected.contains(Ambiguous.SEPARATOR))); - assertThat(uri.hasAmbiguousParameter(), is(expected.contains(Ambiguous.PARAM))); - assertThat(uri.hasAmbiguousEncoding(), is(expected.contains(Ambiguous.ENCODING))); + assertThat(uri.hasAmbiguousEmptySegment(), is(expected.contains(Violation.EMPTY))); + assertThat(uri.hasAmbiguousSegment(), is(expected.contains(Violation.SEGMENT))); + assertThat(uri.hasAmbiguousSeparator(), is(expected.contains(Violation.SEPARATOR))); + assertThat(uri.hasAmbiguousParameter(), is(expected.contains(Violation.PARAM))); + assertThat(uri.hasAmbiguousEncoding(), is(expected.contains(Violation.ENCODING))); + } + + public static Stream parseData() + { + return Stream.of( + // Nothing but path + Arguments.of("path", null, null, "-1", "path", null, null, null), + Arguments.of("path/path", null, null, "-1", "path/path", null, null, null), + Arguments.of("%65ncoded/path", null, null, "-1", "%65ncoded/path", null, null, null), + + // Basic path reference + Arguments.of("/path/to/context", null, null, "-1", "/path/to/context", null, null, null), + + // Basic with encoded query + Arguments.of("http://example.com/path/to/context;param?query=%22value%22#fragment", "http", "example.com", "-1", "/path/to/context;param", "param", "query=%22value%22", "fragment"), + Arguments.of("http://[::1]/path/to/context;param?query=%22value%22#fragment", "http", "[::1]", "-1", "/path/to/context;param", "param", "query=%22value%22", "fragment"), + + // Basic with parameters and query + Arguments.of("http://example.com:8080/path/to/context;param?query=%22value%22#fragment", "http", "example.com", "8080", "/path/to/context;param", "param", "query=%22value%22", "fragment"), + Arguments.of("http://[::1]:8080/path/to/context;param?query=%22value%22#fragment", "http", "[::1]", "8080", "/path/to/context;param", "param", "query=%22value%22", "fragment"), + + // Path References + Arguments.of("/path/info", null, null, null, "/path/info", null, null, null), + Arguments.of("/path/info#fragment", null, null, null, "/path/info", null, null, "fragment"), + Arguments.of("/path/info?query", null, null, null, "/path/info", null, "query", null), + Arguments.of("/path/info?query#fragment", null, null, null, "/path/info", null, "query", "fragment"), + Arguments.of("/path/info;param", null, null, null, "/path/info;param", "param", null, null), + Arguments.of("/path/info;param#fragment", null, null, null, "/path/info;param", "param", null, "fragment"), + Arguments.of("/path/info;param?query", null, null, null, "/path/info;param", "param", "query", null), + Arguments.of("/path/info;param?query#fragment", null, null, null, "/path/info;param", "param", "query", "fragment"), + Arguments.of("/path/info;a=b/foo;c=d", null, null, null, "/path/info;a=b/foo;c=d", "c=d", null, null), // TODO #405 + + // Protocol Less (aka scheme-less) URIs + Arguments.of("//host/path/info", null, "host", null, "/path/info", null, null, null), + Arguments.of("//user@host/path/info", null, "host", null, "/path/info", null, null, null), + Arguments.of("//user@host:8080/path/info", null, "host", "8080", "/path/info", null, null, null), + Arguments.of("//host:8080/path/info", null, "host", "8080", "/path/info", null, null, null), + + // Host Less + Arguments.of("http:/path/info", "http", null, null, "/path/info", null, null, null), + Arguments.of("http:/path/info#fragment", "http", null, null, "/path/info", null, null, "fragment"), + Arguments.of("http:/path/info?query", "http", null, null, "/path/info", null, "query", null), + Arguments.of("http:/path/info?query#fragment", "http", null, null, "/path/info", null, "query", "fragment"), + Arguments.of("http:/path/info;param", "http", null, null, "/path/info;param", "param", null, null), + Arguments.of("http:/path/info;param#fragment", "http", null, null, "/path/info;param", "param", null, "fragment"), + Arguments.of("http:/path/info;param?query", "http", null, null, "/path/info;param", "param", "query", null), + Arguments.of("http:/path/info;param?query#fragment", "http", null, null, "/path/info;param", "param", "query", "fragment"), + + // Everything and the kitchen sink + Arguments.of("http://user@host:8080/path/info;param?query#fragment", "http", "host", "8080", "/path/info;param", "param", "query", "fragment"), + Arguments.of("xxxxx://user@host:8080/path/info;param?query#fragment", "xxxxx", "host", "8080", "/path/info;param", "param", "query", "fragment"), + + // No host, parameter with no content + Arguments.of("http:///;?#", "http", null, null, "/;", "", "", ""), + + // Path with query that has no value + Arguments.of("/path/info?a=?query", null, null, null, "/path/info", null, "a=?query", null), + + // Path with query alt syntax + Arguments.of("/path/info?a=;query", null, null, null, "/path/info", null, "a=;query", null), + + // URI with host character + Arguments.of("/@path/info", null, null, null, "/@path/info", null, null, null), + Arguments.of("/user@path/info", null, null, null, "/user@path/info", null, null, null), + Arguments.of("//user@host/info", null, "host", null, "/info", null, null, null), + Arguments.of("//@host/info", null, "host", null, "/info", null, null, null), + Arguments.of("@host/info", null, null, null, "@host/info", null, null, null), + + // Scheme-less, with host and port (overlapping with path) + Arguments.of("//host:8080//", null, "host", "8080", "//", null, null, null), + + // File reference + Arguments.of("file:///path/info", "file", null, null, "/path/info", null, null, null), + Arguments.of("file:/path/info", "file", null, null, "/path/info", null, null, null), + + // Bad URI (no scheme, no host, no path) + Arguments.of("//", null, null, null, null, null, null, null), + + // Simple localhost references + Arguments.of("http://localhost/", "http", "localhost", null, "/", null, null, null), + Arguments.of("http://localhost:8080/", "http", "localhost", "8080", "/", null, null, null), + Arguments.of("http://localhost/?x=y", "http", "localhost", null, "/", null, "x=y", null), + + // Simple path with parameter + Arguments.of("/;param", null, null, null, "/;param", "param", null, null), + Arguments.of(";param", null, null, null, ";param", "param", null, null), + + // Simple path with query + Arguments.of("/?x=y", null, null, null, "/", null, "x=y", null), + Arguments.of("/?abc=test", null, null, null, "/", null, "abc=test", null), + + // Simple path with fragment + Arguments.of("/#fragment", null, null, null, "/", null, null, "fragment"), + + // Simple IPv4 host with port (default path) + Arguments.of("http://192.0.0.1:8080/", "http", "192.0.0.1", "8080", "/", null, null, null), + + // Simple IPv6 host with port (default path) + + Arguments.of("http://[2001:db8::1]:8080/", "http", "[2001:db8::1]", "8080", "/", null, null, null), + // IPv6 authenticated host with port (default path) + + Arguments.of("http://user@[2001:db8::1]:8080/", "http", "[2001:db8::1]", "8080", "/", null, null, null), + + // Simple IPv6 host no port (default path) + Arguments.of("http://[2001:db8::1]/", "http", "[2001:db8::1]", null, "/", null, null, null), + + // Scheme-less IPv6, host with port (default path) + Arguments.of("//[2001:db8::1]:8080/", null, "[2001:db8::1]", "8080", "/", null, null, null), + + // Interpreted as relative path of "*" (no host/port/scheme/query/fragment) + Arguments.of("*", null, null, null, "*", null, null, null), + + // Path detection Tests (seen from JSP/JSTL and use) + Arguments.of("http://host:8080/path/info?q1=v1&q2=v2", "http", "host", "8080", "/path/info", null, "q1=v1&q2=v2", null), + Arguments.of("/path/info?q1=v1&q2=v2", null, null, null, "/path/info", null, "q1=v1&q2=v2", null), + Arguments.of("/info?q1=v1&q2=v2", null, null, null, "/info", null, "q1=v1&q2=v2", null), + Arguments.of("info?q1=v1&q2=v2", null, null, null, "info", null, "q1=v1&q2=v2", null), + Arguments.of("info;q1=v1?q2=v2", null, null, null, "info;q1=v1", "q1=v1", "q2=v2", null), + + // Path-less, query only (seen from JSP/JSTL and use) + Arguments.of("?q1=v1&q2=v2", null, null, null, "", null, "q1=v1&q2=v2", null) + ); + } + + @ParameterizedTest + @MethodSource("parseData") + public void testParseString(String input, String scheme, String host, Integer port, String path, String param, String query, String fragment) + { + HttpURI httpUri = HttpURI.from(input); + + try + { + new URI(input); + // URI is valid (per java.net.URI parsing) + + // Test case sanity check + assertThat("[" + input + "] expected path (test case) cannot be null", path, notNullValue()); + + // Assert expectations + assertThat("[" + input + "] .scheme", httpUri.getScheme(), is(scheme)); + assertThat("[" + input + "] .host", httpUri.getHost(), is(host)); + assertThat("[" + input + "] .port", httpUri.getPort(), is(port == null ? -1 : port)); + assertThat("[" + input + "] .path", httpUri.getPath(), is(path)); + assertThat("[" + input + "] .param", httpUri.getParam(), is(param)); + assertThat("[" + input + "] .query", httpUri.getQuery(), is(query)); + assertThat("[" + input + "] .fragment", httpUri.getFragment(), is(fragment)); + assertThat("[" + input + "] .toString", httpUri.toString(), is(input)); + } + catch (URISyntaxException e) + { + // Assert HttpURI values for invalid URI (such as "//") + assertThat("[" + input + "] .scheme", httpUri.getScheme(), is(nullValue())); + assertThat("[" + input + "] .host", httpUri.getHost(), is(nullValue())); + assertThat("[" + input + "] .port", httpUri.getPort(), is(-1)); + assertThat("[" + input + "] .path", httpUri.getPath(), is(nullValue())); + assertThat("[" + input + "] .param", httpUri.getParam(), is(nullValue())); + assertThat("[" + input + "] .query", httpUri.getQuery(), is(nullValue())); + assertThat("[" + input + "] .fragment", httpUri.getFragment(), is(nullValue())); + } + } + + @ParameterizedTest + @MethodSource("parseData") + public void testParseURI(String input, String scheme, String host, Integer port, String path, String param, String query, String fragment) throws Exception + { + URI javaUri = null; + try + { + javaUri = new URI(input); + } + catch (URISyntaxException ignore) + { + // Ignore, as URI is invalid anyway + } + assumeTrue(javaUri != null, "Skipping, not a valid input URI: " + input); + + HttpURI httpUri = HttpURI.from(javaUri); + + assertThat("[" + input + "] .scheme", httpUri.getScheme(), is(scheme)); + assertThat("[" + input + "] .host", httpUri.getHost(), is(host)); + assertThat("[" + input + "] .port", httpUri.getPort(), is(port == null ? -1 : port)); + assertThat("[" + input + "] .path", httpUri.getPath(), is(path)); + assertThat("[" + input + "] .param", httpUri.getParam(), is(param)); + assertThat("[" + input + "] .query", httpUri.getQuery(), is(query)); + assertThat("[" + input + "] .fragment", httpUri.getFragment(), is(fragment)); + + assertThat("[" + input + "] .toString", httpUri.toString(), is(input)); + } + + @ParameterizedTest + @MethodSource("parseData") + public void testCompareToJavaNetURI(String input, String scheme, String host, Integer port, String path, String param, String query, String fragment) throws Exception + { + URI javaUri = null; + try + { + javaUri = new URI(input); + } + catch (URISyntaxException ignore) + { + // Ignore, as URI is invalid anyway + } + assumeTrue(javaUri != null, "Skipping, not a valid input URI"); + + HttpURI httpUri = HttpURI.from(javaUri); + + assertThat("[" + input + "] .scheme", httpUri.getScheme(), is(javaUri.getScheme())); + assertThat("[" + input + "] .host", httpUri.getHost(), is(javaUri.getHost())); + assertThat("[" + input + "] .port", httpUri.getPort(), is(javaUri.getPort())); + assertThat("[" + input + "] .path", httpUri.getPath(), is(javaUri.getRawPath())); + // Not Relevant for java.net.URI -- assertThat("["+input+"] .param", httpUri.getParam(), is(param)); + assertThat("[" + input + "] .query", httpUri.getQuery(), is(javaUri.getRawQuery())); + assertThat("[" + input + "] .fragment", httpUri.getFragment(), is(javaUri.getFragment())); + assertThat("[" + input + "] .toString", httpUri.toString(), is(javaUri.toASCIIString())); } } 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 770191dbe5a2..691bf8d40fc1 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 @@ -1687,22 +1687,31 @@ public void setMetaData(MetaData.Request request) _method = request.getMethod(); _httpFields = request.getFields(); final HttpURI uri = request.getURI(); - + boolean ambiguous = false; UriCompliance compliance = null; - boolean ambiguous = uri.isAmbiguous(); - if (ambiguous) + if (uri.hasViolations()) { + ambiguous = uri.isAmbiguous(); compliance = _channel == null || _channel.getHttpConfiguration() == null ? null : _channel.getHttpConfiguration().getUriCompliance(); - if (uri.hasAmbiguousSegment() && (compliance == null || !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_SEGMENT))) - throw new BadMessageException("Ambiguous segment in URI"); - if (uri.hasAmbiguousEmptySegment() && (compliance == null || !compliance.allows(UriCompliance.Violation.AMBIGUOUS_EMPTY_SEGMENT))) - throw new BadMessageException("Ambiguous empty segment in URI"); - if (uri.hasAmbiguousSeparator() && (compliance == null || !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_SEPARATOR))) - throw new BadMessageException("Ambiguous segment in URI"); - if (uri.hasAmbiguousParameter() && (compliance == null || !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_PARAMETER))) - throw new BadMessageException("Ambiguous path parameter in URI"); - if (uri.hasAmbiguousEncoding() && (compliance == null || !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_ENCODING))) - throw new BadMessageException("Ambiguous path encoding in URI"); + if (compliance != null) + { + if (!compliance.allows(UriCompliance.Violation.UTF16_ENCODINGS) && uri.hasUtf16Encoding()) + throw new BadMessageException("UTF16 % encoding not supported"); + + if (ambiguous) + { + if (uri.hasAmbiguousSegment() && !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_SEGMENT)) + throw new BadMessageException("Ambiguous segment in URI"); + if (uri.hasAmbiguousEmptySegment() && !compliance.allows(UriCompliance.Violation.AMBIGUOUS_EMPTY_SEGMENT)) + throw new BadMessageException("Ambiguous empty segment in URI"); + if (uri.hasAmbiguousSeparator() && !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_SEPARATOR)) + throw new BadMessageException("Ambiguous segment in URI"); + if (uri.hasAmbiguousParameter() && !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_PARAMETER)) + throw new BadMessageException("Ambiguous path parameter in URI"); + if (uri.hasAmbiguousEncoding() && !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_ENCODING)) + throw new BadMessageException("Ambiguous path encoding in URI"); + } + } } if (uri.isAbsolute() && uri.hasAuthority() && uri.getPath() != null) 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 index bdc249610c88..e0ba04fa6b73 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java @@ -14,7 +14,6 @@ package org.eclipse.jetty.server; import java.io.BufferedReader; -import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileReader; import java.io.IOException; @@ -36,7 +35,6 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Stream; -import javax.servlet.DispatcherType; import javax.servlet.MultipartConfigElement; import javax.servlet.ServletException; import javax.servlet.ServletInputStream; @@ -69,8 +67,6 @@ import org.eclipse.jetty.server.session.Session; import org.eclipse.jetty.server.session.SessionData; import org.eclipse.jetty.server.session.SessionHandler; -import org.eclipse.jetty.toolchain.test.FS; -import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.jupiter.WorkDir; import org.eclipse.jetty.toolchain.test.jupiter.WorkDirExtension; import org.eclipse.jetty.util.BufferUtil; @@ -78,7 +74,6 @@ import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.slf4j.Logger; @@ -1638,6 +1633,19 @@ public void testGetterSafeFromNullPointerException() assertEquals(0, request.getParameterMap().size()); } + @Test + public void testEncoding() throws Exception + { + _handler._checker = (request, response) -> "/foo/bar".equals(request.getPathInfo()); + String request = "GET /f%6f%6F/b%u0061r HTTP/1.0\r\n" + + "Host: whatever\r\n" + + "\r\n"; + _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.DEFAULT); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); + _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.LEGACY); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); + } + @Test public void testAmbiguousParameters() throws Exception { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java b/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java index 98f97352c3de..f18e685f2e8b 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/URIUtil.java @@ -470,8 +470,7 @@ public static String decodePath(String path, int offset, int length) char u = path.charAt(i + 1); if (u == 'u') { - // TODO remove %u support in jetty-10 - // this is wrong. This is a codepoint not a char + // UTF16 encoding is only supported with UriCompliance.Violation.UTF16_ENCODINGS. builder.append((char)(0xffff & TypeUtil.parseInt(path, i + 2, 4, 16))); i += 5; } @@ -558,8 +557,7 @@ private static String decodeISO88591Path(String path, int offset, int length) char u = path.charAt(i + 1); if (u == 'u') { - // TODO remove %u encoding support in jetty-10 - // This is wrong. This is a codepoint not a char + // UTF16 encoding is only supported with UriCompliance.Violation.UTF16_ENCODINGS. builder.append((char)(0xffff & TypeUtil.parseInt(path, i + 2, 4, 16))); i += 5; } diff --git a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java index 88f2380ccedb..83f67fdc2d2d 100644 --- a/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java +++ b/jetty-webapp/src/test/java/org/eclipse/jetty/webapp/WebAppContextTest.java @@ -28,7 +28,6 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; -import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.http.UriCompliance; @@ -310,16 +309,22 @@ public void testProtectedTarget() throws Exception assertThat(HttpTester.parseResponse(connector.getResponse("GET /test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200)); assertThat(HttpTester.parseResponse(connector.getResponse("GET /%2e/%2e/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /%u002e/%u002e/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200)); assertThat(HttpTester.parseResponse(connector.getResponse("GET /foo/%2e%2e/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /foo/%u002e%u002e/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.OK_200)); assertThat(HttpTester.parseResponse(connector.getResponse("GET /WEB-INF HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); assertThat(HttpTester.parseResponse(connector.getResponse("GET /WEB-INF/ HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); assertThat(HttpTester.parseResponse(connector.getResponse("GET /WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); assertThat(HttpTester.parseResponse(connector.getResponse("GET /web-inf/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); assertThat(HttpTester.parseResponse(connector.getResponse("GET /%2e/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /%u002e/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); assertThat(HttpTester.parseResponse(connector.getResponse("GET /%2e/%2e/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /%u002e/%u002e/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); assertThat(HttpTester.parseResponse(connector.getResponse("GET /foo/%2e%2e/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /foo/%u002e%u002e/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); assertThat(HttpTester.parseResponse(connector.getResponse("GET /%2E/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /%u002E/WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); assertThat(HttpTester.parseResponse(connector.getResponse("GET //WEB-INF/test.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); assertThat(HttpTester.parseResponse(connector.getResponse("GET /WEB-INF%2ftest.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); }