Microsoft.AspNetCore.Mvc.NewtonsoftJson Converts a number like 1234 to true boolean; also accepts "true" as string

[mihaibozesan123]

Hi, I've been using this package(in the title) with Newtonsoft.Json. It has got some nice bugs.

1. you can pass a numeric value like 1234 for boolean without the deserialization failing; instead of failing, the number will be converted to the boolean value True.
2. you can pass "true" or "false" string for boolean and again, it won't fail, while it should(suspect it's the same issue).

Example: Model:

public class WeatherForecast
    {
        public int TemperatureC { get; set; }

        public int TemperatureF => 32 + (int)(TemperatureC / 0.5556);

        public string Summary { get; set; }
        public bool Flag { get; set; }
    }

POST Body JSON:

    {
        "temperatureC": 15,
        "summary": "current temperature",
        "flag": 123
    }

If I use only Newtonsoft.Json, the above scenario is not possible. However if I also use Microsoft.AspNetCore.Mvc.NewtonsoftJson, it makes it possible to pass a numeric value for a bool.

Regards, Mihai

[javiercn]

@mikebowman123 thanks for contacting us.

I don't think we change any behavior from what NewtonSoft.Json provides by default. Does serializing and deserializing the same payloads with Newtonsoft.Json directly offer a different result?

If that's not the case, then this is not a bug and is the by-design behavior, as we don't change the way Newtonsoft.Json serializes or deserializes types.

It's likely that Netwonsoft.Json might be doing some C-style type coercion under the covers and you are seeing just that.

I'm closing this issue as we don't think there's a problem here. If you happen to determine that the behavior deserializing JSON directly is different, feel free to comment here and we will re-evaluate this issue.

Thanks.

[mihaibozesan123]

This has to be reopened. So for more clarity. I now have installed both Newtonsoft.Json and Microsoft.AspNetCore.Mvc.NewtonsoftJson. Without the latter if I try to pass an int for boolean, an exception is thrown. I install the latter, then passing a numeric value for boolean will result into converting that value to True boolean value(just like in my example earlier).

@javiercn To answer your question "Does serializing and deserializing the same payloads with Newtonsoft.Json directly offer a different result?" Yes the result is different. If I use just Newtonsoft.Json without Microsoft.AspNetCore.Mvc.NewtonsoftJson, then a numeric value won't be serialized into a boolean, resulting instead into an error.

Problem is that I need Microsoft.AspNetCore.Mvc.NewtonsoftJson because I use PATCH for update, and without this package, the JSON for PATCH is not considered valid. And also because of the constraint on my project, we can't ignore this issue and accept what's happening as ok. So we would need a fix to this ASAP.

Regards, Mihai

[javiercn]

@mikebowman123 thanks for the additional information.

@pranavkm any idea of what's going on here?

[javiercn]

@mikebowman123 Can you reproduce this with the out of the box template? (just adding the package).

In other words, are you customizing JSON.NET in any way?

A minimal repro project would help us to investigate.

[mihaibozesan123]

@javiercn No customization on POST and PATCH.

[mihaibozesan123]

Here is a repo containing the code sample needed to reproduce the issue. You can try a POST request, with the following body:

{
    "temperatureC": 15,
    "summary": "current temperature",
    "flag": 123
}

Flag should accept only true/false since it is boolean, but because of the issue that is being reported, numeric value 123, will be converted to boolean value True.

[ryanbrandenburg]

I don't think this is a bug so much as it is a difference in behavior between Newtonsoft.Json and System.Text.Json (what gets used if you don't do AddNewtonsoftJson()). Basically Newtonsoft is a bit more permissive and will try to do some cleaning up for you. If your int is 0 that's false, otherwise it's true. String "true" becomes true, "false" becomes false, anything else throws an exception. You can actually replicate this behavior with a bare Newtonsoft.Json.JsonConvert

var str = "{\temperatureC\": 45,\"temperatureF\": 112, \"summary\": \"Chilly\",\"flag\": \"true\"}"
var value = Newtonsoft.Json.JsonConvert.DeserializeObject<WeatherForecast>(str);
Console.WriteLine(value.Flag);

My advice would be to either:

1. Pick one of System.Text.Json|Newtonsoft.Json and don't switch between them.
2. Adhere strictly to RFC7159 when generating your JSON, so that there's no room for differences on edge-cases to interfere. You can get a bit of context on how the Newtonsoft folks think about this kind of thing from this thread.

Actually ideally you would do both of those, but you know your scenario best.

[mihaibozesan123]

@ryanbrandenburg Actually Newtonsoft.Json is less permissive, while Microsoft.AspNetCore.Mvc.NewtonsoftJson is actually more permissive. I don't understand how you don't consider this a bug. I mean passing a type for another and not getting an error and actually converting to the receiver's type default value instead is so wrong. I would think this "permissivivty" would subject an API to security concerns.

I assume you have run the example, so you know that my int is 1234 not 1 or 0, which would be acceptable. So converting a non-binary numeric value to a boolean is totally wrong. Also, strings of true and false, shouldn't be allowed either.

If I remove AddNewtonsoftJson(), then the PATCH is not working because Microsoft.AspNetCore.Mvc.NewtonsoftJson is not going to be used. As about, picking System.Text.Json|Newtonsoft.Json, I am using Newtonsoft.Json. But Microsoft.AspNetCore.Mvc.NewtonsoftJson is overrinding the validation.

So you see, Microsoft.AspNetCore.Mvc.NewtonsoftJson is causing this issues, but I can't do without it because I need it for PATCH.

The workaround isn't an option. We rely on the default validation. We are in the middle of migrating a lot of APIs to .Net Core. That means we are going to have to use Microsoft.AspNetCore.Mvc.NewtonsoftJson to perform PATCH(we have patches on every API, that's how we do updates). It's going to take a whole lot of effort to add manual validation to our models for every API. And we don't want to do workarounds for validation. We should not have to do this. Period.

I don't think and neither do any of my colleagues with whom I discussed the situation, that this is an ok behavior.

Edit: Why, in the first place, does this package override the validation from Newtonsoft.Json? It should extend the latter, rather than alter it like this. And why did you not provide a way to configure the package to use the Newtonsoft.Json validation if that is what is needed? Like .AddNewtonsoftJson(options => options.UseStrictJsonValidation = true);

[javiercn]

@ryanbrandenburg Can you check if this is a change in existing behavior?

[pranavkm]

Flag should accept only true/false since it is boolean, but because of the issue that is being reported, numeric value 123, will be converted to boolean value True.

@JamesNK do you happen to know if this is JSON.NET's default behavior? MVC"s default JSON serializer settings haven't changed significantly since forever. We did update to a newer version of JSON.NET in 3.0, but I suspect this behavior isn't new in 12.0

@mihaibozesan123 you could consider using JSON.NET specifically for JSON patch responses while using System.Text.Json if that suits your requirements better. Our docs have some guidance on doing this: https://docs.microsoft.com/en-us/aspnet/core/web-api/jsonpatch?view=aspnetcore-3.0#jsonpatch-addnewtonsoftjson-and-systemtextjson

[mihaibozesan123]

@pranavkm Tried the solution at the provided link. Not working. The formatters and validators from Microsoft.AspNetCore.Mvc.NewtonsoftJson are still applied.

[ryanbrandenburg]

@pranavkm, my snippet from https://github.com/aspnet/AspNetCore/issues/16884#issuecomment-551314664 demonstrates that converting an int to a bool is in fact the default behavior of at least Newtonsoft.Json.JsonConvert.DeserializeObject, and I assume that all the Newtonsoft.Json converters have the same config under the covers by default.

@mihaibozesan123 I think there's some confusion about what happens when you don't use Microsoft.AspNetCore.Mvc.NewtonsoftJson. Let me know if I've misinterpreted you, but your comments seem to indicate that you believe if you remove the reference to Microsoft.AspNetCore.Mvc.NewtonsoftJson from your repro project it will instead use "base" Newtonsoft.Json. Newtonsoft.Json (aka JSON.Net) was the default in previous versions, but is not the case in 3.0. The "default" JSON Serializer/Deserializer was changed to System.Text.Json (the announcments for and discussions of this change can be found here and here). If you remove <PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="3.0.0" /> from the csproj in your repro project it will no longer contain Newtonsoft.Json at all. You can verify this yourself by looking at obj\project.assets.json, the file which gives a record of which versions of project dependencies were resolved. When you leave your Microsoft.AspNetCore.Mvc.NewtonsoftJson reference in you can Ctrl-F Newtonsoft.Json in obj\project.assets.json and see that you get some version like 12.0.6, but when you remove that reference and rebuild you'll see that it is gone entirely.

So the behavior you're seeing is a difference between Newtonsoft.Json (which is wrapped by Microsoft.AspNetCore.Mvc.NewtonsoftJson) and System.Text.Json. We're currently unable to provide an option like .AddNewtonsoftJson(options => options.UseStrictJsonValidation = true); because no such option exists within Newtonsoft.Json. On re-reading this issue it does seem that @JamesNK has reconsidered including a Strict option to Newtonsoft.Json. If that change was made on their end then it would be possible for us to pass through such an option to Newtonsoft.Json, enabling the scenario you want.

[mihaibozesan123]

@ryanbrandenburg I understand the situation. We'll have to find way to deal with this validation in without relying on an fix/feature on this package. WHat I actually meant by removing Microsoft.AspNetCore.Mvc.NewtonsoftJson, was removing this package, but adding stand-alone NewtonsoftJson instead, so then I would still have NewtonsoftJson on my project. If I do this, the validations will work as expected. I only need Microsoft.AspNetCore.Mvc.NewtonsoftJson for patch.. Bottom Line, you said that if I remove Microsoft.AspNetCore.Mvc.NewtonsoftJson I would no longer have NewtonsoftJson on my project, but my alternative is to istall the stand-alone version. Anyway, that does not suffice.