Don't double-flush HTTP/1 Content-Length responses (#11825)v3.0.0-preview7.19353.9

* Don't double-flush HTTP/1 Content-Length responses

* PR feedback

2 files changed, 77 insertions, 10 deletions

diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs
index c1ee388abf..a45a740950 100644
--- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs
+++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs
@@ -55,7 +55,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
         // and append the end terminator.
 
         private bool _autoChunk;
-        private bool _suffixSent;
+
+        // We rely on the TimingPipeFlusher to give us ValueTasks that can be safely awaited multiple times.
+        private bool _writeStreamSuffixCalled;
+        private ValueTask<FlushResult> _writeStreamSuffixValueTask;
+
         private int _advancedBytesForChunk;
         private Memory<byte> _currentChunkMemory;
         private bool _currentChunkMemoryUpdated;
@@ -113,15 +117,28 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
         {
             lock (_contextLock)
             {
-                if (_suffixSent || !_autoChunk)
+                if (_writeStreamSuffixCalled)
                 {
-                    _suffixSent = true;
-                    return FlushAsync();
+                    // If WriteStreamSuffixAsync has already been called, no-op and return the previously returned ValueTask.
+                    return _writeStreamSuffixValueTask;
                 }
 
-                _suffixSent = true;
-                var writer = new BufferWriter<PipeWriter>(_pipeWriter);
-                return WriteAsyncInternal(ref writer, EndChunkedResponseBytes);
+                if (_autoChunk)
+                {
+                    var writer = new BufferWriter<PipeWriter>(_pipeWriter);
+                    _writeStreamSuffixValueTask = WriteAsyncInternal(ref writer, EndChunkedResponseBytes);
+                }
+                else if (_unflushedBytes > 0)
+                {
+                    _writeStreamSuffixValueTask = FlushAsync();
+                }
+                else
+                {
+                    _writeStreamSuffixValueTask = default;
+                }
+
+                _writeStreamSuffixCalled = true;
+                return _writeStreamSuffixValueTask;
             }
         }
 
@@ -510,7 +527,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
             // Cleared in sequential address ascending order 
             _currentMemoryPrefixBytes = 0;
             _autoChunk = false;
-            _suffixSent = false;
+            _writeStreamSuffixCalled = false;
+            _writeStreamSuffixValueTask = default;
             _currentChunkMemoryUpdated = false;
             _startCalled = false;
         }
@@ -701,7 +719,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http
         [StackTraceHidden]
         private void ThrowIfSuffixSent()
         {
-            if (_suffixSent)
+            if (_writeStreamSuffixCalled)
             {
                 throw new InvalidOperationException("Writing is not allowed after writer was completed.");
             }
diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs
index ac98e909bd..a859fec144 100644
--- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs
+++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs
@@ -2903,7 +2903,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests
             var expectedString = new string('a', expectedLength);
             await using (var server = new TestServer(async httpContext =>
             {
-                httpContext.Response.Headers["Content-Length"] = new[] { expectedLength.ToString() };
+                httpContext.Response.ContentLength = expectedLength;
                 await httpContext.Response.WriteAsync(expectedString);
                 Assert.True(httpContext.Response.HasStarted);
             }, testContext))
@@ -2926,6 +2926,55 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests
         }
 
         [Fact]
+        public async Task UnflushedContentLengthResponseIsFlushedAutomatically()
+        {
+            var testContext = new TestServiceContext(LoggerFactory);
+            var expectedLength = 100000;
+            var expectedString = new string('a', expectedLength);
+
+            void WriteStringWithoutFlushing(PipeWriter writer, string content)
+            {
+                var encoder = Encoding.ASCII.GetEncoder();
+                var encodedLength = Encoding.ASCII.GetByteCount(expectedString);
+                var source = expectedString.AsSpan();
+                var completed = false;
+
+                while (!completed)
+                {
+                    encoder.Convert(source, writer.GetSpan(), flush: source.Length == 0, out var charsUsed, out var bytesUsed, out completed);
+                    writer.Advance(bytesUsed);
+                    source = source.Slice(charsUsed);
+                }
+            }
+
+            await using (var server = new TestServer(httpContext =>
+            {
+                httpContext.Response.ContentLength = expectedLength;
+
+                WriteStringWithoutFlushing(httpContext.Response.BodyWriter, expectedString);
+
+                Assert.False(httpContext.Response.HasStarted);
+                return Task.CompletedTask;
+            }, testContext))
+            {
+                using (var connection = server.CreateConnection())
+                {
+                    await connection.Send(
+                        "GET / HTTP/1.1",
+                        "Host:",
+                        "",
+                        "");
+                    await connection.Receive(
+                        "HTTP/1.1 200 OK",
+                        $"Date: {testContext.DateHeaderValue}",
+                        $"Content-Length: {expectedLength}",
+                        "",
+                        expectedString);
+                }
+            }
+        }
+
+        [Fact]
         public async Task StartAsyncAndFlushWorks()
         {
             var testContext = new TestServiceContext(LoggerFactory);