raygun4net icon indicating copy to clipboard operation
raygun4net copied to clipboard

Improper Resource Shutdown or Release

Open Andrew1178 opened this issue 6 years ago • 2 comments

Really small but this was highlighted in a veracode scan of ours.

In RaygunRequestMessageBuilder.cs line 114 you guys create a new instance of StreamReader and don' t dispose of it. Can you please wrap it in a using? Or in your try catch dispose of it?

Thanks

https://github.com/MindscapeHQ/raygun4net/blob/master/Mindscape.Raygun4Net/Builders/RaygunRequestMessageBuilder.cs

Andrew1178 avatar May 05 '18 00:05 Andrew1178

Calling Dispose() on the StreamReader will also dispose the underlying stream, in this case request.InputStream. It's not raygun's responsibility to dispose of that since it didn't create it (the framework did, and it will dispose of it at the end of the request).

If you dispose of the underlying stream then any caller after raygun won't have access to it. There's a constructor overload that takes a bool indicating if you want to leave the stream open when calling dispose, but that achieves the same result as what the code does currently.

This is the Dispose() method of the StreamReader class

protected override void Dispose(bool disposing)
{
    // Dispose of our resources if this StreamReader is closable.
    // Note that Console.In should be left open.
    try {
        // Note that Stream.Close() can potentially throw here. So we need to 
        // ensure cleaning up internal resources, inside the finally block.  
        if (!LeaveOpen && disposing && (stream != null))
            stream.Close();
    }
    finally {
        if (!LeaveOpen && (stream != null)) {
            stream = null;
            encoding = null;
            decoder = null;
            byteBuffer = null;
            charBuffer = null;
            charPos = 0;
            charLen = 0;
            base.Dispose(disposing);
        }
    }
}

xt0rted avatar May 05 '18 01:05 xt0rted

Thanks, we could look at using the constructor overload or perhaps using Stream.Read instead of creating a new reader.

QuantumNightmare avatar May 07 '18 01:05 QuantumNightmare