WebSocketSession.getExtensions consistently exposes unmodifiable/empty list

Issue: SPR-15180

spring-websocket/src/main/java/org/springframework/web/socket/WebSocketSession.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -46,7 +46,7 @@ public interface WebSocketSession extends Closeable {
 	URI getUri();
 
 	/**
-	 * Return the headers used in the handshake request.
+	 * Return the headers used in the handshake request (never {@code null}).
 	 */
 	HttpHeaders getHandshakeHeaders();
 
@@ -57,13 +57,13 @@ public interface WebSocketSession extends Closeable {
 	 * HandshakeInterceptor}. On the client side the map can be populated via
 	 * {@link org.springframework.web.socket.client.WebSocketClient
 	 * WebSocketClient} handshake methods.
-	 * @return a Map with the session attributes, never {@code null}.
+	 * @return a Map with the session attributes (never {@code null})
 	 */
 	Map<String, Object> getAttributes();
 
 	/**
-	 * Return a {@link java.security.Principal} instance containing the name of the
+	 * Return a {@link java.security.Principal} instance containing the name
-	 * authenticated user.
+	 * of the authenticated user.
 	 * <p>If the user has not been authenticated, the method returns <code>null</code>.
 	 */
 	Principal getPrincipal();
@@ -79,8 +79,9 @@ public interface WebSocketSession extends Closeable {
 	InetSocketAddress getRemoteAddress();
 
 	/**
-	 * Return the negotiated sub-protocol or {@code null} if none was specified or
+	 * Return the negotiated sub-protocol.
-	 * negotiated successfully.
+	 * @return the protocol identifier, or {@code null} if no protocol
+	 * was specified or negotiated successfully
 	 */
 	String getAcceptedProtocol();
 
@@ -105,8 +106,9 @@ public interface WebSocketSession extends Closeable {
 	int getBinaryMessageSizeLimit();
 
 	/**
-	 * Return the negotiated extensions or {@code null} if none was specified or
+	 * Determine the negotiated extensions.
-	 * negotiated successfully.
+	 * @return the list of extensions, or an empty list if no extension
+	 * was specified or negotiated successfully
 	 */
 	List<WebSocketExtension> getExtensions();
 

spring-websocket/src/main/java/org/springframework/web/socket/adapter/jetty/JettyWebSocketSession.java

@@ -22,6 +22,7 @@ import java.net.InetSocketAddress;
 import java.net.URI;
 import java.security.Principal;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -33,6 +34,7 @@ import org.eclipse.jetty.websocket.api.WebSocketException;
 import org.eclipse.jetty.websocket.api.extensions.ExtensionConfig;
 
 import org.springframework.http.HttpHeaders;
+import org.springframework.util.CollectionUtils;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.ReflectionUtils;
 import org.springframework.web.socket.BinaryMessage;
@@ -213,19 +215,20 @@ public class JettyWebSocketSession extends AbstractWebSocketSession<Session> {
 
 		this.headers = new HttpHeaders();
 		this.headers.putAll(session.getUpgradeRequest().getHeaders());
-		this.headers = HttpHeaders.readOnlyHttpHeaders(headers);
+		this.headers = HttpHeaders.readOnlyHttpHeaders(this.headers);
 
 		this.acceptedProtocol = session.getUpgradeResponse().getAcceptedSubProtocol();
 
-		List<ExtensionConfig> source = session.getUpgradeResponse().getExtensions();
+		List<ExtensionConfig> jettyExtensions = session.getUpgradeResponse().getExtensions();
-		if (source != null) {
+		if (!CollectionUtils.isEmpty(jettyExtensions)) {
-			this.extensions = new ArrayList<>(source.size());
+			this.extensions = new ArrayList<>(jettyExtensions.size());
-			for (ExtensionConfig ec : source) {
+			for (ExtensionConfig jettyExtension : jettyExtensions) {
-				this.extensions.add(new WebSocketExtension(ec.getName(), ec.getParameters()));
+				this.extensions.add(new WebSocketExtension(jettyExtension.getName(), jettyExtension.getParameters()));
 			}
+			this.extensions = Collections.unmodifiableList(this.extensions);
 		}
 		else {
-			this.extensions = new ArrayList<>(0);
+			this.extensions = Collections.emptyList();
 		}
 
 		if (this.user == null) {
@@ -243,19 +246,20 @@ public class JettyWebSocketSession extends AbstractWebSocketSession<Session> {
 
 		this.headers = new HttpHeaders();
 		this.headers.putAll((Map<String, List<String>>) ReflectionUtils.invokeMethod(getHeaders, request));
-		this.headers = HttpHeaders.readOnlyHttpHeaders(headers);
+		this.headers = HttpHeaders.readOnlyHttpHeaders(this.headers);
 
 		this.acceptedProtocol = (String) ReflectionUtils.invokeMethod(getAcceptedSubProtocol, response);
 
-		List<ExtensionConfig> source = (List<ExtensionConfig>) ReflectionUtils.invokeMethod(getExtensions, response);
+		List<ExtensionConfig> extensions = (List<ExtensionConfig>) ReflectionUtils.invokeMethod(getExtensions, response);
-		if (source != null) {
+		if (!CollectionUtils.isEmpty(extensions)) {
-			this.extensions = new ArrayList<>(source.size());
+			this.extensions = new ArrayList<>(extensions.size());
-			for (ExtensionConfig ec : source) {
+			for (ExtensionConfig extension : extensions) {
-				this.extensions.add(new WebSocketExtension(ec.getName(), ec.getParameters()));
+				this.extensions.add(new WebSocketExtension(extension.getName(), extension.getParameters()));
 			}
+			this.extensions = Collections.unmodifiableList(this.extensions);
 		}
 		else {
-			this.extensions = new ArrayList<>(0);
+			this.extensions = Collections.emptyList();
 		}
 
 		if (this.user == null) {

spring-websocket/src/main/java/org/springframework/web/socket/adapter/standard/StandardWebSocketSession.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2017 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.
@@ -21,6 +21,7 @@ import java.net.InetSocketAddress;
 import java.net.URI;
 import java.security.Principal;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import javax.websocket.CloseReason;
@@ -29,6 +30,7 @@ import javax.websocket.Extension;
 import javax.websocket.Session;
 
 import org.springframework.http.HttpHeaders;
+import org.springframework.util.CollectionUtils;
 import org.springframework.web.socket.BinaryMessage;
 import org.springframework.web.socket.CloseStatus;
 import org.springframework.web.socket.PingMessage;
@@ -64,8 +66,7 @@ public class StandardWebSocketSession extends AbstractWebSocketSession<Session>
 
 
 	/**
-	 * Class constructor.
+	 * Constructor for a standard WebSocket session.
-	 *
 	 * @param headers the headers of the handshake request
 	 * @param attributes attributes from the HTTP handshake to associate with the WebSocket
 	 * session; the provided attributes are copied, the original map is not used.
@@ -79,8 +80,7 @@ public class StandardWebSocketSession extends AbstractWebSocketSession<Session>
 	}
 
 	/**
-	 * Class constructor that associates a user with the WebSocket session.
+	 * Constructor that associates a user with the WebSocket session.
-	 *
 	 * @param headers the headers of the handshake request
 	 * @param attributes attributes from the HTTP handshake to associate with the WebSocket session
 	 * @param localAddress the address on which the request was received
@@ -181,10 +181,16 @@ public class StandardWebSocketSession extends AbstractWebSocketSession<Session>
 
 		this.acceptedProtocol = session.getNegotiatedSubprotocol();
 
-		List<Extension> source = getNativeSession().getNegotiatedExtensions();
+		List<Extension> standardExtensions = getNativeSession().getNegotiatedExtensions();
-		this.extensions = new ArrayList<>(source.size());
+		if (!CollectionUtils.isEmpty(standardExtensions)) {
-		for (Extension ext : source) {
+			this.extensions = new ArrayList<>(standardExtensions.size());
-			this.extensions.add(new StandardToWebSocketExtensionAdapter(ext));
+			for (Extension standardExtension : standardExtensions) {
+				this.extensions.add(new StandardToWebSocketExtensionAdapter(standardExtension));
+			}
+			this.extensions = Collections.unmodifiableList(this.extensions);
+		}
+		else {
+			this.extensions = Collections.emptyList();
 		}
 
 		if (this.user == null) {

spring-websocket/src/main/java/org/springframework/web/socket/sockjs/client/XhrClientSockJsSession.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2017 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.sockjs.client;
 
 import java.net.InetSocketAddress;
 import java.net.URI;
+import java.util.Collections;
 import java.util.List;
 
 import org.springframework.http.HttpHeaders;
@@ -31,7 +32,6 @@ import org.springframework.web.socket.WebSocketHandler;
 import org.springframework.web.socket.WebSocketSession;
 import org.springframework.web.socket.sockjs.transport.TransportType;
 
-
 /**
  * An extension of {@link AbstractClientSockJsSession} for use with HTTP
  * transports simulating a WebSocket session.
@@ -58,7 +58,7 @@ public class XhrClientSockJsSession extends AbstractClientSockJsSession {
 			XhrTransport transport, SettableListenableFuture<WebSocketSession> connectFuture) {
 
 		super(request, handler, connectFuture);
-		Assert.notNull(transport, "'restTemplate' is required");
+		Assert.notNull(transport, "'transport' is required");
 		this.transport = transport;
 		this.headers = request.getHttpRequestHeaders();
 		this.sendHeaders = new HttpHeaders();
@@ -111,7 +111,7 @@ public class XhrClientSockJsSession extends AbstractClientSockJsSession {
 
 	@Override
 	public List<WebSocketExtension> getExtensions() {
-		return null;
+		return Collections.emptyList();
 	}
 
 	@Override
@@ -121,7 +121,7 @@ public class XhrClientSockJsSession extends AbstractClientSockJsSession {
 
 	@Override
 	protected void disconnect(CloseStatus status) {
-		// Nothing to do, XHR transports check if session is disconnected
+		// Nothing to do: XHR transports check if session is disconnected.
 	}
 
 }