Fix #4275 fail URIs with ambiguous segments (#5954)

Handle URIs by first resolving relative paths and then decoding.
Added compliance mode to return 400 if there are ambiguous path segments.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2021-02-16 14:47:41 +01:00 committed by GitHub
parent 5dd987779c
commit 20ef71fe5d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 456 additions and 160 deletions

View File

@ -56,13 +56,14 @@ public enum HttpCompliance // TODO in Jetty-10 convert this enum to a class so t
LEGACY(sectionsBySpec("0,METHOD_CASE_SENSITIVE")),
/**
* The legacy RFC2616 support, which incorrectly excludes
* The legacy RFC2616 support, which excludes
* {@link HttpComplianceSection#METHOD_CASE_SENSITIVE},
* {@link HttpComplianceSection#FIELD_COLON},
* {@link HttpComplianceSection#TRANSFER_ENCODING_WITH_CONTENT_LENGTH},
* {@link HttpComplianceSection#MULTIPLE_CONTENT_LENGTHS},
* {@link HttpComplianceSection#MULTIPLE_CONTENT_LENGTHS} and
* {@link HttpComplianceSection#NO_AMBIGUOUS_PATH_SEGMENTS}.
*/
RFC2616_LEGACY(sectionsBySpec("RFC2616,-FIELD_COLON,-METHOD_CASE_SENSITIVE,-TRANSFER_ENCODING_WITH_CONTENT_LENGTH,-MULTIPLE_CONTENT_LENGTHS")),
RFC2616_LEGACY(sectionsBySpec("RFC2616,-FIELD_COLON,-METHOD_CASE_SENSITIVE,-TRANSFER_ENCODING_WITH_CONTENT_LENGTH,-MULTIPLE_CONTENT_LENGTHS,-NO_AMBIGUOUS_PATH_SEGMENTS")),
/**
* The strict RFC2616 support mode
@ -70,9 +71,11 @@ public enum HttpCompliance // TODO in Jetty-10 convert this enum to a class so t
RFC2616(sectionsBySpec("RFC2616")),
/**
* Jetty's current RFC7230 support, which incorrectly excludes {@link HttpComplianceSection#METHOD_CASE_SENSITIVE}
* Jetty's current RFC7230 support, which excludes
* {@link HttpComplianceSection#METHOD_CASE_SENSITIVE} and
* {@link HttpComplianceSection#NO_AMBIGUOUS_PATH_SEGMENTS}.
*/
RFC7230_LEGACY(sectionsBySpec("RFC7230,-METHOD_CASE_SENSITIVE")),
RFC7230_LEGACY(sectionsBySpec("RFC7230,-METHOD_CASE_SENSITIVE,-NO_AMBIGUOUS_PATH_SEGMENTS")),
/**
* The RFC7230 support mode
@ -123,11 +126,6 @@ public enum HttpCompliance // TODO in Jetty-10 convert this enum to a class so t
i++;
break;
case "*":
i++;
sections = EnumSet.allOf(HttpComplianceSection.class);
break;
case "RFC2616":
sections = EnumSet.complementOf(EnumSet.of(
HttpComplianceSection.NO_FIELD_FOLDING,
@ -135,6 +133,7 @@ public enum HttpCompliance // TODO in Jetty-10 convert this enum to a class so t
i++;
break;
case "*":
case "RFC7230":
i++;
sections = EnumSet.allOf(HttpComplianceSection.class);
@ -152,11 +151,6 @@ public enum HttpCompliance // TODO in Jetty-10 convert this enum to a class so t
if (exclude)
element = element.substring(1);
HttpComplianceSection section = HttpComplianceSection.valueOf(element);
if (section == null)
{
LOG.warn("Unknown section '" + element + "' in HttpCompliance spec: " + spec);
continue;
}
if (exclude)
sections.remove(section);
else

View File

@ -31,7 +31,8 @@ public enum HttpComplianceSection
NO_FIELD_FOLDING("https://tools.ietf.org/html/rfc7230#section-3.2.4", "No line Folding"),
NO_HTTP_0_9("https://tools.ietf.org/html/rfc7230#appendix-A.2", "No HTTP/0.9"),
TRANSFER_ENCODING_WITH_CONTENT_LENGTH("https://tools.ietf.org/html/rfc7230#section-3.3.1", "Transfer-Encoding and Content-Length"),
MULTIPLE_CONTENT_LENGTHS("https://tools.ietf.org/html/rfc7230#section-3.3.1", "Multiple Content-Lengths");
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");
final String url;
final String description;

View File

@ -311,6 +311,11 @@ public class HttpParser
return _handler;
}
public HttpCompliance getHttpCompliance()
{
return _compliance;
}
/**
* Check RFC compliance violation
*

View File

@ -23,8 +23,11 @@ import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.Objects;
import org.eclipse.jetty.util.ArrayTrie;
import org.eclipse.jetty.util.MultiMap;
import org.eclipse.jetty.util.Trie;
import org.eclipse.jetty.util.TypeUtil;
import org.eclipse.jetty.util.URIUtil;
import org.eclipse.jetty.util.UrlEncoded;
@ -65,6 +68,30 @@ public class HttpURI
ASTERISK
}
/**
* The concept of URI path parameters was originally specified in
* <a href="https://tools.ietf.org/html/rfc2396#section-3.3">RFC2396</a>, but that was
* obsoleted by
* <a href="https://tools.ietf.org/html/rfc3986#section-3.3">RFC3986</a> which removed
* a normative definition of path parameters. Specifically it excluded them from the
* <a href="https://tools.ietf.org/html/rfc3986#section-5.2.4">Remove Dot Segments</a>
* algorithm. This results in some ambiguity as dot segments can result from later
* parameter removal or % encoding expansion, that are not removed from the URI
* by {@link URIUtil#canonicalPath(String)}. Thus this class flags such ambiguous
* path segments, so that they may be rejected by the server if so configured.
*/
private static final Trie<Boolean> __ambiguousSegments = new ArrayTrie<>();
static
{
__ambiguousSegments.put("%2e", Boolean.TRUE);
__ambiguousSegments.put("%2e%2e", Boolean.TRUE);
__ambiguousSegments.put(".%2e", Boolean.TRUE);
__ambiguousSegments.put("%2e.", Boolean.TRUE);
__ambiguousSegments.put("..", Boolean.FALSE);
__ambiguousSegments.put(".", Boolean.FALSE);
}
private String _scheme;
private String _user;
private String _host;
@ -73,9 +100,9 @@ public class HttpURI
private String _param;
private String _query;
private String _fragment;
String _uri;
String _decodedPath;
private String _uri;
private String _decodedPath;
private boolean _ambiguousSegment;
/**
* Construct a normalized URI.
@ -108,16 +135,29 @@ public class HttpURI
_scheme = scheme;
_host = host;
_port = port;
_path = path;
_param = param;
_query = query;
_fragment = fragment;
if (path != null)
parse(State.PATH, path, 0, path.length());
if (param != null)
_param = param;
if (query != null)
_query = query;
if (fragment != null)
_fragment = fragment;
}
public HttpURI(HttpURI uri)
{
this(uri._scheme, uri._host, uri._port, uri._path, uri._param, uri._query, uri._fragment);
_scheme = uri._scheme;
_user = uri._user;
_host = uri._host;
_port = uri._port;
_path = uri._path;
_param = uri._param;
_query = uri._query;
_fragment = uri._fragment;
_uri = uri._uri;
_decodedPath = uri._decodedPath;
_ambiguousSegment = uri._ambiguousSegment;
}
public HttpURI(String uri)
@ -129,40 +169,44 @@ public class HttpURI
public HttpURI(URI uri)
{
_uri = null;
_scheme = uri.getScheme();
_host = uri.getHost();
if (_host == null && uri.getRawSchemeSpecificPart().startsWith("//"))
_host = "";
_port = uri.getPort();
_user = uri.getUserInfo();
_path = uri.getRawPath();
_decodedPath = uri.getPath();
if (_decodedPath != null)
{
int p = _decodedPath.lastIndexOf(';');
if (p >= 0)
_param = _decodedPath.substring(p + 1);
}
String path = uri.getRawPath();
if (path != null)
parse(State.PATH, path, 0, path.length());
_query = uri.getRawQuery();
_fragment = uri.getFragment();
_decodedPath = null;
}
public HttpURI(String scheme, String host, int port, String pathQuery)
{
_uri = null;
_scheme = scheme;
_host = host;
_port = port;
if (pathQuery != null)
parse(State.PATH, pathQuery, 0, pathQuery.length());
}
public void clear()
{
_uri = null;
_scheme = null;
_user = null;
_host = null;
_port = -1;
_path = null;
_param = null;
_query = null;
_fragment = null;
_decodedPath = null;
_ambiguousSegment = false;
}
public void parse(String uri)
{
clear();
@ -205,9 +249,12 @@ public class HttpURI
private void parse(State state, final String uri, final int offset, final int end)
{
boolean encoded = false;
int mark = offset;
int pathMark = 0;
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
for (int i = offset; i < end; i++)
{
@ -241,21 +288,30 @@ public class HttpURI
_path = "*";
state = State.ASTERISK;
break;
case '%':
encoded = true;
escapedSlash = 1;
mark = pathMark = segment = i;
state = State.PATH;
break;
case '.' :
dot = true;
pathMark = segment = i;
state = State.PATH;
break;
default:
mark = i;
if (_scheme == null)
state = State.SCHEME_OR_PATH;
else
{
pathMark = i;
pathMark = segment = i;
state = State.PATH;
}
}
continue;
}
case SCHEME_OR_PATH:
{
switch (c)
@ -266,40 +322,38 @@ public class HttpURI
// Start again with scheme set
state = State.START;
break;
case '/':
// must have been in a path and still are
segment = i + 1;
state = State.PATH;
break;
case ';':
// must have been in a path
mark = i + 1;
state = State.PARAM;
break;
case '?':
// 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;
state = State.PATH;
break;
case '#':
// must have been in a path
_path = uri.substring(mark, i);
state = State.FRAGMENT;
break;
default:
break;
}
continue;
}
case HOST_OR_PATH:
{
switch (c)
@ -310,23 +364,26 @@ public class HttpURI
state = State.HOST;
break;
case '%':
case '@':
case ';':
case '?':
case '#':
case '.':
// was a path, look again
i--;
pathMark = mark;
segment = mark + 1;
state = State.PATH;
break;
default:
// it is a path
pathMark = mark;
segment = mark + 1;
state = State.PATH;
}
continue;
}
case HOST:
{
switch (c)
@ -334,6 +391,7 @@ public class HttpURI
case '/':
_host = uri.substring(mark, i);
pathMark = mark = i;
segment = mark + 1;
state = State.PATH;
break;
case ':':
@ -348,14 +406,14 @@ public class HttpURI
_user = uri.substring(mark, i);
mark = i + 1;
break;
case '[':
state = State.IPV6;
break;
default:
break;
}
continue;
}
case IPV6:
{
switch (c)
@ -376,11 +434,11 @@ public class HttpURI
state = State.PATH;
}
break;
default:
break;
}
continue;
}
case PORT:
{
if (c == '@')
@ -396,36 +454,57 @@ public class HttpURI
{
_port = TypeUtil.parseInt(uri, mark, i - mark, 10);
pathMark = mark = i;
segment = i + 1;
state = State.PATH;
}
continue;
}
case PATH:
{
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 '/':
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':
_ambiguousSegment |= (escapedSlash == 2);
escapedSlash = 0;
break;
default:
escapedSlash = 0;
break;
}
continue;
}
case PARAM:
{
switch (c)
@ -444,17 +523,18 @@ public class HttpURI
break;
case '/':
encoded = true;
// ignore internal params
segment = i + 1;
state = State.PATH;
break;
case ';':
// multiple parameters
mark = i + 1;
break;
default:
break;
}
continue;
}
case QUERY:
{
if (c == '#')
@ -465,17 +545,18 @@ public class HttpURI
}
continue;
}
case ASTERISK:
{
throw new IllegalArgumentException("Bad character '*'");
}
case FRAGMENT:
{
_fragment = uri.substring(mark, end);
i = end;
break;
}
default:
break;
}
}
@ -486,51 +567,78 @@ public class HttpURI
case SCHEME_OR_PATH:
_path = uri.substring(mark, end);
break;
case HOST_OR_PATH:
_path = uri.substring(mark, end);
break;
case HOST:
if (end > mark)
_host = uri.substring(mark, end);
break;
case IPV6:
throw new IllegalArgumentException("No closing ']' for ipv6 in " + uri);
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);
break;
case PATH:
checkSegment(uri, segment, end, false);
_path = uri.substring(pathMark, end);
break;
case QUERY:
_query = uri.substring(mark, end);
break;
default:
break;
}
if (!encoded)
if (!encoded && !dot)
{
if (_param == null)
_decodedPath = _path;
else
_decodedPath = _path.substring(0, _path.length() - _param.length() - 1);
}
else if (_path != null)
{
String canonical = URIUtil.canonicalPath(_path);
if (canonical == null)
throw new BadMessageException("Bad URI");
_decodedPath = URIUtil.decodePath(canonical);
}
}
/**
* Check for ambiguous path segments.
*
* 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 (!_ambiguousSegment)
{
Boolean ambiguous = __ambiguousSegments.get(uri, segment, end - segment);
_ambiguousSegment |= ambiguous == Boolean.TRUE || (param && ambiguous == Boolean.FALSE);
}
}
/**
* @return True if the URI has a possibly ambiguous segment like '..;' or '%2e%2e'
*/
public boolean hasAmbiguousSegment()
{
return _ambiguousSegment;
}
public String getScheme()
@ -561,10 +669,12 @@ public class HttpURI
return _path;
}
/**
* @return The decoded canonical path.
* @see URIUtil#canonicalPath(String)
*/
public String getDecodedPath()
{
if (_decodedPath == null && _path != null)
_decodedPath = URIUtil.decodePath(_path);
return _decodedPath;
}
@ -575,10 +685,14 @@ public class HttpURI
public void setParam(String param)
{
_param = param;
if (_path != null && !_path.contains(_param))
if (!Objects.equals(_param, param))
{
_path += ";" + _param;
if (_param != null && _path.endsWith(";" + _param))
_path = _path.substring(0, _path.length() - 1 - _param.length());
_param = param;
if (_param != null)
_path = (_path == null ? "" : _path) + ";" + _param;
_uri = null;
}
}
@ -620,21 +734,6 @@ public class HttpURI
UrlEncoded.decodeTo(_query, parameters, encoding);
}
public void clear()
{
_uri = null;
_scheme = null;
_host = null;
_port = -1;
_path = null;
_param = null;
_query = null;
_fragment = null;
_decodedPath = null;
}
public boolean isAbsolute()
{
return _scheme != null && !_scheme.isEmpty();
@ -688,6 +787,12 @@ public class HttpURI
return toString().equals(o.toString());
}
@Override
public int hashCode()
{
return toString().hashCode();
}
public void setScheme(String scheme)
{
_scheme = scheme;
@ -711,8 +816,9 @@ public class HttpURI
public void setPath(String path)
{
_uri = null;
_path = path;
_decodedPath = null;
_path = null;
if (path != null)
parse(State.PATH, path, 0, path.length());
}
public void setPathQuery(String path)
@ -722,6 +828,7 @@ public class HttpURI
_decodedPath = null;
_param = null;
_fragment = null;
_query = null;
if (path != null)
parse(State.PATH, path, 0, path.length());
}

View File

@ -20,9 +20,14 @@ package org.eclipse.jetty.http;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.stream.Stream;
import org.eclipse.jetty.util.MultiMap;
import org.junit.jupiter.api.Test;
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;
@ -214,11 +219,137 @@ public class HttpURITest
}
@Test
public void testBasicAuthCredentials() throws Exception
public void testSetters() throws Exception
{
HttpURI uri = new HttpURI("http://user:password@example.com:8888/blah");
assertEquals("http://user:password@example.com:8888/blah", uri.toString());
assertEquals(uri.getAuthority(), "example.com:8888");
assertEquals(uri.getUser(), "user:password");
HttpURI uri = new HttpURI();
assertEquals("", uri.toString());
uri = new HttpURI(null, null, 0, null, null, null, null);
assertEquals("", uri.toString());
uri.setPath("/path/info");
assertEquals("/path/info", uri.toString());
uri.setAuthority("host", 8080);
assertEquals("//host:8080/path/info", uri.toString());
uri.setParam("param");
assertEquals("//host:8080/path/info;param", uri.toString());
uri.setQuery("a=b");
assertEquals("//host:8080/path/info;param?a=b", uri.toString());
uri.setScheme("http");
assertEquals("http://host:8080/path/info;param?a=b", uri.toString());
uri.setPathQuery("/other;xxx/path;ppp?query");
assertEquals("http://host:8080/other;xxx/path;ppp?query", uri.toString());
assertThat(uri.getScheme(), is("http"));
assertThat(uri.getAuthority(), is("host:8080"));
assertThat(uri.getHost(), is("host"));
assertThat(uri.getPort(), is(8080));
assertThat(uri.getPath(), is("/other;xxx/path;ppp"));
assertThat(uri.getDecodedPath(), is("/other/path"));
assertThat(uri.getParam(), is("ppp"));
assertThat(uri.getQuery(), is("query"));
assertThat(uri.getPathQuery(), is("/other;xxx/path;ppp?query"));
uri.setPathQuery(null);
assertEquals("http://host:8080", uri.toString());
uri.setPathQuery("/other;xxx/path;ppp?query");
assertEquals("http://host:8080/other;xxx/path;ppp?query", uri.toString());
uri.setScheme(null);
assertEquals("//host:8080/other;xxx/path;ppp?query", uri.toString());
uri.setAuthority(null,-1);
assertEquals("/other;xxx/path;ppp?query", uri.toString());
uri.setParam(null);
assertEquals("/other;xxx/path?query", uri.toString());
uri.setQuery(null);
assertEquals("/other;xxx/path", uri.toString());
uri.setPath(null);
assertEquals("", uri.toString());
}
public static Stream<Arguments> decodePathTests()
{
return Arrays.stream(new Object[][]
{
// Simple path example
{"http://host/path/info", "/path/info", false},
{"//host/path/info", "/path/info", false},
{"/path/info", "/path/info", false},
// legal non ambiguous relative paths
{"http://host/../path/info", null, false},
{"http://host/path/../info", "/info", false},
{"http://host/path/./info", "/path/info", false},
{"//host/path/../info", "/info", false},
{"//host/path/./info", "/path/info", false},
{"/path/../info", "/info", false},
{"/path/./info", "/path/info", false},
{"path/../info", "info", false},
{"path/./info", "path/info", false},
// illegal paths
{"//host/../path/info", null, false},
{"/../path/info", null, false},
{"../path/info", null, false},
{"/path/%XX/info", null, false},
{"/path/%2/F/info", null, false},
// ambiguous dot encodings or parameter inclusions
{"scheme://host/path/%2e/info", "/path/./info", true},
{"scheme:/path/%2e/info", "/path/./info", true},
{"/path/%2e/info", "/path/./info", true},
{"path/%2e/info/", "path/./info/", true},
{"/path/%2e%2e/info", "/path/../info", true},
{"/path/%2e%2e;/info", "/path/../info", true},
{"/path/%2e%2e;param/info", "/path/../info", true},
{"/path/%2e%2e;param;other/info;other", "/path/../info", true},
{"/path/.;/info", "/path/./info", true},
{"/path/.;param/info", "/path/./info", true},
{"/path/..;/info", "/path/../info", true},
{"/path/..;param/info", "/path/../info", true},
{"%2e/info", "./info", true},
{"%2e%2e/info", "../info", true},
{"%2e%2e;/info", "../info", true},
{".;/info", "./info", true},
{".;param/info", "./info", true},
{"..;/info", "../info", true},
{"..;param/info", "../info", true},
{"%2e", ".", true},
{"%2e.", "..", true},
{".%2e", "..", true},
{"%2e%2e", "..", true},
// ambiguous segment separators
{"/path/%2f/info", "/path///info", true},
{"%2f/info", "//info", true},
{"%2F/info", "//info", true},
}).map(Arguments::of);
}
@ParameterizedTest
@MethodSource("decodePathTests")
public void testDecodedPath(String input, String decodedPath, boolean ambiguous)
{
try
{
HttpURI uri = new HttpURI(input);
assertThat(uri.getDecodedPath(), is(decodedPath));
assertThat(uri.hasAmbiguousSegment(), is(ambiguous));
}
catch (Exception e)
{
assertThat(decodedPath, nullValue());
}
}
}

View File

@ -114,6 +114,12 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
LOG.debug("New HTTP Connection {}", this);
}
@Deprecated
public HttpCompliance getHttpCompliance()
{
return _parser.getHttpCompliance();
}
public HttpConfiguration getHttpConfiguration()
{
return _config;

View File

@ -65,6 +65,8 @@ import javax.servlet.http.Part;
import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.HostPortHttpField;
import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.http.HttpComplianceSection;
import org.eclipse.jetty.http.HttpCookie;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpFields;
@ -77,6 +79,7 @@ import org.eclipse.jetty.http.HttpURI;
import org.eclipse.jetty.http.HttpVersion;
import org.eclipse.jetty.http.MetaData;
import org.eclipse.jetty.http.MimeTypes;
import org.eclipse.jetty.io.Connection;
import org.eclipse.jetty.io.RuntimeIOException;
import org.eclipse.jetty.server.handler.ContextHandler;
import org.eclipse.jetty.server.handler.ContextHandler.Context;
@ -1815,6 +1818,19 @@ public class Request implements HttpServletRequest
setMethod(request.getMethod());
HttpURI uri = request.getURI();
if (uri.hasAmbiguousSegment())
{
// TODO replace in jetty-10 with HttpCompliance from the HttpConfiguration
Connection connection = _channel.getConnection();
HttpCompliance compliance = connection instanceof HttpConnection
? ((HttpConnection)connection).getHttpCompliance()
: _channel.getConnector().getBean(HttpCompliance.class);
boolean allow = compliance != null && !compliance.sections().contains(HttpComplianceSection.NO_AMBIGUOUS_PATH_SEGMENTS);
if (!allow)
throw new BadMessageException("Ambiguous segment in URI");
}
_originalURI = uri.isAbsolute() && request.getHttpVersion() != HttpVersion.HTTP_2 ? uri.toString() : uri.getPathQuery();
String encoded = uri.getPath();
@ -1826,7 +1842,7 @@ public class Request implements HttpServletRequest
}
else if (encoded.startsWith("/"))
{
path = (encoded.length() == 1) ? "/" : URIUtil.canonicalPath(uri.getDecodedPath());
path = (encoded.length() == 1) ? "/" : uri.getDecodedPath();
}
else if ("*".equals(encoded) || HttpMethod.CONNECT.is(getMethod()))
{

View File

@ -500,43 +500,28 @@ public class HttpConnectionTest
public void testBadPathDotDotPath() throws Exception
{
String response = connector.getResponse("GET /ooops/../../path HTTP/1.0\r\nHost: localhost:80\r\n\n");
checkContains(response, 0, "HTTP/1.1 400 Bad URI");
}
@Test
public void testOKPathEncodedDotDotPath() throws Exception
{
String response = connector.getResponse("GET /ooops/%2e%2e/path HTTP/1.0\r\nHost: localhost:80\r\n\n");
checkContains(response, 0, "HTTP/1.1 200 OK");
checkContains(response, 0, "pathInfo=/path");
}
@Test
public void testBadPathEncodedDotDotPath() throws Exception
{
String response = connector.getResponse("GET /ooops/%2e%2e/%2e%2e/path HTTP/1.0\r\nHost: localhost:80\r\n\n");
checkContains(response, 0, "HTTP/1.1 400 Bad URI");
checkContains(response, 0, "HTTP/1.1 400 ");
}
@Test
public void testBadDotDotPath() throws Exception
{
String response = connector.getResponse("GET ../path HTTP/1.0\r\nHost: localhost:80\r\n\n");
checkContains(response, 0, "HTTP/1.1 400 Bad URI");
checkContains(response, 0, "HTTP/1.1 400 ");
}
@Test
public void testBadSlashDotDotPath() throws Exception
{
String response = connector.getResponse("GET /../path HTTP/1.0\r\nHost: localhost:80\r\n\n");
checkContains(response, 0, "HTTP/1.1 400 Bad URI");
checkContains(response, 0, "HTTP/1.1 400 ");
}
@Test
public void testEncodedBadDotDotPath() throws Exception
{
String response = connector.getResponse("GET %2e%2e/path HTTP/1.0\r\nHost: localhost:80\r\n\n");
checkContains(response, 0, "HTTP/1.1 400 Bad URI");
checkContains(response, 0, "HTTP/1.1 400 ");
}
@Test

View File

@ -1836,6 +1836,28 @@ public class RequestTest
assertEquals(0, request.getParameterMap().size());
}
@Test
public void testAmbiguousPaths() 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).setHttpCompliance(HttpCompliance.RFC7230);
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400"));
_connector.getBean(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC7230_LEGACY);
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200"));
_connector.getBean(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC2616);
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400"));
_connector.getBean(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC2616_LEGACY);
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200"));
}
private static long getFileCount(Path path)
{
try (Stream<Path> s = Files.list(path))

View File

@ -229,7 +229,7 @@ public class AsyncContextTest
@Test
public void testDispatchAsyncContextEncodedUrl() throws Exception
{
String request = "GET /ctx/test/hello%2fthere?dispatch=true HTTP/1.1\r\n" +
String request = "GET /ctx/test/hello%20there?dispatch=true HTTP/1.1\r\n" +
"Host: localhost\r\n" +
"Content-Type: application/x-www-form-urlencoded\r\n" +
"Connection: close\r\n" +
@ -253,16 +253,16 @@ public class AsyncContextTest
// async run attributes
assertThat("async run attr servlet path is original", responseBody, containsString("async:run:attr:servletPath:/test"));
assertThat("async run attr path info has correct encoding", responseBody, containsString("async:run:attr:pathInfo:/hello/there"));
assertThat("async run attr path info has correct encoding", responseBody, containsString("async:run:attr:pathInfo:/hello there"));
assertThat("async run attr query string", responseBody, containsString("async:run:attr:queryString:dispatch=true"));
assertThat("async run context path", responseBody, containsString("async:run:attr:contextPath:/ctx"));
assertThat("async run request uri has correct encoding", responseBody, containsString("async:run:attr:requestURI:/ctx/test/hello%2fthere"));
assertThat("async run request uri has correct encoding", responseBody, containsString("async:run:attr:requestURI:/ctx/test/hello%20there"));
}
@Test
public void testDispatchAsyncContextSelfEncodedUrl() throws Exception
{
String request = "GET /ctx/self/hello%2fthere?dispatch=true HTTP/1.1\r\n" +
String request = "GET /ctx/self/hello%20there?dispatch=true HTTP/1.1\r\n" +
"Host: localhost\r\n" +
"Content-Type: application/x-www-form-urlencoded\r\n" +
"Connection: close\r\n" +
@ -272,8 +272,8 @@ public class AsyncContextTest
String responseBody = response.getContent();
assertThat("servlet request uri initial", responseBody, containsString("doGet.REQUEST.requestURI:/ctx/self/hello%2fthere"));
assertThat("servlet request uri async", responseBody, containsString("doGet.ASYNC.requestURI:/ctx/self/hello%2fthere"));
assertThat("servlet request uri initial", responseBody, containsString("doGet.REQUEST.requestURI:/ctx/self/hello%20there"));
assertThat("servlet request uri async", responseBody, containsString("doGet.ASYNC.requestURI:/ctx/self/hello%20there"));
}
@Test

View File

@ -47,12 +47,14 @@ import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.http.DateGenerator;
import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.http.HttpContent;
import org.eclipse.jetty.http.HttpField;
import org.eclipse.jetty.http.HttpHeader;
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.http.HttpTester;
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;
@ -116,6 +118,7 @@ public class DefaultServletTest
connector = new LocalConnector(server);
connector.getConnectionFactory(HttpConfiguration.ConnectionFactory.class).getHttpConfiguration().setSendServerVersion(false);
connector.getBean(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC7230_LEGACY); // allow ambiguous path segments
File extraJarResources = MavenTestingUtils.getTestResourceFile(ODD_JAR);
URL[] urls = new URL[]{extraJarResources.toURI().toURL()};

View File

@ -34,6 +34,8 @@ import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.server.HttpConnectionFactory;
import org.eclipse.jetty.server.Server;
import org.eclipse.jetty.server.ServerConnector;
import org.hamcrest.Matchers;
@ -112,6 +114,7 @@ public class RequestURITest
ServerConnector connector = new ServerConnector(server);
connector.setPort(0);
server.addConnector(connector);
connector.getBean(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC7230_LEGACY); // Allow ambiguous segments
ServletContextHandler context = new ServletContextHandler();
context.setContextPath("/");

View File

@ -782,11 +782,9 @@ public class URIUtil
}
/**
* Convert a decoded path to a canonical form.
* Convert an encoded path to a canonical form.
* <p>
* All instances of "." and ".." are factored out.
* </p>
* <p>
* Null is returned if the path tries to .. above its root.
* </p>
*
@ -795,31 +793,35 @@ public class URIUtil
*/
public static String canonicalPath(String path)
{
// See https://tools.ietf.org/html/rfc3986#section-5.2.4
if (path == null || path.isEmpty())
return path;
boolean slash = true;
int end = path.length();
int i = 0;
int dots = 0;
loop:
while (i < end)
loop: while (i < end)
{
char c = path.charAt(i);
switch (c)
{
case '/':
slash = true;
dots = 0;
break;
case '.':
if (slash)
if (dots == 0)
{
dots = 1;
break loop;
slash = false;
}
dots = -1;
break;
default:
slash = false;
dots = -1;
}
i++;
@ -831,7 +833,6 @@ public class URIUtil
StringBuilder canonical = new StringBuilder(path.length());
canonical.append(path, 0, i);
int dots = 1;
i++;
while (i <= end)
{
@ -839,14 +840,18 @@ public class URIUtil
switch (c)
{
case '\0':
if (dots == 2)
{
if (canonical.length() < 2)
return null;
canonical.setLength(canonical.length() - 1);
canonical.setLength(canonical.lastIndexOf("/") + 1);
}
break;
case '/':
switch (dots)
{
case 0:
if (c != '\0')
canonical.append(c);
break;
case 1:
break;
@ -858,36 +863,42 @@ public class URIUtil
break;
default:
while (dots-- > 0)
{
canonical.append('.');
}
if (c != '\0')
canonical.append(c);
canonical.append(c);
}
slash = true;
dots = 0;
break;
case '.':
if (dots > 0)
dots++;
else if (slash)
dots = 1;
else
canonical.append('.');
slash = false;
switch (dots)
{
case 0:
dots = 1;
break;
case 1:
dots = 2;
break;
case 2:
canonical.append("...");
dots = -1;
break;
default:
canonical.append('.');
}
break;
default:
while (dots-- > 0)
switch (dots)
{
canonical.append('.');
case 1:
canonical.append('.');
break;
case 2:
canonical.append("..");
break;
default:
}
canonical.append(c);
dots = 0;
slash = false;
dots = -1;
}
i++;

View File

@ -34,6 +34,10 @@ public class URIUtilCanonicalPathTest
{
String[][] canonical =
{
// Examples from RFC
{"/a/b/c/./../../g", "/a/g"},
{"mid/content=5/../6", "mid/6"},
// Basic examples (no changes expected)
{"/hello.html", "/hello.html"},
{"/css/main.css", "/css/main.css"},
@ -56,8 +60,12 @@ public class URIUtilCanonicalPathTest
{"/aaa/./bbb/", "/aaa/bbb/"},
{"/aaa/./bbb", "/aaa/bbb"},
{"./bbb/", "bbb/"},
{"./aaa", "aaa"},
{"./aaa/", "aaa/"},
{"/./aaa/", "/aaa/"},
{"./aaa/../bbb/", "bbb/"},
{"/foo/.", "/foo/"},
{"/foo/./", "/foo/"},
{"./", ""},
{".", ""},
{".//", "/"},
@ -121,6 +129,10 @@ public class URIUtilCanonicalPathTest
{"/foo/.;/bar", "/foo/.;/bar"},
{"/foo/..;/bar", "/foo/..;/bar"},
{"/foo/..;/..;/bar", "/foo/..;/..;/bar"},
// Trailing / is preserved
{"/foo/bar/..", "/foo/"},
{"/foo/bar/../", "/foo/"},
};
ArrayList<Arguments> ret = new ArrayList<>();
@ -135,6 +147,6 @@ public class URIUtilCanonicalPathTest
@MethodSource("data")
public void testCanonicalPath(String input, String expectedResult)
{
assertThat("Canonical", URIUtil.canonicalPath(input), is(expectedResult));
assertThat(URIUtil.canonicalPath(input), is(expectedResult));
}
}