From 8fecbab2c56d3f49d0353d58ee1681f3e6d3feca Mon Sep 17 00:00:00 2001
From: Artem Morozov <ar.m.morozov@gmail.com>
Date: Thu, 14 Feb 2019 04:14:58 +0300
Subject: [PATCH] Handle null "origin" header in "Old Hixie 75 handshake" as
 proper bad request. (#8864)

Motivation:

Gracefully respond on bad client request.
We have a set of errors produced by Android 7.1.1/7.1.2 clients where both headers `HttpHeaderNames.SEC_WEBSOCKET_VERSION` and `HttpHeaderNames.ORIGIN` are not present. Absence of the first headers leads to WebSocketServerHandshaker00 be applied as a handshaker. However, null 2nd header causes

```
java.lang.NullPointerException: value
 io.netty.util.internal.ObjectUtil.checkNotNull(ObjectUtil.java:33)
 io.netty.handler.codec.DefaultHeaders.addObject(DefaultHeaders.java:327)
 io.netty.handler.codec.http.DefaultHttpHeaders.add(DefaultHttpHeaders.java:123)
 io.netty.handler.codec.http.websocketx.WebSocketServerHandshaker00.newHandshakeResponse(WebSocketServerHandshaker00.java:162)
```
Which causes connection close with unclear reason.

Modification:

Added null-check, and in case of null an appropriate WebSocketHandshakeException is thrown.

Result:

In case of null `HttpHeaderNames.ORIGIN` header a WebSocketHandshakeException is caught by WebSocketServerProtocolHandler which sends a graceful `BAD_REQUEST`.
---
 .../WebSocketServerHandshaker00.java          |  6 +++-
 .../WebSocketServerHandshaker00Test.java      | 32 ++++++++++++++++++-
 2 files changed, 36 insertions(+), 2 deletions(-)

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 ff1797db64..1d06d9cc06 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
@@ -159,7 +159,11 @@ public class WebSocketServerHandshaker00 extends WebSocketServerHandshaker {
             res.content().writeBytes(WebSocketUtil.md5(input.array()));
         } else {
             // Old Hixie 75 handshake getMethod with no challenge:
-            res.headers().add(HttpHeaderNames.WEBSOCKET_ORIGIN, req.headers().get(HttpHeaderNames.ORIGIN));
+            String origin = req.headers().get(HttpHeaderNames.ORIGIN);
+            if (origin == null) {
+                throw new WebSocketHandshakeException("Missing origin header, got only " + req.headers().names());
+            }
+            res.headers().add(HttpHeaderNames.WEBSOCKET_ORIGIN, origin);
             res.headers().add(HttpHeaderNames.WEBSOCKET_LOCATION, uri());
 
             String protocol = req.headers().get(HttpHeaderNames.WEBSOCKET_PROTOCOL);
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00Test.java b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00Test.java
index 76826aba8b..8783e0b5bb 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00Test.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker00Test.java
@@ -32,7 +32,9 @@ import io.netty.util.CharsetUtil;
 import org.junit.Assert;
 import org.junit.Test;
 
-import static io.netty.handler.codec.http.HttpVersion.*;
+import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 
 public class WebSocketServerHandshaker00Test {
 
@@ -46,6 +48,34 @@ public class WebSocketServerHandshaker00Test {
         testPerformOpeningHandshake0(false);
     }
 
+    @Test
+    public void testPerformHandshakeWithoutOriginHeader() {
+        EmbeddedChannel ch = new EmbeddedChannel(
+            new HttpObjectAggregator(42), new HttpRequestDecoder(), new HttpResponseEncoder());
+
+        FullHttpRequest req = new DefaultFullHttpRequest(
+            HTTP_1_1, HttpMethod.GET, "/chat", Unpooled.copiedBuffer("^n:ds[4U", CharsetUtil.US_ASCII));
+
+        req.headers().set(HttpHeaderNames.HOST, "server.example.com");
+        req.headers().set(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET);
+        req.headers().set(HttpHeaderNames.CONNECTION, "Upgrade");
+        req.headers().set(HttpHeaderNames.SEC_WEBSOCKET_KEY1, "4 @1  46546xW%0l 1 5");
+        req.headers().set(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL, "chat, superchat");
+
+        WebSocketServerHandshaker00 handshaker00 = new WebSocketServerHandshaker00(
+            "ws://example.com/chat", "chat", Integer.MAX_VALUE);
+        try {
+            handshaker00.handshake(ch, req);
+            fail("Expecting WebSocketHandshakeException");
+        } catch (WebSocketHandshakeException e) {
+            assertEquals("Missing origin header, got only "
+                    + "[host, upgrade, connection, sec-websocket-key1, sec-websocket-protocol]",
+                e.getMessage());
+        } finally {
+            req.release();
+        }
+    }
+
     private static void testPerformOpeningHandshake0(boolean subProtocol) {
         EmbeddedChannel ch = new EmbeddedChannel(
                 new HttpObjectAggregator(42), new HttpRequestDecoder(), new HttpResponseEncoder());