Bunny83 / SimpleJSON

A simple JSON parser in C#
MIT License
735 stars 294 forks source link

Issues with long values near max/min #26

Open wvdvegt opened 4 years ago

wvdvegt commented 4 years ago

Hi

I changed the code of ParseElement a bit so it has some checks on long values near their min/max (see https://wvdvegt.wordpress.com/2018/04/26/double-precision-issues/ for details on the issue):

        private static JSONNode ParseElement(string token, bool quoted)
        {
            ...
            double val;
            if (double.TryParse(token, NumberStyles.Float, CultureInfo.InvariantCulture, out val))
                return val;
            else
                return token;
        }

into

        private static JSONNode ParseElement(string token, bool quoted)
        {
            ...

            //! Patched start.
            Boolean isLong = (longAsString && long.TryParse(token, out long lval));

            if (double.TryParse(token, NumberStyles.Float, CultureInfo.InvariantCulture, out double rval))
            {
                if (isLong && !rval.ToString("F0").Equals(token))
                    return token;
                else
                    return rval;
            }
            else
                return token;
            //! Patched end.
        }

Fix can probably optimized a bit by mainly using the rval.ToString("F0").Equals(token) check

wvdvegt commented 4 years ago

The test

        long l = long.MaxValue; //9223372036854775807

        [TestMethod]
        public void TestMethod1()
        {
            JSONNode n = JSON.Parse($"{{ \"test\":  {l} }}");

            Debug.WriteLine(l);
            Debug.WriteLine(n["test"].AsLong);
        }

returns: 9223372036854775807 -9223372036854775808

using long.MaxValue-1 returns: 9223372036854775806 -9223372036854775808

wvdvegt commented 4 years ago

if (isLong && !rval.ToString("F0").Equals(token)) can I think be replaced by if (longAsString && !rval.ToString("F0").Equals(token)) which saves one conversion.

Another (quite radical) solution would be to replace the m_Data fields by a overlayed struct as

        [StructLayout(LayoutKind.Explicit, Pack = 1)]
        struct Data
        {
            // Stores a value indicating which field at offset to access.
            [FieldOffset(0)]
            public double d;

            [FieldOffset(0)]
            public long l;

            [FieldOffset(8)]
            public byte kind;
        }

However structs have some issues when trying to modifying it's fields.