diff --git a/spring-websocket/src/main/java/org/springframework/sockjs/AbstractSockJsSession.java b/spring-websocket/src/main/java/org/springframework/sockjs/AbstractSockJsSession.java index 3912dcb268b..30514835ad1 100644 --- a/spring-websocket/src/main/java/org/springframework/sockjs/AbstractSockJsSession.java +++ b/spring-websocket/src/main/java/org/springframework/sockjs/AbstractSockJsSession.java @@ -118,7 +118,7 @@ public abstract class AbstractSockJsSession implements WebSocketSession { this.timeLastActive = System.currentTimeMillis(); } - public void delegateConnectionEstablished() { + public void delegateConnectionEstablished() throws Exception { this.state = State.OPEN; this.handler.afterConnectionEstablished(this); } @@ -127,23 +127,28 @@ public abstract class AbstractSockJsSession implements WebSocketSession { * Close due to error arising from SockJS transport handling. */ protected void tryCloseWithSockJsTransportError(Throwable ex, CloseStatus closeStatus) { - delegateError(ex); + logger.error("Closing due to transport error for " + this, ex); try { - logger.error("Closing due to transport error for " + this, ex); - close(closeStatus); + delegateError(ex); } - catch (Throwable t) { - // ignore + catch (Throwable delegateEx) { + logger.error("Unhandled error for " + this, delegateEx); + try { + close(closeStatus); + } + catch (Throwable closeEx) { + logger.error("Unhandled error for " + this, closeEx); + } } } - public void delegateMessages(String[] messages) { + public void delegateMessages(String[] messages) throws Exception { for (String message : messages) { this.handler.handleMessage(this, new TextMessage(message)); } } - public void delegateError(Throwable ex) { + public void delegateError(Throwable ex) throws Exception { this.handler.handleTransportError(this, ex); } @@ -153,7 +158,7 @@ public abstract class AbstractSockJsSession implements WebSocketSession { * {@link TextMessageHandler}. This is in contrast to {@link #close()} that pro-actively * closes the connection. */ - public final void delegateConnectionClosed(CloseStatus status) { + public final void delegateConnectionClosed(CloseStatus status) throws Exception { if (!isClosed()) { if (logger.isDebugEnabled()) { logger.debug(this + " was closed, " + status); @@ -193,7 +198,12 @@ public abstract class AbstractSockJsSession implements WebSocketSession { } finally { this.state = State.CLOSED; - this.handler.afterConnectionClosed(this, status); + try { + this.handler.afterConnectionClosed(this, status); + } + catch (Throwable t) { + logger.error("Unhandled error for " + this, t); + } } } } diff --git a/spring-websocket/src/main/java/org/springframework/sockjs/server/AbstractServerSockJsSession.java b/spring-websocket/src/main/java/org/springframework/sockjs/server/AbstractServerSockJsSession.java index fdd9f153305..554d813a551 100644 --- a/spring-websocket/src/main/java/org/springframework/sockjs/server/AbstractServerSockJsSession.java +++ b/spring-websocket/src/main/java/org/springframework/sockjs/server/AbstractServerSockJsSession.java @@ -111,7 +111,7 @@ public abstract class AbstractServerSockJsSession extends AbstractSockJsSession catch (Throwable ex) { logger.warn("Terminating connection due to failure to send message: " + ex.getMessage()); close(); - throw new NestedSockJsRuntimeException("Failed to write " + frame, ex); + throw new SockJsRuntimeException("Failed to write " + frame, ex); } } diff --git a/spring-websocket/src/main/java/org/springframework/sockjs/server/NestedSockJsRuntimeException.java b/spring-websocket/src/main/java/org/springframework/sockjs/server/SockJsRuntimeException.java similarity index 81% rename from spring-websocket/src/main/java/org/springframework/sockjs/server/NestedSockJsRuntimeException.java rename to spring-websocket/src/main/java/org/springframework/sockjs/server/SockJsRuntimeException.java index 63f82da2575..e9026012bb0 100644 --- a/spring-websocket/src/main/java/org/springframework/sockjs/server/NestedSockJsRuntimeException.java +++ b/spring-websocket/src/main/java/org/springframework/sockjs/server/SockJsRuntimeException.java @@ -24,13 +24,13 @@ import org.springframework.core.NestedRuntimeException; * @since 4.0 */ @SuppressWarnings("serial") -public class NestedSockJsRuntimeException extends NestedRuntimeException { +public class SockJsRuntimeException extends NestedRuntimeException { - public NestedSockJsRuntimeException(String msg) { + public SockJsRuntimeException(String msg) { super(msg); } - public NestedSockJsRuntimeException(String msg, Throwable cause) { + public SockJsRuntimeException(String msg, Throwable cause) { super(msg, cause); } diff --git a/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpReceivingTransportHandler.java b/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpReceivingTransportHandler.java index 0f8d492eb1b..078c354eee9 100644 --- a/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpReceivingTransportHandler.java +++ b/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpReceivingTransportHandler.java @@ -27,9 +27,11 @@ import org.springframework.http.MediaType; import org.springframework.http.server.ServerHttpRequest; import org.springframework.http.server.ServerHttpResponse; import org.springframework.sockjs.AbstractSockJsSession; +import org.springframework.sockjs.server.SockJsRuntimeException; import org.springframework.sockjs.server.TransportErrorException; import org.springframework.sockjs.server.TransportHandler; import org.springframework.websocket.WebSocketHandler; +import org.springframework.websocket.support.ExceptionWebSocketHandlerDecorator; import com.fasterxml.jackson.databind.JsonMappingException; import com.fasterxml.jackson.databind.ObjectMapper; @@ -89,7 +91,13 @@ public abstract class AbstractHttpReceivingTransportHandler implements Transport logger.trace("Received messages: " + Arrays.asList(messages)); } - session.delegateMessages(messages); + try { + session.delegateMessages(messages); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(session, t, logger); + throw new SockJsRuntimeException("Unhandled WebSocketHandler error in " + this, t); + } response.setStatusCode(getResponseStatus()); response.getHeaders().setContentType(new MediaType("text", "plain", Charset.forName("UTF-8"))); diff --git a/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpServerSockJsSession.java b/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpServerSockJsSession.java index 055102b681e..856296ceb64 100644 --- a/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpServerSockJsSession.java +++ b/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/AbstractHttpServerSockJsSession.java @@ -31,6 +31,7 @@ import org.springframework.sockjs.server.TransportErrorException; import org.springframework.util.Assert; import org.springframework.websocket.CloseStatus; import org.springframework.websocket.WebSocketHandler; +import org.springframework.websocket.support.ExceptionWebSocketHandlerDecorator; /** * An abstract base class for use with HTTP-based transports. @@ -65,7 +66,12 @@ public abstract class AbstractHttpServerSockJsSession extends AbstractServerSock tryCloseWithSockJsTransportError(t, null); throw new TransportErrorException("Failed open SockJS session", t, getId()); } - delegateConnectionEstablished(); + try { + delegateConnectionEstablished(); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this, t, logger); + } } protected void writePrelude() throws IOException { diff --git a/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/SockJsWebSocketHandler.java b/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/SockJsWebSocketHandler.java index 13a0b9e12e4..e1978fe226b 100644 --- a/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/SockJsWebSocketHandler.java +++ b/spring-websocket/src/main/java/org/springframework/sockjs/server/transport/SockJsWebSocketHandler.java @@ -34,8 +34,16 @@ import com.fasterxml.jackson.databind.ObjectMapper; /** - * A wrapper around a {@link WebSocketHandler} instance that parses as well as adds SockJS - * messages frames as well as sends SockJS heartbeat messages. + * A wrapper around a {@link WebSocketHandler} instance that parses and adds SockJS + * messages frames and also sends SockJS heartbeat messages. + * + *

+ * Implementations of the {@link WebSocketHandler} interface in this class allow + * exceptions from the wrapped {@link WebSocketHandler} to propagate. However, any + * exceptions resulting from SockJS message handling (e.g. while sending SockJS frames or + * heartbeat messages) are caught and treated as transport errors, i.e. routed to the + * {@link WebSocketHandler#handleTransportError(WebSocketSession, Throwable) + * handleTransportError} method of the wrapped handler and the session closed. * * @author Rossen Stoyanchev * @since 4.0 @@ -66,24 +74,24 @@ public class SockJsWebSocketHandler extends TextWebSocketHandlerAdapter { } @Override - public void afterConnectionEstablished(WebSocketSession wsSession) { + public void afterConnectionEstablished(WebSocketSession wsSession) throws Exception { Assert.isTrue(this.sessionCount.compareAndSet(0, 1), "Unexpected connection"); this.sockJsSession = new WebSocketServerSockJsSession(getSockJsSessionId(wsSession), getSockJsConfig()); this.sockJsSession.initWebSocketSession(wsSession); } @Override - public void handleTextMessage(WebSocketSession wsSession, TextMessage message) { + public void handleTextMessage(WebSocketSession wsSession, TextMessage message) throws Exception { this.sockJsSession.handleMessage(message, wsSession); } @Override - public void afterConnectionClosed(WebSocketSession wsSession, CloseStatus status) { + public void afterConnectionClosed(WebSocketSession wsSession, CloseStatus status) throws Exception { this.sockJsSession.delegateConnectionClosed(status); } @Override - public void handleTransportError(WebSocketSession webSocketSession, Throwable exception) { + public void handleTransportError(WebSocketSession webSocketSession, Throwable exception) throws Exception { this.sockJsSession.delegateError(exception); } @@ -105,7 +113,7 @@ public class SockJsWebSocketHandler extends TextWebSocketHandlerAdapter { super(sessionId, config, SockJsWebSocketHandler.this.webSocketHandler); } - public void initWebSocketSession(WebSocketSession wsSession) { + public void initWebSocketSession(WebSocketSession wsSession) throws Exception { this.wsSession = wsSession; try { TextMessage message = new TextMessage(SockJsFrame.openFrame().getContent()); @@ -124,7 +132,7 @@ public class SockJsWebSocketHandler extends TextWebSocketHandlerAdapter { return this.wsSession.isOpen(); } - public void handleMessage(TextMessage message, WebSocketSession wsSession) { + public void handleMessage(TextMessage message, WebSocketSession wsSession) throws Exception { String payload = message.getPayload(); if (StringUtils.isEmpty(payload)) { logger.trace("Ignoring empty message"); diff --git a/spring-websocket/src/main/java/org/springframework/websocket/PartialMessageHandler.java b/spring-websocket/src/main/java/org/springframework/websocket/PartialMessageHandler.java deleted file mode 100644 index 30524be5fa4..00000000000 --- a/spring-websocket/src/main/java/org/springframework/websocket/PartialMessageHandler.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright 2002-2013 the original author or authors. - * - * Licensed 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.springframework.websocket; - -/** - * A "marker" interface for {@link BinaryMessageHandler} types that wish to handle partial - * messages. - * - * @author Rossen Stoyanchev - * @since 4.0 - */ -public interface PartialMessageHandler { - -} diff --git a/spring-websocket/src/main/java/org/springframework/websocket/WebSocketHandler.java b/spring-websocket/src/main/java/org/springframework/websocket/WebSocketHandler.java index 4220facf782..21dc2150973 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/WebSocketHandler.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/WebSocketHandler.java @@ -17,7 +17,15 @@ package org.springframework.websocket; /** - * A handler for WebSocket sessions. + * A handler for WebSocket messages and lifecycle events. + * + *

Implementations of this interface are encouraged to handle exceptions locally where + * it makes sense or alternatively let the exception bubble up in which case the exception + * is logged and the session closed with {@link CloseStatus#SERVER_ERROR SERVER_ERROR(101)} by default. + * The exception handling strategy is provided by + * {@link org.springframework.websocket.support.ExceptionWebSocketHandlerDecorator ExceptionWebSocketHandlerDecorator}, + * which can be customized or replaced by decorating the {@link WebSocketHandler} with a + * different decorator. * * @param The type of message being handled {@link TextMessage}, {@link BinaryMessage} * (or {@link WebSocketMessage} for both). @@ -29,24 +37,40 @@ package org.springframework.websocket; public interface WebSocketHandler { /** - * A new WebSocket connection has been opened and is ready to be used. + * Invoked after WebSocket negotiation has succeeded and the WebSocket connection is + * opened and ready for use. + * + * @throws Exception this method can handle or propagate exceptions; see class-level + * Javadoc for details. */ - void afterConnectionEstablished(WebSocketSession session); + void afterConnectionEstablished(WebSocketSession session) throws Exception; /** - * Handle an incoming WebSocket message. + * Invoked when a new WebSocket message arrives. + * + * @throws Exception this method can handle or propagate exceptions; see class-level + * Javadoc for details. */ - void handleMessage(WebSocketSession session, WebSocketMessage message); + void handleMessage(WebSocketSession session, WebSocketMessage message) throws Exception; /** * Handle an error from the underlying WebSocket message transport. + * + * @throws Exception this method can handle or propagate exceptions; see class-level + * Javadoc for details. */ - void handleTransportError(WebSocketSession session, Throwable exception); + void handleTransportError(WebSocketSession session, Throwable exception) throws Exception; /** - * A WebSocket connection has been closed. + * Invoked after the WebSocket connection has been closed by either side, or after a + * transport error has occurred. Although the session may technically still be open, + * depending on the underlying implementation, sending messages at this point is + * discouraged and most likely will not succeed. + * + * @throws Exception this method can handle or propagate exceptions; see class-level + * Javadoc for details. */ - void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus); + void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) throws Exception; /** * Whether this WebSocketHandler wishes to receive messages broken up in parts. diff --git a/spring-websocket/src/main/java/org/springframework/websocket/adapter/JettyWebSocketListenerAdapter.java b/spring-websocket/src/main/java/org/springframework/websocket/adapter/JettyWebSocketListenerAdapter.java index 163ffa057fd..04458b35601 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/adapter/JettyWebSocketListenerAdapter.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/adapter/JettyWebSocketListenerAdapter.java @@ -16,6 +16,8 @@ package org.springframework.websocket.adapter; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.eclipse.jetty.websocket.api.Session; import org.eclipse.jetty.websocket.api.WebSocketListener; import org.springframework.util.Assert; @@ -24,6 +26,7 @@ import org.springframework.websocket.CloseStatus; import org.springframework.websocket.TextMessage; import org.springframework.websocket.WebSocketHandler; import org.springframework.websocket.WebSocketSession; +import org.springframework.websocket.support.ExceptionWebSocketHandlerDecorator; /** * Adapts Spring's {@link WebSocketHandler} to Jetty's {@link WebSocketListener}. @@ -33,6 +36,8 @@ import org.springframework.websocket.WebSocketSession; */ public class JettyWebSocketListenerAdapter implements WebSocketListener { + private static final Log logger = LogFactory.getLog(JettyWebSocketListenerAdapter.class); + private final WebSocketHandler webSocketHandler; private WebSocketSession wsSession; @@ -47,30 +52,55 @@ public class JettyWebSocketListenerAdapter implements WebSocketListener { @Override public void onWebSocketConnect(Session session) { this.wsSession = new JettyWebSocketSessionAdapter(session); - this.webSocketHandler.afterConnectionEstablished(this.wsSession); - } - - @Override - public void onWebSocketClose(int statusCode, String reason) { - CloseStatus closeStatus = new CloseStatus(statusCode, reason); - this.webSocketHandler.afterConnectionClosed(this.wsSession, closeStatus); + try { + this.webSocketHandler.afterConnectionEstablished(this.wsSession); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + } } @Override public void onWebSocketText(String payload) { TextMessage message = new TextMessage(payload); - this.webSocketHandler.handleMessage(this.wsSession, message); + try { + this.webSocketHandler.handleMessage(this.wsSession, message); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + } } @Override public void onWebSocketBinary(byte[] payload, int offset, int len) { BinaryMessage message = new BinaryMessage(payload, offset, len); - this.webSocketHandler.handleMessage(this.wsSession, message); + try { + this.webSocketHandler.handleMessage(this.wsSession, message); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + } + } + + @Override + public void onWebSocketClose(int statusCode, String reason) { + CloseStatus closeStatus = new CloseStatus(statusCode, reason); + try { + this.webSocketHandler.afterConnectionClosed(this.wsSession, closeStatus); + } + catch (Throwable t) { + logger.error("Unhandled error for " + this.wsSession, t); + } } @Override public void onWebSocketError(Throwable cause) { - this.webSocketHandler.handleTransportError(this.wsSession, cause); + try { + this.webSocketHandler.handleTransportError(this.wsSession, cause); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + } } } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/adapter/StandardEndpointAdapter.java b/spring-websocket/src/main/java/org/springframework/websocket/adapter/StandardEndpointAdapter.java index a36249078e9..4f6f731281e 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/adapter/StandardEndpointAdapter.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/adapter/StandardEndpointAdapter.java @@ -23,12 +23,15 @@ import javax.websocket.Endpoint; import javax.websocket.EndpointConfig; import javax.websocket.MessageHandler; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.util.Assert; import org.springframework.websocket.BinaryMessage; import org.springframework.websocket.CloseStatus; import org.springframework.websocket.TextMessage; import org.springframework.websocket.WebSocketHandler; import org.springframework.websocket.WebSocketSession; +import org.springframework.websocket.support.ExceptionWebSocketHandlerDecorator; /** @@ -39,6 +42,8 @@ import org.springframework.websocket.WebSocketSession; */ public class StandardEndpointAdapter extends Endpoint { + private static final Log logger = LogFactory.getLog(StandardEndpointAdapter.class); + private final WebSocketHandler handler; private WebSocketSession wsSession; @@ -54,7 +59,13 @@ public class StandardEndpointAdapter extends Endpoint { public void onOpen(final javax.websocket.Session session, EndpointConfig config) { this.wsSession = new StandardWebSocketSessionAdapter(session); - this.handler.afterConnectionEstablished(this.wsSession); + try { + this.handler.afterConnectionEstablished(this.wsSession); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + return; + } session.addMessageHandler(new MessageHandler.Whole() { @Override @@ -84,23 +95,43 @@ public class StandardEndpointAdapter extends Endpoint { private void handleTextMessage(javax.websocket.Session session, String payload) { TextMessage textMessage = new TextMessage(payload); - this.handler.handleMessage(this.wsSession, textMessage); + try { + this.handler.handleMessage(this.wsSession, textMessage); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + } } private void handleBinaryMessage(javax.websocket.Session session, ByteBuffer payload, boolean isLast) { BinaryMessage binaryMessage = new BinaryMessage(payload, isLast); - this.handler.handleMessage(this.wsSession, binaryMessage); + try { + this.handler.handleMessage(this.wsSession, binaryMessage); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + } } @Override public void onClose(javax.websocket.Session session, CloseReason reason) { CloseStatus closeStatus = new CloseStatus(reason.getCloseCode().getCode(), reason.getReasonPhrase()); - this.handler.afterConnectionClosed(this.wsSession, closeStatus); + try { + this.handler.afterConnectionClosed(this.wsSession, closeStatus); + } + catch (Throwable t) { + logger.error("Unhandled error for " + this.wsSession, t); + } } @Override public void onError(javax.websocket.Session session, Throwable exception) { - this.handler.handleTransportError(this.wsSession, exception); + try { + this.handler.handleTransportError(this.wsSession, exception); + } + catch (Throwable t) { + ExceptionWebSocketHandlerDecorator.tryCloseWithError(this.wsSession, t, logger); + } } } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/adapter/WebSocketHandlerAdapter.java b/spring-websocket/src/main/java/org/springframework/websocket/adapter/WebSocketHandlerAdapter.java index 850cb2cacb2..fd88b052d67 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/adapter/WebSocketHandlerAdapter.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/adapter/WebSocketHandlerAdapter.java @@ -37,11 +37,11 @@ import org.springframework.websocket.WebSocketSession; public class WebSocketHandlerAdapter implements WebSocketHandler { @Override - public void afterConnectionEstablished(WebSocketSession session) { + public void afterConnectionEstablished(WebSocketSession session) throws Exception { } @Override - public final void handleMessage(WebSocketSession session, WebSocketMessage message) { + public final void handleMessage(WebSocketSession session, WebSocketMessage message) throws Exception { if (message instanceof TextMessage) { handleTextMessage(session, (TextMessage) message); } @@ -54,18 +54,18 @@ public class WebSocketHandlerAdapter implements WebSocketHandler { } } - protected void handleTextMessage(WebSocketSession session, TextMessage message) { + protected void handleTextMessage(WebSocketSession session, TextMessage message) throws Exception { } - protected void handleBinaryMessage(WebSocketSession session, BinaryMessage message) { + protected void handleBinaryMessage(WebSocketSession session, BinaryMessage message) throws Exception { } @Override - public void handleTransportError(WebSocketSession session, Throwable exception) { + public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception { } @Override - public void afterConnectionClosed(WebSocketSession session, CloseStatus status) { + public void afterConnectionClosed(WebSocketSession session, CloseStatus status) throws Exception { } @Override diff --git a/spring-websocket/src/main/java/org/springframework/websocket/client/WebSocketConnectFailureException.java b/spring-websocket/src/main/java/org/springframework/websocket/client/WebSocketConnectFailureException.java index a6f904b1ef1..0ed7c32e8e6 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/client/WebSocketConnectFailureException.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/client/WebSocketConnectFailureException.java @@ -25,6 +25,7 @@ import org.springframework.core.NestedRuntimeException; @SuppressWarnings("serial") public class WebSocketConnectFailureException extends NestedRuntimeException { + public WebSocketConnectFailureException(String msg, Throwable cause) { super(msg, cause); } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/DefaultHandshakeHandler.java b/spring-websocket/src/main/java/org/springframework/websocket/server/DefaultHandshakeHandler.java index b369aca0850..5e658feccc6 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/server/DefaultHandshakeHandler.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/DefaultHandshakeHandler.java @@ -89,7 +89,7 @@ public class DefaultHandshakeHandler implements HandshakeHandler { @Override public final boolean doHandshake(ServerHttpRequest request, ServerHttpResponse response, - WebSocketHandler webSocketHandler) throws IOException { + WebSocketHandler webSocketHandler) throws IOException, HandshakeFailureException { logger.debug("Starting handshake for " + request.getURI()); @@ -201,14 +201,14 @@ public class DefaultHandshakeHandler implements HandshakeHandler { return null; } - private String getWebSocketKeyHash(String key) { + private String getWebSocketKeyHash(String key) throws HandshakeFailureException { try { - MessageDigest digest = MessageDigest.getInstance("SHA1"); - byte[] bytes = digest.digest((key + GUID).getBytes(Charset.forName("ISO-8859-1"))); - return DatatypeConverter.printBase64Binary(bytes); + MessageDigest digest = MessageDigest.getInstance("SHA1"); + byte[] bytes = digest.digest((key + GUID).getBytes(Charset.forName("ISO-8859-1"))); + return DatatypeConverter.printBase64Binary(bytes); } catch (NoSuchAlgorithmException ex) { - throw new IllegalStateException("Failed to generate value for Sec-WebSocket-Key header", ex); + throw new HandshakeFailureException("Failed to generate value for Sec-WebSocket-Key header", ex); } } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/HandshakeFailureException.java b/spring-websocket/src/main/java/org/springframework/websocket/server/HandshakeFailureException.java new file mode 100644 index 00000000000..0c17392d39c --- /dev/null +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/HandshakeFailureException.java @@ -0,0 +1,48 @@ +/* + * Copyright 2002-2013 the original author or authors. + * + * Licensed 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.springframework.websocket.server; + +import org.springframework.core.NestedRuntimeException; + + +/** + * Thrown when handshake processing failed to complete due to an internal, unrecoverable + * error. This implies a server error (HTTP status code 500) as opposed to a failure in + * the handshake negotiation. + * + *

+ * By contrast, when handshake negotiation fails, the response status code will be 200 and + * the response headers and body will have been updated to reflect the cause for the + * failure. A {@link HandshakeHandler} implementation will have protected methods to + * customize updates to the response in those cases. + * + * @author Rossen Stoyanchev + * @since 4.0 + */ +@SuppressWarnings("serial") +public class HandshakeFailureException extends NestedRuntimeException { + + + public HandshakeFailureException(String msg, Throwable cause) { + super(msg, cause); + } + + public HandshakeFailureException(String msg) { + super(msg); + } + +} diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/HandshakeHandler.java b/spring-websocket/src/main/java/org/springframework/websocket/server/HandshakeHandler.java index fbdf2ca1a60..4a2df4928c0 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/server/HandshakeHandler.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/HandshakeHandler.java @@ -31,7 +31,21 @@ import org.springframework.websocket.WebSocketHandler; public interface HandshakeHandler { + /** + * + * @param request + * @param response + * @param webSocketHandler + * @return + * + * @throws IOException thrown when accessing or setting the response + * + * @throws HandshakeFailureException thrown when handshake processing failed to + * complete due to an internal, unrecoverable error, i.e. a server error as + * opposed to a failure to successfully negotiate the requirements of the + * handshake request. + */ boolean doHandshake(ServerHttpRequest request, ServerHttpResponse response, WebSocketHandler webSocketHandler) - throws IOException; + throws IOException, HandshakeFailureException; } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/RequestUpgradeStrategy.java b/spring-websocket/src/main/java/org/springframework/websocket/server/RequestUpgradeStrategy.java index 70a7653af49..f717ac81e3a 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/server/RequestUpgradeStrategy.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/RequestUpgradeStrategy.java @@ -37,12 +37,17 @@ public interface RequestUpgradeStrategy { String[] getSupportedVersions(); /** - * Perform runtime specific steps to complete the upgrade. - * Invoked only if the handshake is successful. + * Perform runtime specific steps to complete the upgrade. Invoked after successful + * negotiation of the handshake request. * * @param webSocketHandler the handler for WebSocket messages + * + * @throws HandshakeFailureException thrown when handshake processing failed to + * complete due to an internal, unrecoverable error, i.e. a server error as + * opposed to a failure to successfully negotiate the requirements of the + * handshake request. */ void upgrade(ServerHttpRequest request, ServerHttpResponse response, String selectedProtocol, - WebSocketHandler webSocketHandler) throws IOException; + WebSocketHandler webSocketHandler) throws IOException, HandshakeFailureException; } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/support/GlassfishRequestUpgradeStrategy.java b/spring-websocket/src/main/java/org/springframework/websocket/server/support/GlassfishRequestUpgradeStrategy.java index a5ca56c5b50..358813af800 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/server/support/GlassfishRequestUpgradeStrategy.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/support/GlassfishRequestUpgradeStrategy.java @@ -48,6 +48,7 @@ import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; +import org.springframework.websocket.server.HandshakeFailureException; import org.springframework.websocket.server.endpoint.EndpointRegistration; /** @@ -69,7 +70,7 @@ public class GlassfishRequestUpgradeStrategy extends AbstractEndpointUpgradeStra @Override public void upgradeInternal(ServerHttpRequest request, ServerHttpResponse response, - String selectedProtocol, Endpoint endpoint) throws IOException { + String selectedProtocol, Endpoint endpoint) throws IOException, HandshakeFailureException { Assert.isTrue(request instanceof ServletServerHttpRequest); HttpServletRequest servletRequest = ((ServletServerHttpRequest) request).getServletRequest(); @@ -85,12 +86,12 @@ public class GlassfishRequestUpgradeStrategy extends AbstractEndpointUpgradeStra engine.register(tyrusEndpoint); } catch (DeploymentException ex) { - throw new IllegalStateException("Failed to deploy endpoint in Glassfish", ex); + throw new HandshakeFailureException("Failed to deploy endpoint in Glassfish", ex); } try { if (!performUpgrade(servletRequest, servletResponse, request.getHeaders(), tyrusEndpoint)) { - throw new IllegalStateException("Failed to upgrade HttpServletRequest"); + throw new HandshakeFailureException("Failed to upgrade HttpServletRequest"); } } finally { diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/support/JettyRequestUpgradeStrategy.java b/spring-websocket/src/main/java/org/springframework/websocket/server/support/JettyRequestUpgradeStrategy.java index ef5674c6bd0..b536138e240 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/server/support/JettyRequestUpgradeStrategy.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/support/JettyRequestUpgradeStrategy.java @@ -34,6 +34,7 @@ import org.springframework.http.server.ServletServerHttpResponse; import org.springframework.util.Assert; import org.springframework.websocket.WebSocketHandler; import org.springframework.websocket.adapter.JettyWebSocketListenerAdapter; +import org.springframework.websocket.server.HandshakeFailureException; import org.springframework.websocket.server.RequestUpgradeStrategy; /** @@ -106,7 +107,7 @@ public class JettyRequestUpgradeStrategy implements RequestUpgradeStrategy { if (!this.factory.acceptWebSocket(request, response)) { // should never happen - throw new IllegalStateException("WebSocket request not accepted by Jetty"); + throw new HandshakeFailureException("WebSocket request not accepted by Jetty"); } } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/support/TomcatRequestUpgradeStrategy.java b/spring-websocket/src/main/java/org/springframework/websocket/server/support/TomcatRequestUpgradeStrategy.java index 0dcced9a125..c5a262b3a8d 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/server/support/TomcatRequestUpgradeStrategy.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/support/TomcatRequestUpgradeStrategy.java @@ -32,6 +32,7 @@ import org.springframework.http.server.ServerHttpResponse; import org.springframework.http.server.ServletServerHttpRequest; import org.springframework.util.Assert; import org.springframework.util.ReflectionUtils; +import org.springframework.websocket.server.HandshakeFailureException; import org.springframework.websocket.server.endpoint.EndpointRegistration; /** @@ -63,7 +64,7 @@ public class TomcatRequestUpgradeStrategy extends AbstractEndpointUpgradeStrateg method.invoke(webSocketRequest); } catch (Exception ex) { - throw new IllegalStateException("Failed to upgrade HttpServletRequest", ex); + throw new HandshakeFailureException("Failed to upgrade HttpServletRequest", ex); } // TODO: use ServletContext attribute when Tomcat is updated diff --git a/spring-websocket/src/main/java/org/springframework/websocket/server/support/WebSocketHttpRequestHandler.java b/spring-websocket/src/main/java/org/springframework/websocket/server/support/WebSocketHttpRequestHandler.java index 6bb1bfa171f..0b8b8c2d8b5 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/server/support/WebSocketHttpRequestHandler.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/server/support/WebSocketHttpRequestHandler.java @@ -28,7 +28,6 @@ import org.springframework.http.server.ServletServerHttpRequest; import org.springframework.http.server.ServletServerHttpResponse; import org.springframework.util.Assert; import org.springframework.web.HttpRequestHandler; -import org.springframework.web.util.NestedServletException; import org.springframework.websocket.WebSocketHandler; import org.springframework.websocket.server.DefaultHandshakeHandler; import org.springframework.websocket.server.HandshakeHandler; @@ -79,14 +78,11 @@ public class WebSocketHttpRequestHandler implements HttpRequestHandler { try { this.handshakeHandler.doHandshake(httpRequest, httpResponse, this.webSocketHandler); - } - catch (Exception e) { - // TODO - throw new NestedServletException("HandshakeHandler failure", e); - } - finally { httpResponse.flush(); } + catch (IOException ex) { + throw ex; + } } } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/support/ExceptionWebSocketHandlerDecorator.java b/spring-websocket/src/main/java/org/springframework/websocket/support/ExceptionWebSocketHandlerDecorator.java index 8d45636d198..0e82d032494 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/support/ExceptionWebSocketHandlerDecorator.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/support/ExceptionWebSocketHandlerDecorator.java @@ -43,11 +43,11 @@ public class ExceptionWebSocketHandlerDecorator extends WebSocketHandlerDecorato getDelegate().afterConnectionEstablished(session); } catch (Throwable ex) { - tryCloseWithError(session, ex); + tryCloseWithError(session, ex, logger); } } - private void tryCloseWithError(WebSocketSession session, Throwable exception) { + public static void tryCloseWithError(WebSocketSession session, Throwable exception, Log logger) { logger.error("Closing due to exception for " + session, exception); if (session.isOpen()) { try { @@ -65,7 +65,7 @@ public class ExceptionWebSocketHandlerDecorator extends WebSocketHandlerDecorato getDelegate().handleMessage(session, message); } catch (Throwable ex) { - tryCloseWithError(session,ex); + tryCloseWithError(session, ex, logger); } } @@ -75,7 +75,7 @@ public class ExceptionWebSocketHandlerDecorator extends WebSocketHandlerDecorato getDelegate().handleTransportError(session, exception); } catch (Throwable ex) { - tryCloseWithError(session, ex); + tryCloseWithError(session, ex, logger); } } @@ -84,8 +84,8 @@ public class ExceptionWebSocketHandlerDecorator extends WebSocketHandlerDecorato try { getDelegate().afterConnectionClosed(session, closeStatus); } - catch (Throwable ex) { - logger.error("Unhandled error for " + this, ex); + catch (Throwable t) { + logger.error("Unhandled error for " + this, t); } } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/support/LoggingWebSocketHandlerDecorator.java b/spring-websocket/src/main/java/org/springframework/websocket/support/LoggingWebSocketHandlerDecorator.java index 8cb0e0e8c21..6dbad07312f 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/support/LoggingWebSocketHandlerDecorator.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/support/LoggingWebSocketHandlerDecorator.java @@ -39,7 +39,7 @@ public class LoggingWebSocketHandlerDecorator extends WebSocketHandlerDecorator @Override - public void afterConnectionEstablished(WebSocketSession session) { + public void afterConnectionEstablished(WebSocketSession session) throws Exception { if (logger.isDebugEnabled()) { logger.debug("Connection established, " + session + ", uri=" + session.getURI()); } @@ -47,7 +47,7 @@ public class LoggingWebSocketHandlerDecorator extends WebSocketHandlerDecorator } @Override - public void handleMessage(WebSocketSession session, WebSocketMessage message) { + public void handleMessage(WebSocketSession session, WebSocketMessage message) throws Exception { if (logger.isTraceEnabled()) { logger.trace("Received " + message + ", " + session); } @@ -55,7 +55,7 @@ public class LoggingWebSocketHandlerDecorator extends WebSocketHandlerDecorator } @Override - public void handleTransportError(WebSocketSession session, Throwable exception) { + public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception { if (logger.isDebugEnabled()) { logger.debug("Transport error for " + session, exception); } @@ -63,7 +63,7 @@ public class LoggingWebSocketHandlerDecorator extends WebSocketHandlerDecorator } @Override - public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) { + public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) throws Exception { if (logger.isDebugEnabled()) { logger.debug("Connection closed for " + session + ", " + closeStatus); } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/support/PerConnectionWebSocketHandlerProxy.java b/spring-websocket/src/main/java/org/springframework/websocket/support/PerConnectionWebSocketHandlerProxy.java index 350c23bc052..64ed5c4bec9 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/support/PerConnectionWebSocketHandlerProxy.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/support/PerConnectionWebSocketHandlerProxy.java @@ -81,14 +81,14 @@ public class PerConnectionWebSocketHandlerProxy implements WebSocketHandler, Bea } @Override - public void afterConnectionEstablished(WebSocketSession session) { + public void afterConnectionEstablished(WebSocketSession session) throws Exception { WebSocketHandler handler = this.provider.getHandler(); this.handlers.put(session, handler); handler.afterConnectionEstablished(session); } @Override - public void handleMessage(WebSocketSession session, WebSocketMessage message) { + public void handleMessage(WebSocketSession session, WebSocketMessage message) throws Exception { getHandler(session).handleMessage(session, message); } @@ -99,12 +99,12 @@ public class PerConnectionWebSocketHandlerProxy implements WebSocketHandler, Bea } @Override - public void handleTransportError(WebSocketSession session, Throwable exception) { + public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception { getHandler(session).handleTransportError(session, exception); } @Override - public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) { + public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) throws Exception { try { getHandler(session).afterConnectionClosed(session, closeStatus); } diff --git a/spring-websocket/src/main/java/org/springframework/websocket/support/WebSocketHandlerDecorator.java b/spring-websocket/src/main/java/org/springframework/websocket/support/WebSocketHandlerDecorator.java index a94fbb8b518..5fb9d6d7d3a 100644 --- a/spring-websocket/src/main/java/org/springframework/websocket/support/WebSocketHandlerDecorator.java +++ b/spring-websocket/src/main/java/org/springframework/websocket/support/WebSocketHandlerDecorator.java @@ -43,22 +43,22 @@ public class WebSocketHandlerDecorator implements WebSocketHandler { } @Override - public void afterConnectionEstablished(WebSocketSession session) { + public void afterConnectionEstablished(WebSocketSession session) throws Exception { this.delegate.afterConnectionEstablished(session); } @Override - public void handleMessage(WebSocketSession session, WebSocketMessage message) { + public void handleMessage(WebSocketSession session, WebSocketMessage message) throws Exception { this.delegate.handleMessage(session, message); } @Override - public void handleTransportError(WebSocketSession session, Throwable exception) { + public void handleTransportError(WebSocketSession session, Throwable exception) throws Exception { this.delegate.handleTransportError(session, exception); } @Override - public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) { + public void afterConnectionClosed(WebSocketSession session, CloseStatus closeStatus) throws Exception { this.delegate.afterConnectionClosed(session, closeStatus); }