From 6624a3508c5ac7768e578e4d01a884c4979ef3fe Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Thu, 24 Jun 2021 12:12:32 +1000 Subject: [PATCH] Issue #6447 - Deprecate support for UTF16 encoding in URIs - Merge from PR #6457. - Also brought some other ComplianceModes back to disable ambiguous empty segments, and ambiguous encodings. Signed-off-by: Lachlan Roberts --- .../eclipse/jetty/http/HttpCompliance.java | 10 +- .../jetty/http/HttpComplianceSection.java | 5 +- .../java/org/eclipse/jetty/http/HttpURI.java | 315 +++++++---- .../eclipse/jetty/http/HttpURIParseTest.java | 252 --------- .../org/eclipse/jetty/http/HttpURITest.java | 498 ++++++++++++++++-- .../org/eclipse/jetty/server/Request.java | 30 +- .../org/eclipse/jetty/server/RequestTest.java | 82 +++ .../java/org/eclipse/jetty/util/URIUtil.java | 6 +- .../jetty/webapp/WebAppContextTest.java | 7 + 9 files changed, 787 insertions(+), 418 deletions(-) delete mode 100644 jetty-http/src/test/java/org/eclipse/jetty/http/HttpURIParseTest.java diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java index e1f42d4183fa..fd0dece4b80a 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java @@ -132,7 +132,10 @@ static EnumSet sectionsBySpec(String spec) HttpComplianceSection.NO_FIELD_FOLDING, HttpComplianceSection.NO_HTTP_0_9, HttpComplianceSection.NO_AMBIGUOUS_PATH_SEGMENTS, - HttpComplianceSection.NO_AMBIGUOUS_PATH_SEPARATORS)); + HttpComplianceSection.NO_AMBIGUOUS_PATH_SEPARATORS, + HttpComplianceSection.NO_UTF16_ENCODINGS, + HttpComplianceSection.NO_AMBIGUOUS_EMPTY_SEGMENT, + HttpComplianceSection.NO_AMBIGUOUS_PATH_ENCODING)); break; case "*": @@ -140,7 +143,10 @@ static EnumSet sectionsBySpec(String spec) i++; sections = EnumSet.complementOf(EnumSet.of( HttpComplianceSection.NO_AMBIGUOUS_PATH_SEGMENTS, - HttpComplianceSection.NO_AMBIGUOUS_PATH_SEPARATORS)); + HttpComplianceSection.NO_AMBIGUOUS_PATH_SEPARATORS, + HttpComplianceSection.NO_UTF16_ENCODINGS, + HttpComplianceSection.NO_AMBIGUOUS_EMPTY_SEGMENT, + HttpComplianceSection.NO_AMBIGUOUS_PATH_ENCODING)); break; default: diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpComplianceSection.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpComplianceSection.java index 338b6b781c73..2ebef2456392 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpComplianceSection.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpComplianceSection.java @@ -34,7 +34,10 @@ public enum HttpComplianceSection MULTIPLE_CONTENT_LENGTHS("https://tools.ietf.org/html/rfc7230#section-3.3.1", "Multiple Content-Lengths"), NO_AMBIGUOUS_PATH_SEGMENTS("https://tools.ietf.org/html/rfc3986#section-3.3", "No ambiguous URI path segments"), NO_AMBIGUOUS_PATH_SEPARATORS("https://tools.ietf.org/html/rfc3986#section-3.3", "No ambiguous URI path separators"), - NO_AMBIGUOUS_PATH_PARAMETERS("https://tools.ietf.org/html/rfc3986#section-3.3", "No ambiguous URI path parameters"); + NO_AMBIGUOUS_PATH_PARAMETERS("https://tools.ietf.org/html/rfc3986#section-3.3", "No ambiguous URI path parameters"), + NO_UTF16_ENCODINGS("https://www.w3.org/International/iri-edit/draft-duerst-iri.html#anchor29", "UTF16 encoding"), + NO_AMBIGUOUS_EMPTY_SEGMENT("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI empty segment"), + NO_AMBIGUOUS_PATH_ENCODING("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path encoding"); final String url; final String description; 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 74c04e0da07c..4087291dacbb 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 @@ -69,11 +69,14 @@ private enum State ASTERISK } - enum Ambiguous + enum Violation { SEGMENT, SEPARATOR, - PARAM + PARAM, + ENCODING, + EMPTY, + UTF16 } /** @@ -92,12 +95,18 @@ enum Ambiguous static { + __ambiguousSegments.put(".", Boolean.FALSE); __ambiguousSegments.put("%2e", Boolean.TRUE); - __ambiguousSegments.put("%2e%2e", Boolean.TRUE); + __ambiguousSegments.put("%u002e", Boolean.TRUE); + __ambiguousSegments.put("..", Boolean.FALSE); __ambiguousSegments.put(".%2e", Boolean.TRUE); + __ambiguousSegments.put(".%u002e", Boolean.TRUE); __ambiguousSegments.put("%2e.", Boolean.TRUE); - __ambiguousSegments.put("..", Boolean.FALSE); - __ambiguousSegments.put(".", Boolean.FALSE); + __ambiguousSegments.put("%2e%2e", Boolean.TRUE); + __ambiguousSegments.put("%2e%u002e", Boolean.TRUE); + __ambiguousSegments.put("%u002e.", Boolean.TRUE); + __ambiguousSegments.put("%u002e%2e", Boolean.TRUE); + __ambiguousSegments.put("%u002e%u002e", Boolean.TRUE); } private String _scheme; @@ -110,7 +119,8 @@ enum Ambiguous 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; /** * Construct a normalized URI. @@ -165,7 +175,8 @@ public HttpURI(HttpURI uri) _fragment = uri._fragment; _uri = uri._uri; _decodedPath = uri._decodedPath; - _ambiguous.addAll(uri._ambiguous); + _violations.addAll(uri._violations); + _emptySegment = false; } public HttpURI(String uri) @@ -212,7 +223,8 @@ public void clear() _query = null; _fragment = null; _decodedPath = null; - _ambiguous.clear(); + _emptySegment = false; + _violations.clear(); } public void parse(String uri) @@ -260,11 +272,13 @@ private void parse(State state, final String uri, final int offset, final int en int mark = offset; // 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 escapedSlash = 0; // state of parsing a %2f + 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 - for (int i = offset; i < end; i++) + for (int i = 0; i < end; i++) { char c = uri.charAt(i); @@ -279,16 +293,21 @@ private void parse(State state, final String uri, final int offset, final int en state = State.HOST_OR_PATH; break; case ';': + checkSegment(uri, segment, i, true); mark = i + 1; state = State.PARAM; break; case '?': // assume empty path (if seen at start) + checkSegment(uri, segment, i, false); _path = ""; mark = i + 1; state = State.QUERY; break; case '#': + // assume empty path (if seen at start) + checkSegment(uri, segment, i, false); + _path = ""; mark = i + 1; state = State.FRAGMENT; break; @@ -297,12 +316,13 @@ private void parse(State state, final String uri, final int offset, final int en state = State.ASTERISK; break; case '%': - encoded = true; - escapedSlash = 1; + encodedPath = true; + encodedCharacters = 2; + encodedValue = 0; mark = pathMark = segment = i; state = State.PATH; break; - case '.' : + case '.': dot = true; pathMark = segment = i; state = State.PATH; @@ -316,10 +336,11 @@ private void parse(State state, final String uri, final int offset, final int en pathMark = segment = i; state = State.PATH; } + break; } - continue; } + case SCHEME_OR_PATH: { switch (c) @@ -336,24 +357,25 @@ private void parse(State state, final String uri, final int offset, final int en state = State.PATH; break; case ';': - // must have been in a path + // must have been in a path mark = i + 1; state = State.PARAM; break; case '?': - // must have been in a path + // must have been in a path _path = uri.substring(mark, i); mark = i + 1; state = State.QUERY; break; case '%': - // must have be in an encoded path - encoded = true; - escapedSlash = 1; + // must have been in an encoded path + encodedPath = true; + encodedCharacters = 2; + encodedValue = 0; state = State.PATH; break; case '#': - // must have been in a path + // must have been in a path _path = uri.substring(mark, i); state = State.FRAGMENT; break; @@ -371,7 +393,6 @@ private void parse(State state, final String uri, final int offset, final int en mark = i + 1; state = State.HOST; break; - case '%': case '@': case ';': @@ -392,6 +413,7 @@ private void parse(State state, final String uri, final int offset, final int en } continue; } + case HOST: { switch (c) @@ -420,7 +442,7 @@ private void parse(State state, final String uri, final int offset, final int en default: break; } - continue; + break; } case IPV6: { @@ -445,7 +467,7 @@ private void parse(State state, final String uri, final int offset, final int en default: break; } - continue; + break; } case PORT: { @@ -465,54 +487,77 @@ else if (c == '/') segment = i + 1; state = State.PATH; } - continue; + break; } 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 '/': - checkSegment(uri, segment, i, false); - segment = i + 1; - break; - case '.': - dot |= segment == i; - break; - case '%': - encoded = true; - escapedSlash = 1; - break; - case '2': - escapedSlash = escapedSlash == 1 ? 2 : 0; - break; - case 'f': - case 'F': - if (escapedSlash == 2) - _ambiguous.add(Ambiguous.SEPARATOR); - escapedSlash = 0; - break; - default: - escapedSlash = 0; - break; + 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; + } + } } - continue; + else + { + switch (c) + { + 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) + 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; } case PARAM: { @@ -531,7 +576,7 @@ else if (c == '/') state = State.FRAGMENT; break; case '/': - encoded = true; + encodedPath = true; segment = i + 1; state = State.PATH; break; @@ -542,17 +587,32 @@ else if (c == '/') default: break; } - continue; + break; } 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; } - continue; + break; } case ASTERISK: { @@ -565,17 +625,19 @@ else if (c == '/') break; } default: - break; + throw new IllegalStateException(state.toString()); } } switch (state) { case START: + _path = ""; + checkSegment(uri, segment, end, false); break; - case SCHEME_OR_PATH: - _path = uri.substring(mark, end); + case ASTERISK: break; + case SCHEME_OR_PATH: case HOST_OR_PATH: _path = uri.substring(mark, end); break; @@ -588,11 +650,6 @@ else if (c == '/') case PORT: _port = TypeUtil.parseInt(uri, mark, end - mark, 10); break; - case ASTERISK: - break; - case FRAGMENT: - _fragment = uri.substring(mark, end); - break; case PARAM: _path = uri.substring(pathMark, end); _param = uri.substring(mark, end); @@ -604,11 +661,14 @@ else if (c == '/') case QUERY: _query = uri.substring(mark, end); break; - default: + case FRAGMENT: + _fragment = uri.substring(mark, end); break; + default: + throw new IllegalStateException(state.toString()); } - if (!encoded && !dot) + if (!encodedPath && !dot) { if (_param == null) _decodedPath = _path; @@ -629,19 +689,52 @@ else if (_path != null) * * An ambiguous path segment is one that is perhaps technically legal, but is considered undesirable to handle * due to possible ambiguity. Examples include segments like '..;', '%2e', '%2e%2e' etc. + * * @param uri The URI string * @param segment The inclusive starting index of the segment (excluding any '/') * @param end The exclusive end index of the segment */ private void checkSegment(String uri, int segment, int end, boolean param) { - if (!_ambiguous.contains(Ambiguous.SEGMENT)) + // This method is called once for every segment parsed. + // A URI like "/foo/" has two segments: "foo" and an empty segment. + // 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) + _violations.add(Violation.EMPTY); + + if (end == segment) { - Boolean ambiguous = __ambiguousSegments.get(uri, segment, end - segment); - if (ambiguous == Boolean.TRUE) - _ambiguous.add(Ambiguous.SEGMENT); - else if (param && ambiguous == Boolean.FALSE) - _ambiguous.add(Ambiguous.PARAM); + // Empty segments are not ambiguous if followed by a '#', '?' or end of string. + if (end >= uri.length() || ("#?".indexOf(uri.charAt(end)) >= 0)) + return; + + // If this empty segment is the first segment then it is ambiguous. + if (segment == 0) + { + _violations.add(Violation.EMPTY); + return; + } + + // Otherwise remember we have seen an empty segment, which is check if we see a subsequent segment. + if (!_emptySegment) + { + _emptySegment = true; + return; + } + } + + // Look for segment in the ambiguous segment index. + Boolean ambiguous = __ambiguousSegments.get(uri, segment, end - segment); + if (ambiguous == Boolean.TRUE) + { + // The segment is always ambiguous. + _violations.add(Violation.SEGMENT); + } + else if (param && ambiguous == Boolean.FALSE) + { + // The segment is ambiguous only when followed by a parameter. + _violations.add(Violation.PARAM); } } @@ -650,7 +743,15 @@ else if (param && ambiguous == Boolean.FALSE) */ public boolean hasAmbiguousSegment() { - return _ambiguous.contains(Ambiguous.SEGMENT); + return _violations.contains(Violation.SEGMENT); + } + + /** + * @return True if the URI empty segment that is ambiguous like '//' or '/;param/'. + */ + public boolean hasAmbiguousEmptySegment() + { + return _violations.contains(Violation.EMPTY); } /** @@ -658,7 +759,7 @@ public boolean hasAmbiguousSegment() */ public boolean hasAmbiguousSeparator() { - return _ambiguous.contains(Ambiguous.SEPARATOR); + return _violations.contains(Violation.SEPARATOR); } /** @@ -666,7 +767,15 @@ public boolean hasAmbiguousSeparator() */ public boolean hasAmbiguousParameter() { - return _ambiguous.contains(Ambiguous.PARAM); + return _violations.contains(Violation.PARAM); + } + + /** + * @return True if the URI has an encoded '%' character. + */ + public boolean hasAmbiguousEncoding() + { + return _violations.contains(Violation.ENCODING); } /** @@ -674,7 +783,23 @@ public boolean hasAmbiguousParameter() */ public boolean isAmbiguous() { - return !_ambiguous.isEmpty(); + return !_violations.isEmpty() && !(_violations.size() == 1 && _violations.contains(Violation.UTF16)); + } + + /** + * @return True if the URI has any Violations. + */ + public boolean hasViolations() + { + return !_violations.isEmpty(); + } + + /** + * @return True if the URI encodes UTF-16 characters with '%u'. + */ + public boolean hasUtf16Encoding() + { + return _violations.contains(Violation.UTF16); } public String getScheme() 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 291186e62be4..000000000000 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURIParseTest.java +++ /dev/null @@ -1,252 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995-2021 Mort Bay Consulting Pty Ltd and others. -// ------------------------------------------------------------------------ -// All rights reserved. This program and the accompanying materials -// are made available under the terms of the Eclipse Public License v1.0 -// and Apache License v2.0 which accompanies this distribution. -// -// The Eclipse Public License is available at -// http://www.eclipse.org/legal/epl-v10.html -// -// The Apache License v2.0 is available at -// http://www.opensource.org/licenses/apache2.0.php -// -// You may elect to redistribute this code under either of these licenses. -// ======================================================================== -// - -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 = new HttpURI(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"); - - HttpURI httpUri = new HttpURI(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 = new HttpURI(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 f6b95ba447e6..ddb71f29d940 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 @@ -18,13 +18,15 @@ package org.eclipse.jetty.http; +import java.net.URI; +import java.net.URISyntaxException; import java.net.URLEncoder; import java.nio.charset.StandardCharsets; 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.eclipse.jetty.util.MultiMap; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -33,10 +35,13 @@ 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.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 { @@ -287,87 +292,472 @@ 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(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(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 - {"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)}, + // @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(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 = new HttpURI(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))); + 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()); } } + + public static Stream testPathQueryTests() + { + return Arrays.stream(new Object[][] + { + // Simple path example + {"/path/info", "/path/info", EnumSet.noneOf(Violation.class)}, + + // legal non ambiguous relative paths + {"/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}, + {"../path/info", null, null}, + {"/path/%XX/info", null, null}, + {"/path/%2/F/info", null, null}, + + // ambiguous dot encodings + {"/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(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(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(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(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(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) + { + HttpURI uri = new HttpURI(); + + // If expected is null then it is a bad URI and should throw. + if (expected == null) + { + assertThrows(Throwable.class, () -> uri.parseRequestTarget(HttpMethod.GET.asString(), input)); + return; + } + + uri.parseRequestTarget(HttpMethod.GET.asString(), input); + assertThat(uri.getDecodedPath(), is(decodedPath)); + assertThat(uri.isAmbiguous(), is(!expected.isEmpty())); + 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 = new HttpURI(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 = new HttpURI(input); + + 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 = new HttpURI(input); + + 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 41182fc5551c..48b706c1cb92 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 @@ -1824,22 +1824,32 @@ public void setMetaData(org.eclipse.jetty.http.MetaData.Request request) setMethod(request.getMethod()); HttpURI uri = request.getURI(); - - boolean ambiguous = uri.isAmbiguous(); - if (ambiguous) + boolean ambiguous = false; + if (uri.hasViolations()) { - // replaced in jetty-10 with URICompliance from the HttpConfiguration + // Replaced in jetty-10 with URICompliance from the HttpConfiguration. Connection connection = _channel == null ? null : _channel.getConnection(); HttpCompliance compliance = connection instanceof HttpConnection ? ((HttpConnection)connection).getHttpCompliance() : _channel != null ? _channel.getConnector().getBean(HttpCompliance.class) : null; - if (uri.hasAmbiguousSegment() && (compliance == null || compliance.sections().contains(HttpComplianceSection.NO_AMBIGUOUS_PATH_SEGMENTS))) - throw new BadMessageException("Ambiguous segment in URI"); - if (uri.hasAmbiguousSeparator() && (compliance == null || compliance.sections().contains(HttpComplianceSection.NO_AMBIGUOUS_PATH_SEPARATORS))) - throw new BadMessageException("Ambiguous separator in URI"); - if (uri.hasAmbiguousParameter() && (compliance == null || compliance.sections().contains(HttpComplianceSection.NO_AMBIGUOUS_PATH_PARAMETERS))) - throw new BadMessageException("Ambiguous path parameter in URI"); + if (uri.hasUtf16Encoding() && (compliance == null || compliance.sections().contains(HttpComplianceSection.NO_UTF16_ENCODINGS))) + throw new BadMessageException("UTF16 % encoding not supported"); + + ambiguous = uri.isAmbiguous(); + if (ambiguous) + { + if (uri.hasAmbiguousSegment() && (compliance == null || compliance.sections().contains(HttpComplianceSection.NO_AMBIGUOUS_PATH_SEGMENTS))) + throw new BadMessageException("Ambiguous segment in URI"); + if (uri.hasAmbiguousEmptySegment() && (compliance == null || compliance.sections().contains(HttpComplianceSection.NO_AMBIGUOUS_EMPTY_SEGMENT))) + throw new BadMessageException("Ambiguous empty segment in URI"); + if (uri.hasAmbiguousSeparator() && (compliance == null || compliance.sections().contains(HttpComplianceSection.NO_AMBIGUOUS_PATH_SEPARATORS))) + throw new BadMessageException("Ambiguous separator in URI"); + if (uri.hasAmbiguousParameter() && (compliance == null || compliance.sections().contains(HttpComplianceSection.NO_AMBIGUOUS_PATH_PARAMETERS))) + throw new BadMessageException("Ambiguous path parameter in URI"); + if (uri.hasAmbiguousEncoding() && (compliance == null || compliance.sections().contains(HttpComplianceSection.NO_AMBIGUOUS_PATH_ENCODING))) + throw new BadMessageException("Ambiguous path encoding in URI"); + } } _originalURI = uri.isAbsolute() && request.getHttpVersion() != HttpVersion.HTTP_2 ? uri.toString() : uri.getPathQuery(); 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 f33bbceaffe7..1ccd7b404d55 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 @@ -1837,6 +1837,24 @@ 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).setHttpCompliance(HttpCompliance.RFC7230); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); + + HttpCompliance.CUSTOM0.sections().clear(); + HttpCompliance.CUSTOM0.sections().addAll(HttpCompliance.RFC7230.sections()); + HttpCompliance.CUSTOM0.sections().add(HttpComplianceSection.NO_UTF16_ENCODINGS); + _connector.getBean(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.CUSTOM0); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); + } + @Test public void testAmbiguousParameters() throws Exception { @@ -1895,6 +1913,70 @@ public void testAmbiguousSeparators() throws Exception assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); } + public void setComplianceModes(HttpComplianceSection... complianceSections) + { + setComplianceModes(null, complianceSections); + } + + public void setComplianceModes(HttpCompliance compliance, HttpComplianceSection... additionalSections) + { + HttpCompliance.CUSTOM0.sections().clear(); + if (compliance != null) + HttpCompliance.CUSTOM0.sections().addAll(compliance.sections()); + HttpCompliance.CUSTOM0.sections().addAll(Arrays.asList(additionalSections)); + _connector.getBean(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.CUSTOM0); + } + + @Test + public void testAmbiguousPaths() throws Exception + { + _handler._checker = (request, response) -> + { + response.getOutputStream().println("servletPath=" + request.getServletPath()); + response.getOutputStream().println("pathInfo=" + request.getPathInfo()); + return true; + }; + String request = "GET /unnormal/.././path/ambiguous%2f%2e%2e/%2e;/info HTTP/1.0\r\n" + + "Host: whatever\r\n" + + "\r\n"; + + setComplianceModes(HttpCompliance.RFC7230); + assertThat(_connector.getResponse(request), Matchers.allOf( + startsWith("HTTP/1.1 200"), + containsString("pathInfo=/path/info"))); + + setComplianceModes(HttpComplianceSection.NO_AMBIGUOUS_PATH_SEGMENTS); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); + } + + @Test + public void testAmbiguousEncoding() throws Exception + { + _handler._checker = (request, response) -> true; + String request = "GET /ambiguous/encoded/%25/path HTTP/1.0\r\n" + + "Host: whatever\r\n" + + "\r\n"; + + setComplianceModes(HttpCompliance.RFC7230); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); + setComplianceModes(HttpCompliance.RFC7230, HttpComplianceSection.NO_AMBIGUOUS_PATH_ENCODING); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); + } + + @Test + public void testAmbiguousDoubleSlash() throws Exception + { + _handler._checker = (request, response) -> true; + String request = "GET /ambiguous/doubleSlash// HTTP/1.0\r\n" + + "Host: whatever\r\n" + + "\r\n"; + + setComplianceModes(HttpCompliance.RFC7230); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); + setComplianceModes(HttpCompliance.RFC7230, HttpComplianceSection.NO_AMBIGUOUS_EMPTY_SEGMENT); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); + } + private static long getFileCount(Path path) { try (Stream s = Files.list(path)) 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 6818e5440928..19ed06e0db13 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 @@ -475,8 +475,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 + // In Jetty-10 UTF16 encoding is only supported with UriCompliance.Violation.UTF16_ENCODINGS. // This is wrong. This is a codepoint not a char builder.append((char)(0xffff & TypeUtil.parseInt(path, i + 2, 4, 16))); i += 5; } @@ -562,8 +561,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 + // In Jetty-10 UTF16 encoding is only supported with UriCompliance.Violation.UTF16_ENCODINGS. // This is wrong. This is a codepoint not a char 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 03a4aa7a327a..48e9b3fa3147 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 @@ -271,18 +271,25 @@ public void testProtectedTarget() throws Exception server.start(); 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)); + assertThat(HttpTester.parseResponse(connector.getResponse("GET /WEB-INF%u002ftest.xml HTTP/1.1\r\nHost: localhost:8080\r\nConnection: close\r\n\r\n")).getStatus(), is(HttpStatus.NOT_FOUND_404)); } @Test