Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking change: New JsonSerializerDefaults.Web option leveraged by ASP.NET allows quoted numbers when deserializing JSON #21067

Closed
3 of 22 tasks
layomia opened this issue Oct 13, 2020 · 11 comments · Fixed by #21135
Assignees
Labels
breaking-change Indicates a .NET Core breaking change 🏁 Release: .NET 5 Work items for the .NET 5 release doc-idea Indicates issues that are suggestions for new topics [org][type][category]

Comments

@layomia
Copy link
Contributor

layomia commented Oct 13, 2020

Breaking change: New JsonSerializerDefaults.Web option leveraged by ASP.NET allows quoted numbers when deserializing JSON

In 5.0, we introduced a JsonSerializerDefaults enum along with a constructor on JsonSerializerOptions that takes this enum (dotnet/runtime#34626). This allows the creation of a JsonSerializerOptions instance with predetermined options, depending on the scenario. JsonSerializerDefaults.Web specifies the following options:

PropertyNamingPolicy = JsonNamingPolicy.CamelCase
PropertyNameCaseInsensitive = true
NumberHandling = JsonNumberHandling.AllowReadingFromString

The number handling feature is new in .NET 5 (dotnet/runtime#30255) and allows custom number handling, including allowing the serializer to read quoted numbers (i.e JSON strings), rather than throw JsonException. With JsonSerializerDefaults.Web, the serializer is permitted to read strings as numbers.

In 3.x, the default JsonSerializerOptions used by ASP.NET Core specified the camel case naming policy, and allowed case-insensitive property naming matching, but per the serializer's capabilities in 3.x, reading JSON strings as numbers was not allowed. In 5.0, ASP.NET has adopted the new JsonSerializerDefaults.Web as the default (de)serialization options. This means that all ASP.NET applications by default will now allow reading JSON strings as numbers.

Note that the new JSON extension methods on HttpClient and HttpContent exposed by System.Net.Http.Json (dotnet/runtime#32937) also use the new JsonSerializerDefaults.Web options.

Version introduced

5.0

Old behavior

Quoted numbers in JSON payloads which were to map with number properties in object graphs on deserialization would cause JsonException to be thrown.

New behavior

Quoted numbers in JSON payloads which are to map with number properties in object graphs on deserialization are valid.

This is not the new behavior for the default stand-alone JsonSerializer or JsonSerializerOptions, but rather it is the default behavior specifically within ASP.NET Core apps, the System.Net.Http.Json package, or when the user opts-in and chooses the JsonSerializerDefaults.Web option.

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. However, since this will affect many ASP.NET Core apps, it is a significant behavioral change which should be advertised.

Reason for change

User feedback and requests for opt-in, more permissive number handling in JsonSerializer (dotnet/runtime#30255) indicated that many JSON producers (e.g. services across the web) emit quoted numbers. Allowing reading quoted numbers helps .NET applications successfully parse these payloads by default in web contexts. This API is exposed via JsonSerializerDefaults.Web so that there is a handy way to specify the same options across different application layers e.g. client, server, shared. More detail here - dotnet/runtime#42240 (comment).

Recommended action

If this change is disruptive, e.g. the strict/default number handling in JsonSerializer is depended upon for validation, the previous behavior can be reenabled by setting the JsonSerializeOptions.NumberHandling option used by the application to JsonNumberHandling.Strict.

For ASP.NET Core MVC and web API applications, this can be configured in Startup using the following snippet:

services.AddControllers()
   .AddJsonOptions(options.NumberHandling = JsonNumberHandling.Strict);

Category

  • ASP.NET Core
  • C#
  • Code analysis
  • Core .NET libraries
  • Cryptography
  • Data
  • Debugger
  • Deployment
  • Globalization
  • Interop
  • JIT
  • LINQ
  • Managed Extensibility Framework (MEF)
  • MSBuild
  • Networking
  • Printing
  • Security
  • Serialization
  • Visual Basic
  • Windows Forms
  • Windows Presentation Foundation (WPF)
  • XML, XSLT

Affected APIs

JsonSerializer.Deserialize
JsonSerializer.DeserializeAsync


Issue metadata

  • Issue type: breaking-change
@layomia layomia added doc-idea Indicates issues that are suggestions for new topics [org][type][category] breaking-change Indicates a .NET Core breaking change labels Oct 13, 2020
@dotnet-bot dotnet-bot added the ⌚ Not Triaged Not triaged label Oct 13, 2020
@layomia
Copy link
Contributor Author

layomia commented Oct 13, 2020

FYI - @tdykstra, @pranavkm, @terrajobst

@pranavkm
Copy link
Contributor

For the recommended action, could we exhibit what this would look like for MVC:

For ASP.NET Core MVC and web API applications, this can be configured in Startup using the following snippet:

services.AddControllers()
   .AddJsonOptions(options.NumberHandling = JsonNumberHandling.Strict);

@tdykstra would this get listed under this: https://docs.microsoft.com/en-us/dotnet/core/compatibility/3.1-5.0? It's the one ASP.NET Core's migration notes links.

@tdykstra
Copy link
Contributor

Yes, that's the right place for it. And the quoted numbers section of the Newtonsoft migration doc would mention this and link to the 3.1-5.0 doc as well.

@ahsonkhan
Copy link
Member

ahsonkhan commented Oct 14, 2020

Quoted numbers in JSON payloads which are to map with number properties in object graphs on deserialization are valid.

@layomia, please add a clarification that this is not the new behavior for the default stand-alone JsonSerializer API behavior (reading it at a glance makes it seem we are changing that), but rather it is the default behavior specifically within aspnet apps or when the user opts-in and chooses the JsonSerializerDefaults.Web option.

@gewarren
Copy link
Contributor

@tdykstra Do you have any interest in documenting the serialization breaking changes?

@tdykstra
Copy link
Contributor

I'm interested in reviewing them to make sure I'm aware of changes that affect the STJ docs. If we get a bunch of them and you'd like some assistance I can help out. I'd need a crash course in how to document breaking changes. 😄

@layomia
Copy link
Contributor Author

layomia commented Oct 15, 2020

@gewarren @tdykstra FWIW I believe this is the last System.Text.Json breaking change to be documented.

@seanbecker15
Copy link

For anyone wondering how to (quickly) patch the problem this issue creates when transitioning from .net core 2.0+ to .net core 3.0+...

You can use the old newtonsoft json serializer by adding services.AddControllers().AddNewtonsoftJson() from the Microsoft.AspNetCore.Mvc.NewtonsoftJson package.

@ahsonkhan
Copy link
Member

ahsonkhan commented Nov 12, 2020

What problem exactly would that workaround be for, @seanbecker15, that would require adding back newtonsoft?

  • If you want quoted numbers allowed while deserializing JSON, it will work be default with STJ within an ASP.NET app.
  • If you don't want quoted numbers allowed, the workaround is listed in the original post of this issue above, for STJ:
services.AddControllers()
   .AddJsonOptions(options.NumberHandling = JsonNumberHandling.Strict);

@seanbecker15
Copy link

seanbecker15 commented Nov 13, 2020

@ahsonkhan My apologies for the confusion — I was referring to a few problems caused by migrating from .net core 2.0+ to 3.0+ that this issue happens to address.

For context, here are the problems I ran into when upgrading:

  • Unable to deserialize quoted numbers from JSON
  • Properties with annotation [JsonProperty(“item”)] no longer use “item”, but rather the name of the property

E: If you’d like me to reword that let me know and I’d be happy to do so.

@ahsonkhan
Copy link
Member

ahsonkhan commented Nov 13, 2020

Unable to deserialize quoted numbers from JSON

That shouldn't happen. Please file an issue in https://github.com/dotnet/runtime with your aspnet details, .net version and provide a minimal repro of your application, C# object model, and example JSON you are deserializing, and someone there will take a look and help you.

Properties with annotation [JsonProperty(“item”)] no longer use “item”, but rather the name of the property

That is expected since JsonProperty is a Newtonsoft.Json specific attribute. You need to use the attribute that System.Text.Json will understand. Rename it to JsonPropertyName and update the namespace/using directives:
https://docs.microsoft.com/en-us/dotnet/api/system.text.json.serialization.jsonpropertynameattribute?view=net-5.0

This migration guide should be useful:
https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to?pivots=dotnet-5-0

For anything else, please file new issues in https://github.com/dotnet/runtime so the conversation is in the right place :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Indicates a .NET Core breaking change 🏁 Release: .NET 5 Work items for the .NET 5 release doc-idea Indicates issues that are suggestions for new topics [org][type][category]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants