search

aspnet / AspNetWebStack

ASP.NET MVC 5.x, Web API 2.x, and Web Pages 3.x (not ASP.NET Core)
Other
858 stars 354 forks source link

Shouldn't the StreamReader be disposed here? #276

Closed Bouke closed 4 years ago

Bouke commented 4 years ago

https://github.com/aspnet/AspNetWebStack/blob/ebef5b7d821b64ed5c48765d0caf6ce8a9bcfaf5/src/System.Web.Mvc/JsonValueProviderFactory.cs#L51

The StreamReader isn't disposed, which I suppose should happen here?

Bouke commented 4 years ago

By default StreamReader will close the stream it's reading from. This would close the request's input stream, which is not desired. Looking at StreamReader.Dispose it looks like closing the stream is the only thing it does. So it probably doesn't really matter that much, other than not disposing an IDisposable being a code smell.

dougbu commented 4 years ago

At worst the StreamReader object will live a little longer. Not worth using the (ugly) constructor that tells the StreamReader it doesn't own the underlying Stream.