Merge remote-tracking branch 'origin/jetty-10.0.x' into jetty-10.0.x-3012-Compliance

jetty-documentation/src/main/asciidoc/configuring/jsp/configuring-jsp.adoc

@@ -171,7 +171,7 @@ The JSP engine has many configuration parameters.
 Some parameters affect only precompilation, and some affect runtime recompilation checking.
 Parameters also differ among the various versions of the JSP engine.
 This page lists the configuration parameters, their meanings, and their default settings.
-Set all parameters on the `org.apache.jasper.JspServlet` instance defined in the link:#webdefault-xml[`webdefault.xml`] file.
+Set all parameters on the `org.apache.jasper.servlet.JspServlet` instance defined in the link:#webdefault-xml[`webdefault.xml`] file.
 
 ____
 [NOTE]

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

@@ -111,7 +111,7 @@ public abstract class LoginAuthenticator implements Authenticator
                         s.renewId(request);
                         s.setAttribute(Session.SESSION_CREATED_SECURE, Boolean.TRUE);
                         if (s.isIdChanged() && (response instanceof Response))
-                            ((Response)response).addCookie(s.getSessionHandler().getSessionCookie(s, request.getContextPath(), request.isSecure()));
+                            ((Response)response).replaceCookie(s.getSessionHandler().getSessionCookie(s, request.getContextPath(), request.isSecure()));
                         if (LOG.isDebugEnabled())
                             LOG.debug("renew {}->{}", oldId, s.getId());
                     }

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

@@ -1487,7 +1487,7 @@ public class Request implements HttpServletRequest
             if (getRemoteUser() != null)
                 s.setAttribute(Session.SESSION_CREATED_SECURE, Boolean.TRUE);
             if (s.isIdChanged() && _sessionHandler.isUsingCookies())
-                _channel.getResponse().addCookie(_sessionHandler.getSessionCookie(s, getContextPath(), isSecure()));
+                _channel.getResponse().replaceCookie(_sessionHandler.getSessionCookie(s, getContextPath(), isSecure()));
         }
 
         return session.getId();
@@ -1530,7 +1530,7 @@ public class Request implements HttpServletRequest
         _session = _sessionHandler.newHttpSession(this);
         HttpCookie cookie = _sessionHandler.getSessionCookie(_session,getContextPath(),isSecure());
         if (cookie != null)
-            _channel.getResponse().addCookie(cookie);
+            _channel.getResponse().replaceCookie(cookie);
 
         return _session;
     }

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

@@ -24,7 +24,9 @@ import java.nio.channels.IllegalSelectorException;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
+import java.util.Iterator;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Locale;
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -194,6 +196,94 @@ public class Response implements HttpServletResponse
                 cookie.isHttpOnly());
     }
 
+    /**
+     * Replace (or add) a cookie.
+     * Using name, path and domain, look for a matching set-cookie header and replace it.
+     * @param cookie The cookie to add/replace
+     */
+    public void replaceCookie(HttpCookie cookie)
+    {
+        for (ListIterator<HttpField> i = _fields.listIterator(); i.hasNext();)
+        {
+            HttpField field = i.next();
+
+            if (field.getHeader() == HttpHeader.SET_COOKIE)
+            {
+                String old_set_cookie = field.getValue();
+                String name = cookie.getName();
+                if (!old_set_cookie.startsWith(name) || old_set_cookie.length()<= name.length() || old_set_cookie.charAt(name.length())!='=')
+                    continue;
+
+                String domain = cookie.getDomain();
+                if (domain!=null)
+                {
+                    if (getHttpChannel().getHttpConfiguration().getResponseCookieCompliance()==CookieCompliance.RFC2965)
+                    {
+                        StringBuilder buf = new StringBuilder();
+                        buf.append(";Domain=");
+                        quoteOnlyOrAppend(buf,domain,isQuoteNeededForCookie(domain));
+                        domain = buf.toString();
+                    }
+                    else
+                    {
+                        domain = ";Domain="+domain;
+                    }
+                    if (!old_set_cookie.contains(domain))
+                        continue;
+                }
+                else if (old_set_cookie.contains(";Domain="))
+                    continue;
+
+                String path = cookie.getPath();
+                if (path!=null)
+                {
+                    if (getHttpChannel().getHttpConfiguration().getResponseCookieCompliance()==CookieCompliance.RFC2965)
+                    {
+                        StringBuilder buf = new StringBuilder();
+                        buf.append(";Path=");
+                        quoteOnlyOrAppend(buf,path,isQuoteNeededForCookie(path));
+                        path = buf.toString();
+                    }
+                    else
+                    {
+                        path = ";Path="+path;
+                    }
+                    if (!old_set_cookie.contains(path))
+                        continue;
+                }
+                else if (old_set_cookie.contains(";Path="))
+                    continue;
+
+                if (getHttpChannel().getHttpConfiguration().getResponseCookieCompliance() == CookieCompliance.RFC2965)
+                    i.set(new HttpField(HttpHeader.CONTENT_ENCODING.SET_COOKIE, newRFC2965SetCookie(
+                            cookie.getName(),
+                            cookie.getValue(),
+                            cookie.getDomain(),
+                            cookie.getPath(),
+                            cookie.getMaxAge(),
+                            cookie.getComment(),
+                            cookie.isSecure(),
+                            cookie.isHttpOnly(),
+                            cookie.getVersion())
+                    ));
+                else
+                    i.set(new HttpField(HttpHeader.CONTENT_ENCODING.SET_COOKIE, newRFC6265SetCookie(
+                            cookie.getName(),
+                            cookie.getValue(),
+                            cookie.getDomain(),
+                            cookie.getPath(),
+                            cookie.getMaxAge(),
+                            cookie.isSecure(),
+                            cookie.isHttpOnly()
+                    )));
+                return;
+            }
+        }
+
+        // Not replaced, so add normally
+        addCookie(cookie);
+    }
+
     @Override
     public void addCookie(Cookie cookie)
     {
@@ -257,6 +347,18 @@ public class Response implements HttpServletResponse
             final long maxAge,
             final boolean isSecure,
             final boolean isHttpOnly)
+    {
+        String set_cookie = newRFC6265SetCookie(name, value, domain, path, maxAge, isSecure, isHttpOnly);
+
+        // add the set cookie
+        _fields.add(HttpHeader.SET_COOKIE, set_cookie);
+
+        // Expire responses with set-cookie headers so they do not get cached.
+        _fields.put(__EXPIRES_01JAN1970);
+        
+    }
+
+    private String newRFC6265SetCookie(String name, String value, String domain, String path, long maxAge, boolean isSecure, boolean isHttpOnly)
     {
         // Check arguments
         if (name == null || name.length() == 0)
@@ -272,11 +374,11 @@ public class Response implements HttpServletResponse
         StringBuilder buf = __cookieBuilder.get();
         buf.setLength(0);
         buf.append(name).append('=').append(value==null?"":value);
-        
+
         // Append path
         if (path!=null && path.length()>0)
             buf.append(";Path=").append(path);
-        
+
         // Append domain
         if (domain!=null && domain.length()>0)
             buf.append(";Domain=").append(domain);
@@ -301,15 +403,9 @@ public class Response implements HttpServletResponse
             buf.append(";Secure");
         if (isHttpOnly)
             buf.append(";HttpOnly");
-        
+        return buf.toString();
-        // add the set cookie
-        _fields.add(HttpHeader.SET_COOKIE, buf.toString());
-
-        // Expire responses with set-cookie headers so they do not get cached.
-        _fields.put(__EXPIRES_01JAN1970);
-        
     }
-    
+
     /**
      * Format a set cookie value
      *
@@ -333,6 +429,17 @@ public class Response implements HttpServletResponse
             final boolean isSecure,
             final boolean isHttpOnly,
             int version)
+    {
+        String set_cookie = newRFC2965SetCookie(name, value, domain, path, maxAge, comment, isSecure, isHttpOnly, version);
+
+        // add the set cookie
+        _fields.add(HttpHeader.SET_COOKIE, set_cookie);
+
+        // Expire responses with set-cookie headers so they do not get cached.
+        _fields.put(__EXPIRES_01JAN1970);
+    }
+
+    private String newRFC2965SetCookie(String name, String value, String domain, String path, long maxAge, String comment, boolean isSecure, boolean isHttpOnly, int version)
     {
         // Check arguments
         if (name == null || name.length() == 0)
@@ -347,7 +454,7 @@ public class Response implements HttpServletResponse
         quoteOnlyOrAppend(buf,name,quote_name);
 
         buf.append('=');
-       
+
         // Append the value
         boolean quote_value=isQuoteNeededForCookie(value);
         quoteOnlyOrAppend(buf,value,quote_value);
@@ -413,12 +520,7 @@ public class Response implements HttpServletResponse
             buf.append(";Comment=");
             quoteOnlyOrAppend(buf,comment,isQuoteNeededForCookie(comment));
         }
-        
+        return buf.toString();
-        // add the set cookie
-        _fields.add(HttpHeader.SET_COOKIE, buf.toString());
-
-        // Expire responses with set-cookie headers so they do not get cached.
-        _fields.put(__EXPIRES_01JAN1970);
     }
 
 

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

@@ -1641,7 +1641,7 @@ public class SessionHandler extends ScopedHandler
                 HttpCookie cookie = access(existingSession,request.isSecure());
                 // Handle changed ID or max-age refresh, but only if this is not a redispatched request
                 if ((cookie != null) && (request.getDispatcherType() == DispatcherType.ASYNC || request.getDispatcherType() == DispatcherType.REQUEST))
-                    baseRequest.getResponse().addCookie(cookie);
+                    baseRequest.getResponse().replaceCookie(cookie);
             }
 
             if (LOG.isDebugEnabled())

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

@@ -18,11 +18,11 @@
 
 package org.eclipse.jetty.server;
 
+import static org.hamcrest.Matchers.contains;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.LineNumberReader;
 import java.io.PrintWriter;
-import java.net.HttpCookie;
 import java.net.Inet4Address;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
@@ -44,6 +44,7 @@ import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 
 import org.eclipse.jetty.http.CookieCompliance;
+import org.eclipse.jetty.http.HttpCookie;
 import org.eclipse.jetty.http.HttpField;
 import org.eclipse.jetty.http.HttpFields;
 import org.eclipse.jetty.http.HttpHeader;
@@ -1025,7 +1026,7 @@ public class ResponseTest
     @Test
     public void testAddCookie_JavaNet() throws Exception
     {
-        HttpCookie cookie = new HttpCookie("foo", URLEncoder.encode("bar;baz", UTF_8.toString()));
+        java.net.HttpCookie cookie = new java.net.HttpCookie("foo", URLEncoder.encode("bar;baz", UTF_8.toString()));
         cookie.setPath("/secure");
         
         assertEquals("foo=\"bar%3Bbaz\";$Path=\"/secure\"", cookie.toString());
@@ -1067,6 +1068,38 @@ public class ResponseTest
         assertFalse(set.hasMoreElements());
     }
 
+    @Test
+    public void testReplaceHttpCookie()
+    {
+        Response response = getResponse();
+
+        response.replaceCookie(new HttpCookie("Foo","123456"));
+        response.replaceCookie(new HttpCookie("Foo","123456", "A", "/path"));
+        response.replaceCookie(new HttpCookie("Foo","123456", "B", "/path"));
+
+        response.replaceCookie(new HttpCookie("Bar","123456"));
+        response.replaceCookie(new HttpCookie("Bar","123456",null, "/left"));
+        response.replaceCookie(new HttpCookie("Bar","123456", null, "/right"));
+
+        response.replaceCookie(new HttpCookie("Bar","value", null, "/right"));
+        response.replaceCookie(new HttpCookie("Bar","value",null, "/left"));
+        response.replaceCookie(new HttpCookie("Bar","value"));
+
+        response.replaceCookie(new HttpCookie("Foo","value", "B", "/path"));
+        response.replaceCookie(new HttpCookie("Foo","value", "A", "/path"));
+        response.replaceCookie(new HttpCookie("Foo","value"));
+
+        assertThat(Collections.list(response.getHttpFields().getValues("Set-Cookie")),
+                contains(
+                        "Foo=value",
+                        "Foo=value;Path=/path;Domain=A",
+                        "Foo=value;Path=/path;Domain=B",
+                        "Bar=value",
+                        "Bar=value;Path=/left",
+                        "Bar=value;Path=/right"
+                ));
+    }
+
     @Test
     public void testFlushAfterFullContent() throws Exception
     {

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

@@ -210,6 +210,31 @@ public interface Callback extends Invocable
         };
     }
 
+    /**
+     * Create a nested callback which always fails the nested callback on completion.
+     * @param callback The nested callback
+     * @param cause The cause to fail the nested callback, if the new callback is failed the reason
+     *             will be added to this cause as a suppressed exception.
+     * @return a new callback.
+     */
+    static Callback from(Callback callback, Throwable cause)
+    {
+        return new Callback()
+        {
+            @Override
+            public void succeeded()
+            {
+                callback.failed(cause);
+            }
+
+            @Override
+            public void failed(Throwable x)
+            {
+                cause.addSuppressed(x);
+                callback.failed(cause);
+            }
+        };
+    }
 
     class Completing implements Callback
     {

jetty-websocket/jetty-websocket-client/src/main/java/org/eclipse/jetty/websocket/client/WebSocketClient.java

@@ -135,7 +135,8 @@ public class WebSocketClient extends ContainerLifeCycle implements WebSocketPoli
     public CompletableFuture<Session> connect(Object websocket, URI toUri, UpgradeRequest request, UpgradeListener listener) throws IOException
     {
         JettyClientUpgradeRequest upgradeRequest = new JettyClientUpgradeRequest(this, coreClient, request, toUri, websocket);
-        upgradeRequest.addListener(listener);
+        if (listener != null)
+            upgradeRequest.addListener(listener);
         coreClient.connect(upgradeRequest);
         return upgradeRequest.getFutureSession();
     }

jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketStatsTest.java

@@ -0,0 +1,219 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.websocket.tests;
+
+import java.net.URI;
+import java.nio.ByteBuffer;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import org.eclipse.jetty.io.ByteBufferPool;
+import org.eclipse.jetty.io.Connection;
+import org.eclipse.jetty.io.ConnectionStatistics;
+import org.eclipse.jetty.io.MappedByteBufferPool;
+import org.eclipse.jetty.server.HttpConnection;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.ServerConnector;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.util.BufferUtil;
+import org.eclipse.jetty.websocket.api.Session;
+import org.eclipse.jetty.websocket.api.WriteCallback;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketClose;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketError;
+import org.eclipse.jetty.websocket.api.annotations.OnWebSocketMessage;
+import org.eclipse.jetty.websocket.api.annotations.WebSocket;
+import org.eclipse.jetty.websocket.client.WebSocketClient;
+import org.eclipse.jetty.websocket.core.Frame;
+import org.eclipse.jetty.websocket.core.OpCode;
+import org.eclipse.jetty.websocket.core.internal.Generator;
+import org.eclipse.jetty.websocket.core.internal.WebSocketConnection;
+import org.eclipse.jetty.websocket.server.JettyWebSocketServletContainerInitializer;
+import org.eclipse.jetty.websocket.servlet.WebSocketServlet;
+import org.eclipse.jetty.websocket.servlet.WebSocketServletFactory;
+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.is;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class WebSocketStatsTest
+{
+
+    @WebSocket
+    public static class ClientSocket
+    {
+        CountDownLatch closed = new CountDownLatch(1);
+
+        String behavior;
+
+        @OnWebSocketConnect
+        public void onOpen(Session session)
+        {
+            behavior = session.getPolicy().getBehavior().name();
+            System.err.println(toString() + " Socket Connected: " + session);
+        }
+
+        @OnWebSocketClose
+        public void onClose(int statusCode, String reason)
+        {
+            System.err.println(toString() + " Socket Closed: " + statusCode + ":" + reason);
+            closed.countDown();
+        }
+
+        @OnWebSocketError
+        public void onError(Throwable cause)
+        {
+            cause.printStackTrace(System.err);
+        }
+
+        @Override
+        public String toString()
+        {
+            return String.format("[%s@%s]", behavior, Integer.toHexString(hashCode()));
+        }
+    }
+
+    @WebSocket
+    public static class EchoSocket extends ClientSocket
+    {
+        @OnWebSocketMessage
+        public void onMessage(Session session, String message)
+        {
+            session.getRemote().sendString(message, WriteCallback.NOOP);
+        }
+    }
+
+    public static class MyWebSocketServlet extends WebSocketServlet
+    {
+        @Override
+        public void configure(WebSocketServletFactory factory)
+        {
+            factory.setAutoFragment(false);
+            factory.addMapping("/",(req, resp)->new EchoSocket());
+        }
+    }
+
+    Server server;
+    WebSocketClient client;
+    ConnectionStatistics statistics;
+    CountDownLatch wsUpgradeComplete = new CountDownLatch(1);
+    CountDownLatch wsConnectionClosed = new CountDownLatch(1);
+
+    @BeforeEach
+    public void start() throws Exception
+    {
+        statistics = new ConnectionStatistics()
+        {
+            @Override
+            public void onClosed(Connection connection)
+            {
+                super.onClosed(connection);
+
+                if (connection instanceof WebSocketConnection)
+                    wsConnectionClosed.countDown();
+                else if (connection instanceof HttpConnection)
+                    wsUpgradeComplete.countDown();
+            }
+        };
+
+        server = new Server();
+        ServerConnector connector = new ServerConnector(server);
+        connector.setPort(8080);
+        connector.addBean(statistics);
+        server.addConnector(connector);
+
+        ServletContextHandler contextHandler = new ServletContextHandler(ServletContextHandler.SESSIONS);
+        contextHandler.setContextPath("/");
+        contextHandler.addServlet(MyWebSocketServlet.class, "/testPath");
+        server.setHandler(contextHandler);
+
+        JettyWebSocketServletContainerInitializer.configure(contextHandler);
+        client = new WebSocketClient();
+
+        server.start();
+        client.start();
+    }
+
+    @AfterEach
+    public void stop() throws Exception
+    {
+        client.stop();
+        server.stop();
+    }
+
+    long getFrameByteSize(Frame frame)
+    {
+        ByteBufferPool bufferPool = new MappedByteBufferPool();
+        Generator generator = new Generator(bufferPool);
+        ByteBuffer buffer = bufferPool.acquire(frame.getPayloadLength()+10, true);
+        int pos = BufferUtil.flipToFill(buffer);
+        generator.generateWholeFrame(frame, buffer);
+        return buffer.position() - pos;
+    }
+
+
+    @Test
+    public void echoStatsTest() throws Exception
+    {
+        URI uri = URI.create("ws://localhost:8080/testPath");
+        ClientSocket socket = new ClientSocket();
+        CompletableFuture<Session> connect = client.connect(socket, uri);
+
+        final long numMessages = 10000;
+        final String msgText = "hello world";
+
+        long upgradeSentBytes;
+        long upgradeReceivedBytes;
+
+        try (Session session = connect.get(5, TimeUnit.SECONDS))
+        {
+            wsUpgradeComplete.await(5, TimeUnit.SECONDS);
+            upgradeSentBytes = statistics.getSentBytes();
+            upgradeReceivedBytes = statistics.getReceivedBytes();
+
+            for (int i=0; i<numMessages; i++)
+                session.getRemote().sendString(msgText);
+        }
+        assertTrue(socket.closed.await(5, TimeUnit.SECONDS));
+        assertTrue(wsConnectionClosed.await(5, TimeUnit.SECONDS));
+
+        assertThat(statistics.getConnectionsMax(), is(1L));
+        assertThat(statistics.getConnections(), is(0L));
+
+        assertThat(statistics.getSentMessages(), is(numMessages + 2L));
+        assertThat(statistics.getReceivedMessages(), is(numMessages + 2L));
+
+        Frame textFrame = new Frame(OpCode.TEXT, msgText);
+        Frame closeFrame = new Frame(OpCode.CLOSE);
+
+        final long textFrameSize = getFrameByteSize(textFrame);
+        final long closeFrameSize = getFrameByteSize(closeFrame);
+        final int maskSize = 4; // We use 4 byte mask for client frames in WSConnection
+
+        final long expectedSent = upgradeSentBytes + numMessages*textFrameSize + closeFrameSize;
+        final long expectedReceived = upgradeReceivedBytes + numMessages*(textFrameSize+maskSize) + closeFrameSize+maskSize;
+
+        assertThat(statistics.getSentBytes(), is(expectedSent));
+        assertThat(statistics.getReceivedBytes(), is(expectedReceived));
+    }
+}

jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/FrameHandler.java

@@ -66,8 +66,13 @@ public interface FrameHandler extends IncomingFrames
     /**
      * Async notification that Connection is being opened.
      * <p>
-     * FrameHandler can write during this call, but will not receive frames until
+     * FrameHandler can write during this call, but can not receive frames until the callback is succeeded.
-     * the onOpen() completes.
+     * </p>
+     * <p>
+     * If the FrameHandler succeeds the callback we transition to OPEN state and can now receive frames if
+     * not demanding, or can now call {@link CoreSession#demand(long)} to receive frames if demanding.
+     * If the FrameHandler fails the callback a close frame will be sent with {@link CloseStatus#SERVER_ERROR} and
+     *the connection will be closed. <br>
      * </p>
      *
      * @param coreSession the channel associated with this connection.
@@ -81,9 +86,8 @@ public interface FrameHandler extends IncomingFrames
      * sequentially to satisfy all outstanding demand signaled by calls to
      * {@link CoreSession#demand(long)}.
      * Control and Data frames are passed to this method.
-     * Control frames that require a response (eg PING and CLOSE) may be responded to by the
+     * Close frames may be responded to by the handler, but if an appropriate close response is not
-     * the handler, but if an appropriate response is not sent once the callback is succeeded,
+     * sent once the callback is succeeded, then a response close will be generated and sent.
-     * then a response will be generated and sent.
      *
      * @param frame    the raw frame
      * @param callback the callback to indicate success in processing frame (or failure)
@@ -93,7 +97,8 @@ public interface FrameHandler extends IncomingFrames
     /**
      * An error has occurred or been detected in websocket-core and being reported to FrameHandler.
      * A call to onError will be followed by a call to {@link #onClosed(CloseStatus, Callback)} giving the close status
-     * derived from the error.
+     * derived from the error. This will not be called more than once, {@link #onClosed(CloseStatus, Callback)}
+     * will be called on the callback completion.
      *
      * @param cause the reason for the error
      * @param callback the callback to indicate success in processing (or failure)
@@ -105,6 +110,7 @@ public interface FrameHandler extends IncomingFrames
      * <p>
      * The connection is now closed, no reading or writing is possible anymore.
      * Implementations of FrameHandler can cleanup their resources for this connection now.
+     * This method will be called only once.
      * </p>
      *
      * @param closeStatus the close status received from remote, or in the case of abnormal closure from local.

jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/internal/FrameFlusher.java

@@ -25,6 +25,7 @@ import java.util.ArrayList;
 import java.util.Deque;
 import java.util.List;
 import java.util.Objects;
+import java.util.concurrent.atomic.LongAdder;
 
 import org.eclipse.jetty.io.ByteBufferPool;
 import org.eclipse.jetty.io.EndPoint;
@@ -53,6 +54,8 @@ public class FrameFlusher extends IteratingCallback
     private ByteBuffer batchBuffer = null;
     private boolean canEnqueue = true;
     private Throwable closedCause;
+    private LongAdder messagesOut = new LongAdder();
+    private LongAdder bytesOut = new LongAdder();
 
     public FrameFlusher(ByteBufferPool bufferPool, Generator generator, EndPoint endPoint, int bufferSize, int maxGather)
     {
@@ -159,6 +162,8 @@ public class FrameFlusher extends IteratingCallback
                     break;
                 }
 
+                messagesOut.increment();
+
                 int batchSpace = batchBuffer == null?bufferSize:BufferUtil.space(batchBuffer);
 
                 boolean batch = entry.batch
@@ -224,7 +229,16 @@ public class FrameFlusher extends IteratingCallback
 
         if (flush)
         {
-            endPoint.write(this, buffers.toArray(new ByteBuffer[buffers.size()]));
+            int i = 0;
+            int bytes = 0;
+            ByteBuffer bufferArray[] = new ByteBuffer[buffers.size()];
+            for (ByteBuffer bb : buffers)
+            {
+                bytes += bb.limit() - bb.position();
+                bufferArray[i++] = bb;
+            }
+            bytesOut.add(bytes);
+            endPoint.write(this, bufferArray);
             buffers.clear();
         }
         else
@@ -258,10 +272,10 @@ public class FrameFlusher extends IteratingCallback
         for (Entry entry : entries)
         {
             hadEntries = true;
-            notifyCallbackSuccess(entry.callback);
-            entry.release();
             if (entry.frame.getOpCode() == OpCode.CLOSE)
                 endPoint.shutdownOutput();
+            notifyCallbackSuccess(entry.callback);
+            entry.release();
         }
         entries.clear();
         return hadEntries;
@@ -333,6 +347,16 @@ public class FrameFlusher extends IteratingCallback
         }
     }
 
+    public long getMessagesOut()
+    {
+        return messagesOut.longValue();
+    }
+
+    public long getBytesOut()
+    {
+        return bytesOut.longValue();
+    }
+
     @Override
     public String toString()
     {

jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketChannel.java

@@ -395,9 +395,11 @@ public class WebSocketChannel implements IncomingFrames, FrameHandler.CoreSessio
         CloseStatus closeStatus = abnormalCloseStatusFor(cause);
 
         if (closeStatus.getCode() == CloseStatus.PROTOCOL)
-            close(closeStatus, NOOP);
+            close(closeStatus, callback);
         else if (channelState.onClosed(closeStatus))
             closeConnection(cause, closeStatus, callback);
+        else
+            callback.failed(cause);
     }
 
     /**
@@ -428,7 +430,7 @@ public class WebSocketChannel implements IncomingFrames, FrameHandler.CoreSessio
         if (LOG.isDebugEnabled())
             LOG.debug("ConnectionState: Transition to CONNECTED");
 
-        Callback openCallback =  Callback.from(()->
+        Callback openCallback = Callback.from(()->
                 {
                     channelState.onOpen();
                     if (!demanding)
@@ -450,6 +452,11 @@ public class WebSocketChannel implements IncomingFrames, FrameHandler.CoreSessio
         catch (Throwable t)
         {
             openCallback.failed(t);
+
+            /* This is double handling of the exception but we need to do this because we have two separate
+            mechanisms for returning the CoreSession, onOpen and the CompletableFuture and both the onOpen callback
+            and the CompletableFuture require the exception. */
+            throw new RuntimeException(t);
         }
 
     }
@@ -481,9 +488,9 @@ public class WebSocketChannel implements IncomingFrames, FrameHandler.CoreSessio
         {
             assertValidIncoming(frame);
         }
-        catch (Throwable ex)
+        catch (Throwable t)
         {
-            callback.failed(ex);
+            callback.failed(t);
             return;
         }
 
@@ -497,9 +504,9 @@ public class WebSocketChannel implements IncomingFrames, FrameHandler.CoreSessio
         {
             assertValidOutgoing(frame);
         }
-        catch (Throwable ex)
+        catch (Throwable t)
         {
-            callback.failed(ex);
+            callback.failed(t);
             return;
         }
 
@@ -517,13 +524,7 @@ public class WebSocketChannel implements IncomingFrames, FrameHandler.CoreSessio
 
                     Callback closeConnectionCallback = Callback.from(
                             ()->closeConnection(cause, channelState.getCloseStatus(), callback),
-                            x->closeConnection(cause, channelState.getCloseStatus(), Callback.from(
+                            t->closeConnection(cause, channelState.getCloseStatus(), Callback.from(callback, t)));
-                                    ()-> callback.failed(x),
-                                    x2->
-                                    {
-                                        x.addSuppressed(x2);
-                                        callback.failed(x);
-                                    })));
 
                     flusher.queue.offer(new FrameEntry(frame, closeConnectionCallback, false));
                 }
@@ -534,24 +535,18 @@ public class WebSocketChannel implements IncomingFrames, FrameHandler.CoreSessio
             }
             flusher.iterate();
         }
-        catch (Throwable ex)
+        catch (Throwable t)
         {
             if (frame.getOpCode() == OpCode.CLOSE)
             {
                 CloseStatus closeStatus = CloseStatus.getCloseStatus(frame);
                 if (closeStatus instanceof AbnormalCloseStatus && channelState.onClosed(closeStatus))
-                    closeConnection(null, closeStatus, Callback.from(
+                    closeConnection(AbnormalCloseStatus.getCause(closeStatus), closeStatus, Callback.from(callback, t));
-                            ()->callback.failed(ex),
-                            x2->
-                            {
-                                ex.addSuppressed(x2);
-                                callback.failed(ex);
-                            }));
                 else
-                    callback.failed(ex);
+                    callback.failed(t);
             }
             else
-                callback.failed(ex);
+                callback.failed(t);
         }
     }
 

jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketConnection.java

@@ -21,10 +21,10 @@ package org.eclipse.jetty.websocket.core.internal;
 import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.nio.ByteBuffer;
-import java.nio.channels.ClosedChannelException;
 import java.util.Objects;
 import java.util.Random;
 import java.util.concurrent.Executor;
+import java.util.concurrent.atomic.LongAdder;
 
 import org.eclipse.jetty.io.AbstractConnection;
 import org.eclipse.jetty.io.ByteBufferPool;
@@ -64,6 +64,8 @@ public class WebSocketConnection extends AbstractConnection implements Connectio
 
     private long demand;
     private boolean fillingAndParsing;
+    private LongAdder messagesIn = new LongAdder();
+    private LongAdder bytesIn = new LongAdder();
 
     // Read / Parse variables
     private RetainableByteBuffer networkBuffer;
@@ -400,6 +402,8 @@ public class WebSocketConnection extends AbstractConnection implements Connectio
                     if (frame == null)
                         break;
 
+                    messagesIn.increment();
+
                     if (meetDemand())
                         onFrame(frame);
 
@@ -438,6 +442,8 @@ public class WebSocketConnection extends AbstractConnection implements Connectio
                     fillInterested();
                     return;
                 }
+
+                bytesIn.add(filled);
             }
         }
         catch (Throwable t)
@@ -540,6 +546,30 @@ public class WebSocketConnection extends AbstractConnection implements Connectio
         setInitialBuffer(prefilled);
     }
 
+    @Override
+    public long getMessagesIn()
+    {
+        return messagesIn.longValue();
+    }
+
+    @Override
+    public long getBytesIn()
+    {
+        return bytesIn.longValue();
+    }
+
+    @Override
+    public long getMessagesOut()
+    {
+        return flusher.getMessagesOut();
+    }
+
+    @Override
+    public long getBytesOut()
+    {
+        return flusher.getBytesOut();
+    }
+
     /**
      * Enqueue a Frame to be sent.
      * @param frame The frame to queue

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

@@ -21,6 +21,7 @@ package org.eclipse.jetty.server.session;
 
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
 
 /**
  * FileSessionDataStoreTest
@@ -68,7 +69,17 @@ public class FileSessionDataStoreTest extends AbstractSessionDataStoreTest
     @Override
     public boolean checkSessionExists(SessionData data) throws Exception
     {
-        return (FileTestHelper.getFile(data.getId()) != null);
+        ClassLoader old = Thread.currentThread().getContextClassLoader();
+        Thread.currentThread().setContextClassLoader (_contextClassLoader); 
+        try
+        {
+            return (FileTestHelper.getFile(data.getId()) != null);
+        }
+        finally
+        {
+            Thread.currentThread().setContextClassLoader(old); 
+        }
+       
     }
 
     /** 
@@ -77,7 +88,29 @@ public class FileSessionDataStoreTest extends AbstractSessionDataStoreTest
     @Override
     public boolean checkSessionPersisted(SessionData data) throws Exception
     {
-        return FileTestHelper.checkSessionPersisted(data);
+        ClassLoader old = Thread.currentThread().getContextClassLoader();
+        Thread.currentThread().setContextClassLoader (_contextClassLoader); 
+        try
+        {
+            return FileTestHelper.checkSessionPersisted(data);
+        }
+        catch (Throwable e)
+        {
+            e.printStackTrace();
+            throw e;
+        }
+        finally
+        {
+            Thread.currentThread().setContextClassLoader(old); 
+        }
     }
 
+    @Override
+    @Test
+    public void testStoreSession() throws Exception
+    {
+        super.testStoreSession();
+    }
+
+    
 }

tests/test-sessions/test-gcloud-sessions/src/test/java/org/eclipse/jetty/gcloud/session/GCloudSessionDataStoreTest.java

@@ -96,7 +96,16 @@ public class GCloudSessionDataStoreTest extends AbstractSessionDataStoreTest
     @Override
     public boolean checkSessionPersisted(SessionData data) throws Exception
     {
-        return __testSupport.checkSessionPersisted(data);
+        ClassLoader old = Thread.currentThread().getContextClassLoader();
+        Thread.currentThread().setContextClassLoader (_contextClassLoader); 
+        try
+        {
+            return __testSupport.checkSessionPersisted(data);
+        }
+        finally
+        {
+            Thread.currentThread().setContextClassLoader(old); 
+        }
     }
 
 }

tests/test-sessions/test-hazelcast-sessions/src/test/java/org/eclipse/jetty/hazelcast/session/HazelcastSessionDataStoreTest.java

@@ -152,6 +152,15 @@ public class HazelcastSessionDataStoreTest extends AbstractSessionDataStoreTest
     @Override
     public boolean checkSessionPersisted(SessionData data) throws Exception
     {
-        return _testHelper.checkSessionPersisted(data);
+        ClassLoader old = Thread.currentThread().getContextClassLoader();
+        Thread.currentThread().setContextClassLoader (_contextClassLoader); 
+        try
+        {
+            return _testHelper.checkSessionPersisted(data);
+        }
+        finally
+        {
+            Thread.currentThread().setContextClassLoader(old); 
+        }   
     }
 }

tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/InfinispanSessionDataStoreTest.java

@@ -152,7 +152,16 @@ public class InfinispanSessionDataStoreTest extends AbstractSessionDataStoreTest
     @Override
     public boolean checkSessionPersisted(SessionData data) throws Exception
     {
-        return __testSupport.checkSessionPersisted(data);
+        ClassLoader old = Thread.currentThread().getContextClassLoader();
+        Thread.currentThread().setContextClassLoader (_contextClassLoader); 
+        try
+        {
+            return __testSupport.checkSessionPersisted(data);
+        }
+        finally
+        {
+            Thread.currentThread().setContextClassLoader(old); 
+        }   
     }
     
 }

tests/test-sessions/test-infinispan-sessions/src/test/java/org/eclipse/jetty/server/session/remote/RemoteInfinispanSessionDataStoreTest.java

@@ -96,7 +96,16 @@ public class RemoteInfinispanSessionDataStoreTest extends AbstractSessionDataSto
     @Override
     public boolean checkSessionPersisted(SessionData data) throws Exception
     {
-        return __testSupport.checkSessionPersisted(data);
+        ClassLoader old = Thread.currentThread().getContextClassLoader();
+        Thread.currentThread().setContextClassLoader (_contextClassLoader); 
+        try
+        {
+            return __testSupport.checkSessionPersisted(data);
+        }
+        finally
+        {
+            Thread.currentThread().setContextClassLoader(old); 
+        }  
     }
     
     

tests/test-sessions/test-jdbc-sessions/src/test/java/org/eclipse/jetty/server/session/JDBCSessionDataStoreTest.java

@@ -89,7 +89,17 @@ public class JDBCSessionDataStoreTest extends AbstractSessionDataStoreTest
     @Override
     public boolean checkSessionPersisted(SessionData data) throws Exception
     {
-        return JdbcTestHelper.checkSessionPersisted(data);
+        ClassLoader old = Thread.currentThread().getContextClassLoader();
+        Thread.currentThread().setContextClassLoader (_contextClassLoader); 
+        try
+        {
+            return JdbcTestHelper.checkSessionPersisted(data);
+        }
+        finally
+        {
+            Thread.currentThread().setContextClassLoader(old); 
+        }
+
     }
-    
+
 }

tests/test-sessions/test-mongodb-sessions/src/test/java/org/eclipse/jetty/nosql/mongodb/MongoSessionDataStoreTest.java

@@ -92,7 +92,16 @@ public class MongoSessionDataStoreTest extends AbstractSessionDataStoreTest
     @Override
     public boolean checkSessionPersisted(SessionData data) throws Exception
     {
-       return MongoTestHelper.checkSessionPersisted(data);
+       ClassLoader old = Thread.currentThread().getContextClassLoader();
+       Thread.currentThread().setContextClassLoader (_contextClassLoader); 
+       try
+       {
+           return MongoTestHelper.checkSessionPersisted(data);
+       }
+       finally
+       {
+           Thread.currentThread().setContextClassLoader(old); 
+       }
     }
     
     

tests/test-sessions/test-mongodb-sessions/src/test/java/org/eclipse/jetty/nosql/mongodb/MongoTestHelper.java

@@ -56,7 +56,7 @@ public class MongoTestHelper
         try
         {
             _mongoClient =
-                new MongoClient( System.getProperty( "embedmongo.host" ), Integer.getInteger( "embedmongoPort" ) );
+                    new MongoClient( System.getProperty( "embedmongo.host" ), Integer.getInteger( "embedmongoPort" ) );
         }
         catch ( UnknownHostException e )
         {

tests/test-sessions/test-sessions-common/src/main/java/org/eclipse/jetty/server/session/AbstractSessionDataStoreTest.java

@@ -28,13 +28,20 @@ import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.InputStream;
 import java.lang.reflect.Proxy;
+import java.net.URL;
+import java.net.URLClassLoader;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
 import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.toolchain.test.IO;
+import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
 import org.junit.jupiter.api.Test;
 
 /**
@@ -54,6 +61,7 @@ public abstract class AbstractSessionDataStoreTest
     public static final long ANCIENT_TIMESTAMP = 100L;
     public static final long RECENT_TIMESTAMP = System.currentTimeMillis() - TimeUnit.SECONDS.toMillis(3*GRACE_PERIOD_SEC);
     
+    protected URLClassLoader _contextClassLoader;
 
     
     
@@ -70,31 +78,86 @@ public abstract class AbstractSessionDataStoreTest
     
     
     /**
-     * Test that the store can persist a session.
+     * Test that the store can persist a session. The session uses an attribute
+     * class that is only known to the webapp classloader. This tests that
+     * we use the webapp loader when we serialize the session data (ie save the session).
      * 
      * @throws Exception 
      */
     @Test
     public void testStoreSession() throws Exception
     {
+        //Use a class that would only be known to the webapp classloader
+        InputStream foostream = Thread.currentThread().getContextClassLoader().getResourceAsStream("Foo.clazz");
+        File foodir = new File (MavenTestingUtils.getTargetDir(), "foo");
+        foodir.mkdirs();
+        File fooclass = new File (foodir, "Foo.class");
+        IO.copy(foostream, new FileOutputStream(fooclass));
+        
+        assertTrue(fooclass.exists());
+        assertTrue(fooclass.length() != 0);
+        
+        URL[] foodirUrls = new URL[]{foodir.toURI().toURL()};
+        _contextClassLoader = new URLClassLoader(foodirUrls, Thread.currentThread().getContextClassLoader());
+        
         //create the SessionDataStore
         ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS);
-        context.setContextPath("/test");       
+        context.setContextPath("/test");
+        //use the classloader with the special class in it
+        context.setClassLoader(_contextClassLoader);
+        
         SessionDataStoreFactory factory = createSessionDataStoreFactory();
         ((AbstractSessionDataStoreFactory)factory).setGracePeriodSec(GRACE_PERIOD_SEC);
         SessionDataStore store = factory.getSessionDataStore(context.getSessionHandler());
         SessionContext sessionContext = new SessionContext("foo", context.getServletContext());
         store.initialize(sessionContext);
-        
+
         store.start();
+
+        ClassLoader old = Thread.currentThread().getContextClassLoader();
+        SessionData data = null;
+        try
+        {
+            Thread.currentThread().setContextClassLoader(_contextClassLoader);
+            Class fooclazz = Class.forName("Foo", true, _contextClassLoader);
+            //create a session
+            long now = System.currentTimeMillis();
+            data = store.newSessionData("1234", 100, now, now-1, -1);//never expires
+            data.setLastNode(sessionContext.getWorkerName());
+
+            //Make an attribute that uses the class only known to the webapp classloader
+            data.setAttribute("a", fooclazz.getConstructor(null).newInstance());
+        }
+        finally
+        {
+            Thread.currentThread().setContextClassLoader(old);
+        }
+
+        //store the session, using a different thread to ensure
+        //that the thread is adorned with the webapp classloader
+        //before serialization
+        final SessionData finalData = data;
         
-        //create a session
+        Runnable r = new Runnable()
-        long now = System.currentTimeMillis();
+        {
-        SessionData data = store.newSessionData("1234", 100, now, now-1, -1);//never expires
+
-        data.setAttribute("a", "b");
+            @Override
-        data.setLastNode(sessionContext.getWorkerName());
+            public void run()
-        
+            {
-        store.store("1234", data);
+                try
+                {
+                    store.store("1234", finalData);
+                }
+                catch (Exception e)
+                {
+                    fail(e);
+                }
+            }
+        };
+       
+        Thread t = new Thread(r, "saver");
+        t.start();
+        t.join(TimeUnit.SECONDS.toMillis(10));
         
         //check that the store contains all of the session data
         assertTrue(checkSessionPersisted(data));
@@ -148,33 +211,31 @@ public abstract class AbstractSessionDataStoreTest
      * 
      * @throws Exception
      */
-    @Test
+    /*
-    public void testStoreObjectAttributes() throws Exception
+     * @Test public void testStoreObjectAttributes() throws Exception { //create
-    {
+     * the SessionDataStore ServletContextHandler context = new
-        //create the SessionDataStore
+     * ServletContextHandler(ServletContextHandler.SESSIONS);
-        ServletContextHandler context = new ServletContextHandler(ServletContextHandler.SESSIONS);
+     * context.setContextPath("/test"); SessionDataStoreFactory factory =
-        context.setContextPath("/test");       
+     * createSessionDataStoreFactory();
-        SessionDataStoreFactory factory = createSessionDataStoreFactory();
+     * ((AbstractSessionDataStoreFactory)factory).setGracePeriodSec(
-        ((AbstractSessionDataStoreFactory)factory).setGracePeriodSec(GRACE_PERIOD_SEC);
+     * GRACE_PERIOD_SEC); SessionDataStore store =
-        SessionDataStore store = factory.getSessionDataStore(context.getSessionHandler());
+     * factory.getSessionDataStore(context.getSessionHandler()); SessionContext
-        SessionContext sessionContext = new SessionContext("foo", context.getServletContext());
+     * sessionContext = new SessionContext("foo", context.getServletContext());
-        store.initialize(sessionContext);
+     * store.initialize(sessionContext);
-        
+     * 
-        store.start();
+     * store.start();
-        
+     * 
-        //create a session
+     * //create a session SessionData data = store.newSessionData("1234", 100,
-        SessionData data = store.newSessionData("1234", 100, 200, 199, -1);//never expires
+     * 200, 199, -1);//never expires TestFoo testFoo = new TestFoo();
-        TestFoo testFoo = new TestFoo();
+     * testFoo.setInt(33); FooInvocationHandler handler = new
-        testFoo.setInt(33);
+     * FooInvocationHandler(testFoo); Foo foo =
-        FooInvocationHandler handler = new FooInvocationHandler(testFoo);
+     * (Foo)Proxy.newProxyInstance(Thread.currentThread().getContextClassLoader(
-        Foo foo = (Foo)Proxy.newProxyInstance(Thread.currentThread().getContextClassLoader(), new Class[] {Foo.class}, handler);
+     * ), new Class[] {Foo.class}, handler); data.setAttribute("foo", foo);
-        data.setAttribute("foo", foo);
+     * data.setLastNode(sessionContext.getWorkerName());
-        data.setLastNode(sessionContext.getWorkerName());
+     * 
-        
+     * //test that it can be persisted store.store("1234", data);
-        //test that it can be persisted
+     * checkSessionPersisted(data); }
-        store.store("1234", data);
+     */
-        checkSessionPersisted(data);
-    }
 
     /**
      * Test that we can load a persisted session.

tests/test-sessions/test-sessions-common/src/main/resources/Foo.java

@@ -0,0 +1,41 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+public class Foo implements java.io.Serializable
+{
+    int myI = 0;
+
+    public Foo()
+    {
+    }
+
+    public void setI(int i)
+    {
+      myI = i;
+    }
+
+    public int getI()
+    {
+        return myI;
+    }
+
+    public boolean equals(Object o)
+    {
+        return ((Foo)o).getI() == myI;
+    }
+}

tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/CreationTest.java

@@ -19,6 +19,7 @@
 
 package org.eclipse.jetty.server.session;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 import java.io.IOException;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
@@ -33,11 +34,13 @@ import javax.servlet.http.HttpSession;
 import org.eclipse.jetty.client.HttpClient;
 import org.eclipse.jetty.client.api.ContentResponse;
 import org.eclipse.jetty.client.api.Request;
+import org.eclipse.jetty.http.HttpHeader;
 import org.eclipse.jetty.servlet.ServletContextHandler;
 import org.eclipse.jetty.servlet.ServletHolder;
 import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.util.log.Log;
 import org.eclipse.jetty.util.log.StacklessLogging;
+import org.hamcrest.Matchers;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -236,12 +239,60 @@ public class CreationTest
             server1.stop();
         }
     }
-    
+
-    
+
-    
+
-    
+    /**
-    
+     * Create and then invalidate and then create a session in the same request
-    
+     * @throws Exception
+     */
+    @Test
+    public void testSessionCreateInvalidateCreate() throws Exception
+    {
+        String contextPath = "";
+        String servletMapping = "/server";
+        int inactivePeriod = 20;
+        int scavengePeriod = 3;
+        DefaultSessionCacheFactory cacheFactory = new DefaultSessionCacheFactory();
+        cacheFactory.setEvictionPolicy(SessionCache.NEVER_EVICT);
+        SessionDataStoreFactory storeFactory = new TestSessionDataStoreFactory();
+        TestServer server1 = new TestServer(0, inactivePeriod, scavengePeriod, cacheFactory, storeFactory);
+        TestServlet servlet = new TestServlet();
+        ServletHolder holder = new ServletHolder(servlet);
+        ServletContextHandler contextHandler = server1.addContext(contextPath);
+        TestContextScopeListener scopeListener = new TestContextScopeListener();
+        contextHandler.addEventListener(scopeListener);
+        contextHandler.addServlet(holder, servletMapping);
+        servlet.setStore(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore());
+        server1.start();
+        int port1 = server1.getPort();
+
+        try (StacklessLogging stackless = new StacklessLogging(Log.getLogger("org.eclipse.jetty.server.session")))
+        {
+            HttpClient client = new HttpClient();
+            client.start();
+            String url = "http://localhost:" + port1 + contextPath + servletMapping+"?action=createinvcreate&check=false";
+
+            CountDownLatch synchronizer = new CountDownLatch(1);
+            scopeListener.setExitSynchronizer(synchronizer);
+
+            //make a request to set up a session on the server
+            ContentResponse response = client.GET(url);
+            assertEquals(HttpServletResponse.SC_OK,response.getStatus());
+
+            //ensure request has finished being handled
+            synchronizer.await(5, TimeUnit.SECONDS);
+
+            //check that the session does not exist
+            assertTrue(contextHandler.getSessionHandler().getSessionCache().getSessionDataStore().exists(servlet._id));
+            assertThat(response.getHeaders().getValuesList(HttpHeader.SET_COOKIE).size(), Matchers.is(1));
+        }
+        finally
+        {
+            server1.stop();
+        }
+    }
+
     /**
      * Create a session in a context, forward to another context and create a 
      * session in it too. Check that both sessions exist after the response
@@ -435,6 +486,14 @@ public class CreationTest
                     assertNull(request.getSession(false));
                     assertNotNull(session);
                 }
+                else if ("createinvcreate".equals(action))
+                {
+                    session.invalidate();
+                    assertNull(request.getSession(false));
+                    assertNotNull(session);
+                    session = request.getSession(true);
+                    _id = session.getId();
+                }
             }
         }
     }

tests/test-sessions/test-sessions-common/src/test/java/org/eclipse/jetty/server/session/SessionRenewTest.java

@@ -51,9 +51,6 @@ public class SessionRenewTest
 {
     protected TestServer _server;
     
- 
-    
-    
     /**
      * Tests renewing a session id when sessions are not being cached.
      * @throws Exception
@@ -236,9 +233,7 @@ public class SessionRenewTest
                 assertNull(session);
 
                 if (((Session)afterSession).isIdChanged())
-                {
+                    ((org.eclipse.jetty.server.Response)response).replaceCookie(sessionManager.getSessionCookie(afterSession, request.getContextPath(), request.isSecure()));
-                    ((org.eclipse.jetty.server.Response)response).addCookie(sessionManager.getSessionCookie(afterSession, request.getContextPath(), request.isSecure()));
-                }
             }
         }
     }