Fix #500 (crash from look_ahead on nested fragments) (#800)

Author: LucasPickering

integration_tests/juniper_tests/src/issue_500.rs

@@ -0,0 +1,92 @@
+use juniper::*;
+
+struct Query;
+
+#[juniper::graphql_object]
+impl Query {
+    fn users(executor: &Executor) -> Vec<User> {
+        executor.look_ahead();
+
+        vec![User {
+            city: City {
+                country: Country { id: 1 },
+            },
+        }]
+    }
+}
+
+struct User {
+    city: City,
+}
+
+#[juniper::graphql_object]
+impl User {
+    fn city(&self, executor: &Executor) -> &City {
+        executor.look_ahead();
+        &self.city
+    }
+}
+
+struct City {
+    country: Country,
+}
+
+#[juniper::graphql_object]
+impl City {
+    fn country(&self, executor: &Executor) -> &Country {
+        executor.look_ahead();
+        &self.country
+    }
+}
+
+struct Country {
+    id: i32,
+}
+
+#[juniper::graphql_object]
+impl Country {
+    fn id(&self) -> i32 {
+        self.id
+    }
+}
+
+type Schema = juniper::RootNode<'static, Query, EmptyMutation<()>, EmptySubscription<()>>;
+
+#[tokio::test]
+async fn test_nested_fragments() {
+    let query = r#"
+        query Query {
+            users {
+                ...UserFragment
+            }
+        }
+
+        fragment UserFragment on User {
+            city {
+                ...CityFragment
+            }
+        }
+
+        fragment CityFragment on City {
+            country {
+                ...CountryFragment
+            }
+        }
+
+        fragment CountryFragment on Country {
+            id
+        }
+    "#;
+
+    let (_, errors) = juniper::execute(
+        query,
+        None,
+        &Schema::new(Query, EmptyMutation::new(), EmptySubscription::new()),
+        &Variables::new(),
+        &(),
+    )
+    .await
+    .unwrap();
+
+    assert_eq!(errors.len(), 0);
+}

integration_tests/juniper_tests/src/lib.rs

@@ -12,3 +12,5 @@ mod infallible_as_field_error;
 mod issue_371;
 #[cfg(test)]
 mod issue_398;
+#[cfg(test)]
+mod issue_500;

juniper/CHANGELOG.md

@@ -84,6 +84,8 @@
 
 - When enabled, the optional `bson` integration now requires `bson-1.0.0`. ([#678](https://github.com/graphql-rust/juniper/pull/678))
 
+- Fixed panic on `executor.look_ahead()` for nested fragments ([#500](https://github.com/graphql-rust/juniper/issues/500))
+
 ## Breaking Changes
 
 - `GraphQLType` trait was split into 2 traits: ([#685](https://github.com/graphql-rust/juniper/pull/685))

juniper/src/executor/look_ahead.rs

@@ -190,7 +190,7 @@ where
         Self::build_from_selection_with_parent(s, None, vars, fragments)
     }
 
-    fn build_from_selection_with_parent(
+    pub(super) fn build_from_selection_with_parent(
         s: &'a Selection<'a, S>,
         parent: Option<&mut Self>,
         vars: &'a Variables<S>,

juniper/src/executor/mod.rs

@@ -654,6 +654,7 @@ where
         };
         self.parent_selection_set
             .map(|p| {
+                // Search the parent's fields to find this field within the set
                 let found_field = p.iter().find(|&x| {
                     match *x {
                         Selection::Field(ref field) => {
@@ -672,31 +673,30 @@ where
                     None
                 }
             })
-            .filter(|s| s.is_some())
+            .flatten()
             .unwrap_or_else(|| {
-                Some(LookAheadSelection {
-                    name: self.current_type.innermost_concrete().name().unwrap_or(""),
+                // We didn't find a field in the parent's selection matching
+                // this field, which means we're inside a FragmentSpread
+                let mut ret = LookAheadSelection {
+                    name: field_name,
                     alias: None,
                     arguments: Vec::new(),
-                    children: self
-                        .current_selection_set
-                        .map(|s| {
-                            s.iter()
-                                .map(|s| ChildSelection {
-                                    inner: LookAheadSelection::build_from_selection(
-                                        &s,
-                                        self.variables,
-                                        self.fragments,
-                                    )
-                                    .expect("a child selection"),
-                                    applies_for: Applies::All,
-                                })
-                                .collect()
-                        })
-                        .unwrap_or_else(Vec::new),
-                })
+                    children: Vec::new(),
+                };
+
+                // Add in all the children - this will mutate `ret`
+                if let Some(selection_set) = self.current_selection_set {
+                    for c in selection_set {
+                        LookAheadSelection::build_from_selection_with_parent(
+                            c,
+                            Some(&mut ret),
+                            self.variables,
+                            self.fragments,
+                        );
+                    }
+                }
+                ret
             })
-            .unwrap_or_default()
     }
 
     /// Create new `OwnedExecutor` and clone all current data