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.

Answer 3

Generally, you should put your FileStream inside a using block as already described by other answers or have some faith that it will be disposed of by other parts of your code. However, this is tricky when you are returning a FileStream from your controller. For example:

public ActionResult GetImage()
{            
    Stream stream = //get the stream
    return base.File( stream, "image/png" ); 
}

Happily, the stream is disposed of by the framework once written, so you don't need to worry about the disposing of it. See here for details.

Answer 4

This is not an answer to your exact requirement, but thought of sharing it so that it can give you an idea of other ways of doing this.

public async Task<HttpResponseMessage> Get(string acctNumber, string stmtDate)
{
    HttpResponseMessage response2 = new HttpResponseMessage();

    HttpClient client= new HttpClient();
    string url = "http://localhost:9090/BusinessLayer?acctNumber=" + acctNumber + "&stmtDate=" + stmtDate;
    // NOTE 1: here we are only reading the response1's headers and not the body. The body of response1 would be later be 
    // read by response2.
    HttpResponseMessage response1 = await client.GetAsync(url, HttpCompletionOption.ResponseHeadersRead);

    // NOTE 2: here we need not close response1's stream as Web API would close it when disposing
    // response2's stream content.
    response2.Content = new StreamContent(await response1.Content.ReadAsStreamAsync());
    response2.Content.Headers.ContentLength = response1.Content.Headers.ContentLength;
    response2.Content.Headers.ContentType = response1.Content.Headers.ContentType;

    return response2;
}