Issue #8025. Ignoring HEADER and DATA frames for streams that may have existed in the past.

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

@@ -585,11 +585,11 @@ public class DefaultHttp2ConnectionDecoder implements Http2ConnectionDecoder {
                             ctx.channel(), frameName, streamId);
                     return true;
                 }
-                // Its possible that this frame would result in stream ID out of order creation (PROTOCOL ERROR) and its
+                // If the stream could have existed in the past we assume this is a frame sent by the remote
-                // also possible that this frame is received on a CLOSED stream (STREAM_CLOSED after a RST_STREAM is
+                // after a RST_STREAM has been sent. Since we don't retain metadata about streams that have been
-                // sent). We don't have enough information to know for sure, so we choose the lesser of the two errors.
+                // reset we can't know this for sure.
-                throw streamError(streamId, STREAM_CLOSED, "Received %s frame for an unknown stream %d",
+                verifyStreamMayHaveExisted(streamId);
-                                  frameName, streamId);
+                return true;
             } else if (stream.isResetSent() || streamCreatedAfterGoAwaySent(streamId)) {
                 // If we have sent a reset stream it is assumed the stream will be closed after the write completes.
                 // If we have not sent a reset, but the stream was created after a GoAway this is not supported by

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

@@ -272,17 +272,26 @@ public class DefaultHttp2ConnectionDecoderTest {
         }
     }
 
+    @Test(expected = Http2Exception.class)
+    public void dataReadForUnknownStreamThatNeverExistedShouldThrow() throws Exception {
+        when(connection.streamMayHaveExisted(STREAM_ID)).thenReturn(false);
+        when(connection.stream(STREAM_ID)).thenReturn(null);
+        final ByteBuf data = dummyData();
+        try {
+            decode().onDataRead(ctx, STREAM_ID, data, 0, true);
+        } finally {
+            data.release();
+        }
+    }
+
     @Test
     public void dataReadForUnknownStreamShouldApplyFlowControl() throws Exception {
         when(connection.stream(STREAM_ID)).thenReturn(null);
         final ByteBuf data = dummyData();
         int padding = 10;
         int processedBytes = data.readableBytes() + padding;
-        try {
         try {
             decode().onDataRead(ctx, STREAM_ID, data, padding, true);
-                fail();
-            } catch (Http2Exception e) {
             verify(localFlow)
                 .receiveFlowControlledFrame(eq((Http2Stream) null), eq(data), eq(padding), eq(true));
             verify(localFlow).consumeBytes(eq((Http2Stream) null), eq(processedBytes));
@@ -291,7 +300,6 @@ public class DefaultHttp2ConnectionDecoderTest {
 
             // Verify that the event was absorbed and not propagated to the observer.
             verify(listener, never()).onDataRead(eq(ctx), anyInt(), any(ByteBuf.class), anyInt(), anyBoolean());
-            }
         } finally {
             data.release();
         }
@@ -430,10 +438,15 @@ public class DefaultHttp2ConnectionDecoderTest {
         }
     }
 
-    @Test(expected = Http2Exception.class)
+    @Test
-    public void headersReadForUnknownStreamShouldThrow() throws Exception {
+    public void headersReadForUnknownStreamThatMayHaveExistedShouldBeIgnored() throws Exception {
         when(connection.stream(STREAM_ID)).thenReturn(null);
         decode().onHeadersRead(ctx, STREAM_ID, EmptyHttp2Headers.INSTANCE, 0, false);
+
+        // Verify that the event was absorbed and not propagated to the observer.
+        verify(listener, never()).onHeadersRead(eq(ctx), anyInt(), any(Http2Headers.class), anyInt(), anyBoolean());
+        verify(remote, never()).createStream(anyInt(), anyBoolean());
+        verify(stream, never()).open(anyBoolean());
     }
 
     @Test
@@ -587,8 +600,21 @@ public class DefaultHttp2ConnectionDecoderTest {
         verify(listener).onPushPromiseRead(eq(ctx), anyInt(), anyInt(), any(Http2Headers.class), anyInt());
     }
 
+    @Test
+    public void pushPromiseReadForUnknownStreamThatMayHaveExistedShouldBeIgnored() throws Exception {
+        when(connection.stream(STREAM_ID)).thenReturn(null);
+        decode().onPushPromiseRead(ctx, STREAM_ID, PUSH_STREAM_ID, EmptyHttp2Headers.INSTANCE, 0);
+
+        // Verify that the event was absorbed and not propagated to the observer.
+        verify(listener, never()).onPushPromiseRead(eq(ctx), anyInt(), anyInt(),
+                                                    any(Http2Headers.class), anyInt());
+        verify(remote, never()).createStream(anyInt(), anyBoolean());
+        verify(stream, never()).open(anyBoolean());
+    }
+
     @Test(expected = Http2Exception.class)
-    public void pushPromiseReadForUnknownStreamShouldThrow() throws Exception {
+    public void pushPromiseReadForUnknownStreamThatNeverExistedShouldThrow() throws Exception {
+        when(connection.streamMayHaveExisted(STREAM_ID)).thenReturn(false);
         when(connection.stream(STREAM_ID)).thenReturn(null);
         decode().onPushPromiseRead(ctx, STREAM_ID, PUSH_STREAM_ID, EmptyHttp2Headers.INSTANCE, 0);
     }