Skip to content

Commit 4175b26

Browse files
Prevent Infinite Loop in OverlappingFieldsCanBeMergedRule (#3442)
Co-authored-by: Ivan Goncharov <[email protected]>
1 parent 29b811f commit 4175b26

File tree

2 files changed

+62
-0
lines changed

2 files changed

+62
-0
lines changed

src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts

+46
Original file line numberDiff line numberDiff line change
@@ -1011,18 +1011,64 @@ describe('Validate: Overlapping fields can be merged', () => {
10111011

10121012
it('does not infinite loop on recursive fragment', () => {
10131013
expectValid(`
1014+
{
1015+
...fragA
1016+
}
1017+
10141018
fragment fragA on Human { name, relatives { name, ...fragA } }
10151019
`);
10161020
});
10171021

10181022
it('does not infinite loop on immediately recursive fragment', () => {
10191023
expectValid(`
1024+
{
1025+
...fragA
1026+
}
1027+
10201028
fragment fragA on Human { name, ...fragA }
10211029
`);
10221030
});
10231031

1032+
it('does not infinite loop on recursive fragment with a field named after fragment', () => {
1033+
expectValid(`
1034+
{
1035+
...fragA
1036+
fragA
1037+
}
1038+
1039+
fragment fragA on Query { ...fragA }
1040+
`);
1041+
});
1042+
1043+
it('finds invalid cases even with field named after fragment', () => {
1044+
expectErrors(`
1045+
{
1046+
fragA
1047+
...fragA
1048+
}
1049+
1050+
fragment fragA on Type {
1051+
fragA: b
1052+
}
1053+
`).toDeepEqual([
1054+
{
1055+
message:
1056+
'Fields "fragA" conflict because "fragA" and "b" are different fields. Use different aliases on the fields to fetch both if this was intentional.',
1057+
locations: [
1058+
{ line: 3, column: 9 },
1059+
{ line: 8, column: 9 },
1060+
],
1061+
},
1062+
]);
1063+
});
1064+
10241065
it('does not infinite loop on transitively recursive fragment', () => {
10251066
expectValid(`
1067+
{
1068+
...fragA
1069+
fragB
1070+
}
1071+
10261072
fragment fragA on Human { name, ...fragB }
10271073
fragment fragB on Human { name, ...fragC }
10281074
fragment fragC on Human { name, ...fragA }

src/validation/rules/OverlappingFieldsCanBeMergedRule.ts

+16
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,22 @@ function collectConflictsBetweenFieldsAndFragment(
266266
// (E) Then collect any conflicts between the provided collection of fields
267267
// and any fragment names found in the given fragment.
268268
for (const referencedFragmentName of referencedFragmentNames) {
269+
// Memoize so two fragments are not compared for conflicts more than once.
270+
if (
271+
comparedFragmentPairs.has(
272+
referencedFragmentName,
273+
fragmentName,
274+
areMutuallyExclusive,
275+
)
276+
) {
277+
continue;
278+
}
279+
comparedFragmentPairs.add(
280+
referencedFragmentName,
281+
fragmentName,
282+
areMutuallyExclusive,
283+
);
284+
269285
collectConflictsBetweenFieldsAndFragment(
270286
context,
271287
conflicts,

0 commit comments

Comments
 (0)