From 85b0eb2d3e43ef57702b4d9e176b325b7eafbb80 Mon Sep 17 00:00:00 2001 From: DD Date: Fri, 13 Aug 2021 00:04:18 +0800 Subject: [PATCH] server h2c upgrade fail when request doesn't have connection header (#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 #11568 --- .../codec/http/HttpServerUpgradeHandler.java | 2 +- .../http/HttpServerUpgradeHandlerTest.java | 39 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java index 800f8f435ed..8b733a1421f 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/HttpServerUpgradeHandler.java @@ -320,7 +320,7 @@ private boolean upgrade(final ChannelHandlerContext ctx, final FullHttpRequest r // Make sure the CONNECTION header is present. List connectionHeaderValues = request.headers().getAll(HttpHeaderNames.CONNECTION); - if (connectionHeaderValues == null) { + if (connectionHeaderValues == null || connectionHeaderValues.isEmpty()) { return false; } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/HttpServerUpgradeHandlerTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/HttpServerUpgradeHandlerTest.java index 6c17ee34af3..ff9d237b6d5 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/HttpServerUpgradeHandlerTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/HttpServerUpgradeHandlerTest.java @@ -28,6 +28,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; @@ -171,4 +172,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()); + } }