Skip to content

Commit

Permalink
server h2c upgrade fail when request doesn't have connection header (n…
Browse files Browse the repository at this point in the history
…etty#11569)

__Motivation__
Since request.headers().getAll() will never return null. And the check null condition will not work as expected.

__Modification__

Add isEmpty() checking as well.

__Result__

Fixes netty#11568
  • Loading branch information
dpy1123 authored and laosijikaichele committed Dec 16, 2021
1 parent fff2884 commit 4d56399
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 1 deletion.
Expand Up @@ -323,7 +323,7 @@ private boolean upgrade(final ChannelHandlerContext ctx, final FullHttpRequest r
// Make sure the CONNECTION header is present.
List<String> connectionHeaderValues = request.headers().getAll(HttpHeaderNames.CONNECTION);

if (connectionHeaderValues == null) {
if (connectionHeaderValues == null || connectionHeaderValues.isEmpty()) {
return false;
}

Expand Down
Expand Up @@ -31,6 +31,7 @@
import io.netty.handler.codec.http.HttpServerUpgradeHandler.UpgradeCodec;
import io.netty.handler.codec.http.HttpServerUpgradeHandler.UpgradeCodecFactory;
import io.netty.util.CharsetUtil;
import io.netty.util.ReferenceCountUtil;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
Expand Down Expand Up @@ -189,4 +190,42 @@ protected boolean shouldHandleUpgradeRequest(HttpRequest req) {
assertNull(channel.readOutbound());
assertFalse(channel.finishAndReleaseAll());
}

@Test
public void upgradeFail() {
final HttpServerCodec httpServerCodec = new HttpServerCodec();
final UpgradeCodecFactory factory = new UpgradeCodecFactory() {
@Override
public UpgradeCodec newUpgradeCodec(CharSequence protocol) {
return new TestUpgradeCodec();
}
};

HttpServerUpgradeHandler upgradeHandler = new HttpServerUpgradeHandler(httpServerCodec, factory);

EmbeddedChannel channel = new EmbeddedChannel(httpServerCodec, upgradeHandler);

// Build a h2c upgrade request, but without connection header.
String upgradeString = "GET / HTTP/1.1\r\n" +
"Host: example.com\r\n" +
"Upgrade: h2c\r\n\r\n";
ByteBuf upgrade = Unpooled.copiedBuffer(upgradeString, CharsetUtil.US_ASCII);

assertTrue(channel.writeInbound(upgrade));
assertNotNull(channel.pipeline().get(HttpServerCodec.class));
assertNotNull(channel.pipeline().get(HttpServerUpgradeHandler.class)); // Should not be removed.
assertNull(channel.pipeline().get("marker"));

HttpRequest req = channel.readInbound();
assertEquals(HttpVersion.HTTP_1_1, req.protocolVersion());
assertTrue(req.headers().contains(HttpHeaderNames.UPGRADE, "h2c", false));
assertFalse(req.headers().contains(HttpHeaderNames.CONNECTION));
ReferenceCountUtil.release(req);
assertNull(channel.readInbound());

// No response should be written because we're just passing through.
channel.flushOutbound();
assertNull(channel.readOutbound());
assertFalse(channel.finishAndReleaseAll());
}
}

0 comments on commit 4d56399

Please sign in to comment.