-
Notifications
You must be signed in to change notification settings - Fork 35
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
inf,nan etc in json output #14
Comments
Hi. Why did you pass 0+'Inf' to encode_json() in the first place? |
IIRC that was an occasional float overflow, that I stored in json, and that subsequently caused the json deserializer code to crash on reading 'inf' without quotes. I have since protected the serializer with '$_ = 1e37 if $_ > 1e37', but it would be nice to have the module to conform to RFC, I think. |
I'm happy to hear you've already done the right thing :) As for the JSON RFCs, they say only
and they don't define what to do when a JSON encoder sees a non-permitted value (please forget about RFCs for a JavaScript/EcmaScript encoder). So some croak, some turn them into null, some even turn them into strings (like "inf" or "nan"), and some let users to choose how to behave. I asked the maintainer of JSON::XS and he really patiently told me that this was too controversial and it'd be better to keep the current behavior, considering the cost of migration, performance and maintenance (especially for JSON::XS) ,even though both of us knew values like inf and nan were "not permitted". Fundamentally it's users' responsibility to validate values (as you know, an acceptable value (for (probably IEEE) floating point, in this case) differs from environment to environment, so eventually you need to decide whatever). We might make our encoder croak when it sees an invalid value if we definitely need to do so (as both of us prefer croaking to making them to null and such), but as of this writing, nobody convinced him (or us) to do so. And we trust you pass a valid value to our encoders. As the doc says,
I'd welcome a patch to improve this doc (maybe we should explain this kind of stuff in the pod). I hope this decision makes sense to you, too. |
I agree that there are lots of values one can send to encoding that won't make sense. I don't know about croaking (or warning, or anything else), my concern is that there is an implicit generation of invalid JSON, that one needs to be really protective against. Also, unfortunately the author of JSON::XS, in my opinion, is quite an opinionated person, and I do not know a single case throughout of his interaction with other perl people where he was successfully reasoned with -- it's not to say any bad words about him, but rather that I don't take any words from him as a valid argument knowing that there will be argument just for the argument sake. I also agree that inf is indeed controversial, and that's why I called the heavy-weight argument from RFC, which seems at least a reliable source for software architecture. In my eyes, RFC vs Mr.Lehmann clearly wins, therefore basing your decision on the latter doesn't really make sense to me. You are of course free to pursue whatever behavior you consider most reasonable. I don't have a stake here, I just wanted to point out that the module violates RFC by producing invalid JSON. It's up to you how to deal with it - either fixing the code, or documentation, or adding a conservative encoder option as default/non-default croak/warning on inf/nan. Hope this make sense, |
I vote for croak: interoperability issues should be detected early on the emitter side, not on the receiving side of the JSON document. Even at the cost of slower serialisation. As Nicolas Seriot wrote, JSON parsing is a minefield while most expect it to be just simple and clean. |
For the record, Mojo::JSON and JSON::Tiny encode inf/nan to strings, and Cpanel::JSON::XS has an option to stringify it, and will by default encode it to null (like other options do with invalid input like blessed objects etc) -- https://metacpan.org/pod/Cpanel::JSON::XS#$json->stringify_infnan-([$infnan_mode-=-1]) My opinion is that there is no "correct" operation, so encoding to null or a representative string or croaking based on the user option is the most reasonable choice; but croaking by default seems unreasonable (just like producing invalid JSON) as numbers can become inf or nan unintentionally. |
I agree with @dolmen. |
Hello,
encode_json([0+'Inf']) produces "[inf]" which is in violation of rfc4627 2.4 that does not permit non-digit numerical values.
Sincerely,
Dmitry
The text was updated successfully, but these errors were encountered: