Fix #4275 fail URIs with ambiguous segments

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

@@ -53,7 +53,7 @@ public enum HttpCompliance // TODO in Jetty-10 convert this enum to a class so t
     /**
      * A Legacy compliance mode to match jetty's behavior prior to RFC2616 and RFC7230.
      */
-    LEGACY(sectionsBySpec("0,METHOD_CASE_SENSITIVE")),
+    LEGACY(sectionsBySpec("0,METHOD_CASE_SENSITIVE,AMBIGUOUS_PATH_SEGMENTS")),
 
     /**
      * The legacy RFC2616 support, which incorrectly excludes

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

@@ -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"),
+    AMBIGUOUS_PATH_SEGMENTS("", "Allows ambiguous URI path segments");
 
     final String url;
     final String description;

jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java

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

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

@@ -24,7 +24,9 @@
 import java.nio.charset.Charset;
 import java.nio.charset.StandardCharsets;
 
+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 +67,18 @@ private enum State
         ASTERISK
     }
 
+    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;
@@ -77,6 +91,8 @@ private enum State
     String _uri;
     String _decodedPath;
 
+    boolean _ambiguousSegment;
+
     /**
      * Construct a normalized URI.
      * Port is not set if it is the default port.
@@ -208,6 +224,8 @@ private void parse(State state, final String uri, final int offset, final int en
         boolean encoded = false;
         int mark = offset;
         int pathMark = 0;
+        int segment = 0;
+        boolean encodedSegment = false;
 
         for (int i = offset; i < end; i++)
         {
@@ -241,14 +259,18 @@ private void parse(State state, final String uri, final int offset, final int en
                             _path = "*";
                             state = State.ASTERISK;
                             break;
-
+                        case '%':
+                            encoded = encodedSegment = true;
+                            mark = 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;
                             }
                     }
@@ -269,6 +291,7 @@ private void parse(State state, final String uri, final int offset, final int en
 
                         case '/':
                             // must have been in a path and still are
+                            segment = i + 1;
                             state = State.PATH;
                             break;
 
@@ -288,6 +311,7 @@ private void parse(State state, final String uri, final int offset, final int en
                         case '%':
                             // must have be in an encoded path 
                             encoded = true;
+                            encodedSegment = true;
                             state = State.PATH;
                             break;
 
@@ -317,11 +341,13 @@ private void parse(State state, final String uri, final int offset, final int en
                             // 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;
@@ -334,6 +360,7 @@ private void parse(State state, final String uri, final int offset, final int en
                         case '/':
                             _host = uri.substring(mark, i);
                             pathMark = mark = i;
+                            segment = mark + 1;
                             state = State.PATH;
                             break;
                         case ':':
@@ -396,6 +423,7 @@ else if (c == '/')
                     {
                         _port = TypeUtil.parseInt(uri, mark, i - mark, 10);
                         pathMark = mark = i;
+                        segment = i + 1;
                         state = State.PATH;
                     }
                     continue;
@@ -406,21 +434,30 @@ else if (c == '/')
                     switch (c)
                     {
                         case ';':
+                            checkSegment(encodedSegment, true, uri, segment, i);
                             mark = i + 1;
                             state = State.PARAM;
                             break;
                         case '?':
+                            checkSegment(encodedSegment, false, uri, segment, i);
                             _path = uri.substring(pathMark, i);
                             mark = i + 1;
                             state = State.QUERY;
                             break;
                         case '#':
+                            checkSegment(encodedSegment, false, uri, segment, i);
                             _path = uri.substring(pathMark, i);
                             mark = i + 1;
                             state = State.FRAGMENT;
                             break;
                         case '%':
                             encoded = true;
+                            encodedSegment = true;
+                            break;
+                        case '/':
+                            checkSegment(encodedSegment, false, uri, segment, i);
+                            encodedSegment = false;
+                            segment = i + 1;
                             break;
                     }
                     continue;
@@ -443,8 +480,10 @@ else if (c == '/')
                             state = State.FRAGMENT;
                             break;
                         case '/':
-                            encoded = true;
                             // ignore internal params
+                            encoded = true;
+                            encodedSegment = false;
+                            segment = i + 1;
                             state = State.PATH;
                             break;
                         case ';':
@@ -516,6 +555,7 @@ else if (c == '/')
                 break;
 
             case PATH:
+                checkSegment(encodedSegment, false, uri, segment, end);
                 _path = uri.substring(pathMark, end);
                 break;
 
@@ -533,6 +573,38 @@ else if (c == '/')
         }
     }
 
+    /**
+     * 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 segmentEncoded True if the segment is encoded
+     * @param param true if the segment has a parameter
+     * @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(boolean segmentEncoded, boolean param, String uri, int segment, int end)
+    {
+        // if the segment is encoded or there is a segment parameter
+        if (!_ambiguousSegment && (segmentEncoded || param))
+        {
+            // Look for possible bad segments
+            Boolean ambiguous = __ambiguousSegments.get(uri, segment, end - segment);
+
+            // A segment is ambiguous if it is always ambiguous (TRUE) or ambiguous only with params (FALSE) and we have a param
+            _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()
     {
         return _scheme;
@@ -633,6 +705,8 @@ public void clear()
         _fragment = null;
 
         _decodedPath = null;
+
+        _ambiguousSegment = false;
     }
 
     public boolean isAbsolute()

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

@@ -20,9 +20,12 @@
 
 import java.net.URLEncoder;
 import java.nio.charset.StandardCharsets;
+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.MethodSource;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
@@ -221,4 +224,40 @@ public void testBasicAuthCredentials() throws Exception
         assertEquals(uri.getAuthority(), "example.com:8888");
         assertEquals(uri.getUser(), "user:password");
     }
+
+    public static Stream<String> pathsWithAmbiguousSegments()
+    {
+        // @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck
+        return Stream.of(
+            "/foo/%2e%2e/bar",
+            "/foo/%2e%2e;/bar",
+            "/foo/%2e%2e;param/bar",
+            "/foo/..;/bar",
+            "/foo/..;param/bar",
+            "foo/%2e%2e/bar",
+            "%2e%2e/bar",
+            "//host/%2e%2e/bar",
+            "scheme://host/%2e%2e/bar",
+            "scheme:/%2e%2e/bar",
+            "/foo/%2e%2e",
+            "/foo/%2e%2e?query",
+            "/foo/%2e%2e#fragment",
+            "foo/%2e.",
+            ".%2e",
+            "//host/%2e%2e",
+            "scheme://host/.%2e",
+            "scheme:/%2e%2e",
+            "%2e",
+            "%2e.",
+            ".%2e",
+            "%2e%2e"
+        );
+    }
+
+    @ParameterizedTest(name = "[{index}] {0}")
+    @MethodSource("pathsWithAmbiguousSegments")
+    public void testAmbiguousSegments(String uriWithBadSegment)
+    {
+        assertTrue(new HttpURI(uriWithBadSegment).hasAmbiguousSegment());
+    }
 }

jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java

@@ -114,6 +114,12 @@ public HttpConnection(HttpConfiguration config, Connector connector, EndPoint en
             LOG.debug("New HTTP Connection {}", this);
     }
 
+    @Deprecated
+    public HttpCompliance getHttpCompliance()
+    {
+        return _parser.getHttpCompliance();
+    }
+
     public HttpConfiguration getHttpConfiguration()
     {
         return _config;

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

@@ -65,6 +65,7 @@
 
 import org.eclipse.jetty.http.BadMessageException;
 import org.eclipse.jetty.http.HostPortHttpField;
+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 +78,7 @@
 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 +1817,18 @@ public void setMetaData(org.eclipse.jetty.http.MetaData.Request request)
 
         setMethod(request.getMethod());
         HttpURI uri = request.getURI();
+
+        if (uri.hasAmbiguousSegment())
+        {
+            // TODO replace in jetty-10 with HttpCompliance from the HttpConfiguration
+            Connection connection = _channel.getConnection();
+            boolean allow = connection instanceof HttpConnection
+                ? ((HttpConnection)connection).getHttpCompliance().sections().contains(HttpComplianceSection.AMBIGUOUS_PATH_SEGMENTS)
+                : _channel.getServer().getAttribute(HttpComplianceSection.AMBIGUOUS_PATH_SEGMENTS.toString()) == Boolean.TRUE;
+            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();