Skip to content

Commit 77767b8

Browse files
committed
doesn't infinitely recurse on fragment cycle
1 parent 95fcced commit 77767b8

File tree

2 files changed

+42
-5
lines changed

2 files changed

+42
-5
lines changed

src/validation/__tests__/MaxIntrospectionDepthRule-test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -536,4 +536,19 @@ describe('Validate: Max introspection nodes rule', () => {
536536
}
537537
`);
538538
});
539+
540+
it("doesn't infinitely recurse on fragment cycle", () => {
541+
expectValid(`
542+
query test {
543+
__schema {
544+
types {
545+
...Cycle
546+
}
547+
}
548+
}
549+
fragment Cycle on __Type {
550+
...Cycle
551+
}
552+
`);
553+
});
539554
});

src/validation/rules/MaxIntrospectionDepthRule.ts

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,36 @@ export function MaxIntrospectionDepthRule(
1515
* Counts the depth of list fields in "__Type" recursively and
1616
* returns `true` if the limit has been reached.
1717
*/
18-
function checkDepth(node: ASTNode, depth: number = 0): boolean {
18+
function checkDepth(
19+
node: ASTNode,
20+
visitedFragments: {
21+
[fragmentName: string]: true | undefined;
22+
} = Object.create(null),
23+
depth: number = 0,
24+
): boolean {
1925
if (node.kind === Kind.FRAGMENT_SPREAD) {
20-
const fragment = context.getFragment(node.name.value);
26+
const fragmentName = node.name.value;
27+
if (visitedFragments[fragmentName]) {
28+
// Fragment cycles are handled by `NoFragmentCyclesRule`.
29+
return false;
30+
}
31+
const fragment = context.getFragment(fragmentName);
2132
if (!fragment) {
22-
// missing fragments checks are handled by the `KnownFragmentNamesRule`
33+
// Missing fragments checks are handled by the `KnownFragmentNamesRule`.
2334
return false;
2435
}
25-
return checkDepth(fragment, depth);
36+
37+
// Rather than following an immutable programming pattern which has
38+
// significant memory and garbage collection overhead, we've opted to
39+
// take a mutable approach for efficiency's sake. Importantly visiting a
40+
// fragment twice is fine, so long as you don't do one visit inside the
41+
// other.
42+
try {
43+
visitedFragments[fragmentName] = true;
44+
return checkDepth(fragment, visitedFragments, depth);
45+
} finally {
46+
visitedFragments[fragmentName] = undefined;
47+
}
2648
}
2749

2850
if (
@@ -43,7 +65,7 @@ export function MaxIntrospectionDepthRule(
4365
// handles fields and inline fragments
4466
if ('selectionSet' in node && node.selectionSet) {
4567
for (const child of node.selectionSet.selections) {
46-
if (checkDepth(child, depth)) {
68+
if (checkDepth(child, visitedFragments, depth)) {
4769
return true;
4870
}
4971
}

0 commit comments

Comments
 (0)