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`.
This commit is contained in:
Artem Morozov 2019-02-14 04:14:58 +03:00 committed by Norman Maurer
parent c68e85b749
commit 8fecbab2c5
2 changed files with 36 additions and 2 deletions

View File

@ -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);

View File

@ -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());