Fix DirtyJson decoding \u surrogate pairs as unpaired surrogates#1774
Open
King0James0 wants to merge 1 commit into
Open
Fix DirtyJson decoding \u surrogate pairs as unpaired surrogates#1774King0James0 wants to merge 1 commit into
King0James0 wants to merge 1 commit into
Conversation
DirtyJson._parse_string decodes each \uXXXX escape independently, so an
emoji written in its legal JSON escape form (e.g. \ud83c\udf78 for
U+1F378) parses into two lone UTF-16 surrogate code points instead of
one character. Python strings tolerate lone surrogates, but UTF-8 does
not: the parsed string raises UnicodeEncodeError (surrogates not
allowed) at the first .encode("utf-8") downstream - observed live as
code_execution_tool crashing in tty_session.send ("Critical error
occurred, retrying..."), and the same poison crashes print_style HTML
log writes (agent0ai#1384).
Combine high+low surrogate escape pairs into the real character,
matching json.loads. Any remaining unpaired surrogate becomes U+FFFD so
the parsed string is always UTF-8-encodable. BMP escapes and invalid
hex handling are unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1773.
Fixes the
UnicodeEncodeError: surrogates not allowedcrash class described in #1773 (also the actual root cause of #1384).Problem
DirtyJson._parse_stringdecodes each\uXXXXescape independently, so a surrogate-pair escape like\ud83c\udf78(U+1F378, the legal JSON encoding any conforming serializer produces for non-BMP characters) parses into two lone UTF-16 surrogates. The string then crashes at the first.encode("utf-8")downstream — observed live incode_execution_tool(tty_session.send) as "Critical error occurred, retrying...", and the same poison hitsprint_stylelog writes (#1384). Every tool envelope goes through this parser viaextract_tools.json_parse_dirty, so any model that escape-encodes an emoji in a code/text arg trips it.Change
\uXXXXescape to a high surrogate (D800-DBFF), look ahead for an immediately following\uXXXXlow-surrogate escape and combine the pair into the real code point — the same behavior asjson.loads.json.loadspasses lone surrogates through; given every consumer here eventually encodes to UTF-8, emitting the replacement character instead of a guaranteed downstream crash seemed the right call for this parser — happy to matchjson.loadsexactly instead if preferred.)\u00e9,\u2713, ...) and the existing invalid-hex literal fallback are unchanged.Tests
Four new cases in
tests/test_dirty_json.py: pair combining (with an encode assertion on the exact crash site), lone high/low replacement, BMP escapes unchanged, and a high surrogate followed by plain text.python -m pytest tests/test_dirty_json.pypasses 11/11.