fix(native): report deeply nested header values instead of crashing (#1015)

* fix(native): report deeply nested header values instead of crashing

The native header value parser recursed once per nesting level, so a
header value nested about 20,000 levels deep overflowed the stack and
crashed discovery under every compiler engine.

The parser now refuses to open more than 256 nested containers, the
same limit the YAML loader uses. Discovery reports it under each
statement's existing header syntax code, naming the file line of the
container that is too deep, with help on how to fix it. The in-memory
parse entry points run on the discovery worker pool.

* fix(native): project nested header values without Python recursion

Python projected native header values recursively, spending about four
frames per call level, so call syntax nested 231 to 256 levels deep
raised RecursionError in every engine. The projection now walks values
with an explicit stack, and a plain call counts as the two levels it
builds (a hook's arguments count as one more), so the 256-level limit
measures the values discovery actually produces. Lint suggests the same
flatten help for a header nested too deeply.

Author: kvlonge

crates/sqlbuild-discovery/src/_helpers/header_syntax.rs

@@ -0,0 +1,36 @@
+//! The invalid-header-syntax failure every statement header reports, as Python's discovery does.
+
+use crate::models::{DiscoveryFailure, FailureKind};
+use sqlbuild_sqltext::compiler::main::model_header_errors::header_nesting_failure;
+
+/// One header that failed to parse, with the one-based file line its text starts on.
+pub(crate) struct FailedHeader<'a> {
+    pub(crate) kind: FailureKind,
+    /// The name in syntax errors, such as `MODEL`.
+    pub(crate) statement_name: &'a str,
+    pub(crate) file_path: &'a str,
+    pub(crate) text: &'a str,
+    pub(crate) line: usize,
+}
+
+/// The invalid-syntax failure for `error`; a nesting error names its file line and a fix.
+pub(crate) fn header_syntax_failure(header: &FailedHeader<'_>, error: &str) -> DiscoveryFailure {
+    let (statement_name, file_path) = (header.statement_name, header.file_path);
+    let Some(nesting) = header_nesting_failure(error, header.text, header.line) else {
+        return DiscoveryFailure::new(
+            header.kind,
+            format!(
+                "{statement_name}(...) in '{file_path}' contains invalid SQLBuild header syntax: \
+                 {error}"
+            ),
+        );
+    };
+    DiscoveryFailure {
+        kind: header.kind,
+        message: format!(
+            "{statement_name}(...) in '{file_path}:{}' contains invalid SQLBuild header syntax: {}",
+            nesting.line, nesting.message
+        ),
+        help: Some(nesting.help),
+    }
+}

crates/sqlbuild-discovery/src/_helpers/mod.rs

@@ -1,4 +1,5 @@
 pub(crate) mod header_keys;
+pub(crate) mod header_syntax;
 pub(crate) mod locations;
 pub(crate) mod pool;
 pub(crate) mod reading;

crates/sqlbuild-discovery/src/_helpers/pool.rs

@@ -1,4 +1,4 @@
-//! The worker pool native discovery runs on; header parsing recurses, so stacks are large.
+//! The worker pool native discovery runs on; parsing recurses, so stacks are large.
 
 use rayon::{ThreadPool, ThreadPoolBuilder};
 use std::sync::OnceLock;
@@ -18,3 +18,11 @@ pub(crate) fn discovery_pool() -> Result<&'static ThreadPool, String> {
     .as_ref()
     .map_err(Clone::clone)
 }
+
+/// Run `work` on the discovery pool, or on the calling thread (safe: nesting is bounded).
+pub(crate) fn on_discovery_pool<T: Send>(work: impl FnOnce() -> T + Send) -> T {
+    match discovery_pool() {
+        Ok(pool) => pool.install(work),
+        Err(_) => work(),
+    }
+}

crates/sqlbuild-discovery/src/_helpers/statement_headers.rs

@@ -1,6 +1,7 @@
 //! Parse a statement header and reject its unsupported keys, as Python's discovery does.
 
 use crate::_helpers::header_keys::{UnsupportedKeys, unsupported_keys_failure};
+use crate::_helpers::header_syntax::{FailedHeader, header_syntax_failure};
 use crate::models::{DiscoveryFailure, FailureKind};
 use sqlbuild_core::text::models::PythonText;
 use sqlbuild_sqltext::compiler::main::model_header_single_parsing::parse_one;
@@ -25,11 +26,13 @@ pub(crate) fn parse_statement_header(
     header_line: usize,
 ) -> Result<Vec<(String, AuthoredValue)>, DiscoveryFailure> {
     let values: Vec<(String, AuthoredValue)> = match parse_one(header) {
-        (_, _, Some(error)) => return Err(syntax_failure(contract, &error)),
+        (_, _, Some(error)) => return Err(syntax_failure(contract, header, header_line, &error)),
         (Some(AuthoredValue::Map(values)), _, None) => values,
         _ => {
             return Err(syntax_failure(
                 contract,
+                header,
+                header_line,
                 "Native MODEL header parser returned neither values nor an error",
             ));
         }
@@ -54,13 +57,21 @@ pub(crate) fn parse_statement_header(
     }))
 }
 
-fn syntax_failure(contract: &StatementHeader<'_>, error: &str) -> DiscoveryFailure {
-    DiscoveryFailure::new(
-        contract.kind,
-        format!(
-            "{}(...) in '{}' contains invalid SQLBuild header syntax: {error}",
-            contract.statement_name, contract.file_path
-        ),
+fn syntax_failure(
+    contract: &StatementHeader<'_>,
+    header: &str,
+    header_line: usize,
+    error: &str,
+) -> DiscoveryFailure {
+    header_syntax_failure(
+        &FailedHeader {
+            kind: contract.kind,
+            statement_name: contract.statement_name,
+            file_path: contract.file_path,
+            text: header,
+            line: header_line,
+        },
+        error,
     )
 }
 

crates/sqlbuild-discovery/src/declaration_files/_helpers/parsing/declaration_headers.rs

@@ -1,5 +1,6 @@
 //! Python's `_parse_declaration_headers`: split a file into `KIND(...);` headers and parse each.
 
+use crate::_helpers::header_syntax::{FailedHeader, header_syntax_failure};
 use crate::declaration_files::_helpers::checks::python_values::failure;
 use crate::declaration_files::_helpers::checks::stops::ParseStop;
 use crate::models::FailureKind;
@@ -55,21 +56,21 @@ pub(crate) fn declaration_headers(
             )));
         }
         let header_start: usize = open_index + 1;
-        let (values, column_offsets) = match parse_one(&contents[header_start..close_index]) {
-            (_, _, Some(error)) => {
-                return Err(declaration(syntax_message(
-                    expected_kind,
-                    file_path,
-                    &error,
-                )));
-            }
+        let header = FailedHeader {
+            kind: FailureKind::Declaration,
+            statement_name: expected_kind,
+            file_path,
+            text: &contents[header_start..close_index],
+            line: contents[..header_start].matches('\n').count() + 1,
+        };
+        let syntax_failure = |error: &str| ParseStop::Failed(header_syntax_failure(&header, error));
+        let (values, column_offsets) = match parse_one(header.text) {
+            (_, _, Some(error)) => return Err(syntax_failure(&error)),
             (Some(AuthoredValue::Map(values)), Some(offsets), None) => (values, offsets),
             _ => {
-                return Err(declaration(syntax_message(
-                    expected_kind,
-                    file_path,
+                return Err(syntax_failure(
                     "Native MODEL header parser returned neither values nor an error",
-                )));
+                ));
             }
         };
         headers.push(DeclarationHeader {
@@ -88,10 +89,6 @@ pub(crate) fn declaration_headers(
     Ok(headers)
 }
 
-fn syntax_message(kind: &str, file_path: &str, error: &str) -> String {
-    format!("{kind}(...) in '{file_path}' contains invalid SQLBuild header syntax: {error}")
-}
-
 /// `(ENUM|CONSTANT|SCHEMA)\s*\(` at `start`: the kind and the byte offset of the parenthesis.
 fn declaration_start(contents: &str, start: usize) -> Option<(&'static str, usize)> {
     let rest: &str = &contents[start..];

crates/sqlbuild-discovery/src/declaration_files/main/parse_declaration_text.rs

@@ -1,5 +1,6 @@
 //! Parse in-memory declaration file contents as discovery parses a file of one kind.
 
+use crate::_helpers::pool::on_discovery_pool;
 use crate::declaration_files::_helpers::checks::stops::ParseStop;
 use crate::declaration_files::_helpers::parsing::audit_files::parse_audit_file;
 use crate::declaration_files::_helpers::parsing::constant_files::parse_constant_file;
@@ -12,12 +13,21 @@ use crate::declaration_files::models::{
 };
 use crate::models::FileOutcome;
 
-/// `contents` parsed as a `kind` file named `file_path`; `None` for seeds and macros, which have none.
+/// `contents` parsed on the discovery pool as a `kind` file; `None` for seeds and macros.
 pub fn parse_declaration_text(
     kind: CollectionKind,
     (file_path, hook_name): (&str, &str),
     contents: String,
     options: &DeclarationFileOptions,
+) -> Option<FileOutcome<ParsedDeclarationText>> {
+    on_discovery_pool(|| parse_text(kind, (file_path, hook_name), contents, options))
+}
+
+fn parse_text(
+    kind: CollectionKind,
+    (file_path, hook_name): (&str, &str),
+    contents: String,
+    options: &DeclarationFileOptions,
 ) -> Option<FileOutcome<ParsedDeclarationText>> {
     let parsed: Result<ParsedDeclarationText, ParseStop> = match kind {
         CollectionKind::Enums => {

crates/sqlbuild-discovery/src/declaration_files/tests/helpers.rs

@@ -82,3 +82,18 @@ pub(super) fn session_reads(
 fn collection_len(collection: &DeclarationCollection) -> usize {
     format!("{collection:?}").matches("ScopedFile {").count()
 }
+
+/// The debug text of the failure a header nested too deeply reports at `(statement, line)`.
+pub(super) fn nesting_failure_debug((statement, line): (&str, usize)) -> String {
+    format!(
+        "Some(Failed(DiscoveryFailure {{ kind: {}, message: \"{statement}(...) in '{FILE_PATH}:{line}' \
+         contains invalid SQLBuild header syntax: values nest deeper than 256 levels\", help: \
+         Some(\"flatten the value so it nests at most 256 levels deep\") }}))",
+        match statement {
+            "CONSTANT" => "Declaration",
+            "HOOK" => "SqlHook",
+            "AUDIT" => "SqlAudit",
+            _ => "ModelSql",
+        }
+    )
+}

crates/sqlbuild-discovery/src/declaration_files/tests/test_parse_declaration_text.rs

@@ -1,6 +1,8 @@
 use crate::declaration_files::models::CollectionKind;
-use crate::declaration_files::tests::helpers::parsed_debug;
-use crate::declaration_files::tests::test_types::DeclarationTextTestCase;
+use crate::declaration_files::tests::helpers::{nesting_failure_debug, parsed_debug};
+use crate::declaration_files::tests::test_types::{
+    DeclarationTextTestCase, DeepDeclarationTestCase,
+};
 
 #[test]
 fn given_declaration_files_when_parsing_then_values_and_failures_match_python() {
@@ -213,3 +215,56 @@ fn given_declaration_files_when_parsing_then_values_and_failures_match_python()
         );
     }
 }
+
+#[test]
+fn given_deeply_nested_declaration_headers_when_parsing_then_each_reports_its_file_line() {
+    let test_cases = [
+        DeepDeclarationTestCase {
+            description: "a constant value",
+            kind: CollectionKind::Constants,
+            prefix: "CONSTANT (\n  name limits,\n  value ",
+            suffix: ",\n);\n",
+            expected_location: ("CONSTANT", 3),
+        },
+        DeepDeclarationTestCase {
+            description: "a hook description",
+            kind: CollectionKind::SqlHooks,
+            prefix: "HOOK (\n  description ",
+            suffix: ",\n);\n\nSELECT 1\n",
+            expected_location: ("HOOK", 2),
+        },
+        DeepDeclarationTestCase {
+            description: "a function return type",
+            kind: CollectionKind::SqlFunctions,
+            prefix: "FUNCTION (\n  description \"Whether placed\",\n  returns ",
+            suffix: ",\n);\n\norder_status = 'placed'\n",
+            expected_location: ("FUNCTION", 3),
+        },
+        DeepDeclarationTestCase {
+            description: "an audit value on a later block",
+            kind: CollectionKind::Audits,
+            prefix: "AUDIT (name a);\nSELECT 1\nAUDIT (\n  name b,\n  value ",
+            suffix: ",\n);\nSELECT 2\n",
+            expected_location: ("AUDIT", 5),
+        },
+    ];
+
+    for test_case in test_cases {
+        let contents: String = format!(
+            "{}{}1{}{}",
+            test_case.prefix,
+            "[".repeat(20_000),
+            "]".repeat(20_000),
+            test_case.suffix
+        );
+
+        let debug: String = parsed_debug(test_case.kind, &contents);
+
+        assert_eq!(
+            debug,
+            nesting_failure_debug(test_case.expected_location),
+            "{}",
+            test_case.description
+        );
+    }
+}

crates/sqlbuild-discovery/src/declaration_files/tests/test_types.rs

@@ -18,3 +18,15 @@ pub(super) struct SessionReadTestCase {
     /// The layouts walked: one shared by every kind, plus one per isolated kind.
     pub(super) expected_layouts: usize,
 }
+
+/// A declaration file whose header nests one value far too deeply, on header line `line`.
+pub(super) struct DeepDeclarationTestCase {
+    pub(super) description: &'static str,
+    pub(super) kind: CollectionKind,
+    /// The file text before the nested value.
+    pub(super) prefix: &'static str,
+    /// The file text after the nested value.
+    pub(super) suffix: &'static str,
+    /// The statement name and file line the failure reports.
+    pub(super) expected_location: (&'static str, usize),
+}

crates/sqlbuild-discovery/src/model_files/_helpers/parsing.rs

@@ -1,6 +1,7 @@
 //! Parse one model file: header checks, model-local declaration checks and their messages.
 
 use crate::_helpers::header_keys::{UnsupportedKeys, unsupported_keys_failure};
+use crate::_helpers::header_syntax::{FailedHeader, header_syntax_failure};
 use crate::_helpers::locations::header_column_locations;
 use crate::model_files::_helpers::output_columns::output_column_locations;
 use crate::model_files::models::{DiscoveredModelFile, ModelFileOptions};
@@ -28,12 +29,20 @@ pub(crate) fn parse_model_file(
         )));
     };
     let header = &contents[header_start..header_end];
+    let header_line: usize = contents[..header_start].matches('\n').count() + 1;
+    let failed_header = FailedHeader {
+        kind: FailureKind::ModelSql,
+        statement_name: "MODEL",
+        file_path,
+        text: header,
+        line: header_line,
+    };
     let (header_values, column_offsets) = match parse_one(header) {
-        (_, _, Some(error)) => return Err(syntax_failure(file_path, &error)),
+        (_, _, Some(error)) => return Err(header_syntax_failure(&failed_header, &error)),
         (Some(AuthoredValue::Map(values)), Some(offsets), None) => (values, offsets),
         _ => {
-            return Err(syntax_failure(
-                file_path,
+            return Err(header_syntax_failure(
+                &failed_header,
                 "Native MODEL header parser returned neither values nor an error",
             ));
         }
@@ -58,7 +67,7 @@ pub(crate) fn parse_model_file(
             statement: MODEL_STATEMENT,
             file_path,
             header,
-            header_line: contents[..header_start].matches('\n').count() + 1,
+            header_line,
             keys: &unsupported,
             supported_keys: &options.supported_keys,
             python: options.python,
@@ -91,12 +100,6 @@ pub(crate) fn parse_model_file(
     })
 }
 
-fn syntax_failure(file_path: &str, error: &str) -> DiscoveryFailure {
-    failure(format!(
-        "MODEL(...) in '{file_path}' contains invalid SQLBuild header syntax: {error}"
-    ))
-}
-
 fn header_keys(values: &[(String, AuthoredValue)]) -> impl Iterator<Item = &str> {
     values.iter().map(|(key, _)| key.as_str())
 }