When ApiVersionOptions.ReportApiVersions is true and Response.Body stream in used in a controller an exception is thrown

[gpcastro]

In a ASP.NET Core web API project, an exception is thrown when ReportApiVersions is true as the versioning framework tries to add a header to an HTTP response after the response started being streamed to the client.

Simple repro:

1. In an ASP.NET core web API, have this in the Startup:

services.AddApiVersioning(o =>
{
    o.ReportApiVersions = true;
    o.AssumeDefaultVersionWhenUnspecified = true; // only needed to facilitate the test, not needed for repro if the api version is supplied.
});

2. In a controller class, add a version, e.g.

[ApiVersion("1.0")]
[Route("api/[controller]")]
public class ValuesController : Controller

3. Then in the method, write something to Reponse.Body, for example:

[HttpGet]
public async Task Get()
{
    Response.StatusCode = 200;
    using (var writer = new StreamWriter(Response.Body))
    {
        await writer.WriteAsync("Hello");
    }
}

When the controller method is fired, an exception is thrown as the versioning framework tries to add a header to the response after the HTTP streaming has started, and thus the header had already been sent.

[commonsensesoftware]

Interesting ... there are really two parts here. First, the action filter should not fail, so I'll accept that as a bug. Second, you should arguably not be writing a response like that. Usually you'd use something like ContentResult here.

One solution would be to add support for the equivalent of the 'ol filter overrides from Web API. While it's possible in Core, it's unfortunately not supported out-of-the-box. Refer to aspnet/Mvc#474.

To make the action filter do the right thing, there are two options:

1. Skip writing headers if the response is started
2. Write the headers when the action starts instead of when it finishes

I think I prefer the second option since the first option would break the feature. In Web API, you couldn't necessarily add the response headers before the action started. I don't think is an issue with Core so I don't immediately see any issues with writing the headers when the action starts instead of when it finishes. Even if the action fails, you'd almost certainly still want the headers written.

Unless there are other concerns or opinions, I'll queue up the options 2 change for the next round of fixes. In the meantime, I recommend using encapsulating the response in an IActionResult such as ContentResult.

[gpcastro]

I agree this is not the best way to write a response, and use something as IActionResult would be better. However the provided steps are a simplification of my scenario for the purposes repro'ing the bug; I need the StreamWriter because in my case it is actually an http stream, where something is running in the server and data has to be sent to the client as it happens.

In other scenarios where we have to deal with this issue we use the option 2 that you suggested, as the clients might in fact expect the headers to be there.

[DrEsteban]

@commonsensesoftware Assuming the versioning ActionFilter is implemented similarly to middleware classes, we have found this to be the proper way of setting response headers when HTTP streaming is involved:

internal class RequestIdMiddleware
{
    public RequestIdMiddleware(RequestDelegate next)
    { 
        this._next = next;
    }

    public async Task Invoke(HttpContext httpContext)
    {
        httpContext.Response.OnStarting(state =>
        {
            // Ensure x-ms-request-id, x-ms-correlation-request-id and Date header go out in the response
            var context = (HttpContext)state;
            context.Response.Headers.Add(...);
        }, httpContext);

        await this._next(httpContext);
    }
}

If you attempt to set the headers after the await this._next(), you'll probably get a "headers cannot be modified because the response has already started" exception, in HTTP streaming scenarios, because the _next handler only returns once the Controller method releases control. (At which point headers have already been sent, and the response has already finished streaming, in this case.)

It is much safer to do the work with a callback-type operation.

[maritiren]

Hi @commonsensesoftware , any plans for a new release containing the fix for this issue? Do you have any date in mind?

[commonsensesoftware]

I'm hoping by end of week. Worst case scenario will be beginning of next week. There's another fix I'm trying to roll in. Thanks for your patience.

[maritiren]

Awesome, thanks! 😁