Ensure we replace WebSocketServerProtocolHandshakeHandler before doing the handshake (#9472)
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:
parent
1a53df1031
commit
67b851209f
|
@ -88,6 +88,15 @@ class WebSocketServerProtocolHandshakeHandler implements ChannelInboundHandler {
|
||||||
if (handshaker == null) {
|
if (handshaker == null) {
|
||||||
WebSocketServerHandshakerFactory.sendUnsupportedVersionResponse(ctx.channel());
|
WebSocketServerHandshakerFactory.sendUnsupportedVersionResponse(ctx.channel());
|
||||||
} else {
|
} 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);
|
final ChannelFuture handshakeFuture = handshaker.handshake(ctx.channel(), req);
|
||||||
handshakeFuture.addListener((ChannelFutureListener) future -> {
|
handshakeFuture.addListener((ChannelFutureListener) future -> {
|
||||||
if (!future.isSuccess()) {
|
if (!future.isSuccess()) {
|
||||||
|
@ -104,9 +113,6 @@ class WebSocketServerProtocolHandshakeHandler implements ChannelInboundHandler {
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
applyHandshakeTimeout();
|
applyHandshakeTimeout();
|
||||||
WebSocketServerProtocolHandler.setHandshaker(ctx.channel(), handshaker);
|
|
||||||
ctx.pipeline().replace(this, "WS403Responder",
|
|
||||||
WebSocketServerProtocolHandler.forbiddenHttpRequestResponder());
|
|
||||||
}
|
}
|
||||||
} finally {
|
} finally {
|
||||||
req.release();
|
req.release();
|
||||||
|
|
|
@ -62,6 +62,28 @@ public class WebSocketServerProtocolHandlerTest {
|
||||||
assertFalse(ch.finish());
|
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
|
@Test
|
||||||
public void testSubsequentHttpRequestsAfterUpgradeShouldReturn403() throws Exception {
|
public void testSubsequentHttpRequestsAfterUpgradeShouldReturn403() throws Exception {
|
||||||
EmbeddedChannel ch = createChannel();
|
EmbeddedChannel ch = createChannel();
|
||||||
|
|
Loading…
Reference in New Issue
Block a user