dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.35k stars 9.99k forks source link

API Endpoint Request.Body Stream Always Disposed #20419

Closed ahwm closed 4 years ago

ahwm commented 4 years ago

Describe the bug

When receiving POST requests from Amazon's Simple Notification Service, it's unreliable. Sometimes it will work and other times I get an "object disposed" exception (below). I've tried variations but none have been reliable. Once I downgraded to .NET Core 2.1 (or even 2.2) everything worked as expected.

On .NET Core 2 I could do this and have it work 100% of the time:

string body = "";
using (StreamReader sr = new StreamReader(Request.Body))
    body = sr.ReadToEnd();

To Reproduce

https://github.com/ahwm/aspnet3-closed-stream

--date------------
 2020-04-01T18:29:05
--type------------
 System.ObjectDisposedException
--Message---------
 Cannot access a disposed object.
Object name: 'FileBufferingReadStream'.
--Source----------
 Microsoft.AspNetCore.WebUtilities
--StackTrace------
    at Microsoft.AspNetCore.WebUtilities.FileBufferingReadStream.ThrowIfDisposed()
   at Microsoft.AspNetCore.WebUtilities.FileBufferingReadStream.ReadAsync(Byte[] buffer, Int32 offset, Int32 count, CancellationToken cancellationToken)
   at System.IO.StreamReader.ReadBufferAsync(CancellationToken cancellationToken)
   at System.IO.StreamReader.ReadToEndAsyncInternal()
   at SnsApiTest.Controllers.EmailController.Post() in C:\Websites\aspnet3-closed-stream\SnsApiTest\SnsApiTest\Controllers\EmailController.cs:line 39

Further technical details

From the server:

>dotnet --info
  It was not possible to find any installed .NET Core SDKs
  Did you mean to run .NET Core SDK commands? Install a .NET Core SDK from:
      https://aka.ms/dotnet-download

Host (useful for support):
  Version: 3.1.0
  Commit:  157910edee

.NET Core SDKs installed:
  No SDKs were found.

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.14 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.14 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download
Tratcher commented 4 years ago

The relevant code from your sample:

                Request.EnableBuffering();
                string body;
                using (StreamReader sr = new StreamReader(Request.Body, Encoding.UTF8, true, 1024, true))
                    body = await sr.ReadToEndAsync();
                Request.Body.Position = 0;

What happens if you remove Request.EnableBuffering(); and Request.Body.Position = 0;? You don't seem to be using that buffer for anything.

ahwm commented 4 years ago

Pretty much the same behavior. That is the code I ended up with from a number of suggestions I found on sites like stackoverflow. Initially, I just had:

string body;
using (StreamReader sr = new StreamReader(Request.Body))
    body = await sr.ReadToEndAsync();

But I ended up with some unreliable results. What I finally ended up with (the sample you pointed out) was more reliable but still wasn't the winning solution.

Tratcher commented 4 years ago

I'd expect different errors at least. The error given above is specific to the buffering code.

The main issue to expect when consuming the request body is that the client can disconnect and cause an IOException.

ahwm commented 4 years ago

I just set it back to that and ran it again. It worked several times but then failed with this:

--date------------
 2020-04-01T22:19:13
--type------------
 System.ObjectDisposedException
--Message---------
 Cannot access a disposed object.
Object name: 'HttpRequestStream'.
--Source----------
 Microsoft.AspNetCore.Server.IIS
--StackTrace------
    at Microsoft.AspNetCore.Server.IIS.Core.HttpRequestStream.ValidateState(CancellationToken cancellationToken)
   at Microsoft.AspNetCore.Server.IIS.Core.HttpRequestStream.ReadAsync(Memory`1 destination, CancellationToken cancellationToken)
   at Microsoft.AspNetCore.Server.IIS.Core.WrappingStream.ReadAsync(Memory`1 destination, CancellationToken cancellationToken)
   at System.IO.StreamReader.ReadBufferAsync(CancellationToken cancellationToken)
   at System.IO.StreamReader.ReadToEndAsyncInternal()
   at SnsApiTest.Controllers.EmailController.Post() in C:\Websites\aspnet3-closed-stream\SnsApiTest\SnsApiTest\Controllers\EmailController.cs:line 37

https://github.com/ahwm/aspnet3-closed-stream/blob/4ff9100b7b1d2adfd8043d97efb74efc576fbe79/SnsApiTest/SnsApiTest/Controllers/EmailController.cs#L37

Tratcher commented 4 years ago

Ah, I see it: https://github.com/ahwm/aspnet3-closed-stream/blob/4ff9100b7b1d2adfd8043d97efb74efc576fbe79/SnsApiTest/SnsApiTest/Controllers/EmailController.cs#L30

NEVER use async void, it doesn't wait for your code to run and it starts cleaning up the request. Use async Task instead.

ahwm commented 4 years ago

Ohhh. I see. The async "requirement" was new-ish with .NET Core 3, I think. In the .NET Core 2 version of this it was always just void. I'll give that a whirl.

Thanks.

ghost commented 4 years ago

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.