Add query depth and field count limits to Validator#4230
Closed
andimarek wants to merge 301 commits intovalidation-refactorfrom
Closed
Add query depth and field count limits to Validator#4230andimarek wants to merge 301 commits intovalidation-refactorfrom
andimarek wants to merge 301 commits intovalidation-refactorfrom
Conversation
2656347 to
2c49495
Compare
…ltiple-schemas Benchmark introspection against multiple large schemas
Bumps [EnricoMi/publish-unit-test-result-action](https://github.com/enricomi/publish-unit-test-result-action) from 2.22.0 to 2.23.0. - [Release notes](https://github.com/enricomi/publish-unit-test-result-action/releases) - [Commits](EnricoMi/publish-unit-test-result-action@v2.22.0...v2.23.0) --- updated-dependencies: - dependency-name: EnricoMi/publish-unit-test-result-action dependency-version: 2.23.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [net.bytebuddy:byte-buddy](https://github.com/raphw/byte-buddy) from 1.18.1 to 1.18.5. - [Release notes](https://github.com/raphw/byte-buddy/releases) - [Changelog](https://github.com/raphw/byte-buddy/blob/master/release-notes.md) - [Commits](raphw/byte-buddy@byte-buddy-1.18.1...byte-buddy-1.18.5) --- updated-dependencies: - dependency-name: net.bytebuddy:byte-buddy dependency-version: 1.18.5 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…buddy-byte-buddy-1.18.5 Bump net.bytebuddy:byte-buddy from 1.18.1 to 1.18.5
Bumps [com.fasterxml.jackson.core:jackson-databind](https://github.com/FasterXML/jackson) from 2.21.0 to 2.21.1. - [Commits](https://github.com/FasterXML/jackson/commits) --- updated-dependencies: - dependency-name: com.fasterxml.jackson.core:jackson-databind dependency-version: 2.21.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com>
…-unit-test-result-action-2.23.0
…erxml.jackson.core-jackson-databind-2.21.1
Bumps [me.bechberger:ap-loader-all](https://github.com/jvm-profiling-tools/ap-loader) from 4.0-10 to 4.3-12. - [Release notes](https://github.com/jvm-profiling-tools/ap-loader/releases) - [Commits](https://github.com/jvm-profiling-tools/ap-loader/commits) --- updated-dependencies: - dependency-name: me.bechberger:ap-loader-all dependency-version: 4.3-12 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…erger-ap-loader-all-4.3-12 Bump me.bechberger:ap-loader-all from 4.0-10 to 4.3-12
Bumps [com.google.errorprone:error_prone_core](https://github.com/google/error-prone) from 2.44.0 to 2.47.0. - [Release notes](https://github.com/google/error-prone/releases) - [Commits](google/error-prone@v2.44.0...v2.47.0) --- updated-dependencies: - dependency-name: com.google.errorprone:error_prone_core dependency-version: 2.47.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>
…-unit-test-result-action-2.23.0
Annotates the following classes with @NullMarked and appropriate @nullable annotations: - DeferredExecution (label is @nullable per GraphQL spec) - ChainedInstrumentation (removes @nonnull, propagates @nullable from Instrumentation interface) - DocumentAndVariables (@NullUnmarked on Builder) - NoContextChainedInstrumentation (runAll returns @nullable T) - ResponseMapFactory (values are @nullable per Javadoc contract) - SimpleInstrumentation - SimpleInstrumentationContext (@nullable fields, params, onCompleted) - SimplePerformantInstrumentation (removes @nonnull on instrument* overrides) - FieldAndArguments (getParentFieldAndArguments @nullable, getArgumentValue @nullable T) - FieldValidationEnvironment Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…le.errorprone-error_prone_core-2.47.0
…arameter chainedCtx can return null in the single-instrumentation case since it passes the mapper result through directly, and the underlying Instrumentation begin* methods are @nullable (null is the opt-out signal). The BiFunction return type also needs @nullable so that mapper.apply() is correctly typed as nullable, allowing the nullability to flow from the underlying instrumentation through the lambda to chainedCtx's return. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
# Conflicts: # .claude/commands/jspecify-annotate.md # build.gradle # src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy
Resolve JSpecifyAnnotationsCheck conflict by taking master's exemption list updates (classes already annotated on master removed, new exemptions added). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
NodeVisitor, NodeVisitorStub, SourceLocation, and Type are already annotated with @NullMarked so they should not be in the exemption list. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
NodeVisitor, NodeVisitorStub, SourceLocation, and Type are already annotated with @NullMarked so they should not be in the exemption list. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Coverage-only failures trigger CI Failure Doctor but result in no-op runs that spam issue #4342 with "no action required" comments. Setting report-as-issue: false for noop outputs suppresses this noise. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ations-to-classes Add JSpecify annotations to 10 classes in graphql.language package and improve annotation prompt
…kflows Disable no-op issue reporting for CI Failure Doctor
Member
|
Adding a breaking change label, this is technically a breaking change to add a new depth/field limit |
…ations Add JSpecify annotations to 10 graphql.language classes
dondonz
approved these changes
Mar 22, 2026
Member
dondonz
left a comment
There was a problem hiding this comment.
Approved, although there's a merge conflict with another recent PR
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>
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>
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>
- 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
QueryComplexityLimitsclass for configuringmaxDepthandmaxFieldsCountlimitsExecutableNormalizedOperation(ENO) for tracking query complexityMaxQueryDepthExceeded,MaxQueryFieldsExceededUsage
Test plan
🤖 Generated with Claude Code