Remove usage of forbiddenHttpRequestResponder (#9941)

Motivation:

At the moment we add a handler which will respond with 403 forbidden if a websocket handshake is in progress (and after). This makes not much sense as it is unexpected to have a remote peer to send another http request when the handshake was started. In this case it is much better to let the websocket decoder bail out.

Modifications:

Remove usage of forbiddenHttpRequestResponder

Result:

Fixes https://github.com/netty/netty/issues/9913
This commit is contained in:
Norman Maurer 2020-01-28 06:04:20 +01:00 committed by GitHub
parent a61e844c9a
commit 2023a4f607
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 4 additions and 41 deletions

View File

@ -18,13 +18,10 @@ package io.netty.handler.codec.http.websocketx;
import io.netty.buffer.Unpooled; import io.netty.buffer.Unpooled;
import io.netty.channel.Channel; import io.netty.channel.Channel;
import io.netty.channel.ChannelFutureListener; import io.netty.channel.ChannelFutureListener;
import io.netty.channel.ChannelHandler;
import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelInboundHandler; import io.netty.channel.ChannelInboundHandler;
import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.channel.ChannelPipeline; import io.netty.channel.ChannelPipeline;
import io.netty.handler.codec.http.DefaultFullHttpResponse; import io.netty.handler.codec.http.DefaultFullHttpResponse;
import io.netty.handler.codec.http.FullHttpRequest;
import io.netty.handler.codec.http.FullHttpResponse; import io.netty.handler.codec.http.FullHttpResponse;
import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpHeaders;
import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.codec.http.HttpResponseStatus;
@ -269,20 +266,4 @@ public class WebSocketServerProtocolHandler extends WebSocketProtocolHandler {
static void setHandshaker(Channel channel, WebSocketServerHandshaker handshaker) { static void setHandshaker(Channel channel, WebSocketServerHandshaker handshaker) {
channel.attr(HANDSHAKER_ATTR_KEY).set(handshaker); channel.attr(HANDSHAKER_ATTR_KEY).set(handshaker);
} }
static ChannelHandler forbiddenHttpRequestResponder() {
return new ChannelInboundHandlerAdapter() {
@Override
public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
if (msg instanceof FullHttpRequest) {
((FullHttpRequest) msg).release();
FullHttpResponse response =
new DefaultFullHttpResponse(HTTP_1_1, HttpResponseStatus.FORBIDDEN, ctx.alloc().buffer(0));
ctx.channel().writeAndFlush(response);
} else {
ctx.fireChannelRead(msg);
}
}
};
}
} }

View File

@ -53,7 +53,7 @@ class WebSocketServerProtocolHandshakeHandler extends ChannelInboundHandlerAdapt
} }
@Override @Override
public void handlerAdded(ChannelHandlerContext ctx) throws Exception { public void handlerAdded(ChannelHandlerContext ctx) {
this.ctx = ctx; this.ctx = ctx;
handshakePromise = ctx.newPromise(); handshakePromise = ctx.newPromise();
} }
@ -86,13 +86,12 @@ class WebSocketServerProtocolHandshakeHandler extends ChannelInboundHandlerAdapt
// //
// See https://github.com/netty/netty/issues/9471. // See https://github.com/netty/netty/issues/9471.
WebSocketServerProtocolHandler.setHandshaker(ctx.channel(), handshaker); WebSocketServerProtocolHandler.setHandshaker(ctx.channel(), handshaker);
ctx.pipeline().replace(this, "WS403Responder", ctx.pipeline().remove(this);
WebSocketServerProtocolHandler.forbiddenHttpRequestResponder());
final ChannelFuture handshakeFuture = handshaker.handshake(ctx.channel(), req); final ChannelFuture handshakeFuture = handshaker.handshake(ctx.channel(), req);
handshakeFuture.addListener(new ChannelFutureListener() { handshakeFuture.addListener(new ChannelFutureListener() {
@Override @Override
public void operationComplete(ChannelFuture future) throws Exception { public void operationComplete(ChannelFuture future) {
if (!future.isSuccess()) { if (!future.isSuccess()) {
localHandshakePromise.tryFailure(future.cause()); localHandshakePromise.tryFailure(future.cause());
ctx.fireExceptionCaught(future.cause()); ctx.fireExceptionCaught(future.cause());
@ -158,7 +157,7 @@ class WebSocketServerProtocolHandshakeHandler extends ChannelInboundHandlerAdapt
// Cancel the handshake timeout when handshake is finished. // Cancel the handshake timeout when handshake is finished.
localHandshakePromise.addListener(new FutureListener<Void>() { localHandshakePromise.addListener(new FutureListener<Void>() {
@Override @Override
public void operationComplete(Future<Void> f) throws Exception { public void operationComplete(Future<Void> f) {
timeoutFuture.cancel(false); timeoutFuture.cancel(false);
} }
}); });

View File

@ -89,23 +89,6 @@ public class WebSocketServerProtocolHandlerTest {
assertFalse(ch.finish()); assertFalse(ch.finish());
} }
@Test
public void testSubsequentHttpRequestsAfterUpgradeShouldReturn403() {
EmbeddedChannel ch = createChannel();
writeUpgradeRequest(ch);
FullHttpResponse response = responses.remove();
assertEquals(SWITCHING_PROTOCOLS, response.status());
response.release();
ch.writeInbound(new DefaultFullHttpRequest(HTTP_1_1, HttpMethod.GET, "/test"));
response = responses.remove();
assertEquals(FORBIDDEN, response.status());
response.release();
assertFalse(ch.finish());
}
@Test @Test
public void testHttpUpgradeRequestInvalidUpgradeHeader() { public void testHttpUpgradeRequestInvalidUpgradeHeader() {
EmbeddedChannel ch = createChannel(); EmbeddedChannel ch = createChannel();