HttpConnectionHandler should send a GO_AWAY frame with PROTOCOL_ERROR…

… when it receives an unknown stream instead of resetting the stream.

Motivation:

When an HTTP/2 connection process an unnkown stream it should throw a GO_AWAY frame and close the connection as per RFC 7540. The current implementation instead sends a RST frame.

Modifications:

Change the HttpConnectionHandler behavior to throw an error instead of writing a rst frame, the error will trigger a GO_AWAY frame to be sent and then close the connection appropriately.

Result:

HttpConnectionHandler should send a GO_AWAY frame with PROTOCOL_ERROR when it receives an unknown stream and fix #12974

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

@@ -738,7 +738,7 @@ protected void onStreamError(ChannelHandlerContext ctx, boolean outbound,
 
         if (stream == null) {
             if (!outbound || connection().local().mayHaveCreatedStream(streamId)) {
-                resetUnknownStream(ctx, streamId, http2Ex.error().code(), ctx.newPromise());
+                onError(ctx, outbound, connectionError(PROTOCOL_ERROR, "Unknown stream"));
             }
         } else {
             resetStream(ctx, stream, http2Ex.error().code(), ctx.newPromise());

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

@@ -146,7 +146,7 @@ public void teardown() throws Exception {
     }
 
     @Test
-    public void inflightFrameAfterStreamResetShouldNotMakeConnectionUnusable() throws Exception {
+    public void unknownStreamShouldGoAway() throws Exception {
         final CountDownLatch latch = new CountDownLatch(1);
         doAnswer(new Answer<Void>() {
             @Override
@@ -170,8 +170,8 @@ public Void answer(InvocationOnMock invocationOnMock) throws Throwable {
                 latch.countDown();
                 return null;
             }
-        }).when(clientListener).onHeadersRead(any(ChannelHandlerContext.class), eq(5), any(Http2Headers.class),
-                anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean());
+        }).when(serverListener).onGoAwayRead(any(ChannelHandlerContext.class), any(Integer.class), any(Long.class),
+            any(ByteBuf.class));
 
         bootstrapEnv(1, 1, 2, 1);
 
@@ -188,14 +188,6 @@ public void run() throws Http2Exception {
             }
         });
 
-        runInChannel(clientChannel, new Http2Runnable() {
-            @Override
-            public void run() throws Http2Exception {
-                http2Client.encoder().writeHeaders(ctx(), 5, headers, 0, weight, false, 0, false, newPromise());
-                http2Client.flush(ctx());
-            }
-        });
-
         assertTrue(latch.await(DEFAULT_AWAIT_TIMEOUT_SECONDS, SECONDS));
     }
 
@@ -487,6 +479,16 @@ public void run() throws Http2Exception {
     public void headersUsingHigherValuedStreamIdPreventsUsingLowerStreamId() throws Exception {
         bootstrapEnv(1, 1, 2, 0);
 
+        final CountDownLatch clientGoAwayLatch = new CountDownLatch(1);
+
+        doAnswer(new Answer<Void>() {
+            @Override
+            public Void answer(InvocationOnMock invocationOnMock) throws Throwable {
+                clientGoAwayLatch.countDown();
+                return null;
+            }
+        }).when(clientListener).onGoAwayRead(any(ChannelHandlerContext.class), anyInt(), anyLong(), any(ByteBuf.class));
+
         final Http2Headers headers = dummyHeaders();
         runInChannel(clientChannel, new Http2Runnable() {
             @Override
@@ -507,13 +509,12 @@ public void run() throws Http2Exception {
         verify(serverListener, never()).onHeadersRead(any(ChannelHandlerContext.class), eq(3), any(Http2Headers.class),
                 anyInt(), anyShort(), anyBoolean(), anyInt(), anyBoolean());
 
-        // Client should receive a RST_STREAM for stream 3, but there is not Http2Stream object so the listener is never
-        // notified.
+        // Client should receive a GO_AWAY frame
+        assertTrue(clientGoAwayLatch.await(DEFAULT_AWAIT_TIMEOUT_SECONDS, SECONDS));
+
         verify(serverListener, never()).onGoAwayRead(any(ChannelHandlerContext.class), anyInt(), anyLong(),
-                any(ByteBuf.class));
+            any(ByteBuf.class));
         verify(serverListener, never()).onRstStreamRead(any(ChannelHandlerContext.class), anyInt(), anyLong());
-        verify(clientListener, never()).onGoAwayRead(any(ChannelHandlerContext.class), anyInt(), anyLong(),
-                any(ByteBuf.class));
         verify(clientListener, never()).onRstStreamRead(any(ChannelHandlerContext.class), anyInt(), anyLong());
     }