From 2c6a07ff902b087eb8a0eeb37dc2045146d5b391 Mon Sep 17 00:00:00 2001 From: Marc-Andre Giroux Date: Tue, 22 Sep 2026 10:33:04 -0400 Subject: [PATCH] Fix surrogate pairs mis-decoded when split across a read chunk boundary 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. --- src/main/java/graphql/parser/Parser.java | 46 +++++++++- .../SurrogatePairChunkBoundaryTest.groovy | 85 +++++++++++++++++++ 2 files changed, 127 insertions(+), 4 deletions(-) create mode 100644 src/test/groovy/graphql/parser/SurrogatePairChunkBoundaryTest.groovy diff --git a/src/main/java/graphql/parser/Parser.java b/src/main/java/graphql/parser/Parser.java index 86441d2b3..63b161c73 100644 --- a/src/main/java/graphql/parser/Parser.java +++ b/src/main/java/graphql/parser/Parser.java @@ -17,9 +17,10 @@ import graphql.parser.exceptions.ParseCancelledTooManyCharsException; import graphql.parser.exceptions.ParseCancelledTooManyNumericLiteralCharactersException; import org.antlr.v4.runtime.BaseErrorListener; -import org.antlr.v4.runtime.CharStreams; +import org.antlr.v4.runtime.CodePointBuffer; import org.antlr.v4.runtime.CodePointCharStream; import org.antlr.v4.runtime.CommonTokenStream; +import org.antlr.v4.runtime.IntStream; import org.antlr.v4.runtime.ParserRuleContext; import org.antlr.v4.runtime.RecognitionException; import org.antlr.v4.runtime.Recognizer; @@ -32,6 +33,7 @@ import java.io.IOException; import java.io.Reader; import java.io.UncheckedIOException; +import java.nio.CharBuffer; import java.util.List; import java.util.Optional; import java.util.function.BiConsumer; @@ -309,15 +311,51 @@ private static SafeTokenReader setupSafeTokenReader(ParserEnvironment environmen return new SafeTokenReader(multiSourceReader, maxCharacters, onTooManyCharacters); } + // matches the chunk size ANTLR's CharStreams.fromReader() uses internally + private static final int CHAR_STREAM_BUFFER_SIZE = 4096; + @NonNull private static CodePointCharStream setupCharStream(SafeTokenReader safeTokenReader) { - CodePointCharStream charStream; try { - charStream = CharStreams.fromReader(safeTokenReader); + return readCodePointCharStream(safeTokenReader, CHAR_STREAM_BUFFER_SIZE); } catch (IOException e) { throw new UncheckedIOException(e); } - return charStream; + } + + /** + * This mirrors {@link CharStreams#fromReader(Reader)}, except that a surrogate pair split across + * two chunk reads is held back and carried into the next chunk instead of being mis-decoded. + *

+ * ANTLR's {@link CodePointBuffer.Builder#append(CharBuffer)} keeps a pending high surrogate across + * calls, but its flush at the end of a chunk fails to clear that pending state, so a surrogate pair + * split across two {@code append()} calls gets decoded into two separate code points instead of one. + * Never handing it a chunk that ends on an unpaired high surrogate avoids the bug entirely. + * + * @see antlr/antlr4#4943 + */ + private static CodePointCharStream readCodePointCharStream(Reader reader, int bufferSize) throws IOException { + CodePointBuffer.Builder builder = CodePointBuffer.builder(bufferSize); + CharBuffer charBuffer = CharBuffer.allocate(bufferSize); + try { + while (reader.read(charBuffer) != -1) { + charBuffer.flip(); + int realLimit = charBuffer.limit(); + if (realLimit > charBuffer.position() && Character.isHighSurrogate(charBuffer.get(realLimit - 1))) { + charBuffer.limit(realLimit - 1); + } + builder.append(charBuffer); + charBuffer.limit(realLimit); + charBuffer.compact(); + } + charBuffer.flip(); + if (charBuffer.hasRemaining()) { + builder.append(charBuffer); + } + return CodePointCharStream.fromBuffer(builder.build(), IntStream.UNKNOWN_SOURCE_NAME); + } finally { + reader.close(); + } } @NonNull diff --git a/src/test/groovy/graphql/parser/SurrogatePairChunkBoundaryTest.groovy b/src/test/groovy/graphql/parser/SurrogatePairChunkBoundaryTest.groovy new file mode 100644 index 000000000..525192f62 --- /dev/null +++ b/src/test/groovy/graphql/parser/SurrogatePairChunkBoundaryTest.groovy @@ -0,0 +1,85 @@ +package graphql.parser + +import graphql.language.Argument +import graphql.language.Document +import graphql.language.Field +import graphql.language.ObjectTypeDefinition +import graphql.language.OperationDefinition +import graphql.language.SelectionSet +import graphql.language.StringValue +import graphql.schema.idl.SchemaParser +import spock.lang.Specification +import spock.lang.Unroll + +/** + * ANTLR's {@link org.antlr.v4.runtime.CharStreams#fromReader(java.io.Reader)} reads a document in fixed + * size chunks and hands each one to {@link org.antlr.v4.runtime.CodePointBuffer.Builder#append}. When a + * surrogate pair (e.g. an emoji outside the Basic Multilingual Plane) is split across two of those chunks, + * ANTLR mis-decodes it into two separate code points instead of one - see + * antlr/antlr4#4943. + *

+ * Parser.setupCharStream() avoids this by holding back a trailing unpaired high surrogate so it is carried + * into the next chunk. This test proves it by placing an astral character at every offset around the chunk + * boundary. + */ +class SurrogatePairChunkBoundaryTest extends Specification { + + // the emoji U+1F600 (😀), expressed as a Java surrogate pair + static final String EMOJI = "😀" + + static String queryWithEmojiAt(int highSurrogateIndex) { + def prefix = 'query { f(arg: "' + def pad = highSurrogateIndex - prefix.length() + assert pad >= 0 + return prefix + ('a' * pad) + EMOJI + '") }' + } + + @Unroll + def "an astral character surviving a document of length #docLength with the high surrogate at index #highSurrogateIndex"() { + given: + def query = queryWithEmojiAt(highSurrogateIndex) + assert query.indexOf(0xD83D as int) == highSurrogateIndex + + when: + Document viaString = Parser.parse(query) + Document viaReader = Parser.parse(ParserEnvironment.newParserEnvironment().document(query).build()) + + then: + [viaString, viaReader].each { Document document -> + def field = ((document.definitions[0] as OperationDefinition).selectionSet as SelectionSet).selections[0] as Field + def argument = field.arguments[0] as Argument + def value = argument.value as StringValue + assert value.value == ('a' * (highSurrogateIndex - 'query { f(arg: "'.length())) + "😀" + } + + where: + highSurrogateIndex << (4085..4100) + docLength = queryWithEmojiAt(highSurrogateIndex).length() + } + + def "a document ending in an unpaired high surrogate is not silently dropped"() { + given: + // deliberately malformed: a lone high surrogate with nothing after it, inside a string that never closes. + // this asserts today's (pre-existing) behaviour is preserved, not that new semantics are introduced. + def query = 'query { f(arg: "' + ('a' * 4080) + "\uD83D" + + when: + Parser.parse(query) + + then: + thrown(InvalidSyntaxException) + } + + def "an astral character in an SDL description survives a chunk boundary"() { + given: + def pad = 'a' * (4096 - '"""'.length()) + def sdl = '"""' + pad + EMOJI + '"""' + '\ntype Query { f: String }' + + when: + def registry = new SchemaParser().parse(sdl) + def objectType = registry.getType("Query", ObjectTypeDefinition).get() + + then: + objectType.description.content == pad + "😀" + } +}