[release/5.0-rc2] Add JsonNumberHandling.AllowReadingFromString as a JsonSerializer web default (#41539)

[layomia]

Backport of #41539 to release/5.0-rc2

Customer Impact

Follow up from #39363 / #30255.

During API review for the number handling feature, it was discussed that number types being written as strings is a common pattern in many JSON producers (e.g API endpoints) across the web. Adding JsonNumberHandling.AllowReadingFromString as a JsonSerializer web default makes it easier for .NET web applications to consume these numbers in a consistent manner (same options for client/shared/server).

Testing

• The number handling feature was implemented and tested independently of this change (Add JsonNumberHandling(Attribute) & support for (de)serializing numbers from/to string #39363). It is already checked in for .NET 5.
• The web defaults for the newly added JSON extension methods on HttpClient and JsonContent were updated to use the JsonSerializerDefaults.Web setting. Appropriate tests were added to make sure the options are honored accordingly.

Risk

The JsonSerializerDefaults and number handling features are new in .NET 5.0. .NET web products such as ASP.NET MVC, HttpRequestExtensions, and the extension methods on HttpClient/JsonContent have been updated to use JsonSerializerDefaults by default. The change in this PR affects these areas and makes reading numbers from JSON strings the default behavior.

[danmoseley]

Thanks for the detailed write up. This change seems like a feature. It will be hard to get the okay for it without strong customer motivation. Can you share more about the customers? How many or how large? How impactful if it to them if we don’t take this change eg they can’t migrate?

[danmoseley]

Hit close by mistake

[ahsonkhan]

This is changing the default settings of JsonContent and JsonSerializerDefaults.Web. If not done in 5.0, it will become a behavioral breaking change in 6.0 that will be difficult for app authors to workaround the break.

[danmoseley]

Thanks @ahsonkhan . That's helpful but still we would need strong customer data to support the change.

[layomia]

@danmosemsft

The number handling feature is the most requested feature in System.Text.Json (both closed and open) with 91 likes. The next highest, the reference handling feature, has 77 likes. The major motivation of the feature was to support interchanging data with various web services including JavaScript APIs, Blazor, Python etc. The primary capability desired was being able to read numbers from JSON strings produced by these entities (JsonNumberHandling.AllowReadingFromString).

The setting of JsonNumberHandling.AllowReadingFromString when JsonSerializerDefaults.Web is specified allows this highly desired behavior to work without additional configuration in user projects. Also, since ASP.NET MVC, HttpClientExtensions/Blazor are using JsonSerializerDefaults.Web, users won't have to worry about specifying this behavior at multiple layers of their application stack (client, shared, server) and have potential bugs with leaving out options at one layer. This a pain-point experienced by some users, including @terrajobst on our team.

FWIW during API review for the number handling feature, there was even discussion about making this a general default on JsonSerializerOptions. It seemed a bit too general for low level serializer usages, so we settled on making it a web default where it is certainly more applicable.

If not done in 5.0, it will become a behavioral breaking change in 6.0

Just to clarify that this behavior is making a scenario more permissive, specifically going from throwing JsonException to successfully coercing a number from a JSON string. This is technically not a breaking change. While some people might rely on the strict behavior of the serializer by default, I imagine this subset to be small in web interchange contexts.

BTW, the breaking change angle here would be from the perspective of changing the behavior of a named default (JsonSerializerDefaults.Web), not from ASP.NET MVC etc defaults changing. The settings change from the perspective of ASP.NET and System.Http.Net.Json, while significant, was already agreed on in API review and further discussions with @terrajobst and @pranavkm, and is anticipated on their side.

will be difficult for app authors to workaround the break.

App authors can revert to strict settings by setting JsonNumberHandling.Strict on JsonSerializerOptions. If the setting was made in a library that the app depends on, hopefully the library author is looking out for updates in System.Text.Json or can be contacted to update the setting. Again I think this subset is small.

That said, I believe it is still worth it to make the change now to avoid a potential inconvenience to some users in 6.0. Specifically, this will be users that directly use JsonSerializerDefaults.Web and rely on strict number handling.

[danmoseley]

Removing label while discussion continues. We can take Thurs if needed

[steveharter]

LGTM; direct port except for minor doc change that will be added to master.

[theolivenbaum]

My 2 cents: was also surprised by this when updating to use System.Text.Json. Number serialization in json has a few limitations, specially for anything outside the float range/precision, thus quite often one has to fall back to use strings instead