-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
System.Text.Json: (De)serialization support for quoted numbers #30255
Comments
From Twitter: a lot of people correctly point out that numbers are intentionally quoted in APIs so they're not assumed to be floats and lose precision. It's an excellent point, and even more reason to support deserializing them I think. |
To me that says that the user should still deserialize them as strings and only turn them into numbers when they're ready to accept the loss of precision. |
That's the intention. The data types for the class properties are what we want the final result to be. If we have to use string-based properties first and then parse ourselves then there's not much utility for this serialization library in terms of mapping to types at all, especially for APIs. |
With support for a custom double and\or decimal converter you can do this yourself. Perhaps there will be future support for a set of "Json.NET-compat" converters at some point but that is not available now. Below is an skeleton of such a custom converter. Also see https://github.com/dotnet/corefx/issues/38713 which has a custom public class DoubleConverterWithStringSupport : JsonConverter<double>
{
public override double Read(ref Utf8JsonReader reader, Type type, JsonSerializerOptions options)
{
if (reader.TokenType == JsonTokenType.String)
{
return ...;
}
// Default behavior; will throw if TokenType != Number
return reader.GetDouble();
}
public override void Write(Utf8JsonWriter writer, double value, JsonSerializerOptions options)
{
// Write as number or string? Could also use [JsonConverter] to specify a different converter for certain properties.
}
} |
Moving to Future. cc @rynowak |
I'm trying to unwrap all that (thanks for the workaround!) - is that saying we'd never consider adding it to the base serializer and it'd only ever live as an extension point? I'm finding the lack of this to be a very common pain point, so I'm wondering what the bar is for inclusion if so. |
@NickCraver I think it's just "there's a whole lot of policy here, and we would need to completely spec it out, and we're trying to be feature complete 30 days ago... so this isn't required for v1 (.NET Core 3.0)"
It's a simple-sounding request, but there are a lot of details and many of them are hard to change once the feature first ships. So the .NET Core 3.0 answer is "deserialization doesn't coerce numbers out of JSON strings; but the converters feature lets you effectively opt-in on a property-by-property basis". |
Totally understand, I was mostly curious about where the bar is and if moving to this was a good from a future standpoint (or we shouldn't try to move certain code sections to it). That list helps a lot - I agree this can't be just tossed in. Thanks for the quick reply! |
The converter option mentioned by @steveharter works great now in the .NET Core 3.0 preview-7 release. Confirmed server-side and in Blazor. public class LongToStringConverter : JsonConverter<long>
{
public override long Read(ref Utf8JsonReader reader, Type type, JsonSerializerOptions options)
{
if (reader.TokenType == JsonTokenType.String)
{
ReadOnlySpan<byte> span = reader.HasValueSequence ? reader.ValueSequence.ToArray() : reader.ValueSpan;
if (Utf8Parser.TryParse(span, out long number, out int bytesConsumed) && span.Length == bytesConsumed)
return number;
if (Int64.TryParse(reader.GetString(), out number))
return number;
}
return reader.GetInt64();
}
public override void Write(Utf8JsonWriter writer, long value, JsonSerializerOptions options)
{
writer.WriteStringValue(value.ToString());
}
} services.AddControllersWithViews()
.AddJsonOptions(options =>
{
options.JsonSerializerOptions.Converters.Add(new LongToStringSupport());
}); This was developed in the other thread: https://github.com/dotnet/corefx/issues/36563#issuecomment-519788518 |
It's not every type... this case is about Javascript/JSON not supporting large numeric types so it needs a string conversion, and the reasoning by the team on why this isn't built-in makes sense. The converter code is right there so you copy/paste if you need it, but I guess they can add some default converters for What other problems are you having? You don't have to use System.Text.Json if you need more flexibility, Json.NET is still available and easily enabled. |
We have a similar situation: we decided to move our project to .net core 3.0. and use the built-in System.Text.Json. We had to rewrite a small part of the functionality, but in general, the build was successful. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
For 5.0 we can consider an option to support "looser" deserialization modes depending on how common these scenarios are, and whether we can agree on common semantics. However I believe all such modes can be supported today by using custom converters and by extending A 5.0 feature could be to add a Similarly, there has been feedback on what primitive types to deserialize when the property is a Adding additional option(s) for this is also possible, however there are subtleties: precision can be lost (e.g. |
#Comment comment #I12XVH 字典表 BootstrapDict 实体类 Define 为 int 类型 #Issue https://github.com/dotnet/corefx/issues/36639 https://github.com/dotnet/corefx/issues/39473 link #I12XVH
From @dansiegel in dotnet/corefx#42396
|
Maybe i'm mistaken, but we shouldn't have to have a custom converter. Int32 |
I would hate to think I would have to decorate all of my properties to handle this. Really the built in converter should be smart enough to try to parse the string to the given numeric type. |
So long as it's an option. I definitely don't want it coercing values in my payloads. If I said I want a number, thou shalt not send me a string. |
Just don’t forget that JSON (JavaScript Object Notation) is a lightweight data-interchange format. So, JavaScript doesn’t have types. So, If you are going to communicate with “JavaScript” services, Angular, React, TypeScript, Java (currently ~99%) you have to use Newtonsoft json. Unfortunately System.Text.Json doesn’t like “JavaScript” typeless word , cannot find proper type mapping, … and throws exceptions. Unfortunately, it was not developed the “JavaScript” version of System.Text.Json, therefor you can use it (System.Text.Json) in communication only (System.Text.Json) <-> (System.Text.Json) only. |
Please add this option |
From @IgorMenshikov in #725
|
(De)serialization support for quoted numbers should also work for nullable types. |
Per https://github.com/dotnet/corefx/issues/41442, this issue should consider the deserialization of From @pranavkm in https://github.com/dotnet/corefx/issues/41442:
|
Renaming this issue to explicitly mark that serialization of quoted numbers is being considered here as well, per @steveharter's comment above:
|
If added, (de)serialization support for numbers as dictionary keys (https://github.com/dotnet/corefx/issues/40120) will use semantics defined here. |
Isn't JSON a spec? I see a lot of requests that are app specific, that would be solved simply by having the app specify the value properly. For example, Angular wants to put something like: As soon as you start converting things you will experience feature bloat and bugs like no other, actually creating pain instead of helping future roadmaps for any apps that use the library. JSON isn't meant to be a text processor. |
Using Newtonsoft.Json instead of System.System.Text.Json which is the default since 3.1. The latter doesn't convert strings to ints automatically. This was the cause of the issue. https://github.com/dotnet/corefx/issues/39473
namespace System.Text.Json
{
public partial sealed class JsonSerializerOptions
{
public JsonNumberHandling NumberHandling { get; set; }
}
}
namespace System.Text.Json.Serialization
{
[Flags]
public enum JsonNumberHandling
{
Strict = 0x0,
AllowReadingFromString = 0x1,
WriteAsString = 0x2,
AllowNamedFloatingPointLiterals = 0x4
}
[AttributeUsage(
AttributeTargets.Class |
AttributeTargets.Struct |
AttributeTargets.Property |
AttributeTargets.Field, AllowMultiple = false)]
public partial sealed class JsonNumberHandlingAttribute : JsonAttribute
{
public JsonNumberHandlingAttribute(JsonNumberHandling handling);
public JsonNumberHandling Handling { get; }
}
} |
I'd like to this proposal include an overload on int x= 5;
writer.WriteNumberValue(x,true) // writes "5"
writer.WriteNumberValue(x,false) // writes 5 This avoids having to do the extra memory allocation in writer.WriteStringValue(x.ToString()) // writes "5" and it allows you to write efficient code when the JSON schema you have to comply with has some numbers quoted but not all of them. |
@paulhickman-a365 this issue addresses support at the serializer level only. Please open a new issue with the "API proposal" format for your suggestion for the writer to be considered. |
It seems this is a missing feature. Better to add this feature like newtonsoft.json |
@ironpython2001 this feature is checked in and coming in .NET 5. You can try it in .NET 5 RC1 - https://devblogs.microsoft.com/dotnet/announcing-net-5-0-rc-1/. |
Original proposal by @NickCraver (click to view)
Apologies if this issue exists already...I hunted and couldn't find one.
I've been trying to switch a lot of usages over to
System.Text.Json
from Newtonsoft and Jil, but a recurring theme is handling external APIs and their not-quite-right returns of JSON.Unfortunately, an example I'm seeing over and over is:
They're numbers, but as strings.
int
,double
,decimal
, whatever. It happens all the time. And since it's someone's API, it's unlikely we'll get the world to fix this.Note: this is something Newtonsoft handles by default, which is why I don't think most people realize it's an issue.
Is there any chance we can let
System.Text.Json
handle number types being quoted when deserializing? Something a user opts-into viaJsonSerializerOptions
?Small repro example:
cc @ahsonkhan @steveharter
Edited by @layomia:
Users want to be able to deserialize JSON strings into number properties. Scenarios include deserializing "NaN", "Infinity" and "-Infinity" (#31024).
We should add an option to enable this scenario, and possibly allow serializing numbers as strings.
These semantics allow better interop with various API endpoints accross the web.
API Proposal
Usage
Allow reading numbers from strings; write numbers as strings.
Allow reading numbers from floating point constants; write floating point constants
Given a class:
Without the new option, reading floating-point-constant representations fails:
Writing also fails:
With the new option, reading floating-point-constant representations works:
Writing floating-point-constant representations also works:
Allow reading numbers from string; support reading and writing floating point constants
Reading:
Writing:
Write numbers as strings; support reading and writing floating point constants
Reading:
Writing:
Read semantics are the same as the serializer's default (no quotes)
For example, integers cannot have decimal places:
Similarly, there can't be any leading or trailing trivia:
Leading or trailing whitespace is also not allowed:
Note that formats such as the currency and percentage formats are not allowed. Commas (,), the percent symbol (%), and underscores (_) are not permitted in input strings.
Notes
Newtonsoft compat
Read/write semantics
double
representations.float
representations.JsonTokenType.String
tokens will be unescaped if needed when deserializing. This is in keeping with other string-based parsing for types likeDateTime
andGuid
.The text was updated successfully, but these errors were encountered: