feat(parser): configurable max rule-nesting depth to bound adversarial input - #199
Merged
Conversation
📝 WalkthroughWalkthroughThe parser runtime and generated parser now support configurable rule-nesting depth limits, including left-recursive expansion accounting, fatal error propagation through recovery, and parser reuse after violations. Deep-nesting fixtures and CLI tests cover capped, uncapped, recursive, and recovery scenarios. ChangesGenerated parser safety
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant GeneratedParser
participant BaseParser
Caller->>GeneratedParser: set_max_rule_depth(Some(limit))
Caller->>GeneratedParser: parse input
GeneratedParser->>BaseParser: check rule and recursion depth
BaseParser-->>GeneratedParser: continue or return positioned depth error
GeneratedParser-->>Caller: parse result
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deeply nested input parses safely since the segmented-stack guard (#193), but each nesting level still costs CPU and tree memory. Callers parsing attacker-supplied text (CEL policy engines cap at depth 96 via an ANTLR parse listener) need to bound that work at parse time, not after the tree is already built. Parser::set_max_rule_depth(Option<usize>) (default None = unlimited) makes generated rule dispatch abort with a positioned "rule nesting depth limit of {n} exceeded" error at the first rule entry past the cap. The violation is sticky: rule-level recovery cannot absorb it and keep spending bounded resources — the top-level entry drains it and fails the parse even when recovery produced a tree, and reset() clears it so a reused parser starts clean. Fixes #198
tinovyatkin
force-pushed
the
feat/198-max-rule-depth
branch
from
July 25, 2026 13:46
be15c73 to
cc92376
Compare
Copy/Paste DetectionFound 15 duplication(s) across 3 changed Rust file(s) (threshold: 100 tokens). Show duplicationsFound a 27 line (145 tokens) duplication in the following files:
fn generated_match_token_recovers_missing_token_from_context_follow() {
let atn = generated_match_recovery_atn();
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new(
[None, Some("'X'"), Some("'Y'")],
[None, Some("X"), Some("Y")],
[None::<&str>, None, None],
),
);
let mut parser = BaseParser::new(
CommonTokenStream::new(Source {
tokens: vec![TestToken::eof("parser-test", 3, 1, 3)],
index: 0,
}),
data,
);
parser.rule_context_stack = vec![
RuleContextFrame {
rule_index: 0,
invoking_state: 0,
},
RuleContextFrame {
rule_index: 1,
invoking_state: 1,
},
];
```rust
---
Found a 22 line (132 tokens) duplication in the following files:
* Starting at line 1100 of tests/antlr4_rust_gen_cli.rs
* Starting at line 1200 of tests/antlr4_rust_gen_cli.rs
```rust
"parser grammar Delegate;\ndelegated: {isTypeName()}? ID;\n",
)
.expect("delegate grammar should be writable");
fs::write(&tokens, "lexer grammar Tokens;\nID: [a-z]+;\n")
.expect("token grammar should be writable");
let output = run_antlr4_rust_gen(&[
root.as_os_str(),
tokens.as_os_str(),
OsStr::new("-I"),
temp.path().as_os_str(),
OsStr::new("--out-dir"),
out.as_os_str(),
]);
assert!(
output.status.success(),
"stdout: {}\nstderr: {}",
utf8(&output.stdout),
utf8(&output.stderr)
);
let parser = fs::read_to_string(out.join("root.rs")).expect("parser should be emitted");
assert!(parser.contains("pub trait RootHooks"), "{parser}");Found a 27 line (127 tokens) duplication in the following files:
let mut atn = ParserAtnBuilder::new(2);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(0))
.expect("state")
.index(),
0
);
assert_eq!(
atn.add_state(AtnStateKind::BlockStart, Some(0))
.expect("state")
.index(),
1
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
2
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
3
);
assert_eq!(
atn.add_state(AtnStateKind::BlockEnd, Some(0))
```rust
---
Found a 22 line (125 tokens) duplication in the following files:
* Starting at line 14513 of src/parser.rs
* Starting at line 14585 of src/parser.rs
```rust
atn.add_state(AtnStateKind::BlockEnd, Some(0))
.expect("state")
.index(),
4
);
assert_eq!(
atn.add_state(AtnStateKind::RuleStop, Some(0))
.expect("state")
.index(),
5
);
atn.set_rule_to_start_state(vec![0])
.expect("rule start states");
atn.set_rule_to_stop_state(vec![5])
.expect("rule stop states");
atn.add_decision_state(1).expect("decision state");
atn.add_transition(0, ParserTransitionSpec::Epsilon { target: 1 })
.expect("transition");
atn.add_transition(
1,
ParserTransitionSpec::Atom {
target: 2,Found a 34 line (119 tokens) duplication in the following files:
outcomes.extend(
self.recognize_state(
atn,
RecognizeRequest {
state_number: *target,
stop_state,
index,
rule_start_index,
decision_start_index: next_decision_start_index,
init_action_rules,
predicates,
semantics,
rule_args,
member_actions,
return_actions,
local_int_arg,
member_values: member_values.clone(),
return_values: return_values.clone(),
rule_alt_number: next_alt_number,
track_alt_numbers,
consumed_eof,
committed_decision: transition_committed,
precedence,
depth: depth + 1,
recovery_symbols: epsilon_recovery_symbols.clone(),
recovery_state: epsilon_recovery_state,
},
visiting,
memo,
expected,
)
.into_iter()
.map(|mut outcome| {
prepend_decision(&mut outcome, decision);
```rust
---
Found a 15 line (113 tokens) duplication in the following files:
* Starting at line 15587 of src/parser.rs
* Starting at line 15863 of src/parser.rs
```rust
fn generated_match_token_counts_single_token_deletion_recovery() {
let atn = generated_match_recovery_atn();
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new(
[None, Some("'X'"), Some("'Y'"), Some("'Z'")],
[None, Some("X"), Some("Y"), Some("Z")],
[None::<&str>, None, None, None],
),
);
let mut parser = BaseParser::new(
CommonTokenStream::new(Source {
tokens: vec![
TestToken::new(3).with_text("z"),
TestToken::new(2).with_text("y"),Found a 12 line (112 tokens) duplication in the following files:
let mut atn = ParserAtnBuilder::new(1);
for (state, kind, rule) in [
(0, AtnStateKind::RuleStart, 0),
(1, AtnStateKind::StarLoopEntry, 0),
(2, AtnStateKind::Basic, 0), // ops hub
(3, AtnStateKind::Basic, 0), // shift prec
(4, AtnStateKind::Basic, 0), // shift first >
(5, AtnStateKind::Basic, 0), // shift second >
(6, AtnStateKind::Basic, 0), // rel prec
(7, AtnStateKind::Basic, 0), // rel >
(8, AtnStateKind::LoopEnd, 0),
(9, AtnStateKind::RuleStop, 0),
```rust
---
Found a 22 line (112 tokens) duplication in the following files:
* Starting at line 14924 of src/parser.rs
* Starting at line 15125 of src/parser.rs
```rust
fn predicate_after_token_atn() -> Atn {
let mut atn = ParserAtnBuilder::new(2);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(0))
.expect("state")
.index(),
0
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
1
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
2
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))Found a 22 line (111 tokens) duplication in the following files:
fn left_recursive_loop_with_caller_follow_atn(caller_symbol: i32) -> Atn {
let mut atn = ParserAtnBuilder::new(2);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(0))
.expect("state")
.index(),
0
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
1
);
assert_eq!(
atn.add_state(AtnStateKind::Basic, Some(0))
.expect("state")
.index(),
2
);
assert_eq!(
atn.add_state(AtnStateKind::RuleStart, Some(1))
```rust
---
Found a 14 line (110 tokens) duplication in the following files:
* Starting at line 15258 of src/parser.rs
* Starting at line 18014 of src/parser.rs
```rust
fn parser_matches_token_and_reports_mismatch() {
let source = Source {
tokens: vec![
TestToken::new(1).with_text("x"),
TestToken::eof("parser-test", 1, 1, 1),
],
index: 0,
};
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new([None, Some("'x'")], [None, Some("X")], [None::<&str>, None]),
);
let mut parser = BaseParser::new(CommonTokenStream::new(source), data);
let matched = parser.match_token(1).expect("token 1 should match");Found a 13 line (109 tokens) duplication in the following files:
fn parser_matches_token_and_reports_mismatch() {
let source = Source {
tokens: vec![
TestToken::new(1).with_text("x"),
TestToken::eof("parser-test", 1, 1, 1),
],
index: 0,
};
let data = RecognizerData::new(
"Mini.g4",
Vocabulary::new([None, Some("'x'")], [None, Some("X")], [None::<&str>, None]),
);
let mut parser = BaseParser::new(CommonTokenStream::new(source), data);
```rust
---
Found a 22 line (108 tokens) duplication in the following files:
* Starting at line 7708 of src/parser.rs
* Starting at line 8099 of src/parser.rs
```rust
) -> Option<RecognizeOutcome> {
let (error_index, message) = self.expected_error_message(rule_index, start_index, expected);
let diagnostic = diagnostic_for_token(self.token_at(error_index), message);
let mut next_index = error_index;
loop {
let symbol = self.token_type_at(next_index);
if sync_symbols.contains(&symbol) {
if next_index == error_index {
return None;
}
break;
}
if symbol == TOKEN_EOF {
break;
}
let after = self.consume_index(next_index, symbol);
if after == next_index {
break;
}
next_index = after;
}
let mut nodes = NodeSeqId::EMPTY;Found a 15 line (108 tokens) duplication in the following files:
fn outcome_ties_keep_later_non_recursive_alternative() {
let arena = RecognitionArena::default();
let first = RecognizeOutcome {
index: 1,
consumed_eof: false,
alt_number: 0,
member_values: BTreeMap::new(),
return_values: BTreeMap::new(),
diagnostics: DiagnosticSeqId::EMPTY,
decisions: Vec::new(),
actions: vec![ParserAction::new(1, 0, 0, None)],
nodes: NodeSeqId::EMPTY,
};
let second = RecognizeOutcome {
actions: vec![ParserAction::new(2, 0, 0, None)],
```rust
---
Found a 16 line (105 tokens) duplication in the following files:
* Starting at line 6847 of src/parser.rs
* Starting at line 7467 of src/parser.rs
```rust
let start_state = atn.rule_to_start_state().get(rule_index).ok_or_else(|| {
AntlrError::Unsupported(format!("rule {rule_index} has no start state"))
})?;
let stop_state = atn
.rule_to_stop_state()
.get(rule_index)
.filter(|state| *state != usize::MAX)
.ok_or_else(|| {
AntlrError::Unsupported(format!("rule {rule_index} has no stop state"))
})?;
let start_index = self.current_visible_index();
self.clear_prediction_diagnostics();
self.reset_per_parse_caches();
self.reset_recognition_arena();
let caller_follow_state = self.pending_invoking_follow_state(atn);Found a 13 line (100 tokens) duplication in the following files:
let mut expected = BTreeSet::new();
for index in (1..self.rule_context_stack.len()).rev() {
let invoking_state = self.rule_context_stack[index].invoking_state;
let Ok(state_number) = usize::try_from(invoking_state) else {
continue;
};
let Some(Transition::Rule { follow_state, .. }) = atn
.state(state_number)
.and_then(|state| state.transitions().first())
.map(ParserTransition::data)
else {
continue;
};
```rust
</details> |
|
Claude encountered an error after 1s —— View job I'll analyze this and get back to you. |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📊 Source Code Metrics (this PR vs
|
| File | Cyclomatic | Cognitive | Functions | LLOC | MI |
|---|---|---|---|---|---|
| src/parser.rs | 2116 (main: 2102) 🔴 | 1409 (main: 1404) 🔴 | 664 (main: 656) 🔴 | 4609 (main: 4586) 🔴 | 0 ⚪ |
| src/bin/antlr4-rust-gen.rs | 2249 ⚪ | 1415 ⚪ | 498 ⚪ | 3940 (main: 3939) 🔴 | 0 ⚪ |
Generated by mehen v1.7.0 — the code quality watcher.
Contributor
Author
|
Local conformance sweep on this branch (rebased onto main incl. #194): 357 passed, 0 failed, 0 skipped. No descriptor sets a depth cap, so the unset path ( |
Contributor
Author
|
Kotlin-parity dumper timings on this branch (cap unset, min of 3×30 iters, quiet machine): 01-nested-types 0.108 ms, 02-dataframe 0.770 ms, 03-string-templates 0.323 ms — identical to the pre-#193 baseline (0.105/0.767/0.321). The cap check adds no measurable cost when disabled. |
Three review findings on #199, all reproduced and fixed: 1. Sticky violation leaked on the Err exit path: a cap hit absorbed by recovery followed by an outer sync failure surfaced the derived syntax error and left the flag set, poisoning subsequent entry-rule calls on the same instance. The generated top-level entry now clears a stale violation at true entry and drains it in the Err branch, preferring the depth error over errors derived from it. 2. ATN-preferred rules bypassed the cap entirely (their dispatch arm routed to the interpreted path, which never checks it). A configured cap now overrides the ATN preference via has_rule_depth_cap() — correctness of the resource bound beats the long-call-chain optimization. The deep-nesting fixture grew to an 8-rule chain so the classifier fires and the e2e test pins this path. 3. Left-recursive operator expansions deepened the tree without counting: a 2000-term a+a+... chain built a 2000-deep tree under any cap. Both recursion-context pushes now count expansions (mirroring upstream Parser.pushNewRecursionContext firing triggerEnterRuleEvent), scoped per invocation so finished rules release their depth, and the generated LR loop checks the cap each iteration.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/bin/antlr4-rust-gen.rs`:
- Around line 6944-6953: Adjust the generated left-recursive expansion logic
around push_new_recursion_context_with_previous in
src/bin/antlr4-rust-gen.rs:6944-6953 so rule_depth_limit_error() uses the
pre-expansion depth and permits an expansion reaching exactly max_rule_depth,
rejecting only the first entry beyond the limit. Add coverage in
tests/antlr4_rust_cli.rs:1633-1659 for both an exact-limit expansion that
succeeds and one additional expansion that fails.
In `@src/parser.rs`:
- Around line 1155-1159: Update the ATN interpreter fallback path, including
parse_atn_rule* and recursive expansions, to enforce the configured depth cap at
every rule-entry stack push. Ensure interpreter-only rules cannot bypass the
limit; alternatively reject interpreter fallback whenever the cap is enabled.
Revise the adjacent documentation to describe the enforced behavior rather than
claiming those rules are uncapped.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0b941170-acca-4767-8f09-57f12b1f30fa
📒 Files selected for processing (4)
src/bin/antlr4-rust-gen.rssrc/parser.rstests/antlr4_rust_gen_cli.rstests/fixtures/antlr4-rust-gen/deep-nesting/Nest.g4
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3270fb2650
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| writeln!( | ||
| out, | ||
| " {index} if self.generated_only() => Some(self.parse_generated_rule_{index}_dispatch(precedence, allow_fallback))," | ||
| " {index} if self.generated_only() || self.base.has_rule_depth_cap() => Some(self.parse_generated_rule_{index}_dispatch(precedence, allow_fallback))," |
There was a problem hiding this comment.
This changes the emitted arm to include || self.base.has_rule_depth_cap(), but renders_atn_preferred_dispatch_only_for_generated_only_mode still requires the old substring without that condition. Consequently, cargo test --locked --features codegen --bin antlr4-rust-gen renders_atn_preferred_dispatch_only_for_generated_only_mode fails at src/bin/antlr4-rust-gen.rs:12593; update the assertion and its description to match the new guarded dispatch.
Useful? React with 👍 / 👎.
| // dispatch) still abort promptly instead of at the top-level drain. | ||
| writeln!( | ||
| out, | ||
| "{pad} if let Some(__depth_error) = self.base.rule_depth_limit_error() {{\n\ |
There was a problem hiding this comment.
For a left-recursive operator iteration, push_new_recursion_context_with_previous has already incremented recursion_expansions before this check. rule_depth_limit_error uses effective_depth >= max because dispatch calls it before adding a rule frame, so reusing it here rejects an expansion whose resulting depth is exactly the advertised maximum. For example, with a cap one level above the active s -> expr frames, the first a+a expansion is rejected even though it only reaches the cap; this post-increment path needs to allow equality or check before incrementing.
Useful? React with 👍 / 👎.
Contributor
Author
|
Review findings addressed in 3270fb2 + bd7309d:
Local verification on the revised branch: conformance 357/357, unit+all-features tests green, clippy |
CI parse-bench flagged a 1.15x regression on the Java issue-174-return-expression fixture: every generated dispatch paid an out-of-line #[cold] call for depth accounting even with no cap set (the default). Gate both emitted checks (dispatch boundary and LR loop) on the const-inlinable has_rule_depth_cap() load so uncapped parses keep a single predictable branch, and mark rule_depth_limit_error #[cold] — it only runs when a cap is configured.
Second review round on #199, all three findings addressed: 1. The emitted checks used let-chains, breaking generated output for edition-2021 consumers (generated code was previously edition-clean; the CI check crate pins 2024 so nothing caught it). The gate and the cold error construction fold into one #[inline] runtime helper, rule_depth_cap_violation(), and both sites emit a plain `if let`. This also un-pessimizes the capped path: the always-on state for the cel-rust use case now pays an inlined add+compare per entry instead of an out-of-line #[cold] call. Verified: generated Nest parser has zero let-chains and cargo-checks in an edition = "2021" crate. 2. The deep-nesting fixture never actually made a rule ATN-preferred (7-rule chain, and every decision has an LL(1) fast path so the decision-cost gate cannot fire). Corrected the fixture and test comments to say so — the cap-overrides-ATN-preference guard is pinned by the dispatch-rendering unit test. 3. The LR-loop probe ran after push_new_recursion_context_with_previous had already counted the expansion, admitting expansions only to max-1 while rule frames reached max. The probe now runs before the push, and a boundary e2e test pins that one more operator costs exactly one more depth level. Also corrected two stale comments: recovery DOES absorb the Fatal depth error (the sticky flag + top-level drain enforce the abort), and the ATN-preferred dispatch arm is now guarded by generated_only() OR has_rule_depth_cap().
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/parser.rs (1)
5825-5833: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftInterpreter fallback still bypasses the cap.
has_rule_depth_cap()only redirects ATN-preferred generated rules. Rules without generated bodies can still reachparse_atn_rule*, whose rule transitions and left-recursive boundaries never callrule_depth_cap_violation(). A capped parse can therefore recurse unbounded through an interpreted child. Enforce the cap in interpreter rule entry/expansion paths, or reject all interpreted fallback while capped; add an interpreted-only regression test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/parser.rs` around lines 5825 - 5833, Extend rule-depth enforcement beyond has_rule_depth_cap and the generated recursive-descent path: ensure interpreted entry and expansion paths such as parse_atn_rule* check the configured cap, including rule transitions and left-recursive boundaries, before recursing. Alternatively, disable interpreted fallback whenever a cap is configured, but preserve capped parsing for generated rules. Add a regression test that exercises an interpreted-only rule and verifies the depth cap is enforced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/parser.rs`:
- Around line 5825-5833: Extend rule-depth enforcement beyond has_rule_depth_cap
and the generated recursive-descent path: ensure interpreted entry and expansion
paths such as parse_atn_rule* check the configured cap, including rule
transitions and left-recursive boundaries, before recursing. Alternatively,
disable interpreted fallback whenever a cap is configured, but preserve capped
parsing for generated rules. Add a regression test that exercises an
interpreted-only rule and verifies the depth cap is enforced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 53b64db3-a8e2-4df5-8408-2be826e93e85
📒 Files selected for processing (4)
src/bin/antlr4-rust-gen.rssrc/parser.rstests/antlr4_rust_gen_cli.rstests/fixtures/antlr4-rust-gen/deep-nesting/Nest.g4
Contributor
Author
|
Second review round addressed in 708866e — local verification: conformance 357/357, unit + codegen + e2e tests green, clippy |
This was referenced Jul 25, 2026
tinovyatkin
added a commit
that referenced
this pull request
Jul 26, 2026
… (ANTLR parity) (#204) * feat(runtime): pass the offending token to ErrorListener::syntax_error ErrorListener::syntax_error now receives offending: Option<TokenView> right after the recognizer, matching ANTLR's canonical syntaxError(recognizer, offendingSymbol, ...) contract. Every reference runtime passes the offending symbol; span-building error reporters (e.g. avdl's miette diagnostics with byte-offset underlines) need start_byte/stop_byte from the token, not just (line, column). ParserDiagnostic records the anchoring TokenId at each creation site (diagnostic_for_token already had the token in hand; the extraneous/ missing recovery paths record the current token) and dispatch resolves it to a TokenView from the token store. Lexer-originated diagnostics pass None, matching ANTLR's null offendingSymbol for lexer errors. Fixes #195 * fix(runtime): carry the offending token on AntlrError::ParserError Review on #196 found the offending-token contract held only for extraneous-input/missing-token recovery and prediction diagnostics: generated parsers route ordinary mismatched-input, no-viable-alternative, failed-predicate, and sync errors through AntlrError::ParserError, whose diagnostic arm hard-coded offending: None — exactly the errors a real recognizer reports most. ParserError gains offending: Option<TokenId>, recorded where each error is built (recover_generated_match, generated sync, failed-predicate builders, recognition_error) rather than resolved at reporting time: prediction restores the input cursor, so lt(1) at dispatch can point at the decision start instead of the error index — no_viable_alternative_error_at now forwards the anchor its diagnostic already computed. Also converts the new listener test to a named insta snapshot per house style. * ci: displace wedged Copy/Paste Detection run * merge: combine offending-token (#195) with main incl. depth cap (#199) The depth-cap violation error now carries its offending token like every other ParserError built at a known input position. * feat(runtime): add_parse_listener — parse-time rule enter/exit events ANTLR's addParseListener delivers enterEveryRule/exitEveryRule during recognition; cel-rust's RecursionListener (live expr-depth counting with parse abort) is built on it. Our runtime only had post-parse listeners, forcing ports to rework such listeners into post-parse tree walks. ParseListener (enter fallible for aborts, exit infallible) dispatches from generated rule bodies: enter before the body after the depth-cap probe, exit on every exit path, one simulated enter per left-recursive operator expansion (upstream Parser.pushNewRecursionContext fires triggerEnterRuleEvent) with matching exits as the rule unrolls — pairs always balance. A listener abort is sticky through rule-level recovery, drained at the top-level entry via take_parse_abort() (unified with the depth-cap violation, depth error preferred), and cleared at entry so instances never poison the next parse. Costs nothing when unused: dispatch sites gate on list emptiness (one predictable branch; Kotlin parse timings unchanged, ktor parse-bench fixture at baseline). When a listener is registered, generated dispatch routes ATN-preferred rules through their generated bodies so real grammars observe every rule; interpreter-only rules do not fire events (documented divergence, matching the depth cap). Emitted probes are plain if-let — generated output stays edition-2021 clean. e2e: cel-rust's RecursionListener ported verbatim in the deep-nesting fixture test — live counting, positioned abort error, LR expansion entries counted, clean reuse after abort. Closes #202 * fix(runtime): address parse-listener review findings 1. Reserved-name collision: add_parse_listener/remove_parse_listeners join GENERATED_PARSER_RESERVED_RULE_METHODS so a grammar rule named addParseListener renames to add_parse_listener_rule instead of colliding with the facade (E0592). 2. Exit events now fire in reverse registration order, matching upstream Parser.triggerExitRuleEvent; new e2e test registers two tracing listeners and pins enter A,B / exit B,A plus pair balance on both the success and recovery paths. 3. Doc correction: the aborting enter_every_rule receives no matching exit (same as Java, where enterRule throws before try/finally); listener state shared across parses must be reset after an abort. 4. remove_parse_listeners clears the sticky abort and returns the boxed listeners, giving callers their accumulated state back without external shared handles. 5. enter_every_rule takes #[non_exhaustive] EnterRuleEvent so future fields extend the event without breaking implementors; the dispatch helper loses its #[cold] (it is the hot path once registered). 6. New coverage: multi-listener order, recovery balance, depth-cap + listener coexistence (either bound trips first and surfaces). Measured with-listener cost (finding 7): on the CEL grammar — the migration target, no ATN-preferred rules — a counting listener adds ~5% (0.0433 -> 0.0453 ms on cel-rust's criterion stress expression). On grammars with ATN-preferred rules the dominant cost is the routing override those rules take (Kotlin ktor fixture 10.2 -> 86 ms), shared byte-for-byte with the depth cap (cap-only run measures identically); listener dispatch on top of it is noise. Documented on the trait. * fix(runtime): parse-listener round-2 review findings 1. Parser trait gains add_parse_listener(Box<dyn ParseListener>) / remove_parse_listeners() (mirroring set_max_rule_depth), fixing the broken intra-doc link and letting code generic over P: Parser reach listener registration. 2. LR-expansion aborts now match Java exactly: the listener enter probe fires AFTER push_new_recursion_context_with_previous (upstream assigns _ctx before triggerEnterRuleEvent), so an aborting expansion is already counted and the unroll fires its matching exit — Java parity via the finally-driven unrollRecursionContexts. Ordinary-rule aborts keep the documented no-exit behavior (also Java parity). Trait doc states both cases precisely. 3. Success-path LR coverage: a+a+a+a under a tracing listener pins 7 RULE_EXPR enters (1 dispatch + 3 expansions + 3 operands) and full enter/exit balance through the unroll loop. 4. Stale ATN-preferred routing comment updated for the listener guard. * fix(runtime): make removed parse listeners re-registrable CodeRabbit round 3: Box<dyn ParseListener> returned by remove_parse_listeners could not be passed back to the inherent add_parse_listener (Box did not implement ParseListener). Add the forwarding impl for Box<T: ParseListener + ?Sized> and pin the round-trip in the e2e test — a removed listener re-registers with its accumulated state and still enforces its limit. Also cover the left-recursive success path: one listener instance parses the same under-limit operator chain twice; identical high-water marks prove the live depth counter returned to zero after the first LR unroll (balanced enter/exit through expansions). * fix(runtime): match upstream left-recursive listener exit timing Round-4 review built a Java oracle (ANTLR 4.13.2 on this PR's own Nest.g4) and caught that batching expansion exits at unroll diverges from upstream on live depth: recRuleSetPrevCtx fires triggerExitRuleEvent at the TOP of each operator-loop pass, so the outgoing iteration exits before the next expansion enters and flat chains never accumulate depth (a+a+...+a peaks at 2 in every ANTLR target; ours peaked at chain width). The motivating cel-rust RecursionListener would have rejected wide flat CEL expressions every other target accepts. The generated operator loop now exits the outgoing iteration first, and unroll_recursion_context drops the batched exits (upstream's unrollRecursionContexts walks exactly one link — the dispatch wrapper's single exit plays it). Event counts unchanged; timing now matches the oracle: 7/7 depth 2, 79/79 depth 2, bracket cases unchanged. The e2e that encoded the divergence now pins the Java-oracle depth of 2 for a 40-term chain. Also from review: generated impl Parser blocks forward add_parse_listener/remove_parse_listeners (they inherited the no-op trait defaults — generic registration silently dropped listeners and trait-removal left them firing with the sticky abort uncleared), and the trait doc gains the expansion-anchor note (EnterRuleEvent::current is the operator-side lookahead, not Java's whole-expression start).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #198. Stacked on #194 (contains its commits; the depth cap builds on the same dispatch boundary the stack guard added).
What
Parser::set_max_rule_depth(Option<usize>)— defaultNone(unlimited). When set, generated rule dispatch aborts at the first rule entry past the cap with a positioned error:The violation is sticky: rule-level recovery would otherwise absorb the error and keep spending exactly the resources the cap exists to bound. Every subsequent rule entry fails immediately, the generated top-level entry drains the recorded violation and fails the parse even when recovery produced a tree, and
reset()/set_token_stream()clear it so a reused parser starts clean.Why
After #193 deeply nested input no longer crashes — but it still costs unbounded CPU and tree memory. Consumers parsing attacker-supplied input need a parse-time bound: cel-rust (the
celcrate, CEL policy engines) enforces depth 96 via an ANTLR parse listener (RecursionListener), an API we don't have. This gives its port a first-class replacement, checked at the same dispatch boundary as the stack guard (zero new state reads on the hot path when unset — oneOptionbranch).Verification
cargo test --lockedall green; clippy-D warningsall-targets all-features clean.Nonecheck).Summary by CodeRabbit
max_rule_depth/set_max_rule_depth.