Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Simplify determination of SockJS path

Browse Source 
The SockJS path is now passed to the SockJsService handleRequest method
thus removing the need to guess it.

Issue: SPR-11058
This commit is contained in:
Rossen Stoyanchev
2013-11-07 22:26:00 -05:00
parent 6a18daea33
commit 2a6c1f75e7
8 changed files with 46 additions and 242 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
spring-messaging/src/main/java/org/springframework/messaging/simp/config/AbstractStompEndpointRegistration.java
View File

@@ -147,7 +147,7 @@ public abstract class AbstractStompEndpointRegistration<M> implements StompEndpo
}
protected SockJsService getSockJsService() {
return super.getSockJsService(paths);
return super.getSockJsService();
}
}

12
spring-websocket/src/main/java/org/springframework/web/socket/server/config/AbstractWebSocketHandlerRegistration.java
View File

@@ -16,9 +16,7 @@
package org.springframework.web.socket.server.config;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import org.springframework.scheduling.TaskScheduler;
import org.springframework.util.Assert;
@@ -110,7 +108,7 @@ public abstract class AbstractWebSocketHandlerRegistration<M> implements WebSock
M mappings = createMappings();
if (this.sockJsServiceRegistration != null) {
SockJsService sockJsService = this.sockJsServiceRegistration.getSockJsService(getAllPrefixes());
SockJsService sockJsService = this.sockJsServiceRegistration.getSockJsService();
for (WebSocketHandler wsHandler : this.handlerMap.keySet()) {
for (String path : this.handlerMap.get(wsHandler)) {
String pathPattern = path.endsWith("/") ? path + "**" : path + "/**";
@@ -130,14 +128,6 @@ public abstract class AbstractWebSocketHandlerRegistration<M> implements WebSock
return mappings;
}
private final String[] getAllPrefixes() {
List<String> all = new ArrayList<String>();
for (List<String> prefixes: this.handlerMap.values()) {
all.addAll(prefixes);
}
return all.toArray(new String[all.size()]);
}
private HandshakeHandler getOrCreateHandshakeHandler() {
return (this.handshakeHandler != null) ? this.handshakeHandler : new DefaultHandshakeHandler();
}

5
spring-websocket/src/main/java/org/springframework/web/socket/server/config/SockJsServiceRegistration.java
View File

@@ -199,11 +199,8 @@ public class SockJsServiceRegistration {
return this;
}
protected SockJsService getSockJsService(String[] sockJsPrefixes) {
protected SockJsService getSockJsService() {
DefaultSockJsService service = createSockJsService();
if (sockJsPrefixes != null) {
service.setValidSockJsPrefixes(sockJsPrefixes);
}
if (this.clientLibraryUrl != null) {
service.setSockJsClientLibraryUrl(this.clientLibraryUrl);
}

9
spring-websocket/src/main/java/org/springframework/web/socket/sockjs/SockJsHttpRequestHandler.java
View File

@@ -28,6 +28,7 @@ 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.servlet.HandlerMapping;
import org.springframework.web.socket.WebSocketHandler;
import org.springframework.web.socket.support.ExceptionWebSocketHandlerDecorator;
import org.springframework.web.socket.support.LoggingWebSocketHandlerDecorator;
@@ -81,11 +82,17 @@ public class SockJsHttpRequestHandler implements HttpRequestHandler {
ServerHttpResponse response = new ServletServerHttpResponse(servletResponse);
try {
this.sockJsService.handleRequest(request, response, this.wsHandler);
this.sockJsService.handleRequest(request, response, getSockJsPath(servletRequest), this.wsHandler);
}
catch (Throwable t) {
throw new SockJsException("Uncaught failure in SockJS request, uri=" + request.getURI(), t);
}
}
private String getSockJsPath(HttpServletRequest servletRequest) {
String attribute = HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE;
String path = (String) servletRequest.getAttribute(attribute);
return ((path.length() > 0) && (path.charAt(0) != '/')) ? "/" + path : path;
}
}

10
spring-websocket/src/main/java/org/springframework/web/socket/sockjs/SockJsService.java
View File

@@ -29,11 +29,6 @@ import org.springframework.web.socket.support.ExceptionWebSocketHandlerDecorator
* asynchronous support enabled through the ServletContext API or by adding an
* {@code <async-support>true</async-support>} element to servlet and filter declarations
* in web.xml.
* <p>
* The service can be integrated into any HTTP request handling mechanism (e.g. plain
* Servlet, Spring MVC, or other). It is expected that it will be mapped, as expected
* by the SockJS protocol, to a specific prefix (e.g. "/echo") including all sub-URLs
* (i.e. Ant path pattern "/echo/**").
*
* @author Rossen Stoyanchev
* @since 4.0
@@ -52,6 +47,7 @@ public interface SockJsService {
*
* @param request the current request
* @param response the current response
* @param sockJsPath the remainder of the path within the SockJS service prefix
* @param handler the handler that will exchange messages with the SockJS client
*
* @throws SockJsException raised when request processing fails; generally, failed
@@ -64,7 +60,7 @@ public interface SockJsService {
* The former is automatically added when using
* {@link SockJsHttpRequestHandler}.
*/
void handleRequest(ServerHttpRequest request, ServerHttpResponse response, WebSocketHandler handler)
throws SockJsException;
void handleRequest(ServerHttpRequest request, ServerHttpResponse response, String sockJsPath,
WebSocketHandler handler) throws SockJsException;
}

119
spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java
View File

@@ -18,15 +18,11 @@ package org.springframework.web.socket.sockjs.support;
import java.io.IOException;
import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Random;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.TimeUnit;
import org.apache.commons.logging.Log;
@@ -51,14 +47,6 @@ import org.springframework.web.socket.sockjs.SockJsService;
* An abstract base class for {@link SockJsService} implementations that provides SockJS
* path resolution and handling of static SockJS requests (e.g. "/info", "/iframe.html",
* etc). Sub-classes must handle session URLs (i.e. transport-specific requests).
* <p>
* This service is unaware of the underlying HTTP request processing mechanism and URL
* mappings but nevertheless needs to know the "SockJS path" for a given request, i.e. the
* portion of the URL path that follows the SockJS prefix. In most cases, this can be
* auto-detected since the <a href="https://github.com/sockjs/sockjs-client">SockJS
* client</a> sends a "greeting URL" first. However it is recommended to configure
* explicitly the expected SockJS prefixes via {@link #setValidSockJsPrefixes(String...)}
* to eliminate any potential issues.
*
* @author Rossen Stoyanchev
* @since 4.0
@@ -90,10 +78,6 @@ public abstract class AbstractSockJsService implements SockJsService {
private final TaskScheduler taskScheduler;
private final List<String> validSockJsPrefixes = new ArrayList<String>();
private final List<String> knownSockJsPrefixes = new CopyOnWriteArrayList<String>();
public AbstractSockJsService(TaskScheduler scheduler) {
Assert.notNull(scheduler, "scheduler must not be null");
@@ -112,39 +96,6 @@ public abstract class AbstractSockJsService implements SockJsService {
return this.name;
}
/**
* Use this property to configure one or more prefixes that this SockJS service is
* allowed to serve. The prefix (e.g. "/echo") is needed to extract the SockJS
* specific portion of the URL (e.g. "${prefix}/info", "${prefix}/iframe.html", etc).
*
* <p>This property is not strictly required. In most cases, the SockJS path can be
* auto-detected since the initial request from the SockJS client is of the form
* "{prefix}/info". Assuming the SockJS service is mapped correctly (e.g. using
* Ant-style pattern "/echo/**") this should work fine. This property can be used
* to configure explicitly the prefixes this service is allowed to service.
*
* @param prefixes the prefixes to use; prefixes do not need to include the portions
* of the path that represent Servlet container context or Servlet path.
*/
public void setValidSockJsPrefixes(String... prefixes) {
this.validSockJsPrefixes.clear();
for (String prefix : prefixes) {
if (prefix.endsWith("/") && (prefix.length() > 1)) {
prefix = prefix.substring(0, prefix.length() - 1);
}
this.validSockJsPrefixes.add(prefix);
}
// sort with longest prefix at the top
Collections.sort(this.validSockJsPrefixes, Collections.reverseOrder(new Comparator<String>() {
@Override
public int compare(String o1, String o2) {
return new Integer(o1.length()).compareTo(new Integer(o2.length()));
}
}));
}
/**
* Transports which don't support cross-domain communication natively (e.g.
* "eventsource", "htmlfile") rely on serving a simple page (using the
@@ -308,12 +259,10 @@ public abstract class AbstractSockJsService implements SockJsService {
*/
@Override
public final void handleRequest(ServerHttpRequest request, ServerHttpResponse response,
WebSocketHandler wsHandler) throws SockJsException {
String sockJsPath, WebSocketHandler wsHandler) throws SockJsException {
String sockJsPath = getSockJsPath(request);
if (sockJsPath == null) {
logger.warn("Could not determine SockJS path for URL \"" + request.getURI().getPath() +
". Consider setting validSockJsPrefixes.");
logger.warn("No SockJS path provided, URI=\"" + request.getURI());
response.setStatusCode(HttpStatus.NOT_FOUND);
return;
}
@@ -365,70 +314,6 @@ public abstract class AbstractSockJsService implements SockJsService {
}
}
/**
* Return the SockJS path or null if the path could not be determined.
*/
private String getSockJsPath(ServerHttpRequest request) {
String path = request.getURI().getPath();
// Try SockJS prefix hints
if (!this.validSockJsPrefixes.isEmpty()) {
for (String prefix : this.validSockJsPrefixes) {
int index = path.lastIndexOf(prefix);
if (index != -1) {
return path.substring(index + prefix.length());
}
}
return null;
}
// Try SockJS info request
if (path.endsWith("/info")) {
addKnownSockJsPrefix(path.substring(0, path.length() - "/info".length()));
return "/info";
}
// Have we seen this prefix before (following the initial /info request)?
String match = null;
for (String sockJsPath : this.knownSockJsPrefixes) {
if (path.startsWith(sockJsPath)) {
if ((match == null) || (match.length() < sockJsPath.length())) {
match = sockJsPath;
}
}
}
if (match != null) {
String result = path.substring(match.length());
Assert.isTrue(result.charAt(0) == '/', "Invalid SockJS path extracted from incoming path \"" +
path + "\". The extracted SockJS path is \"" + result +
"\". It was extracted from these known SockJS prefixes " + this.knownSockJsPrefixes +
". Consider setting 'validSockJsPrefixes' on DefaultSockJsService.");
return result;
}
// Try SockJS greeting
String pathNoSlash = path.endsWith("/") ? path.substring(0, path.length() - 1) : path;
String lastSegment = pathNoSlash.substring(pathNoSlash.lastIndexOf('/') + 1);
if (!isValidTransportType(lastSegment) && !lastSegment.startsWith("iframe")) {
addKnownSockJsPrefix(path);
return "";
}
return null;
}
private void addKnownSockJsPrefix(String path) {
if (this.knownSockJsPrefixes.size() > MAX_KNOWN_SOCKJS_PREFIX_COUNT) {
String removed = this.knownSockJsPrefixes.remove(0);
if (logger.isWarnEnabled()) {
logger.warn("MAX_KNOWN_SOCKJS_PREFIX_COUNT reached, removed prefix " + removed);
}
}
this.knownSockJsPrefixes.add(path);
}
/**
* Validate whether the given transport String extracted from the URL is a valid
* SockJS transport type (regardless of whether a transport handler is configured).

96
spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/AbstractSockJsServiceTests.java
View File

@@ -51,84 +51,9 @@ public class AbstractSockJsServiceTests extends AbstractHttpRequestTests {
this.service = new TestSockJsService(new ThreadPoolTaskScheduler());
}
@Test
public void getSockJsPathForGreetingRequest() throws Exception {
handleRequest("GET", "/a", HttpStatus.OK);
assertEquals("Welcome to SockJS!\n", this.servletResponse.getContentAsString());
handleRequest("GET", "/a/", HttpStatus.OK);
assertEquals("Welcome to SockJS!\n", this.servletResponse.getContentAsString());
this.service.setValidSockJsPrefixes("/b");
handleRequest("GET", "/a", HttpStatus.NOT_FOUND);
handleRequest("GET", "/a/", HttpStatus.NOT_FOUND);
handleRequest("GET", "/b", HttpStatus.OK);
assertEquals("Welcome to SockJS!\n", this.servletResponse.getContentAsString());
}
@Test
public void getSockJsPathForInfoRequest() throws Exception {
handleRequest("GET", "/a/info", HttpStatus.OK);
assertTrue(this.servletResponse.getContentAsString().startsWith("{\"entropy\":"));
handleRequest("GET", "/a/server/session/xhr", HttpStatus.OK);
assertEquals("session", this.service.sessionId);
assertEquals(TransportType.XHR.value(), this.service.transport);
assertSame(this.handler, this.service.handler);
this.service.setValidSockJsPrefixes("/b");
handleRequest("GET", "/a/info", HttpStatus.NOT_FOUND);
handleRequest("GET", "/b/info", HttpStatus.OK);
assertTrue(this.servletResponse.getContentAsString().startsWith("{\"entropy\":"));
}
@Test
public void getSockJsPathForTransportRequest() throws Exception {
// Info or greeting requests must be first so "/a" is cached as a known prefix
handleRequest("GET", "/a/info", HttpStatus.OK);
handleRequest("GET", "/a/server/session/xhr", HttpStatus.OK);
assertEquals("session", this.service.sessionId);
assertEquals(TransportType.XHR.value(), this.service.transport);
assertSame(this.handler, this.service.handler);
}
@Test
public void getSockJsPathForTransportRequestWithConfiguredPrefix() throws Exception {
this.service.setValidSockJsPrefixes("/a");
handleRequest("GET", "/a/server/session/xhr", HttpStatus.OK);
assertEquals("session", this.service.sessionId);
assertEquals(TransportType.XHR.value(), this.service.transport);
assertSame(this.handler, this.service.handler);
}
// SPR-10923
@Test
public void getSockJsPathWithPartlyMatchingServletPath() throws Exception {
this.service.setValidSockJsPrefixes("/snake");
handleRequest("GET", "/snakedemo/snake/info", HttpStatus.OK);
assertTrue(this.servletResponse.getContentAsString().startsWith("{\"entropy\":"));
}
@Test
public void validateRequest() throws Exception {
this.service.setValidSockJsPrefixes("/echo");
this.service.setWebSocketsEnabled(false);
handleRequest("GET", "/echo/server/session/websocket", HttpStatus.NOT_FOUND);
@@ -148,7 +73,7 @@ public class AbstractSockJsServiceTests extends AbstractHttpRequestTests {
@Test
public void handleInfoGet() throws Exception {
handleRequest("GET", "/a/info", HttpStatus.OK);
handleRequest("GET", "/echo/info", HttpStatus.OK);
assertEquals("application/json;charset=UTF-8", this.servletResponse.getContentType());
assertEquals("*", this.servletResponse.getHeader("Access-Control-Allow-Origin"));
@@ -162,7 +87,7 @@ public class AbstractSockJsServiceTests extends AbstractHttpRequestTests {
this.service.setSessionCookieNeeded(false);
this.service.setWebSocketsEnabled(false);
handleRequest("GET", "/a/info", HttpStatus.OK);
handleRequest("GET", "/echo/info", HttpStatus.OK);
body = this.servletResponse.getContentAsString();
assertEquals(",\"origins\":[\"*:*\"],\"cookie_needed\":false,\"websocket\":false}",
@@ -174,7 +99,7 @@ public class AbstractSockJsServiceTests extends AbstractHttpRequestTests {
this.servletRequest.addHeader("Access-Control-Request-Headers", "Last-Modified");
handleRequest("OPTIONS", "/a/info", HttpStatus.NO_CONTENT);
handleRequest("OPTIONS", "/echo/info", HttpStatus.NO_CONTENT);
this.response.flush();
assertEquals("*", this.servletResponse.getHeader("Access-Control-Allow-Origin"));
@@ -187,8 +112,7 @@ public class AbstractSockJsServiceTests extends AbstractHttpRequestTests {
@Test
public void handleIframeRequest() throws Exception {
this.service.setValidSockJsPrefixes("/a");
handleRequest("GET", "/a/iframe.html", HttpStatus.OK);
handleRequest("GET", "/echo/iframe.html", HttpStatus.OK);
assertEquals("text/html;charset=UTF-8", this.servletResponse.getContentType());
assertTrue(this.servletResponse.getContentAsString().startsWith("<!DOCTYPE html>\n"));
@@ -202,17 +126,16 @@ public class AbstractSockJsServiceTests extends AbstractHttpRequestTests {
this.servletRequest.addHeader("If-None-Match", "\"0da1ed070012f304e47b83c81c48ad620\"");
this.service.setValidSockJsPrefixes("/a");
handleRequest("GET", "/a/iframe.html", HttpStatus.NOT_MODIFIED);
handleRequest("GET", "/echo/iframe.html", HttpStatus.NOT_MODIFIED);
}
@Test
public void handleRawWebSocketRequest() throws Exception {
handleRequest("GET", "/a", HttpStatus.OK);
handleRequest("GET", "/echo", HttpStatus.OK);
assertEquals("Welcome to SockJS!\n", this.servletResponse.getContentAsString());
handleRequest("GET", "/a/websocket", HttpStatus.OK);
handleRequest("GET", "/echo/websocket", HttpStatus.OK);
assertNull("Raw WebSocket should not open a SockJS session", this.service.sessionId);
assertSame(this.handler, this.service.handler);
}
@@ -221,7 +144,8 @@ public class AbstractSockJsServiceTests extends AbstractHttpRequestTests {
private void handleRequest(String httpMethod, String uri, HttpStatus httpStatus) throws IOException {
resetResponse();
setRequest(httpMethod, uri);
this.service.handleRequest(this.request, this.response, this.handler);
String sockJsPath = uri.substring("/echo".length());
this.service.handleRequest(this.request, this.response, sockJsPath, this.handler);
assertEquals(httpStatus.value(), this.servletResponse.getStatus());
}
@@ -230,7 +154,7 @@ public class AbstractSockJsServiceTests extends AbstractHttpRequestTests {
public void handleEmptyContentType() throws Exception {
servletRequest.setContentType("");
handleRequest("GET", "/a/info", HttpStatus.OK);
handleRequest("GET", "/echo/info", HttpStatus.OK);
assertEquals("Invalid/empty content should have been ignored", 200, this.servletResponse.getStatus());
}

35
spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsServiceTests.java
View File

@@ -44,11 +44,11 @@ import static org.mockito.Mockito.*;
*/
public class DefaultSockJsServiceTests extends AbstractHttpRequestTests {
private static final String sockJsPrefix = "mysockjs";
private static final String sockJsPrefix = "/mysockjs";
private static final String sessionId = "session1";
private static final String sessionUrlPrefix = "/mysockjs/server1/" + sessionId + "/";
private static final String sessionUrlPrefix = "/server1/" + sessionId + "/";
@Mock private SessionCreatingTransportHandler xhrHandler;
@@ -80,7 +80,6 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests {
this.service = new DefaultSockJsService(this.taskScheduler,
Arrays.<TransportHandler>asList(this.xhrHandler, this.xhrSendHandler));
this.service.setValidSockJsPrefixes(sockJsPrefix);
}
@Test
@@ -127,8 +126,9 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests {
@Test
public void handleTransportRequestXhr() throws Exception {
setRequest("POST", sessionUrlPrefix + "xhr");
this.service.handleRequest(this.request, this.response, this.wsHandler);
String sockJsPath = sessionUrlPrefix + "xhr";
setRequest("POST", sockJsPrefix + sockJsPath);
this.service.handleRequest(this.request, this.response, sockJsPath, this.wsHandler);
assertEquals(200, this.servletResponse.getStatus());
verify(this.xhrHandler).handleRequest(this.request, this.response, this.wsHandler, this.session);
@@ -142,8 +142,9 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests {
@Test
public void handleTransportRequestXhrOptions() throws Exception {
setRequest("OPTIONS", sessionUrlPrefix + "xhr");
this.service.handleRequest(this.request, this.response, this.wsHandler);
String sockJsPath = sessionUrlPrefix + "xhr";
setRequest("OPTIONS", sockJsPrefix + sockJsPath);
this.service.handleRequest(this.request, this.response, sockJsPath, this.wsHandler);
assertEquals(204, this.servletResponse.getStatus());
assertEquals("*", this.response.getHeaders().getFirst("Access-Control-Allow-Origin"));
@@ -154,8 +155,9 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests {
@Test
public void handleTransportRequestNoSuitableHandler() throws Exception {
setRequest("POST", sessionUrlPrefix + "eventsource");
this.service.handleRequest(this.request, this.response, this.wsHandler);
String sockJsPath = sessionUrlPrefix + "eventsource";
setRequest("POST", sockJsPrefix + sockJsPath);
this.service.handleRequest(this.request, this.response, sockJsPath, this.wsHandler);
assertEquals(404, this.servletResponse.getStatus());
}
@@ -163,21 +165,24 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests {
@Test
public void handleTransportRequestXhrSend() throws Exception {
setRequest("POST", sessionUrlPrefix + "xhr_send");
this.service.handleRequest(this.request, this.response, this.wsHandler);
String sockJsPath = sessionUrlPrefix + "xhr_send";
setRequest("POST", sockJsPrefix + sockJsPath);
this.service.handleRequest(this.request, this.response, sockJsPath, this.wsHandler);
assertEquals(404, this.servletResponse.getStatus()); // no session yet
resetResponse();
setRequest("POST", sessionUrlPrefix + "xhr");
this.service.handleRequest(this.request, this.response, this.wsHandler);
sockJsPath = sessionUrlPrefix + "xhr";
setRequest("POST", sockJsPrefix + sockJsPath);
this.service.handleRequest(this.request, this.response, sockJsPath, this.wsHandler);
assertEquals(200, this.servletResponse.getStatus()); // session created
verify(this.xhrHandler).handleRequest(this.request, this.response, this.wsHandler, this.session);
resetResponse();
setRequest("POST", sessionUrlPrefix + "xhr_send");
this.service.handleRequest(this.request, this.response, this.wsHandler);
sockJsPath = sessionUrlPrefix + "xhr_send";
setRequest("POST", sockJsPrefix + sockJsPath);
this.service.handleRequest(this.request, this.response, sockJsPath, this.wsHandler);
assertEquals(200, this.servletResponse.getStatus()); // session exists
verify(this.xhrSendHandler).handleRequest(this.request, this.response, this.wsHandler, this.session);