Safer parse by not allowing beyond a final state

jetty · Mar 27, 2024 · fed10f7 · fed10f7
1 parent 50d8c94
commit fed10f7
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 10 deletions.
diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpURI.java
@@ -509,6 +509,7 @@ class Mutable implements HttpURI
         private enum State
         {
             START,
+            ASTERISK,
             HOST_OR_PATH,
             SCHEME_OR_PATH,
             HOST,
@@ -517,8 +518,7 @@ private enum State
             PATH,
             PARAM,
             QUERY,
-            FRAGMENT,
-            ASTERISK
+            FRAGMENT
         }
 
         /**
@@ -703,7 +703,7 @@ private Mutable(String scheme, String host, int port, String pathQuery)
             _port = port;
 
             if (pathQuery != null)
-                parse(State.PATH, pathQuery);
+                parse(State.PATH, State.QUERY, pathQuery);
         }
 
         @Override
@@ -960,7 +960,7 @@ public Mutable param(String param)
         }
 
         /**
-         * @param path the path
+         * @param path the encoded path
          * @return this Mutable
          */
         public Mutable path(String path)
@@ -974,7 +974,7 @@ public Mutable path(String path)
             _canonicalPath = null;
             String param = _param;
             _param = null;
-            parse(State.PATH, path);
+            parse(State.PATH, State.PARAM, path);
 
             // If the passed path does not have a parameter, then keep the current parameter
             // else delete the current parameter
@@ -997,7 +997,7 @@ public Mutable pathQuery(String pathQuery)
             _param = null;
             _query = null;
             if (pathQuery != null)
-                parse(State.PATH, pathQuery);
+                parse(State.PATH, State.QUERY, pathQuery);
             return this;
         }
 
@@ -1092,6 +1092,11 @@ public Mutable user(String user)
         }
 
         private void parse(State state, final String uri)
+        {
+            parse(state, null, uri);
+        }
+
+        private void parse(State state, State last, final String uri)
         {
             int mark = 0; // the start of the current section being parsed
             int pathMark = 0; // the start of the path section
@@ -1452,6 +1457,9 @@ else if (c == '/')
                 }
             }
 
+            if (last != null && state.ordinal() > last.ordinal())
+                throw new IllegalArgumentException("uri cannot go beyond " + last);
+
             switch (state)
             {
                 case START:

diff --git a/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java b/jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/HttpURITest.java
@@ -552,6 +552,10 @@ public static Stream<Arguments> testPathQueryTests()
                 {"%2e%2e", null, null, null},
                 {"..;/info", null, null, null},
                 {"..;param/info", null, null, null},
+                {"#n=v", null, null, null},
+                {"/#", null, null, null},
+                {"/foo/#bar", null, null, null},
+                {"/foo#bar", null, null, null},
 
                 // ambiguous dot encodings
                 {"/path/%2e/info", "/path/info", "/path/info", EnumSet.of(Violation.AMBIGUOUS_PATH_SEGMENT)},
@@ -565,18 +569,15 @@ public static Stream<Arguments> testPathQueryTests()
 
                 // empty segment treated as ambiguous
                 {"/", "/", "/", EnumSet.noneOf(Violation.class)},
-                {"/#", "/", "/", EnumSet.noneOf(Violation.class)},
                 {"/path", "/path", "/path", EnumSet.noneOf(Violation.class)},
                 {"/path/", "/path/", "/path/", EnumSet.noneOf(Violation.class)},
                 {"//", "//", "//", EnumSet.of(Violation.AMBIGUOUS_EMPTY_SEGMENT)},
                 {"/foo//", "/foo//", "/foo//", EnumSet.of(Violation.AMBIGUOUS_EMPTY_SEGMENT)},
                 {"/foo//bar", "/foo//bar", "/foo//bar", EnumSet.of(Violation.AMBIGUOUS_EMPTY_SEGMENT)},
                 {"//foo/bar", "//foo/bar", "//foo/bar", EnumSet.of(Violation.AMBIGUOUS_EMPTY_SEGMENT)},
                 {"/foo?bar", "/foo", "/foo", EnumSet.noneOf(Violation.class)},
-                {"/foo#bar", "/foo", "/foo", EnumSet.noneOf(Violation.class)},
                 {"/foo;bar", "/foo", "/foo", EnumSet.noneOf(Violation.class)},
                 {"/foo/?bar", "/foo/", "/foo/", EnumSet.noneOf(Violation.class)},
-                {"/foo/#bar", "/foo/", "/foo/", EnumSet.noneOf(Violation.class)},
                 {"/foo/;param", "/foo/", "/foo/", EnumSet.noneOf(Violation.class)},
                 {"/foo/;param/bar", "/foo//bar", "/foo//bar", EnumSet.of(Violation.AMBIGUOUS_EMPTY_SEGMENT)},
                 {"/foo//bar", "/foo//bar", "/foo//bar", EnumSet.of(Violation.AMBIGUOUS_EMPTY_SEGMENT)},
@@ -591,7 +592,6 @@ public static Stream<Arguments> testPathQueryTests()
                 {";/bar", "/bar", "/bar", EnumSet.of(Violation.AMBIGUOUS_EMPTY_SEGMENT)},
                 {";?n=v", "", "", EnumSet.of(Violation.AMBIGUOUS_EMPTY_SEGMENT)},
                 {"?n=v", "", "", EnumSet.noneOf(Violation.class)},
-                {"#n=v", "", "", EnumSet.noneOf(Violation.class)},
                 {"", "", "", EnumSet.noneOf(Violation.class)},
 
                 // ambiguous parameter inclusions