dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.47k stars 10.03k forks source link

Expires header set to '-1' automatically when writing to HttpResponse.Body in IExceptionHandler.TryHandleAsync() method. #58655

Open GillesTourreau opened 2 weeks ago

GillesTourreau commented 2 weeks ago

Is there an existing issue for this?

Describe the bug

I created an implementation of the IExceptionHandler to write a custom error when an unhandled exception is occured in an action method of a controller:

public class CustomExceptionHandler : IExceptionHandler
{
    public async ValueTask<bool> TryHandleAsync(HttpContext httpContext, Exception exception, CancellationToken cancellationToken)
    {
        await httpContext.Response.Body.WriteAsync(Encoding.UTF8.GetBytes("The error"));

        return true;
    }
}

After calling the httpContext.Response.Body.WriteAsync() the header Expires is automatically added with the -1 value.

Expected Behavior

After calling httpContext.Response.Body.WriteAsync() no Expires header should be added. Specially with the -1 value which is not a standard value for an "http-date" (See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Expires).

Steps To Reproduce

To reproduce the error:

[HttpGet(Name = "GetWeatherForecast")]
public string Get()
{
    throw new DivideByZeroException("The exception !");
}

Run the application and execute the action on a browser. When the action is called, the CustomExceptionHandler is called, the The error message is returned as response of the HTTP query, but the header Expires is automatically added with the -1 value.

I created a simple project to reproduce the issue and available here aspnetcore.ExceptionHandlerExpiresHeaderBug.

Image

Exceptions (if any)

No response

.NET Version

8.0.403

Anything else?

No response

BrennanConroy commented 2 weeks ago

After calling httpContext.Response.Body.WriteAsync() no Expires header should be added.

Why not? We don't want responses created by exceptions to be cached.

Specially with the -1 value which is not a standard value for an "http-date"

From the spec https://httpwg.org/specs/rfc9111.html#rfc.section.5.3:

A cache recipient MUST interpret invalid date formats, especially the value "0", as representing a time in the past (i.e., "already expired").

-1 is a perfectly valid invalid value.

GillesTourreau commented 2 weeks ago

From the spec https://httpwg.org/specs/rfc9111.html#rfc.section.5.3: -1 is a perfectly valid invalid value.

I see, I did not know read this part of the RFC.

Why not? We don't want responses created by exceptions to be cached.

But in this case, why we can't update the Expires headers in the TryHandleAsync() method? For example, if I define explicitly the Expires header, when calling the WriteAsync() method, the Expires header is automatically reset to -1. We should let the developer to customize the headers of the response to return to the caller.

public async ValueTask<bool> TryHandleAsync(HttpContext httpContext, Exception exception, CancellationToken cancellationToken)
{
    httpContext.Response.Headers.Expires = new StringValues("Thu, 30 Oct 2025 07:28:00 GMT");

    await httpContext.Response.Body.WriteAsync(Encoding.UTF8.GetBytes("The error"));

    return true;
}