Review UriCompliance.LEGACY behavior with bad UTF-8 in query (#12791)

* Added tests for DEFAULT, ALLOW_BAD_UTF8 and LEGACY.
* LEGACY expectations are set to jetty-11 results
* Added TRUNCATED_UTF8_ENCODING for LEGACY mode
* Adding up-to-date tests for query extraction behavior.
* Left `queryBehaviorsLegacy` alone, as expectations from Jetty 11.
* The `queryBehaviorsDefault` expectations are based Jetty 12.0.16.
* The `queryBehaviorsBadUtf8Allowed` has its expectations updated.

---------

Co-authored-by: Joakim Erdfelt <[email protected]>

Author: gregw

jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java

@@ -100,6 +100,11 @@ public enum Violation implements ComplianceViolation
          */
         BAD_UTF8_ENCODING("https://datatracker.ietf.org/doc/html/rfc5987#section-3.2.1", "Bad UTF-8 encoding"),
 
+        /**
+         * Allow truncated UTF-8 encodings to be substituted by the replacement character.
+         */
+        TRUNCATED_UTF8_ENCODING("https://datatracker.ietf.org/doc/html/rfc5987#section-3.2.1", "Truncated UTF-8 encoding"),
+
         /**
          * Allow encoded path characters not allowed by the Servlet spec rules.
          */
@@ -210,7 +215,7 @@ public String getDescription()
             Violation.AMBIGUOUS_PATH_SEPARATOR,
             Violation.AMBIGUOUS_PATH_ENCODING,
             Violation.AMBIGUOUS_EMPTY_SEGMENT,
-            Violation.BAD_UTF8_ENCODING,
+            Violation.TRUNCATED_UTF8_ENCODING,
             Violation.UTF16_ENCODINGS,
             Violation.USER_INFO));
 

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

@@ -36,6 +36,7 @@
 import java.util.function.Function;
 import java.util.function.Predicate;
 
+import org.eclipse.jetty.http.BadMessageException;
 import org.eclipse.jetty.http.ComplianceViolation;
 import org.eclipse.jetty.http.HttpCookie;
 import org.eclipse.jetty.http.HttpFields;
@@ -559,27 +560,38 @@ static Fields extractQueryParameters(Request request)
 
     static Fields extractQueryParameters(Request request, Charset charset)
     {
-        String query = request.getHttpURI().getQuery();
-        if (StringUtil.isBlank(query))
-            return Fields.EMPTY;
-        Fields fields = new Fields(true);
-
-        if (charset == null || StandardCharsets.UTF_8.equals(charset))
+        UriCompliance uriCompliance = null;
+        try
         {
-            UriCompliance uriCompliance = request.getConnectionMetaData().getHttpConfiguration().getUriCompliance();
-            boolean allowBadUtf8 = uriCompliance.allows(UriCompliance.Violation.BAD_UTF8_ENCODING);
-            if (!UrlEncoded.decodeUtf8To(query, 0, query.length(), fields::add, allowBadUtf8))
+            String query = request.getHttpURI().getQuery();
+            if (StringUtil.isBlank(query))
+                return Fields.EMPTY;
+            Fields fields = new Fields(true);
+
+            if (charset == null || StandardCharsets.UTF_8.equals(charset))
+            {
+                uriCompliance = request.getConnectionMetaData().getHttpConfiguration().getUriCompliance();
+                boolean allowTruncatedUtf8 = uriCompliance.allows(UriCompliance.Violation.TRUNCATED_UTF8_ENCODING);
+                boolean allowBadUtf8 = uriCompliance.allows(UriCompliance.Violation.BAD_UTF8_ENCODING);
+                if (!UrlEncoded.decodeUtf8To(query, 0, query.length(), fields::add, allowTruncatedUtf8, allowBadUtf8))
+                {
+                    HttpChannel httpChannel = HttpChannel.from(request);
+                    if (httpChannel != null && httpChannel.getComplianceViolationListener() != null)
+                        httpChannel.getComplianceViolationListener().onComplianceViolation(new ComplianceViolation.Event(uriCompliance, UriCompliance.Violation.BAD_UTF8_ENCODING, "query=" + query));
+                }
+            }
+            else
             {
-                HttpChannel httpChannel = HttpChannel.from(request);
-                if (httpChannel != null && httpChannel.getComplianceViolationListener() != null)
-                    httpChannel.getComplianceViolationListener().onComplianceViolation(new ComplianceViolation.Event(uriCompliance, UriCompliance.Violation.BAD_UTF8_ENCODING, "query=" + query));
+                UrlEncoded.decodeTo(query, fields::add, charset);
             }
+            return fields;
         }
-        else
+        catch (Throwable t)
         {
-            UrlEncoded.decodeTo(query, fields::add, charset);
+//            if (uriCompliance == UriCompliance.LEGACY)
+//                throw t;
+            throw new BadMessageException("Bad query", t);
         }
-        return fields;
     }
 
     static Fields getParameters(Request request) throws Exception

jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/FormFieldsTest.java

@@ -111,7 +111,7 @@ public static Stream<Arguments> invalidData()
             Arguments.of(List.of("name%"), UTF_8, -1, -1, IllegalStateException.class),
             Arguments.of(List.of("name%A"), UTF_8, -1, -1, IllegalStateException.class),
 
-            Arguments.of(List.of("name%A="), UTF_8, -1, -1, CharacterCodingException.class),
+            Arguments.of(List.of("name%A="), UTF_8, -1, -1, IllegalArgumentException.class),
             Arguments.of(List.of("name%A&"), UTF_8, -1, -1, IllegalArgumentException.class),
 
             Arguments.of(List.of("name=%"), UTF_8, -1, -1, IllegalStateException.class),