Efficient and consistent setAllowedOrigins collection type

Issue: SPR-13761

spring-websocket/src/main/java/org/springframework/web/socket/server/support/OriginHandshakeInterceptor.java

@@ -16,11 +16,11 @@
 
 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.LinkedHashSet;
 import java.util.Map;
+import java.util.Set;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -34,8 +34,8 @@ import org.springframework.web.socket.server.HandshakeInterceptor;
 import org.springframework.web.util.WebUtils;
 
 /**
- * An interceptor to check request {@code Origin} header value against a collection of
- * allowed origins.
+ * An interceptor to check request {@code Origin} header value against a
+ * collection of allowed origins.
  *
  * @author Sebastien Deleuze
  * @since 4.1.2
@@ -44,60 +44,57 @@ public class OriginHandshakeInterceptor implements HandshakeInterceptor {
 
 	protected Log logger = LogFactory.getLog(getClass());
 
-	private final List<String> allowedOrigins;
+	private final Set<String> allowedOrigins = new LinkedHashSet<String>();
 
 
 	/**
 	 * Default constructor with only same origin requests allowed.
 	 */
 	public OriginHandshakeInterceptor() {
-		this.allowedOrigins = new ArrayList<String>();
 	}
 
 	/**
 	 * Constructor using the specified allowed origin values.
-	 *
 	 * @see #setAllowedOrigins(Collection)
 	 */
 	public OriginHandshakeInterceptor(Collection<String> allowedOrigins) {
-		this();
 		setAllowedOrigins(allowedOrigins);
 	}
 
+
 	/**
 	 * Configure allowed {@code Origin} header values. This check is mostly
 	 * designed for browsers. There is nothing preventing other types of client
 	 * to modify the {@code Origin} header value.
-	 *
 	 * <p>Each provided allowed origin must have a scheme, and optionally a port
 	 * (e.g. "http://example.org", "http://example.org:9090"). An allowed origin
 	 * string may also be "*" in which case all origins are allowed.
-	 *
 	 * @see <a href="https://tools.ietf.org/html/rfc6454">RFC 6454: The Web Origin Concept</a>
 	 */
 	public void setAllowedOrigins(Collection<String> allowedOrigins) {
-		Assert.notNull(allowedOrigins, "Allowed origin Collection must not be null");
+		Assert.notNull(allowedOrigins, "Allowed origins Collection must not be null");
 		this.allowedOrigins.clear();
 		this.allowedOrigins.addAll(allowedOrigins);
 	}
 
 	/**
-	 * @see #setAllowedOrigins(Collection)
 	 * @since 4.1.5
+	 * @see #setAllowedOrigins
 	 */
 	public Collection<String> getAllowedOrigins() {
-		return Collections.unmodifiableList(this.allowedOrigins);
+		return Collections.unmodifiableSet(this.allowedOrigins);
 	}
 
 
 	@Override
 	public boolean beforeHandshake(ServerHttpRequest request, ServerHttpResponse response,
 			WebSocketHandler wsHandler, Map<String, Object> attributes) throws Exception {
+
 		if (!WebUtils.isSameOrigin(request) && !WebUtils.isValidOrigin(request, this.allowedOrigins)) {
 			response.setStatusCode(HttpStatus.FORBIDDEN);
 			if (logger.isDebugEnabled()) {
-				logger.debug("Handshake request rejected, Origin header value "
-						+ request.getHeaders().getOrigin() + " not allowed");
+				logger.debug("Handshake request rejected, Origin header value " +
+						request.getHeaders().getOrigin() + " not allowed");
 			}
 			return false;
 		}

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

@@ -18,13 +18,15 @@ package org.springframework.web.socket.sockjs.support;
 
 import java.io.IOException;
 import java.nio.charset.Charset;
-import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.Date;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Random;
+import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import javax.servlet.http.HttpServletRequest;
 
@@ -56,7 +58,7 @@ import org.springframework.web.util.WebUtils;
  * path resolution and handling of static SockJS requests (e.g. "/info", "/iframe.html",
  * etc). Sub-classes must handle session URLs (i.e. transport-specific requests).
  *
- * By default, only same origin requests are allowed. Use {@link #setAllowedOrigins(List)}
+ * By default, only same origin requests are allowed. Use {@link #setAllowedOrigins}
  * to specify a list of allowed origins (a list containing "*" will allow all origins).
  *
  * @author Rossen Stoyanchev
@@ -94,10 +96,10 @@ public abstract class AbstractSockJsService implements SockJsService, CorsConfig
 
 	private boolean webSocketEnabled = true;
 
-	private final List<String> allowedOrigins = new ArrayList<String>();
-
 	private boolean suppressCors = false;
 
+	protected final Set<String> allowedOrigins = new LinkedHashSet<String>();
+
 
 	public AbstractSockJsService(TaskScheduler scheduler) {
 		Assert.notNull(scheduler, "TaskScheduler must not be null");
@@ -274,35 +276,6 @@ public abstract class AbstractSockJsService implements SockJsService, CorsConfig
 		return this.webSocketEnabled;
 	}
 
-	/**
-	 * Configure allowed {@code Origin} header values. This check is mostly
-	 * designed for browsers. There is nothing preventing other types of client
-	 * to modify the {@code Origin} header value.
-	 * <p>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.
-	 * <p>Each provided allowed origin must have a scheme, and optionally a port
-	 * (e.g. "http://example.org", "http://example.org:9090"). An allowed origin
-	 * string may also be "*" in which case all origins are allowed.
-	 * @since 4.1.2
-	 * @see <a href="https://tools.ietf.org/html/rfc6454">RFC 6454: The Web Origin Concept</a>
-	 * @see <a href="https://github.com/sockjs/sockjs-client#supported-transports-by-browser-html-served-from-http-or-https">SockJS supported transports by browser</a>
-	 */
-	public void setAllowedOrigins(List<String> allowedOrigins) {
-		Assert.notNull(allowedOrigins, "Allowed origin List must not be null");
-		this.allowedOrigins.clear();
-		this.allowedOrigins.addAll(allowedOrigins);
-	}
-
-	/**
-	 * @since 4.1.2
-	 * @see #setAllowedOrigins(List)
-	 */
-	public List<String> getAllowedOrigins() {
-		return Collections.unmodifiableList(this.allowedOrigins);
-	}
-
 	/**
 	 * This option can be used to disable automatic addition of CORS headers for
 	 * SockJS requests.
@@ -321,6 +294,35 @@ public abstract class AbstractSockJsService implements SockJsService, CorsConfig
 		return this.suppressCors;
 	}
 
+	/**
+	 * Configure allowed {@code Origin} header values. This check is mostly
+	 * designed for browsers. There is nothing preventing other types of client
+	 * to modify the {@code Origin} header value.
+	 * <p>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.
+	 * <p>Each provided allowed origin must have a scheme, and optionally a port
+	 * (e.g. "http://example.org", "http://example.org:9090"). An allowed origin
+	 * string may also be "*" in which case all origins are allowed.
+	 * @since 4.1.2
+	 * @see <a href="https://tools.ietf.org/html/rfc6454">RFC 6454: The Web Origin Concept</a>
+	 * @see <a href="https://github.com/sockjs/sockjs-client#supported-transports-by-browser-html-served-from-http-or-https">SockJS supported transports by browser</a>
+	 */
+	public void setAllowedOrigins(Collection<String> allowedOrigins) {
+		Assert.notNull(allowedOrigins, "Allowed origins Collection must not be null");
+		this.allowedOrigins.clear();
+		this.allowedOrigins.addAll(allowedOrigins);
+	}
+
+	/**
+	 * @since 4.1.2
+	 * @see #setAllowedOrigins
+	 */
+	public Collection<String> getAllowedOrigins() {
+		return Collections.unmodifiableSet(this.allowedOrigins);
+	}
+
 
 	/**
 	 * This method determines the SockJS path and handles SockJS static URLs.
@@ -465,24 +467,11 @@ public abstract class AbstractSockJsService implements SockJsService, CorsConfig
 		String path = request.getURI().getPath();
 		int index = path.lastIndexOf('/') + 1;
 		String filename = path.substring(index);
-		return filename.indexOf(';') == -1;
+		return (filename.indexOf(';') == -1);
 	}
 
-	/**
-	 * Handle request for raw WebSocket communication, i.e. without any SockJS message framing.
-	 */
-	protected abstract void handleRawWebSocketRequest(ServerHttpRequest request,
-			ServerHttpResponse response, WebSocketHandler webSocketHandler) throws IOException;
-
-	/**
-	 * Handle a SockJS session URL (i.e. transport-specific request).
-	 */
-	protected abstract void handleTransportRequest(ServerHttpRequest request, ServerHttpResponse response,
-			WebSocketHandler webSocketHandler, String sessionId, String transport) throws SockJsException;
-
-
-	protected boolean checkOrigin(ServerHttpRequest request, ServerHttpResponse response,
-			HttpMethod... httpMethods) throws IOException {
+	protected boolean checkOrigin(ServerHttpRequest request, ServerHttpResponse response, HttpMethod... httpMethods)
+			throws IOException {
 
 		if (WebUtils.isSameOrigin(request)) {
 			return true;
@@ -529,6 +518,19 @@ public abstract class AbstractSockJsService implements SockJsService, CorsConfig
 	}
 
 
+	/**
+	 * Handle request for raw WebSocket communication, i.e. without any SockJS message framing.
+	 */
+	protected abstract void handleRawWebSocketRequest(ServerHttpRequest request,
+			ServerHttpResponse response, WebSocketHandler webSocketHandler) throws IOException;
+
+	/**
+	 * Handle a SockJS session URL (i.e. transport-specific request).
+	 */
+	protected abstract void handleTransportRequest(ServerHttpRequest request, ServerHttpResponse response,
+			WebSocketHandler webSocketHandler, String sessionId, String transport) throws SockJsException;
+
+
 	private interface SockJsRequestHandler {
 
 		void handle(ServerHttpRequest request, ServerHttpResponse response) throws IOException;
@@ -546,8 +548,8 @@ public abstract class AbstractSockJsService implements SockJsService, CorsConfig
 				addNoCacheHeaders(response);
 				if (checkOrigin(request, response)) {
 					response.getHeaders().setContentType(new MediaType("application", "json", UTF8_CHARSET));
-					String content = String.format(INFO_CONTENT, random.nextInt(),
-							isSessionCookieNeeded(), isWebSocketEnabled());
+					String content = String.format(
+							INFO_CONTENT, random.nextInt(), isSessionCookieNeeded(), isWebSocketEnabled());
 					response.getBody().write(content.getBytes());
 				}
 

spring-websocket/src/main/java/org/springframework/web/socket/sockjs/transport/TransportHandlingSockJsService.java

@@ -326,7 +326,7 @@ public class TransportHandlingSockJsService extends AbstractSockJsService implem
 			return false;
 		}
 
-		if (!getAllowedOrigins().contains("*")) {
+		if (!this.allowedOrigins.contains("*")) {
 			TransportType transportType = TransportType.fromValue(transport);
 			if (transportType == null || !transportType.supportsOrigin()) {
 				if (logger.isWarnEnabled()) {