From abd10d9089bb40dc75b067bdfe90a9befbfbe694 Mon Sep 17 00:00:00 2001 From: vibul Date: Sat, 12 May 2012 21:05:15 +1000 Subject: [PATCH 1/2] Fixed bug where subprotocol not sent by client --- .../websocketx/WebSocketClientHandshaker.java | 9 ++++--- .../WebSocketClientHandshaker00.java | 10 +++++--- .../WebSocketClientHandshaker08.java | 11 +++++--- .../WebSocketClientHandshaker13.java | 11 +++++--- .../websocketx/WebSocketServerHandshaker.java | 25 ++++++++++++++++--- .../WebSocketServerHandshaker00.java | 12 ++++++--- .../WebSocketServerHandshaker08.java | 13 +++++++--- .../WebSocketServerHandshaker13.java | 13 +++++++--- 8 files changed, 75 insertions(+), 29 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java index 04d6e2c1f7..ca91699a52 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java @@ -50,7 +50,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 */ @@ -78,7 +78,7 @@ public abstract class WebSocketClientHandshaker { Map 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; diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker00.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker00.java index b52b840907..0797e9132f 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker00.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker00.java @@ -169,10 +169,12 @@ public class WebSocketClientHandshaker00 extends WebSocketClientHandshaker { request.addHeader(Names.SEC_WEBSOCKET_KEY1, key1); request.addHeader(Names.SEC_WEBSOCKET_KEY2, key2); - if (getExpectedSubprotocol() != null && !getExpectedSubprotocol().equals("")) { - request.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, getExpectedSubprotocol()); + String expectedSubprotocol = this.getExpectedSubprotocol(); + if (expectedSubprotocol != null && !expectedSubprotocol.equals("")) { + request.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, expectedSubprotocol); } + if (customHeaders != null) { for (String header : customHeaders.keySet()) { request.addHeader(header, customHeaders.get(header)); @@ -235,8 +237,8 @@ public class WebSocketClientHandshaker00 extends WebSocketClientHandshaker { throw new WebSocketHandshakeException("Invalid challenge"); } - String protocol = response.getHeader(Names.SEC_WEBSOCKET_PROTOCOL); - setActualSubprotocol(protocol); + String subprotocol = response.getHeader(Names.SEC_WEBSOCKET_PROTOCOL); + setActualSubprotocol(subprotocol); channel.getPipeline().replace(HttpResponseDecoder.class, "ws-decoder", new WebSocket00FrameDecoder(this.getMaxFramePayloadLength())); diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker08.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker08.java index 5ba568e3cc..67bbca0c30 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker08.java +++ b/codec-http/src/main/java/io/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; /** @@ -157,9 +155,11 @@ public class WebSocketClientHandshaker08 extends WebSocketClientHandshaker { // See https://github.com/netty/netty/issues/264 request.addHeader(Names.SEC_WEBSOCKET_ORIGIN, originValue); - if (protocol != null && !protocol.equals("")) { - request.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, protocol); + String expectedSubprotocol = this.getExpectedSubprotocol(); + if (expectedSubprotocol != null && !expectedSubprotocol.equals("")) { + request.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, expectedSubprotocol); } + request.addHeader(Names.SEC_WEBSOCKET_VERSION, "8"); if (customHeaders != null) { @@ -224,6 +224,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())); diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker13.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker13.java index 5f31eed5b0..a4a079f3d3 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker13.java +++ b/codec-http/src/main/java/io/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("")) { - request.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, protocol); + String expectedSubprotocol = this.getExpectedSubprotocol(); + 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())); diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker.java index e648915a6f..4ccc548682 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker.java @@ -35,6 +35,8 @@ public abstract class WebSocketServerHandshaker { private final long maxFramePayloadLength; + private String selectedSubprotocol = null; + /** * Constructor using default values * @@ -49,7 +51,7 @@ public abstract class WebSocketServerHandshaker { protected WebSocketServerHandshaker(WebSocketVersion version, String webSocketUrl, String subprotocols) { this(version, webSocketUrl, subprotocols, Long.MAX_VALUE); } - + /** * Constructor specifying the destination web socket location * @@ -105,7 +107,7 @@ public abstract class WebSocketServerHandshaker { } /** - * Returns the max length for any frame's payload + * Returns the max length for any frame's payload */ public long getMaxFramePayloadLength() { return maxFramePayloadLength; @@ -143,8 +145,8 @@ public abstract class WebSocketServerHandshaker { return null; } - String[] requesteSubprotocolArray = requestedSubprotocols.split(","); - for (String p : requesteSubprotocolArray) { + String[] requestedSubprotocolArray = requestedSubprotocols.split(","); + for (String p : requestedSubprotocolArray) { String requestedSubprotocol = p.trim(); for (String supportedSubprotocol : subprotocols) { @@ -157,4 +159,19 @@ public abstract class WebSocketServerHandshaker { // No match found return null; } + + /** + * Returns the selected subprotocol. Null if no subprotocol has been selected. + *

+ * This is only available AFTER handshake() has been called. + *

+ */ + public String getSelectedSubprotocol() { + return selectedSubprotocol; + } + + protected void setSelectedSubprotocol(String value) { + selectedSubprotocol = value; + } + } diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00.java index 78a5f7669b..2e1c426c30 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00.java +++ b/codec-http/src/main/java/io/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); - if (protocol != null) { - res.addHeader(SEC_WEBSOCKET_PROTOCOL, selectSubprotocol(protocol)); + String subprotocols = req.getHeader(Names.SEC_WEBSOCKET_PROTOCOL); + if (subprotocols != null) { + 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. diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker08.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker08.java index 96eedfc580..a246347f2e 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker08.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker08.java @@ -148,11 +148,18 @@ 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); - if (protocol != null) { - res.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, selectSubprotocol(protocol)); + String subprotocols = req.getHeader(Names.SEC_WEBSOCKET_PROTOCOL); + if (subprotocols != null) { + 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); // Upgrade the connection and send the handshake response. diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker13.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker13.java index dab9a8f8d9..cdb410c386 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker13.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker13.java @@ -149,11 +149,18 @@ 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); - if (protocol != null) { - res.addHeader(Names.SEC_WEBSOCKET_PROTOCOL, selectSubprotocol(protocol)); + String subprotocols = req.getHeader(Names.SEC_WEBSOCKET_PROTOCOL); + if (subprotocols != null) { + 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); // Upgrade the connection and send the handshake response. From b09962f4c26c5231523584f82fc6cf86fb687970 Mon Sep 17 00:00:00 2001 From: vibul Date: Sat, 12 May 2012 21:22:33 +1000 Subject: [PATCH 2/2] forgot 1 more change --- .../codec/http/websocketx/WebSocketClientHandshaker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java index ca91699a52..2dfd44b670 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker.java +++ b/codec-http/src/main/java/io/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 customHeaders;