From 48d5cbe798d99a9d34eb0e3b0af11aea25059f61 Mon Sep 17 00:00:00 2001 From: Benjie Date: Fri, 21 Jun 2024 14:26:09 +0100 Subject: [PATCH] Backport "Prevent Infinite Loop in OverlappingFieldsCanBeMergedRule" to v15 (#4000) fix from /~https://github.com/graphql/graphql-js/pull/3442 --- .../OverlappingFieldsCanBeMergedRule-test.js | 45 +++++++++++++++++++ .../rules/OverlappingFieldsCanBeMergedRule.js | 18 ++++++++ 2 files changed, 63 insertions(+) diff --git a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.js b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.js index 080f859b89..0fec7be6fc 100644 --- a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.js +++ b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.js @@ -1011,18 +1011,63 @@ describe('Validate: Overlapping fields can be merged', () => { it('does not infinite loop on recursive fragment', () => { expectValid(` + { + ...fragA + } + fragment fragA on Human { name, relatives { name, ...fragA } } `); }); it('does not infinite loop on immediately recursive fragment', () => { expectValid(` + { + ...fragA + } + fragment fragA on Human { name, ...fragA } `); }); + it('does not infinite loop on recursive fragment with a field named after fragment', () => { + expectValid(` + { + ...fragA + fragA + } + fragment fragA on Query { ...fragA } + `); + }); + + it('finds invalid cases even with field named after fragment', () => { + expectErrors(` + { + fragA + ...fragA + } + + fragment fragA on Type { + fragA: b + } + `).to.deep.equal([ + { + message: + 'Fields "fragA" conflict because "fragA" and "b" are different fields. Use different aliases on the fields to fetch both if this was intentional.', + locations: [ + { line: 3, column: 9 }, + { line: 8, column: 9 }, + ], + }, + ]); + }); + it('does not infinite loop on transitively recursive fragment', () => { expectValid(` + { + ...fragA + fragB + } + fragment fragA on Human { name, ...fragB } fragment fragB on Human { name, ...fragC } fragment fragC on Human { name, ...fragA } diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.js b/src/validation/rules/OverlappingFieldsCanBeMergedRule.js index 2d79dd098f..4b1dbb0eca 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.js +++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.js @@ -264,6 +264,24 @@ function collectConflictsBetweenFieldsAndFragment( // (E) Then collect any conflicts between the provided collection of fields // and any fragment names found in the given fragment. for (let i = 0; i < fragmentNames2.length; i++) { + const referencedFragmentName = fragmentNames2[i]; + + // Memoize so two fragments are not compared for conflicts more than once. + if ( + comparedFragmentPairs.has( + referencedFragmentName, + fragmentName, + areMutuallyExclusive, + ) + ) { + continue; + } + comparedFragmentPairs.add( + referencedFragmentName, + fragmentName, + areMutuallyExclusive, + ); + collectConflictsBetweenFieldsAndFragment( context, conflicts,