Merge remote-tracking branch 'origin/jetty-10.0.x' into jetty-10.0.x-4919-WebSocketContainerStop

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

Jenkinsfile

@@ -14,7 +14,7 @@ pipeline {
           steps {
             container( 'jetty-build' ) {
               timeout( time: 120, unit: 'MINUTES' ) {
-                mavenBuild( "jdk11", "-T3 -Pmongodb clean install", "maven3", true ) // -Pautobahn
+                mavenBuild( "jdk11", "-T3 clean install", "maven3", true ) // -Pautobahn
                 // Collect up the jacoco execution results (only on main build)
                 jacoco inclusionPattern: '**/org/eclipse/jetty/**/*.class',
                        exclusionPattern: '' +
@@ -44,7 +44,7 @@ pipeline {
           steps {
             container( 'jetty-build' ) {
               timeout( time: 120, unit: 'MINUTES' ) {
-                mavenBuild( "jdk14", "-T3 -Pmongodb clean install", "maven3", true )
+                mavenBuild( "jdk14", "-T3 clean install", "maven3", true )
                 warnings consoleParsers: [[parserName: 'Maven'], [parserName: 'Java']]
                 junit testResults: '**/target/surefire-reports/*.xml,**/target/invoker-reports/TEST*.xml'
               }

KEYS.txt

@@ -5,3 +5,4 @@ Joakim Erdfelt  <joakim.erdfelt@gmail.com>  5989 BAF7 6217 B843 D66B  E55B 2D0E
 Joakim Erdfelt  <joakime@apache.org>        B59B 67FD 7904 9843 67F9  3180 0818 D9D6 8FB6 7BAC
 Joakim Erdfelt  <joakim@erdfelt.com>        BFBB 21C2 46D7 7768 3628  7A48 A04E 0C74 ABB3 5FEA
 Simone Bordet   <simone.bordet@gmail.com>   8B09 6546 B1A8 F026 56B1  5D3B 1677 D141 BCF3 584D
+Olivier Lamy    <olamy@apache.org>          F254 B356 17DC 255D 9344  BCFA 873A 8E86 B437 2146

VERSION.txt

@@ -1,5 +1,25 @@
 jetty-10.0.0-SNAPSHOT
 
+jetty-9.4.30.v20200611 - 11 June 2020
+ + 4776 Incorrect path matching for WebSocket using PathMappings
+ + 4826 Upgrade to Apache Jasper 8.5.54
+ + 4855 occasional h2spec failures on jenkins
+ + 4873 Server.join not working when used with ExecutorThreadPool
+ + 4885 setCookie() must not change the headers in a response during an include
+ + 4890 JettyClient behavior when SETTINGS_HEADER_TABLE_SIZE is set to 0 in
+   SETTINGS Frame.
+ + 4894 JDBCSessionDataStore fails to create multiple JettySessions for server
+   with multiple databases
+ + 4903 Give better errors for non public Websocket Endpoints
+ + 4904 WebsocketClient creates more connections than needed
+ + 4913 DirectoryNotEmptyException when using mvn jetty:run-distro
+ + 4920 Restore ability to delete sessions on stop
+ + 4921 Quickstart run improperly runs dynamically added context initializers
+ + 4923 SecureRequestCustomizer.SslAttributes does not cache cert chain like
+   before
+ + 4929 HttpClient: HttpCookieStore.Empty prevents sending cookies
+ + 4936 Response header overflow leads to buffer corruptions
+
 jetty-9.4.29.v20200521 - 21 May 2020
  + 2188 Lock contention creating HTTP/2 streams
  + 4235 communicate the reason of failure to the OpenID error page
@@ -12,8 +32,8 @@ jetty-9.4.29.v20200521 - 21 May 2020
  + 4798 Better handling of fatal Selector failures
  + 4814 Allow a ConnectionFactory (eg SslConnectionFactory) to automatically
    add a Customizer
- + 4820 Jetty OSGi DefaultJettyAtJettyHomeHelper refers to non-existent
+ + 4820 Jetty OSGi DefaultJettyAtJettyHomeHelper refers to non-existent config
-   config file
+   file
  + 4824 WebSocket server outgoing message queue memory growth
  + 4828 NIO ByteBuffer corruption in embedded Jetty server
  + 4835 GzipHandler and GzipHttpOutputInterceptor do not flush response when

examples/embedded/src/main/java/org/eclipse/jetty/embedded/FastFileServer.java

@@ -54,6 +54,22 @@ import org.eclipse.jetty.util.resource.Resource;
  * extent so does the {@link ResourceHandler}, so unless you have exceptional
  * circumstances it is best to use those classes for static content
  * </p>
+ * <p>
+ *     <em>WARNING</em>: This is an example on how to send content fast.
+ *     It is not secure, is highly vulnerable, and does not contain the
+ *     common set of mitigations for malicious requests that bypass
+ *     your controls over what a client can access.
+ *
+ *     If you want to continue this codebase, consider adding
+ *     checks for content outside of the resourceBase, and other
+ *     bypasses such as alias references, alternate stream references,
+ *     filesystem case sensitivity differences, filesystem utf-8 handling
+ *     differences, bad filename concerns, etc..
+ *
+ *     Or just use the existing {@link DefaultServlet} or
+ *     {@link ResourceHandler} that gives you all of these protections
+ *     (and more) built-in.
+ * </p>
  */
 public class FastFileServer
 {

jetty-client/src/main/java/org/eclipse/jetty/client/AbstractConnectionPool.java

@@ -22,7 +22,6 @@ import java.util.Collection;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import org.eclipse.jetty.client.api.Connection;
-import org.eclipse.jetty.client.api.Destination;
 import org.eclipse.jetty.util.AtomicBiInteger;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.Promise;
@@ -37,24 +36,28 @@ public abstract class AbstractConnectionPool implements ConnectionPool, Dumpable
 {
     private static final Logger LOG = LoggerFactory.getLogger(AbstractConnectionPool.class);
 
-    private final AtomicBoolean closed = new AtomicBoolean();
-
     /**
      * The connectionCount encodes both the total connections plus the pending connection counts, so both can be atomically changed.
      * The bottom 32 bits represent the total connections and the top 32 bits represent the pending connections.
      */
     private final AtomicBiInteger connections = new AtomicBiInteger();
-    private final Destination destination;
+    private final AtomicBoolean closed = new AtomicBoolean();
+    private final HttpDestination destination;
     private final int maxConnections;
     private final Callback requester;
 
-    protected AbstractConnectionPool(Destination destination, int maxConnections, Callback requester)
+    protected AbstractConnectionPool(HttpDestination destination, int maxConnections, Callback requester)
     {
         this.destination = destination;
         this.maxConnections = maxConnections;
         this.requester = requester;
     }
 
+    protected HttpDestination getHttpDestination()
+    {
+        return destination;
+    }
+
     @ManagedAttribute(value = "The max number of connections", readonly = true)
     public int getMaxConnectionCount()
     {
@@ -86,17 +89,28 @@ public abstract class AbstractConnectionPool implements ConnectionPool, Dumpable
     }
 
     @Override
-    public Connection acquire()
+    public Connection acquire(boolean create)
     {
         Connection connection = activate();
         if (connection == null)
         {
-            tryCreate(-1);
+            if (create)
+                tryCreate(destination.getQueuedRequestCount());
             connection = activate();
         }
         return connection;
     }
 
+    /**
+     * <p>Schedules the opening of a new connection.</p>
+     * <p>Whether a new connection is scheduled for opening is determined by the {@code maxPending} parameter:
+     * if {@code maxPending} is greater than the current number of connections scheduled for opening,
+     * then this method returns without scheduling the opening of a new connection;
+     * if {@code maxPending} is negative, a new connection is always scheduled for opening.</p>
+     *
+     * @param maxPending the max desired number of connections scheduled for opening,
+     * or a negative number to always trigger the opening of a new connection
+     */
     protected void tryCreate(int maxPending)
     {
         while (true)

jetty-client/src/main/java/org/eclipse/jetty/client/ConnectionPool.java

@@ -45,12 +45,16 @@ public interface ConnectionPool extends Closeable
     boolean isClosed();
 
     /**
-     * <p>Returns an idle connection, if available, or schedules the opening
+     * <p>Returns an idle connection, if available;
-     * of a new connection and returns {@code null}.</p>
+     * if an idle connection is not available, and the given {@code create} parameter is {@code true},
+     * then schedules the opening of a new connection, if possible within the configuration of this
+     * connection pool (for example, if it does not exceed the max connection count);
+     * otherwise returns {@code null}.</p>
      *
-     * @return an available connection, or null
+     * @param create whether to schedule the opening of a connection if no idle connections are available
+     * @return an idle connection or {@code null} if no idle connections are available
      */
-    Connection acquire();
+    Connection acquire(boolean create);
 
     /**
      * <p>Accepts the given connection to be managed by this ConnectionPool.</p>
@@ -61,7 +65,7 @@ public interface ConnectionPool extends Closeable
     boolean accept(Connection connection);
 
     /**
-     * <p>Returns the given connection, previously obtained via {@link #acquire()},
+     * <p>Returns the given connection, previously obtained via {@link #acquire(boolean)},
      * back to this ConnectionPool.</p>
      *
      * @param connection the connection to release

jetty-client/src/main/java/org/eclipse/jetty/client/DuplexConnectionPool.java

@@ -31,7 +31,6 @@ import java.util.concurrent.locks.ReentrantLock;
 import java.util.stream.Collectors;
 
 import org.eclipse.jetty.client.api.Connection;
-import org.eclipse.jetty.client.api.Destination;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.annotation.ManagedAttribute;
 import org.eclipse.jetty.util.annotation.ManagedObject;
@@ -50,7 +49,7 @@ public class DuplexConnectionPool extends AbstractConnectionPool implements Swee
     private final Deque<Connection> idleConnections;
     private final Set<Connection> activeConnections;
 
-    public DuplexConnectionPool(Destination destination, int maxConnections, Callback requester)
+    public DuplexConnectionPool(HttpDestination destination, int maxConnections, Callback requester)
     {
         super(destination, maxConnections, requester);
         this.idleConnections = new ArrayDeque<>(maxConnections);

jetty-client/src/main/java/org/eclipse/jetty/client/HttpConnection.java

@@ -105,6 +105,8 @@ public abstract class HttpConnection implements IConnection
             }
             else
             {
+                // Association may fail, for example if the application
+                // aborted the request, so we must release the channel.
                 channel.release();
                 result = new SendFailure(new HttpRequestException("Could not associate request to connection", request), false);
             }
@@ -119,6 +121,8 @@ public abstract class HttpConnection implements IConnection
         }
         else
         {
+            // This connection has been timed out by another thread
+            // that will take care of removing it from the pool.
             return new SendFailure(new TimeoutException(), true);
         }
     }
@@ -187,19 +191,18 @@ public abstract class HttpConnection implements IConnection
         }
 
         // Cookies
+        StringBuilder cookies = convertCookies(request.getCookies(), null);
         CookieStore cookieStore = getHttpClient().getCookieStore();
         if (cookieStore != null && cookieStore.getClass() != HttpCookieStore.Empty.class)
         {
-            StringBuilder cookies = null;
             URI uri = request.getURI();
             if (uri != null)
-                cookies = convertCookies(HttpCookieStore.matchPath(uri, cookieStore.get(uri)), null);
+                cookies = convertCookies(HttpCookieStore.matchPath(uri, cookieStore.get(uri)), cookies);
-            cookies = convertCookies(request.getCookies(), cookies);
+        }
-            if (cookies != null)
+        if (cookies != null)
-            {
+        {
-                HttpField cookieField = new HttpField(HttpHeader.COOKIE, cookies.toString());
+            HttpField cookieField = new HttpField(HttpHeader.COOKIE, cookies.toString());
-                request.addHeader(cookieField);
+            request.addHeader(cookieField);
-            }
         }
 
         // Authentication

jetty-client/src/main/java/org/eclipse/jetty/client/HttpDestination.java

@@ -55,7 +55,7 @@ import org.slf4j.LoggerFactory;
 @ManagedObject
 public abstract class HttpDestination extends ContainerLifeCycle implements Destination, Closeable, Callback, Dumpable
 {
-    protected static final Logger LOG = LoggerFactory.getLogger(HttpDestination.class);
+    private static final Logger LOG = LoggerFactory.getLogger(HttpDestination.class);
 
     private final HttpClient client;
     private final Origin origin;
@@ -234,7 +234,7 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
     @Override
     public void succeeded()
     {
-        send();
+        send(false);
     }
 
     @Override
@@ -291,32 +291,42 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
         }
     }
 
-    public void send()
-    {
-        if (getHttpExchanges().isEmpty())
-            return;
-        process();
-    }
-
     protected boolean enqueue(Queue<HttpExchange> queue, HttpExchange exchange)
     {
         return queue.offer(exchange);
     }
 
-    private void process()
+    public void send()
     {
+        send(true);
+    }
+
+    private void send(boolean create)
+    {
+        if (getHttpExchanges().isEmpty())
+            return;
+        process(create);
+    }
+
+    private void process(boolean create)
+    {
+        // The loop is necessary in case of a new multiplexed connection,
+        // when a single thread notified of the connection opening must
+        // process all queued exchanges.
+        // In other cases looping is a work-stealing optimization.
         while (true)
         {
-            Connection connection = connectionPool.acquire();
+            Connection connection = connectionPool.acquire(create);
             if (connection == null)
                 break;
-            boolean proceed = process(connection);
+            ProcessResult result = process(connection);
-            if (!proceed)
+            if (result == ProcessResult.FINISH)
                 break;
+            create = result == ProcessResult.RESTART;
         }
     }
 
-    public boolean process(Connection connection)
+    private ProcessResult process(Connection connection)
     {
         HttpClient client = getHttpClient();
         HttpExchange exchange = getHttpExchanges().poll();
@@ -332,7 +342,7 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
                     LOG.debug("{} is stopping", client);
                 connection.close();
             }
-            return false;
+            return ProcessResult.FINISH;
         }
         else
         {
@@ -343,31 +353,37 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
                 if (LOG.isDebugEnabled())
                     LOG.debug("Aborted before processing {}: {}", exchange, cause);
                 // Won't use this connection, release it back.
-                if (!connectionPool.release(connection))
+                boolean released = connectionPool.release(connection);
+                if (!released)
                     connection.close();
                 // It may happen that the request is aborted before the exchange
                 // is created. Aborting the exchange a second time will result in
                 // a no-operation, so we just abort here to cover that edge case.
                 exchange.abort(cause);
+                return getHttpExchanges().size() > 0
+                    ?  (released ? ProcessResult.CONTINUE : ProcessResult.RESTART)
+                    : ProcessResult.FINISH;
             }
-            else
+
+            SendFailure failure = send((IConnection)connection, exchange);
+            if (failure == null)
             {
-                SendFailure result = send((IConnection)connection, exchange);
+                // Aggressively send other queued requests
-                if (result != null)
+                // in case connections are multiplexed.
-                {
+                return getHttpExchanges().size() > 0 ? ProcessResult.CONTINUE : ProcessResult.FINISH;
-                    if (LOG.isDebugEnabled())
-                        LOG.debug("Send failed {} for {}", result, exchange);
-                    if (result.retry)
-                    {
-                        // Resend this exchange, likely on another connection,
-                        // and return false to avoid to re-enter this method.
-                        send(exchange);
-                        return false;
-                    }
-                    request.abort(result.failure);
-                }
             }
-            return getHttpExchanges().peek() != null;
+
+            if (LOG.isDebugEnabled())
+                LOG.debug("Send failed {} for {}", failure, exchange);
+            if (failure.retry)
+            {
+                // Resend this exchange, likely on another connection,
+                // and return false to avoid to re-enter this method.
+                send(exchange);
+                return ProcessResult.FINISH;
+            }
+            request.abort(failure.failure);
+            return getHttpExchanges().size() > 0 ? ProcessResult.RESTART : ProcessResult.FINISH;
         }
     }
 
@@ -392,11 +408,6 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
         return exchanges.remove(exchange);
     }
 
-    public boolean remove(Connection connection)
-    {
-        return connectionPool.remove(connection);
-    }
-
     @Override
     public void close()
     {
@@ -407,24 +418,6 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
         timeout.destroy();
     }
 
-    public void close(Connection connection)
-    {
-        boolean removed = remove(connection);
-
-        if (getHttpExchanges().isEmpty())
-        {
-            tryRemoveIdleDestination();
-        }
-        else
-        {
-            // We need to execute queued requests even if this connection failed.
-            // We may create a connection that is not needed, but it will eventually
-            // idle timeout, so no worries.
-            if (removed)
-                process();
-        }
-    }
-
     public void release(Connection connection)
     {
         if (LOG.isDebugEnabled())
@@ -435,7 +428,7 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
             if (connectionPool.isActive(connection))
             {
                 if (connectionPool.release(connection))
-                    send();
+                    send(false);
                 else
                     connection.close();
             }
@@ -453,6 +446,24 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
         }
     }
 
+    public boolean remove(Connection connection)
+    {
+        boolean removed = connectionPool.remove(connection);
+
+        if (getHttpExchanges().isEmpty())
+        {
+            tryRemoveIdleDestination();
+        }
+        else if (removed)
+        {
+            // Process queued requests that may be waiting.
+            // We may create a connection that is not
+            // needed, but it will eventually idle timeout.
+            process(true);
+        }
+        return removed;
+    }
+
     /**
      * Aborts all the {@link HttpExchange}s queued in this destination.
      *
@@ -580,4 +591,9 @@ public abstract class HttpDestination extends ContainerLifeCycle implements Dest
             }
         }
     }
+
+    private enum ProcessResult
+    {
+        RESTART, CONTINUE, FINISH
+    }
 }

jetty-client/src/main/java/org/eclipse/jetty/client/LeakTrackingConnectionPool.java

@@ -19,7 +19,6 @@
 package org.eclipse.jetty.client;
 
 import org.eclipse.jetty.client.api.Connection;
-import org.eclipse.jetty.client.api.Destination;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.LeakDetector;
 import org.slf4j.Logger;
@@ -38,7 +37,7 @@ public class LeakTrackingConnectionPool extends DuplexConnectionPool
         }
     };
 
-    public LeakTrackingConnectionPool(Destination destination, int maxConnections, Callback requester)
+    public LeakTrackingConnectionPool(HttpDestination destination, int maxConnections, Callback requester)
     {
         super(destination, maxConnections, requester);
         start();

jetty-client/src/main/java/org/eclipse/jetty/client/MultiplexConnectionPool.java

@@ -40,7 +40,6 @@ public class MultiplexConnectionPool extends AbstractConnectionPool implements C
 {
     private static final Logger LOG = LoggerFactory.getLogger(MultiplexConnectionPool.class);
 
-    private final HttpDestination destination;
     private final Deque<Holder> idleConnections;
     private final Map<Connection, Holder> activeConnections;
     private int maxMultiplex;
@@ -48,25 +47,36 @@ public class MultiplexConnectionPool extends AbstractConnectionPool implements C
     public MultiplexConnectionPool(HttpDestination destination, int maxConnections, Callback requester, int maxMultiplex)
     {
         super(destination, maxConnections, requester);
-        this.destination = destination;
         this.idleConnections = new ArrayDeque<>(maxConnections);
         this.activeConnections = new LinkedHashMap<>(maxConnections);
         this.maxMultiplex = maxMultiplex;
     }
 
     @Override
-    public Connection acquire()
+    public Connection acquire(boolean create)
     {
         Connection connection = activate();
         if (connection == null)
         {
-            int maxPending = 1 + destination.getQueuedRequestCount() / getMaxMultiplex();
+            int queuedRequests = getHttpDestination().getQueuedRequestCount();
+            int maxMultiplex = getMaxMultiplex();
+            int maxPending = ceilDiv(queuedRequests, maxMultiplex);
             tryCreate(maxPending);
             connection = activate();
         }
         return connection;
     }
 
+    /**
+     * @param a the dividend
+     * @param b the divisor
+     * @return the ceiling of the algebraic quotient
+     */
+    private static int ceilDiv(int a, int b)
+    {
+        return (a + b - 1) / b;
+    }
+
     @Override
     public int getMaxMultiplex()
     {

jetty-client/src/main/java/org/eclipse/jetty/client/RoundRobinConnectionPool.java

@@ -23,7 +23,6 @@ import java.util.ArrayList;
 import java.util.List;
 
 import org.eclipse.jetty.client.api.Connection;
-import org.eclipse.jetty.client.api.Destination;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.annotation.ManagedObject;
 import org.eclipse.jetty.util.component.Dumpable;
@@ -35,12 +34,12 @@ public class RoundRobinConnectionPool extends AbstractConnectionPool implements
     private int maxMultiplex;
     private int index;
 
-    public RoundRobinConnectionPool(Destination destination, int maxConnections, Callback requester)
+    public RoundRobinConnectionPool(HttpDestination destination, int maxConnections, Callback requester)
     {
         this(destination, maxConnections, requester, 1);
     }
 
-    public RoundRobinConnectionPool(Destination destination, int maxConnections, Callback requester, int maxMultiplex)
+    public RoundRobinConnectionPool(HttpDestination destination, int maxConnections, Callback requester, int maxMultiplex)
     {
         super(destination, maxConnections, requester);
         entries = new ArrayList<>(maxConnections);
@@ -69,6 +68,21 @@ public class RoundRobinConnectionPool extends AbstractConnectionPool implements
         }
     }
 
+    /**
+     * <p>Returns an idle connection, if available, following a round robin algorithm;
+     * otherwise it always tries to create a new connection, up until the max connection count.</p>
+     *
+     * @param create this parameter is ignored and assumed to be always {@code true}
+     * @return an idle connection or {@code null} if no idle connections are available
+     */
+    @Override
+    public Connection acquire(boolean create)
+    {
+        // The nature of this connection pool is such that a
+        // connection must always be present in the next slot.
+        return super.acquire(true);
+    }
+
     @Override
     protected void onCreated(Connection connection)
     {

jetty-client/src/main/java/org/eclipse/jetty/client/ValidatingConnectionPool.java

@@ -26,7 +26,6 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.Stream;
 
 import org.eclipse.jetty.client.api.Connection;
-import org.eclipse.jetty.client.api.Destination;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.annotation.ManagedAttribute;
 import org.eclipse.jetty.util.component.DumpableCollection;
@@ -65,7 +64,7 @@ public class ValidatingConnectionPool extends DuplexConnectionPool
     private final long timeout;
     private final Map<Connection, Holder> quarantine;
 
-    public ValidatingConnectionPool(Destination destination, int maxConnections, Callback requester, Scheduler scheduler, long timeout)
+    public ValidatingConnectionPool(HttpDestination destination, int maxConnections, Callback requester, Scheduler scheduler, long timeout)
     {
         super(destination, maxConnections, requester);
         this.scheduler = scheduler;

jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpConnectionOverHTTP.java

@@ -206,7 +206,7 @@ public class HttpConnectionOverHTTP extends AbstractConnection implements IConne
     {
         if (closed.compareAndSet(false, true))
         {
-            getHttpDestination().close(this);
+            getHttpDestination().remove(this);
             abort(failure);
             channel.destroy();
             getEndPoint().shutdownOutput();

jetty-client/src/main/java/org/eclipse/jetty/client/util/BufferingResponseListener.java

@@ -119,9 +119,10 @@ public abstract class BufferingResponseListener extends Listener.Adapter
         int length = content.remaining();
         if (length > BufferUtil.space(buffer))
         {
-            int requiredCapacity = buffer == null ? length : buffer.capacity() + length;
+            int remaining = buffer == null ? 0 : buffer.remaining();
-            if (requiredCapacity > maxLength)
+            if (remaining + length > maxLength)
                 response.abort(new IllegalArgumentException("Buffering capacity " + maxLength + " exceeded"));
+            int requiredCapacity = buffer == null ? length : buffer.capacity() + length;
             int newCapacity = Math.min(Integer.highestOneBit(requiredCapacity) << 1, maxLength);
             buffer = BufferUtil.ensureCapacity(buffer, newCapacity);
         }

jetty-client/src/test/java/org/eclipse/jetty/client/ConnectionPoolTest.java

@@ -19,6 +19,7 @@
 package org.eclipse.jetty.client;
 
 import java.io.IOException;
+import java.net.InetSocketAddress;
 import java.util.List;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
@@ -30,6 +31,7 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.eclipse.jetty.client.api.ContentResponse;
+import org.eclipse.jetty.client.api.Destination;
 import org.eclipse.jetty.client.api.Request;
 import org.eclipse.jetty.client.http.HttpClientTransportOverHTTP;
 import org.eclipse.jetty.client.util.BytesRequestContent;
@@ -38,44 +40,62 @@ import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpHeaderValue;
 import org.eclipse.jetty.http.HttpMethod;
 import org.eclipse.jetty.http.HttpStatus;
+import org.eclipse.jetty.io.ClientConnector;
 import org.eclipse.jetty.server.Handler;
 import org.eclipse.jetty.server.Server;
 import org.eclipse.jetty.server.ServerConnector;
 import org.eclipse.jetty.util.IO;
+import org.eclipse.jetty.util.Promise;
+import org.eclipse.jetty.util.SocketAddressResolver;
+import org.eclipse.jetty.util.thread.QueuedThreadPool;
+import org.hamcrest.Matchers;
 import org.junit.jupiter.api.AfterEach;
-import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-@Disabled // Disabled by @gregw on issue #2540 - commit 621b946b10884e7308eacca241dcf8b5d6f6cff2
 public class ConnectionPoolTest
 {
     private Server server;
     private ServerConnector connector;
     private HttpClient client;
 
-    public static Stream<ConnectionPool.Factory> pools()
+    public static Stream<ConnectionPoolFactory> pools()
     {
-        return Stream.of(destination -> new DuplexConnectionPool(destination, 8, destination),
+        return Stream.of(
-            destination -> new RoundRobinConnectionPool(destination, 8, destination));
+            new ConnectionPoolFactory("duplex", destination -> new DuplexConnectionPool(destination, destination.getHttpClient().getMaxConnectionsPerDestination(), destination)),
+            new ConnectionPoolFactory("round-robin", destination -> new RoundRobinConnectionPool(destination, destination.getHttpClient().getMaxConnectionsPerDestination(), destination)),
+            new ConnectionPoolFactory("multiplex", destination -> new MultiplexConnectionPool(destination, destination.getHttpClient().getMaxConnectionsPerDestination(), destination, 1))
+        );
     }
 
-    private void start(final ConnectionPool.Factory factory, Handler handler) throws Exception
+    private void start(ConnectionPool.Factory factory, Handler handler) throws Exception
+    {
+        startServer(handler);
+        startClient(factory);
+    }
+
+    private void startClient(ConnectionPool.Factory factory) throws Exception
+    {
+        ClientConnector connector = new ClientConnector();
+        connector.setSelectors(1);
+        HttpClientTransport transport = new HttpClientTransportOverHTTP(connector);
+        transport.setConnectionPoolFactory(factory);
+        client = new HttpClient(transport);
+        client.start();
+    }
+
+    private void startServer(Handler handler) throws Exception
     {
         server = new Server();
         connector = new ServerConnector(server);
         server.addConnector(connector);
         server.setHandler(handler);
-
-        HttpClientTransport transport = new HttpClientTransportOverHTTP(1);
-        transport.setConnectionPoolFactory(factory);
         server.start();
-
-        client = new HttpClient(transport);
-        client.start();
     }
 
     @AfterEach
@@ -99,11 +119,11 @@ public class ConnectionPoolTest
         }
     }
 
-    @ParameterizedTest(name = "[{index}] {0}")
+    @ParameterizedTest
     @MethodSource("pools")
-    public void test(ConnectionPool.Factory factory) throws Exception
+    public void test(ConnectionPoolFactory factory) throws Exception
     {
-        start(factory, new EmptyServerHandler()
+        start(factory.factory, new EmptyServerHandler()
         {
             @Override
             protected void service(String target, org.eclipse.jetty.server.Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
@@ -221,4 +241,140 @@ public class ConnectionPoolTest
             failures.add(x);
         }
     }
+
+    @ParameterizedTest
+    @MethodSource("pools")
+    public void testQueuedRequestsDontOpenTooManyConnections(ConnectionPoolFactory factory) throws Exception
+    {
+        startServer(new EmptyServerHandler());
+
+        ClientConnector clientConnector = new ClientConnector();
+        clientConnector.setSelectors(1);
+        HttpClientTransport transport = new HttpClientTransportOverHTTP(clientConnector);
+        transport.setConnectionPoolFactory(factory.factory);
+        client = new HttpClient(transport);
+        long delay = 1000;
+        client.setSocketAddressResolver(new SocketAddressResolver.Sync()
+        {
+            @Override
+            public void resolve(String host, int port, Promise<List<InetSocketAddress>> promise)
+            {
+                client.getExecutor().execute(() ->
+                {
+                    try
+                    {
+                        Thread.sleep(delay);
+                        super.resolve(host, port, promise);
+                    }
+                    catch (InterruptedException x)
+                    {
+                        promise.failed(x);
+                    }
+                });
+            }
+        });
+        client.start();
+
+        CountDownLatch latch = new CountDownLatch(2);
+        client.newRequest("localhost", connector.getLocalPort())
+            .path("/one")
+            .send(result ->
+            {
+                if (result.isSucceeded())
+                    latch.countDown();
+            });
+        Thread.sleep(delay / 2);
+        client.newRequest("localhost", connector.getLocalPort())
+            .path("/two")
+            .send(result ->
+            {
+                if (result.isSucceeded())
+                    latch.countDown();
+            });
+
+        assertTrue(latch.await(2 * delay, TimeUnit.MILLISECONDS));
+        List<Destination> destinations = client.getDestinations();
+        assertEquals(1, destinations.size());
+        HttpDestination destination = (HttpDestination)destinations.get(0);
+        AbstractConnectionPool connectionPool = (AbstractConnectionPool)destination.getConnectionPool();
+        assertEquals(2, connectionPool.getConnectionCount());
+    }
+
+    @ParameterizedTest
+    @MethodSource("pools")
+    public void testConcurrentRequestsDontOpenTooManyConnections(ConnectionPoolFactory factory) throws Exception
+    {
+        // Round robin connection pool does open a few more connections than expected.
+        Assumptions.assumeFalse(factory.name.equals("round-robin"));
+
+        startServer(new EmptyServerHandler());
+
+        int count = 500;
+        ClientConnector clientConnector = new ClientConnector();
+        clientConnector.setSelectors(1);
+        QueuedThreadPool clientThreads = new QueuedThreadPool(2 * count);
+        clientThreads.setName("client");
+        clientConnector.setExecutor(clientThreads);
+        HttpClientTransport transport = new HttpClientTransportOverHTTP(clientConnector);
+        transport.setConnectionPoolFactory(factory.factory);
+        client = new HttpClient(transport);
+        client.setExecutor(clientThreads);
+        client.setMaxConnectionsPerDestination(2 * count);
+        client.setSocketAddressResolver(new SocketAddressResolver.Sync()
+        {
+            @Override
+            public void resolve(String host, int port, Promise<List<InetSocketAddress>> promise)
+            {
+                client.getExecutor().execute(() ->
+                {
+                    try
+                    {
+                        Thread.sleep(100);
+                        super.resolve(host, port, promise);
+                    }
+                    catch (InterruptedException x)
+                    {
+                        promise.failed(x);
+                    }
+                });
+            }
+        });
+        client.start();
+
+        CountDownLatch latch = new CountDownLatch(count);
+        for (int i = 0; i < count; ++i)
+        {
+            clientThreads.execute(() -> client.newRequest("localhost", connector.getLocalPort())
+                .send(result ->
+                {
+                    if (result.isSucceeded())
+                        latch.countDown();
+                }));
+        }
+
+        assertTrue(latch.await(count, TimeUnit.SECONDS));
+        List<Destination> destinations = client.getDestinations();
+        assertEquals(1, destinations.size());
+        HttpDestination destination = (HttpDestination)destinations.get(0);
+        AbstractConnectionPool connectionPool = (AbstractConnectionPool)destination.getConnectionPool();
+        assertThat(connectionPool.getConnectionCount(), Matchers.lessThanOrEqualTo(count));
+    }
+
+    private static class ConnectionPoolFactory
+    {
+        private final String name;
+        private final ConnectionPool.Factory factory;
+
+        private ConnectionPoolFactory(String name, ConnectionPool.Factory factory)
+        {
+            this.name = name;
+            this.factory = factory;
+        }
+
+        @Override
+        public String toString()
+        {
+            return name;
+        }
+    }
 }

jetty-client/src/test/java/org/eclipse/jetty/client/HttpCookieTest.java

@@ -18,6 +18,7 @@
 
 package org.eclipse.jetty.client;
 
+import java.net.CookieStore;
 import java.net.HttpCookie;
 import java.net.URI;
 import java.util.Arrays;
@@ -34,6 +35,7 @@ import org.eclipse.jetty.client.api.ContentResponse;
 import org.eclipse.jetty.client.api.Response;
 import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.server.Request;
+import org.eclipse.jetty.util.HttpCookieStore;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ArgumentsSource;
 
@@ -132,10 +134,22 @@ public class HttpCookieTest extends AbstractHttpClientServerTest
     @ParameterizedTest
     @ArgumentsSource(ScenarioProvider.class)
     public void testPerRequestCookieIsSent(Scenario scenario) throws Exception
+    {
+        testPerRequestCookieIsSent(scenario, null);
+    }
+
+    @ParameterizedTest
+    @ArgumentsSource(ScenarioProvider.class)
+    public void testPerRequestCookieIsSentWithEmptyCookieStore(Scenario scenario) throws Exception
+    {
+        testPerRequestCookieIsSent(scenario, new HttpCookieStore.Empty());
+    }
+
+    private void testPerRequestCookieIsSent(Scenario scenario, CookieStore cookieStore) throws Exception
     {
         final String name = "foo";
         final String value = "bar";
-        start(scenario, new EmptyServerHandler()
+        startServer(scenario, new EmptyServerHandler()
         {
             @Override
             protected void service(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response)
@@ -148,6 +162,11 @@ public class HttpCookieTest extends AbstractHttpClientServerTest
                 assertEquals(value, cookie.getValue());
             }
         });
+        startClient(scenario, client ->
+        {
+            if (cookieStore != null)
+                client.setCookieStore(cookieStore);
+        });
 
         ContentResponse response = client.newRequest("localhost", connector.getLocalPort())
             .scheme(scenario.getScheme())

jetty-client/src/test/java/org/eclipse/jetty/client/http/HttpDestinationOverHTTPTest.java

@@ -18,6 +18,7 @@
 
 package org.eclipse.jetty.client.http;
 
+import java.lang.reflect.Method;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.TimeUnit;
@@ -37,6 +38,7 @@ import org.eclipse.jetty.client.api.Destination;
 import org.eclipse.jetty.client.api.Request;
 import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpHeaderValue;
+import org.eclipse.jetty.util.Callback;
 import org.hamcrest.Matchers;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ArgumentsSource;
@@ -54,7 +56,7 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest
 {
     @ParameterizedTest
     @ArgumentsSource(ScenarioProvider.class)
-    public void testFirstAcquireWithEmptyQueue(Scenario scenario) throws Exception
+    public void testAcquireWithEmptyQueue(Scenario scenario) throws Exception
     {
         start(scenario, new EmptyServerHandler());
 
@@ -62,7 +64,29 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest
         {
             destination.start();
             DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool();
-            Connection connection = connectionPool.acquire();
+            Connection connection = connectionPool.acquire(true);
+            assertNull(connection);
+            // There are no queued requests, so no connection should be created.
+            connection = pollIdleConnection(connectionPool, 1, TimeUnit.SECONDS);
+            assertNull(connection);
+        }
+    }
+
+    @ParameterizedTest
+    @ArgumentsSource(ScenarioProvider.class)
+    public void testAcquireWithOneExchangeQueued(Scenario scenario) throws Exception
+    {
+        start(scenario, new EmptyServerHandler());
+
+        try (TestDestination destination = new TestDestination(client, new Origin("http", "localhost", connector.getLocalPort())))
+        {
+            destination.start();
+            TestDestination.TestConnectionPool connectionPool = (TestDestination.TestConnectionPool)destination.getConnectionPool();
+
+            // Trigger creation of one connection.
+            connectionPool.tryCreate(1);
+
+            Connection connection = connectionPool.acquire(false);
             if (connection == null)
             {
                 // There are no queued requests, so the newly created connection will be idle
@@ -78,19 +102,22 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest
     {
         start(scenario, new EmptyServerHandler());
 
-        try (HttpDestination destination = new DuplexHttpDestination(client, new Origin("http", "localhost", connector.getLocalPort())))
+        try (TestDestination destination = new TestDestination(client, new Origin("http", "localhost", connector.getLocalPort())))
         {
             destination.start();
+            TestDestination.TestConnectionPool connectionPool = (TestDestination.TestConnectionPool)destination.getConnectionPool();
 
-            DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool();
+            // Trigger creation of one connection.
-            Connection connection1 = connectionPool.acquire();
+            connectionPool.tryCreate(1);
+
+            Connection connection1 = connectionPool.acquire(true);
             if (connection1 == null)
             {
                 // There are no queued requests, so the newly created connection will be idle
                 connection1 = peekIdleConnection(connectionPool, 5, TimeUnit.SECONDS);
                 assertNotNull(connection1);
 
-                Connection connection2 = connectionPool.acquire();
+                Connection connection2 = connectionPool.acquire(true);
                 assertSame(connection1, connection2);
             }
         }
@@ -102,14 +129,14 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest
     {
         start(scenario, new EmptyServerHandler());
 
-        final CountDownLatch idleLatch = new CountDownLatch(1);
+        CountDownLatch idleLatch = new CountDownLatch(1);
-        final CountDownLatch latch = new CountDownLatch(1);
+        CountDownLatch latch = new CountDownLatch(1);
-        HttpDestination destination = new DuplexHttpDestination(client, new Origin("http", "localhost", connector.getLocalPort()))
+        try (TestDestination destination = new TestDestination(client, new Origin("http", "localhost", connector.getLocalPort()))
         {
             @Override
             protected ConnectionPool newConnectionPool(HttpClient client)
             {
-                return new DuplexConnectionPool(this, client.getMaxConnectionsPerDestination(), this)
+                return new TestConnectionPool(this, client.getMaxConnectionsPerDestination(), this)
                 {
                     @Override
                     protected void onCreated(Connection connection)
@@ -127,30 +154,37 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest
                     }
                 };
             }
-        };
+        })
+        {
+            destination.start();
+            TestDestination.TestConnectionPool connectionPool = (TestDestination.TestConnectionPool)destination.getConnectionPool();
 
-        destination.start();
+            // Trigger creation of one connection.
-        DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool();
+            connectionPool.tryCreate(1);
-        Connection connection1 = connectionPool.acquire();
 
-        // Make sure we entered idleCreated().
+            // Make sure we entered idleCreated().
-        assertTrue(idleLatch.await(5, TimeUnit.SECONDS));
+            assertTrue(idleLatch.await(5, TimeUnit.SECONDS));
 
-        // There are no available existing connections, so acquire()
+            // There are no available existing connections, so acquire()
-        // returns null because we delayed idleCreated() above
+            // returns null because we delayed idleCreated() above.
-        assertNull(connection1);
+            Connection connection1 = connectionPool.acquire(true);
+            assertNull(connection1);
 
-        // Second attempt also returns null because we delayed idleCreated() above.
+            // Trigger creation of a second connection.
-        Connection connection2 = connectionPool.acquire();
+            connectionPool.tryCreate(1);
-        assertNull(connection2);
 
-        latch.countDown();
+            // Second attempt also returns null because we delayed idleCreated() above.
+            Connection connection2 = connectionPool.acquire(true);
+            assertNull(connection2);
 
-        // There must be 2 idle connections.
+            latch.countDown();
-        Connection connection = pollIdleConnection(connectionPool, 5, TimeUnit.SECONDS);
+
-        assertNotNull(connection);
+            // There must be 2 idle connections.
-        connection = pollIdleConnection(connectionPool, 5, TimeUnit.SECONDS);
+            Connection connection = pollIdleConnection(connectionPool, 5, TimeUnit.SECONDS);
-        assertNotNull(connection);
+            assertNotNull(connection);
+            connection = pollIdleConnection(connectionPool, 5, TimeUnit.SECONDS);
+            assertNotNull(connection);
+        }
     }
 
     @ParameterizedTest
@@ -159,23 +193,30 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest
     {
         start(scenario, new EmptyServerHandler());
 
-        try (HttpDestination destination = new DuplexHttpDestination(client, new Origin("http", "localhost", connector.getLocalPort())))
+        try (TestDestination destination = new TestDestination(client, new Origin("http", "localhost", connector.getLocalPort())))
         {
             destination.start();
-            DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool();
+            TestDestination.TestConnectionPool connectionPool = (TestDestination.TestConnectionPool)destination.getConnectionPool();
-            Connection connection1 = connectionPool.acquire();
+
+            // Trigger creation of one connection.
+            connectionPool.tryCreate(1);
+
+            Connection connection1 = connectionPool.acquire(true);
             if (connection1 == null)
             {
                 connection1 = peekIdleConnection(connectionPool, 5, TimeUnit.SECONDS);
                 assertNotNull(connection1);
                 // Acquire the connection to make it active.
-                assertSame(connection1, connectionPool.acquire(), "From idle");
+                assertSame(connection1, connectionPool.acquire(true), "From idle");
             }
 
-            destination.process(connection1);
+            // There are no exchanges so process() is a no-op.
+            Method process = HttpDestination.class.getDeclaredMethod("process", Connection.class);
+            process.setAccessible(true);
+            process.invoke(destination, connection1);
             destination.release(connection1);
 
-            Connection connection2 = connectionPool.acquire();
+            Connection connection2 = connectionPool.acquire(true);
             assertSame(connection1, connection2, "After release");
         }
     }
@@ -184,15 +225,20 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest
     @ArgumentsSource(ScenarioProvider.class)
     public void testIdleConnectionIdleTimeout(Scenario scenario) throws Exception
     {
-        startServer(scenario, new EmptyServerHandler());
+        start(scenario, new EmptyServerHandler());
+
         long idleTimeout = 1000;
         startClient(scenario, httpClient -> httpClient.setIdleTimeout(idleTimeout));
 
-        try (HttpDestination destination = new DuplexHttpDestination(client, new Origin("http", "localhost", connector.getLocalPort())))
+        try (TestDestination destination = new TestDestination(client, new Origin("http", "localhost", connector.getLocalPort())))
         {
             destination.start();
-            DuplexConnectionPool connectionPool = (DuplexConnectionPool)destination.getConnectionPool();
+            TestDestination.TestConnectionPool connectionPool = (TestDestination.TestConnectionPool)destination.getConnectionPool();
-            Connection connection1 = connectionPool.acquire();
+
+            // Trigger creation of one connection.
+            connectionPool.tryCreate(1);
+
+            Connection connection1 = connectionPool.acquire(true);
             if (connection1 == null)
             {
                 connection1 = peekIdleConnection(connectionPool, 5, TimeUnit.SECONDS);
@@ -261,7 +307,7 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest
         String host = "localhost";
         int port = connector.getLocalPort();
         Request request = client.newRequest(host, port)
-                .scheme(scenario.getScheme())
+            .scheme(scenario.getScheme())
                 .headers(headers -> headers.put(HttpHeader.CONNECTION, HttpHeaderValue.CLOSE));
         Destination destinationBefore = client.resolveDestination(request);
         ContentResponse response = request.send();
@@ -297,7 +343,7 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest
 
         server.stop();
         Request request = client.newRequest(host, port).scheme(scenario.getScheme());
-        assertThrows(Exception.class, () ->  request.send());
+        assertThrows(Exception.class, request::send);
 
         long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(1);
         while (!client.getDestinations().isEmpty() && System.nanoTime() < deadline)
@@ -329,4 +375,38 @@ public class HttpDestinationOverHTTPTest extends AbstractHttpClientServerTest
         }
         return null;
     }
+
+    private static class TestDestination extends DuplexHttpDestination
+    {
+        public TestDestination(HttpClient client, Origin origin)
+        {
+            super(client, origin);
+        }
+
+        @Override
+        protected ConnectionPool newConnectionPool(HttpClient client)
+        {
+            return new TestConnectionPool(this, client.getMaxConnectionsPerDestination(), this);
+        }
+
+        public static class TestConnectionPool extends DuplexConnectionPool
+        {
+            public TestConnectionPool(HttpDestination destination, int maxConnections, Callback requester)
+            {
+                super(destination, maxConnections, requester);
+            }
+
+            @Override
+            public void tryCreate(int maxPending)
+            {
+                super.tryCreate(maxPending);
+            }
+
+            @Override
+            public Connection acquire(boolean create)
+            {
+                return super.acquire(create);
+            }
+        }
+    }
 }

jetty-documentation/src/main/asciidoc/distribution-guide/connectors/configuring-connectors.adoc

@@ -254,7 +254,7 @@ For example, if we examine the `http.ini` file in our `start.d` directory create
 # jetty.http.selectors=-1
 
 ## ServerSocketChannel backlog (0 picks platform default)
-# jetty.http.acceptorQueueSize=0
+# jetty.http.acceptQueueSize=0
 
 ## Thread priority delta to give to acceptor threads
 # jetty.http.acceptorPriorityDelta=0

jetty-documentation/src/main/asciidoc/quickstart-guide/configuring/how-to-configure.adoc

@@ -134,7 +134,7 @@ $ cat start.d/http.ini
 # jetty.http.selectors=-1
 
 ## ServerSocketChannel backlog (0 picks platform default)
-# jetty.http.acceptorQueueSize=0
+# jetty.http.acceptQueueSize=0
 
 ## Thread priority delta to give to acceptor threads
 # jetty.http.acceptorPriorityDelta=0

jetty-documentation/src/main/asciidoc/quickstart-guide/introduction/what-version.adoc

@@ -33,13 +33,14 @@ _____
 [width="100%",cols="12%,9%,15%,6%,21%,10%,6%,21%",options="header",]
 |=======================================================================
 |Version |Year |Home |Min JVM |Protocols |Servlet |JSP |Status
-|10  |2019- |Eclipse |11 ^(1)^ |HTTP/1.1 (RFC 7230), HTTP/2 (RFC 7540), WebSocket (RFC 6455, JSR 356), FastCGI |4.0.2 |2.3 |*UNSTABLE / Alpha*
+|11  |2020- |Eclipse |11 ^(2)^ |HTTP/1.1 (RFC 7230), HTTP/2 (RFC 7540), WebSocket (RFC 6455, JSR 356), FastCGI, *JakartaEE Namespace*^(1)^ |4.0.2 |2.3 |*UNSTABLE / Alpha*
+|10  |2019- |Eclipse |11 ^(2)^ |HTTP/1.1 (RFC 7230), HTTP/2 (RFC 7540), WebSocket (RFC 6455, JSR 356), FastCGI |4.0.2 |2.3 |*UNSTABLE / Beta*
 |9.4 |2016- |Eclipse |1.8 |HTTP/1.1 (RFC 7230), HTTP/2 (RFC 7540), WebSocket (RFC 6455, JSR 356), FastCGI |3.1 |2.3 |Stable
-|9.3 |2015- |Eclipse |1.8 ^(2)^ |HTTP/1.1 (RFC 7230), HTTP/2 (RFC 7540), WebSocket (RFC 6455, JSR 356), FastCGI |3.1 |2.3 |Deprecated
+|9.3 |2015- |Eclipse |1.8 ^(3)^ |HTTP/1.1 (RFC 7230), HTTP/2 (RFC 7540), WebSocket (RFC 6455, JSR 356), FastCGI |3.1 |2.3 |Deprecated
-|9.2 |2014-2018 |Eclipse |1.7 ^(2)^ |HTTP/1.1 RFC2616, javax.websocket, SPDY v3 |3.1 |2.3 |Deprecated / *End of Life January 2018*
+|9.2 |2014-2018 |Eclipse |1.7 ^(3)^ |HTTP/1.1 RFC2616, javax.websocket, SPDY v3 |3.1 |2.3 |Deprecated / *End of Life January 2018*
-|9.1 |2013-2014 |Eclipse |1.7 ^(2)^ |HTTP/1.1 RFC2616 |3.1 |2.3 |Deprecated / *End of Life May 2014*
+|9.1 |2013-2014 |Eclipse |1.7 ^(3)^ |HTTP/1.1 RFC2616 |3.1 |2.3 |Deprecated / *End of Life May 2014*
-|9.0 |2013-2013 |Eclipse |1.7 ^(2)^ |HTTP/1.1 RFC2616 |3.1-beta |2.3 |Deprecated / *End of Life November 2013*
+|9.0 |2013-2013 |Eclipse |1.7 ^(3)^ |HTTP/1.1 RFC2616 |3.1-beta |2.3 |Deprecated / *End of Life November 2013*
-|8 |2009-2014 |Eclipse/Codehaus |1.6 ^(2)^ |HTTP/1.1 RFC2616, WebSocket RFC 6455, SPDY v3 |3.0 |2.2 |Deprecated / *End of Life November 2014*
+|8 |2009-2014 |Eclipse/Codehaus |1.6 ^(3)^ |HTTP/1.1 RFC2616, WebSocket RFC 6455, SPDY v3 |3.0 |2.2 |Deprecated / *End of Life November 2014*
 |7 |2008-2014 |Eclipse/Codehaus |1.5 |HTTP/1.1 RFC2616, WebSocket RFC 6455, SPDY v3 |2.5 |2.1 |Deprecated / *End of Life November 2014*
 |6 |2006-2010 |Codehaus |1.4-1.5 |HTTP/1.1 RFC2616 |2.5 |2.0 |Deprecated / *End of Life November 2010*
 |5 |2003-2009 |Sourceforge |1.2-1.5 |HTTP/1.1 RFC2616 |2.4 |2.0 |Antique
@@ -49,5 +50,6 @@ _____
 |1 |1995-1998 |Mortbay |1.0 |HTTP/1.0 RFC1945 |- |- |Mythical
 |=======================================================================
 
- 1. JPMS module support is optional
+ 1. Due to Oracle's ownership of the "Java" trademark, usage of the javax.* namespace has been restricted and the jakarta.* namespace link:https://www.eclipse.org/lists/jakartaee-platform-dev/msg00029.html[was adopted] by the Eclipse Foundation.
- 2. JDK9 and newer is not supported if using MultiRelease JAR Files, or Bytecode / Annotation scanning.
+ 2. JPMS module support is optional
+ 3. JDK9 and newer is not supported if using MultiRelease JAR Files, or Bytecode / Annotation scanning.

jetty-fcgi/fcgi-client/src/main/java/org/eclipse/jetty/fcgi/client/http/HttpConnectionOverFCGI.java

@@ -246,7 +246,7 @@ public class HttpConnectionOverFCGI extends AbstractConnection implements IConne
     {
         if (closed.compareAndSet(false, true))
         {
-            getHttpDestination().close(this);
+            getHttpDestination().remove(this);
 
             abort(failure);
 

jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/AbstractPathSpec.java

@@ -0,0 +1,66 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under
+// the terms of the Eclipse Public License 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0
+//
+// This Source Code may also be made available under the following
+// Secondary Licenses when the conditions for such availability set
+// forth in the Eclipse Public License, v. 2.0 are satisfied:
+// the Apache License v2.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.pathmap;
+
+import java.util.Objects;
+
+public abstract class AbstractPathSpec implements PathSpec
+{
+    @Override
+    public int compareTo(PathSpec other)
+    {
+        // Grouping (increasing)
+        int diff = getGroup().ordinal() - other.getGroup().ordinal();
+        if (diff != 0)
+            return diff;
+
+        // Spec Length (decreasing)
+        diff = other.getSpecLength() - getSpecLength();
+        if (diff != 0)
+            return diff;
+
+        // Path Spec Name (alphabetical)
+        return getDeclaration().compareTo(other.getDeclaration());
+    }
+
+    @Override
+    public final boolean equals(Object obj)
+    {
+        if (this == obj)
+            return true;
+        if (obj == null)
+            return false;
+        if (getClass() != obj.getClass())
+            return false;
+
+        return compareTo((AbstractPathSpec)obj) == 0;
+    }
+
+    @Override
+    public final int hashCode()
+    {
+        return Objects.hash(getDeclaration());
+    }
+
+    @Override
+    public String toString()
+    {
+        return String.format("%s@%s{%s}", getClass().getSimpleName(), Integer.toHexString(hashCode()), getDeclaration());
+    }
+}

jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathMappings.java

@@ -20,6 +20,7 @@ package org.eclipse.jetty.http.pathmap;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Comparator;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Optional;
@@ -46,7 +47,7 @@ import org.slf4j.LoggerFactory;
 public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
 {
     private static final Logger LOG = LoggerFactory.getLogger(PathMappings.class);
-    private final Set<MappedResource<E>> _mappings = new TreeSet<>();
+    private final Set<MappedResource<E>> _mappings = new TreeSet<>(Comparator.comparing(MappedResource::getPathSpec));
 
     private Trie<MappedResource<E>> _exactMap = new ArrayTernaryTrie<>(false);
     private Trie<MappedResource<E>> _prefixMap = new ArrayTernaryTrie<>(false);
@@ -100,7 +101,7 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
         List<MappedResource<E>> ret = new ArrayList<>();
         for (MappedResource<E> mr : _mappings)
         {
-            switch (mr.getPathSpec().group)
+            switch (mr.getPathSpec().getGroup())
             {
                 case ROOT:
                     if (isRootPath)
@@ -225,7 +226,7 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
     public boolean put(PathSpec pathSpec, E resource)
     {
         MappedResource<E> entry = new MappedResource<>(pathSpec, resource);
-        switch (pathSpec.group)
+        switch (pathSpec.getGroup())
         {
             case EXACT:
                 String exact = pathSpec.getPrefix();
@@ -260,16 +261,21 @@ public class PathMappings<E> implements Iterable<MappedResource<E>>, Dumpable
     @SuppressWarnings("incomplete-switch")
     public boolean remove(PathSpec pathSpec)
     {
-        switch (pathSpec.group)
+        String prefix = pathSpec.getPrefix();
+        String suffix = pathSpec.getSuffix();
+        switch (pathSpec.getGroup())
         {
             case EXACT:
-                _exactMap.remove(pathSpec.getPrefix());
+                if (prefix != null)
+                    _exactMap.remove(prefix);
                 break;
             case PREFIX_GLOB:
-                _prefixMap.remove(pathSpec.getPrefix());
+                if (prefix != null)
+                    _prefixMap.remove(prefix);
                 break;
             case SUFFIX_GLOB:
-                _suffixMap.remove(pathSpec.getSuffix());
+                if (suffix != null)
+                    _suffixMap.remove(suffix);
                 break;
             default:
                 break;

jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpec.java

@@ -19,72 +19,25 @@
 package org.eclipse.jetty.http.pathmap;
 
 /**
- * The base PathSpec, what all other path specs are based on
+ * A path specification is a URI path template that can be matched against.
+ * <p>
+ * Implementors <i>must</i> override {@link Object#equals(Object)} and {@link Object#hashCode()}.
  */
-public abstract class PathSpec implements Comparable<PathSpec>
+public interface PathSpec extends Comparable<PathSpec>
 {
-    protected String pathSpec;
+    /**
-    protected PathSpecGroup group;
+     * The length of the spec.
-    protected int pathDepth;
+     *
-    protected int specLength;
+     * @return the length of the spec.
-    protected String prefix;
+     */
-    protected String suffix;
+    int getSpecLength();
 
-    @Override
+    /**
-    public int compareTo(PathSpec other)
+     * The spec group.
-    {
+     *
-        // Grouping (increasing)
+     * @return the spec group.
-        int diff = this.group.ordinal() - other.group.ordinal();
+     */
-        if (diff != 0)
+    PathSpecGroup getGroup();
-        {
-            return diff;
-        }
-
-        // Spec Length (decreasing)
-        diff = other.specLength - this.specLength;
-        if (diff != 0)
-        {
-            return diff;
-        }
-
-        // Path Spec Name (alphabetical)
-        return this.pathSpec.compareTo(other.pathSpec);
-    }
-
-    @Override
-    public boolean equals(Object obj)
-    {
-        if (this == obj)
-        {
-            return true;
-        }
-        if (obj == null)
-        {
-            return false;
-        }
-        if (getClass() != obj.getClass())
-        {
-            return false;
-        }
-        PathSpec other = (PathSpec)obj;
-        if (pathSpec == null)
-        {
-            if (other.pathSpec != null)
-            {
-                return false;
-            }
-        }
-        else if (!pathSpec.equals(other.pathSpec))
-        {
-            return false;
-        }
-        return true;
-    }
-
-    public PathSpecGroup getGroup()
-    {
-        return group;
-    }
 
     /**
      * Get the number of path elements that this path spec declares.
@@ -93,10 +46,7 @@ public abstract class PathSpec implements Comparable<PathSpec>
      *
      * @return the depth of the path segments that this spec declares
      */
-    public int getPathDepth()
+    int getPathDepth();
-    {
-        return pathDepth;
-    }
 
     /**
      * Return the portion of the path that is after the path spec.
@@ -104,7 +54,7 @@ public abstract class PathSpec implements Comparable<PathSpec>
      * @param path the path to match against
      * @return the path info portion of the string
      */
-    public abstract String getPathInfo(String path);
+    String getPathInfo(String path);
 
     /**
      * Return the portion of the path that matches a path spec.
@@ -112,55 +62,28 @@ public abstract class PathSpec implements Comparable<PathSpec>
      * @param path the path to match against
      * @return the match, or null if no match at all
      */
-    public abstract String getPathMatch(String path);
+    String getPathMatch(String path);
 
     /**
      * The as-provided path spec.
      *
      * @return the as-provided path spec
      */
-    public String getDeclaration()
+    String getDeclaration();
-    {
-        return pathSpec;
-    }
 
     /**
      * A simple prefix match for the pathspec or null
      *
      * @return A simple prefix match for the pathspec or null
      */
-    public String getPrefix()
+    String getPrefix();
-    {
-        return prefix;
-    }
 
     /**
      * A simple suffix match for the pathspec or null
      *
      * @return A simple suffix match for the pathspec or null
      */
-    public String getSuffix()
+    String getSuffix();
-    {
-        return suffix;
-    }
-
-    /**
-     * Get the relative path.
-     *
-     * @param base the base the path is relative to
-     * @param path the additional path
-     * @return the base plus path with pathSpec portion removed
-     */
-    public abstract String getRelativePath(String base, String path);
-
-    @Override
-    public int hashCode()
-    {
-        final int prime = 31;
-        int result = 1;
-        result = (prime * result) + ((pathSpec == null) ? 0 : pathSpec.hashCode());
-        return result;
-    }
 
     /**
      * Test to see if the provided path matches this path spec
@@ -168,17 +91,5 @@ public abstract class PathSpec implements Comparable<PathSpec>
      * @param path the path to test
      * @return true if the path matches this path spec, false otherwise
      */
-    public abstract boolean matches(String path);
+    boolean matches(String path);
-
-    @Override
-    public String toString()
-    {
-        StringBuilder str = new StringBuilder();
-        str.append(this.getClass().getSimpleName()).append("[\"");
-        str.append(pathSpec);
-        str.append("\",pathDepth=").append(pathDepth);
-        str.append(",group=").append(group);
-        str.append("]");
-        return str.toString();
-    }
 }

jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/PathSpecGroup.java

@@ -26,8 +26,8 @@ package org.eclipse.jetty.http.pathmap;
  * Search Order:
  * <ol>
  * <li>{@link PathSpecGroup#ordinal()} [increasing]</li>
- * <li>{@link PathSpec#specLength} [decreasing]</li>
+ * <li>{@link PathSpec#getSpecLength()} [decreasing]</li>
- * <li>{@link PathSpec#pathSpec} [natural sort order]</li>
+ * <li>{@link PathSpec#getDeclaration()} [natural sort order]</li>
  * </ol>
  */
 public enum PathSpecGroup

jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/RegexPathSpec.java

@@ -21,27 +21,30 @@ package org.eclipse.jetty.http.pathmap;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-public class RegexPathSpec extends PathSpec
+public class RegexPathSpec extends AbstractPathSpec
 {
-    protected Pattern pattern;
+    private final String _declaration;
-
+    private final PathSpecGroup _group;
-    protected RegexPathSpec()
+    private final int _pathDepth;
-    {
+    private final int _specLength;
-        super();
+    private final Pattern _pattern;
-    }
 
     public RegexPathSpec(String regex)
     {
-        super.pathSpec = regex;
+        String declaration;
         if (regex.startsWith("regex|"))
-            super.pathSpec = regex.substring("regex|".length());
+            declaration = regex.substring("regex|".length());
-        this.pathDepth = 0;
+        else
-        this.specLength = pathSpec.length();
+            declaration = regex;
+        int specLength = declaration.length();
         // build up a simple signature we can use to identify the grouping
         boolean inGrouping = false;
         StringBuilder signature = new StringBuilder();
-        for (char c : pathSpec.toCharArray())
+
+        int pathDepth = 0;
+        for (int i = 0; i < declaration.length(); i++)
         {
+            char c = declaration.charAt(i);
             switch (c)
             {
                 case '[':
@@ -56,54 +59,64 @@ public class RegexPathSpec extends PathSpec
                     break;
                 case '/':
                     if (!inGrouping)
-                    {
+                        pathDepth++;
-                        this.pathDepth++;
-                    }
                     break;
                 default:
-                    if (!inGrouping)
+                    if (!inGrouping && Character.isLetterOrDigit(c))
-                    {
+                        signature.append('l'); // literal (exact)
-                        if (Character.isLetterOrDigit(c))
-                        {
-                            signature.append('l'); // literal (exact)
-                        }
-                    }
                     break;
             }
         }
-        this.pattern = Pattern.compile(pathSpec);
+        Pattern pattern = Pattern.compile(declaration);
 
         // Figure out the grouping based on the signature
         String sig = signature.toString();
 
+        PathSpecGroup group;
         if (Pattern.matches("^l*$", sig))
-        {
+            group = PathSpecGroup.EXACT;
-            this.group = PathSpecGroup.EXACT;
-        }
         else if (Pattern.matches("^l*g+", sig))
-        {
+            group = PathSpecGroup.PREFIX_GLOB;
-            this.group = PathSpecGroup.PREFIX_GLOB;
-        }
         else if (Pattern.matches("^g+l+$", sig))
-        {
+            group = PathSpecGroup.SUFFIX_GLOB;
-            this.group = PathSpecGroup.SUFFIX_GLOB;
-        }
         else
-        {
+            group = PathSpecGroup.MIDDLE_GLOB;
-            this.group = PathSpecGroup.MIDDLE_GLOB;
+
-        }
+        _declaration = declaration;
+        _group = group;
+        _pathDepth = pathDepth;
+        _specLength = specLength;
+        _pattern = pattern;
     }
 
-    public Matcher getMatcher(String path)
+    protected Matcher getMatcher(String path)
     {
-        return this.pattern.matcher(path);
+        return _pattern.matcher(path);
+    }
+
+    @Override
+    public int getSpecLength()
+    {
+        return _specLength;
+    }
+
+    @Override
+    public PathSpecGroup getGroup()
+    {
+        return _group;
+    }
+
+    @Override
+    public int getPathDepth()
+    {
+        return _pathDepth;
     }
 
     @Override
     public String getPathInfo(String path)
     {
         // Path Info only valid for PREFIX_GLOB types
-        if (group == PathSpecGroup.PREFIX_GLOB)
+        if (_group == PathSpecGroup.PREFIX_GLOB)
         {
             Matcher matcher = getMatcher(path);
             if (matcher.matches())
@@ -112,13 +125,9 @@ public class RegexPathSpec extends PathSpec
                 {
                     String pathInfo = matcher.group(1);
                     if ("".equals(pathInfo))
-                    {
                         return "/";
-                    }
                     else
-                    {
                         return pathInfo;
-                    }
                 }
             }
         }
@@ -137,9 +146,7 @@ public class RegexPathSpec extends PathSpec
                 if (idx > 0)
                 {
                     if (path.charAt(idx - 1) == '/')
-                    {
                         idx--;
-                    }
                     return path.substring(0, idx);
                 }
             }
@@ -148,18 +155,29 @@ public class RegexPathSpec extends PathSpec
         return null;
     }
 
-    public Pattern getPattern()
+    @Override
+    public String getDeclaration()
     {
-        return this.pattern;
+        return _declaration;
     }
 
     @Override
-    public String getRelativePath(String base, String path)
+    public String getPrefix()
     {
-        // TODO Auto-generated method stub
         return null;
     }
 
+    @Override
+    public String getSuffix()
+    {
+        return null;
+    }
+
+    public Pattern getPattern()
+    {
+        return _pattern;
+    }
+
     @Override
     public boolean matches(final String path)
     {

jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/ServletPathSpec.java

@@ -23,11 +23,17 @@ import org.eclipse.jetty.util.URIUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-public class ServletPathSpec extends PathSpec
+public class ServletPathSpec extends AbstractPathSpec
 {
-
     private static final Logger LOG = LoggerFactory.getLogger(ServletPathSpec.class);
 
+    private final String _declaration;
+    private final PathSpecGroup _group;
+    private final int _pathDepth;
+    private final int _specLength;
+    private final String _prefix;
+    private final String _suffix;
+
     /**
      * If a servlet or filter path mapping isn't a suffix mapping, ensure
      * it starts with '/'
@@ -197,70 +203,79 @@ public class ServletPathSpec extends PathSpec
         // The Root Path Spec
         if (servletPathSpec.isEmpty())
         {
-            super.pathSpec = "";
+            _declaration = "";
-            super.pathDepth = -1; // force this to be at the end of the sort order
+            _group = PathSpecGroup.ROOT;
-            this.specLength = 1;
+            _pathDepth = -1; // Set pathDepth to -1 to force this to be at the end of the sort order.
-            this.group = PathSpecGroup.ROOT;
+            _specLength = 1;
+            _prefix = null;
+            _suffix = null;
             return;
         }
 
         // The Default Path Spec
         if ("/".equals(servletPathSpec))
         {
-            super.pathSpec = "/";
+            _declaration = "/";
-            super.pathDepth = -1; // force this to be at the end of the sort order
+            _group = PathSpecGroup.DEFAULT;
-            this.specLength = 1;
+            _pathDepth = -1; // Set pathDepth to -1 to force this to be at the end of the sort order.
-            this.group = PathSpecGroup.DEFAULT;
+            _specLength = 1;
+            _prefix = null;
+            _suffix = null;
             return;
         }
 
-        this.specLength = servletPathSpec.length();
+        int specLength = servletPathSpec.length();
-        super.pathDepth = 0;
+        PathSpecGroup group;
-        char lastChar = servletPathSpec.charAt(specLength - 1);
+        String prefix;
+        String suffix;
+
         // prefix based
         if (servletPathSpec.charAt(0) == '/' && servletPathSpec.endsWith("/*"))
         {
-            this.group = PathSpecGroup.PREFIX_GLOB;
+            group = PathSpecGroup.PREFIX_GLOB;
-            this.prefix = servletPathSpec.substring(0, specLength - 2);
+            prefix = servletPathSpec.substring(0, specLength - 2);
+            suffix = null;
         }
         // suffix based
         else if (servletPathSpec.charAt(0) == '*' && servletPathSpec.length() > 1)
         {
-            this.group = PathSpecGroup.SUFFIX_GLOB;
+            group = PathSpecGroup.SUFFIX_GLOB;
-            this.suffix = servletPathSpec.substring(2, specLength);
+            prefix = null;
+            suffix = servletPathSpec.substring(2, specLength);
         }
         else
         {
-            this.group = PathSpecGroup.EXACT;
+            group = PathSpecGroup.EXACT;
-            this.prefix = servletPathSpec;
+            prefix = servletPathSpec;
+            suffix = null;
             if (servletPathSpec.endsWith("*"))
             {
                 LOG.warn("Suspicious URL pattern: '{}'; see sections 12.1 and 12.2 of the Servlet specification",
-                        servletPathSpec);
+                    servletPathSpec);
             }
         }
 
+        int pathDepth = 0;
         for (int i = 0; i < specLength; i++)
         {
             int cp = servletPathSpec.codePointAt(i);
             if (cp < 128)
             {
                 char c = (char)cp;
-                switch (c)
+                if (c == '/')
-                {
+                    pathDepth++;
-                    case '/':
-                        super.pathDepth++;
-                        break;
-                    default:
-                        break;
-                }
             }
         }
 
-        super.pathSpec = servletPathSpec;
+        _declaration = servletPathSpec;
+        _group = group;
+        _pathDepth = pathDepth;
+        _specLength = specLength;
+        _prefix = prefix;
+        _suffix = suffix;
     }
 
-    private void assertValidServletPathSpec(String servletPathSpec)
+    private static void assertValidServletPathSpec(String servletPathSpec)
     {
         if ((servletPathSpec == null) || servletPathSpec.equals(""))
         {
@@ -293,16 +308,12 @@ public class ServletPathSpec extends PathSpec
             int idx = servletPathSpec.indexOf('/');
             // cannot have path separator
             if (idx >= 0)
-            {
                 throw new IllegalArgumentException("Servlet Spec 12.2 violation: suffix based path spec cannot have path separators: bad spec \"" + servletPathSpec + "\"");
-            }
 
             idx = servletPathSpec.indexOf('*', 2);
             // only allowed to have 1 glob '*', at the start of the path spec
             if (idx >= 1)
-            {
                 throw new IllegalArgumentException("Servlet Spec 12.2 violation: suffix based path spec cannot have multiple glob '*': bad spec \"" + servletPathSpec + "\"");
-            }
         }
         else
         {
@@ -310,18 +321,36 @@ public class ServletPathSpec extends PathSpec
         }
     }
 
+    @Override
+    public int getSpecLength()
+    {
+        return _specLength;
+    }
+
+    @Override
+    public PathSpecGroup getGroup()
+    {
+        return _group;
+    }
+
+    @Override
+    public int getPathDepth()
+    {
+        return _pathDepth;
+    }
+
     @Override
     public String getPathInfo(String path)
     {
-        switch (group)
+        switch (_group)
         {
             case ROOT:
                 return path;
 
             case PREFIX_GLOB:
-                if (path.length() == (specLength - 2))
+                if (path.length() == (_specLength - 2))
                     return null;
-                return path.substring(specLength - 2);
+                return path.substring(_specLength - 2);
 
             default:
                 return null;
@@ -331,23 +360,23 @@ public class ServletPathSpec extends PathSpec
     @Override
     public String getPathMatch(String path)
     {
-        switch (group)
+        switch (_group)
         {
             case ROOT:
                 return "";
 
             case EXACT:
-                if (pathSpec.equals(path))
+                if (_declaration.equals(path))
                     return path;
                 return null;
 
             case PREFIX_GLOB:
                 if (isWildcardMatch(path))
-                    return path.substring(0, specLength - 2);
+                    return path.substring(0, _specLength - 2);
                 return null;
 
             case SUFFIX_GLOB:
-                if (path.regionMatches(path.length() - (specLength - 1), pathSpec, 1, specLength - 1))
+                if (path.regionMatches(path.length() - (_specLength - 1), _declaration, 1, _specLength - 1))
                     return path;
                 return null;
 
@@ -360,65 +389,43 @@ public class ServletPathSpec extends PathSpec
     }
 
     @Override
-    public String getRelativePath(String base, String path)
+    public String getDeclaration()
     {
-        String info = getPathInfo(path);
+        return _declaration;
-        if (info == null)
+    }
-        {
-            info = path;
-        }
 
-        if (info.startsWith("./"))
+    @Override
-        {
+    public String getPrefix()
-            info = info.substring(2);
+    {
-        }
+        return _prefix;
-        if (base.endsWith(URIUtil.SLASH))
+    }
-        {
+
-            if (info.startsWith(URIUtil.SLASH))
+    @Override
-            {
+    public String getSuffix()
-                path = base + info.substring(1);
+    {
-            }
+        return _suffix;
-            else
-            {
-                path = base + info;
-            }
-        }
-        else if (info.startsWith(URIUtil.SLASH))
-        {
-            path = base + info;
-        }
-        else
-        {
-            path = base + URIUtil.SLASH + info;
-        }
-        return path;
     }
 
     private boolean isWildcardMatch(String path)
     {
         // For a spec of "/foo/*" match "/foo" , "/foo/..." but not "/foobar"
-        int cpl = specLength - 2;
+        int cpl = _specLength - 2;
-        if ((group == PathSpecGroup.PREFIX_GLOB) && (path.regionMatches(0, pathSpec, 0, cpl)))
+        if ((_group == PathSpecGroup.PREFIX_GLOB) && (path.regionMatches(0, _declaration, 0, cpl)))
-        {
+            return (path.length() == cpl) || ('/' == path.charAt(cpl));
-            if ((path.length() == cpl) || ('/' == path.charAt(cpl)))
-            {
-                return true;
-            }
-        }
         return false;
     }
 
     @Override
     public boolean matches(String path)
     {
-        switch (group)
+        switch (_group)
         {
             case EXACT:
-                return pathSpec.equals(path);
+                return _declaration.equals(path);
             case PREFIX_GLOB:
                 return isWildcardMatch(path);
             case SUFFIX_GLOB:
-                return path.regionMatches((path.length() - specLength) + 1, pathSpec, 1, specLength - 1);
+                return path.regionMatches((path.length() - _specLength) + 1, _declaration, 1, _specLength - 1);
             case ROOT:
                 // Only "/" matches
                 return ("/".equals(path));

jetty-http/src/main/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpec.java

@@ -30,7 +30,6 @@ import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import org.eclipse.jetty.util.TypeUtil;
-import org.eclipse.jetty.util.UrlEncoded;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -39,7 +38,7 @@ import org.slf4j.LoggerFactory;
  *
  * @see <a href="https://tools.ietf.org/html/rfc6570">URI Templates (Level 1)</a>
  */
-public class UriTemplatePathSpec extends RegexPathSpec
+public class UriTemplatePathSpec extends AbstractPathSpec
 {
     private static final Logger LOG = LoggerFactory.getLogger(UriTemplatePathSpec.class);
 
@@ -63,49 +62,46 @@ public class UriTemplatePathSpec extends RegexPathSpec
         FORBIDDEN_SEGMENTS.add("//");
     }
 
-    private String[] variables;
+    private final String _declaration;
+    private final PathSpecGroup _group;
+    private final int _pathDepth;
+    private final int _specLength;
+    private final Pattern _pattern;
+    private final String[] _variables;
+    /**
+     * The logical (simplified) declaration
+     */
+    private final String _logicalDeclaration;
 
     public UriTemplatePathSpec(String rawSpec)
     {
-        super();
         Objects.requireNonNull(rawSpec, "Path Param Spec cannot be null");
 
         if ("".equals(rawSpec) || "/".equals(rawSpec))
         {
-            super.pathSpec = "/";
+            _declaration = "/";
-            super.pattern = Pattern.compile("^/$");
+            _group = PathSpecGroup.EXACT;
-            super.pathDepth = 1;
+            _pathDepth = 1;
-            this.specLength = 1;
+            _specLength = 1;
-            this.variables = new String[0];
+            _pattern = Pattern.compile("^/$");
-            this.group = PathSpecGroup.EXACT;
+            _variables = new String[0];
+            _logicalDeclaration = "/";
             return;
         }
 
         if (rawSpec.charAt(0) != '/')
         {
             // path specs must start with '/'
-            StringBuilder err = new StringBuilder();
+            throw new IllegalArgumentException("Syntax Error: path spec \"" + rawSpec + "\" must start with '/'");
-            err.append("Syntax Error: path spec \"");
-            err.append(rawSpec);
-            err.append("\" must start with '/'");
-            throw new IllegalArgumentException(err.toString());
         }
 
         for (String forbidden : FORBIDDEN_SEGMENTS)
         {
             if (rawSpec.contains(forbidden))
-            {
+                throw new IllegalArgumentException("Syntax Error: segment " + forbidden + " is forbidden in path spec: " + rawSpec);
-                StringBuilder err = new StringBuilder();
-                err.append("Syntax Error: segment ");
-                err.append(forbidden);
-                err.append(" is forbidden in path spec: ");
-                err.append(rawSpec);
-                throw new IllegalArgumentException(err.toString());
-            }
         }
 
-        this.pathSpec = rawSpec;
+        String declaration = rawSpec;
-
         StringBuilder regex = new StringBuilder();
         regex.append('^');
 
@@ -113,7 +109,8 @@ public class UriTemplatePathSpec extends RegexPathSpec
         // split up into path segments (ignoring the first slash that will always be empty)
         String[] segments = rawSpec.substring(1).split("/");
         char[] segmentSignature = new char[segments.length];
-        this.pathDepth = segments.length;
+        StringBuilder logicalSignature = new StringBuilder();
+        int pathDepth = segments.length;
         for (int i = 0; i < segments.length; i++)
         {
             String segment = segments[i];
@@ -126,17 +123,13 @@ public class UriTemplatePathSpec extends RegexPathSpec
                 if (varNames.contains(variable))
                 {
                     // duplicate variable names
-                    StringBuilder err = new StringBuilder();
+                    throw new IllegalArgumentException("Syntax Error: variable " + variable + " is duplicated in path spec: " + rawSpec);
-                    err.append("Syntax Error: variable ");
-                    err.append(variable);
-                    err.append(" is duplicated in path spec: ");
-                    err.append(rawSpec);
-                    throw new IllegalArgumentException(err.toString());
                 }
 
-                assertIsValidVariableLiteral(variable);
+                assertIsValidVariableLiteral(variable, declaration);
 
                 segmentSignature[i] = 'v'; // variable
+                logicalSignature.append("/*");
                 // valid variable name
                 varNames.add(variable);
                 // build regex
@@ -145,46 +138,31 @@ public class UriTemplatePathSpec extends RegexPathSpec
             else if (mat.find(0))
             {
                 // variable exists as partial segment
-                StringBuilder err = new StringBuilder();
+                throw new IllegalArgumentException("Syntax Error: variable " + mat.group() + " must exist as entire path segment: " + rawSpec);
-                err.append("Syntax Error: variable ");
-                err.append(mat.group());
-                err.append(" must exist as entire path segment: ");
-                err.append(rawSpec);
-                throw new IllegalArgumentException(err.toString());
             }
             else if ((segment.indexOf('{') >= 0) || (segment.indexOf('}') >= 0))
             {
                 // variable is split with a path separator
-                StringBuilder err = new StringBuilder();
+                throw new IllegalArgumentException("Syntax Error: invalid path segment /" + segment + "/ variable declaration incomplete: " + rawSpec);
-                err.append("Syntax Error: invalid path segment /");
-                err.append(segment);
-                err.append("/ variable declaration incomplete: ");
-                err.append(rawSpec);
-                throw new IllegalArgumentException(err.toString());
             }
             else if (segment.indexOf('*') >= 0)
             {
                 // glob segment
-                StringBuilder err = new StringBuilder();
+                throw new IllegalArgumentException("Syntax Error: path segment /" + segment + "/ contains a wildcard symbol (not supported by this uri-template implementation): " + rawSpec);
-                err.append("Syntax Error: path segment /");
-                err.append(segment);
-                err.append("/ contains a wildcard symbol (not supported by this uri-template implementation): ");
-                err.append(rawSpec);
-                throw new IllegalArgumentException(err.toString());
             }
             else
             {
                 // valid path segment
                 segmentSignature[i] = 'e'; // exact
+                logicalSignature.append('/').append(segment);
                 // build regex
                 regex.append('/');
                 // escape regex special characters
-                for (char c : segment.toCharArray())
+                for (int j = 0; j < segment.length(); j++)
                 {
+                    char c = segment.charAt(j);
                     if ((c == '.') || (c == '[') || (c == ']') || (c == '\\'))
-                    {
                         regex.append('\\');
-                    }
                     regex.append(c);
                 }
             }
@@ -194,40 +172,42 @@ public class UriTemplatePathSpec extends RegexPathSpec
         if (rawSpec.charAt(rawSpec.length() - 1) == '/')
         {
             regex.append('/');
+            logicalSignature.append('/');
         }
 
         regex.append('$');
 
-        this.pattern = Pattern.compile(regex.toString());
+        Pattern pattern = Pattern.compile(regex.toString());
 
         int varcount = varNames.size();
-        this.variables = varNames.toArray(new String[varcount]);
+        String[] variables = varNames.toArray(new String[varcount]);
 
         // Convert signature to group
         String sig = String.valueOf(segmentSignature);
 
+        PathSpecGroup group;
         if (Pattern.matches("^e*$", sig))
-        {
+            group = PathSpecGroup.EXACT;
-            this.group = PathSpecGroup.EXACT;
-        }
         else if (Pattern.matches("^e*v+", sig))
-        {
+            group = PathSpecGroup.PREFIX_GLOB;
-            this.group = PathSpecGroup.PREFIX_GLOB;
-        }
         else if (Pattern.matches("^v+e+", sig))
-        {
+            group = PathSpecGroup.SUFFIX_GLOB;
-            this.group = PathSpecGroup.SUFFIX_GLOB;
-        }
         else
-        {
+            group = PathSpecGroup.MIDDLE_GLOB;
-            this.group = PathSpecGroup.MIDDLE_GLOB;
+
-        }
+        _declaration = declaration;
+        _group = group;
+        _pathDepth = pathDepth;
+        _specLength = declaration.length();
+        _pattern = pattern;
+        _variables = variables;
+        _logicalDeclaration = logicalSignature.toString();
     }
 
     /**
      * Validate variable literal name, per RFC6570, Section 2.1 Literals
      */
-    private void assertIsValidVariableLiteral(String variable)
+    private static void assertIsValidVariableLiteral(String variable, String declaration)
     {
         int len = variable.length();
 
@@ -241,16 +221,12 @@ public class UriTemplatePathSpec extends RegexPathSpec
             i += Character.charCount(codepoint);
 
             // basic letters, digits, or symbols
-            if (isValidBasicLiteralCodepoint(codepoint))
+            if (isValidBasicLiteralCodepoint(codepoint, declaration))
-            {
                 continue;
-            }
 
             // The ucschar and iprivate pieces
             if (Character.isSupplementaryCodePoint(codepoint))
-            {
                 continue;
-            }
 
             // pct-encoded
             if (codepoint == '%')
@@ -265,10 +241,8 @@ public class UriTemplatePathSpec extends RegexPathSpec
                 codepoint |= TypeUtil.convertHexDigit(variable.codePointAt(i++));
 
                 // validate basic literal
-                if (isValidBasicLiteralCodepoint(codepoint))
+                if (isValidBasicLiteralCodepoint(codepoint, declaration))
-                {
                     continue;
-                }
             }
 
             valid = false;
@@ -277,69 +251,174 @@ public class UriTemplatePathSpec extends RegexPathSpec
         if (!valid)
         {
             // invalid variable name
-            StringBuilder err = new StringBuilder();
+            throw new IllegalArgumentException("Syntax Error: variable {" + variable + "} an invalid variable name: " + declaration);
-            err.append("Syntax Error: variable {");
-            err.append(variable);
-            err.append("} an invalid variable name: ");
-            err.append(pathSpec);
-            throw new IllegalArgumentException(err.toString());
         }
     }
 
-    private boolean isValidBasicLiteralCodepoint(int codepoint)
+    private static boolean isValidBasicLiteralCodepoint(int codepoint, String declaration)
     {
         // basic letters or digits
         if ((codepoint >= 'a' && codepoint <= 'z') ||
             (codepoint >= 'A' && codepoint <= 'Z') ||
             (codepoint >= '0' && codepoint <= '9'))
-        {
             return true;
-        }
 
         // basic allowed symbols
         if (VARIABLE_SYMBOLS.indexOf(codepoint) >= 0)
-        {
             return true; // valid simple value
-        }
 
         // basic reserved symbols
         if (VARIABLE_RESERVED.indexOf(codepoint) >= 0)
         {
-            LOG.warn("Detected URI Template reserved symbol [{}] in path spec \"{}\"", (char)codepoint, pathSpec);
+            LOG.warn("Detected URI Template reserved symbol [{}] in path spec \"{}\"", (char)codepoint, declaration);
             return false; // valid simple value
         }
 
         return false;
     }
 
+    @Override
+    public int compareTo(PathSpec other)
+    {
+        if (other instanceof UriTemplatePathSpec)
+        {
+            UriTemplatePathSpec otherUriPathSpec = (UriTemplatePathSpec)other;
+            return otherUriPathSpec._logicalDeclaration.compareTo(this._logicalDeclaration);
+        }
+        else
+        {
+            return super.compareTo(other);
+        }
+    }
+
     public Map<String, String> getPathParams(String path)
     {
         Matcher matcher = getMatcher(path);
         if (matcher.matches())
         {
-            if (group == PathSpecGroup.EXACT)
+            if (_group == PathSpecGroup.EXACT)
-            {
                 return Collections.emptyMap();
-            }
             Map<String, String> ret = new HashMap<>();
             int groupCount = matcher.groupCount();
             for (int i = 1; i <= groupCount; i++)
-            {
+                ret.put(_variables[i - 1], matcher.group(i));
-                String value = UrlEncoded.decodeString(matcher.group(i));
-                ret.put(this.variables[i - 1], value);
-            }
             return ret;
         }
         return null;
     }
 
+    protected Matcher getMatcher(String path)
+    {
+        return _pattern.matcher(path);
+    }
+
+    @Override
+    public int getSpecLength()
+    {
+        return _specLength;
+    }
+
+    @Override
+    public PathSpecGroup getGroup()
+    {
+        return _group;
+    }
+
+    @Override
+    public int getPathDepth()
+    {
+        return _pathDepth;
+    }
+
+    @Override
+    public String getPathInfo(String path)
+    {
+        // Path Info only valid for PREFIX_GLOB types
+        if (_group == PathSpecGroup.PREFIX_GLOB)
+        {
+            Matcher matcher = getMatcher(path);
+            if (matcher.matches())
+            {
+                if (matcher.groupCount() >= 1)
+                {
+                    String pathInfo = matcher.group(1);
+                    if ("".equals(pathInfo))
+                        return "/";
+                    else
+                        return pathInfo;
+                }
+            }
+        }
+        return null;
+    }
+
+    @Override
+    public String getPathMatch(String path)
+    {
+        Matcher matcher = getMatcher(path);
+        if (matcher.matches())
+        {
+            if (matcher.groupCount() >= 1)
+            {
+                int idx = matcher.start(1);
+                if (idx > 0)
+                {
+                    if (path.charAt(idx - 1) == '/')
+                        idx--;
+                    return path.substring(0, idx);
+                }
+            }
+            return path;
+        }
+        return null;
+    }
+
+    @Override
+    public String getDeclaration()
+    {
+        return _declaration;
+    }
+
+    @Override
+    public String getPrefix()
+    {
+        return null;
+    }
+
+    @Override
+    public String getSuffix()
+    {
+        return null;
+    }
+
+    public Pattern getPattern()
+    {
+        return _pattern;
+    }
+
+    @Override
+    public boolean matches(final String path)
+    {
+        int idx = path.indexOf('?');
+        if (idx >= 0)
+        {
+            // match only non-query part
+            return getMatcher(path.substring(0, idx)).matches();
+        }
+        else
+        {
+            // match entire path
+            return getMatcher(path).matches();
+        }
+    }
+
     public int getVariableCount()
     {
-        return variables.length;
+        return _variables.length;
     }
 
     public String[] getVariables()
     {
-        return this.variables;
+        return _variables;
     }
 }

jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/PathMappingsTest.java

@@ -22,8 +22,11 @@ import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
 
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -199,7 +202,7 @@ public class PathMappingsTest
         p.put(new ServletPathSpec("/*"), "0");
 
         // assertEquals("1", p.get("/abs/path"), "Get absolute path");
-        assertEquals("/abs/path", p.getMatch("/abs/path").getPathSpec().pathSpec, "Match absolute path");
+        assertEquals("/abs/path", p.getMatch("/abs/path").getPathSpec().getDeclaration(), "Match absolute path");
         assertEquals("1", p.getMatch("/abs/path").getResource(), "Match absolute path");
         assertEquals("0", p.getMatch("/abs/path/xxx").getResource(), "Mismatch absolute path");
         assertEquals("0", p.getMatch("/abs/pith").getResource(), "Mismatch absolute path");
@@ -302,4 +305,160 @@ public class PathMappingsTest
             new ServletPathSpec(str);
         });
     }
+
+    @Test
+    public void testPutRejectsDuplicates()
+    {
+        PathMappings<String> p = new PathMappings<>();
+        assertThat(p.put(new UriTemplatePathSpec("/a/{var1}/c"), "resourceA"), is(true));
+        assertThat(p.put(new UriTemplatePathSpec("/a/{var2}/c"), "resourceAA"), is(false));
+        assertThat(p.put(new UriTemplatePathSpec("/a/b/c"), "resourceB"), is(true));
+        assertThat(p.put(new UriTemplatePathSpec("/a/b/c"), "resourceBB"), is(false));
+        assertThat(p.put(new ServletPathSpec("/a/b/c"), "resourceBB"), is(false));
+        assertThat(p.put(new RegexPathSpec("/a/b/c"), "resourceBB"), is(false));
+
+        assertThat(p.put(new ServletPathSpec("/*"), "resourceC"), is(true));
+        assertThat(p.put(new RegexPathSpec("/(.*)"), "resourceCC"), is(true));
+    }
+
+    @Test
+    public void testGetUriTemplatePathSpec()
+    {
+        PathMappings<String> p = new PathMappings<>();
+        p.put(new UriTemplatePathSpec("/a/{var1}/c"), "resourceA");
+        p.put(new UriTemplatePathSpec("/a/b/c"), "resourceB");
+
+        assertThat(p.get(new UriTemplatePathSpec("/a/{var1}/c")), equalTo("resourceA"));
+        assertThat(p.get(new UriTemplatePathSpec("/a/{foo}/c")), equalTo("resourceA"));
+        assertThat(p.get(new UriTemplatePathSpec("/a/b/c")), equalTo("resourceB"));
+        assertThat(p.get(new UriTemplatePathSpec("/a/d/c")), nullValue());
+        assertThat(p.get(new RegexPathSpec("/a/b/c")), nullValue());
+    }
+
+    @Test
+    public void testGetRegexPathSpec()
+    {
+        PathMappings<String> p = new PathMappings<>();
+        p.put(new RegexPathSpec("/a/b/c"), "resourceA");
+        p.put(new RegexPathSpec("/(.*)/b/c"), "resourceB");
+        p.put(new RegexPathSpec("/a/(.*)/c"), "resourceC");
+        p.put(new RegexPathSpec("/a/b/(.*)"), "resourceD");
+
+        assertThat(p.get(new RegexPathSpec("/a/(.*)/c")), equalTo("resourceC"));
+        assertThat(p.get(new RegexPathSpec("/a/b/c")), equalTo("resourceA"));
+        assertThat(p.get(new RegexPathSpec("/(.*)/b/c")), equalTo("resourceB"));
+        assertThat(p.get(new RegexPathSpec("/a/b/(.*)")), equalTo("resourceD"));
+        assertThat(p.get(new RegexPathSpec("/a/d/c")), nullValue());
+        assertThat(p.get(new ServletPathSpec("/a/b/c")), nullValue());
+    }
+
+    @Test
+    public void testGetServletPathSpec()
+    {
+        PathMappings<String> p = new PathMappings<>();
+        p.put(new ServletPathSpec("/"), "resourceA");
+        p.put(new ServletPathSpec("/*"), "resourceB");
+        p.put(new ServletPathSpec("/a/*"), "resourceC");
+        p.put(new ServletPathSpec("*.do"), "resourceD");
+
+        assertThat(p.get(new ServletPathSpec("/")), equalTo("resourceA"));
+        assertThat(p.get(new ServletPathSpec("/*")), equalTo("resourceB"));
+        assertThat(p.get(new ServletPathSpec("/a/*")), equalTo("resourceC"));
+        assertThat(p.get(new ServletPathSpec("*.do")), equalTo("resourceD"));
+        assertThat(p.get(new ServletPathSpec("*.gz")), nullValue());
+        assertThat(p.get(new ServletPathSpec("/a/b/*")), nullValue());
+        assertThat(p.get(new ServletPathSpec("/a/d/c")), nullValue());
+        assertThat(p.get(new RegexPathSpec("/a/b/c")), nullValue());
+    }
+
+    @Test
+    public void testRemoveUriTemplatePathSpec()
+    {
+        PathMappings<String> p = new PathMappings<>();
+
+        p.put(new UriTemplatePathSpec("/a/{var1}/c"), "resourceA");
+        assertThat(p.remove(new UriTemplatePathSpec("/a/{var1}/c")), is(true));
+
+        p.put(new UriTemplatePathSpec("/a/{var1}/c"), "resourceA");
+        assertThat(p.remove(new UriTemplatePathSpec("/a/b/c")), is(false));
+        assertThat(p.remove(new UriTemplatePathSpec("/a/{b}/c")), is(true));
+        assertThat(p.remove(new UriTemplatePathSpec("/a/{b}/c")), is(false));
+
+        p.put(new UriTemplatePathSpec("/{var1}/b/c"), "resourceA");
+        assertThat(p.remove(new UriTemplatePathSpec("/a/b/c")), is(false));
+        assertThat(p.remove(new UriTemplatePathSpec("/{a}/b/c")), is(true));
+        assertThat(p.remove(new UriTemplatePathSpec("/{a}/b/c")), is(false));
+
+        p.put(new UriTemplatePathSpec("/a/b/{var1}"), "resourceA");
+        assertThat(p.remove(new UriTemplatePathSpec("/a/b/c")), is(false));
+        assertThat(p.remove(new UriTemplatePathSpec("/a/b/{c}")), is(true));
+        assertThat(p.remove(new UriTemplatePathSpec("/a/b/{c}")), is(false));
+
+        p.put(new UriTemplatePathSpec("/{var1}/{var2}/{var3}"), "resourceA");
+        assertThat(p.remove(new UriTemplatePathSpec("/a/b/c")), is(false));
+        assertThat(p.remove(new UriTemplatePathSpec("/{a}/{b}/{c}")), is(true));
+        assertThat(p.remove(new UriTemplatePathSpec("/{a}/{b}/{c}")), is(false));
+    }
+
+    @Test
+    public void testRemoveRegexPathSpec()
+    {
+        PathMappings<String> p = new PathMappings<>();
+
+        p.put(new RegexPathSpec("/a/(.*)/c"), "resourceA");
+        assertThat(p.remove(new RegexPathSpec("/a/b/c")), is(false));
+        assertThat(p.remove(new RegexPathSpec("/a/(.*)/c")), is(true));
+        assertThat(p.remove(new RegexPathSpec("/a/(.*)/c")), is(false));
+
+        p.put(new RegexPathSpec("/(.*)/b/c"), "resourceA");
+        assertThat(p.remove(new RegexPathSpec("/a/b/c")), is(false));
+        assertThat(p.remove(new RegexPathSpec("/(.*)/b/c")), is(true));
+        assertThat(p.remove(new RegexPathSpec("/(.*)/b/c")), is(false));
+
+        p.put(new RegexPathSpec("/a/b/(.*)"), "resourceA");
+        assertThat(p.remove(new RegexPathSpec("/a/b/c")), is(false));
+        assertThat(p.remove(new RegexPathSpec("/a/b/(.*)")), is(true));
+        assertThat(p.remove(new RegexPathSpec("/a/b/(.*)")), is(false));
+
+        p.put(new RegexPathSpec("/a/b/c"), "resourceA");
+        assertThat(p.remove(new RegexPathSpec("/a/b/d")), is(false));
+        assertThat(p.remove(new RegexPathSpec("/a/b/c")), is(true));
+        assertThat(p.remove(new RegexPathSpec("/a/b/c")), is(false));
+    }
+
+    @Test
+    public void testRemoveServletPathSpec()
+    {
+        PathMappings<String> p = new PathMappings<>();
+
+        p.put(new ServletPathSpec("/a/*"), "resourceA");
+        assertThat(p.remove(new ServletPathSpec("/a/b")), is(false));
+        assertThat(p.remove(new ServletPathSpec("/a/*")), is(true));
+        assertThat(p.remove(new ServletPathSpec("/a/*")), is(false));
+
+        p.put(new ServletPathSpec("/a/b/*"), "resourceA");
+        assertThat(p.remove(new ServletPathSpec("/a/b/c")), is(false));
+        assertThat(p.remove(new ServletPathSpec("/a/b/*")), is(true));
+        assertThat(p.remove(new ServletPathSpec("/a/b/*")), is(false));
+
+        p.put(new ServletPathSpec("*.do"), "resourceA");
+        assertThat(p.remove(new ServletPathSpec("*.gz")), is(false));
+        assertThat(p.remove(new ServletPathSpec("*.do")), is(true));
+        assertThat(p.remove(new ServletPathSpec("*.do")), is(false));
+
+        p.put(new ServletPathSpec("/"), "resourceA");
+        assertThat(p.remove(new ServletPathSpec("/a")), is(false));
+        assertThat(p.remove(new ServletPathSpec("/")), is(true));
+        assertThat(p.remove(new ServletPathSpec("/")), is(false));
+
+        p.put(new ServletPathSpec(""), "resourceA");
+        assertThat(p.remove(new ServletPathSpec("/")), is(false));
+        assertThat(p.remove(new ServletPathSpec("")), is(true));
+        assertThat(p.remove(new ServletPathSpec("")), is(false));
+
+        p.put(new ServletPathSpec("/a/b/c"), "resourceA");
+        assertThat(p.remove(new ServletPathSpec("/a/b/d")), is(false));
+        assertThat(p.remove(new ServletPathSpec("/a/b/c")), is(true));
+        assertThat(p.remove(new ServletPathSpec("/a/b/c")), is(false));
+    }
 }

jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/RegexPathSpecTest.java

@@ -21,7 +21,9 @@ package org.eclipse.jetty.http.pathmap;
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 public class RegexPathSpecTest
@@ -45,7 +47,7 @@ public class RegexPathSpecTest
         assertEquals("^/a$", spec.getDeclaration(), "Spec.pathSpec");
         assertEquals("^/a$", spec.getPattern().pattern(), "Spec.pattern");
         assertEquals(1, spec.getPathDepth(), "Spec.pathDepth");
-        assertEquals(PathSpecGroup.EXACT, spec.group, "Spec.group");
+        assertEquals(PathSpecGroup.EXACT, spec.getGroup(), "Spec.group");
 
         assertMatches(spec, "/a");
 
@@ -60,7 +62,7 @@ public class RegexPathSpecTest
         assertEquals("^/rest/([^/]*)/list$", spec.getDeclaration(), "Spec.pathSpec");
         assertEquals("^/rest/([^/]*)/list$", spec.getPattern().pattern(), "Spec.pattern");
         assertEquals(3, spec.getPathDepth(), "Spec.pathDepth");
-        assertEquals(PathSpecGroup.MIDDLE_GLOB, spec.group, "Spec.group");
+        assertEquals(PathSpecGroup.MIDDLE_GLOB, spec.getGroup(), "Spec.group");
 
         assertMatches(spec, "/rest/api/list");
         assertMatches(spec, "/rest/1.0/list");
@@ -81,7 +83,7 @@ public class RegexPathSpecTest
         assertEquals("^/rest/[^/]+/list$", spec.getDeclaration(), "Spec.pathSpec");
         assertEquals("^/rest/[^/]+/list$", spec.getPattern().pattern(), "Spec.pattern");
         assertEquals(3, spec.getPathDepth(), "Spec.pathDepth");
-        assertEquals(PathSpecGroup.MIDDLE_GLOB, spec.group, "Spec.group");
+        assertEquals(PathSpecGroup.MIDDLE_GLOB, spec.getGroup(), "Spec.group");
 
         assertMatches(spec, "/rest/api/list");
         assertMatches(spec, "/rest/1.0/list");
@@ -102,7 +104,7 @@ public class RegexPathSpecTest
         assertEquals("^/a/(.*)$", spec.getDeclaration(), "Spec.pathSpec");
         assertEquals("^/a/(.*)$", spec.getPattern().pattern(), "Spec.pattern");
         assertEquals(2, spec.getPathDepth(), "Spec.pathDepth");
-        assertEquals(PathSpecGroup.PREFIX_GLOB, spec.group, "Spec.group");
+        assertEquals(PathSpecGroup.PREFIX_GLOB, spec.getGroup(), "Spec.group");
 
         assertMatches(spec, "/a/");
         assertMatches(spec, "/a/b");
@@ -120,7 +122,7 @@ public class RegexPathSpecTest
         assertEquals("^(.*).do$", spec.getDeclaration(), "Spec.pathSpec");
         assertEquals("^(.*).do$", spec.getPattern().pattern(), "Spec.pattern");
         assertEquals(0, spec.getPathDepth(), "Spec.pathDepth");
-        assertEquals(PathSpecGroup.SUFFIX_GLOB, spec.group, "Spec.group");
+        assertEquals(PathSpecGroup.SUFFIX_GLOB, spec.getGroup(), "Spec.group");
 
         assertMatches(spec, "/a.do");
         assertMatches(spec, "/a/b/c.do");
@@ -132,4 +134,14 @@ public class RegexPathSpecTest
         assertNotMatches(spec, "/aa/bb");
         assertNotMatches(spec, "/aa/bb.do/more");
     }
+
+    @Test
+    public void testEquals()
+    {
+        assertThat(new RegexPathSpec("^(.*).do$"), equalTo(new RegexPathSpec("^(.*).do$")));
+        assertThat(new RegexPathSpec("/foo"), equalTo(new RegexPathSpec("/foo")));
+        assertThat(new RegexPathSpec("^(.*).do$"), not(equalTo(new RegexPathSpec("^(.*).gz$"))));
+        assertThat(new RegexPathSpec("^(.*).do$"), not(equalTo(new RegexPathSpec("^.*.do$"))));
+        assertThat(new RegexPathSpec("/foo"), not(equalTo(new ServletPathSpec("/foo"))));
+    }
 }

jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecMatchListTest.java

@@ -82,7 +82,7 @@ public class ServletPathSpecMatchListTest
         {
             if (delim)
                 actual.append(", ");
-            actual.append(res.getPathSpec().pathSpec).append('=').append(res.getResource());
+            actual.append(res.getPathSpec().getDeclaration()).append('=').append(res.getResource());
             delim = true;
         }
         actual.append(']');

jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/ServletPathSpecTest.java

@@ -21,7 +21,9 @@ package org.eclipse.jetty.http.pathmap;
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -187,4 +189,17 @@ public class ServletPathSpecTest
 
         assertEquals(null, spec.getPathInfo("/downloads/distribution.tar.gz"), "Spec.pathInfo");
     }
+
+    @Test
+    public void testEquals()
+    {
+        assertThat(new ServletPathSpec("*.gz"), equalTo(new ServletPathSpec("*.gz")));
+        assertThat(new ServletPathSpec("/foo"), equalTo(new ServletPathSpec("/foo")));
+        assertThat(new ServletPathSpec("/foo/bar"), equalTo(new ServletPathSpec("/foo/bar")));
+        assertThat(new ServletPathSpec("*.gz"), not(equalTo(new ServletPathSpec("*.do"))));
+        assertThat(new ServletPathSpec("/foo"), not(equalTo(new ServletPathSpec("/bar"))));
+        assertThat(new ServletPathSpec("/bar/foo"), not(equalTo(new ServletPathSpec("/foo/bar"))));
+        assertThat(new ServletPathSpec("/foo"), not(equalTo(new RegexPathSpec("/foo"))));
+    }
+
 }

jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/UriTemplatePathSpecTest.java

@@ -23,7 +23,9 @@ import java.util.Map;
 import org.junit.jupiter.api.Test;
 
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
@@ -281,4 +283,15 @@ public class UriTemplatePathSpecTest
         assertThat("Spec.pathParams.size", mapped.size(), is(1));
         assertEquals("a", mapped.get("var1"), "Spec.pathParams[var1]");
     }
+
+    @Test
+    public void testEquals()
+    {
+        assertThat(new UriTemplatePathSpec("/{var1}"), equalTo(new UriTemplatePathSpec("/{var1}")));
+        assertThat(new UriTemplatePathSpec("/{var1}"), equalTo(new UriTemplatePathSpec("/{var2}")));
+        assertThat(new UriTemplatePathSpec("/{var1}/{var2}"), equalTo(new UriTemplatePathSpec("/{var2}/{var1}")));
+        assertThat(new UriTemplatePathSpec("/{var1}"), not(equalTo(new UriTemplatePathSpec("/{var1}/{var2}"))));
+        assertThat(new UriTemplatePathSpec("/a/b/c"), not(equalTo(new UriTemplatePathSpec("/a/{var}/c"))));
+        assertThat(new UriTemplatePathSpec("/foo"), not(equalTo(new ServletPathSpec("/foo"))));
+    }
 }

jetty-http/src/test/java/org/eclipse/jetty/http/pathmap/WebSocketUriMappingTest.java

@@ -0,0 +1,182 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under
+// the terms of the Eclipse Public License 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0
+//
+// This Source Code may also be made available under the following
+// Secondary Licenses when the conditions for such availability set
+// forth in the Eclipse Public License, v. 2.0 are satisfied:
+// the Apache License v2.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.pathmap;
+
+import java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.jupiter.api.Assertions.assertNull;
+
+public class WebSocketUriMappingTest
+{
+    private PathMappings<String> mapping = new PathMappings<>();
+
+    private String getBestMatch(String uriPath)
+    {
+        List<MappedResource<String>> resources = mapping.getMatches(uriPath);
+        assertThat("Matches on " + uriPath, resources, is(not(nullValue())));
+        if (resources.isEmpty())
+            return null;
+        return resources.get(0).getResource();
+    }
+
+    @Test
+    public void testJsrExampleI()
+    {
+        mapping.put("/a/b", "endpointA");
+
+        assertThat(getBestMatch("/a/b"), is("endpointA"));
+        assertNull(getBestMatch("/a/c"));
+    }
+
+    @Test
+    public void testJsrExampleII()
+    {
+        mapping.put(new UriTemplatePathSpec("/a/{var}"), "endpointA");
+
+        assertThat(getBestMatch("/a/b"), is("endpointA"));
+        assertThat(getBestMatch("/a/apple"), is("endpointA"));
+        assertNull(getBestMatch("/a"));
+        assertNull(getBestMatch("/a/b/c"));
+    }
+
+    @Test
+    public void testJsrExampleIII()
+    {
+        mapping.put(new UriTemplatePathSpec("/a/{var}/c"), "endpointA");
+        mapping.put(new UriTemplatePathSpec("/a/b/c"), "endpointB");
+        mapping.put(new UriTemplatePathSpec("/a/{var1}/{var2}"), "endpointC");
+
+        assertThat(getBestMatch("/a/b/c"), is("endpointB"));
+        assertThat(getBestMatch("/a/d/c"), is("endpointA"));
+        assertThat(getBestMatch("/a/x/y"), is("endpointC"));
+    }
+
+    @Test
+    public void testJsrExampleIV()
+    {
+        mapping.put(new UriTemplatePathSpec("/{var1}/d"), "endpointA");
+        mapping.put(new UriTemplatePathSpec("/b/{var2}"), "endpointB");
+
+        assertThat(getBestMatch("/b/d"), is("endpointB"));
+    }
+
+    @Test
+    public void testPrefixVsSuffix()
+    {
+        mapping.put(new UriTemplatePathSpec("/{a}/b"), "suffix");
+        mapping.put(new UriTemplatePathSpec("/{a}/{b}"), "prefix");
+
+        List<MappedResource<String>> matches = mapping.getMatches("/a/b");
+
+        assertThat(getBestMatch("/a/b"), is("suffix"));
+    }
+
+    @Test
+    public void testMiddleVsSuffix()
+    {
+        mapping.put(new UriTemplatePathSpec("/a/{b}/c"), "middle");
+        mapping.put(new UriTemplatePathSpec("/a/b/{c}"), "suffix");
+
+        assertThat(getBestMatch("/a/b/c"), is("suffix"));
+    }
+
+    @Test
+    public void testMiddleVsSuffix2()
+    {
+        mapping.put(new UriTemplatePathSpec("/{a}/b/{c}"), "middle");
+        mapping.put(new UriTemplatePathSpec("/{a}/b/c"), "suffix");
+
+        assertThat(getBestMatch("/a/b/c"), is("suffix"));
+    }
+
+    @Test
+    public void testMiddleVsPrefix()
+    {
+        mapping.put(new UriTemplatePathSpec("/a/{b}/{c}/d"), "middle");
+        mapping.put(new UriTemplatePathSpec("/a/b/c/{d}"), "prefix");
+
+        assertThat(getBestMatch("/a/b/c/d"), is("prefix"));
+    }
+
+    @Test
+    public void testMiddleVsMiddle()
+    {
+        // This works but only because its an alphabetical check and '{' > 'c'.
+        mapping.put(new UriTemplatePathSpec("/a/{b}/{c}/d"), "middle1");
+        mapping.put(new UriTemplatePathSpec("/a/{b}/c/d"), "middle2");
+
+        assertThat(getBestMatch("/a/b/c/d"), is("middle2"));
+    }
+
+    @Test
+    public void testMiddleVsMiddle2()
+    {
+        mapping.put(new UriTemplatePathSpec("/{a}/{bz}/c/{d}"), "middle1");
+        mapping.put(new UriTemplatePathSpec("/{a}/{ba}/{c}/d"), "middle2");
+
+        assertThat(getBestMatch("/a/b/c/d"), is("middle1"));
+    }
+
+    @Test
+    public void testMiddleVsMiddle3()
+    {
+        mapping.put(new UriTemplatePathSpec("/{a}/{ba}/c/{d}"), "middle1");
+        mapping.put(new UriTemplatePathSpec("/{a}/{bz}/{c}/d"), "middle2");
+
+        assertThat(getBestMatch("/a/b/c/d"), is("middle1"));
+    }
+
+    @Test
+    public void testPrefixVsPrefix()
+    {
+        // This works but only because its an alphabetical check and '{' > 'b'.
+        mapping.put(new UriTemplatePathSpec("/a/{b}/{c}"), "prefix1");
+        mapping.put(new UriTemplatePathSpec("/a/b/{c}"), "prefix2");
+
+        assertThat(getBestMatch("/a/b/c"), is("prefix2"));
+    }
+
+    @Test
+    public void testSuffixVsSuffix()
+    {
+        // This works but only because its an alphabetical check and '{' > 'b'.
+        mapping.put(new UriTemplatePathSpec("/{a}/{b}/c"), "suffix1");
+        mapping.put(new UriTemplatePathSpec("/{a}/b/c"), "suffix2");
+
+        assertThat(getBestMatch("/a/b/c"), is("suffix2"));
+    }
+
+    @Test
+    public void testDifferentLengths()
+    {
+        mapping.put(new UriTemplatePathSpec("/a/{var}/c"), "endpointA");
+        mapping.put(new UriTemplatePathSpec("/a/{var}/c/d"), "endpointB");
+        mapping.put(new UriTemplatePathSpec("/a/{var1}/{var2}/d/e"), "endpointC");
+
+        assertThat(getBestMatch("/a/b/c"), is("endpointA"));
+        assertThat(getBestMatch("/a/d/c/d"), is("endpointB"));
+        assertThat(getBestMatch("/a/x/y/d/e"), is("endpointC"));
+    }
+}

jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/AbstractTest.java

@@ -21,7 +21,6 @@ package org.eclipse.jetty.http2.client;
 import java.net.InetSocketAddress;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Consumer;
-
 import javax.servlet.http.HttpServlet;
 
 import org.eclipse.jetty.http.HostPortHttpField;
@@ -33,7 +32,7 @@ import org.eclipse.jetty.http2.FlowControlStrategy;
 import org.eclipse.jetty.http2.api.Session;
 import org.eclipse.jetty.http2.api.server.ServerSessionListener;
 import org.eclipse.jetty.http2.server.AbstractHTTP2ServerConnectionFactory;
-import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory;
+import org.eclipse.jetty.http2.server.HTTP2CServerConnectionFactory;
 import org.eclipse.jetty.http2.server.RawHTTP2ServerConnectionFactory;
 import org.eclipse.jetty.server.ConnectionFactory;
 import org.eclipse.jetty.server.HttpConfiguration;
@@ -54,7 +53,7 @@ public class AbstractTest
 
     protected void start(HttpServlet servlet) throws Exception
     {
-        HTTP2ServerConnectionFactory connectionFactory = new HTTP2ServerConnectionFactory(new HttpConfiguration());
+        HTTP2CServerConnectionFactory connectionFactory = new HTTP2CServerConnectionFactory(new HttpConfiguration());
         connectionFactory.setInitialSessionRecvWindow(FlowControlStrategy.DEFAULT_WINDOW_SIZE);
         connectionFactory.setInitialStreamRecvWindow(FlowControlStrategy.DEFAULT_WINDOW_SIZE);
         prepareServer(connectionFactory);

jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/InvalidServerTest.java

@@ -1,80 +0,0 @@
-//
-// ========================================================================
-// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
-//
-// This program and the accompanying materials are made available under
-// the terms of the Eclipse Public License 2.0 which is available at
-// https://www.eclipse.org/legal/epl-2.0
-//
-// This Source Code may also be made available under the following
-// Secondary Licenses when the conditions for such availability set
-// forth in the Eclipse Public License, v. 2.0 are satisfied:
-// the Apache License v2.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.http2.client;
-
-import java.io.InputStream;
-import java.io.OutputStream;
-import java.net.InetSocketAddress;
-import java.net.ServerSocket;
-import java.net.Socket;
-import java.nio.charset.StandardCharsets;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
-
-import org.eclipse.jetty.http2.api.Session;
-import org.eclipse.jetty.util.Promise;
-import org.junit.jupiter.api.Test;
-
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
-public class InvalidServerTest extends AbstractTest
-{
-    @Test
-    public void testInvalidPreface() throws Exception
-    {
-        try (ServerSocket server = new ServerSocket(0))
-        {
-            prepareClient();
-            client.start();
-
-            CountDownLatch failureLatch = new CountDownLatch(1);
-            Promise.Completable<Session> promise = new Promise.Completable<>();
-            InetSocketAddress address = new InetSocketAddress("localhost", server.getLocalPort());
-            client.connect(address, new Session.Listener.Adapter()
-            {
-                @Override
-                public void onFailure(Session session, Throwable failure)
-                {
-                    failureLatch.countDown();
-                }
-            }, promise);
-
-            try (Socket socket = server.accept())
-            {
-                OutputStream output = socket.getOutputStream();
-                output.write("enough_junk_bytes".getBytes(StandardCharsets.UTF_8));
-
-                Session session = promise.get(5, TimeUnit.SECONDS);
-                assertNotNull(session);
-
-                assertTrue(failureLatch.await(5, TimeUnit.SECONDS));
-
-                // Verify that the client closed the socket.
-                InputStream input = socket.getInputStream();
-                while (true)
-                {
-                    int read = input.read();
-                    if (read < 0)
-                        break;
-                }
-            }
-        }
-    }
-}

jetty-http2/http2-client/src/test/java/org/eclipse/jetty/http2/client/PrefaceTest.java

@@ -18,7 +18,11 @@
 
 package org.eclipse.jetty.http2.client;
 
+import java.io.InputStream;
+import java.io.OutputStream;
 import java.net.InetSocketAddress;
+import java.net.ServerSocket;
+import java.net.Socket;
 import java.nio.ByteBuffer;
 import java.nio.channels.SocketChannel;
 import java.nio.charset.StandardCharsets;
@@ -41,6 +45,7 @@ import org.eclipse.jetty.http2.ErrorCode;
 import org.eclipse.jetty.http2.api.Session;
 import org.eclipse.jetty.http2.api.Stream;
 import org.eclipse.jetty.http2.api.server.ServerSessionListener;
+import org.eclipse.jetty.http2.frames.FrameType;
 import org.eclipse.jetty.http2.frames.HeadersFrame;
 import org.eclipse.jetty.http2.frames.PingFrame;
 import org.eclipse.jetty.http2.frames.PrefaceFrame;
@@ -63,6 +68,7 @@ import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class PrefaceTest extends AbstractTest
@@ -332,4 +338,71 @@ public class PrefaceTest extends AbstractTest
             assertTrue(clientSettingsLatch.await(5, TimeUnit.SECONDS));
         }
     }
+
+    @Test
+    public void testInvalidServerPreface() throws Exception
+    {
+        try (ServerSocket server = new ServerSocket(0))
+        {
+            prepareClient();
+            client.start();
+
+            CountDownLatch failureLatch = new CountDownLatch(1);
+            Promise.Completable<Session> promise = new Promise.Completable<>();
+            InetSocketAddress address = new InetSocketAddress("localhost", server.getLocalPort());
+            client.connect(address, new Session.Listener.Adapter()
+            {
+                @Override
+                public void onFailure(Session session, Throwable failure)
+                {
+                    failureLatch.countDown();
+                }
+            }, promise);
+
+            try (Socket socket = server.accept())
+            {
+                OutputStream output = socket.getOutputStream();
+                output.write("enough_junk_bytes".getBytes(StandardCharsets.UTF_8));
+
+                Session session = promise.get(5, TimeUnit.SECONDS);
+                assertNotNull(session);
+
+                assertTrue(failureLatch.await(5, TimeUnit.SECONDS));
+
+                // Verify that the client closed the socket.
+                InputStream input = socket.getInputStream();
+                while (true)
+                {
+                    int read = input.read();
+                    if (read < 0)
+                        break;
+                }
+            }
+        }
+    }
+
+    @Test
+    public void testInvalidClientPreface() throws Exception
+    {
+        start(new ServerSessionListener.Adapter());
+
+        try (Socket client = new Socket("localhost", connector.getLocalPort()))
+        {
+            OutputStream output = client.getOutputStream();
+            output.write("enough_junk_bytes".getBytes(StandardCharsets.UTF_8));
+            output.flush();
+
+            byte[] bytes = new byte[1024];
+            InputStream input = client.getInputStream();
+            int read = input.read(bytes);
+            if (read < 0)
+            {
+                // Closing the connection without GOAWAY frame is fine.
+                return;
+            }
+
+            int type = bytes[3];
+            assertEquals(FrameType.GO_AWAY.getType(), type);
+        }
+    }
 }

jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Flusher.java

@@ -425,12 +425,21 @@ public class HTTP2Flusher extends IteratingCallback implements Dumpable
             super.failed(x);
         }
 
+        /**
+         * @return whether the entry is stale and must not be processed
+         */
         private boolean isStale()
         {
-            return !isProtocol() && stream != null && stream.isReset();
+            // If it is a protocol frame, process it.
+            if (isProtocolFrame(frame))
+                return false;
+            // It's an application frame; is the stream gone already?
+            if (stream == null)
+                return true;
+            return stream.isReset();
         }
 
-        private boolean isProtocol()
+        private boolean isProtocolFrame(Frame frame)
         {
             switch (frame.getType())
             {

jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java

@@ -447,7 +447,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio
             // We received a GO_AWAY, so try to write
             // what's in the queue and then disconnect.
             closeFrame = frame;
-            notifyClose(this, frame, new DisconnectCallback());
+            onClose(frame, new DisconnectCallback());
             return;
         }
 
@@ -514,9 +514,15 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio
     public void onStreamFailure(int streamId, int error, String reason)
     {
         Callback callback = new ResetCallback(streamId, error, Callback.NOOP);
+        Throwable failure = toFailure("Stream failure", error, reason);
+        onStreamFailure(streamId, error, reason, failure, callback);
+    }
+
+    private void onStreamFailure(int streamId, int error, String reason, Throwable failure, Callback callback)
+    {
         IStream stream = getStream(streamId);
         if (stream != null)
-            stream.process(new FailureFrame(error, reason), callback);
+            stream.process(new FailureFrame(error, reason, failure), callback);
         else
             callback.succeeded();
     }
@@ -529,38 +535,51 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio
 
     protected void onConnectionFailure(int error, String reason, Callback callback)
     {
-        notifyFailure(this, new IOException(String.format("%d/%s", error, reason)), new FailureCallback(error, reason, callback));
+        Throwable failure = toFailure("Session failure", error, reason);
+        onFailure(error, reason, failure, new FailureCallback(error, reason, callback));
+    }
+
+    protected void abort(Throwable failure)
+    {
+        onFailure(ErrorCode.NO_ERROR.code, null, failure, new TerminateCallback(failure));
+    }
+
+    private void onFailure(int error, String reason, Throwable failure, Callback callback)
+    {
+        Collection<Stream> streams = getStreams();
+        int count = streams.size();
+        Callback countCallback = new CountingCallback(callback, count + 1);
+        for (Stream stream : streams)
+        {
+            onStreamFailure(stream.getId(), error, reason, failure, countCallback);
+        }
+        notifyFailure(this, failure, countCallback);
+    }
+
+    private void onClose(GoAwayFrame frame, Callback callback)
+    {
+        int error = frame.getError();
+        String reason = frame.tryConvertPayload();
+        Throwable failure = toFailure("Session close", error, reason);
+        Collection<Stream> streams = getStreams();
+        int count = streams.size();
+        Callback countCallback = new CountingCallback(callback, count + 1);
+        for (Stream stream : streams)
+        {
+            onStreamFailure(stream.getId(), error, reason, failure, countCallback);
+        }
+        notifyClose(this, frame, countCallback);
+    }
+
+    private Throwable toFailure(String message, int error, String reason)
+    {
+        return new IOException(String.format("%s %s/%s", message, ErrorCode.toString(error, null), reason));
     }
 
     @Override
     public void newStream(HeadersFrame frame, Promise<Stream> promise, Stream.Listener listener)
     {
         streamCreator.newStream(frame, promise, listener);
-/*
-        try
-        {
-            // Synchronization is necessary to atomically create
-            // the stream id and enqueue the frame to be sent.
-            IStream stream;
-            boolean queued;
-            synchronized (this)
-            {
-                HeadersFrame[] frameOut = new HeadersFrame[1];
-                stream = newLocalStream(frame, frameOut);
-                stream.setListener(listener);
-                ControlEntry entry = new ControlEntry(frameOut[0], stream, new StreamPromiseCallback(promise, stream));
-                queued = flusher.append(entry);
-            }
-            stream.process(new PrefaceFrame(), Callback.NOOP);
-            // Iterate outside the synchronized block.
-            if (queued)
-                flusher.iterate();
-        }
-        catch (Throwable x)
-        {
-            promise.failed(x);
-        }
-*/
     }
 
     /**
@@ -1100,11 +1119,6 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio
         }
     }
 
-    protected void abort(Throwable failure)
-    {
-        notifyFailure(this, failure, new TerminateCallback(failure));
-    }
-
     public boolean isDisconnected()
     {
         return !endPoint.isOpen();
@@ -1629,7 +1643,7 @@ public abstract class HTTP2Session extends ContainerLifeCycle implements ISessio
         public void failed(Throwable x)
         {
             if (LOG.isDebugEnabled())
-                LOG.debug("CloseCallback failed", x);
+                LOG.debug("FailureCallback failed", x);
             complete();
         }
 

jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java

@@ -151,7 +151,6 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa
     {
         if (writing.compareAndSet(null, callback))
             return true;
-        close();
         callback.failed(new WritePendingException());
         return false;
     }
@@ -190,7 +189,7 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa
     public boolean isRemotelyClosed()
     {
         CloseState state = closeState.get();
-        return state == CloseState.REMOTELY_CLOSED || state == CloseState.CLOSING;
+        return state == CloseState.REMOTELY_CLOSED || state == CloseState.CLOSING || state == CloseState.CLOSED;
     }
 
     public boolean isLocallyClosed()
@@ -345,7 +344,7 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa
         if (dataLength != Long.MIN_VALUE)
         {
             dataLength -= frame.remaining();
-            if (frame.isEndStream() && dataLength != 0)
+            if (dataLength < 0 || (frame.isEndStream() && dataLength != 0))
             {
                 reset(new ResetFrame(streamId, ErrorCode.PROTOCOL_ERROR.code), Callback.NOOP);
                 callback.failed(new IOException("invalid_data_length"));
@@ -462,6 +461,8 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa
 
     private void onFailure(FailureFrame frame, Callback callback)
     {
+        // Don't close or remove the stream, as the listener may
+        // want to use it, for example to send a RST_STREAM frame.
         notifyFailure(this, frame, callback);
     }
 
@@ -749,7 +750,7 @@ public class HTTP2Stream extends IdleTimeout implements IStream, Callback, Dumpa
         {
             try
             {
-                listener.onFailure(stream, frame.getError(), frame.getReason(), callback);
+                listener.onFailure(stream, frame.getError(), frame.getReason(), frame.getFailure(), callback);
             }
             catch (Throwable x)
             {

jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java

@@ -305,9 +305,10 @@ public interface Stream
          * @param stream the stream
          * @param error the error code
          * @param reason the error reason, or null
+         * @param failure the failure
          * @param callback the callback to complete when the failure has been handled
          */
-        public default void onFailure(Stream stream, int error, String reason, Callback callback)
+        public default void onFailure(Stream stream, int error, String reason, Throwable failure, Callback callback)
         {
             callback.succeeded();
         }

jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/FailureFrame.java

@@ -22,12 +22,14 @@ public class FailureFrame extends Frame
 {
     private final int error;
     private final String reason;
+    private final Throwable failure;
 
-    public FailureFrame(int error, String reason)
+    public FailureFrame(int error, String reason, Throwable failure)
     {
         super(FrameType.FAILURE);
         this.error = error;
         this.reason = reason;
+        this.failure = failure;
     }
 
     public int getError()
@@ -39,4 +41,9 @@ public class FailureFrame extends Frame
     {
         return reason;
     }
+
+    public Throwable getFailure()
+    {
+        return failure;
+    }
 }

jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/Generator.java

@@ -61,7 +61,7 @@ public class Generator
         this.generators[FrameType.WINDOW_UPDATE.getType()] = new WindowUpdateGenerator(headerGenerator);
         this.generators[FrameType.CONTINUATION.getType()] = null; // Never generated explicitly.
         this.generators[FrameType.PREFACE.getType()] = new PrefaceGenerator();
-        this.generators[FrameType.DISCONNECT.getType()] = new DisconnectGenerator();
+        this.generators[FrameType.DISCONNECT.getType()] = new NoOpGenerator();
 
         this.dataGenerator = new DataGenerator(headerGenerator);
     }

jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/NoOpGenerator.java

@@ -21,9 +21,9 @@ package org.eclipse.jetty.http2.generator;
 import org.eclipse.jetty.http2.frames.Frame;
 import org.eclipse.jetty.io.ByteBufferPool;
 
-public class DisconnectGenerator extends FrameGenerator
+public class NoOpGenerator extends FrameGenerator
 {
-    public DisconnectGenerator()
+    public NoOpGenerator()
     {
         super(null);
     }

jetty-http2/http2-hpack/src/main/java/org/eclipse/jetty/http2/hpack/HpackEncoder.java

@@ -304,11 +304,11 @@ public class HpackEncoder
 
         String encoding = null;
 
-        // Is there an entry for the field?
+        // Is there an index entry for the field?
         Entry entry = _context.get(field);
         if (entry != null)
         {
-            // Known field entry, so encode it as indexed
+            // This is a known indexed field, send as static or dynamic indexed.
             if (entry.isStatic())
             {
                 buffer.put(((StaticEntry)entry).getEncodedField());
@@ -326,10 +326,10 @@ public class HpackEncoder
         }
         else
         {
-            // Unknown field entry, so we will have to send literally.
+            // Unknown field entry, so we will have to send literally, but perhaps add an index.
             final boolean indexed;
 
-            // But do we know it's name?
+            // Do we know its name?
             HttpHeader header = field.getHeader();
 
             // Select encoding strategy
@@ -347,12 +347,11 @@ public class HpackEncoder
                     if (_debug)
                         encoding = indexed ? "PreEncodedIdx" : "PreEncoded";
                 }
-                // has the custom header name been seen before?
+                else if (name == null && fieldSize < _context.getMaxDynamicTableSize())
-                else if (name == null)
                 {
-                    // unknown name and value, so let's index this just in case it is
+                    // unknown name and value that will fit in dynamic table, so let's index
-                    // the first time we have seen a custom name or a custom field.
+                    // this just in case it is the first time we have seen a custom name or a
-                    // unless the name is changing, this is worthwhile
+                    // custom field.  Unless the name is once only, this is worthwhile
                     indexed = true;
                     encodeName(buffer, (byte)0x40, 6, field.getName(), null);
                     encodeValue(buffer, true, field.getValue());
@@ -361,7 +360,7 @@ public class HpackEncoder
                 }
                 else
                 {
-                    // known custom name, but unknown value.
+                    // Known name, but different value.
                     // This is probably a custom field with changing value, so don't index.
                     indexed = false;
                     encodeName(buffer, (byte)0x00, 4, field.getName(), null);
@@ -400,9 +399,9 @@ public class HpackEncoder
                             (huffman ? "HuffV" : "LitV") +
                             (neverIndex ? "!!Idx" : "!Idx");
                 }
-                else if (fieldSize >= _context.getMaxDynamicTableSize() || header == HttpHeader.CONTENT_LENGTH && field.getValue().length() > 2)
+                else if (fieldSize >= _context.getMaxDynamicTableSize() || header == HttpHeader.CONTENT_LENGTH && !"0".equals(field.getValue()))
                 {
-                    // Non indexed if field too large or a content length for 3 digits or more
+                    // The field is too large or a non zero content length, so do not index.
                     indexed = false;
                     encodeName(buffer, (byte)0x00, 4, header.asString(), name);
                     encodeValue(buffer, true, field.getValue());

jetty-http2/http2-hpack/src/test/java/org/eclipse/jetty/http2/hpack/HpackEncoderTest.java

@@ -22,6 +22,7 @@ import java.nio.ByteBuffer;
 
 import org.eclipse.jetty.http.HttpField;
 import org.eclipse.jetty.http.HttpFields;
+import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.http.HttpVersion;
 import org.eclipse.jetty.http.MetaData;
 import org.eclipse.jetty.util.BufferUtil;
@@ -145,6 +146,54 @@ public class HpackEncoderTest
         assertEquals(5, encoder.getHpackContext().size());
     }
 
+    @Test
+    public void testLargeFieldsNotIndexed()
+    {
+        HpackEncoder encoder = new HpackEncoder(38 * 5);
+        HpackContext ctx = encoder.getHpackContext();
+
+        ByteBuffer buffer = BufferUtil.allocate(4096);
+
+        // Index little fields
+        int pos = BufferUtil.flipToFill(buffer);
+        encoder.encode(buffer, new HttpField("Name", "Value"));
+        BufferUtil.flipToFlush(buffer, pos);
+        int dynamicTableSize = ctx.getDynamicTableSize();
+        assertThat(dynamicTableSize, Matchers.greaterThan(0));
+
+        // Do not index big field
+        StringBuilder largeName = new StringBuilder("largeName-");
+        String filler = "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX";
+        while (largeName.length() < ctx.getMaxDynamicTableSize())
+            largeName.append(filler, 0, Math.min(filler.length(), ctx.getMaxDynamicTableSize() - largeName.length()));
+        pos = BufferUtil.flipToFill(buffer);
+        encoder.encode(buffer, new HttpField(largeName.toString(), "Value"));
+        BufferUtil.flipToFlush(buffer, pos);
+        assertThat(ctx.getDynamicTableSize(), Matchers.is(dynamicTableSize));
+    }
+
+    @Test
+    public void testIndexContentLength()
+    {
+        HpackEncoder encoder = new HpackEncoder(38 * 5);
+        HpackContext ctx = encoder.getHpackContext();
+
+        ByteBuffer buffer = BufferUtil.allocate(4096);
+
+        // Index zero content length
+        int pos = BufferUtil.flipToFill(buffer);
+        encoder.encode(buffer, new HttpField(HttpHeader.CONTENT_LENGTH, "0"));
+        BufferUtil.flipToFlush(buffer, pos);
+        int dynamicTableSize = ctx.getDynamicTableSize();
+        assertThat(dynamicTableSize, Matchers.greaterThan(0));
+
+        // Do not index non zero content length
+        pos = BufferUtil.flipToFill(buffer);
+        encoder.encode(buffer, new HttpField(HttpHeader.CONTENT_LENGTH, "42"));
+        BufferUtil.flipToFlush(buffer, pos);
+        assertThat(ctx.getDynamicTableSize(), Matchers.is(dynamicTableSize));
+    }
+
     @Test
     public void testNeverIndexSetCookie() throws Exception
     {

jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpChannelOverHTTP2.java

@@ -18,8 +18,6 @@
 
 package org.eclipse.jetty.http2.client.http;
 
-import java.io.IOException;
-
 import org.eclipse.jetty.client.HttpChannel;
 import org.eclipse.jetty.client.HttpDestination;
 import org.eclipse.jetty.client.HttpExchange;
@@ -208,10 +206,10 @@ public class HttpChannelOverHTTP2 extends HttpChannel
         }
 
         @Override
-        public void onFailure(Stream stream, int error, String reason, Callback callback)
+        public void onFailure(Stream stream, int error, String reason, Throwable failure, Callback callback)
         {
             HTTP2Channel.Client channel = (HTTP2Channel.Client)((IStream)stream).getAttachment();
-            channel.onFailure(new IOException(String.format("Failure %s/%s", ErrorCode.toString(error, null), reason)), callback);
+            channel.onFailure(failure, callback);
         }
 
         @Override

jetty-http2/http2-http-client-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpConnectionOverHTTP2.java

@@ -196,7 +196,7 @@ public class HttpConnectionOverHTTP2 extends HttpConnection implements Sweeper.S
     {
         if (closed.compareAndSet(false, true))
         {
-            getHttpDestination().close(this);
+            getHttpDestination().remove(this);
 
             abort(failure);
 

jetty-http2/http2-server/pom.xml

@@ -30,6 +30,10 @@
         <configuration>
           <mainClass>org.eclipse.jetty.http2.server.H2SpecServer</mainClass>
           <skip>${skipTests}</skip>
+          <excludeSpecs>
+            <!-- see: https://github.com/summerwind/h2spec/issues/115 -->
+            <excludeSpec>6.9.2 - Changes SETTINGS_INITIAL_WINDOW_SIZE after sending HEADERS frame</excludeSpec>
+          </excludeSpecs>
         </configuration>
         <executions>
           <execution>

jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnection.java

@@ -23,7 +23,6 @@ import java.io.IOException;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Base64;
-import java.util.Collection;
 import java.util.List;
 import java.util.Objects;
 import java.util.Queue;
@@ -43,7 +42,6 @@ import org.eclipse.jetty.http2.HTTP2Channel;
 import org.eclipse.jetty.http2.HTTP2Connection;
 import org.eclipse.jetty.http2.ISession;
 import org.eclipse.jetty.http2.IStream;
-import org.eclipse.jetty.http2.api.Stream;
 import org.eclipse.jetty.http2.api.server.ServerSessionListener;
 import org.eclipse.jetty.http2.frames.DataFrame;
 import org.eclipse.jetty.http2.frames.Frame;
@@ -59,7 +57,6 @@ import org.eclipse.jetty.server.Connector;
 import org.eclipse.jetty.server.HttpConfiguration;
 import org.eclipse.jetty.util.BufferUtil;
 import org.eclipse.jetty.util.Callback;
-import org.eclipse.jetty.util.CountingCallback;
 import org.eclipse.jetty.util.TypeUtil;
 
 public class HTTP2ServerConnection extends HTTP2Connection
@@ -208,13 +205,17 @@ public class HTTP2ServerConnection extends HTTP2Connection
     public void onStreamFailure(IStream stream, Throwable failure, Callback callback)
     {
         if (LOG.isDebugEnabled())
-            LOG.debug("Processing failure on {}: {}", stream, failure);
+            LOG.debug("Processing stream failure on {}", stream, failure);
         HTTP2Channel.Server channel = (HTTP2Channel.Server)stream.getAttachment();
         if (channel != null)
         {
             Runnable task = channel.onFailure(failure, callback);
             if (task != null)
+            {
+                // We must dispatch to another thread because the task
+                // may call application code that performs blocking I/O.
                 offerTask(task, true);
+            }
         }
         else
         {
@@ -239,22 +240,10 @@ public class HTTP2ServerConnection extends HTTP2Connection
 
     public void onSessionFailure(Throwable failure, Callback callback)
     {
-        ISession session = getSession();
         if (LOG.isDebugEnabled())
-            LOG.debug("Processing failure on {}: {}", session, failure);
+            LOG.debug("Processing session failure on {}", getSession(), failure);
-        Collection<Stream> streams = session.getStreams();
+        // All the streams have already been failed, just succeed the callback.
-        if (streams.isEmpty())
+        callback.succeeded();
-        {
-            callback.succeeded();
-        }
-        else
-        {
-            CountingCallback counter = new CountingCallback(callback, streams.size());
-            for (Stream stream : streams)
-            {
-                onStreamFailure((IStream)stream, failure, counter);
-            }
-        }
     }
 
     public void push(Connector connector, IStream stream, MetaData.Request request)

jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnectionFactory.java

@@ -34,6 +34,7 @@ import org.eclipse.jetty.http2.frames.PushPromiseFrame;
 import org.eclipse.jetty.http2.frames.ResetFrame;
 import org.eclipse.jetty.io.EndPoint;
 import org.eclipse.jetty.io.EofException;
+import org.eclipse.jetty.io.QuietException;
 import org.eclipse.jetty.server.Connector;
 import org.eclipse.jetty.server.HttpConfiguration;
 import org.eclipse.jetty.server.NegotiatingServerConnection.CipherDiscriminator;
@@ -122,19 +123,14 @@ public class HTTP2ServerConnectionFactory extends AbstractHTTP2ServerConnectionF
             return getConnection().onSessionTimeout(new TimeoutException("Session idle timeout " + idleTimeout + " ms"));
         }
 
-        @Override
-        public boolean onIdleTimeout(Stream stream, Throwable x)
-        {
-            return getConnection().onStreamTimeout((IStream)stream, x);
-        }
-
         @Override
         public void onClose(Session session, GoAwayFrame frame, Callback callback)
         {
             String reason = frame.tryConvertPayload();
             if (!StringUtil.isEmpty(reason))
                 reason = " (" + reason + ")";
-            getConnection().onSessionFailure(new EofException(String.format("Close %s/%s", ErrorCode.toString(frame.getError(), null), reason)), callback);
+            EofException failure = new EofException(String.format("Close %s/%s", ErrorCode.toString(frame.getError(), null), reason));
+            onFailure(session, failure, callback);
         }
 
         @Override
@@ -143,12 +139,6 @@ public class HTTP2ServerConnectionFactory extends AbstractHTTP2ServerConnectionF
             getConnection().onSessionFailure(failure, callback);
         }
 
-        @Override
-        public void onFailure(Stream stream, int error, String reason, Callback callback)
-        {
-            getConnection().onStreamFailure((IStream)stream, new EofException(String.format("Failure %s/%s", ErrorCode.toString(error, null), reason)), callback);
-        }
-
         @Override
         public void onHeaders(Stream stream, HeadersFrame frame)
         {
@@ -175,7 +165,27 @@ public class HTTP2ServerConnectionFactory extends AbstractHTTP2ServerConnectionF
         @Override
         public void onReset(Stream stream, ResetFrame frame, Callback callback)
         {
-            getConnection().onStreamFailure((IStream)stream, new EofException("Reset " + ErrorCode.toString(frame.getError(), null)), callback);
+            EofException failure = new EofException("Reset " + ErrorCode.toString(frame.getError(), null));
+            onFailure(stream, failure, callback);
+        }
+
+        @Override
+        public void onFailure(Stream stream, int error, String reason, Throwable failure, Callback callback)
+        {
+            if (!(failure instanceof QuietException))
+                failure = new EofException(failure);
+            onFailure(stream, failure, callback);
+        }
+
+        private void onFailure(Stream stream, Throwable failure, Callback callback)
+        {
+            getConnection().onStreamFailure((IStream)stream, failure, callback);
+        }
+
+        @Override
+        public boolean onIdleTimeout(Stream stream, Throwable x)
+        {
+            return getConnection().onStreamTimeout((IStream)stream, x);
         }
 
         private void close(Stream stream, String reason)

jetty-http2/http2-server/src/main/java/org/eclipse/jetty/http2/server/HttpTransportOverHTTP2.java

@@ -20,6 +20,7 @@ package org.eclipse.jetty.http2.server;
 
 import java.nio.ByteBuffer;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Consumer;
 import java.util.function.Supplier;
 
 import org.eclipse.jetty.http.BadMessageException;
@@ -82,7 +83,7 @@ public class HttpTransportOverHTTP2 implements HttpTransport
     }
 
     @Override
-    public void send(MetaData.Request request, MetaData.Response response, ByteBuffer content, boolean lastContent, Callback callback)
+    public void send(MetaData.Request request, final MetaData.Response response, ByteBuffer content, boolean lastContent, Callback callback)
     {
         boolean isHeadRequest = HttpMethod.HEAD.is(request.getMethod());
         boolean hasContent = BufferUtil.hasContent(content) && !isHeadRequest;
@@ -100,8 +101,8 @@ public class HttpTransportOverHTTP2 implements HttpTransport
                 }
                 else
                 {
-                    if (transportCallback.start(callback, false))
+                    transportCallback.send(callback, false, c ->
-                        sendHeadersFrame(response, false, transportCallback);
+                        sendHeadersFrame(metaData, false, c));
                 }
             }
             else
@@ -114,7 +115,7 @@ public class HttpTransportOverHTTP2 implements HttpTransport
                         long contentLength = response.getContentLength();
                         if (contentLength < 0)
                         {
-                            response = new MetaData.Response(
+                            metaData = new MetaData.Response(
                                 response.getHttpVersion(),
                                 response.getStatus(),
                                 response.getReason(),
@@ -142,53 +143,53 @@ public class HttpTransportOverHTTP2 implements HttpTransport
                                     HttpFields trailers = retrieveTrailers();
                                     if (trailers != null)
                                     {
-                                        if (transportCallback.start(new SendTrailers(getCallback(), trailers), false))
+                                        transportCallback.send(new SendTrailers(getCallback(), trailers), false, c ->
-                                            sendDataFrame(content, true, false, transportCallback);
+                                            sendDataFrame(content, true, false, c));
                                     }
                                     else
                                     {
-                                        if (transportCallback.start(getCallback(), false))
+                                        transportCallback.send(getCallback(), false, c ->
-                                            sendDataFrame(content, true, true, transportCallback);
+                                            sendDataFrame(content, true, true, c));
                                     }
                                 }
                                 else
                                 {
-                                    if (transportCallback.start(getCallback(), false))
+                                    transportCallback.send(getCallback(), false, c ->
-                                        sendDataFrame(content, false, false, transportCallback);
+                                        sendDataFrame(content, false, false, c));
                                 }
                             }
                         };
-                        if (transportCallback.start(commitCallback, true))
+                        transportCallback.send(commitCallback, true, c ->
-                            sendHeadersFrame(response, false, transportCallback);
+                            sendHeadersFrame(metaData, false, c));
                     }
                     else
                     {
                         if (lastContent)
                         {
-                            if (isTunnel(request, response))
+                            if (isTunnel(request, metaData))
                             {
-                                if (transportCallback.start(callback, true))
+                                transportCallback.send(callback, true, c ->
-                                    sendHeadersFrame(response, false, transportCallback);
+                                    sendHeadersFrame(metaData, false, c));
                             }
                             else
                             {
                                 HttpFields trailers = retrieveTrailers();
                                 if (trailers != null)
                                 {
-                                    if (transportCallback.start(new SendTrailers(callback, trailers), true))
+                                    transportCallback.send(new SendTrailers(callback, trailers), true, c ->
-                                        sendHeadersFrame(response, false, transportCallback);
+                                        sendHeadersFrame(metaData, false, c));
                                 }
                                 else
                                 {
-                                    if (transportCallback.start(callback, true))
+                                    transportCallback.send(callback, true, c ->
-                                        sendHeadersFrame(response, true, transportCallback);
+                                        sendHeadersFrame(metaData, true, c));
                                 }
                             }
                         }
                         else
                         {
-                            if (transportCallback.start(callback, true))
+                            transportCallback.send(callback, true, c ->
-                                sendHeadersFrame(response, false, transportCallback);
+                                sendHeadersFrame(metaData, false, c));
                         }
                     }
                 }
@@ -210,8 +211,8 @@ public class HttpTransportOverHTTP2 implements HttpTransport
                         SendTrailers sendTrailers = new SendTrailers(callback, trailers);
                         if (hasContent)
                         {
-                            if (transportCallback.start(sendTrailers, false))
+                            transportCallback.send(sendTrailers, false, c ->
-                                sendDataFrame(content, true, false, transportCallback);
+                                sendDataFrame(content, true, false, c));
                         }
                         else
                         {
@@ -220,14 +221,14 @@ public class HttpTransportOverHTTP2 implements HttpTransport
                     }
                     else
                     {
-                        if (transportCallback.start(callback, false))
+                        transportCallback.send(callback, false, c ->
-                            sendDataFrame(content, true, true, transportCallback);
+                            sendDataFrame(content, true, true, c));
                     }
                 }
                 else
                 {
-                    if (transportCallback.start(callback, false))
+                    transportCallback.send(callback, false, c ->
-                        sendDataFrame(content, false, false, transportCallback);
+                        sendDataFrame(content, false, false, c));
                 }
             }
             else
@@ -334,7 +335,7 @@ public class HttpTransportOverHTTP2 implements HttpTransport
 
     public boolean onStreamTimeout(Throwable failure)
     {
-        return transportCallback.onIdleTimeout(failure);
+        return transportCallback.idleTimeout(failure);
     }
 
     /**
@@ -397,119 +398,359 @@ public class HttpTransportOverHTTP2 implements HttpTransport
             stream.reset(new ResetFrame(stream.getId(), ErrorCode.CANCEL_STREAM_ERROR.code), Callback.NOOP);
     }
 
+    /**
+     * <p>Callback that controls sends initiated by the transport, by eventually
+     * notifying a nested callback.</p>
+     * <p>There are 3 sources of concurrency after a send is initiated:</p>
+     * <ul>
+     *   <li>the completion of the send operation, either success or failure</li>
+     *   <li>an asynchronous failure coming from the read side such as a stream
+     *   being reset, or the connection being closed</li>
+     *   <li>an asynchronous idle timeout</li>
+     * </ul>
+     * <p>The last 2 cases may happen <em>during</em> a send, when the frames
+     * are being generated in the flusher.
+     * In such cases, this class must avoid that the nested callback is notified
+     * while the frame generation is in progress, because the nested callback
+     * may modify other states (such as clearing the {@code HttpOutput._buffer})
+     * that are accessed during frame generation.</p>
+     * <p>The solution implemented in this class works by splitting the send
+     * operation in 3 parts: {@code pre-send}, {@code send} and {@code post-send}.
+     * Asynchronous state changes happening during {@code send} are stored
+     * and only executed in {@code post-send}, therefore never interfering
+     * with frame generation.</p>
+     *
+     * @see State
+     */
     private class TransportCallback implements Callback
     {
-        private State state = State.IDLE;
+        private State _state = State.IDLE;
-        private Callback callback;
+        private Callback _callback;
-        private Throwable failure;
+        private boolean _commit;
-        private boolean commit;
+        private Throwable _failure;
 
-        public boolean start(Callback callback, boolean commit)
+        private void reset(Throwable failure)
         {
-            State state;
+            assert Thread.holdsLock(this);
+            _state = failure != null ? State.FAILED : State.IDLE;
+            _callback = null;
+            _commit = false;
+            _failure = failure;
+        }
+
+        private void send(Callback callback, boolean commit, Consumer<Callback> sendFrame)
+        {
+            Throwable failure = sending(callback, commit);
+            if (failure == null)
+            {
+                sendFrame.accept(this);
+                pending();
+            }
+            else
+            {
+                callback.failed(failure);
+            }
+        }
+
+        private Throwable sending(Callback callback, boolean commit)
+        {
+            synchronized (this)
+            {
+                switch (_state)
+                {
+                    case IDLE:
+                    {
+                        _state = State.SENDING;
+                        _callback = callback;
+                        _commit = commit;
+                        return null;
+                    }
+                    case FAILED:
+                    {
+                        return _failure;
+                    }
+                    default:
+                    {
+                        return new IllegalStateException("Invalid transport state: " + _state);
+                    }
+                }
+            }
+        }
+
+        private void pending()
+        {
+            Callback callback;
+            boolean commit;
             Throwable failure;
             synchronized (this)
             {
-                state = this.state;
+                switch (_state)
-                failure = this.failure;
-                if (state == State.IDLE)
                 {
-                    this.state = State.WRITING;
+                    case SENDING:
-                    this.callback = callback;
+                    {
-                    this.commit = commit;
+                        // The send has not completed the callback yet,
-                    return true;
+                        // wait for succeeded() or failed() to be called.
+                        _state = State.PENDING;
+                        return;
+                    }
+                    case SUCCEEDING:
+                    {
+                        // The send already completed successfully, but the
+                        // call to succeeded() was delayed, so call it now.
+                        callback = _callback;
+                        commit = _commit;
+                        failure = null;
+                        reset(null);
+                        break;
+                    }
+                    case FAILING:
+                    {
+                        // The send already completed with a failure, but
+                        // the call to failed() was delayed, so call it now.
+                        callback = _callback;
+                        commit = _commit;
+                        failure = _failure;
+                        reset(failure);
+                        break;
+                    }
+                    default:
+                    {
+                        callback = _callback;
+                        commit = _commit;
+                        failure = new IllegalStateException("Invalid transport state: " + _state);
+                        reset(failure);
+                        break;
+                    }
                 }
             }
             if (failure == null)
-                failure = new IllegalStateException("Invalid transport state: " + state);
+                succeed(callback, commit);
-            callback.failed(failure);
+            else
-            return false;
+                fail(callback, commit, failure);
         }
 
         @Override
         public void succeeded()
         {
+            Callback callback;
             boolean commit;
-            Callback callback = null;
             synchronized (this)
             {
-                commit = this.commit;
+                switch (_state)
-                if (state == State.WRITING)
                 {
-                    this.state = State.IDLE;
+                    case SENDING:
-                    callback = this.callback;
+                    {
-                    this.callback = null;
+                        _state = State.SUCCEEDING;
-                    this.commit = false;
+                        // Succeeding the callback will be done in postSend().
+                        return;
+                    }
+                    case PENDING:
+                    {
+                        callback = _callback;
+                        commit = _commit;
+                        reset(null);
+                        break;
+                    }
+                    default:
+                    {
+                        // This thread lost the race to succeed the current
+                        // send, as other threads likely already failed it.
+                        return;
+                    }
                 }
             }
-            if (LOG.isDebugEnabled())
+            succeed(callback, commit);
-                LOG.debug("HTTP2 Response #{}/{} {} {}",
-                    stream.getId(), Integer.toHexString(stream.getSession().hashCode()),
-                    commit ? "commit" : "flush",
-                    callback == null ? "failure" : "success");
-            if (callback != null)
-                callback.succeeded();
         }
 
         @Override
         public void failed(Throwable failure)
         {
-            boolean commit;
             Callback callback;
+            boolean commit;
             synchronized (this)
             {
-                commit = this.commit;
+                switch (_state)
-                this.state = State.FAILED;
+                {
-                callback = this.callback;
+                    case SENDING:
-                this.callback = null;
+                    {
-                this.failure = failure;
+                        _state = State.FAILING;
+                        _failure = failure;
+                        // Failing the callback will be done in postSend().
+                        return;
+                    }
+                    case IDLE:
+                    case PENDING:
+                    {
+                        callback = _callback;
+                        commit = _commit;
+                        reset(failure);
+                        break;
+                    }
+                    default:
+                    {
+                        // This thread lost the race to fail the current send,
+                        // as other threads already succeeded or failed it.
+                        return;
+                    }
+                }
             }
+            fail(callback, commit, failure);
+        }
+
+        private boolean idleTimeout(Throwable failure)
+        {
+            Callback callback;
+            boolean timeout;
+            synchronized (this)
+            {
+                switch (_state)
+                {
+                    case PENDING:
+                    {
+                        // The send was started but idle timed out, fail it.
+                        callback = _callback;
+                        timeout = true;
+                        reset(failure);
+                        break;
+                    }
+                    case IDLE:
+                        // The application may be suspended, ignore the idle timeout.
+                    case SENDING:
+                        // A send has been started at the same time of an idle timeout;
+                        // Ignore the idle timeout and let the write continue normally.
+                    case SUCCEEDING:
+                    case FAILING:
+                        // An idle timeout during these transient states is ignored.
+                    case FAILED:
+                        // Already failed, ignore the idle timeout.
+                    {
+                        callback = null;
+                        timeout = false;
+                        break;
+                    }
+                    default:
+                    {
+                        // Should not happen, but just in case.
+                        callback = _callback;
+                        if (callback == null)
+                            callback = Callback.NOOP;
+                        timeout = true;
+                        failure = new IllegalStateException("Invalid transport state: " + _state, failure);
+                        reset(failure);
+                        break;
+                    }
+                }
+            }
+            idleTimeout(callback, timeout, failure);
+            return timeout;
+        }
+
+        private void succeed(Callback callback, boolean commit)
+        {
             if (LOG.isDebugEnabled())
-                LOG.debug(String.format("HTTP2 Response #%d/%h %s %s", stream.getId(), stream.getSession(),
+                LOG.debug("HTTP2 Response #{}/{} {} success",
-                    commit ? "commit" : "flush", callback == null ? "ignored" : "failed"), failure);
+                    stream.getId(), Integer.toHexString(stream.getSession().hashCode()),
+                    commit ? "commit" : "flush");
+            callback.succeeded();
+        }
+
+        private void fail(Callback callback, boolean commit, Throwable failure)
+        {
+            if (LOG.isDebugEnabled())
+                LOG.debug("HTTP2 Response #{}/{} {} failure",
+                    stream.getId(), Integer.toHexString(stream.getSession().hashCode()),
+                    commit ? "commit" : "flush",
+                    failure);
             if (callback != null)
                 callback.failed(failure);
         }
 
-        private boolean onIdleTimeout(Throwable failure)
+        private void idleTimeout(Callback callback, boolean timeout, Throwable failure)
         {
-            boolean result;
-            Callback callback = null;
-            synchronized (this)
-            {
-                // Ignore idle timeouts if not writing,
-                // as the application may be suspended.
-                result = state == State.WRITING;
-                if (result)
-                {
-                    this.state = State.TIMEOUT;
-                    callback = this.callback;
-                    this.callback = null;
-                    this.failure = failure;
-                }
-            }
             if (LOG.isDebugEnabled())
-                LOG.debug(String.format("HTTP2 Response #%d/%h idle timeout %s", stream.getId(), stream.getSession(), result ? "expired" : "ignored"), failure);
+                LOG.debug("HTTP2 Response #{}/{} idle timeout {}",
-            if (result)
+                    stream.getId(), Integer.toHexString(stream.getSession().hashCode()),
+                    timeout ? "expired" : "ignored",
+                    failure);
+            if (timeout)
                 callback.failed(failure);
-            return result;
-        }
-
-        @Override
-        public InvocationType getInvocationType()
-        {
-            Callback callback;
-            synchronized (this)
-            {
-                callback = this.callback;
-            }
-            return callback != null ? callback.getInvocationType() : Callback.super.getInvocationType();
         }
     }
 
+    /**
+     * <p>Send states for {@link TransportCallback}.</p>
+     *
+     * @see TransportCallback
+     */
     private enum State
     {
-        IDLE, WRITING, FAILED, TIMEOUT
+        /**
+         * <p>No send initiated or in progress.</p>
+         * <p>Next states could be:</p>
+         * <ul>
+         *   <li>{@link #SENDING}, when {@link TransportCallback#send(Callback, boolean, Consumer)}
+         *   is called by the transport to initiate a send</li>
+         *   <li>{@link #FAILED}, when {@link TransportCallback#failed(Throwable)}
+         *   is called by an asynchronous failure</li>
+         * </ul>
+         */
+        IDLE,
+        /**
+         * <p>A send is initiated; the nested callback in {@link TransportCallback}
+         * cannot be notified while in this state.</p>
+         * <p>Next states could be:</p>
+         * <ul>
+         *   <li>{@link #SUCCEEDING}, when {@link TransportCallback#succeeded()}
+         *   is called synchronously because the send succeeded</li>
+         *   <li>{@link #FAILING}, when {@link TransportCallback#failed(Throwable)}
+         *   is called synchronously because the send failed</li>
+         *   <li>{@link #PENDING}, when {@link TransportCallback#pending()}
+         *   is called before the send completes</li>
+         * </ul>
+         */
+        SENDING,
+        /**
+         * <p>A send was initiated and is now pending, waiting for the {@link TransportCallback}
+         * to be notified of success or failure.</p>
+         * <p>Next states could be:</p>
+         * <ul>
+         *   <li>{@link #IDLE}, when {@link TransportCallback#succeeded()}
+         *   is called because the send succeeded</li>
+         *   <li>{@link #FAILED}, when {@link TransportCallback#failed(Throwable)}
+         *   is called because either the send failed, or an asynchronous failure happened</li>
+         * </ul>
+         */
+        PENDING,
+        /**
+         * <p>A send was initiated and succeeded, but {@link TransportCallback#pending()}
+         * has not been called yet.</p>
+         * <p>This state indicates that the success actions (such as notifying the
+         * {@link TransportCallback} nested callback) must be performed when
+         * {@link TransportCallback#pending()} is called.</p>
+         * <p>Next states could be:</p>
+         * <ul>
+         *   <li>{@link #IDLE}, when {@link TransportCallback#pending()}
+         *   is called</li>
+         * </ul>
+         */
+        SUCCEEDING,
+        /**
+         * <p>A send was initiated and failed, but {@link TransportCallback#pending()}
+         * has not been called yet.</p>
+         * <p>This state indicates that the failure actions (such as notifying the
+         * {@link TransportCallback} nested callback) must be performed when
+         * {@link TransportCallback#pending()} is called.</p>
+         * <p>Next states could be:</p>
+         * <ul>
+         *   <li>{@link #FAILED}, when {@link TransportCallback#pending()}
+         *   is called</li>
+         * </ul>
+         */
+        FAILING,
+        /**
+         * <p>The terminal state indicating failure of the send.</p>
+         */
+        FAILED
     }
 
     private class SendTrailers extends Callback.Nested
@@ -525,8 +766,8 @@ public class HttpTransportOverHTTP2 implements HttpTransport
         @Override
         public void succeeded()
         {
-            if (transportCallback.start(getCallback(), false))
+            transportCallback.send(getCallback(), false, c ->
-                sendTrailersFrame(new MetaData(HttpVersion.HTTP_2, trailers), transportCallback);
+                sendTrailersFrame(new MetaData(HttpVersion.HTTP_2, trailers), c));
         }
     }
 }

jetty-openid/src/main/java/org/eclipse/jetty/security/openid/OpenIdAuthenticator.java

@@ -249,7 +249,7 @@ public class OpenIdAuthenticator extends LoginAuthenticator
         if (!mandatory)
             return new DeferredAuthentication(this);
 
-        if (isErrorPage(URIUtil.addPaths(request.getServletPath(), request.getPathInfo())) && !DeferredAuthentication.isDeferred(response))
+        if (isErrorPage(baseRequest.getPathInContext()) && !DeferredAuthentication.isDeferred(response))
             return new DeferredAuthentication(this);
 
         try

jetty-quickstart/src/test/java/org/eclipse/jetty/quickstart/FooContextListener.java

@@ -34,21 +34,31 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
  */
 public class FooContextListener implements ServletContextListener
 {
+    static int ___initialized;
+    static int __destroyed;
+
     @Override
     public void contextInitialized(ServletContextEvent sce)
     {
+        ++___initialized;
+
         ServletRegistration defaultRego = sce.getServletContext().getServletRegistration("default");
         Collection<String> mappings = defaultRego.getMappings();
         assertThat("/", is(in(mappings)));
 
-        Set<String> otherMappings = sce.getServletContext().getServletRegistration("foo").addMapping("/");
+        ServletRegistration rego = sce.getServletContext().getServletRegistration("foo");
-        assertTrue(otherMappings.isEmpty());
+        if (rego != null)
-        Collection<String> fooMappings = sce.getServletContext().getServletRegistration("foo").getMappings();
+        {
-        assertThat("/", is(in(fooMappings)));
+            Set<String> otherMappings = rego.addMapping("/");
+            assertTrue(otherMappings.isEmpty());
+            Collection<String> fooMappings = rego.getMappings();
+            assertThat("/", is(in(fooMappings)));
+        }
     }
 
     @Override
     public void contextDestroyed(ServletContextEvent sce)
     {
+        ++__destroyed;
     }
 }

jetty-quickstart/src/test/java/org/eclipse/jetty/quickstart/TestQuickStart.java

@@ -79,7 +79,7 @@ public class TestQuickStart
         fooHolder.setName("foo");
         quickstart.getServletHandler().addServlet(fooHolder);
         ListenerHolder lholder = new ListenerHolder();
-        lholder.setListener(new FooContextListener());
+        lholder.setClassName("org.eclipse.jetty.quickstart.FooContextListener");
         quickstart.getServletHandler().addListener(lholder);
         server.setHandler(quickstart);
         server.setDryRun(true);
@@ -177,4 +177,30 @@ public class TestQuickStart
         assertEquals("ascii", webapp.getDefaultRequestCharacterEncoding());
         assertEquals("utf-16", webapp.getDefaultResponseCharacterEncoding());
     }
+    
+    @Test
+    public void testListenersNotCalledInPreConfigure() throws Exception
+    {
+        File quickstartXml = new File(webInf, "quickstart-web.xml");
+        assertFalse(quickstartXml.exists());
+        
+        Server server = new Server();
+        
+        WebAppContext quickstart = new WebAppContext();
+        quickstart.addConfiguration(new QuickStartConfiguration());
+        quickstart.setAttribute(QuickStartConfiguration.MODE, QuickStartConfiguration.Mode.GENERATE);
+        quickstart.setAttribute(QuickStartConfiguration.ORIGIN_ATTRIBUTE, "origin");
+
+        //add a listener directly to the ContextHandler so it is there when we start -
+        //if you add them to the ServletHandler (like StandardDescriptorProcessor does)
+        //then they are not added to the ContextHandler in a pre-generate.
+        quickstart.addEventListener(new FooContextListener());
+        quickstart.setResourceBase(testDir.getAbsolutePath());
+        server.setHandler(quickstart);
+        server.setDryRun(true);
+
+        server.start();
+        assertTrue(quickstartXml.exists());
+        assertEquals(0, FooContextListener.___initialized);
+    }
 }

jetty-rewrite/src/main/java/org/eclipse/jetty/rewrite/handler/RuleContainer.java

@@ -195,7 +195,7 @@ public class RuleContainer extends Rule implements Dumpable
                 }
 
                 if (_rewritePathInfo)
-                    baseRequest.setPathInfo(applied);
+                    baseRequest.setContext(baseRequest.getContext(), applied);
 
                 target = applied;
 

jetty-rewrite/src/test/java/org/eclipse/jetty/rewrite/handler/RewriteHandlerTest.java

@@ -85,7 +85,7 @@ public class RewriteHandlerTest extends AbstractRuleTestCase
         _handler.setRewriteRequestURI(true);
         _handler.setRewritePathInfo(true);
         _request.setHttpURI(HttpURI.build(_request.getHttpURI(), "/xxx/bar"));
-        _request.setPathInfo("/xxx/bar");
+        _request.setContext(_request.getContext(), "/xxx/bar");
         _handler.handle("/xxx/bar", _request, _request, _response);
         assertEquals(201, _response.getStatus());
         assertEquals("/bar/zzz", _request.getAttribute("target"));
@@ -99,7 +99,7 @@ public class RewriteHandlerTest extends AbstractRuleTestCase
         _handler.setRewriteRequestURI(false);
         _handler.setRewritePathInfo(false);
         _request.setHttpURI(HttpURI.build(_request.getHttpURI(), "/foo/bar"));
-        _request.setPathInfo("/foo/bar");
+        _request.setContext(_request.getContext(), "/foo/bar");
 
         _handler.handle("/foo/bar", _request, _request, _response);
         assertEquals(201, _response.getStatus());
@@ -112,7 +112,7 @@ public class RewriteHandlerTest extends AbstractRuleTestCase
         _request.setHandled(false);
         _handler.setOriginalPathAttribute(null);
         _request.setHttpURI(HttpURI.build(_request.getHttpURI(), "/aaa/bar"));
-        _request.setPathInfo("/aaa/bar");
+        _request.setContext(_request.getContext(), "/aaa/bar");
         _handler.handle("/aaa/bar", _request, _request, _response);
         assertEquals(201, _response.getStatus());
         assertEquals("/ddd/bar", _request.getAttribute("target"));
@@ -126,7 +126,7 @@ public class RewriteHandlerTest extends AbstractRuleTestCase
         _handler.setRewriteRequestURI(true);
         _handler.setRewritePathInfo(true);
         _request.setHttpURI(HttpURI.build(_request.getHttpURI(), "/aaa/bar"));
-        _request.setPathInfo("/aaa/bar");
+        _request.setContext(_request.getContext(), "/aaa/bar");
         _handler.handle("/aaa/bar", _request, _request, _response);
         assertEquals(201, _response.getStatus());
         assertEquals("/ddd/bar", _request.getAttribute("target"));
@@ -138,7 +138,7 @@ public class RewriteHandlerTest extends AbstractRuleTestCase
         _request.setHandled(false);
         _rule2.setTerminating(true);
         _request.setHttpURI(HttpURI.build(_request.getHttpURI(), "/aaa/bar"));
-        _request.setPathInfo("/aaa/bar");
+        _request.setContext(_request.getContext(), "/aaa/bar");
         _handler.handle("/aaa/bar", _request, _request, _response);
         assertEquals(201, _response.getStatus());
         assertEquals("/ccc/bar", _request.getAttribute("target"));
@@ -154,7 +154,7 @@ public class RewriteHandlerTest extends AbstractRuleTestCase
         _request.setAttribute("URI", null);
         _request.setAttribute("info", null);
         _request.setHttpURI(HttpURI.build(_request.getHttpURI(), "/aaa/bar"));
-        _request.setPathInfo("/aaa/bar");
+        _request.setContext(_request.getContext(), "/aaa/bar");
         _handler.handle("/aaa/bar", _request, _request, _response);
         assertEquals(200, _response.getStatus());
         assertEquals(null, _request.getAttribute("target"));
@@ -173,7 +173,7 @@ public class RewriteHandlerTest extends AbstractRuleTestCase
         _handler.setRewriteRequestURI(true);
         _handler.setRewritePathInfo(false);
         _request.setHttpURI(HttpURI.build(_request.getHttpURI(), "/ccc/x%20y"));
-        _request.setPathInfo("/ccc/x y");
+        _request.setContext(_request.getContext(), "/ccc/x y");
         _handler.handle("/ccc/x y", _request, _request, _response);
         assertEquals(201, _response.getStatus());
         assertEquals("/ddd/x y", _request.getAttribute("target"));
@@ -190,7 +190,7 @@ public class RewriteHandlerTest extends AbstractRuleTestCase
         _handler.setRewriteRequestURI(true);
         _handler.setRewritePathInfo(false);
         _request.setHttpURI(HttpURI.build(_request.getHttpURI(), "/xxx/x%20y"));
-        _request.setPathInfo("/xxx/x y");
+        _request.setContext(_request.getContext(), "/xxx/x y");
         _handler.handle("/xxx/x y", _request, _request, _response);
         assertEquals(201, _response.getStatus());
         assertEquals("/x y/zzz", _request.getAttribute("target"));

jetty-security/src/main/java/org/eclipse/jetty/security/authentication/FormAuthenticator.java

@@ -254,7 +254,7 @@ public class FormAuthenticator extends LoginAuthenticator
         if (!mandatory)
             return new DeferredAuthentication(this);
 
-        if (isLoginOrErrorPage(URIUtil.addPaths(request.getServletPath(), request.getPathInfo())) && !DeferredAuthentication.isDeferred(response))
+        if (isLoginOrErrorPage(baseRequest.getPathInContext()) && !DeferredAuthentication.isDeferred(response))
             return new DeferredAuthentication(this);
 
         try

jetty-server/src/main/config/etc/sessions/session-cache-hash.xml

@@ -15,6 +15,7 @@
      <Set name="saveOnCreate"><Property name="jetty.session.saveOnCreate" default="false" /></Set>
      <Set name="removeUnloadableSessions"><Property name="jetty.session.removeUnloadableSessions" default="false"/></Set>
      <Set name="flushOnResponseCommit"><Property name="jetty.session.flushOnResponseCommit" default="false"/></Set>
+     <Set name="invalidateOnShutdown"><Property name="jetty.session.invalidateOnShutdown" default="false"/></Set>
     </New>
    </Arg>
   </Call>

jetty-server/src/main/config/modules/http.mod

@@ -34,7 +34,7 @@ etc/jetty-http.xml
 # jetty.http.selectors=-1
 
 ## ServerSocketChannel backlog (0 picks platform default)
-# jetty.http.acceptorQueueSize=0
+# jetty.http.acceptQueueSize=0
 
 ## Thread priority delta to give to acceptor threads
 # jetty.http.acceptorPriorityDelta=0

jetty-server/src/main/config/modules/session-cache-hash.mod

@@ -23,3 +23,4 @@ etc/sessions/session-cache-hash.xml
 #jetty.session.saveOnCreate=false
 #jetty.session.removeUnloadableSessions=false
 #jetty.session.flushOnResponseCommit=false
+#jetty.session.invalidateOnShutdown=false

jetty-server/src/main/config/modules/session-cache-null.mod

@@ -18,4 +18,4 @@ etc/sessions/session-cache-null.xml
 [ini-template]
 #jetty.session.saveOnCreate=false
 #jetty.session.removeUnloadableSessions=false
-#jetty.session.flushOnResponseCommit=false
+#jetty.session.flushOnResponseCommit=false

jetty-server/src/main/config/modules/ssl.mod

@@ -35,7 +35,7 @@ etc/jetty-ssl-context.xml
 # jetty.ssl.selectors=-1
 
 ## ServerSocketChannel backlog (0 picks platform default)
-# jetty.ssl.acceptorQueueSize=0
+# jetty.ssl.acceptQueueSize=0
 
 ## Thread priority delta to give to acceptor threads
 # jetty.ssl.acceptorPriorityDelta=0

jetty-server/src/main/java/org/eclipse/jetty/server/Dispatcher.java

@@ -90,6 +90,8 @@ public class Dispatcher implements RequestDispatcher
         final DispatcherType old_type = baseRequest.getDispatcherType();
         final Attributes old_attr = baseRequest.getAttributes();
         final MultiMap<String> old_query_params = baseRequest.getQueryParameters();
+        final ContextHandler.Context old_context = baseRequest.getContext();
+        final ServletPathMapping old_mapping = baseRequest.getServletPathMapping();
         try
         {
             baseRequest.setDispatcherType(DispatcherType.INCLUDE);
@@ -100,7 +102,14 @@ public class Dispatcher implements RequestDispatcher
             }
             else
             {
-                IncludeAttributes attr = new IncludeAttributes(old_attr, _uri.getPath(), _contextHandler.getContextPath(), _pathInContext, _uri.getQuery());
+                IncludeAttributes attr = new IncludeAttributes(
+                    old_attr,
+                    baseRequest,
+                    old_context,
+                    old_mapping,
+                    _uri.getPath(),
+                    _pathInContext,
+                    _uri.getQuery());
                 if (attr._query != null)
                     baseRequest.mergeQueryParameters(baseRequest.getQueryString(), attr._query);
                 baseRequest.setAttributes(attr);
@@ -136,11 +145,10 @@ public class Dispatcher implements RequestDispatcher
             response = new ServletResponseHttpWrapper(response);
 
         final HttpURI old_uri = baseRequest.getHttpURI();
-        final String old_context_path = baseRequest.getContextPath();
+        final ContextHandler.Context old_context = baseRequest.getContext();
-        final String old_servlet_path = baseRequest.getServletPath();
+        final String old_path_in_context = baseRequest.getPathInContext();
-        final String old_path_info = baseRequest.getPathInfo();
         final ServletPathMapping old_mapping = baseRequest.getServletPathMapping();
-
+        final ServletPathMapping source_mapping = baseRequest.findServletPathMapping();
         final MultiMap<String> old_query_params = baseRequest.getQueryParameters();
         final Attributes old_attr = baseRequest.getAttributes();
         final DispatcherType old_type = baseRequest.getDispatcherType();
@@ -161,30 +169,21 @@ public class Dispatcher implements RequestDispatcher
                 // for queryString is allowed to be null, but cannot be null for the other values.
                 // Note: the pathInfo is passed as the pathInContext since it is only used when there is
                 // no mapping, and when there is no mapping the pathInfo is the pathInContext.
-                // TODO Ultimately it is intended for the request to carry the pathInContext for easy access
+                if (old_attr.getAttribute(FORWARD_REQUEST_URI) == null)
-                ForwardAttributes attr = old_attr.getAttribute(FORWARD_REQUEST_URI) != null
+                    baseRequest.setAttributes(new ForwardAttributes(old_attr,
-                    ? new ForwardAttributes(old_attr,
+                        old_uri.getPath(),
-                    (String)old_attr.getAttribute(FORWARD_REQUEST_URI),
+                        old_context == null ? null : old_context.getContextHandler().getContextPathEncoded(),
-                    (String)old_attr.getAttribute(FORWARD_CONTEXT_PATH),
+                        baseRequest.getPathInContext(),
-                    (String)old_attr.getAttribute(FORWARD_PATH_INFO),
+                        source_mapping,
-                    (ServletPathMapping)old_attr.getAttribute(FORWARD_MAPPING),
+                        old_uri.getQuery()));
-                    (String)old_attr.getAttribute(FORWARD_QUERY_STRING))
-                    : new ForwardAttributes(old_attr,
-                    old_uri.getPath(),
-                    old_context_path,
-                    baseRequest.getPathInfo(), // TODO replace with pathInContext
-                    old_mapping,
-                    old_uri.getQuery());
 
                 String query = _uri.getQuery();
                 if (query == null)
                     query = old_uri.getQuery();
 
                 baseRequest.setHttpURI(HttpURI.build(old_uri, _uri.getPath(), _uri.getParam(), query));
-                baseRequest.setContextPath(_contextHandler.getContextPath());
+                baseRequest.setContext(_contextHandler.getServletContext(), _pathInContext);
                 baseRequest.setServletPathMapping(null);
-                baseRequest.setServletPath(null);
-                baseRequest.setPathInfo(_pathInContext);
 
                 if (_uri.getQuery() != null || old_uri.getQuery() != null)
                 {
@@ -207,8 +206,6 @@ public class Dispatcher implements RequestDispatcher
                     }
                 }
 
-                baseRequest.setAttributes(attr);
-
                 _contextHandler.handle(_pathInContext, baseRequest, (HttpServletRequest)request, (HttpServletResponse)response);
 
                 // If we are not async and not closed already, then close via the possibly wrapped response.
@@ -228,9 +225,8 @@ public class Dispatcher implements RequestDispatcher
         finally
         {
             baseRequest.setHttpURI(old_uri);
-            baseRequest.setContextPath(old_context_path);
+            baseRequest.setContext(old_context, old_path_in_context);
-            baseRequest.setServletPath(old_servlet_path);
+            baseRequest.setServletPathMapping(old_mapping);
-            baseRequest.setPathInfo(old_path_info);
             baseRequest.setQueryParameters(old_query_params);
             baseRequest.resetParameters();
             baseRequest.setAttributes(old_attr);
@@ -346,23 +342,44 @@ public class Dispatcher implements RequestDispatcher
         }
     }
 
-    private class IncludeAttributes extends Attributes.Wrapper
+    /**
+     * Attributes Wrapper to provide the {@link DispatcherType#INCLUDE} attributes.
+     *
+     * The source {@link org.eclipse.jetty.server.handler.ContextHandler.Context} and
+     * {@link ServletPathMapping} instances are also retained by this wrapper so they
+     * may be used by {@link Request#getContextPath()}, {@link Request#getServletPath()},
+     * {@link Request#getPathInfo()} and {@link Request#getHttpServletMapping()}.
+     */
+    class IncludeAttributes extends Attributes.Wrapper
     {
+        private final Request _baseRequest;
+        private final ContextHandler.Context _sourceContext;
+        private final ServletPathMapping _sourceMapping;
         private final String _requestURI;
-        private final String _contextPath;
         private final String _pathInContext;
-        private ServletPathMapping _servletPathMapping; // Set later by ServletHandler
         private final String _query;
 
-        public IncludeAttributes(Attributes attributes, String requestURI, String contextPath, String pathInContext, String query)
+        public IncludeAttributes(Attributes attributes, Request baseRequest, ContextHandler.Context sourceContext, ServletPathMapping sourceMapping, String requestURI, String pathInContext, String query)
         {
             super(attributes);
+            _baseRequest = baseRequest;
+            _sourceMapping = sourceMapping;
             _requestURI = requestURI;
-            _contextPath = contextPath;
+            _sourceContext = sourceContext;
             _pathInContext = pathInContext;
             _query = query;
         }
 
+        ContextHandler.Context getSourceContext()
+        {
+            return _sourceContext;
+        }
+
+        ServletPathMapping getSourceMapping()
+        {
+            return _sourceMapping;
+        }
+
         @Override
         public Object getAttribute(String key)
         {
@@ -371,17 +388,26 @@ public class Dispatcher implements RequestDispatcher
                 switch (key)
                 {
                     case INCLUDE_PATH_INFO:
-                        return _servletPathMapping == null ? _pathInContext : _servletPathMapping.getPathInfo();
+                    {
+                        ServletPathMapping mapping = _baseRequest.getServletPathMapping();
+                        return mapping == null ? _pathInContext : mapping.getPathInfo();
+                    }
                     case INCLUDE_SERVLET_PATH:
-                        return _servletPathMapping == null ? null : _servletPathMapping.getServletPath();
+                    {
+                        ServletPathMapping mapping = _baseRequest.getServletPathMapping();
+                        return mapping == null ? null : mapping.getServletPath();
+                    }
                     case INCLUDE_CONTEXT_PATH:
-                        return _contextPath;
+                    {
+                        ContextHandler.Context context = _baseRequest.getContext();
+                        return context == null ? null : context.getContextHandler().getContextPathEncoded();
+                    }
                     case INCLUDE_QUERY_STRING:
                         return _query;
                     case INCLUDE_REQUEST_URI:
                         return _requestURI;
                     case INCLUDE_MAPPING:
-                        return _servletPathMapping;
+                        return _baseRequest.getServletPathMapping();
                     default:
                         break;
                 }
@@ -416,12 +442,9 @@ public class Dispatcher implements RequestDispatcher
         @Override
         public void setAttribute(String key, Object value)
         {
-            if (_servletPathMapping == null && _named == null && INCLUDE_MAPPING.equals(key))
+            // Allow any attribute to be set, even if a reserved name. If a reserved
-                _servletPathMapping = (ServletPathMapping)value;
+            // name is set here, it will be revealed after the include is complete.
-            else
+            _attributes.setAttribute(key, value);
-                // Allow any attribute to be set, even if a reserved name. If a reserved
-                // name is set here, it will be revealed after the include is complete.
-                _attributes.setAttribute(key, value);
         }
 
         @Override

jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java

@@ -822,13 +822,16 @@ public class HttpChannelState
         // check the actions of the listeners
         synchronized (this)
         {
-            // If we are still async and nobody has called sendError
             if (_requestState == RequestState.ASYNC && !_sendError)
-                // Then the listeners did not invoke API methods
+            {
-                // and the container must provide a default error dispatch.
+                // The listeners did not invoke API methods and the
+                // container must provide a default error dispatch.
                 sendError(th);
-            else
+            }
+            else if (_requestState != RequestState.COMPLETE)
+            {
                 LOG.warn("unhandled in state " + _requestState, new IllegalStateException(th));
+            }
         }
     }
 

jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java

@@ -34,7 +34,6 @@ import org.eclipse.jetty.http.HttpHeaderValue;
 import org.eclipse.jetty.http.HttpMethod;
 import org.eclipse.jetty.http.HttpParser;
 import org.eclipse.jetty.http.HttpParser.RequestHandler;
-import org.eclipse.jetty.http.HttpStatus;
 import org.eclipse.jetty.http.MetaData;
 import org.eclipse.jetty.http.PreEncodedHttpField;
 import org.eclipse.jetty.io.AbstractConnection;
@@ -49,6 +48,8 @@ import org.eclipse.jetty.util.IteratingCallback;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import static org.eclipse.jetty.http.HttpStatus.INTERNAL_SERVER_ERROR_500;
+
 /**
  * <p>A {@link Connection} that handles the HTTP protocol.</p>
  */
@@ -67,7 +68,6 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
     private final HttpParser _parser;
     private final AtomicInteger _contentBufferReferences = new AtomicInteger();
     private volatile ByteBuffer _requestBuffer = null;
-    private volatile ByteBuffer _chunk = null;
     private final BlockingReadCallback _blockingReadCallback = new BlockingReadCallback();
     private final AsyncReadCallback _asyncReadCallback = new AsyncReadCallback();
     private final SendCallback _sendCallback = new SendCallback();
@@ -464,11 +464,6 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
                 _parser.close();
         }
 
-        // Not in a race here with onFillable, because it has given up control before calling handle.
-        // in a slight race with #completed, but not sure what to do with that anyway.
-        if (_chunk != null)
-            _bufferPool.release(_chunk);
-        _chunk = null;
         _generator.reset();
 
         // if we are not called from the onfillable thread, schedule completion
@@ -718,6 +713,7 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
         private boolean _lastContent;
         private Callback _callback;
         private ByteBuffer _header;
+        private ByteBuffer _chunk;
         private boolean _shutdownOut;
 
         private SendCallback()
@@ -763,10 +759,9 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
                 throw new IllegalStateException();
 
             boolean useDirectByteBuffers = isUseOutputDirectByteBuffers();
-            ByteBuffer chunk = _chunk;
             while (true)
             {
-                HttpGenerator.Result result = _generator.generateResponse(_info, _head, _header, chunk, _content, _lastContent);
+                HttpGenerator.Result result = _generator.generateResponse(_info, _head, _header, _chunk, _content, _lastContent);
                 if (LOG.isDebugEnabled())
                     LOG.debug("generate: {} for {} ({},{},{})@{}",
                         result,
@@ -788,23 +783,21 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
                     }
                     case HEADER_OVERFLOW:
                     {
-                        int capacity = _header.capacity();
+                        if (_header.capacity() >= _config.getResponseHeaderSize())
-                        _bufferPool.release(_header);
+                            throw new BadMessageException(INTERNAL_SERVER_ERROR_500, "Response header too large");
-                        if (capacity >= _config.getResponseHeaderSize())
+                        releaseHeader();
-                            throw new BadMessageException(HttpStatus.INTERNAL_SERVER_ERROR_500, "Response header too large");
                         _header = _bufferPool.acquire(_config.getResponseHeaderSize(), useDirectByteBuffers);
                         continue;
                     }
                     case NEED_CHUNK:
                     {
-                        chunk = _chunk = _bufferPool.acquire(HttpGenerator.CHUNK_SIZE, useDirectByteBuffers);
+                        _chunk = _bufferPool.acquire(HttpGenerator.CHUNK_SIZE, useDirectByteBuffers);
                         continue;
                     }
                     case NEED_CHUNK_TRAILER:
                     {
-                        if (_chunk != null)
+                        releaseChunk();
-                            _bufferPool.release(_chunk);
+                        _chunk = _bufferPool.acquire(_config.getResponseHeaderSize(), useDirectByteBuffers);
-                        chunk = _chunk = _bufferPool.acquire(_config.getResponseHeaderSize(), useDirectByteBuffers);
                         continue;
                     }
                     case FLUSH:
@@ -812,7 +805,7 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
                         // Don't write the chunk or the content if this is a HEAD response, or any other type of response that should have no content
                         if (_head || _generator.isNoContent())
                         {
-                            BufferUtil.clear(chunk);
+                            BufferUtil.clear(_chunk);
                             BufferUtil.clear(_content);
                         }
 
@@ -823,10 +816,10 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
                             gatherWrite += 4;
                             bytes += _header.remaining();
                         }
-                        if (BufferUtil.hasContent(chunk))
+                        if (BufferUtil.hasContent(_chunk))
                         {
                             gatherWrite += 2;
-                            bytes += chunk.remaining();
+                            bytes += _chunk.remaining();
                         }
                         if (BufferUtil.hasContent(_content))
                         {
@@ -837,10 +830,10 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
                         switch (gatherWrite)
                         {
                             case 7:
-                                getEndPoint().write(this, _header, chunk, _content);
+                                getEndPoint().write(this, _header, _chunk, _content);
                                 break;
                             case 6:
-                                getEndPoint().write(this, _header, chunk);
+                                getEndPoint().write(this, _header, _chunk);
                                 break;
                             case 5:
                                 getEndPoint().write(this, _header, _content);
@@ -849,10 +842,10 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
                                 getEndPoint().write(this, _header);
                                 break;
                             case 3:
-                                getEndPoint().write(this, chunk, _content);
+                                getEndPoint().write(this, _chunk, _content);
                                 break;
                             case 2:
-                                getEndPoint().write(this, chunk);
+                                getEndPoint().write(this, _chunk);
                                 break;
                             case 1:
                                 getEndPoint().write(this, _content);
@@ -896,10 +889,23 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
             _callback = null;
             _info = null;
             _content = null;
+            releaseHeader();
+            releaseChunk();
+            return complete;
+        }
+
+        private void releaseHeader()
+        {
             if (_header != null)
                 _bufferPool.release(_header);
             _header = null;
-            return complete;
+        }
+
+        private void releaseChunk()
+        {
+            if (_chunk != null)
+                _bufferPool.release(_chunk);
+            _chunk = null;
         }
 
         @Override

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

@@ -107,14 +107,16 @@ import org.slf4j.LoggerFactory;
  * request object to be as lightweight as possible and not actually implement any significant behavior. For example
  * <ul>
  *
- * <li>The {@link Request#getContextPath()} method will return null, until the request has been passed to a {@link ContextHandler} which matches the
+ * <li>the {@link Request#getContextPath()} method will return null, until the request has been passed to a {@link ContextHandler} which matches the
- * {@link Request#getPathInfo()} with a context path and calls {@link Request#setContextPath(String)} as a result.</li>
+ * {@link Request#getPathInfo()} with a context path and calls {@link Request#setContext(Context,String)} as a result.  For
+ * some dispatch types (ie include and named dispatch) the context path may not reflect the {@link ServletContext} set
+ * by {@link Request#setContext(Context, String)}.</li>
  *
  * <li>the HTTP session methods will all return null sessions until such time as a request has been passed to a
  * {@link org.eclipse.jetty.server.session.SessionHandler} which checks for session cookies and enables the ability to create new sessions.</li>
  *
- * <li>The {@link Request#getServletPath()} method will return null until the request has been passed to a <code>org.eclipse.jetty.servlet.ServletHandler</code>
+ * <li>The {@link Request#getServletPath()} method will return "" until the request has been passed to a <code>org.eclipse.jetty.servlet.ServletHandler</code>
- * and the pathInfo matched against the servlet URL patterns and {@link Request#setServletPath(String)} called as a result.</li>
+ * and the pathInfo matched against the servlet URL patterns and {@link Request#setServletPathMapping(ServletPathMapping)} called as a result.</li>
  * </ul>
  *
  * <p>
@@ -198,9 +200,7 @@ public class Request implements HttpServletRequest
     private HttpFields _trailers;
     private HttpURI _uri;
     private String _method;
-    private String _contextPath;
+    private String _pathInContext;
-    private String _servletPath;
-    private String _pathInfo;
     private ServletPathMapping _servletPathMapping;
     private boolean _secure;
     private String _asyncNotSupportedSource = null;
@@ -777,7 +777,44 @@ public class Request implements HttpServletRequest
     @Override
     public String getContextPath()
     {
-        return _contextPath;
+        // The context path returned is normally for the current context.  Except during a cross context
+        // INCLUDE dispatch, in which case this method returns the context path of the source context,
+        // which we recover from the IncludeAttributes wrapper.
+        Context context;
+        if (_dispatcherType == DispatcherType.INCLUDE)
+        {
+            Dispatcher.IncludeAttributes include = Attributes.unwrap(_attributes, Dispatcher.IncludeAttributes.class);
+            context = (include == null) ? _context : include.getSourceContext();
+        }
+        else
+        {
+            context = _context;
+        }
+
+        if (context == null)
+            return null;
+
+        // For some reason the spec requires the context path to be encoded (unlike getServletPath).
+        String contextPath = context.getContextHandler().getContextPathEncoded();
+
+        // For the root context, the spec requires that the empty string is returned instead of the leading '/'
+        // which is included in the pathInContext
+        if (URIUtil.SLASH.equals(contextPath))
+            return "";
+        return contextPath;
+    }
+
+    /** Get the path in the context.
+     *
+     * The path relative to the context path, analogous to {@link #getServletPath()} + {@link #getPathInfo()}.
+     * If no context is set, then the path in context is the full path.
+     * @return The decoded part of the {@link #getRequestURI()} path after any {@link #getContextPath()}
+     *         up to any {@link #getQueryString()}, excluding path parameters.
+     * @see #setContext(Context, String)
+     */
+    public String getPathInContext()
+    {
+        return _pathInContext;
     }
 
     @Override
@@ -1048,15 +1085,20 @@ public class Request implements HttpServletRequest
     @Override
     public String getPathInfo()
     {
-        return _pathInfo;
+        // The pathInfo returned is normally for the current servlet.  Except during an
+        // INCLUDE dispatch, in which case this method returns the pathInfo of the source servlet,
+        // which we recover from the IncludeAttributes wrapper.
+        ServletPathMapping mapping = findServletPathMapping();
+        return mapping == null ? _pathInContext : mapping.getPathInfo();
     }
 
     @Override
     public String getPathTranslated()
     {
-        if (_pathInfo == null || _context == null)
+        String pathInfo = getPathInfo();
+        if (pathInfo == null || _context == null)
             return null;
-        return _context.getRealPath(_pathInfo);
+        return _context.getRealPath(pathInfo);
     }
 
     @Override
@@ -1207,7 +1249,7 @@ public class Request implements HttpServletRequest
         // handle relative path
         if (!path.startsWith("/"))
         {
-            String relTo = URIUtil.addPaths(_servletPath, _pathInfo);
+            String relTo = _pathInContext;
             int slash = relTo.lastIndexOf("/");
             if (slash > 1)
                 relTo = relTo.substring(0, slash + 1);
@@ -1335,9 +1377,11 @@ public class Request implements HttpServletRequest
     @Override
     public String getServletPath()
     {
-        if (_servletPath == null)
+        // The servletPath returned is normally for the current servlet.  Except during an
-            _servletPath = "";
+        // INCLUDE dispatch, in which case this method returns the servletPath of the source servlet,
-        return _servletPath;
+        // which we recover from the IncludeAttributes wrapper.
+        ServletPathMapping mapping = findServletPathMapping();
+        return mapping == null ? "" : mapping.getServletPath();
     }
 
     public ServletResponse getServletResponse()
@@ -1678,10 +1722,10 @@ public class Request implements HttpServletRequest
 
         if (path == null || path.isEmpty())
         {
-            setPathInfo(encoded == null ? "" : encoded);
+            _pathInContext = encoded == null ? "" : encoded;
             throw new BadMessageException(400, "Bad URI");
         }
-        setPathInfo(path);
+        _pathInContext = path;
     }
 
     public org.eclipse.jetty.http.MetaData.Request getMetaData()
@@ -1740,13 +1784,12 @@ public class Request implements HttpServletRequest
         }
         _contentType = null;
         _characterEncoding = null;
-        _contextPath = null;
+        _pathInContext = null;
         if (_cookies != null)
             _cookies.reset();
         _cookiesExtracted = false;
         _context = null;
         _newContext = false;
-        _pathInfo = null;
         _queryEncoding = null;
         _requestedSessionId = null;
         _requestedSessionIdFromCookie = false;
@@ -1754,7 +1797,6 @@ public class Request implements HttpServletRequest
         _session = null;
         _sessionHandler = null;
         _scope = null;
-        _servletPath = null;
         _timeStamp = 0;
         _queryParameters = null;
         _contentParameters = null;
@@ -1831,6 +1873,13 @@ public class Request implements HttpServletRequest
         }
     }
 
+    /**
+     * Set the attributes for the request.
+     *
+     * @param attributes The attributes, which must be a {@link org.eclipse.jetty.util.Attributes.Wrapper}
+     *                   for which {@link Attributes#unwrap(Attributes)} will return the
+     *                   original {@link ServletAttributes}.
+     */
     public void setAttributes(Attributes attributes)
     {
         _attributes = attributes;
@@ -1864,7 +1913,7 @@ public class Request implements HttpServletRequest
                 // attributes there, under any other wrappers.
                 ((ServletAttributes)baseAttributes).setAsyncAttributes(getRequestURI(),
                     getContextPath(),
-                    getPathInfo(), // TODO change to pathInContext when cheaply available
+                    getPathInContext(),
                     getServletPathMapping(),
                     getQueryString());
             }
@@ -1955,25 +2004,24 @@ public class Request implements HttpServletRequest
     }
 
     /**
-     * Set request context
+     * Set request context and path in the context.
      *
      * @param context context object
+     * @param pathInContext the part of the URI path that is withing the context.
+     *                      For servlets, this is equal to servletPath + pathInfo
      */
-    public void setContext(Context context)
+    public void setContext(Context context, String pathInContext)
     {
         _newContext = _context != context;
-        if (context == null)
+        _context = context;
-            _context = null;
+        _pathInContext = pathInContext;
-        else
+        if (context != null)
-        {
-            _context = context;
             _errorContext = context;
-        }
     }
 
     /**
      * @return True if this is the first call of <code>takeNewContext()</code> since the last
-     * {@link #setContext(org.eclipse.jetty.server.handler.ContextHandler.Context)} call.
+     * {@link #setContext(org.eclipse.jetty.server.handler.ContextHandler.Context, String)} call.
      */
     public boolean takeNewContext()
     {
@@ -1982,17 +2030,6 @@ public class Request implements HttpServletRequest
         return nc;
     }
 
-    /**
-     * Sets the "context path" for this request
-     *
-     * @param contextPath the context path for this request
-     * @see HttpServletRequest#getContextPath()
-     */
-    public void setContextPath(String contextPath)
-    {
-        _contextPath = contextPath;
-    }
-
     /**
      * @param cookies The cookies to set.
      */
@@ -2026,14 +2063,6 @@ public class Request implements HttpServletRequest
         return HttpMethod.HEAD.is(getMethod());
     }
 
-    /**
-     * @param pathInfo The pathInfo to set.
-     */
-    public void setPathInfo(String pathInfo)
-    {
-        _pathInfo = pathInfo;
-    }
-
     /**
      * Set the character encoding used for the query string. This call will effect the return of getQueryString and getParamaters. It must be called before any
      * getParameter methods.
@@ -2071,14 +2100,6 @@ public class Request implements HttpServletRequest
         _requestedSessionIdFromCookie = requestedSessionIdCookie;
     }
 
-    /**
-     * @param servletPath The servletPath to set.
-     */
-    public void setServletPath(String servletPath)
-    {
-        _servletPath = servletPath;
-    }
-
     /**
      * @param session The session to set.
      */
@@ -2347,32 +2368,48 @@ public class Request implements HttpServletRequest
 
     /**
      * Set the servletPathMapping, the servletPath and the pathInfo.
-     * TODO remove the side effect on servletPath and pathInfo by removing those fields.
      * @param servletPathMapping The mapping used to return from {@link #getHttpServletMapping()}
      */
     public void setServletPathMapping(ServletPathMapping servletPathMapping)
     {
         _servletPathMapping = servletPathMapping;
-        if (servletPathMapping == null)
-        {
-            // TODO reset the servletPath and pathInfo, but currently cannot do that
-            // as we don't know the pathInContext.
-        }
-        else
-        {
-            _servletPath = servletPathMapping.getServletPath();
-            _pathInfo = servletPathMapping.getPathInfo();
-        }
     }
 
+    /**
+     * @return The mapping for the current target servlet, regardless of dispatch type.
+     */
     public ServletPathMapping getServletPathMapping()
     {
         return _servletPathMapping;
     }
 
+    /**
+     * @return The mapping for the target servlet reported by the {@link #getServletPath()} and
+     * {@link #getPathInfo()} methods.  For {@link DispatcherType#INCLUDE} dispatches, this
+     * method returns the mapping of the source servlet, otherwise it returns the mapping of
+     * the target servlet.
+     */
+    ServletPathMapping findServletPathMapping()
+    {
+        ServletPathMapping mapping;
+        if (_dispatcherType == DispatcherType.INCLUDE)
+        {
+            Dispatcher.IncludeAttributes include = Attributes.unwrap(_attributes, Dispatcher.IncludeAttributes.class);
+            mapping = (include == null) ? _servletPathMapping : include.getSourceMapping();
+        }
+        else
+        {
+            mapping = _servletPathMapping;
+        }
+        return mapping;
+    }
+
     @Override
     public HttpServletMapping getHttpServletMapping()
     {
-        return _servletPathMapping;
+        // The mapping returned is normally for the current servlet.  Except during an
+        // INCLUDE dispatch, in which case this method returns the mapping of the source servlet,
+        // which we recover from the IncludeAttributes wrapper.
+        return findServletPathMapping();
     }
 }

jetty-server/src/main/java/org/eclipse/jetty/server/Response.java

@@ -230,26 +230,30 @@ public class Response implements HttpServletResponse
     @Override
     public void addCookie(Cookie cookie)
     {
-        if (StringUtil.isBlank(cookie.getName()))
+        //Servlet Spec 9.3 Include method: cannot set a cookie if handling an include
-            throw new IllegalArgumentException("Cookie.name cannot be blank/null");
+        if (isMutable())
+        {
+            if (StringUtil.isBlank(cookie.getName()))
+                throw new IllegalArgumentException("Cookie.name cannot be blank/null");
 
-        String comment = cookie.getComment();
+            String comment = cookie.getComment();
-        // HttpOnly was supported as a comment in cookie flags before the java.net.HttpCookie implementation so need to check that
+            // HttpOnly was supported as a comment in cookie flags before the java.net.HttpCookie implementation so need to check that
-        boolean httpOnly = cookie.isHttpOnly() || HttpCookie.isHttpOnlyInComment(comment);
+            boolean httpOnly = cookie.isHttpOnly() || HttpCookie.isHttpOnlyInComment(comment);
-        SameSite sameSite = HttpCookie.getSameSiteFromComment(comment);
+            SameSite sameSite = HttpCookie.getSameSiteFromComment(comment);
-        comment = HttpCookie.getCommentWithoutAttributes(comment);
+            comment = HttpCookie.getCommentWithoutAttributes(comment);
 
-        addCookie(new HttpCookie(
+            addCookie(new HttpCookie(
-            cookie.getName(),
+                cookie.getName(),
-            cookie.getValue(),
+                cookie.getValue(),
-            cookie.getDomain(),
+                cookie.getDomain(),
-            cookie.getPath(),
+                cookie.getPath(),
-            cookie.getMaxAge(),
+                cookie.getMaxAge(),
-            httpOnly,
+                httpOnly,
-            cookie.getSecure(),
+                cookie.getSecure(),
-            comment,
+                comment,
-            cookie.getVersion(),
+                cookie.getVersion(),
-            sameSite));
+                sameSite));
+        }
     }
 
     /**
@@ -302,7 +306,6 @@ public class Response implements HttpServletResponse
         addCookie(cookie);
     }
 
-    @Override
     public boolean containsHeader(String name)
     {
         return _fields.contains(name);
@@ -332,7 +335,7 @@ public class Response implements HttpServletResponse
                 return url;
             if (request.getServerPort() != port)
                 return url;
-            if (!path.startsWith(request.getContextPath())) //TODO the root context path is "", with which every non null string starts
+            if (request.getContext() != null && !path.startsWith(request.getContextPath()))
                 return url;
         }
 

jetty-server/src/main/java/org/eclipse/jetty/server/SecureRequestCustomizer.java

@@ -283,10 +283,9 @@ public class SecureRequestCustomizer implements HttpConfiguration.Customizer
             request.getResponse().getHttpFields().add(_stsField);
     }
 
-    private X509Certificate[] getCertChain(Request request, SSLSession sslSession)
+    private X509Certificate[] getCertChain(Connector connector, SSLSession sslSession)
     {
         // The in-use SslContextFactory should be present in the Connector's SslConnectionFactory
-        Connector connector = request.getHttpChannel().getConnector();
         SslConnectionFactory sslConnectionFactory = connector.getConnectionFactory(SslConnectionFactory.class);
         if (sslConnectionFactory != null)
         {
@@ -338,16 +337,16 @@ public class SecureRequestCustomizer implements HttpConfiguration.Customizer
                 switch (name)
                 {
                     case JAVAX_SERVLET_REQUEST_X_509_CERTIFICATE:
-                        return SecureRequestCustomizer.this.getCertChain(_request, _session);
+                        return getSslSessionData().getCerts();
 
                     case JAVAX_SERVLET_REQUEST_CIPHER_SUITE:
                         return _session.getCipherSuite();
 
                     case JAVAX_SERVLET_REQUEST_KEY_SIZE:
-                        return SslContextFactory.deduceKeyLength(_session.getCipherSuite());
+                        return getSslSessionData().getKeySize();
 
                     case JAVAX_SERVLET_REQUEST_SSL_SESSION_ID:
-                        return TypeUtil.toHexString(_session.getId());
+                        return getSslSessionData().getIdStr();
 
                     default:
                         String sessionAttribute = getSslSessionAttribute();
@@ -363,6 +362,31 @@ public class SecureRequestCustomizer implements HttpConfiguration.Customizer
             return null;
         }
 
+        /**
+         * Get data belonging to the {@link SSLSession}.
+         *
+         * @return the SslSessionData
+         */
+        private SslSessionData getSslSessionData()
+        {
+            String key = SslSessionData.class.getName();
+            SslSessionData sslSessionData = (SslSessionData)_session.getValue(key);
+            if (sslSessionData == null)
+            {
+                String cipherSuite = _session.getCipherSuite();
+                int keySize = SslContextFactory.deduceKeyLength(cipherSuite);
+
+                X509Certificate[] certs = getCertChain(_request.getHttpChannel().getConnector(), _session);
+
+                byte[] bytes = _session.getId();
+                String idStr = TypeUtil.toHexString(bytes);
+
+                sslSessionData = new SslSessionData(keySize, certs, idStr);
+                _session.putValue(key, sslSessionData);
+            }
+            return sslSessionData;
+        }
+
         @Override
         public Set<String> getAttributeNameSet()
         {
@@ -377,4 +401,36 @@ public class SecureRequestCustomizer implements HttpConfiguration.Customizer
             return names;
         }
     }
+
+    /**
+     * Simple bundle of data that is cached in the SSLSession.
+     */
+    private static class SslSessionData
+    {
+        private final Integer _keySize;
+        private final X509Certificate[] _certs;
+        private final String _idStr;
+
+        private SslSessionData(Integer keySize, X509Certificate[] certs, String idStr)
+        {
+            this._keySize = keySize;
+            this._certs = certs;
+            this._idStr = idStr;
+        }
+
+        private Integer getKeySize()
+        {
+            return _keySize;
+        }
+
+        private X509Certificate[] getCerts()
+        {
+            return _certs;
+        }
+
+        private String getIdStr()
+        {
+            return _idStr;
+        }
+    }
 }

jetty-server/src/main/java/org/eclipse/jetty/server/Server.java

@@ -622,7 +622,7 @@ public class Server extends HandlerWrapper implements Attributes
                     baseRequest.mergeQueryParameters(oldUri.getQuery(), baseRequest.getQueryString());
             }
 
-            baseRequest.setPathInfo(baseRequest.getHttpURI().getDecodedPath());
+            baseRequest.setContext(null, baseRequest.getHttpURI().getDecodedPath());
             handleAsync(channel, event, baseRequest);
         }
         finally

jetty-server/src/main/java/org/eclipse/jetty/server/handler/BufferedResponseHandler.java

@@ -40,7 +40,6 @@ import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.IncludeExclude;
 import org.eclipse.jetty.util.IteratingCallback;
 import org.eclipse.jetty.util.StringUtil;
-import org.eclipse.jetty.util.URIUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -104,7 +103,7 @@ public class BufferedResponseHandler extends HandlerWrapper
     public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
     {
         final ServletContext context = baseRequest.getServletContext();
-        final String path = context == null ? baseRequest.getRequestURI() : URIUtil.addPaths(baseRequest.getServletPath(), baseRequest.getPathInfo());
+        final String path = baseRequest.getPathInContext();
         LOG.debug("{} handle {} in {}", this, baseRequest, context);
 
         HttpOutput out = baseRequest.getResponse().getHttpOutput();

jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java

@@ -956,6 +956,9 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu
 
     protected void callContextInitialized(ServletContextListener l, ServletContextEvent e)
     {
+        if (getServer().isDryRun())
+            return;
+        
         if (LOG.isDebugEnabled())
             LOG.debug("contextInitialized: {}->{}", e, l);
         l.contextInitialized(e);
@@ -963,6 +966,9 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu
 
     protected void callContextDestroyed(ServletContextListener l, ServletContextEvent e)
     {
+        if (getServer().isDryRun())
+            return;
+        
         if (LOG.isDebugEnabled())
             LOG.debug("contextDestroyed: {}->{}", e, l);
         l.contextDestroyed(e);
@@ -1150,13 +1156,11 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu
         if (LOG.isDebugEnabled())
             LOG.debug("scope {}|{}|{} @ {}", baseRequest.getContextPath(), baseRequest.getServletPath(), baseRequest.getPathInfo(), this);
 
+        final Thread currentThread = Thread.currentThread();
+        final ClassLoader oldClassloader = currentThread.getContextClassLoader();
         Context oldContext;
-        String oldContextPath = null;
+        String oldPathInContext = null;
-        String oldServletPath = null;
+        String pathInContext = target;
-        String oldPathInfo = null;
-        ClassLoader oldClassloader = null;
-        Thread currentThread = null;
-        String pathInfo = target;
 
         DispatcherType dispatch = baseRequest.getDispatcherType();
 
@@ -1177,47 +1181,31 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu
                 {
                     if (_contextPath.length() > 1)
                         target = target.substring(_contextPath.length());
-                    pathInfo = target;
+                    pathInContext = target;
                 }
                 else if (_contextPath.length() == 1)
                 {
                     target = URIUtil.SLASH;
-                    pathInfo = URIUtil.SLASH;
+                    pathInContext = URIUtil.SLASH;
                 }
                 else
                 {
                     target = URIUtil.SLASH;
-                    pathInfo = null;
+                    pathInContext = null;
                 }
             }
-
-            // Set the classloader
-            if (_classLoader != null)
-            {
-                currentThread = Thread.currentThread();
-                oldClassloader = currentThread.getContextClassLoader();
-                currentThread.setContextClassLoader(_classLoader);
-            }
         }
 
+        if (_classLoader != null)
+            currentThread.setContextClassLoader(_classLoader);
+
         try
         {
-            oldContextPath = baseRequest.getContextPath();
+            oldPathInContext = baseRequest.getPathInContext();
-            oldServletPath = baseRequest.getServletPath();
-            oldPathInfo = baseRequest.getPathInfo();
 
             // Update the paths
-            baseRequest.setContext(_scontext);
+            baseRequest.setContext(_scontext, pathInContext);
             __context.set(_scontext);
-            if (!DispatcherType.INCLUDE.equals(dispatch) && target.startsWith("/"))
-            {
-                if (_contextPath.length() == 1)
-                    baseRequest.setContextPath("");
-                else
-                    baseRequest.setContextPath(getContextPathEncoded());
-                baseRequest.setServletPath(null);
-                baseRequest.setPathInfo(pathInfo);
-            }
 
             if (oldContext != _scontext)
                 enterScope(baseRequest, dispatch);
@@ -1234,17 +1222,12 @@ public class ContextHandler extends ScopedHandler implements Attributes, Gracefu
                 exitScope(baseRequest);
 
                 // reset the classloader
-                if (_classLoader != null && currentThread != null)
+                if (_classLoader != null)
-                {
                     currentThread.setContextClassLoader(oldClassloader);
-                }
 
                 // reset the context and servlet path.
-                baseRequest.setContext(oldContext);
+                baseRequest.setContext(oldContext, oldPathInContext);
                 __context.set(oldContext);
-                baseRequest.setContextPath(oldContextPath);
-                baseRequest.setServletPath(oldServletPath);
-                baseRequest.setPathInfo(oldPathInfo);
             }
         }
     }

jetty-server/src/main/java/org/eclipse/jetty/server/handler/gzip/GzipHandler.java

@@ -45,7 +45,6 @@ import org.eclipse.jetty.server.handler.HandlerWrapper;
 import org.eclipse.jetty.util.IncludeExclude;
 import org.eclipse.jetty.util.RegexSet;
 import org.eclipse.jetty.util.StringUtil;
-import org.eclipse.jetty.util.URIUtil;
 import org.eclipse.jetty.util.compression.DeflaterPool;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -582,7 +581,7 @@ public class GzipHandler extends HandlerWrapper implements GzipFactory
     public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
     {
         final ServletContext context = baseRequest.getServletContext();
-        final String path = context == null ? baseRequest.getRequestURI() : URIUtil.addPaths(baseRequest.getServletPath(), baseRequest.getPathInfo());
+        final String path = baseRequest.getPathInContext();
         LOG.debug("{} handle {} in {}", this, baseRequest, context);
 
         if (!_dispatchers.contains(baseRequest.getDispatcherType()))

jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCache.java

@@ -99,6 +99,12 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
      * a dirty session will be flushed to the session store.
      */
     protected boolean _flushOnResponseCommit;
+    
+    /**
+     * If true, when the server shuts down, all sessions in the
+     * cache will be invalidated before being removed.
+     */
+    protected boolean _invalidateOnShutdown;
 
     /**
      * Create a new Session object from pre-existing session data
@@ -796,6 +802,18 @@ public abstract class AbstractSessionCache extends ContainerLifeCycle implements
         _saveOnInactiveEviction = saveOnEvict;
     }
 
+    @Override
+    public void setInvalidateOnShutdown(boolean invalidateOnShutdown)
+    {
+        _invalidateOnShutdown = invalidateOnShutdown;
+    }
+
+    @Override
+    public boolean isInvalidateOnShutdown()
+    {
+        return _invalidateOnShutdown;
+    }
+
     /**
      * Whether we should save a session that has been inactive before
      * we boot it from the cache.

jetty-server/src/main/java/org/eclipse/jetty/server/session/AbstractSessionCacheFactory.java

@@ -31,6 +31,19 @@ public abstract class AbstractSessionCacheFactory implements SessionCacheFactory
     boolean _saveOnCreate;
     boolean _removeUnloadableSessions;
     boolean _flushOnResponseCommit;
+    boolean _invalidateOnShutdown;
+    
+    public abstract SessionCache newSessionCache(SessionHandler handler);
+
+    public boolean isInvalidateOnShutdown()
+    {
+        return _invalidateOnShutdown;
+    }
+
+    public void setInvalidateOnShutdown(boolean invalidateOnShutdown)
+    {
+        _invalidateOnShutdown = invalidateOnShutdown;
+    }
 
     /**
      * @return the flushOnResponseCommit
@@ -111,4 +124,17 @@ public abstract class AbstractSessionCacheFactory implements SessionCacheFactory
     {
         _saveOnInactiveEvict = saveOnInactiveEvict;
     }
+
+    @Override
+    public SessionCache getSessionCache(SessionHandler handler)
+    {
+        SessionCache cache = newSessionCache(handler);
+        cache.setEvictionPolicy(getEvictionPolicy());
+        cache.setSaveOnInactiveEviction(isSaveOnInactiveEvict());
+        cache.setSaveOnCreate(isSaveOnCreate());
+        cache.setRemoveUnloadableSessions(isRemoveUnloadableSessions());
+        cache.setFlushOnResponseCommit(isFlushOnResponseCommit());
+        cache.setInvalidateOnShutdown(isInvalidateOnShutdown());
+        return cache;
+    }
 }

jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionCache.java

@@ -131,29 +131,18 @@ public class DefaultSessionCache extends AbstractSessionCache
     @Override
     public void shutdown()
     {
+        if (LOG.isDebugEnabled())
+            LOG.debug("Shutdown sessions, invalidating = {}", isInvalidateOnShutdown());
+
         // loop over all the sessions in memory (a few times if necessary to catch sessions that have been
         // added while we're running
         int loop = 100;
+
         while (!_sessions.isEmpty() && loop-- > 0)
         {
             for (Session session : _sessions.values())
             {
-                //if we have a backing store so give the session to it to write out if necessary
+                if (isInvalidateOnShutdown())
-                if (_sessionDataStore != null)
-                {
-                    session.willPassivate();
-                    try
-                    {
-                        _sessionDataStore.store(session.getId(), session.getSessionData());
-                    }
-                    catch (Exception e)
-                    {
-                        LOG.warn("Unable to store {}", session, e);
-                    }
-                    doDelete(session.getId()); //remove from memory
-                    session.setResident(false);
-                }
-                else
                 {
                     //not preserving sessions on exit
                     try
@@ -165,6 +154,22 @@ public class DefaultSessionCache extends AbstractSessionCache
                         LOG.trace("IGNORED", e);
                     }
                 }
+                else
+                {
+                    //write out the session and remove from the cache
+                    if (_sessionDataStore.isPassivating())
+                        session.willPassivate();
+                    try
+                    {
+                        _sessionDataStore.store(session.getId(), session.getSessionData());
+                    }
+                    catch (Exception e)
+                    {
+                        LOG.warn("Unable to store {}", session, e);
+                    }
+                    doDelete(session.getId()); //remove from memory
+                    session.setResident(false);
+                }
             }
         }
     }

jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionCacheFactory.java

@@ -26,14 +26,8 @@ package org.eclipse.jetty.server.session;
 public class DefaultSessionCacheFactory extends AbstractSessionCacheFactory
 {
     @Override
-    public SessionCache getSessionCache(SessionHandler handler)
+    public SessionCache newSessionCache(SessionHandler handler)
     {
-        DefaultSessionCache cache = new DefaultSessionCache(handler);
+        return new DefaultSessionCache(handler);
-        cache.setEvictionPolicy(getEvictionPolicy());
-        cache.setSaveOnInactiveEviction(isSaveOnInactiveEvict());
-        cache.setSaveOnCreate(isSaveOnCreate());
-        cache.setRemoveUnloadableSessions(isRemoveUnloadableSessions());
-        cache.setFlushOnResponseCommit(isFlushOnResponseCommit());
-        return cache;
     }
 }

jetty-server/src/main/java/org/eclipse/jetty/server/session/DefaultSessionIdManager.java

@@ -471,7 +471,9 @@ public class DefaultSessionIdManager extends ContainerLifeCycle implements Sessi
         {
             for (Handler h : tmp)
             {
-                if (h.isStarted())
+                //This method can be called on shutdown when the handlers are STOPPING, so only
+                //check that they are not already stopped
+                if (!h.isStopped() && !h.isFailed())
                     handlers.add((SessionHandler)h);
             }
         }

jetty-server/src/main/java/org/eclipse/jetty/server/session/NullSessionCacheFactory.java

@@ -55,14 +55,23 @@ public class NullSessionCacheFactory extends AbstractSessionCacheFactory
         if (LOG.isDebugEnabled())
             LOG.debug("Ignoring eviction policy setting for NullSessionCaches");
     }
+    
+    @Override
+    public boolean isInvalidateOnShutdown()
+    {
+        return false; //meaningless for NullSessionCache
+    }
 
     @Override
-    public SessionCache getSessionCache(SessionHandler handler)
+    public void setInvalidateOnShutdown(boolean invalidateOnShutdown)
     {
-        NullSessionCache cache = new NullSessionCache(handler);
+        if (LOG.isDebugEnabled())
-        cache.setSaveOnCreate(isSaveOnCreate());
+            LOG.debug("Ignoring invalidateOnShutdown setting for NullSessionCaches");
-        cache.setRemoveUnloadableSessions(isRemoveUnloadableSessions());
+    }
-        cache.setFlushOnResponseCommit(isFlushOnResponseCommit());
+
-        return cache;
+    @Override
+    public SessionCache newSessionCache(SessionHandler handler)
+    {
+        return new NullSessionCache(handler);
     }
 }

jetty-server/src/main/java/org/eclipse/jetty/server/session/SessionCache.java

@@ -290,4 +290,13 @@ public interface SessionCache extends LifeCycle
      * before the response is committed.
      */
     boolean isFlushOnResponseCommit();
+    
+    /**
+     * If true, all existing sessions in the cache will be invalidated when
+     * the server shuts down. Default is false.
+     * @param invalidateOnShutdown
+     */
+    void setInvalidateOnShutdown(boolean invalidateOnShutdown);
+    
+    boolean isInvalidateOnShutdown();
 }

jetty-server/src/test/java/org/eclipse/jetty/server/LargeHeaderTest.java

@@ -0,0 +1,142 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under
+// the terms of the Eclipse Public License 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0
+//
+// This Source Code may also be made available under the following
+// Secondary Licenses when the conditions for such availability set
+// forth in the Eclipse Public License, v. 2.0 are satisfied:
+// the Apache License v2.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.server;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.io.PrintWriter;
+import java.net.Socket;
+import java.util.Arrays;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.eclipse.jetty.http.HttpHeader;
+import org.eclipse.jetty.http.MimeTypes;
+import org.eclipse.jetty.http.tools.HttpTester;
+import org.eclipse.jetty.server.handler.AbstractHandler;
+import org.eclipse.jetty.server.handler.ErrorHandler;
+import org.eclipse.jetty.util.IO;
+import org.eclipse.jetty.util.component.LifeCycle;
+import org.eclipse.jetty.util.log.Log;
+import org.eclipse.jetty.util.log.Logger;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+public class LargeHeaderTest
+{
+    private Server server;
+
+    @BeforeEach
+    public void setup() throws Exception
+    {
+        server = new Server();
+
+        HttpConfiguration config = new HttpConfiguration();
+        HttpConnectionFactory http = new HttpConnectionFactory(config);
+
+        ServerConnector connector = new ServerConnector(server, http);
+        connector.setPort(0);
+        connector.setIdleTimeout(5000);
+        server.addConnector(connector);
+
+        server.setErrorHandler(new ErrorHandler());
+
+        server.setHandler(new AbstractHandler()
+        {
+            final String largeHeaderValue;
+
+            {
+                byte[] bytes = new byte[8 * 1024];
+                Arrays.fill(bytes, (byte)'X');
+                largeHeaderValue = "LargeHeaderOver8k-" + new String(bytes, UTF_8) + "_Z_";
+            }
+
+            @Override
+            public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException
+            {
+                response.setHeader(HttpHeader.CONTENT_TYPE.toString(), MimeTypes.Type.TEXT_HTML.toString());
+                response.setHeader("LongStr", largeHeaderValue);
+                PrintWriter writer = response.getWriter();
+                writer.write("<html><h1>FOO</h1></html>");
+                writer.flush();
+                response.flushBuffer();
+                baseRequest.setHandled(true);
+            }
+        });
+        server.start();
+    }
+
+    @AfterEach
+    public void teardown()
+    {
+        LifeCycle.stop(server);
+    }
+
+    @Test
+    public void testLargeHeader() throws Throwable
+    {
+        final Logger CLIENTLOG = Log.getLogger(LargeHeaderTest.class).getLogger(".client");
+        ExecutorService executorService = Executors.newFixedThreadPool(8);
+
+        int localPort = server.getURI().getPort();
+        String rawRequest = "GET / HTTP/1.1\r\n" +
+            "Host: localhost:" + localPort + "\r\n" +
+            "\r\n";
+
+        Throwable issues = new Throwable();
+
+        for (int i = 0; i < 500; ++i)
+        {
+            executorService.submit(() ->
+            {
+                try (Socket client = new Socket("localhost", localPort);
+                     OutputStream output = client.getOutputStream();
+                     InputStream input = client.getInputStream())
+                {
+                    output.write(rawRequest.getBytes(UTF_8));
+                    output.flush();
+
+                    String rawResponse = IO.toString(input, UTF_8);
+                    HttpTester.Response response = HttpTester.parseResponse(rawResponse);
+                    assertThat(response.getStatus(), is(500));
+                }
+                catch (Throwable t)
+                {
+                    CLIENTLOG.warn("Client Issue", t);
+                    issues.addSuppressed(t);
+                }
+            });
+        }
+
+        executorService.awaitTermination(5, TimeUnit.SECONDS);
+        if (issues.getSuppressed().length > 0)
+        {
+            throw issues;
+        }
+    }
+}

jetty-server/src/test/java/org/eclipse/jetty/server/ProxyCustomizerTest.java

@@ -128,7 +128,7 @@ public class ProxyCustomizerTest
     }
 
     @Test
-    void testProxyCustomizerWithProxyData() throws Exception
+    public void testProxyCustomizerWithProxyData() throws Exception
     {
         String proxy =
             // Preamble
@@ -159,7 +159,7 @@ public class ProxyCustomizerTest
     }
 
     @Test
-    void testProxyCustomizerWithoutProxyData() throws Exception
+    public void testProxyCustomizerWithoutProxyData() throws Exception
     {
         String proxy = "";
         String http = "GET /1 HTTP/1.1\r\n" +

jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java

@@ -43,7 +43,6 @@ import javax.servlet.MultipartConfigElement;
 import javax.servlet.ServletException;
 import javax.servlet.ServletInputStream;
 import javax.servlet.http.Cookie;
-import javax.servlet.http.HttpServletMapping;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
@@ -2175,29 +2174,4 @@ public class RequestTest
             return null;
         }
     }
-
-    private class PathMappingHandler extends AbstractHandler
-    {
-        private ServletPathSpec _spec;
-        private String _servletPath;
-        private String _servletName;
-
-        public PathMappingHandler(ServletPathSpec spec, String servletPath, String servletName)
-        {
-            _spec = spec;
-            _servletPath = servletPath;
-            _servletName = servletName;
-        }
-
-        @Override
-        public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
-        {
-            ((Request)request).setHandled(true);
-            baseRequest.setServletPath(_servletPath);
-            if (_servletName != null)
-                baseRequest.setUserIdentityScope(new TestUserIdentityScope(null, null, _servletName));
-            HttpServletMapping mapping = baseRequest.getHttpServletMapping();
-            response.getWriter().println(mapping);
-        }
-    }
 }

jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java

@@ -95,7 +95,6 @@ import static org.junit.jupiter.api.Assertions.assertTrue;
 // @checkstyle-disable-check : AvoidEscapedUnicodeCharactersCheck
 public class ResponseTest
 {
-
     static final InetSocketAddress LOCALADDRESS;
 
     static
@@ -353,7 +352,7 @@ public class ResponseTest
         ContextHandler context = new ContextHandler();
         context.addLocaleEncoding(Locale.ENGLISH.toString(), "ISO-8859-1");
         context.addLocaleEncoding(Locale.ITALIAN.toString(), "ISO-8859-2");
-        response.getHttpChannel().getRequest().setContext(context.getServletContext());
+        response.getHttpChannel().getRequest().setContext(context.getServletContext(), "/");
 
         response.setLocale(java.util.Locale.ITALIAN);
         assertNull(response.getContentType());
@@ -376,7 +375,7 @@ public class ResponseTest
         ContextHandler context = new ContextHandler();
         context.addLocaleEncoding(Locale.ENGLISH.toString(), "ISO-8859-1");
         context.addLocaleEncoding(Locale.ITALIAN.toString(), "ISO-8859-2");
-        response.getHttpChannel().getRequest().setContext(context.getServletContext());
+        response.getHttpChannel().getRequest().setContext(context.getServletContext(), "/");
 
         response.setLocale(java.util.Locale.ITALIAN);
 
@@ -425,46 +424,46 @@ public class ResponseTest
 
         //test setting the default response character encoding
         Response response = getResponse();
-        _channel.getRequest().setContext(handler.getServletContext());
+        response.getHttpChannel().getRequest().setContext(handler.getServletContext(), "/");
         assertThat("utf-16", Matchers.equalTo(response.getCharacterEncoding()));
 
-        _channel.getRequest().setContext(null);
+        _channel.getRequest().setContext(null, "/");
         response.recycle();
         
         //test that explicit overrides default
         response = getResponse();
-        _channel.getRequest().setContext(handler.getServletContext());
+        _channel.getRequest().setContext(handler.getServletContext(), "/");
         response.setCharacterEncoding("ascii");
         assertThat("ascii", Matchers.equalTo(response.getCharacterEncoding()));
         //getWriter should not change explicit character encoding
         response.getWriter();
         assertThat("ascii", Matchers.equalTo(response.getCharacterEncoding()));
         
-        _channel.getRequest().setContext(null);
+        _channel.getRequest().setContext(null, "/");
         response.recycle();
         
         //test that assumed overrides default
         response = getResponse();
-        _channel.getRequest().setContext(handler.getServletContext());
+        _channel.getRequest().setContext(handler.getServletContext(), "/");
         response.setContentType("application/json");
         assertThat("utf-8", Matchers.equalTo(response.getCharacterEncoding()));
         response.getWriter();
         //getWriter should not have modified character encoding
         assertThat("utf-8", Matchers.equalTo(response.getCharacterEncoding()));
         
-        _channel.getRequest().setContext(null);
+        _channel.getRequest().setContext(null, "/");
         response.recycle();
         
         //test that inferred overrides default
         response = getResponse();
-        _channel.getRequest().setContext(handler.getServletContext());
+        _channel.getRequest().setContext(handler.getServletContext(), "/");
         response.setContentType("application/xhtml+xml");
         assertThat("utf-8", Matchers.equalTo(response.getCharacterEncoding()));
         //getWriter should not have modified character encoding
         response.getWriter();
         assertThat("utf-8", Matchers.equalTo(response.getCharacterEncoding()));
         
-        _channel.getRequest().setContext(null);
+        _channel.getRequest().setContext(null, "/");
         response.recycle();
         
         //test that without a default or any content type, use iso-8859-1
@@ -488,7 +487,7 @@ public class ResponseTest
         _server.start();
 
         Response response = getResponse();
-        response.getHttpChannel().getRequest().setContext(handler.getServletContext());
+        response.getHttpChannel().getRequest().setContext(handler.getServletContext(), "/");
         
         response.setContentType("text/html");
         assertEquals("iso-8859-1", response.getCharacterEncoding());
@@ -859,10 +858,11 @@ public class ResponseTest
     @Test
     public void testEncodeRedirect()
     {
+        ContextHandler context = new ContextHandler("/path");
         Response response = getResponse();
         Request request = response.getHttpChannel().getRequest();
         request.setHttpURI(HttpURI.build(request.getHttpURI()).host("myhost").port(8888));
-        request.setContextPath("/path");
+        request.setContext(context.getServletContext(), "/info");
 
         assertEquals("http://myhost:8888/path/info;param?query=0&more=1#target", response.encodeURL("http://myhost:8888/path/info;param?query=0&more=1#target"));
 
@@ -893,7 +893,23 @@ public class ResponseTest
         assertEquals("http://myhost/path/info;param?query=0&more=1#target", response.encodeURL("http://myhost/path/info;param?query=0&more=1#target"));
         assertEquals("http://myhost:8888/other/info;param?query=0&more=1#target", response.encodeURL("http://myhost:8888/other/info;param?query=0&more=1#target"));
 
-        request.setContextPath("");
+        context = new ContextHandler("/");
+        request.setContext(context.getServletContext(), "/");
+        assertEquals("http://myhost:8888/;jsessionid=12345", response.encodeURL("http://myhost:8888"));
+        assertEquals("https://myhost:8888/;jsessionid=12345", response.encodeURL("https://myhost:8888"));
+        assertEquals("mailto:/foo", response.encodeURL("mailto:/foo"));
+        assertEquals("http://myhost:8888/;jsessionid=12345", response.encodeURL("http://myhost:8888/"));
+        assertEquals("http://myhost:8888/;jsessionid=12345", response.encodeURL("http://myhost:8888/;jsessionid=7777"));
+        assertEquals("http://myhost:8888/;param;jsessionid=12345?query=0&more=1#target", response.encodeURL("http://myhost:8888/;param?query=0&more=1#target"));
+        assertEquals("http://other:8888/path/info;param?query=0&more=1#target", response.encodeURL("http://other:8888/path/info;param?query=0&more=1#target"));
+        handler.setCheckingRemoteSessionIdEncoding(false);
+        assertEquals("/foo;jsessionid=12345", response.encodeURL("/foo"));
+        assertEquals("/;jsessionid=12345", response.encodeURL("/"));
+        assertEquals("/foo.html;jsessionid=12345#target", response.encodeURL("/foo.html#target"));
+        assertEquals(";jsessionid=12345", response.encodeURL(""));
+
+        request.setContext(null, "/");
+        handler.setCheckingRemoteSessionIdEncoding(true);
         assertEquals("http://myhost:8888/;jsessionid=12345", response.encodeURL("http://myhost:8888"));
         assertEquals("https://myhost:8888/;jsessionid=12345", response.encodeURL("https://myhost:8888"));
         assertEquals("mailto:/foo", response.encodeURL("mailto:/foo"));
@@ -937,6 +953,7 @@ public class ResponseTest
             {"http://somehost.com/other/location", "http://somehost.com/other/location"},
             };
 
+        ContextHandler context = new ContextHandler("/path");
         int[] ports = new int[]{8080, 80};
         String[] hosts = new String[]{null, "myhost", "192.168.0.1", "0::1"};
         for (int port : ports)
@@ -956,7 +973,7 @@ public class ResponseTest
                     if (host != null)
                         uri.host(host).port(port);
                     request.setHttpURI(uri);
-                    request.setContextPath("/path");
+                    request.setContext(context.getServletContext(), "/info");
                     request.setRequestedSessionId("12345");
                     request.setRequestedSessionIdFromCookie(i > 2);
                     SessionHandler handler = new SessionHandler();
@@ -980,6 +997,7 @@ public class ResponseTest
                         .replace("@HOST@", host == null ? request.getLocalAddr() : (host.contains(":") ? ("[" + host + "]") : host))
                         .replace("@PORT@", host == null ? ":8888" : (port == 80 ? "" : (":" + port)));
                     assertEquals(expected, location, "test-" + i + " " + host + ":" + port);
+                    request.setContext(null, "/info");
                 }
             }
         }
@@ -1094,12 +1112,29 @@ public class ResponseTest
         assertEquals("name=value; Path=/path; Domain=domain; Secure; HttpOnly", set);
     }
     
+    @Test
+    public void testAddCookieInInclude() throws Exception
+    {
+        Response response = getResponse();
+        response.include();
+
+        Cookie cookie = new Cookie("naughty", "value");
+        cookie.setDomain("domain");
+        cookie.setPath("/path");
+        cookie.setSecure(true);
+        cookie.setComment("comment__HTTP_ONLY__");
+
+        response.addCookie(cookie);
+
+        assertNull(response.getHttpFields().get("Set-Cookie"));
+    }
+    
     @Test
     public void testAddCookieSameSiteDefault() throws Exception
     {
         Response response = getResponse();
         TestServletContextHandler context = new TestServletContextHandler();
-        _channel.getRequest().setContext(context.getServletContext());
+        _channel.getRequest().setContext(context.getServletContext(), "/");
         context.setAttribute(HttpCookie.SAME_SITE_DEFAULT_ATTRIBUTE, HttpCookie.SameSite.STRICT);
         Cookie cookie = new Cookie("name", "value");
         cookie.setDomain("domain");
@@ -1265,7 +1300,7 @@ public class ResponseTest
         Response response = getResponse();
         TestServletContextHandler context = new TestServletContextHandler();
         context.setAttribute(HttpCookie.SAME_SITE_DEFAULT_ATTRIBUTE, "LAX");
-        _channel.getRequest().setContext(context.getServletContext());
+        _channel.getRequest().setContext(context.getServletContext(), "/");
         //replace with no prior does an add
         response.replaceCookie(new HttpCookie("Foo", "123456"));
         String set = response.getHttpFields().get("Set-Cookie");
@@ -1307,7 +1342,7 @@ public class ResponseTest
         Response response = getResponse();
         TestServletContextHandler context = new TestServletContextHandler();
         context.setAttribute(HttpCookie.SAME_SITE_DEFAULT_ATTRIBUTE, "LAX");
-        _channel.getRequest().setContext(context.getServletContext());
+        _channel.getRequest().setContext(context.getServletContext(), "/");
         
         response.addHeader(HttpHeader.SET_COOKIE.asString(), "Foo=123456");
         response.replaceCookie(new HttpCookie("Foo", "value"));

jetty-servlet/src/main/java/org/eclipse/jetty/servlet/ServletHandler.java

@@ -37,7 +37,6 @@ import java.util.stream.Stream;
 import javax.servlet.DispatcherType;
 import javax.servlet.Filter;
 import javax.servlet.FilterChain;
-import javax.servlet.RequestDispatcher;
 import javax.servlet.Servlet;
 import javax.servlet.ServletContext;
 import javax.servlet.ServletException;
@@ -66,7 +65,6 @@ import org.eclipse.jetty.util.ArrayUtil;
 import org.eclipse.jetty.util.LazyList;
 import org.eclipse.jetty.util.MultiException;
 import org.eclipse.jetty.util.MultiMap;
-import org.eclipse.jetty.util.URIUtil;
 import org.eclipse.jetty.util.annotation.ManagedAttribute;
 import org.eclipse.jetty.util.annotation.ManagedObject;
 import org.eclipse.jetty.util.component.DumpableCollection;
@@ -431,10 +429,7 @@ public class ServletHandler extends ScopedHandler
             if (servletPathMapping != null)
             {
                 // Setting the servletPathMapping also provides the servletPath and pathInfo
-                if (DispatcherType.INCLUDE.equals(type))
+                baseRequest.setServletPathMapping(servletPathMapping);
-                    baseRequest.setAttribute(RequestDispatcher.INCLUDE_MAPPING, servletPathMapping);
-                else
-                    baseRequest.setServletPathMapping(servletPathMapping);
             }
         }
 
@@ -1405,7 +1400,7 @@ public class ServletHandler extends ScopedHandler
         if (LOG.isDebugEnabled())
             LOG.debug("Not Found {}", request.getRequestURI());
         if (getHandler() != null)
-            nextHandle(URIUtil.addPaths(request.getServletPath(), request.getPathInfo()), baseRequest, request, response);
+            nextHandle(baseRequest.getPathInContext(), baseRequest, request, response);
     }
 
     protected synchronized boolean containsFilterHolder(FilterHolder holder)

jetty-unixsocket/jetty-unixsocket-server/src/main/config/modules/unixsocket.mod

@@ -57,4 +57,4 @@ http://www.apache.org/licenses/LICENSE-2.0.html
 # jetty.unixsocket.selectors=-1
 
 ## ServerSocketChannel backlog (0 picks platform default)
-# jetty.unixsocket.acceptorQueueSize=0
+# jetty.unixsocket.acceptQueueSize=0

jetty-unixsocket/src/main/config/modules/unixsocket.mod

@@ -0,0 +1,59 @@
+DO NOT EDIT - See: https://www.eclipse.org/jetty/documentation/current/startup-modules.html
+
+[description]
+Enables a Unix Domain Socket Connector that can receive
+requests from a local proxy and/or SSL offloader (eg haproxy) in either
+HTTP or TCP mode.  Unix Domain Sockets are more efficient than 
+localhost TCP/IP connections  as they reduce data copies, avoid 
+needless fragmentation and have better dispatch behaviours. 
+When enabled with corresponding support modules, the connector can 
+accept HTTP, HTTPS or HTTP2C traffic.
+
+[tags]
+connector
+
+[depend]
+server
+
+[xml]
+etc/jetty-unixsocket.xml
+
+[files]
+maven://com.github.jnr/jnr-unixsocket/0.22|lib/jnr/jnr-unixsocket-0.22.jar
+maven://com.github.jnr/jnr-ffi/2.1.9|lib/jnr/jnr-ffi-2.1.9.jar
+maven://com.github.jnr/jffi/1.2.17|lib/jnr/jffi-1.2.17.jar
+maven://com.github.jnr/jffi/1.2.16/jar/native|lib/jnr/jffi-1.2.16-native.jar
+maven://org.ow2.asm/asm/7.0|lib/jnr/asm-7.0.jar
+maven://org.ow2.asm/asm-commons/7.0|lib/jnr/asm-commons-7.0.jar
+maven://org.ow2.asm/asm-analysis/7.0|lib/jnr/asm-analysis-7.0.jar
+maven://org.ow2.asm/asm-tree/7.0|lib/jnr/asm-tree-7.0.jar
+maven://org.ow2.asm/asm-util/7.0|lib/jnr/asm-util-7.0.jar
+maven://com.github.jnr/jnr-x86asm/1.0.2|lib/jnr/jnr-x86asm-1.0.2.jar
+maven://com.github.jnr/jnr-constants/0.9.11|lib/jnr/jnr-constants-0.9.11.jar
+maven://com.github.jnr/jnr-enxio/0.20|lib/jnr/jnr-enxio-0.20.jar
+maven://com.github.jnr/jnr-posix/3.0.47|lib/jnr/jnr-posix-3.0.47.jar
+
+[lib]
+lib/jetty-unixsocket-${jetty.version}.jar
+lib/jnr/*.jar
+
+[license]
+Jetty UnixSockets is implemented using the Java Native Runtime, which is an
+open source project hosted on Github and released under the Apache 2.0 license.
+https://github.com/jnr/jnr-unixsocket
+http://www.apache.org/licenses/LICENSE-2.0.html
+
+[ini-template]
+### Unix SocketHTTP Connector Configuration
+
+## Unix socket path to bind to
+# jetty.unixsocket.path=/tmp/jetty.sock
+
+## Connector idle timeout in milliseconds
+# jetty.unixsocket.idleTimeout=30000
+
+## Number of selectors (-1 picks default)
+# jetty.unixsocket.selectors=-1
+
+## ServerSocketChannel backlog (0 picks platform default)
+# jetty.unixsocket.acceptQueueSize=0

jetty-util/src/main/java/org/eclipse/jetty/util/Attributes.java

@@ -43,6 +43,10 @@ public interface Attributes
 
     void clearAttributes();
 
+    /** Unwrap all  {@link Wrapper}s of the attributes
+     * @param attributes The attributes to unwrap, which may be a  {@link Wrapper}.
+     * @return The core attributes
+     */
     static Attributes unwrap(Attributes attributes)
     {
         while (attributes instanceof Wrapper)
@@ -52,6 +56,26 @@ public interface Attributes
         return attributes;
     }
 
+    /** Unwrap attributes to a specific attribute  {@link Wrapper}.
+     * @param attributes The attributes to unwrap, which may be a {@link Wrapper}
+     * @param target The target  {@link Wrapper} class.
+     * @param <T> The type of the target  {@link Wrapper}.
+     * @return The outermost {@link Wrapper} of the matching type of null if not found.
+     */
+    static <T extends Attributes.Wrapper> T unwrap(Attributes attributes, Class<T> target)
+    {
+        while (attributes instanceof Wrapper)
+        {
+            if (target.isAssignableFrom(attributes.getClass()))
+                return (T)attributes;
+            attributes = ((Wrapper)attributes).getAttributes();
+        }
+        return null;
+    }
+
+    /**
+     * A Wrapper of attributes
+     */
     abstract class Wrapper implements Attributes
     {
         protected final Attributes _attributes;

jetty-websocket/websocket-core-common/src/main/java/module-info.java

@@ -29,9 +29,9 @@ module org.eclipse.jetty.websocket.core.common
     exports org.eclipse.jetty.websocket.core.internal to org.eclipse.jetty.websocket.core.client, org.eclipse.jetty.websocket.core.server, org.eclipse.jetty.util;
 
     requires org.eclipse.jetty.http;
-    requires org.slf4j;
     requires transitive org.eclipse.jetty.io;
     requires transitive org.eclipse.jetty.util;
+    requires org.slf4j;
 
     uses Extension;
 

jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/config/ContainerDefaultConfigurator.java

@@ -68,11 +68,12 @@ public final class ContainerDefaultConfigurator extends Configurator
         {
             // Since this is started via a ServiceLoader, this class has no Scope or context
             // that can be used to obtain a ObjectFactory from.
-            return endpointClass.getDeclaredConstructor().newInstance();
+            return endpointClass.getConstructor().newInstance();
         }
         catch (Exception e)
         {
-            InstantiationException instantiationException = new InstantiationException();
+            String errorMsg = String.format("%s: %s", e.getClass().getName(), e.getMessage());
+            InstantiationException instantiationException = new InstantiationException(errorMsg);
             instantiationException.initCause(e);
             throw instantiationException;
         }

jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketCreator.java

@@ -24,6 +24,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import javax.websocket.Extension;
 import javax.websocket.Extension.Parameter;
 import javax.websocket.server.ServerEndpointConfig;
@@ -70,7 +71,7 @@ public class JavaxWebSocketCreator implements WebSocketCreator
         // per upgrade request.
         ServerEndpointConfig config = new ServerEndpointConfigWrapper(baseConfig)
         {
-            Map<String, Object> userProperties = new HashMap<>(baseConfig.getUserProperties());
+            final Map<String, Object> userProperties = new HashMap<>(baseConfig.getUserProperties());
 
             @Override
             public Map<String, Object> getUserProperties()
@@ -183,15 +184,13 @@ public class JavaxWebSocketCreator implements WebSocketCreator
             return false;
 
         JavaxWebSocketCreator that = (JavaxWebSocketCreator)o;
-
+        return Objects.equals(baseConfig, that.baseConfig);
-        return baseConfig != null ? baseConfig.equals(that.baseConfig) : that.baseConfig == null;
     }
 
     @Override
     public int hashCode()
     {
-        int result = (baseConfig != null ? baseConfig.hashCode() : 0);
+        return (baseConfig != null ? baseConfig.hashCode() : 0);
-        return result;
     }
 
     @Override

jetty-websocket/websocket-javax-server/src/main/java/org/eclipse/jetty/websocket/javax/server/internal/JavaxWebSocketServerContainer.java

@@ -18,6 +18,7 @@
 
 package org.eclipse.jetty.websocket.javax.server.internal;
 
+import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
@@ -38,10 +39,12 @@ import org.eclipse.jetty.util.component.Graceful;
 import org.eclipse.jetty.util.component.LifeCycle;
 import org.eclipse.jetty.websocket.core.WebSocketComponents;
 import org.eclipse.jetty.websocket.core.client.WebSocketCoreClient;
-import org.eclipse.jetty.websocket.core.exception.WebSocketException;
 import org.eclipse.jetty.websocket.core.server.WebSocketServerComponents;
 import org.eclipse.jetty.websocket.javax.client.internal.JavaxWebSocketClientContainer;
+import org.eclipse.jetty.websocket.javax.server.config.ContainerDefaultConfigurator;
 import org.eclipse.jetty.websocket.javax.server.config.JavaxWebSocketServletContainerInitializer;
+import org.eclipse.jetty.websocket.util.InvalidSignatureException;
+import org.eclipse.jetty.websocket.util.ReflectUtils;
 import org.eclipse.jetty.websocket.util.server.internal.WebSocketMapping;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -162,28 +165,63 @@ public class JavaxWebSocketServerContainer extends JavaxWebSocketClientContainer
         return frameHandlerFactory;
     }
 
+    private void validateEndpointConfig(ServerEndpointConfig config) throws DeploymentException
+    {
+        if (config == null)
+        {
+            throw new DeploymentException("Unable to deploy null ServerEndpointConfig");
+        }
+
+        ServerEndpointConfig.Configurator configurator = config.getConfigurator();
+        if (configurator == null)
+        {
+            throw new DeploymentException("Unable to deploy with null ServerEndpointConfig.Configurator");
+        }
+
+        Class<?> endpointClass = config.getEndpointClass();
+        if (endpointClass == null)
+        {
+            throw new DeploymentException("Unable to deploy null endpoint class from ServerEndpointConfig: " + config.getClass().getName());
+        }
+
+        if (!Modifier.isPublic(endpointClass.getModifiers()))
+        {
+            throw new DeploymentException("Class is not public: " + endpointClass.getName());
+        }
+
+        if (configurator.getClass() == ContainerDefaultConfigurator.class)
+        {
+            if (!ReflectUtils.isDefaultConstructable(endpointClass))
+            {
+                throw new DeploymentException("Cannot access default constructor for the class: " + endpointClass.getName());
+            }
+        }
+    }
+
+    @Override
     public void addEndpoint(Class<?> endpointClass) throws DeploymentException
     {
         if (endpointClass == null)
         {
-            throw new DeploymentException("EndpointClass is null");
+            throw new DeploymentException("Unable to deploy null endpoint class");
         }
 
         if (isStarted() || isStarting())
         {
-            try
+            ServerEndpoint anno = endpointClass.getAnnotation(ServerEndpoint.class);
+            if (anno == null)
             {
-                ServerEndpoint anno = endpointClass.getAnnotation(ServerEndpoint.class);
+                throw new DeploymentException(String.format("Class must be @%s annotated: %s", ServerEndpoint.class.getName(), endpointClass.getName()));
-                if (anno == null)
+            }
-                    throw new DeploymentException(String.format("Class must be @%s annotated: %s", ServerEndpoint.class.getName(), endpointClass.getName()));
 
-                ServerEndpointConfig config = new AnnotatedServerEndpointConfig(this, endpointClass, anno);
+            if (LOG.isDebugEnabled())
-                addEndpointMapping(config);
-            }
-            catch (WebSocketException e)
             {
-                throw new DeploymentException("Unable to deploy: " + endpointClass.getName(), e);
+                LOG.debug("addEndpoint({})", endpointClass);
             }
+
+            ServerEndpointConfig config = new AnnotatedServerEndpointConfig(this, endpointClass, anno);
+            validateEndpointConfig(config);
+            addEndpointMapping(config);
         }
         else
         {
@@ -201,23 +239,17 @@ public class JavaxWebSocketServerContainer extends JavaxWebSocketClientContainer
 
         if (isStarted() || isStarting())
         {
+            // If we have annotations merge the annotated ServerEndpointConfig with the provided one.
             Class<?> endpointClass = providedConfig.getEndpointClass();
-            try
+            ServerEndpoint anno = endpointClass.getAnnotation(ServerEndpoint.class);
-            {
+            ServerEndpointConfig config = (anno == null) ? providedConfig
-                // If we have annotations merge the annotated ServerEndpointConfig with the provided one.
+                : new AnnotatedServerEndpointConfig(this, endpointClass, anno, providedConfig);
-                ServerEndpoint anno = endpointClass.getAnnotation(ServerEndpoint.class);
-                ServerEndpointConfig config = (anno == null) ? providedConfig
-                    : new AnnotatedServerEndpointConfig(this, endpointClass, anno, providedConfig);
 
-                if (LOG.isDebugEnabled())
+            if (LOG.isDebugEnabled())
-                    LOG.debug("addEndpoint({}) path={} endpoint={}", config, config.getPath(), endpointClass);
+                LOG.debug("addEndpoint({}) path={} endpoint={}", config, config.getPath(), endpointClass);
 
-                addEndpointMapping(config);
+            validateEndpointConfig(config);
-            }
+            addEndpointMapping(config);
-            catch (WebSocketException e)
-            {
-                throw new DeploymentException("Unable to deploy: " + endpointClass.getName(), e);
-            }
         }
         else
         {
@@ -227,14 +259,23 @@ public class JavaxWebSocketServerContainer extends JavaxWebSocketClientContainer
         }
     }
 
-    private void addEndpointMapping(ServerEndpointConfig config) throws WebSocketException
+    private void addEndpointMapping(ServerEndpointConfig config) throws DeploymentException
     {
-        frameHandlerFactory.getMetadata(config.getEndpointClass(), config);
+        try
-
+        {
-        JavaxWebSocketCreator creator = new JavaxWebSocketCreator(this, config, getExtensionRegistry());
+            frameHandlerFactory.getMetadata(config.getEndpointClass(), config);
-
+            JavaxWebSocketCreator creator = new JavaxWebSocketCreator(this, config, getExtensionRegistry());
-        PathSpec pathSpec = new UriTemplatePathSpec(config.getPath());
+            PathSpec pathSpec = new UriTemplatePathSpec(config.getPath());
-        webSocketMapping.addMapping(pathSpec, creator, frameHandlerFactory, defaultCustomizer);
+            webSocketMapping.addMapping(pathSpec, creator, frameHandlerFactory, defaultCustomizer);
+        }
+        catch (InvalidSignatureException e)
+        {
+            throw new DeploymentException(e.getMessage(), e);
+        }
+        catch (Throwable t)
+        {
+            throw new DeploymentException("Unable to deploy: " + config.getEndpointClass().getName(), t);
+        }
     }
 
     @Override

jetty-websocket/websocket-javax-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/server/AddEndpointTest.java

@@ -0,0 +1,317 @@
+//
+// ========================================================================
+// Copyright (c) 1995-2020 Mort Bay Consulting Pty Ltd and others.
+//
+// This program and the accompanying materials are made available under
+// the terms of the Eclipse Public License 2.0 which is available at
+// https://www.eclipse.org/legal/epl-2.0
+//
+// This Source Code may also be made available under the following
+// Secondary Licenses when the conditions for such availability set
+// forth in the Eclipse Public License, v. 2.0 are satisfied:
+// the Apache License v2.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.websocket.javax.tests.server;
+
+import java.util.concurrent.TimeUnit;
+import javax.websocket.CloseReason;
+import javax.websocket.ContainerProvider;
+import javax.websocket.DeploymentException;
+import javax.websocket.Endpoint;
+import javax.websocket.EndpointConfig;
+import javax.websocket.MessageHandler;
+import javax.websocket.OnMessage;
+import javax.websocket.OnOpen;
+import javax.websocket.Session;
+import javax.websocket.WebSocketContainer;
+import javax.websocket.server.ServerContainer;
+import javax.websocket.server.ServerEndpoint;
+import javax.websocket.server.ServerEndpointConfig;
+
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.util.component.LifeCycle;
+import org.eclipse.jetty.websocket.javax.server.config.JavaxWebSocketServletContainerInitializer;
+import org.eclipse.jetty.websocket.javax.tests.WSURI;
+import org.eclipse.jetty.websocket.javax.tests.client.samples.CloseSocket;
+import org.eclipse.jetty.websocket.javax.tests.server.sockets.BasicOpenCloseSocket;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.anyOf;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class AddEndpointTest
+{
+    private Server server;
+    private WebSocketContainer client;
+    private ServletContextHandler contextHandler;
+
+    @BeforeEach
+    public void before()
+    {
+        server = new Server();
+        ServerConnector connector = new ServerConnector(server);
+        server.addConnector(connector);
+
+        contextHandler = new ServletContextHandler();
+        contextHandler.setContextPath("/");
+        server.setHandler(contextHandler);
+        client = ContainerProvider.getWebSocketContainer();
+    }
+
+    @AfterEach
+    public void after() throws Exception
+    {
+        LifeCycle.stop(client);
+        server.stop();
+    }
+
+    public interface CheckedConsumer<T>
+    {
+        void accept(T t) throws DeploymentException;
+    }
+
+    public void start(CheckedConsumer<ServerContainer> containerConsumer) throws Exception
+    {
+        JavaxWebSocketServletContainerInitializer.configure(contextHandler, (context, container) -> containerConsumer.accept(container));
+        server.start();
+    }
+
+    private static class ServerSocket extends Endpoint implements MessageHandler.Whole<String>
+    {
+        @Override
+        public void onOpen(Session session, EndpointConfig config)
+        {
+            session.addMessageHandler(this);
+        }
+
+        @Override
+        public void onMessage(String message)
+        {
+        }
+    }
+
+    @SuppressWarnings("InnerClassMayBeStatic")
+    private class CustomPrivateEndpoint extends Endpoint
+    {
+        @Override
+        public void onOpen(Session session, EndpointConfig config)
+        {
+        }
+    }
+
+    @SuppressWarnings("InnerClassMayBeStatic")
+    @ServerEndpoint(value = "/", configurator = CustomAnnotatedEndpointConfigurator.class)
+    public static class CustomAnnotatedEndpoint
+    {
+        public CustomAnnotatedEndpoint(String id)
+        {
+        }
+
+        @OnOpen
+        public void onOpen(Session session, EndpointConfig config)
+        {
+        }
+    }
+
+    public static class CustomAnnotatedEndpointConfigurator extends ServerEndpointConfig.Configurator
+    {
+        @SuppressWarnings("unchecked")
+        @Override
+        public <T> T getEndpointInstance(Class<T> endpointClass)
+        {
+            return (T)new CustomAnnotatedEndpoint("server");
+        }
+    }
+
+    public static class CustomEndpoint extends Endpoint implements MessageHandler.Whole<String>
+    {
+        public CustomEndpoint(String id)
+        {
+            // This is a valid no-default-constructor implementation, and can be added via a custom
+            // ServerEndpointConfig.Configurator
+        }
+
+        @Override
+        public void onOpen(Session session, EndpointConfig config)
+        {
+            session.addMessageHandler(this);
+        }
+
+        @Override
+        public void onMessage(String message)
+        {
+        }
+    }
+
+    @SuppressWarnings("InnerClassMayBeStatic")
+    public class ServerSocketNonStatic extends Endpoint implements MessageHandler.Whole<String>
+    {
+        @Override
+        public void onOpen(Session session, EndpointConfig config)
+        {
+            session.addMessageHandler(this);
+        }
+
+        @Override
+        public void onMessage(String message)
+        {
+        }
+    }
+
+    @ServerEndpoint("/annotated")
+    private static class AnnotatedServerSocket
+    {
+        @OnMessage
+        public void onMessage(String message)
+        {
+        }
+    }
+
+    @ServerEndpoint("/annotatedMethod")
+    public static class AnnotatedServerMethod
+    {
+        @OnMessage
+        private void onMessage(String message)
+        {
+        }
+    }
+
+    @Test
+    public void testEndpoint()
+    {
+        RuntimeException error = assertThrows(RuntimeException.class, () ->
+        {
+            ServerEndpointConfig config = ServerEndpointConfig.Builder.create(ServerSocket.class, "/").build();
+            start(container -> container.addEndpoint(config));
+        });
+
+        assertThat(error.getCause(), instanceOf(DeploymentException.class));
+        DeploymentException deploymentException = (DeploymentException)error.getCause();
+        assertThat(deploymentException.getMessage(), containsString("Class is not public"));
+    }
+
+    @Test
+    public void testCustomEndpoint() throws Exception
+    {
+        ServerEndpointConfig config = ServerEndpointConfig.Builder.create(CustomEndpoint.class, "/")
+            .configurator(new ServerEndpointConfig.Configurator()
+            {
+                @SuppressWarnings("unchecked")
+                @Override
+                public <T> T getEndpointInstance(Class<T> endpointClass)
+                {
+                    return (T)new CustomEndpoint("server");
+                }
+            }).build();
+        start(container -> container.addEndpoint(config));
+
+        CloseSocket clientEndpoint = new CloseSocket();
+        Session session = client.connectToServer(clientEndpoint, WSURI.toWebsocket(server.getURI().resolve("/")));
+        assertNotNull(session);
+        session.close();
+        assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS));
+        CloseReason closeReason = clientEndpoint.closeDetail.get();
+        assertThat(closeReason, anyOf(nullValue(), is(CloseReason.CloseCodes.NORMAL_CLOSURE)));
+    }
+
+    @Test
+    public void testCustomPrivateEndpoint()
+    {
+        RuntimeException error = assertThrows(RuntimeException.class, () ->
+        {
+            ServerEndpointConfig config = ServerEndpointConfig.Builder.create(CustomPrivateEndpoint.class, "/")
+                .configurator(new ServerEndpointConfig.Configurator()
+                {
+                    @SuppressWarnings("unchecked")
+                    @Override
+                    public <T> T getEndpointInstance(Class<T> endpointClass)
+                    {
+                        return (T)new CustomPrivateEndpoint();
+                    }
+                }).build();
+            start(container -> container.addEndpoint(config));
+        });
+
+        assertThat(error.getCause(), instanceOf(DeploymentException.class));
+        DeploymentException deploymentException = (DeploymentException)error.getCause();
+        assertThat(deploymentException.getMessage(), containsString("Class is not public"));
+    }
+
+    @Test
+    public void testCustomAnnotatedEndpoint() throws Exception
+    {
+        start(container -> container.addEndpoint(CustomAnnotatedEndpoint.class));
+
+        CloseSocket clientEndpoint = new CloseSocket();
+        Session session = client.connectToServer(clientEndpoint, WSURI.toWebsocket(server.getURI().resolve("/")));
+        assertNotNull(session);
+        session.close();
+        assertTrue(clientEndpoint.closeLatch.await(5, TimeUnit.SECONDS));
+        CloseReason closeReason = clientEndpoint.closeDetail.get();
+        assertThat(closeReason, anyOf(nullValue(), is(CloseReason.CloseCodes.NORMAL_CLOSURE)));
+    }
+
+    @Test
+    public void testCustomEndpointNoConfigurator()
+    {
+        RuntimeException error = assertThrows(RuntimeException.class, () ->
+        {
+            ServerEndpointConfig config = ServerEndpointConfig.Builder.create(CustomEndpoint.class, "/").build();
+            start(container -> container.addEndpoint(config));
+        });
+
+        assertThat(error.getCause(), instanceOf(DeploymentException.class));
+        DeploymentException deploymentException = (DeploymentException)error.getCause();
+        assertThat(deploymentException.getMessage(), containsString("Cannot access default constructor"));
+    }
+
+    @Test
+    public void testInnerEndpoint()
+    {
+        RuntimeException error = assertThrows(RuntimeException.class, () ->
+            start(container -> container.addEndpoint(ServerEndpointConfig.Builder.create(ServerSocketNonStatic.class, "/").build())));
+
+        assertThat(error.getCause(), instanceOf(DeploymentException.class));
+        DeploymentException deploymentException = (DeploymentException)error.getCause();
+        assertThat(deploymentException.getMessage(), containsString("Cannot access default constructor"));
+    }
+
+    @Test
+    public void testAnnotatedEndpoint()
+    {
+        RuntimeException error = assertThrows(RuntimeException.class, () ->
+            start(container -> container.addEndpoint(AnnotatedServerSocket.class)));
+
+        assertThat(error.getCause(), instanceOf(DeploymentException.class));
+        DeploymentException deploymentException = (DeploymentException)error.getCause();
+        assertThat(deploymentException.getMessage(), containsString("Class is not public"));
+    }
+
+    @Test
+    public void testAnnotatedMethod()
+    {
+        RuntimeException error = assertThrows(RuntimeException.class, () ->
+            start(container ->
+                container.addEndpoint(AnnotatedServerMethod.class)));
+
+        assertThat(error.getCause(), instanceOf(DeploymentException.class));
+        DeploymentException deploymentException = (DeploymentException)error.getCause();
+        assertThat(deploymentException.getMessage(), containsString("method must be public"));
+    }
+}

jetty-websocket/websocket-util/src/main/java/module-info.java

@@ -21,8 +21,6 @@ module org.eclipse.jetty.websocket.util
     exports org.eclipse.jetty.websocket.util;
     exports org.eclipse.jetty.websocket.util.messages;
 
-    requires org.slf4j;
-    requires transitive org.eclipse.jetty.util;
-    requires transitive org.eclipse.jetty.io;
     requires transitive org.eclipse.jetty.websocket.core.common;
+    requires org.slf4j;
 }

pom.xml

@@ -1287,7 +1287,7 @@
               <execution>
                 <id>attach-sources</id>
                 <goals>
-                  <goal>jar</goal>
+                  <goal>jar-no-fork</goal>
                 </goals>
               </execution>
             </executions>

scripts/release-jetty.sh

@@ -167,7 +167,7 @@ if proceedyn "Are you sure you want to release using above? (y/N)" n; then
     # This is equivalent to 'mvn release:perform'
     if proceedyn "Build/Deploy from tag $TAG_NAME? (Y/n)" y; then
         git checkout $TAG_NAME
-        mvn clean package source:jar javadoc:jar gpg:sign javadoc:aggregate-jar deploy \
+        mvn clean package javadoc:aggregate-jar deploy \
             -Peclipse-release $DEPLOY_OPTS
         reportMavenTestFailures
         git checkout $GIT_BRANCH_ID

tests/test-sessions/test-file-sessions/src/test/java/org/eclipse/jetty/server/session/TestFileSessions.java

@@ -20,6 +20,7 @@ package org.eclipse.jetty.server.session;
 
 import java.util.concurrent.TimeUnit;
 
+import org.eclipse.jetty.logging.StacklessLogging;
 import org.eclipse.jetty.servlet.ServletContextHandler;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -255,8 +256,12 @@ public class TestFileSessions extends AbstractTestBase
         FileTestHelper.createFile(foreignNeverExpired);
         FileTestHelper.assertFileExists(foreignNeverExpired, true);
 
-        //sweep
+        //sweep - we're expecting a debug log with exception stacktrace due to file named 
-        ((FileSessionDataStore)store).sweepDisk();
+        //nonNumber__0.0.0.0_spuriousFile so suppress it
+        try (StacklessLogging ignored = new StacklessLogging(TestFileSessions.class.getPackage()))
+        {
+            ((FileSessionDataStore)store).sweepDisk();
+        }
 
         //check results
         FileTestHelper.assertSessionExists("sessiona", false);

tests/test-sessions/test-jdbc-sessions/pom.xml

@@ -63,11 +63,6 @@
       <artifactId>slf4j-api</artifactId>
       <scope>test</scope>
     </dependency>
-    <dependency>
-      <groupId>org.eclipse.jetty.toolchain</groupId>
-      <artifactId>jetty-test-helper</artifactId>
-      <scope>test</scope>
-    </dependency>
     <dependency>
       <groupId>org.testcontainers</groupId>
       <artifactId>testcontainers</artifactId>

tests/test-sessions/test-memcached-sessions/pom.xml

@@ -72,10 +72,20 @@
       <artifactId>jetty-slf4j-impl</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.testcontainers</groupId>
+      <artifactId>testcontainers</artifactId>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.testcontainers</groupId>
+      <artifactId>junit-jupiter</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
   <profiles>
     <profile>
-      <id>memcached</id>
+      <id>remote-session-tests</id>
       <activation>
         <property>
           <name>memcached.enabled</name>