From 77fa8698b3687d20c2750562778f3a35b0048320 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Thu, 26 Sep 2013 12:39:09 -0400 Subject: [PATCH] Enable cookie_needed by default in SockJS service Issue: SPR-10939 --- .../config/SockJsServiceRegistration.java | 29 +++++++++----- .../sockjs/support/AbstractSockJsService.java | 39 ++++++++++++------- .../handler/DefaultSockJsService.java | 20 ---------- .../support/AbstractSockJsServiceTests.java | 2 +- .../handler/DefaultSockJsServiceTests.java | 34 ---------------- 5 files changed, 46 insertions(+), 78 deletions(-) diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/server/config/SockJsServiceRegistration.java b/spring-websocket/src/main/java/org/springframework/web/socket/server/config/SockJsServiceRegistration.java index 62508e3342..1b67fea6cf 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/server/config/SockJsServiceRegistration.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/server/config/SockJsServiceRegistration.java @@ -44,7 +44,7 @@ public class SockJsServiceRegistration { private Integer streamBytesLimit; - private Boolean sessionCookieEnabled; + private Boolean sessionCookieNeeded; private Long heartbeatTime; @@ -105,14 +105,23 @@ public class SockJsServiceRegistration { } /** - * Some load balancers do sticky sessions, but only if there is a "JSESSIONID" - * cookie. Even if it is set to a dummy value, it doesn't matter since - * session information is added by the load balancer. - * - *

The default value is "false" since Java servers set the session cookie. + * The SockJS protocol requires a server to respond to the initial "/info" request + * from clients with a "cookie_needed" boolean property that indicates whether the use + * of a JSESSIONID cookie is required for the application to function correctly, e.g. + * for load balancing or in Java Servlet containers for the use of an HTTP session. + *

+ * This is especially important for IE 8,9 that support XDomainRequest -- a modified + * AJAX/XHR -- that can do requests across domains but does not send any cookies. In + * those cases, the SockJS client prefers the "iframe-htmlfile" transport over + * "xdr-streaming" in order to be able to send cookies. + *

+ * The default value is "true" to maximize the chance for applications to work + * correctly in IE 8,9 with support for cookies (and the JSESSIONID cookie in + * particular). However, an application can choose to set this to "false" if the use + * of cookies (and HTTP session) is not required. */ - public SockJsServiceRegistration setDummySessionCookieEnabled(boolean sessionCookieEnabled) { - this.sessionCookieEnabled = sessionCookieEnabled; + public SockJsServiceRegistration setSessionCookieNeeded(boolean sessionCookieNeeded) { + this.sessionCookieNeeded = sessionCookieNeeded; return this; } @@ -201,8 +210,8 @@ public class SockJsServiceRegistration { if (this.streamBytesLimit != null) { service.setStreamBytesLimit(this.streamBytesLimit); } - if (this.sessionCookieEnabled != null) { - service.setDummySessionCookieEnabled(this.sessionCookieEnabled); + if (this.sessionCookieNeeded != null) { + service.setSessionCookieNeeded(this.sessionCookieNeeded); } if (this.heartbeatTime != null) { service.setHeartbeatTime(this.heartbeatTime); diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java index 9a8dc410fe..a7ddc28d46 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/support/AbstractSockJsService.java @@ -77,7 +77,7 @@ public abstract class AbstractSockJsService implements SockJsService { private int streamBytesLimit = 128 * 1024; - private boolean sessionCookieEnabled = false; + private boolean sessionCookieNeeded = true; private long heartbeatTime = 25 * 1000; @@ -188,22 +188,35 @@ public abstract class AbstractSockJsService implements SockJsService { } /** - * Some load balancers do sticky sessions, but only if there is a "JSESSIONID" - * cookie. Even if it is set to a dummy value, it doesn't matter since - * session information is added by the load balancer. - * - *

The default value is "false" since Java servers set the session cookie. + * The SockJS protocol requires a server to respond to an initial "/info" request from + * clients with a "cookie_needed" boolean property that indicates whether the use of a + * JSESSIONID cookie is required for the application to function correctly, e.g. for + * load balancing or in Java Servlet containers for the use of an HTTP session. + *

+ * This is especially important for IE 8,9 that support XDomainRequest -- a modified + * AJAX/XHR -- that can do requests across domains but does not send any cookies. In + * those cases, the SockJS client prefers the "iframe-htmlfile" transport over + * "xdr-streaming" in order to be able to send cookies. + *

+ * The SockJS protocol also expects a SockJS service to echo back the JSESSIONID + * cookie when this property is set to true. However, when running in a Servlet + * container this is not necessary since the container takes care of it. + *

+ * The default value is "true" to maximize the chance for applications to work + * correctly in IE 8,9 with support for cookies (and the JSESSIONID cookie in + * particular). However, an application can choose to set this to "false" if + * the use of cookies (and HTTP session) is not required. */ - public void setDummySessionCookieEnabled(boolean sessionCookieEnabled) { - this.sessionCookieEnabled = sessionCookieEnabled; + public void setSessionCookieNeeded(boolean sessionCookieNeeded) { + this.sessionCookieNeeded = sessionCookieNeeded; } /** - * Whether setting JSESSIONID cookie is necessary. - * @see #setDummySessionCookieEnabled(boolean) + * Whether JSESSIONID cookie is required for the application to function. For + * more detail see {@link #setSessionCookieNeeded(boolean)}. */ - public boolean isDummySessionCookieEnabled() { - return this.sessionCookieEnabled; + public boolean isSessionCookieNeeded() { + return this.sessionCookieNeeded; } /** @@ -506,7 +519,7 @@ public abstract class AbstractSockJsService implements SockJsService { addCorsHeaders(request, response); addNoCacheHeaders(response); - String content = String.format(INFO_CONTENT, random.nextInt(), isDummySessionCookieEnabled(), isWebSocketEnabled()); + String content = String.format(INFO_CONTENT, random.nextInt(), isSessionCookieNeeded(), isWebSocketEnabled()); response.getBody().write(content.getBytes()); } else if (HttpMethod.OPTIONS.equals(request.getMethod())) { diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsService.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsService.java index eaf2d13e26..9752031f36 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsService.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsService.java @@ -29,7 +29,6 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledFuture; -import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; import org.springframework.http.server.ServerHttpRequest; @@ -306,11 +305,6 @@ public class DefaultSockJsService extends AbstractSockJsService { addNoCacheHeaders(response); } - if (transportType.sendsSessionCookie() && isDummySessionCookieEnabled()) { - String cookieValue = getJsessionIdCookieValue(request.getHeaders()); - response.getHeaders().set("Set-Cookie", "JSESSIONID=" + cookieValue + ";path=/"); - } - if (transportType.supportsCors()) { addCorsHeaders(request, response); } @@ -386,20 +380,6 @@ public class DefaultSockJsService extends AbstractSockJsService { }, getDisconnectDelay()); } - private String getJsessionIdCookieValue(HttpHeaders headers) { - List rawCookies = headers.get("Cookie"); - if (!CollectionUtils.isEmpty(rawCookies)) { - for (String rawCookie : rawCookies) { - if (rawCookie.startsWith("JSESSIONID=")) { - int start = "JSESSIONID=".length(); - int end = rawCookie.indexOf(';'); - return (end != -1) ? rawCookie.substring(start, end) : rawCookie.substring(start); - } - } - } - return "dummy"; - } - private final SockJsServiceConfig sockJsServiceConfig = new SockJsServiceConfig() { diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/AbstractSockJsServiceTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/AbstractSockJsServiceTests.java index cdf9efe3b6..6a947c56c6 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/AbstractSockJsServiceTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/AbstractSockJsServiceTests.java @@ -149,7 +149,7 @@ public class AbstractSockJsServiceTests extends AbstractHttpRequestTests { assertEquals(",\"origins\":[\"*:*\"],\"cookie_needed\":false,\"websocket\":true}", body.substring(body.indexOf(','))); - this.service.setDummySessionCookieEnabled(false); + this.service.setSessionCookieNeeded(false); this.service.setWebSocketsEnabled(false); handleRequest("GET", "/a/info", HttpStatus.OK); 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 ba1229a92b..ccadca15fe 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 @@ -151,40 +151,6 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests { assertEquals("OPTIONS, POST", this.response.getHeaders().getFirst("Access-Control-Allow-Methods")); } - @Test - public void dummySessionCookieEnabled() throws Exception { - - setRequest("POST", sessionUrlPrefix + "xhr"); - this.service.setDummySessionCookieEnabled(true); - this.service.handleRequest(this.request, this.response, this.wsHandler); - this.response.flush(); - - assertEquals(200, this.servletResponse.getStatus()); - assertEquals("JSESSIONID=dummy;path=/", this.servletResponse.getHeader("Set-Cookie")); - } - - @Test - public void dummySessionCookieDisabled() throws Exception { - - setRequest("POST", sessionUrlPrefix + "xhr"); - this.service.setDummySessionCookieEnabled(false); - this.service.handleTransportRequest(this.request, this.response, this.wsHandler, sessionId, "xhr"); - - assertEquals(200, this.servletResponse.getStatus()); - assertNull(this.servletResponse.getHeader("Set-Cookie")); - } - - @Test - public void dummySessionCookieReuseRequestCookieValue() throws Exception { - - setRequest("POST", sessionUrlPrefix + "xhr"); - this.servletRequest.addHeader("Cookie", "JSESSIONID=123456789"); - this.service.handleTransportRequest(this.request, this.response, this.wsHandler, sessionId, "xhr"); - - assertEquals(200, this.servletResponse.getStatus()); - assertNull(this.servletResponse.getHeader("Set-Cookie")); - } - @Test public void handleTransportRequestNoSuitableHandler() throws Exception {