From 277a5502fd44d565d8086480b829d1c9995a78ab Mon Sep 17 00:00:00 2001 From: "Chris Ross (ASP.NET)" Date: Fri, 19 Oct 2018 10:14:19 -0700 Subject: [PATCH] Flush response headers #3031 --- .../Internal/Http2/Http2FrameWriter.cs | 17 ++----- .../Internal/Http2/Http2OutputProducer.cs | 18 +------ .../Http2/Http2StreamTests.cs | 49 +++++++++++++++++++ 3 files changed, 54 insertions(+), 30 deletions(-) diff --git a/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs b/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs index abaea59245..1242c93131 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2FrameWriter.cs @@ -104,19 +104,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 } } - public Task FlushAsync(IHttpOutputAborter outputAborter, CancellationToken cancellationToken) - { - lock (_writeLock) - { - if (_completed) - { - return Task.CompletedTask; - } - - return _flusher.FlushAsync(outputAborter, cancellationToken); - } - } - public Task Write100ContinueAsync(int streamId) { lock (_writeLock) @@ -171,9 +158,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 throw new InvalidOperationException(hex.Message, hex); // Report the error to the user if this was the first write. } } + + _ = _flusher.FlushAsync(); } - public Task WriteResponseTrailers(int streamId, HttpResponseTrailers headers) + public Task WriteResponseTrailersAsync(int streamId, HttpResponseTrailers headers) { lock (_writeLock) { diff --git a/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs b/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs index fa31210932..83e517d05f 100644 --- a/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs +++ b/src/Kestrel.Core/Internal/Http2/Http2OutputProducer.cs @@ -28,7 +28,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 private readonly object _dataWriterLock = new object(); private readonly Pipe _dataPipe; private readonly Task _dataWriteProcessingTask; - private bool _startedWritingDataFrames; private bool _completed; private bool _disposed; @@ -101,18 +100,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return Task.CompletedTask; } - if (_startedWritingDataFrames) - { - // If there's already been response data written to the stream, just wait for that. Any header - // should be in front of the data frames in the connection pipe. Trailers could change things. - return _flusher.FlushAsync(this, cancellationToken); - } - else - { - // Flushing the connection pipe ensures headers already in the pipe are flushed even if no data - // frames have been written. - return _frameWriter.FlushAsync(this, cancellationToken); - } + return _flusher.FlushAsync(this, cancellationToken); } } @@ -160,8 +148,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 return Task.CompletedTask; } - _startedWritingDataFrames = true; - _dataPipe.Writer.Write(data); return _flusher.FlushAsync(this, cancellationToken); } @@ -212,7 +198,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2 await _frameWriter.WriteDataAsync(_streamId, _flowControl, _stream.MinResponseDataRate, readResult.Buffer, endStream: false); } - await _frameWriter.WriteResponseTrailers(_streamId, _stream.Trailers); + await _frameWriter.WriteResponseTrailersAsync(_streamId, _stream.Trailers); } else { diff --git a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 8375561f9b..1cf437ecab 100644 --- a/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/test/Kestrel.InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -2083,5 +2083,54 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); } + + [Fact] + public async Task ResponseHeaders_NotBlockedByFlowControl() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(context => + { + return context.Response.WriteAsync("hello world"); + }); + + _clientSettings.InitialWindowSize = 0; + await SendSettingsAsync(); + await ExpectAsync(Http2FrameType.SETTINGS, + withLength: 0, + withFlags: (byte)Http2SettingsFrameFlags.ACK, + withStreamId: 0); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + + await SendWindowUpdateAsync(1, 11); + + await ExpectAsync(Http2FrameType.DATA, + withLength: 11, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + + await ExpectAsync(Http2FrameType.DATA, + withLength: 0, + withFlags: (byte)Http2DataFrameFlags.END_STREAM, + withStreamId: 1); + + await StopConnectionAsync(expectedLastStreamId: 1, ignoreNonGoAwayFrames: false); + + _hpackDecoder.Decode(headersFrame.PayloadSequence, endHeaders: false, handler: this); + + Assert.Equal(2, _decodedHeaders.Count); + Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); + Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); + } } } \ No newline at end of file