Aspnetcore: System.Text.Json leaks trace when a validation error occurs

[am071092]

Error trace is shown when an error occurs (eg:line number).
Is there a way to disable that?

[mkArtakMSFT]

Thank you for filing this issue. In order for us to investigate this issue, please provide a minimalistic repro project (ideally a GitHub repo) that illustrates the problem.

[disklosr]

Not op but I know what he's talking about: The problem is with the new Text.Json reader that sets a technical error message inside ModelState when it encounters a binding error. The InvalidStateFilter then picks up the technical error message and returns it in a 400 response which leaks the internals of the app. Here's an example.

Given this Dto:

public class OrderDto {
    public int? itemId {get; set;}
}

And this controller

    [ApiController]
    public class TestController
    {
        [HttpPost]
        [Route("orders")]
        public object CreateOrder(OrderDto dto)
        {
            return dto;
        }
    }

When we issue this request:

POST /api/orders
content-type: application/json

{
    "id": "notanumber"
}

We get this response (even in production env) that leaks internal types and line numbers:

{
  "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1",
  "title": "One or more validation errors occurred.",
  "status": 400,
  "traceId": "|cde9a27d-4207f1e3e6d9bd05.",
  "errors": {
    "$.id": [
      "The JSON value could not be converted to System.Nullable`1[System.Int32]. Path: $.id | LineNumber: 3 | BytePositionInLine: 15."
    ]
  }
}

Json.Net has this option where you can tell it to hide technical details from the error message set in the model state. In aspnet core 3.1 it can be done like this:

services.AddControllers()
    .AddNewtonsoftJson(o => o.AllowInputFormatterExceptionMessages = false)

But the new Text.Json doesn't have an equivalent option.

Also, and contrary to Json.Net, the new Text.Json doesn't tolerate strings that can be mapped to int and will throw a binding error in this case. Reusing my previous example, this request:

```http
POST /api/orders
content-type: application/json

{
"id": "23"
}
```

would still fail with the same previous error message.

It took me hours to investigate the behavior of the new json serializer (it took me even more to understand and configure validation the way I want). Right now I'm not happy with it (nor the way validation is carried out) so I'm sticking with Json.Net.

[mkArtakMSFT]

Thanks for the details, @disklosr.
@pranavkm an you please look into this? Thanks!

[pranavkm]

We get this response (even in production env) that leaks internal types and line numbers:

The line numbers are from the JSON payload that the client sent, so there's nothing interesting there being leaked. The data types are exposed, but these are usually conversion errors for primitives, and it's not any different from the errors you get from Newtonsoft.Json with AllowInputFormatterExceptionMessages = true or model binding errors.

That said, we wouldn't be opposed to adding an equivalent to AllowInputFormatterExceptionMessages to SystemTextJsonInputFormatter if you'd like to send us a PR for it. Here's the relevant bits:

• https://github.com/dotnet/aspnetcore/blob/e00a7c4d52107e8090d06d1345920ef3e39e3233/src/Mvc/Mvc.NewtonsoftJson/src/NewtonsoftJsonInputFormatter.cs#L370-L392
• https://github.com/dotnet/aspnetcore/blob/e00a7c4d52107e8090d06d1345920ef3e39e3233/src/Mvc/Mvc.Core/src/Formatters/SystemTextJsonInputFormatter.cs#L81-L83

[disklosr]

The line numbers are from the JSON payload that the client sent, so there's nothing interesting there being leaked.

My bad the line numbers are definitely helpful in this case I sure got carried away by op's message.

The data types are exposed, but these are usually conversion errors for primitives

I can understand that it's not a big deal with system types, but when using a type converter the current behavior will leak internal types/namespaces and this is not always desirable imo. And even for primitive types I guess it's better not to disclose them as they can give a hint as to what language/framework/webserver an app is using. In all cases, we should at least have the option to hide these kind of details.

I'm interested in making a PR for this, I'll try to have something ready for tomorrow.

[Gaurav-Puri101187]

Hi @disklosr ,

Is above issue fixed,, as i can see exception still being null in items of ActionContext.ModelState.Values.
Though if i choose Newtonsoft and set AllowInputFormatterExceptionMessages to false i do get the exception property being set.Can you please suggest i have a validation filter in which i am trying to cater all the ModelBinding and
DataAnnotation bad request scenarios.. Please suggest.

[disklosr]

@Gaurav-Puri101187 unfortunately I was too optimistic about making a PR, but couldn't do it due to lack of time.
I'm not sure I understand your question. Can't you use Newtonsoft as a workaround?

[fagnercarvalho]

Thank you for the details @pranavkm. Can I work on fixing this issue? I would add a new AllowInputFormatterExceptionMessages option in JsonOptions and include logic to handle these messages in SystemTextJsonInputFormatter similar to the way it is done in NewtonsoftJsonInputFormatter like you pointed out.

[pranavkm]

Sure, we'd be happy to take a PR for it. Could we rename it something specific to S.T.J, for instance, AllowSystemTextJsonExceptionMessages?