Fixed "illegal literal" error when JSON-decoding NaN.

[liautaud]

While running the fuzzer, I discovered that we don't properly support the floating-point value NaN.

When converting it into JSON, we pass it to Jsonm which produces the token nan (because Jsonm uses string_of_float and string_of_float Float.nan = "nan"). Yet, since nan is not a valid JSON token according to the specification, Jsonm doesn't know how to print it and fails with an "illegal literal" error when decoding.

This pull request fixes the issue by encoding NaN floats to null.

[icristescu]

Thanks! Do you know if this problem also occurs with FP_infinite?

[craigfe]

(and negative infinity)

[hannesm]

I'm wondering whether this is an issue in jsonm, and should be fixed upstream instead?

[liautaud]

@icristescu @craigfe Oh, thanks, I didn't think to check these ones. They do not work either, unsurprisingly. I'll update the PR using "inf" and "-inf" as JSON encodings. Since we'll have to use strings for these two, would you prefer using "nan" instead of null for NaN values?

@hannesm I agree in principle, but don't you think it would be wiser to fix the issue on our side first, and then wait for our pull request to be merged before we remove the fix and bump the dependency?

[liautaud]

All non-standard floating-point values (nan, inf and -inf) are now encoded using their OCaml string representation ("nan", "inf" and "-inf" respectively).

[craigfe]

I agree with @hannesm that this is Jsonm's problem. I suggest we report the issue there and see what comes of it; perhaps a different solution will be chosen there, requiring us to adapt here.

[pascutto]

Trying to fix in Jsonm first indeed seems better!
The loss of precision you encountered when encoding floats should also be reported there.

[liautaud]

I agree with @hannesm that this is Jsonm's problem. I suggest we report the issue there and see what comes of it; perhaps a different solution will be chosen there, requiring us to adapt here.

As explained in jsonm/#12, this is apparently the expected behavior of jsonm, so we should fix the issue ourselves.

[samoht]

Could you rebase this PR on master? Thanks!

[liautaud]

@samoht Sure!

[liautaud]

I'm converting to draft for the time being since the PR breaks the to_string pretty-printer.

It seems that to_string falls back to the JSON pretty-printer whenever it encounters a compound type–e.g. an array, which leads to inconsistent results. For instance, to_string float Float.nan is nan (as expected), but to_string (array float) [| Float.nan |] is [| "nan" |] (because the JSON encoding of Float.nan is the string "nan" and not just nan).

I'll quickly submit another PR to solve this issue by removing the dependency of to_string on to_json_string.