Author: markt Date: Tue Mar 19 19:49:51 2013 New Revision: 1458479 URL: http://svn.apache.org/r1458479 Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=54724 Session should throw IllegalStateException if closed when methods on it are called
Modified: tomcat/trunk/java/org/apache/tomcat/websocket/LocalStrings.properties tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java tomcat/trunk/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java Modified: tomcat/trunk/java/org/apache/tomcat/websocket/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/LocalStrings.properties?rev=1458479&r1=1458478&r2=1458479&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/websocket/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/tomcat/websocket/LocalStrings.properties Tue Mar 19 19:49:51 2013 @@ -49,6 +49,7 @@ wsRemoteEndpoint.noEncoder=No encoder sp # as many as 4 bytes. wsSession.timeout=The WebSocket session timeout expired +wsSession.closed=The WebSocket session has been closed and no method (apart from close()) may be called on a closed session wsSession.duplicateHandlerBinary=A binary message handler has already been configured wsSession.duplicateHandlerPong=A pong message handler has already been configured wsSession.duplicateHandlerText=A text message handler has already been configured Modified: tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java?rev=1458479&r1=1458478&r2=1458479&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java (original) +++ tomcat/trunk/java/org/apache/tomcat/websocket/WsSession.java Tue Mar 19 19:49:51 2013 @@ -138,6 +138,7 @@ public class WsSession implements Sessio @Override public WebSocketContainer getContainer() { + checkState(); return webSocketContainer; } @@ -145,6 +146,9 @@ public class WsSession implements Sessio @SuppressWarnings("unchecked") @Override public void addMessageHandler(MessageHandler listener) { + + checkState(); + Type t = Util.getMessageType(listener); if (t.equals(String.class)) { @@ -180,6 +184,7 @@ public class WsSession implements Sessio @Override public Set<MessageHandler> getMessageHandlers() { + checkState(); Set<MessageHandler> result = new HashSet<>(); if (binaryMessageHandler != null) { result.add(binaryMessageHandler); @@ -196,6 +201,7 @@ public class WsSession implements Sessio @Override public void removeMessageHandler(MessageHandler listener) { + checkState(); if (listener == null) { return; } @@ -216,24 +222,28 @@ public class WsSession implements Sessio @Override public String getProtocolVersion() { + checkState(); return Constants.WS_VERSION_HEADER_VALUE; } @Override public String getNegotiatedSubprotocol() { + checkState(); return subProtocol; } @Override public List<Extension> getNegotiatedExtensions() { + checkState(); return Collections.EMPTY_LIST; } @Override public boolean isSecure() { + checkState(); return secure; } @@ -246,54 +256,63 @@ public class WsSession implements Sessio @Override public long getMaxIdleTimeout() { + checkState(); return maxIdleTimeout; } @Override public void setMaxIdleTimeout(long timeout) { + checkState(); this.maxIdleTimeout = timeout; } @Override public void setMaxBinaryMessageBufferSize(int max) { + checkState(); this.maxBinaryMessageBufferSize = max; } @Override public int getMaxBinaryMessageBufferSize() { + checkState(); return maxBinaryMessageBufferSize; } @Override public void setMaxTextMessageBufferSize(int max) { + checkState(); this.maxTextMessageBufferSize = max; } @Override public int getMaxTextMessageBufferSize() { + checkState(); return maxTextMessageBufferSize; } @Override public Set<Session> getOpenSessions() { + checkState(); return webSocketContainer.getOpenSessions(localEndpoint.getClass()); } @Override public RemoteEndpoint.Async getAsyncRemote() { + checkState(); return remoteEndpointAsync; } @Override public RemoteEndpoint.Basic getBasicRemote() { + checkState(); return remoteEndpointBasic; } @@ -314,6 +333,9 @@ public class WsSession implements Sessio if (state != State.OPEN) { return; } + + fireEndpointOnClose(closeReason); + state = State.CLOSING; sendCloseMessage(closeReason); @@ -331,6 +353,7 @@ public class WsSession implements Sessio synchronized (stateLock) { if (state == State.OPEN) { sendCloseMessage = true; + fireEndpointOnClose(closeReason); } state = State.CLOSED; @@ -345,6 +368,19 @@ public class WsSession implements Sessio } + private void fireEndpointOnClose(CloseReason closeReason) { + + // Fire the onClose event + Thread t = Thread.currentThread(); + ClassLoader cl = t.getContextClassLoader(); + t.setContextClassLoader(applicationClassLoader); + try { + localEndpoint.onClose(this, closeReason); + } finally { + t.setContextClassLoader(cl); + } + } + private void sendCloseMessage(CloseReason closeReason) { // 125 is maximum size for the payload of a control message ByteBuffer msg = ByteBuffer.allocate(125); @@ -366,16 +402,6 @@ public class WsSession implements Sessio } finally { webSocketContainer.unregisterSession( localEndpoint.getClass(), this); - - // Fire the onClose event - Thread t = Thread.currentThread(); - ClassLoader cl = t.getContextClassLoader(); - t.setContextClassLoader(applicationClassLoader); - try { - localEndpoint.onClose(this, closeReason); - } finally { - t.setContextClassLoader(cl); - } } } @@ -383,30 +409,35 @@ public class WsSession implements Sessio @Override public URI getRequestURI() { + checkState(); return requestUri; } @Override public Map<String,List<String>> getRequestParameterMap() { + checkState(); return requestParameterMap; } @Override public String getQueryString() { + checkState(); return queryString; } @Override public Principal getUserPrincipal() { + checkState(); return userPrincipal; } @Override public Map<String,String> getPathParameters() { + checkState(); return pathParameters; } @@ -419,6 +450,7 @@ public class WsSession implements Sessio @Override public Map<String,Object> getUserProperties() { + checkState(); return userProperties; } @@ -465,6 +497,12 @@ public class WsSession implements Sessio } + private void checkState() { + if (!isOpen()) { + throw new IllegalStateException(sm.getString("wsSession.closed")); + } + } + private static enum State { OPEN, CLOSING, Modified: tomcat/trunk/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java?rev=1458479&r1=1458478&r2=1458479&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java (original) +++ tomcat/trunk/test/org/apache/tomcat/websocket/TestWsWebSocketContainer.java Tue Mar 19 19:49:51 2013 @@ -605,15 +605,26 @@ public class TestWsWebSocketContainer ex Assert.assertEquals(3, setA.size()); int count = 0; - setA = s3a.getOpenSessions(); - while (setA.size() > 0 && count < 8) { + boolean isOpen = true; + while (isOpen && count < 8) { count ++; Thread.sleep(1000); - setA = s3a.getOpenSessions(); + isOpen = false; + for (Session session : setA) { + if (session.isOpen()) { + isOpen = true; + } + } } - if (setA.size() > 0) { - Assert.fail("There were [" + setA.size() + "] open sessions"); + if (isOpen) { + for (Session session : setA) { + if (session.isOpen()) { + System.err.println("Session with ID [" + session.getId() + + "] is open"); + } + } + Assert.fail("There were open sessions"); } } @@ -649,22 +660,31 @@ public class TestWsWebSocketContainer ex int expected = 3; while (expected > 0) { - Assert.assertEquals(expected, setA.size()); + Assert.assertEquals(expected, getOpenCount(setA)); int count = 0; - while (setA.size() == expected && count < 5) { + while (getOpenCount(setA) == expected && count < 5) { count ++; Thread.sleep(1000); - setA = s3a.getOpenSessions(); } expected--; } - Assert.assertEquals(0, setA.size()); + Assert.assertEquals(0, getOpenCount(setA)); } + private int getOpenCount(Set<Session> sessions) { + int result = 0; + for (Session session : sessions) { + if (session.isOpen()) { + result++; + } + } + return result; + } + private Session connectToEchoServerBasic(WebSocketContainer wsContainer, Class<? extends Endpoint> clazz) throws Exception { return wsContainer.connectToServer(clazz, --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org