diff --git a/Cargo.lock b/Cargo.lock index 8875b8bbf90d..5da1a6602d67 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -417,6 +417,7 @@ dependencies = [ "lazy_static", "rayon", "regex", + "serde", "serde_json", "swift-syntax-rs", "tracing", diff --git a/misc/bazel/3rdparty/tree_sitter_extractors_deps/crates.bzl b/misc/bazel/3rdparty/tree_sitter_extractors_deps/crates.bzl index 9e6627eb70a1..f2fdd420d9ae 100644 --- a/misc/bazel/3rdparty/tree_sitter_extractors_deps/crates.bzl +++ b/misc/bazel/3rdparty/tree_sitter_extractors_deps/crates.bzl @@ -453,6 +453,7 @@ _NORMAL_DEPENDENCIES = { "lazy_static": Label("@vendor_ts//lazy_static-1.5.0"), "rayon": Label("@vendor_ts//rayon-1.12.0"), "regex": Label("@vendor_ts//regex-1.13.1"), + "serde": Label("@vendor_ts//serde-1.0.229"), "serde_json": Label("@vendor_ts//serde_json-1.0.151"), "tracing": Label("@vendor_ts//tracing-0.1.44"), "tracing-subscriber": Label("@vendor_ts//tracing-subscriber-0.3.23"), diff --git a/shared/tree-sitter-extractor/src/extractor/desugaring.rs b/shared/tree-sitter-extractor/src/extractor/desugaring.rs index c52659750c77..3aa7ad36e897 100644 --- a/shared/tree-sitter-extractor/src/extractor/desugaring.rs +++ b/shared/tree-sitter-extractor/src/extractor/desugaring.rs @@ -63,7 +63,7 @@ impl LanguageExtractor for LanguageSpec { trap_writer: &mut trap::Writer, path: &std::path::Path, source: &[u8], - ) { + ) -> Result<(), String> { crate::extractor::extract_parsed( self.parser.as_ref(), self.prefix, @@ -74,7 +74,7 @@ impl LanguageExtractor for LanguageSpec { path, source, self.desugarer.as_ref(), - ); + ) } } diff --git a/shared/tree-sitter-extractor/src/extractor/driver.rs b/shared/tree-sitter-extractor/src/extractor/driver.rs index d97d8f4c75c2..6e6172b7425b 100644 --- a/shared/tree-sitter-extractor/src/extractor/driver.rs +++ b/shared/tree-sitter-extractor/src/extractor/driver.rs @@ -27,6 +27,9 @@ pub(crate) trait LanguageExtractor: Sync { /// Build the TRAP node-type schema used to validate emitted tuples. fn build_schema(&self) -> std::io::Result; /// Extract a single file's `source` into `trap_writer`. + /// + /// A returned error is logged as a failure of this file only; the driver + /// archives its source, omits its TRAP, and continues. fn extract_file( &self, schema: &NodeTypeMap, @@ -34,7 +37,7 @@ pub(crate) trait LanguageExtractor: Sync { trap_writer: &mut trap::Writer, path: &Path, source: &[u8], - ); + ) -> Result<(), String>; } /// Drive extraction over `languages` for every file listed in `file_lists`. @@ -171,7 +174,7 @@ pub(crate) fn run_extractor( languages_processed[i] = true; let lang = &languages[i]; - lang.extract_file( + let result = lang.extract_file( &schemas[i], &mut diagnostics_writer, &mut trap_writer, @@ -180,7 +183,18 @@ pub(crate) fn run_extractor( ); std::fs::create_dir_all(src_archive_file.parent().unwrap())?; std::fs::copy(&path, &src_archive_file)?; - write_trap(trap_dir, &path, &trap_writer, trap_compression)?; + match result { + Ok(()) => { + write_trap(trap_dir, &path, &trap_writer, trap_compression)?; + } + Err(error) => { + tracing::error!( + file = %path.display(), + error, + "Failed to extract file" + ); + } + } } } } @@ -208,3 +222,82 @@ fn write_trap( std::fs::create_dir_all(trap_file.parent().unwrap())?; trap_writer.write_to_file(&trap_file, trap_compression) } + +#[cfg(test)] +mod tests { + use super::*; + use std::io::Write; + + struct TestLanguage { + file_globs: Vec, + } + + impl LanguageExtractor for TestLanguage { + fn file_globs(&self) -> &[String] { + &self.file_globs + } + + fn build_schema(&self) -> std::io::Result { + Ok(NodeTypeMap::new()) + } + + fn extract_file( + &self, + _schema: &NodeTypeMap, + _diagnostics_writer: &mut diagnostics::LogWriter, + trap_writer: &mut trap::Writer, + _path: &Path, + source: &[u8], + ) -> Result<(), String> { + if source == b"bad" { + Err("invalid parser output".to_string()) + } else { + trap_writer.comment("success".to_string()); + Ok(()) + } + } + } + + #[test] + fn file_extraction_error_does_not_abort_other_files() { + let root = std::env::temp_dir().join(format!("codeql-extractor-{}", rand::random::())); + let source_dir = root.join("input"); + let source_archive_dir = root.join("source-archive"); + let trap_dir = root.join("trap"); + std::fs::create_dir_all(&source_dir).unwrap(); + + let good_path = source_dir.join("good.test"); + let bad_path = source_dir.join("bad.test"); + std::fs::write(&good_path, b"good").unwrap(); + std::fs::write(&bad_path, b"bad").unwrap(); + + let file_list = root.join("files.txt"); + let mut file = std::fs::File::create(&file_list).unwrap(); + writeln!(file, "{}", good_path.display()).unwrap(); + writeln!(file, "{}", bad_path.display()).unwrap(); + + run_extractor( + "test", + &[TestLanguage { + file_globs: vec!["*.test".to_string()], + }], + &trap_dir, + &source_archive_dir, + &[file_list], + &Ok(trap::Compression::Gzip), + ) + .unwrap(); + + let good_trap = file_paths::path_for(&trap_dir, &good_path, "trap.gz", None); + let bad_trap = file_paths::path_for(&trap_dir, &bad_path, "trap.gz", None); + assert!(good_trap.is_file()); + assert!(!bad_trap.exists()); + + let archived_good = file_paths::path_for(&source_archive_dir, &good_path, "", None); + let archived_bad = file_paths::path_for(&source_archive_dir, &bad_path, "", None); + assert!(archived_good.is_file()); + assert!(archived_bad.is_file()); + + std::fs::remove_dir_all(root).unwrap(); + } +} diff --git a/shared/tree-sitter-extractor/src/extractor/mod.rs b/shared/tree-sitter-extractor/src/extractor/mod.rs index 13ee3264133b..e8907a7ae67d 100644 --- a/shared/tree-sitter-extractor/src/extractor/mod.rs +++ b/shared/tree-sitter-extractor/src/extractor/mod.rs @@ -419,7 +419,9 @@ fn collect_extras(node: Node<'_>, source: &[u8], out: &mut Vec) { /// TRAP extraction, and the `extra` tokens (comments and similar, which the /// desugared AST does not carry) are emitted from the side channel. Both /// tree-sitter grammars (via [`tree_sitter_parser`]) and custom parsers plug in -/// here; languages that don't desugar use [`extract`] instead. +/// here; languages that don't desugar use [`extract`] instead. Parse and +/// desugaring errors are returned to the multi-file driver so it can skip only +/// this file and continue extracting the rest. #[allow(clippy::too_many_arguments)] pub fn extract_parsed( parse: &(dyn Fn(&[u8]) -> Result + Send + Sync), @@ -431,7 +433,7 @@ pub fn extract_parsed( path: &Path, source: &[u8], desugarer: &dyn yeast::Desugarer, -) { +) -> Result<(), String> { let path_str = file_paths::normalize_and_transform_path(path, transformer); let source_root = std::env::current_dir() .ok() @@ -441,6 +443,11 @@ pub fn extract_parsed( let _enter = span.enter(); tracing::debug!("extracting: {}", path_str); + let parsed = parse(source).map_err(|e| format!("Parsing failed: {e}"))?; + let ast = desugarer + .run_from_ast(parsed.ast) + .map_err(|e| format!("Desugaring failed: {e}"))?; + trap_writer.comment(format!("Auto-generated TRAP file for {path_str}")); let file_label = populate_file(trap_writer, path, transformer); let mut visitor = Visitor::new( @@ -453,16 +460,13 @@ pub fn extract_parsed( schema, ); - let parsed = parse(source).unwrap_or_else(|e| panic!("Parsing failed for {path_str}: {e}")); - let ast = desugarer - .run_from_ast(parsed.ast) - .unwrap_or_else(|e| panic!("Desugaring failed for {path_str}: {e}")); traverse_yeast(&ast, &mut visitor); // Comments and other `extra` tokens are not part of the desugared AST; emit // them directly from the parser's side channel. for extra in &parsed.extras { visitor.emit_extra(extra); } + Ok(()) } /// A lightweight [`AstNode`] over a piece of side-channel `extra` content diff --git a/shared/tree-sitter-extractor/src/extractor/simple.rs b/shared/tree-sitter-extractor/src/extractor/simple.rs index 1c6691fa8cf3..329c9caddc68 100644 --- a/shared/tree-sitter-extractor/src/extractor/simple.rs +++ b/shared/tree-sitter-extractor/src/extractor/simple.rs @@ -32,7 +32,7 @@ impl LanguageExtractor for LanguageSpec { trap_writer: &mut trap::Writer, path: &std::path::Path, source: &[u8], - ) { + ) -> Result<(), String> { crate::extractor::extract( &self.ts_language, self.prefix, @@ -44,6 +44,7 @@ impl LanguageExtractor for LanguageSpec { source, &[], ); + Ok(()) } } diff --git a/shared/yeast/src/lib.rs b/shared/yeast/src/lib.rs index 2e8af818d7f8..315b4bf4323c 100644 --- a/shared/yeast/src/lib.rs +++ b/shared/yeast/src/lib.rs @@ -1139,7 +1139,7 @@ impl Rule { } } -const MAX_REWRITE_DEPTH: usize = 100; +const MAX_REWRITE_DEPTH: usize = 1000; /// Index of rules by their root query kind for fast lookup. struct RuleIndex<'a, C> { diff --git a/unified/extractor/Cargo.toml b/unified/extractor/Cargo.toml index d4bce75cf7fa..2d801bf5c584 100644 --- a/unified/extractor/Cargo.toml +++ b/unified/extractor/Cargo.toml @@ -14,7 +14,8 @@ rayon = "1.12.0" regex = "1.13.1" encoding = "0.2" lazy_static = "1.5.0" -serde_json = "1.0.151" +serde = "1.0.229" +serde_json = { version = "1.0.151", features = ["unbounded_depth"] } codeql-extractor = { path = "../../shared/tree-sitter-extractor" } yeast = { path = "../../shared/yeast" } diff --git a/unified/extractor/src/languages/swift/adapter.rs b/unified/extractor/src/languages/swift/adapter.rs index f1ae56d6c4c2..60a8db208bcd 100644 --- a/unified/extractor/src/languages/swift/adapter.rs +++ b/unified/extractor/src/languages/swift/adapter.rs @@ -23,6 +23,7 @@ use std::collections::BTreeMap; use codeql_extractor::extractor::ExtraToken; +use serde::Deserialize; use serde_json::Value; use yeast::{Ast, Id, NodeContent, Point, Range}; @@ -57,6 +58,53 @@ const VARYING_TOKEN_KINDS: &[&str] = &[ "unknown", ]; +/// Maximum structural nesting accepted in the serialized JSON tree. +/// +/// This exceeds the deepest tree in the Swift corpus while bounding the +/// recursive serde deserialization and AST construction that follow. +const MAX_JSON_DEPTH: usize = 2048; + +/// Check JSON structural depth without recursively parsing it. +/// +/// Brackets and braces inside strings are ignored. Full JSON validation is +/// still performed by serde_json afterward. +fn check_json_depth(json: &str) -> Result<(), String> { + let mut depth = 0; + let mut in_string = false; + let mut escaped = false; + + for byte in json.bytes() { + if in_string { + if escaped { + escaped = false; + } else { + match byte { + b'\\' => escaped = true, + b'"' => in_string = false, + _ => {} + } + } + } else { + match byte { + b'"' => in_string = true, + b'{' | b'[' => { + depth += 1; + if depth > MAX_JSON_DEPTH { + return Err(format!( + "invalid JSON: nesting depth exceeds supported maximum \ + ({MAX_JSON_DEPTH})" + )); + } + } + b'}' | b']' => depth = depth.saturating_sub(1), + _ => {} + } + } + } + + Ok(()) +} + /// Keys of a node object that carry metadata rather than a structural child. fn is_metadata_key(key: &str) -> bool { matches!( @@ -202,13 +250,26 @@ fn field_entries(node: &Value) -> Vec<(&str, &Value)> { .unwrap_or_default() } -/// The child node objects held by a field value, which is either a single node -/// object or an array of them (an elided collection). +/// The child node objects held by a field value. +/// +/// Collection nodes are elided by the Swift serializer, so nested collections +/// can produce nested arrays (notably inside `UnexpectedNodesSyntax`). Flatten +/// arrays recursively to preserve the intended collection elision. fn children_of(value: &Value) -> Vec<&Value> { - match value { - Value::Array(items) => items.iter().collect(), - other => vec![other], + fn collect<'a>(value: &'a Value, children: &mut Vec<&'a Value>) { + match value { + Value::Array(items) => { + for item in items { + collect(item, children); + } + } + other => children.push(other), + } } + + let mut children = Vec::new(); + collect(value, &mut children); + children } /// Recursively build `node` (and its descendants) into `ast`, returning its id. @@ -312,7 +373,13 @@ const SWIFT_NODE_TYPES: &str = include_str!("../../../swift_node_types.yml"); /// authoritative swift-syntax schema ([`SWIFT_NODE_TYPES`]); the adapter only /// ever consumes swift-syntax input, so the schema is not a parameter. pub fn json_to_ast(json: &str) -> Result { - let root: Value = serde_json::from_str(json).map_err(|e| format!("invalid JSON: {e}"))?; + check_json_depth(json)?; + let mut deserializer = serde_json::Deserializer::from_str(json); + deserializer.disable_recursion_limit(); + let root = Value::deserialize(&mut deserializer).map_err(|e| format!("invalid JSON: {e}"))?; + deserializer + .end() + .map_err(|e| format!("invalid JSON: {e}"))?; let locations = LocationTable::from_root(&root)?; let mut ast = Ast::with_schema(yeast::node_types_yaml::schema_from_yaml(SWIFT_NODE_TYPES)?); @@ -330,6 +397,14 @@ pub fn json_to_ast(json: &str) -> Result { mod tests { use super::*; + fn deeply_nested_json(depth: usize) -> String { + let mut child = r#"{"$pos":0,"$end":0,"kind":"sourceFile"}"#.to_string(); + for _ in 0..depth { + child = format!(r#"{{"$pos":0,"$end":0,"kind":"sourceFile","child":{child}}}"#); + } + format!(r#"{{"$lineStarts":[0],"$pos":0,"$end":0,"kind":"sourceFile","child":{child}}}"#) + } + /// A hand-written JSON tree exercising layout nodes, a named (varying) /// token, a fixed keyword token, and an elided collection field — so the /// adapter is tested without needing the Swift toolchain. @@ -456,6 +531,41 @@ mod tests { assert_eq!(ident.end_position(), Point::new(1, 5)); } + #[test] + fn flattens_nested_elided_collections() { + let json = r#"{ + "$lineStarts": [0], + "$pos": 0, + "$end": 20, + "kind": "sourceFile", + "unexpected": [ + { + "$pos": 0, + "$end": 1, + "kind": "token", + "tokenKind": "leftBrace", + "text": "{" + }, + [ + { + "$pos": 2, + "$end": 20, + "kind": "precedenceGroupAssociativity" + } + ] + ] + }"#; + let ast = json_to_ast(json) + .expect("adapter should flatten nested collections") + .ast; + + assert!( + ast.nodes() + .iter() + .any(|node| node.kind_name() == "precedenceGroupAssociativity") + ); + } + #[test] fn rejects_invalid_line_starts() { let json = r#"{"$lineStarts":[1],"$pos":0,"$end":0,"kind":"sourceFile"}"#; @@ -466,6 +576,31 @@ mod tests { assert!(error.contains("must start with offset 0"), "{error}"); } + #[test] + fn accepts_deeply_nested_json() { + let json = deeply_nested_json(256); + json_to_ast(&json).expect("adapter should accept JSON nested beyond serde_json's default"); + } + + #[test] + fn rejects_json_beyond_supported_depth() { + let json = deeply_nested_json(MAX_JSON_DEPTH); + let error = match json_to_ast(&json) { + Ok(_) => panic!("adapter should reject excessively nested JSON"), + Err(error) => error, + }; + assert!( + error.contains("nesting depth exceeds supported maximum"), + "{error}" + ); + } + + #[test] + fn ignores_brackets_inside_json_strings_when_checking_depth() { + check_json_depth(r#"{"text":"[[[{{{\\\""}"#) + .expect("string contents should not contribute to JSON depth"); + } + #[test] fn collects_extras_into_side_channel() { // A token carrying a trailing line comment in its trivia. diff --git a/unified/swift-syntax-rs/src/lib.rs b/unified/swift-syntax-rs/src/lib.rs index f209780f3eee..ac6414d24342 100644 --- a/unified/swift-syntax-rs/src/lib.rs +++ b/unified/swift-syntax-rs/src/lib.rs @@ -10,15 +10,15 @@ //! by the extractor's own pure-Rust adapter module, keeping the Swift toolchain //! out of the extractor's build. -use std::ffi::{CStr, CString}; +use std::ffi::CStr; use std::os::raw::c_char; // C ABI exported by the `SwiftSyntaxFFI` dynamic library. unsafe extern "C" { - /// Parse a NUL-terminated Swift source string, returning a heap-allocated + /// Parse a UTF-8 Swift source buffer, returning a heap-allocated /// NUL-terminated JSON string (or null on failure). The caller owns the /// returned pointer and must release it with `ssr_string_free`. - fn ssr_parse_json(source: *const c_char) -> *mut c_char; + fn ssr_parse_json(source: *const u8, source_len: usize) -> *mut c_char; /// Free a string previously returned by `ssr_parse_json`. fn ssr_string_free(ptr: *mut c_char); @@ -27,8 +27,6 @@ unsafe extern "C" { /// Errors that can occur while parsing Swift source. #[derive(Debug)] pub enum ParseError { - /// The provided source contained an interior NUL byte. - NulByte, /// The Swift shim returned no result. `SwiftParser` recovers from invalid /// syntax (it always produces a tree, possibly with error nodes), so this /// does *not* indicate a syntax error in the source — it means the shim @@ -39,7 +37,6 @@ pub enum ParseError { impl std::fmt::Display for ParseError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - ParseError::NulByte => write!(f, "source contained an interior NUL byte"), ParseError::SwiftFailure => { write!(f, "the swift-syntax shim failed to produce a JSON result") } @@ -58,13 +55,11 @@ impl std::error::Error for ParseError {} /// println!("{json}"); /// ``` pub fn parse_to_json(source: &str) -> Result { - let c_source = CString::new(source).map_err(|_| ParseError::NulByte)?; - - // SAFETY: `c_source` is a valid NUL-terminated string for the duration of - // the call. The returned pointer, if non-null, is owned by us and freed via - // `ssr_string_free` before returning. + // SAFETY: `source` is valid UTF-8 and its buffer remains alive for the + // duration of the call. The returned pointer, if non-null, is owned by us + // and freed via `ssr_string_free` before returning. unsafe { - let ptr = ssr_parse_json(c_source.as_ptr()); + let ptr = ssr_parse_json(source.as_ptr(), source.len()); if ptr.is_null() { return Err(ParseError::SwiftFailure); } @@ -132,6 +127,15 @@ mod tests { ); } + #[test] + fn parses_source_with_interior_nul() { + let json = parse_to_json("let x =\0 1").expect("parsing interior NUL should succeed"); + assert!( + json.contains(r#"\u0000"#), + "interior NUL should be preserved in the JSON tree: {json}" + ); + } + #[test] fn serializes_json_strings_and_keys_deterministically() { let source = "/* quote \" slash / backslash \\ tab \t newline\n emoji 😀 combining e\u{301} control \u{1} */\nlet x = 1"; diff --git a/unified/swift-syntax-rs/swift/Sources/SwiftSyntaxFFI/SwiftSyntaxFFI.swift b/unified/swift-syntax-rs/swift/Sources/SwiftSyntaxFFI/SwiftSyntaxFFI.swift index 7f6af5898191..daf4e6bfdaae 100644 --- a/unified/swift-syntax-rs/swift/Sources/SwiftSyntaxFFI/SwiftSyntaxFFI.swift +++ b/unified/swift-syntax-rs/swift/Sources/SwiftSyntaxFFI/SwiftSyntaxFFI.swift @@ -292,15 +292,19 @@ private func appendJSON(_ value: Any, to output: inout [UInt8]) throws { } } -/// Parse the given NUL-terminated Swift source string and return a -/// heap-allocated, NUL-terminated JSON representation of the syntax tree. +/// Parse the given UTF-8 Swift source buffer and return a heap-allocated, +/// NUL-terminated JSON representation of the syntax tree. /// /// The returned pointer is owned by the caller and MUST be released with /// `ssr_string_free`. Returns `nil` on failure. @_cdecl("ssr_parse_json") -public func ssr_parse_json(_ source: UnsafePointer?) -> UnsafeMutablePointer? { - guard let source = source else { return nil } - let code = String(cString: source) +public func ssr_parse_json( + _ source: UnsafePointer?, + _ sourceLength: Int +) -> UnsafeMutablePointer? { + guard sourceLength >= 0, source != nil || sourceLength == 0 else { return nil } + let sourceBytes = UnsafeBufferPointer(start: source, count: sourceLength) + let code = String(decoding: sourceBytes, as: UTF8.self) let tree = Parser.parse(source: code) // Fold operator sequences before serializing. Source positions are // preserved by folding (the same tokens, in the same places), so a