Merge branch 'release/3.0' => 'release/3.1' (#13790)

src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs

@@ -1009,6 +1009,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
                 return Task.CompletedTask;
             }
 
+            _isLeasedMemoryInvalid = true;
+
             if (_requestRejectedException != null || _applicationException != null)
             {
                 if (HasResponseStarted)
@@ -1339,8 +1341,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
 
             if (_isLeasedMemoryInvalid)
             {
-                throw new InvalidOperationException("Invalid ordering of calling StartAsync and Advance. " +
+                throw new InvalidOperationException("Invalid ordering of calling StartAsync or CompleteAsync and Advance.");
-                    "Call StartAsync before calling GetMemory/GetSpan and Advance.");
             }
 
             if (_canWriteResponseBody)

src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs

@@ -80,6 +80,9 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
 
                 Stop();
 
+                // Make sure the writing side is completed.
+                _pipeWriter.Complete();
+
                 if (_fakeMemoryOwner != null)
                 {
                     _fakeMemoryOwner.Dispose();
@@ -104,7 +107,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
 
             lock (_dataWriterLock)
             {
-                ThrowIfSuffixSent();
+                ThrowIfSuffixSentOrDisposed();
 
                 if (_completed)
                 {
@@ -130,7 +133,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
         {
             lock (_dataWriterLock)
             {
-                ThrowIfSuffixSent();
+                ThrowIfSuffixSentOrDisposed();
 
                 if (_completed)
                 {
@@ -185,7 +188,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
 
             lock (_dataWriterLock)
             {
-                ThrowIfSuffixSent();
+                ThrowIfSuffixSentOrDisposed();
 
                 // This length check is important because we don't want to set _startedWritingDataFrames unless a data
                 // frame will actually be written causing the headers to be flushed.
@@ -233,7 +236,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
         {
             lock (_dataWriterLock)
             {
-                ThrowIfSuffixSent();
+                ThrowIfSuffixSentOrDisposed();
 
                 if (_completed)
                 {
@@ -250,7 +253,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
         {
             lock (_dataWriterLock)
             {
-                ThrowIfSuffixSent();
+                ThrowIfSuffixSentOrDisposed();
 
                 if (_completed)
                 {
@@ -265,7 +268,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
         {
             lock (_dataWriterLock)
             {
-                ThrowIfSuffixSent();
+                ThrowIfSuffixSentOrDisposed();
 
                 if (_completed)
                 {
@@ -298,7 +301,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
 
             lock (_dataWriterLock)
             {
-                ThrowIfSuffixSent();
+                ThrowIfSuffixSentOrDisposed();
 
                 // This length check is important because we don't want to set _startedWritingDataFrames unless a data
                 // frame will actually be written causing the headers to be flushed.
@@ -345,10 +348,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
 
                 _completed = true;
 
-                // Complete with an exception to prevent an end of stream data frame from being sent without an
+                _pipeReader.CancelPendingRead();
-                // explicit call to WriteStreamSuffixAsync. ConnectionAbortedExceptions are swallowed, so the
-                // message doesn't matter
-                _pipeWriter.Complete(new OperationCanceledException());
 
                 _frameWriter.AbortPendingStreamDataWrites(_flowControl);
             }
@@ -369,7 +369,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
                 {
                     readResult = await _pipeReader.ReadAsync();
 
-                    if (readResult.IsCompleted && _stream.ResponseTrailers?.Count > 0)
+                    if (readResult.IsCanceled)
+                    {
+                        // Response body is aborted, break and complete reader.
+                        break;
+                    }
+                    else if (readResult.IsCompleted && _stream.ResponseTrailers?.Count > 0)
                     {
                         // Output is ending and there are trailers to write
                         // Write any remaining content then write trailers
@@ -405,10 +410,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
                     _pipeReader.AdvanceTo(readResult.Buffer.End);
                 } while (!readResult.IsCompleted);
             }
-            catch (OperationCanceledException)
-            {
-                // Writes should not throw for aborted streams/connections.
-            }
             catch (Exception ex)
             {
                 _log.LogCritical(ex, nameof(Http2OutputProducer) + "." + nameof(ProcessDataWrites) + " observed an unexpected exception.");
@@ -435,12 +436,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
         }
 
         [StackTraceHidden]
-        private void ThrowIfSuffixSent()
+        private void ThrowIfSuffixSentOrDisposed()
         {
             if (_suffixSent)
             {
                 ThrowSuffixSent();
             }
+
+            if (_disposed)
+            {
+                ThrowDisposed();
+            }
         }
 
         [StackTraceHidden]
@@ -449,6 +455,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
             throw new InvalidOperationException("Writing is not allowed after writer was completed.");
         }
 
+        [StackTraceHidden]
+        private static void ThrowDisposed()
+        {
+            throw new InvalidOperationException("Cannot write to response after the request has completed.");
+        }
+
         private static Pipe CreateDataPipe(MemoryPool<byte> pool)
             => new Pipe(new PipeOptions
             (

src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs

@@ -3737,6 +3737,52 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             Assert.Equal("Hello World", Encoding.UTF8.GetString(bodyFrame.Payload.Span));
         }
 
+        [Fact]
+        public async Task CompleteAsync_AdvanceAfterComplete_AdvanceThrows()
+        {
+            var tcs = new TaskCompletionSource<object>();
+            var headers = new[]
+            {
+                new KeyValuePair<string, string>(HeaderNames.Method, "GET"),
+                new KeyValuePair<string, string>(HeaderNames.Path, "/"),
+                new KeyValuePair<string, string>(HeaderNames.Scheme, "http"),
+            };
+            await InitializeConnectionAsync(async context =>
+            {
+                var memory = context.Response.BodyWriter.GetMemory(12);
+                await context.Response.CompleteAsync();
+                try
+                {
+                    context.Response.BodyWriter.Advance(memory.Length);
+                }
+                catch (InvalidOperationException)
+                {
+                    tcs.SetResult(null);
+                    return;
+                }
+
+                Assert.True(false);
+            });
+
+            await StartStreamAsync(1, headers, endStream: true);
+
+            var headersFrame = await ExpectAsync(Http2FrameType.HEADERS,
+                withLength: 55,
+                withFlags: (byte)(Http2HeadersFrameFlags.END_HEADERS | Http2HeadersFrameFlags.END_STREAM),
+                withStreamId: 1);
+
+            await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false);
+
+            _hpackDecoder.Decode(headersFrame.PayloadSequence, endHeaders: false, handler: this);
+
+            Assert.Equal(3, _decodedHeaders.Count);
+            Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase);
+            Assert.Equal("200", _decodedHeaders[HeaderNames.Status]);
+            Assert.Equal("0", _decodedHeaders[HeaderNames.ContentLength]);
+
+            await tcs.Task.DefaultTimeout();
+        }
+
         [Fact]
         public async Task CompleteAsync_AfterPipeWrite_WithTrailers_SendsBodyAndTrailersWithEndStream()
         {