Reconsider Validation HTTP Status Code

[khalidabuhakmeh]

Currently the validation pipeline in ASP.NET Core returns a HTTP Status code of 400 which means BadRequest.

httpstatuses.com describes a status code of 400 as the following:

400 BAD REQUEST

The server cannot or will not process the request due to something that is perceived to be a client error (e.g., malformed request syntax, invalid request message framing, or deceptive request routing).

I believe the validation pipeline should return a 422 which is Unprocessable Entity.

422 UNPROCESSABLE ENTITY

The server understands the content type of the request entity (hence a 415 Unsupported Media Type status code is inappropriate), and the syntax of the request entity is correct (thus a 400 Bad Request status code is inappropriate) but was unable to process the contained instructions.

It should be assumed that if the ASP.NET Core pipeline has a chance to get through model binding and run validators, it means the syntax of the request was valid (especially around an API call).

This change would keep the HTTP status code in the family of 4xx error codes.

[rynowak]

This comes up from time to time, different people prefer different things. We're interested in ways to make this super easy for you to configure.

[khellang]

I'm assuming you're referring to the automatic conversion of validation errors into an IActionResult that was introduced with the API conventions stuff? I.e. the ModelStateInvalidFilter?

The InvalidModelStateResponseFactory API was added specifically for this reason, because I moaned about it over a year ago:

https://github.com/aspnet/AspNetCore/blob/afb92018f07bf81c7be092ce93ae36cf1aca5365/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs#L21-L29

It's easy enough to customize it, but it could always be improved. Writing your own quick extension method would be trivial.

services.Configure<ApiBehaviorOptions>(options =>
{
    options.InvalidModelStateResponseFactory = context =>
    {
        // Do whatever you want in here, i.e. return an IActionResult or throw an exception.
    };
});

The default implementation looks like this:

https://github.com/aspnet/AspNetCore/blob/708dc5cb5abd1fe0aa409f355b9e0cea8f6846d4/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/ApiBehaviorOptionsSetup.cs#L98-L103

You could swap BadRequestObjectResult with UnprocessableEntityObjectResult and be done 😄

Otherwise, if you're doing it manually, you can return whatever status code you want. I even added UnprocessableEntityResult, UnprocessableEntityObjectResult and ControllerBase.UnprocessableEntity for convenience.

[RehanSaeed]

@khellang is more of an expert on this (he wrote the code) and can confirm this but I think in 2.2 the default API behaviour is to use the new ProblemDetails response object which has a different InvalidModelStateResponseFactory method (link to full code shown below):

internal static IActionResult ProblemDetailsInvalidModelStateResponse(ActionContext context)
{
    var problemDetails = new ValidationProblemDetails(context.ModelState)
    {
        Status = StatusCodes.Status400BadRequest,
    };

    ProblemDetailsClientErrorFactory.SetTraceId(context, problemDetails);

    var result = new BadRequestObjectResult(problemDetails);

    result.ContentTypes.Add("application/problem+json");
    result.ContentTypes.Add("application/problem+xml");

    return result;
}

So, as well as swapping BadRequestObjectResult with UnprocessableEntityObjectResult, you'd also change the status code in the ValidationProblemDetails object.

[RehanSaeed]

With the above said, I still think there is a case for changing the default to 422 because it is more 'correct'. Using the compatibility version method would be the way to go to do it:

services.SetCompatibilityVersion(CompatibilityVersion.Version_3_0)

[khellang]

To be honest, I think the explicit status code here should be removed:

https://github.com/aspnet/AspNetCore/blob/afb92018f07bf81c7be092ce93ae36cf1aca5365/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/ApiBehaviorOptionsSetup.cs#L108-L111

As MVC now sets the ProblemDetails status code if none has been specified, based on the status code of the result itself:

https://github.com/aspnet/AspNetCore/blob/afb92018f07bf81c7be092ce93ae36cf1aca5365/src/Mvc/src/Microsoft.AspNetCore.Mvc.Core/ObjectResult.cs#L61-L64

[mkArtakMSFT]

@khellang, would you like to send us a PR for this?

[khellang]

I'll see if I can squeeze it in. Is there any way this can be assigned to me?