diff --git a/src/execution/__tests__/executor-test.ts b/src/execution/__tests__/executor-test.ts index c758d3e426..a283dcbf23 100644 --- a/src/execution/__tests__/executor-test.ts +++ b/src/execution/__tests__/executor-test.ts @@ -72,6 +72,47 @@ describe('Execute: Handles basic execution tasks', () => { ); }); + it('works with deeply nested fragments', async () => { + const DataType: GraphQLObjectType = new GraphQLObjectType({ + name: 'Query', + fields: () => ({ + a: { type: GraphQLString, resolve: () => 'Apple' }, + }), + }); + + const n = 10000; + const fragments = Array.from(Array(n).keys()).reduce( + (acc, next) => + acc.concat(`\n + fragment X${next + 1} on Query { + ...X${next} + } + `), + '', + ); + + const document = parse(` + query { + ...X${n} + } + ${fragments} + fragment X0 on Query { + a + } + `); + + const result = await execute({ + schema: new GraphQLSchema({ query: DataType }), + document, + }); + + expect(result).to.deep.equal({ + data: { + a: 'Apple', + }, + }); + }); + it('executes arbitrary code', async () => { const data = { a: () => 'Apple', diff --git a/src/execution/collectFields.ts b/src/execution/collectFields.ts index d0961bfae8..e70f70ce69 100644 --- a/src/execution/collectFields.ts +++ b/src/execution/collectFields.ts @@ -21,6 +21,18 @@ import { typeFromAST } from '../utilities/typeFromAST'; import { getDirectiveValues } from './values'; +interface FieldEntry { + selection: FieldNode; + name: string; +} + +interface EntryWithSelectionset { + selectionSet: SelectionSetNode; + runtimeType: GraphQLObjectType; +} + +type StackEntry = EntryWithSelectionset | FieldEntry; + /** * Given a selectionSet, collects all of the fields and returns them. * @@ -37,16 +49,32 @@ export function collectFields( runtimeType: GraphQLObjectType, selectionSet: SelectionSetNode, ): Map> { + const stack: Array = [{ selectionSet, runtimeType }]; const fields = new Map(); - collectFieldsImpl( - schema, - fragments, - variableValues, - runtimeType, - selectionSet, - fields, - new Set(), - ); + const visited = new Set(); + + let entry; + while ((entry = stack.shift()) !== undefined) { + if ('selectionSet' in entry) { + collectFieldsImpl( + schema, + fragments, + variableValues, + entry.runtimeType, + entry.selectionSet, + visited, + stack, + ); + } else { + const fieldList = fields.get(entry.name); + if (fieldList !== undefined) { + fieldList.push(entry.selection); + } else { + fields.set(entry.name, [entry.selection]); + } + } + } + return fields; } @@ -68,20 +96,36 @@ export function collectSubfields( fieldNodes: ReadonlyArray, ): Map> { const subFieldNodes = new Map(); + const stack: Array = []; const visitedFragmentNames = new Set(); for (const node of fieldNodes) { if (node.selectionSet) { + stack.push({ selectionSet: node.selectionSet, runtimeType: returnType }); + } + } + + let entry; + while ((entry = stack.shift()) !== undefined) { + if ('selectionSet' in entry) { collectFieldsImpl( schema, fragments, variableValues, - returnType, - node.selectionSet, - subFieldNodes, + entry.runtimeType, + entry.selectionSet, visitedFragmentNames, + stack, ); + } else { + const fieldList = subFieldNodes.get(entry.name); + if (fieldList !== undefined) { + fieldList.push(entry.selection); + } else { + subFieldNodes.set(entry.name, [entry.selection]); + } } } + return subFieldNodes; } @@ -91,9 +135,10 @@ function collectFieldsImpl( variableValues: { [variable: string]: unknown }, runtimeType: GraphQLObjectType, selectionSet: SelectionSetNode, - fields: Map>, visitedFragmentNames: Set, + stack: Array, ): void { + const discovered = []; for (const selection of selectionSet.selections) { switch (selection.kind) { case Kind.FIELD: { @@ -101,12 +146,7 @@ function collectFieldsImpl( continue; } const name = getFieldEntryKey(selection); - const fieldList = fields.get(name); - if (fieldList !== undefined) { - fieldList.push(selection); - } else { - fields.set(name, [selection]); - } + discovered.push({ selection, name }); break; } case Kind.INLINE_FRAGMENT: { @@ -116,15 +156,7 @@ function collectFieldsImpl( ) { continue; } - collectFieldsImpl( - schema, - fragments, - variableValues, - runtimeType, - selection.selectionSet, - fields, - visitedFragmentNames, - ); + discovered.push({ selectionSet: selection.selectionSet, runtimeType }); break; } case Kind.FRAGMENT_SPREAD: { @@ -143,19 +175,16 @@ function collectFieldsImpl( ) { continue; } - collectFieldsImpl( - schema, - fragments, - variableValues, - runtimeType, - fragment.selectionSet, - fields, - visitedFragmentNames, - ); + + discovered.push({ selectionSet: fragment.selectionSet, runtimeType }); break; } } } + + if (discovered.length !== 0) { + stack.unshift(...discovered); + } } /** diff --git a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts index 46cf014e46..6365506e01 100644 --- a/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts +++ b/src/validation/__tests__/OverlappingFieldsCanBeMergedRule-test.ts @@ -1138,4 +1138,70 @@ describe('Validate: Overlapping fields can be merged', () => { }, ]); }); + + it('does not hit stack size limits', () => { + const n = 10000; + const fragments = Array.from(Array(n).keys()).reduce( + (acc, next) => + acc.concat(`\n + fragment X${next + 1} on Query { + ...X${next} + } + `), + '', + ); + + const query = ` + query Test { + ...X${n} + } + ${fragments} + fragment X0 on Query { + __typename + } + `; + + expectErrors(query).toDeepEqual([]); + }); + + it('finds conflicts in nested fragments', () => { + const n = 10000; + const fragments = Array.from(Array(n).keys()).reduce( + (acc, next) => + acc.concat(`\n + fragment X${next + 1} on Query { + ...X${next} + } + `), + '', + ); + + const query = ` + query Test { + type: conflict + ...X${n} + } + ${fragments} + fragment X0 on Query { + type: conflict2 + __typename + } + `; + expectErrors(query).toDeepEqual([ + { + locations: [ + { + column: 9, + line: 3, + }, + { + column: 9, + line: 50008, + }, + ], + message: + 'Fields "type" conflict because "conflict" and "conflict2" are different fields. Use different aliases on the fields to fetch both if this was intentional.', + }, + ]); + }); }); diff --git a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts index 4305064a6f..8f5b07756f 100644 --- a/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts +++ b/src/validation/rules/OverlappingFieldsCanBeMergedRule.ts @@ -191,6 +191,7 @@ function findConflictsWithinSelectionSet( fieldMap, ); + const discoveredFragments: Array<[string, string]> = []; if (fragmentNames.length !== 0) { // (B) Then collect conflicts between these fields and those represented by // each spread fragment name found. @@ -203,7 +204,9 @@ function findConflictsWithinSelectionSet( false, fieldMap, fragmentNames[i], + discoveredFragments, ); + // (C) Then compare this fragment with all other fragments found in this // selection set to collect conflicts between fragments spread together. // This compares each item in the list of fragment names to every other @@ -220,6 +223,16 @@ function findConflictsWithinSelectionSet( ); } } + + processDiscoveredFragments( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragmentPairs, + false, + fieldMap, + discoveredFragments, + ); } return conflicts; } @@ -234,6 +247,7 @@ function collectConflictsBetweenFieldsAndFragment( areMutuallyExclusive: boolean, fieldMap: NodeAndDefCollection, fragmentName: string, + discoveredFragments: Array>, ): void { const fragment = context.getFragment(fragmentName); if (!fragment) { @@ -264,35 +278,12 @@ function collectConflictsBetweenFieldsAndFragment( fieldMap2, ); - // (E) Then collect any conflicts between the provided collection of fields - // and any fragment names found in the given fragment. - for (const referencedFragmentName of referencedFragmentNames) { - // Memoize so two fragments are not compared for conflicts more than once. - if ( - comparedFragmentPairs.has( - referencedFragmentName, - fragmentName, - areMutuallyExclusive, - ) - ) { - continue; - } - comparedFragmentPairs.add( - referencedFragmentName, + discoveredFragments.push( + ...referencedFragmentNames.map((referencedFragmentName) => [ fragmentName, - areMutuallyExclusive, - ); - - collectConflictsBetweenFieldsAndFragment( - context, - conflicts, - cachedFieldsAndFragmentNames, - comparedFragmentPairs, - areMutuallyExclusive, - fieldMap, referencedFragmentName, - ); - } + ]), + ); } // Collect all conflicts found between two fragments, including via spreading in @@ -424,6 +415,7 @@ function findConflictsBetweenSubSelectionSets( // (I) Then collect conflicts between the first collection of fields and // those referenced by each fragment name associated with the second. + const discoveredFragments: Array> = []; for (const fragmentName2 of fragmentNames2) { collectConflictsBetweenFieldsAndFragment( context, @@ -433,9 +425,20 @@ function findConflictsBetweenSubSelectionSets( areMutuallyExclusive, fieldMap1, fragmentName2, + discoveredFragments, ); } + processDiscoveredFragments( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragmentPairs, + areMutuallyExclusive, + fieldMap1, + discoveredFragments, + ); + // (I) Then collect conflicts between the second collection of fields and // those referenced by each fragment name associated with the first. for (const fragmentName1 of fragmentNames1) { @@ -447,9 +450,20 @@ function findConflictsBetweenSubSelectionSets( areMutuallyExclusive, fieldMap2, fragmentName1, + discoveredFragments, ); } + processDiscoveredFragments( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragmentPairs, + areMutuallyExclusive, + fieldMap2, + discoveredFragments, + ); + // (J) Also collect conflicts between any fragment names by the first and // fragment names by the second. This compares each item in the first set of // names to each item in the second set of names. @@ -469,6 +483,47 @@ function findConflictsBetweenSubSelectionSets( return conflicts; } +// (E) Then collect any conflicts between the provided collection of fields +// and any fragment names found in the given fragment. +function processDiscoveredFragments( + context: ValidationContext, + conflicts: Array, + cachedFieldsAndFragmentNames: Map, + comparedFragmentPairs: PairSet, + areMutuallyExclusive: boolean, + fieldMap: NodeAndDefCollection, + discoveredFragments: Array>, +) { + let item; + while ((item = discoveredFragments.pop()) !== undefined) { + const [fragmentName, referencedFragmentName] = item; + if ( + comparedFragmentPairs.has( + referencedFragmentName, + fragmentName, + areMutuallyExclusive, + ) + ) { + continue; + } + comparedFragmentPairs.add( + referencedFragmentName, + fragmentName, + areMutuallyExclusive, + ); + collectConflictsBetweenFieldsAndFragment( + context, + conflicts, + cachedFieldsAndFragmentNames, + comparedFragmentPairs, + areMutuallyExclusive, + fieldMap, + referencedFragmentName, + discoveredFragments, + ); + } +} + // Collect all Conflicts "within" one collection of fields. function collectConflictsWithin( context: ValidationContext,