dotnet
TestHost response body does not support 0-byte reads
Is there an existing issue for this?
- [X] I have searched the existing issues
Describe the bug
Similar to #41287 and #41305, TestHost also doesn't expect/support 0-byte reads on the body.
https://github.com/dotnet/aspnetcore/blob/3888fda44771dd6d45f305a831b2f37427303ae4/src/Hosting/TestHost/src/ResponseBodyReaderStream.cs#L118-L121
Expected Behavior
The 0-byte read should not throw and subsequent reads should return available data.
Steps To Reproduce
See yarp.zip (repro attached in the original Yarp issue)
Exceptions (if any)
Yarp.ReverseProxy.Forwarder.HttpForwarder[48]
ResponseBodyDestination: The destination reported an error when copying the response body.
System.ArgumentOutOfRangeException: (Parameter 'count')
Actual value was 0.
at Microsoft.AspNetCore.TestHost.ResponseBodyReaderStream.VerifyBuffer(Byte[] buffer, Int32 offset, Int32 count)
at Microsoft.AspNetCore.TestHost.ResponseBodyReaderStream.ReadAsync(Byte[] buffer, Int32 offset, Int32 count, CancellationToken cancellationToken)
at Yarp.ReverseProxy.Forwarder.StreamCopier.CopyAsync(Stream input, Stream output, Int64 promisedContentLength, StreamCopierTelemetry telemetry, ActivityCancellationTokenSource activityToken, CancellationToken cancellation)
.NET Version
6.0.5
Anything else?
Discovered by a user writing unit tests that include Yarp https://github.com/microsoft/reverse-proxy/issues/1720
cc: @Tratcher @adityamandaleeka
There kind of failures can be caught by StreamConformanceTests we run in runtime. For example, here are 0-byte read tests for HttpClient response streams. Are there any plans to include such tests in AspNetCore (for this specific kind of issue, it may be easier to just manually audit all stream implementations)?
I have a branch where I've started working on it, but I have some higher priorities at the moment.
Any update on this? This prevents us to move to YARP 1.1 in our product (and must effect everyone doing testing with the test host).
I did some testing preparing to fix this and realized that it was partially fixed in 7.0 preview1 by https://github.com/dotnet/aspnetcore/blame/8eadd8f7e2b46387ea6f121a8d9f9bf003cf3f2c/src/Hosting/TestHost/src/ResponseBodyReaderStream.cs#L76. This introduced new Memory overloads and bypassed the problematic validation. YARP is already calling those overloads. Can folks try this on 7.0 to confirm everything works for them end-to-end?
The patch has been approved for 6.0.11 (Nov). I'm not planning to port this to 7.0 since YARP doesn't seem to be affected there, unless someone reports otherwise.
Merged for 6.0.11. Leaving this open to remind me to port the fix to main.