From 8f4ec3569704923b8cf3fb6dce64c4f8554672d6 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 3 Nov 2016 09:35:30 +0200 Subject: [PATCH] Check SockJS session type This commits adds a validation check whether the SockJS session type matches the transport type and rejects requests for which they don't match. Issue: SPR-14867 --- .../web/socket/sockjs/transport/TransportHandler.java | 10 +++++++++- .../transport/TransportHandlingSockJsService.java | 8 ++++++++ .../handler/AbstractHttpReceivingTransportHandler.java | 5 +++++ .../transport/handler/EventSourceTransportHandler.java | 7 +++++++ .../transport/handler/HtmlFileTransportHandler.java | 7 +++++++ .../handler/JsonpPollingTransportHandler.java | 6 ++++++ .../transport/handler/WebSocketTransportHandler.java | 7 ++++++- .../transport/handler/XhrPollingTransportHandler.java | 7 +++++++ .../handler/XhrStreamingTransportHandler.java | 7 +++++++ .../transport/handler/DefaultSockJsServiceTests.java | 5 +++-- 10 files changed, 65 insertions(+), 4 deletions(-) diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandler.java index 48fa86e485d..cd9bb4727f4 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2016 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. @@ -43,6 +43,14 @@ public interface TransportHandler { */ TransportType getTransportType(); + /** + * Whether the type of the given session matches the transport type of this + * {@code TransportHandler} where session id and the transport type are + * extracted from the SockJS URL. + * @since 4.3.3 + */ + boolean checkSessionType(SockJsSession session); + /** * Handle the given request and delegate messages to the provided * {@link WebSocketHandler}. diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandlingSockJsService.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandlingSockJsService.java index 79a33b87e1c..462b04fc8b0 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandlingSockJsService.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandlingSockJsService.java @@ -291,6 +291,11 @@ public class TransportHandlingSockJsService extends AbstractSockJsService implem return; } } + if (!transportHandler.checkSessionType(session)) { + logger.debug("Session type does not match the transport type for the request."); + response.setStatusCode(HttpStatus.NOT_FOUND); + return; + } } if (transportType.sendsNoCacheInstruction()) { @@ -303,7 +308,10 @@ public class TransportHandlingSockJsService extends AbstractSockJsService implem } } + transportHandler.handleRequest(request, response, handler, session); + + chain.applyAfterHandshake(request, response, null); } catch (SockJsException ex) { diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpReceivingTransportHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpReceivingTransportHandler.java index df03d0bc9f5..54f15f41a83 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpReceivingTransportHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/AbstractHttpReceivingTransportHandler.java @@ -98,4 +98,9 @@ public abstract class AbstractHttpReceivingTransportHandler extends AbstractTran protected abstract HttpStatus getResponseStatus(); + @Override + public boolean checkSessionType(SockJsSession session) { + return session instanceof AbstractHttpSockJsSession; + } + } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/EventSourceTransportHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/EventSourceTransportHandler.java index 5648468c8ea..35ba871fabf 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/EventSourceTransportHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/EventSourceTransportHandler.java @@ -24,7 +24,9 @@ import org.springframework.web.socket.WebSocketHandler; import org.springframework.web.socket.sockjs.frame.DefaultSockJsFrameFormat; import org.springframework.web.socket.sockjs.frame.SockJsFrameFormat; import org.springframework.web.socket.sockjs.transport.SockJsServiceConfig; +import org.springframework.web.socket.sockjs.transport.SockJsSession; import org.springframework.web.socket.sockjs.transport.TransportType; +import org.springframework.web.socket.sockjs.transport.session.PollingSockJsSession; import org.springframework.web.socket.sockjs.transport.session.StreamingSockJsSession; /** @@ -46,6 +48,11 @@ public class EventSourceTransportHandler extends AbstractHttpSendingTransportHan return new MediaType("text", "event-stream", UTF8_CHARSET); } + @Override + public boolean checkSessionType(SockJsSession session) { + return session instanceof EventSourceStreamingSockJsSession; + } + @Override public StreamingSockJsSession createSession( String sessionId, WebSocketHandler handler, Map attributes) { diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/HtmlFileTransportHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/HtmlFileTransportHandler.java index 951175fb2aa..b01afd26c2b 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/HtmlFileTransportHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/HtmlFileTransportHandler.java @@ -31,9 +31,11 @@ import org.springframework.web.socket.sockjs.SockJsTransportFailureException; import org.springframework.web.socket.sockjs.frame.DefaultSockJsFrameFormat; import org.springframework.web.socket.sockjs.frame.SockJsFrameFormat; import org.springframework.web.socket.sockjs.transport.SockJsServiceConfig; +import org.springframework.web.socket.sockjs.transport.SockJsSession; import org.springframework.web.socket.sockjs.transport.TransportHandler; import org.springframework.web.socket.sockjs.transport.TransportType; import org.springframework.web.socket.sockjs.transport.session.AbstractHttpSockJsSession; +import org.springframework.web.socket.sockjs.transport.session.PollingSockJsSession; import org.springframework.web.socket.sockjs.transport.session.StreamingSockJsSession; import org.springframework.web.util.JavaScriptUtils; @@ -87,6 +89,11 @@ public class HtmlFileTransportHandler extends AbstractHttpSendingTransportHandle return new MediaType("text", "html", UTF8_CHARSET); } + @Override + public boolean checkSessionType(SockJsSession session) { + return session instanceof HtmlFileStreamingSockJsSession; + } + @Override public StreamingSockJsSession createSession( String sessionId, WebSocketHandler handler, Map attributes) { diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/JsonpPollingTransportHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/JsonpPollingTransportHandler.java index 5571abfdf7e..5385d396136 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/JsonpPollingTransportHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/JsonpPollingTransportHandler.java @@ -29,6 +29,7 @@ import org.springframework.web.socket.sockjs.SockJsException; import org.springframework.web.socket.sockjs.SockJsTransportFailureException; import org.springframework.web.socket.sockjs.frame.DefaultSockJsFrameFormat; import org.springframework.web.socket.sockjs.frame.SockJsFrameFormat; +import org.springframework.web.socket.sockjs.transport.SockJsSession; import org.springframework.web.socket.sockjs.transport.TransportType; import org.springframework.web.socket.sockjs.transport.session.AbstractHttpSockJsSession; import org.springframework.web.socket.sockjs.transport.session.PollingSockJsSession; @@ -52,6 +53,11 @@ public class JsonpPollingTransportHandler extends AbstractHttpSendingTransportHa return new MediaType("application", "javascript", UTF8_CHARSET); } + @Override + public boolean checkSessionType(SockJsSession session) { + return session instanceof PollingSockJsSession; + } + @Override public PollingSockJsSession createSession( String sessionId, WebSocketHandler handler, Map attributes) { diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/WebSocketTransportHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/WebSocketTransportHandler.java index d7316f4786b..69155120686 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/WebSocketTransportHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/WebSocketTransportHandler.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -103,6 +103,11 @@ public class WebSocketTransportHandler extends AbstractTransportHandler } } + @Override + public boolean checkSessionType(SockJsSession session) { + return session instanceof WebSocketServerSockJsSession; + } + @Override public AbstractSockJsSession createSession(String id, WebSocketHandler handler, Map attrs) { return new WebSocketServerSockJsSession(id, getServiceConfig(), handler, attrs); diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/XhrPollingTransportHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/XhrPollingTransportHandler.java index 87355780496..eba5641ec97 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/XhrPollingTransportHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/XhrPollingTransportHandler.java @@ -23,8 +23,10 @@ import org.springframework.http.server.ServerHttpRequest; import org.springframework.web.socket.WebSocketHandler; import org.springframework.web.socket.sockjs.frame.DefaultSockJsFrameFormat; import org.springframework.web.socket.sockjs.frame.SockJsFrameFormat; +import org.springframework.web.socket.sockjs.transport.SockJsSession; import org.springframework.web.socket.sockjs.transport.TransportHandler; import org.springframework.web.socket.sockjs.transport.TransportType; +import org.springframework.web.socket.sockjs.transport.session.AbstractHttpSockJsSession; import org.springframework.web.socket.sockjs.transport.session.PollingSockJsSession; /** @@ -50,6 +52,11 @@ public class XhrPollingTransportHandler extends AbstractHttpSendingTransportHand return new DefaultSockJsFrameFormat("%s\n"); } + @Override + public boolean checkSessionType(SockJsSession session) { + return session instanceof PollingSockJsSession; + } + @Override public PollingSockJsSession createSession( String sessionId, WebSocketHandler handler, Map attributes) { diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/XhrStreamingTransportHandler.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/XhrStreamingTransportHandler.java index 30799a5cf47..35951b824f7 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/XhrStreamingTransportHandler.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/XhrStreamingTransportHandler.java @@ -24,8 +24,10 @@ import org.springframework.web.socket.WebSocketHandler; import org.springframework.web.socket.sockjs.frame.DefaultSockJsFrameFormat; import org.springframework.web.socket.sockjs.frame.SockJsFrameFormat; import org.springframework.web.socket.sockjs.transport.SockJsServiceConfig; +import org.springframework.web.socket.sockjs.transport.SockJsSession; import org.springframework.web.socket.sockjs.transport.TransportHandler; import org.springframework.web.socket.sockjs.transport.TransportType; +import org.springframework.web.socket.sockjs.transport.session.PollingSockJsSession; import org.springframework.web.socket.sockjs.transport.session.StreamingSockJsSession; /** @@ -56,6 +58,11 @@ public class XhrStreamingTransportHandler extends AbstractHttpSendingTransportHa return new MediaType("application", "javascript", UTF8_CHARSET); } + @Override + public boolean checkSessionType(SockJsSession session) { + return session instanceof XhrStreamingSockJsSession; + } + @Override public StreamingSockJsSession createSession( String sessionId, WebSocketHandler handler, Map attributes) { diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsServiceTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsServiceTests.java index aea9b0e802d..a45a3e701b4 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsServiceTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsServiceTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -44,7 +44,7 @@ import static org.junit.Assert.*; import static org.mockito.BDDMockito.*; /** - * Test fixture for {@link org.springframework.web.socket.sockjs.transport.handler.DefaultSockJsService}. + * Test fixture for {@link DefaultSockJsService}. * * @author Rossen Stoyanchev * @author Sebastien Deleuze @@ -239,6 +239,7 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests { resetResponse(); sockJsPath = sessionUrlPrefix + "xhr_send"; setRequest("POST", sockJsPrefix + sockJsPath); + given(this.xhrSendHandler.checkSessionType(this.session)).willReturn(true); this.service.handleRequest(this.request, this.response, sockJsPath, this.wsHandler); assertEquals(200, this.servletResponse.getStatus()); // session exists