No CORS headers sent in case of error 500

[Flavien]

When the controller throws an exception, which results in an error 500, the CORS headers are not sent.

Is that by design? If so, why? If not, that's probably a bug.

[barriestewart]

I just upgraded from beta7 -> beta8 and was wondering why CORS worked for most of my requests but one was showing as an error in Chrome with the response containing no Access-Control-Allow-Origin header. As was the case with @Flavien, it turns out my controller was throwing an exception resulting in a 500 with no CORS headers. I wish I had read this issue before spending 30 minutes trying different CORS configurations in my Startup class.

[Eilon]

@kichalla can you check this out and figure out what the correct behavior should be?

[kspearrin]

This may be happening with any non-200 response. I too am seeing it on 400s.

[kspearrin]

Further investigation has shown me that this only seems to be happening on responses outside of the MVC pipeline. For example, I can return a status of 400 from a controller and do not have this issue, however, if I return a status of 400 from UseExceptionHandler middleware, CORS is not respected.

[kichalla]

This is by design due to the following reasons:

• Even if we decide to support a feature for this scenario, sending full exception details across origins is probably not a good idea.
• In the scenario where an exception is thrown in MVC (ex: controller's action), then you could write an exception filter to handle the exception and sanitize the outgoing message...for example, you could send out a json response with appropriate information and 500 status code.

[kspearrin]

In the scenario where an exception is thrown in MVC (ex: controller's action), then you could write an exception filter to handle the exception and sanitize the outgoing message...for example, you could send out a json response with appropriate information and 500 status code.

This is what I am trying to achieve with the UseExceptionHandler. I guess that's just not the right tool for this job then?

[kspearrin]

For future reference,I was able to achive this with @kichalla's recommendation by using an ExceptionFilterAttribute:

public class ExceptionHandlerFilterAttribute : ExceptionFilterAttribute
{
    public override void OnException(ExceptionContext context)
    {
        var errorModel = new ErrorResponseModel { Message = "An error has occured." };

        var exception = context.Exception;
        if(exception == null)
        {
            // should never happen
            return;
        }

        if(exception is ApplicationException)
        {
            context.HttpContext.Response.StatusCode = 400;
            // ...
        }

        // Other exception types you want to handle ...

        context.Result = new ObjectResult(errorModel);
    }
}

Thanks.

[Eilon]

Right, an MVC exception filter is the place to do this.

[iwbo]

Has this issue been revisited?
I mean, is it really a CORS policies job to prevent sending out full exception details?

But the main problem for me is that if I allow some domain do make CORS requests, I expect them to get all the information.

If the request was made with "XMLHttpRequest" as fallows:
var xhttp = new XMLHttpRequest();
xhttp.onreadystatechange = function () {
console.log(this.status); // with no CORS headers allways 0
if(this.status === 500){
// make some noise
}
};
xhttp.open("GET", 'http://www.example.ee/api', true);
xhttp.send();

If the response is indeed 500 but without CORS headers, then the "XMLHttpRequest" status will be 0 not 500.

So what's your ideas on this? Or am I missing something?

[ekirkco]

I will just comment that this did seem odd to me. I worked through this issue today. I ended up returning a header along with a 400 status(Bad Request). Not necessarily accurate because my use case was a timed out session that I wanted to make the user aware of. I will warn against using 408(Request Timeout) as the browser will re-send the request(at least if it's a jquery AJAX request) and you will likely get stuck in a loop. Here is a code sample(MVC Controller):

Response.Headers.Add("custom-error", "Session Expired");
return new HttpStatusCodeResult(System.Net.HttpStatusCode.BadRequest, "Your session has timed out");

client javascript/jquery sample:
$(document).ajaxError(function (event, jqxhr, settings, exception) {
    console.log(exception);
    if (jqxhr.status === 400 && jqxhr.getResponseHeader("custom-error") == "Session Expired") {
        window.location = '/Area/Controller/SessionExpired';
    }
});

[Eilon]

cc @rynowak @pranavkm - this will be more interesting as part of our work to make APIs great. I could easily see that "problem" responses and other specifically-API-friendly responses should work well w/ CORS.

[rootwalla]

Relevant from the other end, on 403 responses:
Firefox
Chrome

[daniellittledev]

I would like to reopen this issue. The workaround using the MVC filter does not solve the issue in all cases.

The root cause of the issue remains, which is All matching responses should have CORS headers added. Any other middleware (not just the UseExceptionHandler middleware) can run into this issue.

Looking at the source code shows that the CORS headers are added to the response BEFORE the request is processed. This means any subsequent middleware may inadvertently remove these headers.

Instead, we should take advantage of the async Delegating Handlers and add the headers to the response on the way back up the chain (after the response is provided). Note this has been the recommended approach since ASP.NET WebApi as per the documentation on Message Handlers.

The CorsMiddleware could potentially be fixed by moving where the headers are added.

public async Task Invoke(HttpContext context)
{
    await _next(context);
    // Add the headers after calling next
}

[szalapski]

Related issue: dotnet/aspnetcore#2378

[bbsimonbb]

What on earth have errors to do with CORS? If I turn CORS on, I expect it to be on for everything. This behaviour is perverse and patronising, and would send me running in panic from this framework. As a consumer of a service, how can I integrate if I don't have access to error information. Bangs head against wall. Does any other framework do this?

[ry8806]

All the issues relating to this bug are being closed. Plus they all reference each other too - it's just a circle of issues!

Does anyone know if this bug going to be addressed? Or do we have to implement said workarounds?
Thanks

[felixhayashi]

I agree with @bbsimonbb, this is somewhat patronising! It is a common pattern to have some kind of high-level middleware that catches Exceptions that bubble up and wrap them in an Error that is then send down to the client. CORS headers should be added to all responses if it has been defined in startup's Configure to do so.

sending full exception details across origins is probably not a good idea.

@kichalla This argument is pretty weak IMO, as sensitive information can always leak, not just in case of errors. Plus, if a developer writes code that sends down unsanitized exception messages/traces, the whole application is probably poorly designed anyway.

[daniellittledev]

I'm thinking of starting a PR to see if I can fix this, is that ok?

[JulijaRamoskiene]

This issue would not exist, if CORS middleware was adding headers with OnStarting event, instead of putting them directly to headers dictionary. Can CORS middleware be updated to add headers with OnStarting event?

[daniellittledev]

This issue was moved to dotnet/aspnetcore#2378