Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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 <joakim.erdfelt@gmail.com>
This commit is contained in:
Greg Wilkins 2021-03-02 11:59:16 +01:00 committed by GitHub
parent d9eefc9231
commit 06e1a7e88d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 196 additions and 113 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
View File

@ -51,7 +51,8 @@ public interface HttpURI
enum Ambiguous enum Ambiguous
{ {
SEGMENT, SEGMENT,
SEPARATOR SEPARATOR,
PARAM
} }
static Mutable build() static Mutable build()
@ -139,7 +140,7 @@ public interface HttpURI
boolean isAbsolute(); 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(); boolean isAmbiguous();
@ -153,6 +154,11 @@ public interface HttpURI
*/ */
boolean hasAmbiguousSeparator(); boolean hasAmbiguousSeparator();
/**
* @return True if the URI has a possibly ambiguous path parameter like '..;'
*/
boolean hasAmbiguousParameter();
default URI toURI() default URI toURI()
{ {
try try
@ -374,6 +380,12 @@ public interface HttpURI
return _ambiguous.contains(Mutable.Ambiguous.SEPARATOR); return _ambiguous.contains(Mutable.Ambiguous.SEPARATOR);
} }
@Override
public boolean hasAmbiguousParameter()
{
return _ambiguous.contains(Ambiguous.PARAM);
}
@Override @Override
public String toString() public String toString()
{ {
@ -709,6 +721,12 @@ public interface HttpURI
return _ambiguous.contains(Mutable.Ambiguous.SEPARATOR); return _ambiguous.contains(Mutable.Ambiguous.SEPARATOR);
} }
@Override
public boolean hasAmbiguousParameter()
{
return _ambiguous.contains(Ambiguous.PARAM);
}
public Mutable normalize() public Mutable normalize()
{ {
HttpScheme scheme = _scheme == null ? null : HttpScheme.CACHE.get(_scheme); HttpScheme scheme = _scheme == null ? null : HttpScheme.CACHE.get(_scheme);
@ -1241,8 +1259,10 @@ public interface HttpURI
if (!_ambiguous.contains(Ambiguous.SEGMENT)) if (!_ambiguous.contains(Ambiguous.SEGMENT))
{ {
Boolean ambiguous = __ambiguousSegments.get(uri, segment, end - 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); _ambiguous.add(Ambiguous.SEGMENT);
else if (param && ambiguous == Boolean.FALSE)
_ambiguous.add(Ambiguous.PARAM);
} }
} }
} }

96
jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java
View File

@ -15,7 +15,6 @@ package org.eclipse.jetty.http;
import java.util.Arrays; import java.util.Arrays;
import java.util.EnumSet; import java.util.EnumSet;
import java.util.List;
import java.util.Objects; import java.util.Objects;
import java.util.Set; import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;
@ -30,27 +29,39 @@ import static java.util.EnumSet.noneOf;
/** /**
* URI compliance modes for Jetty request handling. * 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. * when the mode is enabled.
*/ */
public final class UriCompliance implements ComplianceViolation.Mode public final class UriCompliance implements ComplianceViolation.Mode
{ {
protected static final Logger LOG = LoggerFactory.getLogger(UriCompliance.class); 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 public enum Violation implements ComplianceViolation
{ {
/**
* Ambiguous path segments e.g. <code>/foo/%2e%2e/bar</code>
*/
AMBIGUOUS_PATH_SEGMENT("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path segment"), 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. <code>/foo/b%2fr</code>
*/
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. <code>/foo/..;/bar</code>
*/
AMBIGUOUS_PATH_PARAMETER("https://tools.ietf.org/html/rfc3986#section-3.3", "Ambiguous URI path parameter");
private final String url; private final String _url;
private final String description; private final String _description;
Violation(String url, String description) Violation(String url, String description)
{ {
this.url = url; _url = url;
this.description = description; _description = description;
} }
@Override @Override
@ -62,24 +73,50 @@ public final class UriCompliance implements ComplianceViolation.Mode
@Override @Override
public String getURL() public String getURL()
{ {
return url; return _url;
} }
@Override @Override
public String getDescription() 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)); * The default compliance mode that extends RFC3986 compliance with additional violations to avoid ambiguous URIs
private static final List<UriCompliance> KNOWN_MODES = Arrays.asList(SAFE, STRICT); */
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(); private static final AtomicInteger __custom = new AtomicInteger();
public static UriCompliance valueOf(String name) 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)) if (compliance.getName().equals(name))
return compliance; return compliance;
@ -91,20 +128,23 @@ public final class UriCompliance implements ComplianceViolation.Mode
/** /**
* Create compliance set from string. * Create compliance set from string.
* <p> * <p>
* Format: * Format: &lt;BASE&gt;[,[-]&lt;violation&gt;]...
* </p> * </p>
* <p>BASE is one of:</p>
* <dl> * <dl>
* <dt>0</dt><dd>No {@link Violation}s</dd> * <dt>0</dt><dd>No {@link Violation}s</dd>
* <dt>*</dt><dd>All {@link Violation}s</dd> * <dt>*</dt><dd>All {@link Violation}s</dd>
* <dt>RFC2616</dt><dd>The set of {@link Violation}s application to https://tools.ietf.org/html/rfc2616, * <dt>&lt;name&gt;</dt><dd>The name of a static instance of {@link UriCompliance} (e.g. {@link UriCompliance#RFC3986}).
* but not https://tools.ietf.org/html/rfc7230</dd>
* <dt>RFC7230</dt><dd>The set of {@link Violation}s application to https://tools.ietf.org/html/rfc7230</dd>
* <dt>name</dt><dd>Any of the known modes defined in {@link UriCompliance#KNOWN_MODES}</dd>
* </dl> * </dl>
* <p> * <p>
* The remainder of the list can contain then names of {@link Violation}s to include them in the mode, or prefixed * 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:
* </p> * </p>
* <dl>
* <dt><code>0,AMBIGUOUS_PATH_PARAMETER</code></dt><dd>Only allow {@link Violation#AMBIGUOUS_PATH_PARAMETER}</dd>
* <dt><code>*,-AMBIGUOUS_PATH_PARAMETER</code></dt><dd>Only all except {@link Violation#AMBIGUOUS_PATH_PARAMETER}</dd>
* <dt><code>RFC3986,AMBIGUOUS_PATH_PARAMETER</code></dt><dd>Same as RFC3986 plus {@link Violation#AMBIGUOUS_PATH_PARAMETER}</dd>
* </dl>
* *
* @param spec A string in the format of a comma separated list starting with one of the following strings: * @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 * @return the compliance from the string spec
@ -150,19 +190,19 @@ public final class UriCompliance implements ComplianceViolation.Mode
} }
private final String _name; private final String _name;
private final Set<Violation> _violations; private final Set<Violation> _allowed;
private UriCompliance(String name, Set<Violation> violations) private UriCompliance(String name, Set<Violation> violations)
{ {
Objects.requireNonNull(violations); Objects.requireNonNull(violations);
_name = name; _name = name;
_violations = unmodifiableSet(violations.isEmpty() ? noneOf(Violation.class) : copyOf(violations)); _allowed = unmodifiableSet(violations.isEmpty() ? noneOf(Violation.class) : copyOf(violations));
} }
@Override @Override
public boolean allows(ComplianceViolation violation) public boolean allows(ComplianceViolation violation)
{ {
return violation instanceof Violation && _violations.contains(violation); return violation instanceof Violation && _allowed.contains(violation);
} }
@Override @Override
@ -179,7 +219,7 @@ public final class UriCompliance implements ComplianceViolation.Mode
@Override @Override
public Set<Violation> getAllowed() public Set<Violation> getAllowed()
{ {
return _violations; return _allowed;
} }
@Override @Override
@ -197,7 +237,7 @@ public final class UriCompliance implements ComplianceViolation.Mode
*/ */
public UriCompliance with(String name, Violation... violations) public UriCompliance with(String name, Violation... violations)
{ {
Set<Violation> union = _violations.isEmpty() ? EnumSet.noneOf(Violation.class) : copyOf(_violations); Set<Violation> union = _allowed.isEmpty() ? EnumSet.noneOf(Violation.class) : copyOf(_allowed);
union.addAll(copyOf(violations)); union.addAll(copyOf(violations));
return new UriCompliance(name, union); return new UriCompliance(name, union);
} }
@ -211,7 +251,7 @@ public final class UriCompliance implements ComplianceViolation.Mode
*/ */
public UriCompliance without(String name, Violation... violations) public UriCompliance without(String name, Violation... violations)
{ {
Set<Violation> remainder = _violations.isEmpty() ? EnumSet.noneOf(Violation.class) : copyOf(_violations); Set<Violation> remainder = _allowed.isEmpty() ? EnumSet.noneOf(Violation.class) : copyOf(_allowed);
remainder.removeAll(copyOf(violations)); remainder.removeAll(copyOf(violations));
return new UriCompliance(name, remainder); return new UriCompliance(name, remainder);
} }
@ -219,7 +259,7 @@ public final class UriCompliance implements ComplianceViolation.Mode
@Override @Override
public String toString() public String toString()
{ {
return String.format("%s%s", _name, _violations); return String.format("%s%s", _name, _allowed);
} }
private static Set<Violation> copyOf(Violation[] violations) private static Set<Violation> copyOf(Violation[] violations)

112
jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java
View File

@ -14,8 +14,10 @@
package org.eclipse.jetty.http; package org.eclipse.jetty.http;
import java.util.Arrays; import java.util.Arrays;
import java.util.EnumSet;
import java.util.stream.Stream; import java.util.stream.Stream;
import org.eclipse.jetty.http.HttpURI.Ambiguous;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.Arguments;
@ -320,79 +322,85 @@ public class HttpURITest
return Arrays.stream(new Object[][] return Arrays.stream(new Object[][]
{ {
// Simple path example // Simple path example
{"http://host/path/info", "/path/info", false, false}, {"http://host/path/info", "/path/info", EnumSet.noneOf(Ambiguous.class)},
{"//host/path/info", "/path/info", false, false}, {"//host/path/info", "/path/info", EnumSet.noneOf(Ambiguous.class)},
{"/path/info", "/path/info", false, false}, {"/path/info", "/path/info", EnumSet.noneOf(Ambiguous.class)},
// legal non ambiguous relative paths // legal non ambiguous relative paths
{"http://host/../path/info", null, false, false}, {"http://host/../path/info", null, EnumSet.noneOf(Ambiguous.class)},
{"http://host/path/../info", "/info", false, false}, {"http://host/path/../info", "/info", EnumSet.noneOf(Ambiguous.class)},
{"http://host/path/./info", "/path/info", false, false}, {"http://host/path/./info", "/path/info", EnumSet.noneOf(Ambiguous.class)},
{"//host/path/../info", "/info", false, false}, {"//host/path/../info", "/info", EnumSet.noneOf(Ambiguous.class)},
{"//host/path/./info", "/path/info", false, false}, {"//host/path/./info", "/path/info", EnumSet.noneOf(Ambiguous.class)},
{"/path/../info", "/info", false, false}, {"/path/../info", "/info", EnumSet.noneOf(Ambiguous.class)},
{"/path/./info", "/path/info", false, false}, {"/path/./info", "/path/info", EnumSet.noneOf(Ambiguous.class)},
{"path/../info", "info", false, false}, {"path/../info", "info", EnumSet.noneOf(Ambiguous.class)},
{"path/./info", "path/info", false, false}, {"path/./info", "path/info", EnumSet.noneOf(Ambiguous.class)},
// illegal paths // illegal paths
{"//host/../path/info", null, false, false}, {"//host/../path/info", null, EnumSet.noneOf(Ambiguous.class)},
{"/../path/info", null, false, false}, {"/../path/info", null, EnumSet.noneOf(Ambiguous.class)},
{"../path/info", null, false, false}, {"../path/info", null, EnumSet.noneOf(Ambiguous.class)},
{"/path/%XX/info", null, false, false}, {"/path/%XX/info", null, EnumSet.noneOf(Ambiguous.class)},
{"/path/%2/F/info", null, false, false}, {"/path/%2/F/info", null, EnumSet.noneOf(Ambiguous.class)},
// ambiguous dot encodings or parameter inclusions // ambiguous dot encodings
{"scheme://host/path/%2e/info", "/path/./info", true, false}, {"scheme://host/path/%2e/info", "/path/./info", EnumSet.of(Ambiguous.SEGMENT)},
{"scheme:/path/%2e/info", "/path/./info", true, false}, {"scheme:/path/%2e/info", "/path/./info", EnumSet.of(Ambiguous.SEGMENT)},
{"/path/%2e/info", "/path/./info", true, false}, {"/path/%2e/info", "/path/./info", EnumSet.of(Ambiguous.SEGMENT)},
{"path/%2e/info/", "path/./info/", true, false}, {"path/%2e/info/", "path/./info/", EnumSet.of(Ambiguous.SEGMENT)},
{"/path/%2e%2e/info", "/path/../info", true, false}, {"/path/%2e%2e/info", "/path/../info", EnumSet.of(Ambiguous.SEGMENT)},
{"/path/%2e%2e;/info", "/path/../info", true, false}, {"/path/%2e%2e;/info", "/path/../info", EnumSet.of(Ambiguous.SEGMENT)},
{"/path/%2e%2e;param/info", "/path/../info", true, false}, {"/path/%2e%2e;param/info", "/path/../info", EnumSet.of(Ambiguous.SEGMENT)},
{"/path/%2e%2e;param;other/info;other", "/path/../info", true, false}, {"/path/%2e%2e;param;other/info;other", "/path/../info", EnumSet.of(Ambiguous.SEGMENT)},
{"/path/.;/info", "/path/./info", true, false}, {"%2e/info", "./info", EnumSet.of(Ambiguous.SEGMENT)},
{"/path/.;param/info", "/path/./info", true, false}, {"%2e%2e/info", "../info", EnumSet.of(Ambiguous.SEGMENT)},
{"/path/..;/info", "/path/../info", true, false}, {"%2e%2e;/info", "../info", EnumSet.of(Ambiguous.SEGMENT)},
{"/path/..;param/info", "/path/../info", true, false}, {"%2e", ".", EnumSet.of(Ambiguous.SEGMENT)},
{"%2e/info", "./info", true, false}, {"%2e.", "..", EnumSet.of(Ambiguous.SEGMENT)},
{"%2e%2e/info", "../info", true, false}, {".%2e", "..", EnumSet.of(Ambiguous.SEGMENT)},
{"%2e%2e;/info", "../info", true, false}, {"%2e%2e", "..", EnumSet.of(Ambiguous.SEGMENT)},
{".;/info", "./info", true, false},
{".;param/info", "./info", true, false}, // ambiguous parameter inclusions
{"..;/info", "../info", true, false}, {"/path/.;/info", "/path/./info", EnumSet.of(Ambiguous.PARAM)},
{"..;param/info", "../info", true, false}, {"/path/.;param/info", "/path/./info", EnumSet.of(Ambiguous.PARAM)},
{"%2e", ".", true, false}, {"/path/..;/info", "/path/../info", EnumSet.of(Ambiguous.PARAM)},
{"%2e.", "..", true, false}, {"/path/..;param/info", "/path/../info", EnumSet.of(Ambiguous.PARAM)},
{".%2e", "..", true, false}, {".;/info", "./info", EnumSet.of(Ambiguous.PARAM)},
{"%2e%2e", "..", true, false}, {".;param/info", "./info", EnumSet.of(Ambiguous.PARAM)},
{"..;/info", "../info", EnumSet.of(Ambiguous.PARAM)},
{"..;param/info", "../info", EnumSet.of(Ambiguous.PARAM)},
// ambiguous segment separators // ambiguous segment separators
{"/path/%2f/info", "/path///info", false, true}, {"/path/%2f/info", "/path///info", EnumSet.of(Ambiguous.SEPARATOR)},
{"%2f/info", "//info", false, true}, {"%2f/info", "//info", EnumSet.of(Ambiguous.SEPARATOR)},
{"%2F/info", "//info", false, true}, {"%2F/info", "//info", EnumSet.of(Ambiguous.SEPARATOR)},
{"/path/%2f../info", "/path//../info", false, true}, {"/path/%2f../info", "/path//../info", EnumSet.of(Ambiguous.SEPARATOR)},
{"/path/%2f/..;/info", "/path///../info", true, true},
// 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 // Non ascii characters
// @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck // @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/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", 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", EnumSet.noneOf(Ambiguous.class)},
// @checkstyle-enable-check : AvoidEscapedUnicodeCharactersCheck // @checkstyle-enable-check : AvoidEscapedUnicodeCharactersCheck
}).map(Arguments::of); }).map(Arguments::of);
} }
@ParameterizedTest @ParameterizedTest
@MethodSource("decodePathTests") @MethodSource("decodePathTests")
public void testDecodedPath(String input, String decodedPath, boolean ambiguousSegment, boolean ambiguousSeparator) public void testDecodedPath(String input, String decodedPath, EnumSet<Ambiguous> expected)
{ {
try try
{ {
HttpURI uri = HttpURI.from(input); HttpURI uri = HttpURI.from(input);
assertThat(uri.getDecodedPath(), is(decodedPath)); assertThat(uri.getDecodedPath(), is(decodedPath));
assertThat(uri.hasAmbiguousSegment(), is(ambiguousSegment)); assertThat(uri.isAmbiguous(), is(!expected.isEmpty()));
assertThat(uri.hasAmbiguousSeparator(), is(ambiguousSeparator)); assertThat(uri.hasAmbiguousSegment(), is(expected.contains(Ambiguous.SEGMENT)));
assertThat(uri.isAmbiguous(), is(ambiguousSegment || ambiguousSeparator)); assertThat(uri.hasAmbiguousSeparator(), is(expected.contains(Ambiguous.SEPARATOR)));
assertThat(uri.hasAmbiguousParameter(), is(expected.contains(Ambiguous.PARAM)));
} }
catch (Exception e) catch (Exception e)
{ {

2
jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java
View File

@ -70,7 +70,7 @@ public class HttpConfiguration implements Dumpable
private long _minRequestDataRate; private long _minRequestDataRate;
private long _minResponseDataRate; private long _minResponseDataRate;
private HttpCompliance _httpCompliance = HttpCompliance.RFC7230; private HttpCompliance _httpCompliance = HttpCompliance.RFC7230;
private UriCompliance _uriCompliance = UriCompliance.SAFE; private UriCompliance _uriCompliance = UriCompliance.DEFAULT;
private CookieCompliance _requestCookieCompliance = CookieCompliance.RFC6265; private CookieCompliance _requestCookieCompliance = CookieCompliance.RFC6265;
private CookieCompliance _responseCookieCompliance = CookieCompliance.RFC6265; private CookieCompliance _responseCookieCompliance = CookieCompliance.RFC6265;
private boolean _notifyRemoteAsyncErrors = true; private boolean _notifyRemoteAsyncErrors = true;

2
jetty-server/src/main/java/org/eclipse/jetty/server/Request.java
View File

@ -1699,6 +1699,8 @@ public class Request implements HttpServletRequest
throw new BadMessageException("Ambiguous segment in URI"); throw new BadMessageException("Ambiguous segment in URI");
if (uri.hasAmbiguousSeparator() && (compliance == null || !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_SEPARATOR))) if (uri.hasAmbiguousSeparator() && (compliance == null || !compliance.allows(UriCompliance.Violation.AMBIGUOUS_PATH_SEPARATOR)))
throw new BadMessageException("Ambiguous segment in URI"); 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) if (uri.isAbsolute() && uri.hasAuthority() && uri.getPath() != null)

29
jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java
View File

@ -1696,15 +1696,32 @@ public class RequestTest
} }
@Test @Test
public void testAmbiguousSegments() throws Exception public void testAmbiguousParameters() throws Exception
{ {
_handler._checker = (request, response) -> true; _handler._checker = (request, response) -> true;
String request = "GET /ambiguous/..;/path HTTP/1.0\r\n" + String request = "GET /ambiguous/..;/path HTTP/1.0\r\n" +
"Host: whatever\r\n" + "Host: whatever\r\n" +
"\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")); 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 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.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")); assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200"));
} }
@ -1715,9 +1732,11 @@ public class RequestTest
String request = "GET /ambiguous/%2f/path HTTP/1.0\r\n" + String request = "GET /ambiguous/%2f/path HTTP/1.0\r\n" +
"Host: whatever\r\n" + "Host: whatever\r\n" +
"\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")); 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")); assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200"));
} }

40
jetty-servlet/src/test/java/org/eclipse/jetty/servlet/DefaultServletTest.java
View File

@ -46,10 +46,8 @@ import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpHeader; import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpTester; import org.eclipse.jetty.http.HttpTester;
import org.eclipse.jetty.http.UriCompliance;
import org.eclipse.jetty.logging.StacklessLogging; import org.eclipse.jetty.logging.StacklessLogging;
import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpConfiguration;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.LocalConnector; import org.eclipse.jetty.server.LocalConnector;
import org.eclipse.jetty.server.ResourceContentFactory; import org.eclipse.jetty.server.ResourceContentFactory;
import org.eclipse.jetty.server.ResourceService; import org.eclipse.jetty.server.ResourceService;
@ -112,7 +110,6 @@ public class DefaultServletTest
connector = new LocalConnector(server); connector = new LocalConnector(server);
connector.getConnectionFactory(HttpConfiguration.ConnectionFactory.class).getHttpConfiguration().setSendServerVersion(false); 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); File extraJarResources = MavenTestingUtils.getTestResourceFile(ODD_JAR);
URL[] urls = new URL[]{extraJarResources.toURI().toURL()}; URL[] urls = new URL[]{extraJarResources.toURI().toURL()};
@ -253,11 +250,8 @@ public class DefaultServletTest
String resBasePath = docRoot.toAbsolutePath().toString(); String resBasePath = docRoot.toAbsolutePath().toString();
defholder.setInitParameter("resourceBase", resBasePath); defholder.setInitParameter("resourceBase", resBasePath);
StringBuffer req1 = new StringBuffer(); String req1 = "GET /context/one/deep/ HTTP/1.0\n\n";
req1.append("GET /context/one/deep/ HTTP/1.0\n"); String rawResponse = connector.getResponse(req1);
req1.append("\n");
String rawResponse = connector.getResponse(req1.toString());
HttpTester.Response response = HttpTester.parseResponse(rawResponse); HttpTester.Response response = HttpTester.parseResponse(rawResponse);
assertThat(response.getStatus(), is(HttpStatus.OK_200)); assertThat(response.getStatus(), is(HttpStatus.OK_200));
@ -454,10 +448,17 @@ public class DefaultServletTest
(response) -> assertThat(response.getContent(), not(containsString("Sssh"))) (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( scenarios.addScenario(
"GET " + prefix + "/%2E%2E/%2E%2E/sekret/pass", "GET " + prefix + "/%2E%2E/%2E%2E/sekret/pass",
"GET " + prefix + "/ HTTP/1.0\r\n\r\n", "GET " + prefix + "/%2E%2E/%2E%2E/sekret/pass HTTP/1.0\r\n\r\n",
HttpStatus.NOT_FOUND_404, prefix.endsWith("?") ? HttpStatus.NOT_FOUND_404 : HttpStatus.BAD_REQUEST_400,
(response) -> assertThat(response.getContent(), not(containsString("Sssh"))) (response) -> assertThat(response.getContent(), not(containsString("Sssh")))
); );
@ -469,12 +470,6 @@ public class DefaultServletTest
"GET " + prefix + "/../index.html HTTP/1.0\r\n\r\n", "GET " + prefix + "/../index.html HTTP/1.0\r\n\r\n",
HttpStatus.NOT_FOUND_404 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 else
{ {
@ -484,15 +479,14 @@ public class DefaultServletTest
HttpStatus.OK_200, HttpStatus.OK_200,
(response) -> assertThat(response.getContent(), containsString("Hello Index")) (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( scenarios.addScenario(
"GET " + prefix + "/../../", "GET " + prefix + "/../../",
"GET " + prefix + "/../../ HTTP/1.0\r\n\r\n", "GET " + prefix + "/../../ HTTP/1.0\r\n\r\n",

2
jetty-servlet/src/test/java/org/eclipse/jetty/servlet/RequestURITest.java
View File

@ -109,7 +109,7 @@ public class RequestURITest
ServerConnector connector = new ServerConnector(server); ServerConnector connector = new ServerConnector(server);
connector.setPort(0); connector.setPort(0);
server.addConnector(connector); 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(); ServletContextHandler context = new ServletContextHandler();
context.setContextPath("/"); context.setContextPath("/");