H2C upgrades should be ineligible for flow control (#7400)

H2C upgrades should be ineligible for flow control

Motivation:

When the h2c upgrade request is too big, the Http2FrameCodec complains
it's too big for flow control reasons, even though it's ineligible for
flow control.

Modifications:

Specially mark upgrade streams and make Http2FrameCodec know not to try
to flow control on those streams.

Result:

Servers won't barf when they receive an upgrade request with a fat
payload.

[Fixes #7280]

codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java

@@ -145,6 +145,7 @@ public class Http2FrameCodec extends Http2ConnectionHandler {
     private static final InternalLogger LOG = InternalLoggerFactory.getInstance(Http2FrameCodec.class);
 
     private final PropertyKey streamKey;
+    private final PropertyKey upgradeKey;
 
     private final Integer initialFlowControlWindowSize;
 
@@ -161,6 +162,7 @@ public class Http2FrameCodec extends Http2ConnectionHandler {
         connection().addListener(new ConnectionListener());
         connection().remote().flowController().listener(new Http2RemoteFlowControllerListener());
         streamKey = connection().newKey();
+        upgradeKey = connection().newKey();
         initialFlowControlWindowSize = initialSettings.initialWindowSize();
     }
 
@@ -247,6 +249,7 @@ public class Http2FrameCodec extends Http2ConnectionHandler {
                 }
                 upgrade.upgradeRequest().headers().setInt(
                         HttpConversionUtil.ExtensionHeaderNames.STREAM_ID.text(), HTTP_UPGRADE_STREAM_ID);
+                stream.setProperty(upgradeKey, true);
                 InboundHttpToHttp2Adapter.handle(
                         ctx, connection(), decoder().frameListener(), upgrade.upgradeRequest().retain());
             } finally {
@@ -312,6 +315,14 @@ public class Http2FrameCodec extends Http2ConnectionHandler {
 
     final boolean consumeBytes(int streamId, int bytes) throws Http2Exception {
         Http2Stream stream = connection().stream(streamId);
+        // upgraded requests are ineligible for stream control
+        if (streamId == Http2CodecUtil.HTTP_UPGRADE_STREAM_ID) {
+            Boolean upgraded = stream.getProperty(upgradeKey);
+            if (Boolean.TRUE.equals(upgraded)) {
+                return false;
+            }
+        }
+
         return connection().local().flowController().consumeBytes(stream, bytes);
     }
 

codec-http2/src/main/java/io/netty/handler/codec/http2/InboundHttpToHttp2Adapter.java

@@ -49,6 +49,9 @@ public class InboundHttpToHttp2Adapter extends ChannelInboundHandlerAdapter {
         }
     }
 
+    // note that this may behave strangely when used for the initial upgrade
+    // message when using h2c, since that message is ineligible for flow
+    // control, but there is not yet an API for signaling that.
     static void handle(ChannelHandlerContext ctx, Http2Connection connection,
                               Http2FrameListener listener, FullHttpMessage message) throws Http2Exception {
         try {

codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java

@@ -21,6 +21,7 @@ import io.netty.buffer.UnpooledByteBufAllocator;
 import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelFutureListener;
 import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.ChannelInboundHandlerAdapter;
 import io.netty.channel.ChannelPromise;
 import io.netty.channel.embedded.EmbeddedChannel;
 import io.netty.handler.codec.UnsupportedMessageTypeException;
@@ -37,6 +38,7 @@ import io.netty.handler.codec.http2.Http2Stream.State;
 import io.netty.handler.logging.LogLevel;
 import io.netty.util.AbstractReferenceCounted;
 import io.netty.util.AsciiString;
+import io.netty.util.ReferenceCountUtil;
 import io.netty.util.ReferenceCounted;
 import io.netty.util.concurrent.DefaultPromise;
 import io.netty.util.concurrent.GlobalEventExecutor;
@@ -666,6 +668,47 @@ public class Http2FrameCodecTest {
         assertEquals(1, upgradeEvent.refCnt());
     }
 
+    @Test
+    public void upgradeWithoutFlowControlling() throws Exception {
+        channel.pipeline().addAfter(http2HandlerCtx.name(), null, new ChannelInboundHandlerAdapter() {
+            @Override
+            public void channelRead(final ChannelHandlerContext ctx, Object msg) throws Exception {
+                if (msg instanceof Http2DataFrame) {
+                    // Simulate consuming the frame and update the flow-controller.
+                    Http2DataFrame data = (Http2DataFrame) msg;
+                    ctx.writeAndFlush(new DefaultHttp2WindowUpdateFrame(data.initialFlowControlledBytes())
+                            .stream(data.stream())).addListener(new ChannelFutureListener() {
+                        @Override
+                        public void operationComplete(ChannelFuture future) throws Exception {
+                            Throwable cause = future.cause();
+                            if (cause != null) {
+                                ctx.fireExceptionCaught(cause);
+                            }
+                        }
+                    });
+                }
+                ReferenceCountUtil.release(msg);
+            }
+        });
+
+        frameListener.onHeadersRead(http2HandlerCtx, Http2CodecUtil.HTTP_UPGRADE_STREAM_ID, request, 31, false);
+
+        // Using reflect as the constructor is package-private and the class is final.
+        Constructor<UpgradeEvent> constructor =
+                UpgradeEvent.class.getDeclaredConstructor(CharSequence.class, FullHttpRequest.class);
+
+        // Check if we could make it accessible which may fail on java9.
+        Assume.assumeTrue(ReflectionUtil.trySetAccessible(constructor) == null);
+
+        String longString = new String(new char[70000]).replace("\0", "*");
+        DefaultFullHttpRequest request =
+            new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, "/", bb(longString));
+
+        HttpServerUpgradeHandler.UpgradeEvent upgradeEvent = constructor.newInstance(
+            "HTTP/2", request);
+        channel.pipeline().fireUserEventTriggered(upgradeEvent);
+    }
+
     private static ChannelPromise anyChannelPromise() {
         return any(ChannelPromise.class);
     }