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

Remove usage of forbiddenHttpRequestResponder (#9941)

Browse Source 
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
parent a0ce45ac2f
commit b19958eda9
3 changed files with 3 additions and 38 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
codec-http/src/main/java/io/netty/handler/codec/http/websocketx/WebSocketServerProtocolHandler.java
View File

@ -22,7 +22,6 @@ import io.netty.channel.ChannelHandler;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelPipeline;
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.HttpHeaders;
import io.netty.handler.codec.http.HttpResponseStatus;
@ -266,20 +265,4 @@ public class WebSocketServerProtocolHandler extends WebSocketProtocolHandler {
static void setHandshaker(Channel channel, WebSocketServerHandshaker handshaker) {
channel.attr(HANDSHAKER_ATTR_KEY).set(handshaker);
}
static ChannelHandler forbiddenHttpRequestResponder() {
return new ChannelHandler() {
@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);
}
}
};
}
}

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

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

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

@ -87,23 +87,6 @@ public class WebSocketServerProtocolHandlerTest {
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
public void testHttpUpgradeRequestInvalidUpgradeHeader() {
EmbeddedChannel ch = createChannel();