From b7cbd094c5531f639cea459c4ad884e564c2089d Mon Sep 17 00:00:00 2001 From: fmcmac Date: Fri, 10 Jul 2026 14:02:30 +1200 Subject: [PATCH] Apply recursive-protection to the Spanned::span walk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `Spanned::span` implementations are plain unbounded recursion — one stack frame per AST node. Computing the span of a deeply nested AST (a deep expression tree, nested subqueries, or a long left-associative boolean chain) therefore recurses as deep as the AST and can overflow the thread stack, aborting the process. A query can parse successfully — the parser is already guarded — and then overflow later when its span is computed (e.g. during query planning by a downstream consumer). The parser already opts into stack-overflow protection behind the `recursive-protection` feature (`recursive::recursive`, backed by `stacker::maybe_grow`); the span walk was simply never given the same treatment. Annotate the recursive `span()` implementations for the container nodes (`Expr`, `SetExpr`, `Query`, `Select`, `SelectItem`, `TableFactor`, `Statement`) so at least one node on every deep-recursion cycle grows the stack on demand. No new dependency and a no-op when the feature is disabled. Adds a regression test that walks a deeply nested AST on a small thread stack: it overflows without the annotations and completes with them. Signed-off-by: fmcmac --- src/ast/spans.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/ast/spans.rs b/src/ast/spans.rs index 6505b209e..9cfffadce 100644 --- a/src/ast/spans.rs +++ b/src/ast/spans.rs @@ -113,6 +113,7 @@ impl Spanned for Parens { } impl Spanned for Query { + #[cfg_attr(feature = "recursive-protection", recursive::recursive)] fn span(&self) -> Span { let Query { with, @@ -217,6 +218,7 @@ impl Spanned for Cte { /// /// [SetExpr::Table] is not implemented. impl Spanned for SetExpr { + #[cfg_attr(feature = "recursive-protection", recursive::recursive)] fn span(&self) -> Span { match self { SetExpr::Select(select) => select.span(), @@ -320,6 +322,7 @@ impl Spanned for Values { /// - [Statement::Unload] /// - [Statement::OptimizeTable] impl Spanned for Statement { + #[cfg_attr(feature = "recursive-protection", recursive::recursive)] fn span(&self) -> Span { match self { Statement::Analyze(analyze) => analyze.span(), @@ -1480,6 +1483,7 @@ impl Spanned for AssignmentTarget { /// - [Expr::Map] # DuckDB specific /// - [Expr::Lambda] impl Spanned for Expr { + #[cfg_attr(feature = "recursive-protection", recursive::recursive)] fn span(&self) -> Span { match self { Expr::Identifier(ident) => ident.span, @@ -1852,6 +1856,7 @@ impl Spanned for SelectItemQualifiedWildcardKind { } impl Spanned for SelectItem { + #[cfg_attr(feature = "recursive-protection", recursive::recursive)] fn span(&self) -> Span { match self { SelectItem::UnnamedExpr(expr) => expr.span(), @@ -1958,6 +1963,7 @@ impl Spanned for ReplaceSelectElement { /// Missing spans: /// - [TableFactor::JsonTable] impl Spanned for TableFactor { + #[cfg_attr(feature = "recursive-protection", recursive::recursive)] fn span(&self) -> Span { match self { TableFactor::Table { @@ -2303,6 +2309,7 @@ impl Spanned for TableWithJoins { } impl Spanned for Select { + #[cfg_attr(feature = "recursive-protection", recursive::recursive)] fn span(&self) -> Span { let Select { select_token, @@ -3116,4 +3123,34 @@ WHERE id = 1 Span::new(Location::new(2, 8), Location::new(4, 52)) ); } + // Regression: the `Spanned::span` walk is otherwise unbounded recursion, so + // a deeply nested expression (the shape a machine-generated query or a broad + // access-control predicate can produce) overflows the stack and aborts the + // process. With the `recursive-protection` feature the walk grows the stack + // on demand and completes. + // + // The AST is built directly (the parser is not the subject here) and walked + // on a deliberately small (512 KiB) thread stack, so the chosen depth far + // exceeds what the stack holds unprotected — the walk overflows without the + // fix and completes with it. + #[test] + fn test_deeply_nested_expr_span_does_not_overflow() { + std::thread::Builder::new() + .stack_size(512 * 1024) + .spawn(|| { + let depth = 200_000; + let mut expr = Expr::Identifier(crate::ast::Ident::new("x")); + for _ in 0..depth { + expr = Expr::Nested(Box::new(expr)); + } + // Must not overflow the stack while walking the deep AST. + let _ = expr.span(); + // The nested `Box` chain has a recursive `Drop`; forget it + // to keep the test fast and isolate the span walk under test. + core::mem::forget(expr); + }) + .unwrap() + .join() + .expect("span walk overflowed the small-stack thread"); + } }