Is it safe to return a FileStream in a method in C#?

Question

In ASP.NET WebForms 4.5, I have WebAPI Controller with a GET method for getting a PDF.

Then down in the business layer of the application, I have an API class with a method that contains the logic for actually finding and returning the PDF to the controller.

So MyController class basically has:

public HttpResponseMessage GetStatement(string acctNumber, string stmtDate) {
    MyApi myApi = new MyApi();
    HttpResponseMessage response = new HttpResponseMessage(HttpStatusCode.OK);
    FileStream stream = myApi.GetStatement(acctNumber, stmtDate);
    ...set the response.Content = stream...
    ... set the mime type..
    ... close the stream...
    return response;
}

And MyApi class has:

public FileStream GetStatement(string acctNumber, string stmtDate) {
    ... makes an HttpWebRequest to get a PDF from another system ...
    HttpWebRequest req = WebRequest.Create(......)....
    FileStream stream = new FileStream(accountNumber +"_" + stmtDate + ".pdf", FileMode.Create);
    response.GetResponseStream().CopyTo(stream);
    return stream;
}

The API class is not in the web layer of the application because it's used by other (non-web) parts of the software.

I guess my concern is there's no explicit closing of the FileStream in the API method. I could do it in the Controller method, but I'd be relying on others to do the same when they're calling it from other areas.

Is there a better way to return the PDF file from the API method? Possibly just as a byte array or something like that? Preferably as little overhead as possible.

Thanks-

Answer 1

You should not be returning a file stream but instead an array of bytes. This way you can correctly dispose of the object correctly and not worry about other calling methods in the stack.

byte[] currentFile = ....

You can then deliver your file as follows this, a byte array is easy to convert to anything. Below example is for MVC4.

return new FileContentResult(currentFile, "application/pdf");

Answer 2

It's not uncommon for methods to return FileStreams, hoping that the caller will remember to put the method call in a using statement. But it's understandable to not want to make that assumption. One alternative is to use an interesting form of Inversion of Control, where you require the caller to give you the callback function that knows what to do with the FileStream, and then you wrap a call to that handler inside a using statement.

public T GetStatement<T>(string acctNumber, string stmtDate,
    Func<FileStream, T> callback) {
    ... makes an HttpWebRequest to get a PDF from another system ...
    HttpWebRequest req = WebRequest.Create(......)....
    using(FileStream stream = new FileStream(accountNumber +"_" + stmtDate + ".pdf", FileMode.Create))
    {
        response.GetResponseStream().CopyTo(stream);
        return callback(stream);
    }
}

However, this is going to require a little extra hackery in your use case because you're returning a response whose content is feeding through the stream, so you'd need to find a way to cause your message to push out the entire response before the callback returns.

It may be best in this case to just throw comments on your method to document the fact that you're expecting the caller to ensure the stream gets closed. That's what we do in our application, and it's worked well so far.