Skip to content

Fix surrogate pairs mis-decoded when split across a read chunk boundary - #4478

Open
xuorig wants to merge 1 commit into
graphql-java:masterfrom
xuorig:fix/parser-surrogate-pair-chunk-boundary
Open

xuorig wants to merge 1 commit into
graphql-java:masterfrom
xuorig:fix/parser-surrogate-pair-chunk-boundary

Conversation

@xuorig

@xuorig xuorig commented Sep 22, 2026

Copy link
Copy Markdown

Hey folks, I discovered (actually re-discovered it seems!) a rare but quite tricky issue with ANTLR while I was working on a PR to allow configuring the reader buffer size.

Somebody actually opened a PR for this recently on ANTLR: antlr/antlr4#4943 but I have no idea if it will get merged and/or when.

It's little work to take control of that read loop if we wanted to, and might actually help with allowing to configure that read buffer size (Will follow up with another PR)

What do you think? This actually explained a really weird issue we saw with a schema update months ago and could never reproduce. Basically a surrogate pair placed at exactly the wrong place (split by buffer size) would cause parsing errors due to a dangling lone surrogate.

cc @rootvector2 & @GeorgEchterling who seem to have faced this issue as well.

Parser.setupCharStream() used ANTLR's CharStreams.fromReader(), which reads
the document in fixed 4096-char chunks. CodePointBuffer.Builder keeps a
pending high surrogate across chunks to support pairs split by a chunk
boundary, but its end-of-chunk flush fails to clear that pending state,
so the surrogate gets decoded twice: once as a lone code point at the
chunk boundary, and again when paired with the next chunk's low surrogate.

In practice this makes graphql-java reject an otherwise valid document
whenever an astral character (e.g. an emoji) lands with its high surrogate
at index 4095, 8191, etc., since the resulting lone surrogate fails
GraphQL's StringCharacter grammar rule.

This is a known upstream defect (antlr/antlr4#4943, open and unreviewed).
Rather than wait on it, this replaces fromReader() with an equivalent local
read loop that holds back a chunk-trailing unpaired high surrogate so it is
carried into the next chunk instead of being flushed prematurely.
*
* @see <a href="https://github.com/antlr/antlr4/pull/4943">antlr/antlr4#4943</a>
*/
private static CodePointCharStream readCodePointCharStream(Reader reader, int bufferSize) throws IOException {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@github-actions

Copy link
Copy Markdown
Contributor

Test Report

Test Results

Java Version Total Passed Failed Errors Skipped
Java 11 5958 (+18 🟢) 5902 (+18 🟢) 0 (±0) 0 (±0) 56 (±0)
Java 17 5958 (+18 🟢) 5901 (+18 🟢) 0 (±0) 0 (±0) 57 (±0)
Java 21 5958 (+18 🟢) 5901 (+18 🟢) 0 (±0) 0 (±0) 57 (±0)
Java 25 5958 (+18 🟢) 5901 (+18 🟢) 0 (±0) 0 (±0) 57 (±0)
jcstress 32 (±0) 32 (±0) 0 (±0) 0 (±0) 0 (±0)
Total 23864 (+72 🟢) 23637 (+72 🟢) 0 (±0) 0 (±0) 227 (±0)

Code Coverage (Java 25)

Metric Covered Missed Coverage vs Master
Lines 30084 3118 90.6% ±0.0%
Branches 8795 1519 85.3% ±0.0%
Methods 8035 1205 87.0% ±0.0%

Changed Class Coverage (2 classes)

Class Line Branch Method
g.p.Parser +0.1% 🟢 +4.4% 🟢 ±0.0%
g.s.i.TypeDefinitionRegistry +0.3% 🟢 ±0.0% +1.3% 🟢

Full HTML report: build artifact jacoco-html-report

Updated: 2026-09-22 14:58:45 UTC

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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant