Issue #6302 - Treat empty path segments are ambiguous.

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
jetty · May 20, 2021 · 36332ab · 36332ab
1 parent 1d5d807
commit 36332ab
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 3 deletions.
diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
@@ -377,13 +377,13 @@ public boolean isAmbiguous()
         @Override
         public boolean hasAmbiguousSegment()
         {
-            return _ambiguous.contains(Mutable.Ambiguous.SEGMENT);
+            return _ambiguous.contains(Ambiguous.SEGMENT);
         }
 
         @Override
         public boolean hasAmbiguousSeparator()
         {
-            return _ambiguous.contains(Mutable.Ambiguous.SEPARATOR);
+            return _ambiguous.contains(Ambiguous.SEPARATOR);
         }
 
         @Override
@@ -453,6 +453,7 @@ private enum State
             .with("%2e%2e", Boolean.TRUE)
             .with(".%2e", Boolean.TRUE)
             .with("%2e.", Boolean.TRUE)
+            .with("", Boolean.TRUE)
             .with("..", Boolean.FALSE)
             .with(".", Boolean.FALSE)
             .build();

diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java
@@ -361,6 +361,13 @@ public static Stream<Arguments> decodePathTests()
                 {".%2e", "..", EnumSet.of(Ambiguous.SEGMENT)},
                 {"%2e%2e", "..", EnumSet.of(Ambiguous.SEGMENT)},
 
+                // empty segment treated as ambiguous
+                {"/foo//bar", "/foo//bar", EnumSet.of(Ambiguous.SEGMENT)},
+                {"/foo//../bar", "/foo/bar", EnumSet.of(Ambiguous.SEGMENT)},
+                {"/foo///../../../bar", "/bar", EnumSet.of(Ambiguous.SEGMENT)},
+                {"/foo/./../bar", "/bar", EnumSet.noneOf(Ambiguous.class)},
+                {"/foo//./bar", "/foo//bar", EnumSet.of(Ambiguous.SEGMENT)},
+
                 // ambiguous parameter inclusions
                 {"/path/.;/info", "/path/./info", EnumSet.of(Ambiguous.PARAM)},
                 {"/path/.;param/info", "/path/./info", EnumSet.of(Ambiguous.PARAM)},
@@ -377,6 +384,11 @@ public static Stream<Arguments> decodePathTests()
                 {"%2F/info", "//info", EnumSet.of(Ambiguous.SEPARATOR)},
                 {"/path/%2f../info", "/path//../info", EnumSet.of(Ambiguous.SEPARATOR)},
 
+                // ambiguous encoding
+                {"/path/%25/info", "/path/%/info", EnumSet.of(Ambiguous.ENCODING)},
+                {"%25/info", "%/info", EnumSet.of(Ambiguous.ENCODING)},
+                {"/path/%25../info", "/path/%../info", EnumSet.of(Ambiguous.ENCODING)},
+
                 // 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)},
@@ -401,6 +413,7 @@ public void testDecodedPath(String input, String decodedPath, EnumSet<Ambiguous>
             assertThat(uri.hasAmbiguousSegment(), is(expected.contains(Ambiguous.SEGMENT)));
             assertThat(uri.hasAmbiguousSeparator(), is(expected.contains(Ambiguous.SEPARATOR)));
             assertThat(uri.hasAmbiguousParameter(), is(expected.contains(Ambiguous.PARAM)));
+            assertThat(uri.hasAmbiguousEncoding(), is(expected.contains(Ambiguous.ENCODING)));
         }
         catch (Exception e)
         {

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java
@@ -1252,7 +1252,7 @@ public String getRemoteUser()
     public RequestDispatcher getRequestDispatcher(String path)
     {
         // path is encoded, potentially with query
-
+        // TODO: why do we do this?
         path = URIUtil.compactPath(path);
 
         if (path == null || _context == null)

diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java
@@ -1238,6 +1238,7 @@ public void doScope(String target, Request baseRequest, HttpServletRequest reque
             // check the target.
             if (DispatcherType.REQUEST.equals(dispatch) || DispatcherType.ASYNC.equals(dispatch))
             {
+                // TODO: remove this once isCompact() has been deprecated for several releases.
                 if (isCompactPath())
                     target = URIUtil.compactPath(target);
                 if (!checkContext(target, baseRequest, response))
@@ -1798,7 +1799,9 @@ public void setMaxFormKeys(int max)
 
     /**
      * @return True if URLs are compacted to replace multiple '/'s with a single '/'
+     * @deprecated use {@code CompactPathRule} with {@code RewriteHandler} instead.
      */
+    @Deprecated
     public boolean isCompactPath()
     {
         return _compactPath;
@@ -1807,6 +1810,7 @@ public boolean isCompactPath()
     /**
      * @param compactPath True if URLs are compacted to replace multiple '/'s with a single '/'
      */
+    @Deprecated
     public void setCompactPath(boolean compactPath)
     {
         _compactPath = compactPath;

diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java
@@ -1790,6 +1790,17 @@ public void testAmbiguousEncoding() throws Exception
         _connector.getBean(HttpConnectionFactory.class).getHttpConfiguration().setUriCompliance(UriCompliance.UNSAFE);
         assertThat(_connector.getResponse(request), startsWith("HTTP/1.1 200"));
     }
+
+    @Test
+    public void testDoubleSlash() throws Exception
+    {
+        _handler._checker = (request, response) -> true;
+        String request = "GET //../foo 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"));
+    }
 
     @Test
     public void testPushBuilder()