From 83ab4ef5e372e0a9b87fdadd4077a551af5b2e64 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Thu, 13 Dec 2018 19:02:20 +0100
Subject: [PATCH] Explict always call ctx.read() when AUTO_READ is false and
 HTTP/2 is used. (#8647)

Motivation:

We should always call ctx.read() even when AUTO_READ is false as flow-control is enforced by the HTTP/2 protocol.

See also https://tools.ietf.org/html/rfc7540#section-5.2.2.

We already did this before but not explicit and only did so because of some implementation details of ByteToMessageDecoder. It's better to be explicit here to not risk of breakage later on.

Modifications:

- Ensure we always call ctx.read() when AUTO_READ is false
- Add unit test.

Result:

No risk of staling the connection when HTTP/2 is used.
---
 .../codec/http2/Http2ConnectionHandler.java       | 14 ++++++++++++--
 .../codec/http2/Http2ConnectionHandlerTest.java   | 15 +++++++++++++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
index 3151f1ccb2..4f66e918d5 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java
@@ -527,8 +527,18 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
         }
     }
 
-    void channelReadComplete0(ChannelHandlerContext ctx) throws Exception {
-        super.channelReadComplete(ctx);
+    final void channelReadComplete0(ChannelHandlerContext ctx) {
+        // Discard bytes of the cumulation buffer if needed.
+        discardSomeReadBytes();
+
+        // Ensure we never stale the HTTP/2 Channel. Flow-control is enforced by HTTP/2.
+        //
+        // See https://tools.ietf.org/html/rfc7540#section-5.2.2
+        if (!ctx.channel().config().isAutoRead()) {
+            ctx.read();
+        }
+
+        ctx.fireChannelReadComplete();
     }
 
     /**
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java
index 223d6005f4..9b8c624027 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java
@@ -22,8 +22,10 @@ import io.netty.channel.Channel;
 import io.netty.channel.ChannelFuture;
 import io.netty.channel.ChannelFutureListener;
 import io.netty.channel.ChannelHandlerContext;
+import io.netty.channel.ChannelMetadata;
 import io.netty.channel.ChannelPipeline;
 import io.netty.channel.ChannelPromise;
+import io.netty.channel.DefaultChannelConfig;
 import io.netty.channel.DefaultChannelPromise;
 import io.netty.handler.codec.http.HttpResponseStatus;
 import io.netty.handler.codec.http2.Http2CodecUtil.SimpleChannelPromiseAggregator;
@@ -142,6 +144,11 @@ public class Http2ConnectionHandlerTest {
 
         promise = new DefaultChannelPromise(channel, ImmediateEventExecutor.INSTANCE);
         voidPromise = new DefaultChannelPromise(channel, ImmediateEventExecutor.INSTANCE);
+
+        when(channel.metadata()).thenReturn(new ChannelMetadata(false));
+        DefaultChannelConfig config = new DefaultChannelConfig(channel);
+        when(channel.config()).thenReturn(config);
+
         Throwable fakeException = new RuntimeException("Fake exception");
         when(encoder.connection()).thenReturn(connection);
         when(decoder.connection()).thenReturn(connection);
@@ -684,6 +691,14 @@ public class Http2ConnectionHandlerTest {
         verify(ctx, times(1)).flush();
     }
 
+    @Test
+    public void channelReadCompleteCallsReadWhenAutoReadFalse() throws Exception {
+        channel.config().setAutoRead(false);
+        handler = newHandler();
+        handler.channelReadComplete(ctx);
+        verify(ctx, times(1)).read();
+    }
+
     @Test
     public void channelClosedDoesNotThrowPrefaceException() throws Exception {
         when(connection.isServer()).thenReturn(true);