From bd41cf77aeaad883f119abc5b24047fc278d0cf1 Mon Sep 17 00:00:00 2001 From: Brandon Guest <29989722+bcmyguest@users.noreply.github.com> Date: Tue, 7 Jul 2026 17:29:41 -0400 Subject: [PATCH 1/5] fix: deduplicate fields in OverlappingFieldsCanBeMerged to prevent DoS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The validation rule had O(n²) time complexity when processing queries with many repeated fields sharing the same response name. A query like `{ hello hello hello ... }` with thousands of fields caused excessive CPU usage, exploitable as a denial of service vector. Fields that are structurally identical (same parent type, field name, arguments, directives, and selection set) can never conflict with each other, so we deduplicate them before pairwise comparison. This ports the graphql-php fix for GHSA-68jq-c3rv-pcrr: https://github.com/webonyx/graphql-php/commit/b5e7c995b6e8b2c49149d1492a0a997da626491c Assisted-by: Claude:claude-fable-5 --- .../rules/overlapping_fields_can_be_merged.py | 52 +++++++++++++- .../test_validate_repeated_fields.py | 24 +++++++ .../test_overlapping_fields_can_be_merged.py | 70 ++++++++++++++++++- 3 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 tests/benchmarks/test_validate_repeated_fields.py diff --git a/src/graphql/validation/rules/overlapping_fields_can_be_merged.py b/src/graphql/validation/rules/overlapping_fields_can_be_merged.py index 1aa60322..9d7f47a1 100644 --- a/src/graphql/validation/rules/overlapping_fields_can_be_merged.py +++ b/src/graphql/validation/rules/overlapping_fields_can_be_merged.py @@ -539,8 +539,11 @@ def collect_conflicts_within( # (except to itself). If the list only has one item, nothing needs to be # compared. if len(fields) > 1: - for i, field in enumerate(fields): - for other_field in fields[i + 1 :]: + # Deduplicate structurally identical fields to avoid quadratic blowup + # when a query repeats the same field many times. + unique_fields = deduplicate_fields(fields) + for i, field in enumerate(unique_fields): + for other_field in unique_fields[i + 1 :]: conflict = find_conflict( context, cached_fields_and_fragment_spreads, @@ -558,6 +561,51 @@ def collect_conflicts_within( conflicts.append(conflict) +def deduplicate_fields(fields: list[NodeAndDef]) -> list[NodeAndDef]: + """Deduplicate structurally identical fields. + + Fields that are structurally identical (same parent type, field name, arguments, + directives and selection set) can never conflict with each other, so only one of + them needs to take part in the pairwise conflict comparison. + """ + unique: list[NodeAndDef] = [] + seen: set[str] = set() + for field in fields: + key = field_fingerprint(field) + if key not in seen: + seen.add(key) + unique.append(field) + return unique + + +def field_fingerprint(field: NodeAndDef) -> str: + """Build a fingerprint identifying the structure of a given field. + + Two fields that share a fingerprint are structurally identical and therefore + cannot conflict with each other, so only one needs to take part in the pairwise + conflict comparison. The fingerprint is derived from normalized AST content + rather than node identity, so that repeated occurrences of the same composite + field (each of which has a distinct ``SelectionSetNode`` instance) collapse. It + mirrors the equivalence relation used by ``find_conflict``: arguments are + compared order-independently with normalized values (see ``same_arguments``), + and directives are included in a stable order because differing stream + directives are treated as a conflict (see ``same_streams``). + """ + parent_type, node, _ = field + parts: list[str] = [parent_type.name if parent_type else "", node.name.value] + parts.extend( + f"{argument.name.value}={stringify_value(argument.value)}" + for argument in sorted(node.arguments or (), key=lambda arg: arg.name.value) + ) + parts.extend( + print_ast(directive) + for directive in sorted(node.directives or (), key=print_ast) + ) + if node.selection_set: + parts.append(print_ast(node.selection_set)) + return "\x00".join(parts) + + def collect_conflicts_between( context: ValidationContext, conflicts: list[Conflict], diff --git a/tests/benchmarks/test_validate_repeated_fields.py b/tests/benchmarks/test_validate_repeated_fields.py new file mode 100644 index 00000000..e0cadd07 --- /dev/null +++ b/tests/benchmarks/test_validate_repeated_fields.py @@ -0,0 +1,24 @@ +import pytest + +from graphql import ( + GraphQLField, + GraphQLObjectType, + GraphQLSchema, + GraphQLString, + parse, + validate, +) + +schema = GraphQLSchema( + query=GraphQLObjectType( + name="Query", + fields={"hello": GraphQLField(GraphQLString)}, + ) +) + + +@pytest.mark.parametrize("count", [100, 500, 1000, 2000, 3000]) +def test_validate_many_repeated_fields(benchmark, count): + document = parse(f"{{ {'hello ' * count}}}") + result = benchmark(lambda: validate(schema, document)) + assert result == [] diff --git a/tests/validation/test_overlapping_fields_can_be_merged.py b/tests/validation/test_overlapping_fields_can_be_merged.py index a9b09665..9429b734 100644 --- a/tests/validation/test_overlapping_fields_can_be_merged.py +++ b/tests/validation/test_overlapping_fields_can_be_merged.py @@ -1,9 +1,12 @@ from functools import partial +import pytest + +from graphql import parse, validate from graphql.utilities import build_schema from graphql.validation import OverlappingFieldsCanBeMergedRule -from .harness import assert_validation_errors +from .harness import assert_validation_errors, test_schema assert_errors = partial(assert_validation_errors, OverlappingFieldsCanBeMergedRule) @@ -1565,6 +1568,71 @@ def does_not_infinite_loop_on_transitively_recursive_fragments(): """ ) + def many_repeated_fields_do_not_cause_quadratic_blowup(): + repeated_fields = "name " * 3000 + assert_valid( + f""" + fragment manyRepeatedFields on Dog {{ + {repeated_fields} + }} + """ + ) + + def many_repeated_fields_with_conflict_still_detected(): + repeated_fields = "name " * 100 + doc = parse( + f""" + fragment conflictsAmongMany on Dog {{ + {repeated_fields} + name: nickname + }} + """ + ) + errors = validate(test_schema, doc, [OverlappingFieldsCanBeMergedRule]) + assert errors + assert "'name' and 'nickname' are different fields" in errors[0].message + + @pytest.mark.timeout(5) + def many_repeated_composite_fields_do_not_cause_quadratic_blowup(): + # Each occurrence of a composite field has its own SelectionSetNode, so + # deduplication must fingerprint the selection set by content, not identity, + # otherwise these still trigger quadratic behavior. + repeated_fields = "mother { name } " * 3000 + assert_valid( + f""" + fragment manyRepeatedCompositeFields on Dog {{ + {repeated_fields} + }} + """ + ) + + @pytest.mark.timeout(5) + def many_repeated_fields_with_reordered_arguments_do_not_cause_quadratic_blowup(): + # Fields whose arguments differ only in order are equivalent and must + # deduplicate, so the fingerprint has to be argument-order independent. + repeated_fields = "isAtLocation(x: 1, y: 2) isAtLocation(y: 2, x: 1) " * 1500 + assert_valid( + f""" + fragment reorderedArgs on Dog {{ + {repeated_fields} + }} + """ + ) + + def many_repeated_composite_fields_with_conflict_still_detected(): + repeated_fields = "mother { name } " * 100 + doc = parse( + f""" + fragment conflictsAmongManyComposites on Dog {{ + {repeated_fields} + mother {{ name: nickname }} + }} + """ + ) + errors = validate(test_schema, doc, [OverlappingFieldsCanBeMergedRule]) + assert errors + assert "'name' and 'nickname' are different fields" in errors[0].message + def finds_invalid_case_even_with_immediately_recursive_fragment(): assert_errors( """ From 730f3713cdbe4531700f3294bd16a3349da65d9d Mon Sep 17 00:00:00 2001 From: Brandon Guest <29989722+bcmyguest@users.noreply.github.com> Date: Tue, 7 Jul 2026 18:03:57 -0400 Subject: [PATCH 2/5] fix: make OverlappingFieldsCanBeMerged dedup canonical field_fingerprint used id(node.selection_set) and printed arguments and directives in source order, so repeated composite fields, and fields whose arguments, directives, or nested arguments were merely reordered, failed to deduplicate. That left the O(n^2) pairwise-comparison DoS shape exploitable by reordering semantically irrelevant parts of an otherwise repeated field. Build the dedup key from a canonical nested tuple of the field's AST: arguments (including directive and nested-field arguments) sorted by name with normalized values, directives and sibling selections in a stable order, and selection sets compared structurally. This matches the equivalence relation used by find_conflict, so reordering can no longer defeat dedup. A nested tuple is used as the key so shared sub-selections are referenced rather than repeatedly copied into a string. Add regression tests for repeated composite fields and for reordered top-level and nested arguments. Assisted-by: Claude:claude-fable-5 --- .../rules/overlapping_fields_can_be_merged.py | 103 +++++++++++++----- .../test_overlapping_fields_can_be_merged.py | 17 +++ 2 files changed, 93 insertions(+), 27 deletions(-) diff --git a/src/graphql/validation/rules/overlapping_fields_can_be_merged.py b/src/graphql/validation/rules/overlapping_fields_can_be_merged.py index 9d7f47a1..d1918d47 100644 --- a/src/graphql/validation/rules/overlapping_fields_can_be_merged.py +++ b/src/graphql/validation/rules/overlapping_fields_can_be_merged.py @@ -17,6 +17,7 @@ ListValueNode, ObjectFieldNode, ObjectValueNode, + SelectionNode, SelectionSetNode, ValueNode, VariableNode, @@ -39,7 +40,7 @@ from . import ValidationContext, ValidationRule if TYPE_CHECKING: - from collections.abc import Sequence + from collections.abc import Hashable, Sequence __all__ = ["OverlappingFieldsCanBeMergedRule"] @@ -562,14 +563,15 @@ def collect_conflicts_within( def deduplicate_fields(fields: list[NodeAndDef]) -> list[NodeAndDef]: - """Deduplicate structurally identical fields. + """Deduplicate structurally equivalent fields. - Fields that are structurally identical (same parent type, field name, arguments, - directives and selection set) can never conflict with each other, so only one of - them needs to take part in the pairwise conflict comparison. + Fields that are structurally equivalent (same parent type, field name, arguments, + directives and selection set, irrespective of the ordering of arguments, + directives or sibling selections) can never conflict with each other, so only one + of them needs to take part in the pairwise conflict comparison. """ unique: list[NodeAndDef] = [] - seen: set[str] = set() + seen: set[tuple[Hashable, ...]] = set() for field in fields: key = field_fingerprint(field) if key not in seen: @@ -578,32 +580,79 @@ def deduplicate_fields(fields: list[NodeAndDef]) -> list[NodeAndDef]: return unique -def field_fingerprint(field: NodeAndDef) -> str: - """Build a fingerprint identifying the structure of a given field. +def field_fingerprint(field: NodeAndDef) -> tuple[Hashable, ...]: + """Build a canonical key identifying a field's structure. - Two fields that share a fingerprint are structurally identical and therefore - cannot conflict with each other, so only one needs to take part in the pairwise - conflict comparison. The fingerprint is derived from normalized AST content - rather than node identity, so that repeated occurrences of the same composite - field (each of which has a distinct ``SelectionSetNode`` instance) collapse. It - mirrors the equivalence relation used by ``find_conflict``: arguments are - compared order-independently with normalized values (see ``same_arguments``), - and directives are included in a stable order because differing stream - directives are treated as a conflict (see ``same_streams``). + Fields with equal keys are structurally equivalent and cannot conflict, so only + one takes part in the pairwise comparison. The key is derived from AST content + (not node identity) and normalizes argument, directive and sibling-selection + ordering recursively, matching the equivalence used by ``find_conflict``, so + reordering those parts cannot defeat deduplication. """ parent_type, node, _ = field - parts: list[str] = [parent_type.name if parent_type else "", node.name.value] - parts.extend( - f"{argument.name.value}={stringify_value(argument.value)}" - for argument in sorted(node.arguments or (), key=lambda arg: arg.name.value) + return (parent_type.name if parent_type else "", _canonical_field(node)) + + +def _canonical_field(node: FieldNode) -> tuple[Hashable, ...]: + return ( + "field", + (node.alias or node.name).value, + node.name.value, + _canonical_arguments(node.arguments), + _canonical_directives(node.directives), + _canonical_selection_set(node.selection_set), ) - parts.extend( - print_ast(directive) - for directive in sorted(node.directives or (), key=print_ast) + + +def _canonical_selection(selection: SelectionNode) -> tuple[Hashable, ...]: + if isinstance(selection, FieldNode): + return _canonical_field(selection) + if isinstance(selection, FragmentSpreadNode): + return ( + "spread", + selection.name.value, + _canonical_arguments(selection.arguments), + _canonical_directives(selection.directives), + ) + if isinstance(selection, InlineFragmentNode): + return ( + "inline", + selection.type_condition.name.value if selection.type_condition else "", + _canonical_directives(selection.directives), + _canonical_selection_set(selection.selection_set), + ) + msg = f"Unexpected selection node: {type(selection).__name__}" # pragma: no cover + raise TypeError(msg) # pragma: no cover + + +def _canonical_selection_set( + selection_set: SelectionSetNode | None, +) -> tuple[Hashable, ...]: + if selection_set is None: + return () + # Sorted so that reordering sibling selections does not change the result. + return tuple(sorted(map(_canonical_selection, selection_set.selections))) + + +def _canonical_arguments( + arguments: Sequence[ArgumentNode | FragmentArgumentNode] | None, +) -> tuple[tuple[str, str], ...]: + # Sorted by name with normalized values, matching ``same_arguments``. + return tuple( + (arg.name.value, stringify_value(arg.value)) + for arg in sorted(arguments or (), key=lambda arg: arg.name.value) + ) + + +def _canonical_directives( + directives: Sequence[DirectiveNode] | None, +) -> tuple[tuple[str, tuple[tuple[str, str], ...]], ...]: + return tuple( + sorted( + (directive.name.value, _canonical_arguments(directive.arguments)) + for directive in directives or () + ) ) - if node.selection_set: - parts.append(print_ast(node.selection_set)) - return "\x00".join(parts) def collect_conflicts_between( diff --git a/tests/validation/test_overlapping_fields_can_be_merged.py b/tests/validation/test_overlapping_fields_can_be_merged.py index 9429b734..7979bd4b 100644 --- a/tests/validation/test_overlapping_fields_can_be_merged.py +++ b/tests/validation/test_overlapping_fields_can_be_merged.py @@ -1619,6 +1619,23 @@ def many_repeated_fields_with_reordered_arguments_do_not_cause_quadratic_blowup( """ ) + @pytest.mark.timeout(5) + def repeated_fields_with_reordered_nested_arguments_do_not_cause_blowup(): + # Reordering arguments inside a nested selection set keeps the fields + # equivalent, so deduplication must canonicalize recursively rather than rely + # on the printed selection set (which preserves source argument order). + repeated_fields = ( + "mother { isAtLocation(x: 1, y: 2) } " + "mother { isAtLocation(y: 2, x: 1) } " + ) * 1500 + assert_valid( + f""" + fragment reorderedNestedArgs on Dog {{ + {repeated_fields} + }} + """ + ) + def many_repeated_composite_fields_with_conflict_still_detected(): repeated_fields = "mother { name } " * 100 doc = parse( From 82a98849670ce2e457cbaf8ae210bbd548a117ce Mon Sep 17 00:00:00 2001 From: bcmyguest Date: Tue, 7 Jul 2026 18:15:13 -0400 Subject: [PATCH 3/5] fix: long running test has no timeout Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- tests/validation/test_overlapping_fields_can_be_merged.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/validation/test_overlapping_fields_can_be_merged.py b/tests/validation/test_overlapping_fields_can_be_merged.py index 7979bd4b..224ea008 100644 --- a/tests/validation/test_overlapping_fields_can_be_merged.py +++ b/tests/validation/test_overlapping_fields_can_be_merged.py @@ -1568,6 +1568,7 @@ def does_not_infinite_loop_on_transitively_recursive_fragments(): """ ) + @pytest.mark.timeout(5) def many_repeated_fields_do_not_cause_quadratic_blowup(): repeated_fields = "name " * 3000 assert_valid( From 3aad157f815442eccb3c91c11b632c839a1aa826 Mon Sep 17 00:00:00 2001 From: Brandon Guest <29989722+bcmyguest@users.noreply.github.com> Date: Wed, 8 Jul 2026 10:09:07 -0400 Subject: [PATCH 4/5] fix: bound OverlappingFieldsCanBeMerged cost to prevent DoS The rule compares overlapping fields pairwise, which is quadratic in the number of fields sharing a response name. Two query shapes trigger this: - Identical repeated fields ({ name name name ... }): handled by deduplicating structurally identical fields, as graphql-php did (CVE-2026-40476). This also extends that dedup to the "between fragments" comparison path, which was still quadratic (and, with the budget below, could falsely reject valid queries). - Distinct conflicting fields ({ x:f(a:0) x:f(a:1) ... }): cannot be deduped because they genuinely differ, so a shared comparison budget bounds them, reporting one "too complex to validate" error. Overridable via max_comparisons. Valid queries do not approach the budget in practice: identical fields dedup, and distinct fields under one response name conflict (so are already invalid). The budget idea is not from graphql-php (dedup only); it follows the reporter's proof-of-concept. Assisted-by: Claude:claude-opus-4-8 --- .../rules/overlapping_fields_can_be_merged.py | 116 ++++++++++++++++-- .../test_overlapping_fields_can_be_merged.py | 53 ++++++++ 2 files changed, 158 insertions(+), 11 deletions(-) diff --git a/src/graphql/validation/rules/overlapping_fields_can_be_merged.py b/src/graphql/validation/rules/overlapping_fields_can_be_merged.py index d1918d47..07568209 100644 --- a/src/graphql/validation/rules/overlapping_fields_can_be_merged.py +++ b/src/graphql/validation/rules/overlapping_fields_can_be_merged.py @@ -3,7 +3,7 @@ from __future__ import annotations from itertools import chain -from typing import TYPE_CHECKING, Any, NamedTuple, TypeAlias, cast +from typing import TYPE_CHECKING, Any, ClassVar, NamedTuple, TypeAlias, cast from ...error import GraphQLError from ...language import ( @@ -63,6 +63,15 @@ class OverlappingFieldsCanBeMergedRule(ValidationRule): See https://spec.graphql.org/draft/#sec-Field-Selection-Merging """ + # Maximum number of pairwise field comparisons allowed for a single document. + # Structurally identical fields are deduplicated before comparison, but a query + # can still force a quadratic number of comparisons by placing many fields with + # differing arguments under the same response name. Such queries are always + # invalid (differing arguments conflict), so a valid query never approaches this + # limit; it exists purely to keep validation of hostile queries bounded (roughly + # sub-second). Override on a subclass to trade strictness for headroom. + max_comparisons: ClassVar[int] = 100_000 + def __init__(self, context: ValidationContext) -> None: super().__init__(context) # A memoization for when fields and a fragment or two fragments are compared @@ -76,15 +85,36 @@ def __init__(self, context: ValidationContext) -> None: # times, so this improves the performance of this validator. self.cached_fields_and_fragment_spreads: dict = {} + # A shared budget bounding the total number of pairwise field comparisons, + # so a hostile query cannot force quadratic work. See ``max_comparisons``. + self.comparison_budget = ComparisonBudget(self.max_comparisons) + self._too_complex = False + def enter_selection_set(self, selection_set: SelectionSetNode, *_args: Any) -> None: - conflicts = find_conflicts_within_selection_set( - self.context, - self.cached_fields_and_fragment_spreads, - self.compared_fields_and_fragment_pairs, - self.compared_fragment_pairs, - self.context.get_parent_type(), - selection_set, - ) + if self._too_complex: + return + try: + conflicts = find_conflicts_within_selection_set( + self.context, + self.cached_fields_and_fragment_spreads, + self.compared_fields_and_fragment_pairs, + self.compared_fragment_pairs, + self.comparison_budget, + self.context.get_parent_type(), + selection_set, + ) + except ComparisonBudgetExceededError: + # The query forced more field comparisons than allowed. Report a single + # error and stop checking so validation stays bounded. + self._too_complex = True + self.report_error( + GraphQLError( + "Query is too complex to validate for overlapping fields." + " Reduce the number of fields sharing a response name.", + selection_set, + ) + ) + return for (reason_name, reason), fields1, fields2 in conflicts: reason_msg = reason_message(reason) self.report_error( @@ -176,6 +206,7 @@ def find_conflicts_within_selection_set( cached_fields_and_fragment_spreads: dict, compared_fields_and_fragment_pairs: OrderedPairSet, compared_fragment_pairs: PairSet, + budget: ComparisonBudget, parent_type: GraphQLNamedType | None, selection_set: SelectionSetNode, ) -> list[Conflict]: @@ -200,6 +231,7 @@ def find_conflicts_within_selection_set( cached_fields_and_fragment_spreads, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, field_map, ) @@ -213,6 +245,7 @@ def find_conflicts_within_selection_set( cached_fields_and_fragment_spreads, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, False, field_map, fragment_spread, @@ -228,6 +261,7 @@ def find_conflicts_within_selection_set( cached_fields_and_fragment_spreads, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, False, fragment_spread, other_fragment_spread, @@ -242,6 +276,7 @@ def collect_conflicts_between_fields_and_fragment( cached_fields_and_fragment_spreads: dict, compared_fields_and_fragment_pairs: OrderedPairSet, compared_fragment_pairs: PairSet, + budget: ComparisonBudget, are_mutually_exclusive: bool, field_map: NodeAndDefCollection, fragment_spread: FragmentSpread, @@ -288,6 +323,7 @@ def collect_conflicts_between_fields_and_fragment( cached_fields_and_fragment_spreads, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, are_mutually_exclusive, field_map, None, @@ -304,6 +340,7 @@ def collect_conflicts_between_fields_and_fragment( cached_fields_and_fragment_spreads, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, are_mutually_exclusive, field_map, referenced_fragment_spread, @@ -316,6 +353,7 @@ def collect_conflicts_between_fragments( cached_fields_and_fragment_spreads: dict, compared_fields_and_fragment_pairs: OrderedPairSet, compared_fragment_pairs: PairSet, + budget: ComparisonBudget, are_mutually_exclusive: bool, fragment_spread1: FragmentSpread, fragment_spread2: FragmentSpread, @@ -387,6 +425,7 @@ def collect_conflicts_between_fragments( cached_fields_and_fragment_spreads, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, are_mutually_exclusive, field_map1, fragment_spread1.var_map, @@ -403,6 +442,7 @@ def collect_conflicts_between_fragments( cached_fields_and_fragment_spreads, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, are_mutually_exclusive, fragment_spread1, referenced_fragment_spread2, @@ -417,6 +457,7 @@ def collect_conflicts_between_fragments( cached_fields_and_fragment_spreads, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, are_mutually_exclusive, referenced_fragment_spread1, fragment_spread2, @@ -428,6 +469,7 @@ def find_conflicts_between_sub_selection_sets( cached_fields_and_fragment_spreads: dict, compared_fields_and_fragment_pairs: OrderedPairSet, compared_fragment_pairs: PairSet, + budget: ComparisonBudget, are_mutually_exclusive: bool, parent_type1: GraphQLNamedType | None, selection_set1: SelectionSetNode, @@ -466,6 +508,7 @@ def find_conflicts_between_sub_selection_sets( cached_fields_and_fragment_spreads, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, are_mutually_exclusive, field_map1, var_map1, @@ -483,6 +526,7 @@ def find_conflicts_between_sub_selection_sets( cached_fields_and_fragment_spreads, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, are_mutually_exclusive, field_map1, fragment_spread2, @@ -498,6 +542,7 @@ def find_conflicts_between_sub_selection_sets( cached_fields_and_fragment_spreads, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, are_mutually_exclusive, field_map2, fragment_spread1, @@ -514,6 +559,7 @@ def find_conflicts_between_sub_selection_sets( cached_fields_and_fragment_spreads, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, are_mutually_exclusive, fragment_spread1, fragment_spread2, @@ -528,6 +574,7 @@ def collect_conflicts_within( cached_fields_and_fragment_spreads: dict, compared_fields_and_fragment_pairs: OrderedPairSet, compared_fragment_pairs: PairSet, + budget: ComparisonBudget, field_map: NodeAndDefCollection, ) -> None: """Collect all Conflicts "within" one collection of fields.""" @@ -550,6 +597,7 @@ def collect_conflicts_within( cached_fields_and_fragment_spreads, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, # within one collection is never mutually exclusive False, response_name, @@ -661,6 +709,7 @@ def collect_conflicts_between( cached_fields_and_fragment_spreads: dict, compared_fields_and_fragment_pairs: OrderedPairSet, compared_fragment_pairs: PairSet, + budget: ComparisonBudget, parent_fields_are_mutually_exclusive: bool, field_map1: NodeAndDefCollection, var_map1: VarMap, @@ -682,13 +731,20 @@ def collect_conflicts_between( for response_name, fields1 in field_map1.items(): fields2 = field_map2.get(response_name) if fields2: - for field1 in fields1: - for field2 in fields2: + # Deduplicate structurally identical fields on both sides, just as + # collect_conflicts_within does. Two identical fields yield the same + # comparison result against every field on the other side, so a valid + # query that repeats a field many times across spread fragments does + # not force a quadratic number of comparisons here either. + unique_fields2 = deduplicate_fields(fields2) + for field1 in deduplicate_fields(fields1): + for field2 in unique_fields2: conflict = find_conflict( context, cached_fields_and_fragment_spreads, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, parent_fields_are_mutually_exclusive, response_name, field1, @@ -705,6 +761,7 @@ def find_conflict( cached_fields_and_fragment_spreads: dict, compared_fields_and_fragment_pairs: OrderedPairSet, compared_fragment_pairs: PairSet, + budget: ComparisonBudget, parent_fields_are_mutually_exclusive: bool, response_name: str, field1: NodeAndDef, @@ -717,6 +774,10 @@ def find_conflict( Determines if there is a conflict between two particular fields, including comparing their sub-fields. """ + # Charge this comparison against the shared budget. When it is exhausted this + # raises, aborting an otherwise quadratic amount of work on a hostile query. + budget.spend() + parent_type1, node1, def1 = field1 parent_type2, node2, def2 = field2 @@ -778,6 +839,7 @@ def find_conflict( cached_fields_and_fragment_spreads, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, are_mutually_exclusive, get_named_type(type1), selection_set1, @@ -1060,6 +1122,38 @@ def subfield_conflicts( return None # no conflict +class ComparisonBudgetExceededError(Exception): + """Raised when field comparisons exceed the allowed budget. + + Signals that a query is too complex to validate for overlapping fields within + a bounded number of comparisons. Caught by the rule, which reports a single + error instead of continuing quadratic work. + """ + + +class ComparisonBudget: + """A shared, decreasing count of allowed pairwise field comparisons. + + ``OverlappingFieldsCanBeMerged`` compares overlapping fields pairwise, which is + quadratic in the number of fields sharing a response name. Structurally identical + fields are deduplicated first, but fields differing only in their arguments cannot + be deduplicated and still force quadratic work. Each comparison spends one unit + from this budget; :exc:`ComparisonBudgetExceededError` is raised once it is + exhausted. + """ + + __slots__ = ("_remaining",) + + def __init__(self, limit: int) -> None: + self._remaining = limit + + def spend(self) -> None: + """Charge a single comparison, raising when the budget is exhausted.""" + if self._remaining <= 0: + raise ComparisonBudgetExceededError + self._remaining -= 1 + + class OrderedPairSet: """Ordered pair set diff --git a/tests/validation/test_overlapping_fields_can_be_merged.py b/tests/validation/test_overlapping_fields_can_be_merged.py index 224ea008..03fed075 100644 --- a/tests/validation/test_overlapping_fields_can_be_merged.py +++ b/tests/validation/test_overlapping_fields_can_be_merged.py @@ -1698,3 +1698,56 @@ def does_not_infinite_loop_on_recursive_fragments_separated_by_fields(): } """ ) + + @pytest.mark.timeout(5) + def many_fields_with_differing_arguments_are_rejected_as_too_complex(): + # Fields differing only in their arguments genuinely conflict, so they + # cannot be deduplicated and would otherwise force a quadratic number of + # comparisons. The comparison budget bounds this by rejecting the query + # with a single error instead of validating every pair. + distinct_fields = " ".join( + f"p: isAtLocation(x: {i})" for i in range(2000) + ) + doc = parse( + f""" + fragment manyDistinctArguments on Dog {{ + {distinct_fields} + }} + """ + ) + errors = validate(test_schema, doc, [OverlappingFieldsCanBeMergedRule]) + assert errors + assert "too complex to validate" in errors[0].message + + @pytest.mark.timeout(5) + def valid_repeated_fields_across_fragments_are_not_too_complex(): + # Deduplication must also apply when comparing fields "between" two spread + # fragments, otherwise a valid query repeating a field many times in each + # fragment forces a quadratic number of comparisons and is wrongly rejected + # by the comparison budget. + repeated_fields = "name " * 400 + assert_valid( + f""" + {{ + dog {{ ...fragA ...fragB }} + }} + + fragment fragA on Dog {{ {repeated_fields} }} + fragment fragB on Dog {{ {repeated_fields} }} + """ + ) + + def modest_differing_arguments_still_report_the_real_conflict(): + # Below the budget the ordinary, specific conflict must still be reported + # rather than the "too complex" fallback. + doc = parse( + """ + fragment fewDistinctArguments on Dog { + p: isAtLocation(x: 0) + p: isAtLocation(x: 1) + } + """ + ) + errors = validate(test_schema, doc, [OverlappingFieldsCanBeMergedRule]) + assert errors + assert "they have differing arguments" in errors[0].message From f4108db0fca856b8e2b3bcb01340c97802f7470f Mon Sep 17 00:00:00 2001 From: Brandon Guest <29989722+bcmyguest@users.noreply.github.com> Date: Wed, 8 Jul 2026 10:43:21 -0400 Subject: [PATCH 5/5] fix: collapse duplicate sibling selections in field fingerprint The fingerprint sorted sibling selections but kept duplicates, so fields whose sub-selections differ only by redundant repeats (e.g. `f { a }` vs `f { a a }`) got distinct fingerprints and failed to deduplicate. A valid query using many such variants under one response name could then exhaust the comparison budget and be wrongly rejected as "too complex". Repeating an identical sibling is redundant and never conflicts, so de-duplicate the canonical selections to match the equivalence find_conflict uses. Reported by GitHub Copilot review. Assisted-by: Claude:claude-opus-4-8 --- .../rules/overlapping_fields_can_be_merged.py | 7 +++++-- .../test_overlapping_fields_can_be_merged.py | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/graphql/validation/rules/overlapping_fields_can_be_merged.py b/src/graphql/validation/rules/overlapping_fields_can_be_merged.py index 07568209..be0bdd20 100644 --- a/src/graphql/validation/rules/overlapping_fields_can_be_merged.py +++ b/src/graphql/validation/rules/overlapping_fields_can_be_merged.py @@ -678,8 +678,11 @@ def _canonical_selection_set( ) -> tuple[Hashable, ...]: if selection_set is None: return () - # Sorted so that reordering sibling selections does not change the result. - return tuple(sorted(map(_canonical_selection, selection_set.selections))) + # Sorted so that reordering sibling selections does not change the result, and + # de-duplicated because repeating an identical sibling selection is redundant and + # never introduces a conflict; collapsing duplicates here matches the equivalence + # used by ``find_conflict`` so such fields still deduplicate. + return tuple(sorted(set(map(_canonical_selection, selection_set.selections)))) def _canonical_arguments( diff --git a/tests/validation/test_overlapping_fields_can_be_merged.py b/tests/validation/test_overlapping_fields_can_be_merged.py index 03fed075..1e5d1a18 100644 --- a/tests/validation/test_overlapping_fields_can_be_merged.py +++ b/tests/validation/test_overlapping_fields_can_be_merged.py @@ -1737,6 +1737,24 @@ def valid_repeated_fields_across_fragments_are_not_too_complex(): """ ) + @pytest.mark.timeout(5) + def redundant_duplicate_sibling_selections_do_not_cause_blowup(): + # Fields whose sub-selections differ only by redundant repeated siblings + # (e.g. `{ name }` vs `{ name name }`) are equivalent and never conflict, so + # the fingerprint must collapse duplicate siblings; otherwise each distinct + # duplicate count yields a distinct fingerprint, defeating deduplication and + # wrongly tripping the comparison budget on a valid query. + repeated = " ".join( + "m: mother { " + "name " * i + "}" for i in range(1, 701) + ) + assert_valid( + f""" + fragment redundantSiblings on Dog {{ + {repeated} + }} + """ + ) + def modest_differing_arguments_still_report_the_real_conflict(): # Below the budget the ordinary, specific conflict must still be reported # rather than the "too complex" fallback.