diff --git a/pest/benches/stack.rs b/pest/benches/stack.rs index 59efae5e..4b4d3773 100644 --- a/pest/benches/stack.rs +++ b/pest/benches/stack.rs @@ -55,14 +55,14 @@ fn snapshot_pop_clear(elements: impl Iterator) { } fn benchmark(b: &mut Criterion) { - use core::iter::repeat; + use core::iter::repeat_n; // use criterion::black_box; let times = 10000usize; let small = 0..times; let medium = ("", 0usize, 1usize); - let medium = repeat(medium).take(times); + let medium = repeat_n(medium, times); let large = [""; 64]; - let large = repeat(large).take(times); + let large = repeat_n(large, times); macro_rules! test_series { ($kind:ident) => { b.bench_function(stringify!(push - restore - $kind), |b| { diff --git a/pest/examples/parens.rs b/pest/examples/parens.rs index f91cb53b..efad9f5e 100644 --- a/pest/examples/parens.rs +++ b/pest/examples/parens.rs @@ -17,7 +17,7 @@ enum Rule { struct ParenParser; impl Parser for ParenParser { - fn parse(rule: Rule, input: &str) -> Result, Error> { + fn parse(rule: Rule, input: &str) -> Result, Error> { fn expr(state: Box>) -> ParseResult>> { state.sequence(|s| s.repeat(paren).and_then(|s| s.end_of_input())) } diff --git a/pest/src/lib.rs b/pest/src/lib.rs index c9e6c886..f655c4be 100644 --- a/pest/src/lib.rs +++ b/pest/src/lib.rs @@ -116,8 +116,8 @@ //! 2. Atomic (`@`) //! //! Atomic rules do not accept whitespace or comments within their expressions and have a -//! cascading effect on any rule they call. I.e. rules that are not atomic but are called by atomic -//! rules behave atomically. +//! cascading effect on any rule they call. I.e. rules that are not atomic but are called by atomic +//! rules behave atomically. //! //! Any rules called by atomic rules do not generate token pairs. //! @@ -133,7 +133,7 @@ //! 3. Compound-atomic (`$`) //! //! Compound-atomic are identical to atomic rules with the exception that rules called by them are -//! not forbidden from generating token pairs. +//! not forbidden from generating token pairs. //! //! ```ignore //! a = { "a" } @@ -147,7 +147,7 @@ //! 4. Non-atomic (`!`) //! //! Non-atomic are identical to normal rules with the exception that they stop the cascading effect -//! of atomic and compound-atomic rules. +//! of atomic and compound-atomic rules. //! //! ```ignore //! a = { "a" } diff --git a/pest/src/parser_state.rs b/pest/src/parser_state.rs index 823c5a74..af6f9141 100644 --- a/pest/src/parser_state.rs +++ b/pest/src/parser_state.rs @@ -90,17 +90,49 @@ pub enum MatchDir { static CALL_LIMIT: AtomicUsize = AtomicUsize::new(0); -/// Sets the maximum call limit for the parser state -/// to prevent stack overflows or excessive execution times -/// in some grammars. -/// If set, the calls are tracked as a running total -/// over all non-terminal rules that can nest closures -/// (which are passed to transform the parser state). +/// Sets the maximum recursion depth limit for the parser state +/// to prevent stack overflows caused by deeply nested grammars. +/// +/// The depth is tracked across recursive rule invocations, incrementing +/// when entering a rule and decrementing when exiting. This provides +/// protection against stack overflow from deeply nested structures +/// (like nested parentheses, nested function calls, etc.). /// /// # Arguments /// -/// * `limit` - The maximum number of calls. If None, -/// the number of calls is unlimited. +/// * `limit` - The maximum recursion depth. If None, +/// the recursion depth is unlimited. +/// +/// # Examples +/// +/// ``` +/// use pest; +/// use core::num::NonZeroUsize; +/// +/// // Set a depth limit of 100 to prevent stack overflow +/// // from deeply nested grammar rules +/// pest::set_call_limit(Some(NonZeroUsize::new(100).unwrap())); +/// +/// // Parse your input... +/// // If the depth exceeds 100, parsing will fail with +/// // an error message "call limit reached" +/// +/// // Remove the limit when done +/// pest::set_call_limit(None); +/// ``` +/// +/// # Use Cases +/// +/// This is useful for: +/// - Protecting against malicious input with excessive nesting +/// - Preventing stack overflow in resource-constrained environments +/// - Ensuring consistent behavior across platforms with different stack sizes +/// +/// # Note +/// +/// Setting the limit too low may prevent parsing of legitimate deeply nested +/// expressions. The appropriate limit depends on your grammar complexity +/// and the expected depth of valid inputs. pub fn set_call_limit(limit: Option) { CALL_LIMIT.store(limit.map(|f| f.get()).unwrap_or(0), Ordering::Relaxed); } @@ -116,35 +148,75 @@ static ERROR_DETAIL: AtomicBool = AtomicBool::new(false); /// # Arguments /// /// * `enabled` - Whether to enable the collection for -/// more error details. +/// more error details. pub fn set_error_detail(enabled: bool) { ERROR_DETAIL.store(enabled, Ordering::Relaxed); } #[derive(Debug)] struct CallLimitTracker { - current_call_limit: Option<(usize, usize)>, + current_depth_limit: Option<(usize, usize)>, + /// When set, indicates depth tracking should stop at this value (limit was reached) + depth_at_limit: Option, } impl Default for CallLimitTracker { fn default() -> Self { - let limit = CALL_LIMIT.load(Ordering::Relaxed); - let current_call_limit = if limit > 0 { Some((0, limit)) } else { None }; - Self { current_call_limit } + let depth_limit = CALL_LIMIT.load(Ordering::Relaxed); + let current_depth_limit = if depth_limit > 0 { + Some((0, depth_limit)) + } else { + None + }; + + Self { + current_depth_limit, + depth_at_limit: None, + } } } impl CallLimitTracker { fn limit_reached(&self) -> bool { - self.current_call_limit + self.depth_at_limit.is_some() + || self + .current_depth_limit + .is_some_and(|(current, limit)| current >= limit) + } + + fn depth_limit_would_be_exceeded(&self) -> bool { + // Check if we're already at the limit + // (incrementing would exceed it) + self.current_depth_limit .is_some_and(|(current, limit)| current >= limit) } fn increment_depth(&mut self) { - if let Some((current, _)) = &mut self.current_call_limit { + // Don't increment if we've already hit the limit + if self.depth_at_limit.is_some() { + return; + } + if let Some((current, _)) = &mut self.current_depth_limit { *current += 1; } } + + fn decrement_depth(&mut self) { + // Don't decrement if limit was already reached - we want to preserve the error state + if self.depth_at_limit.is_some() { + return; + } + if let Some((current, _)) = &mut self.current_depth_limit { + *current = current.saturating_sub(1); + } + } + + fn mark_limit_reached(&mut self) { + // Record the current depth when limit is reached + if let Some((current, _)) = self.current_depth_limit { + self.depth_at_limit = Some(current); + } + } } /// Number of call stacks that may result from a sequence of rules parsing. @@ -516,7 +588,7 @@ where Ok(new(Rc::new(state.queue), input, None, 0, len)) } Err(mut state) => { - let variant = if state.reached_call_limit() { + let variant = if state.call_tracker.limit_reached() { ErrorVariant::CustomError { message: "call limit reached".to_owned(), } @@ -623,7 +695,9 @@ impl<'i, R: RuleType> ParserState<'i, R> { #[inline] fn inc_call_check_limit(mut self: Box) -> ParseResult> { - if self.call_tracker.limit_reached() { + // Check limit before incrementing + if self.call_tracker.depth_limit_would_be_exceeded() { + self.call_tracker.mark_limit_reached(); return Err(self); } self.call_tracker.increment_depth(); @@ -631,8 +705,9 @@ impl<'i, R: RuleType> ParserState<'i, R> { } #[inline] - fn reached_call_limit(&self) -> bool { - self.call_tracker.limit_reached() + fn dec_depth(mut self: Box) -> Box { + self.call_tracker.decrement_depth(); + self } /// Wrapper needed to generate tokens. This will associate the `R` type rule to the closure @@ -755,7 +830,7 @@ impl<'i, R: RuleType> ParserState<'i, R> { if new_state.parse_attempts.enabled { try_add_rule_to_stack(&mut new_state); } - Ok(new_state) + Ok(new_state.dec_depth()) } Err(mut new_state) => { if new_state.lookahead != Lookahead::Negative { @@ -777,7 +852,7 @@ impl<'i, R: RuleType> ParserState<'i, R> { new_state.queue.truncate(index); } - Err(new_state) + Err(new_state.dec_depth()) } } } diff --git a/pest/tests/calculator.rs b/pest/tests/calculator.rs index e6671336..7f572631 100644 --- a/pest/tests/calculator.rs +++ b/pest/tests/calculator.rs @@ -34,7 +34,7 @@ struct CalculatorParser; impl Parser for CalculatorParser { // false positive: pest uses `..` as a complete range (historically) #[allow(clippy::almost_complete_range)] - fn parse(rule: Rule, input: &str) -> Result, Error> { + fn parse(rule: Rule, input: &str) -> Result, Error> { fn expression( state: Box>, ) -> ParseResult>> { diff --git a/pest/tests/depth_limit.rs b/pest/tests/depth_limit.rs new file mode 100644 index 00000000..4df75801 --- /dev/null +++ b/pest/tests/depth_limit.rs @@ -0,0 +1,221 @@ +// pest. The Elegant Parser +// Copyright (c) 2018 Dragoș Tiselice +// +// Licensed under the Apache License, Version 2.0 +// or the MIT +// license , at your +// option. All files in the project carrying such notice may not be copied, +// modified, or distributed except according to those terms. + +use core::num::NonZeroUsize; +use pest::error::Error; +use pest::iterators::Pairs; +use pest::{state, ParseResult, Parser, ParserState}; + +#[allow(dead_code, non_camel_case_types)] +#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +enum Rule { + expression, + add_expr, + mul_expr, + primary, + number, +} + +struct TestParser; + +impl Parser for TestParser { + fn parse(rule: Rule, input: &str) -> Result, Error> { + fn expression( + state: Box>, + ) -> ParseResult>> { + state.rule(Rule::expression, |s| { + s.sequence(|s| { + s.start_of_input() + .and_then(add_expr) + .and_then(|s| s.end_of_input()) + }) + }) + } + + fn add_expr(state: Box>) -> ParseResult>> { + state.rule(Rule::add_expr, |s| { + s.sequence(|s| { + mul_expr(s).and_then(|s| { + s.repeat(|s| s.sequence(|s| s.match_string("+").and_then(mul_expr))) + }) + }) + }) + } + + fn mul_expr(state: Box>) -> ParseResult>> { + state.rule(Rule::mul_expr, |s| { + s.sequence(|s| { + primary(s).and_then(|s| { + s.repeat(|s| s.sequence(|s| s.match_string("*").and_then(primary))) + }) + }) + }) + } + + fn primary(state: Box>) -> ParseResult>> { + state.rule(Rule::primary, |s| { + number(s).or_else(|s| { + s.sequence(|s| { + s.match_string("(") + .and_then(add_expr) + .and_then(|s| s.match_string(")")) + }) + }) + }) + } + + fn number(state: Box>) -> ParseResult>> { + state.rule(Rule::number, |s| { + s.match_char_by(|c| c.is_ascii_digit()) + .and_then(|s| s.repeat(|s| s.match_char_by(|c| c.is_ascii_digit()))) + }) + } + + state(input, |s| match rule { + Rule::expression => expression(s), + Rule::add_expr => add_expr(s), + Rule::mul_expr => mul_expr(s), + Rule::primary => primary(s), + Rule::number => number(s), + }) + } +} + +#[test] +fn test_depth_limit_simple() { + // Set a recursion depth limit + pest::set_call_limit(Some(NonZeroUsize::new(10).unwrap())); + + // This should parse successfully - depth is less than 10 + let result = TestParser::parse(Rule::expression, "1"); + assert!(result.is_ok()); + + // Reset limit + pest::set_call_limit(None); +} + +#[test] +fn test_depth_limit_nested_parens() { + // Make sure call limit is reset first + pest::set_call_limit(None); + // Set a very low recursion depth limit (lower than what "1" needs) + // Parsing "1" requires: expression -> add_expr -> mul_expr -> primary -> number + // That's 5 rule invocations, so limit of 4 should fail + pest::set_call_limit(Some(NonZeroUsize::new(4).unwrap())); + + // Simple expression should fail with this very low limit + let result = TestParser::parse(Rule::expression, "1"); + + match result { + Ok(_) => { + panic!("Expected call limit error with very low limit"); + } + Err(e) => { + let _error_msg = format!("{}", e); + // Check specifically for call limit error + if let pest::error::ErrorVariant::CustomError { message } = &e.variant { + assert_eq!( + message, "call limit reached", + "Expected call limit error, got: {}", + message + ); + } else { + panic!( + "Expected CustomError variant with call limit, got: {:?}", + e.variant + ); + } + } + } + + // Reset limit + pest::set_call_limit(None); +} + +#[test] +fn test_depth_limit_allows_simple_parse() { + // Set a reasonable recursion depth limit + pest::set_call_limit(Some(NonZeroUsize::new(50).unwrap())); + + // Simple expression should work with reasonable limit + let result = TestParser::parse(Rule::expression, "1+2"); + assert!(result.is_ok()); + + // Reset limit + pest::set_call_limit(None); +} + +#[test] +fn test_no_depth_limit() { + // Make sure no limit allows parsing + pest::set_call_limit(None); + + // Even deeply nested expressions should work without a limit + let nested = "((((((1))))))"; + let result = TestParser::parse(Rule::expression, nested); + assert!(result.is_ok()); +} + +#[test] +fn test_depth_limit_reset() { + // Set a limit, then remove it + pest::set_call_limit(Some(NonZeroUsize::new(5).unwrap())); + pest::set_call_limit(None); + + // Should work after reset + let result = TestParser::parse(Rule::expression, "((((((1))))))"); + assert!(result.is_ok()); +} + +/// This test demonstrates the issue from GitHub: +/// Pest grammars can cause overflow within Pest itself with deeply nested structures. +/// +/// The grammar from the issue can cause stack overflow with deeply +/// nested parentheses. With set_call_limit (which now tracks recursion depth), +/// we can prevent this. +#[test] +fn test_prevents_stack_overflow_from_issue() { + // Make sure call limit is disabled initially + pest::set_call_limit(None); + // Set a recursion depth limit that's lower than what 30 nested parens would need + // Each level of parentheses requires several rule invocations + // (expression, add_expr, mul_expr, primary for opening, then recursion for content) + pest::set_call_limit(Some(NonZeroUsize::new(50).unwrap())); + + // Create a deeply nested expression with 30 levels of nesting + // This would need more than 50 depth + let mut deeply_nested = String::new(); + let nesting_depth = 30; + + // Add opening parens + for _ in 0..nesting_depth { + deeply_nested.push('('); + } + deeply_nested.push('1'); + // Add closing parens + for _ in 0..nesting_depth { + deeply_nested.push(')'); + } + + let result = TestParser::parse(Rule::expression, &deeply_nested); + + // Should fail with call limit reached (recursion depth limit), not stack overflow + assert!(result.is_err()); + if let Err(e) = result { + let error_msg = format!("{}", e); + assert!( + error_msg.contains("call limit reached"), + "Expected call limit error, got: {}", + error_msg + ); + } + + // Reset limit + pest::set_call_limit(None); +} diff --git a/pest/tests/json.rs b/pest/tests/json.rs index 0313d81b..978f590d 100644 --- a/pest/tests/json.rs +++ b/pest/tests/json.rs @@ -40,7 +40,7 @@ struct JsonParser; impl Parser for JsonParser { // false positive: pest uses `..` as a complete range (historically) #[allow(clippy::almost_complete_range)] - fn parse(rule: Rule, input: &str) -> Result, Error> { + fn parse(rule: Rule, input: &str) -> Result, Error> { fn json(state: Box>) -> ParseResult>> { value(state) } diff --git a/pest/tests/nested_expr.pest b/pest/tests/nested_expr.pest new file mode 100644 index 00000000..19a5bc22 --- /dev/null +++ b/pest/tests/nested_expr.pest @@ -0,0 +1,9 @@ +// Example grammar from the issue demonstrating deep nesting +// This grammar can cause stack overflow with deeply nested parentheses + +WHITESPACE = _{ " " | "\t" | "\n" | "\r" } +expression = { SOI ~ add_expr ~ EOI } +add_expr = { mul_expr ~ ("+" ~ mul_expr)* } +mul_expr = { primary ~ ("*" ~ primary)* } +primary = { number | "(" ~ add_expr ~ ")" } +number = @{ ASCII_DIGIT+ }