Skip to content

Add query depth and field count limits to Validator#4230

Closed
andimarek wants to merge 301 commits intovalidation-refactorfrom
query-complexity-limits
Closed

Add query depth and field count limits to Validator#4230
andimarek wants to merge 301 commits intovalidation-refactorfrom
query-complexity-limits

Conversation

@andimarek
Copy link
Copy Markdown
Member

Summary

  • Adds QueryComplexityLimits class for configuring maxDepth and maxFieldsCount limits
  • Provides a lightweight alternative to ExecutableNormalizedOperation (ENO) for tracking query complexity
  • Fragment fields are counted at each spread site (matching ENO behavior)
  • New validation error types: MaxQueryDepthExceeded, MaxQueryFieldsExceeded

Usage

QueryComplexityLimits limits = QueryComplexityLimits.newLimits()
    .maxDepth(10)
    .maxFieldsCount(100)
    .build();

ExecutionInput input = ExecutionInput.newExecutionInput()
    .query(query)
    .graphQLContext(ctx -> ctx.put(QueryComplexityLimits.KEY, limits))
    .build();

Test plan

  • Field count limit is enforced
  • Depth limit is enforced
  • Fragment fields counted at each spread site
  • Nested fragments handled correctly
  • Multiple operations have separate limits
  • Inline fragments count their fields
  • Backward compatible (no limits by default)
  • Cyclic fragments don't cause infinite loop
  • All existing validation tests pass (323 tests)
  • All introspection tests pass (68 tests)

🤖 Generated with Claude Code

@andimarek andimarek force-pushed the query-complexity-limits branch 5 times, most recently from 2656347 to 2c49495 Compare January 28, 2026 21:10
andimarek and others added 25 commits February 23, 2026 14:36
…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>
…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>
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>
…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>
github-actions bot and others added 10 commits March 21, 2026 04:34
# 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
@dondonz dondonz added the breaking change requires a new major version to be relased label Mar 22, 2026
@dondonz
Copy link
Copy Markdown
Member

dondonz commented Mar 22, 2026

Adding a breaking change label, this is technically a breaking change to add a new depth/field limit

dondonz and others added 2 commits March 22, 2026 17:24
Copy link
Copy Markdown
Member

@dondonz dondonz left a comment

Choose a reason for hiding this comment

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

Approved, although there's a merge conflict with another recent PR

andimarek and others added 13 commits March 23, 2026 11:30
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>
@andimarek andimarek closed this Mar 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change requires a new major version to be relased

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants