Check SockJS session type

rstoyanchev · rstoyanchev · commit 8f4ec3569704 · 2016-11-03T09:38:24.000+02:00
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
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
@@ -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
@@ -291,6 +291,11 @@ else if (transportType.supportsCors()) {
 						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 @@ else if (transportType.supportsCors()) {
 				}
 			}
 
+
 			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
@@ -98,4 +98,9 @@ private void handleReadError(ServerHttpResponse response, String error, String s
 
 	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
@@ -24,7 +24,9 @@
 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 @@ protected MediaType getContentType() {
 		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<String, Object> 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
@@ -31,9 +31,11 @@
 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 @@ protected MediaType getContentType() {
 		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<String, Object> 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
@@ -29,6 +29,7 @@
 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 @@ protected MediaType getContentType() {
 		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<String, Object> 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
@@ -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 void stop() {
 		}
 	}
 
+	@Override
+	public boolean checkSessionType(SockJsSession session) {
+		return session instanceof WebSocketServerSockJsSession;
+	}
+
 	@Override
 	public AbstractSockJsSession createSession(String id, WebSocketHandler handler, Map<String, Object> 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
@@ -23,8 +23,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.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 @@ protected SockJsFrameFormat getFrameFormat(ServerHttpRequest request) {
 		return new DefaultSockJsFrameFormat("%s\n");
 	}
 
+	@Override
+	public boolean checkSessionType(SockJsSession session) {
+		return session instanceof PollingSockJsSession;
+	}
+
 	@Override
 	public PollingSockJsSession createSession(
 			String sessionId, WebSocketHandler handler, Map<String, Object> 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
@@ -24,8 +24,10 @@
 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 @@ protected MediaType getContentType() {
 		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<String, Object> 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
@@ -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.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 void handleTransportRequestXhrSend() throws Exception {
 		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

Original file line number	Diff line number	Diff line change
`@@ -291,6 +291,11 @@ else if (transportType.supportsCors()) {`
`291`	`291`	`return;`
`292`	`292`	`}`
`293`	`293`	`}`
	`294`	`+ if (!transportHandler.checkSessionType(session)) {`
	`295`	`+ logger.debug("Session type does not match the transport type for the request.");`
	`296`	`+ response.setStatusCode(HttpStatus.NOT_FOUND);`
	`297`	`+ return;`
	`298`	`+ }`
`294`	`299`	`}`
`295`	`300`
`296`	`301`	`if (transportType.sendsNoCacheInstruction()) {`
`@@ -303,7 +308,10 @@ else if (transportType.supportsCors()) {`
`303`	`308`	`}`
`304`	`309`	`}`
`305`	`310`
	`311`	`+`
`306`	`312`	`transportHandler.handleRequest(request, response, handler, session);`
	`313`	`+`
	`314`	`+`
`307`	`315`	`chain.applyAfterHandshake(request, response, null);`
`308`	`316`	`}`
`309`	`317`	`catch (SockJsException ex) {`
Original file line number	Diff line number	Diff line change
`@@ -98,4 +98,9 @@ private void handleReadError(ServerHttpResponse response, String error, String s`
`98`	`98`
`99`	`99`	`protected abstract HttpStatus getResponseStatus();`
`100`	`100`
	`101`	`+ @Override`
	`102`	`+ public boolean checkSessionType(SockJsSession session) {`
	`103`	`+ return session instanceof AbstractHttpSockJsSession;`
	`104`	`+ }`
	`105`	`+`
`101`	`106`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright 2002-2015 the original author or authors.`
	`2`	`+ * Copyright 2002-2016 the original author or authors.`
`3`	`3`	`*`
`4`	`4`	`* Licensed under the Apache License, Version 2.0 (the "License");`
`5`	`5`	`* you may not use this file except in compliance with the License.`
`@@ -103,6 +103,11 @@ public void stop() {`
`103`	`103`	`}`
`104`	`104`	`}`
`105`	`105`
	`106`	`+ @Override`
	`107`	`+ public boolean checkSessionType(SockJsSession session) {`
	`108`	`+ return session instanceof WebSocketServerSockJsSession;`
	`109`	`+ }`
	`110`	`+`
`106`	`111`	`@Override`
`107`	`112`	`public AbstractSockJsSession createSession(String id, WebSocketHandler handler, Map<String, Object> attrs) {`
`108`	`113`	`return new WebSocketServerSockJsSession(id, getServiceConfig(), handler, attrs);`