Skip to content

Commit 49e73df

Browse files
gregwgregw
authored
Fix #4275 #6001 separate compliance modes for ambiguous URI segments and se… (#6003)
Fix #4275 separate compliance modes for ambiguous URI segments and separators
1 parent c9cd1e4 commit 49e73df

6 files changed

Lines changed: 124 additions & 74 deletions

File tree

‎jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,6 @@
2222
import java.util.HashMap;
2323
import java.util.Map;
2424

25-
import org.eclipse.jetty.util.log.Log;
26-
import org.eclipse.jetty.util.log.Logger;
27-
2825
/**
2926
* HTTP compliance modes for Jetty HTTP parsing and handling.
3027
* A Compliance mode consists of a set of {@link HttpComplianceSection}s which are applied
@@ -60,10 +57,11 @@ public enum HttpCompliance // TODO in Jetty-10 convert this enum to a class so t
6057
* {@link HttpComplianceSection#METHOD_CASE_SENSITIVE},
6158
* {@link HttpComplianceSection#FIELD_COLON},
6259
* {@link HttpComplianceSection#TRANSFER_ENCODING_WITH_CONTENT_LENGTH},
63-
* {@link HttpComplianceSection#MULTIPLE_CONTENT_LENGTHS} and
64-
* {@link HttpComplianceSection#NO_AMBIGUOUS_PATH_SEGMENTS}.
60+
* {@link HttpComplianceSection#MULTIPLE_CONTENT_LENGTHS},
61+
* {@link HttpComplianceSection#NO_AMBIGUOUS_PATH_SEGMENTS} and
62+
* {@link HttpComplianceSection#NO_AMBIGUOUS_PATH_SEPARATORS}.
6563
*/
66-
RFC2616_LEGACY(sectionsBySpec("RFC2616,-FIELD_COLON,-METHOD_CASE_SENSITIVE,-TRANSFER_ENCODING_WITH_CONTENT_LENGTH,-MULTIPLE_CONTENT_LENGTHS,-NO_AMBIGUOUS_PATH_SEGMENTS")),
64+
RFC2616_LEGACY(sectionsBySpec("RFC2616,-FIELD_COLON,-METHOD_CASE_SENSITIVE,-TRANSFER_ENCODING_WITH_CONTENT_LENGTH,-MULTIPLE_CONTENT_LENGTHS,-NO_AMBIGUOUS_PATH_SEGMENTS,-NO_AMBIGUOUS_PATH_SEPARATORS")),
6765

6866
/**
6967
* The strict RFC2616 support mode
@@ -72,10 +70,11 @@ public enum HttpCompliance // TODO in Jetty-10 convert this enum to a class so t
7270

7371
/**
7472
* Jetty's current RFC7230 support, which excludes
75-
* {@link HttpComplianceSection#METHOD_CASE_SENSITIVE} and
76-
* {@link HttpComplianceSection#NO_AMBIGUOUS_PATH_SEGMENTS}.
73+
* {@link HttpComplianceSection#METHOD_CASE_SENSITIVE},
74+
* {@link HttpComplianceSection#NO_AMBIGUOUS_PATH_SEGMENTS} and
75+
* {@link HttpComplianceSection#NO_AMBIGUOUS_PATH_SEPARATORS}.
7776
*/
78-
RFC7230_LEGACY(sectionsBySpec("RFC7230,-METHOD_CASE_SENSITIVE,-NO_AMBIGUOUS_PATH_SEGMENTS")),
77+
RFC7230_LEGACY(sectionsBySpec("RFC7230,-METHOD_CASE_SENSITIVE,-NO_AMBIGUOUS_PATH_SEGMENTS,-NO_AMBIGUOUS_PATH_SEPARATORS")),
7978

8079
/**
8180
* The RFC7230 support mode
@@ -105,8 +104,6 @@ public enum HttpCompliance // TODO in Jetty-10 convert this enum to a class so t
105104

106105
public static final String VIOLATIONS_ATTR = "org.eclipse.jetty.http.compliance.violations";
107106

108-
private static final Logger LOG = Log.getLogger(HttpParser.class);
109-
110107
private static EnumSet<HttpComplianceSection> sectionsByProperty(String property)
111108
{
112109
String s = System.getProperty(HttpCompliance.class.getName() + property);

‎jetty-http/src/main/java/org/eclipse/jetty/http/HttpComplianceSection.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ public enum HttpComplianceSection
3232
NO_HTTP_0_9("https://tools.ietf.org/html/rfc7230#appendix-A.2", "No HTTP/0.9"),
3333
TRANSFER_ENCODING_WITH_CONTENT_LENGTH("https://tools.ietf.org/html/rfc7230#section-3.3.1", "Transfer-Encoding and Content-Length"),
3434
MULTIPLE_CONTENT_LENGTHS("https://tools.ietf.org/html/rfc7230#section-3.3.1", "Multiple Content-Lengths"),
35-
NO_AMBIGUOUS_PATH_SEGMENTS("https://tools.ietf.org/html/rfc3986#section-3.3", "No ambiguous URI path segments");
35+
NO_AMBIGUOUS_PATH_SEGMENTS("https://tools.ietf.org/html/rfc3986#section-3.3", "No ambiguous URI path segments"),
36+
NO_AMBIGUOUS_PATH_SEPARATORS("https://tools.ietf.org/html/rfc3986#section-3.3", "No ambiguous URI path separators");
3637

3738
final String url;
3839
final String description;

‎jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java‎

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.net.URISyntaxException;
2424
import java.nio.charset.Charset;
2525
import java.nio.charset.StandardCharsets;
26+
import java.util.EnumSet;
2627
import java.util.Objects;
2728

2829
import org.eclipse.jetty.util.ArrayTrie;
@@ -68,6 +69,12 @@ private enum State
6869
ASTERISK
6970
}
7071

72+
enum Ambiguous
73+
{
74+
SEGMENT,
75+
SEPARATOR
76+
}
77+
7178
/**
7279
* The concept of URI path parameters was originally specified in
7380
* <a href="https://tools.ietf.org/html/rfc2396#section-3.3">RFC2396</a>, but that was
@@ -102,7 +109,7 @@ private enum State
102109
private String _fragment;
103110
private String _uri;
104111
private String _decodedPath;
105-
private boolean _ambiguousSegment;
112+
private final EnumSet<Ambiguous> _ambiguous = EnumSet.noneOf(Ambiguous.class);
106113

107114
/**
108115
* Construct a normalized URI.
@@ -157,7 +164,7 @@ public HttpURI(HttpURI uri)
157164
_fragment = uri._fragment;
158165
_uri = uri._uri;
159166
_decodedPath = uri._decodedPath;
160-
_ambiguousSegment = uri._ambiguousSegment;
167+
_ambiguous.addAll(uri._ambiguous);
161168
}
162169

163170
public HttpURI(String uri)
@@ -204,7 +211,7 @@ public void clear()
204211
_query = null;
205212
_fragment = null;
206213
_decodedPath = null;
207-
_ambiguousSegment = false;
214+
_ambiguous.clear();
208215
}
209216

210217
public void parse(String uri)
@@ -496,7 +503,8 @@ else if (c == '/')
496503
break;
497504
case 'f':
498505
case 'F':
499-
_ambiguousSegment |= (escapedSlash == 2);
506+
if (escapedSlash == 2)
507+
_ambiguous.add(Ambiguous.SEPARATOR);
500508
escapedSlash = 0;
501509
break;
502510
default:
@@ -626,10 +634,11 @@ else if (_path != null)
626634
*/
627635
private void checkSegment(String uri, int segment, int end, boolean param)
628636
{
629-
if (!_ambiguousSegment)
637+
if (!_ambiguous.contains(Ambiguous.SEGMENT))
630638
{
631639
Boolean ambiguous = __ambiguousSegments.get(uri, segment, end - segment);
632-
_ambiguousSegment |= ambiguous == Boolean.TRUE || (param && ambiguous == Boolean.FALSE);
640+
if (ambiguous == Boolean.TRUE || (param && ambiguous == Boolean.FALSE))
641+
_ambiguous.add(Ambiguous.SEGMENT);
633642
}
634643
}
635644

@@ -638,7 +647,23 @@ private void checkSegment(String uri, int segment, int end, boolean param)
638647
*/
639648
public boolean hasAmbiguousSegment()
640649
{
641-
return _ambiguousSegment;
650+
return _ambiguous.contains(Ambiguous.SEGMENT);
651+
}
652+
653+
/**
654+
* @return True if the URI has a possibly ambiguous separator of %2f
655+
*/
656+
public boolean hasAmbiguousSeparator()
657+
{
658+
return _ambiguous.contains(Ambiguous.SEPARATOR);
659+
}
660+
661+
/**
662+
* @return True if the URI has either an {@link #hasAmbiguousSegment()} or {@link #hasAmbiguousSeparator()}.
663+
*/
664+
public boolean isAmbiguous()
665+
{
666+
return !_ambiguous.isEmpty();
642667
}
643668

644669
public String getScheme()

‎jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java‎

Lines changed: 51 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -285,73 +285,77 @@ public static Stream<Arguments> decodePathTests()
285285
return Arrays.stream(new Object[][]
286286
{
287287
// Simple path example
288-
{"http://host/path/info", "/path/info", false},
289-
{"//host/path/info", "/path/info", false},
290-
{"/path/info", "/path/info", false},
288+
{"http://host/path/info", "/path/info", false, false},
289+
{"//host/path/info", "/path/info", false, false},
290+
{"/path/info", "/path/info", false, false},
291291

292292
// legal non ambiguous relative paths
293-
{"http://host/../path/info", null, false},
294-
{"http://host/path/../info", "/info", false},
295-
{"http://host/path/./info", "/path/info", false},
296-
{"//host/path/../info", "/info", false},
297-
{"//host/path/./info", "/path/info", false},
298-
{"/path/../info", "/info", false},
299-
{"/path/./info", "/path/info", false},
300-
{"path/../info", "info", false},
301-
{"path/./info", "path/info", false},
293+
{"http://host/../path/info", null, false, false},
294+
{"http://host/path/../info", "/info", false, false},
295+
{"http://host/path/./info", "/path/info", false, false},
296+
{"//host/path/../info", "/info", false, false},
297+
{"//host/path/./info", "/path/info", false, false},
298+
{"/path/../info", "/info", false, false},
299+
{"/path/./info", "/path/info", false, false},
300+
{"path/../info", "info", false, false},
301+
{"path/./info", "path/info", false, false},
302302

303303
// illegal paths
304-
{"//host/../path/info", null, false},
305-
{"/../path/info", null, false},
306-
{"../path/info", null, false},
307-
{"/path/%XX/info", null, false},
308-
{"/path/%2/F/info", null, false},
304+
{"//host/../path/info", null, false, false},
305+
{"/../path/info", null, false, false},
306+
{"../path/info", null, false, false},
307+
{"/path/%XX/info", null, false, false},
308+
{"/path/%2/F/info", null, false, false},
309309

310310
// ambiguous dot encodings or parameter inclusions
311-
{"scheme://host/path/%2e/info", "/path/./info", true},
312-
{"scheme:/path/%2e/info", "/path/./info", true},
313-
{"/path/%2e/info", "/path/./info", true},
314-
{"path/%2e/info/", "path/./info/", true},
315-
{"/path/%2e%2e/info", "/path/../info", true},
316-
{"/path/%2e%2e;/info", "/path/../info", true},
317-
{"/path/%2e%2e;param/info", "/path/../info", true},
318-
{"/path/%2e%2e;param;other/info;other", "/path/../info", true},
319-
{"/path/.;/info", "/path/./info", true},
320-
{"/path/.;param/info", "/path/./info", true},
321-
{"/path/..;/info", "/path/../info", true},
322-
{"/path/..;param/info", "/path/../info", true},
323-
{"%2e/info", "./info", true},
324-
{"%2e%2e/info", "../info", true},
325-
{"%2e%2e;/info", "../info", true},
326-
{".;/info", "./info", true},
327-
{".;param/info", "./info", true},
328-
{"..;/info", "../info", true},
329-
{"..;param/info", "../info", true},
330-
{"%2e", ".", true},
331-
{"%2e.", "..", true},
332-
{".%2e", "..", true},
333-
{"%2e%2e", "..", true},
311+
{"scheme://host/path/%2e/info", "/path/./info", true, false},
312+
{"scheme:/path/%2e/info", "/path/./info", true, false},
313+
{"/path/%2e/info", "/path/./info", true, false},
314+
{"path/%2e/info/", "path/./info/", true, false},
315+
{"/path/%2e%2e/info", "/path/../info", true, false},
316+
{"/path/%2e%2e;/info", "/path/../info", true, false},
317+
{"/path/%2e%2e;param/info", "/path/../info", true, false},
318+
{"/path/%2e%2e;param;other/info;other", "/path/../info", true, false},
319+
{"/path/.;/info", "/path/./info", true, false},
320+
{"/path/.;param/info", "/path/./info", true, false},
321+
{"/path/..;/info", "/path/../info", true, false},
322+
{"/path/..;param/info", "/path/../info", true, false},
323+
{"%2e/info", "./info", true, false},
324+
{"%2e%2e/info", "../info", true, false},
325+
{"%2e%2e;/info", "../info", true, false},
326+
{".;/info", "./info", true, false},
327+
{".;param/info", "./info", true, false},
328+
{"..;/info", "../info", true, false},
329+
{"..;param/info", "../info", true, false},
330+
{"%2e", ".", true, false},
331+
{"%2e.", "..", true, false},
332+
{".%2e", "..", true, false},
333+
{"%2e%2e", "..", true, false},
334334

335335
// ambiguous segment separators
336-
{"/path/%2f/info", "/path///info", true},
337-
{"%2f/info", "//info", true},
338-
{"%2F/info", "//info", true},
336+
{"/path/%2f/info", "/path///info", false, true},
337+
{"%2f/info", "//info", false, true},
338+
{"%2F/info", "//info", false, true},
339+
{"/path/%2f../info", "/path//../info", false, true},
340+
{"/path/%2f/..;/info", "/path///../info", true, true},
339341

340342
// Non ascii characters
341-
{"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},
342-
{"http://localhost:9000/\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32", "/\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32\uD83C\uDF32", false},
343+
{"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},
344+
{"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},
343345
}).map(Arguments::of);
344346
}
345347

346348
@ParameterizedTest
347349
@MethodSource("decodePathTests")
348-
public void testDecodedPath(String input, String decodedPath, boolean ambiguous)
350+
public void testDecodedPath(String input, String decodedPath, boolean ambiguousSegment, boolean ambiguousSeparator)
349351
{
350352
try
351353
{
352354
HttpURI uri = new HttpURI(input);
353355
assertThat(uri.getDecodedPath(), is(decodedPath));
354-
assertThat(uri.hasAmbiguousSegment(), is(ambiguous));
356+
assertThat(uri.hasAmbiguousSegment(), is(ambiguousSegment));
357+
assertThat(uri.hasAmbiguousSeparator(), is(ambiguousSeparator));
358+
assertThat(uri.isAmbiguous(), is(ambiguousSegment || ambiguousSeparator));
355359
}
356360
catch (Exception e)
357361
{

‎jetty-server/src/main/java/org/eclipse/jetty/server/Request.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1824,16 +1824,18 @@ public void setMetaData(org.eclipse.jetty.http.MetaData.Request request)
18241824
setMethod(request.getMethod());
18251825
HttpURI uri = request.getURI();
18261826

1827-
if (uri.hasAmbiguousSegment())
1827+
if (uri.isAmbiguous())
18281828
{
1829-
// TODO replace in jetty-10 with HttpCompliance from the HttpConfiguration
1830-
Connection connection = _channel.getConnection();
1829+
// replaced in jetty-10 with URICompliance from the HttpConfiguration
1830+
Connection connection = _channel == null ? null : _channel.getConnection();
18311831
HttpCompliance compliance = connection instanceof HttpConnection
18321832
? ((HttpConnection)connection).getHttpCompliance()
1833-
: _channel.getConnector().getBean(HttpCompliance.class);
1834-
boolean allow = compliance != null && !compliance.sections().contains(HttpComplianceSection.NO_AMBIGUOUS_PATH_SEGMENTS);
1835-
if (!allow)
1833+
: _channel != null ? _channel.getConnector().getBean(HttpCompliance.class) : null;
1834+
1835+
if (uri.hasAmbiguousSegment() && (compliance == null || compliance.sections().contains(HttpComplianceSection.NO_AMBIGUOUS_PATH_SEGMENTS)))
18361836
throw new BadMessageException("Ambiguous segment in URI");
1837+
if (uri.hasAmbiguousSeparator() && (compliance == null || compliance.sections().contains(HttpComplianceSection.NO_AMBIGUOUS_PATH_SEPARATORS)))
1838+
throw new BadMessageException("Ambiguous separator in URI");
18371839
}
18381840

18391841
_originalURI = uri.isAbsolute() && request.getHttpVersion() != HttpVersion.HTTP_2 ? uri.toString() : uri.getPathQuery();

‎jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1837,7 +1837,7 @@ public void testGetterSafeFromNullPointerException()
18371837
}
18381838

18391839
@Test
1840-
public void testAmbiguousPaths() throws Exception
1840+
public void testAmbiguousSegments() throws Exception
18411841
{
18421842
_handler._checker = (request, response) -> true;
18431843

@@ -1858,6 +1858,28 @@ public void testAmbiguousPaths() throws Exception
18581858
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200"));
18591859
}
18601860

1861+
@Test
1862+
public void testAmbiguousSeparators() throws Exception
1863+
{
1864+
_handler._checker = (request, response) -> true;
1865+
1866+
String request = "GET /ambiguous/%2f/path HTTP/1.0\r\n" +
1867+
"Host: whatever\r\n" +
1868+
"\r\n";
1869+
1870+
_connector.getBean(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC7230);
1871+
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400"));
1872+
1873+
_connector.getBean(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC7230_LEGACY);
1874+
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200"));
1875+
1876+
_connector.getBean(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC2616);
1877+
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 400"));
1878+
1879+
_connector.getBean(HttpConnectionFactory.class).setHttpCompliance(HttpCompliance.RFC2616_LEGACY);
1880+
assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200"));
1881+
}
1882+
18611883
private static long getFileCount(Path path)
18621884
{
18631885
try (Stream<Path> s = Files.list(path))
@@ -1961,7 +1983,6 @@ public void handle(String target, Request baseRequest, HttpServletRequest reques
19611983
((Request)request).setHandled(true);
19621984
try
19631985
{
1964-
19651986
MultipartConfigElement mpce = new MultipartConfigElement(tmpDir.getAbsolutePath(), -1, -1, 2);
19661987
request.setAttribute(Request.MULTIPART_CONFIG_ELEMENT, mpce);
19671988

0 commit comments

Comments
 (0)