Skip to content

Add query depth and field count limits to Validator#4230

Open
andimarek wants to merge 3 commits intovalidation-refactorfrom
query-complexity-limits
Open

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

Conversation

@andimarek
Copy link
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 4 times, most recently from 2a2074b to 2656347 Compare January 28, 2026 21:04
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 <[email protected]>
@andimarek andimarek force-pushed the query-complexity-limits branch from 2656347 to 2c49495 Compare January 28, 2026 21:10
andimarek and others added 2 commits February 13, 2026 10:29
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 <[email protected]>
Update to use renamed methods from validation-refactor:
- shouldRunNonFragmentSpreadChecks() → shouldRunDocumentLevelRules()
- fragmentSpreadVisitDepth → fragmentRetraversalDepth
- operationScope checks → shouldRunOperationScopedRules()

Fix NullAway errors from master's @NullMarked additions by adding
@nullable annotations to ParseAndValidate.validate() limits param,
GoodFaithIntrospection.goodFaithLimits() param, and
ValidationContext constructor limits param.
@dondonz dondonz added the breaking change requires a new major version to be relased label Mar 22, 2026
@dondonz
Copy link
Member

dondonz commented Mar 22, 2026

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

Copy link
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

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.

2 participants