runtime (original) (raw)

Backport of #118041 to release/10.0

Customer Impact

Customer reported
Found internally

Reported in dotnet/aspnetcore#64323. Customers using request buffering (or similar seekable streams) and reading through the request before framework/app code processes the request are now getting empty data or errors in the app/framework code.

The expectation is that if using a seekable stream and doing some (pre-)processing of the request before letting application code see the request, then the application code should be able to read from the stream and get the full request as if no (pre-)processing occurred.

Regression

StreamPipeReader didn't regress, but ASP.NET Core used it in more places in 10.0 which caused a regression in ASP.NET Core. dotnet/aspnetcore#62895 added PipeReader support for Json deserialization to most of ASP.NET Core's json parsing code which meant the StreamPipeReader ended up getting used during json deserialization where previously it was using the Stream.

Testing

Verified that the original issues described in dotnet/aspnetcore#64323 were fixed with this change. (both MVCs IAsyncResourceFilter and Minimals IEndpointFilter)

Also verified skipped test
https://github.com/dotnet/aspnetcore/blob/1a2c0fd99c05db9f9ef167165386d14d3e24c9b4/src/Http/Http.Extensions/test/RequestDelegateGenerator/RequestDelegateCreationTests.BindAsync.cs#L257-L258
was fixed by this change.

Risk

Low;
This could cause a regression if someone was reading from a Stream after it had returned 0 (exceptions below). This is not a common pattern as most code would exit the read loop or be done processing the Stream at that point.

Scanned a few dozen Stream implementations to verify that calling read after a previous read returned 0 does not throw. Most code that calls read after a previous read returned 0 are either seeking (rewinding) or doing zero-byte reads.