protect and test against cyclic loops

bump 0.0.13

Author: dotansimha

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "graphql-tools"
-version = "0.0.12"
+version = "0.0.13"
 edition = "2021"
 description = "Tools for working with GraphQL in Rust, based on graphql-parser Document."
 license = "MIT/Apache-2.0"

Cargo.toml

@@ -2,7 +2,7 @@ use super::ValidationRule;
 use crate::static_graphql::query::*;
 use crate::validation::utils::{ValidationError, ValidationErrorContext};
 use crate::{ast::QueryVisitor, validation::utils::ValidationContext};
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 
 /// Overlapping fields can be merged
 ///
@@ -17,6 +17,7 @@ struct FindOverlappingFieldsThatCanBeMergedHelper<'a> {
     discoverd_fields: HashMap<String, Field>,
     validation_context: &'a ValidationErrorContext<'a>,
     selection_set_errors: Vec<ValidationError>,
+    visited_fragments: HashSet<String>,
 }
 
 impl<'a> FindOverlappingFieldsThatCanBeMergedHelper<'a> {
@@ -25,6 +26,7 @@ impl<'a> FindOverlappingFieldsThatCanBeMergedHelper<'a> {
             discoverd_fields: HashMap::new(),
             selection_set_errors: Vec::new(),
             validation_context: ctx,
+            visited_fragments: HashSet::new(),
         }
     }
 
@@ -101,6 +103,9 @@ impl<'a> FindOverlappingFieldsThatCanBeMergedHelper<'a> {
                 }
 
                 Selection::FragmentSpread(fragment_spread) => {
+                  if !self.visited_fragments.contains(&fragment_spread.fragment_name) {
+                    self.visited_fragments.insert(fragment_spread.fragment_name.clone());
+
                     if let Some(fragment) = self
                         .validation_context
                         .ctx
@@ -116,6 +121,7 @@ impl<'a> FindOverlappingFieldsThatCanBeMergedHelper<'a> {
                         }
                     }
                 }
+              }
             }
         }
     }

src/validation/rules/overlapping_fields_can_be_merged.rs

@@ -53,36 +53,115 @@ pub fn validate<'a>(
 }
 
 #[test]
-fn test_validate_valid_query() {
+fn cyclic_fragment_should_never_loop() {
     use crate::validation::test_utils::*;
+    use crate::validation::rules::default_rules_validation_plan;
 
-    let mut default_plan = create_default_ruleset_plan();
-    let errors = test_operation_without_schema(
+    let mut default_plan = default_rules_validation_plan();
+    let errors = test_operation_with_schema(
         "
-    query test {
-      foo
-    }
+        {
+          dog {
+            nickname
+            ...bark
+            ...parents
+          }
+        }
+        
+        fragment bark on Dog {
+          barkVolume
+          ...parents
+        }
+        
+        fragment parents on Dog {
+          mother {
+            ...bark
+          }
+        }
+        
     ",
+    TEST_SCHEMA,
         &mut default_plan,
     );
 
-    assert_eq!(errors.len(), 0);
+    let messages = get_messages(&errors);
+    assert_eq!(messages.len(), 1);
+    assert_eq!(messages, vec![
+      "Cannot spread fragment \"bark\" within itself via \"parents\"."
+    ])
 }
 
 #[test]
-fn test_validate_valid_fragment() {
+fn simple_self_reference_fragment_should_not_loop() {
     use crate::validation::test_utils::*;
+    use crate::validation::rules::default_rules_validation_plan;
 
-    let mut default_plan = create_default_ruleset_plan();
-    let errors = test_operation_without_schema(
+    let mut default_plan = default_rules_validation_plan();
+    let errors = test_operation_with_schema(
         "
-        fragment uniqueFields on Dog {
+        query dog {
+          dog {
+            ...DogFields
+          }
+        }
+        
+        fragment DogFields on Dog {
+          mother {
+            ...DogFields
+          }
+          father {
+            ...DogFields
+          }
+        }
+    ",
+    TEST_SCHEMA,
+        &mut default_plan,
+    );
+
+    let messages = get_messages(&errors);
+    assert_eq!(messages.len(), 2);
+    assert_eq!(messages, vec![
+      "Cannot spread fragment \"DogFields\" within itself.",
+      "Cannot spread fragment \"DogFields\" within itself."
+    ])
+}
+
+#[test]
+fn fragment_loop_through_multiple_frags() {
+    use crate::validation::test_utils::*;
+    use crate::validation::rules::default_rules_validation_plan;
+
+    let mut default_plan = default_rules_validation_plan();
+    let errors = test_operation_with_schema(
+        "
+        query dog {
+          dog {
+            ...DogFields1
+          }
+        }
+        
+        fragment DogFields1 on Dog {
+          barks
+          ...DogFields2
+        }
+
+        fragment DogFields2 on Dog {
+          barkVolume
+          ...DogFields3
+        }
+
+        fragment DogFields3 on Dog {
           name
-          nickname
+          ...DogFields1
         }
     ",
+    TEST_SCHEMA,
         &mut default_plan,
     );
 
-    assert_eq!(errors.len(), 0);
+    let messages = get_messages(&errors);
+    assert_eq!(messages.len(), 1);
+    assert_eq!(messages, vec![
+      "Cannot spread fragment \"DogFields1\" within itself via \"DogFields2\", \"DogFields3\"."
+    ])
 }