Issue #12806 - Overhaul ComplianceViolation.Listener reporting

jetty-core/jetty-compression/jetty-compression-server/src/main/java/org/eclipse/jetty/compression/server/CompressionHandler.java

@@ -34,7 +34,7 @@
 import org.eclipse.jetty.http.pathmap.PathMappings;
 import org.eclipse.jetty.http.pathmap.PathSpec;
 import org.eclipse.jetty.server.Handler;
-import org.eclipse.jetty.server.HttpConfiguration;
+import org.eclipse.jetty.server.HttpChannel;
 import org.eclipse.jetty.server.Request;
 import org.eclipse.jetty.server.Response;
 import org.eclipse.jetty.util.Callback;
@@ -271,16 +271,13 @@ public boolean handle(Request request, Response response, Callback callback) thr
                     // Collect all Accept-Encoding headers.
                     if (qualityCSV == null)
                     {
-                        HttpConfiguration httpConfiguration = request.getConnectionMetaData().getHttpConfiguration();
+                        HttpChannel httpChannel = HttpChannel.from(request);
                         qualityCSV = new QuotedQualityCSV()
                         {
                             @Override
                             protected void onComplianceViolation(ComplianceViolation violation, String value)
                             {
-                                if (httpConfiguration.getHttpCompliance().allows(violation))
-                                    httpConfiguration.notifyViolation(violation, value);
-                                else
-                                    throw new BadMessageException(violation.toString());
+                                httpChannel.complianceAssert(violation, value, BadMessageException::new);
                             }
                         };
                     }

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

@@ -82,10 +82,13 @@ interface Mode
         Set<? extends ComplianceViolation> getAllowed();
     }
 
-    static void notify(List<ComplianceViolation.Listener> listeners, ComplianceViolation.Mode mode, ComplianceViolation violation, String details)
+    /**
+     * @deprecated no replacement
+     */
+    @Deprecated(forRemoval = true, since = "12.1.5")
+    static void notify(List<ComplianceViolation.Listener> listeners, ComplianceViolation.Mode mode, ComplianceViolation violation, String details, boolean allowed)
     {
-        Event event = new Event(mode, violation, details);
-
+        Event event = new Event(mode, violation, details, allowed);
         Throwable throwable = null;
         for (Listener listener : listeners)
         {
@@ -101,13 +104,13 @@ static void notify(List<ComplianceViolation.Listener> listeners, ComplianceViola
         ExceptionUtil.ifExceptionThrowUnchecked(throwable);
     }
 
-    record Event(ComplianceViolation.Mode mode, ComplianceViolation violation, String details)
+    record Event(ComplianceViolation.Mode mode, ComplianceViolation violation, String details, boolean allowed)
     {
         @Override
         public String toString()
         {
-            return String.format("%s (see %s) in mode %s for %s",
-                violation.getDescription(), violation.getURL(), mode, details);
+            return String.format("%s (see %s) in mode %s for %s (%s)",
+                violation.getDescription(), violation.getURL(), mode, details, allowed ? "allowed" : "forbidden");
         }
     }
 
@@ -161,7 +164,7 @@ default void onComplianceViolation(Event event)
          * @param mode the mode
          * @param violation the violation
          * @param details the details
-         * @deprecated use {@link #onComplianceViolation(Event)} instead.  Will be removed in Jetty 12.1.0
+         * @deprecated use {@link #onComplianceViolation(Event)} instead.
          */
         @Deprecated(since = "12.0.6", forRemoval = true)
         default void onComplianceViolation(Mode mode, ComplianceViolation violation, String details)
@@ -200,7 +203,10 @@ private CapturingListener(List<Event> events)
         @Override
         public Listener initialize()
         {
-            return new CapturingListener(new ArrayList<>());
+            List<Event> requestEvents = new ArrayList<>();
+            if (events != null)
+                requestEvents.addAll(events);
+            return new CapturingListener(requestEvents);
         }
 
         @Override

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

@@ -19,7 +19,7 @@ public class ComplianceViolationException extends IllegalArgumentException
 
     public ComplianceViolationException(ComplianceViolation.Mode mode, ComplianceViolation violation, String details)
     {
-        this(new ComplianceViolation.Event(mode, violation, details));
+        this(new ComplianceViolation.Event(mode, violation, details, false));
     }
 
     public ComplianceViolationException(ComplianceViolation.Event event)

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

@@ -19,6 +19,7 @@
 import java.util.List;
 import java.util.ListIterator;
 import java.util.Map;
+import java.util.Objects;
 
 import org.eclipse.jetty.util.StringUtil;
 
@@ -85,6 +86,8 @@ public List<HttpCookie> getCookies(HttpFields headers)
 
     public List<HttpCookie> getCookies(HttpFields headers, ComplianceViolation.Listener complianceViolationListener)
     {
+        Objects.requireNonNull(complianceViolationListener);
+
         boolean building = false;
         ListIterator<String> raw = _rawFields.listIterator();
         // For each of the headers
@@ -164,7 +167,9 @@ public List<HttpCookie> getCookies(HttpFields headers, ComplianceViolation.Liste
         }
 
         if (_violations != null && !_violations.isEmpty())
+        {
             _violations.forEach(complianceViolationListener::onComplianceViolation);
+        }
 
         return _cookieList == null ? Collections.emptyList() : _cookieList;
     }

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

@@ -373,8 +373,9 @@ else if (tokenstart >= 0)
 
     protected void reportComplianceViolation(CookieCompliance.Violation violation, String reason)
     {
+        boolean allows = _complianceMode.allows(violation);
         if (_complianceListener != null)
-            _complianceListener.onComplianceViolation(new ComplianceViolation.Event(_complianceMode, violation, reason));
+            _complianceListener.onComplianceViolation(new ComplianceViolation.Event(_complianceMode, violation, reason, allows));
     }
 
     protected boolean isRFC6265RejectedCharacter(char c)

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

@@ -19,6 +19,7 @@
 import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Function;
 
 import org.eclipse.jetty.util.StringUtil;
 import org.slf4j.Logger;
@@ -381,6 +382,41 @@ public HttpCompliance without(String name, Violation... violations)
         return new HttpCompliance(name, remainder);
     }
 
+    /**
+     * Assert that the specified Violation is allowed.
+     *
+     * @param violation the violation to check if allowed
+     * @param listener the listener to report to
+     * @param error the function to produce a Throwable if not allowed
+     * @param <T> the type of Throwable
+     * @throws T Throwable if not allowed
+     */
+    public <T extends Throwable> void assertAllowed(HttpCompliance.Violation violation, ComplianceViolation.Listener listener, Function<String, T> error) throws T
+    {
+        assertAllowed(violation, listener, violation.getDescription(), error);
+    }
+
+    /**
+     * Assert that the specified Violation is allowed.
+     *
+     * @param violation the violation to check if allowed
+     * @param listener the listener to report to
+     * @param detail the detail of violation
+     * @param error the function to produce a Throwable if not allowed
+     * @param <T> the type of Throwable
+     * @throws T Throwable if not allowed
+     */
+    public <T extends Throwable> void assertAllowed(HttpCompliance.Violation violation, ComplianceViolation.Listener listener, String detail, Function<String, T> error) throws T
+    {
+        boolean allowed = this.allows(violation);
+
+        // Always report violation to listeners
+        listener.onComplianceViolation(new ComplianceViolation.Event(this, violation, detail, allowed));
+
+        if (!allowed)
+            throw error.apply(violation.getDescription());
+    }
+
     @Override
     public String toString()
     {
@@ -401,8 +437,14 @@ private static Set<Violation> copyOf(Set<Violation> violations)
         return EnumSet.copyOf(violations);
     }
 
-    public static void checkHttpCompliance(MetaData.Request request, HttpCompliance mode,
-                             ComplianceViolation.Listener listener)
+    /**
+     * Check the provided Request against configured {@link HttpCompliance}.
+     *
+     * @param request the request to check
+     * @param listener the notification method for violations.  (Tip: use the Request specific Listener from the {@code HttpChannelState})
+     * @throws BadMessageException if there is a violation that wasn't allowed
+     */
+    public void check(MetaData.Request request, ComplianceViolation.Listener listener)
     {
         boolean seenContentLength = false;
         boolean seenTransferEncoding = false;
@@ -419,43 +461,43 @@ public static void checkHttpCompliance(MetaData.Request request, HttpCompliance
                 case CONTENT_LENGTH ->
                 {
                     if (seenContentLength)
-                        assertAllowed(Violation.MULTIPLE_CONTENT_LENGTHS, mode, listener);
+                        assertAllowed(Violation.MULTIPLE_CONTENT_LENGTHS, listener, BadMessageException::new);
                     String[] lengths = httpField.getValues();
                     if (lengths.length > 1)
-                        assertAllowed(Violation.MULTIPLE_CONTENT_LENGTHS, mode, listener);
+                        assertAllowed(Violation.MULTIPLE_CONTENT_LENGTHS, listener, BadMessageException::new);
                     if (seenTransferEncoding)
-                        assertAllowed(Violation.TRANSFER_ENCODING_WITH_CONTENT_LENGTH, mode, listener);
+                        assertAllowed(Violation.TRANSFER_ENCODING_WITH_CONTENT_LENGTH, listener, BadMessageException::new);
                     seenContentLength = true;
                 }
                 case TRANSFER_ENCODING ->
                 {
                     if (seenContentLength)
-                        assertAllowed(Violation.TRANSFER_ENCODING_WITH_CONTENT_LENGTH, mode, listener);
+                        assertAllowed(Violation.TRANSFER_ENCODING_WITH_CONTENT_LENGTH, listener, BadMessageException::new);
                     seenTransferEncoding = true;
                 }
                 case HOST ->
                 {
                     if (seenHostHeader)
-                        assertAllowed(Violation.DUPLICATE_HOST_HEADERS, mode, listener);
+                        assertAllowed(Violation.DUPLICATE_HOST_HEADERS, listener, BadMessageException::new);
                     String[] hostValues = httpField.getValues();
                     if (hostValues.length > 1)
-                        assertAllowed(Violation.DUPLICATE_HOST_HEADERS, mode, listener);
+                        assertAllowed(Violation.DUPLICATE_HOST_HEADERS, listener, BadMessageException::new);
                     for (String hostValue: hostValues)
                         if (StringUtil.isBlank(hostValue))
-                            assertAllowed(Violation.UNSAFE_HOST_HEADER, mode, listener);
+                            assertAllowed(Violation.UNSAFE_HOST_HEADER, listener, BadMessageException::new);
                     seenHostHeader = true;
                 }
             }
         }
     }
 
-    private static void assertAllowed(Violation violation, HttpCompliance mode, ComplianceViolation.Listener listener)
+    /**
+     * @deprecated use {@link #check(MetaData.Request, ComplianceViolation.Listener)} instead.
+     */
+    @Deprecated(forRemoval = true, since = "12.1.5")
+    public static void checkHttpCompliance(MetaData.Request request, HttpCompliance httpCompliance,
+                                           ComplianceViolation.Listener listener)
     {
-        if (mode.allows(violation))
-            listener.onComplianceViolation(new ComplianceViolation.Event(
-                mode, violation, violation.getDescription()
-            ));
-        else
-            throw new BadMessageException(violation.getDescription());
+        httpCompliance.check(request, listener);
     }
 }

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

@@ -27,7 +27,6 @@
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.concurrent.TimeUnit;
-import java.util.function.BiConsumer;
 import java.util.function.BiFunction;
 import java.util.function.BiPredicate;
 import java.util.function.Supplier;
@@ -150,9 +149,9 @@ static Mutable build(HttpFields fields, EnumSet<HttpHeader> removeFields)
         return new org.eclipse.jetty.http.MutableHttpFields(fields, removeFields);
     }
 
-    static Mutable build(HttpCompliance httpCompliance, BiConsumer<ComplianceViolation, String> notifyViolation)
+    static Mutable build(HttpCompliance httpCompliance, Supplier<ComplianceViolation.Listener> listenerSupplier)
     {
-        return new org.eclipse.jetty.http.MutableHttpFields.Compliant(httpCompliance, notifyViolation);
+        return new org.eclipse.jetty.http.MutableHttpFields.Compliant(httpCompliance, listenerSupplier);
     }
 
     /**

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

@@ -380,7 +380,10 @@ protected void reportComplianceViolation(Violation violation)
     protected void reportComplianceViolation(Violation violation, String reason)
     {
         if (_requestParser)
-            _requestHandler.onViolation(new ComplianceViolation.Event(_complianceMode, violation, reason));
+        {
+            boolean allowed = _complianceMode.allows(violation);
+            _requestHandler.onViolation(new ComplianceViolation.Event(_complianceMode, violation, reason, allowed));
+        }
     }
 
     protected String caseInsensitiveHeader(String orig, String normative)
@@ -2220,6 +2223,17 @@ default void parsedTrailer(HttpField field)
          */
         default void onViolation(ComplianceViolation.Event event)
         {
+            getComplianceViolationListener().onComplianceViolation(event);
+        }
+
+        /**
+         * Get the request specific {@link ComplianceViolation.Listener}
+         *
+         * @return the ComplianceViolation.Listener belonging to this HttpChannel.
+         */
+        default ComplianceViolation.Listener getComplianceViolationListener()
+        {
+            return ComplianceViolation.Listener.NOOP;
         }
 
         /**
@@ -2360,34 +2374,9 @@ public CompliantHttpField(HttpHeader header, String name, String value)
         protected QuotedCSV newQuotedCSV(boolean keepQuotes, String value)
         {
             if (getHeader() != null && HttpField.ETAG_HEADER.contains(this.getHeader()))
-                return new QuotedCSV.Etags(_complianceMode,
-                    (v, r) ->
-                    {
-                        try
-                        {
-                            _handler.onViolation(new ComplianceViolation.Event(_complianceMode, v, r));
-                        }
-                        catch (BadMessageException bme)
-                        {
-                            throw bme;
-                        }
-                        catch (Throwable t)
-                        {
-                            throw new BadMessageException(t.getMessage(), t);
-                        }
-                    }, value);
+                return new QuotedCSV.Etags(_complianceMode, _handler.getComplianceViolationListener(), value);
 
-            return new QuotedCSV(keepQuotes, value)
-            {
-                @Override
-                protected void onComplianceViolation(ComplianceViolation violation, String value)
-                {
-                    if (_complianceMode.allows(violation))
-                        _handler.onViolation(new ComplianceViolation.Event(_complianceMode, violation, value));
-                    else
-                        throw new BadMessageException(violation.toString());
-                }
-            };
+            return new QuotedCSV.Compliant(_complianceMode, _handler.getComplianceViolationListener(), keepQuotes, value);
         }
     }
 }

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

@@ -18,6 +18,7 @@
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Function;
 
 import static java.util.EnumSet.allOf;
 import static java.util.EnumSet.noneOf;
@@ -209,6 +210,29 @@ public Set<Violation> getAllowed()
         return violations;
     }
 
+    /**
+     * Assert that the specified Violation is allowed.
+     *
+     * @param violation the violation to check if allowed
+     * @param listener the listener to report to
+     * @param error the function to produce a Throwable if not allowed
+     * @param <T> the type of Throwable
+     * @throws T Throwable if not allowed
+     */
+    public <T extends Throwable> void assertAllowed(MultiPartCompliance.Violation violation, ComplianceViolation.Listener listener, String detail, Function<String, T> error) throws T
+    {
+        boolean allowed = this.allows(violation);
+
+        // Always report violation to listeners
+        if (listener != null)
+            listener.onComplianceViolation(new ComplianceViolation.Event(this, violation, detail, allowed));
+
+        if (!allowed)
+        {
+            throw error.apply(detail);
+        }
+    }
+
     @Override
     public String toString()
     {