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

This commit is contained in:
Lucas Pickering 2020-11-04 18:11:25 -05:00 committed by GitHub
parent eca049ac28
commit 200896053a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 118 additions and 22 deletions

View file

@ -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);
}

View file

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

View file

@ -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)) - 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 ## Breaking Changes
- `GraphQLType` trait was split into 2 traits: ([#685](https://github.com/graphql-rust/juniper/pull/685)) - `GraphQLType` trait was split into 2 traits: ([#685](https://github.com/graphql-rust/juniper/pull/685))

View file

@ -190,7 +190,7 @@ where
Self::build_from_selection_with_parent(s, None, vars, fragments) 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>, s: &'a Selection<'a, S>,
parent: Option<&mut Self>, parent: Option<&mut Self>,
vars: &'a Variables<S>, vars: &'a Variables<S>,

View file

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