JamesNK / Newtonsoft.Json

Json.NET is a popular high-performance JSON framework for .NET
https://www.newtonsoft.com/json
MIT License
10.71k stars 3.24k forks source link

JObject does not deserialize double correctly #2889

Closed RDUser-dev closed 11 months ago

RDUser-dev commented 1 year ago

Generate Json from a JObject and load back into a second JObject

JObject obj1 = new JObject() { 
                { "val1", -5.2585885403382 } 
            };
var str1 = obj1.ToString();

str1 contains the expected value:

{
  "val1": -5.2585885403382
}

Try loading the json string into a JObject again

JObject obj2 = JObject.Parse(str1); 

Now obj2["Val1"] contains a different value and stringify to a different Json:

{
  "val1": -5.2585885403382004
}

Expected behavior

obj2 equals obj1

Actual behavior

With .netFramework 4.8 running at 64 bit obj2 and obj1 are different

Steps to reproduce

static void Main(string[] args)
        {
            JObject obj1 = new JObject() { 
                { "val1", -5.2585885403382 } 
            }; 

            JObject obj2 = JObject.Parse(obj1.ToString());

            if (obj2["val1"].Value<double>() != -5.2585885403382) throw new ApplicationException();
        }

Notes

We use JObjects to save configurations by attaching a hash of the json, we expect to be able to read and re-save the same configuration getting the same hash. It is therefore important that the serialization and deserialization of the JObject is perfectly symmetric, and not just for my specific scenario. Do you know if there is an existing workaround to the problem?

elgonzo commented 1 year ago

Edit: I can reproduce with Prefer32Bit being disabled on a x64 machine. Can you please reconfirm that you see the problem when Prefer32Bit is enabled or when it is disabled?

Cannot reproduce with the current Newtonsoft.Json version 13.0.3, using a VS2015-style project targetting .NET Framework 4.8 and the Prefer32Bit flag being enabled. obj2["val1"].Value<double>() yields exactly -5.2585885403382.

Make sure you are using the current Newtonsoft.Json 13.0.3. If the problem still persists with 13.0.3, could you please share your project file you are used for running the sample Main method you posted?

RDUser-dev commented 1 year ago

Edit: I can reproduce with Prefer32Bit being disabled on a x64 machine. Can you please reconfirm that you see the problem when Prefer32Bit is enabled or when it is disabled?

Cannot reproduce with the current Newtonsoft.Json version 13.0.3, using a VS2015-style project targetting .NET Framework 4.8 and the Prefer32Bit flag being enabled. obj2["val1"].Value<double>() yields exactly -5.2585885403382.

Make sure you are using the current Newtonsoft.Json 13.0.3. If the problem still persists with 13.0.3, could you please share your project file you are used for running the sample Main method you posted?

Thanks Elgonzo for the quick reply, I am sending you the test described above. TestDoubleNewtonsoft.zip

elgonzo commented 1 year ago

At least regarding my tests, the problem lies with the compiler. With Prefer32Bit being disabled, the -5.2585885403382 will be turned by the compiler into the effective value -5.2585885403381996. I guess, the intended value probably cannot be represented exactly as a double floating point value. Interestingly, double.Parse(string) results in a slightly different incorrect value. Demonstration:

double d1 = -5.2585885403382;
double d2 = -5.2585885403382d;
double d3 = double.Parse("-5.2585885403382");

Console.WriteLine(d1.ToString("G17"));
Console.WriteLine(d2.ToString("G17"));
Console.WriteLine(d3.ToString("G17"));

running this on a x64 OS will output:

-5.2585885403381996
-5.2585885403381996
-5.2585885403382004

Running it with Prefer32Bit enabled makes double.Parse() exhibiting exactly the same kind of imprecision as that of the compiler-generated const value, thus the const value and the value created by double.Parse still being imprecise but agreeing on the imprecise value.

-5.2585885403381996
-5.2585885403381996
-5.2585885403381996

Long story short: As is often the case, beware precision-related issues with floating point types. If you need exact value and want to do equality comparisons without accounting for epsilon, i'd suggest you switch to using decimal type instead of double/float. For example like this:

JObject obj2 = JObject.Parse(str1);

decimal d = obj2["val1"].Value<decimal>();

if (d != -5.2585885403382m) throw new ApplicationException();    

(P.S.: Thanks for the project file. It confirms that for your project the issue happens when Prefer32Bit is disabled.)

(P.P.S: It seems more modern .NET versions don't seem to suffer from this difference between const value and the double.Parse result anymore. I went back as far as .NET 5, and double.Parse always produces the same imprecise result matching the const value generated by the compiler regardless of the Prefer32Bit setting...)

RDUser-dev commented 1 year ago

At least regarding my tests, the problem lies with the compiler. With Prefer32Bit being disabled, the -5.2585885403382 will be turned by the compiler into the effective value -5.2585885403381996. I guess, the intended value probably cannot be represented exactly as a double floating point value. Interestingly, double.Parse(string) results in a slightly different incorrect value.

I thought there were problems with rounding conversions etc.. But even if the strings do not exactly match the starting number, they coincide with each other consistently. The point is it is possible to do the conversions double=>string=>double=>string and the two strings match. Instead it is not possible to do the conversions JObject=>JsonString=>JObject=>JsonString and obtains two equal JsonString.

image

moving from double to decimal would be a huge job, since the code we are working on is very large. But thanks for the tip

elgonzo commented 1 year ago

Instead it is not possible to do the conversions JObject=>JsonString=>JObject=>JsonString and obtains two equal JsonString.

Why do you think it is not possible to do JObject=>JsonString=>JObject=>JsonString?

Based on the code in your TestDoubleNewtonsoft, if you do:

...

var str1 = obj1.ToString();

JObject obj2 = JObject.Parse(str1);

var str2 = obj2.ToString();
Console.WriteLine(str1 == str2);

str2 will be equal to str1.

Do you have a concrete example where JObject=>JsonString=>JObject=>JsonString fails?

RDUser-dev commented 1 year ago

Good morning @elgonzo

Do you have a concrete example where JObject=>JsonString=>JObject=>JsonString fails?

Of course, the first example does pretty much that. Run in 64 bit it fails


        static void Main(string[] args)
        {

            JObject obj1 = new JObject() { 
                { "val1", -5.2585885403382 } 
            };
            var str1 = obj1.ToString();

            JObject obj2 = JObject.Parse(str1);
            var str2 = obj2.ToString();

            if (str1 != str2) throw new ApplicationException();
        }

TestDoubleNewtonsoft.zip

elgonzo commented 1 year ago

Of course, the first example does pretty much that. Run in 64 bit it fails

Thanks for getting back. I tested again, and i found out why i did see str1 and str2 being equal -- in the legacy .NET Framework x64 runtime, the code behaves differently when running in debug mode compared to running in release mode. (And i apparently did my initial quick str1, str2 comparison test in release mode...)

Basically, building and running your example code in debug mode results in str2 being:

{
  "val1": -5.2585885403382004
}

But building and running the same code in release mode, str2 will be equal to str1:

{
  "val1": -5.2585885403382
}

This problem can be traced back to Newtonsoft.Json using the "R" format specifier when converting a floating point value to its json text representation:

https://github.com/JamesNK/Newtonsoft.Json/blob/0a2e291c0d9c0c7675d445703e51750363a549ef/Src/Newtonsoft.Json/JsonConvert.cs#L293-L296

This is what the official documentation has to say about the "R" specifier (https://learn.microsoft.com/en-us/dotnet/standard/base-types/standard-numeric-format-strings#round-trip-format-specifier-r):

In .NET Framework and in .NET Core versions earlier than 3.0, the "R" format specifier fails to successfully round-trip Double values in some cases. For both Double and Single values, the "R" format specifier offers relatively poor performance. Instead, we recommend that you use the "G17" format specifier for Double values and the "G9" format specifier to successfully round-trip Single values.

And this made me remember that i made a comment to the same effect regarding another issue report about 2 years ago having the same root cause in Newtonsoft.Json using the "R" format specifier: https://github.com/JamesNK/Newtonsoft.Json/issues/2540 Unfortunately, this issue hasn't received any attention so far by the author/maintainer of the library.

A possible workaround can be a custom JsonConverter for double values. This, however, will require the JToken.ToString() invocations to be altered, so i don't know whether this workaround would be feasible for you:

public class RoundtrippableDoubleConverter : JsonConverter<double>
{
    public override bool CanRead => false;

    public override double ReadJson(JsonReader reader, Type objectType, double existingValue, bool hasExistingValue, JsonSerializer serializer)
        => throw new NotSupportedException();

    public override void WriteJson(JsonWriter writer, double value, JsonSerializer serializer)
    {
        var str = value.ToString("G17", CultureInfo.InvariantCulture);
        writer.WriteRawValue(str);
    }
}

static void Main(string[] args)
{
    var doubleConverter = new RoundtrippableDoubleConverter();

    JObject obj1 = new JObject() { 
        { "val1", -5.2585885403382 } 
    };

    //
    // Note the converter being passed to the ToString method.
    //
    var str1 = obj1.ToString(Newtonsoft.Json.Formatting.Indented, doubleConverter);

    JObject obj2 = JObject.Parse(str1);

    //
    // Note the converter being passed to the ToString method.
    //
    var str2 = obj2.ToString(Newtonsoft.Json.Formatting.Indented, doubleConverter);

    if (str1 != str2) throw new ApplicationException();
}
RDUser-dev commented 1 year ago

Hello @elgonzo, we had tried similar solutions in the past without success but your converter seems to solve the problem. I try it inside the complete application and update you. Thanks again

RDUser-dev commented 11 months ago

We applied the solution to a very complex project. Everything worked properly. Thanks for your precious help @elgonzo . I'm closing the issue