JsonInputFormatter - Json Serialization Errors (ASP.NET Core 2.2)

[ColinM9991]

The following StackOverflow question describes some of the detail.

I am trying to control the error messages which are returned when the JsonSerializer fails to deserialize the request body within the JsonInputFormatter, the current implementation of the input formatter forces me to override the ReadRequestBodyAsync method of JsonInputFormatter to access the context and add the custom error message to the ModelStateDictionary.

AllowInputFormatterExceptionMessages does not help because the application returns empty messages when set to false.

The reason for wanting to override these error messages is because they're not extremely helpful when exposing a public API, in my opinion, and having the ability to customize the error message to provide something more readable would be a better solution.

Describe the solution you'd like

1. Add a property to MvcJsonOptions which allows for people to apply a templated string. {0} and {1} could be replaced with path and member from the ReadRequestBodyAsync method.

Example code snippet:

public class MvcJsonOptions
{
    ...
    public string SerializationErrorMessageTemplate { get; set; }
}

line 283 of JsonInputFormatter

if (!string.IsNullOrWhitespace(_jsonOptions.SerializationErrorMessageTemplate))
{
    var modelStateErrorMessage = string.Format(_jsonOptions.SerializationErrorMessageTemplate,
                                               eventArgs.ErrorContext.Path,
                                                   eventArgs.ErrorContext.Member);
    context.ModelState.TryAddModelError(key, modelStateErrorMessage);
} else {
    var metadata = GetPathMetadata(context.Metadata, eventArgs.ErrorContext.Path);
    var modelStateException = WrapExceptionForModelState(eventArgs.ErrorContext.Error);
    context.ModelState.TryAddModelError(key, modelStateException, metadata);
}

2. Allow WrapExceptionForModelState to be overridden in a more derived type to reduce the.

[mkArtakMSFT]

We've moved this issue to the Backlog milestone. This means that it is not going to happen for the coming release. We will reassess the backlog following the current release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources.

[mkArtakMSFT]

As a side note, it looks like the version of the framework you're using is outdated and is not supported any more. Consider upgrading to a newer supported version of the framework to make sure it's secure and you get all the new features. You can learn more about the .NET Core support policy at https://dotnet.microsoft.com/platform/support/policy/dotnet-core

[ColinM9991]

As a side note, it looks like the version of the framework you're using is outdated and is not supported any more. Consider upgrading to a newer supported version of the framework to make sure it's secure and you get all the new features. You can learn more about the .NET Core support policy at https://dotnet.microsoft.com/platform/support/policy/dotnet-core

Upgrading is an ambition of mine, but this is an enterprise application which uses internal Nuget packages that have a tight coupling to ASP. NET Core 2.x and Newtonsoft.Json. Such an upgrade won't happen any time soon unfortunately due to our delivery roadmap.

[ColinM9991]

This is the ASP.NET Core 3 version my proposed change would sit,

https://github.com/dotnet/aspnetcore/blob/a49e084c1515dfaa16de8d7d4b0c00190587ad87/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonInputFormatter.cs#L268

If such a change is one that you'd consider then I'm happy to make this change and raise a PR.

[pranavkm]

Such an upgrade won't happen any time soon unfortunately due to our delivery roadmap.

@ColinM9991 in that case, is this still interesting to you? The earliest version you could possibly see this API would be 5.0.

[ColinM9991]

Such an upgrade won't happen any time soon unfortunately due to our delivery roadmap.

@ColinM9991 in that case, is this still interesting to you? The earliest version you could possibly see this API would be 5.0.

It would be good to have support for this as there is a genuine use case and there have been other attempts (visible on Stackoverflow) at investigating, identifying and implementing support for this. A lot of those questions contain answers which suggest assigning a handler to JsonSerializerSettings.Error, but there's no way to handle the error and hook into the context.ModelState.

At work, we're focusing on designing APIs which implement a RESTful architecture, the inclusion of ProblemJSON in ASP. NET Core helped quite significantly with this, I believe support for this in NewtonsoftJsonInputFormatter and an equivalent in SystemTextJsonInputFormatter would allow people to return what clients may consider a meaningful error message based on the domain which forms the request model, and thus a more self-documenting API.

We have also had pen testers flag the default error messages as a potential risk as it may release information about the technology stack used.

The alternative would be to set AllowInputFormatterExceptionMessages to false and replace the empty model state error message with a generic message, but this could be a naive implementation as an empty model state message isn't necessarily due to a serialization error.

In the meantime, I will again attempt a migration to ASP. NET Core 3 as it has matured since I last looked.

[GREsau]

I would also love to see some improvements in this area. Consider the following app:

using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddControllers(o => {
    o.ModelMetadataDetailsProviders.Add(new SystemTextJsonValidationMetadataProvider());
});
var app = builder.Build();
app.MapControllers();
app.Run();

namespace DotNetTest.JsonError.Example
{
    public class MyPoco
    {
        public required string Foo { get; set; }
    }

    [ApiController]
    [Route("/")]
    public class MyController : ControllerBase
    {
        [HttpPost]
        public void Post(MyPoco poco)
        {
        }
    }
}

When running in .NET 8, and POSTing an empty JSON object, we get this 400 response:

{
    "type": "https://tools.ietf.org/html/rfc9110#section-15.5.1",
    "title": "One or more validation errors occurred.",
    "status": 400,
    "errors": {
        "$": [
            "JSON deserialization for type 'DotNetTest.JsonError.Example.MyPoco' was missing required properties, including the following: foo"
        ],
        "poco": [
            "The poco field is required."
        ]
    },
    "traceId": "00-89db3a32fa6c4571ebe9d500b5028641-00156bbe6de3d749-00"
}

This has two similar problems:

1. The first error message mentions the full type name of the class that failed to deserialize, (DotNetTest.JsonError.Example.MyPoco)
2. The second error message mentions the action method's parameter that failed to deserialize (poco)

Both of these are implementation details, and are meaningless to the consumer of the web API. This arguably has security implications, as it's leaking internal implementation details, although personally I don't think it's practically significant. My concerns are more that this response includes noisy and unhelpful details. Especially the second error message, which could mislead consumers into thinking that they must specify a "poco" JSON property in the request content.

The first problem can be "resolved" by disabling AllowInputFormatterExceptionMessages, but then the error message is The input was not valid., which has the opposite problem - it's too vague to really be useful to the client.

Ideally, it would instead return a 400 response with something more like:

{
    ...
    "errors": {
        "$": [
            "JSON deserialization failed due to missing required properties, including the following: foo"
        ]
    },
    ....
}

[GREsau]

Note that the current behaviour is a bit better when using [Required] instead of a required property, as then deserialization succeeds and model validation returns an error message, e.g.

public class MyPoco
{
    [Required]
    public string Foo { get; set; }
}

{
    "type": "https://tools.ietf.org/html/rfc9110#section-15.5.1",
    "title": "One or more validation errors occurred.",
    "status": 400,
    "errors": {
        "foo": [
            "The Foo field is required."
        ]
    },
    "traceId": "00-af2b62b7fe221d8d24583294187b76e0-d51a218e5d533556-00"
}

Of course then we don't get the code-level benefits of required properties ensuring that we always construct a MyPoco with a Foo property set. And we would still encounter the original problem when JSON deserialization fails for another reason, e.g. due to the client sending a JSON array in the body instead of a JSON object.