rust-peg icon indicating copy to clipboard operation
rust-peg copied to clipboard

Add a way to specify a parser stack limit

Open zsol opened this issue 3 years ago • 1 comments

TODO:

  • [ ] documentation
  • [ ] change placeholder STACK OVERFLOW expected string to something nicer
  • [ ] test failure case: non integer stack limit given

This PR implements a new stack_limit directive in the meta grammar. When specified, the generated parser will keep track of rule invocation depth and error out when the depth is beyond the specified maximum.

I'm not married to any of the naming and syntax here, feel free to suggest better alternatives.

A better way of raising errors from this might be to introduce a new RuleResult variant that immediately terminates parsing instead of carrying on with alternatives. I think that's a great future improvement, but didn't consider for this PR because it would expand the scope quite a bit.

Fixes #282. This PR depends on #307.

zsol avatar Jun 29 '22 18:06 zsol

A better way of raising errors from this might be to introduce a new RuleResult variant that immediately terminates parsing instead of carrying on with alternatives. I think that's a great future improvement, but didn't consider for this PR because it would expand the scope quite a bit.

I don't think it's workable to treat reaching the stack limit as a simple parse failure because it will cause /, &, and ! operators to misbehave when reaching the limit. That's going to end up sometimes resulting in "successful" parses with unexpected parse trees. Possibly could even be weaponized to let an attacker skip parts of a grammar that involve deeper rule nesting, which could bypass data validation and produce a result that shouldn't be allowed.

For example, with the grammar

peg::parser! { grammar test() for str {
    stack_limit 2;

    rule keyword() -> Expr
        = "foo" { Expr::Foo }
        / "bar" { Expr::Bar }

    pub rule expr() -> Expr
        = "(" e: expr() ")" { e }
        / keyword()
        / v:$(['a'..='z']+) { Expr::Variable(v.to_string()) }
}}

the input ((foo)) incorrectly returns Expr::Variable("foo") instead of Expr::Foo because the call to keyword() fails with the stack limit.

kevinmehall avatar Jul 16 '22 01:07 kevinmehall