Repository: netty/netty
Issue: Spaces in WebSocket URIs not handled correctly
Labels: defect

Websocket connections to URIs that have spaces in the path can not be established in Netty 4.0.21.

The problematic code seams to be [this line](https://github.com/netty/netty/blob/master/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker13.java#L129). If the URI contains a space encoded as %20, the [getPath](http://docs.oracle.com/javase/8/docs/api/java/net/URI.html#getPath--) Method decodes this back to a normal space. This decoded space is then send in the HTTP request at some deeper layer without properly reencoding it.


Issue comments:
[This is the Workaround](https://github.com/saxsys/SynchronizeFX/blob/9789b889a160051c4f74097446a4bc563f2beabb/transmitter/netty-transmitter/src/main/java/de/saxsys/synchronizefx/netty/websockets/WhiteSpaceInPathWebSocketClientHandshaker13.java) i use. I basically use [getRawPath()](http://docs.oracle.com/javase/8/docs/api/java/net/URI.html#getRawPath--) instead of `getPath()`.

@rbi - Thanks for reporting. Would you mind submitting a PR?

Working on a fix...

Sorry for not answering but thank you for fixing it. I'm looking forward to Netty 4.0.34. 👍

Fixed via https://github.com/netty/netty/pull/4552


Pull request review comments:
Extract the return value of `getQuery()`

Fixed...


Patch that resolved the issue:
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 d67b85f3798..42ef07d704f 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
@@ -405,4 +405,17 @@ public ChannelFuture close(Channel channel, CloseWebSocketFrame frame, ChannelPr
         }
         return channel.writeAndFlush(frame, promise);
     }
+
+    /**
+     * Return the constructed raw path for the give {@link URI}.
+     */
+    static String rawPath(URI wsURL) {
+        String path = wsURL.getRawPath();
+        String query = wsURL.getQuery();
+        if (query != null && !query.isEmpty()) {
+            path = path + '?' + query;
+        }
+
+        return path == null || path.isEmpty() ? "/" : path;
+    }
 }
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 4ec7833e666..7ebc4bb7085 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
@@ -123,14 +123,7 @@ protected FullHttpRequest newHandshakeRequest() {
 
         // Get path
         URI wsURL = uri();
-        String path = wsURL.getPath();
-        if (wsURL.getQuery() != null && !wsURL.getQuery().isEmpty()) {
-            path = wsURL.getPath() + '?' + wsURL.getQuery();
-        }
-
-        if (path == null || path.isEmpty()) {
-            path = "/";
-        }
+        String path = rawPath(wsURL);
 
         // Format request
         FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, path);
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker07.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker07.java
index 6db3d4c5013..96d503d1a04 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker07.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker07.java
@@ -92,14 +92,7 @@ public WebSocketClientHandshaker07(URI webSocketURL, WebSocketVersion version, S
     protected FullHttpRequest newHandshakeRequest() {
         // Get path
         URI wsURL = uri();
-        String path = wsURL.getPath();
-        if (wsURL.getQuery() != null && !wsURL.getQuery().isEmpty()) {
-            path = wsURL.getPath() + '?' + wsURL.getQuery();
-        }
-
-        if (path == null || path.isEmpty()) {
-            path = "/";
-        }
+        String path = rawPath(wsURL);
 
         // Get 16 bit nonce and base 64 encode it
         byte[] nonce = WebSocketUtil.randomBytes(16);
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 d2eee599cb9..d30d04bb8d7 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
@@ -92,14 +92,7 @@ public WebSocketClientHandshaker08(URI webSocketURL, WebSocketVersion version, S
     protected FullHttpRequest newHandshakeRequest() {
         // Get path
         URI wsURL = uri();
-        String path = wsURL.getPath();
-        if (wsURL.getQuery() != null && !wsURL.getQuery().isEmpty()) {
-            path = wsURL.getPath() + '?' + wsURL.getQuery();
-        }
-
-        if (path == null || path.isEmpty()) {
-            path = "/";
-        }
+        String path = rawPath(wsURL);
 
         // Get 16 bit nonce and base 64 encode it
         byte[] nonce = WebSocketUtil.randomBytes(16);
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 a3d9b9c272a..ef88adc6c12 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
@@ -92,14 +92,7 @@ public WebSocketClientHandshaker13(URI webSocketURL, WebSocketVersion version, S
     protected FullHttpRequest newHandshakeRequest() {
         // Get path
         URI wsURL = uri();
-        String path = wsURL.getPath();
-        if (wsURL.getQuery() != null && !wsURL.getQuery().isEmpty()) {
-            path = wsURL.getPath() + '?' + wsURL.getQuery();
-        }
-
-        if (path == null || path.isEmpty()) {
-            path = "/";
-        }
+        String path = rawPath(wsURL);
 
         // Get 16 bit nonce and base 64 encode it
         byte[] nonce = WebSocketUtil.randomBytes(16);


Test patch:
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker00Test.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker00Test.java
new file mode 100644
index 00000000000..100abf694d4
--- /dev/null
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker00Test.java
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2015 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.handler.codec.http.websocketx;
+
+import java.net.URI;
+
+public class WebSocketClientHandshaker00Test extends WebSocketClientHandshakerTest {
+    @Override
+    protected WebSocketClientHandshaker newHandshaker(URI uri) {
+        return new WebSocketClientHandshaker00(uri, WebSocketVersion.V00, null, null, 1024);
+    }
+}
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker07Test.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker07Test.java
new file mode 100644
index 00000000000..168a2458d17
--- /dev/null
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker07Test.java
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2015 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.handler.codec.http.websocketx;
+
+import java.net.URI;
+
+public class WebSocketClientHandshaker07Test extends WebSocketClientHandshakerTest {
+    @Override
+    protected WebSocketClientHandshaker newHandshaker(URI uri) {
+        return new WebSocketClientHandshaker07(uri, WebSocketVersion.V07, null, false, null, 1024);
+    }
+}
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker08Test.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker08Test.java
new file mode 100644
index 00000000000..249bd958fb0
--- /dev/null
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker08Test.java
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2015 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.handler.codec.http.websocketx;
+
+import java.net.URI;
+
+public class WebSocketClientHandshaker08Test extends WebSocketClientHandshakerTest {
+    @Override
+    protected WebSocketClientHandshaker newHandshaker(URI uri) {
+        return new WebSocketClientHandshaker07(uri, WebSocketVersion.V08, null, false, null, 1024);
+    }
+}
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker13Test.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker13Test.java
new file mode 100644
index 00000000000..2bc2e691b22
--- /dev/null
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshaker13Test.java
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2015 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.handler.codec.http.websocketx;
+
+import java.net.URI;
+
+public class WebSocketClientHandshaker13Test extends WebSocketClientHandshakerTest {
+    @Override
+    protected WebSocketClientHandshaker newHandshaker(URI uri) {
+        return new WebSocketClientHandshaker13(uri, WebSocketVersion.V13, null, false, null, 1024);
+    }
+}
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakerTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakerTest.java
new file mode 100644
index 00000000000..e45c92349eb
--- /dev/null
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketClientHandshakerTest.java
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2015 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.handler.codec.http.websocketx;
+
+import io.netty.handler.codec.http.FullHttpRequest;
+import org.junit.Test;
+
+import java.net.URI;
+
+import static org.junit.Assert.assertEquals;
+
+public abstract class WebSocketClientHandshakerTest {
+    protected abstract WebSocketClientHandshaker newHandshaker(URI uri);
+
+    @Test
+    public void testRawPath() {
+        URI uri = URI.create("ws://localhost:9999/path%20with%20ws");
+        WebSocketClientHandshaker handshaker = newHandshaker(uri);
+        FullHttpRequest request = handshaker.newHandshakeRequest();
+        try {
+            assertEquals("/path%20with%20ws", request.getUri());
+        } finally {
+            request.release();
+        }
+    }
+}