BodyWriter polyfill doesn't flush, causes truncation #11305

Tratcher · 2019-06-17T22:54:52Z

#11268 identified a truncation issue with the HttpContext.Response.BodyWriter polyfill logic.

The BodyWriter polyfill automatically creates a pipe wrapped around the stream. If you use this pipe to call GetMemory and Advance but do not call FlushAsync, this data is not flushed to the underlying stream. When the request unwinds the pipe will eventually be completed by the OnCompleted event, but it's too late to flush the data as the response has already been disposed.

This is different behavior from any server that does expose pipes (Kestrel) where any data in the pipe is automatically flushed by the server when it completes the pipe.

Proposal: This polyfill should have been implemented as middleware that could auto-flush the pipe at the end of the request.

Note that polyfill middleware would also need to intercept CompleteAsync and flush in that case as well.

Counter proposal: The app is always required to call FlushAsync.

@davidfowl @JamesNK

davidfowl · 2019-06-17T23:00:45Z

Proposal: This polyfill should have been implemented as middleware that could auto-flush the pipe at the end of the request.

No way, that's extremely disruptive and shouldn't have to be a middleware, one thing we need to do to a work around problems like this (having a middleware) is to add a new event to the pipeline (IHttpResponse.OnCompleting) that fires before everything is torn down so that we can perform flushes like this without requiring a middleware as part of the pipeline. It's so much cleaner and is localized to the callsite and requires less moving parts.

This event would make it possible to implement the buffering middleware without middleware 😄

PS: Not calling FlushAsync on the Pipe is bad usage anyways so I don't think this is critical, not calling Complete is a much worse behavior. It's likely in the future that PipeWriter will implement IAsyncDisposable which will flush all pending buffers.

Tratcher · 2019-06-17T23:10:45Z

OnCompleting would be an ordering mess vs middleware already in the pipeline. It only works if everybody uses it, and in order.

E.g. You have the response compression middleware that replaces the response Body. The application writes to the BodyPipe via the polyfill. Then the response compression middleware completes and closes out the compression state. Then OnCompleting fires and the polfyfill tries to flush the BodyPipe. Disaster.

Actually, the above scenario isn't much better with the polyfill middleware, you would have to add the middleware after response compression in order for it to work.

Tratcher · 2019-06-17T23:13:44Z

Note the long term solution here is the merger of pipes and streams so the polyfill is built in at a lower level.

davidfowl · 2019-06-17T23:15:33Z

Actually, the above scenario isn't much better with the polyfill middleware, you would have to add the middleware after response compression in order for it to work.

I don't agree. Having to add a middleware to the pipeline to use a property on HttpContext is a mistake (it's why I hate having session on the HttpContext!). We would make everyone use the event, I don't see what the problem is with that, it's just like OnStarting, an established pattern in our stack (everyone has to use it an chain it to make "the right thing happen"). I don't see why we don't have an equivalent on the response side. It's a gap.

Note the long term solution here is the merger of pipes and streams so the polyfill is built in at a lower level.

How does that help?

Tratcher · 2019-06-17T23:30:44Z

We would make everyone use the event

OnStarting has been such a pain that we abandon most uses of it. It doesn't shim, chain, or fail well. Instead, we end up intercepting the response body in almost all cases.

Note the long term solution here is the merger of pipes and streams so the polyfill is built in at a lower level.

How does that help?

Would this adapter be implemented any differently if it were a polyfill built into the Stream class? Maybe it would auto-flush, maybe not. But it would lower the bar for implementing BodyWriter in each server, e.g. if all they had to do was call flush on their own stream at the end of a request, kind of like what James tried in his TestServer PR.

davidfowl · 2019-06-17T23:35:12Z

OnStarting has been such a pain that we abandon most uses of it. It doesn't shim, chain, or fail well. Instead, we end up intercepting the response body in almost all cases.

It's the pain we have today and because we don't buffering everything by default, we'll always need something like this. We need an event that fires at the end of the pipeline so things like this can be implemented without middleware.

Would this adapter be implemented any differently if it were a polyfill built into the Stream class? Maybe it would auto-flush, maybe not. But it would lower the bar for implementing BodyWriter in each server, e.g. if all they had to do was call flush on their own stream at the end of a request, kind of like what James tried in his TestServer PR.

It would not help, Streams have the exact same issue, anything that buffers has the exact same issue, it's the problem of the call site not calling DisposeAsync/Complete/FluhsAsync on anything that buffers, and expecting something else in the pipeline to do it. Merging pipes and streams solves nothing and we need to either put this behavior into every server or empower other pieces of the stack the way we do with OnStarting (i.e. OnCompleting) to achieve similar behavior. That's the solution I'd prefer for something like this. The middleware approach is a non-starter IMO.

Tratcher · 2019-06-18T00:19:46Z

On hold until you get back *

analogrelay · 2019-06-18T22:00:08Z

Requring FlushAsync will break #11270

JamesNK · 2019-06-19T00:54:59Z

If you really want to require FlushAsync, perhaps CompleteAsync could be the way to send data and trailers together?

halter73 · 2019-06-26T21:19:47Z

If you really want to require FlushAsync, perhaps CompleteAsync could be the way to send data and trailers together?

I discussed this a bit with @Tratcher and he brought up a few issues.

I like this idea, but if we do it, any middleware that replaces the response pipe would also need to replace CompleteAsync for that to work. Middleware that just replaces the body stream would be broken too because the pipe get replaced implicitly, but they don't replace CompleteAsync.

analogrelay · 2019-07-09T22:50:36Z

Is there something here that would be ship-blocking for 3.0 or can we look at it in 3.1?

davidfowl · 2019-07-10T02:27:47Z

This will get fixed when we add CompleteAsync to PipeWriter. Assign this to me as the reaction

Tratcher · 2019-07-10T02:31:26Z

How will PipeWriter.CompleteAsync fix this? Are you assuming the app will call it?

davidfowl · 2019-07-10T02:59:49Z

Complete now flushes buffered data and CompleteAsync will do the same but asynchronously

Tratcher · 2019-07-10T03:40:03Z

That doesn't address the issue I originally posted above, Complete is still getting called too late (during response disposal). The adapter only works if you require the app to explicitly call Flush or Complete.

davidfowl · 2019-07-10T03:41:33Z

Ah, then we need a new event in the pipeline where we can complete/flush without blowing up because it's too late (and no, middleware isn't the way to solve this).

Tratcher · 2019-07-10T16:47:55Z

I agree middleware isn't going to work, the pipe can be adapted multiple times in arbitrary places.

Adding an event for this is going to introduce more ordering and state issues. E.g. events would have to fire in reverse registration order (stack) to mimic pipeline order, and each middleware would have to track if its event was fired before the pipeline started to unwind and avoid double work. We abandoned the OnStarting event for similar reasons.

Maybe blowing up and forcing the the app to call Flush or Complete is the best answer here.

analogrelay · 2019-07-16T22:44:16Z

I'm going to set a meeting and we're going to come up with what we're doing (if anything) for 3.0.

Tratcher · 2019-07-17T21:25:17Z

Issues:

PipeWriter.CompleteAsync vs IHttpResponseCompletionFeature.CompleteAsync
Are these redundant? No, because of adapters.

Examples:

Kestrel: native pipes, stream wraps the pipe, implements IHttpResponseCompletionFeature (HTTP/2 only)
Response compression, wraps stream, gets auto-adapted pipe, doesn’t implement IHttpResponseCompletionFeature yet, has terminator data.
HttpSys & IIS in-proc, only exposes stream, gets auto-adapted pipe, don’t implement IHttpResponseCompletionFeature

Scenario:	Native Kestrel Pipes	Adapted Pipe over Stream	Adapted Stream over Pipe
Write & Advance	Auto flushes at the end of request	Doesn’t auto-flush (soon enough), truncates	N/A – Always flushes
Pipe CompleteAsync	Flushes and completes response	Flushes, inconsistent	N/A – DisposeAsync noops
Feature CompleteAsync	Flushes and completes response	Doesn’t auto-flush (soon enough), can truncate	Doesn’t auto-flush (soon enough), can truncate
Pipe Complete	Background Flushes and completes response	Sync flush, throws	N/A – Dispose noops

Kestrel GetMemory & Advance W/Response Compression

info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/1.1 GET http://localhost:5000/pipe
dbug: Microsoft.AspNetCore.Server.Kestrel[9]
      Connection id "0HLOAQKG9CBKA" completed keep alive response.
fail: Microsoft.AspNetCore.Server.Kestrel[13]
      Connection id "0HLOAQKG9CBKA", Request id "0HLOAQKG9CBKA:00000003": An unhandled exception was thrown by the application.
System.InvalidOperationException: Headers are read-only, response has already started.
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpHeaders.ThrowHeadersReadOnlyException() in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http\HttpHeaders.cs:line 85
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpHeaders.Microsoft.AspNetCore.Http.IHeaderDictionary.set_Item(String key, StringValues value) in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http\HttpHeaders.cs:line 49
   at Microsoft.AspNetCore.Http.ParsingHelpers.SetHeaderUnmodified(IHeaderDictionary headers, String key, Nullable`1 values) in D:\github\AspNetCore\src\Http\Http.Abstractions\src\Internal\ParsingHelpers.cs:line 111
   at Microsoft.AspNetCore.Http.ParsingHelpers.AppendHeaderUnmodified(IHeaderDictionary headers, String key, StringValues values) in D:\github\AspNetCore\src\Http\Http.Abstractions\src\Internal\ParsingHelpers.cs:line 161
   at Microsoft.AspNetCore.Http.HeaderDictionaryExtensions.Append(IHeaderDictionary headers, String key, StringValues value) in D:\github\AspNetCore\src\Http\Http.Abstractions\src\Extensions\HeaderDictionaryExtensions.cs:line 18
   at Microsoft.AspNetCore.ResponseCompression.BodyWrapperStream.OnWrite() in D:\github\AspNetCore\src\Middleware\ResponseCompression\src\BodyWrapperStream.cs:line 227
   at Microsoft.AspNetCore.ResponseCompression.BodyWrapperStream.Write(Byte[] buffer, Int32 offset, Int32 count) in D:\github\AspNetCore\src\Middleware\ResponseCompression\src\BodyWrapperStream.cs:line 124
   at System.IO.Stream.Write(ReadOnlySpan`1 buffer)
   at System.IO.Pipelines.StreamPipeWriter.FlushInternal()
   at System.IO.Pipelines.StreamPipeWriter.Complete(Exception exception)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.<>c.<Microsoft.AspNetCore.Http.Features.IResponseBodyPipeFeature.get_Writer>b__353_0(Object self) in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http\HttpProtocol.FeatureCollection.cs:line 250
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.FireOnCompletedMayAwait(Stack`1 onCompleted) in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http\HttpProtocol.cs:line 819
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished in 57.258ms 200 text/plain

HttpSys GetMemory & Advance

https://github.com/aspnet/AspNetCore/issues/5885 NotifyOnCompleted does not catch exceptions, may abort the response/connection
info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/1.1 GET http://localhost:5000/
fail: Microsoft.AspNetCore.Server.HttpSys.MessagePump[0]
      ProcessRequestAsync
System.InvalidOperationException: Synchronous IO APIs are disabled, see AllowSynchronousIO.
   at Microsoft.AspNetCore.Server.HttpSys.ResponseBody.Write(Byte[] buffer, Int32 offset, Int32 count) in D:\github\AspNetCore\src\Servers\HttpSys\src\RequestProcessing\ResponseBody.cs:line 466
   at Microsoft.AspNetCore.Server.HttpSys.ResponseStream.Write(Byte[] buffer, Int32 offset, Int32 count) in D:\github\AspNetCore\src\Servers\HttpSys\src\ResponseStream.cs:line 66
   at System.IO.Stream.Write(ReadOnlySpan`1 buffer)
   at System.IO.Pipelines.StreamPipeWriter.FlushInternal()
   at System.IO.Pipelines.StreamPipeWriter.Complete(Exception exception)
   at Microsoft.AspNetCore.Http.Features.ResponseBodyPipeFeature.<>c.<get_Writer>b__5_0(Object self) in D:\github\AspNetCore\src\http\http\src\Features\ResponseBodyPipeFeature.cs:line 38
   at Microsoft.AspNetCore.Server.HttpSys.FeatureContext.NotifyOnCompletedAsync() in D:\github\AspNetCore\src\Servers\HttpSys\src\FeatureContext.cs:line 616
   at Microsoft.AspNetCore.Server.HttpSys.MessagePump.ProcessRequestAsync(Object requestContextObj) in D:\github\AspNetCore\src\Servers\HttpSys\src\MessagePump.cs:line 211
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished in 140.8066ms 200 text/plain

Kestrel GetMemory, Advance, & FlushAsync W/Response Compression

info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/1.1 GET http://localhost:5000/pipe
dbug: Microsoft.AspNetCore.Server.Kestrel[2]
      Connection id "0HLOAQNF524MG" stopped.
dbug: Microsoft.AspNetCore.ResponseCompression.ResponseCompressionProvider[8]
      The response will be compressed with 'gzip'.
dbug: Microsoft.AspNetCore.Server.Kestrel[9]
      Connection id "0HLOAQNF524MF" completed keep alive response.
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished in 5.2808ms 200 text/plain

HttpSys GetMemory, Advance, & FlushAsync

info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/1.1 GET http://localhost:5000/
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished in 110.8953ms 200 text/plain

Kestrel GetMemory, Advance, & Complete W/Response Compression

info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/1.1 GET http://localhost:5000/pipe
dbug: Microsoft.AspNetCore.ResponseCompression.ResponseCompressionProvider[8]
      The response will be compressed with 'gzip'.
fail: Microsoft.AspNetCore.Server.Kestrel[13]
      Connection id "0HLOAQPE40KS4", Request id "0HLOAQPE40KS4:00000003": An unhandled exception was thrown by the application.
System.InvalidOperationException: Synchronous operations are disallowed. Call WriteAsync or set AllowSynchronousIO to true instead.
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpResponseStream.Write(Byte[] buffer, Int32 offset, Int32 count) in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http\HttpResponseStream.cs:line 85
   at System.IO.Compression.DeflateStream.WriteDeflaterOutput()
   at System.IO.Compression.DeflateStream.WriteCore(ReadOnlySpan`1 buffer)
   at System.IO.Compression.DeflateStream.Write(Byte[] array, Int32 offset, Int32 count)
   at System.IO.Compression.GZipStream.Write(Byte[] array, Int32 offset, Int32 count)
   at Microsoft.AspNetCore.ResponseCompression.BodyWrapperStream.Write(Byte[] buffer, Int32 offset, Int32 count) in D:\github\AspNetCore\src\Middleware\ResponseCompression\src\BodyWrapperStream.cs:line 128
   at System.IO.Stream.Write(ReadOnlySpan`1 buffer)
   at System.IO.Pipelines.StreamPipeWriter.FlushInternal()
   at System.IO.Pipelines.StreamPipeWriter.Complete(Exception exception)
   at ResponseCompressionSample.Startup.<>c.<<Configure>b__1_4>d.MoveNext() in D:\github\AspNetCore\src\Middleware\ResponseCompression\sample\Startup.cs:line 53
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.AspNetCore.Builder.Extensions.MapMiddleware.Invoke(HttpContext context) in D:\github\AspNetCore\src\Http\Http.Abstractions\src\Extensions\MapMiddleware.cs:line 64
   at Microsoft.AspNetCore.ResponseCompression.ResponseCompressionMiddleware.Invoke(HttpContext context) in D:\github\AspNetCore\src\Middleware\ResponseCompression\src\ResponseCompressionMiddleware.cs:line 77
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application) in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http\HttpProtocol.cs:line 634
dbug: Microsoft.AspNetCore.Server.Kestrel[9]
      Connection id "0HLOAQPE40KS4" completed keep alive response.
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished in 41.002ms 500

HttpSys GetMemory, Advance, & Complete

info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/1.1 GET http://localhost:5000/
fail: Microsoft.AspNetCore.Server.HttpSys.MessagePump[0]
      ProcessRequestAsync
System.InvalidOperationException: Synchronous IO APIs are disabled, see AllowSynchronousIO.
   at Microsoft.AspNetCore.Server.HttpSys.ResponseBody.Write(Byte[] buffer, Int32 offset, Int32 count) in D:\github\AspNetCore\src\Servers\HttpSys\src\RequestProcessing\ResponseBody.cs:line 466
   at Microsoft.AspNetCore.Server.HttpSys.ResponseStream.Write(Byte[] buffer, Int32 offset, Int32 count) in D:\github\AspNetCore\src\Servers\HttpSys\src\ResponseStream.cs:line 66
   at System.IO.Stream.Write(ReadOnlySpan`1 buffer)
   at System.IO.Pipelines.StreamPipeWriter.FlushInternal()
   at System.IO.Pipelines.StreamPipeWriter.Complete(Exception exception)
   at SelfHostServer.Startup.<>c.<<Configure>b__1_0>d.MoveNext() in D:\github\AspNetCore\src\Servers\HttpSys\samples\SelfHostServer\Startup.cs:line 39
--- End of stack trace from previous location where exception was thrown ---
   at Microsoft.AspNetCore.Server.HttpSys.MessagePump.ProcessRequestAsync(Object requestContextObj) in D:\github\AspNetCore\src\Servers\HttpSys\src\MessagePump.cs:line 206
   at Microsoft.AspNetCore.Server.HttpSys.MessagePump.ProcessRequestAsync(Object requestContextObj) in D:\github\AspNetCore\src\Servers\HttpSys\src\MessagePump.cs:line 212
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished in 120.74050000000001ms 200 text/plain

Kestrel GetMemory, Advance, & CompleteAsync W/Response Compression

info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/1.1 GET http://localhost:5000/pipe
dbug: Microsoft.AspNetCore.ResponseCompression.ResponseCompressionProvider[8]
      The response will be compressed with 'gzip'.
dbug: Microsoft.AspNetCore.Server.Kestrel[9]
      Connection id "0HLOAQQA4QBLR" completed keep alive response.
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished in 13.105300000000002ms 200 text/plain

HttpSys GetMemory, Advance, & CompleteAsync

info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/1.1 GET http://localhost:5000/
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished in 112.0616ms 200 text/plain

Kestrel GetMemory, Advance, & Feature.CompleteAsync W/Response Compression (No response body received)

info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/2 GET https://localhost:5001/pipe
dbug: Microsoft.AspNetCore.ResponseCompression.ResponseCompressionProvider[2]
      No response compression available for HTTPS requests. See ResponseCompressionOptions.EnableForHttps.
fail: Microsoft.AspNetCore.Server.Kestrel[13]
      Connection id "0HLOARHOOPTIP", Request id "0HLOARHOOPTIP:00000001": An unhandled exception was thrown by the application.
System.InvalidOperationException: Synchronous operations are disallowed. Call WriteAsync or set AllowSynchronousIO to true instead.
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpResponseStream.Write(Byte[] buffer, Int32 offset, Int32 count) in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http\HttpResponseStream.cs:line 85
   at Microsoft.AspNetCore.ResponseCompression.BodyWrapperStream.Write(Byte[] buffer, Int32 offset, Int32 count) in D:\github\AspNetCore\src\Middleware\ResponseCompression\src\BodyWrapperStream.cs:line 136
   at System.IO.Stream.Write(ReadOnlySpan`1 buffer)
   at System.IO.Pipelines.StreamPipeWriter.FlushInternal()
   at System.IO.Pipelines.StreamPipeWriter.Complete(Exception exception)
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.<>c.<Microsoft.AspNetCore.Http.Features.IResponseBodyPipeFeature.get_Writer>b__353_0(Object self) in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http\HttpProtocol.FeatureCollection.cs:line 250
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.FireOnCompletedMayAwait(Stack`1 onCompleted) in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http\HttpProtocol.cs:line 819
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished in 44.0974ms 200 text/plain

Kestrel GetMemory, Advance, FlushAsync, & Feature.CompleteAsync W/Response Compression

info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/2 GET https://localhost:5001/pipe
dbug: Microsoft.AspNetCore.ResponseCompression.ResponseCompressionProvider[8]
      The response will be compressed with 'br'.
fail: Microsoft.AspNetCore.Server.Kestrel[13]
      Connection id "0HLOAR3I2M8FQ", Request id "0HLOAR3I2M8FQ:00000005": An unhandled exception was thrown by the application.
System.InvalidOperationException: Writing is not allowed after writer was completed.
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.Http2OutputProducer.ThrowIfSuffixSent() in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http2\Http2OutputProducer.cs:line 435
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2.Http2OutputProducer.WriteDataToPipeAsync(ReadOnlySpan`1 data, CancellationToken cancellationToken) in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http2\Http2OutputProducer.cs:line 298
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.WritePipeAsync(ReadOnlyMemory`1 data, CancellationToken cancellationToken) in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http\HttpProtocol.cs:line 1495
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpResponsePipeWriter.WriteAsync(ReadOnlyMemory`1 source, CancellationToken cancellationToken) in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http\HttpResponsePipeWriter.cs:line 70
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpResponseStream.WriteAsyncInternal(ReadOnlyMemory`1 source, CancellationToken cancellationToken) in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http\HttpResponseStream.cs:line 138
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpResponseStream.WriteAsync(ReadOnlyMemory`1 source, CancellationToken cancellationToken) in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http\HttpResponseStream.cs:line 133
   at System.IO.Compression.BrotliStream.WriteAsyncMemoryCore(ReadOnlyMemory`1 buffer, CancellationToken cancellationToken, Boolean isFinalBlock)
   at System.IO.Compression.BrotliStream.DisposeAsync()
   at Microsoft.AspNetCore.ResponseCompression.ResponseCompressionMiddleware.Invoke(HttpContext context) in D:\github\AspNetCore\src\Middleware\ResponseCompression\src\ResponseCompressionMiddleware.cs:line 78
   at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.HttpProtocol.ProcessRequests[TContext](IHttpApplication`1 application) in D:\github\AspNetCore\src\Servers\Kestrel\Core\src\Internal\Http\HttpProtocol.cs:line 634
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished in 12.3461ms 200 text/plain

Tratcher · 2019-07-17T21:26:03Z

Proposals:
Kestrel needs to implement PipeWriter.CompleteAsync.
Kestrel's PipeWriter.Complete should throw for unflushed data to be consistent with the adapters.
Consolidate all of the body control APIs to a single modo feature. See below.

HttpRequest/Response.Body Set adapts
Add HttpRequest/Response.BodyWriter Set to adapt

New mondo features:

IHttpRequestBodyFeature
Body {Get;}
BodyReader {Get;}
IHttpBufferingFeature (Request & Resonse)

IHttpResponseBodyFeature
Body
BodyWriter - Note Servers or middleware implementing this with an adapter will become responsible for flushing it as part of CompleteAsync / request unwind.
IHttpBufferingFeature (Request & Resonse)
IHttpSendFileFeature.SendFileAsync
StartAsync
CompleteAsync

Obsolete
IHttpResponseFeature.Body - OR Proxy to IHttpResponseBodyFeature? Set adapts all features?
IHttpBufferingFeature - Polyfill in all servers to proxy to the new feature?

Delete
IHttpResponseBodyPipeFeature
IHttpResponseStartFeature.StartAsync
IHttpResponseCompleteFeature.CompleteAsync

Tratcher · 2019-07-18T17:42:23Z

What about IHttpBodyControlFeature.AllowSynchronousIO?

davidfowl · 2019-07-18T17:53:26Z

I think this feature needs to be an abstract base class the moment we need to add optional features like that. Do you have a shape in mind yet? Can you write a class proposal?

Tratcher · 2019-07-18T18:20:18Z

I'm experimenting locally. An abstract base class would make it really hard for servers to implement, they all have custom feature collections.

davidfowl · 2019-07-18T20:55:10Z

I'm experimenting locally. An abstract base class would make it really hard for servers to implement, they all have custom feature collections.

True, it means we're going to lean on default interface methods.

analogrelay · 2019-07-26T20:54:05Z

@Tratcher to close once the breaking changes are announced.

Tratcher · 2019-07-26T22:34:00Z

#12635

Tratcher added bug This issue describes a behavior which is not expected - a bug. feature-http-abstractions area-servers labels Jun 17, 2019

JamesNK mentioned this issue Jun 17, 2019

TestServer sends remaining content in pipe writer on complete #11268

Closed

analogrelay added the triage-review label Jun 18, 2019

analogrelay added this to the 3.0.0-preview7 milestone Jun 18, 2019

analogrelay modified the milestones: 3.0.0-preview7, 3.0.0-preview8 Jun 25, 2019

Tratcher mentioned this issue Jun 26, 2019

Implement response BodyWriter, CompleteAsync for TestServer #11598

Open

4 tasks

Tratcher self-assigned this Jul 17, 2019

Tratcher added a commit that referenced this issue Jul 18, 2019

Spike of unified response body features #11305

ec12fab

Tratcher mentioned this issue Jul 18, 2019

Unified response body features #12328

Merged

davidfowl added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jul 22, 2019

Tratcher added a commit that referenced this issue Jul 24, 2019

Spike of unified response body features #11305

f2d1406

Tratcher added the Done This issue has been fixed label Jul 26, 2019

Tratcher closed this as completed Jul 26, 2019

ghost locked as resolved and limited conversation to collaborators Dec 3, 2019

amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Jun 2, 2023

BodyWriter polyfill doesn't flush, causes truncation #11305

BodyWriter polyfill doesn't flush, causes truncation #11305

Comments

Tratcher commented Jun 17, 2019

davidfowl commented Jun 17, 2019

Uh oh!

Tratcher commented Jun 17, 2019

Uh oh!

Tratcher commented Jun 17, 2019

Uh oh!

davidfowl commented Jun 17, 2019

Uh oh!

Tratcher commented Jun 17, 2019

Uh oh!

davidfowl commented Jun 17, 2019

Uh oh!

Tratcher commented Jun 18, 2019

Uh oh!

analogrelay commented Jun 18, 2019

Uh oh!

JamesNK commented Jun 19, 2019

Uh oh!

halter73 commented Jun 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

analogrelay commented Jul 9, 2019

Uh oh!

davidfowl commented Jul 10, 2019

Uh oh!

Tratcher commented Jul 10, 2019

Uh oh!

davidfowl commented Jul 10, 2019

Uh oh!

Tratcher commented Jul 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidfowl commented Jul 10, 2019

Uh oh!

Tratcher commented Jul 10, 2019

Uh oh!

analogrelay commented Jul 16, 2019

Uh oh!

Tratcher commented Jul 17, 2019

Uh oh!

Tratcher commented Jul 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Tratcher commented Jul 18, 2019

Uh oh!

davidfowl commented Jul 18, 2019

Uh oh!

Tratcher commented Jul 18, 2019

Uh oh!

davidfowl commented Jul 18, 2019

Uh oh!

analogrelay commented Jul 26, 2019

Uh oh!

Tratcher commented Jul 26, 2019

Uh oh!

halter73 commented Jun 26, 2019 •

edited

Loading

Tratcher commented Jul 10, 2019 •

edited

Loading

Tratcher commented Jul 17, 2019 •

edited

Loading