Allow unescaped U+2028 and U+2029 in strings

[youdie006]

json5.loads rejects strings containing an unescaped U+2028 or U+2029, such as the output of JavaScript's JSON.stringify("a
b"), which json.loads accepts. The JSON5 spec lists both code points as valid string characters ("Like JSON, JSON5 allows the Unicode code points U+2028 and U+2029 to appear unescaped in strings"), and the reference json5 parser accepts them.

The string rules excluded every eol character; this adds the two code points as alternatives in json5.g and regenerates parser.py. The current glop doesn't emit the hand-added pos argument in Parser.__init__, so I kept that hunk as it was.

The tests cover both quote styles, a backslash before U+2028 still being a line continuation, and a raw CR still failing. Compared with json5 2.2.3 on json5-tests and about 33k generated strings, the only results that change are these cases, and all of them now match the reference. python run tests and python run check pass.

Written with AI assistance (Claude); I have reviewed the change.

[dpranke]

Interesting bug! I'm curious, how did you find this?

I think the fix looks correct, but this isn't the way I'd prefer it be written.

I'd argue that the problem is that the line in the grammar ~bslash ~squote ~eol anything:c is actually wrong (or at least misleading); it says that as long as something isn't a backslash, a single quote, or one of the EOL characters, it should be accepted, i.e., this is equivalent to ~(bslash | squote | eol) anything:c. And, while \u2028 is one of the EOL characters, it should be allowed in this context. So, the rule plus your fix is correct, but the underlying check really isn't; it's excluding too much and doesn't really reflect the structure of the grammar that well.

So, what you probably really want is

• not a backslash
• not a single quote
• not (strict==True and 0x00-0x1f)

and then the rule would be

sqchar = bslash esc_char:c                                          -> c
       | bslash eol                                                 -> ''
       | ~(bslash | squote | ?(_strict) '0x00'..'\x1f') anything:c  -> c

(and you'd change the dqchar rule the same way).

Although, from a performance point of view, given the current implementation, it'd probably be slightly faster to have the line be:

sqchar = ~(bslash | squote | ?(_strict) '0x00'..'\x1f') anything:c  -> c
       | bslash esc_char:c                                          -> c
       | bslash eol                                                 -> ''

Since the common case is actually that rule, and not a backslash followed by something.

From your comment, it sounds like you actually figured out how to use glop, too, which is pretty above-and-beyond, kudos.

Given that, do you want to try to rewrite the fix the way I'm suggesting? I haven't actually coded this myself, so I'm not 100% sure I'm right, and I'd want to code it and run the tests to see :).

Separately, in the tests, please don't embed \u2028 or other non-printable characters directly in the test file, use the Python unicode escapes instead. The reason is that you can't tell by just looking at the file that the character is \u2028 and not just \u0020. The difference is critically important to the test, so it should be visible and obvious.

[youdie006]

I found it by running pyjson5 and the reference json5 parser on the same generated strings and comparing the results; these two code points were the only string characters where they disagreed.

Rewritten your way in d82c1e4: the plain-character case comes first and only excludes the backslash, the quote and, in strict mode, a line break. One difference from your sketch: with ?(_strict) '\x00'..'\x1f', strict mode (the default) would start rejecting \t and the other control characters inside strings, which it accepts today. So I excluded only '\r' | '\n' in strict mode, which keeps the current behaviour. I checked that against the previous version of this fix on about 700k generated strings in both modes: no differences. If you'd like strict mode to reject the other control characters too, I can add that, but it seemed like a separate change.

The tests now use 
/
 escapes. run regen --check still reports the hand-edited pos argument in Parser.__init__, the same as on main.

[dpranke]

Ah, interesting, so that's actually another bug (sorta).

I wonder if unconditionally allowing \u0001 was intentional on my part, but poorly documented since the documentation implies IMO that for json compatibility, if strict=True, it should be disallowed), or if this was unintentional.

Regardless, after thinking about it, the former is probably the way to go; the code should probably never reject a legal JSON5 document.

I think your patch looks good. I'll probably follow it up with another commit to add more tests and comments, and to document that strict=True doesn't quite mean what it does w/ json.

[dpranke]

What are you using to generate test cases?

[youdie006]

Thanks for merging. The cases came from a small differential script: it builds JSON5 documents around each code point (inside strings, between tokens, as escapes) and feeds the same text to pyjson5 and to the reference JavaScript json5 package, then reports the inputs where one accepts and the other rejects or they decode differently. U+2028/U+2029 in strings were the only string characters where they disagreed. Agreed that strict=True never rejecting a valid JSON5 document is the more useful rule.