diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/AbstractWebSocketHandlerRegistration.java b/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/AbstractWebSocketHandlerRegistration.java index 679b82fa5cd..b326590d4ff 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/AbstractWebSocketHandlerRegistration.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/AbstractWebSocketHandlerRegistration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -89,6 +89,7 @@ public abstract class AbstractWebSocketHandlerRegistration implements WebSock @Override public WebSocketHandlerRegistration setAllowedOrigins(String... origins) { + Assert.notEmpty(origins, "No allowed origin specified"); this.allowedOrigins.clear(); if (!ObjectUtils.isEmpty(origins)) { this.allowedOrigins.addAll(Arrays.asList(origins)); diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/StompWebSocketEndpointRegistration.java b/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/StompWebSocketEndpointRegistration.java index c0bea5565e7..877eedb8144 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/StompWebSocketEndpointRegistration.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/StompWebSocketEndpointRegistration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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,16 +43,20 @@ public interface StompWebSocketEndpointRegistration { StompWebSocketEndpointRegistration addInterceptors(HandshakeInterceptor... interceptors); /** - * Configure allowed {@code Origin} header values. This check is mostly designed for browser - * clients. There is noting preventing other types of client to modify the Origin header value. + * Configure allowed {@code Origin} header values. This check is mostly designed for + * browser clients. There is nothing preventing other types of client to modify the + * {@code Origin} header value. * - *

When SockJS is enabled and allowed origins are restricted, transport types that do not - * use {@code Origin} headers for cross origin requests (jsonp-polling, iframe-xhr-polling, - * iframe-eventsource and iframe-htmlfile) are disabled. As a consequence, IE6/IE7 won't be - * supported anymore and IE8/IE9 will only be supported without cookies. + *

When SockJS is enabled and origins are restricted, transport types that do not + * allow to check request origin (JSONP and Iframe based transports) are disabled. + * As a consequence, IE 6 to 9 are not supported when origins are restricted. + * + *

Each provided allowed origin must start by "http://", "https://" or be "*" + * (means that all origins are allowed). Empty allowed origin list is not supported. + * By default, all origins are allowed. * - *

By default, all origins are allowed. * @since 4.1.2 + * @see RFC 6454: The Web Origin Concept * @see SockJS supported transports by browser */ StompWebSocketEndpointRegistration setAllowedOrigins(String... origins); diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/WebMvcStompWebSocketEndpointRegistration.java b/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/WebMvcStompWebSocketEndpointRegistration.java index 3ef359d27e0..467dee2d24e 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/WebMvcStompWebSocketEndpointRegistration.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/WebMvcStompWebSocketEndpointRegistration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -86,10 +86,9 @@ public class WebMvcStompWebSocketEndpointRegistration implements StompWebSocketE @Override public StompWebSocketEndpointRegistration setAllowedOrigins(String... origins) { + Assert.notEmpty(origins, "No allowed origin specified"); this.allowedOrigins.clear(); - if (!ObjectUtils.isEmpty(origins)) { - this.allowedOrigins.addAll(Arrays.asList(origins)); - } + this.allowedOrigins.addAll(Arrays.asList(origins)); return this; } diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/WebSocketHandlerRegistration.java b/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/WebSocketHandlerRegistration.java index 4deb3f7c104..eb7eee0eb1e 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/WebSocketHandlerRegistration.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/config/annotation/WebSocketHandlerRegistration.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2015 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. @@ -45,17 +45,20 @@ public interface WebSocketHandlerRegistration { WebSocketHandlerRegistration addInterceptors(HandshakeInterceptor... interceptors); /** - * Configure allowed {@code Origin} header values. This check is mostly designed for browser - * clients. There is noting preventing other types of client to modify the Origin header value. + * Configure allowed {@code Origin} header values. This check is mostly designed for + * browser clients. There is nothing preventing other types of client to modify the + * {@code Origin} header value. * - *

When SockJS is enabled and allowed origins are restricted, transport types that do not - * use {@code Origin} headers for cross origin requests (jsonp-polling, iframe-xhr-polling, - * iframe-eventsource and iframe-htmlfile) are disabled. As a consequence, IE6/IE7 won't be - * supported anymore and IE8/IE9 will only be supported without cookies. + *

When SockJS is enabled and origins are restricted, transport types that do not + * allow to check request origin (JSONP and Iframe based transports) are disabled. + * As a consequence, IE 6 to 9 are not supported when origins are restricted. * - *

By default, all origins are allowed. + *

Each provided allowed origin must start by "http://", "https://" or be "*" + * (means that all origins are allowed). Empty allowed origin list is not supported. + * By default, all origins are allowed. * * @since 4.1.2 + * @see RFC 6454: The Web Origin Concept * @see SockJS supported transports by browser */ WebSocketHandlerRegistration setAllowedOrigins(String... origins); diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/server/support/OriginHandshakeInterceptor.java b/spring-websocket/src/main/java/org/springframework/web/socket/server/support/OriginHandshakeInterceptor.java index 44e171000e7..0f9ee0c8291 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/server/support/OriginHandshakeInterceptor.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/server/support/OriginHandshakeInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -18,6 +18,7 @@ package org.springframework.web.socket.server.support; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; import java.util.Map; @@ -27,6 +28,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.http.HttpStatus; import org.springframework.http.server.ServerHttpRequest; import org.springframework.http.server.ServerHttpResponse; +import org.springframework.util.Assert; import org.springframework.web.socket.WebSocketHandler; import org.springframework.web.socket.server.HandshakeInterceptor; @@ -52,13 +54,32 @@ public class OriginHandshakeInterceptor implements HandshakeInterceptor { } /** - * Use this property to define a collection of allowed origins. + * Configure allowed {@code Origin} header values. This check is mostly designed for + * browser clients. There is nothing preventing other types of client to modify the + * {@code Origin} header value. + * + *

Each provided allowed origin must start by "http://", "https://" or be "*" + * (means that all origins are allowed). + * + * @see RFC 6454: The Web Origin Concept */ public void setAllowedOrigins(Collection allowedOrigins) { - this.allowedOrigins.clear(); - if (allowedOrigins != null) { - this.allowedOrigins.addAll(allowedOrigins); + Assert.notNull(allowedOrigins, "Allowed origin Collection must not be null"); + for (String allowedOrigin : allowedOrigins) { + Assert.isTrue(allowedOrigin.equals("*") || allowedOrigin.startsWith("http://") || + allowedOrigin.startsWith("https://"), "Invalid allowed origin provided: \"" + + allowedOrigin + "\". It must start with \"http://\", \"https://\" or be \"*\""); } + this.allowedOrigins.clear(); + this.allowedOrigins.addAll(allowedOrigins); + } + + /** + * @see #setAllowedOrigins(Collection) + * @since 4.1.5 + */ + public Collection getAllowedOrigins() { + return Collections.unmodifiableList(this.allowedOrigins); } @Override @@ -76,7 +97,14 @@ public class OriginHandshakeInterceptor implements HandshakeInterceptor { } protected boolean isValidOrigin(ServerHttpRequest request) { - return this.allowedOrigins.contains(request.getHeaders().getOrigin()); + String origin = request.getHeaders().getOrigin(); + if (origin == null) { + return true; + } + if (this.allowedOrigins.contains("*")) { + return true; + } + return this.allowedOrigins.contains(origin); } @Override 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 984845b034b..0b01c245635 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -266,24 +266,34 @@ public abstract class AbstractSockJsService implements SockJsService { } /** - * Configure allowed {@code Origin} header values. This check is mostly designed for browser - * clients. There is noting preventing other types of client to modify the Origin header value. + * Configure allowed {@code Origin} header values. This check is mostly designed for + * browser clients. There is nothing preventing other types of client to modify the + * {@code Origin} header value. * - *

When SockJS is enabled and allowed origins are restricted, transport types that do not - * use {@code Origin} headers for cross origin requests (jsonp-polling, iframe-xhr-polling, - * iframe-eventsource and iframe-htmlfile) are disabled. As a consequence, IE6/IE7 won't be - * supported anymore and IE8/IE9 will only be supported without cookies. + *

When SockJS is enabled and origins are restricted, transport types that do not + * allow to check request origin (JSONP and Iframe based transports) are disabled. + * As a consequence, IE 6 to 9 are not supported when origins are restricted. * - *

By default, all origins are allowed. + *

Each provided allowed origin must start by "http://", "https://" or be "*" + * (means that all origins are allowed). Empty allowed origin list is not supported. + * By default, all origins are allowed. * * @since 4.1.2 + * @see RFC 6454: The Web Origin Concept * @see SockJS supported transports by browser */ public void setAllowedOrigins(List allowedOrigins) { - this.allowedOrigins.clear(); - if (allowedOrigins != null) { - this.allowedOrigins.addAll(allowedOrigins); + Assert.notEmpty(allowedOrigins, "Allowed origin List must not be empty"); + for (String allowedOrigin : allowedOrigins) { + Assert.isTrue( + allowedOrigin.equals("*") || allowedOrigin.startsWith("http://") || + allowedOrigin.startsWith("https://"), + "Invalid allowed origin provided: \"" + + allowedOrigin + + "\". It must start with \"http://\", \"https://\" or be \"*\""); } + this.allowedOrigins.clear(); + this.allowedOrigins.addAll(allowedOrigins); } /** @@ -291,7 +301,7 @@ public abstract class AbstractSockJsService implements SockJsService { * @see #setAllowedOrigins(List) */ public List getAllowedOrigins() { - return Collections.unmodifiableList(allowedOrigins); + return Collections.unmodifiableList(this.allowedOrigins); } /** @@ -345,6 +355,11 @@ public abstract class AbstractSockJsService implements SockJsService { this.infoHandler.handle(request, response); } else if (sockJsPath.matches("/iframe[0-9-.a-z_]*.html")) { + if (!this.allowedOrigins.isEmpty() && !this.allowedOrigins.contains("*")) { + logger.debug("Iframe support is disabled when an origin check is required, ignoring " + requestInfo); + response.setStatusCode(HttpStatus.NOT_FOUND); + return; + } logger.debug(requestInfo); this.iframeHandler.handle(request, response); } @@ -423,8 +438,13 @@ public abstract class AbstractSockJsService implements SockJsService { HttpHeaders requestHeaders = request.getHeaders(); HttpHeaders responseHeaders = response.getHeaders(); String origin = requestHeaders.getOrigin(); + String host = requestHeaders.getFirst(HttpHeaders.HOST); + + if (origin == null) { + return true; + } - if (!this.allowedOrigins.contains("*") && (origin == null || !this.allowedOrigins.contains(origin))) { + if (!this.allowedOrigins.contains("*") && !this.allowedOrigins.contains(origin)) { logger.debug("Request rejected, Origin header value " + origin + " not allowed"); response.setStatusCode(HttpStatus.FORBIDDEN); return false; @@ -439,7 +459,7 @@ public abstract class AbstractSockJsService implements SockJsService { // See SPR-11919 and https://issues.jboss.org/browse/WFLY-3474 } - if (!this.suppressCors && origin != null && !hasCorsResponseHeaders) { + if (!this.suppressCors && !hasCorsResponseHeaders) { addCorsHeaders(request, response, httpMethods); } return true; @@ -561,7 +581,8 @@ public abstract class AbstractSockJsService implements SockJsService { response.getHeaders().setContentType(new MediaType("text", "html", UTF8_CHARSET)); response.getHeaders().setContentLength(contentBytes.length); - addCacheHeaders(response); + // No cache in order to check every time if IFrame are authorized + addNoCacheHeaders(response); response.getHeaders().setETag(etagValue); response.getBody().write(contentBytes); } diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/AbstractHttpRequestTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/AbstractHttpRequestTests.java index 82aa0dbf902..62f51b75015 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/AbstractHttpRequestTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/AbstractHttpRequestTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -18,6 +18,7 @@ package org.springframework.web.socket; import org.junit.Before; +import org.springframework.http.HttpHeaders; import org.springframework.http.server.ServerHttpAsyncRequestControl; import org.springframework.http.server.ServerHttpRequest; import org.springframework.http.server.ServerHttpResponse; @@ -55,7 +56,7 @@ public abstract class AbstractHttpRequestTests { } protected void setOrigin(String origin) { - this.servletRequest.addHeader("Origin", origin); + this.request.getHeaders().add(HttpHeaders.ORIGIN, origin); } protected void resetRequestAndResponse() { diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/config/annotation/WebMvcStompWebSocketEndpointRegistrationTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/config/annotation/WebMvcStompWebSocketEndpointRegistrationTests.java index de6e8888d38..a149176893a 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/config/annotation/WebMvcStompWebSocketEndpointRegistrationTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/config/annotation/WebMvcStompWebSocketEndpointRegistrationTests.java @@ -90,6 +90,12 @@ public class WebMvcStompWebSocketEndpointRegistrationTests { assertEquals(OriginHandshakeInterceptor.class, requestHandler.getHandshakeInterceptors().get(0).getClass()); } + @Test(expected = IllegalArgumentException.class) + public void noAllowedOrigin() { + WebMvcStompWebSocketEndpointRegistration registration = new WebMvcStompWebSocketEndpointRegistration(new String[] {"/foo"}, this.handler, this.scheduler); + registration.setAllowedOrigins(); + } + @Test public void allowedOriginsWithSockJsService() { WebMvcStompWebSocketEndpointRegistration registration = diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/config/annotation/WebSocketHandlerRegistrationTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/config/annotation/WebSocketHandlerRegistrationTests.java index 9b2d175c15d..751b268b6ea 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/config/annotation/WebSocketHandlerRegistrationTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/config/annotation/WebSocketHandlerRegistrationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -93,6 +93,11 @@ public class WebSocketHandlerRegistrationTests { assertArrayEquals(new HandshakeInterceptor[] {interceptor}, mapping.interceptors); } + @Test(expected = IllegalArgumentException.class) + public void noAllowedOrigin() { + this.registration.addHandler(Mockito.mock(WebSocketHandler.class), "/foo").setAllowedOrigins(); + } + @Test public void interceptorsWithAllowedOrigins() { WebSocketHandler handler = new TextWebSocketHandler(); diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/server/support/AllowedOriginsInterceptorTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/server/support/OriginHandshakeInterceptorTests.java similarity index 77% rename from spring-websocket/src/test/java/org/springframework/web/socket/server/support/AllowedOriginsInterceptorTests.java rename to spring-websocket/src/test/java/org/springframework/web/socket/server/support/OriginHandshakeInterceptorTests.java index 1543fd9f3d1..a970949820a 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/server/support/AllowedOriginsInterceptorTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/server/support/OriginHandshakeInterceptorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -35,7 +35,25 @@ import org.springframework.web.socket.WebSocketHandler; * * @author Sebastien Deleuze */ -public class AllowedOriginsInterceptorTests extends AbstractHttpRequestTests { +public class OriginHandshakeInterceptorTests extends AbstractHttpRequestTests { + + @Test(expected = IllegalArgumentException.class) + public void nullAllowedOriginList() { + OriginHandshakeInterceptor interceptor = new OriginHandshakeInterceptor(); + interceptor.setAllowedOrigins(null); + } + + @Test(expected = IllegalArgumentException.class) + public void invalidAllowedOrigin() { + OriginHandshakeInterceptor interceptor = new OriginHandshakeInterceptor(); + interceptor.setAllowedOrigins(Arrays.asList("domain.com")); + } + + @Test + public void validAllowedOrigins() { + OriginHandshakeInterceptor interceptor = new OriginHandshakeInterceptor(); + interceptor.setAllowedOrigins(Arrays.asList("http://domain.com", "https://domain.com", "*")); + } @Test public void originValueMatch() throws Exception { @@ -82,24 +100,27 @@ public class AllowedOriginsInterceptorTests extends AbstractHttpRequestTests { } @Test - public void noOriginNoMatchWithNullHostileCollection() throws Exception { + public void originNoMatchWithNullHostileCollection() throws Exception { Map attributes = new HashMap(); WebSocketHandler wsHandler = Mockito.mock(WebSocketHandler.class); + setOrigin("http://mydomain4.com"); + OriginHandshakeInterceptor interceptor = new OriginHandshakeInterceptor(); Set allowedOrigins = new ConcurrentSkipListSet(); allowedOrigins.add("http://mydomain1.com"); - OriginHandshakeInterceptor interceptor = new OriginHandshakeInterceptor(); interceptor.setAllowedOrigins(allowedOrigins); assertFalse(interceptor.beforeHandshake(request, response, wsHandler, attributes)); assertEquals(servletResponse.getStatus(), HttpStatus.FORBIDDEN.value()); } @Test - public void noOriginNoMatch() throws Exception { + public void originMatchAll() throws Exception { Map attributes = new HashMap(); WebSocketHandler wsHandler = Mockito.mock(WebSocketHandler.class); + setOrigin("http://mydomain1.com"); OriginHandshakeInterceptor interceptor = new OriginHandshakeInterceptor(); - assertFalse(interceptor.beforeHandshake(request, response, wsHandler, attributes)); - assertEquals(servletResponse.getStatus(), HttpStatus.FORBIDDEN.value()); + interceptor.setAllowedOrigins(Arrays.asList("*")); + assertTrue(interceptor.beforeHandshake(request, response, wsHandler, attributes)); + assertNotEquals(servletResponse.getStatus(), HttpStatus.FORBIDDEN.value()); } } diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java index f339b4a014d..f2a1126d6f2 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/support/SockJsServiceTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -18,7 +18,6 @@ package org.springframework.web.socket.sockjs.support; import java.io.IOException; import java.util.Arrays; -import java.util.List; import javax.servlet.ServletOutputStream; import javax.servlet.http.HttpServletResponse; @@ -103,13 +102,13 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { body.substring(body.indexOf(','))); this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com")); - resetResponseAndHandleRequest("GET", "/echo/info", HttpStatus.FORBIDDEN); + resetResponseAndHandleRequest("GET", "/echo/info", HttpStatus.OK); assertNull(this.servletResponse.getHeader("Access-Control-Allow-Origin")); assertNull(this.servletResponse.getHeader("Access-Control-Allow-Credentials")); assertNull(this.servletResponse.getHeader("Vary")); } - @Test // SPR-12226 + @Test // SPR-12226 and SPR-12660 public void handleInfoGetWithOrigin() throws Exception { setOrigin("http://mydomain2.com"); resetResponseAndHandleRequest("GET", "/echo/info", HttpStatus.OK); @@ -125,12 +124,6 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { assertEquals(",\"origins\":[\"*:*\"],\"cookie_needed\":true,\"websocket\":true}", body.substring(body.indexOf(','))); - this.service.setAllowedOrigins(null); - resetResponseAndHandleRequest("GET", "/echo/info", HttpStatus.FORBIDDEN); - assertNull(this.servletResponse.getHeader("Access-Control-Allow-Origin")); - assertNull(this.servletResponse.getHeader("Access-Control-Allow-Credentials")); - assertNull(this.servletResponse.getHeader("Vary")); - this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com")); resetResponseAndHandleRequest("GET", "/echo/info", HttpStatus.FORBIDDEN); assertNull(this.servletResponse.getHeader("Access-Control-Allow-Origin")); @@ -168,7 +161,7 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { verify(mockResponse, times(1)).getOutputStream(); } - @Test + @Test // SPR-12660 public void handleInfoOptions() throws Exception { this.servletRequest.addHeader("Access-Control-Request-Headers", "Last-Modified"); resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.NO_CONTENT); @@ -182,19 +175,19 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { assertEquals("Origin", this.servletResponse.getHeader("Vary")); this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com")); - resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.FORBIDDEN); + resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.NO_CONTENT); assertNull(this.servletResponse.getHeader("Access-Control-Allow-Origin")); assertNull(this.servletResponse.getHeader("Access-Control-Allow-Credentials")); assertNull(this.servletResponse.getHeader("Access-Control-Allow-Headers")); assertNull(this.servletResponse.getHeader("Access-Control-Allow-Methods")); assertNull(this.servletResponse.getHeader("Access-Control-Max-Age")); - assertNull(this.servletResponse.getHeader("Vary")); + assertEquals("Origin", this.servletResponse.getHeader("Vary")); } - @Test // SPR-12226 + @Test // SPR-12226 and SPR-12660 public void handleInfoOptionsWithOrigin() throws Exception { setOrigin("http://mydomain2.com"); - this.servletRequest.addHeader("Access-Control-Request-Headers", "Last-Modified"); + this.request.getHeaders().add("Access-Control-Request-Headers", "Last-Modified"); resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.NO_CONTENT); this.response.flush(); assertEquals("http://mydomain2.com", this.servletResponse.getHeader("Access-Control-Allow-Origin")); @@ -204,16 +197,6 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { assertEquals("31536000", this.servletResponse.getHeader("Access-Control-Max-Age")); assertEquals("Origin", this.servletResponse.getHeader("Vary")); - this.service.setAllowedOrigins(null); - resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.FORBIDDEN); - this.response.flush(); - assertNull(this.servletResponse.getHeader("Access-Control-Allow-Origin")); - assertNull(this.servletResponse.getHeader("Access-Control-Allow-Credentials")); - assertNull(this.servletResponse.getHeader("Access-Control-Allow-Headers")); - assertNull(this.servletResponse.getHeader("Access-Control-Allow-Methods")); - assertNull(this.servletResponse.getHeader("Access-Control-Max-Age")); - assertNull(this.servletResponse.getHeader("Vary")); - this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com")); resetResponseAndHandleRequest("OPTIONS", "/echo/info", HttpStatus.FORBIDDEN); this.response.flush(); @@ -236,8 +219,9 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { } @Test // SPR-12283 - public void handleInfoOptionsWithOriginAndCorsDisabled() throws Exception { + public void handleInfoOptionsWithOriginAndCorsHeadersDisabled() throws Exception { setOrigin("http://mydomain2.com"); + this.service.setAllowedOrigins(Arrays.asList("*")); this.service.setSuppressCors(true); this.servletRequest.addHeader("Access-Control-Request-Headers", "Last-Modified"); @@ -278,7 +262,7 @@ public class SockJsServiceTests extends AbstractHttpRequestTests { assertEquals("text/html;charset=UTF-8", this.servletResponse.getContentType()); assertTrue(this.servletResponse.getContentAsString().startsWith("\n")); assertEquals(490, this.servletResponse.getContentLength()); - assertEquals("public, max-age=31536000", this.response.getHeaders().getCacheControl()); + assertEquals("no-store, no-cache, must-revalidate, max-age=0", this.response.getHeaders().getCacheControl()); assertEquals("\"06b486b3208b085d9e3220f456a6caca4\"", this.response.getHeaders().getETag()); } 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 b424661cc83..35c049d1436 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsServiceTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/sockjs/transport/handler/DefaultSockJsServiceTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -18,9 +18,9 @@ package org.springframework.web.socket.sockjs.transport.handler; import java.util.Arrays; import java.util.Collections; -import java.util.List; import java.util.Map; +import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Test; import org.mockito.Mock; @@ -56,8 +56,6 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests { private static final String sessionUrlPrefix = "/server1/" + sessionId + "/"; - private static final List origins = Arrays.asList("http://mydomain1.com", "http://mydomain2.com"); - @Mock private SessionCreatingTransportHandler xhrHandler; @@ -124,6 +122,31 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests { assertSame(xhrHandler, handlers.get(xhrHandler.getTransportType())); } + @Test + public void defaultAllowedOrigin() { + assertThat(this.service.getAllowedOrigins(), Matchers.contains("*")); + } + + @Test(expected = IllegalArgumentException.class) + public void nullAllowedOriginList() { + this.service.setAllowedOrigins(null); + } + + @Test(expected = IllegalArgumentException.class) + public void emptyAllowedOriginList() { + this.service.setAllowedOrigins(Arrays.asList()); + } + + @Test(expected = IllegalArgumentException.class) + public void invalidAllowedOrigin() { + this.service.setAllowedOrigins(Arrays.asList("domain.com")); + } + + @Test + public void validAllowedOrigins() { + this.service.setAllowedOrigins(Arrays.asList("http://domain.com", "https://domain.com", "*")); + } + @Test public void customizedTransportHandlerList() { TransportHandlingSockJsService service = new TransportHandlingSockJsService( @@ -148,27 +171,16 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests { assertNull(this.response.getHeaders().getFirst("Access-Control-Allow-Credentials")); } - @Test // SPR-12226 - public void handleTransportRequestXhrAllowNullOrigin() throws Exception { - String sockJsPath = sessionUrlPrefix + "xhr"; - setRequest("POST", sockJsPrefix + sockJsPath); - this.service.setAllowedOrigins(null); - this.service.handleRequest(this.request, this.response, sockJsPath, this.wsHandler); - - assertNull(this.response.getHeaders().getFirst("Access-Control-Allow-Origin")); - assertNull(this.response.getHeaders().getFirst("Access-Control-Allow-Credentials")); - } - @Test // SPR-12226 public void handleTransportRequestXhrAllowedOriginsMatch() throws Exception { String sockJsPath = sessionUrlPrefix + "xhr"; setRequest("POST", sockJsPrefix + sockJsPath); - setOrigin(origins.get(0)); - this.service.setAllowedOrigins(origins); + this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com", "http://mydomain2.com")); + setOrigin("http://mydomain1.com"); this.service.handleRequest(this.request, this.response, sockJsPath, this.wsHandler); assertEquals(200, this.servletResponse.getStatus()); - assertEquals(origins.get(0), this.response.getHeaders().getFirst("Access-Control-Allow-Origin")); + assertEquals("http://mydomain1.com", this.response.getHeaders().getFirst("Access-Control-Allow-Origin")); assertEquals("true", this.response.getHeaders().getFirst("Access-Control-Allow-Credentials")); } @@ -176,8 +188,8 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests { public void handleTransportRequestXhrAllowedOriginsNoMatch() throws Exception { String sockJsPath = sessionUrlPrefix + "xhr"; setRequest("POST", sockJsPrefix + sockJsPath); + this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com", "http://mydomain2.com")); setOrigin("http://mydomain3.com"); - this.service.setAllowedOrigins(origins); this.service.handleRequest(this.request, this.response, sockJsPath, this.wsHandler); assertEquals(403, this.servletResponse.getStatus()); @@ -197,19 +209,6 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests { assertNull(this.response.getHeaders().getFirst("Access-Control-Allow-Methods")); } - @Test // SPR-12226 - public void handleTransportRequestXhrOptionsAllowNullOrigin() throws Exception { - String sockJsPath = sessionUrlPrefix + "xhr"; - setRequest("OPTIONS", sockJsPrefix + sockJsPath); - this.service.setAllowedOrigins(null); - this.service.handleRequest(this.request, this.response, sockJsPath, this.wsHandler); - - assertEquals(403, this.servletResponse.getStatus()); - assertNull(this.response.getHeaders().getFirst("Access-Control-Allow-Origin")); - assertNull(this.response.getHeaders().getFirst("Access-Control-Allow-Credentials")); - assertNull(this.response.getHeaders().getFirst("Access-Control-Allow-Methods")); - } - @Test public void handleTransportRequestNoSuitableHandler() throws Exception { String sockJsPath = sessionUrlPrefix + "eventsource"; @@ -279,12 +278,6 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests { setRequest("GET", sockJsPrefix + sockJsPath); jsonpService.handleRequest(this.request, this.response, sockJsPath, this.wsHandler); assertEquals(404, this.servletResponse.getStatus()); - - resetRequestAndResponse(); - jsonpService.setAllowedOrigins(null); - setRequest("GET", sockJsPrefix + sockJsPath); - jsonpService.handleRequest(this.request, this.response, sockJsPath, this.wsHandler); - assertEquals(404, this.servletResponse.getStatus()); } @Test @@ -311,6 +304,21 @@ public class DefaultSockJsServiceTests extends AbstractHttpRequestTests { assertEquals(403, this.servletResponse.getStatus()); } + @Test + public void handleTransportRequestIframe() throws Exception { + String sockJsPath = "/iframe.html"; + setRequest("GET", sockJsPrefix + sockJsPath); + this.service.handleRequest(this.request, this.response, sockJsPath, this.wsHandler); + assertNotEquals(404, this.servletResponse.getStatus()); + assertNull(this.servletResponse.getHeader("X-Frame-Options")); + + resetRequestAndResponse(); + setRequest("GET", sockJsPrefix + sockJsPath); + this.service.setAllowedOrigins(Arrays.asList("http://mydomain1.com")); + this.service.handleRequest(this.request, this.response, sockJsPath, this.wsHandler); + assertEquals(404, this.servletResponse.getStatus()); + } + interface SessionCreatingTransportHandler extends TransportHandler, SockJsSessionFactory { }