Disallow any frames after a reset is received #2154

src/Kestrel.Core/CoreStrings.resx

@@ -587,4 +587,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l
   <data name="ConnectionAbortedByClient" xml:space="preserve">
     <value>The client closed the connection.</value>
   </data>
+  <data name="Http2ErrorStreamAborted" xml:space="preserve">
+    <value>A frame of type {frameType} was received after stream {streamId} was reset or aborted.</value>
+  </data>
 </root>

src/Kestrel.Core/Internal/Http2/Http2Connection.cs

@@ -224,6 +224,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
                     catch (Http2StreamErrorException ex)
                     {
                         Log.Http2StreamError(ConnectionId, ex);
+                        // The client doesn't know this error is coming, allow draining additional frames for now.
                         AbortStream(_incomingFrame.StreamId, new IOException(ex.Message, ex));
                         await _frameWriter.WriteRstStreamAsync(ex.StreamId, ex.ErrorCode);
                     }
@@ -448,6 +449,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
 
             if (_streams.TryGetValue(_incomingFrame.StreamId, out var stream))
             {
+                if (stream.DoNotDrainRequest)
+                {
+                    // Hard abort, do not allow any more frames on this stream.
+                    throw new Http2ConnectionErrorException(CoreStrings.FormatHttp2ErrorStreamAborted(_incomingFrame.Type, stream.StreamId), Http2ErrorCode.STREAM_CLOSED);
+                }
                 if (stream.EndStreamReceived)
                 {
                     // http://httpwg.org/specs/rfc7540.html#rfc.section.5.1
@@ -501,6 +507,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
 
             if (_streams.TryGetValue(_incomingFrame.StreamId, out var stream))
             {
+                if (stream.DoNotDrainRequest)
+                {
+                    // Hard abort, do not allow any more frames on this stream.
+                    throw new Http2ConnectionErrorException(CoreStrings.FormatHttp2ErrorStreamAborted(_incomingFrame.Type, stream.StreamId), Http2ErrorCode.STREAM_CLOSED);
+                }
+
                 // http://httpwg.org/specs/rfc7540.html#rfc.section.5.1
                 //
                 // ...an endpoint that receives any frames after receiving a frame with the
@@ -609,7 +621,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
             }
 
             ThrowIfIncomingFrameSentToIdleStream();
-            AbortStream(_incomingFrame.StreamId, new IOException(CoreStrings.Http2StreamResetByClient));
+
+            if (_streams.TryGetValue(_incomingFrame.StreamId, out var stream))
+            {
+                // Second reset
+                if (stream.DoNotDrainRequest)
+                {
+                    // Hard abort, do not allow any more frames on this stream.
+                    throw new Http2ConnectionErrorException(CoreStrings.FormatHttp2ErrorStreamAborted(_incomingFrame.Type, stream.StreamId), Http2ErrorCode.STREAM_CLOSED);
+                }
+
+                // No additional inbound header or data frames are allowed for this stream after receiving a reset.
+                stream.DisallowAdditionalRequestFrames();
+                stream.Abort(new IOException(CoreStrings.Http2StreamResetByClient));
+            }
 
             return Task.CompletedTask;
         }
@@ -771,6 +796,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
             }
             else if (_streams.TryGetValue(_incomingFrame.StreamId, out var stream))
             {
+                if (stream.DoNotDrainRequest)
+                {
+                    // Hard abort, do not allow any more frames on this stream.
+                    throw new Http2ConnectionErrorException(CoreStrings.FormatHttp2ErrorStreamAborted(_incomingFrame.Type, stream.StreamId), Http2ErrorCode.STREAM_CLOSED);
+                }
+
                 if (!stream.TryUpdateOutputWindow(_incomingFrame.WindowUpdateSizeIncrement))
                 {
                     throw new Http2StreamErrorException(_incomingFrame.StreamId, CoreStrings.Http2ErrorWindowUpdateSizeInvalid, Http2ErrorCode.FLOW_CONTROL_ERROR);

src/Kestrel.Core/Internal/Http2/Http2Stream.cs

@@ -53,6 +53,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
         public bool RequestBodyStarted { get; private set; }
         public bool EndStreamReceived => (_completionState & StreamCompletionFlags.EndStreamReceived) == StreamCompletionFlags.EndStreamReceived;
         private bool IsAborted => (_completionState & StreamCompletionFlags.Aborted) == StreamCompletionFlags.Aborted;
+        internal bool DoNotDrainRequest => (_completionState & StreamCompletionFlags.DoNotDrainRequest) == StreamCompletionFlags.DoNotDrainRequest;
 
         public override bool IsUpgradableRequest => false;
 
@@ -381,6 +382,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
             return _context.FrameWriter.TryUpdateStreamWindow(_outputFlowControl, bytes);
         }
 
+        public void DisallowAdditionalRequestFrames()
+        {
+            ApplyCompletionFlag(StreamCompletionFlags.DoNotDrainRequest);
+        }
+
         public void Abort(IOException abortReason)
         {
             var states = ApplyCompletionFlag(StreamCompletionFlags.Aborted);
@@ -415,6 +421,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
 
         private void ResetAndAbort(ConnectionAbortedException abortReason, Http2ErrorCode error)
         {
+            // Future incoming frames will drain for a default grace period to avoid destabilizing the connection.
             var states = ApplyCompletionFlag(StreamCompletionFlags.Aborted);
 
             if (states.OldState == states.NewState)
@@ -507,6 +514,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
             RequestProcessingEnded = 1,
             EndStreamReceived = 2,
             Aborted = 4,
+            DoNotDrainRequest = 8,
         }
     }
 }

src/Kestrel.Core/Properties/CoreStrings.Designer.cs

@@ -2198,6 +2198,20 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
         internal static string FormatConnectionAbortedByClient()
             => GetString("ConnectionAbortedByClient");
 
+        /// <summary>
+        /// A frame of type {frameType} was received after stream {streamId} was reset or aborted.
+        /// </summary>
+        internal static string Http2ErrorStreamAborted
+        {
+            get => GetString("Http2ErrorStreamAborted");
+        }
+
+        /// <summary>
+        /// A frame of type {frameType} was received after stream {streamId} was reset or aborted.
+        /// </summary>
+        internal static string FormatHttp2ErrorStreamAborted(object frameType, object streamId)
+            => string.Format(CultureInfo.CurrentCulture, GetString("Http2ErrorStreamAborted", "frameType", "streamId"), frameType, streamId);
+
         private static string GetString(string name, params string[] formatterNames)
         {
             var value = _resourceManager.GetString(name);

test/Kestrel.InMemory.FunctionalTests/Http2/Http2ConnectionTests.cs

@@ -2195,6 +2195,101 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
                 expectedErrorMessage: CoreStrings.FormatHttp2ErrorHeadersInterleaved(Http2FrameType.RST_STREAM, streamId: 1, headersStreamId: 1));
         }
 
+        // Compare to h2spec http2/5.1/8
+        [Fact]
+        public async Task RST_STREAM_IncompleteRequest_AdditionalDataFrames_ConnectionAborted()
+        {
+            var tcs = new TaskCompletionSource<int>(TaskCreationOptions.RunContinuationsAsynchronously);
+
+            var headers = new[]
+            {
+                new KeyValuePair<string, string>(HeaderNames.Method, "POST"),
+                new KeyValuePair<string, string>(HeaderNames.Path, "/"),
+                new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
+            };
+            await InitializeConnectionAsync(context => tcs.Task);
+
+            await StartStreamAsync(1, headers, endStream: false);
+            await SendDataAsync(1, new byte[1], endStream: false);
+            await SendDataAsync(1, new byte[2], endStream: false);
+            await SendRstStreamAsync(1);
+            await SendDataAsync(1, new byte[10], endStream: false);
+            tcs.TrySetResult(0);
+
+            await WaitForConnectionErrorAsync<Http2ConnectionErrorException>(ignoreNonGoAwayFrames: false, expectedLastStreamId: 1,
+                Http2ErrorCode.STREAM_CLOSED, CoreStrings.FormatHttp2ErrorStreamAborted(Http2FrameType.DATA, 1));
+        }
+
+        [Fact]
+        public async Task RST_STREAM_IncompleteRequest_AdditionalTrailerFrames_ConnectionAborted()
+        {
+            var tcs = new TaskCompletionSource<int>(TaskCreationOptions.RunContinuationsAsynchronously);
+
+            var headers = new[]
+            {
+                new KeyValuePair<string, string>(HeaderNames.Method, "POST"),
+                new KeyValuePair<string, string>(HeaderNames.Path, "/"),
+                new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
+            };
+            await InitializeConnectionAsync(context => tcs.Task);
+
+            await StartStreamAsync(1, headers, endStream: false);
+            await SendDataAsync(1, new byte[1], endStream: false);
+            await SendDataAsync(1, new byte[2], endStream: false);
+            await SendRstStreamAsync(1);
+            await SendHeadersAsync(1, Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM, _requestTrailers);
+            tcs.TrySetResult(0);
+
+            await WaitForConnectionErrorAsync<Http2ConnectionErrorException>(ignoreNonGoAwayFrames: false, expectedLastStreamId: 1,
+                Http2ErrorCode.STREAM_CLOSED, CoreStrings.FormatHttp2ErrorStreamAborted(Http2FrameType.HEADERS, 1));
+        }
+
+        [Fact]
+        public async Task RST_STREAM_IncompleteRequest_AdditionalResetFrame_ConnectionAborted()
+        {
+            var tcs = new TaskCompletionSource<int>(TaskCreationOptions.RunContinuationsAsynchronously);
+
+            var headers = new[]
+            {
+                new KeyValuePair<string, string>(HeaderNames.Method, "POST"),
+                new KeyValuePair<string, string>(HeaderNames.Path, "/"),
+                new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
+            };
+            await InitializeConnectionAsync(context => tcs.Task);
+
+            await StartStreamAsync(1, headers, endStream: false);
+            await SendDataAsync(1, new byte[1], endStream: false);
+            await SendRstStreamAsync(1);
+            await SendRstStreamAsync(1);
+            tcs.TrySetResult(0);
+
+            await WaitForConnectionErrorAsync<Http2ConnectionErrorException>(ignoreNonGoAwayFrames: false, expectedLastStreamId: 1,
+                Http2ErrorCode.STREAM_CLOSED, CoreStrings.FormatHttp2ErrorStreamAborted(Http2FrameType.RST_STREAM, 1));
+        }
+
+        [Fact]
+        public async Task RST_STREAM_IncompleteRequest_AdditionalWindowUpdateFrame_ConnectionAborted()
+        {
+            var tcs = new TaskCompletionSource<int>(TaskCreationOptions.RunContinuationsAsynchronously);
+
+            var headers = new[]
+            {
+                new KeyValuePair<string, string>(HeaderNames.Method, "POST"),
+                new KeyValuePair<string, string>(HeaderNames.Path, "/"),
+                new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
+            };
+            await InitializeConnectionAsync(context => tcs.Task);
+
+            await StartStreamAsync(1, headers, endStream: false);
+            await SendDataAsync(1, new byte[1], endStream: false);
+            await SendRstStreamAsync(1);
+            await SendWindowUpdateAsync(1, 1024);
+            tcs.TrySetResult(0);
+
+            await WaitForConnectionErrorAsync<Http2ConnectionErrorException>(ignoreNonGoAwayFrames: false, expectedLastStreamId: 1,
+                Http2ErrorCode.STREAM_CLOSED, CoreStrings.FormatHttp2ErrorStreamAborted(Http2FrameType.WINDOW_UPDATE, 1));
+        }
+
         [Fact]
         public async Task SETTINGS_KestrelDefaults_Sent()
         {

test/Kestrel.Transport.FunctionalTests/Http2/H2SpecTests.cs

@@ -55,7 +55,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests.Http2
             get
             {
                 var dataset = new TheoryData<H2SpecTestCase>();
-                var toSkip = new[] { "http2/5.1/8" };
+                var toSkip = new string[] { /*"http2/5.1/8"*/ };
 
                 foreach (var testcase in H2SpecCommands.EnumerateTestCases())
                 {