Fix #1683: reject JSON-escaped lone surrogates in ReaderBasedJsonParser

[elang2]

Fixes #1683.

Mirror of #1541 for ReaderBasedJsonParser. The \uXXXX escape decoder now rejects lone / reversed surrogates in field-names. Error messages match #1541's wording; helpers are private and per-parser to keep duplication low across the four sites.

Not gated behind a JsonReadFeature — matches #1542/#1583. If your #1494 note about wanting this class of validation feature-gated has become the preferred shape since, happy to rebase behind a new StreamReadFeature.

Scope: PR 1 of a series covering ReaderBasedJsonParser alone, and just for field names.
UTF8DataInputJsonParser (never received #1541), and the async parser (string-value) may follow as separate PRs matching the #1541 → #1567 → #1583 progression.

[Dongnyoung]

One clarification: ReaderBasedJsonParser can also receive raw UTF-16 surrogate code units directly from a Reader.

With this change, escaped pairs (\uD83D\uDE00) are validated, but mixed raw/escaped pairs are rejected, while fully raw pairs and raw lone surrogates are still accepted.

Is this intentional, with validation limited to JSON-escaped surrogates? Or would it make sense to validate surrogate pairs consistently regardless of whether each code unit is escaped or raw?

[elang2]

Good catch @Dongnyoung. Yes, intentional. This PR only checks what \uXXXX escapes produce.

"\uD800" + "\uDC00"    accepted    escaped pair
U+D800 + U+DC00        accepted    raw pair
"\uD800" + U+DC00      rejected    escaped + raw
U+D800 + "\uDC00"      rejected    raw + escaped
"\uD800"               rejected    escaped lone
U+D800                 accepted    raw lone

Raw lone surrogates are accepted, like you said.

The mixed cases are deliberate rather than incidental; the tests here assert both directions
reject. My thinking was that an escaped half next to a raw half is more likely a producer bug than
intent. Whether that's the right call is fair to question. It's a policy question first, and for
the raw + escaped direction it also needs a lookbehind the parser doesn't currently keep.

Rejecting raw lone surrogates is a different matter -- it would change behaviour for input that
parses today. I'll file that one as a separate issue so it's tracked and out of this PR's scope.

[Dongnyoung]

@elang2 Thanks, that makes sense. A separate issue sounds good — I think it would also help clarify the expected behavior for raw and mixed raw/escaped surrogate pairs.

[elang2]

@elang2 Thanks, that makes sense. A separate issue sounds good — I think it would also help clarify the expected behavior for raw and mixed raw/escaped surrogate pairs.

@Dongnyoung here you go. please review #1705

[Dongnyoung]

Thanks for opening #1705 and for looking into this! @elang2

[cowtowncoder]

Hi @elang2 ! This looks legit and I will review it.

One thing before I can merge it (beside the review): CLA, see:

https://github.com/FasterXML/jackson/blob/main/CONTRIBUTING.md#paperwork

(unless you already sent one)

[elang2]

Hi @elang2 ! This looks legit and I will review it.

One thing before I can merge it (beside the review): CLA, see:

https://github.com/FasterXML/jackson/blob/main/CONTRIBUTING.md#paperwork

(unless you already sent one)

Hi @cowtowncoder , sent one now.

[cowtowncoder]

I trimmed the PR to drop String value validation and just keep field name validation. I am also bit torn on actual value of this validation altogether; but let's complete field name validation across backends.

[pjfanning]

Minor: release-notes/CREDITS-2.x says this rejects lone surrogates "in field names and string values", but 5ef043a removed the string-value check (and escapedSurrogatesInStringValuePassThrough confirms ["\uD800"] still parses). The credit should say field names only.