diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ComplianceViolation.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ComplianceViolation.java index 664053f66465..d4e46e55922a 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ComplianceViolation.java +++ b/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 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 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 the type of class + */ + public T getUserListener(Class 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 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); } } } diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ComplianceViolations.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ComplianceViolations.java deleted file mode 100644 index 58c2ff2727d7..000000000000 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ComplianceViolations.java +++ /dev/null @@ -1,111 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.http; - -import java.util.ArrayList; -import java.util.List; - -import org.eclipse.jetty.util.Attributes; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * A representation of ComplianceViolation.Listeners events that have occurred. - */ -public class ComplianceViolations implements ComplianceViolation.Listener -{ - public static final String VIOLATIONS_ATTR = ComplianceViolations.class.getName(); - private static final Logger LOG = LoggerFactory.getLogger(ComplianceViolations.class); - private final List userListeners; - private List events; - private Attributes attributes; - - /** - * Construct a new ComplianceViolations that will notify user listeners. - * @param userListeners the user listeners to notify, null or empty is allowed. - */ - public ComplianceViolations(List userListeners) - { - this.userListeners = userListeners; - } - - @Override - public void onComplianceViolation(ComplianceViolation.Event event) - { - assert event != null; - - notifyUserListeners(event); - addEvent(event); - if (LOG.isDebugEnabled()) - LOG.debug(event.toString()); - } - - /** - * Clear out the tracked {@link Attributes} object and events; - */ - public void recycle() - { - this.events = null; - this.attributes = null; - } - - /** - * Start tracking an {@link Attributes} implementation to store the events in. - * @param attributes the attributes object to store the event list in. Stored in key name {@link #VIOLATIONS_ATTR} - */ - public void setAttribute(Attributes attributes) - { - this.attributes = attributes; - this.attributes.setAttribute(VIOLATIONS_ATTR, events); - } - - /** - * The events that have been recorded. - * - * @return the list of Events recorded. - */ - public List getEvents() - { - return events; - } - - private void addEvent(ComplianceViolation.Event event) - { - if (this.events == null) - { - this.events = new ArrayList<>(); - if (this.attributes != null) - setAttribute(this.attributes); - } - this.events.add(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); - } - } - } -} diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java index aa9674c753ea..4180c331c506 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java +++ b/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. * diff --git a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java b/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java index e237ad9b6fdb..a45a9403a931 100644 --- a/jetty-core/jetty-http2/jetty-http2-server/src/main/java/org/eclipse/jetty/http2/server/internal/HttpStreamOverHTTP2.java +++ b/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()) diff --git a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2CServerTest.java b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2CServerTest.java index ffbc90e291df..a2c657488894 100644 --- a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/HTTP2CServerTest.java +++ b/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() diff --git a/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpStreamOverHTTP3.java b/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpStreamOverHTTP3.java index 74f7af1422e2..366bfece5062 100644 --- a/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpStreamOverHTTP3.java +++ b/jetty-core/jetty-http3/jetty-http3-server/src/main/java/org/eclipse/jetty/http3/server/internal/HttpStreamOverHTTP3.java @@ -19,6 +19,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; @@ -33,6 +34,8 @@ import org.eclipse.jetty.io.Content; 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.util.BufferUtil; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.util.thread.AutoLock; @@ -74,6 +77,13 @@ public Runnable onRequest(HeadersFrame frame) Runnable handler = httpChannel.onRequest(requestMetaData); + ComplianceViolation.Listener listener = Server.newComplianceViolationListener(this.httpChannel.getConnectionMetaData().getConnector()); + Request request = this.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.isLast()) { try (AutoLock ignored = lock.lock()) diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java index 55167f4ae631..070d96c08809 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnectionFactory.java @@ -15,6 +15,7 @@ import java.util.Objects; +import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.HttpVersion; import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.io.EndPoint; @@ -27,7 +28,7 @@ * {@link HttpConnection}s are configured by a {@link HttpConfiguration} instance that is either created by * default or passed in to the constructor. */ -public class HttpConnectionFactory extends AbstractConnectionFactory implements HttpConfiguration.ConnectionFactory +public class HttpConnectionFactory extends AbstractConnectionFactory implements HttpConfiguration.ConnectionFactory, ConnectionFactory.Configuring { private final HttpConfiguration _config; private boolean _recordHttpComplianceViolations; @@ -54,6 +55,13 @@ public HttpConfiguration getHttpConfiguration() return _config; } + @Override + public void configure(Connector connector) + { + if (isRecordHttpComplianceViolations()) + addBean(new ComplianceViolation.LoggingListenerFactory()); + } + public boolean isRecordHttpComplianceViolations() { return _recordHttpComplianceViolations; @@ -87,7 +95,7 @@ public void setUseOutputDirectByteBuffers(boolean useOutputDirectByteBuffers) @Override public Connection newConnection(Connector connector, EndPoint endPoint) { - HttpConnection connection = new HttpConnection(_config, connector, endPoint, isRecordHttpComplianceViolations()); + HttpConnection connection = new HttpConnection(_config, connector, endPoint); connection.setUseInputDirectByteBuffers(isUseInputDirectByteBuffers()); connection.setUseOutputDirectByteBuffers(isUseOutputDirectByteBuffers()); return configure(connection, connector, endPoint); diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java index 90d475ad4b5a..724af9850c4e 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Request.java @@ -34,7 +34,7 @@ import java.util.function.Function; import java.util.function.Predicate; -import org.eclipse.jetty.http.ComplianceViolations; +import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.CookieCache; import org.eclipse.jetty.http.HttpCookie; import org.eclipse.jetty.http.HttpFields; @@ -590,8 +590,8 @@ static List getCookies(Request request) CookieCache cookieCache = (CookieCache)request.getComponents().getCache().getAttribute(CACHE_ATTRIBUTE); if (cookieCache == null) { - ComplianceViolations complianceViolations = request.getConnectionMetaData().getConnector().getBean(ComplianceViolations.class); - cookieCache = new CookieCache(request.getConnectionMetaData().getHttpConfiguration().getRequestCookieCompliance(), complianceViolations); + ComplianceViolation.Listener complianceViolationListener = (ComplianceViolation.Listener)request.getAttribute(ComplianceViolation.Listener.class.getName()); + cookieCache = new CookieCache(request.getConnectionMetaData().getHttpConfiguration().getRequestCookieCompliance(), complianceViolationListener); request.getComponents().getCache().setAttribute(CACHE_ATTRIBUTE, cookieCache); } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index e7ce14882dc9..3bca1397e00f 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -20,8 +20,8 @@ import java.util.concurrent.TimeoutException; import java.util.function.Supplier; +import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.ComplianceViolationException; -import org.eclipse.jetty.http.ComplianceViolations; import org.eclipse.jetty.http.CookieCompliance; import org.eclipse.jetty.http.HttpCookie; import org.eclipse.jetty.http.HttpException; @@ -386,8 +386,8 @@ static void addCookie(Response response, HttpCookie cookie) } catch (ComplianceViolationException e) { - ComplianceViolations complianceViolations = request.getConnectionMetaData().getConnector().getBean(ComplianceViolations.class); - complianceViolations.onComplianceViolation(e.getEvent()); + ComplianceViolation.Listener complianceViolationListener = (ComplianceViolation.Listener)request.getAttribute(ComplianceViolation.Listener.class.getName()); + complianceViolationListener.onComplianceViolation(e.getEvent()); throw e; } @@ -421,8 +421,8 @@ static void putCookie(Response response, HttpCookie cookie) } catch (ComplianceViolationException e) { - ComplianceViolations complianceViolations = request.getConnectionMetaData().getConnector().getBean(ComplianceViolations.class); - complianceViolations.onComplianceViolation(e.getEvent()); + ComplianceViolation.Listener complianceViolationListener = (ComplianceViolation.Listener)request.getAttribute(ComplianceViolation.Listener.class.getName()); + complianceViolationListener.onComplianceViolation(e.getEvent()); throw e; } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java index bed6d6fbcffa..793be20afc94 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/Server.java @@ -29,6 +29,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeUnit; +import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.DateGenerator; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpHeader; @@ -151,6 +152,44 @@ public Server(@Name("threadPool") ThreadPool threadPool, @Name("scheduler") Sche addBean(FileSystemPool.INSTANCE, false); } + /** + * Get a new ComplianceViolation.Listener suitable for given Connector. + * + * @param connector the connector to base the ComplianceViolation.Listener off of. + * @return the ComplianceViolation.Listener implementation, or null if {@link HttpConnectionFactory#isRecordHttpComplianceViolations()} is false, + * or there are no ComplianceViolation.Listener implementations registered. + */ + public static ComplianceViolation.Listener newComplianceViolationListener(Connector connector) + { + HttpConnectionFactory httpConnectionFactory = connector.getConnectionFactory(HttpConnectionFactory.class); + if (httpConnectionFactory == null) + return null; + + // Is this connector recording compliance violations? + if (!httpConnectionFactory.isRecordHttpComplianceViolations()) + return null; + + // Only add the ComplianceViolations instance if the recording of Compliance Violations is enabled + // This also means that any user provided ComplianceViolation.Listener beans will only be + // used when the configuration on the HttpConnectionFactory allows then to be used. + + // Look for optional user provided ComplianceViolation.ListenerSupplier + List userListeners = new ArrayList<>(); + for (ComplianceViolation.ListenerFactory listenerFactory: connector.getBeans(ComplianceViolation.ListenerFactory.class)) + userListeners.add(listenerFactory.newComplianceViolationListener()); + for (ComplianceViolation.ListenerFactory listenerFactory: connector.getServer().getBeans(ComplianceViolation.ListenerFactory.class)) + userListeners.add(listenerFactory.newComplianceViolationListener()); + + // No listeners? then we are done + if (userListeners.isEmpty()) + return null; + // Only 1 listener, just return it. + if (userListeners.size() == 1) + return userListeners.get(0); + // More than 1, establish ComplianceViolations collection + return new ComplianceViolation.ListenerCollection(userListeners); + } + public Handler getDefaultHandler() { return _defaultHandler; diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java index fc9a09ea1d4c..f1016530bf3f 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpChannelState.java @@ -16,9 +16,7 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.nio.channels.WritePendingException; -import java.util.ArrayList; import java.util.HashMap; -import java.util.List; import java.util.Objects; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeoutException; @@ -30,7 +28,6 @@ import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.ComplianceViolation; -import org.eclipse.jetty.http.ComplianceViolations; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpHeader; @@ -47,12 +44,10 @@ import org.eclipse.jetty.io.Content; import org.eclipse.jetty.server.Components; import org.eclipse.jetty.server.ConnectionMetaData; -import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.Context; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.HttpConfiguration; -import org.eclipse.jetty.server.HttpConnectionFactory; import org.eclipse.jetty.server.HttpStream; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.RequestLog; @@ -108,7 +103,6 @@ private enum StreamSendState private final SerializedInvoker _readInvoker; private final SerializedInvoker _writeInvoker; private final ResponseHttpFields _responseHeaders = new ResponseHttpFields(); - private final ComplianceViolations _complianceViolations; private Thread _handling; private boolean _handled; private StreamSendState _streamSendState = StreamSendState.SENDING; @@ -132,46 +126,6 @@ public HttpChannelState(ConnectionMetaData connectionMetaData) // The SerializedInvoker is used to prevent infinite recursion of callbacks calling methods calling callbacks etc. _readInvoker = new HttpChannelSerializedInvoker(); _writeInvoker = new HttpChannelSerializedInvoker(); - _complianceViolations = newComplianceViolations(); - } - - /** - * @return a new ComplianceViolations if configuration enables it. - */ - private ComplianceViolations newComplianceViolations() - { - Connector connector = getConnectionMetaData().getConnector(); - if (connector == null) - return null; - - HttpConnectionFactory httpConnectionFactory = connector.getConnectionFactory(HttpConnectionFactory.class); - if (httpConnectionFactory != null) - { - // Is this connector recording compliance violations? - if (httpConnectionFactory.isRecordHttpComplianceViolations()) - { - // Only add the ComplianceViolations instance if the recording of Compliance Violations is enabled - // This also means that any user provided ComplianceViolation.Listener beans will only be - // used when the configuration on the HttpConnectionFactory allows then to be used. - - // Look for optional user provided ComplianceViolation.Listeners - List userListeners = new ArrayList<>(); - userListeners.addAll(connector.getBeans(ComplianceViolation.Listener.class)); - userListeners.addAll(connector.getServer().getBeans(ComplianceViolation.Listener.class)); - - // Establish ComplianceViolations Tracker - ComplianceViolations violations = new ComplianceViolations(userListeners); - - // Store tracker (to be accessed elsewhere) - // If this bean is not present on the connector, then it is safe to assume that the - // HttpConnectionFactory is not allowing the recording of compliance violations. - connector.addBean(violations); - - return violations; - } - } - - return null; } @Override @@ -195,8 +149,6 @@ public void recycle() _request = null; _response = null; _oldIdleTimeout = 0; - if (_complianceViolations != null) - _complianceViolations.recycle(); // Break the link between channel and stream. _stream = null; _committedContentLength = -1; @@ -286,15 +238,6 @@ public Runnable onRequest(MetaData.Request request) if (LOG.isDebugEnabled()) LOG.debug("onRequest {} {}", request, this); - // Handle UriCompliance Violations - if (request.getHttpURI().hasViolations()) - { - UriCompliance compliance = getHttpConfiguration().getUriCompliance(); - String badMessage = UriCompliance.checkUriCompliance(compliance, request.getHttpURI(), _complianceViolations); - if (badMessage != null) - throw new BadMessageException(badMessage); - } - try (AutoLock ignored = _lock.lock()) { if (_stream == null) @@ -319,8 +262,6 @@ public Runnable onRequest(MetaData.Request request) if (idleTO >= 0 && _oldIdleTimeout != idleTO) _stream.setIdleTimeout(idleTO); - _complianceViolations.setAttribute(_request); - // This is deliberately not serialized to allow a handler to block. return _handlerInvoker; } @@ -632,7 +573,8 @@ public void run() HttpURI uri = request.getHttpURI(); if (uri.hasViolations()) { - String badMessage = UriCompliance.checkUriCompliance(getConnectionMetaData().getHttpConfiguration().getUriCompliance(), uri, _complianceViolations); + ComplianceViolation.Listener complianceViolationListener = (ComplianceViolation.Listener)request.getAttribute(ComplianceViolation.Listener.class.getName()); + String badMessage = UriCompliance.checkUriCompliance(getConnectionMetaData().getHttpConfiguration().getUriCompliance(), uri, complianceViolationListener); if (badMessage != null) throw new BadMessageException(badMessage); } diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java index 1ba352374beb..f6b6b1375c35 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java @@ -27,7 +27,7 @@ import java.util.concurrent.atomic.LongAdder; import org.eclipse.jetty.http.BadMessageException; -import org.eclipse.jetty.http.ComplianceViolations; +import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.HostPortHttpField; import org.eclipse.jetty.http.HttpCompliance; import org.eclipse.jetty.http.HttpException; @@ -99,7 +99,6 @@ public class HttpConnection extends AbstractMetaDataConnection implements Runnab private final Lazy _attributes = new Lazy(); private final DemandContentCallback _demandContentCallback = new DemandContentCallback(); private final SendCallback _sendCallback = new SendCallback(); - private final boolean _recordHttpComplianceViolations; private final LongAdder bytesIn = new LongAdder(); private final LongAdder bytesOut = new LongAdder(); private final AtomicBoolean _handling = new AtomicBoolean(false); @@ -132,7 +131,16 @@ protected static HttpConnection setCurrentConnection(HttpConnection connection) return last; } + /** + * @deprecated use {@link #HttpConnection(HttpConfiguration, Connector, EndPoint)} instead. Will be removed in Jetty 12.1.0 + */ + @Deprecated(since = "12.0.5", forRemoval = true) public HttpConnection(HttpConfiguration configuration, Connector connector, EndPoint endPoint, boolean recordComplianceViolations) + { + this(configuration, connector, endPoint); + } + + public HttpConnection(HttpConfiguration configuration, Connector connector, EndPoint endPoint) { super(connector, configuration, endPoint); _id = __connectionIdGenerator.getAndIncrement(); @@ -141,7 +149,6 @@ public HttpConnection(HttpConfiguration configuration, Connector connector, EndP _httpChannel = newHttpChannel(connector.getServer(), configuration); _requestHandler = newRequestHandler(); _parser = newHttpParser(configuration.getHttpCompliance()); - _recordHttpComplianceViolations = recordComplianceViolations; if (LOG.isDebugEnabled()) LOG.debug("New HTTP Connection {}", this); } @@ -152,9 +159,13 @@ public InvocationType getInvocationType() return getServer().getInvocationType(); } + /** + * @deprecated No replacement, no longer used within {@link HttpConnection}, will be removed in Jetty 12.1.0 + */ + @Deprecated(since = "12.0.5", forRemoval = true) public boolean isRecordHttpComplianceViolations() { - return _recordHttpComplianceViolations; + return false; } protected HttpGenerator newHttpGenerator() @@ -164,8 +175,7 @@ protected HttpGenerator newHttpGenerator() protected HttpParser newHttpParser(HttpCompliance compliance) { - ComplianceViolations complianceViolations = getHttpChannel().getConnectionMetaData().getConnector().getBean(ComplianceViolations.class); - HttpParser parser = new HttpParser(_requestHandler, getHttpConfiguration().getRequestHeaderSize(), compliance, complianceViolations); + HttpParser parser = new HttpParser(_requestHandler, getHttpConfiguration().getRequestHeaderSize(), compliance); parser.setHeaderCacheSize(getHttpConfiguration().getHeaderCacheSize()); parser.setHeaderCacheCaseSensitive(getHttpConfiguration().isHeaderCacheCaseSensitive()); return parser; @@ -183,7 +193,7 @@ protected HttpStreamOverHTTP1 newHttpStream(String method, String uri, HttpVersi protected RequestHandler newRequestHandler() { - return new RequestHandler(); + return new RequestHandler(getConnector()); } public Server getServer() @@ -912,6 +922,12 @@ public String toString() protected class RequestHandler implements HttpParser.RequestHandler { private Throwable _failure; + private ComplianceViolation.Listener _listener; + + public RequestHandler(Connector connector) + { + _listener = Server.newComplianceViolationListener(connector); + } @Override public void startRequest(String method, String uri, HttpVersion version) @@ -959,6 +975,13 @@ public boolean contentComplete() return false; } + @Override + public void onViolation(ComplianceViolation.Event event) + { + if (_listener != null) + _listener.onComplianceViolation(event); + } + @Override public void parsedTrailer(HttpField field) { @@ -977,6 +1000,8 @@ public boolean messageComplete() stream._chunk = new Trailers(_trailers.asImmutable()); else stream._chunk = Content.Chunk.EOF; + + _listener.onRequestEnd(getHttpChannel().getRequest()); return false; } @@ -986,6 +1011,8 @@ public void badMessage(HttpException failure) if (LOG.isDebugEnabled()) LOG.debug("badMessage {} {}", HttpConnection.this, failure); + _listener.onRequestEnd(getHttpChannel().getRequest()); + _failure = (Throwable)failure; _generator.setPersistent(false); @@ -1154,13 +1181,12 @@ public Runnable headerComplete() if (_uri.hasViolations()) { compliance = getHttpConfiguration().getUriCompliance(); - ComplianceViolations complianceViolations = getHttpChannel().getConnectionMetaData().getConnector().getBean(ComplianceViolations.class); - String badMessage = UriCompliance.checkUriCompliance(compliance, _uri, complianceViolations); + String badMessage = UriCompliance.checkUriCompliance(compliance, _uri, _requestHandler._listener); if (badMessage != null) throw new BadMessageException(badMessage); } - // Check host field matches the authority in the any absolute URI or is not blank + // Check host field matches the authority in the absolute URI or is not blank if (_hostField != null) { if (_uri.isAbsolute()) @@ -1170,9 +1196,8 @@ public Runnable headerComplete() HttpCompliance httpCompliance = getHttpConfiguration().getHttpCompliance(); if (httpCompliance.allows(MISMATCHED_AUTHORITY)) { - ComplianceViolations complianceViolations = getHttpChannel().getConnectionMetaData().getConnector().getBean(ComplianceViolations.class); - if (complianceViolations != null) - complianceViolations.onComplianceViolation(httpCompliance, MISMATCHED_AUTHORITY, _uri.asString()); + if (_requestHandler._listener != null) + _requestHandler._listener.onComplianceViolation(httpCompliance, MISMATCHED_AUTHORITY, _uri.asString()); } else throw new BadMessageException("Authority!=Host"); @@ -1217,6 +1242,10 @@ public boolean is100ContinueExpected() Runnable handle = _httpChannel.onRequest(_request); ++_requests; + Request request = _httpChannel.getRequest(); + request.setAttribute(ComplianceViolation.Listener.class.getName(), _requestHandler._listener); + _requestHandler._listener.onRequestBegin(request); + if (_complianceViolations != null && !_complianceViolations.isEmpty()) { _httpChannel.getRequest().setAttribute(HttpCompliance.VIOLATIONS_ATTR, _complianceViolations); diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/DelayedServerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/DelayedServerTest.java index 0047a14fc050..3a1ec920d762 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/DelayedServerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/DelayedServerTest.java @@ -48,7 +48,7 @@ private static class DelayedHttpConnection extends HttpConnection { public DelayedHttpConnection(HttpConfiguration config, Connector connector, EndPoint endPoint) { - super(config, connector, endPoint, false); + super(config, connector, endPoint); } @Override diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ExtendedServerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ExtendedServerTest.java index b5517412145a..7d9aa6812f80 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ExtendedServerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ExtendedServerTest.java @@ -91,7 +91,7 @@ private static class ExtendedHttpConnection extends HttpConnection { public ExtendedHttpConnection(HttpConfiguration config, Connector connector, EndPoint endPoint) { - super(config, connector, endPoint, false); + super(config, connector, endPoint); } @Override diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java index a2e154d19d97..2b8b97036a4d 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ForwardedRequestCustomizerTest.java @@ -85,7 +85,7 @@ public void init() 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 SocketAddress getLocalSocketAddress() diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseCompleteTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseCompleteTest.java index e6e1d003aa77..482cfe2a1e5c 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseCompleteTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseCompleteTest.java @@ -129,7 +129,7 @@ public void testHandleCallbackCompleting(boolean throwFromHandler) throws Except @Override public Connection newConnection(Connector connector, EndPoint endPoint) { - HttpConnection connection = new HttpConnection(getHttpConfiguration(), connector, endPoint, isRecordHttpComplianceViolations()) + HttpConnection connection = new HttpConnection(getHttpConfiguration(), connector, endPoint) { @Override protected HttpStreamOverHTTP1 newHttpStream(String method, String uri, HttpVersion version) diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerHttpCookieTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerHttpCookieTest.java index cec91cfd72bf..a1a163e9bebb 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerHttpCookieTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerHttpCookieTest.java @@ -16,7 +16,7 @@ import java.util.List; import java.util.stream.Stream; -import org.eclipse.jetty.http.ComplianceViolations; +import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.CookieCompliance; import org.eclipse.jetty.http.CookieParser; import org.eclipse.jetty.http.HttpCookie; @@ -64,7 +64,7 @@ public boolean handle(Request request, Response response, Callback callback) thr Fields.Field setCookie = parameters.get("SetCookie"); if (setCookie != null) { - ComplianceViolations complianceViolations = request.getConnectionMetaData().getConnector().getBean(ComplianceViolations.class); + ComplianceViolation.Listener complianceViolationListener = (ComplianceViolation.Listener)request.getAttribute(ComplianceViolation.Listener.class.getName()); CookieParser parser = CookieParser.newParser(new CookieParser.Handler() { @Override @@ -72,7 +72,7 @@ public void addCookie(String name, String value, int version, String domain, Str { Response.addCookie(response, HttpCookie.build(name, value, version).domain(domain).path(path).comment(comment).build()); } - }, RFC2965, complianceViolations); + }, RFC2965, complianceViolationListener); parser.parseField(setCookie.getValue()); } diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerTest.java index 60f5c817b331..a29615be93ef 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/ServerTest.java @@ -63,7 +63,7 @@ public void prepare() 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 protected HttpChannel newHttpChannel(Server server, HttpConfiguration configuration) diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/SlowClientWithPipelinedRequestTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/SlowClientWithPipelinedRequestTest.java index 9defb422283a..12069ad7e157 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/SlowClientWithPipelinedRequestTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/SlowClientWithPipelinedRequestTest.java @@ -47,7 +47,7 @@ public void startServer(Handler handler) throws Exception @Override public Connection newConnection(Connector connector, EndPoint endPoint) { - return configure(new HttpConnection(getHttpConfiguration(), connector, endPoint, isRecordHttpComplianceViolations()) + return configure(new HttpConnection(getHttpConfiguration(), connector, endPoint) { @Override public void onFillable() diff --git a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/StopTest.java b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/StopTest.java index 8406d6f1a72f..48bcf7e9a61b 100644 --- a/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/StopTest.java +++ b/jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/StopTest.java @@ -134,7 +134,7 @@ public void testSlowClose(long stopTimeout, long closeWait, Matcher stopTi public Connection newConnection(Connector con, EndPoint endPoint) { // Slow closing connection - HttpConnection conn = new HttpConnection(getHttpConfiguration(), con, endPoint, isRecordHttpComplianceViolations()) + HttpConnection conn = new HttpConnection(getHttpConfiguration(), con, endPoint) { @Override public void onClose(Throwable cause) diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Dispatcher.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Dispatcher.java index 2c140f89c56b..d289bc1f0878 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Dispatcher.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Dispatcher.java @@ -26,7 +26,7 @@ import jakarta.servlet.http.HttpServletMapping; import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletResponse; -import org.eclipse.jetty.http.ComplianceViolations; +import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.HttpException; import org.eclipse.jetty.http.HttpURI; import org.eclipse.jetty.http.UriCompliance; @@ -247,8 +247,8 @@ private static void checkUriViolations(HttpURI uri, Request baseRequest) { HttpChannel channel = baseRequest.getHttpChannel(); UriCompliance compliance = channel == null || channel.getHttpConfiguration() == null ? null : channel.getHttpConfiguration().getUriCompliance(); - ComplianceViolations complianceViolations = channel.getConnector().getBean(ComplianceViolations.class); - String illegalState = UriCompliance.checkUriCompliance(compliance, uri, complianceViolations); + ComplianceViolation.Listener complianceViolationListener = (ComplianceViolation.Listener)baseRequest.getAttribute(ComplianceViolation.Listener.class.getName()); + String illegalState = UriCompliance.checkUriCompliance(compliance, uri, complianceViolationListener); if (illegalState != null) throw new IllegalStateException(illegalState); } diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java index b23d46a143c1..b862bb7aa0f6 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/Request.java @@ -65,7 +65,6 @@ import jakarta.servlet.http.PushBuilder; import org.eclipse.jetty.http.BadMessageException; import org.eclipse.jetty.http.ComplianceViolation; -import org.eclipse.jetty.http.ComplianceViolations; import org.eclipse.jetty.http.CookieCache; import org.eclipse.jetty.http.CookieCompliance; import org.eclipse.jetty.http.HttpCookie; @@ -83,7 +82,6 @@ import org.eclipse.jetty.io.Connection; import org.eclipse.jetty.io.RuntimeIOException; import org.eclipse.jetty.security.UserIdentity; -import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.HttpCookieUtils; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.Session; @@ -1988,18 +1986,14 @@ else if (getCharacterEncoding() != null) private void reportComplianceViolations() { - Connector connector = getHttpChannel().getConnector(); - if (connector == null) - return; // not using a connector? - - ComplianceViolations complianceViolations = connector.getBean(ComplianceViolations.class); - if (complianceViolations == null) + ComplianceViolation.Listener complianceViolationListener = (ComplianceViolation.Listener)getAttribute(ComplianceViolation.Listener.class.getName()); + if (complianceViolationListener == null) return; // no violation reporting active List nonComplianceWarnings = _multiParts.getNonComplianceWarnings(); for (MultiPartFormInputStream.NonCompliance nc : nonComplianceWarnings) { - complianceViolations.onComplianceViolation(nc.mode(), nc.violation(), nc.detail()); + complianceViolationListener.onComplianceViolation(nc.mode(), nc.violation(), nc.detail()); } }