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..be0bdd20 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 ( @@ -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"] @@ -62,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 @@ -75,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( @@ -175,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]: @@ -199,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, ) @@ -212,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, @@ -227,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, @@ -241,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, @@ -287,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, @@ -303,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, @@ -315,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, @@ -386,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, @@ -402,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, @@ -416,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, @@ -427,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, @@ -465,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, @@ -482,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, @@ -497,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, @@ -513,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, @@ -527,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.""" @@ -539,13 +587,17 @@ 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, compared_fields_and_fragment_pairs, compared_fragment_pairs, + budget, # within one collection is never mutually exclusive False, response_name, @@ -558,12 +610,109 @@ def collect_conflicts_within( conflicts.append(conflict) +def deduplicate_fields(fields: list[NodeAndDef]) -> list[NodeAndDef]: + """Deduplicate structurally equivalent fields. + + 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[tuple[Hashable, ...]] = 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) -> tuple[Hashable, ...]: + """Build a canonical key identifying a field's structure. + + 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 + 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), + ) + + +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, 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( + 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 () + ) + ) + + def collect_conflicts_between( context: ValidationContext, conflicts: list[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, field_map1: NodeAndDefCollection, var_map1: VarMap, @@ -585,13 +734,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, @@ -608,6 +764,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, @@ -620,6 +777,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 @@ -681,6 +842,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, @@ -963,6 +1125,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/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..1e5d1a18 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,89 @@ 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( + 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} + }} + """ + ) + + @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( + 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( """ @@ -1612,3 +1698,74 @@ 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} }} + """ + ) + + @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. + 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