Fixed bug where subprotocol not sent by client

src/main/java/org/jboss/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java

@@ -35,7 +35,7 @@ public abstract class WebSocketClientHandshaker {
 
     private final String expectedSubprotocol;
 
-    private String actualSubprotocol;
+    private String actualSubprotocol = null;
 
     protected final Map<String, String> customHeaders;
 
@@ -68,7 +68,7 @@ public abstract class WebSocketClientHandshaker {
      * @param version
      *            Version of web socket specification to use to connect to the server
      * @param subprotocol
-     *            Sub protocol request sent to the server.
+     *            CSV of requested subprotocol(s) sent to the server.
      * @param customHeaders
      *            Map of custom headers to add to the client request
      * @param maxFramePayloadLength
@@ -78,7 +78,7 @@ public abstract class WebSocketClientHandshaker {
             Map<String, String> customHeaders, long maxFramePayloadLength) {
         this.webSocketUrl = webSocketUrl;
         this.version = version;
-        expectedSubprotocol = subprotocol;
+        this.expectedSubprotocol = subprotocol;
         this.customHeaders = customHeaders;
         this.maxFramePayloadLength = maxFramePayloadLength;
     }
@@ -116,14 +116,15 @@ public abstract class WebSocketClientHandshaker {
     }
 
     /**
-     * Returns the sub protocol request sent to the server as specified in the constructor
+     * Returns the CSV of requested subprotocol(s) sent to the server as specified in the constructor
      */
     public String getExpectedSubprotocol() {
         return expectedSubprotocol;
     }
 
     /**
-     * Returns the sub protocol response and sent by the server. Only available after end of handshake.
+     * Returns the subprotocol response sent by the server. Only available after end of handshake.
+     * Null if no subprotocol was requested or confirmed by the server.
      */
     public String getActualSubprotocol() {
         return actualSubprotocol;

src/main/java/org/jboss/netty/handler/codec/http/websocketx/WebSocketClientHandshaker00.java

@@ -170,8 +170,9 @@ public class WebSocketClientHandshaker00 extends WebSocketClientHandshaker {
         
         request.addHeader(Names.SEC_WEBSOCKET_KEY1, key1);
         request.addHeader(Names.SEC_WEBSOCKET_KEY2, key2);
-        if (getExpectedSubprotocol() != null && !getExpectedSubprotocol().equals("")) {
+        String expectedSubprotocol = this.getExpectedSubprotocol(); 
-            request.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, getExpectedSubprotocol());
+        if (expectedSubprotocol != null && !expectedSubprotocol.equals("")) {
+            request.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, expectedSubprotocol);
         }
 
         if (customHeaders != null) {
@@ -236,8 +237,8 @@ public class WebSocketClientHandshaker00 extends WebSocketClientHandshaker {
             throw new WebSocketHandshakeException("Invalid challenge");
         }
 
-        String protocol = response.getHeader(Names.SEC_WEBSOCKET_PROTOCOL);
+        String subprotocol = response.getHeader(Names.SEC_WEBSOCKET_PROTOCOL);
-        setActualSubprotocol(protocol);
+        setActualSubprotocol(subprotocol);
 
         channel.getPipeline().replace(HttpResponseDecoder.class, "ws-decoder",
                 new WebSocket00FrameDecoder(this.getMaxFramePayloadLength()));

src/main/java/org/jboss/netty/handler/codec/http/websocketx/WebSocketClientHandshaker08.java

@@ -49,8 +49,6 @@ public class WebSocketClientHandshaker08 extends WebSocketClientHandshaker {
 
     private String expectedChallengeResponseString;
 
-    private static final String protocol = null;
-
     private final boolean allowExtensions;
 
     /**
@@ -158,10 +156,11 @@ public class WebSocketClientHandshaker08 extends WebSocketClientHandshaker {
         // See https://github.com/netty/netty/issues/264
         request.addHeader(Names.SEC_WEBSOCKET_ORIGIN, originValue);
       
-        
+        String expectedSubprotocol = this.getExpectedSubprotocol(); 
-        if (protocol != null && !protocol.equals("")) {
+        if (expectedSubprotocol != null && !expectedSubprotocol.equals("")) {
-            request.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, protocol);
+            request.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, expectedSubprotocol);
         }
+        
         request.addHeader(Names.SEC_WEBSOCKET_VERSION, "8");
 
         if (customHeaders != null) {
@@ -226,6 +225,9 @@ public class WebSocketClientHandshaker08 extends WebSocketClientHandshaker {
                     expectedChallengeResponseString));
         }
 
+        String subprotocol = response.getHeader(Names.SEC_WEBSOCKET_PROTOCOL);
+        setActualSubprotocol(subprotocol);
+
         channel.getPipeline().replace(HttpResponseDecoder.class, "ws-decoder",
                 new WebSocket08FrameDecoder(false, allowExtensions, this.getMaxFramePayloadLength()));
 

src/main/java/org/jboss/netty/handler/codec/http/websocketx/WebSocketClientHandshaker13.java

@@ -49,8 +49,6 @@ public class WebSocketClientHandshaker13 extends WebSocketClientHandshaker {
 
     private String expectedChallengeResponseString;
 
-    private static final String protocol = null;
-
     private final boolean allowExtensions;
 
     /**
@@ -154,9 +152,11 @@ public class WebSocketClientHandshaker13 extends WebSocketClientHandshaker {
         }
         request.addHeader(Names.ORIGIN, originValue);
 
-        if (protocol != null && !protocol.equals("")) {
+        String expectedSubprotocol = this.getExpectedSubprotocol(); 
-            request.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, protocol);
+        if (expectedSubprotocol != null && !expectedSubprotocol.equals("")) {
+            request.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, expectedSubprotocol);
         }
+        
         request.addHeader(Names.SEC_WEBSOCKET_VERSION, "13");
 
         if (customHeaders != null) {
@@ -221,6 +221,9 @@ public class WebSocketClientHandshaker13 extends WebSocketClientHandshaker {
                     expectedChallengeResponseString));
         }
 
+        String subprotocol = response.getHeader(Names.SEC_WEBSOCKET_PROTOCOL);
+        setActualSubprotocol(subprotocol);
+
         channel.getPipeline().replace(HttpResponseDecoder.class, "ws-decoder",
                 new WebSocket13FrameDecoder(false, allowExtensions, this.getMaxFramePayloadLength()));
 

src/main/java/org/jboss/netty/handler/codec/http/websocketx/WebSocketServerHandshaker.java

@@ -37,8 +37,11 @@ public abstract class WebSocketServerHandshaker {
 
     private final long maxFramePayloadLength;
 
+    private String selectedSubprotocol = null;
+
     /**
-     * {@link ChannelFutureListener} which will call {@link Channels#fireExceptionCaught(org.jboss.netty.channel.ChannelHandlerContext, Throwable)} 
+     * {@link ChannelFutureListener} which will call
+     * {@link Channels#fireExceptionCaught(org.jboss.netty.channel.ChannelHandlerContext, Throwable)}
      * if the {@link ChannelFuture} was not successful.
      */
     public static final ChannelFutureListener HANDSHAKE_LISTENER = new ChannelFutureListener() {
@@ -55,10 +58,12 @@ public abstract class WebSocketServerHandshaker {
      * @param version
      *            the protocol version
      * @param webSocketUrl
-     *            URL for web socket communications. e.g "ws://myhost.com/mypath". Subsequent web socket frames will be
+     *            URL for web socket communications. e.g
+     *            "ws://myhost.com/mypath". Subsequent web socket frames will be
      *            sent to this URL.
      * @param subprotocols
-     *            CSV of supported protocols. Null if sub protocols not supported.
+     *            CSV of supported protocols. Null if sub protocols not
+     *            supported.
      */
     protected WebSocketServerHandshaker(WebSocketVersion version, String webSocketUrl, String subprotocols) {
         this(version, webSocketUrl, subprotocols, Long.MAX_VALUE);
@@ -70,10 +75,12 @@ public abstract class WebSocketServerHandshaker {
      * @param version
      *            the protocol version
      * @param webSocketUrl
-     *            URL for web socket communications. e.g "ws://myhost.com/mypath". Subsequent web socket frames will be
+     *            URL for web socket communications. e.g
+     *            "ws://myhost.com/mypath". Subsequent web socket frames will be
      *            sent to this URL.
      * @param subprotocols
-     *            CSV of supported protocols. Null if sub protocols not supported.
+     *            CSV of supported protocols. Null if sub protocols not
+     *            supported.
      * @param maxFramePayloadLength
      *            Maximum length of a frame's payload
      */
@@ -93,7 +100,6 @@ public abstract class WebSocketServerHandshaker {
         this.maxFramePayloadLength = maxFramePayloadLength;
     }
 
-
     /**
      * Returns the URL of the web socket
      */
@@ -158,8 +164,8 @@ public abstract class WebSocketServerHandshaker {
             return null;
         }
 
-        String[] requesteSubprotocolArray = requestedSubprotocols.split(",");
+        String[] requestedSubprotocolArray = requestedSubprotocols.split(",");
-        for (String p: requesteSubprotocolArray) {
+        for (String p : requestedSubprotocolArray) {
             String requestedSubprotocol = p.trim();
 
             for (String supportedSubprotocol : subprotocols) {
@@ -172,4 +178,19 @@ public abstract class WebSocketServerHandshaker {
         // No match found
         return null;
     }
+    
+    /**
+     * Returns the selected subprotocol. Null if no subprotocol has been selected. 
+     * <p>
+     * This is only available AFTER <tt>handshake()</tt> has been called.
+     * </p>
+     */
+    public String getSelectedSubprotocol() {
+        return selectedSubprotocol;
+    }
+    
+    protected void setSelectedSubprotocol(String value) {
+        selectedSubprotocol = value;
+    }
+    
 }

src/main/java/org/jboss/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00.java

@@ -151,9 +151,15 @@ public class WebSocketServerHandshaker00 extends WebSocketServerHandshaker {
             // New handshake method with a challenge:
             res.addHeader(SEC_WEBSOCKET_ORIGIN, req.getHeader(ORIGIN));
             res.addHeader(SEC_WEBSOCKET_LOCATION, getWebSocketUrl());
-            String protocol = req.getHeader(SEC_WEBSOCKET_PROTOCOL);
+            String subprotocols = req.getHeader(Names.SEC_WEBSOCKET_PROTOCOL);
-            if (protocol != null) {
+            if (subprotocols != null) {
-                res.addHeader(SEC_WEBSOCKET_PROTOCOL, selectSubprotocol(protocol));
+                String selectedSubprotocol = selectSubprotocol(subprotocols);
+                if (selectedSubprotocol == null) {
+                    throw new WebSocketHandshakeException("Requested subprotocol(s) not supported: " + subprotocols);
+                } else {
+                    res.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, selectedSubprotocol);
+                    this.setSelectedSubprotocol(selectedSubprotocol);
+                }
             }
 
             // Calculate the answer of the challenge.

src/main/java/org/jboss/netty/handler/codec/http/websocketx/WebSocketServerHandshaker08.java

@@ -156,9 +156,15 @@ public class WebSocketServerHandshaker08 extends WebSocketServerHandshaker {
         res.addHeader(Names.UPGRADE, WEBSOCKET.toLowerCase());
         res.addHeader(Names.CONNECTION, Names.UPGRADE);
         res.addHeader(Names.SEC_WEBSOCKET_ACCEPT, accept);
-        String protocol = req.getHeader(Names.SEC_WEBSOCKET_PROTOCOL);
+        String subprotocols = req.getHeader(Names.SEC_WEBSOCKET_PROTOCOL);
-        if (protocol != null) {
+        if (subprotocols != null) {
-            res.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, selectSubprotocol(protocol));
+            String selectedSubprotocol = selectSubprotocol(subprotocols);
+            if (selectedSubprotocol == null) {
+                throw new WebSocketHandshakeException("Requested subprotocol(s) not supported: " + subprotocols);
+            } else {
+                res.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, selectedSubprotocol);
+                this.setSelectedSubprotocol(selectedSubprotocol);
+            }
         }
 
         ChannelFuture future = channel.write(res);

src/main/java/org/jboss/netty/handler/codec/http/websocketx/WebSocketServerHandshaker13.java

@@ -39,10 +39,11 @@ import org.jboss.netty.util.CharsetUtil;
 
 /**
  * <p>
- * Performs server side opening and closing handshakes for <a href="http://tools.ietf.org/html/rfc6455 ">RFC 6455</a>
+ * Performs server side opening and closing handshakes for <a
- * (originally web socket specification version <a
+ * href="http://tools.ietf.org/html/rfc6455 ">RFC 6455</a> (originally web
- * href="http://tools.ietf.org/html/draft-ietf-hybi-thewebsocketprotocol-17" >draft-ietf-hybi-thewebsocketprotocol-
+ * socket specification version <a
- * 17</a>).
+ * href="http://tools.ietf.org/html/draft-ietf-hybi-thewebsocketprotocol-17"
+ * >draft-ietf-hybi-thewebsocketprotocol- 17</a>).
  * </p>
  */
 public class WebSocketServerHandshaker13 extends WebSocketServerHandshaker {
@@ -57,12 +58,14 @@ public class WebSocketServerHandshaker13 extends WebSocketServerHandshaker {
      * Constructor using defaults
      * 
      * @param webSocketURL
-     *            URL for web socket communications. e.g "ws://myhost.com/mypath". Subsequent web socket frames will be
+     *            URL for web socket communications. e.g
+     *            "ws://myhost.com/mypath". Subsequent web socket frames will be
      *            sent to this URL.
      * @param subprotocols
      *            CSV of supported protocols
      * @param allowExtensions
-     *            Allow extensions to be used in the reserved bits of the web socket frame
+     *            Allow extensions to be used in the reserved bits of the web
+     *            socket frame
      */
     public WebSocketServerHandshaker13(String webSocketURL, String subprotocols, boolean allowExtensions) {
         this(webSocketURL, subprotocols, allowExtensions, Long.MAX_VALUE);
@@ -72,15 +75,18 @@ public class WebSocketServerHandshaker13 extends WebSocketServerHandshaker {
      * Constructor specifying the destination web socket location
      * 
      * @param webSocketURL
-     *            URL for web socket communications. e.g "ws://myhost.com/mypath". Subsequent web socket frames will be
+     *            URL for web socket communications. e.g
+     *            "ws://myhost.com/mypath". Subsequent web socket frames will be
      *            sent to this URL.
      * @param subprotocols
      *            CSV of supported protocols
      * @param allowExtensions
-     *            Allow extensions to be used in the reserved bits of the web socket frame
+     *            Allow extensions to be used in the reserved bits of the web
+     *            socket frame
      * @param maxFramePayloadLength
-     *            Maximum allowable frame payload length. Setting this value to your application's requirement may
+     *            Maximum allowable frame payload length. Setting this value to
-     *            reduce denial of service attacks using long data frames.
+     *            your application's requirement may reduce denial of service
+     *            attacks using long data frames.
      */
     public WebSocketServerHandshaker13(String webSocketURL, String subprotocols, boolean allowExtensions,
             long maxFramePayloadLength) {
@@ -88,12 +94,11 @@ public class WebSocketServerHandshaker13 extends WebSocketServerHandshaker {
         this.allowExtensions = allowExtensions;
     }
 
-
     /**
      * <p>
      * Handle the web socket handshake for the web socket specification <a href=
-     * "http://tools.ietf.org/html/draft-ietf-hybi-thewebsocketprotocol-17">HyBi versions 13-17</a>. Versions 13-17
+     * "http://tools.ietf.org/html/draft-ietf-hybi-thewebsocketprotocol-17">HyBi
-     * share the same wire protocol.
+     * versions 13-17</a>. Versions 13-17 share the same wire protocol.
      * </p>
      * 
      * <p>
@@ -158,9 +163,15 @@ public class WebSocketServerHandshaker13 extends WebSocketServerHandshaker {
         res.addHeader(Names.UPGRADE, WEBSOCKET.toLowerCase());
         res.addHeader(Names.CONNECTION, Names.UPGRADE);
         res.addHeader(Names.SEC_WEBSOCKET_ACCEPT, accept);
-        String protocol = req.getHeader(Names.SEC_WEBSOCKET_PROTOCOL);
+        String subprotocols = req.getHeader(Names.SEC_WEBSOCKET_PROTOCOL);
-        if (protocol != null) {
+        if (subprotocols != null) {
-            res.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, selectSubprotocol(protocol));
+            String selectedSubprotocol = selectSubprotocol(subprotocols);
+            if (selectedSubprotocol == null) {
+                throw new WebSocketHandshakeException("Requested subprotocol(s) not supported: " + subprotocols);
+            } else {
+                res.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, selectedSubprotocol);
+                this.setSelectedSubprotocol(selectedSubprotocol);
+            }
         }
 
         ChannelFuture future = channel.write(res);