This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push: new ee51a0d Manually apply PR #346 - fix BZ64644 add write/read idle session timeout ee51a0d is described below commit ee51a0dbfa9fd533b50396417b8c4734c8719ae1 Author: Mark Thomas <ma...@apache.org> AuthorDate: Tue Sep 1 13:53:31 2020 +0100 Manually apply PR #346 - fix BZ64644 add write/read idle session timeout https://bz.apache.org/bugzilla/show_bug.cgi?id=64644 --- java/org/apache/tomcat/websocket/Constants.java | 6 ++ .../tomcat/websocket/LocalStrings.properties | 6 +- java/org/apache/tomcat/websocket/WsFrameBase.java | 2 +- .../tomcat/websocket/WsRemoteEndpointImplBase.java | 6 +- java/org/apache/tomcat/websocket/WsSession.java | 53 +++++++++++--- .../tomcat/websocket/TestWsWebSocketContainer.java | 85 +++++++++++++++++++++- webapps/docs/changelog.xml | 6 ++ webapps/docs/web-socket-howto.xml | 13 ++++ 8 files changed, 159 insertions(+), 18 deletions(-) diff --git a/java/org/apache/tomcat/websocket/Constants.java b/java/org/apache/tomcat/websocket/Constants.java index fa01921..09dbc73 100644 --- a/java/org/apache/tomcat/websocket/Constants.java +++ b/java/org/apache/tomcat/websocket/Constants.java @@ -114,6 +114,12 @@ public class Constants { // Milliseconds so this is 20 seconds public static final long DEFAULT_BLOCKING_SEND_TIMEOUT = 20 * 1000; + // Configuration for read idle timeout on WebSocket session + public static final String READ_IDLE_TIMEOUT_MS = "org.apache.tomcat.websocket.READ_IDLE_TIMEOUT_MS"; + + // Configuration for write idle timeout on WebSocket session + public static final String WRITE_IDLE_TIMEOUT_MS = "org.apache.tomcat.websocket.WRITE_IDLE_TIMEOUT_MS"; + // Configuration for background processing checks intervals static final int DEFAULT_PROCESS_PERIOD = Integer.getInteger( "org.apache.tomcat.websocket.DEFAULT_PROCESS_PERIOD", 10) diff --git a/java/org/apache/tomcat/websocket/LocalStrings.properties b/java/org/apache/tomcat/websocket/LocalStrings.properties index 8c3c505..22e9f17 100644 --- a/java/org/apache/tomcat/websocket/LocalStrings.properties +++ b/java/org/apache/tomcat/websocket/LocalStrings.properties @@ -98,11 +98,13 @@ wsRemoteEndpoint.tooMuchData=Ping or pong may not send more than 125 bytes wsRemoteEndpoint.writeTimeout=Blocking write timeout wsRemoteEndpoint.wrongState=The remote endpoint was in state [{0}] which is an invalid state for called method -# Note the following message is used as a close reason in a WebSocket control +# Note the following messages are used as a close reason in a WebSocket control # frame and therefore must be 123 bytes (not characters) or less in length. # Messages are encoded using UTF-8 where a single character may be encoded in # as many as 4 bytes. -wsSession.timeout=The WebSocket session [{0}] timeout expired +wsSession.timeout=The WebSocket session [{0}] idle timeout expired +wsSession.timeoutRead=The WebSocket session [{0}] read idle timeout expired +wsSession.timeoutWrite=The WebSocket session [{0}] write idle timeout expired wsSession.closed=The WebSocket session [{0}] has been closed and no method (apart from close()) may be called on a closed session wsSession.created=Created WebSocket session [{0}] diff --git a/java/org/apache/tomcat/websocket/WsFrameBase.java b/java/org/apache/tomcat/websocket/WsFrameBase.java index a6df700..3993c6a 100644 --- a/java/org/apache/tomcat/websocket/WsFrameBase.java +++ b/java/org/apache/tomcat/websocket/WsFrameBase.java @@ -113,7 +113,7 @@ public abstract class WsFrameBase { protected void processInputBuffer() throws IOException { while (!isSuspended()) { - wsSession.updateLastActive(); + wsSession.updateLastActiveRead(); if (state == State.NEW_FRAME) { if (!processInitialHeader()) { break; diff --git a/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java b/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java index c87f99e..103f80d 100644 --- a/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java +++ b/java/org/apache/tomcat/websocket/WsRemoteEndpointImplBase.java @@ -275,7 +275,7 @@ public abstract class WsRemoteEndpointImplBase implements RemoteEndpoint { private void sendMessageBlock(byte opCode, ByteBuffer payload, boolean last, long timeoutExpiry) throws IOException { - wsSession.updateLastActive(); + wsSession.updateLastActiveWrite(); BlockingSendHandler bsh = new BlockingSendHandler(); @@ -340,7 +340,7 @@ public abstract class WsRemoteEndpointImplBase implements RemoteEndpoint { void startMessage(byte opCode, ByteBuffer payload, boolean last, SendHandler handler) { - wsSession.updateLastActive(); + wsSession.updateLastActiveWrite(); List<MessagePart> messageParts = new ArrayList<>(); messageParts.add(new MessagePart(last, 0, opCode, payload, @@ -423,7 +423,7 @@ public abstract class WsRemoteEndpointImplBase implements RemoteEndpoint { writeMessagePart(mpNext); } - wsSession.updateLastActive(); + wsSession.updateLastActiveWrite(); // Some handlers, such as the IntermediateMessageHandler, do not have a // nested handler so handler may be null. diff --git a/java/org/apache/tomcat/websocket/WsSession.java b/java/org/apache/tomcat/websocket/WsSession.java index 37bff32..1f85100 100644 --- a/java/org/apache/tomcat/websocket/WsSession.java +++ b/java/org/apache/tomcat/websocket/WsSession.java @@ -97,7 +97,8 @@ public class WsSession implements Session { private volatile int maxBinaryMessageBufferSize = Constants.DEFAULT_BUFFER_SIZE; private volatile int maxTextMessageBufferSize = Constants.DEFAULT_BUFFER_SIZE; private volatile long maxIdleTimeout = 0; - private volatile long lastActive = System.currentTimeMillis(); + private volatile long lastActiveRead = System.currentTimeMillis(); + private volatile long lastActiveWrite = System.currentTimeMillis(); private Map<FutureToSendHandler, FutureToSendHandler> futures = new ConcurrentHashMap<>(); /** @@ -805,25 +806,59 @@ public class WsSession implements Session { } - protected void updateLastActive() { - lastActive = System.currentTimeMillis(); + protected void updateLastActiveRead() { + lastActiveRead = System.currentTimeMillis(); + } + + + protected void updateLastActiveWrite() { + lastActiveWrite = System.currentTimeMillis(); } protected void checkExpiration() { + // Local copies to ensure consistent behaviour during method execution long timeout = maxIdleTimeout; - if (timeout < 1) { - return; + long timeoutRead = getMaxIdleTimeoutRead(); + long timeoutWrite = getMaxIdleTimeoutWrite(); + + long currentTime = System.currentTimeMillis(); + String key = null; + + if (timeoutRead > 0 && (currentTime - lastActiveRead) > timeoutRead) { + key = "wsSession.timeoutRead"; + } else if (timeoutWrite > 0 && (currentTime - lastActiveWrite) > timeoutRead) { + key = "wsSession.timeoutWrite"; + } else if (timeout > 0 && (currentTime - lastActiveRead) > timeout && + (currentTime - lastActiveWrite) > timeout) { + key = "wsSession.timeout"; } - if (System.currentTimeMillis() - lastActive > timeout) { - String msg = sm.getString("wsSession.timeout", getId()); + if (key != null) { + String msg = sm.getString(key, getId()); if (log.isDebugEnabled()) { log.debug(msg); } - doClose(new CloseReason(CloseCodes.GOING_AWAY, msg), - new CloseReason(CloseCodes.CLOSED_ABNORMALLY, msg)); + doClose(new CloseReason(CloseCodes.GOING_AWAY, msg), new CloseReason(CloseCodes.CLOSED_ABNORMALLY, msg)); + } + } + + + private long getMaxIdleTimeoutRead() { + Object timeout = userProperties.get(Constants.READ_IDLE_TIMEOUT_MS); + if (timeout instanceof Long) { + return ((Long) timeout).longValue(); + } + return 0; + } + + + private long getMaxIdleTimeoutWrite() { + Object timeout = userProperties.get(Constants.WRITE_IDLE_TIMEOUT_MS); + if (timeout instanceof Long) { + return ((Long) timeout).longValue(); } + return 0; } diff --git a/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java b/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java index efa4c39..7fa452a 100644 --- a/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java +++ b/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java @@ -62,7 +62,6 @@ import org.apache.tomcat.websocket.TesterMessageCountClient.BasicHandler; import org.apache.tomcat.websocket.TesterMessageCountClient.BasicText; import org.apache.tomcat.websocket.TesterMessageCountClient.TesterEndpoint; import org.apache.tomcat.websocket.TesterMessageCountClient.TesterProgrammaticEndpoint; -import org.apache.tomcat.websocket.server.Constants; import org.apache.tomcat.websocket.server.WsContextListener; public class TestWsWebSocketContainer extends WebSocketBaseTest { @@ -466,7 +465,7 @@ public class TestWsWebSocketContainer extends WebSocketBaseTest { super.contextInitialized(sce); ServerContainer sc = (ServerContainer) sce.getServletContext().getAttribute( - Constants.SERVER_CONTAINER_SERVLET_CONTEXT_ATTRIBUTE); + org.apache.tomcat.websocket.server.Constants.SERVER_CONTAINER_SERVLET_CONTEXT_ATTRIBUTE); try { // Reset blocking state BlockingPojo.resetBlock(); @@ -607,7 +606,7 @@ public class TestWsWebSocketContainer extends WebSocketBaseTest { super.contextInitialized(sce); ServerContainer sc = (ServerContainer) sce.getServletContext().getAttribute( - Constants.SERVER_CONTAINER_SERVLET_CONTEXT_ATTRIBUTE); + org.apache.tomcat.websocket.server.Constants.SERVER_CONTAINER_SERVLET_CONTEXT_ATTRIBUTE); try { sc.addEndpoint(ServerEndpointConfig.Builder.create( ConstantTxEndpoint.class, PATH).build()); @@ -786,6 +785,86 @@ public class TestWsWebSocketContainer extends WebSocketBaseTest { } + @Test + public void testSessionExpiryOnUserPropertyReadIdleTimeout() throws Exception { + Tomcat tomcat = getTomcatInstance(); + // No file system docBase required + Context ctx = tomcat.addContext("", null); + ctx.addApplicationListener(TesterEchoServer.Config.class.getName()); + Tomcat.addServlet(ctx, "default", new DefaultServlet()); + ctx.addServletMappingDecoded("/", "default"); + + tomcat.start(); + + // Need access to implementation methods for configuring unit tests + WsWebSocketContainer wsContainer = (WsWebSocketContainer) ContainerProvider.getWebSocketContainer(); + + wsContainer.setDefaultMaxSessionIdleTimeout(90000); + wsContainer.setProcessPeriod(1); + + EndpointA endpointA = new EndpointA(); + Session s1a = connectToEchoServer(wsContainer, endpointA, TesterEchoServer.Config.PATH_BASIC); + s1a.setMaxIdleTimeout(90000); + + s1a.getUserProperties().put(Constants.READ_IDLE_TIMEOUT_MS, Long.valueOf(5000)); + + // maxIdleTimeout is 90s but the readIdleTimeout is 5s. The session + // should get closed after 5 seconds as nothing is read on it. + + // First confirm the session has been opened. + Assert.assertEquals(1, s1a.getOpenSessions().size()); + + // Now wait for it to close. Allow up to 30s as some CI systems are slow + // but that is still well under the 90s configured for the session. + int count = 0; + while (count < 300 && s1a.isOpen()) { + count ++; + Thread.sleep(100); + } + Assert.assertFalse(s1a.isOpen()); + } + + + @Test + public void testSessionExpiryOnUserPropertyWriteIdleTimeout() throws Exception { + Tomcat tomcat = getTomcatInstance(); + // No file system docBase required + Context ctx = tomcat.addContext("", null); + ctx.addApplicationListener(TesterEchoServer.Config.class.getName()); + Tomcat.addServlet(ctx, "default", new DefaultServlet()); + ctx.addServletMappingDecoded("/", "default"); + + tomcat.start(); + + // Need access to implementation methods for configuring unit tests + WsWebSocketContainer wsContainer = (WsWebSocketContainer) ContainerProvider.getWebSocketContainer(); + + wsContainer.setDefaultMaxSessionIdleTimeout(90000); + wsContainer.setProcessPeriod(1); + + EndpointA endpointA = new EndpointA(); + Session s1a = connectToEchoServer(wsContainer, endpointA, TesterEchoServer.Config.PATH_BASIC); + s1a.setMaxIdleTimeout(90000); + + s1a.getUserProperties().put(Constants.WRITE_IDLE_TIMEOUT_MS, Long.valueOf(5000)); + + // maxIdleTimeout is 90s but the writeIdleTimeout is 5s. The session + // should get closed after 5 seconds as nothing is written on it. + + // First confirm the session has been opened. + Assert.assertEquals(1, s1a.getOpenSessions().size()); + + // Now wait for it to close. Allow up to 30s as some CI systems are slow + // but that is still well under the 90s configured for the session. + int count = 0; + while (count < 300 && s1a.isOpen()) { + count ++; + Thread.sleep(100); + } + Assert.assertFalse(s1a.isOpen()); + } + + private int getOpenCount(Set<Session> sessions) { int result = 0; for (Session session : sessions) { diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 2108bf2..7211544 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -152,6 +152,12 @@ Fix a potential issue where the write lock for a WebSocket connection may not be released if an exception occurs during the write. (markt) </fix> + <add> + <bug>64644</bug>: Add support for a read idle timeout and a write idle + timeout to the WebSocket session via custom properties in the user + properties instance associated with the session. Based on a pull request + by sakshamverma. (markt) + </add> </changelog> </subsection> <subsection name="Web applications"> diff --git a/webapps/docs/web-socket-howto.xml b/webapps/docs/web-socket-howto.xml index 143c699..6bbd56c 100644 --- a/webapps/docs/web-socket-howto.xml +++ b/webapps/docs/web-socket-howto.xml @@ -63,6 +63,19 @@ the timeout to use in milliseconds. For an infinite timeout, use <code>-1</code>.</p> +<p>In addition to the <code>Session.setMaxIdleTimeout(long)</code> method which + is part of the Java WebSocket API, Tomcat provides greater control of the + timing out the session due to lack of activity. Setting the property + <code>org.apache.tomcat.websocket.READ_IDLE_TIMEOUT_MS</code> in the user + properties collection attached to the WebSocket session will trigger a + session timeout if no WebSocket message is received for the specified number + of milliseconds. Setting the property + <code>org.apache.tomcat.websocket.WRITE_IDLE_TIMEOUT_MS</code> will trigger a + session timeout if no WebSocket message is sent for the specified number of + milliseconds. These can be used separately or together, with or wihout + <code>Session.setMaxIdleTimeout(long)</code>. If the associated property is + not specified, the read and/or write idle timeout will be applied.</p> + <p>If the application does not define a <code>MessageHandler.Partial</code> for incoming binary messages, any incoming binary messages must be buffered so the entire message can be delivered in a single call to the registered --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org