From 97cb50ead9a8fff370ca21a7d948a3ea29068d68 Mon Sep 17 00:00:00 2001
From: Joakim Erdfelt <joakim.erdfelt@gmail.com>
Date: Wed, 28 Feb 2024 04:15:56 -0800
Subject: [PATCH] Improve Error messages for Ambiguous URIs (#11457)

* Some testing of HttpURI for Issue #11448
* Issue #11448 - improved stacktrace message for ambiguous URI
---
 .../org/eclipse/jetty/http/HttpURITest.java   | 20 +++++++++++++++
 .../jetty/ee10/servlet/ServletApiRequest.java |  9 ++++---
 .../ee10/servlet/ServletContextRequest.java   | 25 ++++++++++++++++---
 3 files changed, 47 insertions(+), 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
index aa54abe7a6a..434af0c7612 100644
--- 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 class HttpURITest
             // 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 class HttpURITest
         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/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java
index 28a6facd63e..b0a711d0f9d 100644
--- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java
+++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletApiRequest.java
@@ -1307,21 +1307,24 @@ public class ServletApiRequest implements HttpServletRequest
 
     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/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java
index b9ab64d1b30..143e0fbd680 100644
--- a/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java
+++ b/jetty-ee10/jetty-ee10-servlet/src/main/java/org/eclipse/jetty/ee10/servlet/ServletContextRequest.java
@@ -197,11 +197,28 @@ public class ServletContextRequest extends ContextRequest implements ServletCont
         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());
             }
         }