Skip to content

Add recursion checks for parse_interval - #2422

Open
shuvamk wants to merge 1 commit into
apache:mainfrom
shuvamk:interval-recursion-limit
Open

Add recursion checks for parse_interval#2422
shuvamk wants to merge 1 commit into
apache:mainfrom
shuvamk:interval-recursion-limit

Conversation

@shuvamk

@shuvamk shuvamk commented Aug 4, 2026

Copy link
Copy Markdown
SELECT INTERVAL INTERVAL INTERVAL ... 1   -- 600 repetitions, ~5 KB

This aborts the process with fatal runtime error: stack overflow on a 2 MiB stack
(the default for a spawned thread) instead of returning RecursionLimitExceeded.

For dialects where require_interval_qualifier() is false, parse_interval calls
parse_prefix directly rather than parse_expr, so it never passes through the
recursion counter that lives in parse_subexpr. parse_prefix re-enters
parse_interval on the next INTERVAL keyword, leaving that cycle unguarded.
Reproduced on generic, duckdb, snowflake and postgres.

The fix applies the same two guards #2199 used for the parenthesis gap; the 600-level
input then returns Err(RecursionLimitExceeded).

Test added beside the existing recursion-limit tests in tests/sqlparser_common.rs.
Without the parser change it fails as a stack-overflow abort. Same defect class as
#2411, on a different path.

`SELECT INTERVAL INTERVAL INTERVAL ... 1` (600 levels, ~5 KB) aborts the
process with `fatal runtime error: stack overflow` on a 2 MiB stack
instead of returning `ParserError::RecursionLimitExceeded`. In Rust a
stack overflow is `abort()`, not a panic, so a consumer parsing
untrusted SQL cannot contain it.

For dialects where `require_interval_qualifier()` is false,
`parse_interval` calls `parse_prefix` directly rather than `parse_expr`,
so it bypasses the counter that lives in `parse_subexpr`. `parse_prefix`
re-enters `parse_interval` on the next INTERVAL keyword, leaving the
cycle unguarded. Reproduced on generic, duckdb, snowflake and postgres.

Apply the same guards used for the parenthesis gap in apache#2199: the
recursion counter plus `recursive::recursive` under the
`recursive-protection` feature.

Test in tests/sqlparser_common.rs alongside the other recursion-limit
tests.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@LucaCappelletti94 LucaCappelletti94 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks sensible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants