From 2ef412a707c48f0be1b299570bd45bb0f0c61a52 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Thu, 29 Jan 2026 06:24:37 +1000 Subject: [PATCH 01/11] Add query depth and field count limits to Validator 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 --- src/main/java/graphql/GraphQL.java | 6 +- src/main/java/graphql/ParseAndValidate.java | 18 +- .../validation/FragmentComplexityInfo.java | 44 ++ .../validation/OperationValidator.java | 99 +++- .../validation/QueryComplexityLimits.java | 170 +++++++ .../QueryComplexityLimitsExceeded.java | 42 ++ .../graphql/validation/ValidationContext.java | 10 + .../validation/ValidationErrorType.java | 4 +- .../java/graphql/validation/Validator.java | 12 +- src/main/resources/i18n/Validation.properties | 5 +- .../GoodFaithIntrospectionTest.groovy | 4 + ...tableNormalizedOperationFactoryTest.groovy | 9 + .../QueryComplexityLimitsTest.groovy | 458 ++++++++++++++++++ 13 files changed, 871 insertions(+), 10 deletions(-) create mode 100644 src/main/java/graphql/validation/FragmentComplexityInfo.java create mode 100644 src/main/java/graphql/validation/QueryComplexityLimits.java create mode 100644 src/main/java/graphql/validation/QueryComplexityLimitsExceeded.java create mode 100644 src/test/groovy/graphql/validation/QueryComplexityLimitsTest.groovy diff --git a/src/main/java/graphql/GraphQL.java b/src/main/java/graphql/GraphQL.java index 1441238b89..14bf273a53 100644 --- a/src/main/java/graphql/GraphQL.java +++ b/src/main/java/graphql/GraphQL.java @@ -26,6 +26,7 @@ import graphql.language.Document; import graphql.schema.GraphQLSchema; import graphql.validation.OperationValidationRule; +import graphql.validation.QueryComplexityLimits; import graphql.validation.ValidationError; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.NullUnmarked; @@ -600,8 +601,9 @@ private List validate(ExecutionInput executionInput, Document d validationCtx.onDispatched(); Predicate validationRulePredicate = executionInput.getGraphQLContext().getOrDefault(ParseAndValidate.INTERNAL_VALIDATION_PREDICATE_HINT, r -> true); - Locale locale = executionInput.getLocale(); - List validationErrors = ParseAndValidate.validate(graphQLSchema, document, validationRulePredicate, locale); + Locale locale = executionInput.getLocale() != null ? executionInput.getLocale() : Locale.getDefault(); + QueryComplexityLimits limits = executionInput.getGraphQLContext().get(QueryComplexityLimits.KEY); + List validationErrors = ParseAndValidate.validate(graphQLSchema, document, validationRulePredicate, locale, limits); validationCtx.onCompleted(validationErrors, null); return validationErrors; diff --git a/src/main/java/graphql/ParseAndValidate.java b/src/main/java/graphql/ParseAndValidate.java index 12af6af3a8..26c02db19c 100644 --- a/src/main/java/graphql/ParseAndValidate.java +++ b/src/main/java/graphql/ParseAndValidate.java @@ -7,6 +7,7 @@ import graphql.parser.ParserOptions; import graphql.schema.GraphQLSchema; import graphql.validation.OperationValidationRule; +import graphql.validation.QueryComplexityLimits; import graphql.validation.ValidationError; import graphql.validation.Validator; import org.jspecify.annotations.NonNull; @@ -118,8 +119,23 @@ public static List validate(@NonNull GraphQLSchema graphQLSchem * @return a result object that indicates how this operation went */ public static List validate(@NonNull GraphQLSchema graphQLSchema, @NonNull Document parsedDocument, @NonNull Predicate rulePredicate, @NonNull Locale locale) { + return validate(graphQLSchema, parsedDocument, rulePredicate, locale, null); + } + + /** + * This can be called to validate a parsed graphql query. + * + * @param graphQLSchema the graphql schema to validate against + * @param parsedDocument the previously parsed document + * @param rulePredicate this predicate is used to decide what validation rules will be applied + * @param locale the current locale + * @param limits optional query complexity limits to enforce + * + * @return a result object that indicates how this operation went + */ + public static List validate(@NonNull GraphQLSchema graphQLSchema, @NonNull Document parsedDocument, @NonNull Predicate rulePredicate, @NonNull Locale locale, QueryComplexityLimits limits) { Validator validator = new Validator(); - return validator.validateDocument(graphQLSchema, parsedDocument, rulePredicate, locale); + return validator.validateDocument(graphQLSchema, parsedDocument, rulePredicate, locale, limits); } /** diff --git a/src/main/java/graphql/validation/FragmentComplexityInfo.java b/src/main/java/graphql/validation/FragmentComplexityInfo.java new file mode 100644 index 0000000000..ae04911e75 --- /dev/null +++ b/src/main/java/graphql/validation/FragmentComplexityInfo.java @@ -0,0 +1,44 @@ +package graphql.validation; + +import graphql.Internal; +import org.jspecify.annotations.NullMarked; + +/** + * Holds pre-calculated complexity metrics for a fragment definition. + * This is used to efficiently track query complexity when fragments are spread + * at multiple locations in a query. + */ +@Internal +@NullMarked +class FragmentComplexityInfo { + + private final int fieldCount; + private final int maxDepth; + + FragmentComplexityInfo(int fieldCount, int maxDepth) { + this.fieldCount = fieldCount; + this.maxDepth = maxDepth; + } + + /** + * @return the total number of fields in this fragment, including fields from nested fragments + */ + int getFieldCount() { + return fieldCount; + } + + /** + * @return the maximum depth of fields within this fragment + */ + int getMaxDepth() { + return maxDepth; + } + + @Override + public String toString() { + return "FragmentComplexityInfo{" + + "fieldCount=" + fieldCount + + ", maxDepth=" + maxDepth + + '}'; + } +} diff --git a/src/main/java/graphql/validation/OperationValidator.java b/src/main/java/graphql/validation/OperationValidator.java index 947955b937..34f8602d7a 100644 --- a/src/main/java/graphql/validation/OperationValidator.java +++ b/src/main/java/graphql/validation/OperationValidator.java @@ -328,6 +328,16 @@ public class OperationValidator implements DocumentVisitor { // --- State: SubscriptionUniqueRootField --- private final FieldCollector fieldCollector = new FieldCollector(); + // --- State: Query Complexity Limits --- + private int fieldCount = 0; + private int currentFieldDepth = 0; + private int maxFieldDepthSeen = 0; + private final QueryComplexityLimits complexityLimits; + // Fragment complexity calculated lazily during first spread + private final Map fragmentComplexityMap = new HashMap<>(); + // Max depth seen during current fragment traversal (for calculating fragment's internal depth) + private int fragmentTraversalMaxDepth = 0; + // --- Track whether we're in a context where fragment spread rules should run --- // fragmentRetraversalDepth == 0 means we're NOT inside a manually-traversed fragment => run non-fragment-spread checks // operationScope means we're inside an operation => can trigger fragment traversal @@ -340,6 +350,7 @@ public OperationValidator(ValidationContext validationContext, ValidationErrorCo this.validationUtil = new ValidationUtil(); this.rulePredicate = rulePredicate; this.allRulesEnabled = detectAllRulesEnabled(rulePredicate); + this.complexityLimits = validationContext.getQueryComplexityLimits(); prepareFragmentSpreadsMap(); } @@ -388,6 +399,29 @@ private boolean shouldRunOperationScopedRules() { return operationScope; } + // ==================== Query Complexity Limit Helpers ==================== + + private void checkFieldCountLimit() { + if (fieldCount > complexityLimits.getMaxFieldsCount()) { + throw new QueryComplexityLimitsExceeded( + ValidationErrorType.MaxQueryFieldsExceeded, + complexityLimits.getMaxFieldsCount(), + fieldCount); + } + } + + private void checkDepthLimit(int depth) { + if (depth > maxFieldDepthSeen) { + maxFieldDepthSeen = depth; + if (maxFieldDepthSeen > complexityLimits.getMaxDepth()) { + throw new QueryComplexityLimitsExceeded( + ValidationErrorType.MaxQueryDepthExceeded, + complexityLimits.getMaxDepth(), + maxFieldDepthSeen); + } + } + } + @Override public void enter(Node node, List ancestors) { validationContext.getTraversalContext().enter(node, ancestors); @@ -401,6 +435,17 @@ public void enter(Node node, List ancestors) { } else if (node instanceof VariableDefinition) { checkVariableDefinition((VariableDefinition) node); } else if (node instanceof Field) { + // Track complexity only during operation scope + if (operationScope) { + fieldCount++; + currentFieldDepth++; + checkFieldCountLimit(); + checkDepthLimit(currentFieldDepth); + // Track max depth during fragment traversal for storing later + if (fragmentRetraversalDepth > 0 && currentFieldDepth > fragmentTraversalMaxDepth) { + fragmentTraversalMaxDepth = currentFieldDepth; + } + } checkField((Field) node); } else if (node instanceof InlineFragment) { checkInlineFragment((InlineFragment) node); @@ -433,6 +478,10 @@ public void leave(Node node, List ancestors) { leaveSelectionSet(); } else if (node instanceof FragmentDefinition) { leaveFragmentDefinition(); + } else if (node instanceof Field) { + if (operationScope) { + currentFieldDepth--; + } } } @@ -611,14 +660,50 @@ private void checkFragmentSpread(FragmentSpread node, List ancestors) { } } - // Manually traverse into fragment definition during operation scope + // Handle complexity tracking and fragment traversal if (operationScope) { - FragmentDefinition fragment = validationContext.getFragment(node.getName()); - if (fragment != null && !visitedFragmentSpreads.contains(node.getName())) { - visitedFragmentSpreads.add(node.getName()); + String fragmentName = node.getName(); + FragmentDefinition fragment = validationContext.getFragment(fragmentName); + + if (visitedFragmentSpreads.contains(fragmentName)) { + // Subsequent spread - add stored complexity (don't traverse again) + FragmentComplexityInfo info = fragmentComplexityMap.get(fragmentName); + if (info != null) { + fieldCount += info.getFieldCount(); + checkFieldCountLimit(); + int potentialDepth = currentFieldDepth + info.getMaxDepth(); + checkDepthLimit(potentialDepth); + // Update max depth if we're inside a fragment traversal + if (fragmentRetraversalDepth > 0 && potentialDepth > fragmentTraversalMaxDepth) { + fragmentTraversalMaxDepth = potentialDepth; + } + } + } else if (fragment != null) { + // First spread - traverse and track complexity + visitedFragmentSpreads.add(fragmentName); + + int fieldCountBefore = fieldCount; + int depthAtEntry = currentFieldDepth; + int previousFragmentMaxDepth = fragmentTraversalMaxDepth; + + // Initialize max depth tracking for this fragment + fragmentTraversalMaxDepth = currentFieldDepth; + fragmentRetraversalDepth++; new LanguageTraversal(ancestors).traverse(fragment, this); fragmentRetraversalDepth--; + + // Calculate and store fragment complexity + int fragmentFieldCount = fieldCount - fieldCountBefore; + int fragmentMaxInternalDepth = fragmentTraversalMaxDepth - depthAtEntry; + + fragmentComplexityMap.put(fragmentName, + new FragmentComplexityInfo(fragmentFieldCount, fragmentMaxInternalDepth)); + + // Restore max depth for outer fragment (if nested) + if (fragmentRetraversalDepth > 0 && previousFragmentMaxDepth > fragmentTraversalMaxDepth) { + fragmentTraversalMaxDepth = previousFragmentMaxDepth; + } } } } @@ -724,6 +809,12 @@ private void leaveOperationDefinition() { } } } + + // Reset complexity counters for next operation + fieldCount = 0; + currentFieldDepth = 0; + maxFieldDepthSeen = 0; + fragmentTraversalMaxDepth = 0; } private void leaveSelectionSet() { diff --git a/src/main/java/graphql/validation/QueryComplexityLimits.java b/src/main/java/graphql/validation/QueryComplexityLimits.java new file mode 100644 index 0000000000..8c4e0f4ad6 --- /dev/null +++ b/src/main/java/graphql/validation/QueryComplexityLimits.java @@ -0,0 +1,170 @@ +package graphql.validation; + +import graphql.PublicApi; +import org.jspecify.annotations.NullMarked; + +/** + * Configuration class for query complexity limits enforced during validation. + * This provides a lightweight alternative to ExecutableNormalizedOperation (ENO) for tracking + * query depth and field count. + * + *

By default, validation enforces limits (maxDepth=100, maxFieldsCount=100000). + * To customize limits per-request, put a custom instance in the GraphQLContext: + * 

{@code
+ * QueryComplexityLimits limits = QueryComplexityLimits.newLimits()
+ *     .maxDepth(10)
+ *     .maxFieldsCount(100)
+ *     .build();
+ *
+ * ExecutionInput executionInput = ExecutionInput.newExecutionInput()
+ *     .query(query)
+ *     .graphQLContext(ctx -> ctx.put(QueryComplexityLimits.KEY, limits))
+ *     .build();
+ * }
+ * + *

To disable limits for a request, use {@link #NONE}: + * 

{@code
+ * executionInput.getGraphQLContext().put(QueryComplexityLimits.KEY, QueryComplexityLimits.NONE);
+ * }
+ * + *

To change the default limits globally (e.g., for testing), use {@link #setDefaultLimits(QueryComplexityLimits)}: + * 

{@code
+ * QueryComplexityLimits.setDefaultLimits(QueryComplexityLimits.NONE); // disable for tests
+ * }
+ */ +@PublicApi +@NullMarked +public class QueryComplexityLimits { + + /** + * Default maximum query depth. + */ + public static final int DEFAULT_MAX_DEPTH = 100; + + /** + * Default maximum field count. + */ + public static final int DEFAULT_MAX_FIELDS_COUNT = 100_000; + + /** + * The key used to store QueryComplexityLimits in GraphQLContext. + */ + public static final String KEY = "graphql.validation.QueryComplexityLimits"; + + /** + * Standard limits (maxDepth=100, maxFieldsCount=100000). + */ + public static final QueryComplexityLimits DEFAULT = new QueryComplexityLimits(DEFAULT_MAX_DEPTH, DEFAULT_MAX_FIELDS_COUNT); + + /** + * No limits (all limits set to Integer.MAX_VALUE). Use this to disable complexity checking. + */ + public static final QueryComplexityLimits NONE = new QueryComplexityLimits(Integer.MAX_VALUE, Integer.MAX_VALUE); + + private static volatile QueryComplexityLimits defaultLimits = DEFAULT; + + /** + * Sets the default limits used when no limits are specified in GraphQLContext. + * This is useful for testing or for applications that want different global defaults. + * + * @param limits the default limits to use (use {@link #NONE} to disable, {@link #DEFAULT} to restore) + */ + public static void setDefaultLimits(QueryComplexityLimits limits) { + defaultLimits = limits != null ? limits : DEFAULT; + } + + /** + * Returns the current default limits. + * + * @return the default limits + */ + public static QueryComplexityLimits getDefaultLimits() { + return defaultLimits; + } + + private final int maxDepth; + private final int maxFieldsCount; + + private QueryComplexityLimits(int maxDepth, int maxFieldsCount) { + this.maxDepth = maxDepth; + this.maxFieldsCount = maxFieldsCount; + } + + /** + * @return the maximum allowed depth for queries, where depth is measured as the number of nested Field nodes + */ + public int getMaxDepth() { + return maxDepth; + } + + /** + * @return the maximum allowed number of fields in a query, counting fields at each fragment spread site + */ + public int getMaxFieldsCount() { + return maxFieldsCount; + } + + /** + * @return a new builder for creating QueryComplexityLimits + */ + public static Builder newLimits() { + return new Builder(); + } + + @Override + public String toString() { + return "QueryComplexityLimits{" + + "maxDepth=" + maxDepth + + ", maxFieldsCount=" + maxFieldsCount + + '}'; + } + + /** + * Builder for QueryComplexityLimits. + */ + @PublicApi + public static class Builder { + private int maxDepth = Integer.MAX_VALUE; + private int maxFieldsCount = Integer.MAX_VALUE; + + private Builder() { + } + + /** + * Sets the maximum allowed depth for queries. + * Depth is measured as the number of nested Field nodes. + * + * @param maxDepth the maximum depth (must be positive) + * @return this builder + */ + public Builder maxDepth(int maxDepth) { + if (maxDepth <= 0) { + throw new IllegalArgumentException("maxDepth must be positive"); + } + this.maxDepth = maxDepth; + return this; + } + + /** + * Sets the maximum allowed number of fields in a query. + * Fields inside fragments are counted at each spread site. + * + * @param maxFieldsCount the maximum field count (must be positive) + * @return this builder + */ + public Builder maxFieldsCount(int maxFieldsCount) { + if (maxFieldsCount <= 0) { + throw new IllegalArgumentException("maxFieldsCount must be positive"); + } + this.maxFieldsCount = maxFieldsCount; + return this; + } + + /** + * @return a new QueryComplexityLimits instance + */ + public QueryComplexityLimits build() { + return new QueryComplexityLimits(maxDepth, maxFieldsCount); + } + } +} diff --git a/src/main/java/graphql/validation/QueryComplexityLimitsExceeded.java b/src/main/java/graphql/validation/QueryComplexityLimitsExceeded.java new file mode 100644 index 0000000000..adaffba2af --- /dev/null +++ b/src/main/java/graphql/validation/QueryComplexityLimitsExceeded.java @@ -0,0 +1,42 @@ +package graphql.validation; + +import graphql.Internal; +import org.jspecify.annotations.NullMarked; + +/** + * Exception thrown when query complexity limits (depth or field count) are exceeded during validation. + * This exception is caught by the Validator and converted to a ValidationError. + */ +@Internal +@NullMarked +public class QueryComplexityLimitsExceeded extends RuntimeException { + + private final ValidationErrorType errorType; + private final int limit; + private final int actual; + + public QueryComplexityLimitsExceeded(ValidationErrorType errorType, int limit, int actual) { + super(errorType.name() + ": limit=" + limit + ", actual=" + actual); + this.errorType = errorType; + this.limit = limit; + this.actual = actual; + } + + public ValidationErrorType getErrorType() { + return errorType; + } + + public int getLimit() { + return limit; + } + + public int getActual() { + return actual; + } + + @Override + public synchronized Throwable fillInStackTrace() { + // No stack trace for performance - this is a control flow exception + return this; + } +} diff --git a/src/main/java/graphql/validation/ValidationContext.java b/src/main/java/graphql/validation/ValidationContext.java index 873783785f..9e88fa0973 100644 --- a/src/main/java/graphql/validation/ValidationContext.java +++ b/src/main/java/graphql/validation/ValidationContext.java @@ -33,13 +33,19 @@ public class ValidationContext { private final Map fragmentDefinitionMap = new LinkedHashMap<>(); private final I18n i18n; private final GraphQLContext graphQLContext; + private final QueryComplexityLimits queryComplexityLimits; public ValidationContext(GraphQLSchema schema, Document document, I18n i18n) { + this(schema, document, i18n, null); + } + + public ValidationContext(GraphQLSchema schema, Document document, I18n i18n, QueryComplexityLimits limits) { this.schema = schema; this.document = document; this.traversalContext = new TraversalContext(schema); this.i18n = i18n; this.graphQLContext = GraphQLContext.newContext().of(Locale.class, i18n.getLocale()).build(); + this.queryComplexityLimits = limits != null ? limits : QueryComplexityLimits.getDefaultLimits(); buildFragmentMap(); } @@ -109,6 +115,10 @@ public GraphQLContext getGraphQLContext() { return graphQLContext; } + public QueryComplexityLimits getQueryComplexityLimits() { + return queryComplexityLimits; + } + /** * Creates an I18N message using the key and arguments * diff --git a/src/main/java/graphql/validation/ValidationErrorType.java b/src/main/java/graphql/validation/ValidationErrorType.java index 59d5c3ac0f..afd75cda54 100644 --- a/src/main/java/graphql/validation/ValidationErrorType.java +++ b/src/main/java/graphql/validation/ValidationErrorType.java @@ -45,5 +45,7 @@ public enum ValidationErrorType implements ValidationErrorClassification { SubscriptionMultipleRootFields, SubscriptionIntrospectionRootField, UniqueObjectFieldName, - UnknownOperation + UnknownOperation, + MaxQueryDepthExceeded, + MaxQueryFieldsExceeded } diff --git a/src/main/java/graphql/validation/Validator.java b/src/main/java/graphql/validation/Validator.java index 654eec5cef..d6237bf131 100644 --- a/src/main/java/graphql/validation/Validator.java +++ b/src/main/java/graphql/validation/Validator.java @@ -37,8 +37,12 @@ public List validateDocument(GraphQLSchema schema, Document doc } public List validateDocument(GraphQLSchema schema, Document document, Predicate rulePredicate, Locale locale) { + return validateDocument(schema, document, rulePredicate, locale, null); + } + + public List validateDocument(GraphQLSchema schema, Document document, Predicate rulePredicate, Locale locale, QueryComplexityLimits limits) { I18n i18n = I18n.i18n(I18n.BundleType.Validation, locale); - ValidationContext validationContext = new ValidationContext(schema, document, i18n); + ValidationContext validationContext = new ValidationContext(schema, document, i18n, limits); ValidationErrorCollector validationErrorCollector = new ValidationErrorCollector(MAX_VALIDATION_ERRORS); OperationValidator operationValidator = new OperationValidator(validationContext, validationErrorCollector, rulePredicate); @@ -47,6 +51,12 @@ public List validateDocument(GraphQLSchema schema, Document doc languageTraversal.traverse(document, operationValidator); } catch (ValidationErrorCollector.MaxValidationErrorsReached ignored) { // if we have generated enough errors, then we can shortcut out + } catch (QueryComplexityLimitsExceeded e) { + String message = i18n.msg(e.getErrorType().name() + ".message", e.getLimit(), e.getActual()); + validationErrorCollector.addError(ValidationError.newValidationError() + .validationErrorType(e.getErrorType()) + .description(message) + .build()); } return validationErrorCollector.getErrors(); diff --git a/src/main/resources/i18n/Validation.properties b/src/main/resources/i18n/Validation.properties index a9403bea5b..8f2ad5715c 100644 --- a/src/main/resources/i18n/Validation.properties +++ b/src/main/resources/i18n/Validation.properties @@ -110,4 +110,7 @@ ArgumentValidationUtil.handleMissingFieldsError=Validation error ({0}) : argumen ArgumentValidationUtil.handleExtraFieldError=Validation error ({0}) : argument ''{1}'' with value ''{2}'' contains a field not in ''{3}'': ''{4}'' # suppress inspection "UnusedProperty" # suppress inspection "UnusedMessageFormatParameter" -ArgumentValidationUtil.extraOneOfFieldsError=Validation error ({0}) : Exactly one key must be specified for OneOf type ''{3}''. \ No newline at end of file +ArgumentValidationUtil.extraOneOfFieldsError=Validation error ({0}) : Exactly one key must be specified for OneOf type ''{3}''. +# +MaxQueryDepthExceeded.message=Query depth {1} exceeds maximum allowed depth {0} +MaxQueryFieldsExceeded.message=Query has {1} fields which exceeds maximum allowed {0} \ No newline at end of file diff --git a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy index c2d9b2dc87..c45686307c 100644 --- a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy +++ b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy @@ -6,6 +6,7 @@ import graphql.TestUtil import graphql.execution.CoercedVariables import graphql.language.Document import graphql.normalized.ExecutableNormalizedOperationFactory +import graphql.validation.QueryComplexityLimits import spock.lang.Specification class GoodFaithIntrospectionTest extends Specification { @@ -14,10 +15,13 @@ class GoodFaithIntrospectionTest extends Specification { def setup() { GoodFaithIntrospection.enabledJvmWide(true) + // Disable validation complexity limits so GoodFaithIntrospection can be tested + QueryComplexityLimits.setDefaultLimits(QueryComplexityLimits.NONE) } def cleanup() { GoodFaithIntrospection.enabledJvmWide(true) + QueryComplexityLimits.setDefaultLimits(QueryComplexityLimits.DEFAULT) } def "standard introspection query is inside limits just in general"() { diff --git a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy index a04c74f954..7463031d49 100644 --- a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy +++ b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationFactoryTest.groovy @@ -20,6 +20,7 @@ import graphql.util.TraversalControl import graphql.util.Traverser import graphql.util.TraverserContext import graphql.util.TraverserVisitorStub +import graphql.validation.QueryComplexityLimits import spock.lang.Specification import java.util.stream.Collectors @@ -33,6 +34,14 @@ import static graphql.schema.FieldCoordinates.coordinates class ExecutableNormalizedOperationFactoryTest extends Specification { static boolean deferSupport + def setup() { + // Disable validation complexity limits so ENO limits can be tested + QueryComplexityLimits.setDefaultLimits(QueryComplexityLimits.NONE) + } + + def cleanup() { + QueryComplexityLimits.setDefaultLimits(QueryComplexityLimits.DEFAULT) + } def "test"() { String schema = """ diff --git a/src/test/groovy/graphql/validation/QueryComplexityLimitsTest.groovy b/src/test/groovy/graphql/validation/QueryComplexityLimitsTest.groovy new file mode 100644 index 0000000000..ece8cde7a4 --- /dev/null +++ b/src/test/groovy/graphql/validation/QueryComplexityLimitsTest.groovy @@ -0,0 +1,458 @@ +package graphql.validation + +import graphql.TestUtil +import graphql.parser.Parser + +class QueryComplexityLimitsTest extends SpecValidationBase { + + // ==================== ENO Parity Tests ==================== + // These tests verify that our complexity tracking matches ExecutableNormalizedOperation (ENO) + + def "ENO parity - depth and field count match ENO calculation"() { + // This test mirrors ExecutableNormalizedOperationFactoryTest."can capture depth and field count" + // ENO reports: depth=7, fieldCount=8 + def schema = TestUtil.schema(""" + type Query { + foo: Foo + } + type Foo { + stop : String + bar : Bar + } + type Bar { + stop : String + foo : Foo + } + """) + + def query = "{ foo { bar { foo { bar { foo { stop bar { stop }}}}}}}" + def document = new Parser().parseDocument(query) + + when: "we set limits that would fail if counts don't match ENO" + // ENO says fieldCount=8, so limit of 7 should fail + def limitsFieldCount = QueryComplexityLimits.newLimits() + .maxFieldsCount(7) + .build() + def errorsFieldCount = new Validator().validateDocument(schema, document, { r -> true }, Locale.ENGLISH, limitsFieldCount) + + then: "field count of 8 exceeds limit of 7" + errorsFieldCount.size() == 1 + errorsFieldCount[0].validationErrorType == ValidationErrorType.MaxQueryFieldsExceeded + errorsFieldCount[0].message.contains("8") + + when: "we set limits that match ENO exactly" + def limitsExact = QueryComplexityLimits.newLimits() + .maxFieldsCount(8) + .maxDepth(7) + .build() + def errorsExact = new Validator().validateDocument(schema, document, { r -> true }, Locale.ENGLISH, limitsExact) + + then: "validation passes with exact ENO counts" + errorsExact.isEmpty() + + when: "depth limit of 6 should fail (ENO says depth=7)" + def limitsDepth = QueryComplexityLimits.newLimits() + .maxDepth(6) + .build() + def errorsDepth = new Validator().validateDocument(schema, document, { r -> true }, Locale.ENGLISH, limitsDepth) + + then: "depth of 7 exceeds limit of 6" + errorsDepth.size() == 1 + errorsDepth[0].validationErrorType == ValidationErrorType.MaxQueryDepthExceeded + errorsDepth[0].message.contains("7") + } + + def "ENO parity - fragment spread counts fields at each site"() { + // This test mirrors ExecutableNormalizedOperationFactoryTest."query with fragment definition" + // Query: {foo { ...fooData moreFoos { ...fooData }}} fragment fooData on Foo { subFoo } + // ENO output: ['Query.foo', 'Foo.subFoo', 'Foo.moreFoos', 'Foo.subFoo'] + // So subFoo is counted TWICE (once per spread) = 4 total fields + def schema = TestUtil.schema(""" + type Query { + foo: Foo + } + type Foo { + subFoo: String + moreFoos: Foo + } + """) + + def query = "{foo { ...fooData moreFoos { ...fooData }}} fragment fooData on Foo { subFoo }" + def document = new Parser().parseDocument(query) + + when: "limit of 3 should fail (ENO counts 4 fields)" + def limits = QueryComplexityLimits.newLimits() + .maxFieldsCount(3) + .build() + def errors = new Validator().validateDocument(schema, document, { r -> true }, Locale.ENGLISH, limits) + + then: + errors.size() == 1 + errors[0].validationErrorType == ValidationErrorType.MaxQueryFieldsExceeded + errors[0].message.contains("4") // foo + subFoo + moreFoos + subFoo = 4 + + when: "limit of 4 should pass" + def limitsPass = QueryComplexityLimits.newLimits() + .maxFieldsCount(4) + .build() + def errorsPass = new Validator().validateDocument(schema, document, { r -> true }, Locale.ENGLISH, limitsPass) + + then: + errorsPass.isEmpty() + } + + def "ENO parity - deeply nested fragments multiply field counts"() { + // Similar to ExecutableNormalizedOperationFactoryTest."factory has a default max node count" + // Each fragment spreads 3 times, creating exponential growth + def schema = TestUtil.schema(""" + type Query { + foo: Foo + } + type Foo { + foo: Foo + name: String + } + """) + + // F1 spreads F2 three times, F2 has just 'name' + // F1 contributes: 3 * F2's fields = 3 * 1 = 3 fields + // Query: foo + F1's fields = 1 + 3 = 4 fields + def query = """ + { foo { ...F1 }} + fragment F1 on Foo { + a: foo { ...F2 } + b: foo { ...F2 } + c: foo { ...F2 } + } + fragment F2 on Foo { + name + } + """ + def document = new Parser().parseDocument(query) + + when: + // foo (1) + a:foo (1) + b:foo (1) + c:foo (1) + name*3 (3) = 7 fields + def limits = QueryComplexityLimits.newLimits() + .maxFieldsCount(6) + .build() + def errors = new Validator().validateDocument(schema, document, { r -> true }, Locale.ENGLISH, limits) + + then: + errors.size() == 1 + errors[0].validationErrorType == ValidationErrorType.MaxQueryFieldsExceeded + errors[0].message.contains("7") + + when: "limit of 7 should pass" + def limitsPass = QueryComplexityLimits.newLimits() + .maxFieldsCount(7) + .build() + def errorsPass = new Validator().validateDocument(schema, document, { r -> true }, Locale.ENGLISH, limitsPass) + + then: + errorsPass.isEmpty() + } + + // ==================== Original Tests ==================== + + def "default limits are applied automatically"() { + expect: + QueryComplexityLimits.DEFAULT.getMaxDepth() == 100 + QueryComplexityLimits.DEFAULT.getMaxFieldsCount() == 100_000 + QueryComplexityLimits.getDefaultLimits() == QueryComplexityLimits.DEFAULT + } + + def "default limits can be changed globally"() { + given: + def originalDefault = QueryComplexityLimits.getDefaultLimits() + + when: "we set custom default limits" + def customLimits = QueryComplexityLimits.newLimits().maxDepth(5).maxFieldsCount(10).build() + QueryComplexityLimits.setDefaultLimits(customLimits) + + then: + QueryComplexityLimits.getDefaultLimits() == customLimits + + when: "we can disable limits globally with NONE" + QueryComplexityLimits.setDefaultLimits(QueryComplexityLimits.NONE) + + then: + QueryComplexityLimits.getDefaultLimits() == QueryComplexityLimits.NONE + + cleanup: + QueryComplexityLimits.setDefaultLimits(originalDefault) + } + + def "simple queries pass with default limits"() { + def query = """ + query deepQuery { + dog { + name + owner { + name + } + } + } + """ + when: + def validationErrors = validate(query) + + then: + validationErrors.isEmpty() + } + + def "NONE disables limits entirely"() { + def schema = TestUtil.schema(""" + type Query { a: A } + type A { b: B } + type B { c: C } + type C { d: String } + """) + // This query has depth 4, which exceeds default of 50? No, 4 < 50. Let me create a deeper one. + // Actually let's just verify NONE works by setting a very low custom limit first, then NONE + def query = "{ a { b { c { d }}}}" + def document = new Parser().parseDocument(query) + + when: "using NONE, no limits are enforced" + def errors = new Validator().validateDocument(schema, document, { r -> true }, Locale.ENGLISH, QueryComplexityLimits.NONE) + + then: + errors.isEmpty() + } + + def "field count limit is enforced"() { + def query = """ + query { + dog { + name + nickname + barkVolume + } + } + """ + when: + def limits = QueryComplexityLimits.newLimits() + .maxFieldsCount(3) + .build() + def document = new Parser().parseDocument(query) + def validationErrors = new Validator().validateDocument( + SpecValidationSchema.specValidationSchema, document, { r -> true }, Locale.ENGLISH, limits) + + then: + validationErrors.size() == 1 + validationErrors[0].validationErrorType == ValidationErrorType.MaxQueryFieldsExceeded + validationErrors[0].message.contains("4") // actual + validationErrors[0].message.contains("3") // limit + } + + def "depth limit is enforced"() { + def query = """ + query { + dog { + owner { + name + } + } + } + """ + when: + def limits = QueryComplexityLimits.newLimits() + .maxDepth(2) + .build() + def document = new Parser().parseDocument(query) + def validationErrors = new Validator().validateDocument( + SpecValidationSchema.specValidationSchema, document, { r -> true }, Locale.ENGLISH, limits) + + then: + validationErrors.size() == 1 + validationErrors[0].validationErrorType == ValidationErrorType.MaxQueryDepthExceeded + validationErrors[0].message.contains("3") // actual depth + validationErrors[0].message.contains("2") // limit + } + + def "fragment fields are counted at each spread site"() { + // Fragment F has 2 fields (name, nickname) + // Query has: dog1, dog2, dog3 = 3 fields + 3 spreads * 2 fields = 9 total fields + def query = """ + fragment F on Dog { name nickname } + query { + dog1: dog { ...F } + dog2: dog { ...F } + dog3: dog { ...F } + } + """ + when: + def limits = QueryComplexityLimits.newLimits() + .maxFieldsCount(8) + .build() + def document = new Parser().parseDocument(query) + def validationErrors = new Validator().validateDocument( + SpecValidationSchema.specValidationSchema, document, { r -> true }, Locale.ENGLISH, limits) + + then: + validationErrors.size() == 1 + validationErrors[0].validationErrorType == ValidationErrorType.MaxQueryFieldsExceeded + validationErrors[0].message.contains("9") // actual + validationErrors[0].message.contains("8") // limit + } + + def "fragment depth adds to current depth"() { + // Query depth: dog at depth 1, fragment adds 1 more (name) = max depth 2 + def query = """ + fragment F on Dog { name } + query { + dog { ...F } + } + """ + when: + def limits = QueryComplexityLimits.newLimits() + .maxDepth(1) + .build() + def document = new Parser().parseDocument(query) + def validationErrors = new Validator().validateDocument( + SpecValidationSchema.specValidationSchema, document, { r -> true }, Locale.ENGLISH, limits) + + then: + validationErrors.size() == 1 + validationErrors[0].validationErrorType == ValidationErrorType.MaxQueryDepthExceeded + } + + def "nested fragments are handled correctly"() { + // Fragment A spreads fragment B, each has 1 field + // Total: dog (1) + A's name (1) + B's nickname (1) = 3 fields + def query = """ + fragment A on Dog { name ...B } + fragment B on Dog { nickname } + query { + dog { ...A } + } + """ + when: + def limits = QueryComplexityLimits.newLimits() + .maxFieldsCount(2) + .build() + def document = new Parser().parseDocument(query) + def validationErrors = new Validator().validateDocument( + SpecValidationSchema.specValidationSchema, document, { r -> true }, Locale.ENGLISH, limits) + + then: + validationErrors.size() == 1 + validationErrors[0].validationErrorType == ValidationErrorType.MaxQueryFieldsExceeded + } + + def "multiple operations each have separate limits"() { + // Each operation should be validated independently + def query = """ + query First { + dog { name } + } + query Second { + dog { name nickname barkVolume } + } + """ + when: + def limits = QueryComplexityLimits.newLimits() + .maxFieldsCount(3) + .build() + def document = new Parser().parseDocument(query) + def validationErrors = new Validator().validateDocument( + SpecValidationSchema.specValidationSchema, document, { r -> true }, Locale.ENGLISH, limits) + + then: + // Second operation has 4 fields (dog + 3 scalar fields), which exceeds limit of 3 + validationErrors.size() == 1 + validationErrors[0].validationErrorType == ValidationErrorType.MaxQueryFieldsExceeded + } + + def "inline fragments count their fields"() { + def query = """ + query { + dog { + ... on Dog { + name + nickname + } + } + } + """ + when: + def limits = QueryComplexityLimits.newLimits() + .maxFieldsCount(2) + .build() + def document = new Parser().parseDocument(query) + def validationErrors = new Validator().validateDocument( + SpecValidationSchema.specValidationSchema, document, { r -> true }, Locale.ENGLISH, limits) + + then: + // dog (1) + name (1) + nickname (1) = 3 fields + validationErrors.size() == 1 + validationErrors[0].validationErrorType == ValidationErrorType.MaxQueryFieldsExceeded + } + + def "passes when within limits"() { + def query = """ + query { + dog { + name + owner { + name + } + } + } + """ + when: + def limits = QueryComplexityLimits.newLimits() + .maxFieldsCount(10) + .maxDepth(5) + .build() + def document = new Parser().parseDocument(query) + def validationErrors = new Validator().validateDocument( + SpecValidationSchema.specValidationSchema, document, { r -> true }, Locale.ENGLISH, limits) + + then: + validationErrors.isEmpty() + } + + def "QueryComplexityLimits.NONE has no limits"() { + expect: + QueryComplexityLimits.NONE.getMaxDepth() == Integer.MAX_VALUE + QueryComplexityLimits.NONE.getMaxFieldsCount() == Integer.MAX_VALUE + } + + def "builder validates positive values"() { + when: + QueryComplexityLimits.newLimits().maxDepth(0).build() + + then: + thrown(IllegalArgumentException) + + when: + QueryComplexityLimits.newLimits().maxFieldsCount(-1).build() + + then: + thrown(IllegalArgumentException) + } + + def "cyclic fragments don't cause infinite loop in complexity calculation"() { + // This query has a cycle: A -> B -> A + // The validation should detect the cycle error, but complexity calculation shouldn't hang + def query = """ + fragment A on Dog { ...B } + fragment B on Dog { ...A } + query { + dog { ...A } + } + """ + when: + def limits = QueryComplexityLimits.newLimits() + .maxFieldsCount(100) + .maxDepth(100) + .build() + def document = new Parser().parseDocument(query) + def validationErrors = new Validator().validateDocument( + SpecValidationSchema.specValidationSchema, document, { r -> true }, Locale.ENGLISH, limits) + + then: + // Should get fragment cycle error, not hang + validationErrors.any { it.validationErrorType == ValidationErrorType.FragmentCycle } + } +} From c496ac16bb0435f9748a32a6d9b45357f9628020 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Fri, 13 Feb 2026 10:29:34 +1000 Subject: [PATCH 02/11] Refactor GoodFaithIntrospection to use validation instead of ENO 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 --- src/main/java/graphql/GraphQL.java | 32 +++- .../introspection/GoodFaithIntrospection.java | 142 +++++++++--------- .../graphql/introspection/Introspection.java | 5 - .../GoodFaithIntrospectionExceeded.java | 45 ++++++ .../validation/OperationValidationRule.java | 3 + .../validation/OperationValidator.java | 47 ++++++ .../validation/QueryComplexityLimits.java | 1 + .../archunit/JSpecifyAnnotationsCheck.groovy | 1 - .../GoodFaithIntrospectionTest.groovy | 4 - 9 files changed, 198 insertions(+), 82 deletions(-) create mode 100644 src/main/java/graphql/validation/GoodFaithIntrospectionExceeded.java diff --git a/src/main/java/graphql/GraphQL.java b/src/main/java/graphql/GraphQL.java index 14bf273a53..898a092c5c 100644 --- a/src/main/java/graphql/GraphQL.java +++ b/src/main/java/graphql/GraphQL.java @@ -23,11 +23,14 @@ import graphql.execution.preparsed.NoOpPreparsedDocumentProvider; import graphql.execution.preparsed.PreparsedDocumentEntry; import graphql.execution.preparsed.PreparsedDocumentProvider; +import graphql.introspection.GoodFaithIntrospection; import graphql.language.Document; import graphql.schema.GraphQLSchema; +import graphql.validation.GoodFaithIntrospectionExceeded; import graphql.validation.OperationValidationRule; import graphql.validation.QueryComplexityLimits; import graphql.validation.ValidationError; +import graphql.validation.ValidationErrorType; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.NullUnmarked; @@ -568,7 +571,12 @@ private PreparsedDocumentEntry parseAndValidate(AtomicReference executionInput = executionInput.transform(builder -> builder.variables(parseResult.getVariables())); executionInputRef.set(executionInput); - final List errors = validate(executionInput, assertNotNull(document, "Document cannot be null when parse succeeded"), graphQLSchema, instrumentationState); + final List errors; + try { + errors = validate(executionInput, assertNotNull(document, "Document cannot be null when parse succeeded"), graphQLSchema, instrumentationState); + } catch (GoodFaithIntrospectionExceeded e) { + return new PreparsedDocumentEntry(document, List.of(e.toBadFaithError())); + } if (!errors.isEmpty()) { return new PreparsedDocumentEntry(document, errors); } @@ -603,8 +611,30 @@ private List validate(ExecutionInput executionInput, Document d Predicate validationRulePredicate = executionInput.getGraphQLContext().getOrDefault(ParseAndValidate.INTERNAL_VALIDATION_PREDICATE_HINT, r -> true); Locale locale = executionInput.getLocale() != null ? executionInput.getLocale() : Locale.getDefault(); QueryComplexityLimits limits = executionInput.getGraphQLContext().get(QueryComplexityLimits.KEY); + + // Good Faith Introspection: apply tighter limits and enable the rule for introspection queries + boolean goodFaithActive = GoodFaithIntrospection.isEnabled(executionInput.getGraphQLContext()) + && GoodFaithIntrospection.containsIntrospectionFields(document); + if (goodFaithActive) { + limits = GoodFaithIntrospection.goodFaithLimits(limits); + } else { + Predicate existing = validationRulePredicate; + validationRulePredicate = rule -> rule != OperationValidationRule.GOOD_FAITH_INTROSPECTION && existing.test(rule); + } + List validationErrors = ParseAndValidate.validate(graphQLSchema, document, validationRulePredicate, locale, limits); + // If good faith is active and a complexity limit error was produced, convert it to a bad faith error + if (goodFaithActive) { + for (ValidationError error : validationErrors) { + if (error.getValidationErrorType() == ValidationErrorType.MaxQueryFieldsExceeded + || error.getValidationErrorType() == ValidationErrorType.MaxQueryDepthExceeded) { + validationCtx.onCompleted(null, null); + throw GoodFaithIntrospectionExceeded.tooBigOperation(error.getDescription()); + } + } + } + validationCtx.onCompleted(validationErrors, null); return validationErrors; } diff --git a/src/main/java/graphql/introspection/GoodFaithIntrospection.java b/src/main/java/graphql/introspection/GoodFaithIntrospection.java index ae7da12569..ae9747568b 100644 --- a/src/main/java/graphql/introspection/GoodFaithIntrospection.java +++ b/src/main/java/graphql/introspection/GoodFaithIntrospection.java @@ -1,31 +1,25 @@ package graphql.introspection; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableListMultimap; import graphql.ErrorClassification; -import graphql.ExecutionResult; import graphql.GraphQLContext; import graphql.GraphQLError; import graphql.PublicApi; -import graphql.execution.AbortExecutionException; -import graphql.execution.ExecutionContext; +import graphql.language.Definition; +import graphql.language.Document; +import graphql.language.Field; +import graphql.language.OperationDefinition; +import graphql.language.Selection; +import graphql.language.SelectionSet; import graphql.language.SourceLocation; -import graphql.normalized.ExecutableNormalizedField; -import graphql.normalized.ExecutableNormalizedOperation; -import graphql.schema.FieldCoordinates; +import graphql.validation.QueryComplexityLimits; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; import java.util.List; -import java.util.Map; -import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; -import static graphql.normalized.ExecutableNormalizedOperationFactory.Options; -import static graphql.normalized.ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation; -import static graphql.schema.FieldCoordinates.coordinates; - /** - * This {@link graphql.execution.instrumentation.Instrumentation} ensure that a submitted introspection query is done in - * good faith. + * Good Faith Introspection ensures that introspection queries are not abused to cause denial of service. *

* There are attack vectors where a crafted introspection query can cause the engine to spend too much time * producing introspection data. This is especially true on large schemas with lots of types and fields. @@ -33,11 +27,18 @@ * Schemas form a cyclic graph and hence it's possible to send in introspection queries that can reference those cycles * and in large schemas this can be expensive and perhaps a "denial of service". *

- * This instrumentation only allows one __schema field or one __type field to be present, and it does not allow the `__Type` fields - * to form a cycle, i.e., that can only be present once. This allows the standard and common introspection queries to work - * so tooling such as graphiql can work. + * When enabled, the validation layer enforces that: + *

    + *
  • Only one {@code __schema} and one {@code __type} field can appear per operation
    • + *
  • The {@code __Type} fields {@code fields}, {@code inputFields}, {@code interfaces}, and {@code possibleTypes} + * can each only appear once (preventing cyclic traversals)
    • + *
  • The query complexity is limited to {@link #GOOD_FAITH_MAX_FIELDS_COUNT} fields and + * {@link #GOOD_FAITH_MAX_DEPTH_COUNT} depth
    • + *
+ * This allows the standard and common introspection queries to work so tooling such as graphiql can work. */ @PublicApi +@NullMarked public class GoodFaithIntrospection { /** @@ -74,67 +75,66 @@ public static boolean enabledJvmWide(boolean flag) { return ENABLED_STATE.getAndSet(flag); } - private static final Map ALLOWED_FIELD_INSTANCES = Map.of( - coordinates("Query", "__schema"), 1 - , coordinates("Query", "__type"), 1 - - , coordinates("__Type", "fields"), 1 - , coordinates("__Type", "inputFields"), 1 - , coordinates("__Type", "interfaces"), 1 - , coordinates("__Type", "possibleTypes"), 1 - ); - - public static Optional checkIntrospection(ExecutionContext executionContext) { - if (isIntrospectionEnabled(executionContext.getGraphQLContext())) { - ExecutableNormalizedOperation operation; - try { - operation = mkOperation(executionContext); - } catch (AbortExecutionException e) { - BadFaithIntrospectionError error = BadFaithIntrospectionError.tooBigOperation(e.getMessage()); - return Optional.of(ExecutionResult.newExecutionResult().addError(error).build()); - } - ImmutableListMultimap coordinatesToENFs = operation.getCoordinatesToNormalizedFields(); - for (Map.Entry entry : ALLOWED_FIELD_INSTANCES.entrySet()) { - FieldCoordinates coordinates = entry.getKey(); - Integer allowSize = entry.getValue(); - ImmutableList normalizedFields = coordinatesToENFs.get(coordinates); - if (normalizedFields.size() > allowSize) { - BadFaithIntrospectionError error = BadFaithIntrospectionError.tooManyFields(coordinates.toString()); - return Optional.of(ExecutionResult.newExecutionResult().addError(error).build()); - } - } + /** + * Checks whether Good Faith Introspection is enabled for the given request context. + * + * @param graphQLContext the per-request context + * + * @return true if good faith introspection checks should be applied + */ + public static boolean isEnabled(GraphQLContext graphQLContext) { + if (!isEnabledJvmWide()) { + return false; } - return Optional.empty(); + return !graphQLContext.getBoolean(GOOD_FAITH_INTROSPECTION_DISABLED, false); } /** - * This makes an executable operation limited in size then which suits a good faith introspection query. This helps guard - * against malicious queries. + * Performs a shallow scan of the document to check if any operation's top-level selections + * contain introspection fields ({@code __schema} or {@code __type}). * - * @param executionContext the execution context + * @param document the parsed document * - * @return an executable operation + * @return true if the document contains top-level introspection fields */ - private static ExecutableNormalizedOperation mkOperation(ExecutionContext executionContext) throws AbortExecutionException { - Options options = Options.defaultOptions() - .maxFieldsCount(GOOD_FAITH_MAX_FIELDS_COUNT) - .maxChildrenDepth(GOOD_FAITH_MAX_DEPTH_COUNT) - .locale(executionContext.getLocale()) - .graphQLContext(executionContext.getGraphQLContext()); - - return createExecutableNormalizedOperation(executionContext.getGraphQLSchema(), - executionContext.getOperationDefinition(), - executionContext.getFragmentsByName(), - executionContext.getCoercedVariables(), - options); - + public static boolean containsIntrospectionFields(Document document) { + for (Definition definition : document.getDefinitions()) { + if (definition instanceof OperationDefinition) { + SelectionSet selectionSet = ((OperationDefinition) definition).getSelectionSet(); + if (selectionSet != null) { + for (Selection selection : selectionSet.getSelections()) { + if (selection instanceof Field) { + String name = ((Field) selection).getName(); + if ("__schema".equals(name) || "__type".equals(name)) { + return true; + } + } + } + } + } + } + return false; } - private static boolean isIntrospectionEnabled(GraphQLContext graphQlContext) { - if (!isEnabledJvmWide()) { - return false; + /** + * Returns query complexity limits that are the minimum of the existing limits and the + * good faith introspection limits. This ensures introspection queries are bounded + * without overriding tighter user-specified limits. + * + * @param existing the existing complexity limits (may be null, in which case defaults are used) + * + * @return complexity limits with good faith bounds applied + */ + public static QueryComplexityLimits goodFaithLimits(QueryComplexityLimits existing) { + if (existing == null) { + existing = QueryComplexityLimits.getDefaultLimits(); } - return !graphQlContext.getBoolean(GOOD_FAITH_INTROSPECTION_DISABLED, false); + int maxFields = Math.min(existing.getMaxFieldsCount(), GOOD_FAITH_MAX_FIELDS_COUNT); + int maxDepth = Math.min(existing.getMaxDepth(), GOOD_FAITH_MAX_DEPTH_COUNT); + return QueryComplexityLimits.newLimits() + .maxFieldsCount(maxFields) + .maxDepth(maxDepth) + .build(); } public static class BadFaithIntrospectionError implements GraphQLError { @@ -163,7 +163,7 @@ public ErrorClassification getErrorType() { } @Override - public List getLocations() { + public @Nullable List getLocations() { return null; } diff --git a/src/main/java/graphql/introspection/Introspection.java b/src/main/java/graphql/introspection/Introspection.java index a455ec9d78..e8c7173e68 100644 --- a/src/main/java/graphql/introspection/Introspection.java +++ b/src/main/java/graphql/introspection/Introspection.java @@ -116,7 +116,6 @@ public static boolean isEnabledJvmWide() { public static Optional isIntrospectionSensible(MergedSelectionSet mergedSelectionSet, ExecutionContext executionContext) { GraphQLContext graphQLContext = executionContext.getGraphQLContext(); - boolean isIntrospection = false; for (String key : mergedSelectionSet.getKeys()) { String fieldName = mergedSelectionSet.getSubField(key).getName(); if (fieldName.equals(SchemaMetaFieldDef.getName()) @@ -124,13 +123,9 @@ public static Optional isIntrospectionSensible(MergedSelectionS if (!isIntrospectionEnabled(graphQLContext)) { return mkDisabledError(mergedSelectionSet.getSubField(key)); } - isIntrospection = true; break; } } - if (isIntrospection) { - return GoodFaithIntrospection.checkIntrospection(executionContext); - } return Optional.empty(); } diff --git a/src/main/java/graphql/validation/GoodFaithIntrospectionExceeded.java b/src/main/java/graphql/validation/GoodFaithIntrospectionExceeded.java new file mode 100644 index 0000000000..8da4f09d4a --- /dev/null +++ b/src/main/java/graphql/validation/GoodFaithIntrospectionExceeded.java @@ -0,0 +1,45 @@ +package graphql.validation; + +import graphql.Internal; +import graphql.introspection.GoodFaithIntrospection; +import org.jspecify.annotations.NullMarked; + +/** + * Exception thrown when a good-faith introspection check fails during validation. + * This exception is NOT caught by the Validator — it propagates up to GraphQL.parseAndValidate() + * where it is converted to a {@link GoodFaithIntrospection.BadFaithIntrospectionError}. + */ +@Internal +@NullMarked +public class GoodFaithIntrospectionExceeded extends RuntimeException { + + private final boolean tooBig; + private final String detail; + + private GoodFaithIntrospectionExceeded(boolean tooBig, String detail) { + super(detail); + this.tooBig = tooBig; + this.detail = detail; + } + + public static GoodFaithIntrospectionExceeded tooManyFields(String fieldCoordinate) { + return new GoodFaithIntrospectionExceeded(false, fieldCoordinate); + } + + public static GoodFaithIntrospectionExceeded tooBigOperation(String message) { + return new GoodFaithIntrospectionExceeded(true, message); + } + + public GoodFaithIntrospection.BadFaithIntrospectionError toBadFaithError() { + if (tooBig) { + return GoodFaithIntrospection.BadFaithIntrospectionError.tooBigOperation(detail); + } + return GoodFaithIntrospection.BadFaithIntrospectionError.tooManyFields(detail); + } + + @Override + public synchronized Throwable fillInStackTrace() { + // No stack trace for performance - this is a control flow exception + return this; + } +} diff --git a/src/main/java/graphql/validation/OperationValidationRule.java b/src/main/java/graphql/validation/OperationValidationRule.java index d5645aa5af..aa4214f0d9 100644 --- a/src/main/java/graphql/validation/OperationValidationRule.java +++ b/src/main/java/graphql/validation/OperationValidationRule.java @@ -184,4 +184,7 @@ public enum OperationValidationRule { /** Defer directive must not be used in subscription operations. Requires operation context. */ DEFER_DIRECTIVE_ON_VALID_OPERATION, + + /** Good faith introspection check. */ + GOOD_FAITH_INTROSPECTION, } diff --git a/src/main/java/graphql/validation/OperationValidator.java b/src/main/java/graphql/validation/OperationValidator.java index 34f8602d7a..75c1ce53cc 100644 --- a/src/main/java/graphql/validation/OperationValidator.java +++ b/src/main/java/graphql/validation/OperationValidator.java @@ -338,6 +338,9 @@ public class OperationValidator implements DocumentVisitor { // Max depth seen during current fragment traversal (for calculating fragment's internal depth) private int fragmentTraversalMaxDepth = 0; + // --- State: Good Faith Introspection --- + private final Map introspectionFieldCounts = new HashMap<>(); + // --- Track whether we're in a context where fragment spread rules should run --- // fragmentRetraversalDepth == 0 means we're NOT inside a manually-traversed fragment => run non-fragment-spread checks // operationScope means we're inside an operation => can trigger fragment traversal @@ -603,6 +606,49 @@ private void checkField(Field field) { validateUniqueDirectiveNamesPerLocation(field, field.getDirectives()); } } + // Good Faith Introspection: runs during fragment spread traversal too (operationScope) + if (operationScope && isRuleEnabled(OperationValidationRule.GOOD_FAITH_INTROSPECTION)) { + checkGoodFaithIntrospection(field); + } + } + + // --- GoodFaithIntrospection --- + private void checkGoodFaithIntrospection(Field field) { + GraphQLCompositeType parentType = validationContext.getParentType(); + if (parentType == null) { + return; + } + String fieldName = field.getName(); + String key = null; + + // Check query-level introspection fields (__schema, __type). + // Only counted at the structural level (not during fragment traversal) to match ENO merging + // behavior where the same field from a direct selection and a fragment spread merge into one. + if (shouldRunNonFragmentSpreadChecks()) { + GraphQLObjectType queryType = validationContext.getSchema().getQueryType(); + if (queryType != null && parentType.getName().equals(queryType.getName())) { + if ("__schema".equals(fieldName) || "__type".equals(fieldName)) { + key = parentType.getName() + "." + fieldName; + } + } + } + + // Check __Type fields that can form cycles. + // Counted during ALL traversals (including fragment spreads) because each occurrence + // at a different depth represents a separate cycle risk. + if ("__Type".equals(parentType.getName())) { + if ("fields".equals(fieldName) || "inputFields".equals(fieldName) + || "interfaces".equals(fieldName) || "possibleTypes".equals(fieldName)) { + key = "__Type." + fieldName; + } + } + + if (key != null) { + int count = introspectionFieldCounts.merge(key, 1, Integer::sum); + if (count > 1) { + throw GoodFaithIntrospectionExceeded.tooManyFields(key); + } + } } private void checkInlineFragment(InlineFragment inlineFragment) { @@ -815,6 +861,7 @@ private void leaveOperationDefinition() { currentFieldDepth = 0; maxFieldDepthSeen = 0; fragmentTraversalMaxDepth = 0; + introspectionFieldCounts.clear(); } private void leaveSelectionSet() { diff --git a/src/main/java/graphql/validation/QueryComplexityLimits.java b/src/main/java/graphql/validation/QueryComplexityLimits.java index 8c4e0f4ad6..2ba7bac830 100644 --- a/src/main/java/graphql/validation/QueryComplexityLimits.java +++ b/src/main/java/graphql/validation/QueryComplexityLimits.java @@ -123,6 +123,7 @@ public String toString() { * Builder for QueryComplexityLimits. */ @PublicApi + @NullMarked public static class Builder { private int maxDepth = Integer.MAX_VALUE; private int maxFieldsCount = Integer.MAX_VALUE; diff --git a/src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy b/src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy index 26eaf56493..3a7cfa3457 100644 --- a/src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy +++ b/src/test/groovy/graphql/archunit/JSpecifyAnnotationsCheck.groovy @@ -86,7 +86,6 @@ class JSpecifyAnnotationsCheck extends Specification { "graphql.incremental.IncrementalExecutionResultImpl", "graphql.incremental.IncrementalPayload", "graphql.incremental.StreamPayload", - "graphql.introspection.GoodFaithIntrospection", "graphql.introspection.Introspection", "graphql.introspection.IntrospectionQuery", "graphql.introspection.IntrospectionQueryBuilder", diff --git a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy index c45686307c..c2d9b2dc87 100644 --- a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy +++ b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy @@ -6,7 +6,6 @@ import graphql.TestUtil import graphql.execution.CoercedVariables import graphql.language.Document import graphql.normalized.ExecutableNormalizedOperationFactory -import graphql.validation.QueryComplexityLimits import spock.lang.Specification class GoodFaithIntrospectionTest extends Specification { @@ -15,13 +14,10 @@ class GoodFaithIntrospectionTest extends Specification { def setup() { GoodFaithIntrospection.enabledJvmWide(true) - // Disable validation complexity limits so GoodFaithIntrospection can be tested - QueryComplexityLimits.setDefaultLimits(QueryComplexityLimits.NONE) } def cleanup() { GoodFaithIntrospection.enabledJvmWide(true) - QueryComplexityLimits.setDefaultLimits(QueryComplexityLimits.DEFAULT) } def "standard introspection query is inside limits just in general"() { From 97d10d2cf8f1053fb4db0ca78202208e47b100a5 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Mon, 23 Mar 2026 07:44:43 +1000 Subject: [PATCH 03/11] Fix NullAway errors and adapt to master's naming conventions after rebase - Add @Nullable annotations for QueryComplexityLimits parameters - Replace shouldRunNonFragmentSpreadChecks() with shouldRunDocumentLevelRules() - Replace fragmentSpreadVisitDepth with fragmentRetraversalDepth Co-Authored-By: Claude Opus 4.6 (1M context) --- src/main/java/graphql/ParseAndValidate.java | 3 ++- .../java/graphql/introspection/GoodFaithIntrospection.java | 2 +- src/main/java/graphql/validation/OperationValidator.java | 2 +- src/main/java/graphql/validation/ValidationContext.java | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/graphql/ParseAndValidate.java b/src/main/java/graphql/ParseAndValidate.java index 26c02db19c..05c2556ee8 100644 --- a/src/main/java/graphql/ParseAndValidate.java +++ b/src/main/java/graphql/ParseAndValidate.java @@ -12,6 +12,7 @@ import graphql.validation.Validator; import org.jspecify.annotations.NonNull; import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; import java.util.List; import java.util.Locale; @@ -133,7 +134,7 @@ public static List validate(@NonNull GraphQLSchema graphQLSchem * * @return a result object that indicates how this operation went */ - public static List validate(@NonNull GraphQLSchema graphQLSchema, @NonNull Document parsedDocument, @NonNull Predicate rulePredicate, @NonNull Locale locale, QueryComplexityLimits limits) { + public static List validate(@NonNull GraphQLSchema graphQLSchema, @NonNull Document parsedDocument, @NonNull Predicate rulePredicate, @NonNull Locale locale, @Nullable QueryComplexityLimits limits) { Validator validator = new Validator(); return validator.validateDocument(graphQLSchema, parsedDocument, rulePredicate, locale, limits); } diff --git a/src/main/java/graphql/introspection/GoodFaithIntrospection.java b/src/main/java/graphql/introspection/GoodFaithIntrospection.java index ae9747568b..3809571fe7 100644 --- a/src/main/java/graphql/introspection/GoodFaithIntrospection.java +++ b/src/main/java/graphql/introspection/GoodFaithIntrospection.java @@ -125,7 +125,7 @@ public static boolean containsIntrospectionFields(Document document) { * * @return complexity limits with good faith bounds applied */ - public static QueryComplexityLimits goodFaithLimits(QueryComplexityLimits existing) { + public static QueryComplexityLimits goodFaithLimits(@Nullable QueryComplexityLimits existing) { if (existing == null) { existing = QueryComplexityLimits.getDefaultLimits(); } diff --git a/src/main/java/graphql/validation/OperationValidator.java b/src/main/java/graphql/validation/OperationValidator.java index 75c1ce53cc..9ee3b9f859 100644 --- a/src/main/java/graphql/validation/OperationValidator.java +++ b/src/main/java/graphql/validation/OperationValidator.java @@ -624,7 +624,7 @@ private void checkGoodFaithIntrospection(Field field) { // Check query-level introspection fields (__schema, __type). // Only counted at the structural level (not during fragment traversal) to match ENO merging // behavior where the same field from a direct selection and a fragment spread merge into one. - if (shouldRunNonFragmentSpreadChecks()) { + if (shouldRunDocumentLevelRules()) { GraphQLObjectType queryType = validationContext.getSchema().getQueryType(); if (queryType != null && parentType.getName().equals(queryType.getName())) { if ("__schema".equals(fieldName) || "__type".equals(fieldName)) { diff --git a/src/main/java/graphql/validation/ValidationContext.java b/src/main/java/graphql/validation/ValidationContext.java index 9e88fa0973..9440931027 100644 --- a/src/main/java/graphql/validation/ValidationContext.java +++ b/src/main/java/graphql/validation/ValidationContext.java @@ -39,7 +39,7 @@ public ValidationContext(GraphQLSchema schema, Document document, I18n i18n) { this(schema, document, i18n, null); } - public ValidationContext(GraphQLSchema schema, Document document, I18n i18n, QueryComplexityLimits limits) { + public ValidationContext(GraphQLSchema schema, Document document, I18n i18n, @Nullable QueryComplexityLimits limits) { this.schema = schema; this.document = document; this.traversalContext = new TraversalContext(schema); From 455c7c8767a1672af51dcaf7716b229cb60ebf19 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Mon, 23 Mar 2026 08:00:25 +1000 Subject: [PATCH 04/11] Fix benchmark test to use unlimited QueryComplexityLimits 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) --- .../OverlappingFieldsCanBeMergedBenchmarkTest.groovy | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/test/groovy/graphql/validation/OverlappingFieldsCanBeMergedBenchmarkTest.groovy b/src/test/groovy/graphql/validation/OverlappingFieldsCanBeMergedBenchmarkTest.groovy index 1e2cdbbe33..af09868cc6 100644 --- a/src/test/groovy/graphql/validation/OverlappingFieldsCanBeMergedBenchmarkTest.groovy +++ b/src/test/groovy/graphql/validation/OverlappingFieldsCanBeMergedBenchmarkTest.groovy @@ -53,7 +53,7 @@ class OverlappingFieldsCanBeMergedBenchmarkTest extends Specification { private List validateQuery(GraphQLSchema schema, Document document) { ValidationErrorCollector errorCollector = new ValidationErrorCollector() I18n i18n = I18n.i18n(I18n.BundleType.Validation, Locale.ENGLISH) - ValidationContext validationContext = new ValidationContext(schema, document, i18n) + ValidationContext validationContext = new ValidationContext(schema, document, i18n, QueryComplexityLimits.NONE) OperationValidator operationValidator = new OperationValidator(validationContext, errorCollector, { r -> r == OperationValidationRule.OVERLAPPING_FIELDS_CAN_BE_MERGED }) LanguageTraversal languageTraversal = new LanguageTraversal() @@ -74,7 +74,11 @@ class OverlappingFieldsCanBeMergedBenchmarkTest extends Specification { def "large schema query executes without errors"() { when: GraphQL graphQL = GraphQL.newGraphQL(schema).build() - ExecutionResult executionResult = graphQL.execute(loadResource("large-schema-4-query.graphql")) + def executionInput = graphql.ExecutionInput.newExecutionInput() + .query(loadResource("large-schema-4-query.graphql")) + .graphQLContext([(QueryComplexityLimits.KEY): QueryComplexityLimits.NONE]) + .build() + ExecutionResult executionResult = graphQL.execute(executionInput) then: executionResult.errors.size() == 0 From deb35beccc6668bdd9735f97c15bfb4e063a2d4a Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Mon, 23 Mar 2026 08:32:26 +1000 Subject: [PATCH 05/11] Add tests for tooBigOperation good faith introspection path 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) --- .../GoodFaithIntrospectionTest.groovy | 72 +++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy index c2d9b2dc87..479a549501 100644 --- a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy +++ b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy @@ -6,6 +6,7 @@ import graphql.TestUtil import graphql.execution.CoercedVariables import graphql.language.Document import graphql.normalized.ExecutableNormalizedOperationFactory +import graphql.validation.QueryComplexityLimits import spock.lang.Specification class GoodFaithIntrospectionTest extends Specification { @@ -188,6 +189,77 @@ class GoodFaithIntrospectionTest extends Specification { 100 | GoodFaithIntrospection.BadFaithIntrospectionError.class } + def "good faith limits are applied on top of custom user limits"() { + given: + def limits = QueryComplexityLimits.newLimits().maxFieldsCount(200).maxDepth(15).build() + def executionInput = ExecutionInput.newExecutionInput(IntrospectionQuery.INTROSPECTION_QUERY) + .graphQLContext([(QueryComplexityLimits.KEY): limits]) + .build() + + when: + ExecutionResult er = graphql.execute(executionInput) + + then: + er.errors.isEmpty() + } + + def "containsIntrospectionFields handles operation with no selection set"() { + given: + def op = graphql.language.OperationDefinition.newOperationDefinition() + .name("empty") + .operation(graphql.language.OperationDefinition.Operation.QUERY) + .build() + def doc = Document.newDocument().definition(op).build() + + expect: + !GoodFaithIntrospection.containsIntrospectionFields(doc) + } + + def "introspection query exceeding field count limit is detected as bad faith"() { + given: + // Build a wide introspection query that exceeds GOOD_FAITH_MAX_FIELDS_COUNT (500) + // using non-cycle-forming fields (aliases of 'name') so the tooManyFields check + // does not fire first, exercising the tooBigOperation code path instead + def sb = new StringBuilder() + sb.append("query { __schema { types { ") + for (int i = 0; i < 510; i++) { + sb.append("a${i}: name ") + } + sb.append("} } }") + + when: + ExecutionResult er = graphql.execute(sb.toString()) + + then: + !er.errors.isEmpty() + er.errors[0] instanceof GoodFaithIntrospection.BadFaithIntrospectionError + er.errors[0].message.contains("too big") + } + + def "introspection query exceeding depth limit is detected as bad faith"() { + given: + // Build a deep introspection query using ofType (not a cycle-forming field) + // that exceeds GOOD_FAITH_MAX_DEPTH_COUNT (20) + def sb = new StringBuilder() + sb.append("query { __schema { types { ") + for (int i = 0; i < 20; i++) { + sb.append("ofType { ") + } + sb.append("name ") + for (int i = 0; i < 20; i++) { + sb.append("} ") + } + sb.append("} } }") + + when: + ExecutionResult er = graphql.execute(sb.toString()) + + then: + !er.errors.isEmpty() + er.errors[0] instanceof GoodFaithIntrospection.BadFaithIntrospectionError + er.errors[0].message.contains("too big") + } + String createDeepQuery(int depth = 25) { def result = """ query test { From 901dafc476e0a2f7379b1f275d86db8343c76175 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Mon, 23 Mar 2026 09:35:16 +1000 Subject: [PATCH 06/11] Detect introspection queries dynamically during validation 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) --- src/main/java/graphql/GraphQL.java | 22 ++------- .../introspection/GoodFaithIntrospection.java | 38 +-------------- .../validation/OperationValidator.java | 16 ++++++- .../GoodFaithIntrospectionTest.groovy | 47 ++++++++++++++----- 4 files changed, 54 insertions(+), 69 deletions(-) diff --git a/src/main/java/graphql/GraphQL.java b/src/main/java/graphql/GraphQL.java index 898a092c5c..d3c7e45f9f 100644 --- a/src/main/java/graphql/GraphQL.java +++ b/src/main/java/graphql/GraphQL.java @@ -30,7 +30,6 @@ import graphql.validation.OperationValidationRule; import graphql.validation.QueryComplexityLimits; import graphql.validation.ValidationError; -import graphql.validation.ValidationErrorType; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.NullUnmarked; @@ -609,32 +608,17 @@ private List validate(ExecutionInput executionInput, Document d validationCtx.onDispatched(); Predicate validationRulePredicate = executionInput.getGraphQLContext().getOrDefault(ParseAndValidate.INTERNAL_VALIDATION_PREDICATE_HINT, r -> true); - Locale locale = executionInput.getLocale() != null ? executionInput.getLocale() : Locale.getDefault(); + Locale locale = executionInput.getLocale(); QueryComplexityLimits limits = executionInput.getGraphQLContext().get(QueryComplexityLimits.KEY); - // Good Faith Introspection: apply tighter limits and enable the rule for introspection queries - boolean goodFaithActive = GoodFaithIntrospection.isEnabled(executionInput.getGraphQLContext()) - && GoodFaithIntrospection.containsIntrospectionFields(document); - if (goodFaithActive) { - limits = GoodFaithIntrospection.goodFaithLimits(limits); - } else { + // Good Faith Introspection: disable the rule if good faith is off + if (!GoodFaithIntrospection.isEnabled(executionInput.getGraphQLContext())) { Predicate existing = validationRulePredicate; validationRulePredicate = rule -> rule != OperationValidationRule.GOOD_FAITH_INTROSPECTION && existing.test(rule); } List validationErrors = ParseAndValidate.validate(graphQLSchema, document, validationRulePredicate, locale, limits); - // If good faith is active and a complexity limit error was produced, convert it to a bad faith error - if (goodFaithActive) { - for (ValidationError error : validationErrors) { - if (error.getValidationErrorType() == ValidationErrorType.MaxQueryFieldsExceeded - || error.getValidationErrorType() == ValidationErrorType.MaxQueryDepthExceeded) { - validationCtx.onCompleted(null, null); - throw GoodFaithIntrospectionExceeded.tooBigOperation(error.getDescription()); - } - } - } - validationCtx.onCompleted(validationErrors, null); return validationErrors; } diff --git a/src/main/java/graphql/introspection/GoodFaithIntrospection.java b/src/main/java/graphql/introspection/GoodFaithIntrospection.java index 3809571fe7..2d40425cee 100644 --- a/src/main/java/graphql/introspection/GoodFaithIntrospection.java +++ b/src/main/java/graphql/introspection/GoodFaithIntrospection.java @@ -4,12 +4,6 @@ import graphql.GraphQLContext; import graphql.GraphQLError; import graphql.PublicApi; -import graphql.language.Definition; -import graphql.language.Document; -import graphql.language.Field; -import graphql.language.OperationDefinition; -import graphql.language.Selection; -import graphql.language.SelectionSet; import graphql.language.SourceLocation; import graphql.validation.QueryComplexityLimits; import org.jspecify.annotations.NullMarked; @@ -89,33 +83,6 @@ public static boolean isEnabled(GraphQLContext graphQLContext) { return !graphQLContext.getBoolean(GOOD_FAITH_INTROSPECTION_DISABLED, false); } - /** - * Performs a shallow scan of the document to check if any operation's top-level selections - * contain introspection fields ({@code __schema} or {@code __type}). - * - * @param document the parsed document - * - * @return true if the document contains top-level introspection fields - */ - public static boolean containsIntrospectionFields(Document document) { - for (Definition definition : document.getDefinitions()) { - if (definition instanceof OperationDefinition) { - SelectionSet selectionSet = ((OperationDefinition) definition).getSelectionSet(); - if (selectionSet != null) { - for (Selection selection : selectionSet.getSelections()) { - if (selection instanceof Field) { - String name = ((Field) selection).getName(); - if ("__schema".equals(name) || "__type".equals(name)) { - return true; - } - } - } - } - } - } - return false; - } - /** * Returns query complexity limits that are the minimum of the existing limits and the * good faith introspection limits. This ensures introspection queries are bounded @@ -125,10 +92,7 @@ public static boolean containsIntrospectionFields(Document document) { * * @return complexity limits with good faith bounds applied */ - public static QueryComplexityLimits goodFaithLimits(@Nullable QueryComplexityLimits existing) { - if (existing == null) { - existing = QueryComplexityLimits.getDefaultLimits(); - } + public static QueryComplexityLimits goodFaithLimits(QueryComplexityLimits existing) { int maxFields = Math.min(existing.getMaxFieldsCount(), GOOD_FAITH_MAX_FIELDS_COUNT); int maxDepth = Math.min(existing.getMaxDepth(), GOOD_FAITH_MAX_DEPTH_COUNT); return QueryComplexityLimits.newLimits() diff --git a/src/main/java/graphql/validation/OperationValidator.java b/src/main/java/graphql/validation/OperationValidator.java index 9ee3b9f859..a9091107b8 100644 --- a/src/main/java/graphql/validation/OperationValidator.java +++ b/src/main/java/graphql/validation/OperationValidator.java @@ -14,6 +14,7 @@ import graphql.execution.TypeFromAST; import graphql.execution.ValuesResolver; import graphql.i18n.I18nMsg; +import graphql.introspection.GoodFaithIntrospection; import graphql.introspection.Introspection.DirectiveLocation; import graphql.language.Argument; import graphql.language.AstComparator; @@ -332,7 +333,7 @@ public class OperationValidator implements DocumentVisitor { private int fieldCount = 0; private int currentFieldDepth = 0; private int maxFieldDepthSeen = 0; - private final QueryComplexityLimits complexityLimits; + private QueryComplexityLimits complexityLimits; // Fragment complexity calculated lazily during first spread private final Map fragmentComplexityMap = new HashMap<>(); // Max depth seen during current fragment traversal (for calculating fragment's internal depth) @@ -340,6 +341,7 @@ public class OperationValidator implements DocumentVisitor { // --- State: Good Faith Introspection --- private final Map introspectionFieldCounts = new HashMap<>(); + private boolean introspectionQueryDetected = false; // --- Track whether we're in a context where fragment spread rules should run --- // fragmentRetraversalDepth == 0 means we're NOT inside a manually-traversed fragment => run non-fragment-spread checks @@ -406,6 +408,10 @@ private boolean shouldRunOperationScopedRules() { private void checkFieldCountLimit() { if (fieldCount > complexityLimits.getMaxFieldsCount()) { + if (introspectionQueryDetected) { + throw GoodFaithIntrospectionExceeded.tooBigOperation( + "Query has " + fieldCount + " fields which exceeds maximum allowed " + complexityLimits.getMaxFieldsCount()); + } throw new QueryComplexityLimitsExceeded( ValidationErrorType.MaxQueryFieldsExceeded, complexityLimits.getMaxFieldsCount(), @@ -417,6 +423,10 @@ private void checkDepthLimit(int depth) { if (depth > maxFieldDepthSeen) { maxFieldDepthSeen = depth; if (maxFieldDepthSeen > complexityLimits.getMaxDepth()) { + if (introspectionQueryDetected) { + throw GoodFaithIntrospectionExceeded.tooBigOperation( + "Query depth " + maxFieldDepthSeen + " exceeds maximum allowed depth " + complexityLimits.getMaxDepth()); + } throw new QueryComplexityLimitsExceeded( ValidationErrorType.MaxQueryDepthExceeded, complexityLimits.getMaxDepth(), @@ -629,6 +639,10 @@ private void checkGoodFaithIntrospection(Field field) { if (queryType != null && parentType.getName().equals(queryType.getName())) { if ("__schema".equals(fieldName) || "__type".equals(fieldName)) { key = parentType.getName() + "." + fieldName; + if (!introspectionQueryDetected) { + introspectionQueryDetected = true; + complexityLimits = GoodFaithIntrospection.goodFaithLimits(complexityLimits); + } } } } diff --git a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy index 479a549501..538363d9bb 100644 --- a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy +++ b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy @@ -189,6 +189,41 @@ class GoodFaithIntrospectionTest extends Specification { 100 | GoodFaithIntrospection.BadFaithIntrospectionError.class } + def "introspection via inline fragment on Query is detected as bad faith"() { + def query = """ + query badActor { + ...on Query { + __schema{types{fields{type{fields{type{fields{type{fields{type{name}}}}}}}}}} + } + } + """ + + when: + ExecutionResult er = graphql.execute(query) + + then: + !er.errors.isEmpty() + er.errors[0] instanceof GoodFaithIntrospection.BadFaithIntrospectionError + } + + def "introspection via fragment spread is detected as bad faith"() { + def query = """ + query badActor { + ...IntrospectionFragment + } + fragment IntrospectionFragment on Query { + __schema{types{fields{type{fields{type{fields{type{fields{type{name}}}}}}}}}} + } + """ + + when: + ExecutionResult er = graphql.execute(query) + + then: + !er.errors.isEmpty() + er.errors[0] instanceof GoodFaithIntrospection.BadFaithIntrospectionError + } + def "good faith limits are applied on top of custom user limits"() { given: def limits = QueryComplexityLimits.newLimits().maxFieldsCount(200).maxDepth(15).build() @@ -203,18 +238,6 @@ class GoodFaithIntrospectionTest extends Specification { er.errors.isEmpty() } - def "containsIntrospectionFields handles operation with no selection set"() { - given: - def op = graphql.language.OperationDefinition.newOperationDefinition() - .name("empty") - .operation(graphql.language.OperationDefinition.Operation.QUERY) - .build() - def doc = Document.newDocument().definition(op).build() - - expect: - !GoodFaithIntrospection.containsIntrospectionFields(doc) - } - def "introspection query exceeding field count limit is detected as bad faith"() { given: // Build a wide introspection query that exceeds GOOD_FAITH_MAX_FIELDS_COUNT (500) From 8c8ea0a74ab5d124126dc3268b00b7851444c8c5 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Mon, 23 Mar 2026 10:55:21 +1000 Subject: [PATCH 07/11] Fix IntelliJ warnings in new code - 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) --- src/main/java/graphql/ParseAndValidate.java | 2 +- src/main/java/graphql/validation/QueryComplexityLimits.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/graphql/ParseAndValidate.java b/src/main/java/graphql/ParseAndValidate.java index 05c2556ee8..a092f28fc0 100644 --- a/src/main/java/graphql/ParseAndValidate.java +++ b/src/main/java/graphql/ParseAndValidate.java @@ -134,7 +134,7 @@ public static List validate(@NonNull GraphQLSchema graphQLSchem * * @return a result object that indicates how this operation went */ - public static List validate(@NonNull GraphQLSchema graphQLSchema, @NonNull Document parsedDocument, @NonNull Predicate rulePredicate, @NonNull Locale locale, @Nullable QueryComplexityLimits limits) { + public static List validate(GraphQLSchema graphQLSchema, Document parsedDocument, Predicate rulePredicate, Locale locale, @Nullable QueryComplexityLimits limits) { Validator validator = new Validator(); return validator.validateDocument(graphQLSchema, parsedDocument, rulePredicate, locale, limits); } diff --git a/src/main/java/graphql/validation/QueryComplexityLimits.java b/src/main/java/graphql/validation/QueryComplexityLimits.java index 2ba7bac830..f46020cedf 100644 --- a/src/main/java/graphql/validation/QueryComplexityLimits.java +++ b/src/main/java/graphql/validation/QueryComplexityLimits.java @@ -70,7 +70,7 @@ public class QueryComplexityLimits { * @param limits the default limits to use (use {@link #NONE} to disable, {@link #DEFAULT} to restore) */ public static void setDefaultLimits(QueryComplexityLimits limits) { - defaultLimits = limits != null ? limits : DEFAULT; + defaultLimits = limits; } /** From 31572d8be8149dc7b681366eb80ae4a53d6e759e Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Mon, 23 Mar 2026 10:56:29 +1000 Subject: [PATCH 08/11] Remove redundant null check on queryType A GraphQL schema always has a query type. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/main/java/graphql/validation/OperationValidator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/graphql/validation/OperationValidator.java b/src/main/java/graphql/validation/OperationValidator.java index 252cf43c5d..5d1b89e445 100644 --- a/src/main/java/graphql/validation/OperationValidator.java +++ b/src/main/java/graphql/validation/OperationValidator.java @@ -636,7 +636,7 @@ private void checkGoodFaithIntrospection(Field field) { // behavior where the same field from a direct selection and a fragment spread merge into one. if (shouldRunDocumentLevelRules()) { GraphQLObjectType queryType = validationContext.getSchema().getQueryType(); - if (queryType != null && parentType.getName().equals(queryType.getName())) { + if (parentType.getName().equals(queryType.getName())) { if ("__schema".equals(fieldName) || "__type".equals(fieldName)) { key = parentType.getName() + "." + fieldName; if (!introspectionQueryDetected) { From 5147481bffd0e5a47d844305b84a02fca90af3ec Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Mon, 23 Mar 2026 10:58:59 +1000 Subject: [PATCH 09/11] Use Introspection constants instead of string literals 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) --- src/main/java/graphql/validation/OperationValidator.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/graphql/validation/OperationValidator.java b/src/main/java/graphql/validation/OperationValidator.java index 5d1b89e445..27a1bfc18b 100644 --- a/src/main/java/graphql/validation/OperationValidator.java +++ b/src/main/java/graphql/validation/OperationValidator.java @@ -15,6 +15,7 @@ import graphql.execution.ValuesResolver; import graphql.i18n.I18nMsg; import graphql.introspection.GoodFaithIntrospection; +import graphql.introspection.Introspection; import graphql.introspection.Introspection.DirectiveLocation; import graphql.language.Argument; import graphql.language.AstComparator; @@ -637,7 +638,7 @@ private void checkGoodFaithIntrospection(Field field) { if (shouldRunDocumentLevelRules()) { GraphQLObjectType queryType = validationContext.getSchema().getQueryType(); if (parentType.getName().equals(queryType.getName())) { - if ("__schema".equals(fieldName) || "__type".equals(fieldName)) { + if (Introspection.SchemaMetaFieldDef.getName().equals(fieldName) || Introspection.TypeMetaFieldDef.getName().equals(fieldName)) { key = parentType.getName() + "." + fieldName; if (!introspectionQueryDetected) { introspectionQueryDetected = true; @@ -650,7 +651,7 @@ private void checkGoodFaithIntrospection(Field field) { // Check __Type fields that can form cycles. // Counted during ALL traversals (including fragment spreads) because each occurrence // at a different depth represents a separate cycle risk. - if ("__Type".equals(parentType.getName())) { + if (Introspection.__Type.getName().equals(parentType.getName())) { if ("fields".equals(fieldName) || "inputFields".equals(fieldName) || "interfaces".equals(fieldName) || "possibleTypes".equals(fieldName)) { key = "__Type." + fieldName; From eadf4326eb7fcadde05b5f7c9e7f76e65605184d Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Mon, 23 Mar 2026 11:20:38 +1000 Subject: [PATCH 10/11] Fix undefined sanitizedBranchName in dev version string Bad merge in 0c7b9c3b6 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) --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 89e65c14c1..0d183d4a89 100644 --- a/build.gradle +++ b/build.gradle @@ -90,7 +90,7 @@ def getDevelopmentVersion() { gitRevParse.waitForProcessOutput(gitRevParseOutput, gitRevParseError) def branchName = gitRevParseOutput.toString().trim().replaceAll('[/\\\\]', '-') - return makeDevelopmentVersion(["0.0.0", sanitizedBranchName, "SNAPSHOT"]) + return makeDevelopmentVersion(["0.0.0", branchName, "SNAPSHOT"]) } def reactiveStreamsVersion = '1.0.3' From e9be74ed0ed5137a95a3394a441d89070a3aec05 Mon Sep 17 00:00:00 2001 From: Andreas Marek Date: Mon, 23 Mar 2026 11:21:12 +1000 Subject: [PATCH 11/11] Add test for good faith disabled with custom validation predicate 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) --- .../GoodFaithIntrospectionTest.groovy | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy index 538363d9bb..9da21c42be 100644 --- a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy +++ b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionTest.groovy @@ -2,10 +2,12 @@ package graphql.introspection import graphql.ExecutionInput import graphql.ExecutionResult +import graphql.ParseAndValidate import graphql.TestUtil import graphql.execution.CoercedVariables import graphql.language.Document import graphql.normalized.ExecutableNormalizedOperationFactory +import graphql.validation.OperationValidationRule import graphql.validation.QueryComplexityLimits import spock.lang.Specification @@ -143,6 +145,24 @@ class GoodFaithIntrospectionTest extends Specification { er.errors.isEmpty() } + def "disabling good faith composes with custom validation rule predicates"() { + given: + // Custom predicate that disables a specific rule + def customPredicate = { OperationValidationRule rule -> rule != OperationValidationRule.KNOWN_ARGUMENT_NAMES } as java.util.function.Predicate + + when: + def context = [ + (GoodFaithIntrospection.GOOD_FAITH_INTROSPECTION_DISABLED) : true, + (ParseAndValidate.INTERNAL_VALIDATION_PREDICATE_HINT) : customPredicate + ] + ExecutionInput executionInput = ExecutionInput.newExecutionInput("{ normalField }") + .graphQLContext(context).build() + ExecutionResult er = graphql.execute(executionInput) + + then: + er.errors.isEmpty() + } + def "can be disabled per request"() { when: def context = [(GoodFaithIntrospection.GOOD_FAITH_INTROSPECTION_DISABLED): true]