This is an automated email from the ASF dual-hosted git repository. markt-asf pushed a commit to branch 9.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 4b41a73a2f1a16647d7444ad6ee87d41a3ec414b Author: Mark Thomas <[email protected]> AuthorDate: Fri Aug 7 12:38:32 2026 +0100 Handle HTTP sessionID changes for WsSession close on HTTP session expiry --- .../apache/catalina/ha/session/DeltaSession.java | 74 ++++++----- java/org/apache/tomcat/websocket/WsSession.java | 14 +- .../server/WsHttpSessionBindingListener.java | 61 +++++++++ .../websocket/server/WsHttpUpgradeHandler.java | 16 +-- java/org/apache/tomcat/websocket/server/WsSci.java | 1 - .../tomcat/websocket/server/WsServerContainer.java | 121 +++++++++++++----- .../tomcat/websocket/server/WsSessionListener.java | 36 ------ .../tomcat/websocket/TestWebSocketFrameClient.java | 142 ++++++++++++++++++++- webapps/docs/changelog.xml | 5 + 9 files changed, 355 insertions(+), 115 deletions(-) diff --git a/java/org/apache/catalina/ha/session/DeltaSession.java b/java/org/apache/catalina/ha/session/DeltaSession.java index f3f751fecd..0549d599c7 100644 --- a/java/org/apache/catalina/ha/session/DeltaSession.java +++ b/java/org/apache/catalina/ha/session/DeltaSession.java @@ -116,11 +116,11 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu } /** - * Create a DeltaRequest instance. This protected method enables subclasses to override and use - * custom DeltaRequest implementations. + * Create a DeltaRequest instance. This protected method enables subclasses to override and use custom DeltaRequest + * implementations. * - * @param sessionId Session identifier - * @param recordAllActions Record all actions, including duplicates + * @param sessionId Session identifier + * @param recordAllActions Record all actions, including duplicates * * @return New DeltaRequest instance */ @@ -313,8 +313,8 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu /** * Set the maximum inactive interval. * - * @param interval Max inactive interval in seconds - * @param addDeltaRequest Whether to add a delta request entry + * @param interval Max inactive interval in seconds + * @param addDeltaRequest Whether to add a delta request entry */ public void setMaxInactiveInterval(int interval, boolean addDeltaRequest) { super.maxInactiveInterval = interval; @@ -336,8 +336,8 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu /** * Set the new flag. * - * @param isNew New flag value - * @param addDeltaRequest Whether to add a delta request entry + * @param isNew New flag value + * @param addDeltaRequest Whether to add a delta request entry */ public void setNew(boolean isNew, boolean addDeltaRequest) { super.setNew(isNew); @@ -359,8 +359,8 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu /** * Set the session principal. * - * @param principal Session principal - * @param addDeltaRequest Whether to add a delta request entry + * @param principal Session principal + * @param addDeltaRequest Whether to add a delta request entry */ public void setPrincipal(Principal principal, boolean addDeltaRequest) { lockInternal(); @@ -382,8 +382,8 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu /** * Set the authentication type. * - * @param authType Authentication type - * @param addDeltaRequest Whether to add a delta request entry + * @param authType Authentication type + * @param addDeltaRequest Whether to add a delta request entry */ public void setAuthType(String authType, boolean addDeltaRequest) { lockInternal(); @@ -446,8 +446,8 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu /** * Expire this session. * - * @param notify Whether to notify session listeners - * @param notifyCluster Whether to notify the cluster of expiration + * @param notify Whether to notify session listeners + * @param notifyCluster Whether to notify the cluster of expiration */ public void expire(boolean notify, boolean notifyCluster) { @@ -519,8 +519,8 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu /** * Add a session listener. * - * @param listener Session listener to add - * @param addDeltaRequest Whether to add a delta request entry + * @param listener Session listener to add + * @param addDeltaRequest Whether to add a delta request entry */ public void addSessionListener(SessionListener listener, boolean addDeltaRequest) { lockInternal(); @@ -542,8 +542,8 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu /** * Remove a session listener. * - * @param listener Session listener to remove - * @param addDeltaRequest Whether to add a delta request entry + * @param listener Session listener to remove + * @param addDeltaRequest Whether to add a delta request entry */ public void removeSessionListener(SessionListener listener, boolean addDeltaRequest) { lockInternal(); @@ -698,9 +698,9 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu /** * Remove an attribute from this session. * - * @param name Attribute name - * @param notify Whether to notify listeners - * @param addDeltaRequest Whether to add a delta request entry + * @param name Attribute name + * @param notify Whether to notify listeners + * @param addDeltaRequest Whether to add a delta request entry * * @throws IllegalStateException If this session is no longer valid */ @@ -720,10 +720,10 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu /** * Set an attribute on this session. * - * @param name Attribute name - * @param value Attribute value - * @param notify Whether to notify listeners - * @param addDeltaRequest Whether to add a delta request entry + * @param name Attribute name + * @param value Attribute value + * @param notify Whether to notify listeners + * @param addDeltaRequest Whether to add a delta request entry * * @throws IllegalArgumentException If name is null */ @@ -743,7 +743,13 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu lockInternal(); try { super.setAttribute(name, value, notify); - if (addDeltaRequest && !exclude(name, value)) { + /* + * It is possible that the session expires concurrently with the attribute being added. Depending on the + * exact timing, one of two things will happen. Either an IllegalStateException will be thrown or the + * attribute will be added and then immediately removed from the session. The exception will be re-thrown. + * If the attribute is removed, don't update the deltaRequest. + */ + if (getAttribute(name) != null && addDeltaRequest && !exclude(name, value)) { deltaRequest.setAttribute(name, value); } } finally { @@ -760,8 +766,8 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu /** * Remove a note from this session. * - * @param name Note name - * @param addDeltaRequest Whether to add a delta request entry + * @param name Note name + * @param addDeltaRequest Whether to add a delta request entry */ @SuppressWarnings("deprecation") public void removeNote(String name, boolean addDeltaRequest) { @@ -786,9 +792,9 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu /** * Set a note on this session. * - * @param name Note name - * @param value Note value - * @param addDeltaRequest Whether to add a delta request entry + * @param name Note name + * @param value Note value + * @param addDeltaRequest Whether to add a delta request entry */ @SuppressWarnings("deprecation") public void setNote(String name, Object value, boolean addDeltaRequest) { @@ -993,9 +999,9 @@ public class DeltaSession extends StandardSession implements Externalizable, Clu /** * Remove an attribute from this session without additional validation. * - * @param name Attribute name - * @param notify Whether to notify listeners - * @param addDeltaRequest Whether to add a delta request entry + * @param name Attribute name + * @param notify Whether to notify listeners + * @param addDeltaRequest Whether to add a delta request entry */ protected void removeAttributeInternal(String name, boolean notify, boolean addDeltaRequest) { lockInternal(); diff --git a/java/org/apache/tomcat/websocket/WsSession.java b/java/org/apache/tomcat/websocket/WsSession.java index c6ccb180a2..bcf0e123b9 100644 --- a/java/org/apache/tomcat/websocket/WsSession.java +++ b/java/org/apache/tomcat/websocket/WsSession.java @@ -372,6 +372,7 @@ public class WsSession implements Session { /** * Returns the instance manager for this session. + * * @return the instance manager */ public InstanceManager getInstanceManager() { @@ -553,6 +554,7 @@ public class WsSession implements Session { /** * Checks if the session is closed. + * * @return true if the session is closed */ public boolean isClosed() { @@ -767,6 +769,7 @@ public class WsSession implements Session { /** * Returns the session close timeout in milliseconds. + * * @return the session close timeout */ protected long getSessionCloseTimeout() { @@ -1038,6 +1041,7 @@ public class WsSession implements Session { /** * Returns the user principal for this session. + * * @return the user principal */ public Principal getUserPrincipalInternal() { @@ -1067,6 +1071,7 @@ public class WsSession implements Session { /** * Returns the local endpoint for this session. + * * @return the local endpoint */ public Endpoint getLocal() { @@ -1075,9 +1080,13 @@ public class WsSession implements Session { /** - * Returns the HTTP session ID associated with this WebSocket session. + * Returns the HTTP session ID associated with this WebSocket session at the time the WebSocket session was created. + * * @return the HTTP session ID, or null if not associated + * + * @deprecated Unused. Will be removed from Tomcat 12 onwards */ + @Deprecated public String getHttpSessionId() { return httpSessionId; } @@ -1085,6 +1094,7 @@ public class WsSession implements Session { /** * Returns the text message handler for this session. + * * @return the text message handler */ protected MessageHandler getTextMessageHandler() { @@ -1094,6 +1104,7 @@ public class WsSession implements Session { /** * Returns the binary message handler for this session. + * * @return the binary message handler */ protected MessageHandler getBinaryMessageHandler() { @@ -1103,6 +1114,7 @@ public class WsSession implements Session { /** * Returns the pong message handler for this session. + * * @return the pong message handler */ protected MessageHandler.Whole<PongMessage> getPongMessageHandler() { diff --git a/java/org/apache/tomcat/websocket/server/WsHttpSessionBindingListener.java b/java/org/apache/tomcat/websocket/server/WsHttpSessionBindingListener.java new file mode 100644 index 0000000000..94cfabca63 --- /dev/null +++ b/java/org/apache/tomcat/websocket/server/WsHttpSessionBindingListener.java @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.websocket.server; + +import java.io.Serializable; + +import javax.servlet.http.HttpSession; +import javax.servlet.http.HttpSessionBindingEvent; +import javax.servlet.http.HttpSessionBindingListener; + +public class WsHttpSessionBindingListener implements HttpSessionBindingListener, Serializable { + + private static final long serialVersionUID = 1L; + + private final String key; + + public WsHttpSessionBindingListener(String key) { + this.key = key; + } + + + public String getKey() { + return key; + } + + + @Override + public void valueUnbound(HttpSessionBindingEvent event) { + HttpSession httpSession = event.getSession(); + /* + * During replication this event will be triggered when the attribute is updated. Updates should not trigger a + * call to the WebSocket server container. If this is an update, the session will still be valid. + */ + try { + httpSession.getCreationTime(); + // No exception. Session is valid. Nothing to do. + return; + } catch (IllegalStateException ise) { + // Ignore + + } + Object obj = httpSession.getServletContext().getAttribute(Constants.SERVER_CONTAINER_SERVLET_CONTEXT_ATTRIBUTE); + if (obj instanceof WsServerContainer) { + ((WsServerContainer) obj).handleHttpSessionKeyUnbound(key); + } + } +} diff --git a/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java b/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java index f3a9326426..bbf0b2339a 100644 --- a/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java +++ b/java/org/apache/tomcat/websocket/server/WsHttpUpgradeHandler.java @@ -90,14 +90,14 @@ public class WsHttpUpgradeHandler implements InternalHttpUpgradeHandler { /** * Performs initialization before the WebSocket handshake is completed. * - * @param serverEndpointConfig the endpoint configuration - * @param wsc the WebSocket server container - * @param handshakeRequest the handshake request + * @param serverEndpointConfig the endpoint configuration + * @param wsc the WebSocket server container + * @param handshakeRequest the handshake request * @param negotiatedExtensionsPhase2 negotiated extensions - * @param subProtocol the negotiated sub-protocol - * @param transformation the data transformation - * @param pathParameters the path parameters - * @param secure whether the connection is secure + * @param subProtocol the negotiated sub-protocol + * @param transformation the data transformation + * @param pathParameters the path parameters + * @param secure whether the connection is secure */ public void preInit(ServerEndpointConfig serverEndpointConfig, WsServerContainer wsc, WsHandshakeRequest handshakeRequest, List<Extension> negotiatedExtensionsPhase2, String subProtocol, @@ -158,7 +158,7 @@ public class WsHttpUpgradeHandler implements InternalHttpUpgradeHandler { } throw new IllegalArgumentException(t); } - webSocketContainer.registerSession(serverEndpointConfig.getPath(), wsSession); + webSocketContainer.registerSession(serverEndpointConfig.getPath(), wsSession, session); } catch (DeploymentException e) { throw new IllegalArgumentException(e); } finally { diff --git a/java/org/apache/tomcat/websocket/server/WsSci.java b/java/org/apache/tomcat/websocket/server/WsSci.java index 7bc2776491..98deee2414 100644 --- a/java/org/apache/tomcat/websocket/server/WsSci.java +++ b/java/org/apache/tomcat/websocket/server/WsSci.java @@ -133,7 +133,6 @@ public class WsSci implements ServletContainerInitializer { servletContext.setAttribute(Constants.SERVER_CONTAINER_SERVLET_CONTEXT_ATTRIBUTE, sc); - servletContext.addListener(new WsSessionListener(sc)); // Can't register the ContextListener again if the ContextListener is // calling this method if (initBySciMechanism) { diff --git a/java/org/apache/tomcat/websocket/server/WsServerContainer.java b/java/org/apache/tomcat/websocket/server/WsServerContainer.java index 93e6469f1c..d1b2abaa39 100644 --- a/java/org/apache/tomcat/websocket/server/WsServerContainer.java +++ b/java/org/apache/tomcat/websocket/server/WsServerContainer.java @@ -20,6 +20,8 @@ import java.io.IOException; import java.util.Arrays; import java.util.Collections; import java.util.EnumSet; +import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -32,6 +34,7 @@ import javax.servlet.ServletContext; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.servlet.http.HttpSession; import javax.websocket.CloseReason; import javax.websocket.CloseReason.CloseCodes; import javax.websocket.DeploymentException; @@ -71,7 +74,9 @@ public class WsServerContainer extends WsWebSocketContainer implements ServerCon @SuppressWarnings("deprecation") private volatile boolean enforceNoAddAfterHandshake = org.apache.tomcat.websocket.Constants.STRICT_SPEC_COMPLIANCE; private volatile boolean addAllowed = true; - private final Map<String,Set<WsSession>> authenticatedSessions = new ConcurrentHashMap<>(); + private final Object authenticatedSessionMapLock = new Object(); + private final Map<String,Set<WsSession>> httpSessionKeyToWebSocketSession = new HashMap<>(); + private final Map<WsSession,String> webSocketSessionToHttpSessionKey = new HashMap<>(); private volatile boolean endpointsRegistered = false; private volatile boolean deploymentFailed = false; @@ -338,7 +343,9 @@ public class WsServerContainer extends WsWebSocketContainer implements ServerCon /** * Finds the endpoint configuration that matches the given path. + * * @param path the URI path to match + * * @return the mapping result, or null if no match is found */ public WsMappingResult findMapping(String path) { @@ -409,6 +416,7 @@ public class WsServerContainer extends WsWebSocketContainer implements ServerCon /** * Returns the write timeout handler. + * * @return the write timeout handler */ protected WsWriteTimeout getTimeout() { @@ -425,14 +433,10 @@ public class WsServerContainer extends WsWebSocketContainer implements ServerCon } - /** - * {@inheritDoc} Overridden to make it visible to other classes in this package. - */ - @Override - protected void registerSession(Object key, WsSession wsSession) { + protected void registerSession(Object key, WsSession wsSession, Object httpSession) { super.registerSession(key, wsSession); - if (wsSession.isOpen() && wsSession.getUserPrincipal() != null && wsSession.getHttpSessionId() != null) { - registerAuthenticatedSession(wsSession, wsSession.getHttpSessionId()); + if (wsSession.isOpen() && wsSession.getUserPrincipal() != null && httpSession != null) { + registerAuthenticatedSession(wsSession, (HttpSession) httpSession); } } @@ -442,53 +446,106 @@ public class WsServerContainer extends WsWebSocketContainer implements ServerCon */ @Override protected void unregisterSession(Object key, WsSession wsSession) { - if (wsSession.getUserPrincipalInternal() != null && wsSession.getHttpSessionId() != null) { - unregisterAuthenticatedSession(wsSession, wsSession.getHttpSessionId()); + if (wsSession.getUserPrincipalInternal() != null) { + unregisterAuthenticatedSession(wsSession); } super.unregisterSession(key, wsSession); } - private void registerAuthenticatedSession(WsSession wsSession, String httpSessionId) { - Set<WsSession> wsSessions = authenticatedSessions.get(httpSessionId); - if (wsSessions == null) { - wsSessions = ConcurrentHashMap.newKeySet(); - authenticatedSessions.putIfAbsent(httpSessionId, wsSessions); - wsSessions = authenticatedSessions.get(httpSessionId); + private void registerAuthenticatedSession(WsSession wsSession, HttpSession httpSession) { + boolean mustCloseWsSession = false; + String httpSessionKey = null; + + synchronized (authenticatedSessionMapLock) { + try { + boolean mustAddSessionAttribute = false; + WsHttpSessionBindingListener listener = (WsHttpSessionBindingListener) httpSession + .getAttribute(WsHttpSessionBindingListener.class.getCanonicalName()); + if (listener == null) { + httpSessionKey = httpSession.getId(); + mustAddSessionAttribute = true; + } else { + httpSessionKey = listener.getKey(); + } + if (mustAddSessionAttribute) { + /* + * It is possible that the session expires concurrently with the attribute being added. Depending on + * the exact timing, one of two things will happen. Either an IllegalStateException will be thrown + * or the attribute will be added and then immediately removed from the session. Handle both of + * these scenarios here. + */ + httpSession.setAttribute(WsHttpSessionBindingListener.class.getCanonicalName(), + new WsHttpSessionBindingListener(httpSessionKey)); + if (httpSession.getAttribute(WsHttpSessionBindingListener.class.getCanonicalName()) == null) { + mustCloseWsSession = true; + } + } + } catch (IllegalStateException ise) { + // Failing to set the attribute indicates that the session has already expired + mustCloseWsSession = true; + } + + if (!mustCloseWsSession) { + Set<WsSession> wsSessions = httpSessionKeyToWebSocketSession.get(httpSessionKey); + if (wsSessions == null) { + wsSessions = new HashSet<>(); + httpSessionKeyToWebSocketSession.put(httpSessionKey, wsSessions); + } + wsSessions.add(wsSession); + webSocketSessionToHttpSessionKey.put(wsSession, httpSessionKey); + } + } + + if (mustCloseWsSession) { + closeAuthenticatedWebSocketSession(wsSession); } - wsSessions.add(wsSession); } - private void unregisterAuthenticatedSession(WsSession wsSession, String httpSessionId) { - Set<WsSession> wsSessions = authenticatedSessions.get(httpSessionId); - // wsSessions will be null if the HTTP session has ended - if (wsSessions != null) { - wsSessions.remove(wsSession); + private void unregisterAuthenticatedSession(WsSession wsSession) { + synchronized (authenticatedSessionMapLock) { + String httpSessionKey = webSocketSessionToHttpSessionKey.remove(wsSession); + if (httpSessionKey != null) { + Set<WsSession> wsSessions = httpSessionKeyToWebSocketSession.get(httpSessionKey); + if (wsSessions != null) { + wsSessions.remove(wsSession); + } + } } } /** * Closes all WebSocket sessions associated with the given authenticated HTTP session. - * @param httpSessionId the HTTP session ID + * + * @param httpSessionKey the HTTP session key */ - public void closeAuthenticatedSession(String httpSessionId) { - Set<WsSession> wsSessions = authenticatedSessions.remove(httpSessionId); + public void handleHttpSessionKeyUnbound(String httpSessionKey) { + Set<WsSession> wsSessions; + + synchronized (authenticatedSessionMapLock) { + wsSessions = httpSessionKeyToWebSocketSession.remove(httpSessionKey); + } - if (wsSessions != null && !wsSessions.isEmpty()) { + if (wsSessions != null) { for (WsSession wsSession : wsSessions) { - try { - wsSession.close(AUTHENTICATED_HTTP_SESSION_CLOSED); - } catch (IOException ignore) { - // Any IOExceptions during close will have been caught and the - // onError method called. - } + closeAuthenticatedWebSocketSession(wsSession); } } } + private void closeAuthenticatedWebSocketSession(WsSession wsSession) { + try { + wsSession.close(AUTHENTICATED_HTTP_SESSION_CLOSED); + } catch (IOException ignore) { + // Any IOExceptions during close will have been caught and the + // onError method called. + } + } + + private static void validateEncoders(Class<? extends Encoder>[] encoders, InstanceManager instanceManager) throws DeploymentException { diff --git a/java/org/apache/tomcat/websocket/server/WsSessionListener.java b/java/org/apache/tomcat/websocket/server/WsSessionListener.java deleted file mode 100644 index afa55cf183..0000000000 --- a/java/org/apache/tomcat/websocket/server/WsSessionListener.java +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.tomcat.websocket.server; - -import javax.servlet.http.HttpSessionEvent; -import javax.servlet.http.HttpSessionListener; - -public class WsSessionListener implements HttpSessionListener { - - private final WsServerContainer wsServerContainer; - - - public WsSessionListener(WsServerContainer wsServerContainer) { - this.wsServerContainer = wsServerContainer; - } - - - @Override - public void sessionDestroyed(HttpSessionEvent se) { - wsServerContainer.closeAuthenticatedSession(se.getSession().getId()); - } -} diff --git a/test/org/apache/tomcat/websocket/TestWebSocketFrameClient.java b/test/org/apache/tomcat/websocket/TestWebSocketFrameClient.java index 50282f9dbf..663944104d 100644 --- a/test/org/apache/tomcat/websocket/TestWebSocketFrameClient.java +++ b/test/org/apache/tomcat/websocket/TestWebSocketFrameClient.java @@ -16,17 +16,24 @@ */ package org.apache.tomcat.websocket; +import java.io.IOException; import java.net.URI; import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Queue; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; +import javax.servlet.http.HttpServlet; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import javax.websocket.ClientEndpointConfig; import javax.websocket.ClientEndpointConfig.Configurator; import javax.websocket.ContainerProvider; +import javax.websocket.HandshakeResponse; import javax.websocket.Session; import javax.websocket.WebSocketContainer; @@ -37,10 +44,12 @@ import org.apache.catalina.Context; import org.apache.catalina.authenticator.AuthenticatorBase; import org.apache.catalina.servlets.DefaultServlet; import org.apache.catalina.startup.Tomcat; +import org.apache.tomcat.util.buf.ByteChunk; import org.apache.tomcat.util.descriptor.web.LoginConfig; import org.apache.tomcat.util.descriptor.web.SecurityCollection; import org.apache.tomcat.util.descriptor.web.SecurityConstraint; import org.apache.tomcat.websocket.TesterMessageCountClient.BasicText; +import org.apache.tomcat.websocket.TesterMessageCountClient.TesterEndpoint; import org.apache.tomcat.websocket.TesterMessageCountClient.TesterProgrammaticEndpoint; public class TestWebSocketFrameClient extends WebSocketBaseTest { @@ -64,10 +73,10 @@ public class TestWebSocketFrameClient extends WebSocketBaseTest { WebSocketContainer wsContainer = ContainerProvider.getWebSocketContainer(); // BZ 62596 - ClientEndpointConfig clientEndpointConfig = ClientEndpointConfig.Builder.create() - .configurator(new Configurator() { + ClientEndpointConfig clientEndpointConfig = + ClientEndpointConfig.Builder.create().configurator(new Configurator() { @Override - public void beforeRequest(Map<String, List<String>> headers) { + public void beforeRequest(Map<String,List<String>> headers) { headers.put("Dummy", Collections.singletonList(String.join("", Collections.nCopies(4000, "A")))); super.beforeRequest(headers); @@ -178,6 +187,133 @@ public class TestWebSocketFrameClient extends WebSocketBaseTest { echoTester(URI_PROTECTED, clientEndpointConfig); } + @Test + public void testAuthenticatedWebSocketClosedWhenHttpSessionEndsWithoutRotatedSession() throws Exception { + doTestAuthenticatedWebSocketClosedWhenHttpSessionEnds(false); + } + + + @Test + public void testAuthenticatedWebSocketClosedWhenHttpSessionEndsWithRotatedSession() throws Exception { + doTestAuthenticatedWebSocketClosedWhenHttpSessionEnds(true); + } + + + private void doTestAuthenticatedWebSocketClosedWhenHttpSessionEnds(boolean rotateSessionID) throws Exception { + + Tomcat tomcat = getTomcatInstance(); + Context ctx = tomcat.addContext(URI_PROTECTED, null); + ctx.addApplicationListener(TesterEchoServer.Config.class.getName()); + Tomcat.addServlet(ctx, "default", new DefaultServlet()); + ctx.addServletMapping("/", "default"); + Tomcat.addServlet(ctx, "invalidate", new HttpServlet() { + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + req.getSession(false).invalidate(); + } + }); + ctx.addServletMapping("/invalidate", "invalidate"); + Tomcat.addServlet(ctx, "changeSessionID", new HttpServlet() { + + @Override + protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { + req.changeSessionId(); + } + }); + ctx.addServletMapping("/changeSessionID", "changeSessionID"); + + SecurityCollection collection = new SecurityCollection(); + collection.addPattern("/*"); + + tomcat.addUser(USER, PWD); + tomcat.addRole(USER, ROLE); + + SecurityConstraint sc = new SecurityConstraint(); + sc.addAuthRole(ROLE); + sc.addCollection(collection); + ctx.addConstraint(sc); + + LoginConfig lc = new LoginConfig(); + lc.setAuthMethod("BASIC"); + ctx.setLoginConfig(lc); + + AuthenticatorBase basicAuthenticator = new org.apache.catalina.authenticator.BasicAuthenticator(); + basicAuthenticator.setAlwaysUseSession(true); + ctx.getPipeline().addValve(basicAuthenticator); + + tomcat.start(); + + AtomicReference<String> sessionCookie = new AtomicReference<>(); + ClientEndpointConfig clientEndpointConfig = + ClientEndpointConfig.Builder.create().configurator(new Configurator() { + + @Override + public void afterResponse(HandshakeResponse hr) { + List<String> cookies = hr.getHeaders().get("Set-Cookie"); + if (cookies != null) { + for (String cookie : cookies) { + if (cookie.startsWith("JSESSIONID=")) { + sessionCookie.set(cookie.split(";", 2)[0]); + break; + } + } + } + } + }).build(); + clientEndpointConfig.getUserProperties().put(Constants.WS_AUTHENTICATION_USER_NAME, USER); + clientEndpointConfig.getUserProperties().put(Constants.WS_AUTHENTICATION_PASSWORD, PWD); + + WebSocketContainer wsContainer = ContainerProvider.getWebSocketContainer(); + Session wsSession = wsContainer.connectToServer(TesterProgrammaticEndpoint.class, clientEndpointConfig, + new URI("ws://localhost:" + getPort() + URI_PROTECTED + TesterEchoServer.Config.PATH_BASIC)); + + CountDownLatch messageLatch = new CountDownLatch(1); + BasicText handler = new BasicText(messageLatch); + wsSession.addMessageHandler(handler); + wsSession.getBasicRemote().sendText("Hello"); + Assert.assertTrue(messageLatch.await(10, TimeUnit.SECONDS)); + Assert.assertEquals("Hello", handler.getMessages().poll()); + + if (rotateSessionID) { + Assert.assertNotNull(sessionCookie.get()); + Map<String,List<String>> requestHeaders = new HashMap<>(); + requestHeaders.put("Cookie", List.of(sessionCookie.get())); + Map<String,List<String>> responseHeaders = new HashMap<>(); + int status = getUrl("http://localhost:"; + getPort() + URI_PROTECTED + "/changeSessionID", new ByteChunk(), + requestHeaders, responseHeaders); + List<String> cookies = responseHeaders.get("Set-Cookie"); + Assert.assertNotNull(cookies); + Assert.assertEquals(1, cookies.size()); + sessionCookie.set(cookies.get(0).split(";", 2)[0]); + Assert.assertEquals(HttpServletResponse.SC_OK, status); + + // CyclicBarrier would be cleaner but that requires a larger refactoring + wsSession.removeMessageHandler(handler); + CountDownLatch messageLatch2 = new CountDownLatch(1); + BasicText handler2 = new BasicText(messageLatch2); + wsSession.addMessageHandler(handler2); + wsSession.getBasicRemote().sendText("Hello"); + Assert.assertTrue(messageLatch2.await(10, TimeUnit.SECONDS)); + Assert.assertEquals("Hello", handler2.getMessages().poll()); + } + + CountDownLatch closeLatch = new CountDownLatch(1); + TesterEndpoint endpoint = (TesterEndpoint) wsSession.getUserProperties().get("endpoint"); + endpoint.setLatch(closeLatch); + + Assert.assertNotNull(sessionCookie.get()); + Map<String,List<String>> requestHeaders = new HashMap<>(); + requestHeaders.put("Cookie", List.of(sessionCookie.get())); + int status = getUrl("http://localhost:"; + getPort() + URI_PROTECTED + "/invalidate", new ByteChunk(), + requestHeaders, null); + Assert.assertEquals(HttpServletResponse.SC_OK, status); + + Assert.assertTrue(closeLatch.await(10, TimeUnit.SECONDS)); + Assert.assertFalse(wsSession.isOpen()); + } + + @Test public void testConnectToDigestEndpoint() throws Exception { diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index e5c8694597..234e599abf 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -288,6 +288,11 @@ template ends in a variable without a trailing slash, that variable might be expanded to the empty string. (markt) </fix> + <fix> + Account for session ID changes when tracking WebSocket connections for + closure because they were created under an authenticated HTTP session + that has since ended. (markt) + </fix> </changelog> </subsection> <subsection name="Web applications"> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
