From 9cefd66c1e3b460e3242f0193cf7fa2b1b28c8e0 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Fri, 18 Jan 2019 14:51:33 -0800 Subject: [PATCH 01/33] Expose response PipeWriter in Kestrel --- .../HttpResponseWritingExtensions.cs | 4 +- .../Http.Features/src/FeatureReferences.cs | 2 +- src/Http/Http/src/WriteOnlyPipeStream.cs | 2 + .../Core/src/Internal/Http/ChunkWriter.cs | 3 +- .../src/Internal/Http/Http1OutputProducer.cs | 162 ++++- .../Http/HttpProtocol.FeatureCollection.cs | 69 +- .../Internal/Http/HttpProtocol.Generated.cs | 23 + .../Core/src/Internal/Http/HttpProtocol.cs | 254 +++++--- .../Internal/Http/HttpResponsePipeWriter.cs | 110 ++++ .../src/Internal/Http/HttpResponseStream.cs | 138 +--- .../src/Internal/Http/IHttpOutputAborter.cs | 2 +- .../src/Internal/Http/IHttpOutputProducer.cs | 17 +- .../src/Internal/Http/IHttpResponseControl.cs | 10 +- .../Http/IHttpResponsePipeWriterControl.cs | 23 + .../src/Internal/Http2/Http2Connection.cs | 4 +- .../src/Internal/Http2/Http2FrameWriter.cs | 52 +- .../src/Internal/Http2/Http2OutputProducer.cs | 89 ++- .../src/Internal/Infrastructure/Streams.cs | 6 +- .../Infrastructure/TimingPipeFlusher.cs | 32 +- .../HttpProtocolFeatureCollectionTests.cs | 3 + .../Core/test/HttpResponseStreamTests.cs | 45 +- ...spNetCore.Server.Kestrel.Core.Tests.csproj | 4 + src/Servers/Kestrel/Core/test/StreamsTests.cs | 8 +- .../TestHelpers/MockHttpResponseControl.cs | 27 - .../test/HttpResponseWritingExtensions.cs | 62 ++ src/Servers/Kestrel/shared/test/TestApp.cs | 37 +- .../test/FunctionalTests/ResponseTests.cs | 19 +- .../ChunkedResponseTests.cs | 438 ++++++++++++- .../ConnectionAdapterTests.cs | 85 ++- .../Http2/Http2StreamTests.cs | 604 +++++++++++++++++- .../Http2/Http2TestBase.cs | 4 +- .../InMemory.FunctionalTests.csproj | 4 + .../InMemory.FunctionalTests/ResponseTests.cs | 324 ++++++++-- .../HttpProtocolFeatureCollection.cs | 4 +- 34 files changed, 2234 insertions(+), 436 deletions(-) create mode 100644 src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs create mode 100644 src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponsePipeWriterControl.cs delete mode 100644 src/Servers/Kestrel/Core/test/TestHelpers/MockHttpResponseControl.cs create mode 100644 src/Servers/Kestrel/shared/test/HttpResponseWritingExtensions.cs diff --git a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs index 0b24a7d4f48b..b94d35dcee1b 100644 --- a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs +++ b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs @@ -61,7 +61,7 @@ public static class HttpResponseWritingExtensions } byte[] data = encoding.GetBytes(text); - return response.Body.WriteAsync(data, 0, data.Length, cancellationToken); + return response.BodyPipe.WriteAsync(new Memory(data, 0, data.Length), cancellationToken).AsTask(); } } -} \ No newline at end of file +} diff --git a/src/Http/Http.Features/src/FeatureReferences.cs b/src/Http/Http.Features/src/FeatureReferences.cs index 04bc8d14be61..918a77fa9bee 100644 --- a/src/Http/Http.Features/src/FeatureReferences.cs +++ b/src/Http/Http.Features/src/FeatureReferences.cs @@ -95,4 +95,4 @@ private TFeature UpdateCached(ref TFeature cached, TState stat public TFeature Fetch(ref TFeature cached, Func factory) where TFeature : class => Fetch(ref cached, Collection, factory); } -} \ No newline at end of file +} diff --git a/src/Http/Http/src/WriteOnlyPipeStream.cs b/src/Http/Http/src/WriteOnlyPipeStream.cs index 0f5121cc2f3e..904069b73542 100644 --- a/src/Http/Http/src/WriteOnlyPipeStream.cs +++ b/src/Http/Http/src/WriteOnlyPipeStream.cs @@ -60,6 +60,8 @@ public override int ReadTimeout set => throw new NotSupportedException(); } + public PipeWriter InnerPipeWriter => _pipeWriter; + /// public override int Read(byte[] buffer, int offset, int count) => throw new NotSupportedException(); diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/ChunkWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http/ChunkWriter.cs index 7bb2a3781f26..500d5124caa1 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/ChunkWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/ChunkWriter.cs @@ -44,13 +44,14 @@ public static int BeginChunkBytes(int dataCount, Span span) return count; } - internal static void WriteBeginChunkBytes(this ref BufferWriter start, int dataCount) + internal static int WriteBeginChunkBytes(this ref BufferWriter start, int dataCount) { // 10 bytes is max length + \r\n start.Ensure(10); var count = BeginChunkBytes(dataCount, start.Span); start.Advance(count); + return count; } internal static void WriteEndChunkBytes(this ref BufferWriter start) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs index e304824db255..5ba5e12ee853 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs @@ -3,6 +3,7 @@ using System; using System.Buffers; +using System.Diagnostics; using System.IO.Pipelines; using System.Text; using System.Threading; @@ -26,15 +27,26 @@ public class Http1OutputProducer : IHttpOutputProducer, IHttpOutputAborter, IDis private readonly IHttpMinResponseDataRateFeature _minResponseDataRateFeature; private readonly TimingPipeFlusher _flusher; - // This locks access to to all of the below fields + // This locks access to all of the below fields private readonly object _contextLock = new object(); private bool _completed = false; private bool _aborted; private long _unflushedBytes; - + private bool _autoChunk; private readonly PipeWriter _pipeWriter; + private const int BeginChunkLengthMax = 5; + private const int EndChunkLength = 2; + + // Chunked responses need to be treated uniquely when using GetMemory + Advance. + // We need to know the size of the data written to the chunk before calling Advance on the + // PipeWriter, meaning we internally track how far we have advanced through a current chunk (_advancedBytesForChunk). + // Once write or flush is called, we modify the _currentChunkMemory to prepend the size of data written + // and append the end terminator. + private int _advancedBytesForChunk; + private Memory _currentChunkMemory; + public Http1OutputProducer( PipeWriter pipeWriter, string connectionId, @@ -58,28 +70,92 @@ public Task WriteDataAsync(ReadOnlySpan buffer, CancellationToken cancella return Task.FromCanceled(cancellationToken); } + return WriteAsync(buffer, cancellationToken).AsTask(); + } + + public ValueTask WriteDataToPipeAsync(ReadOnlySpan buffer, CancellationToken cancellationToken = default) + { + if (cancellationToken.IsCancellationRequested) + { + return new ValueTask(Task.FromCanceled(cancellationToken)); + } + return WriteAsync(buffer, cancellationToken); } - public Task WriteStreamSuffixAsync() + public ValueTask WriteStreamSuffixAsync() { return WriteAsync(_endChunkedResponseBytes.Span); } - public Task FlushAsync(CancellationToken cancellationToken = default) + public ValueTask FlushAsync(CancellationToken cancellationToken = default) { return WriteAsync(Constants.EmptyData, cancellationToken); } - public Task WriteChunkAsync(ReadOnlySpan buffer, CancellationToken cancellationToken) + public Memory GetMemory(int sizeHint = 0) + { + if (_autoChunk) + { + return GetChunkedMemory(sizeHint); + } + else + { + return _pipeWriter.GetMemory(sizeHint); + } + } + + public Span GetSpan(int sizeHint = 0) + { + if (_autoChunk) + { + return GetChunkedMemory(sizeHint).Span; + } + else + { + return _pipeWriter.GetMemory(sizeHint).Span; + } + } + + public void Advance(int bytes) + { + if (_autoChunk) + { + if (bytes < 0) + { + throw new ArgumentOutOfRangeException(nameof(bytes)); + } + + if (bytes + _advancedBytesForChunk > _currentChunkMemory.Length - BeginChunkLengthMax - EndChunkLength) + { + throw new InvalidOperationException("Can't advance past buffer size."); + } + _advancedBytesForChunk += bytes; + } + else + { + _pipeWriter.Advance(bytes); + } + } + + public void CancelPendingFlush() + { + // TODO we may not want to support this. + _pipeWriter.CancelPendingFlush(); + } + + // This method is for chunked http responses + public ValueTask WriteChunkAsync(ReadOnlySpan buffer, CancellationToken cancellationToken) { lock (_contextLock) { if (_completed) { - return Task.CompletedTask; + return default; } + CommitChunkToPipe(); + if (buffer.Length > 0) { var writer = new BufferWriter(_pipeWriter); @@ -96,7 +172,7 @@ public Task WriteChunkAsync(ReadOnlySpan buffer, CancellationToken cancell return FlushAsync(cancellationToken); } - public void WriteResponseHeaders(int statusCode, string reasonPhrase, HttpResponseHeaders responseHeaders) + public void WriteResponseHeaders(int statusCode, string reasonPhrase, HttpResponseHeaders responseHeaders, bool autoChunk) { lock (_contextLock) { @@ -117,6 +193,7 @@ public void WriteResponseHeaders(int statusCode, string reasonPhrase, HttpRespon writer.Commit(); _unflushedBytes += writer.BytesCommitted; + _autoChunk = autoChunk; } } @@ -139,7 +216,6 @@ public void Abort(ConnectionAbortedException error) { // Abort can be called after Dispose if there's a flush timeout. // It's important to still call _lifetimeFeature.Abort() in this case. - lock (_contextLock) { if (_aborted) @@ -153,12 +229,18 @@ public void Abort(ConnectionAbortedException error) } } - public Task Write100ContinueAsync() + public ValueTask Write100ContinueAsync() { return WriteAsync(_continueBytes.Span); } - private Task WriteAsync( + public void Complete(Exception exception = null) + { + // TODO What to do with exception. + // and how to handle writing to response here. + } + + private ValueTask WriteAsync( ReadOnlySpan buffer, CancellationToken cancellationToken = default) { @@ -166,7 +248,14 @@ private Task WriteAsync( { if (_completed) { - return Task.CompletedTask; + return default; + } + + if (_autoChunk) + { + // If there is data that was chunked before writing (ex someone did GetMemory->Advance->WriteAsync) + // make sure to write whatever was advanced first + CommitChunkToPipe(); } var writer = new BufferWriter(_pipeWriter); @@ -188,5 +277,56 @@ private Task WriteAsync( cancellationToken); } } + + private Memory GetChunkedMemory(int sizeHint) + { + // The max size of a chunk will be the size of memory returned from the PipeWriter (today 4096) + // minus 5 for the max chunked prefix size and minus 2 for the chunked ending, leaving a total of + // 4089. + + if (_currentChunkMemory.Length == 0) + { + // First time calling GetMemory + _currentChunkMemory = _pipeWriter.GetMemory(sizeHint); + } + + var memoryMaxLength = _currentChunkMemory.Length - BeginChunkLengthMax - EndChunkLength; + if (_advancedBytesForChunk == memoryMaxLength) + { + // Chunk is completely written, commit it to the pipe so GetMemory will return a new chunk of memory. + CommitChunkToPipe(); + _currentChunkMemory = _pipeWriter.GetMemory(sizeHint); + } + + var actualMemory = _currentChunkMemory.Slice( + BeginChunkLengthMax + _advancedBytesForChunk, + memoryMaxLength - _advancedBytesForChunk); + + return actualMemory; + } + + private void CommitChunkToPipe() + { + var writer = new BufferWriter(_pipeWriter); + + Debug.Assert(_advancedBytesForChunk <= _currentChunkMemory.Length); + + if (_advancedBytesForChunk > 0) + { + var bytesWritten = writer.WriteBeginChunkBytes(_advancedBytesForChunk); + if (bytesWritten < BeginChunkLengthMax) + { + // If the current chunk of memory isn't completely utilized, we need to copy the contents forwards. + // This occurs if someone uses less than 255 bytes of the current Memory segment. + // Therefore, we need to copy it forwards by either 1 or 2 bytes (depending on number of bytes) + _currentChunkMemory.Slice(BeginChunkLengthMax, _advancedBytesForChunk).CopyTo(_currentChunkMemory.Slice(bytesWritten)); + } + + writer.Write(_currentChunkMemory.Slice(bytesWritten, _advancedBytesForChunk).Span); + writer.WriteEndChunkBytes(); + writer.Commit(); + _advancedBytesForChunk = 0; + } + } } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs index 7a2510078862..eadb7c8226c9 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs @@ -2,7 +2,9 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Diagnostics; using System.IO; +using System.IO.Pipelines; using System.Net; using System.Threading; using System.Threading.Tasks; @@ -21,7 +23,8 @@ public partial class HttpProtocol : IHttpRequestFeature, IHttpRequestIdentifierFeature, IHttpBodyControlFeature, IHttpMaxRequestBodySizeFeature, - IHttpResponseStartFeature + IHttpResponseStartFeature, + IResponseBodyPipeFeature { // NOTE: When feature interfaces are added to or removed from this HttpProtocol class implementation, // then the list of `implementedFeatures` in the generated code project MUST also be updated. @@ -111,12 +114,6 @@ IHeaderDictionary IHttpResponseFeature.Headers set => ResponseHeaders = value; } - Stream IHttpResponseFeature.Body - { - get => ResponseBody; - set => ResponseBody = value; - } - CancellationToken IHttpRequestLifetimeFeature.RequestAborted { get => RequestAborted; @@ -193,6 +190,64 @@ bool IHttpBodyControlFeature.AllowSynchronousIO } } + PipeWriter IResponseBodyPipeFeature.ResponseBodyPipe + { + get + { + if (_userSetPipeWriter != null) + { + return _userSetPipeWriter; + } + + if (!object.ReferenceEquals(_cachedResponseBodyStream, ResponseBody)) + { + var responsePipeWriter = new StreamPipeWriter(ResponseBody); + ResponsePipeWriter = responsePipeWriter; + _cachedResponseBodyStream = ResponseBody; + OnCompleted((rpw) => + { + ((StreamPipeWriter)rpw).Dispose(); + return Task.CompletedTask; + }, responsePipeWriter); + + } + + return ResponsePipeWriter; + } + set + { + _userSetPipeWriter = value ?? throw new ArgumentNullException(nameof(value)); + ResponsePipeWriter = _userSetPipeWriter; + } + } + + + Stream IHttpResponseFeature.Body + { + get + { + if (_userSetResponseBody != null) + { + return _userSetResponseBody; + } + + if (!object.ReferenceEquals(_cachedResponsePipeWriter, ResponsePipeWriter)) + { + var responseBody = new WriteOnlyPipeStream(ResponsePipeWriter); + ResponseBody = responseBody; + _cachedResponsePipeWriter = ResponsePipeWriter; + OnCompleted(async (rb) => await ((WriteOnlyPipeStream)rb).DisposeAsync(), responseBody); + } + + return ResponseBody; + } + set + { + _userSetResponseBody = value ?? throw new ArgumentNullException(nameof(value)); + ResponseBody = _userSetResponseBody; + } + } + protected void ResetHttp1Features() { _currentIHttpMinRequestBodyDataRateFeature = this; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs index 14abaab13ca9..8e5fbc44aab1 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.Generated.cs @@ -15,6 +15,7 @@ public partial class HttpProtocol : IFeatureCollection { private static readonly Type IHttpRequestFeatureType = typeof(IHttpRequestFeature); private static readonly Type IHttpResponseFeatureType = typeof(IHttpResponseFeature); + private static readonly Type IResponseBodyPipeFeatureType = typeof(IResponseBodyPipeFeature); private static readonly Type IHttpRequestIdentifierFeatureType = typeof(IHttpRequestIdentifierFeature); private static readonly Type IServiceProvidersFeatureType = typeof(IServiceProvidersFeature); private static readonly Type IHttpRequestLifetimeFeatureType = typeof(IHttpRequestLifetimeFeature); @@ -39,6 +40,7 @@ public partial class HttpProtocol : IFeatureCollection private object _currentIHttpRequestFeature; private object _currentIHttpResponseFeature; + private object _currentIResponseBodyPipeFeature; private object _currentIHttpRequestIdentifierFeature; private object _currentIServiceProvidersFeature; private object _currentIHttpRequestLifetimeFeature; @@ -69,6 +71,7 @@ private void FastReset() { _currentIHttpRequestFeature = this; _currentIHttpResponseFeature = this; + _currentIResponseBodyPipeFeature = this; _currentIHttpUpgradeFeature = this; _currentIHttpRequestIdentifierFeature = this; _currentIHttpRequestLifetimeFeature = this; @@ -153,6 +156,10 @@ object IFeatureCollection.this[Type key] { feature = _currentIHttpResponseFeature; } + else if (key == IResponseBodyPipeFeatureType) + { + feature = _currentIResponseBodyPipeFeature; + } else if (key == IHttpRequestIdentifierFeatureType) { feature = _currentIHttpRequestIdentifierFeature; @@ -257,6 +264,10 @@ object IFeatureCollection.this[Type key] { _currentIHttpResponseFeature = value; } + else if (key == IResponseBodyPipeFeatureType) + { + _currentIResponseBodyPipeFeature = value; + } else if (key == IHttpRequestIdentifierFeatureType) { _currentIHttpRequestIdentifierFeature = value; @@ -359,6 +370,10 @@ TFeature IFeatureCollection.Get() { feature = (TFeature)_currentIHttpResponseFeature; } + else if (typeof(TFeature) == typeof(IResponseBodyPipeFeature)) + { + feature = (TFeature)_currentIResponseBodyPipeFeature; + } else if (typeof(TFeature) == typeof(IHttpRequestIdentifierFeature)) { feature = (TFeature)_currentIHttpRequestIdentifierFeature; @@ -467,6 +482,10 @@ void IFeatureCollection.Set(TFeature feature) { _currentIHttpResponseFeature = feature; } + else if (typeof(TFeature) == typeof(IResponseBodyPipeFeature)) + { + _currentIResponseBodyPipeFeature = feature; + } else if (typeof(TFeature) == typeof(IHttpRequestIdentifierFeature)) { _currentIHttpRequestIdentifierFeature = feature; @@ -567,6 +586,10 @@ private IEnumerable> FastEnumerable() { yield return new KeyValuePair(IHttpResponseFeatureType, _currentIHttpResponseFeature); } + if (_currentIResponseBodyPipeFeature != null) + { + yield return new KeyValuePair(IResponseBodyPipeFeatureType, _currentIResponseBodyPipeFeature); + } if (_currentIHttpRequestIdentifierFeature != null) { yield return new KeyValuePair(IHttpRequestIdentifierFeatureType, _currentIHttpRequestIdentifierFeature); diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 1a274cbc0d75..ca87df19cb32 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Buffers; using System.Collections.Generic; using System.Diagnostics; using System.IO; @@ -64,6 +63,11 @@ public abstract partial class HttpProtocol : IDefaultHttpContextContainer, IHttp protected string _methodText = null; private string _scheme = null; + protected PipeWriter _userSetPipeWriter; + protected Stream _userSetResponseBody; + protected PipeWriter _cachedResponsePipeWriter; + protected Stream _cachedResponseBodyStream; + public HttpProtocol(HttpConnectionContext context) { _context = context; @@ -227,6 +231,7 @@ public string ReasonPhrase public IHeaderDictionary ResponseHeaders { get; set; } public Stream ResponseBody { get; set; } + public PipeWriter ResponsePipeWriter { get; set; } public CancellationToken RequestAborted { @@ -295,10 +300,15 @@ public void InitializeStreams(MessageBody messageBody) { if (_streams == null) { - _streams = new Streams(bodyControl: this, httpResponseControl: this); + var pipeWriter = new HttpResponsePipeWriter(this); + _streams = new Streams(bodyControl: this, pipeWriter); + ResponsePipeWriter = pipeWriter; } (RequestBody, ResponseBody) = _streams.Start(messageBody); + + _cachedResponseBodyStream = ResponseBody; + _cachedResponsePipeWriter = ResponsePipeWriter; } public void StopStreams() => _streams.Stop(); @@ -780,21 +790,6 @@ private async Task FireOnCompletedAwaited(Task currentTask, Stack FlushAsyncInternal(CancellationToken cancellationToken) { _requestProcessingStatus = RequestProcessingStatus.HeadersFlushed; return Output.FlushAsync(cancellationToken); } - public Task WriteAsync(ReadOnlyMemory data, CancellationToken cancellationToken = default(CancellationToken)) - { - // For the first write, ensure headers are flushed if WriteDataAsync isn't called. - var firstWrite = !HasResponseStarted; - - if (firstWrite) - { - var initializeTask = InitializeResponseAsync(data.Length); - // If return is Task.CompletedTask no awaiting is required - if (!ReferenceEquals(initializeTask, Task.CompletedTask)) - { - return WriteAsyncAwaited(initializeTask, data, cancellationToken); - } - } - else - { - VerifyAndUpdateWrite(data.Length); - } - - if (_canWriteResponseBody) - { - if (_autoChunk) - { - if (data.Length == 0) - { - return !firstWrite ? Task.CompletedTask : FlushAsyncInternal(cancellationToken); - } - return WriteChunkedAsync(data.Span, cancellationToken); - } - else - { - CheckLastWrite(); - return WriteDataAsync(data, cancellationToken: cancellationToken); - } - } - else - { - HandleNonBodyResponseWrite(); - return !firstWrite ? Task.CompletedTask : FlushAsyncInternal(cancellationToken); - } - } - - public async Task WriteAsyncAwaited(Task initializeTask, ReadOnlyMemory data, CancellationToken cancellationToken) - { - await initializeTask; - - // WriteAsyncAwaited is only called for the first write to the body. - // Ensure headers are flushed if Write(Chunked)Async isn't called. - if (_canWriteResponseBody) - { - if (_autoChunk) - { - if (data.Length == 0) - { - await FlushAsyncInternal(cancellationToken); - return; - } - - await WriteChunkedAsync(data.Span, cancellationToken); - } - else - { - CheckLastWrite(); - await WriteDataAsync(data, cancellationToken); - } - } - else - { - HandleNonBodyResponseWrite(); - await FlushAsyncInternal(cancellationToken); - } - } - private Task WriteDataAsync(ReadOnlyMemory data, CancellationToken cancellationToken) { _requestProcessingStatus = RequestProcessingStatus.HeadersFlushed; @@ -958,7 +880,7 @@ protected void VerifyResponseContentLength() } } - private Task WriteChunkedAsync(ReadOnlySpan data, CancellationToken cancellationToken) + private ValueTask WriteChunkedAsync(ReadOnlySpan data, CancellationToken cancellationToken) { _requestProcessingStatus = RequestProcessingStatus.HeadersFlushed; return Output.WriteChunkAsync(data, cancellationToken); @@ -1090,7 +1012,7 @@ private Task WriteSuffix() if (!HasFlushedHeaders) { - return FlushAsyncInternal(default); + return FlushAsyncInternal(default).AsTask(); } return Task.CompletedTask; @@ -1208,7 +1130,7 @@ private void CreateResponseHeader(bool appCompleted) responseHeaders.SetRawDate(dateHeaderValues.String, dateHeaderValues.Bytes); } - Output.WriteResponseHeaders(StatusCode, ReasonPhrase, responseHeaders); + Output.WriteResponseHeaders(StatusCode, ReasonPhrase, responseHeaders, _autoChunk); } private bool CanWriteResponseBody() @@ -1345,5 +1267,149 @@ protected void ReportApplicationError(Exception ex) Log.ApplicationError(ConnectionId, TraceIdentifier, ex); } + + public void Advance(int bytes) + { + VerifyAndUpdateWrite(bytes); + Output.Advance(bytes); + } + + public Memory GetMemory(int sizeHint = 0) + { + ThrowIfResponseNotStarted(); + + return Output.GetMemory(sizeHint); + } + + public Span GetSpan(int sizeHint = 0) + { + ThrowIfResponseNotStarted(); + + return Output.GetSpan(sizeHint); + } + + [StackTraceHidden] + private void ThrowIfResponseNotStarted() + { + if (!HasResponseStarted) + { + throw new InvalidOperationException("Cannot call GetMemory() until response has started. " + + "Call HttpResponse.StartAsync() before calling GetMemory()."); + } + } + + public ValueTask FlushPipeAsync(CancellationToken cancellationToken) + { + if (!HasResponseStarted) + { + var initializeTask = InitializeResponseAsync(0); + // If return is Task.CompletedTask no awaiting is required + if (!ReferenceEquals(initializeTask, Task.CompletedTask)) + { + return FlushAsyncAwaited(initializeTask, cancellationToken); + } + } + + return Output.FlushAsync(cancellationToken); + } + + public void CancelPendingFlush() + { + Output.CancelPendingFlush(); + } + + public void Complete(Exception exception = null) + { + Output.Complete(exception); + } + + public ValueTask WritePipeAsync(ReadOnlyMemory data, CancellationToken cancellationToken) + { + // For the first write, ensure headers are flushed if WriteDataAsync isn't called. + var firstWrite = !HasResponseStarted; + + if (firstWrite) + { + var initializeTask = InitializeResponseAsync(data.Length); + // If return is Task.CompletedTask no awaiting is required + if (!ReferenceEquals(initializeTask, Task.CompletedTask)) + { + return WriteAsyncAwaited(initializeTask, data, cancellationToken); + } + } + else + { + VerifyAndUpdateWrite(data.Length); + } + + if (_canWriteResponseBody) + { + if (_autoChunk) + { + if (data.Length == 0) + { + return !firstWrite ? default : FlushAsyncInternal(cancellationToken); + } + return WriteChunkedAsync(data.Span, cancellationToken); + } + else + { + CheckLastWrite(); + return Output.WriteDataToPipeAsync(data.Span, cancellationToken: cancellationToken); + } + } + else + { + HandleNonBodyResponseWrite(); + return !firstWrite ? default : FlushAsyncInternal(cancellationToken); + } + } + + public Task FlushAsync(CancellationToken cancellationToken = default(CancellationToken)) + { + return FlushPipeAsync(cancellationToken).AsTask(); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private async ValueTask FlushAsyncAwaited(Task initializeTask, CancellationToken cancellationToken) + { + await initializeTask; + return await Output.FlushAsync(cancellationToken); + } + + public Task WriteAsync(ReadOnlyMemory data, CancellationToken cancellationToken = default(CancellationToken)) + { + return WritePipeAsync(data, cancellationToken).AsTask(); + } + + public async ValueTask WriteAsyncAwaited(Task initializeTask, ReadOnlyMemory data, CancellationToken cancellationToken) + { + await initializeTask; + + // WriteAsyncAwaited is only called for the first write to the body. + // Ensure headers are flushed if Write(Chunked)Async isn't called. + if (_canWriteResponseBody) + { + if (_autoChunk) + { + if (data.Length == 0) + { + return await FlushAsyncInternal(cancellationToken); + } + + return await WriteChunkedAsync(data.Span, cancellationToken); + } + else + { + CheckLastWrite(); + return await Output.WriteDataToPipeAsync(data.Span, cancellationToken: cancellationToken); + } + } + else + { + HandleNonBodyResponseWrite(); + return await FlushAsyncInternal(cancellationToken); + } + } } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs new file mode 100644 index 000000000000..807a05825a44 --- /dev/null +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs @@ -0,0 +1,110 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.IO.Pipelines; +using System.Runtime.CompilerServices; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http +{ + public class HttpResponsePipeWriter : PipeWriter + { + private HttpStreamState _state; + private readonly IHttpResponseControl _pipeControl; + + public HttpResponsePipeWriter(IHttpResponseControl pipeControl) + { + _pipeControl = pipeControl; + _state = HttpStreamState.Closed; + } + + public override void Advance(int bytes) + { + _pipeControl.Advance(bytes); + } + + public override void CancelPendingFlush() + { + _pipeControl.CancelPendingFlush(); + } + + public override void Complete(Exception exception = null) + { + _pipeControl.Complete(exception); + } + + public override ValueTask FlushAsync(CancellationToken cancellationToken = default) + { + ValidateState(cancellationToken); + return _pipeControl.FlushPipeAsync(cancellationToken); + } + + public override Memory GetMemory(int sizeHint = 0) + { + return _pipeControl.GetMemory(sizeHint); + } + + public override Span GetSpan(int sizeHint = 0) + { + return _pipeControl.GetSpan(sizeHint); + } + + public override void OnReaderCompleted(Action callback, object state) + { + // noop + } + + public override ValueTask WriteAsync(ReadOnlyMemory source, CancellationToken cancellationToken = default) + { + ValidateState(cancellationToken); + return _pipeControl.WritePipeAsync(source, cancellationToken); + } + + public void StartAcceptingWrites() + { + // Only start if not aborted + if (_state == HttpStreamState.Closed) + { + _state = HttpStreamState.Open; + } + } + + public void StopAcceptingWrites() + { + // Can't use dispose (or close) as can be disposed too early by user code + // As exampled in EngineTests.ZeroContentLengthNotSetAutomaticallyForCertainStatusCodes + _state = HttpStreamState.Closed; + } + + public void Abort() + { + // We don't want to throw an ODE until the app func actually completes. + if (_state != HttpStreamState.Closed) + { + _state = HttpStreamState.Aborted; + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private void ValidateState(CancellationToken cancellationToken) + { + var state = _state; + if (state == HttpStreamState.Open || state == HttpStreamState.Aborted) + { + // Aborted state only throws on write if cancellationToken requests it + cancellationToken.ThrowIfCancellationRequested(); + } + else + { + ThrowObjectDisposedException(); + } + + void ThrowObjectDisposedException() + { + throw new ObjectDisposedException(nameof(HttpResponseStream), CoreStrings.WritingToResponseBodyAfterResponseCompleted); + } + } + } +} diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponseStream.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponseStream.cs index 4ec97117cbbd..054ffaffa09f 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponseStream.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponseStream.cs @@ -2,59 +2,21 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.IO; -using System.Runtime.CompilerServices; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; +using System.IO.Pipelines; using Microsoft.AspNetCore.Http.Features; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { - internal class HttpResponseStream : WriteOnlyStream + internal class HttpResponseStream : WriteOnlyPipeStream { + private readonly HttpResponsePipeWriter _pipeWriter; private readonly IHttpBodyControlFeature _bodyControl; - private readonly IHttpResponseControl _httpResponseControl; - private HttpStreamState _state; - public HttpResponseStream(IHttpBodyControlFeature bodyControl, IHttpResponseControl httpResponseControl) + public HttpResponseStream(IHttpBodyControlFeature bodyControl, HttpResponsePipeWriter pipeWriter) + : base(pipeWriter) { _bodyControl = bodyControl; - _httpResponseControl = httpResponseControl; - _state = HttpStreamState.Closed; - } - - public override bool CanSeek => false; - - public override long Length - => throw new NotSupportedException(); - - public override long Position - { - get => throw new NotSupportedException(); - set => throw new NotSupportedException(); - } - - public override void Flush() - { - FlushAsync(default).GetAwaiter().GetResult(); - } - - public override Task FlushAsync(CancellationToken cancellationToken) - { - ValidateState(cancellationToken); - - return _httpResponseControl.FlushAsync(cancellationToken); - } - - public override long Seek(long offset, SeekOrigin origin) - { - throw new NotSupportedException(); - } - - public override void SetLength(long value) - { - throw new NotSupportedException(); + _pipeWriter = pipeWriter; } public override void Write(byte[] buffer, int offset, int count) @@ -64,104 +26,32 @@ public override void Write(byte[] buffer, int offset, int count) throw new InvalidOperationException(CoreStrings.SynchronousWritesDisallowed); } - WriteAsync(buffer, offset, count, default).GetAwaiter().GetResult(); + base.Write(buffer, offset, count); } - public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback callback, object state) + public override void Flush() { - var task = WriteAsync(buffer, offset, count, default, state); - if (callback != null) + if (!_bodyControl.AllowSynchronousIO) { - task.ContinueWith(t => callback.Invoke(t)); + throw new InvalidOperationException(CoreStrings.SynchronousWritesDisallowed); } - return task; - } - - public override void EndWrite(IAsyncResult asyncResult) - { - ((Task)asyncResult).GetAwaiter().GetResult(); - } - - private Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken, object state) - { - var tcs = new TaskCompletionSource(state); - var task = WriteAsync(buffer, offset, count, cancellationToken); - task.ContinueWith((task2, state2) => - { - var tcs2 = (TaskCompletionSource)state2; - if (task2.IsCanceled) - { - tcs2.SetCanceled(); - } - else if (task2.IsFaulted) - { - tcs2.SetException(task2.Exception); - } - else - { - tcs2.SetResult(null); - } - }, tcs, cancellationToken); - return tcs.Task; - } - - public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) - { - ValidateState(cancellationToken); - return _httpResponseControl.WriteAsync(new ReadOnlyMemory(buffer, offset, count), cancellationToken); - } - - public override ValueTask WriteAsync(ReadOnlyMemory source, CancellationToken cancellationToken = default) - { - ValidateState(cancellationToken); - - return new ValueTask(_httpResponseControl.WriteAsync(source, cancellationToken)); + base.Flush(); } public void StartAcceptingWrites() { - // Only start if not aborted - if (_state == HttpStreamState.Closed) - { - _state = HttpStreamState.Open; - } + _pipeWriter.StartAcceptingWrites(); } public void StopAcceptingWrites() { - // Can't use dispose (or close) as can be disposed too early by user code - // As exampled in EngineTests.ZeroContentLengthNotSetAutomaticallyForCertainStatusCodes - _state = HttpStreamState.Closed; + _pipeWriter.StopAcceptingWrites(); } public void Abort() { - // We don't want to throw an ODE until the app func actually completes. - if (_state != HttpStreamState.Closed) - { - _state = HttpStreamState.Aborted; - } - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void ValidateState(CancellationToken cancellationToken) - { - var state = _state; - if (state == HttpStreamState.Open || state == HttpStreamState.Aborted) - { - // Aborted state only throws on write if cancellationToken requests it - cancellationToken.ThrowIfCancellationRequested(); - } - else - { - ThrowObjectDisposedException(); - } - - void ThrowObjectDisposedException() - { - throw new ObjectDisposedException(nameof(HttpResponseStream), CoreStrings.WritingToResponseBodyAfterResponseCompleted); - } + _pipeWriter.Abort(); } } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputAborter.cs b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputAborter.cs index 989fe91ce8ae..ba0d2cb123b4 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputAborter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputAborter.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using Microsoft.AspNetCore.Connections; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs index ecd7ad3b8ff7..f6f3156578ce 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.IO.Pipelines; using System.Threading; using System.Threading.Tasks; @@ -9,12 +10,18 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { public interface IHttpOutputProducer { - Task WriteChunkAsync(ReadOnlySpan data, CancellationToken cancellationToken); - Task FlushAsync(CancellationToken cancellationToken); - Task Write100ContinueAsync(); - void WriteResponseHeaders(int statusCode, string ReasonPhrase, HttpResponseHeaders responseHeaders); + ValueTask WriteChunkAsync(ReadOnlySpan data, CancellationToken cancellationToken); + ValueTask FlushAsync(CancellationToken cancellationToken); + ValueTask Write100ContinueAsync(); + void WriteResponseHeaders(int statusCode, string ReasonPhrase, HttpResponseHeaders responseHeaders, bool autoChunk); // This takes ReadOnlySpan instead of ReadOnlyMemory because it always synchronously copies data before flushing. + ValueTask WriteDataToPipeAsync(ReadOnlySpan data, CancellationToken cancellationToken); Task WriteDataAsync(ReadOnlySpan data, CancellationToken cancellationToken); - Task WriteStreamSuffixAsync(); + ValueTask WriteStreamSuffixAsync(); + void Advance(int bytes); + Span GetSpan(int sizeHint = 0); + Memory GetMemory(int sizeHint = 0); + void CancelPendingFlush(); + void Complete(Exception exception = null); } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponseControl.cs b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponseControl.cs index 9a42aa6116c6..6ef2b4339d66 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponseControl.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponseControl.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.IO.Pipelines; using System.Threading; using System.Threading.Tasks; @@ -10,7 +11,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http public interface IHttpResponseControl { void ProduceContinue(); - Task WriteAsync(ReadOnlyMemory data, CancellationToken cancellationToken); - Task FlushAsync(CancellationToken cancellationToken); + Memory GetMemory(int sizeHint = 0); + Span GetSpan(int sizeHint = 0); + void Advance(int bytes); + ValueTask FlushPipeAsync(CancellationToken cancellationToken); + ValueTask WritePipeAsync(ReadOnlyMemory source, CancellationToken cancellationToken); + void CancelPendingFlush(); + void Complete(Exception exception = null); } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponsePipeWriterControl.cs b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponsePipeWriterControl.cs new file mode 100644 index 000000000000..eafafbad7db7 --- /dev/null +++ b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpResponsePipeWriterControl.cs @@ -0,0 +1,23 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.IO.Pipelines; +using System.Text; +using System.Threading; +using System.Threading.Tasks; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http +{ + public interface IHttpResponsePipeWriterControl + { + void ProduceContinue(); + Memory GetMemory(int sizeHint = 0); + Span GetSpan(int sizeHint = 0); + void Advance(int bytes); + ValueTask FlushPipeAsync(CancellationToken cancellationToken); + ValueTask WritePipeAsync(ReadOnlyMemory source, CancellationToken cancellationToken); + void CancelPendingFlush(); + } +} diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs index d80df01e4409..3179b2f82ab8 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs @@ -705,7 +705,7 @@ private Task ProcessSettingsFrameAsync(ReadOnlySequence payload) } } - return ackTask; + return ackTask.AsTask(); } catch (Http2SettingsParameterOutOfRangeException ex) { @@ -738,7 +738,7 @@ private Task ProcessPingFrameAsync(ReadOnlySequence payload) return Task.CompletedTask; } - return _frameWriter.WritePingAsync(Http2PingFrameFlags.ACK, payload); + return _frameWriter.WritePingAsync(Http2PingFrameFlags.ACK, payload).AsTask(); } private Task ProcessGoAwayFrameAsync() diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index 73be2ed64ad4..af9927e1a6a8 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -109,13 +109,13 @@ public void Abort(ConnectionAbortedException error) } } - public Task FlushAsync(IHttpOutputAborter outputAborter, CancellationToken cancellationToken) + public ValueTask FlushAsync(IHttpOutputAborter outputAborter, CancellationToken cancellationToken) { lock (_writeLock) { if (_completed) { - return Task.CompletedTask; + return default; } var bytesWritten = _unflushedBytes; @@ -125,13 +125,13 @@ public Task FlushAsync(IHttpOutputAborter outputAborter, CancellationToken cance } } - public Task Write100ContinueAsync(int streamId) + public ValueTask Write100ContinueAsync(int streamId) { lock (_writeLock) { if (_completed) { - return Task.CompletedTask; + return default; } _outgoingFrame.PrepareHeaders(Http2HeadersFrameFlags.END_HEADERS, streamId); @@ -181,13 +181,13 @@ public void WriteResponseHeaders(int streamId, int statusCode, IHeaderDictionary } } - public Task WriteResponseTrailers(int streamId, HttpResponseTrailers headers) + public ValueTask WriteResponseTrailers(int streamId, HttpResponseTrailers headers) { lock (_writeLock) { if (_completed) { - return Task.CompletedTask; + return default; } try @@ -236,7 +236,7 @@ private void FinishWritingHeaders(int streamId, int payloadLength, bool done) } } - public Task WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, ReadOnlySequence data, bool endStream) + public ValueTask WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, ReadOnlySequence data, bool endStream) { // The Length property of a ReadOnlySequence can be expensive, so we cache the value. var dataLength = data.Length; @@ -245,7 +245,7 @@ public Task WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, Re { if (_completed || flowControl.IsAborted) { - return Task.CompletedTask; + return default; } // Zero-length data frames are allowed to be sent immediately even if there is no space available in the flow control window. @@ -312,12 +312,14 @@ private void WriteDataUnsynchronized(int streamId, ReadOnlySequence data, // Plus padding } - private async Task WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, ReadOnlySequence data, long dataLength, bool endStream) + private async ValueTask WriteDataAsync(int streamId, StreamOutputFlowControl flowControl, ReadOnlySequence data, long dataLength, bool endStream) { + FlushResult flushResult = default; + while (dataLength > 0) { OutputFlowControlAwaitable availabilityAwaitable; - var writeTask = Task.CompletedTask; + var writeTask = new ValueTask(new FlushResult()); lock (_writeLock) { @@ -373,7 +375,7 @@ private async Task WriteDataAsync(int streamId, StreamOutputFlowControl flowCont await availabilityAwaitable; } - await writeTask; + flushResult = await writeTask; if (_minResponseDataRate != null) { @@ -383,6 +385,8 @@ private async Task WriteDataAsync(int streamId, StreamOutputFlowControl flowCont // Ensure that the application continuation isn't executed inline by ProcessWindowUpdateFrameAsync. await ThreadPoolAwaitable.Instance; + + return flushResult; } /* https://tools.ietf.org/html/rfc7540#section-6.9 @@ -390,13 +394,13 @@ private async Task WriteDataAsync(int streamId, StreamOutputFlowControl flowCont |R| Window Size Increment (31) | +-+-------------------------------------------------------------+ */ - public Task WriteWindowUpdateAsync(int streamId, int sizeIncrement) + public ValueTask WriteWindowUpdateAsync(int streamId, int sizeIncrement) { lock (_writeLock) { if (_completed) { - return Task.CompletedTask; + return default; } _outgoingFrame.PrepareWindowUpdate(streamId, sizeIncrement); @@ -413,13 +417,13 @@ public Task WriteWindowUpdateAsync(int streamId, int sizeIncrement) | Error Code (32) | +---------------------------------------------------------------+ */ - public Task WriteRstStreamAsync(int streamId, Http2ErrorCode errorCode) + public ValueTask WriteRstStreamAsync(int streamId, Http2ErrorCode errorCode) { lock (_writeLock) { if (_completed) { - return Task.CompletedTask; + return default; } _outgoingFrame.PrepareRstStream(streamId, errorCode); @@ -440,13 +444,13 @@ public Task WriteRstStreamAsync(int streamId, Http2ErrorCode errorCode) | Value (32) | +---------------------------------------------------------------+ */ - public Task WriteSettingsAsync(IList settings) + public ValueTask WriteSettingsAsync(IList settings) { lock (_writeLock) { if (_completed) { - return Task.CompletedTask; + return default; } _outgoingFrame.PrepareSettings(Http2SettingsFrameFlags.NONE); @@ -473,13 +477,13 @@ internal static void WriteSettings(IList settings, Span } // No payload - public Task WriteSettingsAckAsync() + public ValueTask WriteSettingsAckAsync() { lock (_writeLock) { if (_completed) { - return Task.CompletedTask; + return default; } _outgoingFrame.PrepareSettings(Http2SettingsFrameFlags.ACK); @@ -495,13 +499,13 @@ public Task WriteSettingsAckAsync() | | +---------------------------------------------------------------+ */ - public Task WritePingAsync(Http2PingFrameFlags flags, ReadOnlySequence payload) + public ValueTask WritePingAsync(Http2PingFrameFlags flags, ReadOnlySequence payload) { lock (_writeLock) { if (_completed) { - return Task.CompletedTask; + return default; } _outgoingFrame.PreparePing(Http2PingFrameFlags.ACK); @@ -525,13 +529,13 @@ public Task WritePingAsync(Http2PingFrameFlags flags, ReadOnlySequence pay | Additional Debug Data (*) | (not implemented) +---------------------------------------------------------------+ */ - public Task WriteGoAwayAsync(int lastStreamId, Http2ErrorCode errorCode) + public ValueTask WriteGoAwayAsync(int lastStreamId, Http2ErrorCode errorCode) { lock (_writeLock) { if (_completed) { - return Task.CompletedTask; + return default; } _outgoingFrame.PrepareGoAway(lastStreamId, errorCode); @@ -583,7 +587,7 @@ internal static void WriteHeader(Http2Frame frame, PipeWriter output) output.Advance(Http2FrameReader.HeaderLength); } - private Task TimeFlushUnsynchronizedAsync() + private ValueTask TimeFlushUnsynchronizedAsync() { var bytesWritten = _unflushedBytes; _unflushedBytes = 0; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs index 44552fee7620..4b2b5362ebee 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs @@ -27,7 +27,7 @@ public class Http2OutputProducer : IHttpOutputProducer, IHttpOutputAborter private readonly Http2Stream _stream; private readonly object _dataWriterLock = new object(); private readonly Pipe _dataPipe; - private readonly Task _dataWriteProcessingTask; + private readonly ValueTask _dataWriteProcessingTask; private bool _startedWritingDataFrames; private bool _completed; private bool _disposed; @@ -88,18 +88,18 @@ public Task WriteChunkAsync(ReadOnlySpan span, CancellationToken cancellat throw new NotImplementedException(); } - public Task FlushAsync(CancellationToken cancellationToken) + public ValueTask FlushAsync(CancellationToken cancellationToken) { if (cancellationToken.IsCancellationRequested) { - return Task.FromCanceled(cancellationToken); + return new ValueTask(Task.FromCanceled(cancellationToken)); } lock (_dataWriterLock) { if (_completed) { - return Task.CompletedTask; + return default; } if (_startedWritingDataFrames) @@ -117,20 +117,20 @@ public Task FlushAsync(CancellationToken cancellationToken) } } - public Task Write100ContinueAsync() + public ValueTask Write100ContinueAsync() { lock (_dataWriterLock) { if (_completed) { - return Task.CompletedTask; + return default; } return _frameWriter.Write100ContinueAsync(_streamId); } } - public void WriteResponseHeaders(int statusCode, string ReasonPhrase, HttpResponseHeaders responseHeaders) + public void WriteResponseHeaders(int statusCode, string ReasonPhrase, HttpResponseHeaders responseHeaders, bool autoChunk) { lock (_dataWriterLock) { @@ -164,17 +164,17 @@ public Task WriteDataAsync(ReadOnlySpan data, CancellationToken cancellati _startedWritingDataFrames = true; _dataPipe.Writer.Write(data); - return _flusher.FlushAsync(this, cancellationToken); + return _flusher.FlushAsync(this, cancellationToken).AsTask(); } } - public Task WriteStreamSuffixAsync() + public ValueTask WriteStreamSuffixAsync() { lock (_dataWriterLock) { if (_completed) { - return Task.CompletedTask; + return default; } _completed = true; @@ -184,7 +184,7 @@ public Task WriteStreamSuffixAsync() } } - public Task WriteRstStreamAsync(Http2ErrorCode error) + public ValueTask WriteRstStreamAsync(Http2ErrorCode error) { lock (_dataWriterLock) { @@ -196,8 +196,9 @@ public Task WriteRstStreamAsync(Http2ErrorCode error) } } - private async Task ProcessDataWrites() + private async ValueTask ProcessDataWrites() { + FlushResult flushResult = default; try { ReadResult readResult; @@ -210,14 +211,14 @@ private async Task ProcessDataWrites() { if (readResult.Buffer.Length > 0) { - await _frameWriter.WriteDataAsync(_streamId, _flowControl, readResult.Buffer, endStream: false); + flushResult = await _frameWriter.WriteDataAsync(_streamId, _flowControl, readResult.Buffer, endStream: false); } - await _frameWriter.WriteResponseTrailers(_streamId, _stream.Trailers); + flushResult = await _frameWriter.WriteResponseTrailers(_streamId, _stream.Trailers); } else { - await _frameWriter.WriteDataAsync(_streamId, _flowControl, readResult.Buffer, endStream: readResult.IsCompleted); + flushResult = await _frameWriter.WriteDataAsync(_streamId, _flowControl, readResult.Buffer, endStream: readResult.IsCompleted); } _dataPipe.Reader.AdvanceTo(readResult.Buffer.End); @@ -233,6 +234,8 @@ private async Task ProcessDataWrites() } _dataPipe.Reader.Complete(); + + return flushResult; } private static Pipe CreateDataPipe(MemoryPool pool) @@ -246,5 +249,61 @@ private static Pipe CreateDataPipe(MemoryPool pool) useSynchronizationContext: false, minimumSegmentSize: KestrelMemoryPool.MinimumSegmentSize )); + + public void Advance(int bytes) + { + _startedWritingDataFrames = true; + + _dataPipe.Writer.Advance(bytes); + } + + public Span GetSpan(int sizeHint = 0) + { + return _dataPipe.Writer.GetSpan(sizeHint); + } + + public Memory GetMemory(int sizeHint = 0) + { + return _dataPipe.Writer.GetMemory(sizeHint); + } + + public void CancelPendingFlush() + { + _dataPipe.Writer.CancelPendingFlush(); + } + + public ValueTask WriteDataToPipeAsync(ReadOnlySpan data, CancellationToken cancellationToken) + { + if (cancellationToken.IsCancellationRequested) + { + return new ValueTask(Task.FromCanceled(cancellationToken)); + } + + lock (_dataWriterLock) + { + // 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. + if (_completed || data.Length == 0) + { + return default; + } + + _startedWritingDataFrames = true; + + _dataPipe.Writer.Write(data); + return _flusher.FlushAsync(this, cancellationToken); + } + } + + ValueTask IHttpOutputProducer.WriteChunkAsync(ReadOnlySpan data, CancellationToken cancellationToken) + { + throw new NotImplementedException(); + } + + public void Complete(Exception exception = null) + { + // TODO What to do with exception. + // and how to handle writing to response here. + } } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/Streams.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/Streams.cs index 64522047f8fe..ee75017f6342 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/Streams.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/Streams.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -18,11 +18,11 @@ private static readonly ThrowingWasUpgradedWriteOnlyStream _throwingResponseStre private readonly HttpRequestStream _emptyRequest; private readonly Stream _upgradeStream; - public Streams(IHttpBodyControlFeature bodyControl, IHttpResponseControl httpResponseControl) + public Streams(IHttpBodyControlFeature bodyControl, HttpResponsePipeWriter writer) { _request = new HttpRequestStream(bodyControl); _emptyRequest = new HttpRequestStream(bodyControl); - _response = new HttpResponseStream(bodyControl, httpResponseControl); + _response = new HttpResponseStream(bodyControl, writer); _upgradeableResponse = new WrappingStream(_response); _upgradeStream = new HttpUpgradeStream(_request, _response); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/TimingPipeFlusher.cs b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/TimingPipeFlusher.cs index 1825a42bb9ae..190aaf5ac237 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Infrastructure/TimingPipeFlusher.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Infrastructure/TimingPipeFlusher.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -22,7 +22,7 @@ public class TimingPipeFlusher private readonly IKestrelTrace _log; private readonly object _flushLock = new object(); - private Task _lastFlushTask = Task.CompletedTask; + private Task _lastFlushTask = null; public TimingPipeFlusher( PipeWriter writer, @@ -34,22 +34,22 @@ public TimingPipeFlusher( _log = log; } - public Task FlushAsync() + public ValueTask FlushAsync() { return FlushAsync(outputAborter: null, cancellationToken: default); } - public Task FlushAsync(IHttpOutputAborter outputAborter, CancellationToken cancellationToken) + public ValueTask FlushAsync(IHttpOutputAborter outputAborter, CancellationToken cancellationToken) { return FlushAsync(minRate: null, count: 0, outputAborter: outputAborter, cancellationToken: cancellationToken); } - public Task FlushAsync(MinDataRate minRate, long count) + public ValueTask FlushAsync(MinDataRate minRate, long count) { return FlushAsync(minRate, count, outputAborter: null, cancellationToken: default); } - public Task FlushAsync(MinDataRate minRate, long count, IHttpOutputAborter outputAborter, CancellationToken cancellationToken) + public ValueTask FlushAsync(MinDataRate minRate, long count, IHttpOutputAborter outputAborter, CancellationToken cancellationToken) { var flushValueTask = _writer.FlushAsync(cancellationToken); @@ -60,7 +60,7 @@ public Task FlushAsync(MinDataRate minRate, long count, IHttpOutputAborter outpu if (flushValueTask.IsCompletedSuccessfully) { - return Task.CompletedTask; + return new ValueTask(flushValueTask.Result); } // https://github.com/dotnet/corefxlab/issues/1334 @@ -70,7 +70,7 @@ public Task FlushAsync(MinDataRate minRate, long count, IHttpOutputAborter outpu // we find previous flush Task which still accounts for any newly committed bytes and await that. lock (_flushLock) { - if (_lastFlushTask.IsCompleted) + if (_lastFlushTask == null || _lastFlushTask.IsCompleted) { _lastFlushTask = flushValueTask.AsTask(); } @@ -79,7 +79,7 @@ public Task FlushAsync(MinDataRate minRate, long count, IHttpOutputAborter outpu } } - private async Task TimeFlushAsync(MinDataRate minRate, long count, IHttpOutputAborter outputAborter, CancellationToken cancellationToken) + private async ValueTask TimeFlushAsync(MinDataRate minRate, long count, IHttpOutputAborter outputAborter, CancellationToken cancellationToken) { if (minRate != null) { @@ -88,7 +88,7 @@ private async Task TimeFlushAsync(MinDataRate minRate, long count, IHttpOutputAb try { - await _lastFlushTask; + return await _lastFlushTask; } catch (OperationCanceledException ex) when (outputAborter != null) { @@ -99,13 +99,17 @@ private async Task TimeFlushAsync(MinDataRate minRate, long count, IHttpOutputAb // A canceled token is the only reason flush should ever throw. _log.LogError(0, ex, $"Unexpected exception in {nameof(TimingPipeFlusher)}.{nameof(TimeFlushAsync)}."); } - - if (minRate != null) + finally { - _timeoutControl.StopTimingWrite(); + if (minRate != null) + { + _timeoutControl.StopTimingWrite(); + } + + cancellationToken.ThrowIfCancellationRequested(); } - cancellationToken.ThrowIfCancellationRequested(); + return default; } } } diff --git a/src/Servers/Kestrel/Core/test/HttpProtocolFeatureCollectionTests.cs b/src/Servers/Kestrel/Core/test/HttpProtocolFeatureCollectionTests.cs index 24cce16454d3..5021fe334c9a 100644 --- a/src/Servers/Kestrel/Core/test/HttpProtocolFeatureCollectionTests.cs +++ b/src/Servers/Kestrel/Core/test/HttpProtocolFeatureCollectionTests.cs @@ -117,6 +117,7 @@ public void FeaturesSetByTypeSameAsGeneric() { _collection[typeof(IHttpRequestFeature)] = CreateHttp1Connection(); _collection[typeof(IHttpResponseFeature)] = CreateHttp1Connection(); + _collection[typeof(IResponseBodyPipeFeature)] = CreateHttp1Connection(); _collection[typeof(IHttpRequestIdentifierFeature)] = CreateHttp1Connection(); _collection[typeof(IHttpRequestLifetimeFeature)] = CreateHttp1Connection(); _collection[typeof(IHttpConnectionFeature)] = CreateHttp1Connection(); @@ -136,6 +137,7 @@ public void FeaturesSetByGenericSameAsByType() { _collection.Set(CreateHttp1Connection()); _collection.Set(CreateHttp1Connection()); + _collection.Set(CreateHttp1Connection()); _collection.Set(CreateHttp1Connection()); _collection.Set(CreateHttp1Connection()); _collection.Set(CreateHttp1Connection()); @@ -173,6 +175,7 @@ private void CompareGenericGetterToIndexer() { Assert.Same(_collection.Get(), _collection[typeof(IHttpRequestFeature)]); Assert.Same(_collection.Get(), _collection[typeof(IHttpResponseFeature)]); + Assert.Same(_collection.Get(), _collection[typeof(IResponseBodyPipeFeature)]); Assert.Same(_collection.Get(), _collection[typeof(IHttpRequestIdentifierFeature)]); Assert.Same(_collection.Get(), _collection[typeof(IHttpRequestLifetimeFeature)]); Assert.Same(_collection.Get(), _collection[typeof(IHttpConnectionFeature)]); diff --git a/src/Servers/Kestrel/Core/test/HttpResponseStreamTests.cs b/src/Servers/Kestrel/Core/test/HttpResponseStreamTests.cs index 5e49e8a5a2c8..883faaf2288c 100644 --- a/src/Servers/Kestrel/Core/test/HttpResponseStreamTests.cs +++ b/src/Servers/Kestrel/Core/test/HttpResponseStreamTests.cs @@ -1,13 +1,13 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; using System.IO; +using System.IO.Pipelines; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; -using Microsoft.AspNetCore.Server.Kestrel.Core.Tests.TestHelpers; using Moq; using Xunit; @@ -18,88 +18,90 @@ public class HttpResponseStreamTests [Fact] public void CanReadReturnsFalse() { - var stream = new HttpResponseStream(Mock.Of(), new MockHttpResponseControl()); + var stream = CreateMockHttpResponseStream(); Assert.False(stream.CanRead); } [Fact] public void CanSeekReturnsFalse() { - var stream = new HttpResponseStream(Mock.Of(), new MockHttpResponseControl()); + var stream = CreateMockHttpResponseStream(); Assert.False(stream.CanSeek); } [Fact] public void CanWriteReturnsTrue() { - var stream = new HttpResponseStream(Mock.Of(), new MockHttpResponseControl()); + var stream = CreateMockHttpResponseStream(); Assert.True(stream.CanWrite); } [Fact] public void ReadThrows() { - var stream = new HttpResponseStream(Mock.Of(), new MockHttpResponseControl()); + var stream = CreateMockHttpResponseStream(); Assert.Throws(() => stream.Read(new byte[1], 0, 1)); } [Fact] public void ReadByteThrows() { - var stream = new HttpResponseStream(Mock.Of(), new MockHttpResponseControl()); + var stream = CreateMockHttpResponseStream(); Assert.Throws(() => stream.ReadByte()); } [Fact] public async Task ReadAsyncThrows() { - var stream = new HttpResponseStream(Mock.Of(), new MockHttpResponseControl()); + var stream = CreateMockHttpResponseStream(); await Assert.ThrowsAsync(() => stream.ReadAsync(new byte[1], 0, 1)); } [Fact] public void BeginReadThrows() { - var stream = new HttpResponseStream(Mock.Of(), new MockHttpResponseControl()); + var stream = CreateMockHttpResponseStream(); Assert.Throws(() => stream.BeginRead(new byte[1], 0, 1, null, null)); } [Fact] public void SeekThrows() { - var stream = new HttpResponseStream(Mock.Of(), new MockHttpResponseControl()); + var stream = CreateMockHttpResponseStream(); Assert.Throws(() => stream.Seek(0, SeekOrigin.Begin)); } [Fact] public void LengthThrows() { - var stream = new HttpResponseStream(Mock.Of(), new MockHttpResponseControl()); + var stream = CreateMockHttpResponseStream(); Assert.Throws(() => stream.Length); } [Fact] public void SetLengthThrows() { - var stream = new HttpResponseStream(Mock.Of(), new MockHttpResponseControl()); + var stream = CreateMockHttpResponseStream(); Assert.Throws(() => stream.SetLength(0)); } [Fact] public void PositionThrows() { - var stream = new HttpResponseStream(Mock.Of(), new MockHttpResponseControl()); + var stream = CreateMockHttpResponseStream(); Assert.Throws(() => stream.Position); Assert.Throws(() => stream.Position = 0); } [Fact] - public void StopAcceptingWritesCausesWriteToThrowObjectDisposedException() + public async Task StopAcceptingWritesCausesWriteToThrowObjectDisposedException() { - var stream = new HttpResponseStream(Mock.Of(), Mock.Of()); + var pipeWriter = new HttpResponsePipeWriter(Mock.Of()); + var stream = new HttpResponseStream(Mock.Of(), pipeWriter); stream.StartAcceptingWrites(); stream.StopAcceptingWrites(); - var ex = Assert.Throws(() => { stream.WriteAsync(new byte[1], 0, 1); }); + // This test had to change to awaiting the stream.WriteAsync + var ex = await Assert.ThrowsAsync(async () => { await stream.WriteAsync(new byte[1], 0, 1); }); Assert.Contains(CoreStrings.WritingToResponseBodyAfterResponseCompleted, ex.Message); } @@ -110,9 +112,10 @@ public async Task SynchronousWritesThrowIfDisallowedByIHttpBodyControlFeature() var mockBodyControl = new Mock(); mockBodyControl.Setup(m => m.AllowSynchronousIO).Returns(() => allowSynchronousIO); var mockHttpResponseControl = new Mock(); - mockHttpResponseControl.Setup(m => m.WriteAsync(It.IsAny>(), CancellationToken.None)).Returns(Task.CompletedTask); + mockHttpResponseControl.Setup(m => m.WritePipeAsync(It.IsAny>(), CancellationToken.None)).Returns(new ValueTask(new FlushResult())); - var stream = new HttpResponseStream(mockBodyControl.Object, mockHttpResponseControl.Object); + var pipeWriter = new HttpResponsePipeWriter(mockHttpResponseControl.Object); + var stream = new HttpResponseStream(mockBodyControl.Object, pipeWriter); stream.StartAcceptingWrites(); // WriteAsync doesn't throw. @@ -125,5 +128,11 @@ public async Task SynchronousWritesThrowIfDisallowedByIHttpBodyControlFeature() // If IHttpBodyControlFeature.AllowSynchronousIO is true, Write no longer throws. stream.Write(new byte[1], 0, 1); } + + private static HttpResponseStream CreateMockHttpResponseStream() + { + var pipeWriter = new HttpResponsePipeWriter(Mock.Of()); + return new HttpResponseStream(Mock.Of(), pipeWriter); + } } } diff --git a/src/Servers/Kestrel/Core/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests.csproj b/src/Servers/Kestrel/Core/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests.csproj index bcfc3e6f4e19..30f028bde32c 100644 --- a/src/Servers/Kestrel/Core/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests.csproj +++ b/src/Servers/Kestrel/Core/test/Microsoft.AspNetCore.Server.Kestrel.Core.Tests.csproj @@ -13,6 +13,10 @@ + + + + diff --git a/src/Servers/Kestrel/Core/test/StreamsTests.cs b/src/Servers/Kestrel/Core/test/StreamsTests.cs index 673b1586e1a1..a3c09848e338 100644 --- a/src/Servers/Kestrel/Core/test/StreamsTests.cs +++ b/src/Servers/Kestrel/Core/test/StreamsTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -16,7 +16,7 @@ public class StreamsTests [Fact] public async Task StreamsThrowAfterAbort() { - var streams = new Streams(Mock.Of(), Mock.Of()); + var streams = new Streams(Mock.Of(), new HttpResponsePipeWriter(Mock.Of())); var (request, response) = streams.Start(new MockMessageBody()); var ex = new Exception("My error"); @@ -30,7 +30,7 @@ public async Task StreamsThrowAfterAbort() [Fact] public async Task StreamsThrowOnAbortAfterUpgrade() { - var streams = new Streams(Mock.Of(), Mock.Of()); + var streams = new Streams(Mock.Of(), new HttpResponsePipeWriter(Mock.Of())); var (request, response) = streams.Start(new MockMessageBody(upgradeable: true)); var upgrade = streams.Upgrade(); @@ -52,7 +52,7 @@ public async Task StreamsThrowOnAbortAfterUpgrade() [Fact] public async Task StreamsThrowOnUpgradeAfterAbort() { - var streams = new Streams(Mock.Of(), Mock.Of()); + var streams = new Streams(Mock.Of(), new HttpResponsePipeWriter(Mock.Of())); var (request, response) = streams.Start(new MockMessageBody(upgradeable: true)); var ex = new Exception("My error"); diff --git a/src/Servers/Kestrel/Core/test/TestHelpers/MockHttpResponseControl.cs b/src/Servers/Kestrel/Core/test/TestHelpers/MockHttpResponseControl.cs deleted file mode 100644 index 738a07063525..000000000000 --- a/src/Servers/Kestrel/Core/test/TestHelpers/MockHttpResponseControl.cs +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; - -namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests.TestHelpers -{ - public class MockHttpResponseControl : IHttpResponseControl - { - public Task FlushAsync(CancellationToken cancellationToken) - { - return Task.CompletedTask; - } - - public void ProduceContinue() - { - } - - public Task WriteAsync(ReadOnlyMemory data, CancellationToken cancellationToken) - { - return Task.CompletedTask; - } - } -} diff --git a/src/Servers/Kestrel/shared/test/HttpResponseWritingExtensions.cs b/src/Servers/Kestrel/shared/test/HttpResponseWritingExtensions.cs new file mode 100644 index 000000000000..93cf4325b1d2 --- /dev/null +++ b/src/Servers/Kestrel/shared/test/HttpResponseWritingExtensions.cs @@ -0,0 +1,62 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Server.Kestrel.Tests +{ + public static class HttpResponseExtensions + { + public static Task WriteResponseBodyAsync(this HttpResponse response, byte[] buffer, int start, int length, bool isPipeTest = true, CancellationToken token = default) + { + if (isPipeTest) + { + return response.BodyPipe.WriteAsync(new ReadOnlyMemory(buffer, start, length), token).AsTask(); + } + else + { + return response.Body.WriteAsync(buffer, start, length, token); + } + } + + public static Task WriteResponseAsync(this HttpResponse response, string responseString, bool isPipeTest = true, CancellationToken token = default) + { + if (isPipeTest) + { + return response.BodyPipe.WriteAsync(Encoding.ASCII.GetBytes(responseString), token).AsTask(); + } + else + { + return response.WriteAsync(responseString, token); + } + } + + public static Task FlushResponseAsync(this HttpResponse response, bool isPipeTest = true, CancellationToken token = default) + { + if (isPipeTest) + { + return response.BodyPipe.FlushAsync(token).AsTask(); + } + else + { + return response.Body.FlushAsync(token); + } + } + + public static void WriteResponse(this HttpResponse response, byte[] buffer, int start, int length, bool isPipeTest = true) + { + if (isPipeTest) + { + response.BodyPipe.WriteAsync(new ReadOnlyMemory(buffer, start, length)).AsTask().GetAwaiter().GetResult(); + } + else + { + response.Body.Write(buffer, start, length); + } + } + } +} diff --git a/src/Servers/Kestrel/shared/test/TestApp.cs b/src/Servers/Kestrel/shared/test/TestApp.cs index c5ed58b44f76..c59b9308c394 100644 --- a/src/Servers/Kestrel/shared/test/TestApp.cs +++ b/src/Servers/Kestrel/shared/test/TestApp.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.IO; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; @@ -38,5 +39,39 @@ public static Task EmptyApp(HttpContext httpContext) { return Task.CompletedTask; } + + public static async Task EchoAppPipeWriter(HttpContext httpContext) + { + var request = httpContext.Request; + var response = httpContext.Response; + var buffer = new byte[httpContext.Request.ContentLength ?? 0]; + + if (buffer.Length > 0) + { + await request.Body.ReadUntilEndAsync(buffer).DefaultTimeout(); + await response.StartAsync(); + var memory = response.BodyPipe.GetMemory(buffer.Length); + buffer.CopyTo(memory); + response.BodyPipe.Advance(buffer.Length); + await response.BodyPipe.FlushAsync(); + } + } + + public static async Task EchoAppPipeWriterChunked(HttpContext httpContext) + { + var request = httpContext.Request; + var response = httpContext.Response; + var data = new MemoryStream(); + await request.Body.CopyToAsync(data); + var bytes = data.ToArray(); + + response.Headers["Content-Length"] = bytes.Length.ToString(); + await response.StartAsync(); + + var memory = response.BodyPipe.GetMemory(bytes.Length); + bytes.CopyTo(memory); + response.BodyPipe.Advance(bytes.Length); + await response.BodyPipe.FlushAsync(); + } } -} \ No newline at end of file +} diff --git a/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs index 9172a7350663..dede4ce9fd86 100644 --- a/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs @@ -20,6 +20,7 @@ using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; using Microsoft.AspNetCore.Server.Kestrel.Https; using Microsoft.AspNetCore.Server.Kestrel.Https.Internal; +using Microsoft.AspNetCore.Server.Kestrel.Tests; using Microsoft.AspNetCore.Testing; using Microsoft.AspNetCore.Testing.xunit; using Microsoft.Extensions.Logging; @@ -62,7 +63,7 @@ public async Task LargeDownload() for (int i = 0; i < 1024; i++) { - await context.Response.Body.WriteAsync(bytes, 0, bytes.Length); + await context.Response.BodyPipe.WriteAsync(new Memory(bytes, 0, bytes.Length)); } }); }); @@ -108,7 +109,7 @@ public async Task IgnoreNullHeaderValues(string headerName, StringValues headerV { context.Response.Headers.Add(headerName, headerValue); - await context.Response.WriteAsync(""); + await context.Response.WriteResponseAsync(""); }); }); @@ -152,7 +153,7 @@ public async Task WriteAfterConnectionCloseNoops(ListenOptions listenOptions) requestStarted.SetResult(null); await connectionClosed.Task.DefaultTimeout(); httpContext.Response.ContentLength = 12; - await httpContext.Response.WriteAsync("hello, world"); + await httpContext.Response.WriteResponseAsync("hello, world"); appCompleted.TrySetResult(null); } catch (Exception ex) @@ -208,7 +209,7 @@ public async Task ThrowsOnWriteWithRequestAbortedTokenAfterRequestIsAborted(List try { - await response.WriteAsync(largeString, lifetime.RequestAborted); + await response.WriteResponseAsync(largeString, token: lifetime.RequestAborted); } catch (Exception ex) { @@ -297,7 +298,7 @@ public async Task WritingToConnectionAfterUnobservedCloseTriggersRequestAbortedT { for (var i = 0; i < 1000; i++) { - await context.Response.Body.WriteAsync(scratchBuffer, 0, scratchBuffer.Length, context.RequestAborted); + await context.Response.BodyPipe.WriteAsync(new Memory(scratchBuffer, 0, scratchBuffer.Length), context.RequestAborted); await Task.Delay(10); } } @@ -499,7 +500,7 @@ async Task App(HttpContext context) { for (; i < chunks; i++) { - await context.Response.Body.WriteAsync(chunkData, 0, chunkData.Length, context.RequestAborted); + await context.Response.BodyPipe.WriteAsync(new Memory(chunkData, 0, chunkData.Length), context.RequestAborted); await Task.Yield(); } @@ -606,7 +607,7 @@ public async Task HttpsConnectionClosedWhenResponseDoesNotSatisfyMinimumDataRate { for (var i = 0; i < chunks; i++) { - await context.Response.Body.WriteAsync(chunkData, 0, chunkData.Length, context.RequestAborted); + await context.Response.BodyPipe.WriteAsync(new Memory(chunkData, 0, chunkData.Length), context.RequestAborted); } } catch (OperationCanceledException) @@ -770,7 +771,7 @@ async Task App(HttpContext context) for (var i = 0; i < chunkCount; i++) { - await context.Response.Body.WriteAsync(chunkData, 0, chunkData.Length, context.RequestAborted); + await context.Response.BodyPipe.WriteAsync(new Memory(chunkData, 0, chunkData.Length), context.RequestAborted); } appFuncCompleted.SetResult(null); @@ -847,7 +848,7 @@ async Task App(HttpContext context) context.Response.Headers[$"X-Custom-Header"] = headerStringValues; context.Response.ContentLength = 0; - await context.Response.Body.FlushAsync(); + await context.Response.BodyPipe.FlushAsync(); } using (var server = new TestServer(App, testContext, listenOptions)) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs index 9ef6ee5417e4..1cf062471612 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs @@ -23,8 +23,8 @@ public async Task ResponsesAreChunkedAutomatically() using (var server = new TestServer(async httpContext => { var response = httpContext.Response; - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello "), 0, 6); - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("World!"), 0, 6); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("Hello "), 0, 6)); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("World!"), 0, 6)); }, testContext)) { using (var connection = server.CreateConnection()) @@ -162,9 +162,9 @@ public async Task ZeroLengthWritesAreIgnored() using (var server = new TestServer(async httpContext => { var response = httpContext.Response; - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello "), 0, 6); - await response.Body.WriteAsync(new byte[0], 0, 0); - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("World!"), 0, 6); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("Hello "), 0, 6)); + await response.BodyPipe.WriteAsync(new Memory(new byte[0], 0, 0)); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("World!"), 0, 6)); }, testContext)) { using (var connection = server.CreateConnection()) @@ -244,7 +244,7 @@ public async Task EmptyResponseBodyHandledCorrectlyWithZeroLengthWrite() using (var server = new TestServer(async httpContext => { var response = httpContext.Response; - await response.Body.WriteAsync(new byte[0], 0, 0); + await response.BodyPipe.WriteAsync(new Memory(new byte[0], 0, 0)); }, testContext)) { using (var connection = server.CreateConnection()) @@ -275,7 +275,7 @@ public async Task ConnectionClosedIfExceptionThrownAfterWrite() using (var server = new TestServer(async httpContext => { var response = httpContext.Response; - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello World!"), 0, 12); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("Hello World!"), 0, 12)); throw new Exception(); }, testContext)) { @@ -309,7 +309,7 @@ public async Task ConnectionClosedIfExceptionThrownAfterZeroLengthWrite() using (var server = new TestServer(async httpContext => { var response = httpContext.Response; - await response.Body.WriteAsync(new byte[0], 0, 0); + await response.BodyPipe.WriteAsync(new Memory(new byte[0], 0, 0)); throw new Exception(); }, testContext)) { @@ -344,12 +344,12 @@ public async Task WritesAreFlushedPriorToResponseCompletion() using (var server = new TestServer(async httpContext => { var response = httpContext.Response; - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello "), 0, 6); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("Hello "), 0, 6)); // Don't complete response until client has received the first chunk. await flushWh.Task.DefaultTimeout(); - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("World!"), 0, 6); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("World!"), 0, 6)); }, testContext)) { using (var connection = server.CreateConnection()) @@ -391,9 +391,9 @@ public async Task ChunksCanBeWrittenManually() var response = httpContext.Response; response.Headers["Transfer-Encoding"] = "chunked"; - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("6\r\nHello \r\n"), 0, 11); - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("6\r\nWorld!\r\n"), 0, 11); - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("0\r\n\r\n"), 0, 5); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("6\r\nHello \r\n"), 0, 11)); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("6\r\nWorld!\r\n"), 0, 11)); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("0\r\n\r\n"), 0, 5)); }, testContext)) { using (var connection = server.CreateConnection()) @@ -420,6 +420,418 @@ await connection.Receive( await server.StopAsync(); } } + + [Fact] + public async Task ChunksWithGetMemoryBeforeFirstFlushStillFlushes() + { + var testContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(async httpContext => + { + var response = httpContext.Response; + await response.StartAsync(); + var memory = response.BodyPipe.GetMemory(); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("Hello "); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + + memory = response.BodyPipe.GetMemory(); + var secondPartOfResponse = Encoding.ASCII.GetBytes("World!"); + secondPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + + await response.BodyPipe.FlushAsync(); + }, 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}", + "Transfer-Encoding: chunked", + "", + "c", + "Hello World!", + "0", + "", + ""); + } + + await server.StopAsync(); + } + } + + [Fact] + public async Task ChunksWithGetMemoryLargeWriteBeforeFirstFlush() + { + var testContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(async httpContext => + { + var response = httpContext.Response; + await response.StartAsync(); + + var memory = response.BodyPipe.GetMemory(5000); // This will return 4089 + var fisrtPartOfResponse = Encoding.ASCII.GetBytes(new string('a', memory.Length)); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(memory.Length); + + memory = response.BodyPipe.GetMemory(); + var secondPartOfResponse = Encoding.ASCII.GetBytes("World!"); + secondPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + + await response.BodyPipe.FlushAsync(); + }, 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}", + "Transfer-Encoding: chunked", + "", + "ff9", + new string('a', 4089), + "6", + "World!", + "0", + "", + ""); + } + + await server.StopAsync(); + } + } + + [Fact] + public async Task ChunksWithGetMemoryWithInitialFlushWorks() + { + var testContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(async httpContext => + { + var response = httpContext.Response; + + await response.BodyPipe.FlushAsync(); + + var memory = response.BodyPipe.GetMemory(5000); // This will return 4089 + var fisrtPartOfResponse = Encoding.ASCII.GetBytes(new string('a', memory.Length)); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(memory.Length); + + memory = response.BodyPipe.GetMemory(); + var secondPartOfResponse = Encoding.ASCII.GetBytes("World!"); + secondPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + + await response.BodyPipe.FlushAsync(); + }, 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}", + "Transfer-Encoding: chunked", + "", + "ff9", + new string('a', 4089), + "6", + "World!", + "0", + "", + ""); + } + + await server.StopAsync(); + } + } + + [Fact] + public async Task ChunkGetMemoryMultipleAdvance() + { + var testContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(async httpContext => + { + var response = httpContext.Response; + + await response.StartAsync(); + + var memory = response.BodyPipe.GetMemory(4096); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("Hello "); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + + var secondPartOfResponse = Encoding.ASCII.GetBytes("World!"); + secondPartOfResponse.CopyTo(memory.Slice(6)); + response.BodyPipe.Advance(6); + }, 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}", + "Transfer-Encoding: chunked", + "", + "c", + "Hello World!", + "0", + "", + ""); + } + + await server.StopAsync(); + } + } + + [Fact] + public async Task ChunkGetSpanMultipleAdvance() + { + var testContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(async httpContext => + { + var response = httpContext.Response; + + // To avoid using span in an async method + void NonAsyncMethod() + { + var span = response.BodyPipe.GetSpan(4096); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("Hello "); + fisrtPartOfResponse.CopyTo(span); + response.BodyPipe.Advance(6); + + var secondPartOfResponse = Encoding.ASCII.GetBytes("World!"); + secondPartOfResponse.CopyTo(span.Slice(6)); + response.BodyPipe.Advance(6); + } + + await response.StartAsync(); + + NonAsyncMethod(); + }, 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}", + "Transfer-Encoding: chunked", + "", + "c", + "Hello World!", + "0", + "", + ""); + } + + await server.StopAsync(); + } + } + + [Fact] + public async Task ChunkGetMemoryAndWrite() + { + var testContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(async httpContext => + { + var response = httpContext.Response; + + await response.StartAsync(); + + var memory = response.BodyPipe.GetMemory(4096); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("Hello "); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + + await response.WriteAsync("World!"); + }, 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}", + "Transfer-Encoding: chunked", + "", + "6", + "Hello ", + "6", + "World!", + "0", + "", + ""); + } + + await server.StopAsync(); + } + } + + [Fact] + public async Task GetMemoryWithSizeHint() + { + var testContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(async httpContext => + { + var response = httpContext.Response; + + await response.StartAsync(); + + var memory = response.BodyPipe.GetMemory(0); + + // Headers are already written to memory, sliced appropriately + Assert.Equal(4005, memory.Length); + + memory = response.BodyPipe.GetMemory(1000000); + Assert.Equal(4005, memory.Length); + }, 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}", + "Transfer-Encoding: chunked", + "", + "0", + "", + ""); + } + + await server.StopAsync(); + } + } + + [Theory] + [InlineData(15)] + [InlineData(255)] + public async Task ChunkGetMemoryWithSmallerSizesWork(int writeSize) + { + var testContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(async httpContext => + { + var response = httpContext.Response; + + await response.StartAsync(); + + var memory = response.BodyPipe.GetMemory(4096); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes(new string('a', writeSize)); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(writeSize); + }, 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}", + "Transfer-Encoding: chunked", + "", + writeSize.ToString("X").ToLower(), + new string('a', writeSize), + "0", + "", + ""); + } + + await server.StopAsync(); + } + } + + [Fact] + public async Task ChunkedWithBothPipeAndStreamWorks() + { + using (var server = new TestServer(async httpContext => + { + var response = httpContext.Response; + await response.StartAsync(); + var memory = response.BodyPipe.GetMemory(4096); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("hello,"); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + var secondPartOfResponse = Encoding.ASCII.GetBytes(" world"); + secondPartOfResponse.CopyTo(memory.Slice(6)); + response.BodyPipe.Advance(6); + + await response.Body.WriteAsync(Encoding.ASCII.GetBytes("hello, world")); + await response.BodyPipe.WriteAsync(Encoding.ASCII.GetBytes("hello, world")); + await response.WriteAsync("hello, world"); + + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Transfer-Encoding: chunked", + "", + "c", + "hello, world", + "c", + "hello, world", + "c", + "hello, world", + "c", + "hello, world", + "0", + "", + ""); + } + await server.StopAsync(); + } + } } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ConnectionAdapterTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ConnectionAdapterTests.cs index 9d426285f811..017e175fa6ae 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ConnectionAdapterTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ConnectionAdapterTests.cs @@ -4,6 +4,7 @@ using System; using System.IO; using System.Net; +using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; @@ -18,8 +19,17 @@ namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests { public class ConnectionAdapterTests : TestApplicationErrorLoggerLoggedTest { - [Fact] - public async Task CanReadAndWriteWithRewritingConnectionAdapter() + + public static TheoryData EchoAppRequestDelegates => + new TheoryData + { + { TestApp.EchoApp }, + { TestApp.EchoAppPipeWriter } + }; + + [Theory] + [MemberData(nameof(EchoAppRequestDelegates))] + public async Task CanReadAndWriteWithRewritingConnectionAdapter(RequestDelegate requestDelegate) { var adapter = new RewritingConnectionAdapter(); var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) @@ -31,7 +41,7 @@ public async Task CanReadAndWriteWithRewritingConnectionAdapter() var sendString = "POST / HTTP/1.0\r\nContent-Length: 12\r\n\r\nHello World?"; - using (var server = new TestServer(TestApp.EchoApp, serviceContext, listenOptions)) + using (var server = new TestServer(requestDelegate, serviceContext, listenOptions)) { using (var connection = server.CreateConnection()) { @@ -50,8 +60,9 @@ await connection.ReceiveEnd( Assert.Equal(sendString.Length, adapter.BytesRead); } - [Fact] - public async Task CanReadAndWriteWithAsyncConnectionAdapter() + [Theory] + [MemberData(nameof(EchoAppRequestDelegates))] + public async Task CanReadAndWriteWithAsyncConnectionAdapter(RequestDelegate requestDelegate) { var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) { @@ -60,7 +71,7 @@ public async Task CanReadAndWriteWithAsyncConnectionAdapter() var serviceContext = new TestServiceContext(LoggerFactory); - using (var server = new TestServer(TestApp.EchoApp, serviceContext, listenOptions)) + using (var server = new TestServer(requestDelegate, serviceContext, listenOptions)) { using (var connection = server.CreateConnection()) { @@ -80,8 +91,9 @@ await connection.ReceiveEnd( } } - [Fact] - public async Task ImmediateFinAfterOnConnectionAsyncClosesGracefully() + [Theory] + [MemberData(nameof(EchoAppRequestDelegates))] + public async Task ImmediateFinAfterOnConnectionAsyncClosesGracefully(RequestDelegate requestDelegate) { var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) { @@ -90,7 +102,7 @@ public async Task ImmediateFinAfterOnConnectionAsyncClosesGracefully() var serviceContext = new TestServiceContext(LoggerFactory); - using (var server = new TestServer(TestApp.EchoApp, serviceContext, listenOptions)) + using (var server = new TestServer(requestDelegate, serviceContext, listenOptions)) { using (var connection = server.CreateConnection()) { @@ -102,8 +114,9 @@ public async Task ImmediateFinAfterOnConnectionAsyncClosesGracefully() } } - [Fact] - public async Task ImmediateFinAfterThrowingClosesGracefully() + [Theory] + [MemberData(nameof(EchoAppRequestDelegates))] + public async Task ImmediateFinAfterThrowingClosesGracefully(RequestDelegate requestDelegate) { var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) { @@ -112,7 +125,7 @@ public async Task ImmediateFinAfterThrowingClosesGracefully() var serviceContext = new TestServiceContext(LoggerFactory); - using (var server = new TestServer(TestApp.EchoApp, serviceContext, listenOptions)) + using (var server = new TestServer(requestDelegate, serviceContext, listenOptions)) { using (var connection = server.CreateConnection()) { @@ -124,9 +137,10 @@ public async Task ImmediateFinAfterThrowingClosesGracefully() } } - [Fact] + [Theory] [CollectDump] - public async Task ImmediateShutdownAfterOnConnectionAsyncDoesNotCrash() + [MemberData(nameof(EchoAppRequestDelegates))] + public async Task ImmediateShutdownAfterOnConnectionAsyncDoesNotCrash(RequestDelegate requestDelegate) { var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) { @@ -136,7 +150,7 @@ public async Task ImmediateShutdownAfterOnConnectionAsyncDoesNotCrash() var serviceContext = new TestServiceContext(LoggerFactory); var stopTask = Task.CompletedTask; - using (var server = new TestServer(TestApp.EchoApp, serviceContext, listenOptions)) + using (var server = new TestServer(requestDelegate, serviceContext, listenOptions)) using (var shutdownCts = new CancellationTokenSource(TimeSpan.FromSeconds(5))) { using (var connection = server.CreateConnection()) @@ -180,8 +194,9 @@ public async Task ImmediateShutdownDuringOnConnectionAsyncDoesNotCrash() } } - [Fact] - public async Task ThrowingSynchronousConnectionAdapterDoesNotCrashServer() + [Theory] + [MemberData(nameof(EchoAppRequestDelegates))] + public async Task ThrowingSynchronousConnectionAdapterDoesNotCrashServer(RequestDelegate requestDelegate) { var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) { @@ -190,7 +205,7 @@ public async Task ThrowingSynchronousConnectionAdapterDoesNotCrashServer() var serviceContext = new TestServiceContext(LoggerFactory); - using (var server = new TestServer(TestApp.EchoApp, serviceContext, listenOptions)) + using (var server = new TestServer(requestDelegate, serviceContext, listenOptions)) { using (var connection = server.CreateConnection()) { @@ -241,6 +256,40 @@ await connection.ReceiveEnd( } } + [Fact] + public async Task CanFlushAsyncWithConnectionAdapterPipeWriter() + { + var listenOptions = new ListenOptions(new IPEndPoint(IPAddress.Loopback, 0)) + { + ConnectionAdapters = { new PassThroughConnectionAdapter() } + }; + + var serviceContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(async context => + { + await context.Response.BodyPipe.WriteAsync(Encoding.ASCII.GetBytes("Hello ")); + await context.Response.BodyPipe.FlushAsync(); + await context.Response.BodyPipe.WriteAsync(Encoding.ASCII.GetBytes("World!")); + }, serviceContext, listenOptions)) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.0", + "", + ""); + await connection.ReceiveEnd( + "HTTP/1.1 200 OK", + "Connection: close", + $"Date: {serviceContext.DateHeaderValue}", + "", + "Hello World!"); + } + await server.StopAsync(); + } + } + private class RewritingConnectionAdapter : IConnectionAdapter { private RewritingStream _rewritingStream; diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs index bc898995c04c..85303ba64943 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -7,6 +7,7 @@ using System.IO; using System.Linq; using System.Runtime.ExceptionServices; +using System.Text; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; @@ -228,7 +229,7 @@ await ExpectAsync(Http2FrameType.DATA, } [Theory] - [InlineData("/","/")] + [InlineData("/", "/")] [InlineData("/a%5E", "/a^")] [InlineData("/a%E2%82%AC", "/a€")] [InlineData("/a%2Fb", "/a%2Fb")] // Forward slash, not decoded @@ -2468,5 +2469,606 @@ await ExpectAsync(Http2FrameType.SETTINGS, Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); } + + [Fact] + public async Task GetMemoryAdvance_Works() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async httpContext => + { + var response = httpContext.Response; + await response.StartAsync(); + var memory = response.BodyPipe.GetMemory(); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("hello,"); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + + memory = response.BodyPipe.GetMemory(); + var secondPartOfResponse = Encoding.ASCII.GetBytes(" world"); + secondPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + var dataFrame = await ExpectAsync(Http2FrameType.DATA, + withLength: 12, + 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]); + + Assert.True(_helloWorldBytes.AsSpan().SequenceEqual(dataFrame.PayloadSequence.ToArray())); + } + + [Fact] + public async Task WriteAsync_GetMemoryLargeWriteBeforeFirstFlush() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async httpContext => + { + var response = httpContext.Response; + await response.StartAsync(); + + var memory = response.BodyPipe.GetMemory(5000); // This will return 4096 + var fisrtPartOfResponse = Encoding.ASCII.GetBytes(new string('a', memory.Length)); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(memory.Length); + + memory = response.BodyPipe.GetMemory(); + var secondPartOfResponse = Encoding.ASCII.GetBytes("aaaaaa"); + secondPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + + await response.BodyPipe.FlushAsync(); + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + var dataFrame = await ExpectAsync(Http2FrameType.DATA, + withLength: 4102, + 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]); + Assert.Equal(Encoding.ASCII.GetBytes(new string('a', 4102)), dataFrame.PayloadSequence.ToArray()); + } + + [Fact] + public async Task WriteAsync_WithGetMemoryWithInitialFlushWorks() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async httpContext => + { + var response = httpContext.Response; + + await response.BodyPipe.FlushAsync(); + + var memory = response.BodyPipe.GetMemory(5000); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes(new string('a', memory.Length)); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(memory.Length); + + memory = response.BodyPipe.GetMemory(); + var secondPartOfResponse = Encoding.ASCII.GetBytes("aaaaaa"); + secondPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + + await response.BodyPipe.FlushAsync(); + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + var dataFrame = await ExpectAsync(Http2FrameType.DATA, + withLength: 4102, + 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]); + Assert.Equal(Encoding.ASCII.GetBytes(new string('a', 4102)), dataFrame.PayloadSequence.ToArray()); + } + + [Fact] + public async Task WriteAsync_GetMemoryMultipleAdvance() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async httpContext => + { + var response = httpContext.Response; + + await response.BodyPipe.FlushAsync(); + + var memory = response.BodyPipe.GetMemory(4096); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("hello,"); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + + var secondPartOfResponse = Encoding.ASCII.GetBytes(" world"); + secondPartOfResponse.CopyTo(memory.Slice(6)); + response.BodyPipe.Advance(6); + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + var dataFrame = await ExpectAsync(Http2FrameType.DATA, + withLength: 12, + 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]); + Assert.True(_helloWorldBytes.AsSpan().SequenceEqual(dataFrame.PayloadSequence.ToArray())); + } + + [Fact] + public async Task WriteAsync_GetSpanMultipleAdvance() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async httpContext => + { + var response = httpContext.Response; + + await response.BodyPipe.FlushAsync(); + + void NonAsyncMethod() + { + var span = response.BodyPipe.GetSpan(); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("hello,"); + fisrtPartOfResponse.CopyTo(span); + response.BodyPipe.Advance(6); + + var secondPartOfResponse = Encoding.ASCII.GetBytes(" world"); + secondPartOfResponse.CopyTo(span.Slice(6)); + response.BodyPipe.Advance(6); + } + NonAsyncMethod(); + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + var dataFrame = await ExpectAsync(Http2FrameType.DATA, + withLength: 12, + 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]); + Assert.True(_helloWorldBytes.AsSpan().SequenceEqual(dataFrame.PayloadSequence.ToArray())); + } + + [Fact] + public async Task WriteAsync_GetMemoryAndWrite() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async httpContext => + { + var response = httpContext.Response; + + await response.StartAsync(); + + var memory = response.BodyPipe.GetMemory(4096); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("hello,"); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + + await response.WriteAsync(" world"); + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + var dataFrame = await ExpectAsync(Http2FrameType.DATA, + withLength: 12, + 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]); + Assert.True(_helloWorldBytes.AsSpan().SequenceEqual(dataFrame.PayloadSequence.ToArray())); + } + + [Fact] + public async Task WriteAsync_GetMemoryWithSizeHintAlwaysReturnsSameSize() + { + var headers = new[] +{ + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async httpContext => + { + var response = httpContext.Response; + + await response.StartAsync(); + + var memory = response.BodyPipe.GetMemory(0); + Assert.Equal(4096, memory.Length); + + memory = response.BodyPipe.GetMemory(1000000); + Assert.Equal(4096, memory.Length); + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + var dataFrame = 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]); + } + + [Fact] + public async Task WriteAsync_BothPipeAndStreamWorks() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async httpContext => + { + var response = httpContext.Response; + await response.StartAsync(); + var memory = response.BodyPipe.GetMemory(4096); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("hello,"); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + var secondPartOfResponse = Encoding.ASCII.GetBytes(" world"); + secondPartOfResponse.CopyTo(memory.Slice(6)); + response.BodyPipe.Advance(6); + + await response.BodyPipe.FlushAsync(); + + await response.Body.WriteAsync(Encoding.ASCII.GetBytes("hello, world")); + await response.BodyPipe.WriteAsync(Encoding.ASCII.GetBytes("hello, world")); + await response.WriteAsync("hello, world"); + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 37, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 12, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 12, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 12, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 12, + 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]); + } + + [Fact] + public async Task ContentLengthWithGetSpanWorks() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async httpContext => + { + var response = httpContext.Response; + response.ContentLength = 12; + await response.StartAsync(); + + void NonAsyncMethod() + { + var span = response.BodyPipe.GetSpan(4096); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("hello,"); + fisrtPartOfResponse.CopyTo(span); + response.BodyPipe.Advance(6); + + var secondPartOfResponse = Encoding.ASCII.GetBytes(" world"); + secondPartOfResponse.CopyTo(span.Slice(6)); + response.BodyPipe.Advance(6); + } + + NonAsyncMethod(); + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 56, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 12, + withFlags: (byte)Http2DataFrameFlags.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("12", _decodedHeaders[HeaderNames.ContentLength]); + } + + [Fact] + public async Task ContentLengthWithGetMemoryWorks() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async httpContext => + { + var response = httpContext.Response; + response.ContentLength = 12; + await response.StartAsync(); + + var memory = response.BodyPipe.GetMemory(4096); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("Hello "); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + + var secondPartOfResponse = Encoding.ASCII.GetBytes("World!"); + secondPartOfResponse.CopyTo(memory.Slice(6)); + response.BodyPipe.Advance(6); + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 56, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 12, + withFlags: (byte)Http2DataFrameFlags.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("12", _decodedHeaders[HeaderNames.ContentLength]); + } + + [Fact] + public async Task ResponseBodyCanWrite() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async httpContext => + { + httpContext.Response.ContentLength = 12; + await httpContext.Response.Body.WriteAsync(Encoding.ASCII.GetBytes("hello, world")); + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 56, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 12, + 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(3, _decodedHeaders.Count); + Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); + Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); + Assert.Equal("12", _decodedHeaders[HeaderNames.ContentLength]); + } + + [Fact] + public async Task ResponseBodyAndResponsePipeWorks() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async httpContext => + { + var response = httpContext.Response; + response.ContentLength = 54; + await response.StartAsync(); + var memory = response.BodyPipe.GetMemory(4096); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("hello,"); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + var secondPartOfResponse = Encoding.ASCII.GetBytes(" world\r\n"); + secondPartOfResponse.CopyTo(memory.Slice(6)); + response.BodyPipe.Advance(8); + await response.BodyPipe.FlushAsync(); + await response.Body.WriteAsync(Encoding.ASCII.GetBytes("hello, world\r\n")); + await response.BodyPipe.WriteAsync(Encoding.ASCII.GetBytes("hello, world\r\n")); + await response.WriteAsync("hello, world"); + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 56, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 14, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 14, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 14, + withFlags: (byte)Http2DataFrameFlags.NONE, + withStreamId: 1); + await ExpectAsync(Http2FrameType.DATA, + withLength: 12, + 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(3, _decodedHeaders.Count); + Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); + Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); + Assert.Equal("54", _decodedHeaders[HeaderNames.ContentLength]); + } } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs index 05b54a8d12fd..be7d6a3ad664 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TestBase.cs @@ -177,7 +177,7 @@ public Http2TestBase() _mockConnectionContext.Setup(c => c.Abort(It.IsAny())).Callback(ex => { // Emulate transport abort so the _connectionTask completes. - _pair.Application.Output.Complete(ex); + Task.Run(() => _pair.Application.Output.Complete(ex)); }); _noopApplication = context => Task.CompletedTask; @@ -380,7 +380,7 @@ public override void Initialize(MethodInfo methodInfo, object[] testMethodArgume _serviceContext = new TestServiceContext(LoggerFactory, _mockKestrelTrace.Object) { - Scheduler = PipeScheduler.Inline + Scheduler = PipeScheduler.Inline, }; } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/InMemory.FunctionalTests.csproj b/src/Servers/Kestrel/test/InMemory.FunctionalTests/InMemory.FunctionalTests.csproj index c2046f8cd88a..74d7eb775c28 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/InMemory.FunctionalTests.csproj +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/InMemory.FunctionalTests.csproj @@ -14,6 +14,10 @@ + + + + diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs index 7ced52b531cc..514655f6ce04 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs @@ -77,7 +77,7 @@ public async Task OnStartingThrowsWhenSetAfterResponseHasAlreadyStarted() using (var server = new TestServer(async context => { await context.Response.WriteAsync("hello, world"); - await context.Response.Body.FlushAsync(); + await context.Response.BodyPipe.FlushAsync(); ex = Assert.Throws(() => context.Response.OnStarting(_ => Task.CompletedTask, null)); }, new TestServiceContext(LoggerFactory))) { @@ -155,14 +155,14 @@ public async Task ResponseBodyWriteAsyncCanBeCancelled() var data = new byte[1024 * 1024 * 10]; var timerTask = Task.Delay(TimeSpan.FromSeconds(1)); - var writeTask = context.Response.Body.WriteAsync(data, 0, data.Length, cts.Token).DefaultTimeout(); + var writeTask = context.Response.BodyPipe.WriteAsync(new Memory(data, 0, data.Length), cts.Token).AsTask().DefaultTimeout(); var completedTask = await Task.WhenAny(writeTask, timerTask); while (completedTask == writeTask) { await writeTask; timerTask = Task.Delay(TimeSpan.FromSeconds(1)); - writeTask = context.Response.Body.WriteAsync(data, 0, data.Length, cts.Token).DefaultTimeout(); + writeTask = context.Response.BodyPipe.WriteAsync(new Memory(data, 0, data.Length), cts.Token).AsTask().DefaultTimeout(); completedTask = await Task.WhenAny(writeTask, timerTask); } @@ -660,7 +660,7 @@ public async Task ResponseBodyNotWrittenOnHeadResponseAndLoggedOnlyOnce() using (var server = new TestServer(async httpContext => { await httpContext.Response.WriteAsync(response); - await httpContext.Response.Body.FlushAsync(); + await httpContext.Response.BodyPipe.FlushAsync(); }, new TestServiceContext(LoggerFactory, mockKestrelTrace.Object))) { using (var connection = server.CreateConnection()) @@ -696,12 +696,11 @@ public async Task ThrowsAndClosesConnectionWhenAppWritesMoreThanContentLengthWri ServerOptions = { AllowSynchronousIO = true } }; - using (var server = new TestServer(httpContext => + using (var server = new TestServer(async httpContext => { httpContext.Response.ContentLength = 11; - httpContext.Response.Body.Write(Encoding.ASCII.GetBytes("hello,"), 0, 6); - httpContext.Response.Body.Write(Encoding.ASCII.GetBytes(" world"), 0, 6); - return Task.CompletedTask; + await httpContext.Response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("hello,"), 0, 6)); + await httpContext.Response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes(" world"), 0, 6)); }, serviceContext)) { using (var connection = server.CreateConnection()) @@ -774,12 +773,11 @@ public async Task InternalServerErrorAndConnectionClosedOnWriteWithMoreThanConte ServerOptions = { AllowSynchronousIO = true } }; - using (var server = new TestServer(httpContext => + using (var server = new TestServer(async httpContext => { var response = Encoding.ASCII.GetBytes("hello, world"); httpContext.Response.ContentLength = 5; - httpContext.Response.Body.Write(response, 0, response.Length); - return Task.CompletedTask; + await httpContext.Response.BodyPipe.WriteAsync(new Memory(response, 0, response.Length)); }, serviceContext)) { using (var connection = server.CreateConnection()) @@ -811,11 +809,11 @@ public async Task InternalServerErrorAndConnectionClosedOnWriteAsyncWithMoreThan { var serviceContext = new TestServiceContext(LoggerFactory); - using (var server = new TestServer(httpContext => + using (var server = new TestServer(async httpContext => { var response = Encoding.ASCII.GetBytes("hello, world"); httpContext.Response.ContentLength = 5; - return httpContext.Response.Body.WriteAsync(response, 0, response.Length); + await httpContext.Response.BodyPipe.WriteAsync(new Memory(response, 0, response.Length)); }, serviceContext)) { using (var connection = server.CreateConnection()) @@ -986,8 +984,8 @@ await connection.Receive( } [Theory] - [InlineData(false)] [InlineData(true)] + [InlineData(false)] public async Task WhenAppSetsContentLengthToZeroAndDoesNotWriteNoErrorIsThrown(bool flushResponse) { var serviceContext = new TestServiceContext(LoggerFactory); @@ -998,7 +996,7 @@ public async Task WhenAppSetsContentLengthToZeroAndDoesNotWriteNoErrorIsThrown(b if (flushResponse) { - await httpContext.Response.Body.FlushAsync(); + await httpContext.Response.BodyPipe.FlushAsync(); } }, serviceContext)) { @@ -1165,7 +1163,7 @@ public async Task HeadResponseBodyNotWrittenWithSyncWrite() using (var server = new TestServer(async httpContext => { httpContext.Response.ContentLength = 12; - httpContext.Response.Body.Write(Encoding.ASCII.GetBytes("hello, world"), 0, 12); + await httpContext.Response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("hello, world"), 0, 12)); await flushed.Task; }, serviceContext)) { @@ -1408,7 +1406,7 @@ public async Task FirstWriteVerifiedAfterOnStarting() { var serviceContext = new TestServiceContext(LoggerFactory) { ServerOptions = { AllowSynchronousIO = true } }; - using (var server = new TestServer(httpContext => + using (var server = new TestServer(async httpContext => { httpContext.Response.OnStarting(() => { @@ -1421,8 +1419,7 @@ public async Task FirstWriteVerifiedAfterOnStarting() httpContext.Response.ContentLength = response.Length - 1; // If OnStarting is not run before verifying writes, an error response will be sent. - httpContext.Response.Body.Write(response, 0, response.Length); - return Task.CompletedTask; + await httpContext.Response.BodyPipe.WriteAsync(new Memory(response, 0, response.Length)); }, serviceContext)) { using (var connection = server.CreateConnection()) @@ -1452,7 +1449,7 @@ public async Task SubsequentWriteVerifiedAfterOnStarting() { var serviceContext = new TestServiceContext(LoggerFactory) { ServerOptions = { AllowSynchronousIO = true } }; - using (var server = new TestServer(httpContext => + using (var server = new TestServer(async httpContext => { httpContext.Response.OnStarting(() => { @@ -1465,9 +1462,8 @@ public async Task SubsequentWriteVerifiedAfterOnStarting() httpContext.Response.ContentLength = response.Length - 1; // If OnStarting is not run before verifying writes, an error response will be sent. - httpContext.Response.Body.Write(response, 0, response.Length / 2); - httpContext.Response.Body.Write(response, response.Length / 2, response.Length - response.Length / 2); - return Task.CompletedTask; + await httpContext.Response.BodyPipe.WriteAsync(new Memory(response, 0, response.Length / 2)); + await httpContext.Response.BodyPipe.WriteAsync(new Memory(response, response.Length / 2, response.Length - response.Length / 2)); }, serviceContext)) { using (var connection = server.CreateConnection()) @@ -1510,7 +1506,7 @@ public async Task FirstWriteAsyncVerifiedAfterOnStarting() httpContext.Response.ContentLength = response.Length - 1; // If OnStarting is not run before verifying writes, an error response will be sent. - return httpContext.Response.Body.WriteAsync(response, 0, response.Length); + return httpContext.Response.BodyPipe.WriteAsync(new Memory(response, 0, response.Length)).AsTask(); }, new TestServiceContext(LoggerFactory))) { using (var connection = server.CreateConnection()) @@ -1551,8 +1547,8 @@ public async Task SubsequentWriteAsyncVerifiedAfterOnStarting() httpContext.Response.ContentLength = response.Length - 1; // If OnStarting is not run before verifying writes, an error response will be sent. - await httpContext.Response.Body.WriteAsync(response, 0, response.Length / 2); - await httpContext.Response.Body.WriteAsync(response, response.Length / 2, response.Length - response.Length / 2); + await httpContext.Response.BodyPipe.WriteAsync(new Memory(response, 0, response.Length / 2)); + await httpContext.Response.BodyPipe.WriteAsync(new Memory(response, response.Length / 2, response.Length - response.Length / 2)); }, new TestServiceContext(LoggerFactory))) { using (var connection = server.CreateConnection()) @@ -2112,7 +2108,7 @@ public async Task ThrowingInOnStartingResultsInFailedWritesAnd500Response() throw onStartingException; }, null); - var writeException = await Assert.ThrowsAsync(async () => await response.Body.FlushAsync()); + var writeException = await Assert.ThrowsAsync(async () => await response.BodyPipe.FlushAsync()); Assert.Same(onStartingException, writeException.InnerException); }, testContext)) { @@ -2171,7 +2167,7 @@ public async Task OnStartingThrowsInsideOnStartingCallbacksRuns() response.Headers["Content-Length"] = new[] { "11" }; - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello World"), 0, 11); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("Hello World"), 0, 11)); }, testContext)) { using (var connection = server.CreateConnection()) @@ -2218,7 +2214,7 @@ public async Task OnCompletedThrowsInsideOnCompletedCallbackRuns() response.Headers["Content-Length"] = new[] { "11" }; - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello World"), 0, 11); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("Hello World"), 0, 11)); }, testContext)) { using (var connection = server.CreateConnection()) @@ -2264,7 +2260,7 @@ public async Task ThrowingInOnCompletedIsLogged() response.Headers["Content-Length"] = new[] { "11" }; - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello World"), 0, 11); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("Hello World"), 0, 11)); }, testContext)) { using (var connection = server.CreateConnection()) @@ -2307,7 +2303,7 @@ public async Task ThrowingAfterWritingKillsConnection() }, null); response.Headers["Content-Length"] = new[] { "11" }; - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello World"), 0, 11); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("Hello World"), 0, 11)); throw new Exception(); }, testContext)) { @@ -2349,7 +2345,7 @@ public async Task ThrowingAfterPartialWriteKillsConnection() }, null); response.Headers["Content-Length"] = new[] { "11" }; - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello"), 0, 5); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("Hello"), 0, 5)); throw new Exception(); }, testContext)) { @@ -2384,7 +2380,7 @@ public async Task NoErrorsLoggedWhenServerEndsConnectionBeforeClient() { var response = httpContext.Response; response.Headers["Content-Length"] = new[] { "11" }; - await response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello World"), 0, 11); + await response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("Hello World"), 0, 11)); }, testContext)) { using (var connection = server.CreateConnection()) @@ -2829,7 +2825,7 @@ public async Task StartAsyncAndFlushWorks() using (var server = new TestServer(async httpContext => { await httpContext.Response.StartAsync(); - await httpContext.Response.Body.FlushAsync(); + await httpContext.Response.BodyPipe.FlushAsync(); Assert.True(httpContext.Response.HasStarted); }, testContext)) { @@ -2983,7 +2979,7 @@ public async Task SynchronousWritesAllowedByDefault() var ioEx = Assert.Throws(() => context.Response.Body.Write(Encoding.ASCII.GetBytes("What!?"), 0, 6)); Assert.Equal(CoreStrings.SynchronousWritesDisallowed, ioEx.Message); - await context.Response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello2"), 0, 6); + await context.Response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("Hello2"), 0, 6)); } }, new TestServiceContext(LoggerFactory))) { @@ -3028,7 +3024,7 @@ public async Task SynchronousWritesCanBeDisallowedGlobally() var ioEx = Assert.Throws(() => context.Response.Body.Write(Encoding.ASCII.GetBytes("What!?"), 0, 6)); Assert.Equal(CoreStrings.SynchronousWritesDisallowed, ioEx.Message); - return context.Response.Body.WriteAsync(Encoding.ASCII.GetBytes("Hello!"), 0, 6); + return context.Response.BodyPipe.WriteAsync(new Memory(Encoding.ASCII.GetBytes("Hello!"), 0, 6)).AsTask(); }, testContext)) { using (var connection = server.CreateConnection()) @@ -3079,6 +3075,262 @@ await connection.Receive( } } + [Fact] + public async Task AdvanceNegativeValueThrowsArgumentOutOfRangeException() + { + var testContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(async httpContext => + { + var response = httpContext.Response; + + await response.StartAsync(); + + Assert.Throws(() => response.BodyPipe.Advance(-1)); + }, 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}", + "Transfer-Encoding: chunked", + "", + "0", + "", + ""); + } + + await server.StopAsync(); + } + } + + [Fact] + public async Task AdvanceWithTooLargeOfAValueThrowInvalidOperationException() + { + var testContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(async httpContext => + { + var response = httpContext.Response; + + await response.StartAsync(); + + Assert.Throws(() => response.BodyPipe.Advance(1)); + }, 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}", + "Transfer-Encoding: chunked", + "", + "0", + "", + ""); + } + + await server.StopAsync(); + } + } + + [Fact] + public async Task GetMemoryBeforeStartAsyncThrows() + { + var testContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(httpContext => + { + Assert.Throws(() => httpContext.Response.BodyPipe.GetMemory()); + 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: 0", + "", + ""); + } + + await server.StopAsync(); + } + } + + [Fact] + public async Task ContentLengthWithGetSpanWorks() + { + var testContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(async httpContext => + { + var response = httpContext.Response; + response.ContentLength = 12; + await response.StartAsync(); + + void NonAsyncMethod() + { + var span = response.BodyPipe.GetSpan(4096); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("Hello "); + fisrtPartOfResponse.CopyTo(span); + response.BodyPipe.Advance(6); + + var secondPartOfResponse = Encoding.ASCII.GetBytes("World!"); + secondPartOfResponse.CopyTo(span.Slice(6)); + response.BodyPipe.Advance(6); + } + + NonAsyncMethod(); + + }, 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: 12", + "", + "Hello World!"); + } + + await server.StopAsync(); + } + } + + [Fact] + public async Task ContentLengthWithGetMemoryWorks() + { + var testContext = new TestServiceContext(LoggerFactory); + + using (var server = new TestServer(async httpContext => + { + var response = httpContext.Response; + response.ContentLength = 12; + await response.StartAsync(); + + var memory = response.BodyPipe.GetMemory(4096); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("Hello "); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + + var secondPartOfResponse = Encoding.ASCII.GetBytes("World!"); + secondPartOfResponse.CopyTo(memory.Slice(6)); + response.BodyPipe.Advance(6); + }, 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: 12", + "", + "Hello World!"); + } + + await server.StopAsync(); + } + } + + [Fact] + public async Task ResponseBodyCanWrite() + { + using (var server = new TestServer(async httpContext => + { + httpContext.Response.ContentLength = 12; + await httpContext.Response.Body.WriteAsync(Encoding.ASCII.GetBytes("hello, world")); + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 12", + "", + "hello, world"); + } + await server.StopAsync(); + } + } + + [Fact] + public async Task ResponseBodyAndResponsePipeWorks() + { + using (var server = new TestServer(async httpContext => + { + var response = httpContext.Response; + response.ContentLength = 54; + await response.StartAsync(); + var memory = response.BodyPipe.GetMemory(4096); + var fisrtPartOfResponse = Encoding.ASCII.GetBytes("hello,"); + fisrtPartOfResponse.CopyTo(memory); + response.BodyPipe.Advance(6); + var secondPartOfResponse = Encoding.ASCII.GetBytes(" world\r\n"); + secondPartOfResponse.CopyTo(memory.Slice(6)); + response.BodyPipe.Advance(8); + + await response.Body.WriteAsync(Encoding.ASCII.GetBytes("hello, world\r\n")); + await response.BodyPipe.WriteAsync(Encoding.ASCII.GetBytes("hello, world\r\n")); + await response.WriteAsync("hello, world"); + + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 54", + "", + "hello, world", + "hello, world", + "hello, world", + "hello, world"); + } + await server.StopAsync(); + } + } + private static async Task ResponseStatusCodeSetBeforeHttpContextDispose( ITestSink testSink, ILoggerFactory loggerFactory, diff --git a/src/Servers/Kestrel/tools/CodeGenerator/HttpProtocolFeatureCollection.cs b/src/Servers/Kestrel/tools/CodeGenerator/HttpProtocolFeatureCollection.cs index d6828450d0ba..4e7b1a689fb5 100644 --- a/src/Servers/Kestrel/tools/CodeGenerator/HttpProtocolFeatureCollection.cs +++ b/src/Servers/Kestrel/tools/CodeGenerator/HttpProtocolFeatureCollection.cs @@ -13,6 +13,7 @@ public static string GenerateFile() { "IHttpRequestFeature", "IHttpResponseFeature", + "IResponseBodyPipeFeature", "IHttpRequestIdentifierFeature", "IServiceProvidersFeature", "IHttpRequestLifetimeFeature", @@ -60,6 +61,7 @@ public static string GenerateFile() { "IHttpRequestFeature", "IHttpResponseFeature", + "IResponseBodyPipeFeature", "IHttpUpgradeFeature", "IHttpRequestIdentifierFeature", "IHttpRequestLifetimeFeature", @@ -68,7 +70,7 @@ public static string GenerateFile() "IHttpBodyControlFeature", "IHttpResponseStartFeature" }; - + var usings = $@" using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Http.Features.Authentication; From 0e1301b25ae59958ab231ef4800d38aa37617b30 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Tue, 29 Jan 2019 17:36:21 -0800 Subject: [PATCH 02/33] small cleanup --- src/Servers/Kestrel/Core/src/CoreStrings.resx | 3 + .../Core/src/Internal/Http/HttpProtocol.cs | 3 +- .../src/Properties/CoreStrings.Designer.cs | 13 ++++ .../test/HttpResponseWritingExtensions.cs | 62 ------------------- .../test/FunctionalTests/ResponseTests.cs | 6 +- .../InMemory.FunctionalTests.csproj | 4 -- 6 files changed, 20 insertions(+), 71 deletions(-) delete mode 100644 src/Servers/Kestrel/shared/test/HttpResponseWritingExtensions.cs diff --git a/src/Servers/Kestrel/Core/src/CoreStrings.resx b/src/Servers/Kestrel/Core/src/CoreStrings.resx index c90468e5a141..d2b23f176c44 100644 --- a/src/Servers/Kestrel/Core/src/CoreStrings.resx +++ b/src/Servers/Kestrel/Core/src/CoreStrings.resx @@ -596,4 +596,7 @@ For more information on configuring HTTPS see https://go.microsoft.com/fwlink/?l Server shutdown started during connection initialization. + + Cannot call GetMemory() until response has started. Call HttpResponse.StartAsync() before calling GetMemory(). + \ No newline at end of file diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index ca87df19cb32..1d363869ae4b 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -1293,8 +1293,7 @@ private void ThrowIfResponseNotStarted() { if (!HasResponseStarted) { - throw new InvalidOperationException("Cannot call GetMemory() until response has started. " + - "Call HttpResponse.StartAsync() before calling GetMemory()."); + throw new InvalidOperationException(CoreStrings.StartAsyncBeforeGetMemory); } } diff --git a/src/Servers/Kestrel/Core/src/Properties/CoreStrings.Designer.cs b/src/Servers/Kestrel/Core/src/Properties/CoreStrings.Designer.cs index 592d92e4d37f..9a577877e8fd 100644 --- a/src/Servers/Kestrel/Core/src/Properties/CoreStrings.Designer.cs +++ b/src/Servers/Kestrel/Core/src/Properties/CoreStrings.Designer.cs @@ -2239,6 +2239,19 @@ internal static string ServerShutdownDuringConnectionInitialization /// internal static string FormatServerShutdownDuringConnectionInitialization() => GetString("ServerShutdownDuringConnectionInitialization"); + /// + /// Cannot call GetMemory() until response has started. Call HttpResponse.StartAsync() before calling GetMemory(). + /// + internal static string StartAsyncBeforeGetMemory + { + get => GetString("StartAsyncBeforeGetMemory"); + } + + /// + /// Cannot call GetMemory() until response has started. Call HttpResponse.StartAsync() before calling GetMemory(). + /// + internal static string FormatStartAsyncBeforeGetMemory() + => GetString("StartAsyncBeforeGetMemory"); private static string GetString(string name, params string[] formatterNames) { diff --git a/src/Servers/Kestrel/shared/test/HttpResponseWritingExtensions.cs b/src/Servers/Kestrel/shared/test/HttpResponseWritingExtensions.cs deleted file mode 100644 index 93cf4325b1d2..000000000000 --- a/src/Servers/Kestrel/shared/test/HttpResponseWritingExtensions.cs +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Text; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Http; - -namespace Microsoft.AspNetCore.Server.Kestrel.Tests -{ - public static class HttpResponseExtensions - { - public static Task WriteResponseBodyAsync(this HttpResponse response, byte[] buffer, int start, int length, bool isPipeTest = true, CancellationToken token = default) - { - if (isPipeTest) - { - return response.BodyPipe.WriteAsync(new ReadOnlyMemory(buffer, start, length), token).AsTask(); - } - else - { - return response.Body.WriteAsync(buffer, start, length, token); - } - } - - public static Task WriteResponseAsync(this HttpResponse response, string responseString, bool isPipeTest = true, CancellationToken token = default) - { - if (isPipeTest) - { - return response.BodyPipe.WriteAsync(Encoding.ASCII.GetBytes(responseString), token).AsTask(); - } - else - { - return response.WriteAsync(responseString, token); - } - } - - public static Task FlushResponseAsync(this HttpResponse response, bool isPipeTest = true, CancellationToken token = default) - { - if (isPipeTest) - { - return response.BodyPipe.FlushAsync(token).AsTask(); - } - else - { - return response.Body.FlushAsync(token); - } - } - - public static void WriteResponse(this HttpResponse response, byte[] buffer, int start, int length, bool isPipeTest = true) - { - if (isPipeTest) - { - response.BodyPipe.WriteAsync(new ReadOnlyMemory(buffer, start, length)).AsTask().GetAwaiter().GetResult(); - } - else - { - response.Body.Write(buffer, start, length); - } - } - } -} diff --git a/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs index dede4ce9fd86..87865ca543e2 100644 --- a/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/FunctionalTests/ResponseTests.cs @@ -109,7 +109,7 @@ public async Task IgnoreNullHeaderValues(string headerName, StringValues headerV { context.Response.Headers.Add(headerName, headerValue); - await context.Response.WriteResponseAsync(""); + await context.Response.WriteAsync(""); }); }); @@ -153,7 +153,7 @@ public async Task WriteAfterConnectionCloseNoops(ListenOptions listenOptions) requestStarted.SetResult(null); await connectionClosed.Task.DefaultTimeout(); httpContext.Response.ContentLength = 12; - await httpContext.Response.WriteResponseAsync("hello, world"); + await httpContext.Response.WriteAsync("hello, world"); appCompleted.TrySetResult(null); } catch (Exception ex) @@ -209,7 +209,7 @@ public async Task ThrowsOnWriteWithRequestAbortedTokenAfterRequestIsAborted(List try { - await response.WriteResponseAsync(largeString, token: lifetime.RequestAborted); + await response.WriteAsync(largeString, cancellationToken: lifetime.RequestAborted); } catch (Exception ex) { diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/InMemory.FunctionalTests.csproj b/src/Servers/Kestrel/test/InMemory.FunctionalTests/InMemory.FunctionalTests.csproj index 74d7eb775c28..c2046f8cd88a 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/InMemory.FunctionalTests.csproj +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/InMemory.FunctionalTests.csproj @@ -14,10 +14,6 @@ - - - - From 53af11d5614f95ab71141e5eee8c6bbd0171c2fb Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 30 Jan 2019 11:51:13 -0800 Subject: [PATCH 03/33] Make tests pass --- src/Http/Http/test/StreamPipeWriterTests.cs | 17 ++++------ .../test/ResponseCompressionMiddlewareTest.cs | 32 +++++++++---------- 2 files changed, 22 insertions(+), 27 deletions(-) diff --git a/src/Http/Http/test/StreamPipeWriterTests.cs b/src/Http/Http/test/StreamPipeWriterTests.cs index c2bfce4d1a03..5095de9d330d 100644 --- a/src/Http/Http/test/StreamPipeWriterTests.cs +++ b/src/Http/Http/test/StreamPipeWriterTests.cs @@ -25,27 +25,22 @@ public async Task CanWriteAsyncMultipleTimesIntoSameBlock() } [Theory] - [InlineData(100, 1000)] - [InlineData(100, 8000)] - [InlineData(100, 10000)] - [InlineData(8000, 100)] - [InlineData(8000, 8000)] - public async Task CanAdvanceWithPartialConsumptionOfFirstSegment(int firstWriteLength, int secondWriteLength) + [InlineData(100)] + [InlineData(4000)] + public async Task CanAdvanceWithPartialConsumptionOfFirstSegment(int firstWriteLength) { Writer = new StreamPipeWriter(Stream, MinimumSegmentSize, new TestMemoryPool(maxBufferSize: 20000)); await Writer.WriteAsync(Encoding.ASCII.GetBytes("a")); - var expectedLength = firstWriteLength + secondWriteLength + 1; - var memory = Writer.GetMemory(firstWriteLength); Writer.Advance(firstWriteLength); - memory = Writer.GetMemory(secondWriteLength); - Writer.Advance(secondWriteLength); + memory = Writer.GetMemory(); + Writer.Advance(memory.Length); await Writer.FlushAsync(); - Assert.Equal(expectedLength, Read().Length); + Assert.Equal(firstWriteLength + memory.Length + 1, Read().Length); } [Fact] diff --git a/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs b/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs index 3c6be0aa8eea..aef87c545055 100644 --- a/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs +++ b/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs @@ -37,8 +37,8 @@ private static IEnumerable TestData { get { - yield return new EncodingTestData("gzip", expectedBodyLength: 24); - yield return new EncodingTestData("br", expectedBodyLength: 20); + yield return new EncodingTestData("gzip", expectedBodyLength: 30); + yield return new EncodingTestData("br", expectedBodyLength: 21); } } @@ -64,7 +64,7 @@ public async Task Request_AcceptGzipDeflate_CompressedGzip() { var (response, logMessages) = await InvokeMiddleware(100, requestAcceptEncodings: new[] { "gzip", "deflate" }, responseType: TextPlain); - CheckResponseCompressed(response, expectedBodyLength: 24, expectedEncoding: "gzip"); + CheckResponseCompressed(response, expectedBodyLength: 30, expectedEncoding: "gzip"); AssertCompressedWithLog(logMessages, "gzip"); } @@ -73,7 +73,7 @@ public async Task Request_AcceptBrotli_CompressedBrotli() { var (response, logMessages) = await InvokeMiddleware(100, requestAcceptEncodings: new[] { "br" }, responseType: TextPlain); - CheckResponseCompressed(response, expectedBodyLength: 20, expectedEncoding: "br"); + CheckResponseCompressed(response, expectedBodyLength: 21, expectedEncoding: "br"); AssertCompressedWithLog(logMessages, "br"); } @@ -84,7 +84,7 @@ public async Task Request_AcceptMixed_CompressedBrotli(string encoding1, string { var (response, logMessages) = await InvokeMiddleware(100, new[] { encoding1, encoding2 }, responseType: TextPlain); - CheckResponseCompressed(response, expectedBodyLength: 20, expectedEncoding: "br"); + CheckResponseCompressed(response, expectedBodyLength: 21, expectedEncoding: "br"); AssertCompressedWithLog(logMessages, "br"); } @@ -101,7 +101,7 @@ void Configure(ResponseCompressionOptions options) var (response, logMessages) = await InvokeMiddleware(100, new[] { encoding1, encoding2 }, responseType: TextPlain, configure: Configure); - CheckResponseCompressed(response, expectedBodyLength: 24, expectedEncoding: "gzip"); + CheckResponseCompressed(response, expectedBodyLength: 30, expectedEncoding: "gzip"); AssertCompressedWithLog(logMessages, "gzip"); } @@ -126,7 +126,7 @@ public async Task ContentType_WithCharset_Compress(string contentType) { var (response, logMessages) = await InvokeMiddleware(uncompressedBodyLength: 100, requestAcceptEncodings: new[] { "gzip" }, contentType); - CheckResponseCompressed(response, expectedBodyLength: 24, expectedEncoding: "gzip"); + CheckResponseCompressed(response, expectedBodyLength: 30, expectedEncoding: "gzip"); AssertCompressedWithLog(logMessages, "gzip"); } @@ -158,7 +158,7 @@ public async Task GZipCompressionProvider_OptionsSetInDI_Compress() var response = await client.SendAsync(request); - CheckResponseCompressed(response, expectedBodyLength: 123, expectedEncoding: "gzip"); + CheckResponseCompressed(response, expectedBodyLength: 133, expectedEncoding: "gzip"); } [Theory] @@ -251,7 +251,7 @@ bool compress if (compress) { - CheckResponseCompressed(response, expectedBodyLength: 24, expectedEncoding: "gzip"); + CheckResponseCompressed(response, expectedBodyLength: 30, expectedEncoding: "gzip"); AssertCompressedWithLog(logMessages, "gzip"); } else @@ -272,7 +272,7 @@ public async Task NoIncludedMimeTypes_UseDefaults() options.ExcludedMimeTypes = new[] { "text/*" }; }); - CheckResponseCompressed(response, expectedBodyLength: 24, expectedEncoding: "gzip"); + CheckResponseCompressed(response, expectedBodyLength: 30, expectedEncoding: "gzip"); AssertCompressedWithLog(logMessages, "gzip"); } @@ -317,7 +317,7 @@ public async Task Request_AcceptStar_Compressed() { var (response, logMessages) = await InvokeMiddleware(100, requestAcceptEncodings: new[] { "*" }, responseType: TextPlain); - CheckResponseCompressed(response, expectedBodyLength: 20, expectedEncoding: "br"); + CheckResponseCompressed(response, expectedBodyLength: 21, expectedEncoding: "br"); AssertCompressedWithLog(logMessages, "br"); } @@ -334,9 +334,9 @@ public async Task Request_AcceptIdentity_NotCompressed() } [Theory] - [InlineData(new[] { "identity;q=0.5", "gzip;q=1" }, 24)] - [InlineData(new[] { "identity;q=0", "gzip;q=0.8" }, 24)] - [InlineData(new[] { "identity;q=0.5", "gzip" }, 24)] + [InlineData(new[] { "identity;q=0.5", "gzip;q=1" }, 30)] + [InlineData(new[] { "identity;q=0", "gzip;q=0.8" }, 30)] + [InlineData(new[] { "identity;q=0.5", "gzip" }, 30)] public async Task Request_AcceptWithHigherCompressionQuality_Compressed(string[] acceptEncodings, int expectedBodyLength) { var (response, logMessages) = await InvokeMiddleware(100, requestAcceptEncodings: acceptEncodings, responseType: TextPlain); @@ -404,7 +404,7 @@ public async Task Response_WithContentEncodingAlreadySet_NotReCompressed() [Theory] [InlineData(false, 100)] - [InlineData(true, 24)] + [InlineData(true, 30)] public async Task Request_Https_CompressedIfEnabled(bool enableHttps, int expectedLength) { var sink = new TestSink( @@ -908,7 +908,7 @@ public async Task SendFileAsync_AfterFirstWrite_CompressesAndFlushes() var response = await client.SendAsync(request); - CheckResponseCompressed(response, expectedBodyLength: 40, expectedEncoding: "gzip"); + CheckResponseCompressed(response, expectedBodyLength: 46, expectedEncoding: "gzip"); Assert.False(fakeSendFile.Invoked); } From b3c101e2546c8f4a686517ef20f952ca400ae707 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 30 Jan 2019 15:01:26 -0800 Subject: [PATCH 04/33] Making WriteAsync use GetMemory --- .../HttpResponseWritingExtensions.cs | 37 ++++++++++++++++++- .../Http/src/Internal/DefaultHttpResponse.cs | 2 +- .../src/Internal/Http/Http1OutputProducer.cs | 13 +++++-- .../Http/HttpProtocol.FeatureCollection.cs | 2 + .../ChunkedResponseTests.cs | 6 +-- 5 files changed, 50 insertions(+), 10 deletions(-) diff --git a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs index b94d35dcee1b..65370bd94fc6 100644 --- a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs +++ b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.IO.Pipelines; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -60,8 +61,42 @@ public static class HttpResponseWritingExtensions throw new ArgumentNullException(nameof(encoding)); } + if (!response.HasStarted) + { + return StartAndFlushAsyncAwaited(response, text, encoding, cancellationToken); + } + + WriteDataToPipe(response.BodyPipe, text, encoding); + + var flushTask = response.BodyPipe.FlushAsync(cancellationToken); + + if (flushTask.IsCompleted) + { + return Task.CompletedTask; + } + + return FlushTaskAwaited(flushTask); + } + + private static void WriteDataToPipe(PipeWriter responsePipeWriter, string text, Encoding encoding) + { byte[] data = encoding.GetBytes(text); - return response.BodyPipe.WriteAsync(new Memory(data, 0, data.Length), cancellationToken).AsTask(); + var memory = responsePipeWriter.GetMemory(data.Length); + data.CopyTo(memory); + responsePipeWriter.Advance(data.Length); + } + + private static async Task FlushTaskAwaited(ValueTask flushTask) + { + await flushTask; } + + private static async Task StartAndFlushAsyncAwaited(HttpResponse response, string text, Encoding encoding, CancellationToken cancellationToken) + { + await response.StartAsync(cancellationToken); + WriteDataToPipe(response.BodyPipe, text, encoding); + await response.BodyPipe.FlushAsync(cancellationToken); + } + } } diff --git a/src/Http/Http/src/Internal/DefaultHttpResponse.cs b/src/Http/Http/src/Internal/DefaultHttpResponse.cs index 6fea6198bf11..6c6068e44152 100644 --- a/src/Http/Http/src/Internal/DefaultHttpResponse.cs +++ b/src/Http/Http/src/Internal/DefaultHttpResponse.cs @@ -151,7 +151,7 @@ public override Task StartAsync(CancellationToken cancellationToken = default) return HttpResponseFeature.Body.FlushAsync(cancellationToken); } - return HttpResponseStartFeature.StartAsync(); + return HttpResponseStartFeature.StartAsync(cancellationToken); } struct FeatureInterfaces diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs index 5ba5e12ee853..922bd089eb90 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs @@ -35,7 +35,7 @@ public class Http1OutputProducer : IHttpOutputProducer, IHttpOutputAborter, IDis private long _unflushedBytes; private bool _autoChunk; private readonly PipeWriter _pipeWriter; - + private const int MemorySizeThreshold = 1024; private const int BeginChunkLengthMax = 5; private const int EndChunkLength = 2; @@ -140,7 +140,6 @@ public void Advance(int bytes) public void CancelPendingFlush() { - // TODO we may not want to support this. _pipeWriter.CancelPendingFlush(); } @@ -269,6 +268,7 @@ private ValueTask WriteAsync( var bytesWritten = _unflushedBytes; _unflushedBytes = 0; + _currentChunkMemory = default; return _flusher.FlushAsync( _minResponseDataRateFeature.MinDataRate, @@ -291,7 +291,7 @@ private Memory GetChunkedMemory(int sizeHint) } var memoryMaxLength = _currentChunkMemory.Length - BeginChunkLengthMax - EndChunkLength; - if (_advancedBytesForChunk == memoryMaxLength) + if (_advancedBytesForChunk > memoryMaxLength - MemorySizeThreshold) { // Chunk is completely written, commit it to the pipe so GetMemory will return a new chunk of memory. CommitChunkToPipe(); @@ -302,6 +302,8 @@ private Memory GetChunkedMemory(int sizeHint) BeginChunkLengthMax + _advancedBytesForChunk, memoryMaxLength - _advancedBytesForChunk); + Debug.Assert(actualMemory.Length <= 4089); + return actualMemory; } @@ -314,6 +316,9 @@ private void CommitChunkToPipe() if (_advancedBytesForChunk > 0) { var bytesWritten = writer.WriteBeginChunkBytes(_advancedBytesForChunk); + + Debug.Assert(bytesWritten <= BeginChunkLengthMax); + if (bytesWritten < BeginChunkLengthMax) { // If the current chunk of memory isn't completely utilized, we need to copy the contents forwards. @@ -322,7 +327,7 @@ private void CommitChunkToPipe() _currentChunkMemory.Slice(BeginChunkLengthMax, _advancedBytesForChunk).CopyTo(_currentChunkMemory.Slice(bytesWritten)); } - writer.Write(_currentChunkMemory.Slice(bytesWritten, _advancedBytesForChunk).Span); + writer.Advance(_advancedBytesForChunk); writer.WriteEndChunkBytes(); writer.Commit(); _advancedBytesForChunk = 0; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs index eadb7c8226c9..c73581d2c03f 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs @@ -314,6 +314,8 @@ Task IHttpResponseStartFeature.StartAsync(CancellationToken cancellationToken) return Task.CompletedTask; } + cancellationToken.ThrowIfCancellationRequested(); + return InitializeResponseAsync(0); } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs index 1cf062471612..383bb631a3ec 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs @@ -688,10 +688,8 @@ await connection.Receive( $"Date: {testContext.DateHeaderValue}", "Transfer-Encoding: chunked", "", - "6", - "Hello ", - "6", - "World!", + "c", + "Hello World!", "0", "", ""); From 93245386c7ea3f57c8ab3d1992064347d8be1261 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 30 Jan 2019 15:02:23 -0800 Subject: [PATCH 05/33] nit --- .../Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs index 922bd089eb90..36e712156fb9 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs @@ -291,7 +291,7 @@ private Memory GetChunkedMemory(int sizeHint) } var memoryMaxLength = _currentChunkMemory.Length - BeginChunkLengthMax - EndChunkLength; - if (_advancedBytesForChunk > memoryMaxLength - MemorySizeThreshold) + if (_advancedBytesForChunk > memoryMaxLength - Math.Min(MemorySizeThreshold, sizeHint)) { // Chunk is completely written, commit it to the pipe so GetMemory will return a new chunk of memory. CommitChunkToPipe(); From 89284d614485e7e1788ff7713c645f52f70c02d4 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 4 Feb 2019 09:30:09 -0800 Subject: [PATCH 06/33] update response writing extensions --- .../HttpResponseWritingExtensions.cs | 33 ++++--------------- 1 file changed, 6 insertions(+), 27 deletions(-) diff --git a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs index 65370bd94fc6..315c6e150e99 100644 --- a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs +++ b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs @@ -61,42 +61,21 @@ public static class HttpResponseWritingExtensions throw new ArgumentNullException(nameof(encoding)); } - if (!response.HasStarted) - { - return StartAndFlushAsyncAwaited(response, text, encoding, cancellationToken); - } - - WriteDataToPipe(response.BodyPipe, text, encoding); - - var flushTask = response.BodyPipe.FlushAsync(cancellationToken); + byte[] data = encoding.GetBytes(text); + var writeTask = response.BodyPipe.WriteAsync(data, cancellationToken); - if (flushTask.IsCompleted) + if (writeTask.IsCompleted) { + writeTask.GetAwaiter().GetResult(); return Task.CompletedTask; } - return FlushTaskAwaited(flushTask); + return WriteTaskAwaited(writeTask); } - private static void WriteDataToPipe(PipeWriter responsePipeWriter, string text, Encoding encoding) - { - byte[] data = encoding.GetBytes(text); - var memory = responsePipeWriter.GetMemory(data.Length); - data.CopyTo(memory); - responsePipeWriter.Advance(data.Length); - } - - private static async Task FlushTaskAwaited(ValueTask flushTask) + private static async Task WriteTaskAwaited(ValueTask flushTask) { await flushTask; } - - private static async Task StartAndFlushAsyncAwaited(HttpResponse response, string text, Encoding encoding, CancellationToken cancellationToken) - { - await response.StartAsync(cancellationToken); - WriteDataToPipe(response.BodyPipe, text, encoding); - await response.BodyPipe.FlushAsync(cancellationToken); - } - } } From d75c9e4d5881eb1609e80276df903d2b938328c6 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 4 Feb 2019 09:35:17 -0800 Subject: [PATCH 07/33] Add inner stream/pipe to all adapters --- src/Http/Http/src/ReadOnlyPipeStream.cs | 13 +++++++------ src/Http/Http/src/StreamPipeReader.cs | 8 +++----- src/Http/Http/src/StreamPipeWriter.cs | 11 +++++------ src/Http/Http/src/WriteOnlyPipeStream.cs | 9 ++++----- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/src/Http/Http/src/ReadOnlyPipeStream.cs b/src/Http/Http/src/ReadOnlyPipeStream.cs index 7585947d2c7d..c3f9600fcd1b 100644 --- a/src/Http/Http/src/ReadOnlyPipeStream.cs +++ b/src/Http/Http/src/ReadOnlyPipeStream.cs @@ -13,7 +13,6 @@ namespace System.IO.Pipelines /// public class ReadOnlyPipeStream : Stream { - private readonly PipeReader _pipeReader; private bool _allowSynchronousIO = true; /// @@ -33,7 +32,7 @@ public ReadOnlyPipeStream(PipeReader pipeReader) : public ReadOnlyPipeStream(PipeReader pipeReader, bool allowSynchronousIO) { _allowSynchronousIO = allowSynchronousIO; - _pipeReader = pipeReader; + InnerPipeReader = pipeReader; } /// @@ -62,6 +61,8 @@ public override int WriteTimeout set => throw new NotSupportedException(); } + public PipeReader InnerPipeReader { get; } + /// public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); @@ -160,7 +161,7 @@ private async ValueTask ReadAsyncInternal(Memory buffer, Cancellation { while (true) { - var result = await _pipeReader.ReadAsync(cancellationToken); + var result = await InnerPipeReader.ReadAsync(cancellationToken); var readableBuffer = result.Buffer; var readableBufferLength = readableBuffer.Length; @@ -186,7 +187,7 @@ private async ValueTask ReadAsyncInternal(Memory buffer, Cancellation } finally { - _pipeReader.AdvanceTo(consumed); + InnerPipeReader.AdvanceTo(consumed); } } } @@ -211,7 +212,7 @@ private async Task CopyToAsyncInternal(Stream destination, CancellationToken can { while (true) { - var result = await _pipeReader.ReadAsync(cancellationToken); + var result = await InnerPipeReader.ReadAsync(cancellationToken); var readableBuffer = result.Buffer; var readableBufferLength = readableBuffer.Length; @@ -232,7 +233,7 @@ private async Task CopyToAsyncInternal(Stream destination, CancellationToken can } finally { - _pipeReader.AdvanceTo(readableBuffer.End); + InnerPipeReader.AdvanceTo(readableBuffer.End); } } } diff --git a/src/Http/Http/src/StreamPipeReader.cs b/src/Http/Http/src/StreamPipeReader.cs index 5ddcdb2cc3e8..36e4f0e21a25 100644 --- a/src/Http/Http/src/StreamPipeReader.cs +++ b/src/Http/Http/src/StreamPipeReader.cs @@ -17,7 +17,6 @@ public class StreamPipeReader : PipeReader, IDisposable { private readonly int _minimumSegmentSize; private readonly int _minimumReadThreshold; - private readonly Stream _readingStream; private readonly MemoryPool _pool; private CancellationTokenSource _internalTokenSource; @@ -42,7 +41,6 @@ public StreamPipeReader(Stream readingStream) { } - /// /// Creates a new StreamPipeReader. /// @@ -50,7 +48,7 @@ public StreamPipeReader(Stream readingStream) /// The options to use. public StreamPipeReader(Stream readingStream, StreamPipeReaderOptions options) { - _readingStream = readingStream ?? throw new ArgumentNullException(nameof(readingStream)); + InnerStream = readingStream ?? throw new ArgumentNullException(nameof(readingStream)); if (options == null) { @@ -70,7 +68,7 @@ public StreamPipeReader(Stream readingStream, StreamPipeReaderOptions options) /// /// Gets the inner stream that is being read from. /// - public Stream InnerStream => _readingStream; + public Stream InnerStream { get; } /// public override void AdvanceTo(SequencePosition consumed) @@ -235,7 +233,7 @@ public override async ValueTask ReadAsync(CancellationToken cancella { AllocateReadTail(); #if NETCOREAPP3_0 - var length = await _readingStream.ReadAsync(_readTail.AvailableMemory.Slice(_readTail.End), tokenSource.Token); + var length = await InnerStream.ReadAsync(_readTail.AvailableMemory.Slice(_readTail.End), tokenSource.Token); #elif NETSTANDARD2_0 if (!MemoryMarshal.TryGetArray(_readTail.AvailableMemory.Slice(_readTail.End), out var arraySegment)) { diff --git a/src/Http/Http/src/StreamPipeWriter.cs b/src/Http/Http/src/StreamPipeWriter.cs index 3c327d71d31b..b95ee5875da9 100644 --- a/src/Http/Http/src/StreamPipeWriter.cs +++ b/src/Http/Http/src/StreamPipeWriter.cs @@ -14,7 +14,6 @@ namespace System.IO.Pipelines public class StreamPipeWriter : PipeWriter, IDisposable { private readonly int _minimumSegmentSize; - private readonly Stream _writingStream; private int _bytesWritten; private List _completedSegments; @@ -53,14 +52,14 @@ public StreamPipeWriter(Stream writingStream) : this(writingStream, 4096) public StreamPipeWriter(Stream writingStream, int minimumSegmentSize, MemoryPool pool = null) { _minimumSegmentSize = minimumSegmentSize; - _writingStream = writingStream; + InnerStream = writingStream; _pool = pool ?? MemoryPool.Shared; } /// /// Gets the inner stream that is being written to. /// - public Stream InnerStream => _writingStream; + public Stream InnerStream { get; } /// public override void Advance(int count) @@ -180,7 +179,7 @@ private async ValueTask FlushAsyncInternal(CancellationToken cancel { var segment = _completedSegments[0]; #if NETCOREAPP3_0 - await _writingStream.WriteAsync(segment.Buffer.Slice(0, segment.Length), localToken); + await InnerStream.WriteAsync(segment.Buffer.Slice(0, segment.Length), localToken); #elif NETSTANDARD2_0 MemoryMarshal.TryGetArray(segment.Buffer, out var arraySegment); await _writingStream.WriteAsync(arraySegment.Array, 0, segment.Length, localToken); @@ -196,7 +195,7 @@ private async ValueTask FlushAsyncInternal(CancellationToken cancel if (!_currentSegment.IsEmpty) { #if NETCOREAPP3_0 - await _writingStream.WriteAsync(_currentSegment.Slice(0, _position), localToken); + await InnerStream.WriteAsync(_currentSegment.Slice(0, _position), localToken); #elif NETSTANDARD2_0 MemoryMarshal.TryGetArray(_currentSegment, out var arraySegment); await _writingStream.WriteAsync(arraySegment.Array, 0, _position, localToken); @@ -207,7 +206,7 @@ private async ValueTask FlushAsyncInternal(CancellationToken cancel _position = 0; } - await _writingStream.FlushAsync(localToken); + await InnerStream.FlushAsync(localToken); return new FlushResult(isCanceled: false, _isCompleted); } diff --git a/src/Http/Http/src/WriteOnlyPipeStream.cs b/src/Http/Http/src/WriteOnlyPipeStream.cs index 904069b73542..8737696316fc 100644 --- a/src/Http/Http/src/WriteOnlyPipeStream.cs +++ b/src/Http/Http/src/WriteOnlyPipeStream.cs @@ -11,7 +11,6 @@ namespace System.IO.Pipelines /// public class WriteOnlyPipeStream : Stream { - private PipeWriter _pipeWriter; private bool _allowSynchronousIO = true; /// @@ -30,7 +29,7 @@ public WriteOnlyPipeStream(PipeWriter pipeWriter) : /// Whether synchronous IO is allowed. public WriteOnlyPipeStream(PipeWriter pipeWriter, bool allowSynchronousIO) { - _pipeWriter = pipeWriter; + InnerPipeWriter = pipeWriter; _allowSynchronousIO = allowSynchronousIO; } @@ -60,7 +59,7 @@ public override int ReadTimeout set => throw new NotSupportedException(); } - public PipeWriter InnerPipeWriter => _pipeWriter; + public PipeWriter InnerPipeWriter { get; } /// public override int Read(byte[] buffer, int offset, int count) @@ -84,7 +83,7 @@ public override void Flush() /// public override async Task FlushAsync(CancellationToken cancellationToken) { - await _pipeWriter.FlushAsync(cancellationToken); + await InnerPipeWriter.FlushAsync(cancellationToken); } /// @@ -158,7 +157,7 @@ public override Task WriteAsync(byte[] buffer, int offset, int count, Cancellati /// public override async ValueTask WriteAsync(ReadOnlyMemory source, CancellationToken cancellationToken = default) { - await _pipeWriter.WriteAsync(source, cancellationToken); + await InnerPipeWriter.WriteAsync(source, cancellationToken); } } } From 9781e9e954b30a7b579f45f593da80940bfcc56a Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 4 Feb 2019 09:55:44 -0800 Subject: [PATCH 08/33] Clarify chunking code --- .../Core/src/Internal/Http/Http1OutputProducer.cs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs index 36e712156fb9..8bd46c8fe8f7 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs @@ -143,7 +143,7 @@ public void CancelPendingFlush() _pipeWriter.CancelPendingFlush(); } - // This method is for chunked http responses + // This method is for chunked http responses that directly call response.WriteAsync public ValueTask WriteChunkAsync(ReadOnlySpan buffer, CancellationToken cancellationToken) { lock (_contextLock) @@ -153,7 +153,9 @@ public ValueTask WriteChunkAsync(ReadOnlySpan buffer, Cancell return default; } - CommitChunkToPipe(); + // Make sure any memory used with GetMemory/Advance is written before the chunk + // passed in. + WriteCurrentMemoryToPipeWriter(); if (buffer.Length > 0) { @@ -254,7 +256,7 @@ private ValueTask WriteAsync( { // If there is data that was chunked before writing (ex someone did GetMemory->Advance->WriteAsync) // make sure to write whatever was advanced first - CommitChunkToPipe(); + WriteCurrentMemoryToPipeWriter(); } var writer = new BufferWriter(_pipeWriter); @@ -294,7 +296,7 @@ private Memory GetChunkedMemory(int sizeHint) if (_advancedBytesForChunk > memoryMaxLength - Math.Min(MemorySizeThreshold, sizeHint)) { // Chunk is completely written, commit it to the pipe so GetMemory will return a new chunk of memory. - CommitChunkToPipe(); + WriteCurrentMemoryToPipeWriter(); _currentChunkMemory = _pipeWriter.GetMemory(sizeHint); } @@ -307,7 +309,8 @@ private Memory GetChunkedMemory(int sizeHint) return actualMemory; } - private void CommitChunkToPipe() + // This method is for chunked http responses that use GetMemory/Advance + private void WriteCurrentMemoryToPipeWriter() { var writer = new BufferWriter(_pipeWriter); @@ -331,6 +334,7 @@ private void CommitChunkToPipe() writer.WriteEndChunkBytes(); writer.Commit(); _advancedBytesForChunk = 0; + _unflushedBytes += writer.BytesCommitted; } } } From eae44487262990b08c104bfe702bb5891d7800d4 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 4 Feb 2019 14:43:07 -0800 Subject: [PATCH 09/33] Implement Complete() --- .../src/Internal/Http/Http1OutputProducer.cs | 8 +- .../Http/HttpProtocol.FeatureCollection.cs | 1 - .../Core/src/Internal/Http/HttpProtocol.cs | 15 ++- .../Internal/Http/HttpResponsePipeWriter.cs | 1 + .../src/Internal/Http/IHttpOutputProducer.cs | 1 - .../Http2/Http2StreamTests.cs | 112 ++++++++++++++++++ .../InMemory.FunctionalTests/ResponseTests.cs | 85 +++++++++++++ 7 files changed, 212 insertions(+), 11 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs index 8bd46c8fe8f7..dbb60cb7b333 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs @@ -235,12 +235,6 @@ public ValueTask Write100ContinueAsync() return WriteAsync(_continueBytes.Span); } - public void Complete(Exception exception = null) - { - // TODO What to do with exception. - // and how to handle writing to response here. - } - private ValueTask WriteAsync( ReadOnlySpan buffer, CancellationToken cancellationToken = default) @@ -280,6 +274,7 @@ private ValueTask WriteAsync( } } + // These methods are for chunked http responses that use GetMemory/Advance private Memory GetChunkedMemory(int sizeHint) { // The max size of a chunk will be the size of memory returned from the PipeWriter (today 4096) @@ -309,7 +304,6 @@ private Memory GetChunkedMemory(int sizeHint) return actualMemory; } - // This method is for chunked http responses that use GetMemory/Advance private void WriteCurrentMemoryToPipeWriter() { var writer = new BufferWriter(_pipeWriter); diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs index c73581d2c03f..9e9dc4995283 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs @@ -305,7 +305,6 @@ void IHttpRequestLifetimeFeature.Abort() ApplicationAbort(); } - protected abstract void ApplicationAbort(); Task IHttpResponseStartFeature.StartAsync(CancellationToken cancellationToken) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 1d363869ae4b..96e239d3f8ad 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -392,6 +392,8 @@ public void Reset() protected abstract void OnReset(); + protected abstract void ApplicationAbort(); + protected virtual void OnRequestProcessingEnding() { } @@ -1317,9 +1319,18 @@ public void CancelPendingFlush() Output.CancelPendingFlush(); } - public void Complete(Exception exception = null) + public void Complete(Exception ex) { - Output.Complete(exception); + if (ex != null) + { + var wrappedException = new ConnectionAbortedException("The BodyPipe was completed with an exception.", ex); + ReportApplicationError(wrappedException); + + if (HasResponseStarted) + { + ApplicationAbort(); + } + } } public ValueTask WritePipeAsync(ReadOnlyMemory data, CancellationToken cancellationToken) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs index 807a05825a44..fa7629ebf9ea 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs @@ -32,6 +32,7 @@ public override void CancelPendingFlush() public override void Complete(Exception exception = null) { + StopAcceptingWrites(); _pipeControl.Complete(exception); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs index f6f3156578ce..27f474b8669c 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs @@ -22,6 +22,5 @@ public interface IHttpOutputProducer Span GetSpan(int sizeHint = 0); Memory GetMemory(int sizeHint = 0); void CancelPendingFlush(); - void Complete(Exception exception = null); } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 85303ba64943..6e4900cd37ba 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -3070,5 +3070,117 @@ await ExpectAsync(Http2FrameType.DATA, Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); Assert.Equal("54", _decodedHeaders[HeaderNames.ContentLength]); } + + [Fact] + public async Task ResponseBodyPipeCompleteWithoutExceptionDoesNotThrow() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async context => + { + context.Response.BodyPipe.Complete(); + await Task.CompletedTask; + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 55, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + 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(3, _decodedHeaders.Count); + Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); + Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); + } + + [Fact] + public async Task ResponseBodyPipeCompleteWithoutExceptionWritesThrow() + { + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async context => + { + context.Response.BodyPipe.Complete(); + await Assert.ThrowsAsync( + async () => await context.Response.WriteAsync("test")); + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 55, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + 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(3, _decodedHeaders.Count); + Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); + Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); + } + + [Fact] + public async Task ResponseBodyPipeCompleteWithExceptionThrows() + { + var expectedException = new Exception(); + var headers = new[] + { + new KeyValuePair(HeaderNames.Method, "GET"), + new KeyValuePair(HeaderNames.Path, "/"), + new KeyValuePair(HeaderNames.Scheme, "http"), + }; + await InitializeConnectionAsync(async context => + { + context.Response.BodyPipe.Complete(expectedException); + await Task.CompletedTask; + }); + + await StartStreamAsync(1, headers, endStream: true); + + var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, + withLength: 55, + withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, + 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(3, _decodedHeaders.Count); + Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); + Assert.Equal("500", _decodedHeaders[HeaderNames.Status]); + + Assert.Contains(TestSink.Writes, w => w.EventId.Id == 13 && w.LogLevel == LogLevel.Error + && w.Exception is ConnectionAbortedException && w.Exception.InnerException == expectedException); + } } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs index 514655f6ce04..d011de09276e 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs @@ -3331,6 +3331,91 @@ await connection.Receive( } } + [Fact] + public async Task ResponsePipeWriterCompleteWithoutExceptionDoesNotThrow() + { + using (var server = new TestServer(async httpContext => + { + httpContext.Response.BodyPipe.Complete(); + await Task.CompletedTask; + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 0", + "", + ""); + } + await server.StopAsync(); + } + } + + [Fact] + public async Task ResponsePipeWriterCompleteWithoutExceptionCausesWritesToThrow() + { + using (var server = new TestServer(async httpContext => + { + httpContext.Response.BodyPipe.Complete(); + var ex = await Assert.ThrowsAsync( + async () => await httpContext.Response.WriteAsync("test")); + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 0", + "", + ""); + } + await server.StopAsync(); + } + } + + [Fact] + public async Task ResponsePipeWriterCompleteWithException() + { + var expectedException = new Exception(); + using (var server = new TestServer(async httpContext => + { + httpContext.Response.BodyPipe.Complete(expectedException); + await Task.CompletedTask; + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + $"HTTP/1.1 500 Internal Server Error", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 0", + "", + ""); + Assert.Contains(TestSink.Writes, w => w.EventId.Id == 13 && w.LogLevel == LogLevel.Error + && w.Exception is ConnectionAbortedException && w.Exception.InnerException == expectedException); + } + await server.StopAsync(); + } + } + private static async Task ResponseStatusCodeSetBeforeHttpContextDispose( ITestSink testSink, ILoggerFactory loggerFactory, From 7b0db9e4f740ac39612aabd422809abd9606c9ca Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 4 Feb 2019 15:06:07 -0800 Subject: [PATCH 10/33] Use bool for checking chunked status --- .../Core/src/Internal/Http/Http1OutputProducer.cs | 9 ++++++--- .../InMemory.FunctionalTests/ChunkedResponseTests.cs | 6 ++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs index dbb60cb7b333..8fa9e77a9311 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs @@ -46,6 +46,7 @@ public class Http1OutputProducer : IHttpOutputProducer, IHttpOutputAborter, IDis // and append the end terminator. private int _advancedBytesForChunk; private Memory _currentChunkMemory; + private bool _currentChunkMemoryUpdated; public Http1OutputProducer( PipeWriter pipeWriter, @@ -264,7 +265,7 @@ private ValueTask WriteAsync( var bytesWritten = _unflushedBytes; _unflushedBytes = 0; - _currentChunkMemory = default; + _currentChunkMemoryUpdated = false; return _flusher.FlushAsync( _minResponseDataRateFeature.MinDataRate, @@ -281,18 +282,20 @@ private Memory GetChunkedMemory(int sizeHint) // minus 5 for the max chunked prefix size and minus 2 for the chunked ending, leaving a total of // 4089. - if (_currentChunkMemory.Length == 0) + if (!_currentChunkMemoryUpdated) { // First time calling GetMemory _currentChunkMemory = _pipeWriter.GetMemory(sizeHint); + _currentChunkMemoryUpdated = true; } var memoryMaxLength = _currentChunkMemory.Length - BeginChunkLengthMax - EndChunkLength; - if (_advancedBytesForChunk > memoryMaxLength - Math.Min(MemorySizeThreshold, sizeHint)) + if (_advancedBytesForChunk >= memoryMaxLength - Math.Min(MemorySizeThreshold, sizeHint)) { // Chunk is completely written, commit it to the pipe so GetMemory will return a new chunk of memory. WriteCurrentMemoryToPipeWriter(); _currentChunkMemory = _pipeWriter.GetMemory(sizeHint); + _currentChunkMemoryUpdated = true; } var actualMemory = _currentChunkMemory.Slice( diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs index 383bb631a3ec..1cf062471612 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs @@ -688,8 +688,10 @@ await connection.Receive( $"Date: {testContext.DateHeaderValue}", "Transfer-Encoding: chunked", "", - "c", - "Hello World!", + "6", + "Hello ", + "6", + "World!", "0", "", ""); From df0412cd752b784ae6b4ff99554f58078a907f0a Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Mon, 4 Feb 2019 16:12:45 -0800 Subject: [PATCH 11/33] Handle wrapping logic for dispose --- .../Http/HttpProtocol.FeatureCollection.cs | 17 +++++++----- .../Core/src/Internal/Http/HttpProtocol.cs | 24 +++++++++++------ .../src/Internal/Http2/Http2OutputProducer.cs | 6 ----- .../InMemory.FunctionalTests/RequestTests.cs | 27 +++++++++++++++++++ 4 files changed, 54 insertions(+), 20 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs index 9e9dc4995283..7682f684b79f 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.IO.Pipelines; @@ -201,15 +202,15 @@ PipeWriter IResponseBodyPipeFeature.ResponseBodyPipe if (!object.ReferenceEquals(_cachedResponseBodyStream, ResponseBody)) { + // TODO use kestrel's memory pool here var responsePipeWriter = new StreamPipeWriter(ResponseBody); ResponsePipeWriter = responsePipeWriter; _cachedResponseBodyStream = ResponseBody; - OnCompleted((rpw) => + if (_wrapperObjectsToDispose == null) { - ((StreamPipeWriter)rpw).Dispose(); - return Task.CompletedTask; - }, responsePipeWriter); - + _wrapperObjectsToDispose = new List(); + } + _wrapperObjectsToDispose.Add(responsePipeWriter); } return ResponsePipeWriter; @@ -236,7 +237,11 @@ Stream IHttpResponseFeature.Body var responseBody = new WriteOnlyPipeStream(ResponsePipeWriter); ResponseBody = responseBody; _cachedResponsePipeWriter = ResponsePipeWriter; - OnCompleted(async (rb) => await ((WriteOnlyPipeStream)rb).DisposeAsync(), responseBody); + if (_wrapperObjectsToDispose == null) + { + _wrapperObjectsToDispose = new List(); + } + _wrapperObjectsToDispose.Add(responseBody); } return ResponseBody; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 96e239d3f8ad..195da950a529 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -67,6 +67,7 @@ public abstract partial class HttpProtocol : IDefaultHttpContextContainer, IHttp protected Stream _userSetResponseBody; protected PipeWriter _cachedResponsePipeWriter; protected Stream _cachedResponseBodyStream; + private List _wrapperObjectsToDispose; public HttpProtocol(HttpConnectionContext context) { @@ -344,6 +345,8 @@ public void Reset() _httpVersion = Http.HttpVersion.Unknown; _statusCode = StatusCodes.Status200OK; _reasonPhrase = null; + _userSetPipeWriter = null; + _userSetResponseBody = null; var remoteEndPoint = RemoteEndPoint; RemoteIpAddress = remoteEndPoint?.Address; @@ -381,6 +384,14 @@ public void Reset() } } + if (_wrapperObjectsToDispose != null) + { + foreach (var disposable in _wrapperObjectsToDispose) + { + disposable.Dispose(); + } + } + _requestHeadersParsed = 0; _responseBytesWritten = 0; @@ -882,12 +893,6 @@ protected void VerifyResponseContentLength() } } - private ValueTask WriteChunkedAsync(ReadOnlySpan data, CancellationToken cancellationToken) - { - _requestProcessingStatus = RequestProcessingStatus.HeadersFlushed; - return Output.WriteChunkAsync(data, cancellationToken); - } - public void ProduceContinue() { if (HasResponseStarted) @@ -1360,7 +1365,9 @@ public ValueTask WritePipeAsync(ReadOnlyMemory data, Cancella { return !firstWrite ? default : FlushAsyncInternal(cancellationToken); } - return WriteChunkedAsync(data.Span, cancellationToken); + + _requestProcessingStatus = RequestProcessingStatus.HeadersFlushed; + return Output.WriteChunkAsync(data.Span, cancellationToken); } else { @@ -1407,7 +1414,8 @@ public async ValueTask WriteAsyncAwaited(Task initializeTask, ReadO return await FlushAsyncInternal(cancellationToken); } - return await WriteChunkedAsync(data.Span, cancellationToken); + _requestProcessingStatus = RequestProcessingStatus.HeadersFlushed; + return await Output.WriteChunkAsync(data.Span, cancellationToken); } else { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs index 4b2b5362ebee..ace72f301309 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs @@ -299,11 +299,5 @@ ValueTask IHttpOutputProducer.WriteChunkAsync(ReadOnlySpan da { throw new NotImplementedException(); } - - public void Complete(Exception exception = null) - { - // TODO What to do with exception. - // and how to handle writing to response here. - } } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs index b2be3c2db8ee..291dd9bdb095 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs @@ -18,6 +18,7 @@ using Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests.TestTransport; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.Logging.Testing; +using Moq; using Xunit; namespace Microsoft.AspNetCore.Server.Kestrel.InMemory.FunctionalTests @@ -58,6 +59,32 @@ public async Task StreamsAreNotPersistedAcrossRequests() } } + [Fact] + public async Task PipesAreNotPersistedAcrossRequests() + { + var responseBodyPersisted = false; + PipeWriter bodyPipe = null; + using (var server = new TestServer(async context => + { + if (context.Response.BodyPipe == bodyPipe) + { + responseBodyPersisted = true; + } + bodyPipe = new StreamPipeWriter(new MemoryStream()); + context.Response.BodyPipe = bodyPipe; + + await context.Response.WriteAsync("hello, world"); + }, new TestServiceContext(LoggerFactory))) + { + Assert.Equal(string.Empty, await server.HttpClientSlim.GetStringAsync($"http://localhost:{server.Port}/")); + Assert.Equal(string.Empty, await server.HttpClientSlim.GetStringAsync($"http://localhost:{server.Port}/")); + + Assert.False(responseBodyPersisted); + + await server.StopAsync(); + } + } + [Fact] public async Task RequestBodyReadAsyncCanBeCancelled() { From ba59c4c28306c15b8766017ca82eb15c35814e2f Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Tue, 5 Feb 2019 13:53:10 -0800 Subject: [PATCH 12/33] Make HttpResponseWriteAsync good and react in tests. TODO many many tests --- .../HttpResponseWritingExtensions.cs | 62 ++++++++++++++++--- .../Core/src/Internal/Http/Http1Connection.cs | 3 +- .../src/Internal/Http/Http1OutputProducer.cs | 43 ++++++++++--- .../Internal/Http/HttpResponsePipeWriter.cs | 8 ++- .../Kestrel/Core/test/OutputProducerTests.cs | 5 +- .../ChunkedResponseTests.cs | 6 +- .../Http2/Http2StreamTests.cs | 19 +++--- .../InMemory.FunctionalTests/ResponseTests.cs | 4 +- 8 files changed, 117 insertions(+), 33 deletions(-) diff --git a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs index 315c6e150e99..cbbe3d72fb9d 100644 --- a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs +++ b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs @@ -61,21 +61,69 @@ public static class HttpResponseWritingExtensions throw new ArgumentNullException(nameof(encoding)); } - byte[] data = encoding.GetBytes(text); - var writeTask = response.BodyPipe.WriteAsync(data, cancellationToken); + if (!response.HasStarted) + { + var startAsyncTask = response.StartAsync(cancellationToken); + if (!startAsyncTask.IsCompletedSuccessfully) + { + return StartAndWriteAsyncAwaited(response, text, encoding, cancellationToken, startAsyncTask); + } + } + + Write(response, text, encoding); - if (writeTask.IsCompleted) + var flushAsyncTask = response.BodyPipe.FlushAsync(cancellationToken); + if (flushAsyncTask.IsCompleted) { - writeTask.GetAwaiter().GetResult(); + flushAsyncTask.GetAwaiter().GetResult(); return Task.CompletedTask; } - return WriteTaskAwaited(writeTask); + return flushAsyncTask.AsTask(); + } + + private static async Task StartAndWriteAsyncAwaited(this HttpResponse response, string text, Encoding encoding, CancellationToken cancellationToken, Task startAsyncTask) + { + await startAsyncTask; + Write(response, text, encoding); + await response.BodyPipe.FlushAsync(cancellationToken); + } + + private static void Write(this HttpResponse response, string text, Encoding encoding) + { + var pipeWriter = response.BodyPipe; + var encodedLength = encoding.GetByteCount(text); + var destination = pipeWriter.GetSpan(); + + if (encodedLength <= destination.Length) + { + var bytesWritten = encoding.GetBytes(text, destination); + pipeWriter.Advance(bytesWritten); + } + else + { + WriteMutliSegmentEncoded(pipeWriter, text, encoding, destination); + } } - private static async Task WriteTaskAwaited(ValueTask flushTask) + private static void WriteMutliSegmentEncoded(PipeWriter writer, string text, Encoding encoding, Span destination) { - await flushTask; + var encoder = encoding.GetEncoder(); + var readOnlySpan = text.AsSpan(); + var completed = false; + while (!completed) + { + encoder.Convert(readOnlySpan, destination, readOnlySpan.Length == 0, out var charsUsed, out var bytesUsed, out completed); + + writer.Advance(bytesUsed); + if (completed) + { + return; + } + readOnlySpan = readOnlySpan.Slice(charsUsed); + + destination = writer.GetSpan(readOnlySpan.Length); + } } } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs index 0219d2ca3a74..2baffd56fd17 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1Connection.cs @@ -52,7 +52,8 @@ public Http1Connection(HttpConnectionContext context) _context.ConnectionContext, _context.ServiceContext.Log, _context.TimeoutControl, - this); + this, + _context.MemoryPool); Input = _context.Transport.Input; Output = _http1Output; diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs index 8fa9e77a9311..1de3a40d07c5 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs @@ -26,6 +26,7 @@ public class Http1OutputProducer : IHttpOutputProducer, IHttpOutputAborter, IDis private readonly IKestrelTrace _log; private readonly IHttpMinResponseDataRateFeature _minResponseDataRateFeature; private readonly TimingPipeFlusher _flusher; + private readonly MemoryPool _memoryPool; // This locks access to all of the below fields private readonly object _contextLock = new object(); @@ -47,6 +48,7 @@ public class Http1OutputProducer : IHttpOutputProducer, IHttpOutputAborter, IDis private int _advancedBytesForChunk; private Memory _currentChunkMemory; private bool _currentChunkMemoryUpdated; + private IMemoryOwner _completedMemoryOwner; public Http1OutputProducer( PipeWriter pipeWriter, @@ -54,7 +56,8 @@ public Http1OutputProducer( ConnectionContext connectionContext, IKestrelTrace log, ITimeoutControl timeoutControl, - IHttpMinResponseDataRateFeature minResponseDataRateFeature) + IHttpMinResponseDataRateFeature minResponseDataRateFeature, + MemoryPool memoryPool) { _pipeWriter = pipeWriter; _connectionId = connectionId; @@ -62,6 +65,7 @@ public Http1OutputProducer( _log = log; _minResponseDataRateFeature = minResponseDataRateFeature; _flusher = new TimingPipeFlusher(pipeWriter, timeoutControl, log); + _memoryPool = memoryPool; } public Task WriteDataAsync(ReadOnlySpan buffer, CancellationToken cancellationToken = default) @@ -96,6 +100,15 @@ public ValueTask FlushAsync(CancellationToken cancellationToken = d public Memory GetMemory(int sizeHint = 0) { + if (_completed) + { + if (_completedMemoryOwner == null) + { + _completedMemoryOwner = _memoryPool.Rent(sizeHint); + } + return _completedMemoryOwner.Memory; + } + if (_autoChunk) { return GetChunkedMemory(sizeHint); @@ -108,6 +121,15 @@ public Memory GetMemory(int sizeHint = 0) public Span GetSpan(int sizeHint = 0) { + if (_completed) + { + if (_completedMemoryOwner == null) + { + _completedMemoryOwner = _memoryPool.Rent(sizeHint); + } + return _completedMemoryOwner.Memory.Span; + } + if (_autoChunk) { return GetChunkedMemory(sizeHint).Span; @@ -120,6 +142,11 @@ public Span GetSpan(int sizeHint = 0) public void Advance(int bytes) { + if (_completed) + { + return; + } + if (_autoChunk) { if (bytes < 0) @@ -203,14 +230,16 @@ public void Dispose() { lock (_contextLock) { - if (_completed) + if (_completedMemoryOwner != null) { - return; + _completedMemoryOwner.Dispose(); + } + if (!_completed) + { + _log.ConnectionDisconnect(_connectionId); + _completed = true; + _pipeWriter.Complete(); } - - _log.ConnectionDisconnect(_connectionId); - _completed = true; - _pipeWriter.Complete(); } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs index fa7629ebf9ea..c37c1da4e011 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs @@ -22,11 +22,13 @@ public HttpResponsePipeWriter(IHttpResponseControl pipeControl) public override void Advance(int bytes) { + ValidateState(); _pipeControl.Advance(bytes); } public override void CancelPendingFlush() { + ValidateState(); _pipeControl.CancelPendingFlush(); } @@ -44,17 +46,19 @@ public override ValueTask FlushAsync(CancellationToken cancellation public override Memory GetMemory(int sizeHint = 0) { + ValidateState(); return _pipeControl.GetMemory(sizeHint); } public override Span GetSpan(int sizeHint = 0) { + ValidateState(); return _pipeControl.GetSpan(sizeHint); } public override void OnReaderCompleted(Action callback, object state) { - // noop + throw new NotSupportedException(); } public override ValueTask WriteAsync(ReadOnlyMemory source, CancellationToken cancellationToken = default) @@ -89,7 +93,7 @@ public void Abort() } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void ValidateState(CancellationToken cancellationToken) + private void ValidateState(CancellationToken cancellationToken = default) { var state = _state; if (state == HttpStreamState.Open || state == HttpStreamState.Aborted) diff --git a/src/Servers/Kestrel/Core/test/OutputProducerTests.cs b/src/Servers/Kestrel/Core/test/OutputProducerTests.cs index 537e199e7491..749e2473a561 100644 --- a/src/Servers/Kestrel/Core/test/OutputProducerTests.cs +++ b/src/Servers/Kestrel/Core/test/OutputProducerTests.cs @@ -89,14 +89,15 @@ private TestHttpOutputProducer CreateOutputProducer( connectionContext, serviceContext.Log, Mock.Of(), - Mock.Of()); + Mock.Of(), + _memoryPool); return socketOutput; } private class TestHttpOutputProducer : Http1OutputProducer { - public TestHttpOutputProducer(Pipe pipe, string connectionId, ConnectionContext connectionContext, IKestrelTrace log, ITimeoutControl timeoutControl, IHttpMinResponseDataRateFeature minResponseDataRateFeature) : base(pipe.Writer, connectionId, connectionContext, log, timeoutControl, minResponseDataRateFeature) + public TestHttpOutputProducer(Pipe pipe, string connectionId, ConnectionContext connectionContext, IKestrelTrace log, ITimeoutControl timeoutControl, IHttpMinResponseDataRateFeature minResponseDataRateFeature, MemoryPool memoryPool) : base(pipe.Writer, connectionId, connectionContext, log, timeoutControl, minResponseDataRateFeature, memoryPool) { Pipe = pipe; } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs index 1cf062471612..383bb631a3ec 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ChunkedResponseTests.cs @@ -688,10 +688,8 @@ await connection.Receive( $"Date: {testContext.DateHeaderValue}", "Transfer-Encoding: chunked", "", - "6", - "Hello ", - "6", - "World!", + "c", + "Hello World!", "0", "", ""); diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 6e4900cd37ba..1e69a737847a 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -911,7 +911,7 @@ await InitializeConnectionAsync(async context => Assert.IsType(thrownEx.InnerException); } - [Fact] + [Fact] // TODO https://github.com/aspnet/AspNetCore/issues/7034 public async Task ContentLength_Response_FirstWriteMoreBytesWritten_Throws_Sends500() { var headers = new[] @@ -929,12 +929,12 @@ await InitializeConnectionAsync(async context => await StartStreamAsync(1, headers, endStream: true); var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, - withLength: 55, + withLength: 56, withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, withStreamId: 1); - await ExpectAsync(Http2FrameType.DATA, - withLength: 0, - withFlags: (byte)Http2DataFrameFlags.END_STREAM, + await ExpectAsync(Http2FrameType.RST_STREAM, + withLength: 4, + withFlags: (byte)Http2DataFrameFlags.NONE, withStreamId: 1); Assert.Contains(TestApplicationErrorLogger.Messages, m => m.Exception?.Message.Contains("Response Content-Length mismatch: too many bytes written (12 of 11).") ?? false); @@ -945,8 +945,8 @@ await ExpectAsync(Http2FrameType.DATA, Assert.Equal(3, _decodedHeaders.Count); Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); - Assert.Equal("500", _decodedHeaders[HeaderNames.Status]); - Assert.Equal("0", _decodedHeaders[HeaderNames.ContentLength]); + Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); + Assert.Equal("11", _decodedHeaders[HeaderNames.ContentLength]); } [Fact] @@ -3124,8 +3124,9 @@ await Assert.ThrowsAsync( await StartStreamAsync(1, headers, endStream: true); + // Don't receive content length because we called WriteAsync which caused an invalid response var headersFrame = await ExpectAsync(Http2FrameType.HEADERS, - withLength: 55, + withLength: 37, withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS, withStreamId: 1); @@ -3138,7 +3139,7 @@ await ExpectAsync(Http2FrameType.DATA, _hpackDecoder.Decode(headersFrame.PayloadSequence, endHeaders: false, handler: this); - Assert.Equal(3, _decodedHeaders.Count); + Assert.Equal(2, _decodedHeaders.Count); Assert.Contains("date", _decodedHeaders.Keys, StringComparer.OrdinalIgnoreCase); Assert.Equal("200", _decodedHeaders[HeaderNames.Status]); } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs index d011de09276e..47b865817a8f 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs @@ -3378,7 +3378,9 @@ await connection.Send( await connection.Receive( "HTTP/1.1 200 OK", $"Date: {server.Context.DateHeaderValue}", - "Content-Length: 0", + "Transfer-Encoding: chunked", + "", + "0", "", ""); } From 258485efb322c5f62c8fde1efaf763bd706349e3 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Tue, 5 Feb 2019 14:12:44 -0800 Subject: [PATCH 13/33] Pass in memory pool to streampipewriter --- .../Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs | 3 +-- .../perf/Kestrel.Performance/InMemoryTransportBenchmark.cs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs index 7682f684b79f..0128964504cc 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs @@ -202,8 +202,7 @@ PipeWriter IResponseBodyPipeFeature.ResponseBodyPipe if (!object.ReferenceEquals(_cachedResponseBodyStream, ResponseBody)) { - // TODO use kestrel's memory pool here - var responsePipeWriter = new StreamPipeWriter(ResponseBody); + var responsePipeWriter = new StreamPipeWriter(ResponseBody, 4096, _context.MemoryPool); ResponsePipeWriter = responsePipeWriter; _cachedResponseBodyStream = ResponseBody; if (_wrapperObjectsToDispose == null) diff --git a/src/Servers/Kestrel/perf/Kestrel.Performance/InMemoryTransportBenchmark.cs b/src/Servers/Kestrel/perf/Kestrel.Performance/InMemoryTransportBenchmark.cs index 1816531088fe..8242f3ba17ee 100644 --- a/src/Servers/Kestrel/perf/Kestrel.Performance/InMemoryTransportBenchmark.cs +++ b/src/Servers/Kestrel/perf/Kestrel.Performance/InMemoryTransportBenchmark.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using BenchmarkDotNet.Attributes; From 1e3c50a96e0488f064f8230251ddac8e21d4d553 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Tue, 5 Feb 2019 17:24:09 -0800 Subject: [PATCH 14/33] Improve WriteOnlyPipeStream and small nit --- src/Http/Http/src/WriteOnlyPipeStream.cs | 17 ++++++++++++++--- .../src/Internal/Http/Http1OutputProducer.cs | 2 +- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/Http/Http/src/WriteOnlyPipeStream.cs b/src/Http/Http/src/WriteOnlyPipeStream.cs index 8737696316fc..e3bf1b6e78fb 100644 --- a/src/Http/Http/src/WriteOnlyPipeStream.cs +++ b/src/Http/Http/src/WriteOnlyPipeStream.cs @@ -151,13 +151,24 @@ private Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken /// public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) { - return WriteAsync(new ReadOnlyMemory(buffer, offset, count), cancellationToken).AsTask(); + return WriteAsyncInternal(new ReadOnlyMemory(buffer, offset, count), cancellationToken); } /// - public override async ValueTask WriteAsync(ReadOnlyMemory source, CancellationToken cancellationToken = default) + public override ValueTask WriteAsync(ReadOnlyMemory source, CancellationToken cancellationToken = default) { - await InnerPipeWriter.WriteAsync(source, cancellationToken); + return new ValueTask(WriteAsyncInternal(source, cancellationToken)); + } + + private Task WriteAsyncInternal(ReadOnlyMemory source, CancellationToken cancellationToken = default) + { + var task = InnerPipeWriter.WriteAsync(source, cancellationToken); + if (task.IsCompletedSuccessfully) + { + task.GetAwaiter().GetResult(); + return Task.CompletedTask; + } + return task.AsTask(); } } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs index 1de3a40d07c5..a29a243e73ea 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs @@ -294,7 +294,6 @@ private ValueTask WriteAsync( var bytesWritten = _unflushedBytes; _unflushedBytes = 0; - _currentChunkMemoryUpdated = false; return _flusher.FlushAsync( _minResponseDataRateFeature.MinDataRate, @@ -361,6 +360,7 @@ private void WriteCurrentMemoryToPipeWriter() writer.Commit(); _advancedBytesForChunk = 0; _unflushedBytes += writer.BytesCommitted; + _currentChunkMemoryUpdated = false; } } } From 0dc48032163a7cc43439c95f1d58f672d959585e Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Tue, 5 Feb 2019 18:35:02 -0800 Subject: [PATCH 15/33] Attempted fix for BodyWrapper stream and check nonresponsebody writes in Advance --- .../Extensions/HttpResponseWritingExtensions.cs | 15 ++++++++------- .../ResponseCompression/src/BodyWrapperStream.cs | 7 +++++++ .../Core/src/Internal/Http/HttpProtocol.cs | 11 +++++++++-- 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs index cbbe3d72fb9d..98897114ebcd 100644 --- a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs +++ b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs @@ -75,6 +75,7 @@ public static class HttpResponseWritingExtensions var flushAsyncTask = response.BodyPipe.FlushAsync(cancellationToken); if (flushAsyncTask.IsCompleted) { + // Most implementations of ValueTask reset state in GetResult, so call it before returning a completed task. flushAsyncTask.GetAwaiter().GetResult(); return Task.CompletedTask; } @@ -93,7 +94,7 @@ private static void Write(this HttpResponse response, string text, Encoding enco { var pipeWriter = response.BodyPipe; var encodedLength = encoding.GetByteCount(text); - var destination = pipeWriter.GetSpan(); + var destination = pipeWriter.GetSpan(encodedLength); if (encodedLength <= destination.Length) { @@ -102,27 +103,27 @@ private static void Write(this HttpResponse response, string text, Encoding enco } else { - WriteMutliSegmentEncoded(pipeWriter, text, encoding, destination); + WriteMutliSegmentEncoded(pipeWriter, text, encoding, destination, encodedLength); } } - private static void WriteMutliSegmentEncoded(PipeWriter writer, string text, Encoding encoding, Span destination) + private static void WriteMutliSegmentEncoded(PipeWriter writer, string text, Encoding encoding, Span destination, int encodedLength) { var encoder = encoding.GetEncoder(); - var readOnlySpan = text.AsSpan(); + var source = text.AsSpan(); var completed = false; while (!completed) { - encoder.Convert(readOnlySpan, destination, readOnlySpan.Length == 0, out var charsUsed, out var bytesUsed, out completed); + encoder.Convert(source, destination, source.Length == 0, out var charsUsed, out var bytesUsed, out completed); writer.Advance(bytesUsed); if (completed) { return; } - readOnlySpan = readOnlySpan.Slice(charsUsed); + source = source.Slice(charsUsed); - destination = writer.GetSpan(readOnlySpan.Length); + destination = writer.GetSpan(encodedLength - bytesUsed); } } } diff --git a/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs b/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs index 43ad207d0855..db14edf56677 100644 --- a/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs +++ b/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs @@ -37,6 +37,7 @@ internal BodyWrapperStream(HttpContext context, Stream bodyOriginalStream, IResp _provider = provider; _innerBufferFeature = innerBufferFeature; _innerSendFileFeature = innerSendFileFeature; + _context.Response.OnStarting((b) => ((BodyWrapperStream)b).OnWriteWrapper(), this); } internal ValueTask FinishCompressionAsync() @@ -236,6 +237,12 @@ private void OnWrite() } } + private Task OnWriteWrapper() + { + OnWrite(); + return Task.CompletedTask; + } + private ICompressionProvider ResolveCompressionProvider() { if (!_providerCreated) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 195da950a529..2a46ae4a951c 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -1277,8 +1277,15 @@ protected void ReportApplicationError(Exception ex) public void Advance(int bytes) { - VerifyAndUpdateWrite(bytes); - Output.Advance(bytes); + if (_canWriteResponseBody) + { + VerifyAndUpdateWrite(bytes); + Output.Advance(bytes); + } + else + { + HandleNonBodyResponseWrite(); + } } public Memory GetMemory(int sizeHint = 0) From e308e5fe6b3270ca5a50c8bd3dd15a22ecab257c Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 6 Feb 2019 09:31:44 -0800 Subject: [PATCH 16/33] Fix a few tests for Advance scenarios, disable a few response compression tests, and add httpresponsepipewriter tests --- .../test/ResponseCompressionMiddlewareTest.cs | 8 +- .../src/Internal/Http/Http1OutputProducer.cs | 4 +- .../Core/src/Internal/Http/HttpProtocol.cs | 4 + .../Core/test/HttpResponsePipeWriterTests.cs | 84 +++++++++++++++++++ .../Core/test/HttpResponseStreamTests.cs | 29 ++++--- 5 files changed, 109 insertions(+), 20 deletions(-) create mode 100644 src/Servers/Kestrel/Core/test/HttpResponsePipeWriterTests.cs diff --git a/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs b/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs index aef87c545055..a2f27755a5eb 100644 --- a/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs +++ b/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs @@ -278,10 +278,10 @@ public async Task NoIncludedMimeTypes_UseDefaults() [Theory] [InlineData("")] - [InlineData("text/plain")] - [InlineData("text/PLAIN")] - [InlineData("text/plain; charset=ISO-8859-4")] - [InlineData("text/plain ; charset=ISO-8859-4")] + [InlineData("text/plain", Skip = "https://github.com/aspnet/AspNetCore/issues/7296")] + [InlineData("text/PLAIN", Skip = "https://github.com/aspnet/AspNetCore/issues/7296")] + [InlineData("text/plain; charset=ISO-8859-4", Skip = "https://github.com/aspnet/AspNetCore/issues/7296")] + [InlineData("text/plain ; charset=ISO-8859-4", Skip = "https://github.com/aspnet/AspNetCore/issues/7296")] [InlineData("text/plain2")] public async Task NoBody_NotCompressed(string contentType) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs index a29a243e73ea..ef8d5197075e 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs @@ -360,8 +360,10 @@ private void WriteCurrentMemoryToPipeWriter() writer.Commit(); _advancedBytesForChunk = 0; _unflushedBytes += writer.BytesCommitted; - _currentChunkMemoryUpdated = false; } + + // If there is an empty write, we still need to update the current chunk + _currentChunkMemoryUpdated = false; } } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 2a46ae4a951c..d38729a37b3b 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -1285,6 +1285,10 @@ public void Advance(int bytes) else { HandleNonBodyResponseWrite(); + + // For HEAD requests, we still use the number of bytes written for logging + // how many bytes were written. + VerifyAndUpdateWrite(bytes); } } diff --git a/src/Servers/Kestrel/Core/test/HttpResponsePipeWriterTests.cs b/src/Servers/Kestrel/Core/test/HttpResponsePipeWriterTests.cs new file mode 100644 index 000000000000..9f57f1dffb32 --- /dev/null +++ b/src/Servers/Kestrel/Core/test/HttpResponsePipeWriterTests.cs @@ -0,0 +1,84 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests +{ + public class HttpResponsePipeWriterTests + { + [Fact] + public void OnReaderCompletedThrowsNotSupported() + { + var pipeWriter = CreateHttpResponsePipeWriter(); + Assert.Throws(() => pipeWriter.OnReaderCompleted((a, b) => { }, null)); + } + + [Fact] + public void AdvanceAfterStopAcceptingWritesThrowsObjectDisposedException() + { + var pipeWriter = CreateHttpResponsePipeWriter(); + pipeWriter.StartAcceptingWrites(); + pipeWriter.StopAcceptingWrites(); + var ex = Assert.Throws(() => { pipeWriter.Advance(1); }); + Assert.Contains(CoreStrings.WritingToResponseBodyAfterResponseCompleted, ex.Message); + } + + [Fact] + public void GetMemoryAfterStopAcceptingWritesThrowsObjectDisposedException() + { + var pipeWriter = CreateHttpResponsePipeWriter(); + pipeWriter.StartAcceptingWrites(); + pipeWriter.StopAcceptingWrites(); + var ex = Assert.Throws(() => { pipeWriter.GetMemory(); }); + Assert.Contains(CoreStrings.WritingToResponseBodyAfterResponseCompleted, ex.Message); + } + + [Fact] + public void GetSpanAfterStopAcceptingWritesThrowsObjectDisposedException() + { + var pipeWriter = CreateHttpResponsePipeWriter(); + pipeWriter.StartAcceptingWrites(); + pipeWriter.StopAcceptingWrites(); + var ex = Assert.Throws(() => { pipeWriter.GetSpan(); }); + Assert.Contains(CoreStrings.WritingToResponseBodyAfterResponseCompleted, ex.Message); + } + + [Fact] + public void FlushAsyncAfterStopAcceptingWritesThrowsObjectDisposedException() + { + var pipeWriter = CreateHttpResponsePipeWriter(); + pipeWriter.StartAcceptingWrites(); + pipeWriter.StopAcceptingWrites(); + var ex = Assert.Throws(() => { pipeWriter.FlushAsync(); }); + Assert.Contains(CoreStrings.WritingToResponseBodyAfterResponseCompleted, ex.Message); + } + + [Fact] + public void WriteAsyncAfterStopAcceptingWritesThrowsObjectDisposedException() + { + var pipeWriter = CreateHttpResponsePipeWriter(); + pipeWriter.StartAcceptingWrites(); + pipeWriter.StopAcceptingWrites(); + var ex = Assert.Throws(() => { pipeWriter.WriteAsync(new Memory()); }); + Assert.Contains(CoreStrings.WritingToResponseBodyAfterResponseCompleted, ex.Message); + } + + [Fact] + public void CompleteCallsStopAcceptingWrites() + { + var pipeWriter = CreateHttpResponsePipeWriter(); + pipeWriter.Complete(); + var ex = Assert.Throws(() => { pipeWriter.WriteAsync(new Memory()); }); + Assert.Contains(CoreStrings.WritingToResponseBodyAfterResponseCompleted, ex.Message); + } + + private static HttpResponsePipeWriter CreateHttpResponsePipeWriter() + { + return new HttpResponsePipeWriter(Mock.Of()); + } + } +} diff --git a/src/Servers/Kestrel/Core/test/HttpResponseStreamTests.cs b/src/Servers/Kestrel/Core/test/HttpResponseStreamTests.cs index 883faaf2288c..3f00c58e0acc 100644 --- a/src/Servers/Kestrel/Core/test/HttpResponseStreamTests.cs +++ b/src/Servers/Kestrel/Core/test/HttpResponseStreamTests.cs @@ -18,90 +18,89 @@ public class HttpResponseStreamTests [Fact] public void CanReadReturnsFalse() { - var stream = CreateMockHttpResponseStream(); + var stream = CreateHttpResponseStream(); Assert.False(stream.CanRead); } [Fact] public void CanSeekReturnsFalse() { - var stream = CreateMockHttpResponseStream(); + var stream = CreateHttpResponseStream(); Assert.False(stream.CanSeek); } [Fact] public void CanWriteReturnsTrue() { - var stream = CreateMockHttpResponseStream(); + var stream = CreateHttpResponseStream(); Assert.True(stream.CanWrite); } [Fact] public void ReadThrows() { - var stream = CreateMockHttpResponseStream(); + var stream = CreateHttpResponseStream(); Assert.Throws(() => stream.Read(new byte[1], 0, 1)); } [Fact] public void ReadByteThrows() { - var stream = CreateMockHttpResponseStream(); + var stream = CreateHttpResponseStream(); Assert.Throws(() => stream.ReadByte()); } [Fact] public async Task ReadAsyncThrows() { - var stream = CreateMockHttpResponseStream(); + var stream = CreateHttpResponseStream(); await Assert.ThrowsAsync(() => stream.ReadAsync(new byte[1], 0, 1)); } [Fact] public void BeginReadThrows() { - var stream = CreateMockHttpResponseStream(); + var stream = CreateHttpResponseStream(); Assert.Throws(() => stream.BeginRead(new byte[1], 0, 1, null, null)); } [Fact] public void SeekThrows() { - var stream = CreateMockHttpResponseStream(); + var stream = CreateHttpResponseStream(); Assert.Throws(() => stream.Seek(0, SeekOrigin.Begin)); } [Fact] public void LengthThrows() { - var stream = CreateMockHttpResponseStream(); + var stream = CreateHttpResponseStream(); Assert.Throws(() => stream.Length); } [Fact] public void SetLengthThrows() { - var stream = CreateMockHttpResponseStream(); + var stream = CreateHttpResponseStream(); Assert.Throws(() => stream.SetLength(0)); } [Fact] public void PositionThrows() { - var stream = CreateMockHttpResponseStream(); + var stream = CreateHttpResponseStream(); Assert.Throws(() => stream.Position); Assert.Throws(() => stream.Position = 0); } [Fact] - public async Task StopAcceptingWritesCausesWriteToThrowObjectDisposedException() + public void StopAcceptingWritesCausesWriteToThrowObjectDisposedException() { var pipeWriter = new HttpResponsePipeWriter(Mock.Of()); var stream = new HttpResponseStream(Mock.Of(), pipeWriter); stream.StartAcceptingWrites(); stream.StopAcceptingWrites(); - // This test had to change to awaiting the stream.WriteAsync - var ex = await Assert.ThrowsAsync(async () => { await stream.WriteAsync(new byte[1], 0, 1); }); + var ex = Assert.Throws(() => { stream.WriteAsync(new byte[1], 0, 1); }); Assert.Contains(CoreStrings.WritingToResponseBodyAfterResponseCompleted, ex.Message); } @@ -129,7 +128,7 @@ public async Task SynchronousWritesThrowIfDisallowedByIHttpBodyControlFeature() stream.Write(new byte[1], 0, 1); } - private static HttpResponseStream CreateMockHttpResponseStream() + private static HttpResponseStream CreateHttpResponseStream() { var pipeWriter = new HttpResponsePipeWriter(Mock.Of()); return new HttpResponseStream(Mock.Of(), pipeWriter); From 532c63d69555737230a0984e1370bcf7a4a6bcf0 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 6 Feb 2019 11:01:51 -0800 Subject: [PATCH 17/33] Add tests for response writing with encoding --- .../HttpResponseWritingExtensions.cs | 11 ++-- .../HttpResponseWritingExtensionsTests.cs | 59 +++++++++++++++++++ ....AspNetCore.Http.Abstractions.Tests.csproj | 4 ++ 3 files changed, 68 insertions(+), 6 deletions(-) diff --git a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs index 98897114ebcd..f77b65307413 100644 --- a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs +++ b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs @@ -112,18 +112,17 @@ private static void WriteMutliSegmentEncoded(PipeWriter writer, string text, Enc var encoder = encoding.GetEncoder(); var source = text.AsSpan(); var completed = false; - while (!completed) + var totalBytesUsed = 0; + + while (!completed || encodedLength - totalBytesUsed != 0) { encoder.Convert(source, destination, source.Length == 0, out var charsUsed, out var bytesUsed, out completed); + totalBytesUsed += bytesUsed; writer.Advance(bytesUsed); - if (completed) - { - return; - } source = source.Slice(charsUsed); - destination = writer.GetSpan(encodedLength - bytesUsed); + destination = writer.GetSpan(encodedLength - totalBytesUsed); } } } diff --git a/src/Http/Http.Abstractions/test/HttpResponseWritingExtensionsTests.cs b/src/Http/Http.Abstractions/test/HttpResponseWritingExtensionsTests.cs index f8e9e27d1ce9..f7dd3712bd1f 100644 --- a/src/Http/Http.Abstractions/test/HttpResponseWritingExtensionsTests.cs +++ b/src/Http/Http.Abstractions/test/HttpResponseWritingExtensionsTests.cs @@ -2,6 +2,9 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.IO; +using System.IO.Pipelines; +using System.IO.Pipelines.Tests; +using System.Text; using System.Threading.Tasks; using Xunit; @@ -28,6 +31,62 @@ public async Task WritingText_MultipleWrites() Assert.Equal(22, context.Response.Body.Length); } + [Theory] + [MemberData(nameof(Encodings))] + public async Task WritingTextThatRequiresMultipleSegmentsWorks(Encoding encoding) + { + // Need to change the StreamPipeWriter with a capped MemoryPool + var memoryPool = new TestMemoryPool(maxBufferSize: 16); + var outputStream = new MemoryStream(); + var streamPipeWriter = new StreamPipeWriter(outputStream, minimumSegmentSize: 0, memoryPool); + + HttpContext context = new DefaultHttpContext(); + context.Response.BodyPipe = streamPipeWriter; + + var inputString = "丂丂丂丂丂丂丂丂丂丂丂丂丂丂丂"; + var expected = encoding.GetBytes(inputString); + await context.Response.WriteAsync(inputString, encoding); + + outputStream.Position = 0; + var actual = new byte[expected.Length]; + var length = outputStream.Read(actual); + + var res1 = encoding.GetString(actual); + var res2 = encoding.GetString(expected); + Assert.Equal(expected.Length, length); + Assert.Equal(expected, actual); + streamPipeWriter.Complete(); + } + + public static TheoryData Encodings => + new TheoryData + { + { Encoding.ASCII }, + { Encoding.BigEndianUnicode }, + { Encoding.Unicode }, + { Encoding.UTF32 }, + { Encoding.UTF7 }, + { Encoding.UTF8 } + }; + + [Theory] + [MemberData(nameof(Encodings))] + public async Task WritingTextWithPassedInEncodingWorks(Encoding encoding) + { + HttpContext context = CreateRequest(); + + var inputString = "丂丂丂丂丂"; + var expected = encoding.GetBytes(inputString); + await context.Response.WriteAsync(inputString, encoding); + + context.Response.Body.Position = 0; + var actual = new byte[expected.Length]; + var length = context.Response.Body.Read(actual); + + Assert.Equal(expected.Length, length); + Assert.Equal(expected, actual); + } + private HttpContext CreateRequest() { HttpContext context = new DefaultHttpContext(); diff --git a/src/Http/Http.Abstractions/test/Microsoft.AspNetCore.Http.Abstractions.Tests.csproj b/src/Http/Http.Abstractions/test/Microsoft.AspNetCore.Http.Abstractions.Tests.csproj index 4b3a83f1d993..962602a87265 100644 --- a/src/Http/Http.Abstractions/test/Microsoft.AspNetCore.Http.Abstractions.Tests.csproj +++ b/src/Http/Http.Abstractions/test/Microsoft.AspNetCore.Http.Abstractions.Tests.csproj @@ -4,6 +4,10 @@ netcoreapp3.0 + + + + From 2b227964265b0cac65bf4b5aed3a248f5d2fd401 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 6 Feb 2019 11:23:02 -0800 Subject: [PATCH 18/33] Shim StartAsync --- .../ResponseCompression/src/BodyWrapperStream.cs | 13 ++++++++++--- .../src/ResponseCompressionMiddleware.cs | 14 ++++++++++++-- .../test/BodyWrapperStreamTests.cs | 12 ++++++------ .../test/ResponseCompressionMiddlewareTest.cs | 8 ++++---- .../WebSockets/samples/EchoApp/Program.cs | 2 +- 5 files changed, 33 insertions(+), 16 deletions(-) diff --git a/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs b/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs index db14edf56677..a22d605ee130 100644 --- a/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs +++ b/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs @@ -15,13 +15,14 @@ namespace Microsoft.AspNetCore.ResponseCompression /// /// Stream wrapper that create specific compression stream only if necessary. /// - internal class BodyWrapperStream : Stream, IHttpBufferingFeature, IHttpSendFileFeature + internal class BodyWrapperStream : Stream, IHttpBufferingFeature, IHttpSendFileFeature, IHttpResponseStartFeature { private readonly HttpContext _context; private readonly Stream _bodyOriginalStream; private readonly IResponseCompressionProvider _provider; private readonly IHttpBufferingFeature _innerBufferFeature; private readonly IHttpSendFileFeature _innerSendFileFeature; + private readonly IHttpResponseStartFeature _innerStartFeature; private ICompressionProvider _compressionProvider = null; private bool _compressionChecked = false; @@ -30,14 +31,14 @@ internal class BodyWrapperStream : Stream, IHttpBufferingFeature, IHttpSendFileF private bool _autoFlush = false; internal BodyWrapperStream(HttpContext context, Stream bodyOriginalStream, IResponseCompressionProvider provider, - IHttpBufferingFeature innerBufferFeature, IHttpSendFileFeature innerSendFileFeature) + IHttpBufferingFeature innerBufferFeature, IHttpSendFileFeature innerSendFileFeature, IHttpResponseStartFeature innerStartFeature) { _context = context; _bodyOriginalStream = bodyOriginalStream; _provider = provider; _innerBufferFeature = innerBufferFeature; _innerSendFileFeature = innerSendFileFeature; - _context.Response.OnStarting((b) => ((BodyWrapperStream)b).OnWriteWrapper(), this); + _innerStartFeature = innerStartFeature; } internal ValueTask FinishCompressionAsync() @@ -325,5 +326,11 @@ private async Task InnerSendFileAsync(string path, long offset, long? count, Can } } } + + public Task StartAsync(CancellationToken token = default) + { + OnWrite(); + return _innerStartFeature.StartAsync(token); + } } } diff --git a/src/Middleware/ResponseCompression/src/ResponseCompressionMiddleware.cs b/src/Middleware/ResponseCompression/src/ResponseCompressionMiddleware.cs index 3f593c1e2076..5c8b6447545a 100644 --- a/src/Middleware/ResponseCompression/src/ResponseCompressionMiddleware.cs +++ b/src/Middleware/ResponseCompression/src/ResponseCompressionMiddleware.cs @@ -5,7 +5,6 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; -using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.ResponseCompression { @@ -55,9 +54,10 @@ public async Task Invoke(HttpContext context) var bodyStream = context.Response.Body; var originalBufferFeature = context.Features.Get(); var originalSendFileFeature = context.Features.Get(); + var originalStartFeature = context.Features.Get(); var bodyWrapperStream = new BodyWrapperStream(context, bodyStream, _provider, - originalBufferFeature, originalSendFileFeature); + originalBufferFeature, originalSendFileFeature, originalStartFeature); context.Response.Body = bodyWrapperStream; context.Features.Set(bodyWrapperStream); if (originalSendFileFeature != null) @@ -65,6 +65,11 @@ public async Task Invoke(HttpContext context) context.Features.Set(bodyWrapperStream); } + if (originalStartFeature != null) + { + context.Features.Set(bodyWrapperStream); + } + try { await _next(context); @@ -78,6 +83,11 @@ public async Task Invoke(HttpContext context) { context.Features.Set(originalSendFileFeature); } + + if (originalStartFeature != null) + { + context.Features.Set(originalStartFeature); + } } } } diff --git a/src/Middleware/ResponseCompression/test/BodyWrapperStreamTests.cs b/src/Middleware/ResponseCompression/test/BodyWrapperStreamTests.cs index 4ffd70b745e3..28679194ec52 100644 --- a/src/Middleware/ResponseCompression/test/BodyWrapperStreamTests.cs +++ b/src/Middleware/ResponseCompression/test/BodyWrapperStreamTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; @@ -26,7 +26,7 @@ public void OnWrite_AppendsAcceptEncodingToVaryHeader_IfNotPresent(string provid { var httpContext = new DefaultHttpContext(); httpContext.Response.Headers[HeaderNames.Vary] = providedVaryHeader; - var stream = new BodyWrapperStream(httpContext, new MemoryStream(), new MockResponseCompressionProvider(flushable: true), null, null); + var stream = new BodyWrapperStream(httpContext, new MemoryStream(), new MockResponseCompressionProvider(flushable: true), null, null, null); stream.Write(new byte[] { }, 0, 0); @@ -51,7 +51,7 @@ public void Write_IsPassedToUnderlyingStream_WhenDisableResponseBuffering(bool f written = new ArraySegment(b, 0, c).ToArray(); }); - var stream = new BodyWrapperStream(new DefaultHttpContext(), mock.Object, new MockResponseCompressionProvider(flushable), null, null); + var stream = new BodyWrapperStream(new DefaultHttpContext(), mock.Object, new MockResponseCompressionProvider(flushable), null, null, null); stream.DisableResponseBuffering(); stream.Write(buffer, 0, buffer.Length); @@ -67,7 +67,7 @@ public async Task WriteAsync_IsPassedToUnderlyingStream_WhenDisableResponseBuffe var buffer = new byte[] { 1 }; var memoryStream = new MemoryStream(); - var stream = new BodyWrapperStream(new DefaultHttpContext(), memoryStream, new MockResponseCompressionProvider(flushable), null, null); + var stream = new BodyWrapperStream(new DefaultHttpContext(), memoryStream, new MockResponseCompressionProvider(flushable), null, null, null); stream.DisableResponseBuffering(); await stream.WriteAsync(buffer, 0, buffer.Length); @@ -80,7 +80,7 @@ public async Task SendFileAsync_IsPassedToUnderlyingStream_WhenDisableResponseBu { var memoryStream = new MemoryStream(); - var stream = new BodyWrapperStream(new DefaultHttpContext(), memoryStream, new MockResponseCompressionProvider(true), null, null); + var stream = new BodyWrapperStream(new DefaultHttpContext(), memoryStream, new MockResponseCompressionProvider(true), null, null, null); stream.DisableResponseBuffering(); @@ -99,7 +99,7 @@ public void BeginWrite_IsPassedToUnderlyingStream_WhenDisableResponseBuffering(b var memoryStream = new MemoryStream(); - var stream = new BodyWrapperStream(new DefaultHttpContext(), memoryStream, new MockResponseCompressionProvider(flushable), null, null); + var stream = new BodyWrapperStream(new DefaultHttpContext(), memoryStream, new MockResponseCompressionProvider(flushable), null, null, null); stream.DisableResponseBuffering(); stream.BeginWrite(buffer, 0, buffer.Length, (o) => {}, null); diff --git a/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs b/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs index a2f27755a5eb..aef87c545055 100644 --- a/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs +++ b/src/Middleware/ResponseCompression/test/ResponseCompressionMiddlewareTest.cs @@ -278,10 +278,10 @@ public async Task NoIncludedMimeTypes_UseDefaults() [Theory] [InlineData("")] - [InlineData("text/plain", Skip = "https://github.com/aspnet/AspNetCore/issues/7296")] - [InlineData("text/PLAIN", Skip = "https://github.com/aspnet/AspNetCore/issues/7296")] - [InlineData("text/plain; charset=ISO-8859-4", Skip = "https://github.com/aspnet/AspNetCore/issues/7296")] - [InlineData("text/plain ; charset=ISO-8859-4", Skip = "https://github.com/aspnet/AspNetCore/issues/7296")] + [InlineData("text/plain")] + [InlineData("text/PLAIN")] + [InlineData("text/plain; charset=ISO-8859-4")] + [InlineData("text/plain ; charset=ISO-8859-4")] [InlineData("text/plain2")] public async Task NoBody_NotCompressed(string contentType) { diff --git a/src/Middleware/WebSockets/samples/EchoApp/Program.cs b/src/Middleware/WebSockets/samples/EchoApp/Program.cs index 2775a095ce68..07dd98e134be 100644 --- a/src/Middleware/WebSockets/samples/EchoApp/Program.cs +++ b/src/Middleware/WebSockets/samples/EchoApp/Program.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using System.IO; using System.Linq; From 5b213c2a989e0a345a11be92069b961bd915f893 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 6 Feb 2019 11:31:33 -0800 Subject: [PATCH 19/33] Add some comments --- .../src/Extensions/HttpResponseWritingExtensions.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs index f77b65307413..8da5b5faa6fe 100644 --- a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs +++ b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs @@ -61,6 +61,7 @@ public static class HttpResponseWritingExtensions throw new ArgumentNullException(nameof(encoding)); } + // Need to call StartAsync before GetMemory/GetSpan if (!response.HasStarted) { var startAsyncTask = response.StartAsync(cancellationToken); @@ -98,6 +99,7 @@ private static void Write(this HttpResponse response, string text, Encoding enco if (encodedLength <= destination.Length) { + // Just call Encoding.GetBytes if everything will fit into a single segment. var bytesWritten = encoding.GetBytes(text, destination); pipeWriter.Advance(bytesWritten); } @@ -114,6 +116,8 @@ private static void WriteMutliSegmentEncoded(PipeWriter writer, string text, Enc var completed = false; var totalBytesUsed = 0; + // This may be a bug, but encoder.Convert returns completed = true for UTF7 too early. + // Therefore, we check encodedLength - totalBytesUsed too. while (!completed || encodedLength - totalBytesUsed != 0) { encoder.Convert(source, destination, source.Length == 0, out var charsUsed, out var bytesUsed, out completed); From 7d9647a84403edd1080bb09bb05583bf03b7c29b Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 6 Feb 2019 15:30:15 -0800 Subject: [PATCH 20/33] Avoid throwing in writes as much as possible and general cleanup --- .../HttpResponseWritingExtensionsTests.cs | 22 +++--- src/Http/Http/src/WriteOnlyPipeStream.cs | 3 + .../test/Internal/DefaultHttpResponseTests.cs | 16 ++++ src/Http/Http/test/ReadOnlyPipeStreamTests.cs | 7 ++ src/Http/Http/test/StreamPipeReaderTests.cs | 5 ++ src/Http/Http/test/StreamPipeWriterTests.cs | 6 ++ .../Http/test/WriteOnlyPipeStreamTests.cs | 7 ++ .../src/BodyWrapperStream.cs | 6 -- .../src/Internal/Http/Http1OutputProducer.cs | 22 ++++-- .../Core/src/Internal/Http/HttpProtocol.cs | 1 + .../Internal/Http/HttpResponsePipeWriter.cs | 1 - .../src/Internal/Http/IHttpOutputProducer.cs | 1 + .../src/Internal/Http2/Http2OutputProducer.cs | 5 ++ .../Http2/Http2StreamTests.cs | 9 ++- .../InMemory.FunctionalTests/ResponseTests.cs | 73 ++++++++++++++++++- 15 files changed, 155 insertions(+), 29 deletions(-) diff --git a/src/Http/Http.Abstractions/test/HttpResponseWritingExtensionsTests.cs b/src/Http/Http.Abstractions/test/HttpResponseWritingExtensionsTests.cs index f7dd3712bd1f..d07a263880d5 100644 --- a/src/Http/Http.Abstractions/test/HttpResponseWritingExtensionsTests.cs +++ b/src/Http/Http.Abstractions/test/HttpResponseWritingExtensionsTests.cs @@ -58,17 +58,6 @@ public async Task WritingTextThatRequiresMultipleSegmentsWorks(Encoding encoding streamPipeWriter.Complete(); } - public static TheoryData Encodings => - new TheoryData - { - { Encoding.ASCII }, - { Encoding.BigEndianUnicode }, - { Encoding.Unicode }, - { Encoding.UTF32 }, - { Encoding.UTF7 }, - { Encoding.UTF8 } - }; - [Theory] [MemberData(nameof(Encodings))] public async Task WritingTextWithPassedInEncodingWorks(Encoding encoding) @@ -87,6 +76,17 @@ public async Task WritingTextWithPassedInEncodingWorks(Encoding encoding) Assert.Equal(expected, actual); } + public static TheoryData Encodings => + new TheoryData + { + { Encoding.ASCII }, + { Encoding.BigEndianUnicode }, + { Encoding.Unicode }, + { Encoding.UTF32 }, + { Encoding.UTF7 }, + { Encoding.UTF8 } + }; + private HttpContext CreateRequest() { HttpContext context = new DefaultHttpContext(); diff --git a/src/Http/Http/src/WriteOnlyPipeStream.cs b/src/Http/Http/src/WriteOnlyPipeStream.cs index e3bf1b6e78fb..ada377b3001c 100644 --- a/src/Http/Http/src/WriteOnlyPipeStream.cs +++ b/src/Http/Http/src/WriteOnlyPipeStream.cs @@ -163,11 +163,14 @@ public override ValueTask WriteAsync(ReadOnlyMemory source, CancellationTo private Task WriteAsyncInternal(ReadOnlyMemory source, CancellationToken cancellationToken = default) { var task = InnerPipeWriter.WriteAsync(source, cancellationToken); + if (task.IsCompletedSuccessfully) { + // Most ValueTask implementations reset in GetResult, so call it before returning completed task task.GetAwaiter().GetResult(); return Task.CompletedTask; } + return task.AsTask(); } } diff --git a/src/Http/Http/test/Internal/DefaultHttpResponseTests.cs b/src/Http/Http/test/Internal/DefaultHttpResponseTests.cs index 3dfffb7ede9d..1dbab3587f99 100644 --- a/src/Http/Http/test/Internal/DefaultHttpResponseTests.cs +++ b/src/Http/Http/test/Internal/DefaultHttpResponseTests.cs @@ -116,6 +116,22 @@ public async Task ResponseStart_CallsFeatureIfSet() mock.Verify(m => m.StartAsync(default), Times.Once()); } + [Fact] + public async Task ResponseStart_CallsFeatureIfSetWithProvidedCancellationToken() + { + var features = new FeatureCollection(); + + var mock = new Mock(); + var ct = new CancellationToken(); + mock.Setup(o => o.StartAsync(It.Is((localCt) => localCt.Equals(ct)))).Returns(Task.CompletedTask); + features.Set(mock.Object); + + var context = new DefaultHttpContext(features); + await context.Response.StartAsync(ct); + + mock.Verify(m => m.StartAsync(default), Times.Once()); + } + [Fact] public async Task ResponseStart_CallsResponseBodyFlushIfNotSet() { diff --git a/src/Http/Http/test/ReadOnlyPipeStreamTests.cs b/src/Http/Http/test/ReadOnlyPipeStreamTests.cs index 427c30696a20..fe1e5f8613d9 100644 --- a/src/Http/Http/test/ReadOnlyPipeStreamTests.cs +++ b/src/Http/Http/test/ReadOnlyPipeStreamTests.cs @@ -150,6 +150,13 @@ public void BlockSyncIOThrows() Assert.Throws(() => readOnlyPipeStream.Read(new byte[0], 0, 0)); } + [Fact] + public void InnerPipeReaderReturnsPipeReader() + { + var readOnlyPipeStream = new ReadOnlyPipeStream(Reader, allowSynchronousIO: false); + Assert.Equal(Reader, readOnlyPipeStream.InnerPipeReader); + } + private async Task> SetupMockPipeReader() { await WriteByteArrayToPipeAsync(new byte[1]); diff --git a/src/Http/Http/test/StreamPipeReaderTests.cs b/src/Http/Http/test/StreamPipeReaderTests.cs index 8d777110f168..aa08cbfaf0b9 100644 --- a/src/Http/Http/test/StreamPipeReaderTests.cs +++ b/src/Http/Http/test/StreamPipeReaderTests.cs @@ -610,6 +610,11 @@ public async Task ReadAsyncAfterReceivingCompletedReadResultDoesNotThrow() Assert.True(readResult.Buffer.IsEmpty); Assert.True(readResult.IsCompleted); } + + public void InnerStreamReturnsStream() + { + Assert.Equal(MemoryStream, ((StreamPipeReader)Reader).InnerStream); + } private async Task ReadFromPipeAsString() { diff --git a/src/Http/Http/test/StreamPipeWriterTests.cs b/src/Http/Http/test/StreamPipeWriterTests.cs index 5095de9d330d..1160d5759231 100644 --- a/src/Http/Http/test/StreamPipeWriterTests.cs +++ b/src/Http/Http/test/StreamPipeWriterTests.cs @@ -403,6 +403,12 @@ void NonAsyncMethod() Assert.Equal(expectedString, ReadAsString()); } + [Fact] + public void InnerStreamReturnsStream() + { + Assert.Equal(MemoryStream, ((StreamPipeWriter)Writer).InnerStream); + } + private void WriteStringToStream(string input) { var buffer = Encoding.ASCII.GetBytes(input); diff --git a/src/Http/Http/test/WriteOnlyPipeStreamTests.cs b/src/Http/Http/test/WriteOnlyPipeStreamTests.cs index c6ee97660c26..4f4bbc19e3a1 100644 --- a/src/Http/Http/test/WriteOnlyPipeStreamTests.cs +++ b/src/Http/Http/test/WriteOnlyPipeStreamTests.cs @@ -195,5 +195,12 @@ public void BlockSyncIOThrows() Assert.Throws(() => writeOnlyPipeStream.Write(new byte[0], 0, 0)); Assert.Throws(() => writeOnlyPipeStream.Flush()); } + + [Fact] + public void InnerPipeWriterReturnsPipeWriter() + { + var writeOnlyPipeStream = new WriteOnlyPipeStream(Writer, allowSynchronousIO: false); + Assert.Equal(Writer, writeOnlyPipeStream.InnerPipeWriter); + } } } diff --git a/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs b/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs index a22d605ee130..0dd8fd57d5a5 100644 --- a/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs +++ b/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs @@ -238,12 +238,6 @@ private void OnWrite() } } - private Task OnWriteWrapper() - { - OnWrite(); - return Task.CompletedTask; - } - private ICompressionProvider ResolveCompressionProvider() { if (!_providerCreated) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs index ef8d5197075e..60201d9a35b7 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs @@ -31,7 +31,8 @@ public class Http1OutputProducer : IHttpOutputProducer, IHttpOutputAborter, IDis // This locks access to all of the below fields private readonly object _contextLock = new object(); - private bool _completed = false; + private bool _pipeWriterCompleted; + private bool _completed; private bool _aborted; private long _unflushedBytes; private bool _autoChunk; @@ -176,7 +177,7 @@ public ValueTask WriteChunkAsync(ReadOnlySpan buffer, Cancell { lock (_contextLock) { - if (_completed) + if (_pipeWriterCompleted) { return default; } @@ -205,7 +206,7 @@ public void WriteResponseHeaders(int statusCode, string reasonPhrase, HttpRespon { lock (_contextLock) { - if (_completed) + if (_pipeWriterCompleted) { return; } @@ -233,10 +234,13 @@ public void Dispose() if (_completedMemoryOwner != null) { _completedMemoryOwner.Dispose(); + _completedMemoryOwner = null; } - if (!_completed) + + if (!_pipeWriterCompleted) { _log.ConnectionDisconnect(_connectionId); + _pipeWriterCompleted = true; _completed = true; _pipeWriter.Complete(); } @@ -260,6 +264,14 @@ public void Abort(ConnectionAbortedException error) } } + public void Complete(Exception ex) + { + lock (_contextLock) + { + _completed = true; + } + } + public ValueTask Write100ContinueAsync() { return WriteAsync(_continueBytes.Span); @@ -271,7 +283,7 @@ private ValueTask WriteAsync( { lock (_contextLock) { - if (_completed) + if (_pipeWriterCompleted) { return default; } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index d38729a37b3b..6a9d9d7ab5cf 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -1347,6 +1347,7 @@ public void Complete(Exception ex) ApplicationAbort(); } } + Output.Complete(ex); } public ValueTask WritePipeAsync(ReadOnlyMemory data, CancellationToken cancellationToken) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs index c37c1da4e011..0cc5a65fa0b2 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpResponsePipeWriter.cs @@ -34,7 +34,6 @@ public override void CancelPendingFlush() public override void Complete(Exception exception = null) { - StopAcceptingWrites(); _pipeControl.Complete(exception); } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs index 27f474b8669c..3e6e1dbff477 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs @@ -22,5 +22,6 @@ public interface IHttpOutputProducer Span GetSpan(int sizeHint = 0); Memory GetMemory(int sizeHint = 0); void CancelPendingFlush(); + void Complete(Exception ex); } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs index ace72f301309..fe4845ca4e73 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs @@ -299,5 +299,10 @@ ValueTask IHttpOutputProducer.WriteChunkAsync(ReadOnlySpan da { throw new NotImplementedException(); } + + public void Complete(Exception ex) + { + //_dataPipe.Writer.Complete(ex); + } } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 1e69a737847a..41845c3f3713 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -2530,7 +2530,8 @@ await InitializeConnectionAsync(async httpContext => var response = httpContext.Response; await response.StartAsync(); - var memory = response.BodyPipe.GetMemory(5000); // This will return 4096 + var memory = response.BodyPipe.GetMemory(5000); + Assert.Equal(4096, memory.Length); var fisrtPartOfResponse = Encoding.ASCII.GetBytes(new string('a', memory.Length)); fisrtPartOfResponse.CopyTo(memory); response.BodyPipe.Advance(memory.Length); @@ -3107,7 +3108,7 @@ await ExpectAsync(Http2FrameType.DATA, } [Fact] - public async Task ResponseBodyPipeCompleteWithoutExceptionWritesThrow() + public async Task ResponseBodyPipeCompleteWithoutExceptionWritesDoNotThrow() { var headers = new[] { @@ -3118,8 +3119,7 @@ public async Task ResponseBodyPipeCompleteWithoutExceptionWritesThrow() await InitializeConnectionAsync(async context => { context.Response.BodyPipe.Complete(); - await Assert.ThrowsAsync( - async () => await context.Response.WriteAsync("test")); + await context.Response.WriteAsync("test"); }); await StartStreamAsync(1, headers, endStream: true); @@ -3157,6 +3157,7 @@ public async Task ResponseBodyPipeCompleteWithExceptionThrows() await InitializeConnectionAsync(async context => { context.Response.BodyPipe.Complete(expectedException); + await context.Response.WriteAsync("test"); await Task.CompletedTask; }); diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs index 47b865817a8f..00c4eda4ff48 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs @@ -3364,8 +3364,7 @@ public async Task ResponsePipeWriterCompleteWithoutExceptionCausesWritesToThrow( using (var server = new TestServer(async httpContext => { httpContext.Response.BodyPipe.Complete(); - var ex = await Assert.ThrowsAsync( - async () => await httpContext.Response.WriteAsync("test")); + await httpContext.Response.WriteAsync("test"); }, new TestServiceContext(LoggerFactory))) { using (var connection = server.CreateConnection()) @@ -3418,6 +3417,76 @@ await connection.Receive( } } + [Fact] + public async Task ResponseCompleteGetMemoryReturnsRentedMemory() + { + using (var server = new TestServer(async httpContext => + { + await httpContext.Response.StartAsync(); + httpContext.Response.BodyPipe.Complete(); + var memory = httpContext.Response.BodyPipe.GetMemory(); // Shouldn't throw + Assert.Equal(4096, memory.Length); + + await Task.CompletedTask; + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Transfer-Encoding: chunked", + "", + "0", + "", + ""); + } + await server.StopAsync(); + } + } + + [Fact] + public async Task ResponseCompleteGetMemoryAdvanceInLoopDoesNotThrow() + { + using (var server = new TestServer(async httpContext => + { + await httpContext.Response.StartAsync(); + + httpContext.Response.BodyPipe.Complete(); + for (var i = 0; i < 5; i++) + { + var memory = httpContext.Response.BodyPipe.GetMemory(); // Shouldn't throw + httpContext.Response.BodyPipe.Advance(memory.Length); + } + + await Task.CompletedTask; + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Transfer-Encoding: chunked", + "", + "0", + "", + ""); + } + await server.StopAsync(); + } + } + private static async Task ResponseStatusCodeSetBeforeHttpContextDispose( ITestSink testSink, ILoggerFactory loggerFactory, From 7d821b4198d37d16baa30eafd5a87f200e61c46f Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 6 Feb 2019 15:53:15 -0800 Subject: [PATCH 21/33] Trying to figure out what to do with pipeWriter.Complete for Http2. --- .../ResponseCompression/src/BodyWrapperStream.cs | 8 +++++++- .../Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs | 2 +- .../Kestrel/Core/src/Internal/Http/HttpProtocol.cs | 2 +- .../Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs | 2 +- .../Core/src/Internal/Http2/Http2OutputProducer.cs | 5 +++-- .../InMemory.FunctionalTests/Http2/Http2StreamTests.cs | 1 - .../test/InMemory.FunctionalTests/ResponseTests.cs | 4 ++-- 7 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs b/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs index 0dd8fd57d5a5..2b756bf04151 100644 --- a/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs +++ b/src/Middleware/ResponseCompression/src/BodyWrapperStream.cs @@ -324,7 +324,13 @@ private async Task InnerSendFileAsync(string path, long offset, long? count, Can public Task StartAsync(CancellationToken token = default) { OnWrite(); - return _innerStartFeature.StartAsync(token); + + if (_innerStartFeature != null) + { + return _innerStartFeature.StartAsync(token); + } + + return Task.CompletedTask; } } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs index 60201d9a35b7..a594e2a0c9cd 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs @@ -264,7 +264,7 @@ public void Abort(ConnectionAbortedException error) } } - public void Complete(Exception ex) + public void Complete() { lock (_contextLock) { diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index 6a9d9d7ab5cf..c5806d733e06 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -1347,7 +1347,7 @@ public void Complete(Exception ex) ApplicationAbort(); } } - Output.Complete(ex); + Output.Complete(); } public ValueTask WritePipeAsync(ReadOnlyMemory data, CancellationToken cancellationToken) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs index 3e6e1dbff477..427844fc2583 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/IHttpOutputProducer.cs @@ -22,6 +22,6 @@ public interface IHttpOutputProducer Span GetSpan(int sizeHint = 0); Memory GetMemory(int sizeHint = 0); void CancelPendingFlush(); - void Complete(Exception ex); + void Complete(); } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs index fe4845ca4e73..8b1053e53647 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs @@ -300,9 +300,10 @@ ValueTask IHttpOutputProducer.WriteChunkAsync(ReadOnlySpan da throw new NotImplementedException(); } - public void Complete(Exception ex) + public void Complete() { - //_dataPipe.Writer.Complete(ex); + // This can't complete the pipe today because headers NEED to be added as part of the protocol + //_dataPipe.Writer.Complete(); } } } diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 41845c3f3713..08072597a4fb 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -3157,7 +3157,6 @@ public async Task ResponseBodyPipeCompleteWithExceptionThrows() await InitializeConnectionAsync(async context => { context.Response.BodyPipe.Complete(expectedException); - await context.Response.WriteAsync("test"); await Task.CompletedTask; }); diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs index 00c4eda4ff48..a8371a732702 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs @@ -3332,7 +3332,7 @@ await connection.Receive( } [Fact] - public async Task ResponsePipeWriterCompleteWithoutExceptionDoesNotThrow() + public async Task ResponseBodyPipeCompleteWithoutExceptionDoesNotThrow() { using (var server = new TestServer(async httpContext => { @@ -3359,7 +3359,7 @@ await connection.Receive( } [Fact] - public async Task ResponsePipeWriterCompleteWithoutExceptionCausesWritesToThrow() + public async Task ResponseBodyPipeCompleteWithoutExceptionWritesDoNotThrow() { using (var server = new TestServer(async httpContext => { From a2796fcc3a9111c5cc0eacdcc6735a2156c28610 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Wed, 6 Feb 2019 17:57:56 -0800 Subject: [PATCH 22/33] Don't actually write data --- .../test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs index 08072597a4fb..5ef31c894440 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2StreamTests.cs @@ -3119,7 +3119,7 @@ public async Task ResponseBodyPipeCompleteWithoutExceptionWritesDoNotThrow() await InitializeConnectionAsync(async context => { context.Response.BodyPipe.Complete(); - await context.Response.WriteAsync("test"); + await context.Response.WriteAsync(""); }); await StartStreamAsync(1, headers, endStream: true); From 8714576f77b01bd487afab30e06f2ceca065b00b Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Thu, 7 Feb 2019 12:48:37 -0800 Subject: [PATCH 23/33] Some feedback --- .../HttpResponseWritingExtensions.cs | 2 +- .../HttpResponseWritingExtensionsTests.cs | 12 +- src/Http/Http/src/StreamPipeWriter.cs | 4 +- .../src/Internal/Http/Http1OutputProducer.cs | 90 +++++++------- .../src/Internal/Http2/Http2OutputProducer.cs | 111 +++++++++--------- 5 files changed, 115 insertions(+), 104 deletions(-) diff --git a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs index 8da5b5faa6fe..ac29c1fdf46f 100644 --- a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs +++ b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs @@ -120,7 +120,7 @@ private static void WriteMutliSegmentEncoded(PipeWriter writer, string text, Enc // Therefore, we check encodedLength - totalBytesUsed too. while (!completed || encodedLength - totalBytesUsed != 0) { - encoder.Convert(source, destination, source.Length == 0, out var charsUsed, out var bytesUsed, out completed); + encoder.Convert(source, destination, flush: source.Length == 0, out var charsUsed, out var bytesUsed, out completed); totalBytesUsed += bytesUsed; writer.Advance(bytesUsed); diff --git a/src/Http/Http.Abstractions/test/HttpResponseWritingExtensionsTests.cs b/src/Http/Http.Abstractions/test/HttpResponseWritingExtensionsTests.cs index d07a263880d5..addaa0f35d5d 100644 --- a/src/Http/Http.Abstractions/test/HttpResponseWritingExtensionsTests.cs +++ b/src/Http/Http.Abstractions/test/HttpResponseWritingExtensionsTests.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.IO; using System.IO.Pipelines; using System.IO.Pipelines.Tests; @@ -43,7 +44,7 @@ public async Task WritingTextThatRequiresMultipleSegmentsWorks(Encoding encoding HttpContext context = new DefaultHttpContext(); context.Response.BodyPipe = streamPipeWriter; - var inputString = "丂丂丂丂丂丂丂丂丂丂丂丂丂丂丂"; + var inputString = "昨日すき焼きを食べました"; var expected = encoding.GetBytes(inputString); await context.Response.WriteAsync(inputString, encoding); @@ -64,16 +65,19 @@ public async Task WritingTextWithPassedInEncodingWorks(Encoding encoding) { HttpContext context = CreateRequest(); - var inputString = "丂丂丂丂丂"; + var inputString = "昨日すき焼きを食べました"; var expected = encoding.GetBytes(inputString); await context.Response.WriteAsync(inputString, encoding); context.Response.Body.Position = 0; - var actual = new byte[expected.Length]; + var actual = new byte[expected.Length * 2]; var length = context.Response.Body.Read(actual); + var actualShortened = new byte[length]; + Array.Copy(actual, actualShortened, length); + Assert.Equal(expected.Length, length); - Assert.Equal(expected, actual); + Assert.Equal(expected, actualShortened); } public static TheoryData Encodings => diff --git a/src/Http/Http/src/StreamPipeWriter.cs b/src/Http/Http/src/StreamPipeWriter.cs index b95ee5875da9..549f2b521ae4 100644 --- a/src/Http/Http/src/StreamPipeWriter.cs +++ b/src/Http/Http/src/StreamPipeWriter.cs @@ -182,7 +182,7 @@ private async ValueTask FlushAsyncInternal(CancellationToken cancel await InnerStream.WriteAsync(segment.Buffer.Slice(0, segment.Length), localToken); #elif NETSTANDARD2_0 MemoryMarshal.TryGetArray(segment.Buffer, out var arraySegment); - await _writingStream.WriteAsync(arraySegment.Array, 0, segment.Length, localToken); + await InnerStream.WriteAsync(arraySegment.Array, 0, segment.Length, localToken); #else #error Target frameworks need to be updated. #endif @@ -198,7 +198,7 @@ private async ValueTask FlushAsyncInternal(CancellationToken cancel await InnerStream.WriteAsync(_currentSegment.Slice(0, _position), localToken); #elif NETSTANDARD2_0 MemoryMarshal.TryGetArray(_currentSegment, out var arraySegment); - await _writingStream.WriteAsync(arraySegment.Array, 0, _position, localToken); + await InnerStream.WriteAsync(arraySegment.Array, 0, _position, localToken); #else #error Target frameworks need to be updated. #endif diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs index a594e2a0c9cd..76426684b04f 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs @@ -101,69 +101,68 @@ public ValueTask FlushAsync(CancellationToken cancellationToken = d public Memory GetMemory(int sizeHint = 0) { - if (_completed) + lock (_contextLock) { - if (_completedMemoryOwner == null) + if (_completed) { - _completedMemoryOwner = _memoryPool.Rent(sizeHint); + return GetFakeMemory(sizeHint); + } + else if (_autoChunk) + { + return GetChunkedMemory(sizeHint); + } + else + { + return _pipeWriter.GetMemory(sizeHint); } - return _completedMemoryOwner.Memory; - } - - if (_autoChunk) - { - return GetChunkedMemory(sizeHint); - } - else - { - return _pipeWriter.GetMemory(sizeHint); } } public Span GetSpan(int sizeHint = 0) { - if (_completed) + lock (_contextLock) { - if (_completedMemoryOwner == null) + if (_completed) { - _completedMemoryOwner = _memoryPool.Rent(sizeHint); + return GetFakeMemory(sizeHint).Span; + } + else if (_autoChunk) + { + return GetChunkedMemory(sizeHint).Span; + } + else + { + return _pipeWriter.GetMemory(sizeHint).Span; } - return _completedMemoryOwner.Memory.Span; - } - - if (_autoChunk) - { - return GetChunkedMemory(sizeHint).Span; - } - else - { - return _pipeWriter.GetMemory(sizeHint).Span; } } public void Advance(int bytes) { - if (_completed) - { - return; - } - - if (_autoChunk) + lock (_contextLock) { - if (bytes < 0) + if (_completed) { - throw new ArgumentOutOfRangeException(nameof(bytes)); + return; } - if (bytes + _advancedBytesForChunk > _currentChunkMemory.Length - BeginChunkLengthMax - EndChunkLength) + if (_autoChunk) { - throw new InvalidOperationException("Can't advance past buffer size."); + if (bytes < 0) + { + throw new ArgumentOutOfRangeException(nameof(bytes)); + } + + if (bytes + _advancedBytesForChunk > _currentChunkMemory.Length - BeginChunkLengthMax - EndChunkLength) + { + throw new InvalidOperationException("Can't advance past buffer size."); + } + _advancedBytesForChunk += bytes; + } + else + { + _pipeWriter.Advance(bytes); } - _advancedBytesForChunk += bytes; - } - else - { - _pipeWriter.Advance(bytes); } } @@ -377,5 +376,14 @@ private void WriteCurrentMemoryToPipeWriter() // If there is an empty write, we still need to update the current chunk _currentChunkMemoryUpdated = false; } + + private Memory GetFakeMemory(int sizeHint) + { + if (_completedMemoryOwner == null) + { + _completedMemoryOwner = _memoryPool.Rent(sizeHint); + } + return _completedMemoryOwner.Memory; + } } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs index 8b1053e53647..d5ed461f02a6 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs @@ -196,60 +196,6 @@ public ValueTask WriteRstStreamAsync(Http2ErrorCode error) } } - private async ValueTask ProcessDataWrites() - { - FlushResult flushResult = default; - try - { - ReadResult readResult; - - do - { - readResult = await _dataPipe.Reader.ReadAsync(); - - if (readResult.IsCompleted && _stream.Trailers?.Count > 0) - { - if (readResult.Buffer.Length > 0) - { - flushResult = await _frameWriter.WriteDataAsync(_streamId, _flowControl, readResult.Buffer, endStream: false); - } - - flushResult = await _frameWriter.WriteResponseTrailers(_streamId, _stream.Trailers); - } - else - { - flushResult = await _frameWriter.WriteDataAsync(_streamId, _flowControl, readResult.Buffer, endStream: readResult.IsCompleted); - } - - _dataPipe.Reader.AdvanceTo(readResult.Buffer.End); - } while (!readResult.IsCompleted); - } - catch (OperationCanceledException) - { - // Writes should not throw for aborted streams/connections. - } - catch (Exception ex) - { - Debug.Assert(false, ex.ToString()); - } - - _dataPipe.Reader.Complete(); - - return flushResult; - } - - private static Pipe CreateDataPipe(MemoryPool pool) - => new Pipe(new PipeOptions - ( - pool: pool, - readerScheduler: PipeScheduler.Inline, - writerScheduler: PipeScheduler.ThreadPool, - pauseWriterThreshold: 1, - resumeWriterThreshold: 1, - useSynchronizationContext: false, - minimumSegmentSize: KestrelMemoryPool.MinimumSegmentSize - )); - public void Advance(int bytes) { _startedWritingDataFrames = true; @@ -302,8 +248,61 @@ ValueTask IHttpOutputProducer.WriteChunkAsync(ReadOnlySpan da public void Complete() { - // This can't complete the pipe today because headers NEED to be added as part of the protocol - //_dataPipe.Writer.Complete(); + _dataPipe.Writer.Complete(); } + + private async ValueTask ProcessDataWrites() + { + FlushResult flushResult = default; + try + { + ReadResult readResult; + + do + { + readResult = await _dataPipe.Reader.ReadAsync(); + + if (readResult.IsCompleted && _stream.Trailers?.Count > 0) + { + if (readResult.Buffer.Length > 0) + { + flushResult = await _frameWriter.WriteDataAsync(_streamId, _flowControl, readResult.Buffer, endStream: false); + } + + flushResult = await _frameWriter.WriteResponseTrailers(_streamId, _stream.Trailers); + } + else + { + flushResult = await _frameWriter.WriteDataAsync(_streamId, _flowControl, readResult.Buffer, endStream: readResult.IsCompleted); + } + + _dataPipe.Reader.AdvanceTo(readResult.Buffer.End); + } while (!readResult.IsCompleted); + } + catch (OperationCanceledException) + { + // Writes should not throw for aborted streams/connections. + } + catch (Exception ex) + { + Debug.Assert(false, ex.ToString()); + } + + _dataPipe.Reader.Complete(); + + return flushResult; + } + + private static Pipe CreateDataPipe(MemoryPool pool) + => new Pipe(new PipeOptions + ( + pool: pool, + readerScheduler: PipeScheduler.Inline, + writerScheduler: PipeScheduler.ThreadPool, + pauseWriterThreshold: 1, + resumeWriterThreshold: 1, + useSynchronizationContext: false, + minimumSegmentSize: KestrelMemoryPool.MinimumSegmentSize + )); } } From c1c0247dfcae284a0d8a9c8354ae6ccc5e2892b7 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Thu, 7 Feb 2019 15:58:37 -0800 Subject: [PATCH 24/33] Offline feedback --- .../Http/HttpProtocol.FeatureCollection.cs | 19 +++------------ .../src/Internal/Http2/Http2OutputProducer.cs | 24 ++++++++++++++----- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs index 0128964504cc..c38fc036702d 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs @@ -195,14 +195,9 @@ PipeWriter IResponseBodyPipeFeature.ResponseBodyPipe { get { - if (_userSetPipeWriter != null) - { - return _userSetPipeWriter; - } - if (!object.ReferenceEquals(_cachedResponseBodyStream, ResponseBody)) { - var responsePipeWriter = new StreamPipeWriter(ResponseBody, 4096, _context.MemoryPool); + var responsePipeWriter = new StreamPipeWriter(ResponseBody, minimumSegmentSize: 4096, _context.MemoryPool); ResponsePipeWriter = responsePipeWriter; _cachedResponseBodyStream = ResponseBody; if (_wrapperObjectsToDispose == null) @@ -216,21 +211,14 @@ PipeWriter IResponseBodyPipeFeature.ResponseBodyPipe } set { - _userSetPipeWriter = value ?? throw new ArgumentNullException(nameof(value)); - ResponsePipeWriter = _userSetPipeWriter; + ResponsePipeWriter = value; } } - Stream IHttpResponseFeature.Body { get { - if (_userSetResponseBody != null) - { - return _userSetResponseBody; - } - if (!object.ReferenceEquals(_cachedResponsePipeWriter, ResponsePipeWriter)) { var responseBody = new WriteOnlyPipeStream(ResponsePipeWriter); @@ -247,8 +235,7 @@ Stream IHttpResponseFeature.Body } set { - _userSetResponseBody = value ?? throw new ArgumentNullException(nameof(value)); - ResponseBody = _userSetResponseBody; + ResponseBody = value; } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs index d5ed461f02a6..fdc05dcffd9e 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs @@ -198,24 +198,36 @@ public ValueTask WriteRstStreamAsync(Http2ErrorCode error) public void Advance(int bytes) { - _startedWritingDataFrames = true; + lock (_dataWriterLock) + { + _startedWritingDataFrames = true; - _dataPipe.Writer.Advance(bytes); + _dataPipe.Writer.Advance(bytes); + } } public Span GetSpan(int sizeHint = 0) { - return _dataPipe.Writer.GetSpan(sizeHint); + lock (_dataWriterLock) + { + return _dataPipe.Writer.GetSpan(sizeHint); + } } public Memory GetMemory(int sizeHint = 0) { - return _dataPipe.Writer.GetMemory(sizeHint); + lock (_dataWriterLock) + { + return _dataPipe.Writer.GetMemory(sizeHint); + } } public void CancelPendingFlush() { - _dataPipe.Writer.CancelPendingFlush(); + lock (_dataWriterLock) + { + _dataPipe.Writer.CancelPendingFlush(); + } } public ValueTask WriteDataToPipeAsync(ReadOnlySpan data, CancellationToken cancellationToken) @@ -248,7 +260,7 @@ ValueTask IHttpOutputProducer.WriteChunkAsync(ReadOnlySpan da public void Complete() { - _dataPipe.Writer.Complete(); + // This will noop for now. See: } private async ValueTask ProcessDataWrites() From 1ee90a82cc407ae9f17500e21cceed0729b21965 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Thu, 7 Feb 2019 16:02:00 -0800 Subject: [PATCH 25/33] Renames --- .../src/Internal/Http/Http1OutputProducer.cs | 36 +++++++++++-------- .../src/Internal/Http2/Http2OutputProducer.cs | 2 +- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs index 76426684b04f..297d79d13ee6 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/Http1OutputProducer.cs @@ -49,7 +49,7 @@ public class Http1OutputProducer : IHttpOutputProducer, IHttpOutputAborter, IDis private int _advancedBytesForChunk; private Memory _currentChunkMemory; private bool _currentChunkMemoryUpdated; - private IMemoryOwner _completedMemoryOwner; + private IMemoryOwner _fakeMemoryOwner; public Http1OutputProducer( PipeWriter pipeWriter, @@ -230,19 +230,24 @@ public void Dispose() { lock (_contextLock) { - if (_completedMemoryOwner != null) + if (_fakeMemoryOwner != null) { - _completedMemoryOwner.Dispose(); - _completedMemoryOwner = null; + _fakeMemoryOwner.Dispose(); + _fakeMemoryOwner = null; } - if (!_pipeWriterCompleted) - { - _log.ConnectionDisconnect(_connectionId); - _pipeWriterCompleted = true; - _completed = true; - _pipeWriter.Complete(); - } + CompletePipe(); + } + } + + private void CompletePipe() + { + if (!_pipeWriterCompleted) + { + _log.ConnectionDisconnect(_connectionId); + _pipeWriterCompleted = true; + _completed = true; + _pipeWriter.Complete(); } } @@ -259,7 +264,8 @@ public void Abort(ConnectionAbortedException error) _aborted = true; _connectionContext.Abort(error); - Dispose(); + + CompletePipe(); } } @@ -379,11 +385,11 @@ private void WriteCurrentMemoryToPipeWriter() private Memory GetFakeMemory(int sizeHint) { - if (_completedMemoryOwner == null) + if (_fakeMemoryOwner == null) { - _completedMemoryOwner = _memoryPool.Rent(sizeHint); + _fakeMemoryOwner = _memoryPool.Rent(sizeHint); } - return _completedMemoryOwner.Memory; + return _fakeMemoryOwner.Memory; } } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs index fdc05dcffd9e..071518150461 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2OutputProducer.cs @@ -260,7 +260,7 @@ ValueTask IHttpOutputProducer.WriteChunkAsync(ReadOnlySpan da public void Complete() { - // This will noop for now. See: + // This will noop for now. See: https://github.com/aspnet/AspNetCore/issues/7370 } private async ValueTask ProcessDataWrites() From 177d34d5b9cfaad88fb3d8672c750f07ec452562 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Thu, 7 Feb 2019 16:55:30 -0800 Subject: [PATCH 26/33] IsCompletedSuccessfully --- .../src/Extensions/HttpResponseWritingExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs index ac29c1fdf46f..1d2a44cd311f 100644 --- a/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs +++ b/src/Http/Http.Abstractions/src/Extensions/HttpResponseWritingExtensions.cs @@ -74,7 +74,7 @@ public static class HttpResponseWritingExtensions Write(response, text, encoding); var flushAsyncTask = response.BodyPipe.FlushAsync(cancellationToken); - if (flushAsyncTask.IsCompleted) + if (flushAsyncTask.IsCompletedSuccessfully) { // Most implementations of ValueTask reset state in GetResult, so call it before returning a completed task. flushAsyncTask.GetAwaiter().GetResult(); From 1a7b2c2b70f361675bed228b3aa18fbf7fcef70e Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Fri, 8 Feb 2019 08:40:38 -0800 Subject: [PATCH 27/33] Feedback --- .../Http/HttpProtocol.FeatureCollection.cs | 48 ++--- .../src/Internal/Http2/Http2FrameWriter.cs | 2 +- .../InMemory.FunctionalTests/ResponseTests.cs | 176 ++++++++++++++++++ 3 files changed, 201 insertions(+), 25 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs index c38fc036702d..23a6fdabadf7 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs @@ -195,48 +195,48 @@ PipeWriter IResponseBodyPipeFeature.ResponseBodyPipe { get { - if (!object.ReferenceEquals(_cachedResponseBodyStream, ResponseBody)) - { - var responsePipeWriter = new StreamPipeWriter(ResponseBody, minimumSegmentSize: 4096, _context.MemoryPool); - ResponsePipeWriter = responsePipeWriter; - _cachedResponseBodyStream = ResponseBody; - if (_wrapperObjectsToDispose == null) - { - _wrapperObjectsToDispose = new List(); - } - _wrapperObjectsToDispose.Add(responsePipeWriter); - } - return ResponsePipeWriter; } set { ResponsePipeWriter = value; - } - } - Stream IHttpResponseFeature.Body - { - get - { if (!object.ReferenceEquals(_cachedResponsePipeWriter, ResponsePipeWriter)) { var responseBody = new WriteOnlyPipeStream(ResponsePipeWriter); ResponseBody = responseBody; - _cachedResponsePipeWriter = ResponsePipeWriter; - if (_wrapperObjectsToDispose == null) - { - _wrapperObjectsToDispose = new List(); - } - _wrapperObjectsToDispose.Add(responseBody); + + RegisterWrapperForDisposal(responseBody); } + } + } + Stream IHttpResponseFeature.Body + { + get + { return ResponseBody; } set { ResponseBody = value; + if (!object.ReferenceEquals(_cachedResponseBodyStream, ResponseBody)) + { + var responsePipeWriter = new StreamPipeWriter(ResponseBody, minimumSegmentSize: 4096, _context.MemoryPool); + ResponsePipeWriter = responsePipeWriter; + _cachedResponseBodyStream = ResponseBody; + RegisterWrapperForDisposal(responsePipeWriter); + } + } + } + + private void RegisterWrapperForDisposal(IDisposable responseBody) + { + if (_wrapperObjectsToDispose == null) + { + _wrapperObjectsToDispose = new List(); } + _wrapperObjectsToDispose.Add(responseBody); } protected void ResetHttp1Features() diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs index af9927e1a6a8..58b122cc4c6a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2FrameWriter.cs @@ -319,7 +319,7 @@ private async ValueTask WriteDataAsync(int streamId, StreamOutputFl while (dataLength > 0) { OutputFlowControlAwaitable availabilityAwaitable; - var writeTask = new ValueTask(new FlushResult()); + var writeTask = default(ValueTask); lock (_writeLock) { diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs index a8371a732702..47d041b21a5b 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs @@ -3487,6 +3487,182 @@ await connection.Receive( } } + [Fact] + public async Task ResponseSetBodyAndPipeBodyIsWrapped() + { + using (var server = new TestServer(async httpContext => + { + httpContext.Response.Body = new MemoryStream(); + httpContext.Response.BodyPipe = new Pipe().Writer; + Assert.IsType(httpContext.Response.Body); + + await Task.CompletedTask; + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 0", + "", + ""); + } + await server.StopAsync(); + } + } + + [Fact] + public async Task ResponseSetPipeAndBodyPipeIsWrapped() + { + using (var server = new TestServer(async httpContext => + { + httpContext.Response.BodyPipe = new Pipe().Writer; + httpContext.Response.Body = new MemoryStream(); + Assert.IsType(httpContext.Response.BodyPipe); + Assert.IsType(httpContext.Response.Body); + + await Task.CompletedTask; + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 0", + "", + ""); + } + await server.StopAsync(); + } + } + + [Fact] + public async Task ResponseWriteToBodyPipeAndStreamAllBlocksDisposed() + { + using (var server = new TestServer(async httpContext => + { + for (var i = 0; i < 3; i++) + { + httpContext.Response.BodyPipe = new Pipe().Writer; + await httpContext.Response.Body.WriteAsync(new byte[1]); + httpContext.Response.Body = new MemoryStream(); + await httpContext.Response.BodyPipe.WriteAsync(new byte[1]); + } + + // TestMemoryPool will confirm that all rented blocks have been disposed, meaning dispose was called. + + await Task.CompletedTask; + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 0", + "", + ""); + } + await server.StopAsync(); + } + } + + [Fact] + public async Task ResponseStreamWrappingWorks() + { + using (var server = new TestServer(async httpContext => + { + var oldBody = httpContext.Response.Body; + httpContext.Response.Body = new MemoryStream(); + + await httpContext.Response.BodyPipe.WriteAsync(new byte[1]); + await httpContext.Response.Body.WriteAsync(new byte[1]); + + Assert.Equal(2, httpContext.Response.Body.Length); + + httpContext.Response.Body = oldBody; + + // Even though we are restoring the original response body, we will create a + // wrapper rather than restoring the original pipe. + Assert.IsType(httpContext.Response.BodyPipe); + + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 0", + "", + ""); + } + await server.StopAsync(); + } + } + + [Fact] + public async Task ResponsePipeWrappingWorks() + { + using (var server = new TestServer(async httpContext => + { + var oldPipeWriter = httpContext.Response.BodyPipe; + var pipe = new Pipe(); + httpContext.Response.BodyPipe = pipe.Writer; + + await httpContext.Response.Body.WriteAsync(new byte[1]); + await httpContext.Response.BodyPipe.WriteAsync(new byte[1]); + + var readResult = await pipe.Reader.ReadAsync(); + Assert.Equal(2, readResult.Buffer.Length); + + httpContext.Response.BodyPipe = oldPipeWriter; + + // Even though we are restoring the original response body, we will create a + // wrapper rather than restoring the original pipe. + Assert.IsType(httpContext.Response.Body); + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 0", + "", + ""); + } + await server.StopAsync(); + } + } + private static async Task ResponseStatusCodeSetBeforeHttpContextDispose( ITestSink testSink, ILoggerFactory loggerFactory, From 67023fa3646cadffc2152abf52435a313357aed0 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Fri, 8 Feb 2019 08:56:49 -0800 Subject: [PATCH 28/33] Only dispose the stream pipe wrapper and more tests --- .../Http/HttpProtocol.FeatureCollection.cs | 20 +++--- .../InMemory.FunctionalTests/ResponseTests.cs | 68 +++++++++++++++++++ 2 files changed, 76 insertions(+), 12 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs index 23a6fdabadf7..a770f8bf99b9 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs @@ -205,8 +205,7 @@ PipeWriter IResponseBodyPipeFeature.ResponseBodyPipe { var responseBody = new WriteOnlyPipeStream(ResponsePipeWriter); ResponseBody = responseBody; - - RegisterWrapperForDisposal(responseBody); + _cachedResponsePipeWriter = ResponsePipeWriter; } } } @@ -225,18 +224,15 @@ Stream IHttpResponseFeature.Body var responsePipeWriter = new StreamPipeWriter(ResponseBody, minimumSegmentSize: 4096, _context.MemoryPool); ResponsePipeWriter = responsePipeWriter; _cachedResponseBodyStream = ResponseBody; - RegisterWrapperForDisposal(responsePipeWriter); - } - } - } - private void RegisterWrapperForDisposal(IDisposable responseBody) - { - if (_wrapperObjectsToDispose == null) - { - _wrapperObjectsToDispose = new List(); + // The StreamPipeWrapper needs to be disposed as it hold onto blocks of memory + if (_wrapperObjectsToDispose == null) + { + _wrapperObjectsToDispose = new List(); + } + _wrapperObjectsToDispose.Add(responsePipeWriter); + } } - _wrapperObjectsToDispose.Add(responseBody); } protected void ResetHttp1Features() diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs index 47d041b21a5b..66fb484f85fa 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs @@ -3517,6 +3517,74 @@ await connection.Receive( } } + [Fact] + public async Task ResponseSetBodyToSameValueTwiceGetPipeMultipleTimesSameObject() + { + using (var server = new TestServer(async httpContext => + { + var memoryStream = new MemoryStream(); + httpContext.Response.Body = memoryStream; + var bodyPipe1 = httpContext.Response.BodyPipe; + + httpContext.Response.Body = memoryStream; + var bodyPipe2 = httpContext.Response.BodyPipe; + + Assert.Equal(bodyPipe1, bodyPipe2); + await Task.CompletedTask; + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 0", + "", + ""); + } + await server.StopAsync(); + } + } + + [Fact] + public async Task ResponseSetPipeToSameValueTwiceGetBodyMultipleTimesSameObject() + { + using (var server = new TestServer(async httpContext => + { + var pipeWriter = new Pipe().Writer; + httpContext.Response.BodyPipe = pipeWriter; + var body1 = httpContext.Response.Body; + + httpContext.Response.BodyPipe = pipeWriter; + var body2 = httpContext.Response.Body; + + Assert.Equal(body1, body2); + await Task.CompletedTask; + }, new TestServiceContext(LoggerFactory))) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + "Content-Length: 0", + "", + ""); + } + await server.StopAsync(); + } + } + [Fact] public async Task ResponseSetPipeAndBodyPipeIsWrapped() { From 45f373927baab38a0f3b3f20fa551b5150d6340b Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Fri, 8 Feb 2019 09:15:53 -0800 Subject: [PATCH 29/33] Use kestrel's minimum segment size --- .../Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs index a770f8bf99b9..4138babadf17 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs @@ -13,6 +13,7 @@ using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Features; using Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Infrastructure; +using Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions.Internal; namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http { @@ -221,7 +222,7 @@ Stream IHttpResponseFeature.Body ResponseBody = value; if (!object.ReferenceEquals(_cachedResponseBodyStream, ResponseBody)) { - var responsePipeWriter = new StreamPipeWriter(ResponseBody, minimumSegmentSize: 4096, _context.MemoryPool); + var responsePipeWriter = new StreamPipeWriter(ResponseBody, minimumSegmentSize: KestrelMemoryPool.MinimumSegmentSize, _context.MemoryPool); ResponsePipeWriter = responsePipeWriter; _cachedResponseBodyStream = ResponseBody; From 4d16db062b53742f48ced43609d80379c934bcbd Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Fri, 8 Feb 2019 10:30:46 -0800 Subject: [PATCH 30/33] Bad rebase --- src/Http/Http/test/StreamPipeReaderTests.cs | 5 +++-- src/Http/Http/test/StreamPipeWriterTests.cs | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Http/Http/test/StreamPipeReaderTests.cs b/src/Http/Http/test/StreamPipeReaderTests.cs index aa08cbfaf0b9..91ef75dc6b64 100644 --- a/src/Http/Http/test/StreamPipeReaderTests.cs +++ b/src/Http/Http/test/StreamPipeReaderTests.cs @@ -610,10 +610,11 @@ public async Task ReadAsyncAfterReceivingCompletedReadResultDoesNotThrow() Assert.True(readResult.Buffer.IsEmpty); Assert.True(readResult.IsCompleted); } - + + [Fact] public void InnerStreamReturnsStream() { - Assert.Equal(MemoryStream, ((StreamPipeReader)Reader).InnerStream); + Assert.Equal(Stream, ((StreamPipeReader)Reader).InnerStream); } private async Task ReadFromPipeAsString() diff --git a/src/Http/Http/test/StreamPipeWriterTests.cs b/src/Http/Http/test/StreamPipeWriterTests.cs index 1160d5759231..709458dab67e 100644 --- a/src/Http/Http/test/StreamPipeWriterTests.cs +++ b/src/Http/Http/test/StreamPipeWriterTests.cs @@ -406,7 +406,7 @@ void NonAsyncMethod() [Fact] public void InnerStreamReturnsStream() { - Assert.Equal(MemoryStream, ((StreamPipeWriter)Writer).InnerStream); + Assert.Equal(Stream, ((StreamPipeWriter)Writer).InnerStream); } private void WriteStringToStream(string input) From db79af5b1e24c853056b4fec10f396736b2ee2ae Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Fri, 8 Feb 2019 12:20:10 -0800 Subject: [PATCH 31/33] Feedback --- .../Http/HttpProtocol.FeatureCollection.cs | 26 ++---- .../Core/src/Internal/Http/HttpProtocol.cs | 14 +-- .../InMemory.FunctionalTests/RequestTests.cs | 27 +++++- .../InMemory.FunctionalTests/ResponseTests.cs | 89 +++++++++++++++++++ 4 files changed, 126 insertions(+), 30 deletions(-) diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs index 4138babadf17..9ef4f4cd4a1a 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.FeatureCollection.cs @@ -201,13 +201,7 @@ PipeWriter IResponseBodyPipeFeature.ResponseBodyPipe set { ResponsePipeWriter = value; - - if (!object.ReferenceEquals(_cachedResponsePipeWriter, ResponsePipeWriter)) - { - var responseBody = new WriteOnlyPipeStream(ResponsePipeWriter); - ResponseBody = responseBody; - _cachedResponsePipeWriter = ResponsePipeWriter; - } + ResponseBody = new WriteOnlyPipeStream(ResponsePipeWriter); } } @@ -220,19 +214,15 @@ Stream IHttpResponseFeature.Body set { ResponseBody = value; - if (!object.ReferenceEquals(_cachedResponseBodyStream, ResponseBody)) + var responsePipeWriter = new StreamPipeWriter(ResponseBody, minimumSegmentSize: KestrelMemoryPool.MinimumSegmentSize, _context.MemoryPool); + ResponsePipeWriter = responsePipeWriter; + + // The StreamPipeWrapper needs to be disposed as it hold onto blocks of memory + if (_wrapperObjectsToDispose == null) { - var responsePipeWriter = new StreamPipeWriter(ResponseBody, minimumSegmentSize: KestrelMemoryPool.MinimumSegmentSize, _context.MemoryPool); - ResponsePipeWriter = responsePipeWriter; - _cachedResponseBodyStream = ResponseBody; - - // The StreamPipeWrapper needs to be disposed as it hold onto blocks of memory - if (_wrapperObjectsToDispose == null) - { - _wrapperObjectsToDispose = new List(); - } - _wrapperObjectsToDispose.Add(responsePipeWriter); + _wrapperObjectsToDispose = new List(); } + _wrapperObjectsToDispose.Add(responsePipeWriter); } } diff --git a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs index c5806d733e06..5e873f14585e 100644 --- a/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs +++ b/src/Servers/Kestrel/Core/src/Internal/Http/HttpProtocol.cs @@ -30,7 +30,7 @@ public abstract partial class HttpProtocol : IDefaultHttpContextContainer, IHttp private static readonly byte[] _bytesServer = Encoding.ASCII.GetBytes("\r\nServer: " + Constants.ServerName); protected Streams _streams; - + private HttpResponsePipeWriter _originalPipeWriter; private Stack, object>> _onStarting; private Stack, object>> _onCompleted; @@ -63,10 +63,6 @@ public abstract partial class HttpProtocol : IDefaultHttpContextContainer, IHttp protected string _methodText = null; private string _scheme = null; - protected PipeWriter _userSetPipeWriter; - protected Stream _userSetResponseBody; - protected PipeWriter _cachedResponsePipeWriter; - protected Stream _cachedResponseBodyStream; private List _wrapperObjectsToDispose; public HttpProtocol(HttpConnectionContext context) @@ -303,13 +299,11 @@ public void InitializeStreams(MessageBody messageBody) { var pipeWriter = new HttpResponsePipeWriter(this); _streams = new Streams(bodyControl: this, pipeWriter); - ResponsePipeWriter = pipeWriter; + _originalPipeWriter = pipeWriter; } (RequestBody, ResponseBody) = _streams.Start(messageBody); - - _cachedResponseBodyStream = ResponseBody; - _cachedResponsePipeWriter = ResponsePipeWriter; + ResponsePipeWriter = _originalPipeWriter; } public void StopStreams() => _streams.Stop(); @@ -345,8 +339,6 @@ public void Reset() _httpVersion = Http.HttpVersion.Unknown; _statusCode = StatusCodes.Status200OK; _reasonPhrase = null; - _userSetPipeWriter = null; - _userSetResponseBody = null; var remoteEndPoint = RemoteEndPoint; RemoteIpAddress = remoteEndPoint?.Address; diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs index 291dd9bdb095..0fb509bfea62 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/RequestTests.cs @@ -60,7 +60,7 @@ public async Task StreamsAreNotPersistedAcrossRequests() } [Fact] - public async Task PipesAreNotPersistedAcrossRequests() + public async Task PipesAreNotPersistedBySettingStreamPipeWriterAcrossRequests() { var responseBodyPersisted = false; PipeWriter bodyPipe = null; @@ -85,6 +85,31 @@ public async Task PipesAreNotPersistedAcrossRequests() } } + [Fact] + public async Task PipesAreNotPersistedAcrossRequests() + { + var responseBodyPersisted = false; + PipeWriter bodyPipe = null; + using (var server = new TestServer(async context => + { + if (context.Response.BodyPipe == bodyPipe) + { + responseBodyPersisted = true; + } + bodyPipe = context.Response.BodyPipe; + + await context.Response.WriteAsync("hello, world"); + }, new TestServiceContext(LoggerFactory))) + { + Assert.Equal("hello, world", await server.HttpClientSlim.GetStringAsync($"http://localhost:{server.Port}/")); + Assert.Equal("hello, world", await server.HttpClientSlim.GetStringAsync($"http://localhost:{server.Port}/")); + + Assert.False(responseBodyPersisted); + + await server.StopAsync(); + } + } + [Fact] public async Task RequestBodyReadAsyncCanBeCancelled() { diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs index 66fb484f85fa..5b8a917b54df 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs @@ -1444,6 +1444,49 @@ await connection.Receive( } } + [Fact] + public async Task FirstWriteVerifiedAfterOnStartingWithResponseBody() + { + var serviceContext = new TestServiceContext(LoggerFactory) { ServerOptions = { AllowSynchronousIO = true } }; + + using (var server = new TestServer(async httpContext => + { + httpContext.Response.OnStarting(() => + { + // Change response to chunked + httpContext.Response.ContentLength = null; + return Task.CompletedTask; + }); + + var response = Encoding.ASCII.GetBytes("hello, world"); + httpContext.Response.ContentLength = response.Length - 1; + + // If OnStarting is not run before verifying writes, an error response will be sent. + await httpContext.Response.Body.WriteAsync(new Memory(response, 0, response.Length)); + }, serviceContext)) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + $"Transfer-Encoding: chunked", + "", + "c", + "hello, world", + "0", + "", + ""); + } + await server.StopAsync(); + } + } + [Fact] public async Task SubsequentWriteVerifiedAfterOnStarting() { @@ -1490,6 +1533,52 @@ await connection.Receive( } } + [Fact] + public async Task SubsequentWriteVerifiedAfterOnStartingWithResponseBody() + { + var serviceContext = new TestServiceContext(LoggerFactory) { ServerOptions = { AllowSynchronousIO = true } }; + + using (var server = new TestServer(async httpContext => + { + httpContext.Response.OnStarting(() => + { + // Change response to chunked + httpContext.Response.ContentLength = null; + return Task.CompletedTask; + }); + + var response = Encoding.ASCII.GetBytes("hello, world"); + httpContext.Response.ContentLength = response.Length - 1; + + // If OnStarting is not run before verifying writes, an error response will be sent. + await httpContext.Response.Body.WriteAsync(new Memory(response, 0, response.Length / 2)); + await httpContext.Response.BodyPipe.WriteAsync(new Memory(response, response.Length / 2, response.Length - response.Length / 2)); + }, serviceContext)) + { + using (var connection = server.CreateConnection()) + { + await connection.Send( + "GET / HTTP/1.1", + "Host:", + "", + ""); + await connection.Receive( + "HTTP/1.1 200 OK", + $"Date: {server.Context.DateHeaderValue}", + $"Transfer-Encoding: chunked", + "", + "6", + "hello,", + "6", + " world", + "0", + "", + ""); + } + await server.StopAsync(); + } + } + [Fact] public async Task FirstWriteAsyncVerifiedAfterOnStarting() { From 31a6dab6bea858dd3267c4d5c22278fc277de520 Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Fri, 8 Feb 2019 12:28:21 -0800 Subject: [PATCH 32/33] whoops --- .../Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs index 5b8a917b54df..41bcac74982e 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs @@ -1552,7 +1552,7 @@ public async Task SubsequentWriteVerifiedAfterOnStartingWithResponseBody() // If OnStarting is not run before verifying writes, an error response will be sent. await httpContext.Response.Body.WriteAsync(new Memory(response, 0, response.Length / 2)); - await httpContext.Response.BodyPipe.WriteAsync(new Memory(response, response.Length / 2, response.Length - response.Length / 2)); + await httpContext.Response.Body.WriteAsync(new Memory(response, response.Length / 2, response.Length - response.Length / 2)); }, serviceContext)) { using (var connection = server.CreateConnection()) From 312f42aef8cad69dcc0505186bd5b5fb68a8bf7b Mon Sep 17 00:00:00 2001 From: Justin Kotalik Date: Fri, 8 Feb 2019 15:09:12 -0800 Subject: [PATCH 33/33] Fix test --- .../test/InMemory.FunctionalTests/ResponseTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs index 41bcac74982e..b38e97f465a4 100644 --- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs +++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/ResponseTests.cs @@ -3607,7 +3607,7 @@ await connection.Receive( } [Fact] - public async Task ResponseSetBodyToSameValueTwiceGetPipeMultipleTimesSameObject() + public async Task ResponseSetBodyToSameValueTwiceGetPipeMultipleTimesDifferentObject() { using (var server = new TestServer(async httpContext => { @@ -3618,7 +3618,7 @@ public async Task ResponseSetBodyToSameValueTwiceGetPipeMultipleTimesSameObject( httpContext.Response.Body = memoryStream; var bodyPipe2 = httpContext.Response.BodyPipe; - Assert.Equal(bodyPipe1, bodyPipe2); + Assert.NotEqual(bodyPipe1, bodyPipe2); await Task.CompletedTask; }, new TestServiceContext(LoggerFactory))) { @@ -3641,7 +3641,7 @@ await connection.Receive( } [Fact] - public async Task ResponseSetPipeToSameValueTwiceGetBodyMultipleTimesSameObject() + public async Task ResponseSetPipeToSameValueTwiceGetBodyMultipleTimesDifferent() { using (var server = new TestServer(async httpContext => { @@ -3652,7 +3652,7 @@ public async Task ResponseSetPipeToSameValueTwiceGetBodyMultipleTimesSameObject( httpContext.Response.BodyPipe = pipeWriter; var body2 = httpContext.Response.Body; - Assert.Equal(body1, body2); + Assert.NotEqual(body1, body2); await Task.CompletedTask; }, new TestServiceContext(LoggerFactory))) {