From b2e5ab66f84688ec304cfcf2c6f749c86aded549 Mon Sep 17 00:00:00 2001 From: Chris Lenfest Date: Thu, 20 Jun 2024 14:53:57 -0500 Subject: [PATCH] generateQueryFragments error with nested selection set (#3043) Fixes #3042 generateQueryFragments does't handle repeated (... on B { ... on B ... on B } } }) fragments well. Given the query planner can actually generate these query structures, this causes this option to be potentially dangerous, generating invalid subgraph queries. --------- Co-authored-by: Marc-Andre Giroux --- .changeset/calm-candles-rest.md | 5 ++ internals-js/src/__tests__/operations.test.ts | 60 +++++++++++++++++++ internals-js/src/operations.ts | 7 ++- 3 files changed, 69 insertions(+), 3 deletions(-) create mode 100644 .changeset/calm-candles-rest.md diff --git a/.changeset/calm-candles-rest.md b/.changeset/calm-candles-rest.md new file mode 100644 index 000000000..cb2f7525e --- /dev/null +++ b/.changeset/calm-candles-rest.md @@ -0,0 +1,5 @@ +--- +"@apollo/federation-internals": patch +--- + +generateQueryFragments() could generate fragments with naming collisions when nested diff --git a/internals-js/src/__tests__/operations.test.ts b/internals-js/src/__tests__/operations.test.ts index 7e34d83d5..6e26b7fc2 100644 --- a/internals-js/src/__tests__/operations.test.ts +++ b/internals-js/src/__tests__/operations.test.ts @@ -34,6 +34,66 @@ function astSSet(...selections: SelectionNode[]): SelectionSetNode { }; } +describe('generate query fragments', () => { + test('generateQueryFragments handles repeated fragment spreads', () => { + const schema = parseSchema(` + type Query { + entities: [Entity] + } + + union Entity = A | B + + type A { + a: Int + } + + type B { + b: Int + } + `); + + const operation = parseOperation(schema, ` + query { + entities { + ... on B { + ... on B { + ... on B { + ... on B { + b + } + } + } + } + } + } + `); + + const withGeneratedFragments = operation.generateQueryFragments(); + console.log(withGeneratedFragments.toString()); + expect(withGeneratedFragments.toString()).toMatchString(` + fragment _generated_onB1_0 on B { + ... on B { + b + } + } + + fragment _generated_onB1_1 on B { + ..._generated_onB1_0 + } + + fragment _generated_onB1_2 on B { + ..._generated_onB1_1 + } + + { + entities { + ..._generated_onB1_2 + } + } + `); + }); +}); + describe('fragments optimization', () => { // Takes a query with fragments as inputs, expand all those fragments, and ensures that all the // fragments gets optimized back, and that we get back the exact same query. diff --git a/internals-js/src/operations.ts b/internals-js/src/operations.ts index 3ac780491..fb2adb6a9 100644 --- a/internals-js/src/operations.ts +++ b/internals-js/src/operations.ts @@ -1594,16 +1594,17 @@ export class SelectionSet { // No match, so we need to create a new fragment. First, we minimize the // selection set before creating the fragment with it. const [minimizedSelectionSet] = selection.selectionSet.minimizeSelectionSet(namedFragments, seenSelections); + const updatedEquivalentSelectionSetCandidates = seenSelections.get(mockHashCode); // may have changed after previous statement const fragmentDefinition = new NamedFragmentDefinition( this.parentType.schema(), - `_generated_${mockHashCode}_${equivalentSelectionSetCandidates?.length ?? 0}`, + `_generated_${mockHashCode}_${updatedEquivalentSelectionSetCandidates?.length ?? 0}`, selection.element.typeCondition ).setSelectionSet(minimizedSelectionSet); namedFragments.add(fragmentDefinition); // Create a new "hash code" bucket or add to the existing one. - if (equivalentSelectionSetCandidates) { - equivalentSelectionSetCandidates.push([selection.selectionSet, fragmentDefinition]); + if (updatedEquivalentSelectionSetCandidates) { + updatedEquivalentSelectionSetCandidates.push([selection.selectionSet, fragmentDefinition]); } else { seenSelections.set(mockHashCode, [[selection.selectionSet, fragmentDefinition]]); }