Skip to content

Commit 4ebc4ee

Browse files
amizurovamizurov
authored
Add check for |Connection| and |Upgrade| headers inside WebSocketServerHandshaker13, see https://datatracker.ietf.org/doc/html/rfc6455#section-4.2.1 (netty#12606)
Motivation: We faced with problem between proxies and Netty backend server. If we received a handshake request which doesn't contain a |Connection| or |Upgrade| headers with proper value the Netty accept this request and upgrade pipeline to handle websocket frames. But proxy thinks that is not upgrade request and can reuse this connection to any others http requests which we don't expect. Modification: Add check for |Connection| and |Upgrade| headers inside `WebSocketServerrHandshaker13` according spec https://datatracker.ietf.org/doc/html/rfc6455#section-4.2.1. Result: Proper handshake request handling for websocket version 13.
1 parent 2e29042 commit 4ebc4ee

File tree

3 files changed

+97
-3
lines changed

3 files changed

+97
-3
lines changed

‎codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker13.java‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,19 @@ public WebSocketServerHandshaker13(
134134
*/
135135
@Override
136136
protected FullHttpResponse newHandshakeResponse(FullHttpRequest req, HttpHeaders headers) {
137-
CharSequence key = req.headers().get(HttpHeaderNames.SEC_WEBSOCKET_KEY);
137+
HttpHeaders reqHeaders = req.headers();
138+
if (!reqHeaders.contains(HttpHeaderNames.CONNECTION) ||
139+
!reqHeaders.containsValue(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE, true)) {
140+
throw new WebSocketServerHandshakeException(
141+
"not a WebSocket request: a |Connection| header must includes a token 'Upgrade'", req);
142+
}
143+
144+
if (!reqHeaders.contains(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET, true)) {
145+
throw new WebSocketServerHandshakeException(
146+
"not a WebSocket request: a |Upgrade| header must containing the value 'websocket'", req);
147+
}
148+
149+
CharSequence key = reqHeaders.get(HttpHeaderNames.SEC_WEBSOCKET_KEY);
138150
if (key == null) {
139151
throw new WebSocketServerHandshakeException("not a WebSocket request: missing key", req);
140152
}
@@ -157,7 +169,7 @@ protected FullHttpResponse newHandshakeResponse(FullHttpRequest req, HttpHeaders
157169
.set(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE)
158170
.set(HttpHeaderNames.SEC_WEBSOCKET_ACCEPT, accept);
159171

160-
String subprotocols = req.headers().get(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL);
172+
String subprotocols = reqHeaders.get(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL);
161173
if (subprotocols != null) {
162174
String selectedSubprotocol = selectSubprotocol(subprotocols);
163175
if (selectedSubprotocol == null) {

‎codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketRequestBuilder.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ public static HttpRequest successful() {
156156
.uri("/test")
157157
.host("server.example.com")
158158
.upgrade(HttpHeaderValues.WEBSOCKET)
159+
.connection(HttpHeaderValues.UPGRADE)
159160
.key("dGhlIHNhbXBsZSBub25jZQ==")
160161
.origin("http://example.com")
161162
.version13()

‎codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerHandshaker13Test.java‎

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,18 +30,22 @@
3030
import io.netty.handler.codec.http.HttpResponseDecoder;
3131
import io.netty.handler.codec.http.HttpResponseEncoder;
3232
import io.netty.handler.codec.http.HttpServerCodec;
33+
import io.netty.handler.codec.http.HttpVersion;
3334
import io.netty.util.ReferenceCountUtil;
3435
import io.netty.util.ReferenceCounted;
3536
import org.hamcrest.CoreMatchers;
3637
import org.junit.jupiter.api.Test;
38+
import org.junit.jupiter.api.function.Executable;
3739

3840
import java.util.Iterator;
3941

40-
import static io.netty.handler.codec.http.HttpVersion.*;
42+
import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
4143
import static org.hamcrest.MatcherAssert.assertThat;
4244
import static org.junit.jupiter.api.Assertions.assertEquals;
4345
import static org.junit.jupiter.api.Assertions.assertFalse;
4446
import static org.junit.jupiter.api.Assertions.assertNull;
47+
import static org.junit.jupiter.api.Assertions.assertThrows;
48+
import static org.junit.jupiter.api.Assertions.assertTrue;
4549
import static org.junit.jupiter.api.Assertions.fail;
4650

4751
public class WebSocketServerHandshaker13Test extends WebSocketServerHandshakerTest {
@@ -91,6 +95,83 @@ public void testCloseReasonWithCodec() {
9195
testCloseReason0(new HttpServerCodec());
9296
}
9397

98+
@Test
99+
public void testHandshakeExceptionWhenConnectionHeaderIsAbsent() {
100+
final WebSocketServerHandshaker serverHandshaker = newHandshaker("ws://example.com/chat",
101+
"chat", WebSocketDecoderConfig.DEFAULT);
102+
final FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
103+
"ws://example.com/chat");
104+
request.headers()
105+
.set(HttpHeaderNames.HOST, "server.example.com")
106+
.set(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET)
107+
.set(HttpHeaderNames.SEC_WEBSOCKET_KEY, "dGhlIHNhbXBsZSBub25jZQ==")
108+
.set(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN, "http://example.com")
109+
.set(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL, "chat, superchat")
110+
.set(HttpHeaderNames.SEC_WEBSOCKET_VERSION, "13");
111+
Throwable exception = assertThrows(WebSocketServerHandshakeException.class, new Executable() {
112+
@Override
113+
public void execute() throws Throwable {
114+
serverHandshaker.handshake(null, request, null, null);
115+
}
116+
});
117+
118+
assertEquals("not a WebSocket request: a |Connection| header must includes a token 'Upgrade'",
119+
exception.getMessage());
120+
assertTrue(request.release());
121+
}
122+
123+
@Test
124+
public void testHandshakeExceptionWhenInvalidConnectionHeader() {
125+
final WebSocketServerHandshaker serverHandshaker = newHandshaker("ws://example.com/chat",
126+
"chat", WebSocketDecoderConfig.DEFAULT);
127+
final FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
128+
"ws://example.com/chat");
129+
request.headers()
130+
.set(HttpHeaderNames.HOST, "server.example.com")
131+
.set(HttpHeaderNames.CONNECTION, "close")
132+
.set(HttpHeaderNames.UPGRADE, HttpHeaderValues.WEBSOCKET)
133+
.set(HttpHeaderNames.SEC_WEBSOCKET_KEY, "dGhlIHNhbXBsZSBub25jZQ==")
134+
.set(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN, "http://example.com")
135+
.set(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL, "chat, superchat")
136+
.set(HttpHeaderNames.SEC_WEBSOCKET_VERSION, "13");
137+
Throwable exception = assertThrows(WebSocketServerHandshakeException.class, new Executable() {
138+
@Override
139+
public void execute() throws Throwable {
140+
serverHandshaker.handshake(null, request, null, null);
141+
}
142+
});
143+
144+
assertEquals("not a WebSocket request: a |Connection| header must includes a token 'Upgrade'",
145+
exception.getMessage());
146+
assertTrue(request.release());
147+
}
148+
149+
@Test
150+
public void testHandshakeExceptionWhenInvalidUpgradeHeader() {
151+
final WebSocketServerHandshaker serverHandshaker = newHandshaker("ws://example.com/chat",
152+
"chat", WebSocketDecoderConfig.DEFAULT);
153+
final FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
154+
"ws://example.com/chat");
155+
request.headers()
156+
.set(HttpHeaderNames.HOST, "server.example.com")
157+
.set(HttpHeaderNames.CONNECTION, HttpHeaderValues.UPGRADE)
158+
.set(HttpHeaderNames.UPGRADE, "my_websocket")
159+
.set(HttpHeaderNames.SEC_WEBSOCKET_KEY, "dGhlIHNhbXBsZSBub25jZQ==")
160+
.set(HttpHeaderNames.SEC_WEBSOCKET_ORIGIN, "http://example.com")
161+
.set(HttpHeaderNames.SEC_WEBSOCKET_PROTOCOL, "chat, superchat")
162+
.set(HttpHeaderNames.SEC_WEBSOCKET_VERSION, "13");
163+
Throwable exception = assertThrows(WebSocketServerHandshakeException.class, new Executable() {
164+
@Override
165+
public void execute() throws Throwable {
166+
serverHandshaker.handshake(null, request, null, null);
167+
}
168+
});
169+
170+
assertEquals("not a WebSocket request: a |Upgrade| header must containing the value 'websocket'",
171+
exception.getMessage());
172+
assertTrue(request.release());
173+
}
174+
94175
private static void testCloseReason0(ChannelHandler... handlers) {
95176
EmbeddedChannel ch = new EmbeddedChannel(
96177
new HttpObjectAggregator(42));

0 commit comments

Comments
 (0)