Skip to content

Commit c7b310f

Browse files
committed
Prevent infinite loop in OverlappingFieldsCanBeMergedRule
Preponed to avoid possible denial of service attacks Replicates graphql/graphql-js@4175b26
1 parent cf85554 commit c7b310f

File tree

2 files changed

+57
-0
lines changed

2 files changed

+57
-0
lines changed

src/graphql/validation/rules/overlapping_fields_can_be_merged.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,15 @@ def collect_conflicts_between_fields_and_fragment(
254254
# (E) Then collect any conflicts between the provided collection of fields and any
255255
# fragment names found in the given fragment.
256256
for referenced_fragment_name in referenced_fragment_names:
257+
# Memoize so two fragments are not compared for conflicts more than once.
258+
if compared_fragment_pairs.has(
259+
referenced_fragment_name, fragment_name, are_mutually_exclusive
260+
):
261+
continue
262+
compared_fragment_pairs.add(
263+
referenced_fragment_name, fragment_name, are_mutually_exclusive
264+
)
265+
257266
collect_conflicts_between_fields_and_fragment(
258267
context,
259268
conflicts,

tests/validation/test_overlapping_fields_can_be_merged.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,20 +1042,68 @@ def works_for_field_names_that_are_python_keywords():
10421042
def does_not_infinite_loop_on_recursive_fragments():
10431043
assert_valid(
10441044
"""
1045+
{
1046+
...fragA
1047+
}
1048+
10451049
fragment fragA on Human { name, relatives { name, ...fragA } }
10461050
"""
10471051
)
10481052

10491053
def does_not_infinite_loop_on_immediately_recursive_fragments():
10501054
assert_valid(
10511055
"""
1056+
{
1057+
...fragA
1058+
}
1059+
10521060
fragment fragA on Human { name, ...fragA }
10531061
"""
10541062
)
10551063

1064+
def does_not_infinite_loop_on_recursive_fragment_with_field_named_after_fragment():
1065+
assert_valid(
1066+
"""
1067+
{
1068+
...fragA
1069+
fragA
1070+
}
1071+
1072+
fragment fragA on Query { ...fragA }
1073+
"""
1074+
)
1075+
1076+
def finds_invalid_cases_even_with_field_named_after_fragment():
1077+
assert_errors(
1078+
"""
1079+
{
1080+
fragA
1081+
...fragA
1082+
}
1083+
1084+
fragment fragA on Type {
1085+
fragA: b
1086+
}
1087+
""",
1088+
[
1089+
{
1090+
"message": "Fields 'fragA' conflict"
1091+
" because 'fragA' and 'b' are different fields."
1092+
" Use different aliases on the fields"
1093+
" to fetch both if this was intentional.",
1094+
"locations": [(3, 15), (8, 15)],
1095+
}
1096+
],
1097+
)
1098+
10561099
def does_not_infinite_loop_on_transitively_recursive_fragments():
10571100
assert_valid(
10581101
"""
1102+
{
1103+
...fragA
1104+
fragB
1105+
}
1106+
10591107
fragment fragA on Human { name, ...fragB }
10601108
fragment fragB on Human { name, ...fragC }
10611109
fragment fragC on Human { name, ...fragA }

0 commit comments

Comments
 (0)