andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Ensure we replace WebSocketServerProtocolHandshakeHandler before doing the handshake (#9472)

Browse Source 
Motivation:

We need to ensure we replace WebSocketServerProtocolHandshakeHandler before doing the actual handshake as the handshake itself may complete directly and so forward pending bytes through the pipeline.

Modifications:

Replace the handler before doing the actual handshake.

Result:

Fixes https://github.com/netty/netty/issues/9471.
This commit is contained in:
Norman Maurer 2019-08-17 10:00:01 +02:00 committed by GitHub
parent d0bfbdde8e
commit 839aab8558
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 31 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandshakeHandler.java
View File

@ -88,6 +88,15 @@ class WebSocketServerProtocolHandshakeHandler extends ChannelInboundHandlerAdapt
if (handshaker == null) {
WebSocketServerHandshakerFactory.sendUnsupportedVersionResponse(ctx.channel());
} else {
// Ensure we set the handshaker and replace this handler before we
// trigger the actual handshake. Otherwise we may receive websocket bytes in this handler
// before we had a chance to replace it.
//
// See https://github.com/netty/netty/issues/9471.
WebSocketServerProtocolHandler.setHandshaker(ctx.channel(), handshaker);
ctx.pipeline().replace(this, "WS403Responder",
WebSocketServerProtocolHandler.forbiddenHttpRequestResponder());
final ChannelFuture handshakeFuture = handshaker.handshake(ctx.channel(), req);
handshakeFuture.addListener(new ChannelFutureListener() {
@Override
@ -107,9 +116,6 @@ class WebSocketServerProtocolHandshakeHandler extends ChannelInboundHandlerAdapt
}
});
applyHandshakeTimeout();
WebSocketServerProtocolHandler.setHandshaker(ctx.channel(), handshaker);
ctx.pipeline().replace(this, "WS403Responder",
WebSocketServerProtocolHandler.forbiddenHttpRequestResponder());
}
} finally {
req.release();

22
codec-http/src/test/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandlerTest.java
View File

@ -63,6 +63,28 @@ public class WebSocketServerProtocolHandlerTest {
assertFalse(ch.finish());
}
@Test
public void testWebSocketServerProtocolHandshakeHandlerReplacedBeforeHandshake() throws Exception {
EmbeddedChannel ch = createChannel(new MockOutboundHandler());
ChannelHandlerContext handshakerCtx = ch.pipeline().context(WebSocketServerProtocolHandshakeHandler.class);
ch.pipeline().addLast(new ChannelInboundHandlerAdapter() {
@Override
public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
if (evt instanceof WebSocketServerProtocolHandler.HandshakeComplete) {
// We should have removed the handler already.
assertNull(ctx.pipeline().context(WebSocketServerProtocolHandshakeHandler.class));
}
}
});
writeUpgradeRequest(ch);
FullHttpResponse response = responses.remove();
assertEquals(SWITCHING_PROTOCOLS, response.status());
response.release();
assertNotNull(WebSocketServerProtocolHandler.getHandshaker(handshakerCtx.channel()));
assertFalse(ch.finish());
}
@Test
public void testSubsequentHttpRequestsAfterUpgradeShouldReturn403() throws Exception {
EmbeddedChannel ch = createChannel();