Repository navigation
quote identifier keys above U+FFFF when ensure_ascii is true - #128
Conversation
With ensure_ascii true (the default), dumps() writes a non-ASCII
identifier key unquoted, with \u escapes. A character above U+FFFF
becomes a surrogate pair of two escapes, so dumps({'\U0001d49c': 1})
returned {\ud835\udc9c: 1}.
Each half of the pair is a lone surrogate, which is not an identifier
character. The json5 npm package (2.2.3) rejects that text with
"invalid identifier character", and json-five (1.1.2) rejects it too.
pyjson5's own reader accepts it, so loads() did not show the problem.
Write such a key as a quoted string when ensure_ascii is true, as
quote_keys=True already does. Keys that are all in the BMP are
unchanged, and with ensure_ascii false the character is still written
unquoted.
The new test checks a key that starts with the astral character, one
with it between ASCII letters, and the first code point above the BMP,
plus the ensure_ascii=False output and a round trip through loads().
|
Thanks for the patch! This sounds plausible, but I'll try to take a closer look at this tomorrow (sorry for the delay). |
|
Okay, I've looked at this and I think it's probably fine, but I have a question first: IIUC, in current versions of JavaScript, you can specify a supplemental (not-BMP) character with And Is that all correct (as far as you know)? If so, I think this patch is fine, but the comment is slightly misleading. Instead of saying "Other readers reject a surrogate pair", wouldn't it be better to say that they are not allowed? |
|
Yes, that matches what I find:
I changed the comment to say it is not allowed (976c14e). |
|
Wait, I'm not sure what you mean by " I can load that just fine in current Node (v26) (as a straight expression) and current JSON5.parse. That should work, shouldn't it? |
|
Also, I'm curious: how did you even find this bug, given that I haven't released a version that has it yet? Are you actually working off of HEAD in the repo (and, if so, why)? It's fine if you are, I just don't typically expect people to be doing so. |
|
Sorry, that was my mistake: the backslashes fell out of my reply. That line was about the escaped form from your comment, Yes, I was working from HEAD. I review main rather than the last release, so a bug can be fixed before it ships. I found this one by parsing |
With the default ensure_ascii=True,
json5.dumps({'\U0001d49c': 1})returns{\ud835\udc9c: 1}. The unquoted-key branch from #124 escapes every non-ascii character of an identifier key, and a character above U+FFFF becomes a surrogate pair of two\uescapes. Each half is a lone surrogate, which is not an identifier character, so the text is not valid JSON5. The json5 npm package (2.2.3) rejects it with "invalid identifier character at 1:2", and json-five (1.1.2) rejects it too. pyjson5's own reader accepts it, soloads(dumps(x))does not show the problem.Write such a key as a quoted string when ensure_ascii is true, the way
quote_keys=Truedoes. Keys inside the BMP stay unquoted, and withensure_ascii=Falsethe character is still written unquoted. The unquoted-key branch:pyjson5/json5/lib.py
Lines 709 to 719 in 2dfeb36
test_ensure_ascii_astral_keyscovers a key that starts with the character, one with it between ASCII letters, and U+10000, plus theensure_ascii=Falseoutput.