-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Abort gracefully closes the response rather than aborting it #31404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Thanks for contacting us. |
30 day stats: 22 failures out of 74 runs, 60% pass rate. Current error:
|
This test is now failing 100% since 7/27. @halter73 that aligns with your change here: Any idea why that would impact Abort? |
Still failing 100%, same error. |
1 similar comment
Still failing 100%, same error. |
Still failing 100%
Is that just a chunked terminator? Oh, the test was supposed to abort the connection rather than close gracefully. |
On further investigation this is a product bug that can cause unreported response truncation of chunked responses (E.g. data corruption). Here's the flow: aspnetcore/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs Lines 277 to 284 in b9fcd82
IISHttpContext.Abort -> OuputProducer.Abort -> _pipe.Writer.Complete() (without an error) aspnetcore/src/Servers/IIS/IIS/src/Core/IISHttpContext.IO.cs Lines 153 to 180 in b9fcd82
WriteBody wakes up and decides the pipe is complete so it gracefully closes the response. There's no check in WriteBody that the response was aborted. Later Abort calls HttpCloseConnection to actually abort the connection, but the chunked terminator has already been incorrectly sent so the client can't tell the response aborted abnormally. Changing Abort to call HttpCloseConnection first does fix the test, but I think there are side-effects where that causes write errors. A more correct fix would probably be for OutputProducer to complete the response body pipe with an error so it knows not to gracefully close the response. |
Thanks for contacting us. We're moving this issue to the |
Keeping open in case we want to port to 6.0 |
Merged to 6.0.6 in #41360 |
Logs
https://dev.azure.com/dnceng/public/_build/results?buildId=1062917&view=ms.vss-test-web.build-test-results-tab&runId=32739106&resultId=110110&paneView=debug
The text was updated successfully, but these errors were encountered: