Skip to content

Commit b26794c

Browse files
authored
Add new CompositionOption to bail if total number of subgraph path infos in the stack gets above a configured value. (#3254)
1 parent 2499e1f commit b26794c

File tree

5 files changed

+137
-11
lines changed

5 files changed

+137
-11
lines changed

.changeset/four-panthers-itch.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@apollo/composition": patch
3+
"@apollo/federation-internals": patch
4+
---
5+
6+
Adding new CompositionOption `maxValidationSubgraphPaths`. This value represents the maximum number of SubgraphPathInfo objects that may exist in a ValidationTraversal when checking for satisfiability. Setting this value can help composition error before running out of memory. Default is 1,000,000.

composition-js/src/__tests__/validation_errors.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,3 +412,80 @@ describe('when shared field has non-intersecting runtime types in different subg
412412
]);
413413
});
414414
});
415+
416+
describe('other validation errors', () => {
417+
418+
it('errors when maxValidationSubgraphPaths is exceeded', () => {
419+
const subgraphA = {
420+
name: 'A',
421+
typeDefs: gql`
422+
type Query {
423+
a: A
424+
}
425+
426+
type A @key(fields: "id") {
427+
id: ID!
428+
b: B
429+
c: C
430+
d: D
431+
}
432+
433+
type B @key(fields: "id") {
434+
id: ID!
435+
a: A @shareable
436+
b: Int @shareable
437+
c: C @shareable
438+
d: D @shareable
439+
}
440+
441+
type C @key(fields: "id") {
442+
id: ID!
443+
a: A @shareable
444+
b: B @shareable
445+
c: Int @shareable
446+
d: D @shareable
447+
}
448+
449+
type D @key(fields: "id") {
450+
id: ID!
451+
a: A @shareable
452+
b: B @shareable
453+
c: C @shareable
454+
d: Int @shareable
455+
}
456+
`
457+
};
458+
const subgraphB = {
459+
name: 'B',
460+
typeDefs: gql`
461+
type B @key(fields: "id") {
462+
id: ID!
463+
b: Int @shareable
464+
c: C @shareable
465+
d: D @shareable
466+
}
467+
468+
type C @key(fields: "id") {
469+
id: ID!
470+
b: B @shareable
471+
c: Int @shareable
472+
d: D @shareable
473+
}
474+
475+
type D @key(fields: "id") {
476+
id: ID!
477+
b: B @shareable
478+
c: C @shareable
479+
d: Int @shareable
480+
}
481+
`
482+
};
483+
const result = composeAsFed2Subgraphs([subgraphA, subgraphB], { maxValidationSubgraphPaths: 10 });
484+
expect(result.errors).toBeDefined();
485+
expect(errorMessages(result)).toMatchStringArray([
486+
`
487+
Maximum number of validation subgraph paths exceeded: 12
488+
`
489+
]);
490+
});
491+
});

composition-js/src/compose.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ export interface CompositionOptions {
3939
allowedFieldTypeMergingSubtypingRules?: SubtypingRule[];
4040
/// Flag to toggle if satisfiability should be performed during composition
4141
runSatisfiability?: boolean;
42+
/// Maximum allowable number of outstanding subgraph paths to validate
43+
maxValidationSubgraphPaths?: number;
4244
}
4345

4446
function validateCompositionOptions(options: CompositionOptions) {
@@ -55,7 +57,7 @@ function validateCompositionOptions(options: CompositionOptions) {
5557
* @param options CompositionOptions
5658
*/
5759
export function compose(subgraphs: Subgraphs, options: CompositionOptions = {}): CompositionResult {
58-
const { runSatisfiability = true, sdlPrintOptions } = options;
60+
const { runSatisfiability = true, sdlPrintOptions, maxValidationSubgraphPaths } = options;
5961

6062
validateCompositionOptions(options);
6163

@@ -67,8 +69,8 @@ export function compose(subgraphs: Subgraphs, options: CompositionOptions = {}):
6769
let satisfiabilityResult;
6870
if (runSatisfiability) {
6971
satisfiabilityResult = validateSatisfiability({
70-
supergraphSchema: mergeResult.supergraph
71-
});
72+
supergraphSchema: mergeResult.supergraph,
73+
}, { maxValidationSubgraphPaths });
7274
if (satisfiabilityResult.errors) {
7375
return { errors: satisfiabilityResult.errors };
7476
}
@@ -123,7 +125,7 @@ type SatisfiabilityArgs = {
123125
* @param args: SatisfiabilityArgs
124126
* @returns { errors? : GraphQLError[], hints? : CompositionHint[] }
125127
*/
126-
export function validateSatisfiability({ supergraphSchema, supergraphSdl} : SatisfiabilityArgs) : {
128+
export function validateSatisfiability({ supergraphSchema, supergraphSdl} : SatisfiabilityArgs, options: CompositionOptions = {}) : {
127129
errors? : GraphQLError[],
128130
hints? : CompositionHint[],
129131
} {
@@ -133,7 +135,7 @@ export function validateSatisfiability({ supergraphSchema, supergraphSdl} : Sati
133135
const supergraph = supergraphSchema ? new Supergraph(supergraphSchema, null) : Supergraph.build(supergraphSdl, { supportedFeatures: null });
134136
const supergraphQueryGraph = buildSupergraphAPIQueryGraph(supergraph);
135137
const federatedQueryGraph = buildFederatedQueryGraph(supergraph, false);
136-
return validateGraphComposition(supergraph.schema, supergraph.subgraphNameToGraphEnumValue(), supergraphQueryGraph, federatedQueryGraph);
138+
return validateGraphComposition(supergraph.schema, supergraph.subgraphNameToGraphEnumValue(), supergraphQueryGraph, federatedQueryGraph, options);
137139
}
138140

139141
type ValidateSubgraphsAndMergeResult = MergeResult | { errors: GraphQLError[] };

composition-js/src/validate.ts

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ import {
6262
} from "@apollo/query-graphs";
6363
import { CompositionHint, HINTS } from "./hints";
6464
import { ASTNode, GraphQLError, print } from "graphql";
65+
import { CompositionOptions } from './compose';
6566

6667
const debug = newDebugLogger('validation');
6768

@@ -310,6 +311,7 @@ export function validateGraphComposition(
310311
subgraphNameToGraphEnumValue: Map<string, string>,
311312
supergraphAPI: QueryGraph,
312313
federatedQueryGraph: QueryGraph,
314+
compositionOptions: CompositionOptions = {},
313315
): {
314316
errors? : GraphQLError[],
315317
hints? : CompositionHint[],
@@ -319,6 +321,7 @@ export function validateGraphComposition(
319321
subgraphNameToGraphEnumValue,
320322
supergraphAPI,
321323
federatedQueryGraph,
324+
compositionOptions,
322325
).validate();
323326
return errors.length > 0 ? { errors, hints } : { hints };
324327
}
@@ -695,19 +698,26 @@ class ValidationTraversal {
695698
private readonly validationHints: CompositionHint[] = [];
696699

697700
private readonly context: ValidationContext;
698-
701+
private totalValidationSubgraphPaths = 0;
702+
private maxValidationSubgraphPaths: number;
703+
704+
private static DEFAULT_MAX_VALIDATION_SUBGRAPH_PATHS = 1000000;
705+
699706
constructor(
700707
supergraphSchema: Schema,
701708
subgraphNameToGraphEnumValue: Map<string, string>,
702709
supergraphAPI: QueryGraph,
703710
federatedQueryGraph: QueryGraph,
711+
compositionOptions: CompositionOptions,
704712
) {
713+
this.maxValidationSubgraphPaths = compositionOptions.maxValidationSubgraphPaths ?? ValidationTraversal.DEFAULT_MAX_VALIDATION_SUBGRAPH_PATHS;
714+
705715
this.conditionResolver = simpleValidationConditionResolver({
706716
supergraph: supergraphSchema,
707717
queryGraph: federatedQueryGraph,
708718
withCaching: true,
709719
});
710-
supergraphAPI.rootKinds().forEach((kind) => this.stack.push(ValidationState.initial({
720+
supergraphAPI.rootKinds().forEach((kind) => this.pushStack(ValidationState.initial({
711721
supergraphAPI,
712722
kind,
713723
federatedQueryGraph,
@@ -720,18 +730,38 @@ class ValidationTraversal {
720730
subgraphNameToGraphEnumValue,
721731
);
722732
}
733+
734+
pushStack(state: ValidationState): { error?: GraphQLError } {
735+
this.totalValidationSubgraphPaths += state.subgraphPathInfos.length;
736+
this.stack.push(state);
737+
if (this.totalValidationSubgraphPaths > this.maxValidationSubgraphPaths) {
738+
return { error: ERRORS.MAX_VALIDATION_SUBGRAPH_PATHS_EXCEEDED.err(`Maximum number of validation subgraph paths exceeded: ${this.totalValidationSubgraphPaths}`) };
739+
}
740+
return {};
741+
}
742+
743+
popStack() {
744+
const state = this.stack.pop();
745+
if (state) {
746+
this.totalValidationSubgraphPaths -= state.subgraphPathInfos.length;
747+
}
748+
return state;
749+
}
723750

724751
validate(): {
725752
errors: GraphQLError[],
726753
hints: CompositionHint[],
727754
} {
728755
while (this.stack.length > 0) {
729-
this.handleState(this.stack.pop()!);
756+
const { error } = this.handleState(this.popStack()!);
757+
if (error) {
758+
return { errors: [error], hints: this.validationHints };
759+
}
730760
}
731761
return { errors: this.validationErrors, hints: this.validationHints };
732762
}
733763

734-
private handleState(state: ValidationState) {
764+
private handleState(state: ValidationState): { error?: GraphQLError } {
735765
debug.group(() => `Validation: ${this.stack.length + 1} open states. Validating ${state}`);
736766
const vertex = state.supergraphPath.tail;
737767

@@ -748,7 +778,7 @@ class ValidationTraversal {
748778
// type, and have strictly more options regarding subgraphs. So whatever comes next, we can handle in the exact
749779
// same way we did previously, and there is thus no way to bother.
750780
debug.groupEnd(`Has already validated this vertex.`);
751-
return;
781+
return {};
752782
}
753783
}
754784
// We're gonna have to validate, but we can save the new set of sources here to hopefully save work later.
@@ -799,12 +829,16 @@ class ValidationTraversal {
799829
// state to the stack this method, `handleState`, will do nothing later. But it's
800830
// worth checking it now and save some memory/cycles.
801831
if (newState && !newState.supergraphPath.isTerminal()) {
802-
this.stack.push(newState);
832+
const { error } = this.pushStack(newState);
833+
if (error) {
834+
return { error };
835+
}
803836
debug.groupEnd(() => `Reached new state ${newState}`);
804837
} else {
805838
debug.groupEnd(`Reached terminal vertex/cycle`);
806839
}
807840
}
808841
debug.groupEnd();
842+
return {};
809843
}
810844
}

internals-js/src/error.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -627,6 +627,12 @@ const LIST_SIZE_INVALID_SIZED_FIELD = makeCodeDefinition(
627627
{ addedIn: '2.9.2' },
628628
);
629629

630+
const MAX_VALIDATION_SUBGRAPH_PATHS_EXCEEDED = makeCodeDefinition(
631+
'MAX_VALIDATION_SUBGRAPH_PATHS_EXCEEDED',
632+
'The maximum number of validation subgraph paths has been exceeded.',
633+
{ addedIn: '2.8.0' },
634+
);
635+
630636
export const ERROR_CATEGORIES = {
631637
DIRECTIVE_FIELDS_MISSING_EXTERNAL,
632638
DIRECTIVE_UNSUPPORTED_ON_INTERFACE,
@@ -727,6 +733,7 @@ export const ERRORS = {
727733
LIST_SIZE_INVALID_ASSUMED_SIZE,
728734
LIST_SIZE_INVALID_SIZED_FIELD,
729735
LIST_SIZE_INVALID_SLICING_ARGUMENT,
736+
MAX_VALIDATION_SUBGRAPH_PATHS_EXCEEDED,
730737
};
731738

732739
const codeDefByCode = Object.values(ERRORS).reduce((obj: {[code: string]: ErrorCodeDefinition}, codeDef: ErrorCodeDefinition) => { obj[codeDef.code] = codeDef; return obj; }, {});

0 commit comments

Comments
 (0)