Allow computing query complexity without variable values - #1970
Open
ruudk wants to merge 1 commit into
Open
Conversation
ruudk
force-pushed
the
fix-graphql-php-issue-1968
branch
from
August 25, 2026 12:49
335870e to
94e4717
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a supported way to compute QueryComplexity for operations when runtime variable values are not available (e.g., validating Apollo persisted operation manifests in CI), by introducing a “worst-case” mode for unprovided variables while keeping default behavior unchanged.
Changes:
- Add
QueryComplexity::$assumeWorstCaseForUnprovidedVariablesto treat unprovided variables as “unknown” (include variable-conditional fields; omit variable-dependent field arguments passed to complexity functions). - Reuse cached coerced variable values (avoid re-coercing per field) and adjust coercion to skip non-coercible variable definitions in worst-case mode.
- Add test coverage and documentation for the new behavior, plus changelog entry.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/Validator/Rules/QueryComplexity.php |
Implements worst-case mode for unprovided variables, updates directive and argument handling, and reuses cached variable coercion. |
tests/Validator/QuerySecuritySchema.php |
Extends test schema arguments/input types to exercise variable-dependent argument omission scenarios. |
tests/Validator/QueryComplexityTest.php |
Adds tests covering worst-case behavior for @include/@skip and variable-dependent arguments. |
docs/security.md |
Documents how to enable and reason about complexity computation without variable values. |
CHANGELOG.md |
Records the new QueryComplexity capability for the next release. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+389
to
+395
| // Arguments that depend on a variable without a value can not be coerced, | ||
| // so they are left out and complexity functions get to decide what to make | ||
| // of their absence. | ||
| $argumentValueMap = []; | ||
| foreach ($node->arguments as $argumentNode) { | ||
| if ($this->dependsOnUnprovidedVariable($argumentNode->value)) { | ||
| continue; |
Validating operations for which no variable values exist yet - such as a manifest of Apollo persisted operations checked in CI - was impossible with QueryComplexity: coercing the variable definitions to evaluate @include/@Skip and the arguments of fields with a complexity function fails for every operation that declares a required variable. The only way out was to subclass the rule and override the protected directiveExcludesField(). QueryComplexity::$assumeWorstCaseForUnprovidedVariables makes that a supported use case by treating variables without a value as unknown instead of coercing them: conditional fields count as included, so only literal @include(if: false)/@Skip(if: true) are dropped, and arguments that depend on such a variable are left out of what is passed to complexity functions. Variables that were provided a value keep working as before, and invalid values still error out.
ruudk
force-pushed
the
fix-graphql-php-issue-1968
branch
from
August 25, 2026 13:05
94e4717 to
d1e1b98
Compare
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.
Fixes #1968.
QueryComplexitycoerces the operation's variable definitions to evaluate@include/@skipand to build the arguments of fields with acomplexityfunction, which fails for every operation that declares a required variable — so validating operation text alone, such as a manifest of Apollo persisted operations checked in CI, was only possible by subclassing the rule and overriding the protecteddirectiveExcludesField().Setting the new
QueryComplexity::$assumeWorstCaseForUnprovidedVariablestreats variables without a value as unknown instead of coercing them: conditional fields count as included, so only literal@include(if: false)/@skip(if: true)are dropped, and arguments that depend on such a variable (including ones nested in list or input object literals) are left out of what is passed tocomplexityfunctions, which can then pick their own fallback.Variables that were provided a value are still used as given, invalid values still error out, and the default behaviour is unchanged. Along the way
buildFieldArguments()now reuses the cached coerced variable values instead of re-coercing every variable definition per field.