Add query complexity limits and refactor GoodFaithIntrospection to use validation#4256
Merged
Add query complexity limits and refactor GoodFaithIntrospection to use validation#4256
Conversation
Contributor
This provides a lightweight alternative to ExecutableNormalizedOperation
(ENO) for tracking query complexity during validation.
New features:
- QueryComplexityLimits class with maxDepth and maxFieldsCount settings
- Configuration via GraphQLContext using QueryComplexityLimits.KEY
- Fragment fields counted at each spread site (like ENO)
- Depth tracking measures nested Field nodes
- New validation error types: MaxQueryDepthExceeded, MaxQueryFieldsExceeded
Implementation notes:
- Fragment complexity is calculated lazily during first spread traversal
- No additional AST traversal needed - complexity tracked during normal
validation traversal
- Subsequent spreads of the same fragment add the stored complexity
Usage:
```java
QueryComplexityLimits limits = QueryComplexityLimits.newLimits()
.maxDepth(10)
.maxFieldsCount(100)
.build();
ExecutionInput input = ExecutionInput.newExecutionInput()
.query(query)
.graphQLContext(ctx -> ctx.put(QueryComplexityLimits.KEY, limits))
.build();
```
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move introspection abuse detection from execution-time ENO creation to the validation layer. This eliminates the expensive ExecutableNormalizedOperation construction for every introspection query. The validator now enforces two checks when GOOD_FAITH_INTROSPECTION is enabled: field repetition (__schema/__type max once, __Type cycle fields max once) and tightened complexity limits (500 fields, 20 depth). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…base - Add @nullable annotations for QueryComplexityLimits parameters - Replace shouldRunNonFragmentSpreadChecks() with shouldRunDocumentLevelRules() - Replace fragmentSpreadVisitDepth with fragmentRetraversalDepth Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
884234a to
97d10d2
Compare
Contributor
Test ReportTest Results
Code Coverage (Java 25)
Changed Class Coverage (11 classes)
|
The OverlappingFieldsCanBeMergedBenchmarkTest uses intentionally large queries (108k fields, depth 101) that exceed default complexity limits. Pass QueryComplexityLimits.NONE to bypass the limits in these tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The refactored GoodFaithIntrospection validation had uncovered code paths for queries that exceed complexity limits (field count or depth) without triggering the tooManyFields cycle check. These tests exercise: - Wide introspection query exceeding field count limit (>500 fields) - Deep introspection query exceeding depth limit (>20 levels via ofType) - Custom user limits combined with good faith limits Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
cdd531a to
deb35be
Compare
Instead of pre-scanning the document with containsIntrospectionFields, let checkGoodFaithIntrospection detect introspection queries at validation time when it first encounters __schema or __type on the Query type. At that point it tightens the complexity limits and sets a flag so that subsequent limit breaches throw GoodFaithIntrospectionExceeded directly. This eliminates the pre-scan (which could miss introspection fields hidden inside inline fragments or fragment spreads) and simplifies GraphQL.validate(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
b4c02fb to
901dafc
Compare
- Remove redundant @nonnull annotations from new validate() overload (class is @NullMarked, only @nullable needed for limits parameter) - Remove redundant null check in QueryComplexityLimits.setDefaultLimits() (class is @NullMarked, parameter cannot be null) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
A GraphQL schema always has a query type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace "__schema", "__type", and "__Type" string literals in checkGoodFaithIntrospection with Introspection.SchemaMetaFieldDef, Introspection.TypeMetaFieldDef, and Introspection.__Type. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bad merge in 0c7b9c3 took the one-liner branchName definition from master (which does replaceAll inline) but kept the two-liner's sanitizedBranchName reference from the copilot branch. The variable branchName already includes sanitization via replaceAll('[/\\]', '-'). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Covers the lambda branch in GraphQL.validate() where good faith is disabled and an existing custom rule predicate also excludes a rule, exercising the && short-circuit path. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds lightweight query complexity checking during validation and refactors GoodFaithIntrospection to use validation rules instead of ExecutableNormalizedOperation (ENO).
Query Complexity Limits
QueryComplexityLimitsclass withmaxDepthandmaxFieldsCountsettingsQueryComplexityLimits.NONEto disable)GraphQLContextusingQueryComplexityLimits.KEYMaxQueryDepthExceeded,MaxQueryFieldsExceededGoodFaithIntrospection Refactor
GOOD_FAITH_INTROSPECTION)__schemaor__typeis encountered on the Query type — no pre-scan of the document neededGoodFaithIntrospectionExceededdirectlyQuery.__schema,Query.__type,__Type.fields/inputFields/interfaces/possibleTypeseach allowed at most onceUsage
Breaking changes
Behavioral changes
MaxQueryDepthExceededorMaxQueryFieldsExceededvalidation errors. UseQueryComplexityLimits.NONEviaGraphQLContextto disable, or callQueryComplexityLimits.setDefaultLimits(QueryComplexityLimits.NONE)globally.GoodFaithIntrospection(@PublicApi)checkIntrospection(ExecutionContext)— was the ENO-based entry point, no longer neededALLOWED_FIELD_INSTANCESmap — field instance limits are now enforced inside the validatorisEnabled(GraphQLContext)— checks whether good faith introspection is enabled for a requestgoodFaithLimits(QueryComplexityLimits)— computes the effective limits for introspection queriesBadFaithIntrospectionError.getLocations()return type changed to@Nullable List<SourceLocation>ValidationErrorType(@PublicApi)MaxQueryDepthExceededenum valueMaxQueryFieldsExceededenum valueOperationValidationRule(@PublicApi)GOOD_FAITH_INTROSPECTIONenum valueParseAndValidate(@PublicApi)validate(GraphQLSchema, Document, Predicate, Locale, QueryComplexityLimits)accepting optional complexity limitsNew public classes
QueryComplexityLimits(@PublicApi) — configuration for depth and field count limitsQueryComplexityLimits.Builder(@PublicApi) — builder forQueryComplexityLimitsTest plan
tooBigOperationpath (field count and depth limit exceeded)QueryComplexityLimits.NONE🤖 Generated with Claude Code