Fix unbounded recursion in Parser::parse_expression

src/compile_error.rs

@@ -198,6 +198,9 @@ impl Display for CompileError<'_> {
           parameter
         )?;
       }
+      RecursionDepthExceeded => {
+        write!(f, "Recursion depth was exceeded")?;
+      }
       RequiredParameterFollowsDefaultParameter { parameter } => {
         write!(
           f,

src/compile_error_kind.rs

@@ -74,6 +74,7 @@ pub(crate) enum CompileErrorKind<'src> {
   ParameterShadowsVariable {
     parameter: &'src str,
   },
+  RecursionDepthExceeded,
   RequiredParameterFollowsDefaultParameter {
     parameter: &'src str,
   },

src/parser.rs

@@ -32,9 +32,13 @@ pub(crate) struct Parser<'tokens, 'src> {
   next: usize,
   /// Current expected tokens
   expected: BTreeSet<TokenKind>,
+  /// Current recursion depth
+  depth: u8,
 }
 
 impl<'tokens, 'src> Parser<'tokens, 'src> {
+  const RECURSION_LIMIT: u8 = u8::MAX;
+
   /// Parse `tokens` into an `Ast`
   pub(crate) fn parse(tokens: &'tokens [Token<'src>]) -> CompileResult<'src, Ast<'src>> {
     Self::new(tokens).parse_ast()
@@ -46,6 +50,7 @@ impl<'tokens, 'src> Parser<'tokens, 'src> {
       next: 0,
       expected: BTreeSet::new(),
       tokens,
+      depth: 0,
     }
   }
 
@@ -389,21 +394,42 @@ impl<'tokens, 'src> Parser<'tokens, 'src> {
     })
   }
 
+  /// Bound recursion to self.depth
+  fn recurse<F>(&mut self, body: F) -> CompileResult<'src, Expression<'src>>
+  where
+    F: Fn(&mut Self) -> CompileResult<'src, Expression<'src>>,
+  {
+    if self.depth == Self::RECURSION_LIMIT {
+      return Err(CompileError {
+        token: self.next()?,
+        kind: CompileErrorKind::RecursionDepthExceeded,
+      });
+    }
+
+    self.depth += 1;
+    let result = body(self);
+    self.depth -= 1;
+
+    result
+  }
+
   /// Parse an expression, e.g. `1 + 2`
   fn parse_expression(&mut self) -> CompileResult<'src, Expression<'src>> {
-    if self.accepted_keyword(Keyword::If)? {
-      self.parse_conditional()
-    } else {
-      let value = self.parse_value()?;
-
-      if self.accepted(Plus)? {
-        let lhs = Box::new(value);
-        let rhs = Box::new(self.parse_expression()?);
-        Ok(Expression::Concatenation { lhs, rhs })
+    self.recurse(|slf| {
+      if slf.accepted_keyword(Keyword::If)? {
+        slf.parse_conditional()
       } else {
-        Ok(value)
+        let value = slf.parse_value()?;
+
+        if slf.accepted(Plus)? {
+          let lhs = Box::new(value);
+          let rhs = Box::new(slf.parse_expression()?);
+          Ok(Expression::Concatenation { lhs, rhs })
+        } else {
+          Ok(value)
+        }
       }
-    }
+    })
   }
 
   /// Parse a conditional, e.g. `if a == b { "foo" } else { "bar" }`

tests/lib.rs

@@ -62,6 +62,7 @@ mod positional_arguments;
 mod quiet;
 mod quote;
 mod readme;
+mod recursion_limit;
 mod regexes;
 mod run;
 mod search;

tests/recursion_limit.rs

@@ -0,0 +1,21 @@
+use super::*;
+
+#[test]
+fn bugfix() {
+  let mut justfile = String::from("foo: (x ");
+  for _ in 0..5000 {
+    justfile.push('(');
+  }
+  Test::new()
+    .justfile(&justfile)
+    .stderr(RECURSION_LIMIT_REACHED)
+    .status(EXIT_FAILURE)
+    .run();
+}
+
+const RECURSION_LIMIT_REACHED: &str = "
+error: Recursion depth was exceeded
+  |
+1 | foo: (x ((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((((
+  |                                                                                                                                                                                                                                                                        ^
+";