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

What is the recomendation of using System.Text.Json and double NaN values ? #31024

Closed
pranavkm opened this issue Sep 30, 2019 · 7 comments
Closed

Comments

@pranavkm
Copy link
Contributor

From @valeriob on Monday, September 30, 2019 12:49:01 PM

Hi,
we are upgrading some applications to aspnetcore 3.0, we have been bitten by something unexpected : System.Text.Json refuse to serialize double.NaN values.
This cause really nasty bugs because something that works suddenly does not because some result of some calculation is different, or some data changes in some way.
I see that the recommendation (https://docs.microsoft.com/en-us/aspnet/core/migration/22-to-30?view=aspnetcore-3.0&tabs=visual-studio#jsonnet-support) is to use Json.Net.

But how can ppl use this library if such a simple and common use case is not covered ?
I do not know any application that can live without this feature, i understand the fact that there is a specification, but it may very well be unpractical.
https://thefactotum.xyz/post/the-devil-is-in-the-json-details/
Python, Go, Javascript,Rust, Ruby handle it without making much fuss 😄

Thanks
Valerio

Copied from original issue: dotnet/aspnetcore#14571

@safern
Copy link
Member

safern commented Sep 30, 2019

cc: @ahsonkhan

@jkotas jkotas changed the title What is the reccomendation of using System.Text.Json and double NaN values ? What is the recomendation of using System.Text.Json and double NaN values ? Oct 1, 2019
@ericstj
Copy link
Member

ericstj commented Nov 4, 2019

@tannergooding can you weigh in on this?

@tannergooding
Copy link
Member

This is a more complicated scenario as some floating-point values (+infinity, -infinity, and NaN) can't be represented as JSON "numbers".

The only way these can be successfully serialized/deserialized is by representing them by using an alternative format (such as strings).

@ericstj
Copy link
Member

ericstj commented Nov 4, 2019

JSon.NET stores these as strings in the payload then automatically converts to double.

Console.WriteLine(JsonConvert.SerializeObject(new double [] { Double.PositiveInfinity, Double.NaN, 0.100, 1.0002, Math.PI }));

Writes

["Infinity","NaN",0.1,1.0002,3.141592653589793]

And the following succeeds:

JsonConvert.DeserializeObject<double[]>("[\"Infinity\",\"NaN\",0.1,1.0002,3.141592653589793]");

Is this something we should do automatically? If not, then what should OP do to handle this? Write a converter? /cc @steveharter

@pranavkm
Copy link
Contributor Author

pranavkm commented Nov 5, 2019

The linked document - https://thefactotum.xyz/post/the-devil-is-in-the-json-details/#javascript - includes examples of what browsers do when when these values are serialized:

> JSON.stringify({"nan": NaN})
'{"nan":null}'
> JSON.stringify({"inf": Infinity})
'{"inf":null}'

// My local tests:
JSON.stringify([Number.NEGATIVE_INFINITY, Number.POSITIVE_INFINITY, Number.NaN])
"[null,null,null]"

IMO, authoring a converter if these uncommon scenarios are important seem like the way to go.

@ahsonkhan
Copy link
Member

ahsonkhan commented Nov 13, 2019

Is this something we should do automatically? If not, then what should OP do to handle this? Write a converter?

Yes, write a converter to handle the special case.

IMO, authoring a converter if these uncommon scenarios are important seem like the way to go.

Agreed.

Here's an example of a converter you could write to be able to round-trip the special double values (like NaN/infinities) and also one to be compatible with Newtonsoft.Json's way of serializing special doubles for round-tripping:

[Fact]
public static void HandleSpecialDoubles_NewtonsoftCompat()
{
    string expectedJson = JsonConvert.SerializeObject(new double[] { double.PositiveInfinity, double.NegativeInfinity, double.NaN, 0.100, 1.0002, Math.PI });
    Console.WriteLine(expectedJson);
    double[] expected = JsonConvert.DeserializeObject<double[]>("[\"Infinity\",\"-Infinity\",\"NaN\",0.1,1.0002,3.141592653589793]");

    var options = new JsonSerializerOptions();
    options.Converters.Add(new HandleSpecialDoublesAsStrings_NewtonsoftCompat());

    string actualJson = JsonSerializer.Serialize(new double[] { double.PositiveInfinity, double.NegativeInfinity, double.NaN, 0.100, 1.0002, Math.PI }, options);
    Console.WriteLine(actualJson);
    double[] actual = JsonSerializer.Deserialize<double[]>("[\"Infinity\",\"-Infinity\",\"NaN\",0.1,1.0002,3.141592653589793]", options);

    Assert.Equal(expectedJson, actualJson);
    Assert.Equal(expected, actual);

    options = new JsonSerializerOptions();
    options.Converters.Add(new HandleSpecialDoublesAsStrings());

    actualJson = JsonSerializer.Serialize(new double[] { double.PositiveInfinity, double.NegativeInfinity, double.NaN, 0.100, 1.0002, Math.PI }, options);
    // Note, this will write the infinities escaped (as "\u221E")
    // You can override the encoder within options, for example: options.Encoder = JavaScriptEncoder.Create(UnicodeRanges.All);
    Console.WriteLine(actualJson);
    actual = JsonSerializer.Deserialize<double[]>("[\"\",\"-∞\",\"NaN\",0.1,1.0002,3.141592653589793]", options);
}

private class HandleSpecialDoublesAsStrings : JsonConverter<double>
{
    public override double Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        if (reader.TokenType == JsonTokenType.String)
        {
            return double.Parse(reader.GetString());
        }
        return reader.GetDouble();
    }

    public override void Write(Utf8JsonWriter writer, double value, JsonSerializerOptions options)
    {
        if (double.IsFinite(value))
        {
            writer.WriteNumberValue(value);
        }
        else
        {
            writer.WriteStringValue(value.ToString());
        }
    }
}

private class HandleSpecialDoublesAsStrings_NewtonsoftCompat : JsonConverter<double>
{
    public override double Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        if (reader.TokenType == JsonTokenType.String)
        {
            string specialDouble = reader.GetString();
            if (specialDouble == "Infinity")
            {
                return double.PositiveInfinity;
            }
            else if (specialDouble == "-Infinity")
            {
                return double.NegativeInfinity;
            }
            else
            {
                return double.NaN;
            }
        }
        return reader.GetDouble();
    }

    private static readonly JsonEncodedText s_nan = JsonEncodedText.Encode("NaN");
    private static readonly JsonEncodedText s_infinity = JsonEncodedText.Encode("Infinity");
    private static readonly JsonEncodedText s_negativeInfinity = JsonEncodedText.Encode("-Infinity");

    public override void Write(Utf8JsonWriter writer, double value, JsonSerializerOptions options)
    {
        if (double.IsFinite(value))
        {
            writer.WriteNumberValue(value);
        }
        else
        {
            if (double.IsPositiveInfinity(value))
            {
                writer.WriteStringValue(s_infinity);
            }
            else if (double.IsNegativeInfinity(value))
            {
                writer.WriteStringValue(s_negativeInfinity);
            }
            else
            {
                writer.WriteStringValue(s_nan);
            }
        }
    }
}

@layomia
Copy link
Contributor

layomia commented Dec 12, 2019

@steveharter @tannergooding @pranavkm @valeriob I believe this issue should be considered as part of https://github.com/dotnet/corefx/issues/39473 which deals with (de)serialization support for quoted numbers. Please re-open if needed.

@layomia layomia closed this as completed Dec 12, 2019
@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@tannergooding tannergooding removed their assignment May 26, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants