Skip to content

Commit

Permalink
Fix validation stack overflow for fragment cycles inside nested fields
Browse files Browse the repository at this point in the history
Previously fragment cycle detection only counted fragments that are
directly recursive, where the top-level selection for the fragment
included a fragment spread of itself. But spreading a fragment anywhere
inside a nested field is also problematic and prohibited by spec,
because even if the field you're spreading into is nullable, the client
can't know how deep that recursion goes, and it could be infinite.

Fixes #716
  • Loading branch information
goto-bus-stop committed Nov 9, 2023
1 parent ef45fd0 commit f7fd841
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 4 deletions.
4 changes: 3 additions & 1 deletion crates/apollo-compiler/src/validation/fragment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,9 @@ pub(crate) fn validate_fragment_cycles(
ast::Selection::InlineFragment(inline) => {
detect_fragment_cycles(named_fragments, &inline.selection_set, visited)?;
}
_ => {}
ast::Selection::Field(field) => {
detect_fragment_cycles(named_fragments, &field.selection_set, visited)?;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,16 @@ type Query {
# Query
{
human { ...fragA }
__schema { types { ...cycle1 } }
}
fragment fragA on Human { name, ...fragB }
fragment fragB on Human { name, ...fragC }
fragment fragC on Human { name, ...fragA }

# Indirect cycle
fragment cycle1 on __Type { kind, ...cycle2 }

fragment cycle2 on __Type {
kind
ofType { ...cycle1 }
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,23 @@
Error: `fragA` fragment cannot reference itself
╭─[0092_recursive_fragment_spread.graphql:13:1]
╭─[0092_recursive_fragment_spread.graphql:14:1]
13 │ fragment fragA on Human { name, ...fragB }
14 │ fragment fragA on Human { name, ...fragB }
│ ───────┬──────
│ ╰──────── recursive fragment definition
15 │ fragment fragC on Human { name, ...fragA }
16 │ fragment fragC on Human { name, ...fragA }
│ ────┬───
│ ╰───── refers to itself here
────╯
Error: `cycle1` fragment cannot reference itself
╭─[0092_recursive_fragment_spread.graphql:19:1]
19 │ fragment cycle1 on __Type { kind, ...cycle2 }
│ ───────┬───────
│ ╰───────── recursive fragment definition
23 │ ofType { ...cycle1 }
│ ────┬────
│ ╰────── refers to itself here
────╯

Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ query {
human {
...fragA
}
__schema {
types {
...cycle1
}
}
}

fragment fragA on Human {
Expand All @@ -26,3 +31,15 @@ fragment fragC on Human {
name
...fragA
}

fragment cycle1 on __Type {
kind
...cycle2
}

fragment cycle2 on __Type {
kind
ofType {
...cycle1
}
}

0 comments on commit f7fd841

Please sign in to comment.