Emit warnings when dumping binary strings #643
Conversation
Because of it's Ruby 1.8 heritage, the C extension doesn't care much about strings encoding. We should get stricter over time.
| VALUE utf8_string = rb_enc_associate_index(rb_str_dup(source), utf8_encindex); | ||
| switch (rb_enc_str_coderange(utf8_string)) { | ||
| case ENC_CODERANGE_7BIT: | ||
| case ENC_CODERANGE_VALID: | ||
| return utf8_string; | ||
| } |
There was a problem hiding this comment.
Whoops, I commented too early and thought this was a YAML issue.
After reexamining the code, it does appear that the json implementation for JRuby works directly with bytes via the ragel parser. It would probably be possible to make it accept invalid characters.
My position remains the same, however. I'm not sure what other parsers you tested, but all json specifications I looked at explicitly require content to be in one of the main unicode UTF encodings. That says to me that providing invalid characters is off spec and should produce an error.
The original RFC from 2006 even says:
JSON text SHALL be encoded in Unicode. The default encoding is UTF-8.
https://www.ietf.org/rfc/rfc4627.txt
The character stream cannot be parsed as Unicode, then it is not up to spec.
There was a problem hiding this comment.
I'm not sure what other parsers you tested,
oj and Python's stdlib.
I totally get your argument, and generally agree, the concern is really about breaking existing (misbehaving) code.
There was a problem hiding this comment.
Maybe I'm crazy, but I would break it in a minor release so it's easy to revert with another minor release, rather than break it in a major release and have to go back. I doubt the impact will be large. If we put out a minor release that starts erroring on bad input and it doesn't get reported for a few months, it's probably fine.
I think it's also important to remember that Ruby has become much more strict about character encodings over the years so the chance of getting bad content has reduced significantly since the original issue.
Fix: #642
Because of it's Ruby 1.8 heritage, the C extension doesn't care much about strings encoding. We should get stricter over time.