From e51a54eff9bb64cae28699b4254dfd2f85eb4065 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Wed, 19 Aug 2026 08:16:37 +1000 Subject: [PATCH 1/4] Limit numeric literal length during parsing (cherry picked from commit 29f90b55cb33f8b146a3244754316bf005a7ad8f) --- src/main/java/graphql/parser/Parser.java | 15 ++++- .../java/graphql/parser/ParserOptions.java | 41 +++++++++++++ .../java/graphql/parser/SafeTokenSource.java | 22 ++++++- ...ManyNumericLiteralCharactersException.java | 17 ++++++ src/main/resources/i18n/Parsing.properties | 1 + src/main/resources/i18n/Parsing_de.properties | 1 + src/main/resources/i18n/Parsing_nl.properties | 1 + .../graphql/parser/ParserOptionsTest.groovy | 7 +++ .../graphql/parser/ParserStressTest.groovy | 61 +++++++++++++++++++ .../graphql/parser/SafeTokenSourceTest.groovy | 33 +++++++++- 10 files changed, 194 insertions(+), 5 deletions(-) create mode 100644 src/main/java/graphql/parser/exceptions/ParseCancelledTooManyNumericLiteralCharactersException.java diff --git a/src/main/java/graphql/parser/Parser.java b/src/main/java/graphql/parser/Parser.java index c2015f274f..86441d2b33 100644 --- a/src/main/java/graphql/parser/Parser.java +++ b/src/main/java/graphql/parser/Parser.java @@ -15,6 +15,7 @@ import graphql.parser.exceptions.ParseCancelledException; import graphql.parser.exceptions.ParseCancelledTooDeepException; 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.CodePointCharStream; @@ -348,13 +349,25 @@ public void syntaxError(Recognizer recognizer, Object offendingSymbol, int private SafeTokenSource getSafeTokenSource(ParserEnvironment environment, ParserOptions parserOptions, MultiSourceReader multiSourceReader, GraphqlLexer lexer) { int maxTokens = parserOptions.getMaxTokens(); int maxWhitespaceTokens = parserOptions.getMaxWhitespaceTokens(); + int maxNumericLiteralCharacters = parserOptions.getMaxNumericLiteralCharacters(); BiConsumer onTooManyTokens = (maxTokenCount, token) -> throwIfTokenProblems( environment, token, maxTokenCount, multiSourceReader, ParseCancelledException.class); - return new SafeTokenSource(lexer, maxTokens, maxWhitespaceTokens, onTooManyTokens); + BiConsumer onTooManyNumericLiteralCharacters = (maxCharacters, token) -> { + SourceLocation sourceLocation = AntlrHelper.createSourceLocation(multiSourceReader, token); + throw new ParseCancelledTooManyNumericLiteralCharactersException(environment.getI18N(), sourceLocation, maxCharacters); + }; + return new SafeTokenSource( + lexer, + maxTokens, + maxWhitespaceTokens, + maxNumericLiteralCharacters, + onTooManyTokens, + onTooManyNumericLiteralCharacters + ); } private void setupParserListener(ParserEnvironment environment, MultiSourceReader multiSourceReader, GraphqlParser parser, GraphqlAntlrToLanguage toLanguage) { diff --git a/src/main/java/graphql/parser/ParserOptions.java b/src/main/java/graphql/parser/ParserOptions.java index 2e04294427..4243136741 100644 --- a/src/main/java/graphql/parser/ParserOptions.java +++ b/src/main/java/graphql/parser/ParserOptions.java @@ -43,6 +43,16 @@ public class ParserOptions { */ public static final int MAX_WHITESPACE_TOKENS = 200_000; + /** + * A numeric literal is represented by a single token, regardless of how many characters it contains. Converting + * very large numeric literals into arbitrary precision numbers can consume excessive CPU and memory. To prevent + * this for most users, graphql-java limits numeric literals to 100 characters. + *

+ * If you want to allow more, then {@link #setDefaultParserOptions(ParserOptions)} allows you to change this + * JVM wide. + */ + public static final int MAX_NUMERIC_LITERAL_CHARACTERS = 100; + /** * A graphql hacking vector is to send nonsensical queries that have lots of grammar rule depth to them which * can cause stack overflow exceptions during the query parsing. To prevent this for most users, graphql-java @@ -61,6 +71,7 @@ public class ParserOptions { .maxCharacters(MAX_QUERY_CHARACTERS) .maxTokens(MAX_QUERY_TOKENS) // to prevent a billion laughs style attacks, we set a default for graphql-java .maxWhitespaceTokens(MAX_WHITESPACE_TOKENS) + .maxNumericLiteralCharacters(MAX_NUMERIC_LITERAL_CHARACTERS) .maxRuleDepth(MAX_RULE_DEPTH) .redactTokenParserErrorMessages(false) .build(); @@ -73,6 +84,7 @@ public class ParserOptions { .maxCharacters(MAX_QUERY_CHARACTERS) .maxTokens(MAX_QUERY_TOKENS) // to prevent a billion laughs style attacks, we set a default for graphql-java .maxWhitespaceTokens(MAX_WHITESPACE_TOKENS) + .maxNumericLiteralCharacters(MAX_NUMERIC_LITERAL_CHARACTERS) .maxRuleDepth(MAX_RULE_DEPTH) .redactTokenParserErrorMessages(false) .build(); @@ -85,6 +97,7 @@ public class ParserOptions { .maxCharacters(Integer.MAX_VALUE) .maxTokens(Integer.MAX_VALUE) // we are less worried about a billion laughs with SDL parsing since the call path is not facing attackers .maxWhitespaceTokens(Integer.MAX_VALUE) + .maxNumericLiteralCharacters(MAX_NUMERIC_LITERAL_CHARACTERS) .maxRuleDepth(Integer.MAX_VALUE) .redactTokenParserErrorMessages(false) .build(); @@ -191,6 +204,7 @@ public static void setDefaultSdlParserOptions(ParserOptions options) { private final int maxCharacters; private final int maxTokens; private final int maxWhitespaceTokens; + private final int maxNumericLiteralCharacters; private final int maxRuleDepth; private final boolean redactTokenParserErrorMessages; private final ParsingListener parsingListener; @@ -203,6 +217,7 @@ private ParserOptions(Builder builder) { this.maxCharacters = builder.maxCharacters; this.maxTokens = builder.maxTokens; this.maxWhitespaceTokens = builder.maxWhitespaceTokens; + this.maxNumericLiteralCharacters = builder.maxNumericLiteralCharacters; this.maxRuleDepth = builder.maxRuleDepth; this.redactTokenParserErrorMessages = builder.redactTokenParserErrorMessages; this.parsingListener = builder.parsingListener; @@ -288,6 +303,17 @@ public int getMaxWhitespaceTokens() { return maxWhitespaceTokens; } + /** + * A numeric literal is represented by a single token, regardless of how many characters it contains. Converting + * very large numeric literals into arbitrary precision numbers can consume excessive CPU and memory. This limit + * stops parsing before that conversion takes place. + * + * @return the maximum number of characters permitted in an integer or floating-point literal + */ + public int getMaxNumericLiteralCharacters() { + return maxNumericLiteralCharacters; + } + /** * A graphql hacking vector is to send nonsensical queries that have lots of rule depth to them which * can cause stack overflow exceptions during the query parsing. To prevent this you can set a value @@ -333,6 +359,7 @@ public static class Builder { private int maxCharacters = MAX_QUERY_CHARACTERS; private int maxTokens = MAX_QUERY_TOKENS; private int maxWhitespaceTokens = MAX_WHITESPACE_TOKENS; + private int maxNumericLiteralCharacters = MAX_NUMERIC_LITERAL_CHARACTERS; private int maxRuleDepth = MAX_RULE_DEPTH; private boolean redactTokenParserErrorMessages = false; @@ -346,6 +373,7 @@ public static class Builder { this.maxCharacters = parserOptions.maxCharacters; this.maxTokens = parserOptions.maxTokens; this.maxWhitespaceTokens = parserOptions.maxWhitespaceTokens; + this.maxNumericLiteralCharacters = parserOptions.maxNumericLiteralCharacters; this.maxRuleDepth = parserOptions.maxRuleDepth; this.redactTokenParserErrorMessages = parserOptions.redactTokenParserErrorMessages; this.parsingListener = parserOptions.parsingListener; @@ -386,6 +414,19 @@ public Builder maxWhitespaceTokens(int maxWhitespaceTokens) { return this; } + /** + * Sets the maximum number of characters permitted in an integer or floating-point literal. Parsing is + * cancelled before converting a larger literal into an arbitrary precision number. + * + * @param maxNumericLiteralCharacters the maximum number of characters permitted in a numeric literal + * + * @return this builder + */ + public Builder maxNumericLiteralCharacters(int maxNumericLiteralCharacters) { + this.maxNumericLiteralCharacters = maxNumericLiteralCharacters; + return this; + } + public Builder maxRuleDepth(int maxRuleDepth) { this.maxRuleDepth = maxRuleDepth; return this; diff --git a/src/main/java/graphql/parser/SafeTokenSource.java b/src/main/java/graphql/parser/SafeTokenSource.java index c92c76d916..c6f2e50ae4 100644 --- a/src/main/java/graphql/parser/SafeTokenSource.java +++ b/src/main/java/graphql/parser/SafeTokenSource.java @@ -1,6 +1,7 @@ package graphql.parser; import graphql.Internal; +import graphql.parser.antlr.GraphqlLexer; import org.antlr.v4.runtime.CharStream; import org.antlr.v4.runtime.Token; import org.antlr.v4.runtime.TokenFactory; @@ -25,14 +26,20 @@ public class SafeTokenSource implements TokenSource { private final TokenSource lexer; private final int maxTokens; private final int maxWhitespaceTokens; + private final int maxNumericLiteralCharacters; private final BiConsumer whenMaxTokensExceeded; + private final BiConsumer whenMaxNumericLiteralCharactersExceeded; private final int channelCounts[]; - public SafeTokenSource(TokenSource lexer, int maxTokens, int maxWhitespaceTokens, BiConsumer whenMaxTokensExceeded) { + public SafeTokenSource(TokenSource lexer, int maxTokens, int maxWhitespaceTokens, int maxNumericLiteralCharacters, + BiConsumer whenMaxTokensExceeded, + BiConsumer whenMaxNumericLiteralCharactersExceeded) { this.lexer = lexer; this.maxTokens = maxTokens; this.maxWhitespaceTokens = maxWhitespaceTokens; + this.maxNumericLiteralCharacters = maxNumericLiteralCharacters; this.whenMaxTokensExceeded = whenMaxTokensExceeded; + this.whenMaxNumericLiteralCharactersExceeded = whenMaxNumericLiteralCharactersExceeded; // this could be a Map however we want it to be faster as possible. // we only have 3 channels - but they are 0,2 and 3 so use 5 for safety - still faster than a map get/put // if we ever add another channel beyond 5 it will IOBEx during tests so future changes will be handled before release! @@ -44,6 +51,7 @@ public SafeTokenSource(TokenSource lexer, int maxTokens, int maxWhitespaceTokens public Token nextToken() { Token token = lexer.nextToken(); if (token != null) { + callbackIfNumericLiteralTooLong(token); int channel = token.getChannel(); int currentCount = ++channelCounts[channel]; if (channel == Parser.CHANNEL_WHITESPACE) { @@ -56,6 +64,18 @@ public Token nextToken() { return token; } + private void callbackIfNumericLiteralTooLong(Token token) { + int tokenType = token.getType(); + if (tokenType != GraphqlLexer.IntValue && tokenType != GraphqlLexer.FloatValue) { + return; + } + + int characterCount = token.getStopIndex() - token.getStartIndex() + 1; + if (characterCount > maxNumericLiteralCharacters) { + whenMaxNumericLiteralCharactersExceeded.accept(maxNumericLiteralCharacters, token); + } + } + private void callbackIfMaxExceeded(int maxCount, int currentCount, Token token) { if (currentCount > maxCount) { whenMaxTokensExceeded.accept(maxCount, token); diff --git a/src/main/java/graphql/parser/exceptions/ParseCancelledTooManyNumericLiteralCharactersException.java b/src/main/java/graphql/parser/exceptions/ParseCancelledTooManyNumericLiteralCharactersException.java new file mode 100644 index 0000000000..83a60e4027 --- /dev/null +++ b/src/main/java/graphql/parser/exceptions/ParseCancelledTooManyNumericLiteralCharactersException.java @@ -0,0 +1,17 @@ +package graphql.parser.exceptions; + +import graphql.Internal; +import graphql.i18n.I18n; +import graphql.language.SourceLocation; +import graphql.parser.InvalidSyntaxException; +import org.jspecify.annotations.NonNull; + +@Internal +public class ParseCancelledTooManyNumericLiteralCharactersException extends InvalidSyntaxException { + + @Internal + public ParseCancelledTooManyNumericLiteralCharactersException(@NonNull I18n i18N, @NonNull SourceLocation sourceLocation, int maxCharacters) { + super(i18N.msg("ParseCancelled.tooManyNumericLiteralCharacters", maxCharacters), + sourceLocation, null, null, null); + } +} diff --git a/src/main/resources/i18n/Parsing.properties b/src/main/resources/i18n/Parsing.properties index 474fba1545..20301dd471 100644 --- a/src/main/resources/i18n/Parsing.properties +++ b/src/main/resources/i18n/Parsing.properties @@ -22,6 +22,7 @@ InvalidSyntaxMoreTokens.full=Invalid syntax encountered. There are extra tokens ParseCancelled.full=More than {0} ''{1}'' tokens have been presented. To prevent Denial Of Service attacks, parsing has been cancelled. ParseCancelled.tooDeep=More than {0} deep ''{1}'' rules have been entered. To prevent Denial Of Service attacks, parsing has been cancelled. ParseCancelled.tooManyChars=More than {0} characters have been presented. To prevent Denial Of Service attacks, parsing has been cancelled. +ParseCancelled.tooManyNumericLiteralCharacters=A numeric literal with more than {0} characters has been presented. To prevent Denial Of Service attacks, parsing has been cancelled. # InvalidUnicode.trailingLeadingSurrogate=Invalid unicode encountered. Trailing surrogate must be preceded with a leading surrogate. Offending token ''{0}'' at line {1} column {2} InvalidUnicode.leadingTrailingSurrogate=Invalid unicode encountered. Leading surrogate must be followed by a trailing surrogate. Offending token ''{0}'' at line {1} column {2} diff --git a/src/main/resources/i18n/Parsing_de.properties b/src/main/resources/i18n/Parsing_de.properties index 55127ff689..41247c67f2 100644 --- a/src/main/resources/i18n/Parsing_de.properties +++ b/src/main/resources/i18n/Parsing_de.properties @@ -22,6 +22,7 @@ InvalidSyntaxMoreTokens.full=Es wurde eine ungültige Syntax festgestellt. Es gi ParseCancelled.full=Es wurden mehr als {0} ''{1}'' Token präsentiert. Um Denial-of-Service-Angriffe zu verhindern, wurde das Parsing abgebrochen. ParseCancelled.tooDeep=Es wurden mehr als {0} tief ''{1}'' Regeln ausgeführt. Um Denial-of-Service-Angriffe zu verhindern, wurde das Parsing abgebrochen. ParseCancelled.tooManyChars=Es wurden mehr als {0} Zeichen vorgelegt. Um Denial-of-Service-Angriffe zu verhindern, wurde das Parsing abgebrochen. +ParseCancelled.tooManyNumericLiteralCharacters=Es wurde ein numerisches Literal mit mehr als {0} Zeichen vorgelegt. Um Denial-of-Service-Angriffe zu verhindern, wurde das Parsing abgebrochen. # InvalidUnicode.trailingLeadingSurrogate=Ungültiger Unicode gefunden. Trailing surrogate muss ein leading surrogate vorangestellt werden. Ungültiges Token ''{0}'' in Zeile {1} Spalte {2} InvalidUnicode.leadingTrailingSurrogate=Ungültiger Unicode gefunden. Auf ein leading surrogate muss ein trailing surrogate folgen. Ungültiges Token ''{0}'' in Zeile {1} Spalte {2} diff --git a/src/main/resources/i18n/Parsing_nl.properties b/src/main/resources/i18n/Parsing_nl.properties index cfd8457825..770397d98d 100644 --- a/src/main/resources/i18n/Parsing_nl.properties +++ b/src/main/resources/i18n/Parsing_nl.properties @@ -21,6 +21,7 @@ InvalidSyntaxMoreTokens.full=Ongeldige syntaxis tegengekomen. Er zijn tokens in ParseCancelled.full=Meer dan {0} ''{1}'' tokens zijn gepresenteerd. Om een DDoS-aanval te voorkomen is het parsen gestopt. ParseCancelled.tooDeep=Meer dan {0} diep, ''{1}'' regels zijn uitgevoerd. Om een DDoS-aanval te voorkomen is het parsen gestopt. ParseCancelled.tooManyChars=Meer dan {0} tekens zijn voorgelegd. Om een DDoS-aanval te voorkomen is het parsen gestopt. +ParseCancelled.tooManyNumericLiteralCharacters=Er is een numerieke literal met meer dan {0} tekens aangeboden. Om een DDoS-aanval te voorkomen is het parsen gestopt. # InvalidUnicode.trailingLeadingSurrogate=Ongeldige Unicode tegengekomen. Trailing surrogate moet vooropgaan aan een leading surrogate. Ongeldige token ''{0}'' op lijn {1} kolom {2} InvalidUnicode.leadingTrailingSurrogate=Ongeldige Unicode tegengekomen. Leading surrogate moet voorafgaan aan een trailing surrogate. Ongeldige token ''{0}'' op lijn {1} kolom {2} diff --git a/src/test/groovy/graphql/parser/ParserOptionsTest.groovy b/src/test/groovy/graphql/parser/ParserOptionsTest.groovy index 9d43543f89..a77f53b782 100644 --- a/src/test/groovy/graphql/parser/ParserOptionsTest.groovy +++ b/src/test/groovy/graphql/parser/ParserOptionsTest.groovy @@ -26,6 +26,7 @@ class ParserOptionsTest extends Specification { defaultOptions.getMaxCharacters() == ONE_MB defaultOptions.getMaxTokens() == 15_000 defaultOptions.getMaxWhitespaceTokens() == 200_000 + defaultOptions.getMaxNumericLiteralCharacters() == 100 defaultOptions.isCaptureSourceLocation() defaultOptions.isCaptureLineComments() !defaultOptions.isCaptureIgnoredChars() @@ -34,6 +35,7 @@ class ParserOptionsTest extends Specification { defaultOperationOptions.getMaxTokens() == 15_000 defaultOperationOptions.getMaxWhitespaceTokens() == 200_000 + defaultOperationOptions.getMaxNumericLiteralCharacters() == 100 defaultOperationOptions.isCaptureSourceLocation() !defaultOperationOptions.isCaptureLineComments() !defaultOperationOptions.isCaptureIgnoredChars() @@ -43,6 +45,7 @@ class ParserOptionsTest extends Specification { defaultSdlOptions.getMaxCharacters() == Integer.MAX_VALUE defaultSdlOptions.getMaxTokens() == Integer.MAX_VALUE defaultSdlOptions.getMaxWhitespaceTokens() == Integer.MAX_VALUE + defaultSdlOptions.getMaxNumericLiteralCharacters() == 100 defaultSdlOptions.isCaptureSourceLocation() defaultSdlOptions.isCaptureLineComments() !defaultSdlOptions.isCaptureIgnoredChars() @@ -61,6 +64,7 @@ class ParserOptionsTest extends Specification { it.captureIgnoredChars(true) .captureLineComments(true) .maxCharacters(1_000_000) + .maxNumericLiteralCharacters(200) .maxWhitespaceTokens(300_000) }) def newDefaultSDlOptions = defaultSdlOptions.transform( @@ -84,6 +88,7 @@ class ParserOptionsTest extends Specification { currentDefaultOptions.getMaxCharacters() == ONE_MB currentDefaultOptions.getMaxTokens() == 15_000 currentDefaultOptions.getMaxWhitespaceTokens() == 200_000 + currentDefaultOptions.getMaxNumericLiteralCharacters() == 100 currentDefaultOptions.isCaptureSourceLocation() currentDefaultOptions.isCaptureLineComments() currentDefaultOptions.isCaptureIgnoredChars() @@ -93,6 +98,7 @@ class ParserOptionsTest extends Specification { currentDefaultOperationOptions.getMaxCharacters() == 1_000_000 currentDefaultOperationOptions.getMaxTokens() == 15_000 currentDefaultOperationOptions.getMaxWhitespaceTokens() == 300_000 + currentDefaultOperationOptions.getMaxNumericLiteralCharacters() == 200 currentDefaultOperationOptions.isCaptureSourceLocation() currentDefaultOperationOptions.isCaptureLineComments() currentDefaultOperationOptions.isCaptureIgnoredChars() @@ -102,6 +108,7 @@ class ParserOptionsTest extends Specification { currentDefaultSdlOptions.getMaxCharacters() == Integer.MAX_VALUE currentDefaultSdlOptions.getMaxTokens() == Integer.MAX_VALUE currentDefaultSdlOptions.getMaxWhitespaceTokens() == 300_000 + currentDefaultSdlOptions.getMaxNumericLiteralCharacters() == 100 currentDefaultSdlOptions.isCaptureSourceLocation() currentDefaultSdlOptions.isCaptureLineComments() currentDefaultSdlOptions.isCaptureIgnoredChars() diff --git a/src/test/groovy/graphql/parser/ParserStressTest.groovy b/src/test/groovy/graphql/parser/ParserStressTest.groovy index c058d56a2f..77bc237a04 100644 --- a/src/test/groovy/graphql/parser/ParserStressTest.groovy +++ b/src/test/groovy/graphql/parser/ParserStressTest.groovy @@ -6,6 +6,7 @@ import graphql.language.Document import graphql.parser.exceptions.ParseCancelledException import graphql.parser.exceptions.ParseCancelledTooDeepException import graphql.parser.exceptions.ParseCancelledTooManyCharsException +import graphql.parser.exceptions.ParseCancelledTooManyNumericLiteralCharactersException import spock.lang.Specification import static graphql.parser.ParserEnvironment.newParserEnvironment @@ -184,6 +185,66 @@ class ParserStressTest extends Specification { document != null // its parsed - its invalid of course but parsed } + def "numeric literals longer than the default limit are prevented"() { + given: + def text = "query { f(arg: ${numericLiteral}) }" + def parserEnvironment = newParserEnvironment().document(text).parserOptions(defaultOperationOptions).build() + + when: + Parser.parse(parserEnvironment) + + then: + def exception = thrown(ParseCancelledTooManyNumericLiteralCharactersException) + exception.message.contains("more than 100 characters") + exception.offendingToken == null + + where: + numericLiteral << ["9" * 101, "9" * 99 + ".0"] + } + + def "numeric literals at the default limit are accepted"() { + given: + def text = "query { f(arg: ${numericLiteral}) }" + def parserEnvironment = newParserEnvironment().document(text).parserOptions(defaultOperationOptions).build() + + when: + def document = Parser.parse(parserEnvironment) + + then: + document != null + + where: + numericLiteral << ["9" * 100, "9" * 98 + ".0"] + } + + def "maximum numeric literal characters can be overridden"() { + given: + def parserOptions = defaultOperationOptions.transform { + it.maxNumericLiteralCharacters(101) + } + def acceptedText = "query { f(arg: ${acceptedLiteral}) }" + def acceptedEnvironment = newParserEnvironment().document(acceptedText).parserOptions(parserOptions).build() + + when: + def document = Parser.parse(acceptedEnvironment) + + then: + document != null + + when: + def rejectedText = "query { f(arg: ${rejectedLiteral}) }" + def rejectedEnvironment = newParserEnvironment().document(rejectedText).parserOptions(parserOptions).build() + Parser.parse(rejectedEnvironment) + + then: + thrown(ParseCancelledTooManyNumericLiteralCharactersException) + + where: + acceptedLiteral | rejectedLiteral + "9" * 101 | "9" * 102 + "9" * 99 + ".0" | "9" * 100 + ".0" + } + String mkDeepQuery(int howMany) { def field = 'f(a:"")' StringBuilder sb = new StringBuilder("query q{") diff --git a/src/test/groovy/graphql/parser/SafeTokenSourceTest.groovy b/src/test/groovy/graphql/parser/SafeTokenSourceTest.groovy index cf8b34658e..25b31a0498 100644 --- a/src/test/groovy/graphql/parser/SafeTokenSourceTest.groovy +++ b/src/test/groovy/graphql/parser/SafeTokenSourceTest.groovy @@ -34,7 +34,7 @@ class SafeTokenSourceTest extends Specification { offendingToken = token throw new IllegalStateException("stop at $max") } - def tokenSource = new SafeTokenSource(graphqlLexer, 50, 1000, onTooManyTokens) + def tokenSource = new SafeTokenSource(graphqlLexer, 50, 1000, Integer.MAX_VALUE, onTooManyTokens, { max, token -> }) consumeAllTokens(tokenSource) assert false, "This is not meant to actually consume all tokens" @@ -59,7 +59,7 @@ class SafeTokenSourceTest extends Specification { offendingToken = token throw new IllegalStateException("stop at $max") } - def tokenSource = new SafeTokenSource(graphqlLexer, 1000, 200_000, onTooManyTokens) + def tokenSource = new SafeTokenSource(graphqlLexer, 1000, 200_000, Integer.MAX_VALUE, onTooManyTokens, { max, token -> }) consumeAllTokens(tokenSource) assert false, "This is not meant to actually consume all tokens" @@ -82,7 +82,7 @@ class SafeTokenSourceTest extends Specification { offendingToken = token throw new IllegalStateException("stop at $max") } - def tokenSource = new SafeTokenSource(graphqlLexer, 1000, 200_000, onTooManyTokens) + def tokenSource = new SafeTokenSource(graphqlLexer, 1000, 200_000, Integer.MAX_VALUE, onTooManyTokens, { max, token -> }) consumeAllTokens(tokenSource) @@ -91,4 +91,31 @@ class SafeTokenSourceTest extends Specification { offendingToken == null } + def "can call back before returning an oversized numeric literal"() { + given: + GraphqlLexer graphqlLexer = lexer("query { field(arg: ${"9" * 101}) }") + Token offendingToken = null + BiConsumer onTooManyNumericLiteralCharacters = { max, token -> + offendingToken = token + throw new IllegalStateException("stop at $max") + } + def tokenSource = new SafeTokenSource( + graphqlLexer, + 1000, + 200_000, + 100, + { max, token -> }, + onTooManyNumericLiteralCharacters + ) + + when: + consumeAllTokens(tokenSource) + + then: + def exception = thrown(IllegalStateException) + exception.message == "stop at 100" + offendingToken.type == GraphqlLexer.IntValue + offendingToken.stopIndex - offendingToken.startIndex + 1 == 101 + } + } From 5ef7f421f901b335637c539af706caa6f3b9b63d Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Wed, 19 Aug 2026 08:36:06 +1000 Subject: [PATCH 2/4] Make fragment cycle validation linear (cherry picked from commit db6f58b56b18eddbdb09c53b0ed73101efe50709) --- .../validation/rules/NoFragmentCycles.java | 91 ++++++++++--------- .../rules/NoFragmentCyclesTest.groovy | 28 ++++++ 2 files changed, 77 insertions(+), 42 deletions(-) diff --git a/src/main/java/graphql/validation/rules/NoFragmentCycles.java b/src/main/java/graphql/validation/rules/NoFragmentCycles.java index 32c0f751b5..575d20e22c 100644 --- a/src/main/java/graphql/validation/rules/NoFragmentCycles.java +++ b/src/main/java/graphql/validation/rules/NoFragmentCycles.java @@ -1,10 +1,13 @@ package graphql.validation.rules; +import java.util.ArrayDeque; import java.util.Collections; -import java.util.HashMap; +import java.util.Deque; import java.util.HashSet; -import java.util.LinkedList; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -26,11 +29,13 @@ @Internal public class NoFragmentCycles extends AbstractRule { - private final Map> fragmentSpreads = new HashMap<>(); + private final Map> fragmentSpreads = new LinkedHashMap<>(); + private final Set fragmentsWithCycleErrors = new HashSet<>(); public NoFragmentCycles(ValidationContext validationContext, ValidationErrorCollector validationErrorCollector) { super(validationContext, validationErrorCollector); prepareFragmentMap(); + findFragmentCycles(); } private void prepareFragmentMap() { @@ -44,7 +49,7 @@ private void prepareFragmentMap() { } private Set gatherSpreads(FragmentDefinition fragmentDefinition) { - final Set fragmentSpreads = new HashSet<>(); + final Set fragmentSpreads = new LinkedHashSet<>(); DocumentVisitor visitor = new DocumentVisitor() { @Override public void enter(Node node, List path) { @@ -65,54 +70,56 @@ public void leave(Node node, List path) { @Override public void checkFragmentDefinition(FragmentDefinition fragmentDefinition) { - LinkedList path = new LinkedList<>(); - path.add(0, fragmentDefinition.getName()); - Map> transitiveSpreads = buildTransitiveSpreads(path, new HashMap<>()); - - for (Map.Entry> entry : transitiveSpreads.entrySet()) { - if (entry.getValue().contains(entry.getKey())) { - String message = i18n(FragmentCycle, "NoFragmentCycles.cyclesNotAllowed"); - addError(ValidationErrorType.FragmentCycle, Collections.singletonList(fragmentDefinition), message); - } + if (!fragmentsWithCycleErrors.contains(fragmentDefinition.getName())) { + return; } + String message = i18n(FragmentCycle, "NoFragmentCycles.cyclesNotAllowed"); + addError(ValidationErrorType.FragmentCycle, Collections.singletonList(fragmentDefinition), message); } - private Map> buildTransitiveSpreads(LinkedList path, Map> transitiveSpreads) { - String name = path.peekFirst(); - - if (transitiveSpreads.containsKey(name)) { - return transitiveSpreads; - } - - Set spreads = fragmentSpreads.get(name); - - // spreads may be null when there is no corresponding FragmentDefinition for this spread. - // This will be handled by KnownFragmentNames - if (spreads == null || spreads.isEmpty()) { - return transitiveSpreads; + private void findFragmentCycles() { + Set visitedFragments = new HashSet<>(); + for (Map.Entry> entry : fragmentSpreads.entrySet()) { + if (!visitedFragments.add(entry.getKey())) { + continue; + } + findFragmentCycles(entry.getKey(), entry.getValue(), visitedFragments); } + } - // Add the current spreads to the transitive spreads of each ancestor in the traversal path - for (String ancestor : path) { - Set ancestorSpreads = transitiveSpreads.get(ancestor); - if (ancestorSpreads == null) { - ancestorSpreads = new HashSet<>(); + private void findFragmentCycles(String firstFragment, Set firstSpreads, Set visitedFragments) { + Set visitingFragments = new HashSet<>(); + Deque fragmentStack = new ArrayDeque<>(); + Deque> spreadIteratorStack = new ArrayDeque<>(); + visitingFragments.add(firstFragment); + fragmentStack.push(firstFragment); + spreadIteratorStack.push(firstSpreads.iterator()); + + while (!fragmentStack.isEmpty()) { + Iterator spreadIterator = spreadIteratorStack.getFirst(); + if (!spreadIterator.hasNext()) { + visitingFragments.remove(fragmentStack.pop()); + spreadIteratorStack.pop(); + continue; } - ancestorSpreads.addAll(spreads); - transitiveSpreads.put(ancestor, ancestorSpreads); - } - for (String child : spreads) { - // don't recurse infinitely, expect the recursion check to happen in checkFragmentDefinition - if (path.contains(child) || transitiveSpreads.containsKey(child)) { + String childFragment = spreadIterator.next(); + Set childSpreads = fragmentSpreads.get(childFragment); + if (childSpreads == null) { + continue; + } + if (visitingFragments.contains(childFragment)) { + fragmentsWithCycleErrors.add(childFragment); + fragmentsWithCycleErrors.add(fragmentStack.getFirst()); + continue; + } + if (!visitedFragments.add(childFragment)) { continue; } - // descend into each spread in the current fragment - LinkedList childPath = new LinkedList<>(path); - childPath.add(0, child); - buildTransitiveSpreads(childPath, transitiveSpreads); + visitingFragments.add(childFragment); + fragmentStack.push(childFragment); + spreadIteratorStack.push(childSpreads.iterator()); } - return transitiveSpreads; } } diff --git a/src/test/groovy/graphql/validation/rules/NoFragmentCyclesTest.groovy b/src/test/groovy/graphql/validation/rules/NoFragmentCyclesTest.groovy index ea3e1807f7..19ea93561b 100644 --- a/src/test/groovy/graphql/validation/rules/NoFragmentCyclesTest.groovy +++ b/src/test/groovy/graphql/validation/rules/NoFragmentCyclesTest.groovy @@ -238,4 +238,32 @@ class NoFragmentCyclesTest extends Specification { errorCollector.containsValidationError(ValidationErrorType.FragmentCycle) errorCollector.getErrors()[0].message == "Validation error (FragmentCycle@[MyFrag]) : Fragment cycles not allowed" } + + def "long acyclic fragment chains are valid"() { + when: + traverse(fragmentChain(1_000, false)) + + then: + errorCollector.getErrors().isEmpty() + } + + def "cycles at the end of long fragment chains are detected"() { + when: + traverse(fragmentChain(1_000, true)) + + then: + errorCollector.containsValidationError(ValidationErrorType.FragmentCycle) + } + + private static String fragmentChain(int fragmentCount, boolean cycle) { + (0.. + String selection = "name" + if (index < fragmentCount - 1) { + selection = "...F${index + 1}" + } else if (cycle) { + selection = "...F${fragmentCount.intdiv(2)}" + } + "fragment F${index} on Dog { ${selection} }" + }.join("\n") + } } From 5b044753afd9f922c51ae6bea06562a9df1b23df Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Wed, 19 Aug 2026 09:07:30 +1000 Subject: [PATCH 3/4] Make block string indentation removal linear (cherry picked from commit 5b07d3cb060a841f9e14d6248144ecba3cd0b6ef) --- .../graphql/parser/StringValueParsing.java | 53 ++++++------------- .../parser/StringValueParsingTest.groovy | 23 ++++++++ 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/main/java/graphql/parser/StringValueParsing.java b/src/main/java/graphql/parser/StringValueParsing.java index 56b1f1ec88..046f4ed8b9 100644 --- a/src/main/java/graphql/parser/StringValueParsing.java +++ b/src/main/java/graphql/parser/StringValueParsing.java @@ -6,9 +6,6 @@ import graphql.language.SourceLocation; import java.io.StringWriter; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; /** * Contains parsing code for the StringValue types in the grammar @@ -44,45 +41,25 @@ public static String removeIndentation(String rawValue) { } } } - List lineList = new ArrayList<>(Arrays.asList(lines)); - if (commonIndent != null) { - for (int i = 0; i < lineList.size(); i++) { - String line = lineList.get(i); - if (i == 0) { - continue; - } - if (line.length() > commonIndent) { - line = line.substring(commonIndent); - lineList.set(i, line); - } - } + int firstLine = 0; + while (firstLine < lines.length && containsOnlyWhiteSpace(lines[firstLine])) { + firstLine++; } - while (!lineList.isEmpty()) { - String line = lineList.get(0); - if (containsOnlyWhiteSpace(line)) { - lineList.remove(0); - } else { - break; - } + int lastLine = lines.length; + while (lastLine > firstLine && containsOnlyWhiteSpace(lines[lastLine - 1])) { + lastLine--; } - while (!lineList.isEmpty()) { - int endIndex = lineList.size() - 1; - String line = lineList.get(endIndex); - if (containsOnlyWhiteSpace(line)) { - lineList.remove(endIndex); - } else { - break; + + StringBuilder formatted = new StringBuilder(rawValue.length()); + for (int i = firstLine; i < lastLine; i++) { + String line = lines[i]; + if (commonIndent != null && i > 0 && line.length() > commonIndent) { + line = line.substring(commonIndent); } - } - StringBuilder formatted = new StringBuilder(); - for (int i = 0; i < lineList.size(); i++) { - String line = lineList.get(i); - if (i == 0) { - formatted.append(line); - } else { - formatted.append("\n"); - formatted.append(line); + if (i > firstLine) { + formatted.append('\n'); } + formatted.append(line); } return formatted.toString(); } diff --git a/src/test/groovy/graphql/parser/StringValueParsingTest.groovy b/src/test/groovy/graphql/parser/StringValueParsingTest.groovy index 2142c84175..9f2ce774db 100644 --- a/src/test/groovy/graphql/parser/StringValueParsingTest.groovy +++ b/src/test/groovy/graphql/parser/StringValueParsingTest.groovy @@ -2,6 +2,7 @@ package graphql.parser import graphql.i18n.I18n import graphql.language.SourceLocation +import graphql.language.StringValue import spock.lang.Specification import static java.util.Arrays.asList @@ -226,4 +227,26 @@ L 2 L 3''' parsed == expected } + + def "removes large runs of leading and trailing blank lines"() { + given: + def input = "\n".repeat(20_000) + " value" + "\n ".repeat(20_000) + + when: + String parsed = StringValueParsing.removeIndentation(input) + + then: + parsed == "value" + } + + def "full parser handles block strings with many leading and trailing blank lines"() { + given: + def input = '"""' + "\n".repeat(20_000) + " value" + "\n ".repeat(20_000) + '"""' + + when: + StringValue parsed = Parser.parseValue(input) + + then: + parsed.getValue() == "value" + } } From 2a03db9504a74098973b4d418fdac42d52d6d188 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Mon, 24 Aug 2026 09:50:32 +1000 Subject: [PATCH 4/4] Enable pull request builds for 25.x --- .github/workflows/pull_request.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index de26c9f353..15bfcdb56d 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -7,6 +7,7 @@ on: pull_request: branches: - master + - 25.x - 23.x - 22.x - 21.x