Call ProduceEnd() before consuming request body if response has already started (#1530) #1537
Conversation
|
@CesarBS can you rebase on top of @halter73 's change |
| @@ -133,16 +133,28 @@ public Frame(IHttpApplication<TContext> application, ConnectionContext context) | |||
| { | |||
| ResumeStreams(); | |||
|
|
|||
| if (HasResponseStarted) | |||
| { | |||
| // If the response has already started, call ProduceEnd() before | |||
There was a problem hiding this comment.
Pointed to the explanation in the corefx bug.
|
@davidfowl Rebased. |
| await httpContext.Response.WriteAsync("hello, world"); | ||
|
|
||
| // Force response start | ||
| await httpContext.Response.Body.FlushAsync(); |
There was a problem hiding this comment.
You don't have to call FlushAsync to start the response, that happens on write
|
Can you make sure we have a test for all of the relevant message body types? (Opaque, ContentLength, Chunked) |
| // If the expected behavior is regressed, this will hang because the | ||
| // server will try to consume the request body before flushing the chunked | ||
| // terminator. | ||
| await connection.Receive( |
There was a problem hiding this comment.
Did you split this into two calls to connection.Receive() just to insert the above comment?
There was a problem hiding this comment.
No. It's to make it clear that the first part will be received fine, but if there's a regression the connection will time out waiting for the chunk terminator.
| } | ||
|
|
||
| [Fact] | ||
| public async Task WhenResponseNotStartedResponseEndedAfterDrainingRequestBody() |
| } | ||
|
|
||
| [Fact] | ||
| public async Task Sending100ContinueDoesNotStartResponse() |
There was a problem hiding this comment.
Can we test sending the chunk terminator pre-consume when we send a 100-continue and the app sends a response?
|
Rebased. |
|
@davidfowl This is unrelated to the message body type, so having tests per type won't add value. What we care about here is the order in which things happen. |
|
📢 |
…dy started (#1537, backported to 1.0.x)
…dy started (#1537, backported to 1.1.x).
…dy started (#1537, backported to 1.0.x)
…dy started (#1537, backported to 1.1.x).
#1530
Prevents hanging clients waiting for the chunk terminator while server consumes the rest of the request body.
See https://github.com/dotnet/corefx/issues/17330#issuecomment-288248663.
cc @stephentoub