diff --git a/src/dirimage/layout.rs b/src/dirimage/layout.rs index 4348be5..ff4974b 100644 --- a/src/dirimage/layout.rs +++ b/src/dirimage/layout.rs @@ -57,7 +57,15 @@ const MAX_CS0_NAME_BYTES: usize = 254; /// A directory File Entry's link count is `u16` and counts one per child /// directory plus one for its own entry in the parent, so the last usable /// value is `u16::MAX - 1`. +/// Lowered under `cfg(test)` ONLY so the guard can actually be executed. +/// Building a folder with 65534 subdirectories to reach the real cap is not a +/// test anyone can run, so the previous test asserted arithmetic about the +/// constant instead and would have passed with the guard deleted. The +/// production value is unchanged. +#[cfg(not(test))] const MAX_SUBDIRS: usize = (u16::MAX - 1) as usize; +#[cfg(test)] +const MAX_SUBDIRS: usize = 4; /// Largest image this planner will synthesize, in sectors (128 GiB). /// @@ -859,16 +867,6 @@ mod tests { assert!(!is_excluded("VTS_01_1.VOB")); } - /// A name too long for the FID's one-byte length field is refused by the - /// PLANNER, on a real folder. - /// - /// Audit finding: the length was narrowed with `as u8`, so a 255-byte ASCII - /// name — POSIX NAME_MAX, legal on ext4/APFS/NTFS — encoded to 256 bytes - /// with the CS0 compression byte and wrote a length of ZERO, making every - /// later entry in that directory read from the wrong offset. - /// - /// An earlier version of this test asserted arithmetic about the constants - /// and never called `plan`, so it would have passed with the guard deleted. /// Two host names that the READER collapses into one must be refused by /// the planner, not written into the image. /// @@ -912,6 +910,16 @@ mod tests { ); } + /// A name too long for the FID's one-byte length field is refused by the + /// PLANNER, on a real folder. + /// + /// Audit finding: the length was narrowed with `as u8`, so a 255-byte ASCII + /// name — POSIX NAME_MAX, legal on ext4/APFS/NTFS — encoded to 256 bytes + /// with the CS0 compression byte and wrote a length of ZERO, making every + /// later entry in that directory read from the wrong offset. + /// + /// An earlier version of this test asserted arithmetic about the constants + /// and never called `plan`, so it would have passed with the guard deleted. #[test] fn an_over_long_name_is_refused_by_the_planner() { let dir = std::env::temp_dir().join(format!( @@ -971,11 +979,30 @@ mod tests { /// so this pins the arithmetic relationship the guard relies on — and says /// plainly that it does NOT exercise `walk`. #[test] - fn the_subdir_cap_keeps_the_link_count_representable() { - assert_eq!( - (MAX_SUBDIRS as u16).checked_add(1), - Some(u16::MAX), - "the largest permitted fan-out must still fit the link count" + fn the_subdir_cap_refuses_a_folder_with_too_many_subdirectories() { + // Executes the guard, rather than restating the constant. A directory's + // File Entry records its link count in 16 bits — one per child + // directory plus one for its own entry in the parent — so exceeding it + // would wrap the count and produce an image whose directory structure + // lies about itself. + let dir = std::env::temp_dir().join(format!("fmkv-fanout-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + for i in 0..=MAX_SUBDIRS { + std::fs::create_dir_all(dir.join(format!("d{i}"))).unwrap(); + } + + let err = plan(&dir).expect_err("more subdirectories than the link count can represent"); + assert!( + matches!(err, Error::DirImageFanout { .. }), + "expected DirImageFanout, got {err:?}" ); + + // And the real production value is what ships: one per child plus the + // parent's own entry must still fit in the 16-bit field. + #[cfg(not(test))] + const _: () = assert!(MAX_SUBDIRS + 1 == u16::MAX as usize); + + let _ = std::fs::remove_dir_all(&dir); } } diff --git a/src/ifo.rs b/src/ifo.rs index 7930b21..6de477a 100644 --- a/src/ifo.rs +++ b/src/ifo.rs @@ -2746,11 +2746,25 @@ mod tests { /// unclamped 65535 is a ~540 MB allocation from a ~800 KB crafted file. #[test] fn tt_srpt_clamps_an_absurd_declared_title_count() { - let mut vmg = vmg_with_tt_srpt(1, &[(1, 1, 1)]); + // The fixture must actually CONTAIN more entries than the cap, or the + // walk stops when the buffer runs out and the clamp is never what + // bounded it. An earlier version of this test declared u16::MAX over a + // two-kilobyte fixture: the loop broke on `base + 12 > len` after three + // iterations and the assertion held with the clamp deleted entirely. + const PRESENT: usize = MAX_TT_SRPT_TITLES + 40; + let entries: Vec<(u16, u8, u8)> = (0..PRESENT) + .map(|i| (1u16, 1u8, (i % 250 + 1) as u8)) + .collect(); + let mut vmg = vmg_with_tt_srpt(1, &entries); let off = crate::consts::SECTOR_BYTES; vmg[off..off + 2].copy_from_slice(&u16::MAX.to_be_bytes()); + let map = parse_tt_srpt(&vmg, off).expect("parse"); let total: usize = map.values().map(|v| v.len()).sum(); - assert!(total <= 99, "clamped to the format maximum, got {total}"); + assert!( + total <= MAX_TT_SRPT_TITLES, + "a declared count of u16::MAX over {PRESENT} real entries must be \ + clamped to {MAX_TT_SRPT_TITLES}, got {total}" + ); } } diff --git a/src/mux/codec/mod.rs b/src/mux/codec/mod.rs index 5c1502f..abde746 100644 --- a/src/mux/codec/mod.rs +++ b/src/mux/codec/mod.rs @@ -465,9 +465,23 @@ mod provenance_guard { // `PesFrame` in the tests of other modules is not ours; only // codec `Frame` literals are scanned, and a test fixture that // builds a PesPacket with `source: None` is legitimate. - if blk.contains("source: None") { + // Two spellings, not one. `source: None` is the obvious way to + // lose provenance; OMITTING the field entirely is the quiet + // one, because `Frame` derives Default, so + // `Frame { pts_ns, .. Default::default() }` compiles and + // yields `source: None` while containing no such text. A guard + // that only knew the first spelling would have watched a + // parser be rewritten into the second and stayed green. + let explicit_none = blk.contains("source: None"); + let no_source_field = !blk.contains("source:"); + if explicit_none || no_source_field { let line = src[..src.find(blk).unwrap_or(0)].lines().count() + 1; - offenders.push(format!("{name}:{line}")); + let how = if explicit_none { + "source: None" + } else { + "no source field (Default fills in None)" + }; + offenders.push(format!("{name}:{line} ({how})")); } } }