From 16be36a55fded330a92a4869e127b6b7249c6d13 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Thu, 16 Jun 2016 21:22:15 +0200 Subject: [PATCH] [#5402] sec-websocket-origin should mention HTTPS Motivation: When HTTPS is used we should use https in the sec-websocket-origin / origin header Modifications: - Correctly generate the sec-websocket-origin / origin header - Add unit tests. Result: Generate correct header. --- .../websocketx/WebSocketClientHandshaker.java | 23 ++++++++++++++ .../WebSocketClientHandshaker00.java | 15 +++------- .../WebSocketClientHandshaker07.java | 15 ++++------ .../WebSocketClientHandshaker08.java | 15 ++++------ .../WebSocketClientHandshaker13.java | 24 +++------------ .../WebSocketClientHandshaker07Test.java | 30 +++++++++++++++++++ .../WebSocketClientHandshaker08Test.java | 2 +- .../WebSocketClientHandshaker13Test.java | 2 +- 8 files changed, 73 insertions(+), 53 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 4f88bc7cba..f91e4c7be0 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 @@ -32,6 +32,7 @@ import io.netty.handler.codec.http.HttpObjectAggregator; import io.netty.handler.codec.http.HttpRequestEncoder; import io.netty.handler.codec.http.HttpResponse; import io.netty.handler.codec.http.HttpResponseDecoder; +import io.netty.handler.codec.http.HttpScheme; import io.netty.util.ReferenceCountUtil; import io.netty.util.internal.EmptyArrays; import io.netty.util.internal.StringUtil; @@ -444,4 +445,26 @@ public abstract class WebSocketClientHandshaker { return path == null || path.isEmpty() ? "/" : path; } + + static int websocketPort(URI wsURL) { + // Format request + int wsPort = wsURL.getPort(); + // check if the URI contained a port if not set the correct one depending on the schema. + // See https://github.com/netty/netty/pull/1558 + if (wsPort == -1) { + return "wss".equals(wsURL.getScheme()) ? HttpScheme.HTTPS.port() : HttpScheme.HTTP.port(); + } + return wsPort; + } + + static CharSequence websocketOriginValue(String host, int wsPort) { + String originValue = (wsPort == HttpScheme.HTTPS.port() ? + HttpScheme.HTTPS.name() : HttpScheme.HTTP.name()) + "://" + host; + if (wsPort != HttpScheme.HTTP.port() && wsPort != HttpScheme.HTTPS.port()) { + // if the port is not standard (80/443) its needed to add the port to the header. + // See http://tools.ietf.org/html/rfc6454#section-6.2 + return originValue + ':' + wsPort; + } + return originValue; + } } 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 b077f9f543..d066069064 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 @@ -127,23 +127,16 @@ public class WebSocketClientHandshaker00 extends WebSocketClientHandshaker { // Get path URI wsURL = uri(); String path = rawPath(wsURL); + int wsPort = websocketPort(wsURL); + String host = wsURL.getHost(); // Format request FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, path); HttpHeaders headers = request.headers(); headers.add(HttpHeaderNames.UPGRADE, WEBSOCKET) .add(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE) - .add(HttpHeaderNames.HOST, wsURL.getHost()); - - int wsPort = wsURL.getPort(); - String originValue = "http://" + wsURL.getHost(); - if (wsPort != 80 && wsPort != 443) { - // if the port is not standard (80/443) its needed to add the port to the header. - // See http://tools.ietf.org/html/rfc6454#section-6.2 - originValue = originValue + ':' + wsPort; - } - - headers.add(HttpHeaderNames.ORIGIN, originValue) + .add(HttpHeaderNames.HOST, host) + .add(HttpHeaderNames.ORIGIN, websocketOriginValue(host, wsPort)) .add(HttpHeaderNames.SEC_WEBSOCKET_KEY1, key1) .add(HttpHeaderNames.SEC_WEBSOCKET_KEY2, key2); 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 c2618b2e47..d8e670f87a 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 @@ -141,6 +141,9 @@ public class WebSocketClientHandshaker07 extends WebSocketClientHandshaker { key, expectedChallengeResponseString); } + int wsPort = websocketPort(wsURL); + String host = wsURL.getHost(); + // Format request FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, path); HttpHeaders headers = request.headers(); @@ -148,16 +151,8 @@ public class WebSocketClientHandshaker07 extends WebSocketClientHandshaker { headers.add(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET) .add(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE) .add(HttpHeaderNames.SEC_WEBSOCKET_KEY, key) - .add(HttpHeaderNames.HOST, wsURL.getHost()); - - int wsPort = wsURL.getPort(); - String originValue = "http://" + wsURL.getHost(); - if (wsPort != 80 && wsPort != 443) { - // if the port is not standard (80/443) its needed to add the port to the header. - // See http://tools.ietf.org/html/rfc6454#section-6.2 - originValue = originValue + ':' + wsPort; - } - headers.add(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN, originValue); + .add(HttpHeaderNames.HOST, host) + .add(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN, websocketOriginValue(host, wsPort)); String expectedSubprotocol = expectedSubprotocol(); if (expectedSubprotocol != null && !expectedSubprotocol.isEmpty()) { 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 d589b7729c..e5ada055b4 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 @@ -142,6 +142,9 @@ public class WebSocketClientHandshaker08 extends WebSocketClientHandshaker { key, expectedChallengeResponseString); } + int wsPort = websocketPort(wsURL); + String host = wsURL.getHost(); + // Format request FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, path); HttpHeaders headers = request.headers(); @@ -149,16 +152,8 @@ public class WebSocketClientHandshaker08 extends WebSocketClientHandshaker { headers.add(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET) .add(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE) .add(HttpHeaderNames.SEC_WEBSOCKET_KEY, key) - .add(HttpHeaderNames.HOST, wsURL.getHost()); - - int wsPort = wsURL.getPort(); - String originValue = "http://" + wsURL.getHost(); - if (wsPort != 80 && wsPort != 443) { - // if the port is not standard (80/443) its needed to add the port to the header. - // See http://tools.ietf.org/html/rfc6454#section-6.2 - originValue = originValue + ':' + wsPort; - } - headers.add(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN, originValue); + .add(HttpHeaderNames.HOST, host) + .add(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN, websocketOriginValue(host, wsPort)); String expectedSubprotocol = expectedSubprotocol(); if (expectedSubprotocol != null && !expectedSubprotocol.isEmpty()) { 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 37a77c72aa..a311b83fbd 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 @@ -143,32 +143,16 @@ public class WebSocketClientHandshaker13 extends WebSocketClientHandshaker { } // Format request - int wsPort = wsURL.getPort(); - // check if the URI contained a port if not set the correct one depending on the schema. - // See https://github.com/netty/netty/pull/1558 - if (wsPort == -1) { - if ("wss".equals(wsURL.getScheme())) { - wsPort = 443; - } else { - wsPort = 80; - } - } - + int wsPort = websocketPort(wsURL); + String host = wsURL.getHost(); FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, path); HttpHeaders headers = request.headers(); headers.add(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET) .add(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE) .add(HttpHeaderNames.SEC_WEBSOCKET_KEY, key) - .add(HttpHeaderNames.HOST, wsURL.getHost() + ':' + wsPort); - - String originValue = "http://" + wsURL.getHost(); - if (wsPort != 80 && wsPort != 443) { - // if the port is not standard (80/443) its needed to add the port to the header. - // See http://tools.ietf.org/html/rfc6454#section-6.2 - originValue = originValue + ':' + wsPort; - } - headers.add(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN, originValue); + .add(HttpHeaderNames.HOST, host + ':' + wsPort) + .add(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN, websocketOriginValue(host, wsPort)); String expectedSubprotocol = expectedSubprotocol(); if (expectedSubprotocol != null && !expectedSubprotocol.isEmpty()) { 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 index 168a2458d1..91963102fd 100644 --- 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 @@ -15,11 +15,41 @@ */ package io.netty.handler.codec.http.websocketx; +import io.netty.handler.codec.http.FullHttpRequest; +import io.netty.handler.codec.http.HttpHeaderNames; +import org.junit.Test; + import java.net.URI; +import static org.junit.Assert.assertEquals; + public class WebSocketClientHandshaker07Test extends WebSocketClientHandshakerTest { @Override protected WebSocketClientHandshaker newHandshaker(URI uri) { return new WebSocketClientHandshaker07(uri, WebSocketVersion.V07, null, false, null, 1024); } + + @Test + public void testSecOriginWss() { + URI uri = URI.create("wss://localhost/path%20with%20ws"); + WebSocketClientHandshaker handshaker = newHandshaker(uri); + FullHttpRequest request = handshaker.newHandshakeRequest(); + try { + assertEquals("https://localhost", request.headers().get(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN)); + } finally { + request.release(); + } + } + + @Test + public void testSecOriginWs() { + URI uri = URI.create("ws://localhost/path%20with%20ws"); + WebSocketClientHandshaker handshaker = newHandshaker(uri); + FullHttpRequest request = handshaker.newHandshakeRequest(); + try { + assertEquals("http://localhost", request.headers().get(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN)); + } finally { + request.release(); + } + } } 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 index 249bd958fb..4ce8016add 100644 --- 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 @@ -17,7 +17,7 @@ package io.netty.handler.codec.http.websocketx; import java.net.URI; -public class WebSocketClientHandshaker08Test extends WebSocketClientHandshakerTest { +public class WebSocketClientHandshaker08Test extends WebSocketClientHandshaker07Test { @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 index 2bc2e691b2..ad89fde6bc 100644 --- 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 @@ -17,7 +17,7 @@ package io.netty.handler.codec.http.websocketx; import java.net.URI; -public class WebSocketClientHandshaker13Test extends WebSocketClientHandshakerTest { +public class WebSocketClientHandshaker13Test extends WebSocketClientHandshaker07Test { @Override protected WebSocketClientHandshaker newHandshaker(URI uri) { return new WebSocketClientHandshaker13(uri, WebSocketVersion.V13, null, false, null, 1024);