diff --git a/CHANGELOG.md b/CHANGELOG.md index 9261995..4dacbb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,13 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). partial set. See #69. ### Changed +- **`debug-files upload --type il2cpp-linemap` validates `LineNumberMappings.json` before packing + it.** It is checked as a stream (these files run to tens of MB) against the shape the + symbolicator reads, `{cpp_path: {cs_path: {cpp_line: cs_line}}}` with non-negative integer line + numbers. A truncated, wrong or otherwise corrupt file now exits 11 naming the file and the + problem, in a dry run too, and uploads nothing; it used to upload "successfully" and leave every + IL2CPP crash unsymbolicated. The optional top-level `__debug-id__` sentinel is ignored, as the symbolicator ignores it. An empty map is still accepted (with a warning). Integrators that + treated this command's success as proof of a usable map can now rely on it. - **`--type elf` collects every input before uploading anything.** A corrupt second zip now aborts (exit 11) before the first zip uploads, instead of after. A path that does not exist now exits 10 (input not found) rather than 11; both are in the no-fallback range. diff --git a/README.md b/README.md index 2c36897..ae02a61 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ Takes one or more paths: AGP's pre-built `native-debug-symbols.zip`, and/or dire #### Unity IL2CPP — `--type il2cpp-linemap` -Uploads `LineNumberMappings.json` (+ sibling `MethodMap.tsv` / `il2cppFileRoot.txt`) as format `il2cpp-linemap`, keyed by the IL2CPP module UUID(s) (`libil2cpp` / `UnityFramework`). See [`docs/unity-il2cpp-linenumber-mappings.md`](docs/unity-il2cpp-linenumber-mappings.md). +Uploads `LineNumberMappings.json` (+ sibling `MethodMap.tsv` / `il2cppFileRoot.txt`) as format `il2cpp-linemap`, keyed by the IL2CPP module UUID(s) (`libil2cpp` / `UnityFramework`). The mappings JSON is validated first (`{cpp_path: {cs_path: {cpp_line: cs_line}}}`, integer line numbers, checked as a stream): a truncated or wrong file exits 11 naming the file and uploads nothing. See [`docs/unity-il2cpp-linenumber-mappings.md`](docs/unity-il2cpp-linenumber-mappings.md). ```sh bugsee-cli debug-files upload path/to/Symbols/LineNumberMappings.json \ diff --git a/src/cli/debug_files.rs b/src/cli/debug_files.rs index f7da7df..068ca0c 100644 --- a/src/cli/debug_files.rs +++ b/src/cli/debug_files.rs @@ -222,7 +222,9 @@ pub enum DebugFileType { /// Unity IL2CPP `LineNumberMappings.json` (+ MethodMap / il2cppFileRoot) /// bundle. Keyed by the IL2CPP module UUID(s) (`libil2cpp` / UnityFramework); /// pass `--uuid` (comma-separated and/or repeated) and/or `--il2cpp-uuid` - /// for multi-ABI. + /// for multi-ABI. The mappings JSON is validated before anything is packed + /// (`{cpp_path: {cs_path: {cpp_line: cs_line}}}` with integer line numbers); + /// a corrupt or wrong file exits 11 and uploads nothing. Il2cppLinemap, } @@ -1437,6 +1439,14 @@ async fn run_il2cpp_linemap_upload( let mut already_existed = 0u32; for bundle in &bundles { let _ = il2cpp_linemap::read_file_root(bundle, il2cpp_root)?; + // Corrupt mappings must fail HERE, before anything is packed or sent. + let mappings = il2cpp_linemap::validate_mappings(&bundle.json_path)?; + tracing::info!( + cpp_files = mappings.cpp_files, + cs_files = mappings.cs_files, + lines = mappings.lines, + "validated the line-number map" + ); tracing::info!( path = %bundle.json_path.display(), uuids = ?uuids, diff --git a/src/symbols/il2cpp_linemap.rs b/src/symbols/il2cpp_linemap.rs index 351a2cb..9d1fef2 100644 --- a/src/symbols/il2cpp_linemap.rs +++ b/src/symbols/il2cpp_linemap.rs @@ -9,15 +9,19 @@ use std::fs; use std::path::{Path, PathBuf}; +use struson::reader::{JsonReader, JsonStreamReader, ReaderError, ValueType}; use walkdir::WalkDir; use super::suffix::ExtraSuffixes; -use crate::error::{config_invalid, input_not_found}; +use crate::error::{config_invalid, input_invalid, input_not_found, Error}; pub const LINE_NUMBER_MAPPINGS: &str = "LineNumberMappings.json"; pub const METHOD_MAP: &str = "MethodMap.tsv"; pub const FILE_ROOT: &str = "il2cppFileRoot.txt"; pub const MANIFEST: &str = "manifest.json"; +/// Optional top-level key the mappings document may carry; parsers ignore it +/// (docs/unity-il2cpp-linenumber-mappings.md, section 2.1). +const DEBUG_ID_SENTINEL: &str = "__debug-id__"; /// A discovered IL2CPP line-map directory (or a direct path to the JSON). #[derive(Debug, Clone)] @@ -88,6 +92,129 @@ fn bundle_from_json(json_path: &Path) -> Option { }) } +/// What a validated `LineNumberMappings.json` holds. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct MappingsStats { + /// Generated C++ files (top-level keys). + pub cpp_files: usize, + /// (C++ file, C# file) pairs. + pub cs_files: usize, + /// `cpp_line -> cs_line` entries. + pub lines: usize, +} + +/// Check that `path` is the mappings document the symbolicator reads, WITHOUT holding it in +/// memory (these files run to tens of MB): `{ cpp_path: { cs_path: { cpp_line: cs_line } } }`, +/// where every line number is a non-negative integer that fits a `u32`. +/// +/// The CLI packs this file as-is, so without a check a truncated or wrong file (an interrupted +/// Unity build, the wrong artefact, a merge-conflicted checkout) uploads "successfully" and +/// every IL2CPP crash then fails to symbolicate with nothing pointing back at the upload. +/// A document with no entries at all is accepted (a project can have none) but flagged. +/// +/// Malformed content is `InputInvalid` (exit 11), naming the file and the JSON path of the +/// problem; an I/O failure reading it stays an I/O error. +pub fn validate_mappings(path: &Path) -> anyhow::Result { + let file = fs::File::open(path).map_err(Error::Io)?; + let bad = |detail: &str| { + // The reader's own message can echo the offending token (a 50 000-digit number): + // cap what reaches stderr. + let detail: String = detail.chars().take(240).collect(); + input_invalid(format!( + "{} is not a valid IL2CPP line-number map ({detail}); expected \ + {{cpp_path: {{cs_path: {{cpp_line: cs_line}}}}}} with integer line numbers: {}", + LINE_NUMBER_MAPPINGS, + path.display() + )) + }; + let from_reader = |e: ReaderError| match e { + // Invalid UTF-8 is damaged content, like any other malformed byte; only a real I/O + // failure is an I/O error. + ReaderError::IoError { error, .. } if error.kind() != std::io::ErrorKind::InvalidData => { + anyhow::Error::from(Error::Io(error)) + } + other => bad(&other.to_string()), + }; + // The reader's default number guard (no exponent beyond +-99, no token over 100 + // characters) stays ON: a line number is at most 10 digits, so anything the guard + // refuses is invalid here anyway, and it stops a huge number from being buffered. + let mut r = JsonStreamReader::new(std::io::BufReader::new(file)); + let mut stats = MappingsStats { + cpp_files: 0, + cs_files: 0, + lines: 0, + }; + let expect = |r: &mut JsonStreamReader<_>, want: ValueType, what: &str| -> anyhow::Result<()> { + let got = r.peek().map_err(from_reader)?; + if got == want { + Ok(()) + } else { + Err(bad(&format!("{what} must be {want}, found {got}"))) + } + }; + + expect(&mut r, ValueType::Object, "the top-level value")?; + r.begin_object().map_err(from_reader)?; + while r.has_next().map_err(from_reader)? { + let cpp = r.next_name().map_err(from_reader)?.to_owned(); + if cpp == DEBUG_ID_SENTINEL { + // The documented optional sentinel (the build's debug id): parsers ignore it, + // whatever it holds. Still read through, so syntax errors in it are caught. + r.skip_value().map_err(from_reader)?; + continue; + } + expect( + &mut r, + ValueType::Object, + &format!("the entry for \"{cpp}\""), + )?; + r.begin_object().map_err(from_reader)?; + stats.cpp_files += 1; + while r.has_next().map_err(from_reader)? { + let cs = r.next_name().map_err(from_reader)?.to_owned(); + expect( + &mut r, + ValueType::Object, + &format!("the entry for \"{cpp}\" -> \"{cs}\""), + )?; + r.begin_object().map_err(from_reader)?; + stats.cs_files += 1; + while r.has_next().map_err(from_reader)? { + let line = r.next_name().map_err(from_reader)?.to_owned(); + if line.is_empty() + || !line.bytes().all(|b| b.is_ascii_digit()) + || line.parse::().is_err() + { + return Err(bad(&format!( + "\"{line}\" under \"{cpp}\" is not a C++ line number" + ))); + } + expect( + &mut r, + ValueType::Number, + &format!("the C# line for {cpp}:{line}"), + )?; + let n = r.next_number_as_string().map_err(from_reader)?; + if n.parse::().is_err() { + let shown: String = n.chars().take(32).collect(); + return Err(bad(&format!( + "{cpp}:{line} maps to {shown}, not a non-negative integer line" + ))); + } + stats.lines += 1; + } + r.end_object().map_err(from_reader)?; + } + r.end_object().map_err(from_reader)?; + } + r.end_object().map_err(from_reader)?; + r.consume_trailing_whitespace().map_err(from_reader)?; + if stats.lines == 0 { + tracing::warn!(path = %path.display(), "the line-number map has no entries"); + } + Ok(stats) +} + /// Parse `--uuid` values: comma-separated and/or repeated spellings. pub fn parse_uuids(raw: &[String]) -> anyhow::Result> { let mut out = Vec::new(); @@ -211,6 +338,262 @@ mod tests { assert!(found[0].method_map.is_some(), "siblings resolve next to it"); } + // ── validate_mappings ────────────────────────────────────────── + + fn fixture(platform: &str) -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/il2cpp-linemap") + .join(platform) + .join(LINE_NUMBER_MAPPINGS) + } + + fn write_json(bytes: &[u8]) -> (tempfile::TempDir, PathBuf) { + let dir = tempfile::tempdir().unwrap(); + let p = dir.path().join(LINE_NUMBER_MAPPINGS); + fs::write(&p, bytes).unwrap(); + (dir, p) + } + + fn invalid(p: &Path) -> String { + match validate_mappings(p).unwrap_err().downcast::() { + Ok(Error::InputInvalid(m)) => m, + other => panic!("expected InputInvalid, got {other:?}"), + } + } + + #[test] + fn the_committed_fixtures_are_valid() { + for platform in ["android", "ios"] { + let stats = validate_mappings(&fixture(platform)).unwrap(); + assert!( + stats.cpp_files >= 1 && stats.cs_files >= 1 && stats.lines >= 1, + "{stats:?}" + ); + } + let stats = validate_mappings(&fixture("android")).unwrap(); + assert_eq!(stats.cpp_files, 2); + assert_eq!(stats.cs_files, 3); + assert_eq!(stats.lines, 7); + } + + /// The optional `__debug-id__` sentinel is ignored whatever it holds; it is not an entry. + #[test] + fn the_debug_id_sentinel_is_ignored_but_still_syntax_checked() { + for sentinel in [r#""3f2a-uuid""#, "7", "null", r#"{"x":[1,2]}"#] { + let doc = format!(r#"{{"__debug-id__":{sentinel},"a.cpp":{{"A.cs":{{"1":2}}}}}}"#); + let (_d, p) = write_json(doc.as_bytes()); + assert_eq!( + validate_mappings(&p).unwrap(), + MappingsStats { + cpp_files: 1, + cs_files: 1, + lines: 1 + }, + "{sentinel}" + ); + } + let (_d, p) = write_json(br#"{"__debug-id__":"unterminated,"a.cpp":{}}"#); + assert!(validate_mappings(&p).is_err()); + // Only that exact key is special: another scalar entry is still a shape error. + let (_d, p) = write_json(br#"{"__other__":"x","a.cpp":{"A.cs":{"1":2}}}"#); + assert!(invalid(&p).contains("__other__")); + } + + #[test] + fn an_empty_object_is_accepted_and_counts_nothing() { + let (_d, p) = write_json(b"{}"); + assert_eq!( + validate_mappings(&p).unwrap(), + MappingsStats { + cpp_files: 0, + cs_files: 0, + lines: 0 + } + ); + let (_d, p) = write_json(br#"{"a.cpp":{"A.cs":{}}}"#); + assert_eq!(validate_mappings(&p).unwrap().cs_files, 1); + } + + /// Everything that is not the documented shape is `InputInvalid`, with a message that + /// names the file and the problem. + #[test] + fn malformed_or_misshapen_documents_are_rejected() { + let good = br#"{"a.cpp":{"A.cs":{"1":2}}}"#; + let cases: Vec<(&str, Vec, &str)> = vec![ + ("empty file", Vec::new(), "LineNumberMappings.json"), + ( + "whitespace only", + b" \n".to_vec(), + "LineNumberMappings.json", + ), + ( + "truncated", + good[..good.len() - 3].to_vec(), + "LineNumberMappings.json", + ), + ( + "trailing junk", + [&good[..], b" x"].concat(), + "LineNumberMappings.json", + ), + ( + "two documents", + [&good[..], &good[..]].concat(), + "LineNumberMappings.json", + ), + ( + "byte-order mark", + [&b"\xef\xbb\xbf"[..], &good[..]].concat(), + "LineNumberMappings.json", + ), + ( + "binary", + vec![0xff, 0xfe, 0x00, 0x01], + "LineNumberMappings.json", + ), + ("an array", b"[]".to_vec(), "top-level value must be"), + ("a string", br#""x""#.to_vec(), "top-level value must be"), + ("cpp entry is a number", br#"{"a.cpp":3}"#.to_vec(), "a.cpp"), + ( + "cs entry is an array", + br#"{"a.cpp":{"A.cs":[1]}}"#.to_vec(), + "A.cs", + ), + ( + "one level short", + br#"{"a.cpp":{"1":2}}"#.to_vec(), + "must be Object", + ), + ( + "string line value", + br#"{"a.cpp":{"A.cs":{"1":"2"}}}"#.to_vec(), + "C# line", + ), + ( + "null line value", + br#"{"a.cpp":{"A.cs":{"1":null}}}"#.to_vec(), + "C# line", + ), + ( + "non-numeric cpp line", + br#"{"a.cpp":{"A.cs":{"one":2}}}"#.to_vec(), + "not a C++ line", + ), + ( + "empty cpp line key", + br#"{"a.cpp":{"A.cs":{"":2}}}"#.to_vec(), + "not a C++ line", + ), + ( + "negative cpp line", + br#"{"a.cpp":{"A.cs":{"-1":2}}}"#.to_vec(), + "not a C++ line", + ), + ( + "cpp line over u32", + br#"{"a.cpp":{"A.cs":{"4294967296":2}}}"#.to_vec(), + "not a C++ line", + ), + ( + "negative cs line", + br#"{"a.cpp":{"A.cs":{"1":-2}}}"#.to_vec(), + "non-negative integer", + ), + ( + "fractional cs line", + br#"{"a.cpp":{"A.cs":{"1":2.5}}}"#.to_vec(), + "non-negative integer", + ), + ( + "exponent cs line", + br#"{"a.cpp":{"A.cs":{"1":1e3}}}"#.to_vec(), + "non-negative integer", + ), + ( + "cs line over u32", + br#"{"a.cpp":{"A.cs":{"1":4294967296}}}"#.to_vec(), + "non-negative integer", + ), + ( + "deeply nested", + format!("{}1{}", r#"{"a":"#.repeat(50_000), "}".repeat(50_000)).into_bytes(), + "LineNumberMappings.json", + ), + ]; + for (label, bytes, needle) in cases { + let (_d, p) = write_json(&bytes); + let msg = invalid(&p); + assert!(msg.contains(needle), "{label}: {msg}"); + assert!( + msg.contains(&p.display().to_string()), + "{label}: names the file: {msg}" + ); + } + } + + /// No prefix of a real document is itself a valid one, and none crashes the check. + #[test] + fn every_truncation_of_a_real_map_is_rejected() { + let whole = fs::read(fixture("ios")).unwrap(); + validate_mappings(&fixture("ios")).unwrap(); + let dir = tempfile::tempdir().unwrap(); + let p = dir.path().join(LINE_NUMBER_MAPPINGS); + let trimmed = whole.trim_ascii_end(); + for cut in 0..trimmed.len() { + fs::write(&p, &trimmed[..cut]).unwrap(); + assert!( + validate_mappings(&p).is_err(), + "a {cut}-byte prefix was accepted" + ); + } + } + + /// A leaf "number" of thousands of digits (a wrong file or a partial write) is refused by + /// the reader's number guard before it is buffered, and the message stays short. + #[test] + fn a_huge_number_is_rejected_cheaply_with_a_short_message() { + for body in ["9".repeat(50_000), format!("1e{}", "9".repeat(40))] { + let doc = format!(r#"{{"a.cpp":{{"A.cs":{{"1":{body}}}}}}}"#); + let (_d, p) = write_json(doc.as_bytes()); + let msg = invalid(&p); + assert!( + msg.len() < 1_000, + "the error must not echo the number: {} bytes", + msg.len() + ); + } + } + + /// A map of tens of MB is checked as a stream: this one has 300 000 entries. + #[test] + fn a_large_map_validates() { + let mut doc = String::from("{"); + for f in 0..30 { + if f > 0 { + doc.push(','); + } + doc.push_str(&format!(r#""f{f}.cpp":{{"F{f}.cs":{{"#)); + for l in 0..10_000 { + if l > 0 { + doc.push(','); + } + doc.push_str(&format!(r#""{l}":{}"#, l / 2)); + } + doc.push_str("}}"); + } + doc.push('}'); + let (_d, p) = write_json(doc.as_bytes()); + let stats = validate_mappings(&p).unwrap(); + assert_eq!((stats.cpp_files, stats.lines), (30, 300_000)); + } + + #[test] + fn a_missing_file_is_an_io_error_not_invalid_content() { + let dir = tempfile::tempdir().unwrap(); + let err = validate_mappings(&dir.path().join("nope.json")).unwrap_err(); + assert!(matches!(err.downcast::(), Ok(Error::Io(_)))); + } + #[test] fn parse_uuids_comma_and_multi() { let got = parse_uuids(&["aaa,bbb".to_string(), "ccc".to_string()]).unwrap(); diff --git a/tests/broken_files.rs b/tests/broken_files.rs index 2787dbd..401c5a4 100644 --- a/tests/broken_files.rs +++ b/tests/broken_files.rs @@ -861,41 +861,99 @@ fn unusable_build_artifacts_are_input_not_found() { } // --------------------------------------------------------------------------- -// IL2CPP line maps: a documented gap (characterisation) +// IL2CPP line maps // --------------------------------------------------------------------------- -/// The IL2CPP mappings JSON is packed as-is and NOT validated: a corrupt file uploads -/// "successfully". This pins that on purpose — rejecting it would be a behaviour -/// change integrators must be told about — so a future validation lands deliberately. -#[test] -fn il2cpp_linemap_does_not_validate_the_mappings_json_today() { +fn il2cpp_args(extra: &[&str]) -> Vec { + let mut a: Vec = [ + "debug-files", + "upload", + "--type", + "il2cpp-linemap", + "--version", + "1", + "--build", + "1", + "--uuid", + "11111111-2222-3333-4444-555555555555", + ] + .iter() + .map(|s| s.to_string()) + .collect(); + a.extend(extra.iter().map(|s| s.to_string())); + a +} + +/// The mappings JSON is validated before anything is packed: a corrupt or wrong file is +/// `InputInvalid` (11) naming the file, in a dry run and for real, and nothing is sent. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn il2cpp_linemap_rejects_a_corrupt_mappings_json_and_sends_nothing() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({"code": 0}))) + .expect(0) + .mount(&server) + .await; + Mock::given(method("PUT")) + .respond_with(ResponseTemplate::new(200)) + .expect(0) + .mount(&server) + .await; let tmp = tempfile::tempdir().unwrap(); - for (label, bytes) in [ - ("empty", &b""[..]), - ("truncated", br#"{"a":"#), - ("binary", &[0xff, 0xfe, 0x00]), - ] { - let dir = tmp.path().join(label); - write(&dir, "LineNumberMappings.json", bytes); + let endpoint = server.uri(); + let root = tmp.path().to_path_buf(); + tokio::task::spawn_blocking(move || { + for (label, bytes) in [ + ("empty", &b""[..]), + ("truncated", br#"{"a.cpp":{"A.cs":{"1":"#), + ("binary", &[0xff, 0xfe, 0x00]), + ("wrong shape", br#"{"a.cpp":{"1":2}}"#), + ("string line", br#"{"a.cpp":{"A.cs":{"1":"2"}}}"#), + ("an array", b"[]"), + ] { + let dir = root.join(label.replace(' ', "_")); + let p = write(&dir, "LineNumberMappings.json", bytes); + for dry in [true, false] { + let mut extra = vec![dir.to_str().unwrap()]; + if dry { + extra.push("--dry-run"); + } + let out = common::cli() + .args(["--endpoint", &endpoint, "--app-token", "TKN"]) + .args(il2cpp_args(&extra)) + .output() + .unwrap(); + assert_eq!(code_of(&out), 11, "{label} (dry={dry}): {}", stderr(&out)); + assert!( + stderr(&out).contains("LineNumberMappings.json") + && stderr(&out).contains(p.to_str().unwrap()), + "{label}: names the file: {}", + stderr(&out) + ); + assert!(out.stdout.is_empty(), "{label}"); + } + } + }) + .await + .unwrap(); +} + +/// The shipped fixtures are valid and still upload (the check does not get in the way). +#[test] +fn il2cpp_linemap_accepts_a_valid_map() { + for platform in ["android", "ios"] { + let dir = Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures/il2cpp-linemap") + .join(platform); let out = common::cli() - .args([ - "debug-files", - "upload", - "--type", - "il2cpp-linemap", - "--version", - "1", - "--build", - "1", - ]) - .args([ - "--uuid", - "11111111-2222-3333-4444-555555555555", - "--dry-run", - ]) - .arg(&dir) + .args(il2cpp_args(&[dir.to_str().unwrap(), "--dry-run"])) .output() .unwrap(); - assert_eq!(code_of(&out), 0, "{label}: {}", stderr(&out)); + assert_eq!(code_of(&out), 0, "{platform}: {}", stderr(&out)); + assert!( + stderr(&out).contains("validated the line-number map"), + "{}", + stderr(&out) + ); } }