Issue #11253 - Alternate approach with split ComplianceViolation.List…

…ener

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

@@ -13,8 +13,14 @@
 
 package org.eclipse.jetty.http;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Set;
 
+import org.eclipse.jetty.util.Attributes;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 /**
  * A Compliance Violation represents a requirement of an RFC, specification or Jetty implementation
  * that may be allowed to be violated if it is included in a {@link ComplianceViolation.Mode}.
@@ -90,16 +96,159 @@ public String toString()
      */
     interface Listener
     {
+        /**
+         * A new Request has begun.
+         *
+         * @param request the request attributes, or null if the Request does not exist yet (eg: during parsing of HTTP/1.1 headers, before request is created)
+         */
+        default void onRequestBegin(Attributes request)
+        {
+        }
+
+        /**
+         * A Request has ended.
+         *
+         * @param request the request attribtues, or null if Request does not exist yet (eg: during handling of a {@link BadMessageException})
+         */
+        default void onRequestEnd(Attributes request)
+        {
+        }
+
         /**
          * The compliance violation event.
+         *
+         * @param event the compliance violation event
          */
         default void onComplianceViolation(Event event)
         {
+            onComplianceViolation(event.mode, event.violation, event.details);
         }
 
+        /**
+         * The compliance violation 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(since = "12.0.5", forRemoval = true)
         default void onComplianceViolation(Mode mode, ComplianceViolation violation, String details)
         {
-            onComplianceViolation(new Event(mode, violation, details));
+        }
+    }
+
+    /**
+     * A Listener that represents multiple user {@link ComplianceViolation.Listener} instances
+     */
+    class ListenerCollection implements Listener
+    {
+        private static final Logger LOG = LoggerFactory.getLogger(ListenerCollection.class);
+        private final List<ComplianceViolation.Listener> userListeners;
+
+        /**
+         * Construct a new ComplianceViolations that will notify user listeners.
+         * @param userListeners the user listeners to notify, null or empty is allowed.
+         */
+        public ListenerCollection(List<ComplianceViolation.Listener> userListeners)
+        {
+            this.userListeners = userListeners;
+        }
+
+        /**
+         * Get a specific ComplianceViolation.Listener from collected user listeners
+         * @param clazz the class to look for
+         * @return the instance of the class in the user listeners
+         * @param <T> the type of class
+         */
+        public <T> T getUserListener(Class<T> clazz)
+        {
+            for (ComplianceViolation.Listener listener : userListeners)
+            {
+                if (clazz.isInstance(listener))
+                    return clazz.cast(listener);
+            }
+            return null;
+        }
+
+        @Override
+        public void onComplianceViolation(ComplianceViolation.Event event)
+        {
+            assert event != null;
+            notifyUserListeners(event);
+        }
+
+        private void notifyUserListeners(ComplianceViolation.Event event)
+        {
+            if (userListeners == null || userListeners.isEmpty())
+                return;
+
+            for (ComplianceViolation.Listener listener : userListeners)
+            {
+                try
+                {
+                    listener.onComplianceViolation(event);
+                }
+                catch (Exception e)
+                {
+                    LOG.warn("Unable to notify ComplianceViolation.Listener implementation at {} of event {}", listener, event, e);
+                }
+            }
+        }
+    }
+
+    interface ListenerFactory
+    {
+        Listener newComplianceViolationListener();
+    }
+
+    public class LoggingListenerFactory implements ListenerFactory, Listener
+    {
+        private static final Logger LOG = LoggerFactory.getLogger(ComplianceViolation.class);
+
+        @Override
+        public void onComplianceViolation(Event event)
+        {
+            if (LOG.isDebugEnabled())
+                LOG.debug(event.toString());
+        }
+
+        @Override
+        public Listener newComplianceViolationListener()
+        {
+            return this;
+        }
+    }
+
+    public class CapturingListenerFactory implements ListenerFactory
+    {
+        @Override
+        public Listener newComplianceViolationListener()
+        {
+            return new CapturingListener();
+        }
+    }
+
+    public class CapturingListener implements Listener
+    {
+        private List<Event> events = new ArrayList<>();
+
+        @Override
+        public void onRequestBegin(Attributes request)
+        {
+            if (request != null)
+                request.setAttribute(ComplianceViolation.class.getPackageName() + ".complianceViolations", events);
+        }
+
+        @Override
+        public void onRequestEnd(Attributes request)
+        {
+        }
+
+        @Override
+        public void onComplianceViolation(Event event)
+        {
+            events.add(event);
         }
     }
 }

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

@@ -229,7 +229,6 @@ public enum State
     private final HttpHandler _handler;
     private final RequestHandler _requestHandler;
     private final ResponseHandler _responseHandler;
-    private final ComplianceViolation.Listener _complianceListener;
     private final int _maxHeaderBytes;
     private final HttpCompliance _complianceMode;
     private final Utf8StringBuilder _uri = new Utf8StringBuilder(INITIAL_URI_LENGTH);
@@ -294,27 +293,21 @@ public HttpParser(RequestHandler handler, HttpCompliance compliance)
 
     public HttpParser(RequestHandler handler, int maxHeaderBytes, HttpCompliance compliance)
     {
-        this(handler, null, maxHeaderBytes, compliance == null ? compliance() : compliance, null);
-    }
-
-    public HttpParser(RequestHandler handler, int maxHeaderBytes, HttpCompliance compliance, ComplianceViolation.Listener complianceListener)
-    {
-        this(handler, null, maxHeaderBytes, compliance == null ? compliance() : compliance, complianceListener);
+        this(handler, null, maxHeaderBytes, compliance == null ? compliance() : compliance);
     }
 
     public HttpParser(ResponseHandler handler, int maxHeaderBytes, HttpCompliance compliance)
     {
-        this(null, handler, maxHeaderBytes, compliance == null ? compliance() : compliance, null);
+        this(null, handler, maxHeaderBytes, compliance == null ? compliance() : compliance);
     }
 
-    private HttpParser(RequestHandler requestHandler, ResponseHandler responseHandler, int maxHeaderBytes, HttpCompliance compliance, ComplianceViolation.Listener complianceListener)
+    private HttpParser(RequestHandler requestHandler, ResponseHandler responseHandler, int maxHeaderBytes, HttpCompliance compliance)
     {
         _handler = requestHandler != null ? requestHandler : responseHandler;
         _requestHandler = requestHandler;
         _responseHandler = responseHandler;
         _maxHeaderBytes = maxHeaderBytes;
         _complianceMode = compliance;
-        _complianceListener = complianceListener;
     }
 
     public long getBeginNanoTime()
@@ -362,8 +355,8 @@ protected void reportComplianceViolation(Violation violation)
 
     protected void reportComplianceViolation(Violation violation, String reason)
     {
-        if (_complianceListener != null)
-            _complianceListener.onComplianceViolation(_complianceMode, violation, reason);
+        if (_requestHandler != null)
+            _requestHandler.onViolation(new ComplianceViolation.Event(_complianceMode, violation, reason));
     }
 
     protected String caseInsensitiveHeader(String orig, String normative)
@@ -2025,6 +2018,14 @@ default void parsedTrailer(HttpField field)
          */
         void earlyEOF();
 
+        /**
+         * Called to indicate that a {@link ComplianceViolation} has occurred.
+         * @param event the Compliance Violation event
+         */
+        default void onViolation(ComplianceViolation.Event event)
+        {
+        }
+
         /**
          * Called to signal that a bad HTTP message has been received.
          *

jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java

@@ -18,6 +18,7 @@
 import java.util.function.BiConsumer;
 import java.util.function.Supplier;
 
+import org.eclipse.jetty.http.ComplianceViolation;
 import org.eclipse.jetty.http.HttpException;
 import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpMethod;
@@ -38,6 +39,8 @@
 import org.eclipse.jetty.io.EndPoint;
 import org.eclipse.jetty.server.HttpChannel;
 import org.eclipse.jetty.server.HttpStream;
+import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.TunnelSupport;
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.Callback;
@@ -84,6 +87,13 @@ public Runnable onRequest(HeadersFrame frame)
 
             Runnable handler = _httpChannel.onRequest(_requestMetaData);
 
+            ComplianceViolation.Listener listener = Server.newComplianceViolationListener(_httpChannel.getConnectionMetaData().getConnector());
+            Request request = _httpChannel.getRequest();
+            request.setAttribute(ComplianceViolation.Listener.class.getName(), listener);
+            listener.onRequestBegin(request);
+            // Note UriCompliance is done by HandlerInvoker
+            // TODO: perform HttpCompliance violation checks?
+
             if (frame.isEndStream())
             {
                 try (AutoLock ignored = lock.lock())

jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2CServerTest.java

@@ -304,7 +304,7 @@ public void testHTTP20DirectWithoutH2C() throws Exception
             @Override
             public Connection newConnection(Connector connector, EndPoint endPoint)
             {
-                HttpConnection connection = new HttpConnection(getHttpConfiguration(), connector, endPoint, isRecordHttpComplianceViolations())
+                HttpConnection connection = new HttpConnection(getHttpConfiguration(), connector, endPoint)
                 {
                     @Override
                     public void onFillable()