Skip to content

combine utf-16 surrogate pairs in cupsJSONImportString - #169

Open
tanjiroK-coder wants to merge 1 commit into
OpenPrinting:masterfrom
tanjiroK-coder:json-surrogate-pairs
Open

tanjiroK-coder wants to merge 1 commit into
OpenPrinting:masterfrom
tanjiroK-coder:json-surrogate-pairs

Conversation

@tanjiroK-coder

Copy link
Copy Markdown
Contributor

cupsJSONImportString decodes each \uXXXX escape on its own and never combines a UTF-16 surrogate pair, so "\uD834\uDD1E" (U+1D11E) decodes to two 3-byte CESU-8 sequences (ed a0 b4 ed b4 9e) instead of the 4-byte UTF-8 f0 9d 84 9e, and a lone surrogate leaves a raw ed a0 b4 in the value. The JSON here is untrusted: cupsJSONImportURL/cupsOAuthGetTokens feed it from an OAuth/OIDC endpoint and cupsJWTImportString runs it over token contents, so a hostile server can drop invalid UTF-8 into strings that later get compared, re-encoded or logged. I spotted it reading the \u branch after noticing dnssd.c already folds surrogate pairs correctly. The decoder now pairs a high surrogate (D800-DBFF) with the following low surrogate (DC00-DFFF) into one code point emitted as 4-byte UTF-8, and substitutes U+FFFD for an unpaired half rather than emitting a bare surrogate. The pre-scan already reserves five bytes per \uXXXX, so a combined pair uses four of the ten reserved bytes and the allocation is unchanged. testjson gets two cases that fail on the current code.

Assisted-by: Claude Code:claude-opus-4-8

@michaelrsweet michaelrsweet self-assigned this Sep 29, 2026
@michaelrsweet michaelrsweet added the enhancement New feature or request label Sep 29, 2026
@michaelrsweet michaelrsweet added this to the Stable milestone Sep 29, 2026

@michaelrsweet michaelrsweet left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really think a specific unit test for this is necessary.

Will look at the rest but I'm inclined to refactor the code a bit first.

@michaelrsweet

Copy link
Copy Markdown
Member

Also, I don't even think that surrogates are technically valid here - they are a UTF-16 encoding side-effect using a block of reserved Unicode code points while \uXXXX escapes a Unicode code point.

@tanjiroK-coder

Copy link
Copy Markdown
Contributor Author

Dropped the testjson cases and rebased on master since CHANGES.md had drifted into conflict. Fine with the shape changing in your refactor.

On the surrogates: RFC 8259 §7 defines \u in terms of UTF-16 code units rather than code points, and says a character outside the BMP is written as its surrogate pair, with 𝄞 for U+1D11E as the worked example. So a pair is the only way JSON can escape anything above U+FFFF. Agree a lone half isn't a character; §8.2 calls \uDEAD out as unpredictable behaviour, so whether that becomes U+FFFD or a parse error is your call.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants