chore(parser): remove recursion in field parsing, fixes #514 (#519)

crates/apollo-compiler/src/database/ast.rs

@@ -93,7 +93,9 @@ mod tests {
         let schema = r#"
         query {
           Q1 {
-            url
+            url {
+              hostname
+            }
           }
         }
         "#;
@@ -104,15 +106,19 @@ mod tests {
 
         assert_eq!(ast.recursion_limit().high, 2);
         assert_eq!(ast.errors().len(), 1);
-        assert_eq!(ast.document().definitions().count(), 2);
+        assert_eq!(ast.document().definitions().count(), 1);
     }
 
     #[test]
     fn it_passes_when_selection_set_recursion_limit_is_not_exceeded() {
         let schema = r#"
         query {
           Q1 {
-            url
+            Q2 {
+              Q3 {
+                url
+              }
+            }
           }
         }
         "#;
@@ -184,7 +190,7 @@ mod tests {
         assert_eq!(ast.errors().len(), 1);
         assert_eq!(
             ast.errors().next(),
-            Some(&apollo_parser::Error::limit("parser limit(3) reached", 61))
+            Some(&apollo_parser::Error::limit("parser limit(3) reached", 121))
         );
     }
 

crates/apollo-compiler/test_data/ok/0001_annonymous_operation_definition.txt

@@ -61,4 +61,4 @@
                         - [email protected] "String"
             - [email protected] "\n"
             - [email protected] "}"
-recursion limit: 4096, high: 4
+recursion limit: 4096, high: 2

crates/apollo-compiler/test_data/ok/0002_multiple_named_operation_definitions.txt

@@ -128,4 +128,4 @@
             - [email protected] "\n"
             - [email protected] "}"
     - [email protected] "\n"
-recursion limit: 4096, high: 6
+recursion limit: 4096, high: 3

crates/apollo-compiler/test_data/ok/0010_operation_with_defined_fields.txt

@@ -214,4 +214,4 @@
         - [email protected]
             - [email protected]
                 - [email protected] "FIELD_DEFINITION"
-recursion limit: 4096, high: 6
+recursion limit: 4096, high: 2

crates/apollo-compiler/test_data/ok/0011_fragment_spreads_in_fragment_definitions.txt

@@ -143,4 +143,4 @@
             - [email protected] "\n"
             - [email protected] "}"
     - [email protected] "\n"
-recursion limit: 4096, high: 3
+recursion limit: 4096, high: 2

crates/apollo-compiler/test_data/ok/0012_introspection_query.txt

@@ -477,4 +477,4 @@
                     - [email protected] "}"
             - [email protected] "\n"
             - [email protected] "}"
-recursion limit: 4096, high: 31
+recursion limit: 4096, high: 8

crates/apollo-compiler/test_data/ok/0013_operation_with_used_variable_in_fragment.txt

@@ -231,4 +231,4 @@
                         - [email protected] "Int"
             - [email protected] "\n"
             - [email protected] "}"
-recursion limit: 4096, high: 5
+recursion limit: 4096, high: 2

crates/apollo-compiler/test_data/ok/0016_same_variables_in_multiple_operations.txt

@@ -186,4 +186,4 @@
                     - [email protected] "!"
             - [email protected] "\n"
             - [email protected] "}"
-recursion limit: 4096, high: 4
+recursion limit: 4096, high: 2

crates/apollo-compiler/test_data/ok/0017_variables_are_input_types.txt

@@ -300,4 +300,4 @@
                         - [email protected] "Boolean"
             - [email protected] "\n"
             - [email protected] "}"
-recursion limit: 4096, high: 4
+recursion limit: 4096, high: 2

crates/apollo-compiler/test_data/ok/0018_non_clashing_names.txt

@@ -110,4 +110,4 @@
             - [email protected] "\n"
             - [email protected] "}"
     - [email protected] "\n"
-recursion limit: 4096, high: 3
+recursion limit: 4096, high: 2

crates/apollo-compiler/test_data/ok/0020_merge_identical_fields.txt

@@ -147,4 +147,4 @@
             - [email protected] "\n"
             - [email protected] "}"
     - [email protected] "\n"
-recursion limit: 4096, high: 3
+recursion limit: 4096, high: 2

crates/apollo-compiler/test_data/ok/0021_merge_identical_fields_with_arguments.txt

@@ -238,4 +238,4 @@
             - [email protected] "\n"
             - [email protected] "}"
     - [email protected] "\n"
-recursion limit: 4096, high: 3
+recursion limit: 4096, high: 2

crates/apollo-compiler/test_data/ok/0022_merge_differing_fields_and_args.txt

@@ -541,4 +541,4 @@
             - [email protected] "\n"
             - [email protected] "}"
     - [email protected] "\n"
-recursion limit: 4096, high: 3
+recursion limit: 4096, high: 2

crates/apollo-compiler/test_data/ok/0024_used_variables_in_directives.txt

@@ -281,4 +281,4 @@
             - [email protected] "\n"
             - [email protected] "}"
     - [email protected] "\n"
-recursion limit: 4096, high: 2
+recursion limit: 4096, high: 1

crates/apollo-compiler/test_data/ok/0025_unique_directives.txt

@@ -88,4 +88,4 @@
             - [email protected] "\n"
             - [email protected] "}"
     - [email protected] "\n"
-recursion limit: 4096, high: 4
+recursion limit: 4096, high: 1

crates/apollo-compiler/test_data/ok/0026_type_introspection.txt

@@ -113,4 +113,4 @@
                     - [email protected] "}"
             - [email protected] "\n"
             - [email protected] "}"
-recursion limit: 4096, high: 10
+recursion limit: 4096, high: 4

crates/apollo-compiler/test_data/ok/0027_typename_introspection_in_object.txt

@@ -72,4 +72,4 @@
                     - [email protected] "__typename"
             - [email protected] "\n"
             - [email protected] "}"
-recursion limit: 4096, high: 3
+recursion limit: 4096, high: 1

crates/apollo-compiler/test_data/ok/0028_typename_introspection_in_union.txt

@@ -152,4 +152,4 @@
                     - [email protected] "}"
             - [email protected] "\n"
             - [email protected] "}"
-recursion limit: 4096, high: 4
+recursion limit: 4096, high: 2

crates/apollo-compiler/test_data/ok/0029_used_variable_in_list_and_input.txt

@@ -214,4 +214,4 @@
             - [email protected] "\n"
             - [email protected] "}"
     - [email protected] "\n"
-recursion limit: 4096, high: 4
+recursion limit: 4096, high: 2

crates/apollo-parser/src/parser/grammar/document.rs

@@ -14,6 +14,10 @@ pub(crate) fn document(p: &mut Parser) {
     let doc = p.start_node(SyntaxKind::DOCUMENT);
 
     while let Some(node) = p.peek() {
+        if p.recursion_limit.limited() {
+            break;
+        }
+
         match node {
             TokenKind::StringValue => {
                 let def = p.peek_data_n(2).unwrap();

crates/apollo-parser/src/parser/grammar/field.rs

@@ -10,16 +10,7 @@ use crate::{
 /// *Field*:
 ///     Alias? Name Arguments? Directives? SelectionSet?
 pub(crate) fn field(p: &mut Parser) {
-    // We need to enforce recursion limits to prevent
-    // excessive resource consumption or (more seriously)
-    // stack overflows.
-    p.recursion_limit.consume();
-    if p.recursion_limit.limited() {
-        p.limit_err(format!("parser limit({}) reached", p.recursion_limit.limit));
-        return;
-    }
-
-    let guard = p.start_node(SyntaxKind::FIELD);
+    let _guard = p.start_node(SyntaxKind::FIELD);
 
     if let Some(TokenKind::Name) = p.peek() {
         if let Some(T![:]) = p.peek_n(2) {
@@ -41,19 +32,6 @@ pub(crate) fn field(p: &mut Parser) {
     if let Some(T!['{']) = p.peek() {
         selection::selection_set(p);
     }
-
-    match p.peek() {
-        Some(TokenKind::Name) => {
-            guard.finish_node();
-
-            field(p)
-        }
-
-        Some(T!['}']) => {
-            guard.finish_node();
-        }
-        _ => guard.finish_node(),
-    }
 }
 
 /// See: https://spec.graphql.org/October2021/#FieldsDefinition

crates/apollo-parser/src/parser/grammar/selection.rs

@@ -312,7 +312,9 @@ query SomeQuery(
         let schema = r#"
         query {
           Q1 {
-            url
+            Q2 {
+              url
+            }
           }
         }
         "#;
@@ -322,23 +324,25 @@ query SomeQuery(
 
         assert_eq!(ast.recursion_limit().high, 2);
         assert_eq!(ast.errors().len(), 1);
-        assert_eq!(ast.document().definitions().count(), 2);
+        assert_eq!(ast.document().definitions().count(), 1);
     }
 
     #[test]
     fn it_passes_when_selection_set_recursion_limit_is_not_exceeded() {
         let schema = r#"
         query {
           Q1 {
-            url
+            Q2 {
+              url
+            }
           }
         }
         "#;
         let parser = Parser::new(schema).recursion_limit(7);
 
         let ast = parser.parse();
 
-        assert_eq!(ast.recursion_limit().high, 4);
+        assert_eq!(ast.recursion_limit().high, 3);
         assert_eq!(ast.errors().len(), 0);
         assert_eq!(ast.document().definitions().count(), 1);
     }
@@ -347,9 +351,13 @@ query SomeQuery(
     fn it_errors_when_selection_set_recursion_limit_is_exceeded_with_inline_fragment() {
         let schema = r#"
         query {
-          ... on Page {
-            price
-            name
+          Q1 {
+            Q2 {
+              ... on Page {
+                price
+                name
+              }
+            }
           }
         }
         "#;
@@ -366,8 +374,12 @@ query SomeQuery(
     fn it_errors_when_selection_set_recursion_limit_is_exceeded_fragment_spread() {
         let schema = r#"
         query {
-          product {
-            ...Page
+          Q1 {
+            Q2 {
+              product {
+                ...Page
+              }
+            }
           }
         }
         "#;
@@ -377,7 +389,7 @@ query SomeQuery(
 
         assert_eq!(ast.recursion_limit().high, 3);
         assert_eq!(ast.errors().len(), 1);
-        assert_eq!(ast.document().definitions().count(), 1);
+        assert_eq!(dbg!(ast.document()).definitions().count(), 1);
     }
 
     #[test]
@@ -403,23 +415,29 @@ query SomeQuery(
 
         assert_eq!(ast.recursion_limit().high, 2);
         assert_eq!(ast.errors().len(), 1);
-        assert_eq!(ast.document().definitions().count(), 4);
+        assert_eq!(ast.document().definitions().count(), 1);
     }
 
     #[test]
     fn it_passes_when_selection_set_recursion_limit_is_at_default() {
         let schema = r#"
         query {
           Q1 {
-            url
+            Q2 {
+              Q3 {
+                Q4 {
+                  Q5 { url }
+                }
+              }
+            }
           },
         }
         "#;
         let parser = Parser::new(schema);
 
         let ast = parser.parse();
 
-        assert_eq!(ast.recursion_limit().high, 4);
+        assert_eq!(ast.recursion_limit().high, 6);
         assert_eq!(ast.errors().len(), 0);
         assert_eq!(ast.document().definitions().count(), 1);
     }

crates/apollo-parser/src/parser/mod.rs

@@ -495,7 +495,7 @@ mod tests {
         let parser = Parser::new(source).recursion_limit(3).token_limit(200);
         let ast = parser.parse();
         let errors = ast.errors().collect::<Vec<_>>();
-        assert_eq!(errors, &[&Error::limit("parser limit(3) reached", 61),]);
+        assert_eq!(errors, &[&Error::limit("parser limit(3) reached", 121),]);
     }
 
     #[test]
@@ -522,8 +522,6 @@ mod tests {
         );
         assert_eq!(errors.next(), None);
 
-        // TODO(@goto-bus-stop) the comment is positioned wrong:
-        // https://github.com/apollographql/apollo-rs/issues/362
         let tree = expect![[r##"
             [email protected]
               [email protected] "\n            "

crates/apollo-parser/test_data/parser/err/0007_fragment_definition_with_invalid_fragment_name.txt

@@ -25,4 +25,4 @@
             - [email protected] "\n"
             - [email protected] "}"
 - ERROR@9:11 "Fragment Name cannot be 'on'" on
-recursion limit: 4096, high: 2
+recursion limit: 4096, high: 1

crates/apollo-parser/test_data/parser/err/0008_fragment_definition_with_invalid_type_condition.txt

@@ -26,4 +26,4 @@
             - [email protected] "\n"
             - [email protected] "}"
 - ERROR@22:26 "exptected 'on'" User
-recursion limit: 4096, high: 2
+recursion limit: 4096, high: 1

crates/apollo-parser/test_data/parser/err/0025_document_with_incorrect_definition_and_selection_set.txt

@@ -15,4 +15,4 @@
             - [email protected] "\n"
             - [email protected] "}"
 - ERROR@0:14 "expected definition" uasdf21230jkdw
-recursion limit: 4096, high: 3
+recursion limit: 4096, high: 1

crates/apollo-parser/test_data/parser/err/0026_invalid_definition_squished_between_two_valid_definitions.txt

@@ -56,4 +56,4 @@
             - [email protected] "\n"
             - [email protected] "}"
 - ERROR@99:113 "expected definition" uasdf21230jkdw
-recursion limit: 4096, high: 3
+recursion limit: 4096, high: 1

crates/apollo-parser/test_data/parser/err/0031_argument_with_erronous_string_value.txt

@@ -18,4 +18,4 @@
             - [email protected] "}"
 - ERROR@13:25 "unexpected escaped character" "\string ID"
 - ERROR@25:26 "expected a valid Value" )
-recursion limit: 4096, high: 2
+recursion limit: 4096, high: 1

crates/apollo-parser/test_data/parser/err/0035_unterminated_string_value_in_object_value.txt

@@ -56,4 +56,4 @@
 - ERROR@116:116 "expected }" EOF
 - ERROR@116:116 "expected R_PAREN, got EOF" EOF
 - ERROR@116:116 "expected R_CURLY, got EOF" EOF
-recursion limit: 4096, high: 2
+recursion limit: 4096, high: 1

crates/apollo-parser/test_data/parser/err/0042_document_with_incorrect_token.txt

@@ -42,4 +42,4 @@
             - [email protected] "}"
 - ERROR@0:1 "expected a StringValue, Name or OperationDefinition" @
 - ERROR@49:50 "expected a StringValue, Name or OperationDefinition" }
-recursion limit: 4096, high: 3
+recursion limit: 4096, high: 1

crates/apollo-parser/test_data/parser/err/0046_incomplete_spreads.txt

@@ -16,4 +16,4 @@
             - [email protected] "}"
     - [email protected] "\n"
 - ERROR@10:13 "expected an Inline Fragment or a Fragment Spread" ...
-recursion limit: 4096, high: 3
+recursion limit: 4096, high: 2