From 0d5901ffb652a4daf2dd6ce17a9eb1f5ff54c369 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 2 Aug 2013 12:23:48 -0400 Subject: [PATCH] Polish Cookie abstraction in http packge of spring-web A getCookies method is now available on ServerHttpRequest with one ServletServerCookie implementation that wraps a Servlet cookie. The SockJS service makes use of this to check for an existing session cookie in the request. --- .../SubProtocolWebSocketHandlerTests.java | 12 +- .../mock/http/MockHttpInputMessage.java | 8 - .../mock/http/MockHttpOutputMessage.java | 7 - .../java/org/springframework/http/Cookie.java | 52 +++++- .../org/springframework/http/Cookies.java | 59 ------ .../springframework/http/DefaultCookie.java | 42 ----- .../org/springframework/http/HttpMessage.java | 5 - .../client/AbstractClientHttpRequest.java | 7 - .../client/AbstractClientHttpResponse.java | 7 - .../BufferingClientHttpRequestWrapper.java | 6 - .../BufferingClientHttpResponseWrapper.java | 6 - .../client/support/HttpRequestWrapper.java | 9 - .../AbstractHttpMessageConverter.java | 7 - .../converter/FormHttpMessageConverter.java | 7 - .../http/server/ServerHttpRequest.java | 8 + .../http/server/ServletServerCookie.java | 80 ++++++++ .../http/server/ServletServerHttpRequest.java | 15 +- .../server/ServletServerHttpResponse.java | 23 +-- .../http/MockHttpInputMessage.java | 7 - .../http/MockHttpOutputMessage.java | 10 +- ...rceptingClientHttpRequestFactoryTests.java | 16 -- .../sockjs/support/AbstractSockJsService.java | 19 +- .../sockjs/transport/TransportType.java | 2 +- .../handler/DefaultSockJsService.java | 11 +- .../support/AbstractSockJsServiceTests.java | 3 +- .../handler/DefaultSockJsServiceTests.java | 173 +++++++++--------- 26 files changed, 253 insertions(+), 348 deletions(-) delete mode 100644 spring-web/src/main/java/org/springframework/http/Cookies.java delete mode 100644 spring-web/src/main/java/org/springframework/http/DefaultCookie.java create mode 100644 spring-web/src/main/java/org/springframework/http/server/ServletServerCookie.java diff --git a/spring-messaging/src/test/java/org/springframework/messaging/handler/websocket/SubProtocolWebSocketHandlerTests.java b/spring-messaging/src/test/java/org/springframework/messaging/handler/websocket/SubProtocolWebSocketHandlerTests.java index ccc4d53e4f3..8654aeaddcc 100644 --- a/spring-messaging/src/test/java/org/springframework/messaging/handler/websocket/SubProtocolWebSocketHandlerTests.java +++ b/spring-messaging/src/test/java/org/springframework/messaging/handler/websocket/SubProtocolWebSocketHandlerTests.java @@ -40,17 +40,13 @@ public class SubProtocolWebSocketHandlerTests { private TestWebSocketSession session; - @Mock - SubProtocolHandler stompHandler; + @Mock SubProtocolHandler stompHandler; - @Mock - SubProtocolHandler mqttHandler; + @Mock SubProtocolHandler mqttHandler; - @Mock - SubProtocolHandler defaultHandler; + @Mock SubProtocolHandler defaultHandler; - @Mock - MessageChannel channel; + @Mock MessageChannel channel; @Before diff --git a/spring-test/src/main/java/org/springframework/mock/http/MockHttpInputMessage.java b/spring-test/src/main/java/org/springframework/mock/http/MockHttpInputMessage.java index 0ee6d4d8194..7e4bf5686ab 100644 --- a/spring-test/src/main/java/org/springframework/mock/http/MockHttpInputMessage.java +++ b/spring-test/src/main/java/org/springframework/mock/http/MockHttpInputMessage.java @@ -19,7 +19,6 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; -import org.springframework.http.Cookies; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpInputMessage; import org.springframework.util.Assert; @@ -36,8 +35,6 @@ public class MockHttpInputMessage implements HttpInputMessage { private final InputStream body; - private final Cookies cookies = new Cookies(); - public MockHttpInputMessage(byte[] contents) { this.body = (contents != null) ? new ByteArrayInputStream(contents) : null; @@ -57,9 +54,4 @@ public class MockHttpInputMessage implements HttpInputMessage { public InputStream getBody() throws IOException { return this.body; } - - @Override - public Cookies getCookies() { - return this.cookies ; - } } diff --git a/spring-test/src/main/java/org/springframework/mock/http/MockHttpOutputMessage.java b/spring-test/src/main/java/org/springframework/mock/http/MockHttpOutputMessage.java index df2995a6ea5..8f31f553bbd 100644 --- a/spring-test/src/main/java/org/springframework/mock/http/MockHttpOutputMessage.java +++ b/spring-test/src/main/java/org/springframework/mock/http/MockHttpOutputMessage.java @@ -21,7 +21,6 @@ import java.io.OutputStream; import java.io.UnsupportedEncodingException; import java.nio.charset.Charset; -import org.springframework.http.Cookies; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpOutputMessage; @@ -39,7 +38,6 @@ public class MockHttpOutputMessage implements HttpOutputMessage { private final ByteArrayOutputStream body = new ByteArrayOutputStream(); - private final Cookies cookies = new Cookies(); /** * Return the headers. @@ -87,9 +85,4 @@ public class MockHttpOutputMessage implements HttpOutputMessage { } } - @Override - public Cookies getCookies() { - return this.cookies; - } - } diff --git a/spring-web/src/main/java/org/springframework/http/Cookie.java b/spring-web/src/main/java/org/springframework/http/Cookie.java index a60c73c87ff..3c81ad42c14 100644 --- a/spring-web/src/main/java/org/springframework/http/Cookie.java +++ b/spring-web/src/main/java/org/springframework/http/Cookie.java @@ -13,13 +13,61 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package org.springframework.http; +/** + * Representation of a cookie value parsed from a "Cookie" request header or a + * "Set-Cookie" response header. + * + * @author Rossen Stoyanchev + * @since 4.0 + * + * @see http://www.ietf.org/rfc/rfc2109.txt + */ public interface Cookie { - String getName(); + /** + * Returns the name of the cookie. + */ + String getName(); + + /** + * Returns the value of the cookie. + */ + String getValue(); + + /** + * Returns the path on the server to which the browser returns this cookie. + */ + String getPath(); + + /** + * Returns the comment describing the purpose of this cookie. + */ + String getComment(); + + /** + * Returns the domain name set for this cookie. + */ + String getDomain(); + + /** + * Returns the maximum age of the cookie, specified in seconds. + */ + int getMaxAge(); + + /** + * Returns true if the browser is sending cookies only over a + * secure protocol, or false if the browser can send cookies + * using any protocol. + */ + boolean isSecure(); - String getValue(); + /** + * Sets the version of the cookie protocol this cookie complies with. + */ + int getVersion(); } diff --git a/spring-web/src/main/java/org/springframework/http/Cookies.java b/spring-web/src/main/java/org/springframework/http/Cookies.java deleted file mode 100644 index 7dc13536c93..00000000000 --- a/spring-web/src/main/java/org/springframework/http/Cookies.java +++ /dev/null @@ -1,59 +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.http; - -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; - - -public class Cookies { - - private final List cookies; - - - public Cookies() { - this.cookies = new ArrayList(); - } - - private Cookies(Cookies cookies) { - this.cookies = Collections.unmodifiableList(cookies.getCookies()); - } - - public static Cookies readOnlyCookies(Cookies cookies) { - return new Cookies(cookies); - } - - public List getCookies() { - return this.cookies; - } - - public Cookie getCookie(String name) { - for (Cookie c : this.cookies) { - if (c.getName().equals(name)) { - return c; - } - } - return null; - } - - public Cookie addCookie(String name, String value) { - DefaultCookie cookie = new DefaultCookie(name, value); - this.cookies.add(cookie); - return cookie; - } - -} diff --git a/spring-web/src/main/java/org/springframework/http/DefaultCookie.java b/spring-web/src/main/java/org/springframework/http/DefaultCookie.java deleted file mode 100644 index b971d33fc57..00000000000 --- a/spring-web/src/main/java/org/springframework/http/DefaultCookie.java +++ /dev/null @@ -1,42 +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.http; - -import org.springframework.util.Assert; - -public class DefaultCookie implements Cookie { - - private final String name; - - private final String value; - - DefaultCookie(String name, String value) { - Assert.hasText(name, "cookie name must not be empty"); - this.name = name; - this.value = value; - } - - @Override - public String getName() { - return name; - } - - @Override - public String getValue() { - return value; - } - -} diff --git a/spring-web/src/main/java/org/springframework/http/HttpMessage.java b/spring-web/src/main/java/org/springframework/http/HttpMessage.java index 05824e67abd..80f7ca292d5 100644 --- a/spring-web/src/main/java/org/springframework/http/HttpMessage.java +++ b/spring-web/src/main/java/org/springframework/http/HttpMessage.java @@ -31,9 +31,4 @@ public interface HttpMessage { */ HttpHeaders getHeaders(); - /** - * TODO .. - */ - Cookies getCookies(); - } diff --git a/spring-web/src/main/java/org/springframework/http/client/AbstractClientHttpRequest.java b/spring-web/src/main/java/org/springframework/http/client/AbstractClientHttpRequest.java index 871ca3c8f3c..0b2c3595496 100644 --- a/spring-web/src/main/java/org/springframework/http/client/AbstractClientHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/client/AbstractClientHttpRequest.java @@ -19,7 +19,6 @@ package org.springframework.http.client; import java.io.IOException; import java.io.OutputStream; -import org.springframework.http.Cookies; import org.springframework.http.HttpHeaders; import org.springframework.util.Assert; @@ -47,12 +46,6 @@ public abstract class AbstractClientHttpRequest implements ClientHttpRequest { return getBodyInternal(this.headers); } - @Override - public Cookies getCookies() { - // TODO - throw new UnsupportedOperationException(); - } - @Override public final ClientHttpResponse execute() throws IOException { assertNotExecuted(); diff --git a/spring-web/src/main/java/org/springframework/http/client/AbstractClientHttpResponse.java b/spring-web/src/main/java/org/springframework/http/client/AbstractClientHttpResponse.java index f7ccdd98743..84a6d9322be 100644 --- a/spring-web/src/main/java/org/springframework/http/client/AbstractClientHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/client/AbstractClientHttpResponse.java @@ -18,7 +18,6 @@ package org.springframework.http.client; import java.io.IOException; -import org.springframework.http.Cookies; import org.springframework.http.HttpStatus; /** @@ -34,10 +33,4 @@ public abstract class AbstractClientHttpResponse implements ClientHttpResponse { return HttpStatus.valueOf(getRawStatusCode()); } - @Override - public Cookies getCookies() { - // TODO - throw new UnsupportedOperationException(); - } - } diff --git a/spring-web/src/main/java/org/springframework/http/client/BufferingClientHttpRequestWrapper.java b/spring-web/src/main/java/org/springframework/http/client/BufferingClientHttpRequestWrapper.java index 95a628bb6ce..204dc67d7d6 100644 --- a/spring-web/src/main/java/org/springframework/http/client/BufferingClientHttpRequestWrapper.java +++ b/spring-web/src/main/java/org/springframework/http/client/BufferingClientHttpRequestWrapper.java @@ -19,7 +19,6 @@ package org.springframework.http.client; import java.io.IOException; import java.net.URI; -import org.springframework.http.Cookies; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.util.Assert; @@ -60,9 +59,4 @@ final class BufferingClientHttpRequestWrapper extends AbstractBufferingClientHtt return new BufferingClientHttpResponseWrapper(response); } - @Override - public Cookies getCookies() { - return this.request.getCookies(); - } - } diff --git a/spring-web/src/main/java/org/springframework/http/client/BufferingClientHttpResponseWrapper.java b/spring-web/src/main/java/org/springframework/http/client/BufferingClientHttpResponseWrapper.java index 958abfad29f..5c3ab74e519 100644 --- a/spring-web/src/main/java/org/springframework/http/client/BufferingClientHttpResponseWrapper.java +++ b/spring-web/src/main/java/org/springframework/http/client/BufferingClientHttpResponseWrapper.java @@ -20,7 +20,6 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; -import org.springframework.http.Cookies; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.util.StreamUtils; @@ -72,11 +71,6 @@ final class BufferingClientHttpResponseWrapper implements ClientHttpResponse { return new ByteArrayInputStream(this.body); } - @Override - public Cookies getCookies() { - return this.response.getCookies(); - } - @Override public void close() { this.response.close(); diff --git a/spring-web/src/main/java/org/springframework/http/client/support/HttpRequestWrapper.java b/spring-web/src/main/java/org/springframework/http/client/support/HttpRequestWrapper.java index 46e893c54b0..d66476d9141 100644 --- a/spring-web/src/main/java/org/springframework/http/client/support/HttpRequestWrapper.java +++ b/spring-web/src/main/java/org/springframework/http/client/support/HttpRequestWrapper.java @@ -18,7 +18,6 @@ package org.springframework.http.client.support; import java.net.URI; -import org.springframework.http.Cookies; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpRequest; @@ -77,12 +76,4 @@ public class HttpRequestWrapper implements HttpRequest { return this.request.getHeaders(); } - /** - * Returns the cookies of the wrapped request. - */ - @Override - public Cookies getCookies() { - return this.request.getCookies(); - } - } diff --git a/spring-web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java index 64b2ea33ddb..5e095b58ffa 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/AbstractHttpMessageConverter.java @@ -25,8 +25,6 @@ import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; - -import org.springframework.http.Cookies; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpInputMessage; import org.springframework.http.HttpOutputMessage; @@ -201,11 +199,6 @@ public abstract class AbstractHttpMessageConverter implements HttpMessageConv public HttpHeaders getHeaders() { return headers; } - - @Override - public Cookies getCookies() { - return null; - } }); } }); diff --git a/spring-web/src/main/java/org/springframework/http/converter/FormHttpMessageConverter.java b/spring-web/src/main/java/org/springframework/http/converter/FormHttpMessageConverter.java index 28d2d055512..e91f05d15f5 100644 --- a/spring-web/src/main/java/org/springframework/http/converter/FormHttpMessageConverter.java +++ b/spring-web/src/main/java/org/springframework/http/converter/FormHttpMessageConverter.java @@ -30,7 +30,6 @@ import java.util.Map; import java.util.Random; import org.springframework.core.io.Resource; -import org.springframework.http.Cookies; import org.springframework.http.HttpEntity; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpInputMessage; @@ -391,12 +390,6 @@ public class FormHttpMessageConverter implements HttpMessageConverter> entry : this.headers.entrySet()) { diff --git a/spring-web/src/main/java/org/springframework/http/server/ServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/ServerHttpRequest.java index c765fd34926..8e6a5647e22 100644 --- a/spring-web/src/main/java/org/springframework/http/server/ServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/ServerHttpRequest.java @@ -17,7 +17,9 @@ package org.springframework.http.server; import java.security.Principal; +import java.util.Map; +import org.springframework.http.Cookie; import org.springframework.http.HttpInputMessage; import org.springframework.http.HttpRequest; import org.springframework.util.MultiValueMap; @@ -35,6 +37,12 @@ public interface ServerHttpRequest extends HttpRequest, HttpInputMessage { */ MultiValueMap getQueryParams(); + /** + * Return the cookie values parsed from the "Cookie" request header. + * @return the cookies + */ + Map getCookies(); + /** * Return a {@link java.security.Principal} instance containing the name of the * authenticated user. If the user has not been authenticated, the method returns diff --git a/spring-web/src/main/java/org/springframework/http/server/ServletServerCookie.java b/spring-web/src/main/java/org/springframework/http/server/ServletServerCookie.java new file mode 100644 index 00000000000..96af943b795 --- /dev/null +++ b/spring-web/src/main/java/org/springframework/http/server/ServletServerCookie.java @@ -0,0 +1,80 @@ +/* + * Copyright 2002-2012 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.http.server; + +import org.springframework.http.Cookie; + + +/** + * A {@link Cookie} that wraps a {@link javax.servlet.http.Cookie}. + * + * @author Rossen Stoyanchev + * @since 4.0 + */ +public class ServletServerCookie implements Cookie { + + private final javax.servlet.http.Cookie servletCookie; + + + public ServletServerCookie(javax.servlet.http.Cookie servletCookie) { + this.servletCookie = servletCookie; + } + + @Override + public String getName() { + return this.servletCookie.getName(); + } + + @Override + public String getValue() { + return this.servletCookie.getValue(); + } + + @Override + public String getPath() { + return this.servletCookie.getPath(); + } + + @Override + public String getComment() { + return this.servletCookie.getComment(); + } + + @Override + public String getDomain() { + return this.servletCookie.getDomain(); + } + + @Override + public int getMaxAge() { + return this.servletCookie.getMaxAge(); + } + + @Override + public boolean isSecure() { + return this.servletCookie.getSecure(); + } + + @Override + public int getVersion() { + return this.servletCookie.getVersion(); + } + + @Override + public String toString() { + return "ServletServerCookie [servletCookie=" + this.servletCookie + "]"; + } +} diff --git a/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpRequest.java b/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpRequest.java index eb97233b73f..b6cda3d97c9 100644 --- a/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpRequest.java +++ b/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpRequest.java @@ -28,16 +28,16 @@ import java.net.URLEncoder; import java.nio.charset.Charset; import java.security.Principal; import java.util.Arrays; +import java.util.Collections; import java.util.Enumeration; import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; -import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; -import org.springframework.http.Cookies; +import org.springframework.http.Cookie; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; @@ -63,7 +63,7 @@ public class ServletServerHttpRequest implements ServerHttpRequest { private HttpHeaders headers; - private Cookies cookies; + private Map cookies; private MultiValueMap queryParams; @@ -151,14 +151,15 @@ public class ServletServerHttpRequest implements ServerHttpRequest { } @Override - public Cookies getCookies() { + public Map getCookies() { if (this.cookies == null) { - this.cookies = new Cookies(); + this.cookies = new HashMap(); if (this.servletRequest.getCookies() != null) { - for (Cookie cookie : this.servletRequest.getCookies()) { - this.cookies.addCookie(cookie.getName(), cookie.getValue()); + for (javax.servlet.http.Cookie cookie : this.servletRequest.getCookies()) { + this.cookies.put(cookie.getName(), new ServletServerCookie(cookie)); } } + this.cookies = Collections.unmodifiableMap(this.cookies); } return this.cookies; } diff --git a/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpResponse.java b/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpResponse.java index ccdf58f475a..b65d0cdcada 100644 --- a/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/server/ServletServerHttpResponse.java @@ -20,10 +20,9 @@ import java.io.IOException; import java.io.OutputStream; import java.util.List; import java.util.Map; + import javax.servlet.http.HttpServletResponse; -import org.springframework.http.Cookie; -import org.springframework.http.Cookies; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.util.Assert; @@ -42,8 +41,6 @@ public class ServletServerHttpResponse implements ServerHttpResponse { private boolean headersWritten = false; - private final Cookies cookies = new Cookies(); - /** * Construct a new instance of the ServletServerHttpResponse based on the given {@link HttpServletResponse}. @@ -72,28 +69,20 @@ public class ServletServerHttpResponse implements ServerHttpResponse { return (this.headersWritten ? HttpHeaders.readOnlyHttpHeaders(this.headers) : this.headers); } - @Override - public Cookies getCookies() { - return (this.headersWritten ? Cookies.readOnlyCookies(this.cookies) : this.cookies); - } - @Override public OutputStream getBody() throws IOException { - writeCookies(); writeHeaders(); return this.servletResponse.getOutputStream(); } @Override public void flush() throws IOException { - writeCookies(); writeHeaders(); this.servletResponse.flushBuffer(); } @Override public void close() { - writeCookies(); writeHeaders(); } @@ -116,14 +105,4 @@ public class ServletServerHttpResponse implements ServerHttpResponse { this.headersWritten = true; } } - - private void writeCookies() { - if (!this.headersWritten) { - for (Cookie source : this.cookies.getCookies()) { - javax.servlet.http.Cookie target = new javax.servlet.http.Cookie(source.getName(), source.getValue()); - target.setPath("/"); - this.servletResponse.addCookie(target); - } - } - } } diff --git a/spring-web/src/test/java/org/springframework/http/MockHttpInputMessage.java b/spring-web/src/test/java/org/springframework/http/MockHttpInputMessage.java index 0ca8a29aca6..f7b4f45b9a1 100644 --- a/spring-web/src/test/java/org/springframework/http/MockHttpInputMessage.java +++ b/spring-web/src/test/java/org/springframework/http/MockHttpInputMessage.java @@ -31,8 +31,6 @@ public class MockHttpInputMessage implements HttpInputMessage { private final InputStream body; - private final Cookies cookies = new Cookies(); - public MockHttpInputMessage(byte[] contents) { Assert.notNull(contents, "'contents' must not be null"); @@ -53,9 +51,4 @@ public class MockHttpInputMessage implements HttpInputMessage { public InputStream getBody() throws IOException { return body; } - - @Override - public Cookies getCookies() { - return this.cookies ; - } } diff --git a/spring-web/src/test/java/org/springframework/http/MockHttpOutputMessage.java b/spring-web/src/test/java/org/springframework/http/MockHttpOutputMessage.java index 3287a7d93f9..dbb39e62b9b 100644 --- a/spring-web/src/test/java/org/springframework/http/MockHttpOutputMessage.java +++ b/spring-web/src/test/java/org/springframework/http/MockHttpOutputMessage.java @@ -21,7 +21,7 @@ import java.io.IOException; import java.io.OutputStream; import java.nio.charset.Charset; -import static org.mockito.BDDMockito.*; +import static org.mockito.Mockito.*; /** * @author Arjen Poutsma @@ -32,9 +32,6 @@ public class MockHttpOutputMessage implements HttpOutputMessage { private final ByteArrayOutputStream body = spy(new ByteArrayOutputStream()); - private final Cookies cookies = new Cookies(); - - @Override public HttpHeaders getHeaders() { return headers; @@ -53,9 +50,4 @@ public class MockHttpOutputMessage implements HttpOutputMessage { byte[] bytes = getBodyAsBytes(); return new String(bytes, charset); } - - @Override - public Cookies getCookies() { - return this.cookies; - } } diff --git a/spring-web/src/test/java/org/springframework/http/client/InterceptingClientHttpRequestFactoryTests.java b/spring-web/src/test/java/org/springframework/http/client/InterceptingClientHttpRequestFactoryTests.java index 0358bf42bff..6b77577b805 100644 --- a/spring-web/src/test/java/org/springframework/http/client/InterceptingClientHttpRequestFactoryTests.java +++ b/spring-web/src/test/java/org/springframework/http/client/InterceptingClientHttpRequestFactoryTests.java @@ -28,8 +28,6 @@ import java.util.List; import org.junit.Before; import org.junit.Test; - -import org.springframework.http.Cookies; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpRequest; @@ -254,8 +252,6 @@ public class InterceptingClientHttpRequestFactoryTests { private boolean executed = false; - private Cookies cookies = new Cookies(); - private RequestMock() { } @@ -292,11 +288,6 @@ public class InterceptingClientHttpRequestFactoryTests { executed = true; return responseMock; } - - @Override - public Cookies getCookies() { - return this.cookies ; - } } private static class ResponseMock implements ClientHttpResponse { @@ -307,8 +298,6 @@ public class InterceptingClientHttpRequestFactoryTests { private HttpHeaders headers = new HttpHeaders(); - private Cookies cookies = new Cookies(); - @Override public HttpStatus getStatusCode() throws IOException { return statusCode; @@ -337,10 +326,5 @@ public class InterceptingClientHttpRequestFactoryTests { @Override public void close() { } - - @Override - public Cookies getCookies() { - return this.cookies ; - } } } 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 f80c92ddbf4..dbd351924cf 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 @@ -78,7 +78,7 @@ public abstract class AbstractSockJsService implements SockJsService { private int streamBytesLimit = 128 * 1024; - private boolean jsessionIdCookieRequired = true; + private boolean sessionCookieEnabled = false; private long heartbeatTime = 25 * 1000; @@ -187,23 +187,22 @@ public abstract class AbstractSockJsService implements SockJsService { } /** - * Some load balancers do sticky sessions, but only if there is a JSESSIONID + * 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. * - *

Set this option to indicate if a JSESSIONID cookie should be created. The - * default value is "true". + *

The default value is "false" since Java servers set the session cookie. */ - public void setJsessionIdCookieRequired(boolean jsessionIdCookieRequired) { - this.jsessionIdCookieRequired = jsessionIdCookieRequired; + public void setDummySessionCookieEnabled(boolean sessionCookieEnabled) { + this.sessionCookieEnabled = sessionCookieEnabled; } /** * Whether setting JSESSIONID cookie is necessary. - * @see #setJsessionIdCookieRequired(boolean) + * @see #setDummySessionCookieEnabled(boolean) */ - public boolean isJsessionIdCookieRequired() { - return this.jsessionIdCookieRequired; + public boolean isDummySessionCookieEnabled() { + return this.sessionCookieEnabled; } /** @@ -476,7 +475,7 @@ public abstract class AbstractSockJsService implements SockJsService { addCorsHeaders(request, response); addNoCacheHeaders(response); - String content = String.format(INFO_CONTENT, random.nextInt(), isJsessionIdCookieRequired(), isWebSocketEnabled()); + String content = String.format(INFO_CONTENT, random.nextInt(), isDummySessionCookieEnabled(), 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/TransportType.java b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportType.java index f9a93d5c1e3..1a17c7f780a 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportType.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportType.java @@ -91,7 +91,7 @@ public enum TransportType { return this.headerHints.contains("cors"); } - public boolean setsJsessionId() { + public boolean sendsSessionCookie() { return this.headerHints.contains("jsessionid"); } 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 2248664a90b..4c01c0b4172 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 @@ -118,7 +118,7 @@ public class DefaultSockJsService extends AbstractSockJsService { * @param transportHandlerOverrides zero or more overrides to the default transport * handler types. */ - public DefaultSockJsService(TaskScheduler taskScheduler, Set transportHandlers, + public DefaultSockJsService(TaskScheduler taskScheduler, Collection transportHandlers, TransportHandler... transportHandlerOverrides) { super(taskScheduler); @@ -254,11 +254,10 @@ public class DefaultSockJsService extends AbstractSockJsService { addNoCacheHeaders(response); } - if (transportType.setsJsessionId() && isJsessionIdCookieRequired()) { - Cookie cookie = request.getCookies().getCookie("JSESSIONID"); - String jsid = (cookie != null) ? cookie.getValue() : "dummy"; - // TODO: bypass use of Cookie object (causes Jetty to set Expires header) - response.getHeaders().set("Set-Cookie", "JSESSIONID=" + jsid + ";path=/"); + if (transportType.sendsSessionCookie() && isDummySessionCookieEnabled()) { + Cookie cookie = request.getCookies().get("JSESSIONID"); + String value = (cookie != null) ? cookie.getValue() : "dummy"; + response.getHeaders().set("Set-Cookie", "JSESSIONID=" + value + ";path=/"); } if (transportType.supportsCors()) { 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 deabe9bf00d..2e34aa92f78 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\":true,\"websocket\":true}", body.substring(body.indexOf(','))); - this.service.setJsessionIdCookieRequired(false); + this.service.setDummySessionCookieEnabled(false); this.service.setWebSocketsEnabled(false); handleRequest("GET", "/a/info", HttpStatus.OK); @@ -214,6 +214,7 @@ public class AbstractSockJsServiceTests extends AbstractHttpRequestTests { assertEquals(httpStatus.value(), this.servletResponse.getStatus()); } + private static class TestSockJsService extends AbstractSockJsService { private String sessionId; 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 c91fe926ff4..0d64d79e6f2 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 @@ -16,26 +16,18 @@ package org.springframework.web.socket.sockjs.transport.handler; -import java.util.Collections; -import java.util.HashSet; +import java.util.Arrays; import java.util.Map; -import java.util.Set; import org.junit.Before; import org.junit.Test; -import org.springframework.http.HttpStatus; -import org.springframework.http.server.ServerHttpRequest; -import org.springframework.http.server.ServerHttpResponse; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; import org.springframework.scheduling.TaskScheduler; import org.springframework.web.socket.AbstractHttpRequestTests; import org.springframework.web.socket.WebSocketHandler; -import org.springframework.web.socket.WebSocketSession; -import org.springframework.web.socket.sockjs.SockJsProcessingException; import org.springframework.web.socket.sockjs.transport.TransportHandler; import org.springframework.web.socket.sockjs.transport.TransportType; -import org.springframework.web.socket.sockjs.transport.handler.DefaultSockJsService; -import org.springframework.web.socket.sockjs.transport.handler.SockJsSessionFactory; -import org.springframework.web.socket.sockjs.transport.session.AbstractSockJsSession; import org.springframework.web.socket.sockjs.transport.session.StubSockJsServiceConfig; import org.springframework.web.socket.sockjs.transport.session.TestSockJsSession; @@ -50,10 +42,42 @@ import static org.mockito.Mockito.*; */ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests { - @Override + private static final String sockJsPrefix = "mysockjs"; + + private static final String sessionId = "session1"; + + private static final String sessionUrlPrefix = "/mysockjs/server1/" + sessionId + "/"; + + + @Mock private SessionCreatingTransportHandler xhrHandler; + + @Mock private TransportHandler xhrSendHandler; + + @Mock private WebSocketHandler wsHandler; + + @Mock private TaskScheduler taskScheduler; + + private TestSockJsSession session; + + private DefaultSockJsService service; + + @Before - public void setUp() { + public void setup() { + super.setUp(); + + MockitoAnnotations.initMocks(this); + + this.session = new TestSockJsSession(sessionId, new StubSockJsServiceConfig(), this.wsHandler); + + when(this.xhrHandler.getTransportType()).thenReturn(TransportType.XHR); + when(this.xhrHandler.createSession(sessionId, this.wsHandler)).thenReturn(this.session); + when(this.xhrSendHandler.getTransportType()).thenReturn(TransportType.XHR_SEND); + + this.service = new DefaultSockJsService(this.taskScheduler, + Arrays.asList(this.xhrHandler, this.xhrSendHandler)); + this.service.setValidSockJsPrefixes(sockJsPrefix); } @Test @@ -76,24 +100,14 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests { @Test public void handleTransportRequestXhr() throws Exception { - setRequest("POST", "/a/server/session/xhr"); - - TaskScheduler taskScheduler = mock(TaskScheduler.class); - StubXhrTransportHandler xhrHandler = new StubXhrTransportHandler(); - Set transportHandlers = Collections.singleton(xhrHandler); - WebSocketHandler webSocketHandler = mock(WebSocketHandler.class); - - DefaultSockJsService service = new DefaultSockJsService(taskScheduler, transportHandlers); - service.handleTransportRequest(this.request, this.response, webSocketHandler, "123", TransportType.XHR.value()); + setRequest("POST", sessionUrlPrefix + "xhr"); + this.service.handleRequest(this.request, this.response, this.wsHandler); assertEquals(200, this.servletResponse.getStatus()); - assertNotNull(xhrHandler.session); - assertSame(webSocketHandler, xhrHandler.webSocketHandler); - + verify(this.xhrHandler).handleRequest(this.request, this.response, this.wsHandler, this.session); verify(taskScheduler).scheduleAtFixedRate(any(Runnable.class), eq(service.getDisconnectDelay())); assertEquals("no-store, no-cache, must-revalidate, max-age=0", this.response.getHeaders().getCacheControl()); - assertEquals("JSESSIONID=dummy;path=/", this.response.getHeaders().getFirst("Set-Cookie")); assertEquals("*", this.response.getHeaders().getFirst("Access-Control-Allow-Origin")); assertEquals("true", this.response.getHeaders().getFirst("Access-Control-Allow-Credentials")); } @@ -101,14 +115,8 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests { @Test public void handleTransportRequestXhrOptions() throws Exception { - setRequest("OPTIONS", "/a/server/session/xhr"); - - TaskScheduler taskScheduler = mock(TaskScheduler.class); - StubXhrTransportHandler xhrHandler = new StubXhrTransportHandler(); - Set transportHandlers = Collections.singleton(xhrHandler); - - DefaultSockJsService service = new DefaultSockJsService(taskScheduler, transportHandlers); - service.handleTransportRequest(this.request, this.response, null, "123", TransportType.XHR.value()); + setRequest("OPTIONS", sessionUrlPrefix + "xhr"); + this.service.handleRequest(this.request, this.response, this.wsHandler); assertEquals(204, this.servletResponse.getStatus()); assertEquals("*", this.response.getHeaders().getFirst("Access-Control-Allow-Origin")); @@ -117,85 +125,72 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests { } @Test - public void handleTransportRequestNoSuitableHandler() throws Exception { - - setRequest("POST", "/a/server/session/xhr"); + public void dummySessionCookieEnabled() throws Exception { - Set transportHandlers = new HashSet<>(); - DefaultSockJsService service = new DefaultSockJsService(mock(TaskScheduler.class), transportHandlers); - service.handleTransportRequest(this.request, this.response, null, "123", TransportType.XHR.value()); + setRequest("POST", sessionUrlPrefix + "xhr"); + this.service.setDummySessionCookieEnabled(true); + this.service.handleRequest(this.request, this.response, this.wsHandler); - assertEquals(404, this.servletResponse.getStatus()); + assertEquals(200, this.servletResponse.getStatus()); + assertEquals("JSESSIONID=dummy;path=/", this.servletResponse.getHeader("Set-Cookie")); } @Test - public void handleTransportRequestXhrSend() throws Exception { - - this.servletRequest.setMethod("POST"); - - Set transportHandlers = new HashSet<>(); - transportHandlers.add(new StubXhrTransportHandler()); - transportHandlers.add(new StubXhrSendTransportHandler()); - WebSocketHandler wsHandler = mock(WebSocketHandler.class); - DefaultSockJsService service = new DefaultSockJsService(mock(TaskScheduler.class), transportHandlers); - - service.handleTransportRequest(this.request, this.response, wsHandler, "123", TransportType.XHR_SEND.value()); + public void dummySessionCookieDisabled() throws Exception { - assertEquals(404, this.servletResponse.getStatus()); // dropped (no session) - - resetResponse(); - service.handleTransportRequest(this.request, this.response, wsHandler, "123", TransportType.XHR.value()); + 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")); + } - resetResponse(); - service.handleTransportRequest(this.request, this.response, wsHandler, "123", TransportType.XHR_SEND.value()); + @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 { - private static class StubXhrTransportHandler implements TransportHandler, SockJsSessionFactory { - - WebSocketHandler webSocketHandler; + setRequest("POST", sessionUrlPrefix + "eventsource"); + this.service.handleRequest(this.request, this.response, this.wsHandler); - WebSocketSession session; + assertEquals(404, this.servletResponse.getStatus()); + } - @Override - public TransportType getTransportType() { - return TransportType.XHR; - } + @Test + public void handleTransportRequestXhrSend() throws Exception { - @Override - public void handleRequest(ServerHttpRequest request, ServerHttpResponse response, - WebSocketHandler handler, WebSocketSession session) throws SockJsProcessingException { + setRequest("POST", sessionUrlPrefix + "xhr_send"); + this.service.handleRequest(this.request, this.response, this.wsHandler); - this.webSocketHandler = handler; - this.session = session; - } + assertEquals(404, this.servletResponse.getStatus()); // no session yet - @Override - public AbstractSockJsSession createSession(String sessionId, WebSocketHandler webSocketHandler) { - return new TestSockJsSession(sessionId, new StubSockJsServiceConfig(), webSocketHandler); - } + resetResponse(); + setRequest("POST", sessionUrlPrefix + "xhr"); + this.service.handleRequest(this.request, this.response, this.wsHandler); - } + assertEquals(200, this.servletResponse.getStatus()); // session created + verify(this.xhrHandler).handleRequest(this.request, this.response, this.wsHandler, this.session); - private static class StubXhrSendTransportHandler implements TransportHandler { + resetResponse(); + setRequest("POST", sessionUrlPrefix + "xhr_send"); + this.service.handleRequest(this.request, this.response, this.wsHandler); - @Override - public TransportType getTransportType() { - return TransportType.XHR_SEND; - } + assertEquals(200, this.servletResponse.getStatus()); // session exists + verify(this.xhrSendHandler).handleRequest(this.request, this.response, this.wsHandler, this.session); + } - @Override - public void handleRequest(ServerHttpRequest request, ServerHttpResponse response, - WebSocketHandler handler, WebSocketSession session) throws SockJsProcessingException { - if (session == null) { - response.setStatusCode(HttpStatus.NOT_FOUND); - } - } + interface SessionCreatingTransportHandler extends TransportHandler, SockJsSessionFactory { } }