From 06e1a7e88d6f00088a6c9233fc87a4d9e965cedd Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Tue, 2 Mar 2021 11:59:16 +0100 Subject: [PATCH] URI compliance modes for #6001 (#6006) * Fix #4275 separate compliance modes for ambiguous URI segments and separators default modes allows both ambiguous separators and segments, but still forbids ambiguous parameters Co-authored-by: Joakim Erdfelt --- .../java/org/eclipse/jetty/http/HttpURI.java | 26 +++- .../org/eclipse/jetty/http/UriCompliance.java | 96 ++++++++++----- .../org/eclipse/jetty/http/HttpURITest.java | 114 ++++++++++-------- .../jetty/server/HttpConfiguration.java | 2 +- .../org/eclipse/jetty/server/Request.java | 2 + .../org/eclipse/jetty/server/RequestTest.java | 29 ++++- .../jetty/servlet/DefaultServletTest.java | 40 +++--- .../eclipse/jetty/servlet/RequestURITest.java | 2 +- 8 files changed, 197 insertions(+), 114 deletions(-) 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 ccac2fd82a01..bc6866ba0b52 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 @@ -51,7 +51,8 @@ public interface HttpURI enum Ambiguous { SEGMENT, - SEPARATOR + SEPARATOR, + PARAM } static Mutable build() @@ -139,7 +140,7 @@ static Immutable from(String scheme, String host, int port, String pathQuery) boolean isAbsolute(); /** - * @return True if the URI has either an {@link #hasAmbiguousSegment()} or {@link #hasAmbiguousSeparator()}. + * @return True if the URI has either an {@link #hasAmbiguousParameter()}, {@link #hasAmbiguousSegment()} or {@link #hasAmbiguousSeparator()}. */ boolean isAmbiguous(); @@ -153,6 +154,11 @@ static Immutable from(String scheme, String host, int port, String pathQuery) */ boolean hasAmbiguousSeparator(); + /** + * @return True if the URI has a possibly ambiguous path parameter like '..;' + */ + boolean hasAmbiguousParameter(); + default URI toURI() { try @@ -374,6 +380,12 @@ public boolean hasAmbiguousSeparator() return _ambiguous.contains(Mutable.Ambiguous.SEPARATOR); } + @Override + public boolean hasAmbiguousParameter() + { + return _ambiguous.contains(Ambiguous.PARAM); + } + @Override public String toString() { @@ -709,6 +721,12 @@ public boolean hasAmbiguousSeparator() return _ambiguous.contains(Mutable.Ambiguous.SEPARATOR); } + @Override + public boolean hasAmbiguousParameter() + { + return _ambiguous.contains(Ambiguous.PARAM); + } + public Mutable normalize() { HttpScheme scheme = _scheme == null ? null : HttpScheme.CACHE.get(_scheme); @@ -1241,8 +1259,10 @@ private void checkSegment(String uri, int segment, int end, boolean param) if (!_ambiguous.contains(Ambiguous.SEGMENT)) { Boolean ambiguous = __ambiguousSegments.get(uri, segment, end - segment); - if (ambiguous == Boolean.TRUE || (param && ambiguous == Boolean.FALSE)) + if (ambiguous == Boolean.TRUE) _ambiguous.add(Ambiguous.SEGMENT); + else if (param && ambiguous == Boolean.FALSE) + _ambiguous.add(Ambiguous.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 9528f68036ac..656c25e9036a 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 @@ -15,7 +15,6 @@ import java.util.Arrays; import java.util.EnumSet; -import java.util.List; import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; @@ -30,27 +29,39 @@ /** * URI compliance modes for Jetty request handling. - * A Compliance mode consists of a set of {@link Violation}s which are applied + * A Compliance mode consists of a set of {@link Violation}s which are allowed * when the mode is enabled. */ public final class UriCompliance implements ComplianceViolation.Mode { protected static final Logger LOG = LoggerFactory.getLogger(UriCompliance.class); - // These are compliance violations, which may optionally be allowed by the compliance mode, which mean that - // the relevant section of the RFC is not strictly adhered to. + /** + * These are URI compliance violations, which may be allowed by the compliance mode. Currently all these + * violations are for additional criteria in excess of the strict requirements of rfc3986. + */ public enum Violation implements ComplianceViolation { + /** + * Ambiguous path segments e.g. /foo/%2e%2e/bar + */ AMBIGUOUS_PATH_SEGMENT("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path segment"), - AMBIGUOUS_PATH_SEPARATOR("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path separator"); + /** + * Ambiguous path separator within a URI segment e.g. /foo/b%2fr + */ + AMBIGUOUS_PATH_SEPARATOR("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path separator"), + /** + * Ambiguous path parameters within a URI segment e.g. /foo/..;/bar + */ + AMBIGUOUS_PATH_PARAMETER("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path parameter"); - private final String url; - private final String description; + private final String _url; + private final String _description; Violation(String url, String description) { - this.url = url; - this.description = description; + _url = url; + _description = description; } @Override @@ -62,24 +73,50 @@ public String getName() @Override public String getURL() { - return url; + return _url; } @Override public String getDescription() { - return description; + return _description; } } - public static final UriCompliance SAFE = new UriCompliance("SAFE", noneOf(Violation.class)); - public static final UriCompliance STRICT = new UriCompliance("STRICT", allOf(Violation.class)); - private static final List KNOWN_MODES = Arrays.asList(SAFE, STRICT); + /** + * The default compliance mode that extends RFC3986 compliance with additional violations to avoid ambiguous URIs + */ + public static final UriCompliance DEFAULT = new UriCompliance("DEFAULT", noneOf(Violation.class)); + + /** + * LEGACY compliance mode that disallows only ambiguous path parameters as per Jetty-9.4 + */ + public static final UriCompliance LEGACY = new UriCompliance("LEGACY", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT, Violation.AMBIGUOUS_PATH_SEPARATOR)); + + /** + * Compliance mode that exactly follows RFC3986, including allowing all additional ambiguous URI Violations + */ + public static final UriCompliance RFC3986 = new UriCompliance("RFC3986", allOf(Violation.class)); + + /** + * @deprecated equivalent to DEFAULT + */ + @SuppressWarnings("DeprecatedIsStillUsed") + @Deprecated + public static final UriCompliance SAFE = new UriCompliance("SAFE", DEFAULT.getAllowed()); + + /** + * @deprecated equivalent to RFC3986 + */ + @SuppressWarnings("DeprecatedIsStillUsed") + @Deprecated + public static final UriCompliance STRICT = new UriCompliance("STRICT", RFC3986.getAllowed()); + private static final AtomicInteger __custom = new AtomicInteger(); public static UriCompliance valueOf(String name) { - for (UriCompliance compliance : KNOWN_MODES) + for (UriCompliance compliance : Arrays.asList(DEFAULT, LEGACY, RFC3986, STRICT, SAFE)) { if (compliance.getName().equals(name)) return compliance; @@ -91,20 +128,23 @@ public static UriCompliance valueOf(String name) /** * Create compliance set from string. *

- * Format: + * Format: <BASE>[,[-]<violation>]... *

+ *

BASE is one of:

*
*
0
No {@link Violation}s
*
*
All {@link Violation}s
- *
RFC2616
The set of {@link Violation}s application to https://tools.ietf.org/html/rfc2616, - * but not https://tools.ietf.org/html/rfc7230
- *
RFC7230
The set of {@link Violation}s application to https://tools.ietf.org/html/rfc7230
- *
name
Any of the known modes defined in {@link UriCompliance#KNOWN_MODES}
+ *
<name>
The name of a static instance of {@link UriCompliance} (e.g. {@link UriCompliance#RFC3986}). *
*

* The remainder of the list can contain then names of {@link Violation}s to include them in the mode, or prefixed - * with a '-' to exclude thm from the mode. + * with a '-' to exclude thm from the mode. Examples are: *

+ *
+ *
0,AMBIGUOUS_PATH_PARAMETER
Only allow {@link Violation#AMBIGUOUS_PATH_PARAMETER}
+ *
*,-AMBIGUOUS_PATH_PARAMETER
Only all except {@link Violation#AMBIGUOUS_PATH_PARAMETER}
+ *
RFC3986,AMBIGUOUS_PATH_PARAMETER
Same as RFC3986 plus {@link Violation#AMBIGUOUS_PATH_PARAMETER}
+ *
* * @param spec A string in the format of a comma separated list starting with one of the following strings: * @return the compliance from the string spec @@ -150,19 +190,19 @@ public static UriCompliance from(String spec) } private final String _name; - private final Set _violations; + private final Set _allowed; private UriCompliance(String name, Set violations) { Objects.requireNonNull(violations); _name = name; - _violations = unmodifiableSet(violations.isEmpty() ? noneOf(Violation.class) : copyOf(violations)); + _allowed = unmodifiableSet(violations.isEmpty() ? noneOf(Violation.class) : copyOf(violations)); } @Override public boolean allows(ComplianceViolation violation) { - return violation instanceof Violation && _violations.contains(violation); + return violation instanceof Violation && _allowed.contains(violation); } @Override @@ -179,7 +219,7 @@ public String getName() @Override public Set getAllowed() { - return _violations; + return _allowed; } @Override @@ -197,7 +237,7 @@ public Set getKnown() */ public UriCompliance with(String name, Violation... violations) { - Set union = _violations.isEmpty() ? EnumSet.noneOf(Violation.class) : copyOf(_violations); + Set union = _allowed.isEmpty() ? EnumSet.noneOf(Violation.class) : copyOf(_allowed); union.addAll(copyOf(violations)); return new UriCompliance(name, union); } @@ -211,7 +251,7 @@ public UriCompliance with(String name, Violation... violations) */ public UriCompliance without(String name, Violation... violations) { - Set remainder = _violations.isEmpty() ? EnumSet.noneOf(Violation.class) : copyOf(_violations); + Set remainder = _allowed.isEmpty() ? EnumSet.noneOf(Violation.class) : copyOf(_allowed); remainder.removeAll(copyOf(violations)); return new UriCompliance(name, remainder); } @@ -219,7 +259,7 @@ public UriCompliance without(String name, Violation... violations) @Override public String toString() { - return String.format("%s%s", _name, _violations); + return String.format("%s%s", _name, _allowed); } private static Set copyOf(Violation[] violations) 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 eb6fd08cbb4d..7c23d38eb2ed 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 @@ -14,8 +14,10 @@ package org.eclipse.jetty.http; import java.util.Arrays; +import java.util.EnumSet; import java.util.stream.Stream; +import org.eclipse.jetty.http.HttpURI.Ambiguous; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -320,79 +322,85 @@ public static Stream decodePathTests() return Arrays.stream(new Object[][] { // Simple path example - {"http://host/path/info", "/path/info", false, false}, - {"//host/path/info", "/path/info", false, false}, - {"/path/info", "/path/info", false, false}, + {"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)}, // legal non ambiguous relative paths - {"http://host/../path/info", null, false, false}, - {"http://host/path/../info", "/info", false, false}, - {"http://host/path/./info", "/path/info", false, false}, - {"//host/path/../info", "/info", false, false}, - {"//host/path/./info", "/path/info", false, false}, - {"/path/../info", "/info", false, false}, - {"/path/./info", "/path/info", false, false}, - {"path/../info", "info", false, false}, - {"path/./info", "path/info", false, false}, + {"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)}, // illegal paths - {"//host/../path/info", null, false, false}, - {"/../path/info", null, false, false}, - {"../path/info", null, false, false}, - {"/path/%XX/info", null, false, false}, - {"/path/%2/F/info", null, false, false}, - - // ambiguous dot encodings or parameter inclusions - {"scheme://host/path/%2e/info", "/path/./info", true, false}, - {"scheme:/path/%2e/info", "/path/./info", true, false}, - {"/path/%2e/info", "/path/./info", true, false}, - {"path/%2e/info/", "path/./info/", true, false}, - {"/path/%2e%2e/info", "/path/../info", true, false}, - {"/path/%2e%2e;/info", "/path/../info", true, false}, - {"/path/%2e%2e;param/info", "/path/../info", true, false}, - {"/path/%2e%2e;param;other/info;other", "/path/../info", true, false}, - {"/path/.;/info", "/path/./info", true, false}, - {"/path/.;param/info", "/path/./info", true, false}, - {"/path/..;/info", "/path/../info", true, false}, - {"/path/..;param/info", "/path/../info", true, false}, - {"%2e/info", "./info", true, false}, - {"%2e%2e/info", "../info", true, false}, - {"%2e%2e;/info", "../info", true, false}, - {".;/info", "./info", true, false}, - {".;param/info", "./info", true, false}, - {"..;/info", "../info", true, false}, - {"..;param/info", "../info", true, false}, - {"%2e", ".", true, false}, - {"%2e.", "..", true, false}, - {".%2e", "..", true, false}, - {"%2e%2e", "..", true, false}, + {"//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)}, + + // 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)}, + + // 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)}, // ambiguous segment separators - {"/path/%2f/info", "/path///info", false, true}, - {"%2f/info", "//info", false, true}, - {"%2F/info", "//info", false, true}, - {"/path/%2f../info", "/path//../info", false, true}, - {"/path/%2f/..;/info", "/path///../info", true, true}, + {"/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)}, + + // 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)}, // 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", false, false}, - {"http://localhost:9000/\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32", "/\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32", false, false}, + {"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-enable-check : AvoidEscapedUnicodeCharactersCheck }).map(Arguments::of); } @ParameterizedTest @MethodSource("decodePathTests") - public void testDecodedPath(String input, String decodedPath, boolean ambiguousSegment, boolean ambiguousSeparator) + public void testDecodedPath(String input, String decodedPath, EnumSet expected) { try { HttpURI uri = HttpURI.from(input); assertThat(uri.getDecodedPath(), is(decodedPath)); - assertThat(uri.hasAmbiguousSegment(), is(ambiguousSegment)); - assertThat(uri.hasAmbiguousSeparator(), is(ambiguousSeparator)); - assertThat(uri.isAmbiguous(), is(ambiguousSegment || ambiguousSeparator)); + 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))); } catch (Exception e) { diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java index ed8071c48491..563de9534cf6 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java @@ -70,7 +70,7 @@ public class HttpConfiguration implements Dumpable private long _minRequestDataRate; private long _minResponseDataRate; private HttpCompliance _httpCompliance = HttpCompliance.RFC7230; - private UriCompliance _uriCompliance = UriCompliance.SAFE; + private UriCompliance _uriCompliance = UriCompliance.DEFAULT; private CookieCompliance _requestCookieCompliance = CookieCompliance.RFC6265; private CookieCompliance _responseCookieCompliance = CookieCompliance.RFC6265; private boolean _notifyRemoteAsyncErrors = true; 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 0fd1442af18d..1812eea6d4c1 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 @@ -1699,6 +1699,8 @@ public void setMetaData(MetaData.Request request) throw new BadMessageException("Ambiguous 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.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 9e712da22a58..28a736d5b728 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 @@ -1696,15 +1696,32 @@ public void testGetterSafeFromNullPointerException() } @Test - public void testAmbiguousSegments() throws Exception + public void testAmbiguousParameters() throws Exception { _handler._checker = (request, response) -> true; String request = "GET /ambiguous/..;/path HTTP/1.0\r\n" + "Host: whatever\r\n" + "\r\n"; - _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.SAFE); + _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 400")); + _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); + } + + @Test + public void testAmbiguousSegments() throws Exception + { + _handler._checker = (request, response) -> true; + String request = "GET /ambiguous/%2e%2e/path 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.STRICT); + _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.LEGACY); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); + _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986); assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); } @@ -1715,9 +1732,11 @@ public void testAmbiguousSeparators() throws Exception String request = "GET /ambiguous/%2f/path HTTP/1.0\r\n" + "Host: whatever\r\n" + "\r\n"; - _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.SAFE); + _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.DEFAULT); assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400")); - _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.STRICT); + _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.LEGACY); + assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); + _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986); assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200")); } diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java index 68db22acaff5..1092fc91f2ff 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java @@ -46,10 +46,8 @@ import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpTester; -import org.eclipse.jetty.http.UriCompliance; import org.eclipse.jetty.logging.StacklessLogging; import org.eclipse.jetty.server.HttpConfiguration; -import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.ResourceContentFactory; import org.eclipse.jetty.server.ResourceService; @@ -112,7 +110,6 @@ public void init() throws Exception connector = new LocalConnector(server); connector.getConnectionFactory(HttpConfiguration.ConnectionFactory.class).getHttpConfiguration().setSendServerVersion(false); - connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.STRICT); // allow ambiguous path segments File extraJarResources = MavenTestingUtils.getTestResourceFile(ODD_JAR); URL[] urls = new URL[]{extraJarResources.toURI().toURL()}; @@ -253,11 +250,8 @@ public void testListingFilenamesOnly() throws Exception String resBasePath = docRoot.toAbsolutePath().toString(); defholder.setInitParameter("resourceBase", resBasePath); - StringBuffer req1 = new StringBuffer(); - req1.append("GET /context/one/deep/ HTTP/1.0\n"); - req1.append("\n"); - - String rawResponse = connector.getResponse(req1.toString()); + String req1 = "GET /context/one/deep/ HTTP/1.0\n\n"; + String rawResponse = connector.getResponse(req1); HttpTester.Response response = HttpTester.parseResponse(rawResponse); assertThat(response.getStatus(), is(HttpStatus.OK_200)); @@ -454,10 +448,17 @@ public static Stream contextBreakoutScenarios() (response) -> assertThat(response.getContent(), not(containsString("Sssh"))) ); + scenarios.addScenario( + "GET " + prefix + "/..;/..;/sekret/pass", + "GET " + prefix + "/..;/..;/sekret/pass HTTP/1.0\r\n\r\n", + prefix.endsWith("?") ? HttpStatus.NOT_FOUND_404 : HttpStatus.BAD_REQUEST_400, + (response) -> assertThat(response.getContent(), not(containsString("Sssh"))) + ); + scenarios.addScenario( "GET " + prefix + "/%2E%2E/%2E%2E/sekret/pass", - "GET " + prefix + "/ HTTP/1.0\r\n\r\n", - HttpStatus.NOT_FOUND_404, + "GET " + prefix + "/%2E%2E/%2E%2E/sekret/pass HTTP/1.0\r\n\r\n", + prefix.endsWith("?") ? HttpStatus.NOT_FOUND_404 : HttpStatus.BAD_REQUEST_400, (response) -> assertThat(response.getContent(), not(containsString("Sssh"))) ); @@ -469,12 +470,6 @@ public static Stream contextBreakoutScenarios() "GET " + prefix + "/../index.html HTTP/1.0\r\n\r\n", HttpStatus.NOT_FOUND_404 ); - - scenarios.addScenario( - "GET " + prefix + "/%2E%2E/index.html", - "GET " + prefix + "/%2E%2E/index.html HTTP/1.0\r\n\r\n", - HttpStatus.NOT_FOUND_404 - ); } else { @@ -484,15 +479,14 @@ public static Stream contextBreakoutScenarios() HttpStatus.OK_200, (response) -> assertThat(response.getContent(), containsString("Hello Index")) ); - - scenarios.addScenario( - "GET " + prefix + "/%2E%2E/index.html", - "GET " + prefix + "/%2E%2E/index.html HTTP/1.0\r\n\r\n", - HttpStatus.OK_200, - (response) -> assertThat(response.getContent(), containsString("Hello Index")) - ); } + scenarios.addScenario( + "GET " + prefix + "/%2E%2E/index.html", + "GET " + prefix + "/%2E%2E/index.html HTTP/1.0\r\n\r\n", + prefix.endsWith("?") ? HttpStatus.NOT_FOUND_404 : HttpStatus.BAD_REQUEST_400 + ); + scenarios.addScenario( "GET " + prefix + "/../../", "GET " + prefix + "/../../ HTTP/1.0\r\n\r\n", diff --git a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/RequestURITest.java b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/RequestURITest.java index 12630dc386fb..730f29165fe3 100644 --- a/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/RequestURITest.java +++ b/jetty-servlet/src/test/java/org/eclipse/jetty/servlet/RequestURITest.java @@ -109,7 +109,7 @@ public static void startServer() throws Exception ServerConnector connector = new ServerConnector(server); connector.setPort(0); server.addConnector(connector); - connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.STRICT); // Allow ambiguous segments + connector.getConnectionFactory(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.RFC3986); ServletContextHandler context = new ServletContextHandler(); context.setContextPath("/");