GeezerVison is showing github.com — segfault on surrogate unicode · Issue #81 · python-rapidjson/python-rapidjson
segfault on surrogate unicode #81DescriptionPython 3.6.1 (default, Mar 23 2017, 02:34:11)
[GCC 4.9.2] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import json
>>> import rapidjson
>>> rapidjson.__version__
'0.2.4'
>>> foo = {"foo": "\ud80d"}
>>> json.dumps(foo)
'{"foo": "\\ud80d"}'
>>> rapidjson.dumps(foo)
Segmentation fault (core dumped)Activitya-tal commented on Sep 29, 2017 loads also fails:
>>> foo = '{"foo": "\\ud80d"}'
>>> json.loads(foo)
{'foo': '\ud80d'}
>>> rapidjson.loads(foo)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: Parse error at offset 9: The surrogate pair in string is invalid.kenrobbins commented on Sep 29, 2017 Reacted by Sebastian Pipping lelit commented on Sep 29, 2017 Interesting:
>>> print("\ud80d".encode('utf-8'))
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
UnicodeEncodeError: 'utf-8' codec can't encode character '\ud80d' in position 0: surrogates not allowedlelit commented on Sep 29, 2017 Any idea of what should be the correct behaviour here? kenrobbins commented on Sep 29, 2017 What you have in the commit seems fine to me. If we want different behavior, seems like we'd have to customize RapidJSON, no? a-tal commented on Sep 29, 2017
Any idea of what should be the correct behaviour here?
it should fallback to ascii string....
as for print, try it with a file (or network stream), works fine:
>>> foo = {"foo": "my cool string with \ud80d in the middle of it"}
>>> with open("somefile.json", "w") as openjson:
... print(json.dumps(foo), file=openjson)
...
>>> with open("somefile.json", "r") as openjson:
... print(openjson.read(), end="")
...
{"foo": "my cool string with \ud80d in the middle of it"}
>>>lelit commented on Sep 30, 2017 I'd be curious to know why RapidJSON opted for not handling those codepoints. Does the JSON standard say anything about? lelit commented on Jan 13, 2018 Also yajl does something debatable:
>>> import yajl
>>> a={"foo": "\ud80d"}
>>> yajl.dumps(a)
'{"foo":"\\ud80d"}'
>>> yajl.loads(yajl.dumps(a))
{'foo': '?'}itamarst commented on Feb 19, 2019 So it seems like the actual coredump is fixed? Does this issue need to be open or can it be closed? lelit commented on Feb 20, 2019 Yes, the crash has been cured.
I left this open for the "fallback to ASCII" desired behavior. I'm surely not going to tweak RJ for that, though. hartwork commented on Feb 20, 2020
I'd be curious to know why RapidJSON opted for not handling those codepoints. Does the JSON standard say anything about?
ECMA-404 says:
Note that the JSON grammar permits code points for which Unicode does not currently provide character assignments.
So it's valid JSON with single surrogate characters in. The processor has two options in my view:
Option (a) might be helpful when doing low-level autopsies on JSON files but passing invalid characters along inside your application is a big problem, e.g. see encode/django-rest-framework#7026 for how this would cause "error 500" eventually during database access with the Django REST Framework until recently.
I gave a talk about this problem just a few days ago, the slides are up at https://github.com/hartwork/slides_python_json_emoji_crash_story/releases if you're interested.
Any idea of what should be the correct behaviour here?
Please keep detecting and rejecting single surrogates as invalid characters.
Besides improvements in speed, it's a core feature of python-rapidjson over stdlib json.
I left this open for the "fallback to ASCII" desired behavior. I'm surely not going to tweak RJ for that, though.
I don't see anything at #81 (comment) that I would consider "falling back to ASCII". I see escaping during JSON encoding and unescaping during JSON decoding. Could you elaborate? lelit commented on Feb 22, 2020 Thanks. Does this mean that in your opinion this issue should be considered as done?
I don't see anything [there] that I would consider "falling back to ASCII".
Uhm, this tricked me... in that comment, Adam answered to my "request for opinion" about the correct behaviour:
Any idea of what should be the correct behaviour here?
it should fallback to ascii string....
I think that means the surrogate unicode would be basically ignored, "left as is", not escaped-unescaped... or am I missing the point? hartwork commented on Feb 23, 2020
Thanks. Does this mean that in your opinion this issue should be considered as done?
I believe so, yes: I was able to reproduce the segfault with 0.2.4 but neither with 0.2.5 nor with 0.9.1 since they come with your fix 6f12323. hartwork commented on Feb 23, 2020 PS: For {"foo": chr(0xd80d)} function dumps has two options:
- A) escape U+D80D and write (literal)
{"foo": "\ud80d"} to file
- B) insist, that invalid characters are not to be written and raise
UnicodeEncodeError as-is:
# python -c 'import rapidjson; rapidjson.dumps({"foo": chr(0xd80d)})'
Traceback (most recent call last):
File "<string>", line 1, in <module>
UnicodeEncodeError: 'utf-8' codec can't encode character '\ud80d' in position 0: surrogates not allowed
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|