Repository: ultrajson/ultrajson (language: python)

Issue:
NaN handling differs from other json packages
```pycon
$ python
Python 2.7.6 (default, Mar 22 2014, 22:59:56) 
[GCC 4.8.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import ujson, json
>>> ujson.__version__
'1.33'
>>> json.__version__
'2.0.9'
>>> 
>>> json.dumps(float('nan'))
'NaN'
>>> ujson.dumps(float('nan'))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: Invalid Nan value when encoding double
>>> 
>>> json_nan = json.dumps(float('nan'))
>>> json.loads(json_nan)
nan
>>> ujson.loads(json_nan)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Expected object or value
>>>
```



Discussion / hints:
Actually NaN is not permitted by the JSON spec.

From RFC 4627 Section 2.4: (http://www.ietf.org/rfc/rfc4627.txt)

```
Numeric values that cannot be represented as sequences of digits
(such as Infinity and NaN) are not permitted.
```

Ah, you are correct.  I'll change the title to reflect the problem more accurately.

It is a design decision to follow the spec or python standard library behavior.

Should make a note in the readme of this difference with json/simplejson.  Some user switching from json  to ujson may depend on this (incorrect) behavior.

`OverflowError` seems like the wrong exception to throw.  A better exception choice and a useful error message would be helpful.

A quick survey:
- json: no error
- simplejson: no error
- yajl: `TypeError: Object is not JSON serializable`
- ujson: `OverflowError: Invalid Nan value when encoding double`

We'll go with the json spec on this one.

No solution offered yet. Does this mean that whenever there is a `nan` in the data we have to use a different package?
Any way to handle this?
Some good news: the first bit will be fixed by https://github.com/ultrajson/ultrajson/pull/327, but means dropping Python 2.7:

```pycon
Python 3.8.2 (v3.8.2:7b3ab5921f, Feb 24 2020, 17:52:18)
[Clang 6.0 (clang-600.0.57)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
(.startup.py)
(imported collections, datetime, itertools, math, os, pprint, re, sys, time)
>>> import ujson, json
>>> ujson.__version__
'2.0.4.dev4'
>>> json.__version__
'2.0.9'
>>> json.dumps(float('nan'))
'NaN'
>>> ujson.dumps(float('nan'))
'NaN'
```

PRs welcome for the second bit:

```pycon
>>> json_nan = json.dumps(float('nan'))
>>> json.loads(json_nan)
nan
>>> ujson.loads(json_nan)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Expected object or value
>>>
```

This was implemented in pandas which vendored ujson quite a few years ago. I think integration here wouldn't be too difficult if anyone wants to take a look:

https://github.com/pandas-dev/pandas/pull/30295/files
I understand this isn't strictly in the json spec, but this issue means I cannot use ujson in my application, which is a bummer considering it seems much faster than what CPython ships with.

This issue also means that `ujson` is not a drop-in replacement for `json`. It might be good to at least add a caveat to that claim on the README page.

Is adding support for nans in order to _actually_ be a drop-in replacement something that would kill the speed of ujson? Is having the ability to call `load` when I know my data might contain nans (which it does because it is real-world data) something that the authors are at all interested in?

Fix patch:
diff --git a/lib/ultrajson.h b/lib/ultrajson.h
index 51136ea8..8334fcae 100644
--- a/lib/ultrajson.h
+++ b/lib/ultrajson.h
@@ -166,6 +166,9 @@ enum JSTYPES
   JT_ARRAY,     // Array structure
   JT_OBJECT,    // Key/Value structure
   JT_INVALID,   // Internal, do not return nor expect
+  JT_NAN,       // Not A Number
+  JT_POS_INF,   // Positive infinity
+  JT_NEG_INF,   // Negative infinity
 };
 
 typedef void * JSOBJ;
@@ -321,6 +324,9 @@ typedef struct __JSONObjectDecoder
   JSOBJ (*newTrue)(void *prv);
   JSOBJ (*newFalse)(void *prv);
   JSOBJ (*newNull)(void *prv);
+  JSOBJ (*newNaN)(void *prv);
+  JSOBJ (*newPosInf)(void *prv);
+  JSOBJ (*newNegInf)(void *prv);
   JSOBJ (*newObject)(void *prv);
   JSOBJ (*newArray)(void *prv);
   JSOBJ (*newInt)(void *prv, JSINT32 value);
diff --git a/lib/ultrajsondec.c b/lib/ultrajsondec.c
index 05b14523..73e8a5db 100644
--- a/lib/ultrajsondec.c
+++ b/lib/ultrajsondec.c
@@ -98,10 +98,16 @@ static FASTCALL_ATTR JSOBJ FASTCALL_MSVC decode_numeric (struct DecoderState *ds
 
   JSUINT64 overflowLimit = LLONG_MAX;
 
-  if (*(offset) == '-')
-  {
-    offset ++;
-    intNeg = -1;
+  if (*(offset) == 'I') {
+      goto DECODE_INF;
+  } else if (*(offset) == 'N') {
+      goto DECODE_NAN;
+  } else if (*(offset) == '-') {
+      offset++;
+      intNeg = -1;
+      if (*(offset) == 'I') {
+          goto DECODE_INF;
+      }
     overflowLimit = LLONG_MIN;
   }
 
@@ -189,6 +195,48 @@ static FASTCALL_ATTR JSOBJ FASTCALL_MSVC decode_numeric (struct DecoderState *ds
   {
     return ds->dec->newInt(ds->prv, (JSINT32) (intValue * intNeg));
   }
+
+DECODE_NAN:
+    offset++;
+    if (*(offset++) != 'a') goto SET_NAN_ERROR;
+    if (*(offset++) != 'N') goto SET_NAN_ERROR;
+
+    ds->lastType = JT_NAN;
+    ds->start = offset;
+    return ds->dec->newNaN(ds->prv);
+
+SET_NAN_ERROR:
+    return SetError(ds, -1, "Unexpected character found when decoding 'NaN'");
+
+DECODE_INF:
+    offset++;
+    if (*(offset++) != 'n') goto SET_INF_ERROR;
+    if (*(offset++) != 'f') goto SET_INF_ERROR;
+    if (*(offset++) != 'i') goto SET_INF_ERROR;
+    if (*(offset++) != 'n') goto SET_INF_ERROR;
+    if (*(offset++) != 'i') goto SET_INF_ERROR;
+    if (*(offset++) != 't') goto SET_INF_ERROR;
+    if (*(offset++) != 'y') goto SET_INF_ERROR;
+
+    ds->start = offset;
+
+    if (intNeg == 1) {
+      ds->lastType = JT_POS_INF;
+      return ds->dec->newPosInf(ds->prv);
+    } else {
+      ds->lastType = JT_NEG_INF;
+      return ds->dec->newNegInf(ds->prv);
+    }
+
+SET_INF_ERROR:
+    if (intNeg == 1) {
+      const char *msg = "Unexpected character found when decoding 'Infinity'";
+      return SetError(ds, -1, msg);
+    } else {
+      const char *msg = "Unexpected character found when decoding '-Infinity'";
+      return SetError(ds, -1, msg);
+    }
+
 }
 
 static FASTCALL_ATTR JSOBJ FASTCALL_MSVC decode_true ( struct DecoderState *ds)
@@ -732,6 +780,8 @@ static FASTCALL_ATTR JSOBJ FASTCALL_MSVC decode_any(struct DecoderState *ds)
       case '7':
       case '8':
       case '9':
+      case 'I':
+      case 'N':
       case '-':
         return decode_numeric (ds);
 
diff --git a/python/JSONtoObj.c b/python/JSONtoObj.c
index f2a1a2c7..f370befe 100644
--- a/python/JSONtoObj.c
+++ b/python/JSONtoObj.c
@@ -79,6 +79,21 @@ static JSOBJ Object_newNull(void *prv)
   Py_RETURN_NONE;
 }
 
+static JSOBJ Object_newNaN(void *prv)
+{
+    return PyFloat_FromDouble(Py_NAN);
+}
+
+static JSOBJ Object_newPosInf(void *prv)
+{
+    return PyFloat_FromDouble(Py_HUGE_VAL);
+}
+
+static JSOBJ Object_newNegInf(void *prv)
+{
+    return PyFloat_FromDouble(-Py_HUGE_VAL);
+}
+
 static JSOBJ Object_newObject(void *prv)
 {
   return PyDict_New();
@@ -129,6 +144,9 @@ PyObject* JSONToObj(PyObject* self, PyObject *args, PyObject *kwargs)
     Object_newTrue,
     Object_newFalse,
     Object_newNull,
+    Object_newNaN,
+    Object_newPosInf,
+    Object_newNegInf,
     Object_newObject,
     Object_newArray,
     Object_newInteger,


Test patch:
diff --git a/tests/test_ujson.py b/tests/test_ujson.py
index ddc065fd..56e7664b 100644
--- a/tests/test_ujson.py
+++ b/tests/test_ujson.py
@@ -670,6 +670,79 @@ def test_encode_raises_allow_nan(test_input, expected_exception):
         ujson.dumps(test_input, allow_nan=False)
 
 
+def test_nan_inf_support():
+    # Test ported from pandas
+    text = '["a", NaN, "NaN", Infinity, "Infinity", -Infinity, "-Infinity"]'
+    data = ujson.loads(text)
+    expected = [
+        "a",
+        float("nan"),
+        "NaN",
+        float("inf"),
+        "Infinity",
+        -float("inf"),
+        "-Infinity",
+    ]
+    for a, b in zip(data, expected):
+        assert a == b or math.isnan(a) and math.isnan(b)
+
+
+def test_special_singletons():
+    pos_inf = ujson.loads("Infinity")
+    neg_inf = ujson.loads("-Infinity")
+    nan = ujson.loads("NaN")
+    null = ujson.loads("null")
+    assert math.isinf(pos_inf) and pos_inf > 0
+    assert math.isinf(neg_inf) and neg_inf < 0
+    assert math.isnan(nan)
+    assert null is None
+
+
+@pytest.mark.parametrize(
+    "test_input, expected_message",
+    [
+        ("n", "Unexpected character .* 'null'"),
+        ("N", "Unexpected character .*'NaN'"),
+        ("NA", "Unexpected character .* 'NaN'"),
+        ("Na N", "Unexpected character .* 'NaN'"),
+        ("nan", "Unexpected character .* 'null'"),
+        ("none", "Unexpected character .* 'null'"),
+        ("i", "Expected object or value"),
+        ("I", "Unexpected character .* 'Infinity'"),
+        ("Inf", "Unexpected character .* 'Infinity'"),
+        ("InfinitY", "Unexpected character .* 'Infinity'"),
+        ("-i", "Trailing data"),
+        ("-I", "Unexpected character .* '-Infinity'"),
+        ("-Inf", "Unexpected character .* '-Infinity'"),
+        ("-InfinitY", "Unexpected character .* '-Infinity'"),
+        ("- i", "Trailing data"),
+        ("- I", "Trailing data"),
+        ("- Inf", "Trailing data"),
+        ("- InfinitY", "Trailing data"),
+    ],
+)
+def test_incomplete_special_inputs(test_input, expected_message):
+    with pytest.raises(ujson.JSONDecodeError, match=expected_message):
+        print(f"test_input = {test_input!r}")
+        ujson.loads(test_input)
+
+
+@pytest.mark.parametrize(
+    "test_input, expected_message",
+    [
+        ("NaNaNaN", "Trailing data"),
+        ("Infinity and Beyond", "Trailing data"),
+        ("-Infinity-and-Beyond", "Trailing data"),
+        ("NaN!", "Trailing data"),
+        ("Infinity!", "Trailing data"),
+        ("-Infinity!", "Trailing data"),
+    ],
+)
+def test_overcomplete_special_inputs(test_input, expected_message):
+    with pytest.raises(ujson.JSONDecodeError, match=expected_message):
+        ujson.loads(test_input)
+
+
 @pytest.mark.parametrize(
     "test_input",
     [