Improve Error messages for Ambiguous URIs (#11457)

* Some testing of HttpURI for Issue #11448 * Issue #11448 - improved stacktrace message for ambiguous URI
jetty · Feb 28, 2024 · 97cb50e · 97cb50e
1 parent 98ceb73
commit 97cb50e
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 7 deletions.
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
@@ -1024,6 +1024,8 @@ public static Stream<Arguments> concatNormalizedURICases()
             // Path choices
             Arguments.of("http", "example.org", 0, "/a/b/c/d", null, null, "http://example.org/a/b/c/d"),
             Arguments.of("http", "example.org", 0, "/a%20b/c%20d", null, null, "http://example.org/a%20b/c%20d"),
+            Arguments.of("http", "example.org", 0, "/foo%2Fbaz", null, null, "http://example.org/foo%2Fbaz"),
+            Arguments.of("http", "example.org", 0, "/foo%252Fbaz", null, null, "http://example.org/foo%252Fbaz"),
             // Query specified
             Arguments.of("http", "example.org", 0, "/", "a=b", null, "http://example.org/?a=b"),
             Arguments.of("http", "example.org", 0, "/documentation/latest/", "a=b", null, "http://example.org/documentation/latest/?a=b"),
@@ -1046,6 +1048,24 @@ public void testFromAsStringNormalized(String scheme, String server, int port, S
         assertThat(httpURI.asString(), is(expectedStr));
     }
 
+    public static Stream<Arguments> fromStringAsStringCases()
+    {
+        return Stream.of(
+            Arguments.of("http://localhost:4444/", "http://localhost:4444/"),
+            Arguments.of("/foo/baz", "/foo/baz"),
+            Arguments.of("/foo%2Fbaz", "/foo%2Fbaz"),
+            Arguments.of("/foo%252Fbaz", "/foo%252Fbaz")
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("fromStringAsStringCases")
+    public void testFromStringAsString(String input, String expected)
+    {
+        HttpURI httpURI = HttpURI.from(input);
+        assertThat(httpURI.asString(), is(expected));
+    }
+
     /**
      * Tests of parameters that result in undesired behaviors.
      * {@link HttpURI#from(String, String, int, String)}

diff --git a/...10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java b/...10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java
@@ -1307,21 +1307,24 @@ public Map<String, String> getTrailerFields()
 
     static class AmbiguousURI extends ServletApiRequest
     {
-        protected AmbiguousURI(ServletContextRequest servletContextRequest)
+        private final String msg;
+
+        protected AmbiguousURI(ServletContextRequest servletContextRequest, String msg)
         {
             super(servletContextRequest);
+            this.msg = msg;
         }
 
         @Override
         public String getPathInfo()
         {
-            throw new HttpException.IllegalArgumentException(HttpStatus.BAD_REQUEST_400, "Ambiguous URI encoding");
+            throw new HttpException.IllegalArgumentException(HttpStatus.BAD_REQUEST_400, msg);
         }
 
         @Override
         public String getServletPath()
         {
-            throw new HttpException.IllegalArgumentException(HttpStatus.BAD_REQUEST_400, "Ambiguous URI encoding");
+            throw new HttpException.IllegalArgumentException(HttpStatus.BAD_REQUEST_400, msg);
         }
     }
 

diff --git a/...etty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java b/...etty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java
@@ -197,11 +197,28 @@ protected ServletApiRequest newServletApiRequest()
         if (getHttpURI().hasViolations() && !getServletChannel().getServletContextHandler().getServletHandler().isDecodeAmbiguousURIs())
         {
             // TODO we should check if current compliance mode allows all the violations?
-
-            for (UriCompliance.Violation violation : getHttpURI().getViolations())
+            if (getHttpURI().hasViolations())
             {
-                if (UriCompliance.AMBIGUOUS_VIOLATIONS.contains(violation))
-                    return new ServletApiRequest.AmbiguousURI(this);
+                StringBuilder msg = null;
+                for (UriCompliance.Violation violation : getHttpURI().getViolations())
+                {
+                    if (UriCompliance.AMBIGUOUS_VIOLATIONS.contains(violation))
+                    {
+                        if (msg == null)
+                        {
+                            msg = new StringBuilder();
+                            msg.append("Ambiguous URI encoding: ");
+                        }
+                        else
+                        {
+                            msg.append(", ");
+                        }
+
+                        msg.append(violation.name());
+                    }
+                }
+                if (msg != null)
+                    return new ServletApiRequest.AmbiguousURI(this, msg.toString());
             }
         }