Repository: netty/netty (Java)

Issue:
Origin header is always sent from WebSocket client
### Expected behavior

No 'Origin' header sent by default.
A way to set custom 'Origin' header when it is needed.

### Actual behavior

'Origin' header is set.
'Origin' header is auto-resolved into meaningless value. In my case - IP of the intermediate proxy between client and server sides.
Enforced 'Origin' header leads to cors check failure on server side and denial of service for legitim client.

### Steps to reproduce

Open WebSocket connection with v13 handshaker 

    WebSocketClientHandshaker13.newHandshakeRequest:

    if (!headers.contains(HttpHeaderNames.ORIGIN)) {
        headers.set(HttpHeaderNames.ORIGIN, websocketOriginValue(wsURL));
    }

### Netty version

4.1.39

Related issues: PR #9312 and PR #9435.

### JVM version (e.g. `java -version`)

1.8.0_223-b27

### OS version (e.g. `uname -a`)

* Windows 7 Enterprise
* Linux gmafxu42 3.0.101-108.98-default #1 SMP Mon Jul 15 13:58:06 UTC 2019 (262a94d) x86_64 x86_64 x86_64 GNU/Linux


Hints:
Hi, @ursaj. I agree that `Origin` and  `Sec-WebSocket-Origin` is not required according the spec https://tools.ietf.org/html/rfc6455#section-1.3 `Origin/Sec-WebSocket-Origin header field is sent by browser clients; for non-browser clients, this header field may be sent if it makes sense in the context of those clients`.  But i'm not sure can we just delete them. WDYT @normanmaurer @slandelle 
I agree there's no way for an HTTP client to be able to infer Origin header and it can only be provided reliably by the user. IMHO, this is a bug, but fixing it would be a breaking change.
I say we should have an option to disable and deprecate sending Origin header for removal in Netty 5.
Actually it is already a breaking change between 4.1.31 and 4.1.39 - when 'Origin' header was forcibly injected. I'd spend quite a lot of effort to sort it out, when Singapore clients failed to connect to our backends during their upgrade. For me it was 4 a.m. on Sunday - quite an inconvenient time to solve such issues.

I'll better interpret it like a "temporary bug" and revert this behaviour to the one of 4.1.31.
@slandelle I agree with @ursaj ... we should fix it.  @ursaj can you do a pr ?

Patch:
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
--- 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
@@ -48,8 +48,6 @@
  */
 public abstract class WebSocketClientHandshaker {
 
-    private static final String HTTP_SCHEME_PREFIX = HttpScheme.HTTP + "://";
-    private static final String HTTPS_SCHEME_PREFIX = HttpScheme.HTTPS + "://";
     protected static final int DEFAULT_FORCE_CLOSE_TIMEOUT_MILLIS = 10000;
 
     private final URI uri;
@@ -601,31 +599,4 @@ static CharSequence websocketHostValue(URI wsURL) {
         // See http://tools.ietf.org/html/rfc6454#section-6.2
         return NetUtil.toSocketAddressString(host, port);
     }
-
-    static CharSequence websocketOriginValue(URI wsURL) {
-        String scheme = wsURL.getScheme();
-        final String schemePrefix;
-        int port = wsURL.getPort();
-        final int defaultPort;
-        if (WebSocketScheme.WSS.name().contentEquals(scheme)
-            || HttpScheme.HTTPS.name().contentEquals(scheme)
-            || (scheme == null && port == WebSocketScheme.WSS.port())) {
-
-            schemePrefix = HTTPS_SCHEME_PREFIX;
-            defaultPort = WebSocketScheme.WSS.port();
-        } else {
-            schemePrefix = HTTP_SCHEME_PREFIX;
-            defaultPort = WebSocketScheme.WS.port();
-        }
-
-        // Convert uri-host to lower case (by RFC 6454, chapter 4 "Origin of a URI")
-        String host = wsURL.getHost().toLowerCase(Locale.US);
-
-        if (port != defaultPort && port != -1) {
-            // 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 schemePrefix + NetUtil.toSocketAddressString(host, port);
-        }
-        return schemePrefix + host;
-    }
 }
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
--- 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
@@ -192,10 +192,6 @@ protected FullHttpRequest newHandshakeRequest() {
                .set(HttpHeaderNames.SEC_WEBSOCKET_KEY1, key1)
                .set(HttpHeaderNames.SEC_WEBSOCKET_KEY2, key2);
 
-        if (!headers.contains(HttpHeaderNames.ORIGIN)) {
-            headers.set(HttpHeaderNames.ORIGIN, websocketOriginValue(wsURL));
-        }
-
         String expectedSubprotocol = expectedSubprotocol();
         if (expectedSubprotocol != null && !expectedSubprotocol.isEmpty()) {
             headers.set(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL, expectedSubprotocol);
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
--- 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
@@ -187,7 +187,6 @@ public WebSocketClientHandshaker07(URI webSocketURL, WebSocketVersion version, S
      * Upgrade: websocket
      * Connection: Upgrade
      * Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==
-     * Sec-WebSocket-Origin: http://example.com
      * Sec-WebSocket-Protocol: chat, superchat
      * Sec-WebSocket-Version: 7
      * </pre>
@@ -225,10 +224,6 @@ protected FullHttpRequest newHandshakeRequest() {
                .set(HttpHeaderNames.SEC_WEBSOCKET_KEY, key)
                .set(HttpHeaderNames.HOST, websocketHostValue(wsURL));
 
-        if (!headers.contains(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN)) {
-            headers.set(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN, websocketOriginValue(wsURL));
-        }
-
         String expectedSubprotocol = expectedSubprotocol();
         if (expectedSubprotocol != null && !expectedSubprotocol.isEmpty()) {
             headers.set(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL, expectedSubprotocol);
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
--- 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
@@ -189,7 +189,6 @@ public WebSocketClientHandshaker08(URI webSocketURL, WebSocketVersion version, S
      * Upgrade: websocket
      * Connection: Upgrade
      * Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==
-     * Sec-WebSocket-Origin: http://example.com
      * Sec-WebSocket-Protocol: chat, superchat
      * Sec-WebSocket-Version: 8
      * </pre>
@@ -227,10 +226,6 @@ protected FullHttpRequest newHandshakeRequest() {
                .set(HttpHeaderNames.SEC_WEBSOCKET_KEY, key)
                .set(HttpHeaderNames.HOST, websocketHostValue(wsURL));
 
-        if (!headers.contains(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN)) {
-            headers.set(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN, websocketOriginValue(wsURL));
-        }
-
         String expectedSubprotocol = expectedSubprotocol();
         if (expectedSubprotocol != null && !expectedSubprotocol.isEmpty()) {
             headers.set(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL, expectedSubprotocol);
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
--- 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
@@ -190,7 +190,6 @@ public WebSocketClientHandshaker13(URI webSocketURL, WebSocketVersion version, S
      * Upgrade: websocket
      * Connection: Upgrade
      * Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==
-     * Origin: http://example.com
      * Sec-WebSocket-Protocol: chat, superchat
      * Sec-WebSocket-Version: 13
      * </pre>
@@ -228,10 +227,6 @@ protected FullHttpRequest newHandshakeRequest() {
                .set(HttpHeaderNames.SEC_WEBSOCKET_KEY, key)
                .set(HttpHeaderNames.HOST, websocketHostValue(wsURL));
 
-        if (!headers.contains(HttpHeaderNames.ORIGIN)) {
-            headers.set(HttpHeaderNames.ORIGIN, websocketOriginValue(wsURL));
-        }
-
         String expectedSubprotocol = expectedSubprotocol();
         if (expectedSubprotocol != null && !expectedSubprotocol.isEmpty()) {
             headers.set(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL, expectedSubprotocol);
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker07.java b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker07.java
--- a/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker07.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker07.java
@@ -109,7 +109,6 @@ public WebSocketServerHandshaker07(String webSocketURL, String subprotocols, Web
      * Upgrade: websocket
      * Connection: Upgrade
      * Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==
-     * Sec-WebSocket-Origin: http://example.com
      * Sec-WebSocket-Protocol: chat, superchat
      * Sec-WebSocket-Version: 7
      * </pre>
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
--- 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
@@ -116,7 +116,6 @@ public WebSocketServerHandshaker08(
      * Upgrade: websocket
      * Connection: Upgrade
      * Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==
-     * Sec-WebSocket-Origin: http://example.com
      * Sec-WebSocket-Protocol: chat, superchat
      * Sec-WebSocket-Version: 8
      * </pre>
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
--- 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
@@ -115,7 +115,6 @@ public WebSocketServerHandshaker13(
      * Upgrade: websocket
      * Connection: Upgrade
      * Sec-WebSocket-Key: dGhlIHNhbXBsZSBub25jZQ==
-     * Origin: http://example.com
      * Sec-WebSocket-Protocol: chat, superchat
      * Sec-WebSocket-Version: 13
      * </pre>


Test patch:
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
--- 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
@@ -31,19 +31,27 @@
 import io.netty.handler.codec.http.HttpObjectAggregator;
 import io.netty.handler.codec.http.HttpRequestEncoder;
 import io.netty.handler.codec.http.HttpResponseDecoder;
+import io.netty.handler.codec.http.HttpScheme;
 import io.netty.util.CharsetUtil;
+import io.netty.util.NetUtil;
 import org.junit.Test;
 
 import java.net.URI;
+import java.util.Locale;
 
 import static org.junit.Assert.*;
 
 public abstract class WebSocketClientHandshakerTest {
+    private static final String HTTP_SCHEME_PREFIX = HttpScheme.HTTP + "://";
+    private static final String HTTPS_SCHEME_PREFIX = HttpScheme.HTTPS + "://";
+
     protected abstract WebSocketClientHandshaker newHandshaker(URI uri, String subprotocol, HttpHeaders headers,
                                                                boolean absoluteUpgradeUrl);
 
     protected WebSocketClientHandshaker newHandshaker(URI uri) {
-        return newHandshaker(uri, null, null, false);
+        HttpHeaders headers = new DefaultHttpHeaders()
+            .add(getOriginHeaderName(), websocketOriginValue(uri));
+        return newHandshaker(uri, null, headers, false);
     }
 
     protected abstract CharSequence getOriginHeaderName();
@@ -360,4 +368,31 @@ public void testDuplicateWebsocketHandshakeHeaders() {
 
         request.release();
     }
+
+    static CharSequence websocketOriginValue(URI wsURL) {
+        String scheme = wsURL.getScheme();
+        final String schemePrefix;
+        int port = wsURL.getPort();
+        final int defaultPort;
+        if (WebSocketScheme.WSS.name().contentEquals(scheme)
+            || HttpScheme.HTTPS.name().contentEquals(scheme)
+            || (scheme == null && port == WebSocketScheme.WSS.port())) {
+
+            schemePrefix = HTTPS_SCHEME_PREFIX;
+            defaultPort = WebSocketScheme.WSS.port();
+        } else {
+            schemePrefix = HTTP_SCHEME_PREFIX;
+            defaultPort = WebSocketScheme.WS.port();
+        }
+
+        // Convert uri-host to lower case (by RFC 6454, chapter 4 "Origin of a URI")
+        String host = wsURL.getHost().toLowerCase(Locale.US);
+
+        if (port != defaultPort && port != -1) {
+            // 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 schemePrefix + NetUtil.toSocketAddressString(host, port);
+        }
+        return schemePrefix + host;
+    }
 }
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketHandshakeHandOverTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketHandshakeHandOverTest.java
--- a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketHandshakeHandOverTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketHandshakeHandOverTest.java
@@ -111,7 +111,7 @@ protected void channelRead0(ChannelHandlerContext ctx, Object msg) throws Except
         assertTrue(serverReceivedHandshake);
         assertNotNull(serverHandshakeComplete);
         assertEquals("/test", serverHandshakeComplete.requestUri());
-        assertEquals(8, serverHandshakeComplete.requestHeaders().size());
+        assertEquals(7, serverHandshakeComplete.requestHeaders().size());
         assertEquals("test-proto-2", serverHandshakeComplete.selectedSubprotocol());
 
         // Transfer the handshake response and the websocket message to the client