A-Mavericks opened a new issue, #2909:
URL: https://github.com/apache/datafusion-sqlparser-rs/issues/2909

   ## Summary
   
   `Parser::parse_sql` accepts a flat left-associative operator chain and 
returns successfully, but dropping the returned `Vec<Statement>` then overflows 
the stack: the derived drop glue for the left-deep `Box<Expr>` tree recurses 
once per level with no guard. The process aborts (rc=134), and `abort` cannot 
be caught by the caller.
   
   On a 2 MB stack (the default for spawned threads, including tokio workers) 
roughly 35,000 `+` operators — about 70 KB of input — is enough:
   
   | stack | operators | input | result |
   |---|---|---|---|
   | 8 MB (main) | 130,625 | 261,258 B | parses and drops, rc=0 |
   | 8 MB (main) | 132,000 | 264,008 B | prints `parsed 1 statement(s)` then 
aborts |
   | 2 MB (thread) | 30,000 | 60,008 B | parses and drops, rc=0 |
   | 2 MB (thread) | 35,000 | 70,008 B | prints `parsed 1 statement(s)` then 
aborts |
   
   The `parsed ...` line is printed before the abort, so the overflow is in 
drop, not in parsing.
   
   ```
   thread 'main' has overflowed its stack
   fatal runtime error: stack overflow, aborting
   ```
   
   ## Root cause
   
   `Expr` and the rest of the AST derive `Drop` (`src/ast/mod.rs`). The only 
recursion guard on these types is
   
   ```rust
   #[cfg_attr(feature = "recursive-protection", recursive::recursive)]
   impl Display for Expr
   ```
   
   at `src/ast/mod.rs:1795`; there is no `impl Drop` and no `ManuallyDrop`.
   
   A flat chain such as `SELECT 1+1+1...` is built by the iterative 
precedence-climbing loop in `parse_subexpr`, so the recursion counter never 
fires, and the result is a left-deep `BinaryOp { left: Box<Expr>, .. }` spine. 
Dropping it walks that spine recursively, one stack frame per operator. 
`Parser::with_recursion_limit` does not help because parsing never recursed.
   
   ## Reproduction
   
   ```toml
   [dependencies]
   sqlparser = "=0.63.0"
   ```
   
   ```rust
   fn main() {
       let n: usize = std::env::args().nth(1).and_then(|s| 
s.parse().ok()).unwrap_or(132_000);
       let mut sql = String::from("SELECT 1");
       for _ in 0..n { sql.push_str("+1"); }
       let dialect = sqlparser::dialect::GenericDialect {};
       let stmts = sqlparser::parser::Parser::parse_sql(&dialect, 
&sql).expect("parse");
       println!("parsed {} statement(s), {} bytes", stmts.len(), sql.len());
       drop(stmts);
       println!("dropped ok");
   }
   ```
   
   ```console
   $ cargo run --release -- 130625
   parsed 1 statement(s), 261258 bytes
   dropped ok
   $ cargo run --release -- 132000
   parsed 1 statement(s), 264008 bytes
   thread 'main' has overflowed its stack
   fatal runtime error: stack overflow, aborting   # rc=134
   ```
   
   `SELECT 1=1 AND 1=1 ...` shows the same behaviour.
   
   ## Suggested fix
   
   Count the chain length against the existing recursion accounting: when the 
precedence loop folds another operator onto the same chain, add it to 
`RecursionCounter` and return `RecursionLimitExceeded` past the limit, matching 
what deeply nested parentheses already do. Alternatively, implement an 
iterative `Drop` for `Expr` with an explicit work stack, which also covers 
trees built programmatically.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to