From 68a1a559583b2cfea2c9b607d1f8695cd7bf2a20 Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Sun, 16 Aug 2026 13:22:24 -0700 Subject: [PATCH] Audit round 4-6: disc parsing, extents, codecs and drive faults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Squashed from 12 commits. Every fix was proven red-before-green and killed by a mutation; the reasoning for each is in the private audit record. UDF and extents Honour ICB types rather than assuming a Short AD, so an AD-type-3 directory is no longer decoded from FID bytes into a silently empty listing. Carry the ECMA-167 recorded flag through to the resolvers: an allocated-but-never- written extent used to reach the read plan as ordinary content and splice undefined sectors into the rip. file_extents now refuses such a file, and only when the hole actually occupies byte space — a zero-length one displaces nothing, and refusing on it dropped whole titles off discs that ripped correctly. Type-2 sparse extents are kept alongside type-1; they were falling into a catch-all that exited the descriptor loop and returned a truncated list as complete. merge_ranges no longer claims a sector neither input covered. A short skip or an over-long AD chain errors instead of truncating. HD-DVD and Blu-ray scanning Bound the XPL nesting depth, title count, clips and chapters per title, and memoize the clip-name fallback probe — four separate amplification axes, each of which alone left the worst case unbounded. The clip and title caps are 512, ~10x any retail disc, and a test pins the product of cap and probe budget. The scan is cancellable: it returned Ok with titles carrying no streams when halted, presenting a cancelled scan as a successful one. A clip dropped for an unrecorded extent now says so. Codecs and muxing Resume a held E-AC-3 access unit rather than rescanning from its first frame, and drop it on a discontinuity — a stale hold indexed past the end of the new buffer. Map every ISO 639-1 code instead of collapsing fifteen languages to und. Correct the DVD palette order. Detect a skip past EOF. Drive and I/O Classify dead-bus faults so the wedged-drive path can see them; a catch-all arm had been flattening the variants before the classifier ran. A prefetch producer that dies now reports SourceTerminated instead of Ok(0), which the reader legitimately read as a short read and zero-filled — a whole title could be fabricated and the pass reported complete. Also: charge Ok(0) reads to the CSS crack budget, drop the unreachable soft re-crack, and send disc-derived strings to logs through the debug formatter so a crafted label cannot paint an operator's terminal. --- src/css/mod.rs | 145 +++- src/decrypt.rs | 15 +- src/disc/bluray.rs | 118 ++- src/disc/dvd.rs | 6 +- src/disc/extract.rs | 251 ++++++- src/disc/hddvd.rs | 1478 ++++++++++++++++++++++++++++++++++++-- src/disc/mod.rs | 195 ++++- src/drive/mod.rs | 95 +++ src/error.rs | 96 ++- src/ifo.rs | 154 ++-- src/labels/deluxe.rs | 16 +- src/labels/mod.rs | 219 +++++- src/labels/vocab.rs | 326 +++++++++ src/mux/codec/ac3.rs | 248 ++++++- src/mux/codec/crc.rs | 24 +- src/mux/codec/dvdsub.rs | 71 +- src/mux/disc.rs | 148 ++++ src/mux/mkv.rs | 299 +++++++- src/mux/mkvstream.rs | 85 ++- src/mux/ps.rs | 153 +++- src/mux/resolve.rs | 7 +- src/sector/prefetched.rs | 209 +++++- src/udf.rs | 810 +++++++++++++++++---- 23 files changed, 4800 insertions(+), 368 deletions(-) diff --git a/src/css/mod.rs b/src/css/mod.rs index 20adb9f..f4922fc 100644 --- a/src/css/mod.rs +++ b/src/css/mod.rs @@ -75,7 +75,7 @@ pub fn crack_key( extents: &[Extent], batch_sectors: u16, ) -> Option { - crack_key_scan(reader, extents, batch_sectors, None, false).into_state() + crack_key_scan(reader, extents, batch_sectors, None).into_state() } /// Outcome of a CSS crack scan that distinguishes the THREE cases the bare @@ -136,7 +136,7 @@ pub fn crack_key_outcome( batch_sectors: u16, halt: Option<&crate::halt::Halt>, ) -> CrackOutcome { - crack_key_scan(reader, extents, batch_sectors, halt, true) + crack_key_scan(reader, extents, batch_sectors, halt) } /// Resolve a DVD title's CSS descramble key from the reader when the caller @@ -212,10 +212,6 @@ fn crack_key_scan( extents: &[Extent], batch_sectors: u16, halt: Option<&crate::halt::Halt>, - // True only on the INITIAL scan: a fully CSS-locked (`05/6F/03`) result is a - // hard `ScrambledUncracked`. False on the per-VTS re-crack so a lapsed-AGID - // locked read returns None instead of killing a genuinely crackable title. - fail_on_locked: bool, ) -> CrackOutcome { // Batch the reads: a live optical drive at 1 sector/read is glacial, and the // crack only needs to FIND one scrambled sector whose 0x80 plaintext matches @@ -301,6 +297,18 @@ fn crack_key_scan( let usable = (got / 2048).min(n as usize); // At least one, so a source returning Ok(0) cannot spin here. advance = (usable as u32).max(1); + if usable == 0 { + // Nothing was inspected, so the per-sector `tried` + // charge below never runs — but the cursor still moves + // one sector (the `.max(1)` above). Charge that sector + // to the budget, or `tried` stays frozen and the loop + // is bounded only by the disc-declared + // `ext.sector_count`: the anti-grind budget stops + // applying to exactly the misbehaving source it exists + // for. Mirrors the `Err` arm's `tried += n`, which + // likewise charges an uninspected advance. + tried += 1; + } for s in 0..usable { tried += 1; let sect = &buf[s * 2048..(s + 1) * 2048]; @@ -344,14 +352,29 @@ fn crack_key_scan( } // Budget exhausted / extents walked / early-bailed with no key recovered. - // The disc is ENCRYPTED-but-uncracked (a hard failure on the initial scan) - // when EITHER a scrambled sector was actually seen, OR — on the initial scan - // only (`fail_on_locked`) — every read was CSS-locked (`05/6F/03`), itself - // proof of scrambling. A re-crack (`fail_on_locked` false) stays soft: a - // lapsed-AGID locked read yields None, not a hard fail, so a crackable title - // in another VTS isn't killed. Only a scan that saw neither a scrambled - // sector nor a CSS-lock is genuinely unencrypted. - if saw_scrambled || (saw_locked && fail_on_locked) { + // The disc is ENCRYPTED-but-uncracked (a hard failure) when EITHER a + // scrambled sector was actually seen, OR every read was CSS-locked + // (`05/6F/03`), itself proof of scrambling. Only a scan that saw neither a + // scrambled sector nor a CSS-lock is genuinely unencrypted. + // + // A prior revision made this conditional on a caller-supplied + // `fail_on_locked: bool`, documented as "false on the per-VTS re-crack, so + // a lapsed-AGID locked read returns None instead of killing a genuinely + // crackable title." That parameter never had an observable effect: its + // ONLY non-test caller with `false` was [`crack_key`], whose `Option` + // return collapses `ScrambledUncracked` and `Unencrypted` alike to `None` + // via [`CrackOutcome::into_state`] — so the branch this comment describes + // was unreachable from the moment it was introduced (see the crate's audit + // notes for the git-archaeology). The one production caller that DOES + // observe the `Cracked` / `Unencrypted` / `ScrambledUncracked` split for a + // per-VTS re-crack (`Disc::decrypt_keys_for_title`) has always gone + // through [`crack_key_outcome`], which hardcoded this to always-hard-fail. + // Reconnecting the soft variant there would mean a locked read on a live + // drive gets reported as `Unencrypted` (`title_is_clear = true`) and the + // title is muxed with NO key — precisely the silent-garbage failure mode + // `CrackOutcome` exists to prevent. So the parameter is removed rather + // than revived: the scan is unconditionally hard-fail-on-locked. + if saw_scrambled || saw_locked { CrackOutcome::ScrambledUncracked } else { CrackOutcome::Unencrypted @@ -402,19 +425,16 @@ pub fn descramble_sector(state: &CssState, sector: &mut [u8]) { /// /// # Errors /// -/// [`Error::DecryptFailed`] when a sector's own crib proves the cached key stale -/// and the re-crack from that same sector also fails. CSS has no external key -/// source — the title key comes only from cracking the data — so on a readable -/// sector this is not a missing input, it is recovery failing on data we can -/// see. Emitting the sector anyway means one of two bad outcomes: descrambled -/// with the key its crib just rejected, which yields garbage behind an intact -/// clear header (valid pack start, passes every structural check the PS demuxer -/// applies, corruption confined to the PES payload where nothing looks); or -/// passed through still scrambled, which is ciphertext delivered where plaintext -/// is meant to be. Both are bad data reported as success. +/// Never returns `Err` — the signature is `Result` only to match the decrypt +/// seam it is dispatched from, and the `usize` is that seam's legacy +/// always-zero loss count (see [`crate::decrypt::decrypt_sectors`]). /// -/// This matches the AACS sibling, which returns [`Error::DecryptFailed`] rather -/// than apply a neighbouring CPS unit's key. +/// This section used to document an [`Error::DecryptFailed`] for the case where +/// a sector's crib rejects the cached key and the re-crack from that sector also +/// fails. That behaviour was tried and REVERTED, for the reason set out at the +/// `None =>` arm below: crib mismatch plus crack failure is the signature of a +/// crib FALSE POSITIVE, not of a stale key, and failing there made real discs +/// unrippable. The arm descrambles with the cached key and returns `Ok`. pub fn descramble_region(buf: &mut [u8], title_key: &mut [u8; 5]) -> crate::error::Result { for chunk in buf.chunks_mut(2048) { // `is_scrambled_pack`, NOT the looser `is_scrambled`. The raw flag test @@ -593,7 +613,22 @@ mod tests { let out = descramble_region(&mut sector, &mut key) .expect("a crib false positive must NOT fail the rip"); - assert_eq!(out, 0, "CSS reports no loss term of its own"); + // The "no loss term" contract belongs to the SEAM, not to this + // function: `decrypt_sectors`' `usize` is a legacy always-zero count + // that the CSS arm feeds from here. Asserting `out == 0` on + // `descramble_region` alone only restates its single `Ok(0)` return — + // a body replaced by `Ok(0)` satisfies it just as well. Assert it one + // level up, where the value is actually assembled and returned, so the + // arm dispatch and the plumbing are exercised too. + assert_eq!(out, 0); + let mut seam_sector = sector; + let mut seam_keys = crate::decrypt::DecryptKeys::Css { title_key: key }; + assert_eq!( + crate::decrypt::decrypt_sectors(&mut seam_sector, &mut seam_keys, 0) + .expect("a crib false positive must NOT fail the rip at the seam either"), + 0, + "CSS reports no loss term of its own through decrypt_sectors" + ); assert_eq!( key, key_before, "a failed re-crack must leave the cached key in place — it is still \ @@ -860,7 +895,7 @@ mod tests { start_lba: 100, sector_count: 4, }]; - let _ = crack_key_scan(&mut src, &ext, 4, None, false); + let _ = crack_key_scan(&mut src, &ext, 4, None); let reads = src.reads.borrow().clone(); assert_eq!( reads, @@ -882,7 +917,7 @@ mod tests { start_lba: 0, sector_count: 8, }]; - let outcome = crack_key_scan(&mut src, &ext, 4, None, false); + let outcome = crack_key_scan(&mut src, &ext, 4, None); assert!( matches!(outcome, CrackOutcome::Unencrypted), "nothing was read, so nothing scrambled was seen" @@ -895,6 +930,39 @@ mod tests { ); } + /// The 50_000-sector budget must hold whatever the source returns, not + /// only when the source delivers sectors. + /// + /// `tried` is incremented ONLY per inspected sector, inside + /// `for s in 0..usable`. An `Ok(0)` inspects nothing, so that loop never + /// runs — yet `advance` is forced to 1 to stop the scan spinning, so the + /// cursor keeps walking. The budget is then never consulted and the scan + /// runs for the extent's full, disc-declared `sector_count`: a misbehaving + /// or adversarial source (an emulated drive, a bridge answering short) + /// converts the anti-grind bound into no bound at all. + /// + /// Mutation: delete the `tried` charge in the `usable == 0` arm and this + /// goes red at 60_000 reads. + #[test] + fn a_source_that_returns_zero_sectors_still_obeys_the_scan_budget() { + const MAX_TRIES: usize = 50_000; + let mut src = MockSource::new(0x00); + src.short_read = Some(0); + // Deliberately LARGER than the budget: if the budget is what stops the + // scan, the extent's own length is never reached. + let ext = [crate::disc::Extent { + start_lba: 0, + sector_count: 60_000, + }]; + let _ = crack_key_scan(&mut src, &ext, 4, None); + let reads = src.reads.borrow().len(); + assert!( + reads <= MAX_TRIES, + "an Ok(0)-returning source must be stopped by the {MAX_TRIES}-sector \ + budget, not by the disc-declared extent length; got {reads} reads" + ); + } + /// crack_key caps total scanned sectors at 50_000 even when extents are /// far larger, and counts EVERY scanned sector (clear ones included) /// toward the budget. With one 200_000-sector extent of clear sectors, it @@ -1100,12 +1168,17 @@ mod tests { ); } - /// MISSING #1 guard: the re-crack path (the `Option`-returning `crack_key`, - /// `fail_on_locked == false`) must NOT hard-fail on a CSS-locked read — it - /// returns `None`. A lapsed-AGID re-crack of another VTS stays soft so a - /// genuinely crackable title isn't killed by a transient locked read. + /// `crack_key` (the `Option`-returning convenience wrapper) collapses + /// `ScrambledUncracked` and `Unencrypted` alike to `None` via + /// [`CrackOutcome::into_state`], so an all-locked scan still reads `None` + /// here even though the scan itself now treats every CSS-lock as a hard + /// `ScrambledUncracked` (see `crack_key_scan`'s removal of the dead + /// `fail_on_locked` parameter). Callers that need to tell "locked/ + /// uncrackable" apart from "genuinely clear" must use `crack_key_outcome`, + /// which the `all_locked_synthetic_iso_yields_css_key_missing_signal` test + /// pins directly. #[test] - fn crack_key_recrack_locked_is_none_not_hard_fail() { + fn crack_key_all_locked_collapses_to_none() { let mut src = MockSource::new(0x30); src.lock_all = true; let extents = [Extent { @@ -1595,8 +1668,8 @@ mod tests { /// PER-VTS RE-CRACK SUCCESS (audit gap "success path missing"): the prior /// re-crack test only covered the locked→None path. Here a re-crack - /// (`crack_key`, `fail_on_locked == false`) over a DIFFERENT VTS's extents - /// finds that VTS's own crackable sector and returns a `CssState` whose + /// (`crack_key`) over a DIFFERENT VTS's extents finds that VTS's own + /// crackable sector and returns a `CssState` whose /// `crack_span` matches the new extents — proving a key cracked for one VTS /// is genuinely re-derived (not reused) for another. #[test] diff --git a/src/decrypt.rs b/src/decrypt.rs index 5fbb0ae..689bd9d 100644 --- a/src/decrypt.rs +++ b/src/decrypt.rs @@ -1899,11 +1899,24 @@ mod tests { /// below its own range start, or a zero unit size, are both map bugs — they /// must return a defined answer rather than panicking on debug overflow or /// dividing by zero. + /// + /// Every case here asserts the DEFINED answer, not merely the absence of a + /// panic. The zero-unit-size case used to be written + /// `assert!(unit_is_our_phase(100, 30, 0, Phase::Even) || true)`, which + /// accepts both answers and so pinned nothing at all: the guards could + /// invert and it would still pass. The answer is knowable — + /// `saturating_sub` gives 70, `max(1)` makes the divisor 1, unit index 70 + /// is even — so pin it. #[test] fn phase_gate_does_not_panic_on_a_malformed_map() { use super::{Phase, unit_is_our_phase}; + // Unit below its own range start: saturating_sub clamps to 0, and unit + // 0 is even. assert!(unit_is_our_phase(10, 100, 3, Phase::Even)); - assert!(unit_is_our_phase(100, 30, 0, Phase::Even) || true); + // Zero unit size: max(1) makes the divisor 1, so the index is the raw + // offset 70 — even. + assert!(unit_is_our_phase(100, 30, 0, Phase::Even)); + // Both malformations at once: offset 0 over divisor 1 is unit 0, even. assert!(unit_is_our_phase(5, 5, 0, Phase::Even)); } } diff --git a/src/disc/bluray.rs b/src/disc/bluray.rs index e1c38ad..c466990 100644 --- a/src/disc/bluray.rs +++ b/src/disc/bluray.rs @@ -132,16 +132,48 @@ impl Disc { // so muxing it captures the full 3D. 2D clips fall back to // the base .m2ts / .fmts as before. let ssif = format!("/BDMV/STREAM/SSIF/{}.ssif", play_item.clip_id); + // A clip stream that carries an unrecorded (never-written) + // extent cannot be turned into a truthful read plan — see + // `UdfFs::file_extents`. Track it separately from an + // ordinary "file absent" error: absence is what the + // extension fallback exists for, whereas a hole means the + // bytes this title needs do not exist on the disc. + let mut unrecorded = false; let file_exts = match udf_fs.file_extents(reader, &ssif) { Ok(exts) => { is_3d = true; Some(exts) } - Err(_) => CLIP_STREAM_EXTS.iter().find_map(|ext| { - let path = format!("/BDMV/STREAM/{}.{}", play_item.clip_id, ext); - udf_fs.file_extents(reader, &path).ok() - }), + Err(e) => { + unrecorded |= matches!(e, Error::UdfUnrecordedExtent { .. }); + CLIP_STREAM_EXTS.iter().find_map(|ext| { + let path = format!("/BDMV/STREAM/{}.{}", play_item.clip_id, ext); + match udf_fs.file_extents(reader, &path) { + Ok(exts) => Some(exts), + Err(e) => { + unrecorded |= + matches!(e, Error::UdfUnrecordedExtent { .. }); + None + } + } + }) + } }; + // Nothing resolved AND a hole was the reason: drop the + // whole title. Letting the clip contribute no extents + // (the ordinary not-found path) would emit a title whose + // feed is silently missing this clip's runtime while its + // durations, spans and size still count it — data loss + // wearing the shape of a normal rip. + if file_exts.is_none() && unrecorded { + tracing::warn!( + target: "freemkv::disc", + playlist = ?filename, + clip = ?play_item.clip_id, + "E{}", crate::error::E_UDF_UNRECORDED_EXTENT + ); + return None; + } if let Some(file_exts) = file_exts { let span_start = feed_pos; for (lba, sectors) in file_exts { @@ -682,7 +714,7 @@ mod tests { let m2ts = format!("{name}.{stream_ext}"); // Size in bytes — file_extents derives sectors via div_ceil(2048). let size = sectors * 2048; - stream_files.push(file(&m2ts, icb, *data_lba, size, true)); + stream_files.push(file(&m2ts, icb, *data_lba, size as u64, true)); icb += 1; let clpi = format!("{name}.clpi"); clipinf_files.push(file_with( @@ -747,7 +779,7 @@ mod tests { for (name, sectors, packets, data_lba) in clips { let ssif = format!("{name}.ssif"); let size = sectors * 2048; - ssif_files.push(file(&ssif, icb, *data_lba, size, true)); + ssif_files.push(file(&ssif, icb, *data_lba, size as u64, true)); icb += 1; let clpi = format!("{name}.clpi"); clipinf_files.push(file_with( @@ -1029,16 +1061,25 @@ mod tests { assert_eq!(t.clips[0].source_packets, 0); } - /// `file_extents` filters extents with `lba == 0` or `sectors == 0` - /// (bluray.rs: `if sectors > 0 && lba > 0`). A clip whose data lands at - /// partition-relative LBA 0 would produce abs LBA == PART_START (> 0), - /// so to exercise the lba==0 guard we'd need partition_start 0; instead - /// verify a zero-length declared file produces no extent. A 0-byte - /// m2ts → sectors == 0 → dropped. + /// A clip stream whose ICB declares an UNRECORDED (ECMA-167 4/14.14.1.1 + /// type-1) extent must not yield a title at all. + /// + /// The extent is allocated to the file but was never written, so the + /// file's content there is zeros while the media holds whatever was left + /// at those sectors. Neither answer a `(lba, sector_count)` read plan can + /// give is true — reading it splices undefined sectors into the rip as + /// content, dropping it slides every later extent's byte space — so the + /// title is refused rather than mis-ripped. This fixture is the shape a + /// crafted disc uses to get such a range into a title's extent list. + /// + /// (The `sectors > 0 && lba > 0` filter below the resolver stays as + /// defence in depth; a zero-length AD is only reachable as an unrecorded + /// descriptor, since a zero-length TYPE 0 one terminates the AD list.) #[test] - fn parse_playlist_zero_length_extent_is_filtered() { + fn parse_playlist_unrecorded_extent_yields_no_title() { let mut disc = MemDisc::new(); - // m2ts declared 0 bytes → file_extents sectors = div_ceil(0,2048)=0. + // The m2ts ICB is rewritten below to carry TWO short ADs: a + // zero-length one (0 sectors) followed by a real 4096-byte one. let udf = { let bdmv = DirSpec { name: "BDMV".to_string(), @@ -1050,7 +1091,7 @@ mod tests { name: "STREAM".to_string(), icb_lba: 22, dir_data_lba: 23, - files: vec![file("00001.m2ts", 100, 5000, 0, true)], + files: vec![file("00001.m2ts", 100, 5000, 4096, false)], subdirs: vec![], }, DirSpec { @@ -1071,8 +1112,44 @@ mod tests { }; build_udf_skeleton(&mut disc, 10); lay_dir(&mut disc, &root); + // Rewrite the .m2ts ICB (laid at PART_START + 100 by `lay_dir`) + // with a two-descriptor short-AD list: + // AD0: ECMA-167 4/14.14.1.1 type 1 (allocated, NOT recorded), + // length 0, at LBA 4999 — a zero-length descriptor that + // SURVIVES `read_icb_extents` (only a zero-length TYPE 0 + // descriptor is the AD-list terminator), so it reaches + // `file_extents` as an extent of div_ceil(0, 2048) = 0 + // sectors. This is the shape a crafted disc uses to put a + // readable-looking but empty range into a title's extent + // list. + // AD1: type 0, 4096 bytes at LBA 5000 — the real content. + let mut icb = build_file_icb(4096, 5000, false); + icb[212..216].copy_from_slice(&16u32.to_le_bytes()); // l_ad: two short ADs + icb[216..220].copy_from_slice(&0x4000_0800u32.to_le_bytes()); // type 1, 2048 bytes + icb[220..224].copy_from_slice(&4999u32.to_le_bytes()); + icb[224..228].copy_from_slice(&4096u32.to_le_bytes()); // type 0, 4096 bytes + icb[228..232].copy_from_slice(&5000u32.to_le_bytes()); + disc.put_bytes(PART_START + 100, &icb); udf::read_filesystem(&mut disc).expect("fs") }; + // The fixture must really carry the unrecorded descriptor, or the + // behaviour under test is never reached. `file_extents_addressing` + // shows what is there: the hole in its byte-space position, followed + // by the real content. + assert_eq!( + udf.file_extents_addressing(&mut disc, "/BDMV/STREAM/00001.m2ts") + .expect("extents"), + vec![(PART_START + 4999, 1), (PART_START + 5000, 2)], + "fixture must present one unrecorded extent that OCCUPIES byte \ + space, and one real one" + ); + assert!( + matches!( + udf.file_extents(&mut disc, "/BDMV/STREAM/00001.m2ts"), + Err(Error::UdfUnrecordedExtent { .. }) + ), + "a read plan over an unrecorded extent must be refused" + ); let mpls = build_mpls( &[PiSpec { clip_id: *b"00001", @@ -1083,10 +1160,13 @@ mod tests { &[], &[], ); - let t = Disc::parse_playlist(&mut disc, &udf, "00001.mpls", &mpls).expect("title"); - // size still counted (from clpi packets) but the empty extent dropped. - assert_eq!(t.size_bytes, 4000 * 192); - assert!(t.extents.is_empty(), "zero-sector extent must be filtered"); + let t = Disc::parse_playlist(&mut disc, &udf, "00001.mpls", &mpls); + assert!( + t.is_none(), + "the only clip has no truthful read plan, so offering the title \ + would mean ripping undefined sectors as content; got {:?}", + t.map(|t| t.extents) + ); } // --------------------------------------------------------------- diff --git a/src/disc/dvd.rs b/src/disc/dvd.rs index 1b5af2a..856e4e2 100644 --- a/src/disc/dvd.rs +++ b/src/disc/dvd.rs @@ -911,7 +911,7 @@ mod tests { .collect(); assert_eq!(audios.len(), 2); assert_eq!(audios[0].codec, Codec::Ac3); - assert_eq!(audios[0].language, "en"); + assert_eq!(audios[0].language, "eng"); assert_eq!(audios[1].codec, Codec::Dts); // Real channel layouts survive the scan (not a 1ch placeholder): the // AC-3 is 5.1 (6ch), the DTS is 2.0 (2ch). @@ -1040,7 +1040,7 @@ mod tests { // Languages preserved in order. assert_eq!( subs.iter().map(|s| s.language.as_str()).collect::>(), - vec!["en", "fr", "de"] + vec!["eng", "fra", "deu"] ); // PIDs are 0x20 + ordinal, all distinct. let pids: Vec = subs.iter().map(|s| s.pid).collect(); @@ -1097,7 +1097,7 @@ mod tests { }) .expect("subtitle stream"); assert_eq!(sub.codec, Codec::DvdSub); - assert_eq!(sub.language, "en"); + assert_eq!(sub.language, "eng"); assert!( sub.codec_data.is_some(), "non-zero palette must yield codec_data" diff --git a/src/disc/extract.rs b/src/disc/extract.rs index 59fa2a4..ca39a7c 100644 --- a/src/disc/extract.rs +++ b/src/disc/extract.rs @@ -107,8 +107,10 @@ struct PlannedFile { size: u64, /// Inline (ICB-embedded) data, if any. When `Some`, `extents` is empty. inline: Option>, - /// Absolute disc extents `(abs_lba, byte_len)`. - extents: Vec<(u32, u32)>, + /// Absolute disc extents, each carrying whether it was ever RECORDED (an + /// ECMA-167 4/14.14.1.1 type-1 extent is allocated but not recorded: it + /// occupies the file's byte space and its contents are zeros). + extents: Vec, } impl Disc { @@ -144,7 +146,7 @@ impl Disc { let fs = udf::read_filesystem(reader)?; let mut planned: Vec = Vec::new(); let mut dirs: Vec = Vec::new(); - let mut seen_hosts: std::collections::HashMap = + let mut seen_hosts: std::collections::HashMap = std::collections::HashMap::new(); plan_tree( reader, @@ -339,10 +341,14 @@ impl Disc { let mut extents: Vec = Vec::new(); for pf in files { - for &(abs_lba, byte_len) in &pf.extents { + // Unrecorded extents hold no bytes the VOB ever wrote, so they can + // carry no scrambled sector for the crack to work from — feeding + // them in spends the shared sector budget on media that is not part + // of the title. + for ext in pf.extents.iter().filter(|e| e.recorded) { extents.push(crate::disc::Extent { - start_lba: abs_lba, - sector_count: (byte_len as u64).div_ceil(SECTOR_BYTES_U64) as u32, + start_lba: ext.lba, + sector_count: (ext.len as u64).div_ceil(SECTOR_BYTES_U64) as u32, }); } } @@ -379,9 +385,22 @@ impl Disc { crate::css::CrackOutcome::Unencrypted => Ok(base_keys.clone()), // Scrambled sectors WERE seen and no key came out. Reusing the // disc-wide key here writes corrupt PES behind an intact header and - // reports a complete extract at exit 0. Skippable per title, which - // is why this is the per-title code and not the disc-level one — a - // sibling VTS may still crack. + // reports a complete extract at exit 0. + // + // `CssKeyMissing` is the per-TITLE code rather than the disc-level + // `CssNoDiscKey` because the two are treated differently elsewhere: + // it is what `error::is_skippable_title_stub` matches, and the MUX + // path really does skip a title on it and carry on with the rest of + // the disc. + // + // EXTRACT does not skip. This error is `?`-propagated by the only + // caller, out of the loop over every planned file, so one + // uncrackable VTS aborts the whole extract — including files from + // sibling VTS groups that would have cracked. That is deliberate: + // aborting loudly is the safe failure here, and pinned by + // `a_scrambled_vts_that_cannot_be_cracked_fails_instead_of_borrowing_a_key`. + // Do not read the code choice as a promise that extraction + // continues past this point. crate::css::CrackOutcome::ScrambledUncracked => Err(Error::CssKeyMissing), } } @@ -429,7 +448,7 @@ fn plan_tree( is_root: bool, files: &mut Vec, dirs: &mut Vec, - seen_hosts: &mut std::collections::HashMap, + seen_hosts: &mut std::collections::HashMap, ) -> Result<()> { for entry in &dir.entries { if entry.name.is_empty() { @@ -447,13 +466,48 @@ fn plan_tree( let safe = sanitize_component(&entry.name)?; let child_rel = host_rel.join(&safe); let child_disc = format!("{disc_path}/{}", entry.name); - // Collision: two distinct disc paths → same host path. - if let Some(prev) = seen_hosts.insert(child_rel.clone(), child_disc.clone()) - && prev != child_disc - { - return Err(Error::DirNameCollision { - host: child_rel.to_string_lossy().into_owned(), - }); + // Collision: two distinct disc paths → same host FILE. The key must + // model the HOST's namespace, not the disc's, and the two differ twice + // over: + // + // * CASE. macOS APFS and Windows NTFS are case-insensitive by default, + // so `Movie` and `movie` are one file there. Keyed by the + // case-preserving path they were two entries, nothing collided, and + // the second extraction overwrote the first with both files still + // reported `complete: true`. + // * The `.partial` SUFFIX. `extract_one_file` streams through + // `.partial` before renaming, so a file's temp path lives in + // the same namespace as every other file's final path: a disc + // holding both `X` and `X.partial` planned two distinct final names, + // and extracting `X` then truncated the real `X.partial`. + // + // Only files get the `.partial` alias — directories are created + // directly and never stream through a temp name — but it is checked + // against every entry's primary key, so a directory `X.partial` beside + // a file `X` is caught too. The alias stores the OWNING file's disc + // path, so the `prev != child_disc` test keeps its meaning: a repeated + // identical disc path is not a collision, two different ones are. + // + // The fold is `to_lowercase`, which closes the ASCII and simple-Unicode + // case classes. It is NOT full case folding, and it does NOT normalize: + // APFS also unifies NFC/NFD, so `é` recorded as U+00E9 and as + // `e`+U+0301 remain two keys here and one file there. That residual is + // a false NEGATIVE (a missed collision, never a spurious one), so this + // is a strict improvement rather than a complete model of the host. + let mut register = |key: PathBuf| -> Result<()> { + let folded = key.to_string_lossy().to_lowercase(); + if let Some(prev) = seen_hosts.insert(folded, child_disc.clone()) + && prev != child_disc + { + return Err(Error::DirNameCollision { + host: key.to_string_lossy().into_owned(), + }); + } + Ok(()) + }; + register(child_rel.clone())?; + if !entry.is_dir { + register(with_partial_suffix(&child_rel))?; } if entry.is_dir { dirs.push(child_rel.clone()); @@ -534,10 +588,47 @@ fn extract_one_file( let mut written: u64 = 0; let mut buf = vec![0u8; READ_BATCH_SECTORS as usize * SECTOR_BYTES]; - 'extents: for &(abs_lba, byte_len) in &pf.extents { + 'extents: for &crate::udf::AbsExtent { + lba: abs_lba, + len: byte_len, + recorded, + } in &pf.extents + { if written >= pf.size { break; } + // ECMA-167 4/14.14.1.1 type 1: allocated but NOT recorded. The extent + // is part of the file's byte space and its contents are defined to be + // zeros, so write the zeros WITHOUT reading the media — those sectors + // hold nothing this file ever wrote (on an AACS disc, ciphertext that + // decrypts to noise). This mirrors `UdfFs::read_file_limited`, which + // takes the same decision from the same flag. Skipping the extent + // entirely instead would slide every later extent's bytes down by the + // hole's length. + if !recorded { + let sectors = (byte_len as u64).div_ceil(SECTOR_BYTES_U64); + let hole_bytes = (sectors * SECTOR_BYTES_U64).min(pf.size.saturating_sub(written)); + let mut left = hole_bytes; + for b in buf.iter_mut() { + *b = 0; + } + while left > 0 { + let n = left.min(buf.len() as u64) as usize; + write_all(&mut writer, &buf[..n], &partial_path)?; + written = written.saturating_add(n as u64); + *done_bytes = done_bytes.saturating_add(n as u64); + left -= n as u64; + } + fr.bytes_good = fr.bytes_good.saturating_add(hole_bytes); + let cont = report(opts, *done_bytes, total_bytes); + if opts.cancelled(cont) { + return Ok((fr, true)); + } + if written >= pf.size { + break 'extents; + } + continue; + } // Anchor AACS unit alignment at THIS extent's start (clip-anchored // gate, not absolute LBA 0 and NOT the file's first extent). A file // may span multiple extents (fragmented / Long-AD / continuation ICB @@ -1185,6 +1276,17 @@ mod tests { s } + /// Build a file ICB whose FIRST short AD is an ECMA-167 4/14.14.1.1 type-1 + /// (allocated, NOT recorded) extent and whose second is ordinary recorded + /// data. Both are `sectors_each` sectors long. + fn build_hole_then_data_icb(sectors_each: u32, hole_lba: u32, data_lba: u32) -> [u8; 2048] { + let mut s = build_two_extent_icb(sectors_each, hole_lba, data_lba); + let len = sectors_each * SECTOR_BYTES as u32; + // Re-stamp AD #0 with extent type 1 in bits 30..31 of the length field. + s[216..220].copy_from_slice(&(0x4000_0000u32 | (len & 0x3FFF_FFFF)).to_le_bytes()); + s + } + /// Encrypt the clear unit from `clear_aacs_unit(tag)` under `unit_key` so /// `aacs::content::decrypt_unit` recovers it cleanly (zero decrypt loss). /// `tag` distinguishes two units' payloads. @@ -1598,6 +1700,62 @@ mod tests { assert!(matches!(err, Error::DirNameCollision { .. })); } + /// Two disc names differing only by CASE are one host file on macOS APFS + /// and Windows NTFS, both case-insensitive by default. Keyed by the + /// case-preserving path, the collision map sees two entries, raises + /// nothing, and the second file extracted overwrites the first — while both + /// `PlannedFile`s report `complete: true`. Silent data loss reported as a + /// clean extract is the one outcome this crate must never produce, so the + /// host-equivalence key has to model the host's namespace, not the disc's. + #[test] + fn names_differing_only_by_case_are_a_collision() { + let root = DirSpec { + name: String::new(), + icb_lba: 10, + dir_data_lba: 11, + files: vec![ + file("Movie", 30, 31, b"a".to_vec(), false), + file("movie", 32, 33, b"b".to_vec(), false), + ], + subdirs: vec![], + }; + let mut disc = build_disc(root); + let out = TmpDir::new("case_collision"); + let err = clear_disc() + .extract_tree(&mut disc, out.path(), &ExtractOptions::default()) + .expect_err("two names that fold to one host file must collide"); + assert!(matches!(err, Error::DirNameCollision { .. }), "got {err:?}"); + } + + /// A file's in-flight `.partial` path shares the host namespace with every + /// other planned file's FINAL path. A disc carrying both `X` and + /// `X.partial` plans two distinct final names, so nothing collides — but + /// extracting `X` writes through `X.partial`, the same host path the other + /// file owns. Whichever lands second truncates the other, and both entries + /// are still reported complete. + #[test] + fn a_files_partial_path_colliding_with_another_files_final_name_is_an_error() { + let root = DirSpec { + name: String::new(), + icb_lba: 10, + dir_data_lba: 11, + files: vec![ + file("X", 30, 31, b"aaaa".to_vec(), false), + file("X.partial", 32, 33, b"bbbb".to_vec(), false), + ], + subdirs: vec![], + }; + let mut disc = build_disc(root); + let out = TmpDir::new("partial_collision"); + let err = clear_disc() + .extract_tree(&mut disc, out.path(), &ExtractOptions::default()) + .expect_err( + "X's temp path IS X.partial's final path — one host file for two \ + disc files, so it must be refused up front", + ); + assert!(matches!(err, Error::DirNameCollision { .. }), "got {err:?}"); + } + /// A non-empty target dir is refused without `--force`, and accepted with. #[test] fn non_empty_target_requires_force() { @@ -1727,6 +1885,63 @@ mod tests { ); } + /// An ECMA-167 4/14.14.1.1 type-1 extent is ALLOCATED BUT NOT RECORDED: + /// the space belongs to the file and occupies its byte range, but nothing + /// was ever written there, and the standard defines its contents as zeros. + /// `read_icb_extents` keeps the flag (`IcbExtent::recorded`) and + /// `read_file_limited` honours it by emitting zeros WITHOUT touching the + /// media. The tree extractor reads the same ICBs and must agree: reading + /// those sectors returns whatever the media happens to hold there — on an + /// AACS disc, ciphertext that decrypts to noise — and writes it into the + /// extracted file as if the disc had recorded it. + /// + /// The hole here is filled with a recognisable non-zero pattern, so + /// "emitted zeros" and "read the media" are told apart by CONTENT. + #[test] + fn extract_tree_zero_fills_an_unrecorded_extent_instead_of_reading_it() { + const SECTORS_EACH: u32 = 1; + const HOLE: u32 = 5000; + const DATA: u32 = 5004; + + let hole_bytes = vec![0xEEu8; SECTOR_BYTES]; + let data_bytes = vec![0x5Au8; SECTOR_BYTES]; + // The file's byte space: the hole's zeros FIRST, then the real data. + let mut expect = vec![0u8; SECTOR_BYTES]; + expect.extend_from_slice(&data_bytes); + + let mut disc = MemDisc::new(); + build_udf_skeleton(&mut disc, 10); + + let mut root_fids = Vec::new(); + push_fid(&mut root_fids, "", 10, true, true); + push_fid(&mut root_fids, "INDEX.BDMV", 42, false, false); + disc.put( + PART_START + 42, + build_hole_then_data_icb(SECTORS_EACH, HOLE, DATA), + ); + disc.put_bytes(PART_START + HOLE, &hole_bytes); + disc.put_bytes(PART_START + DATA, &data_bytes); + disc.put(PART_START + 10, build_dir_icb(11, root_fids.len() as u32)); + disc.put_bytes(PART_START + 11, &root_fids); + + let out = TmpDir::new("unrecorded_extent"); + let res = clear_disc() + .extract_tree(&mut disc, out.path(), &ExtractOptions::default()) + .expect("extract"); + + let got = read_out(out.path(), "INDEX.BDMV").expect("file written"); + assert_eq!( + got, expect, + "an unrecorded extent contributes zeros to the file, never the \ + bytes that happen to sit on those sectors" + ); + assert_eq!( + res.bytes_unreadable, 0, + "a hole is not a read failure — nothing was attempted" + ); + assert!(res.complete); + } + /// Focused alignment-computation check underpinning the per-extent fix: /// when each extent anchors its OWN start as the unit base, the extent's /// own batch starts are always unit-aligned; anchoring a later extent diff --git a/src/disc/hddvd.rs b/src/disc/hddvd.rs index 988d8e2..b66bac3 100644 --- a/src/disc/hddvd.rs +++ b/src/disc/hddvd.rs @@ -183,7 +183,17 @@ fn sniff_audio_codec(es: &[u8]) -> Option { /// dimensions from the H.264 SPS, and E-AC-3 channel counts are not decoded /// here). Returns an empty vec when the clip cannot be read or carries no /// recognizable stream (e.g. an AACS-encrypted clip probed as ciphertext). -fn probe_evo_streams(reader: &mut dyn SectorSource, extents: &[Extent]) -> Vec { +/// +/// Cancellation is checked before every chunk read and is the ONE condition +/// that returns `Err` rather than an empty vec: a probe cut short by a Stop has +/// not established that the clip carries no streams, and reporting it as if it +/// had would enumerate a stream-less title as a scanned fact. A read that fails +/// for any other reason keeps the existing best-effort behaviour. +fn probe_evo_streams( + reader: &mut dyn SectorSource, + extents: &[Extent], + halt: Option<&crate::halt::Halt>, +) -> Result> { let mut demux = PsDemuxer::new(); let mut video: Vec = Vec::new(); // Routing PID of the video track, captured from the first video PES seen: @@ -201,10 +211,25 @@ fn probe_evo_streams(reader: &mut dyn SectorSource, extents: &[Extent]) -> Vec 0 && remaining > 0 { // 1 MiB read chunks (512 sectors) keep buffers small. + // Poll before every chunk, as the PGS probe does: a clip's probe + // budget is 16 MiB of blocking reads, and on marginal media each + // one can sit in the drive's retry path, so per-clip granularity + // alone would leave a Stop waiting on a dead disc. + if halt.is_some_and(|h| h.is_cancelled()) { + return Err(crate::error::Error::Halted); + } let n = left.min(remaining).min(512) as u16; let mut buf = vec![0u8; n as usize * crate::consts::SECTOR_BYTES]; - if reader.read_sectors(lba, n, &mut buf, false).is_err() { - break 'outer; + match reader.read_sectors(lba, n, &mut buf, false) { + Ok(_) => {} + // A live-drive Stop surfaces HERE, not through the token: + // `Drive::checked_exec` fails every command with `Halted` once + // the flag is set. Swallowing it (as any other read error is + // swallowed) reports an un-probed clip as a probed one. + Err(crate::error::Error::Halted) => { + return Err(crate::error::Error::Halted); + } + Err(_) => break 'outer, } for pkt in demux.feed(&buf) { collect_es(&pkt, &mut video, &mut video_pid, &mut audio); @@ -262,7 +287,7 @@ fn probe_evo_streams(reader: &mut dyn SectorSource, extents: &[Extent]) -> Vec Option { Some(format!("{}.evo", stem.to_ascii_lowercase())) } +/// Maximum element nesting depth accepted in an XPL. A real `VPLST000.XPL` is +/// about six levels deep at its deepest +/// (`Playlist`/`TitleSet`/`Title`/`PrimaryAudioVideoClip`/`Video`, or +/// `.../ChapterList/Chapter`); 32 leaves a 5x margin for authoring tools that +/// wrap extra `ApplicationSegment`/`ObjectMappingList` layers, while staying +/// trivially stack-safe. +const MAX_XPL_DEPTH: usize = 32; + +/// Most `` elements one playlist may declare. +/// +/// The depth guard above bounds NESTING; this bounds BREADTH, and they are +/// separate attacks. A flat playlist passes the depth check trivially and can +/// still declare a title per 51 bytes, so a 64 MiB `VPLST000.XPL` — exactly +/// `udf::MAX_FILE_BYTES` — reaches ~1.3 million of them. +/// +/// That matters here and not on the Blu-ray side because of where the count +/// comes from: `bluray.rs` learns its playlists from the MPLS files actually +/// present in the UDF directory, so the medium bounds it. An XPL DECLARES its +/// titles, so nothing does. +/// +/// The cost is not memory but drive time. `compose_xpl_titles` runs +/// `probe_evo_streams` once per title, and each probe reads real sectors — so +/// an uncapped count turns a `Disc::scan()` the operator expects to take +/// seconds into tens of terabytes of optical reads on a disc that is only +/// pretending to be large. +/// +/// The bound that matters is `cap x EVO_PROBE_SECTORS`, since each title costs +/// one probe: `compose_xpl_titles` memoizes on the title's RESOLVED extent list, +/// and distinct titles built from different combinations of the same clips have +/// different lists, so each one misses the memo. At 4096 that product was 64 GiB +/// of optical reads — the same worst case, and the same contradiction with +/// "seconds", as the sibling [`MAX_HDDVD_CLIPS`] path. +/// +/// 512 keeps it at 8 GiB and remains far above any real disc (retail HD-DVDs +/// carry tens of titles). +const MAX_XPL_TITLES: usize = 512; + +/// Cap on `.evo` clips taken from the `HVDVD_TS/` directory listing. +/// +/// [`MAX_XPL_TITLES`] bounds what a PLAYLIST may declare, and +/// [`xpl_depth_within_limit`] bounds how deeply it may nest — but both live on +/// the `ADV_OBJ/*.XPL` path, and a crafted disc simply OMITS `/ADV_OBJ`. Control +/// then reaches the clip-name fallback in [`Disc::scan_hddvd_titles`], which +/// emits one title per DIRECTORY ENTRY and probes each. That is the same +/// unbounded-probe cost, reached without the playlist. +/// +/// The directory is the only bound the medium supplies, and `udf::MAX_DIR_BYTES` +/// (1 MiB) still leaves room for ~24,000 FIDs. Memoizing the probe +/// ([`EvoProbeCache`]) collapses entries that resolve to the SAME extents, which +/// closes the many-names-one-File-Entry case — but it cannot close this one: a +/// File Entry is a single sector, so an attacker can give every FID its own, +/// each declaring its own extent, for ~48 MiB of image. Every probe then misses +/// the memo and costs a full [`EVO_PROBE_SECTORS`] (16 MiB) read — hundreds of +/// GiB on a `Disc::scan()` the operator expects to take seconds. +/// +/// The cap's job is therefore to bound `cap x EVO_PROBE_SECTORS`, and only that +/// product means anything. It was 4096 — chosen to match [`MAX_XPL_TITLES`], +/// which bounds a different path with no per-item read cost — and 4096 x 16 MiB +/// is 64 GiB of optical reads: more than dual-layer HD-DVD media physically +/// holds (reachable anyway, since distinct extent lists may overlap on the +/// medium) and tens of minutes to hours at drive speeds. That is not "a scan +/// the operator expects to take seconds"; the cap stopped literal +/// unboundedness while permitting a worst case the same doc ruled out. +/// +/// 512 keeps the product at 8 GiB and still leaves a wide margin over any real +/// disc — retail HD-DVDs carry TENS of `.evo` clips, so this is ~10x the +/// real-world maximum, the same sizing convention +/// [`MAX_XPL_CHAPTERS_PER_TITLE`] uses against the 99-chapter authoring +/// ceiling. No genuine disc loses a title. +/// +/// Pinned by `the_clip_cap_and_probe_budget_bound_a_scans_worst_case_read_volume`, +/// which asserts the PRODUCT rather than either constant, so raising one +/// without re-examining the other fails. +const MAX_HDDVD_CLIPS: usize = 512; + +/// Most `<PrimaryAudioVideoClip>` elements one `<Title>` may contribute. +/// +/// The fourth amplification axis on this function, and the one the other three +/// guards leave open. [`MAX_XPL_DEPTH`] bounds NESTING, [`MAX_XPL_TITLES`] +/// bounds how many titles a playlist DECLARES, and [`MAX_HDDVD_CLIPS`] bounds +/// the directory fallback — none of them bounds how many clips a SINGLE title +/// collects. +/// +/// Two things multiply here. A 64 MiB XPL (exactly `udf::MAX_FILE_BYTES`) holds +/// ~1.7 million `<PrimaryAudioVideoClip src="A.EVO"/>` elements at ~38 bytes +/// each. And the collector below is `descendants()`, not `children()`, so a clip +/// nested inside N ancestor `<Title>` elements is collected by EVERY one of +/// them — and the depth cap still permits 32 such ancestors (measured: a bare +/// `<Title>` root nests 32 deep and still parses, because the guard counts the +/// self-closing clip element as non-nesting). The product is ~1.7M x 32 ≈ 56 +/// million heap-allocated [`XplClip`]s, each with its own `String`, and then as +/// many [`Clip`]s again in [`compose_xpl_titles`]. +/// +/// Unlike the title cap, the cost here is MEMORY rather than drive time: +/// `compose_xpl_titles` de-duplicates by `.evo` name before probing +/// (`seen_evos`), so repeats cost no extra sector reads — but the `Vec`s are +/// built and held regardless. +/// +/// 256 is far above any real disc. A retail HD-DVD title is a handful of clips — +/// a feature is one, or two across a layer break; even a seamless-branching or +/// multi-angle title is tens. It also keeps the aggregate bounded: at most +/// [`MAX_XPL_TITLES`] x 256 clips, rather than the tens of millions above. +const MAX_XPL_CLIPS_PER_TITLE: usize = 256; + +/// Most `<Chapter>` elements one `<Title>` may contribute. +/// +/// The same `descendants()` amplification as [`MAX_XPL_CLIPS_PER_TITLE`], on the +/// second unbounded `collect()` in the same loop, and bounded separately so that +/// capping clips does not simply move the attack next door. A chapter is a bare +/// `f64` rather than a `String`-carrying struct, so each one is cheaper — but a +/// `<Chapter titleTimeBegin="…"/>` element is also smaller, so the element count +/// an XPL can reach is comparable. +/// +/// 1024 is far above any real disc: the DVD/HD-DVD authoring convention tops out +/// at 99 chapters per title, so this leaves a 10x margin. +const MAX_XPL_CHAPTERS_PER_TITLE: usize = 1024; + +/// Memoized [`probe_evo_streams`], keyed on the RESOLVED extent list. +/// +/// Probing is the expensive half of title composition: each pass reads up to +/// [`EVO_PROBE_SECTORS`] (16 MiB) off the medium. Both composition paths can +/// reach the same physical clip many times over — an XPL naming one `.evo` from +/// many `<Title>`s, or a directory whose FIDs all point at one File Entry — and +/// a probe is a pure function of the extents it reads, so the second and later +/// passes over an identical extent list are re-reads of bytes already seen. +/// +/// The key is the extent list itself, NOT the clip name or title: two names for +/// one File Entry share a key (one probe), while two genuinely different clips +/// have different `start_lba`s and so keep their own probes and their own +/// streams. Cache size is bounded by the caller's title/clip cap +/// ([`MAX_XPL_TITLES`] / [`MAX_HDDVD_CLIPS`]). +#[derive(Default)] +struct EvoProbeCache { + seen: std::collections::HashMap<Vec<(u32, u32)>, Vec<Stream>>, +} + +impl EvoProbeCache { + /// Streams for `extents` — probed on first sight, replayed from the memo + /// afterwards. + fn streams( + &mut self, + reader: &mut dyn SectorSource, + extents: &[Extent], + halt: Option<&crate::halt::Halt>, + ) -> Result<Vec<Stream>> { + let key: Vec<(u32, u32)> = extents + .iter() + .map(|e| (e.start_lba, e.sector_count)) + .collect(); + if let Some(hit) = self.seen.get(&key) { + return Ok(hit.clone()); + } + // A cancelled probe is not memoised: it never established what the + // clip holds, so replaying it for the next title that shares these + // extents would spread one Stop into a disc-wide "no streams" verdict. + let streams = probe_evo_streams(reader, extents, halt)?; + self.seen.insert(key, streams.clone()); + Ok(streams) + } +} + +/// Reject a disc-supplied XML blob whose element nesting exceeds `MAX_XPL_DEPTH`, +/// BEFORE it reaches the parser. `roxmltree` is recursive-descent and its +/// depth-10 limit applies only to ENTITY expansion, so element nesting is +/// unbounded: a few hundred KB of well-formed XML — far under the read cap — +/// overflows the thread stack. That is a process ABORT, not an `Err` and not an +/// unwind, so neither the `Document::parse` fallback nor `catch_unwind` can +/// contain it; the only fix is to not hand the document over at all. A byte-size +/// cap is deliberately NOT added: file size is already bounded by the UDF read +/// path, and a size cap does not close this class (a 180 KB document already +/// aborts) — depth is the bound that matters. +/// +/// This is a conservative scan, not a validator: it tracks `<name …>` / +/// `</name>` while skipping comments, CDATA, processing instructions and +/// declarations, and treats `<… />` as non-nesting. A miscount can only cost a +/// pathological document its fast path, and the failure mode is the existing +/// one — fall back to the clip-name heuristic. +fn xpl_depth_within_limit(text: &str) -> bool { + let b = text.as_bytes(); + let mut i = 0usize; + let mut depth = 0usize; + // Index just past `needle`, or the end of input when unterminated. + let after = |from: usize, needle: &[u8]| -> usize { + b[from..] + .windows(needle.len()) + .position(|w| w == needle) + .map_or(b.len(), |p| from + p + needle.len()) + }; + while i < b.len() { + if b[i] != b'<' { + i += 1; + continue; + } + match b.get(i + 1) { + Some(b'/') => { + depth = depth.saturating_sub(1); + i = after(i, b">"); + } + Some(b'?') => i = after(i, b"?>"), + Some(b'!') if b[i..].starts_with(b"<!--") => i = after(i, b"-->"), + Some(b'!') if b[i..].starts_with(b"<![CDATA[") => i = after(i, b"]]>"), + // `<!DOCTYPE …>` and friends. An internal subset is not tracked; + // `allow_dtd` is off by default, so such a document is rejected by + // the parser anyway. + Some(b'!') => i = after(i, b">"), + _ => { + // Element start tag: walk to its `>`, ignoring `>` inside + // quoted attribute values, and note whether it self-closes. + let mut j = i + 1; + let mut quote = 0u8; + let mut prev = 0u8; + while j < b.len() { + let c = b[j]; + if quote != 0 { + if c == quote { + quote = 0; + } + } else if c == b'"' || c == b'\'' { + quote = c; + } else if c == b'>' { + break; + } + prev = c; + j += 1; + } + if prev != b'/' { + depth += 1; + if depth > MAX_XPL_DEPTH { + return false; + } + } + i = j + 1; + } + } + } + true +} + /// Parse the Advanced-Content playlist into its titles. Elements are matched by /// LOCAL name (the document is in the `HDDVDVideo/Playlist` default namespace). -/// Returns empty for a non-XML / non-playlist blob so the caller falls back to -/// the clip-name heuristic. +/// Returns empty for a non-XML / non-playlist blob, or one nested deeper than +/// [`MAX_XPL_DEPTH`], so the caller falls back to the clip-name heuristic. fn parse_xpl_titles(xpl: &[u8]) -> Vec<XplTitle> { let text = String::from_utf8_lossy(xpl); + if !xpl_depth_within_limit(&text) { + return Vec::new(); + } let Ok(doc) = roxmltree::Document::parse(&text) else { return Vec::new(); }; @@ -404,7 +670,11 @@ fn parse_xpl_titles(xpl: &[u8]) -> Vec<XplTitle> { .unwrap_or(60); let mut titles = Vec::new(); - for tnode in doc.descendants().filter(|n| local(n, "Title")) { + for tnode in doc + .descendants() + .filter(|n| local(n, "Title")) + .take(MAX_XPL_TITLES) + { let number = tnode .attribute("titleNumber") .and_then(|s| s.trim().parse::<u16>().ok()) @@ -424,6 +694,19 @@ fn parse_xpl_titles(xpl: &[u8]) -> Vec<XplTitle> { .descendants() .filter(|n| local(n, "PrimaryAudioVideoClip")) { + // Bounded by MAX_XPL_CLIPS_PER_TITLE. `descendants()` is deliberate + // — a real playlist wraps its clips in `<PrimaryAudioVideoClipList>` + // or an `ApplicationSegment`, so `children()` would miss them — but + // it also means a clip nested inside N ancestor `<Title>`s is + // collected N times over, and the depth guard still allows 32 such + // ancestors. `break` rather than dropping surplus: unlike the + // directory scan (which must keep reading to find the `.vti`), + // nothing later in THIS loop is needed, and stopping bounds the walk + // as well as the allocation. Chapters are collected by a separate + // iterator below, so they are unaffected. + if clips.len() >= MAX_XPL_CLIPS_PER_TITLE { + break; + } let Some(evo) = c.attribute("src").and_then(evo_from_src) else { continue; }; @@ -445,6 +728,8 @@ fn parse_xpl_titles(xpl: &[u8]) -> Vec<XplTitle> { continue; } + // Bounded by MAX_XPL_CHAPTERS_PER_TITLE — the same `descendants()` + // amplification as the clip loop above, on the same crafted playlist. let chapters = tnode .descendants() .filter(|n| local(n, "Chapter")) @@ -452,6 +737,7 @@ fn parse_xpl_titles(xpl: &[u8]) -> Vec<XplTitle> { ch.attribute("titleTimeBegin") .and_then(|s| parse_timecode(s, tick_base)) }) + .take(MAX_XPL_CHAPTERS_PER_TITLE) .collect(); titles.push(XplTitle { @@ -484,18 +770,43 @@ fn compose_xpl_titles( reader: &mut dyn SectorSource, xpl_titles: &[XplTitle], clip_extents: &BTreeMap<String, (String, u64, Vec<Extent>)>, -) -> Vec<DiscTitle> { + unusable: &std::collections::HashSet<String>, + halt: Option<&crate::halt::Halt>, +) -> Result<Vec<DiscTitle>> { let mut titles = Vec::new(); + // One memo for the whole playlist: a playlist legitimately carries several + // titles over the same clip (angles, a branch, a seamless-join variant), and + // a crafted one can name a single `.evo` from all MAX_XPL_TITLES titles. + let mut probes = EvoProbeCache::default(); for t in xpl_titles { + // A clip that exists but has no truthful read plan poisons every + // title that names it: composing around it would emit a title short + // by that clip's bytes while its durations and chapter offsets still + // assume them. + if t.clips.iter().any(|c| unusable.contains(&c.evo)) { + continue; + } let mut extents = Vec::new(); let mut size_bytes = 0u64; let mut parts = Vec::new(); + // A crafted `<PrimaryAudioVideoClip>` list can name the same `.evo` + // any number of times (XML has no fixed element count, unlike MPLS's + // binary PlayItem count). Mirrors bluray.rs's + // `first_ref = seen_clips.insert(...)` gate (`:113`/`:117`): push a + // clip's physical extents and size only the first time its key is + // seen, so a clip named N times contributes its extent list ONCE + // instead of N times over. The analogous key here is the `.evo` + // filename — this format has no clip_id, and `.evo` is what + // `clip_extents` is already keyed by. + let mut seen_evos: std::collections::HashSet<&str> = std::collections::HashSet::new(); for c in &t.clips { let Some((orig, size, exts)) = clip_extents.get(&c.evo) else { continue; }; - extents.extend_from_slice(exts); - size_bytes += *size; + if seen_evos.insert(c.evo.as_str()) { + extents.extend_from_slice(exts); + size_bytes = size_bytes.saturating_add(*size); + } parts.push(Clip { feed_span: None, clip_id: orig @@ -512,7 +823,7 @@ fn compose_xpl_titles( if parts.is_empty() { continue; } - let streams = probe_evo_streams(reader, &extents); + let streams = probes.streams(reader, &extents, halt)?; let chapters = t .chapters .iter() @@ -541,7 +852,7 @@ fn compose_xpl_titles( codec_privates: Vec::new(), }); } - titles + Ok(titles) } impl Disc { @@ -555,12 +866,22 @@ impl Disc { /// its own title. Falls back to one title per clip when the VTI is absent or /// unparseable, so a disc with no readable navigation still enumerates. /// `chapters`/duration are left empty pending deeper VTI parsing. + /// Cancellation: `halt` is polled once per clip and again before every + /// probe chunk, and a read that fails with [`Error::Halted`] — how a live + /// drive reports a Stop, since `Drive::checked_exec` fails every command + /// once its flag is set — is propagated rather than swallowed. `Halted` is + /// the only error this returns; every other read failure keeps its + /// best-effort behaviour. It has to be an error and not a short title + /// list: a cancelled enumeration that returned `Ok` would be + /// indistinguishable from a disc that genuinely holds fewer titles, and + /// the caller would cache and act on it. pub(super) fn scan_hddvd_titles( reader: &mut dyn SectorSource, udf_fs: &udf::UdfFs, - ) -> Vec<DiscTitle> { + halt: Option<&crate::halt::Halt>, + ) -> Result<Vec<DiscTitle>> { let Some(ts_dir) = udf_fs.find_dir("/HVDVD_TS") else { - return Vec::new(); + return Ok(Vec::new()); }; // Snapshot clips (name, size) and the VTI navigation file. The `ts_dir` // borrow must end before the `udf_fs` reads below re-borrow it. @@ -572,7 +893,14 @@ impl Disc { } let lower = e.name.to_ascii_lowercase(); if lower.ends_with(HDDVD_CLIP_EXT) { - clips.push((e.name.clone(), e.size)); + // Bounded by MAX_HDDVD_CLIPS: each clip taken here costs an ICB + // read to resolve its extents and (in the fallback path below) a + // full probe_evo_streams pass. Surplus entries are DROPPED rather + // than `break`ing the loop, so a crafted disc cannot hide the + // `.vti` navigation file behind a wall of `.evo` names. + if clips.len() < MAX_HDDVD_CLIPS { + clips.push((e.name.clone(), e.size)); + } } else if lower.ends_with(".vti") && vti_name.is_none() { vti_name = Some(e.name.clone()); } @@ -586,17 +914,50 @@ impl Disc { // Resolve each clip's physical extents once, keyed by lower-case name. let mut clip_extents: BTreeMap<String, (String, u64, Vec<Extent>)> = BTreeMap::new(); + // Clips that EXIST but cannot be given a truthful read plan, because + // the file carries an unrecorded (never-written) extent — see + // `UdfFs::file_extents`. Kept apart from "did not resolve": a title + // built without one of these is short by that clip's runtime while + // still claiming its duration and size, which is data loss shaped + // like an ordinary rip. Any title naming one is dropped instead. + let mut unusable: std::collections::HashSet<String> = std::collections::HashSet::new(); for (name, size) in &clips { + if halt.is_some_and(|h| h.is_cancelled()) { + return Err(crate::error::Error::Halted); + } let mut extents = Vec::new(); - if let Ok(file_exts) = udf_fs.file_extents(reader, &format!("/HVDVD_TS/{name}")) { - for (lba, sectors) in file_exts { - if sectors > 0 && lba > 0 { - extents.push(Extent { - start_lba: lba, - sector_count: sectors, - }); + match udf_fs.file_extents(reader, &format!("/HVDVD_TS/{name}")) { + Ok(file_exts) => { + for (lba, sectors) in file_exts { + if sectors > 0 && lba > 0 { + extents.push(Extent { + start_lba: lba, + sector_count: sectors, + }); + } } } + Err(crate::error::Error::UdfUnrecordedExtent { .. }) => { + // Say so. Marking the clip unusable drops every title that + // names it, and in the fallback case the OTHER part of a + // split feature is still offered standalone — so half a + // movie is presented as a whole one. Silently returning Ok + // with the feature missing is the shape of data loss this + // refusal exists to prevent, pointed the other way. + // + // The Blu-ray half of this same fix already warns; this + // file had no diagnostics at all. Logging is exempt from + // the crate's no-English rule (errors stay numeric: E6017). + tracing::warn!( + target: "freemkv::disc", + clip = ?name, + code = 6017, + "clip carries an unrecorded extent; dropping every title that names it" + ); + unusable.insert(name.to_ascii_lowercase()); + } + Err(crate::error::Error::Halted) => return Err(crate::error::Error::Halted), + Err(_) => {} } if !extents.is_empty() { clip_extents.insert(name.to_ascii_lowercase(), (name.clone(), *size, extents)); @@ -609,24 +970,47 @@ impl Disc { // clip-name heuristic below is the fallback when the playlist is absent // or unparseable (or resolves to no on-disc clips). if let Some(xpl) = read_adv_obj_xpl(reader, udf_fs) { - let composed = compose_xpl_titles(reader, &parse_xpl_titles(&xpl), &clip_extents); + let composed = compose_xpl_titles( + reader, + &parse_xpl_titles(&xpl), + &clip_extents, + &unusable, + halt, + )?; if !composed.is_empty() { - return composed; + return Ok(composed); } } // Feature clips, in authored order, that actually resolved to extents. - let feature: Vec<String> = order + // If ANY authored feature part has no truthful read plan, the composed + // feature title would silently omit that part's bytes — emit no + // composed feature at all rather than a short one. (The per-clip + // titles below are unaffected: each stands alone and an unusable clip + // simply yields no title.) + let feature: Vec<String> = if order .iter() .filter(|n| is_feature_clip(n)) - .filter(|n| clip_extents.contains_key(&n.to_ascii_lowercase())) - .cloned() - .collect(); + .any(|n| unusable.contains(&n.to_ascii_lowercase())) + { + Vec::new() + } else { + order + .iter() + .filter(|n| is_feature_clip(n)) + .filter(|n| clip_extents.contains_key(&n.to_ascii_lowercase())) + .cloned() + .collect() + }; let feature_set: std::collections::HashSet<String> = feature.iter().map(|n| n.to_ascii_lowercase()).collect(); let mut titles = Vec::new(); let mut next_id = 0u16; + // One memo across the composed feature title and every per-clip title: + // nothing de-duplicates a FID's ICB LBA, so any number of directory + // entries can name ONE File Entry and resolve to identical extents. + let mut probes = EvoProbeCache::default(); // The composed feature title: concatenate its parts' extents in authored // order. Streams are probed from the head (the first part). One `Clip` per @@ -638,7 +1022,7 @@ impl Disc { for n in &feature { if let Some((orig, size, exts)) = clip_extents.get(&n.to_ascii_lowercase()) { extents.extend_from_slice(exts); - size_bytes += *size; + size_bytes = size_bytes.saturating_add(*size); parts.push(Clip { feed_span: None, clip_id: orig @@ -653,7 +1037,7 @@ impl Disc { }); } } - let streams = probe_evo_streams(reader, &extents); + let streams = probes.streams(reader, &extents, halt)?; titles.push(DiscTitle { playlist: "FEATURE".to_string(), playlist_id: next_id, @@ -672,6 +1056,9 @@ impl Disc { // Every remaining clip is its own title (unchanged behaviour). Iterated in // directory order; when there is no VTI/feature this emits ALL clips. for (name, _size) in &clips { + if halt.is_some_and(|h| h.is_cancelled()) { + return Err(crate::error::Error::Halted); + } let key = name.to_ascii_lowercase(); if feature_set.contains(&key) { continue; @@ -681,7 +1068,7 @@ impl Disc { }; // Probe the clip head for its elementary streams so the mux path // builds a non-empty `pid_to_track` and actually routes packets. - let streams = probe_evo_streams(reader, extents); + let streams = probes.streams(reader, extents, halt)?; let clip_id = orig .rsplit_once('.') .map(|(base, _)| base.to_string()) @@ -707,7 +1094,7 @@ impl Disc { }); next_id = next_id.saturating_add(1); } - titles + Ok(titles) } } @@ -725,7 +1112,13 @@ mod tests { .iter() .enumerate() .map(|(i, (name, sectors, data_lba))| { - file(name, 100 + i as u32, *data_lba, sectors * 2048, true) + file( + name, + 100 + i as u32, + *data_lba, + u64::from(*sectors) * 2048, + true, + ) }) .collect(); let root = DirSpec { @@ -755,7 +1148,7 @@ mod tests { &mut disc, &[("FEATURE.EVO", 2000, 5000), ("BLOOP.EVO", 300, 9000)], ); - let titles = Disc::scan_hddvd_titles(&mut disc, &udf); + let titles = Disc::scan_hddvd_titles(&mut disc, &udf, None).expect("scan"); assert_eq!(titles.len(), 2, "one title per .evo clip"); for t in &titles { assert_eq!( @@ -874,6 +1267,58 @@ mod tests { assert!(!is_feature_clip("EPK.EVO")); } + /// The SECOND size accumulator. `compose_xpl_titles` sums an XPL's clip + /// sizes; this one sums the composed FEATURE title's parts, and it reads + /// the same disc-declared `u64` Information Length from the same + /// `clip_extents` map. A split feature whose two parts each declare a size + /// near `u64::MAX` overflows it exactly as the XPL path did. + /// + /// Nothing reconciles the declared size against the extents that back it — + /// here each part declares `u64::MAX` while occupying ten sectors — because + /// the allocation descriptor's length field is only 32 bits wide, so the + /// disc can always claim more than it holds. + /// + /// Mutation: restore `size_bytes += *size` in the feature-composition path + /// and this goes red (debug: attempt to add with overflow). + #[test] + fn scan_hddvd_feature_composition_saturates_absurd_disc_declared_sizes() { + let mut disc = MemDisc::new(); + let vti = synthetic_vti(&["FEATURE_1.EVO", "FEATURE_2.EVO"]); + let files = vec![ + file("FEATURE_1.EVO", 100, 5000, u64::MAX, true), + file("FEATURE_2.EVO", 101, 8000, u64::MAX, true), + file_with("HVA00001.VTI", 103, 15000, vti, true), + ]; + let root = DirSpec { + name: String::new(), + icb_lba: 10, + dir_data_lba: 11, + files: Vec::new(), + subdirs: vec![DirSpec { + name: "HVDVD_TS".to_string(), + icb_lba: 20, + dir_data_lba: 21, + files, + subdirs: vec![], + }], + }; + build_udf_skeleton(&mut disc, 10); + lay_dir(&mut disc, &root); + let udf = crate::udf::read_filesystem(&mut disc).expect("fs"); + + let titles = Disc::scan_hddvd_titles(&mut disc, &udf, None) + .expect("a hostile size field must not fail the scan"); + let feat = titles + .iter() + .find(|t| t.playlist == "FEATURE") + .expect("composed feature title"); + assert_eq!( + feat.size_bytes, + u64::MAX, + "the part sizes must saturate at u64::MAX, never wrap to a small number" + ); + } + #[test] fn scan_hddvd_composes_split_feature_into_one_title() { // A disc whose feature is FEATURE_1 + FEATURE_2 (a layer-break split), plus @@ -905,7 +1350,7 @@ mod tests { lay_dir(&mut disc, &root); let udf = crate::udf::read_filesystem(&mut disc).expect("fs"); - let titles = Disc::scan_hddvd_titles(&mut disc, &udf); + let titles = Disc::scan_hddvd_titles(&mut disc, &udf, None).expect("scan"); // One composed FEATURE title + the trailer. assert_eq!(titles.len(), 2, "feature parts merged, trailer separate"); let feat = titles @@ -927,6 +1372,73 @@ mod tests { assert!(titles.iter().any(|t| t.playlist == "TRAILER.EVO")); } + /// A split feature (FEATURE_1 + FEATURE_2) where ONE part carries an + /// unrecorded extent must not be composed into a FEATURE title at all. + /// + /// Composing around the unusable part is the dangerous outcome: the + /// title would play through as if it were the whole movie while silently + /// missing that part's runtime, and its size still counts it. The other + /// clips remain available as their own titles — refusing the composition + /// is not the same as refusing the disc. + #[test] + fn scan_hddvd_does_not_compose_a_feature_over_an_unrecorded_part() { + let mut disc = MemDisc::new(); + let vti = synthetic_vti(&["FEATURE_1.EVO", "FEATURE_2.EVO", "TRAILER.EVO"]); + let files = vec![ + file("FEATURE_1.EVO", 100, 5000, 10 * 2048, true), + file("FEATURE_2.EVO", 101, 8000, 6 * 2048, true), + file("TRAILER.EVO", 102, 12000, 2 * 2048, true), + file_with("HVA00001.VTI", 103, 15000, vti, true), + ]; + let root = DirSpec { + name: String::new(), + icb_lba: 10, + dir_data_lba: 11, + files: Vec::new(), + subdirs: vec![DirSpec { + name: "HVDVD_TS".to_string(), + icb_lba: 20, + dir_data_lba: 21, + files, + subdirs: vec![], + }], + }; + build_udf_skeleton(&mut disc, 10); + lay_dir(&mut disc, &root); + // FEATURE_2's ICB (laid at PART_START + 101): an unrecorded 2048-byte + // extent at LBA 7999, then its real content at 8000. + let mut icb = build_file_icb(6 * 2048, 8000, false); + icb[212..216].copy_from_slice(&16u32.to_le_bytes()); + icb[216..220].copy_from_slice(&0x4000_0800u32.to_le_bytes()); // type 1 + icb[220..224].copy_from_slice(&7999u32.to_le_bytes()); + icb[224..228].copy_from_slice(&(6u32 * 2048).to_le_bytes()); // type 0 + icb[228..232].copy_from_slice(&8000u32.to_le_bytes()); + disc.put_bytes(PART_START + 101, &icb); + let udf = crate::udf::read_filesystem(&mut disc).expect("fs"); + + let titles = Disc::scan_hddvd_titles(&mut disc, &udf, None).expect("scan"); + assert!( + !titles.iter().any(|t| t.playlist == "FEATURE"), + "a feature part with no truthful read plan must not be composed \ + into a title that silently omits it; got {:?}", + titles + .iter() + .map(|t| (&t.playlist, t.extents.len())) + .collect::<Vec<_>>() + ); + // The clips that DID resolve are still offered on their own. + assert!( + titles.iter().any(|t| t.playlist == "TRAILER.EVO"), + "refusing the composition must not refuse the healthy clips" + ); + assert!( + titles + .iter() + .all(|t| t.extents.iter().all(|e| e.start_lba != PART_START + 7999)), + "no title may read the unrecorded extent" + ); + } + // ── codec sniffing ──────────────────────────────────────────────────── #[test] @@ -1067,6 +1579,51 @@ mod tests { crate::udf::read_filesystem(disc).expect("fs") } + /// An `.evo` whose ICB declares an UNRECORDED (ECMA-167 4/14.14.1.1 + /// type-1) extent must yield NO title. + /// + /// Those sectors were never written, so the clip's real content there is + /// zeros while the media holds whatever was left behind. Feeding them to + /// the mux as stream bytes splices undefined data into the middle of the + /// ripped `.evo`; leaving them out slides the rest of the file. Neither is + /// a rip, so the title is not offered at all — data loss must never look + /// like success. + #[test] + fn scan_hddvd_titles_refuses_a_clip_with_an_unrecorded_extent() { + let mut disc = MemDisc::new(); + let udf = make_hddvd_fs_with_evo(&mut disc, &synthetic_evo()); + + // Control: unpatched, this fixture really does produce a title — so a + // later empty result means the hole was rejected, not that the + // fixture was inert. + assert_eq!( + Disc::scan_hddvd_titles(&mut disc, &udf, None) + .expect("scan") + .len(), + 1, + "fixture must yield a title before the hole is introduced" + ); + + // Rewrite FEATURE.EVO's ICB (laid at PART_START + 100) with a + // two-descriptor short-AD list: an unrecorded 2048-byte extent at LBA + // 4999, then the real 4096-byte content at 5000. + let mut icb = build_file_icb(4096, 5000, false); + icb[212..216].copy_from_slice(&16u32.to_le_bytes()); // l_ad: two short ADs + icb[216..220].copy_from_slice(&0x4000_0800u32.to_le_bytes()); // type 1, 2048 bytes + icb[220..224].copy_from_slice(&4999u32.to_le_bytes()); + icb[224..228].copy_from_slice(&4096u32.to_le_bytes()); // type 0, 4096 bytes + icb[228..232].copy_from_slice(&5000u32.to_le_bytes()); + disc.put_bytes(PART_START + 100, &icb); + + let titles = Disc::scan_hddvd_titles(&mut disc, &udf, None).expect("scan"); + assert!( + titles.is_empty(), + "a clip with an unrecorded extent has no truthful read plan; got \ + extents {:?}", + titles.iter().map(|t| &t.extents).collect::<Vec<_>>() + ); + } + /// End-to-end: scanning an `.evo` whose head carries an H.264 video PES and /// two DD+ audio PES yields a title with the video track (canonical /// DVD_VIDEO_PID) and both DD+ tracks (0xBDC0 / 0xBDC1) — the non-empty @@ -1075,7 +1632,7 @@ mod tests { fn scan_hddvd_titles_probes_streams_from_evo_head() { let mut disc = MemDisc::new(); let udf = make_hddvd_fs_with_evo(&mut disc, &synthetic_evo()); - let titles = Disc::scan_hddvd_titles(&mut disc, &udf); + let titles = Disc::scan_hddvd_titles(&mut disc, &udf, None).expect("scan"); assert_eq!(titles.len(), 1); let t = &titles[0]; @@ -1118,7 +1675,7 @@ mod tests { // 4 KiB of junk with no PS start codes. let junk = vec![0x55u8; 4096]; let udf = make_hddvd_fs_with_evo(&mut disc, &junk); - let titles = Disc::scan_hddvd_titles(&mut disc, &udf); + let titles = Disc::scan_hddvd_titles(&mut disc, &udf, None).expect("scan"); assert_eq!(titles.len(), 1); assert!( titles[0].streams.is_empty(), @@ -1217,7 +1774,7 @@ mod tests { fn scan_hddvd_titles_probes_vc1_on_extended_stream_id() { let mut disc = MemDisc::new(); let udf = make_hddvd_fs_with_evo(&mut disc, &synthetic_evo_vc1()); - let titles = Disc::scan_hddvd_titles(&mut disc, &udf); + let titles = Disc::scan_hddvd_titles(&mut disc, &udf, None).expect("scan"); assert_eq!(titles.len(), 1); let video: Vec<_> = titles[0] @@ -1336,6 +1893,227 @@ mod tests { assert!(parse_xpl_titles(b"<Playlist></Playlist>").is_empty()); } + /// A disc-supplied playlist nested far deeper than any real one must be + /// REFUSED BEFORE the XML parser sees it. `roxmltree` is recursive-descent + /// and its depth-10 limit covers only ENTITY expansion, so unbounded element + /// nesting blows the thread stack — an ABORT, which no `catch_unwind` and no + /// `Err` fallback can contain. ~50k levels is a few hundred KB of well-formed + /// XML, far under the read cap. The correct outcome is the same as any other + /// unusable playlist: an empty `Vec`, so the caller falls back to the + /// clip-name heuristic. + /// BREADTH, not depth: a flat playlist passes the nesting guard trivially + /// and can still declare a title per ~51 bytes, so a 64 MiB XPL (exactly + /// `udf::MAX_FILE_BYTES`) reaches ~1.3 million of them. + /// + /// The cost is drive time, not memory: `compose_xpl_titles` runs + /// `probe_evo_streams` once per title and each probe reads real sectors, + /// so an uncapped count turns a scan the operator expects to take seconds + /// into tens of terabytes of optical reads. `bluray.rs` is not exposed to + /// this because its playlist count comes from the MPLS files present in + /// the UDF directory; an XPL DECLARES its titles, so nothing bounds them. + /// + /// Mutation: delete the `.take(MAX_XPL_TITLES)` and this goes red at the + /// declared count. + #[test] + fn parse_xpl_titles_caps_a_playlist_declaring_absurdly_many_titles() { + const DECLARED: usize = MAX_XPL_TITLES + 500; + let mut xpl = String::with_capacity(DECLARED * 56 + 128); + xpl.push_str(r#"<?xml version="1.0" encoding="utf-8"?><Playlist><TitleSet>"#); + for _ in 0..DECLARED { + xpl.push_str(r#"<Title><PrimaryAudioVideoClip src="A.EVO"/>"#); + } + xpl.push_str(""); + + let titles = parse_xpl_titles(xpl.as_bytes()); + assert_eq!( + titles.len(), + MAX_XPL_TITLES, + "a playlist declaring {DECLARED} titles must be capped at \ + {MAX_XPL_TITLES}; each surviving title costs a real probe_evo_streams \ + pass over the medium" + ); + } + + /// The control: a realistic playlist keeps every title it declares. + #[test] + fn parse_xpl_titles_keeps_every_title_of_a_realistic_playlist() { + let titles = parse_xpl_titles(SYNTH_XPL.as_bytes()); + assert!( + !titles.is_empty() && titles.len() < MAX_XPL_TITLES, + "the synthetic real-world playlist must survive the cap untouched, \ + got {} titles", + titles.len() + ); + } + + /// Build a playlist that nests `nesting` `` elements inside one + /// another and puts `clips` self-closing `<PrimaryAudioVideoClip>` elements + /// at the innermost level, plus `chapters` `<Chapter>` elements. + /// + /// `parse_xpl_titles` collects a title's clips with `descendants()`, not + /// `children()`, so EVERY one of the `nesting` ancestors collects the SAME + /// innermost clip list — the amplification this exercises. + fn nested_title_xpl(nesting: usize, clips: usize, chapters: usize) -> String { + let mut xpl = String::with_capacity(nesting * 16 + clips * 40 + chapters * 40 + 128); + xpl.push_str(r#"<?xml version="1.0" encoding="utf-8"?><Playlist><TitleSet>"#); + for _ in 0..nesting { + xpl.push_str("<Title>"); + } + for _ in 0..clips { + xpl.push_str(r#"<PrimaryAudioVideoClip src="A.EVO"/>"#); + } + for _ in 0..chapters { + xpl.push_str(r#"<Chapter titleTimeBegin="00:00:01:00"/>"#); + } + for _ in 0..nesting { + xpl.push_str(""); + } + xpl.push_str(""); + xpl + } + + /// CLIPS PER TITLE — the axis the title cap, the depth cap and the + /// directory cap all leave open. + /// + /// `MAX_XPL_TITLES` bounds how many titles a playlist may declare and + /// `MAX_XPL_DEPTH` bounds how deeply it may nest, but NEITHER bounds how + /// many `` elements a single title collects. Worse, + /// the collector is `descendants()`, so one clip nested inside N ancestor + /// ``s is collected N times over — the clip count MULTIPLIES by the + /// nesting the depth guard still permits. + /// + /// A 64 MiB XPL (`udf::MAX_FILE_BYTES`) holds ~1.7M clip elements at ~38 + /// bytes each; with the 32 `<Title>` ancestors the depth cap still permits, + /// that is ~56 million heap-allocated `XplClip`s, and then as many `Clip`s + /// again in `compose_xpl_titles`. This uses small numbers that exercise the + /// same multiplication without a 64 MiB fixture: pre-fix it collected 756 + /// clips per title, 18,900 across the 25 nested titles, from a document + /// declaring only 756 clip elements — an exact 25x, the nesting count. + /// + /// Mutation: delete the `clips.len() >= MAX_XPL_CLIPS_PER_TITLE` break and + /// this goes red at the unamplified per-title count. + #[test] + fn parse_xpl_titles_caps_clips_per_title_against_descendant_amplification() { + const NESTING: usize = 25; + const CLIPS: usize = MAX_XPL_CLIPS_PER_TITLE + 500; + + let xpl = nested_title_xpl(NESTING, CLIPS, 0); + let titles = parse_xpl_titles(xpl.as_bytes()); + + // The nesting itself must survive the depth guard, or this test would + // pass for the wrong reason (an empty fallback). + assert_eq!( + titles.len(), + NESTING, + "all {NESTING} nested <Title> elements must parse, so the \ + amplification is really being exercised" + ); + + let total: usize = titles.iter().map(|t| t.clips.len()).sum(); + for t in &titles { + assert!( + t.clips.len() <= MAX_XPL_CLIPS_PER_TITLE, + "a title collected {} clips, above the {MAX_XPL_CLIPS_PER_TITLE} \ + cap; total across titles {total} (a 64 MiB XPL scales this to \ + tens of millions)", + t.clips.len() + ); + } + assert!( + total <= NESTING * MAX_XPL_CLIPS_PER_TITLE, + "aggregate clip count {total} must be bounded by titles x cap" + ); + } + + /// CHAPTERS PER TITLE — the same `descendants()` amplification on the + /// second unbounded `collect()` in the same loop. Bounded separately so + /// closing the clip axis does not simply move the attack next door. + /// + /// Mutation: delete the `.take(MAX_XPL_CHAPTERS_PER_TITLE)` and this goes + /// red at the declared count. + #[test] + fn parse_xpl_titles_caps_chapters_per_title() { + const NESTING: usize = 25; + const CHAPTERS: usize = MAX_XPL_CHAPTERS_PER_TITLE + 500; + + // One clip, so the title is kept (`clips.is_empty()` skips otherwise). + let xpl = nested_title_xpl(NESTING, 1, CHAPTERS); + let titles = parse_xpl_titles(xpl.as_bytes()); + assert_eq!(titles.len(), NESTING); + + for t in &titles { + assert!( + t.chapters.len() <= MAX_XPL_CHAPTERS_PER_TITLE, + "a title collected {} chapters, above the \ + {MAX_XPL_CHAPTERS_PER_TITLE} cap", + t.chapters.len() + ); + } + } + + /// The control: a realistic multi-clip title still resolves EVERY one of its + /// clips and chapters. `SYNTH_XPL`'s main movie is a layer-break split — two + /// clips on one timeline — plus two chapters; losing either to the cap would + /// cost a genuine disc half its feature. + /// + /// Mutation: set `MAX_XPL_CLIPS_PER_TITLE` to 1 and this goes red. + #[test] + fn parse_xpl_titles_keeps_every_clip_of_a_realistic_title() { + let titles = parse_xpl_titles(SYNTH_XPL.as_bytes()); + assert_eq!(titles.len(), 2); + assert_eq!( + titles[0].clips.len(), + 2, + "the layer-break split must keep BOTH clips through the cap" + ); + assert_eq!(titles[0].clips[0].evo, "feature_1.evo"); + assert_eq!(titles[0].clips[1].evo, "feature_2.evo"); + assert_eq!( + titles[0].chapters.len(), + 2, + "both chapters must survive the chapter cap" + ); + assert_eq!(titles[1].clips.len(), 1); + } + + #[test] + fn parse_xpl_titles_refuses_deeply_nested_playlist() { + const DEPTH: usize = 50_000; + let mut xpl = String::with_capacity(DEPTH * 8 + 64); + xpl.push_str(r#"<?xml version="1.0" encoding="utf-8"?>"#); + for _ in 0..DEPTH { + xpl.push_str("<n>"); + } + for _ in 0..DEPTH { + xpl.push_str("</n>"); + } + assert!( + parse_xpl_titles(xpl.as_bytes()).is_empty(), + "a hostile nesting depth must fall back, not abort the process" + ); + } + + /// The depth guard must not reject real discs. A genuine `VPLST000.XPL` is a + /// handful of levels deep; this is the same document the parsing test uses, + /// plus self-closing tags, a comment and a processing instruction that the + /// pre-parse scanner has to account for without inflating its depth count. + #[test] + fn parse_xpl_titles_accepts_real_world_nesting_depth() { + let titles = parse_xpl_titles(SYNTH_XPL.as_bytes()); + assert_eq!(titles.len(), 2, "a real playlist still parses"); + assert_eq!(titles[0].clips.len(), 2); + + // Comments and processing instructions carry `<` and `/>` that a naive + // scanner would miscount as element nesting. + let decl = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>"; + let tricky = SYNTH_XPL.replacen( + decl, + &format!("{decl}\n<!-- <a><a><a><a><a> -->\n<?authoring <b><b><b> ?>"), + 1, + ); + assert_eq!(parse_xpl_titles(tricky.as_bytes()).len(), 2); + } + /// Build a UDF with `HVDVD_TS/` `.evo` clips plus an `ADV_OBJ/VPLST000.XPL` /// carrying `xpl`, so `scan_hddvd_titles` takes the playlist path. fn make_hddvd_fs_xpl( @@ -1349,7 +2127,13 @@ mod tests { .iter() .enumerate() .map(|(i, (name, sectors, data_lba))| { - file(name, 100 + i as u32, *data_lba, sectors * 2048, true) + file( + name, + 100 + i as u32, + *data_lba, + u64::from(*sectors) * 2048, + true, + ) }) .collect(); let root = DirSpec { @@ -1395,7 +2179,7 @@ mod tests { ], SYNTH_XPL.as_bytes(), ); - let titles = Disc::scan_hddvd_titles(&mut disc, &udf); + let titles = Disc::scan_hddvd_titles(&mut disc, &udf, None).expect("scan"); assert_eq!( titles.len(), 2, @@ -1643,6 +2427,64 @@ mod tests { ); } + /// An Advanced-Content playlist title that names a clip with no truthful + /// read plan must be dropped whole, not composed from its other clips. + /// + /// The unusable clip is absent from `clip_extents` (it never resolved), + /// which on its own just makes the loop skip it — emitting a title that + /// plays short by that clip while its chapter offsets and durations still + /// assume it. The `unusable` set is what distinguishes "this clip does not + /// exist" from "this clip exists and cannot be ripped truthfully". + #[test] + fn compose_xpl_titles_drops_a_title_naming_an_unusable_clip() { + let clip_extents: BTreeMap<String, (String, u64, Vec<Extent>)> = [( + "a.evo".to_string(), + ( + "A.EVO".to_string(), + 1000u64, + vec![Extent { + start_lba: 1, + sector_count: 1, + }], + ), + )] + .into_iter() + .collect(); + // b.evo exists on the disc but carries an unrecorded extent. + let unusable: std::collections::HashSet<String> = + ["b.evo".to_string()].into_iter().collect(); + let xpl_titles = vec![XplTitle { + number: 1, + name: "T".to_string(), + duration_secs: 20.0, + clips: vec![ + XplClip { + evo: "a.evo".to_string(), + begin_secs: 0.0, + end_secs: 10.0, + }, + XplClip { + evo: "b.evo".to_string(), + begin_secs: 10.0, + end_secs: 20.0, + }, + ], + chapters: vec![], + }]; + let mut disc = MemDisc::new(); + let titles = compose_xpl_titles(&mut disc, &xpl_titles, &clip_extents, &unusable, None) + .expect("compose"); + assert!( + titles.is_empty(), + "half of this title's runtime cannot be read, so composing it \ + would ship a short title as a complete one; got {:?}", + titles + .iter() + .map(|t| (t.clips.len(), t.size_bytes)) + .collect::<Vec<_>>() + ); + } + // ── compose_xpl_titles: size/offset arithmetic ───────────────────────── /// Direct unit test of the arithmetic composing a title from its XPL @@ -1696,7 +2538,14 @@ mod tests { chapters: vec![], }]; let mut disc = MemDisc::new(); - let titles = compose_xpl_titles(&mut disc, &xpl_titles, &clip_extents); + let titles = compose_xpl_titles( + &mut disc, + &xpl_titles, + &clip_extents, + &std::collections::HashSet::new(), + None, + ) + .expect("compose"); assert_eq!(titles.len(), 1); let t = &titles[0]; assert_eq!(t.size_bytes, 3000, "clip sizes summed, not multiplied"); @@ -1722,6 +2571,242 @@ mod tests { ); } + /// A title's `size_bytes` is a running sum of DISC-DECLARED clip sizes: the + /// `u64` UDF File Entry Information Length, which nothing cross-checks + /// against the extent list that file actually occupies. Two clips can + /// therefore each declare a size near `u64::MAX`. + /// + /// Summed with a plain `+=`, that overflows: a panic in a debug build — + /// a crash on untrusted disc content, which this library must never do — + /// and a wrap to a small garbage total in release, which misreports the + /// title's size while looking entirely normal. + /// + /// Mutation: restore `size_bytes += *size` in `compose_xpl_titles` and this + /// goes red (debug: attempt to add with overflow). + #[test] + fn compose_xpl_titles_saturates_absurd_disc_declared_clip_sizes() { + let clip_extents: BTreeMap<String, (String, u64, Vec<Extent>)> = [ + ( + "a.evo".to_string(), + ( + "A.EVO".to_string(), + u64::MAX, + vec![Extent { + start_lba: 1, + sector_count: 1, + }], + ), + ), + ( + "b.evo".to_string(), + ( + "B.EVO".to_string(), + u64::MAX, + vec![Extent { + start_lba: 2, + sector_count: 1, + }], + ), + ), + ] + .into_iter() + .collect(); + let xpl_titles = vec![XplTitle { + number: 1, + name: "T".to_string(), + duration_secs: 10.0, + clips: vec![ + XplClip { + evo: "a.evo".to_string(), + begin_secs: 0.0, + end_secs: 5.0, + }, + XplClip { + evo: "b.evo".to_string(), + begin_secs: 5.0, + end_secs: 10.0, + }, + ], + chapters: vec![], + }]; + let mut disc = MemDisc::new(); + let titles = compose_xpl_titles( + &mut disc, + &xpl_titles, + &clip_extents, + &std::collections::HashSet::new(), + None, + ) + .expect("a hostile size field must not fail the scan either"); + assert_eq!(titles.len(), 1); + assert_eq!( + titles[0].size_bytes, + u64::MAX, + "the sum must saturate at u64::MAX, never wrap to a small number" + ); + } + + /// A crafted Advanced-Content playlist can name the SAME `.evo` in + /// `<PrimaryAudioVideoClip>` many times over (the XML has no fixed element + /// count, unlike MPLS's binary PlayItem count). Each repeat must NOT push + /// another copy of that clip's extent list onto the title — mirroring + /// bluray.rs's `first_ref = seen_clips.insert(...)` gate (`:113`/`:117`), + /// which pushes a clip's extents only the first time its id is seen. Here + /// the analogous key is the `.evo` filename (this file has no clip_id; + /// `.evo` is what `clip_extents` is keyed by). Without the gate, extents + /// grows by the referenced clip's full extent list on EVERY repetition — + /// unbounded by the number of on-disc files, bounded only by playlist + /// size. + #[test] + fn compose_xpl_titles_dedups_repeated_clip_references_by_evo() { + let clip_extents: BTreeMap<String, (String, u64, Vec<Extent>)> = [( + "a.evo".to_string(), + ( + "A.EVO".to_string(), + 1000u64, + vec![ + Extent { + start_lba: 1, + sector_count: 1, + }, + Extent { + start_lba: 2, + sector_count: 1, + }, + Extent { + start_lba: 3, + sector_count: 1, + }, + ], + ), + )] + .into_iter() + .collect(); + + const REPEATS: usize = 5000; + let clips: Vec<XplClip> = (0..REPEATS) + .map(|i| XplClip { + evo: "a.evo".to_string(), + begin_secs: i as f64, + end_secs: i as f64 + 1.0, + }) + .collect(); + let xpl_titles = vec![XplTitle { + number: 1, + name: "T".to_string(), + duration_secs: REPEATS as f64, + clips, + chapters: vec![], + }]; + let mut disc = MemDisc::new(); + let titles = compose_xpl_titles( + &mut disc, + &xpl_titles, + &clip_extents, + &std::collections::HashSet::new(), + None, + ) + .expect("compose"); + assert_eq!(titles.len(), 1); + assert_eq!( + titles[0].extents.len(), + 3, + "the same .evo named {REPEATS} times must contribute its 3 extents \ + ONCE, not {REPEATS} times over — got {} extents", + titles[0].extents.len() + ); + } + + /// Control for the de-dup above: a playlist naming several DISTINCT + /// clips (not repeats of one) must still resolve every one of them — + /// the de-dup key must not accidentally collapse different clips + /// together. If this test is made to fail by weakening the de-dup key + /// to something constant, that proves the key is doing real work rather + /// than vacuously deduping everything. + #[test] + fn compose_xpl_titles_resolves_all_distinct_clips_despite_dedup() { + let clip_extents: BTreeMap<String, (String, u64, Vec<Extent>)> = [ + ( + "a.evo".to_string(), + ( + "A.EVO".to_string(), + 1000u64, + vec![Extent { + start_lba: 1, + sector_count: 1, + }], + ), + ), + ( + "b.evo".to_string(), + ( + "B.EVO".to_string(), + 2000u64, + vec![Extent { + start_lba: 2, + sector_count: 1, + }], + ), + ), + ( + "c.evo".to_string(), + ( + "C.EVO".to_string(), + 3000u64, + vec![Extent { + start_lba: 3, + sector_count: 1, + }], + ), + ), + ] + .into_iter() + .collect(); + let xpl_titles = vec![XplTitle { + number: 1, + name: "T".to_string(), + duration_secs: 30.0, + clips: vec![ + XplClip { + evo: "a.evo".to_string(), + begin_secs: 0.0, + end_secs: 5.0, + }, + XplClip { + evo: "b.evo".to_string(), + begin_secs: 5.0, + end_secs: 15.0, + }, + XplClip { + evo: "c.evo".to_string(), + begin_secs: 15.0, + end_secs: 30.0, + }, + ], + chapters: vec![], + }]; + let mut disc = MemDisc::new(); + let titles = compose_xpl_titles( + &mut disc, + &xpl_titles, + &clip_extents, + &std::collections::HashSet::new(), + None, + ) + .expect("compose"); + assert_eq!(titles.len(), 1); + assert_eq!( + titles[0].extents.len(), + 3, + "three DISTINCT clips must all resolve — one extent each, not \ + collapsed by an over-eager de-dup key" + ); + assert_eq!( + titles[0].size_bytes, 6000, + "all three distinct sizes summed" + ); + } + // ── Disc::scan_hddvd_titles: VTI-selection / extent-filter guards ────── /// A file carrying the real `ADVANCED-VTS` magic but the WRONG extension @@ -1755,7 +2840,7 @@ mod tests { lay_dir(&mut disc, &root); let udf = crate::udf::read_filesystem(&mut disc).expect("fs"); - let titles = Disc::scan_hddvd_titles(&mut disc, &udf); + let titles = Disc::scan_hddvd_titles(&mut disc, &udf, None).expect("scan"); assert_eq!( titles.len(), 2, @@ -1797,7 +2882,7 @@ mod tests { lay_dir(&mut disc, &root); let udf = crate::udf::read_filesystem(&mut disc).expect("fs"); - let titles = Disc::scan_hddvd_titles(&mut disc, &udf); + let titles = Disc::scan_hddvd_titles(&mut disc, &udf, None).expect("scan"); assert_eq!( titles.len(), 1, @@ -1821,7 +2906,8 @@ mod tests { start_lba: 500_000, sector_count: 0, }; - let streams = probe_evo_streams(&mut disc, std::slice::from_ref(&extent)); + let streams = + probe_evo_streams(&mut disc, std::slice::from_ref(&extent), None).expect("probe"); assert!(streams.is_empty(), "a zero-sector extent yields no streams"); } @@ -1842,7 +2928,8 @@ mod tests { sector_count: 512 + (evo.len() as u32).div_ceil(2048), }; - let streams = probe_evo_streams(&mut disc, std::slice::from_ref(&extent)); + let streams = + probe_evo_streams(&mut disc, std::slice::from_ref(&extent), None).expect("probe"); let has_h264 = streams .iter() .any(|s| matches!(s, Stream::Video(v) if v.codec == Codec::H264)); @@ -1870,7 +2957,8 @@ mod tests { sector_count: declared_sectors, }; - let streams = probe_evo_streams(&mut disc, std::slice::from_ref(&extent)); + let streams = + probe_evo_streams(&mut disc, std::slice::from_ref(&extent), None).expect("probe"); let has_h264 = streams .iter() .any(|s| matches!(s, Stream::Video(v) if v.codec == Codec::H264)); @@ -1905,7 +2993,7 @@ mod tests { sector_count: 10, }, ]; - let streams = probe_evo_streams(&mut disc, &extents); + let streams = probe_evo_streams(&mut disc, &extents, None).expect("probe"); let has_h264 = streams .iter() .any(|s| matches!(s, Stream::Video(v) if v.codec == Codec::H264)); @@ -1914,4 +3002,300 @@ mod tests { "must not read past the total EVO_PROBE_SECTORS budget across extents" ); } + + // ── clip-name fallback: probe amplification ──────────────────────────── + + /// A [`SectorSource`] wrapper that counts how many reads START at each + /// watched LBA, plus the total sectors pulled off the medium. A probe of a + /// clip always issues its first read at the clip's first extent LBA, so the + /// hit count for that LBA IS the number of `probe_evo_streams` passes over + /// that clip. + struct ProbeCounter { + inner: MemDisc, + watch: Vec<u32>, + hits: Vec<u32>, + sectors_read: u64, + } + + impl ProbeCounter { + fn new(inner: MemDisc, watch: Vec<u32>) -> Self { + let hits = vec![0; watch.len()]; + Self { + inner, + watch, + hits, + sectors_read: 0, + } + } + } + + impl SectorSource for ProbeCounter { + fn read_sectors( + &mut self, + lba: u32, + count: u16, + buf: &mut [u8], + recovery: bool, + ) -> crate::error::Result<usize> { + if let Some(i) = self.watch.iter().position(|w| *w == lba) { + self.hits[i] += 1; + } + self.sectors_read += count as u64; + self.inner.read_sectors(lba, count, buf, recovery) + } + } + + /// Lay an `HVDVD_TS/` holding exactly the given `(name, icb_lba, data_lba, + /// size)` clips — unlike `make_hddvd_fs`, the caller chooses each entry's + /// ICB, so many names can be pointed at ONE File Entry. + fn lay_hddvd_clips(disc: &mut MemDisc, specs: Vec<crate::udf::fixture::FileSpec>) { + let root = DirSpec { + name: String::new(), + icb_lba: 10, + dir_data_lba: 11, + files: Vec::new(), + subdirs: vec![DirSpec { + name: "HVDVD_TS".to_string(), + icb_lba: 20, + dir_data_lba: 21, + files: specs, + subdirs: vec![], + }], + }; + build_udf_skeleton(disc, 10); + lay_dir(disc, &root); + } + + /// THE THIRD AMPLIFICATION AXIS on HD-DVD title composition, past both + /// existing guards. `xpl_depth_within_limit` bounds XML NESTING; + /// `MAX_XPL_TITLES` + the `.evo` de-dup bound what an XPL can DECLARE. Both + /// live on the playlist path — and an attacker simply omits `/ADV_OBJ`, so + /// `read_adv_obj_xpl` returns `None` and neither guard is ever consulted. + /// Control then reaches the clip-name FALLBACK, which emits one title per + /// `.evo` DIRECTORY ENTRY and probes each. + /// + /// Nothing de-duplicates a FID's ICB LBA, so every name in a 1 MiB + /// directory (`udf::MAX_DIR_BYTES`, ~24,000 FIDs) can point at ONE File + /// Entry. Each resolves to the SAME extents and each costs a full + /// `EVO_PROBE_SECTORS` (16 MiB) probe: ~375 GiB of optical reads from a + /// directory that describes a single file. + /// + /// Memoizing the probe on the resolved extent list collapses that to ONE + /// probe, because all of those names resolve to one extent list. + /// + /// Mutation: drop the memo lookup in `EvoProbeCache::streams` (always + /// probe) and this goes red at `ENTRIES` probes. + #[test] + fn scan_hddvd_titles_probes_once_for_entries_resolving_to_identical_extents() { + const ENTRIES: usize = 512; + const SHARED_ICB: u32 = 100; + const SHARED_DATA: u32 = 5000; + + let mut disc = MemDisc::new(); + // Every FID names a different file and points at the SAME File Entry. + let specs: Vec<_> = (0..ENTRIES) + .map(|i| { + file( + &format!("C{i:04}.EVO"), + SHARED_ICB, + SHARED_DATA, + 4 * 2048, + true, + ) + }) + .collect(); + lay_hddvd_clips(&mut disc, specs); + let udf = crate::udf::read_filesystem(&mut disc).expect("fs"); + + let probe_lba = PART_START + SHARED_DATA; + let mut counter = ProbeCounter::new(disc, vec![probe_lba]); + let titles = Disc::scan_hddvd_titles(&mut counter, &udf, None).expect("scan"); + + assert_eq!( + counter.hits[0], 1, + "{ENTRIES} directory entries all resolving to the SAME extents must \ + cost ONE probe_evo_streams pass, not one per entry; got {} probes", + counter.hits[0] + ); + // And the whole scan's read volume stays bounded — the real currency + // here is drive time, not title count. + assert!( + counter.sectors_read < 4 * ENTRIES as u64, + "total sectors read must not scale with the entry count, got {}", + counter.sectors_read + ); + assert!(!titles.is_empty(), "the clips still enumerate"); + } + + /// The clip cap and the per-probe read budget are ONE bound, not two, and + /// only their product is meaningful: `MAX_HDDVD_CLIPS` probes each cost up + /// to `EVO_PROBE_SECTORS`, so the pair fixes the worst-case drive time of a + /// `Disc::scan()`. + /// + /// `EvoProbeCache` does not help against a crafted disc here: it keys on the + /// RESOLVED extent list, and distinct-but-overlapping extent lists all + /// reading one physical region miss the memo every time while every read + /// succeeds. So the product is genuinely reachable, and it is the number + /// that has to stay within what a scan can absorb. + /// + /// 8 GiB is the ceiling asserted here. It is far above any genuine disc — + /// retail HD-DVDs carry tens of `.evo` clips, i.e. well under 1 GiB of + /// probing — and far below the 64 GiB the pair used to permit, which + /// exceeded the capacity of the dual-layer media this format tops out at + /// and took tens of minutes of optical reads. + /// + /// This is a relationship, not a magic number: raising EITHER constant + /// without re-examining the other trips it. + #[test] + fn the_clip_cap_and_probe_budget_bound_a_scans_worst_case_read_volume() { + const CEILING_BYTES: u64 = 8 * 1024 * 1024 * 1024; + let budget = |cap: usize| { + cap as u64 * u64::from(EVO_PROBE_SECTORS) * crate::consts::SECTOR_BYTES as u64 + }; + // BOTH title-composition paths pay one probe per item, and a crafted + // disc reaches either one: the directory fallback when `/ADV_OBJ` is + // omitted, the playlist path when it is present. Capping only one moves + // the amplification next door. + for (name, cap) in [ + ("MAX_HDDVD_CLIPS", MAX_HDDVD_CLIPS), + ("MAX_XPL_TITLES", MAX_XPL_TITLES), + ] { + let worst_case = budget(cap); + assert!( + worst_case <= CEILING_BYTES, + "{name} ({cap}) x EVO_PROBE_SECTORS ({EVO_PROBE_SECTORS}) = \ + {worst_case} bytes of probe reads, over the {CEILING_BYTES}-byte \ + ceiling a scan can absorb" + ); + } + } + + /// CONTROL: memoization must not silently collapse DISTINCT clips. A + /// legitimate disc with several different `.evo` files still probes each + /// one, and each title keeps the streams of ITS OWN clip. + /// + /// Mutation: make `EvoProbeCache`'s key a constant (ignore the extents) and + /// this goes red — the junk clip inherits the first clip's H.264 streams. + #[test] + fn scan_hddvd_titles_still_probes_each_distinct_clip() { + let mut disc = MemDisc::new(); + let evo = synthetic_evo(); + let junk = vec![0x55u8; 4 * 2048]; + let (a_data, b_data, c_data) = (5000u32, 6000u32, 7000u32); + lay_hddvd_clips( + &mut disc, + vec![ + file_with("A_REAL.EVO", 100, a_data, evo.clone(), true), + file_with("B_JUNK.EVO", 101, b_data, junk, true), + file_with("C_REAL.EVO", 102, c_data, evo, true), + ], + ); + let udf = crate::udf::read_filesystem(&mut disc).expect("fs"); + + let watch = vec![ + PART_START + a_data, + PART_START + b_data, + PART_START + c_data, + ]; + let mut counter = ProbeCounter::new(disc, watch); + let titles = Disc::scan_hddvd_titles(&mut counter, &udf, None).expect("scan"); + + assert_eq!( + titles.len(), + 3, + "every distinct clip still resolves a title" + ); + + // The substantive assertion first: each title carries the streams of ITS + // OWN clip. This is what a wrong memo key actually costs. + let by_name = |n: &str| titles.iter().find(|t| t.playlist == n).expect(n); + assert!( + by_name("A_REAL.EVO") + .streams + .iter() + .any(|s| matches!(s, Stream::Video(v) if v.codec == Codec::H264)), + "the real clip keeps its own probed streams" + ); + assert!( + by_name("B_JUNK.EVO").streams.is_empty(), + "the unrecognizable clip must NOT inherit another clip's streams" + ); + assert!( + by_name("C_REAL.EVO") + .streams + .iter() + .any(|s| matches!(s, Stream::Video(v) if v.codec == Codec::H264)), + "a third distinct clip is probed on its own extents" + ); + + assert_eq!( + counter.hits, + vec![1, 1, 1], + "each DISTINCT clip is still probed exactly once" + ); + } + + /// Memoization alone is NOT the whole fix. Collapsing identical extent + /// lists bounds the one-File-Entry attack, but distinct extent lists are + /// only as bounded as the File Entries an attacker cares to lay down — and + /// a File Entry is ONE sector. A 1 MiB directory's ~24,000 FIDs can each + /// point at their own 1-sector File Entry (a ~48 MiB image) declaring its + /// own huge extent, so every probe misses the memo and the amplification is + /// back. [`MAX_HDDVD_CLIPS`] is the bound that closes that. + /// + /// Mutation: drop the `clips.len() < MAX_HDDVD_CLIPS` gate in the directory + /// scan and this goes red at `ENTRIES`. + #[test] + fn scan_hddvd_titles_caps_a_directory_declaring_absurdly_many_clips() { + const ENTRIES: usize = MAX_HDDVD_CLIPS + 300; + + let mut disc = MemDisc::new(); + // Each entry gets its OWN File Entry and its OWN data extent, so no two + // resolve to the same extent list and the memo never hits. + let specs: Vec<_> = (0..ENTRIES) + .map(|i| { + // ICBs live far past the directory's own FID data (~230 KB + // from LBA 21) so laying the FIDs cannot clobber them. + file( + &format!("C{i:05}.EVO"), + 100_000 + i as u32, + 1_000_000 + i as u32 * 8, + 2048, + true, + ) + }) + .collect(); + lay_hddvd_clips(&mut disc, specs); + let udf = crate::udf::read_filesystem(&mut disc).expect("fs"); + + let titles = Disc::scan_hddvd_titles(&mut disc, &udf, None).expect("scan"); + assert_eq!( + titles.len(), + MAX_HDDVD_CLIPS, + "a directory declaring {ENTRIES} clips must be capped at \ + {MAX_HDDVD_CLIPS}; each surviving clip costs a real \ + probe_evo_streams pass over the medium" + ); + } + + /// The control for the cap: a realistic disc keeps every clip it carries. + #[test] + fn scan_hddvd_titles_keeps_every_clip_of_a_realistic_disc() { + let mut disc = MemDisc::new(); + let udf = make_hddvd_fs( + &mut disc, + &[ + ("FEATURE_1.EVO", 2000, 5000), + ("FEATURE_2.EVO", 1800, 9000), + ("TRAILER.EVO", 300, 12000), + ("DELOGO.EVO", 40, 13000), + ], + ); + let titles = Disc::scan_hddvd_titles(&mut disc, &udf, None).expect("scan"); + assert_eq!( + titles.len(), + 4, + "a real disc's clips must survive the cap untouched" + ); + } } diff --git a/src/disc/mod.rs b/src/disc/mod.rs index 5f50fd9..68445d3 100644 --- a/src/disc/mod.rs +++ b/src/disc/mod.rs @@ -2061,7 +2061,7 @@ impl Disc { ) } else if udf_fs.find_dir("/HVDVD_TS").is_some() { ( - Self::scan_hddvd_titles(reader, &udf_fs), + Self::scan_hddvd_titles(reader, &udf_fs, opts.halt.as_ref())?, ContentFormat::MpegPs, ) } else if udf_fs.find_dir("/VIDEO_TS").is_some() { @@ -2187,7 +2187,9 @@ impl Disc { /// **Sort priority (titles[0] = most likely main feature):** /// 1. Real titles (`size_bytes ≤ capacity_bytes`) before virtual /// composites. The capacity check is a hard "physically - /// possible data on this disc" gate. + /// possible data on this disc" gate. `capacity_bytes == 0` means + /// the capacity is UNKNOWN (READ CAPACITY failed), so the gate is + /// skipped entirely rather than demoting every real title. /// 2. Among real titles, LARGEST physical size first — the main /// feature is the biggest real title on the disc. (This replaced /// the old clip-count ordering, which mis-ranked chapter-per-clip @@ -2249,8 +2251,18 @@ impl Disc { // A title bigger than the whole disc is a "play-all" composite artifact // (its declared size double-counts clips shared with other playlists) — // demote it below any real single title. - let a_oversize = a.size_bytes > capacity_bytes; - let b_oversize = b.size_bytes > capacity_bytes; + // + // `capacity_bytes == 0` means the capacity is UNKNOWN, not that the + // disc holds nothing: `read_udf` substitutes 0 when READ CAPACITY + // fails and scans on regardless. Applied literally the gate would + // INVERT there — every real title (`size_bytes > 0`) would be + // "oversize" and demoted, while a CLPI-less `size_bytes == 0` title + // would not, landing at `titles[0]` ahead of the feature. With no + // capacity to compare against, the gate is inert and the size / + // duration / audio keys decide the order on their own. + let capacity_known = capacity_bytes > 0; + let a_oversize = capacity_known && a.size_bytes > capacity_bytes; + let b_oversize = capacity_known && b.size_bytes > capacity_bytes; a_oversize .cmp(&b_oversize) // PRIMARY: largest physical size = the main feature. Robust where @@ -3575,8 +3587,8 @@ mod tests { ) -> (crate::udf::fixture::MemDisc, udf::UdfFs) { use crate::udf::fixture::*; let files = vec![ - file("MAIN.EVO", 100, 5_000, main_bytes, true), - file("OTHER.EVO", 101, 50_000, other_bytes, true), + file("MAIN.EVO", 100, 5_000, main_bytes as u64, true), + file("OTHER.EVO", 101, 50_000, other_bytes as u64, true), ]; let root = DirSpec { name: String::new(), @@ -3598,6 +3610,128 @@ mod tests { (disc, udf) } + /// A cancelled [`crate::halt::Halt`] must stop the HD-DVD title scan. + /// + /// The scan is bounded but big — up to `MAX_HDDVD_CLIPS` clips, each + /// costing an ICB resolve plus a 16 MiB `EVO_PROBE_SECTORS` stream probe — + /// so on a live drive an operator Stop that only takes effect after the + /// whole enumerator returns is no Stop at all. `ScanOptions::halt` is + /// already honoured by the CSS crack and the forced-subtitle probe; the + /// title enumerator must honour it too. + /// + /// It must also not report a HALF-ENUMERATED disc as a successful scan: + /// a truncated title list is indistinguishable from a disc that genuinely + /// holds fewer titles. + #[test] + fn scan_with_cancelled_halt_stops_the_hddvd_title_scan() { + use crate::udf::fixture::PART_START; + + /// Counts reads that land on CLIP DATA (at or past the first clip's + /// data extent) — i.e. the per-clip stream probing, the expensive part + /// of the scan. Everything below that is filesystem metadata. + struct CountingReader<'a> { + inner: &'a mut crate::udf::fixture::MemDisc, + clip_reads: usize, + } + impl SectorSource for CountingReader<'_> { + fn read_sectors( + &mut self, + lba: u32, + count: u16, + buf: &mut [u8], + recovery: bool, + ) -> Result<usize> { + if lba >= PART_START + 5_000 { + self.clip_reads += 1; + } + self.inner.read_sectors(lba, count, buf, recovery) + } + } + + let (mut disc, udf) = hddvd_two_clip_disc(3_000_000, 5_000_000); + let halt = crate::halt::Halt::new(); + halt.cancel(); + let opts = ScanOptions { + halt: Some(halt), + ..Default::default() + }; + let mut reader = CountingReader { + inner: &mut disc, + clip_reads: 0, + }; + let res = Disc::scan_with(&mut reader, 3_997_952, None, None, &opts, udf); + let clip_reads = reader.clip_reads; + + assert!( + matches!(res, Err(Error::Halted)), + "a cancelled scan must say so, not return a partial title list as \ + a completed scan; got {:?}", + res.map(|d| d.titles.len()) + ); + assert_eq!( + clip_reads, 0, + "cancellation must be observed before the per-clip stream probes, \ + not after all of them" + ); + } + + /// A Stop on a LIVE DRIVE never touches `ScanOptions::halt`: `Drive` has + /// its own flag and `checked_exec` fails every SCSI command with + /// [`Error::Halted`] once it is set. The HD-DVD enumerator must not + /// swallow that into a successful scan. + /// + /// Measured before this was fixed: the scan returned `Ok` with both + /// titles present and ZERO streams on each — a cancelled scan wearing the + /// shape of a disc whose clips carry no video or audio. Downstream that is + /// a title list to cache, display and rip from. + #[test] + fn halted_reads_do_not_report_the_hddvd_scan_as_successful() { + use crate::udf::fixture::PART_START; + + /// Fails clip-data reads the way a live drive does once Stop is + /// pressed; filesystem metadata below the first clip still resolves, + /// so the scan gets far enough to enumerate titles. + struct HaltingReader<'a> { + inner: &'a mut crate::udf::fixture::MemDisc, + } + impl SectorSource for HaltingReader<'_> { + fn read_sectors( + &mut self, + lba: u32, + count: u16, + buf: &mut [u8], + recovery: bool, + ) -> Result<usize> { + if lba >= PART_START + 5_000 { + return Err(Error::Halted); + } + self.inner.read_sectors(lba, count, buf, recovery) + } + } + + let (mut disc, udf) = hddvd_two_clip_disc(3_000_000, 5_000_000); + let mut reader = HaltingReader { inner: &mut disc }; + let res = Disc::scan_with( + &mut reader, + 3_997_952, + None, + None, + &ScanOptions::default(), + udf, + ); + assert!( + matches!(res, Err(Error::Halted)), + "reads cancelled by the drive's own halt flag must surface as a \ + cancelled scan, not as titles that merely look stream-less; got \ + {:?}", + res.map(|d| d + .titles + .iter() + .map(|t| (t.playlist.clone(), t.streams.len())) + .collect::<Vec<_>>()) + ); + } + /// `scan_with`'s `capacity_bytes = capacity as u64 * 2048` feeds /// `canonical_title_order`'s "bigger than the whole disc = play-all /// composite" threshold. Chosen so `capacity * 2048` clears both titles @@ -6817,6 +6951,55 @@ mod tests { ); } + /// `capacity_bytes == 0` means the disc capacity is UNKNOWN — `read_udf` + /// substitutes 0 when READ CAPACITY fails (transient spin-up, SCSI error) + /// and proceeds with the scan. It does NOT mean "the disc holds nothing". + /// + /// With a literal reading of the gate, 0 inverts it: EVERY real title is + /// `size_bytes > 0` and therefore "oversize", while a title with no CLPI + /// (`size_bytes == 0`) is not — so the empty title sorts to `titles[0]` + /// ahead of the feature and `freemkv -t 1` rips garbage. The gate must be + /// INERT when the capacity is unknown. + #[test] + fn canonical_order_unknown_capacity_does_not_demote_every_real_title() { + const UNKNOWN: u64 = 0; // READ CAPACITY failed + let feature = title_with("00800.mpls", 7_320.0, 57_200_000_000, 1); + // A playlist whose CLPI files are missing/unparseable: no declared size. + let sizeless = title_with("00001.mpls", 120.0, 0, 1); + let mut titles = [sizeless, feature]; + titles.sort_by(|a, b| Disc::canonical_title_order(a, b, UNKNOWN)); + assert_eq!( + titles[0].playlist, "00800.mpls", + "with an UNKNOWN capacity the real feature must still sort first; \ + a size-0 title must not be promoted ahead of it" + ); + assert_eq!(titles[1].playlist, "00001.mpls"); + } + + /// Control for [`canonical_order_unknown_capacity_does_not_demote_every_real_title`]: + /// making the gate inert on an UNKNOWN capacity must not make it dead. With + /// a KNOWN capacity a genuinely oversize play-all composite is still demoted + /// below a smaller real title — even though "largest size first" would + /// otherwise rank it first. Asserted on the comparator in both argument + /// orders so an inconsistent comparator cannot pass. + #[test] + fn canonical_order_known_capacity_still_demotes_a_genuinely_oversize_title() { + use std::cmp::Ordering; + const CAP: u64 = 58_500_000_000; + let composite = title_with("00020.mpls", 15_180.0, 92_400_000_000, 253); + let real = title_with("00800.mpls", 7_320.0, 57_200_000_000, 1); + assert_eq!( + Disc::canonical_title_order(&real, &composite, CAP), + Ordering::Less, + "a known capacity must still demote the oversize composite" + ); + assert_eq!( + Disc::canonical_title_order(&composite, &real, CAP), + Ordering::Greater, + "…in either argument order" + ); + } + // ── audio_richness: the same-size / same-duration tiebreak ───────────── /// A title carrying the given audio tracks, with size and duration fixed so diff --git a/src/drive/mod.rs b/src/drive/mod.rs index 926cf4c..15edab0 100644 --- a/src/drive/mod.rs +++ b/src/drive/mod.rs @@ -8,6 +8,17 @@ pub fn extract_scsi_context(e: &Error) -> (u8, Option<crate::scsi::ScsiSense>) { match e { Error::ScsiError { status, sense, .. } => (*status, *sense), Error::DiscRead { status, sense, .. } => (status.unwrap_or(0), *sense), + // A failed `ioctl(SG_IO)` and a vanished device never produced a SCSI + // reply at all — they are dead-bus faults, not recoverable bad sectors. + // `Error::is_scsi_transport_failure` (error.rs) already declares both + // variants transport failures so sweep / patch / fill_extents abort the + // pass instead of zero-filling against a wedged device; callers that + // flatten an error through here (Drive::read_one, DiscStream::fill_extents, + // freemkv-engine's recovery sweep/patch) would otherwise collapse them to + // status 0 and silently destroy that classification. + Error::IoError { .. } | Error::DeviceNotFound { .. } => { + (crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE, None) + } _ => (0, None), } } @@ -1936,6 +1947,90 @@ mod command_tests { assert!(err.scsi_sense().is_none()); } + /// `extract_scsi_context` must map the two non-SCSI dead-bus faults + /// (a failed `ioctl(SG_IO)` → `Error::IoError`, a vanished fd → + /// `Error::DeviceNotFound`) to the 0xFF TRANSPORT_FAILURE sentinel, not + /// to 0x00. Everything else keeps the (0, None) catch-all. + /// Spec: `Error::is_scsi_transport_failure` (error.rs) declares both + /// variants transport failures so sweep/patch/fill_extents abort + /// the pass instead of zero-filling against a wedged device. + /// Mutation: returning (0, None) here flattens IoError into + /// `DiscRead { status: Some(0) }`, which is_scsi_transport_failure + /// rejects — a wedged USB bridge zero-fills the whole title. + #[test] + fn extract_scsi_context_maps_dead_bus_faults_to_transport_failure() { + let (status, sense) = extract_scsi_context(&Error::IoError { + source: std::io::Error::from(std::io::ErrorKind::NotConnected), + }); + assert_eq!( + status, + crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE, + "a failed ioctl(SG_IO) is a transport-layer fault" + ); + assert!(sense.is_none(), "no SCSI reply means no sense data"); + + let (status, sense) = extract_scsi_context(&Error::DeviceNotFound { + path: "/dev/sg9".into(), + }); + assert_eq!( + status, + crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE, + "a vanished device is a transport-layer fault" + ); + assert!(sense.is_none()); + + // Control: the catch-all still yields (0, None) for unrelated errors, + // so the two asserts above are about these variants specifically. + assert_eq!(extract_scsi_context(&Error::Halted), (0, None)); + + // Control: real SCSI replies still pass their own status through. + let s = crate::scsi::ScsiSense { + sense_key: 3, + asc: 0x11, + ascq: 0x00, + }; + assert_eq!( + extract_scsi_context(&Error::ScsiError { + opcode: 0x28, + status: 0x02, + sense: Some(s), + }), + (0x02, Some(s)) + ); + } + + /// End-to-end: an `Error::IoError` raised by the transport must still be + /// classified as a transport failure after `Drive::read` flattens it into + /// `Error::DiscRead`. Without the `extract_scsi_context` mapping the + /// variant is destroyed (status becomes `Some(0)`) and the caller treats a + /// dead bus as a recoverable bad sector. + #[test] + fn read_io_error_surfaces_as_transport_failure_not_a_bad_sector() { + let mut d = Drive::from_transport_for_test(Box::new(AlwaysErr { + err: || Error::IoError { + source: std::io::Error::from(std::io::ErrorKind::NotConnected), + }, + })); + let mut buf = vec![0u8; 2048]; + let err = d.read(42, 1, &mut buf, false).unwrap_err(); + assert!( + err.is_scsi_transport_failure(), + "a wedged bus must abort the pass, not zero-fill: got {err:?}" + ); + + // The same for a device that vanished mid-read. + let mut d = Drive::from_transport_for_test(Box::new(AlwaysErr { + err: || Error::DeviceNotFound { + path: "/dev/sg9".into(), + }, + })); + let err = d.read(42, 1, &mut buf, false).unwrap_err(); + assert!( + err.is_scsi_transport_failure(), + "a vanished device must abort the pass: got {err:?}" + ); + } + #[test] fn read_returns_halted_before_dispatch_without_touching_transport() { // When the halt flag is set, checked_exec returns Halted BEFORE diff --git a/src/error.rs b/src/error.rs index 83cb1ef..47658fe 100644 --- a/src/error.rs +++ b/src/error.rs @@ -45,6 +45,7 @@ pub const E_INVALID_CDB_LENGTH: u16 = 4001; // I/O (5xxx) pub const E_IO_ERROR: u16 = 5000; +pub const E_SOURCE_TERMINATED: u16 = 5001; // Disc format (6xxx) pub const E_DISC_READ: u16 = 6000; @@ -63,6 +64,8 @@ pub const E_SELECTION_PID_UNKNOWN: u16 = 6014; pub const E_UDF_BUFFER_TOO_SMALL: u16 = 6012; pub const E_UDF_NOT_FILESYSTEM: u16 = 6013; pub const E_IMAGE_TRUNCATED: u16 = 6015; +pub const E_UDF_AD_CHAIN_TOO_LONG: u16 = 6016; +pub const E_UDF_UNRECORDED_EXTENT: u16 = 6017; // AACS (7xxx) pub const E_AACS_NO_KEYS: u16 = 7000; @@ -393,6 +396,18 @@ pub enum Error { UdfNotFound { path: String, }, + /// The file's ICB allocation list contains an unrecorded (ECMA-167 + /// 4/14.14.1.1 type-1/type-2) extent: space allocated to the file at that + /// location but never written, so its true content there is zeros while + /// the media holds something else. + /// + /// Raised by [`crate::udf::UdfFs::file_extents`] because a + /// `(lba, sector_count)` read plan cannot express a hole — reading it + /// splices undefined sectors into the rip as content, and dropping it + /// slides every later extent's byte space. + UdfUnrecordedExtent { + path: String, + }, /// The reader was addressable but the bytes are structurally NOT a UDF /// filesystem — a deterministic tag/format mismatch (e.g. no Anchor Volume /// Descriptor Pointer at sector 256, no partition descriptor, no File Set @@ -405,6 +420,16 @@ pub enum Error { /// 2048-byte sector. A contract violation on the public reader API — /// returned instead of panicking on the slice. UdfBufferTooSmall, + /// A file's allocation-descriptor continuation chain did not end within the + /// hop budget the UDF reader allows. + /// + /// The budget exists so a crafted or corrupt disc cannot loop the reader + /// forever. Hitting it is NOT the end of the chain: the extents beyond that + /// point are unknown, so the extent list in hand describes only part of the + /// file. Returning that list would let a caller zero-pad the remainder to + /// the declared size and report a mostly-empty file as a complete + /// extraction, so the read fails instead. + UdfAdChainTooLong, DiscTitleRange { index: usize, count: usize, @@ -733,6 +758,19 @@ pub enum Error { /// it silently leaves encrypted. The producer surfaces this rather /// than emit still-encrypted bytes. ExtentNotUnitAligned, + /// A [`crate::sector::SectorSource`] that feeds its reads from a + /// producer thread has terminated for good — the thread exited after + /// an error or before delivering the extents it was given — so it can + /// never return another byte. + /// + /// It exists because the alternative answer is a lie: a dead source + /// that reports `Ok(0)` is indistinguishable from end-of-stream, and + /// `DiscStream::fill_extents` legitimately reads a short count as a + /// skippable hole — zero-filling and advancing over every remaining + /// sector of the title and still returning success. Unlike a bad + /// sector, this condition cannot be retried at a smaller size or + /// skipped past, so every consumer must abort the pass on it. + SourceTerminated, /// An MPEG-TS packet under construction violated the 188-byte fixed /// size (over-long adaptation field, overflowing payload, or a /// short/mis-assembled packet). Indicates a muxer invariant break, @@ -850,13 +888,16 @@ impl Error { Error::ScsiError { .. } => E_SCSI_ERROR, Error::InvalidCdbLength { .. } => E_INVALID_CDB_LENGTH, Error::IoError { .. } => E_IO_ERROR, + Error::SourceTerminated => E_SOURCE_TERMINATED, Error::DiscRead { .. } => E_DISC_READ, Error::Halted => E_HALTED, Error::MplsParse => E_MPLS_PARSE, Error::ClpiParse => E_CLPI_PARSE, Error::UdfNotFound { .. } => E_UDF_NOT_FOUND, + Error::UdfUnrecordedExtent { .. } => E_UDF_UNRECORDED_EXTENT, Error::UdfNotFilesystem => E_UDF_NOT_FILESYSTEM, Error::UdfBufferTooSmall => E_UDF_BUFFER_TOO_SMALL, + Error::UdfAdChainTooLong => E_UDF_AD_CHAIN_TOO_LONG, Error::DiscTitleRange { .. } => E_DISC_TITLE_RANGE, Error::ShortImageRead { .. } => E_SHORT_IMAGE_READ, Error::EmptyImage => E_EMPTY_IMAGE, @@ -1063,6 +1104,7 @@ impl std::fmt::Display for Error { }, Error::Halted => write!(f, "E{}", self.code()), Error::UdfNotFound { path } => write!(f, "E{}: {}", self.code(), path), + Error::UdfUnrecordedExtent { path } => write!(f, "E{}: {}", self.code(), path), Error::SeamPlanDroppedMost { dropped, written } => { write!(f, "E{} {dropped}/{written}", self.code()) } @@ -1128,7 +1170,21 @@ impl std::error::Error for Error { impl From<std::io::Error> for Error { fn from(e: std::io::Error) -> Self { - Error::IoError { source: e } + // If this `io::Error` is one WE produced (`From<Error> for io::Error` + // carries the typed value in its boxed payload), give the original + // back instead of burying it in `Error::IoError`. That wrapper is not + // neutral: `is_scsi_transport_failure` treats `IoError` as a + // transport-layer fault (dead bus / wedged bridge), so re-wrapping a + // round-tripped `DiscRead` MEDIUM ERROR turned a skippable bad sector + // into a pass-aborting bridge wedge — the exact inverse of what that + // arm exists for. Any error that crosses a thread boundary as an + // `io::Error` (the prefetch channel's `Batch`) keeps its + // classification, its SCSI status, and its sense data. + match e.downcast::<Error>() { + Ok(typed) => typed, + // A genuine OS/`std` error — the `IoError` wrapper is correct here. + Err(io) => Error::IoError { source: io }, + } } } @@ -1143,7 +1199,6 @@ impl From<Error> for std::io::Error { return source; } let code = e.code(); - let msg = e.to_string(); // Map our error categories to io::ErrorKind let kind = match code { // Device access-denied semantics map to PermissionDenied; @@ -1244,7 +1299,13 @@ impl From<Error> for std::io::Error { E_DIR_IMAGE_FILE_CHANGED => std::io::ErrorKind::InvalidData, _ => std::io::ErrorKind::Other, }; - std::io::Error::new(kind, msg) + // Carry the typed value itself as the payload rather than its + // stringification. `Display` is unchanged (`io::Error` delegates to the + // boxed error, whose `Display` is the same `E<code>[: …]` string), so + // `error_code` and every consumer built on it are unaffected — but the + // typed error now SURVIVES the conversion and `From<io::Error> for + // Error` can hand it back intact. + std::io::Error::new(kind, e) } } @@ -1263,13 +1324,14 @@ pub type Result<T> = std::result::Result<T, Error>; /// removing comes back. /// /// [`From<Error> for io::Error`] is the ONLY path from a typed [`Error`] to an -/// `io::Error` in this crate, and it stringifies (`io::Error::new(kind, msg)` -/// where `msg` is the `Error`'s `E<code>[: …]` [`Display`](std::fmt::Display) -/// string) rather than boxing the typed value — no code path constructs an -/// `io::Error` that still holds a `crate::error::Error` via `get_ref`. So the -/// only recognised shape is the round-tripped `E<code>` message prefix. +/// `io::Error` in this crate. It boxes the typed value as the payload +/// (`io::Error::new(kind, e)`), whose [`Display`](std::fmt::Display) is the +/// same `E<code>[: …]` string the stringifying version produced — so this +/// parse is unaffected, and `From<io::Error> for Error` can additionally +/// `downcast` the payload back to the exact typed error. Errors that did NOT +/// come from this crate carry no `E<code>` prefix and yield `None`. pub fn error_code(e: &std::io::Error) -> Option<u16> { - // Round-tripped: `From<Error> for io::Error` stringifies as "E<code>[: …]". + // Round-tripped: `From<Error> for io::Error` renders as "E<code>[: …]". let s = e.to_string(); let digits = s.strip_prefix('E')?; let end = digits @@ -1398,6 +1460,22 @@ impl Error { ) } + /// True if the read SOURCE itself is gone, as opposed to one range of + /// media being unreadable. Kept separate from + /// [`is_scsi_transport_failure`](Self::is_scsi_transport_failure) — + /// which is about the bus/bridge and drives "power-cycle the drive" + /// advice — because a terminated producer thread is neither a wedged + /// bridge nor a bad sector, and reporting it as SCSI status 0xFF would + /// be a fabricated status byte. + /// + /// What it shares with a transport failure is the only thing the read + /// loops need to know: retrying smaller or skipping ahead cannot + /// recover anything, so the pass must abort rather than fabricate + /// zeros for the rest of the title. + pub fn is_source_terminated(&self) -> bool { + matches!(self, Error::SourceTerminated) + } + /// True if this error indicates bridge degradation — the SCSI status /// is neither GOOD (0x00), CHECK CONDITION (0x02), nor transport failure /// (0xFF). Observed on the Initio INIC-1618L USB bridge preceding a full diff --git a/src/ifo.rs b/src/ifo.rs index 6de477a..92af324 100644 --- a/src/ifo.rs +++ b/src/ifo.rs @@ -49,7 +49,7 @@ pub struct DvdTitle { pub cells: Vec<DvdCell>, /// Chapter start times in seconds (derived from program map + cell times) pub chapter_times: Vec<f64>, - /// Subtitle palette from PGC: 16 entries of [padding, Y, Cb, Cr]. + /// Subtitle palette from PGC: 16 entries of [padding, Y, Cr, Cb]. pub palette: Option<Vec<[u8; 4]>>, } @@ -691,7 +691,16 @@ fn parse_video_attr(data: &[u8]) -> Result<DvdVideoAttr> { } /// Parse one audio stream attribute block (8 bytes at `offset`). -fn parse_audio_attr(data: &[u8], offset: usize) -> Result<DvdAudioAttr> { +/// `pub(crate)` for the CROSS-MODULE tests only. The sole production caller is +/// `parse_vts_attributes` in this file; `src/mux/mkv.rs`'s `#[cfg(test)]` block +/// calls it directly so its language-mapping tests run the real parser over real +/// on-disc IFO bytes end to end, instead of a hand-built `DvdAudioAttr` that +/// could agree with the muxer while both disagree with the disc. Narrowing this +/// would mean either a `#[cfg(test)]`/`#[cfg(not(test))]` pair of signatures +/// that can drift apart, or moving those tests away from the code they exist to +/// pin — both worse than the widened crate-internal visibility, which reaches no +/// public API. +pub(crate) fn parse_audio_attr(data: &[u8], offset: usize) -> Result<DvdAudioAttr> { let b0 = byte_at(data, offset)?; let b1 = byte_at(data, offset + 1)?; @@ -718,25 +727,9 @@ fn parse_audio_attr(data: &[u8], offset: usize) -> Result<DvdAudioAttr> { let channels = (b1 & 0x07) + 1; // (channels - 1) in low 3 bits of byte 1 - // Language code: bytes 2-3 as ISO 639 + // Language code: bytes 2-3 as ISO 639-1 (the DVD-Video spec's form). let lang_bytes = sub_slice(data, offset + 2, 2)?; - let language = if lang_bytes[0] >= b'a' - && lang_bytes[0] <= b'z' - && lang_bytes[1] >= b'a' - && lang_bytes[1] <= b'z' - { - String::from_utf8_lossy(lang_bytes).to_string() - } else if lang_bytes[0] == 0 && lang_bytes[1] == 0 { - String::new() - } else { - // Try to interpret as printable ASCII - let s: String = lang_bytes - .iter() - .filter(|&&b| b.is_ascii_alphanumeric()) - .map(|&b| b as char) - .collect(); - s - }; + let language = dvd_lang_to_iso639_2(&parse_raw_dvd_lang_bytes(lang_bytes)); Ok(DvdAudioAttr { codec, @@ -785,9 +778,27 @@ fn assign_audio_sub_stream_ids(streams: &mut [DvdAudioAttr]) { /// Parse one subtitle stream attribute block (6 bytes at `offset`). fn parse_subtitle_attr(data: &[u8], offset: usize) -> Result<DvdSubtitleAttr> { - // Language code: bytes 2-3 as ISO 639 + // Language code: bytes 2-3 as ISO 639-1 (the DVD-Video spec's form). let lang_bytes = sub_slice(data, offset + 2, 2)?; - let language = if lang_bytes[0] >= b'a' + let language = dvd_lang_to_iso639_2(&parse_raw_dvd_lang_bytes(lang_bytes)); + + Ok(DvdSubtitleAttr { language }) +} + +/// Decode the raw 2-byte on-disc language code shared by the audio and +/// subtitle attribute blocks. A pair of lowercase a-z bytes is taken +/// verbatim (the ISO 639-1 code the DVD-Video spec puts there); an all-zero +/// pair means unspecified (empty string); anything else falls through to an +/// ASCII-alphanumeric salvage — letters (either case) and digits are kept, +/// everything else (control bytes, punctuation, high bytes from a corrupt or +/// hostile disc) is dropped. +/// +/// The salvage is deliberately not narrowed to a-z: whatever survives is only +/// ever a lookup key for [`dvd_lang_to_iso639_2`], which degrades anything it +/// does not recognize to `und`, so a stray `X` or `5` costs nothing and cannot +/// reach an output stream as a language code. +fn parse_raw_dvd_lang_bytes(lang_bytes: &[u8]) -> String { + if lang_bytes[0] >= b'a' && lang_bytes[0] <= b'z' && lang_bytes[1] >= b'a' && lang_bytes[1] <= b'z' @@ -796,15 +807,40 @@ fn parse_subtitle_attr(data: &[u8], offset: usize) -> Result<DvdSubtitleAttr> { } else if lang_bytes[0] == 0 && lang_bytes[1] == 0 { String::new() } else { - let s: String = lang_bytes + lang_bytes .iter() .filter(|&&b| b.is_ascii_alphanumeric()) .map(|&b| b as char) - .collect(); - s - }; + .collect() + } +} - Ok(DvdSubtitleAttr { language }) +/// Convert a DVD IFO audio/subtitle attribute's language code — ISO 639-1 +/// (2 lowercase letters) per the DVD-Video spec, or empty when unspecified — +/// to the ISO 639-2 form every downstream consumer (`disc::AudioStream` / +/// `disc::SubtitleStream::language`, and in turn Matroska's `Language` +/// element per RFC 9559 §12 and the MP4 sink's `mdhd` language) requires. +/// +/// Uses `labels::vocab::iso639_1_to_iso639_2`, which spans the WHOLE of ISO +/// 639-1 (plus the withdrawn spellings `iw`/`in`/`ji` that DVD-Video's +/// frozen-1988 language list still puts on disc). The narrower +/// `vocab::menu_lang` table is deliberately NOT used here: it exists for +/// Blu-ray menu-graphic filename tokens and knows only 25 languages, so a +/// Region-2 disc's Romanian, Bulgarian, Croatian, Serbian, Slovak, Slovenian, +/// Hebrew, Estonian, Latvian, Lithuanian and Icelandic tracks would all fold +/// onto `und` together. DVD streams carry an empty `label`, so the language +/// is the only thing distinguishing one subtitle track from the next — a +/// valid code that is identical for six tracks is worse for the user than the +/// invalid one it replaced. Both tables normalize to ISO 639-2/T, so they +/// agree wherever they overlap. +/// +/// An empty or unrecognized code degrades to `"und"` (ISO 639-2 / Matroska's +/// own "undetermined" value) — a valid element value — rather than passing +/// through an invalid 2-letter code or an empty string. Never guesses. +fn dvd_lang_to_iso639_2(raw: &str) -> String { + crate::labels::vocab::iso639_1_to_iso639_2(raw) + .unwrap_or("und") + .to_string() } // ── PGC parser ────────────────────────────────────────────────────────────── @@ -1016,7 +1052,7 @@ fn parse_pgc(data: &[u8], pgc_offset: usize, chapters: u16) -> Result<DvdTitle> times }; - // Extract subtitle palette at PGC offset 0xA4: 16 colors × 4 bytes [padding, Y, Cb, Cr] + // Extract subtitle palette at PGC offset 0xA4: 16 colors × 4 bytes [padding, Y, Cr, Cb] let palette = if pgc_offset + 0xA4 + 64 <= data.len() { let mut colors = Vec::with_capacity(16); for i in 0..16 { @@ -1441,7 +1477,7 @@ mod tests { data[0] = 0x00; // b1: bits 2-0=101 (channels-1=5) => 0x05 data[1] = 0x05; - // language "en" + // on-disc language "en" (ISO 639-1) -> parsed as ISO 639-2 "eng" data[2] = b'e'; data[3] = b'n'; @@ -1449,7 +1485,7 @@ mod tests { assert_eq!(attr.codec, Codec::Ac3); assert_eq!(attr.sample_rate, 48000); assert_eq!(attr.channels, 6); - assert_eq!(attr.language, "en"); + assert_eq!(attr.language, "eng"); } #[test] @@ -1579,7 +1615,7 @@ mod tests { assert_eq!(attr.codec, Codec::Dts); assert_eq!(attr.sample_rate, 96000); assert_eq!(attr.channels, 2); - assert_eq!(attr.language, "fr"); + assert_eq!(attr.language, "fra"); } // ───────────────────────────────────────────────────────────────────── @@ -1693,15 +1729,17 @@ mod tests { } /// Audio language bytes [offset+2..+4]: when both bytes are 0x00 the - /// language is the empty string (unspecified), per source. + /// on-disc code is unspecified, and `dvd_lang_to_iso639_2` maps that to + /// the valid ISO 639-2 "und" (undetermined) rather than an empty string, + /// which is not a legal Matroska `Language` element value. #[test] - fn audio_attr_zero_language_is_empty() { + fn audio_attr_zero_language_becomes_und() { let mut data = vec![0u8; 8]; data[0] = 0x00; data[2] = 0x00; data[3] = 0x00; let attr = parse_audio_attr(&data, 0).unwrap(); - assert_eq!(attr.language, ""); + assert_eq!(attr.language, "und"); } /// Audio sample_rate flag (b0>>3 & 0x03): 0=48kHz, 1=96kHz, else 48kHz. @@ -1722,11 +1760,11 @@ mod tests { data[2] = b'd'; data[3] = b'e'; let attr = parse_subtitle_attr(&data, 0).unwrap(); - assert_eq!(attr.language, "de"); + assert_eq!(attr.language, "deu"); let zero = vec![0u8; 6]; let attr2 = parse_subtitle_attr(&zero, 0).unwrap(); - assert_eq!(attr2.language, ""); + assert_eq!(attr2.language, "und"); } /// assign_audio_sub_stream_ids: MP1/MP2 and other non-private-stream-1 @@ -1812,8 +1850,8 @@ mod tests { assert_eq!(title.cells[1].first_sector, 20); } - /// parse_pgc palette: at PGC+0xA4, 16 colors × 4 bytes [pad, Y, Cb, Cr]. - /// A palette with at least one non-zero Y/Cb/Cr is returned as Some; + /// parse_pgc palette: at PGC+0xA4, 16 colors × 4 bytes [pad, Y, Cr, Cb]. + /// A palette with at least one non-zero Y/Cr/Cb is returned as Some; /// an all-zero palette returns None (source filters empty palettes). #[test] fn pgc_palette_present_and_empty() { @@ -1833,14 +1871,14 @@ mod tests { assert!(title2.palette.is_none()); } - /// parse_pgc palette layout: each color is [padding, Y, Cb, Cr] and the + /// parse_pgc palette layout: each color is [padding, Y, Cr, Cb] and the /// "non-empty" test ignores the padding byte (index 0). A palette whose /// ONLY non-zero bytes are padding must still be treated as empty (None). #[test] fn pgc_palette_padding_only_is_empty() { let mut pgc = vec![0u8; 0xEA]; pgc[0x03] = 0; - // Set padding byte (index 0) of color 0 non-zero, but Y/Cb/Cr zero. + // Set padding byte (index 0) of color 0 non-zero, but Y/Cr/Cb zero. pgc[0xA4] = 0xFF; let title = parse_pgc(&pgc, 0, 1).unwrap(); assert!( @@ -2278,10 +2316,13 @@ mod tests { /// two bytes at +2. Only a pair of lowercase a-z bytes is taken verbatim; /// anything else falls through to the ASCII-alphanumeric salvage, which /// keeps only the usable characters. A byte outside a-z must never end up - /// in the language string. + /// in the raw salvaged string. This exercises `parse_raw_dvd_lang_bytes` + /// directly — the byte-level salvage step — separately from the ISO + /// 639-1 -> 639-2 mapping `parse_audio_attr`/`parse_subtitle_attr` apply + /// on top (see `dvd_two_letter_and_malformed_language_becomes_iso639_2`). #[test] fn language_code_rejects_non_lowercase_bytes() { - // (byte0, byte1, expected language) + // (byte0, byte1, expected raw salvage) let cases: [(u8, u8, &str); 8] = [ (b'e', b'n', "en"), // both in range → verbatim (0x21, b'n', "n"), // '!' is below 'a' @@ -2292,6 +2333,31 @@ mod tests { (0x00, b'E', "E"), // only the first byte is zero (0x00, 0x00, ""), // both zero → unset ]; + for (b0, b1, want) in cases { + assert_eq!( + parse_raw_dvd_lang_bytes(&[b0, b1]), + want, + "raw language salvage for ({b0:#04x}, {b1:#04x})" + ); + } + } + + /// The full pipeline `parse_audio_attr`/`parse_subtitle_attr` apply on + /// top of the raw salvage: a valid ISO 639-1 code maps to its ISO 639-2 + /// equivalent, and anything the raw salvage does NOT produce a mapped + /// code for (empty, or a single leftover letter from a malformed byte + /// pair) degrades to "und" — never an invalid 2-letter/1-letter code and + /// never an empty string, both of which are illegal Matroska `Language` + /// element values (RFC 9559 §12). + #[test] + fn dvd_two_letter_and_malformed_language_becomes_iso639_2() { + // (byte0, byte1, expected final language) + let cases: [(u8, u8, &str); 4] = [ + (b'e', b'n', "eng"), // valid ISO 639-1 → mapped + (0x21, b'n', "und"), // malformed → salvage "n", unmapped → und + (b'E', 0x00, "und"), // malformed → salvage "E", unmapped → und + (0x00, 0x00, "und"), // unspecified → und + ]; for (b0, b1, want) in cases { let mut audio = vec![0u8; 8]; audio[2] = b0; @@ -2496,7 +2562,7 @@ mod tests { } /// The subtitle palette is 16 entries of 4 bytes at PGC+0xA4, each - /// `[padding, Y, Cb, Cr]`. Every byte of every entry is distinct here, so + /// `[padding, Y, Cr, Cb]`. Every byte of every entry is distinct here, so /// a wrong stride, a wrong base or a shifted component shows up. #[test] // The loop variable is the DOMAIN VALUE being checked (a palette entry number), not a @@ -2524,7 +2590,7 @@ mod tests { } } - /// A palette is "present" when ANY of Y, Cb or Cr is non-zero in ANY + /// A palette is "present" when ANY of Y, Cr or Cb is non-zero in ANY /// entry — a single non-zero chroma component is enough. Only the /// padding byte [0] is ignored. #[test] diff --git a/src/labels/deluxe.rs b/src/labels/deluxe.rs index 1e9fa49..632b5d6 100644 --- a/src/labels/deluxe.rs +++ b/src/labels/deluxe.rs @@ -85,16 +85,16 @@ pub fn parse(reader: &mut dyn SectorSource, udf: &UdfFs) -> Option<ParseResult> let enums = identify_master_enums(archive); if enums.is_empty() { tracing::info!( - jar = %entry_name, + jar = ?entry_name, "deluxe: com/bydeluxe/ present but no master enum fingerprint matched" ); return None; } for (label, m) in &enums { tracing::info!( - jar = %entry_name, + jar = ?entry_name, enum = %label, - class = %m.class_name, + class = ?m.class_name, count = m.values.len(), "deluxe master enum identified", ); @@ -110,15 +110,15 @@ pub fn parse(reader: &mut dyn SectorSource, udf: &UdfFs) -> Option<ParseResult> let binding_classes = find_binding_classes(archive, &master_table.class_name_set()); if binding_classes.is_empty() { tracing::info!( - jar = %entry_name, + jar = ?entry_name, "deluxe: no binding class found (no class has enough getstatic refs to master enums)" ); return None; } for (name, count) in &binding_classes { tracing::info!( - jar = %entry_name, - binding_class = %name, + jar = ?entry_name, + binding_class = ?name, getstatic_count = count, "deluxe binding class candidate", ); @@ -138,7 +138,7 @@ pub fn parse(reader: &mut dyn SectorSource, udf: &UdfFs) -> Option<ParseResult> } if streams.is_empty() { tracing::info!( - jar = %entry_name, + jar = ?entry_name, "deluxe: binding classes found but produced 0 decoded streams" ); return None; @@ -149,7 +149,7 @@ pub fn parse(reader: &mut dyn SectorSource, udf: &UdfFs) -> Option<ParseResult> return None; } tracing::info!( - jar = %entry_name, + jar = ?entry_name, audio = labels.iter().filter(|l| l.stream_type == StreamLabelType::Audio).count(), subtitle = labels.iter().filter(|l| l.stream_type == StreamLabelType::Subtitle).count(), "deluxe emitted labels", diff --git a/src/labels/mod.rs b/src/labels/mod.rs index 5f661dc..02b81c7 100644 --- a/src/labels/mod.rs +++ b/src/labels/mod.rs @@ -450,13 +450,22 @@ pub(crate) fn apply_labels(labels: &[StreamLabel], titles: &mut [DiscTitle]) { }) else { continue; }; - for clip in &title.clips { + // ONLY the anchor's first clip. `disc::bluray` builds a title's + // stream list from `play_items[0]`'s STN table, so that is the only + // clip in which these PIDs were ever observed — the same clip tier 3 + // keys its derived ids on (`clip0`, below). Recording the fact + // against every clip the anchor plays claims knowledge of stream + // tables never read: a sibling playlist over a LATER clip then binds + // the anchor's editorial label onto whichever stream of that clip + // reuses the PID, which is a different physical stream (PIDs are + // unique only within a clip). + if let Some(clip) = title.clips.first() { pid_map.insert((clip.clip_id.as_str(), *pid), pos); } } tracing::info!( stream_type = ?stream_type, - playlist = %titles[anchor].playlist, + playlist = ?titles[anchor].playlist, slots = slots_of(&titles[anchor], stream_type).len(), "label list anchored to a title by its stream-language sequence", ); @@ -1907,6 +1916,139 @@ mod apply_tests { } } + /// Sentinel embedded in the crafted playlist name below. The capture keeps + /// ONLY fields whose rendered form contains it, so installing this + /// subscriber process-wide costs nothing and cannot accumulate other + /// tests' log output. + const LOG_INJECTION_SENTINEL: &str = "FMKV-LOG-INJECTION-PROBE"; + + fn capture_sink() -> &'static std::sync::Mutex<Vec<(String, String)>> { + static SINK: std::sync::OnceLock<std::sync::Mutex<Vec<(String, String)>>> = + std::sync::OnceLock::new(); + SINK.get_or_init(|| std::sync::Mutex::new(Vec::new())) + } + + /// Records how a `tracing` field was RENDERED — the question a disc-derived + /// log field raises is not whether it is logged but how. + /// + /// This is installed as the process-wide default rather than scoped with + /// `with_default`, because `tracing` caches an `Interest` per callsite + /// GLOBALLY: a sibling test running the same code on another thread with no + /// subscriber caches the callsite as "never", and a thread-local subscriber + /// installed afterwards then receives nothing. That failure mode is silent + /// — an empty capture reads as "no raw bytes found" — so the test asserts + /// the capture is non-empty as well. + /// + /// `register_callsite` answers `never` for every callsite outside this + /// module, so the rest of the suite keeps its current no-op logging cost. + struct CapturedFields; + + struct FieldVisitor(Vec<(String, String)>); + + impl tracing::field::Visit for FieldVisitor { + fn record_debug(&mut self, field: &tracing::field::Field, value: &dyn std::fmt::Debug) { + self.0 + .push((field.name().to_string(), format!("{value:?}"))); + } + fn record_str(&mut self, field: &tracing::field::Field, value: &str) { + self.0.push((field.name().to_string(), value.to_string())); + } + } + + fn is_labels_event(meta: &tracing::Metadata<'_>) -> bool { + meta.is_event() && meta.target().starts_with("libfreemkv::labels") + } + + impl tracing::Subscriber for CapturedFields { + fn register_callsite( + &self, + meta: &'static tracing::Metadata<'static>, + ) -> tracing::subscriber::Interest { + if is_labels_event(meta) { + tracing::subscriber::Interest::always() + } else { + tracing::subscriber::Interest::never() + } + } + fn enabled(&self, meta: &tracing::Metadata<'_>) -> bool { + is_labels_event(meta) + } + fn event(&self, event: &tracing::Event<'_>) { + let mut v = FieldVisitor(Vec::new()); + event.record(&mut v); + if v.0 + .iter() + .any(|(_, val)| val.contains(LOG_INJECTION_SENTINEL)) + { + capture_sink().lock().unwrap().extend(v.0); + } + } + fn new_span(&self, _: &tracing::span::Attributes<'_>) -> tracing::span::Id { + tracing::span::Id::from_u64(1) + } + fn record(&self, _: &tracing::span::Id, _: &tracing::span::Record<'_>) {} + fn record_follows_from(&self, _: &tracing::span::Id, _: &tracing::span::Id) {} + fn enter(&self, _: &tracing::span::Id) {} + fn exit(&self, _: &tracing::span::Id) {} + } + + /// A playlist name is a raw UDF directory entry — disc-controlled bytes, + /// validated no further than a lossy UTF-8 decode. Logging it through + /// tracing's `%` (Display) sigil writes those bytes VERBATIM, so a crafted + /// `.mpls` filename carrying ANSI escapes or control characters forges + /// terminal output and log structure in any consumer rendering the event + /// (CWE-117). `?` (Debug) escapes them, and `str`'s Debug is exactly the + /// escaping this needs. + /// + /// `info!` is not covered by the debug/trace-logging exemption: this fires + /// on an ordinary rip of an ordinary disc. + /// + /// Mutation: put `%` back on `playlist` in `apply_labels` and this goes red. + #[test] + fn a_disc_derived_playlist_name_is_escaped_in_the_log_not_written_verbatim() { + // A name whose bytes would clear the line and repaint it. + let evil = format!("\u{1b}[2K\u{1b}[31m{LOG_INJECTION_SENTINEL}\u{7}\u{1b}[0m.mpls"); + let _ = tracing::subscriber::set_global_default(CapturedFields); + + let labels = vec![ + sub_label(1, "eng", LabelQualifier::None), + sub_label(2, "spa", LabelQualifier::None), + sub_label(3, "fra", LabelQualifier::None), + ]; + let mut titles = vec![title_on_clip( + &evil, + "00294", + vec![ + subtitle(0x12A0, "eng"), + subtitle(0x12A1, "spa"), + subtitle(0x12A2, "fra"), + ], + )]; + apply_labels(&labels, &mut titles); + + let fields = capture_sink().lock().unwrap().clone(); + let playlist: Vec<&(String, String)> = fields + .iter() + .filter(|(k, v)| k == "playlist" && v.contains(LOG_INJECTION_SENTINEL)) + .collect(); + assert!( + !playlist.is_empty(), + "the anchoring event must actually have fired, or this test proves \ + nothing; captured: {fields:?}" + ); + for (_, rendered) in playlist { + assert!( + !rendered.contains('\u{1b}') && !rendered.contains('\u{7}'), + "a disc-controlled playlist name reached the log with its raw \ + control bytes intact: {rendered:?}" + ); + assert!( + rendered.contains(LOG_INJECTION_SENTINEL), + "the name must still be legible once escaped: {rendered:?}" + ); + } + } + fn sub_label(num: u16, lang: &str, qualifier: LabelQualifier) -> StreamLabel { StreamLabel { stream_id: None, @@ -3103,6 +3245,79 @@ mod apply_tests { ); } + /// A title that plays SEVERAL clips in order — the shape the anchor's + /// PID facts are harvested from. + fn title_on_clips(playlist: &str, clip_ids: &[&str], streams: Vec<Stream>) -> DiscTitle { + DiscTitle { + playlist: playlist.into(), + clips: clip_ids + .iter() + .map(|id| crate::disc::Clip { + feed_span: None, + clip_id: (*id).into(), + in_time: 0, + out_time: 0, + duration_secs: 3600.0, + source_packets: 0, + }) + .collect(), + ..title_with(streams) + } + } + + /// Spec: the anchor proves a `(clip, PID)` fact only for the clip its + /// stream table was READ FROM — the first play item — never for every clip + /// the anchor happens to play. + /// + /// `disc::bluray` builds a title's stream list from `play_items[0]`'s STN + /// table, and tier 3 fifty lines below says exactly that by keying its + /// derived ids on `clip0`. Tier 2's harvest contradicted it: it recorded + /// the anchor's slot PIDs against EVERY clip the anchor plays, so a sibling + /// playlist that plays a LATER clip of the anchor bound the anchor's + /// editorial label onto whatever stream in that clip happens to reuse the + /// PID — a different physical stream, in a different clip, whose own + /// language says so. + /// + /// Mutation: harvest over `&title.clips` instead of its first clip — the + /// featurette wears the feature's SDH again. + #[test] + fn an_anchor_proves_pids_only_for_the_clip_its_table_came_from() { + let labels = vec![ + sub_label(1, "eng", LabelQualifier::Sdh), + sub_label(2, "fra", LabelQualifier::None), + ]; + let mut titles = vec![ + // The anchor: its stream table is clip 00082's (the first play + // item); it merely CONTINUES into 00090. + title_on_clips( + "00800.mpls", + &["00082", "00090"], + vec![subtitle(0x1200, "eng"), subtitle(0x1201, "fra")], + ), + // A sibling playlist over the anchor's SECOND clip. Its subtitle + // reuses PID 0x1200 — PIDs are only unique within a clip — and it + // is Spanish, so nothing about the anchor's English SDH slot + // describes it. + title_on_clips("00451.mpls", &["00090"], vec![subtitle(0x1200, "spa")]), + ]; + apply_labels(&labels, &mut titles); + + assert_eq!( + sub_state(&titles[0]), + vec![ + (0x1200, false, LabelQualifier::Sdh), + (0x1201, false, LabelQualifier::None) + ], + "the anchor itself keeps the qualifiers the list states for it" + ); + assert_eq!( + sub_state(&titles[1]), + vec![(0x1200, false, LabelQualifier::None)], + "a PID in a clip the anchor's table never described is not that \ + table's stream 1" + ); + } + /// Spec: a vendor codec/variant claim does not follow the ordinal onto a /// bonus clip that carries a different codec. /// diff --git a/src/labels/vocab.rs b/src/labels/vocab.rs index 03ae0eb..e64768e 100644 --- a/src/labels/vocab.rs +++ b/src/labels/vocab.rs @@ -219,6 +219,237 @@ pub fn menu_lang(token: &str) -> Option<&'static str> { Some(code) } +// ── ISO 639-1 → ISO 639-2 ──────────────────────────────────────────────────── + +/// The complete ISO 639-1 set, paired with its ISO 639-2/**T** (terminological) +/// code. Every two-letter code ISO 639-1 defines appears exactly once. +/// +/// /T is the variant the rest of this crate uses — [`lang`] and [`menu_lang`] +/// both normalize to it (`deu` not `ger`, `fra` not `fre`, `zho` not `chi`, +/// `ces`, `nld`, `ell`, `ron`, `slk`, `isl`, `eus`, `hrv`) — so the three +/// tables cannot disagree. `iso639_1_agrees_with_menu_lang` pins that. +/// +/// For the 165 codes where 639-2/B and /T are identical this distinction does +/// not arise; it only matters for the 20-odd languages with a distinct +/// bibliographic code. +const ISO_639_1_TO_2: &[(&str, &str)] = &[ + ("aa", "aar"), + ("ab", "abk"), + ("ae", "ave"), + ("af", "afr"), + ("ak", "aka"), + ("am", "amh"), + ("an", "arg"), + ("ar", "ara"), + ("as", "asm"), + ("av", "ava"), + ("ay", "aym"), + ("az", "aze"), + ("ba", "bak"), + ("be", "bel"), + ("bg", "bul"), + ("bh", "bih"), + ("bi", "bis"), + ("bm", "bam"), + ("bn", "ben"), + ("bo", "bod"), + ("br", "bre"), + ("bs", "bos"), + ("ca", "cat"), + ("ce", "che"), + ("ch", "cha"), + ("co", "cos"), + ("cr", "cre"), + ("cs", "ces"), + ("cu", "chu"), + ("cv", "chv"), + ("cy", "cym"), + ("da", "dan"), + ("de", "deu"), + ("dv", "div"), + ("dz", "dzo"), + ("ee", "ewe"), + ("el", "ell"), + ("en", "eng"), + ("eo", "epo"), + ("es", "spa"), + ("et", "est"), + ("eu", "eus"), + ("fa", "fas"), + ("ff", "ful"), + ("fi", "fin"), + ("fj", "fij"), + ("fo", "fao"), + ("fr", "fra"), + ("fy", "fry"), + ("ga", "gle"), + ("gd", "gla"), + ("gl", "glg"), + ("gn", "grn"), + ("gu", "guj"), + ("gv", "glv"), + ("ha", "hau"), + ("he", "heb"), + ("hi", "hin"), + ("ho", "hmo"), + ("hr", "hrv"), + ("ht", "hat"), + ("hu", "hun"), + ("hy", "hye"), + ("hz", "her"), + ("ia", "ina"), + ("id", "ind"), + ("ie", "ile"), + ("ig", "ibo"), + ("ii", "iii"), + ("ik", "ipk"), + ("io", "ido"), + ("is", "isl"), + ("it", "ita"), + ("iu", "iku"), + ("ja", "jpn"), + ("jv", "jav"), + ("ka", "kat"), + ("kg", "kon"), + ("ki", "kik"), + ("kj", "kua"), + ("kk", "kaz"), + ("kl", "kal"), + ("km", "khm"), + ("kn", "kan"), + ("ko", "kor"), + ("kr", "kau"), + ("ks", "kas"), + ("ku", "kur"), + ("kv", "kom"), + ("kw", "cor"), + ("ky", "kir"), + ("la", "lat"), + ("lb", "ltz"), + ("lg", "lug"), + ("li", "lim"), + ("ln", "lin"), + ("lo", "lao"), + ("lt", "lit"), + ("lu", "lub"), + ("lv", "lav"), + ("mg", "mlg"), + ("mh", "mah"), + ("mi", "mri"), + ("mk", "mkd"), + ("ml", "mal"), + ("mn", "mon"), + ("mr", "mar"), + ("ms", "msa"), + ("mt", "mlt"), + ("my", "mya"), + ("na", "nau"), + ("nb", "nob"), + ("nd", "nde"), + ("ne", "nep"), + ("ng", "ndo"), + ("nl", "nld"), + ("nn", "nno"), + ("no", "nor"), + ("nr", "nbl"), + ("nv", "nav"), + ("ny", "nya"), + ("oc", "oci"), + ("oj", "oji"), + ("om", "orm"), + ("or", "ori"), + ("os", "oss"), + ("pa", "pan"), + ("pi", "pli"), + ("pl", "pol"), + ("ps", "pus"), + ("pt", "por"), + ("qu", "que"), + ("rm", "roh"), + ("rn", "run"), + ("ro", "ron"), + ("ru", "rus"), + ("rw", "kin"), + ("sa", "san"), + ("sc", "srd"), + ("sd", "snd"), + ("se", "sme"), + ("sg", "sag"), + ("si", "sin"), + ("sk", "slk"), + ("sl", "slv"), + ("sm", "smo"), + ("sn", "sna"), + ("so", "som"), + ("sq", "sqi"), + ("sr", "srp"), + ("ss", "ssw"), + ("st", "sot"), + ("su", "sun"), + ("sv", "swe"), + ("sw", "swa"), + ("ta", "tam"), + ("te", "tel"), + ("tg", "tgk"), + ("th", "tha"), + ("ti", "tir"), + ("tk", "tuk"), + ("tl", "tgl"), + ("tn", "tsn"), + ("to", "ton"), + ("tr", "tur"), + ("ts", "tso"), + ("tt", "tat"), + ("tw", "twi"), + ("ty", "tah"), + ("ug", "uig"), + ("uk", "ukr"), + ("ur", "urd"), + ("uz", "uzb"), + ("ve", "ven"), + ("vi", "vie"), + ("vo", "vol"), + ("wa", "wln"), + ("wo", "wol"), + ("xh", "xho"), + ("yi", "yid"), + ("yo", "yor"), + ("za", "zha"), + ("zh", "zho"), + ("zu", "zul"), +]; + +/// The three two-letter codes ISO 639-1 has since withdrawn, mapped to their +/// replacements. DVD-Video froze its language list on the 1988 edition, so +/// discs authored to the spec carry these spellings and no other table sees +/// them: `iw` Hebrew (now `he`), `in` Indonesian (now `id`), `ji` Yiddish +/// (now `yi`). +const ISO_639_1_DEPRECATED: &[(&str, &str)] = &[("iw", "he"), ("in", "id"), ("ji", "yi")]; + +/// Map an ISO 639-1 two-letter language code to its ISO 639-2/T three-letter +/// code, accepting the withdrawn DVD-era spellings (`iw`, `in`, `ji`) as +/// aliases for their replacements. +/// +/// Covers the WHOLE of ISO 639-1, unlike [`menu_lang`], whose table only spans +/// the languages that show up in Blu-ray menu-graphic filenames. Callers that +/// convert a spec field — a DVD IFO attribute block, say — need the whole set: +/// narrowing it to the menu vocabulary would fold every other language onto +/// one value and make a disc's tracks indistinguishable from each other. +/// +/// Case-insensitive and trimmed. Returns `None` for anything that is not an +/// ISO 639-1 code, so callers decide the fallback rather than getting a guess. +pub fn iso639_1_to_iso639_2(code: &str) -> Option<&'static str> { + let c = code.trim().to_ascii_lowercase(); + let c = ISO_639_1_DEPRECATED + .iter() + .find(|(old, _)| *old == c) + .map_or(c.as_str(), |(_, new)| new); + ISO_639_1_TO_2 + .iter() + .find(|(two, _)| *two == c) + .map(|(_, three)| *three) +} + // ── Purpose ────────────────────────────────────────────────────────────────── /// Classify a free-form English label string into a [`LabelPurpose`]. @@ -789,4 +1020,99 @@ mod tests { assert_eq!(menu_lang("xyz"), None); assert_eq!(menu_lang(""), None); } + + /// Structural invariants of `ISO_639_1_TO_2`: it must hold the complete + /// ISO 639-1 set (184 codes), every key a distinct pair of lowercase + /// letters and every value three lowercase letters. A typo'd or duplicated + /// row fails here rather than silently mislabelling a track. + #[test] + fn iso639_1_table_is_complete_and_well_formed() { + assert_eq!( + ISO_639_1_TO_2.len(), + 184, + "ISO 639-1 defines 184 two-letter codes; the table must hold all \ + of them" + ); + let mut keys: Vec<&str> = ISO_639_1_TO_2.iter().map(|(two, _)| *two).collect(); + keys.sort_unstable(); + let unique = keys.len(); + keys.dedup(); + assert_eq!(unique, keys.len(), "no ISO 639-1 code may appear twice"); + for (two, three) in ISO_639_1_TO_2 { + assert!( + two.len() == 2 && two.bytes().all(|b| b.is_ascii_lowercase()), + "{two:?} is not a two-letter lowercase ISO 639-1 code" + ); + assert!( + three.len() == 3 && three.bytes().all(|b| b.is_ascii_lowercase()), + "{three:?} is not a three-letter lowercase ISO 639-2 code" + ); + } + // The withdrawn DVD-era spellings resolve, and are not themselves + // rows in the main table (they are aliases, not codes). + for (old, new) in ISO_639_1_DEPRECATED { + assert!( + !ISO_639_1_TO_2.iter().any(|(two, _)| two == old), + "withdrawn code {old:?} must not be a table row" + ); + assert_eq!( + iso639_1_to_iso639_2(old), + iso639_1_to_iso639_2(new), + "withdrawn code {old:?} must resolve exactly as {new:?}" + ); + } + } + + /// The two tables must not disagree. Every two-letter token `menu_lang` + /// accepts has to yield the same ISO 639-2/T code through + /// `iso639_1_to_iso639_2`, so a DVD-sourced language and a Blu-ray + /// menu-label language for the same tongue never produce different + /// `Language` elements. + #[test] + fn iso639_1_agrees_with_menu_lang() { + for (two, three) in ISO_639_1_TO_2 { + if let Some(via_menu) = menu_lang(two) { + assert_eq!( + via_menu, *three, + "menu_lang({two:?}) = {via_menu:?} disagrees with the ISO \ + 639-1 table's {three:?}" + ); + } + } + // Spot-check the /T choice itself, on the languages where /B differs. + for (two, t_code) in [ + ("de", "deu"), + ("fr", "fra"), + ("zh", "zho"), + ("cs", "ces"), + ("nl", "nld"), + ("el", "ell"), + ("ro", "ron"), + ("sk", "slk"), + ("is", "isl"), + ("hy", "hye"), + ("ka", "kat"), + ("fa", "fas"), + ] { + assert_eq!( + iso639_1_to_iso639_2(two), + Some(t_code), + "the crate standardises on ISO 639-2/T, so {two:?} is \ + {t_code:?} and never the bibliographic form" + ); + } + } + + /// Trimming, case-insensitivity, and the no-guess contract. + #[test] + fn iso639_1_normalizes_input_and_never_guesses() { + assert_eq!(iso639_1_to_iso639_2("RO"), Some("ron")); + assert_eq!(iso639_1_to_iso639_2(" Ro "), Some("ron")); + assert_eq!(iso639_1_to_iso639_2("IW"), Some("heb")); + assert_eq!(iso639_1_to_iso639_2("zz"), None); + assert_eq!(iso639_1_to_iso639_2(""), None); + assert_eq!(iso639_1_to_iso639_2("e"), None); + // A three-letter code is not ISO 639-1 input — that is menu_lang's job. + assert_eq!(iso639_1_to_iso639_2("eng"), None); + } } diff --git a/src/mux/codec/ac3.rs b/src/mux/codec/ac3.rs index e93632f..7f95d10 100644 --- a/src/mux/codec/ac3.rs +++ b/src/mux/codec/ac3.rs @@ -91,6 +91,18 @@ pub struct Ac3Parser { /// across the PES boundary because it may be the core of an AC-3-core + /// E-AC-3-dependent frame set whose remaining substreams are in the next PES. saw_extension: bool, + /// The access unit held open across the last PES boundary, ALREADY + /// scanned. The carry-over begins at its first byte, so without this the + /// next call re-scans and re-CRCs every syncframe of it from byte 0 — and + /// an access unit that keeps gaining substreams grows to [`MAX_AC3_BUF`] + /// (1 MiB) before the resync guard drops it, which on a ~2 KiB DVD PES is + /// three orders of magnitude of repeated work per packet. + held: Option<HeldAu>, + /// Test-only: syncframes examined (sized + CRC-gated) by + /// `scan_access_units`. Pins the resume above — the property it exists for + /// is a WORK bound, which no frame-level assertion can observe. + #[cfg(test)] + frames_scanned: u64, } impl Default for Ac3Parser { @@ -107,6 +119,9 @@ impl Ac3Parser { flush_pts_ns: 0, tally: super::dropgate::DropTally::new("ac3"), saw_extension: false, + held: None, + #[cfg(test)] + frames_scanned: 0, } } @@ -149,20 +164,56 @@ impl Ac3Parser { anchor: Option<PtsAnchor>, at_eos: bool, marks: &[(usize, super::pesbuf::PesFacts)], - ) -> (Vec<Frame>, usize, i64) { + held: Option<HeldAu>, + ) -> ScanOut { let mut frames = Vec::new(); let mut pos = 0usize; // Running PTS for the next access unit to emit in this call. let mut frame_pts_ns = base_pts_ns; let mut anchor = anchor; let mut pending: Option<PendingAu> = None; + // How far this call has proved there is no further syncframe to + // process; carried over so the held access unit's own bytes (and the + // junk after them) are not searched again next call. + let mut scanned_to = 0usize; + + // Resume a held access unit instead of re-deriving it. `keep_from` was + // its first byte, so it starts at 0 of this buffer, and every frame in + // it was sized and CRC-gated on the call that built it. + if let Some(h) = held { + let mut drop_reason = h.drop_reason; + // The one verdict that can have changed since: the track may have + // become poisoned while this access unit was held, and a re-scan + // would have picked that up. + if drop_reason.is_none() && self.tally.is_poisoned() { + drop_reason = Some("track-poisoned"); + } + pending = Some(PendingAu { + start: 0, + end: h.end, + pts_ns: base_pts_ns, + duration_ns: h.duration_ns, + drop_reason, + bsid: h.bsid, + }); + frame_pts_ns = base_pts_ns + h.duration_ns as i64; + pos = h.scanned_to; + scanned_to = h.scanned_to; + } while pos < data.len() { let sync = find_ac3_sync(&data[pos..]); let start = match sync { Some(offset) => pos + offset, - None => break, + None => { + // No syncword in `data[pos..]` at all: every byte but the + // last is proved sync-free (a syncword is two bytes and the + // second may still arrive). + scanned_to = data.len().saturating_sub(1).max(pos); + break; + } }; + scanned_to = start; let remaining = &data[start..]; @@ -182,6 +233,7 @@ impl Ac3Parser { // Invalid/sub-header frame size (e.g. an E-AC-3 frmsiz of 0/1 // sizing to a 2/4-byte fragment) — skip this sync word. pos = start + 2; + scanned_to = pos; continue; } @@ -191,6 +243,10 @@ impl Ac3Parser { } let frame = &data[start..start + frame_size]; + #[cfg(test)] + { + self.frames_scanned += 1; + } // Decodability gate: a syncframe with an out-of-range bsid (> 16) or // a failed native CRC (payload corruption) poisons the access unit it // belongs to — a dependent substream is useless without its parent and @@ -258,6 +314,7 @@ impl Ac3Parser { } pos = start + frame_size; + scanned_to = pos; } // Close or HOLD the trailing access unit. The rest of its frame set — its @@ -271,10 +328,22 @@ impl Ac3Parser { // substream that extends an access unit — a plain AC-3 track keeps // emitting every frame in-call. let mut hold_from = None; + let mut held_out = None; if let Some(au) = pending { if !at_eos && (au.bsid >= 11 || self.saw_extension) { frame_pts_ns = au.pts_ns; hold_from = Some(au.start); + // Everything below `scanned_to` has been searched already, and + // the access unit's own frames have been sized and CRC-gated; + // record both, rebased onto the carry-over (which starts at + // `au.start`), so the next call resumes instead of redoing it. + held_out = Some(HeldAu { + end: au.end - au.start, + scanned_to: scanned_to.max(au.end) - au.start, + duration_ns: au.duration_ns, + drop_reason: au.drop_reason, + bsid: au.bsid, + }); } else { close_access_unit(&mut self.tally, data, &au, marks, &mut frames); } @@ -306,7 +375,12 @@ impl Ac3Parser { None => data.len(), }; - (frames, keep_from, frame_pts_ns) + ScanOut { + frames, + keep_from, + frame_pts_ns, + held: held_out, + } } } @@ -319,6 +393,36 @@ struct PtsAnchor { pts_ns: i64, } +/// What one `scan_access_units` pass produced: the access units it emitted, +/// the offset in the scanned buffer from which bytes must be carried over to +/// the next call, the PTS to stamp on the access unit that begins that +/// carry-over, and — when the trailing access unit was HELD — the state that +/// lets the next call resume rather than re-derive it. +struct ScanOut { + frames: Vec<Frame>, + keep_from: usize, + frame_pts_ns: i64, + held: Option<HeldAu>, +} + +/// A trailing access unit held across the PES boundary, already scanned. +/// Offsets are relative to the carry-over, which begins at the access unit's +/// first byte — so the access unit occupies `0..end`. +#[derive(Clone, Copy)] +struct HeldAu { + /// End of the access unit's bytes. + end: usize, + /// How far the scan that built it had searched (`>= end`). Bytes below it + /// hold no further syncframe to process. + scanned_to: usize, + /// Duration contributed by the access unit's `substreamid`-0 substream. + duration_ns: u64, + /// Decodability verdict reached for it so far. + drop_reason: Option<&'static str>, + /// bsid of the substream that opened it. + bsid: u8, +} + /// An access unit (frame set) under construction: `data[start..end]` is the /// `substreamid`-0 independent substream frame plus every substream appended to it /// so far — its dependents, and any additional independent substreams 1..7 with @@ -477,6 +581,8 @@ impl CodecParser for Ac3Parser { // non-empty PES today; this is defensive for any future caller). if pes.discontinuity { self.acc.clear(); + // The held access unit's bytes went with it. + self.held = None; } if pes.data.is_empty() { return Vec::new(); @@ -516,8 +622,13 @@ impl CodecParser for Ac3Parser { buf.extend_from_slice(self.acc.as_slice()); let marks = self.acc.marks_snapshot(); let data = &buf; - let (frames, keep_from, frame_pts_ns) = - self.scan_access_units(data, self.flush_pts_ns, anchor, false, &marks); + let held = self.held.take(); + let ScanOut { + frames, + keep_from, + frame_pts_ns, + held: still_held, + } = self.scan_access_units(data, self.flush_pts_ns, anchor, false, &marks, held); if keep_from < data.len() { let tail = &data[keep_from..]; @@ -531,6 +642,7 @@ impl CodecParser for Ac3Parser { MAX_AC3_BUF ); self.acc.clear(); + self.held = None; // Advance the cadence, as both sibling branches below do, so the // three paths out of this block cannot disagree. Defensive: no // input reaching this parser was found that both parses frames and @@ -539,6 +651,7 @@ impl CodecParser for Ac3Parser { self.flush_pts_ns = frame_pts_ns; } else { self.acc.drain(keep_from); + self.held = still_held; // The carried bytes, when later completed and emitted (next call // or by flush() at EOS), are timed at the PTS the scanner reached // here: the PTS of the next access unit in presentation order, or @@ -548,6 +661,7 @@ impl CodecParser for Ac3Parser { } } else { self.acc.clear(); + self.held = None; // Nothing carried, but keep the cadence so a following PES with no // PTS (no anchor) continues the timeline instead of reusing a stale // value. @@ -569,9 +683,10 @@ impl CodecParser for Ac3Parser { let buf = self.acc.as_slice().to_vec(); let marks = self.acc.marks_snapshot(); self.acc.clear(); + let held = self.held.take(); let out = self - .scan_access_units(&buf, self.flush_pts_ns, None, true, &marks) - .0; + .scan_access_units(&buf, self.flush_pts_ns, None, true, &marks, held) + .frames; // Aggregate drop report at end-of-stream (warn-level, always visible). self.tally.log_summary(); out @@ -2199,6 +2314,125 @@ mod tests { ); } + /// A 256-byte E-AC-3 syncframe with a valid CRC. `strmtyp`/`substreamid` + /// go into byte 2 (A/52 Annex E), which is what `substream_role` reads: + /// (0, 0) OPENS an access unit, (1, 0) is a dependent substream that + /// EXTENDS the open one. + fn eac3_substream_frame(strmtyp: u8, substreamid: u8) -> Vec<u8> { + const SIZE: usize = 256; + let frmsiz = SIZE / 2 - 1; // (frmsiz + 1) * 2 == SIZE + let mut f = vec![0u8; SIZE]; + f[0] = 0x0B; + f[1] = 0x77; + f[2] = (strmtyp << 6) | (substreamid << 3) | ((frmsiz >> 8) as u8 & 0x07); + f[3] = (frmsiz & 0xFF) as u8; + f[5] = 16 << 3; // bsid 16 → E-AC-3 + finalize_ac3_crc(&mut f); + f + } + + /// An access unit closes only at the next `substreamid`-0 independent + /// substream, so one that keeps gaining dependent substreams stays OPEN + /// across PES boundaries and its bytes stay in the carry-over. The + /// carry-over must not be re-scanned — and re-CRCed — from the access + /// unit's first byte on every packet: the buffer only stops growing at + /// MAX_AC3_BUF (1 MiB), and a PES on a DVD is about 2 KiB, so re-deriving + /// the held access unit costs work quadratic in the packets fed. + /// + /// Measured directly, because a work bound has no frame-level shadow: + /// `frames_scanned` counts the syncframes the scanner sizes and CRC-gates. + /// Re-scanning from byte 0 examines 1 + 2 + ... + (N+1) frames. + /// + /// Mutation: pass `None` for `held` in `parse` (or drop the `if let + /// Some(h) = held` resume) — the count returns to the quadratic figure. + #[test] + fn a_held_access_unit_is_not_rescanned_from_its_first_frame_every_packet() { + const DEPENDENTS: usize = 200; + + let mut parser = Ac3Parser::new(); + // Opens the access unit. + let emitted = parser.parse(&make_eac3_pes(eac3_substream_frame(0, 0))); + assert!( + emitted.is_empty(), + "the access unit is held open, not emitted" + ); + for _ in 0..DEPENDENTS { + let f = parser.parse(&make_eac3_pes(eac3_substream_frame(1, 0))); + assert!(f.is_empty(), "a dependent substream extends the open unit"); + } + + let fed = (DEPENDENTS + 1) as u64; + assert!( + parser.frames_scanned <= 2 * fed, + "the scanner examined {} syncframes for {fed} fed — a held access \ + unit must be resumed, not re-derived", + parser.frames_scanned + ); + + // ...and the resume must not have cost correctness: the whole frame + // set is still one access unit, emitted intact at EOS. + let out = parser.flush(); + assert_eq!(out.len(), 1, "the frame set is a single access unit"); + assert_eq!( + out[0].data.len(), + 256 * (DEPENDENTS + 1), + "every substream of the frame set belongs to it" + ); + } + + /// A concealed gap must drop the HELD access unit, not just the byte + /// buffer. + /// + /// `parse` clears `self.acc` on a discontinuity because the buffered bytes + /// are a truncated frame. The held access unit is described by OFFSETS into + /// exactly those bytes, so it has to go with them. Without + /// `self.held = None`, the next packet resumes a HeldAu whose `start`/`end` + /// were computed against the pre-gap buffer but are applied to the + /// unrelated post-gap bytes — splicing audio across the gap at best, and + /// indexing past the end of the new, shorter buffer at worst. + /// + /// The two existing discontinuity tests use plain AC-3 (bsid < 11), which + /// never holds an access unit open, so neither of them reaches this reset. + #[test] + fn a_discontinuity_drops_the_held_access_unit_with_its_bytes() { + let mut parser = Ac3Parser::new(); + + // Open an access unit and extend it, so a HeldAu exists describing + // offsets into a large buffer. + assert!( + parser + .parse(&make_eac3_pes(eac3_substream_frame(0, 0))) + .is_empty(), + "the access unit is held open, not emitted" + ); + for _ in 0..8 { + assert!( + parser + .parse(&make_eac3_pes(eac3_substream_frame(1, 0))) + .is_empty(), + "a dependent substream extends the open unit" + ); + } + + // The gap. Its post-gap payload is deliberately far SHORTER than the + // held unit's bytes, so a stale HeldAu indexes past its end. + let mut gap = make_eac3_pes(eac3_substream_frame(0, 0)); + gap.discontinuity = true; + let _ = parser.parse(&gap); + + // Whatever comes out, nothing may carry pre-gap bytes: the truncated + // unit was dropped, so the only access unit that can be emitted is the + // one opened after the gap. + let out = parser.flush(); + let total: usize = out.iter().map(|f| f.data.len()).sum(); + assert!( + total <= 256, + "a post-gap access unit must not be spliced onto the 9 frames held \ + before the gap; got {total} bytes across {} frame(s)", + out.len() + ); + } + // helper: PES with a generic pts for E-AC-3 tests fn make_eac3_pes(data: Vec<u8>) -> PesPacket { PesPacket { diff --git a/src/mux/codec/crc.rs b/src/mux/codec/crc.rs index 6d6f3ec..aab6db3 100644 --- a/src/mux/codec/crc.rs +++ b/src/mux/codec/crc.rs @@ -29,10 +29,17 @@ pub(crate) fn crc16_ansi(data: &[u8]) -> u16 { } /// CRC-16 with polynomial 0x002D, init 0, MSB-first, used by the MLP / Dolby -/// TrueHD major-sync header checksum. NOTE: MLP's checksum is the "reversed" -/// scheme — the stored trailer word is the little-endian-read CRC, so this -/// standard CRC must be compared against the stored bytes read big-endian. -/// The caller handles that comparison (see `truehd::mlp_major_sync_ok`). +/// TrueHD major-sync header checksum. +/// +/// NOTE: MLP's checksum is the "reversed" scheme. This function emits its two +/// bytes in the OPPOSITE order to a standard little-endian CRC readout, so the +/// caller swaps them back and compares against the stored trailer word read +/// LITTLE-endian — see `truehd::mlp_major_sync_crc_ok`, which is authoritative. +/// +/// Comparing big-endian instead is precisely the bug that function was fixed +/// for: it could never validate a real extended major sync, so whole TrueHD +/// tracks were dropped silently. This comment used to prescribe exactly that, +/// and to point at a `truehd::mlp_major_sync_ok` that does not exist. /// Verified against real MLP/TrueHD bitstreams (225/225 major-sync AUs). pub(crate) fn crc16_mlp(data: &[u8]) -> u16 { let mut crc: u16 = 0; @@ -103,8 +110,13 @@ mod tests { #[test] fn crc16_mlp_residue_property_holds() { - // Appending the big-endian CRC zeroes the residue over message+crc — the - // scheme `truehd::mlp_major_sync_ok` relies on. + // Appending the big-endian CRC zeroes the residue over message+crc. + // This is a property of the CRC itself, pinned here so a change to the + // polynomial or the bit order is caught. It is NOT how the TrueHD + // caller validates a major sync: `truehd::mlp_major_sync_crc_ok` does a + // swap-and-XOR compare against the little-endian trailer word. (This + // comment used to claim the caller relied on the residue, and named a + // `truehd::mlp_major_sync_ok` that does not exist.) let msg = [0xF8u8, 0x72, 0x6F, 0xBA]; let c = crc16_mlp(&msg); let mut framed = msg.to_vec(); diff --git a/src/mux/codec/dvdsub.rs b/src/mux/codec/dvdsub.rs index 62930e9..ab73da2 100644 --- a/src/mux/codec/dvdsub.rs +++ b/src/mux/codec/dvdsub.rs @@ -159,7 +159,13 @@ impl CodecParser for DvdSubParser { /// Convert a single YCbCr color to RGB, clamping to [0, 255]. /// -/// Input: `[padding, Y, Cb, Cr]` (as stored in DVD IFO PGC data). +/// Input: `[padding, Y, Cr, Cb]` (as stored in DVD IFO PGC data). Note the +/// chroma order: the on-disc PGC CLUT is **Cr before Cb** — byte 2 is Cr and +/// byte 3 is Cb. Reading byte 2 as Cb swaps red and blue on every chromatic +/// entry, and is invisible on the achromatic (white/black/grey, Cb = Cr = 128) +/// entries that dominate real palettes, which is how it survives casual +/// inspection. The order is fixed by the DVD-Video PGC format, not by us. +/// /// Returns `[R, G, B]`. /// /// Range convention (deliberate): this uses the **full-range (JFIF) BT.601** @@ -175,8 +181,8 @@ impl CodecParser for DvdSubParser { /// side in lockstep. pub fn ycbcr_to_rgb(color: &[u8; 4]) -> [u8; 3] { let y = color[1] as f64; - let cb = color[2] as f64; - let cr = color[3] as f64; + let cr = color[2] as f64; + let cb = color[3] as f64; let r = y + 1.402 * (cr - 128.0); let g = y - 0.344 * (cb - 128.0) - 0.714 * (cr - 128.0); @@ -198,7 +204,7 @@ fn clamp_u8(v: f64) -> u8 { /// Format a 16-color YCbCr palette as a VobSub `.idx` header for S_VOBSUB /// CodecPrivate. /// -/// Each entry is `[padding, Y, Cb, Cr]`. Output is a UTF-8 text block carrying +/// Each entry is `[padding, Y, Cr, Cb]`. Output is a UTF-8 text block carrying /// the two `.idx` header lines mkvmerge / libvobsub expect: /// /// ```text @@ -432,24 +438,26 @@ mod tests { #[test] fn ycbcr_to_rgb_clamps_overflow() { - // Y=255, Cr=255 → R would be 255 + 1.402*127 = ~433, should clamp to 255 - let color = [0x00, 255, 128, 255]; + // Y=255, Cr=255 → R would be 255 + 1.402*127 = ~433, should clamp to 255. + // Cr is byte 2 in the on-disc [pad, Y, Cr, Cb] layout. + let color = [0x00, 255, 255, 128]; let [r, _g, _b] = ycbcr_to_rgb(&color); assert_eq!(r, 255); } #[test] fn ycbcr_to_rgb_clamps_underflow() { - // Y=0, Cr=0 → R = 0 + 1.402*(0-128) = -179, should clamp to 0 - let color = [0x00, 0, 128, 0]; + // Y=0, Cr=0 → R = 0 + 1.402*(0-128) = -179, should clamp to 0. + // Cr is byte 2 in the on-disc [pad, Y, Cr, Cb] layout. + let color = [0x00, 0, 0, 128]; let [r, _g, _b] = ycbcr_to_rgb(&color); assert_eq!(r, 0); } #[test] fn ycbcr_to_rgb_red() { - // Approximate red: Y=82, Cb=90, Cr=240 - let color = [0x00, 82, 90, 240]; + // Approximate red: Y=82, Cr=240, Cb=90 — on disc as [pad, Y, Cr, Cb]. + let color = [0x00, 82, 240, 90]; let [r, g, b] = ycbcr_to_rgb(&color); // R = 82 + 1.402*(240-128) = 82 + 156.9 ≈ 239 // G = 82 - 0.344*(90-128) - 0.714*(240-128) = 82 + 13.1 - 79.97 ≈ 15 @@ -459,6 +467,46 @@ mod tests { assert!(b < 30, "B should be low for red, got {}", b); } + /// On-disc DVD PGC CLUT byte order is `[0, Y, Cr, Cb]` — byte 2 is **Cr** + /// and byte 3 is **Cb**, per the DVD-Video PGC format. + /// + /// This fixture uses a real on-disc red entry, so it fails if the two + /// chroma bytes are ever exchanged again. It is deliberately NOT built + /// from this crate's own doc comments: those described the order wrongly + /// for a long time, and the previous version of this test inherited the + /// error from them and therefore could not detect it. + /// + /// A saturated RED entry therefore appears on disc as Y=76, Cr=255, Cb=85 + /// (full-range BT.601 encoding of RGB #FF0000), i.e. bytes + /// `[0x00, 76, 255, 85]`. Reading byte 2 as Cb and byte 3 as Cr instead + /// turns this entry BLUE, which is the exact user-visible symptom. + /// + /// The pre-existing `_white` / `_black` tests cannot catch this: they use + /// Cb = Cr = 128, so exchanging two equal bytes is a literal no-op. + #[test] + fn ycbcr_to_rgb_reads_byte2_as_cr_and_byte3_as_cb() { + // On-disc [pad, Y, Cr, Cb] for saturated red. + let on_disc_red = [0x00u8, 76, 255, 85]; + let [r, g, b] = ycbcr_to_rgb(&on_disc_red); + + assert!( + r > 200 && b < 60, + "on-disc red [0,Y=76,Cr=255,Cb=85] must render red-dominant, \ + got R={r} G={g} B={b} (R and B swapped => byte 2/3 are transposed)" + ); + assert_eq!([r, g, b], [254, 0, 0], "exact full-range BT.601 red"); + + // And the converse: a saturated BLUE on-disc entry (Y=29, Cr=107, Cb=255) + // must not come out red. + let on_disc_blue = [0x00u8, 29, 107, 255]; + let [r2, g2, b2] = ycbcr_to_rgb(&on_disc_blue); + assert!( + b2 > 200 && r2 < 60, + "on-disc blue [0,Y=29,Cr=107,Cb=255] must render blue-dominant, \ + got R={r2} G={g2} B={b2}" + ); + } + // ── Palette formatting tests ────────────────────────────────────────── #[test] @@ -699,7 +747,8 @@ mod tests { #[test] fn ycbcr_blue_channel_clamps_high() { // B = Y + 1.772*(Cb-128). Y=128, Cb=255 → 128 + 1.772*127 ≈ 353 → clamp 255. - let [_r, _g, b] = ycbcr_to_rgb(&[0x00, 128, 255, 128]); + // Cb is byte 3 in the on-disc [pad, Y, Cr, Cb] layout. + let [_r, _g, b] = ycbcr_to_rgb(&[0x00, 128, 128, 255]); assert_eq!(b, 255, "blue clamps at 255"); } diff --git a/src/mux/disc.rs b/src/mux/disc.rs index a05a5c1..fa2cee2 100644 --- a/src/mux/disc.rs +++ b/src/mux/disc.rs @@ -566,6 +566,20 @@ impl DiscStream { .into()); } + // The read SOURCE is gone (a prefetch producer thread that + // terminated), not one range of media. Shrinking and retrying at + // the same LBA asks a dead source for data it can never produce, + // and the `skip_errors` branch below would then zero-fill and + // advance over every remaining sector of the title and still + // return success. Abort with the terminal error itself — a + // fabricated SCSI status would be a lie, so this is deliberately + // NOT folded into the transport-failure arm above. + if let Some(e) = res.as_ref().err() + && e.is_source_terminated() + { + return Err(crate::error::Error::SourceTerminated.into()); + } + if (sectors as u32) <= align { // Bottomed out at one unit (AACS) / one sector (CSS) / the // extent tail. This is single-pass disc→MKV, which has NO Pass N @@ -621,6 +635,16 @@ impl DiscStream { .into()); } + // Same rule as after the first-attempt read: the 60s recovery + // read goes through the same source, so it can be the call + // that discovers the source is dead. Skipping the unit would + // zero-fill the rest of the title as fabricated content. + if let Some(e) = rec.as_ref().err() + && e.is_source_terminated() + { + return Err(crate::error::Error::SourceTerminated.into()); + } + // Recovery read also failed. Skip the WHOLE failed unit or bail. // Zero-filling and advancing by the full unit keeps // current_offset unit-aligned, so the next read still begins on a @@ -1859,6 +1883,130 @@ mod tests { ); } + /// REGRESSION (round-4 audit): an ordinary MEDIUM ERROR bad sector must + /// keep its identity when it crosses the prefetch producer channel — the + /// same `DiscRead` with its SCSI status, NOT a transport failure. + /// + /// `PrefetchedSectorSource::read_sectors` re-wrapped every error that + /// crossed the channel as `Error::IoError`, and `is_scsi_transport_failure` + /// matches `IoError` (the wedged-USB-bridge arm). So a bad sector reached + /// `fill_extents` looking like a dead bus and aborted the pass with a + /// fabricated status 0xFF — the exact inverse of what that short-circuit + /// exists for, and it told the user to power-cycle a healthy drive. + /// + /// Asserted on the source, not on a `fill_extents` skip: the producer + /// thread exits for good after sending an error, so nothing downstream of + /// it can genuinely recover the rest of the title (see + /// `dead_prefetch_producer_does_not_silently_zero_fill_the_title`). An + /// assertion that the pass continues could only ever have been satisfied + /// by fabricated zeros. + #[test] + fn bad_sector_keeps_its_identity_across_the_prefetch_channel() { + const COUNT: u32 = 9; + let log = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let reader = RecordingReader { + capacity: COUNT, + bad_sector: 4, + log: log.clone(), + }; + let mut prefetched = crate::sector::PrefetchedSectorSource::new_with_events( + reader, + vec![crate::disc::Extent { + start_lba: 0, + sector_count: COUNT, + }], + 8, + 1, + None, + None, + ) + .expect("spawn producer"); + + let mut buf = vec![0u8; 8 * 2048]; + let err = crate::sector::SectorSource::read_sectors(&mut prefetched, 0, 8, &mut buf, false) + .expect_err("the batch covering the bad sector must fail"); + assert!( + !err.is_scsi_transport_failure(), + "a MEDIUM ERROR bad sector is not a dead bus; got {err:?}" + ); + assert!( + matches!( + err, + crate::error::Error::DiscRead { + sector: 4, + status: Some(0x02), + .. + } + ), + "the producer's typed error must survive the channel intact; got {err:?}" + ); + } + + /// The prefetch producer thread terminates PERMANENTLY on its first read + /// error, so once one bad sector has crossed the channel the source can + /// never deliver another byte. Driving `fill_extents` to exhaustion after + /// that must NOT look like a completed pass: every remaining sector would + /// be fabricated zeros, and DATA LOSS MUST NEVER LOOK LIKE SUCCESS. + /// + /// The expectation is the product rule, not the code: a source that is + /// permanently out of data must report that, not answer `Ok(0)` forever — + /// which `commit_read` legitimately reads as an ordinary short read and + /// zero-fills. + #[test] + fn dead_prefetch_producer_does_not_silently_zero_fill_the_title() { + const COUNT: u32 = 30; + let log = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let reader = RecordingReader { + capacity: COUNT, + bad_sector: 4, + log: log.clone(), + }; + let prefetched = crate::sector::PrefetchedSectorSource::new_with_events( + reader, + vec![crate::disc::Extent { + start_lba: 0, + sector_count: COUNT, + }], + 8, + 1, + None, + None, + ) + .expect("spawn producer"); + let mut stream = DiscStream::new( + Box::new(prefetched), + synthetic_title(COUNT), + crate::decrypt::DecryptKeys::None, + 8, + ContentFormat::BdTs, + false, + None, + ) + .unwrap(); + stream.skip_errors = true; + + // Drive the whole title. Bounded so a regression cannot hang the suite. + let mut completed_clean = false; + for _ in 0..(COUNT as usize * 4) { + match stream.fill_extents() { + Ok(true) => continue, + Ok(false) => { + completed_clean = true; + break; + } + Err(_) => break, + } + } + assert!( + !completed_clean, + "the producer died at sector 4, so sectors 4..{COUNT} were never \ + read — reporting the pass as complete zero-fills {} of {} bytes \ + and calls it success", + stream.lost_bytes, + COUNT as u64 * 2048 + ); + } + /// AACS unit-alignment skip (the #1 coverage gap). With `unit_align=3` /// (DecryptKeys::Aacs) and `skip_errors=true`, a single bad mid-extent /// sector must NOT desync the rest of the title: every `read_sectors` diff --git a/src/mux/mkv.rs b/src/mux/mkv.rs index e98ce53..6248a1f 100644 --- a/src/mux/mkv.rs +++ b/src/mux/mkv.rs @@ -359,6 +359,22 @@ fn write_hdr10<W: Write + Seek>(w: &mut W, h: &crate::mux::codec::Hdr10Metadata) Ok(()) } +/// The Matroska `Language` element the muxer writes for a stream whose source +/// reported `lang`. RFC 9559 §12 defines the element as an ISO 639-2 code and +/// gives no meaning to an empty one; the code for "no language stated" is +/// `und`, and that is what a source with no language table (the HD-DVD EVO +/// stream probe, a Blu-ray STN slot with no language bytes) has to emit. The +/// element is written unconditionally by `MkvMuxer::new`, so this is the one +/// place that decides it — a source-side default would have to be repeated in +/// every scanner and would still leave the muxer able to ship an invalid file. +fn language_or_und(lang: &str) -> String { + if lang.is_empty() { + "und".to_string() + } else { + lang.to_string() + } +} + impl MkvTrack { /// Build a video track from a [`VideoStream`]. Language defaults to `"und"`; /// colour metadata is derived from the stream's colour space and HDR format @@ -530,7 +546,7 @@ impl MkvTrack { Self { track_type: ebml::TRACK_TYPE_AUDIO, codec_id, - language: a.language.clone(), + language: language_or_und(&a.language), name, codec_private: None, is_default: !a.secondary, @@ -587,7 +603,7 @@ impl MkvTrack { Self { track_type: ebml::TRACK_TYPE_SUBTITLE, codec_id, - language: s.language.clone(), + language: language_or_und(&s.language), name: String::new(), codec_private: s.codec_data.clone(), is_default: false, @@ -3307,6 +3323,285 @@ mod tests { ); } + /// Read back the value of the FIRST `Language` element in `data` as a + /// UTF-8 string. `LANGUAGE` (0x22B59C) is a 3-byte EBML ID; its size is + /// always a 1-byte VINT for the short strings this writer emits. + fn first_language_value(data: &[u8]) -> &str { + let pos = find_id(data, ebml::LANGUAGE).expect("Language element must be present"); + let len = (data[pos + 3] & 0x7F) as usize; + std::str::from_utf8(&data[pos + 4..pos + 4 + len]).unwrap() + } + + /// RFC 9559 §12 / the Matroska `Language` element spec restrict the + /// legacy `Language` element to the Matroska language form (ISO 639-2, + /// three lowercase letters), never ISO 639-1 (two letters). The DVD + /// IFO audio-attribute block itself carries a raw ISO 639-1 code (e.g. + /// "en") on disc — `ifo::parse_audio_attr` converts it to ISO 639-2 + /// before returning, via `ifo::dvd_lang_to_iso639_2`. This mimics the + /// real DVD pipeline (`disc/dvd.rs`'s `Stream::Audio` construction) end + /// to end: real on-disc IFO bytes -> `ifo::parse_audio_attr` -> + /// `disc::AudioStream` -> `MkvTrack::audio` -> the muxer -> the emitted + /// `Language` element. + #[test] + fn dvd_two_letter_language_becomes_iso_639_2_in_language_element() { + // AC-3 (coding_mode=0), 48 kHz, 6 channels, on-disc language "en" — + // the exact byte layout `ifo::audio_attr_parsing` pins. + let mut attr_bytes = vec![0u8; 8]; + attr_bytes[0] = 0x00; + attr_bytes[1] = 0x05; + attr_bytes[2] = b'e'; + attr_bytes[3] = b'n'; + let attr = crate::ifo::parse_audio_attr(&attr_bytes, 0).unwrap(); + assert_eq!( + attr.language, "eng", + "parse_audio_attr must already return the ISO 639-2 form" + ); + + let audio_stream = crate::disc::AudioStream { + pid: 0xBD80, + codec: attr.codec, + channels: crate::disc::AudioChannels::from_count(attr.channels), + language: attr.language, + sample_rate: crate::disc::SampleRate::from_hz(attr.sample_rate), + secondary: false, + purpose: crate::disc::LabelPurpose::Normal, + label: String::new(), + }; + let track = MkvTrack::audio(&audio_stream); + + let buf = Cursor::new(Vec::new()); + let tracks = [track]; + let muxer = MkvMuxer::new(buf, &tracks, None, 60.0, &[]).unwrap(); + let data = muxer.writer.into_inner(); + + assert_eq!( + first_language_value(&data), + "eng", + "a DVD-sourced ISO 639-1 code must be written as its ISO 639-2 \ + equivalent in the Matroska Language element, per RFC 9559 §12" + ); + } + + /// A source that knows no language at all leaves `language` EMPTY, and the + /// muxer writes the `Language` element unconditionally — so an empty + /// string becomes a zero-length `Language` in the shipped file, which is + /// not a Matroska language form (RFC 9559 §12 wants three ISO 639-2 + /// letters) and is not the ISO 639-2 code for "unknown" either. + /// + /// This is the state every HD-DVD rip is in: `disc::hddvd`'s EVO stream + /// probe has no language table to read and sets `language: String::new()` + /// on every audio stream it finds. The DVD path normalises to "und" in + /// `ifo::parse_audio_attr`; the guard has to exist at the muxer too, which + /// is the one place every source funnels through. + #[test] + fn a_source_with_no_language_emits_und_not_an_empty_language_element() { + // Exactly what `disc::hddvd::probe_evo_streams` builds. + let audio_stream = crate::disc::AudioStream { + pid: 0xBD80, + codec: Codec::Ac3Plus, + channels: crate::disc::AudioChannels::Surround51, + language: String::new(), + sample_rate: crate::disc::SampleRate::S48, + secondary: false, + purpose: crate::disc::LabelPurpose::Normal, + label: String::new(), + }; + let subtitle_stream = crate::disc::SubtitleStream { + pid: 0x1200, + codec: Codec::Pgs, + language: String::new(), + forced: false, + qualifier: crate::disc::LabelQualifier::None, + codec_data: None, + }; + + for track in [ + MkvTrack::audio(&audio_stream), + MkvTrack::subtitle(&subtitle_stream), + ] { + let track_type = track.track_type; + let buf = Cursor::new(Vec::new()); + let tracks = [track]; + let muxer = MkvMuxer::new(buf, &tracks, None, 60.0, &[]).unwrap(); + let data = muxer.writer.into_inner(); + assert_eq!( + first_language_value(&data), + "und", + "track type {track_type}: a stream with no known language must \ + emit the ISO 639-2 'undetermined' code, never a zero-length \ + Language element" + ); + } + } + + /// An unmapped or absent DVD language code (bytes 0x00 0x00 in the IFO + /// attribute block) must degrade to the valid Matroska "undetermined" + /// code `und`, never to an empty string or a raw 2-letter code — both of + /// which violate the Matroska language form. + #[test] + fn dvd_unmapped_or_empty_language_becomes_und_in_language_element() { + // Empty IFO language bytes (0x00 0x00). + let mut empty_bytes = vec![0u8; 8]; + empty_bytes[0] = 0x00; // AC-3, 48k + empty_bytes[1] = 0x05; // 6ch + let empty_attr = crate::ifo::parse_audio_attr(&empty_bytes, 0).unwrap(); + assert_eq!( + empty_attr.language, "und", + "IFO zero bytes (unspecified) must resolve to 'und', not empty" + ); + + // An IFO code with no known ISO 639-1 -> 639-2 mapping (e.g. a + // fictitious "zz"). + let mut unmapped_bytes = vec![0u8; 8]; + unmapped_bytes[0] = 0x00; + unmapped_bytes[1] = 0x05; + unmapped_bytes[2] = b'z'; + unmapped_bytes[3] = b'z'; + let unmapped_attr = crate::ifo::parse_audio_attr(&unmapped_bytes, 0).unwrap(); + assert_eq!( + unmapped_attr.language, "und", + "an on-disc code with no known ISO 639-1 -> 639-2 mapping must \ + degrade to 'und', never pass through raw" + ); + + for attr in [empty_attr, unmapped_attr] { + let audio_stream = crate::disc::AudioStream { + pid: 0xBD80, + codec: attr.codec, + channels: crate::disc::AudioChannels::from_count(attr.channels), + language: attr.language, + sample_rate: crate::disc::SampleRate::from_hz(attr.sample_rate), + secondary: false, + purpose: crate::disc::LabelPurpose::Normal, + label: String::new(), + }; + let track = MkvTrack::audio(&audio_stream); + let buf = Cursor::new(Vec::new()); + let tracks = [track]; + let muxer = MkvMuxer::new(buf, &tracks, None, 60.0, &[]).unwrap(); + let data = muxer.writer.into_inner(); + assert_eq!( + first_language_value(&data), + "und", + "an empty or unmapped DVD language code must degrade to 'und', \ + never an empty string or an invalid ISO 639-1 code" + ); + } + } + + /// Build an 8-byte DVD IFO audio-attribute block (AC-3, 48 kHz, 6ch) + /// carrying `code` in the language bytes, run it through the real DVD + /// pipeline (`ifo::parse_audio_attr` -> `disc::AudioStream` -> + /// `MkvTrack::audio` -> the muxer) and return the value that actually + /// lands in the emitted Matroska `Language` element. + fn emitted_language_for_dvd_code(code: &[u8; 2]) -> String { + let mut attr_bytes = vec![0u8; 8]; + attr_bytes[0] = 0x00; // AC-3, 48 kHz + attr_bytes[1] = 0x05; // 6 channels + attr_bytes[2] = code[0]; + attr_bytes[3] = code[1]; + let attr = crate::ifo::parse_audio_attr(&attr_bytes, 0).unwrap(); + + let audio_stream = crate::disc::AudioStream { + pid: 0xBD80, + codec: attr.codec, + channels: crate::disc::AudioChannels::from_count(attr.channels), + language: attr.language, + sample_rate: crate::disc::SampleRate::from_hz(attr.sample_rate), + secondary: false, + purpose: crate::disc::LabelPurpose::Normal, + label: String::new(), + }; + let track = MkvTrack::audio(&audio_stream); + let buf = Cursor::new(Vec::new()); + let tracks = [track]; + let muxer = MkvMuxer::new(buf, &tracks, None, 60.0, &[]).unwrap(); + let data = muxer.writer.into_inner(); + first_language_value(&data).to_string() + } + + /// The ISO 639-1 -> ISO 639-2 conversion must cover the WHOLE of ISO + /// 639-1, not just the handful of languages that happen to appear in + /// Blu-ray menu-graphic filenames. A Region-2 disc routinely carries + /// Romanian, Bulgarian, Croatian, Serbian, Slovak, Slovenian, Hebrew, + /// Estonian, Latvian, Lithuanian, Icelandic and so on; if those all + /// collapse to `und`, every one of a disc's subtitle tracks emits the + /// same `Language` value and nothing else tells them apart (DVD streams + /// carry an empty `label`). A valid-but-identical code is worse for the + /// user than the invalid one it replaced, so each of these must reach the + /// emitted `Language` element as its own correct three-letter code. + #[test] + fn dvd_language_outside_the_menu_vocabulary_is_still_mapped() { + assert_eq!( + emitted_language_for_dvd_code(b"ro"), + "ron", + "Romanian ('ro'), common on Region-2 discs, must reach the \ + Matroska Language element as 'ron' — not 'und'" + ); + // The rest of the set the menu-label table never knew, one per + // language so a single missing table row fails loudly. + for (code, expected) in [ + (b"bg", "bul"), + (b"hr", "hrv"), + (b"sr", "srp"), + (b"sk", "slk"), + (b"sl", "slv"), + (b"he", "heb"), + (b"et", "est"), + (b"lv", "lav"), + (b"lt", "lit"), + (b"is", "isl"), + (b"id", "ind"), + (b"vi", "vie"), + (b"fa", "fas"), + ] { + assert_eq!( + emitted_language_for_dvd_code(code), + expected, + "DVD language {:?} must map to {expected:?} in the emitted \ + Language element", + std::str::from_utf8(code).unwrap() + ); + } + } + + /// DVD-Video froze its language list on the 1988 edition of ISO 639-1, + /// which spelled Hebrew `iw`, Indonesian `in` and Yiddish `ji`. Real + /// discs authored to that list carry those bytes, so they must map to the + /// same ISO 639-2 codes as the modern `he` / `id` / `yi` spellings rather + /// than degrading to `und`. + #[test] + fn dvd_era_language_aliases_map_to_the_modern_code() { + assert_eq!( + emitted_language_for_dvd_code(b"iw"), + "heb", + "the DVD-era spelling of Hebrew ('iw') must emit 'heb'" + ); + assert_eq!(emitted_language_for_dvd_code(b"in"), "ind"); + assert_eq!(emitted_language_for_dvd_code(b"ji"), "yid"); + // ...and agree with the modern spellings. + assert_eq!(emitted_language_for_dvd_code(b"he"), "heb"); + assert_eq!(emitted_language_for_dvd_code(b"id"), "ind"); + assert_eq!(emitted_language_for_dvd_code(b"yi"), "yid"); + } + + /// Widening the table must not weaken the degradation guarantee: a code + /// that is not ISO 639-1 at all still has to yield exactly `und`, a valid + /// Matroska language value, and never a passed-through two-letter code, + /// an empty string, or a guess. + #[test] + fn unknown_dvd_language_still_yields_exactly_und() { + for code in [b"zz", b"qq", b"xx"] { + assert_eq!( + emitted_language_for_dvd_code(code), + "und", + "a code outside ISO 639-1 must degrade to exactly 'und'" + ); + } + // Empty language bytes (0x00 0x00) likewise. + assert_eq!(emitted_language_for_dvd_code(&[0x00, 0x00]), "und"); + } + #[test] fn mkv_forced_flag_on_forced_subtitle() { use crate::disc::SubtitleStream; diff --git a/src/mux/mkvstream.rs b/src/mux/mkvstream.rs index 454069a..1534673 100644 --- a/src/mux/mkvstream.rs +++ b/src/mux/mkvstream.rs @@ -12,8 +12,19 @@ use super::{WriteSeek, ebml}; type MkvHeaderResult = io::Result<(crate::disc::DiscTitle, Vec<(u16, Vec<u8>)>, i64, TrackTable)>; /// Skip `n` bytes on a forward-only reader (no Seek required). +/// +/// A skip that runs out of input before `n` bytes is a TRUNCATED element, and is +/// reported the same way `ebml::read_binary_val` reports a truncated body: as +/// `MkvSourceInvalid`. Discarding `io::copy`'s byte count instead made a skip +/// that hit EOF look like a success, so one corrupt size field mid-Clusters +/// drained the rest of the file, the next element header raised +/// `UnexpectedEof`, and `Stream::read` mapped that to `Ok(None)` — half the +/// title missing, `errors = 0`, `completed = true`. fn skip_bytes(r: &mut impl Read, n: u64) -> io::Result<()> { - io::copy(&mut r.take(n), &mut io::sink())?; + let skipped = io::copy(&mut r.take(n), &mut io::sink())?; + if skipped != n { + return Err(crate::error::Error::MkvSourceInvalid.into()); + } Ok(()) } @@ -3193,6 +3204,78 @@ mod tests { assert!(stream.read().unwrap().is_none(), "clean EOF → None"); } + /// A skipped element whose declared size runs PAST the end of the file is a + /// truncated element, exactly like a truncated `read_binary_val` body — and + /// must be reported the same way, as `MkvSourceInvalid`. + /// + /// `skip_bytes` used to discard `io::copy`'s returned count, so the skip + /// "succeeded" having drained the rest of the file. The next element header + /// then hit `UnexpectedEof`, which `read()` maps to `Ok(None)` — a clean end + /// of stream. One corrupt size field mid-Clusters therefore threw away every + /// remaining frame of the title and reported `errors = 0`, `complete = true`. + #[test] + fn a_skip_past_eof_is_an_error_not_a_clean_end_of_stream() { + let mut cluster = Vec::new(); + ebml::write_id(&mut cluster, ebml::CLUSTER).unwrap(); + ebml::write_unknown_size(&mut cluster).unwrap(); + // Frame 1 — read normally. + let block = [0x81u8, 0x00, 0x00, 0x80, 0xAA]; + ebml::write_id(&mut cluster, ebml::SIMPLE_BLOCK).unwrap(); + ebml::write_size(&mut cluster, block.len() as u64).unwrap(); + cluster.extend_from_slice(&block); + // A VOID whose size field is corrupt: it claims 1 MiB, and the file + // holds only the handful of bytes below. This is the "corrupt size + // field mid-Clusters" case. + ebml::write_id(&mut cluster, ebml::VOID).unwrap(); + ebml::write_size(&mut cluster, 1024 * 1024).unwrap(); + // Frame 2 — the rest of the title, swallowed by the bad skip. + let block2 = [0x81u8, 0x00, 0x01, 0x80, 0xBB]; + ebml::write_id(&mut cluster, ebml::SIMPLE_BLOCK).unwrap(); + ebml::write_size(&mut cluster, block2.len() as u64).unwrap(); + cluster.extend_from_slice(&block2); + + let bytes = mkv_with_track_and_cluster(1, 1, &cluster); + let mut stream = MkvStream::open(Cursor::new(bytes)).unwrap(); + assert!(stream.read().unwrap().is_some(), "first frame reads"); + let e = match stream.read() { + Err(e) => e, + Ok(None) => panic!( + "a skip that hit EOF was reported as a CLEAN END OF STREAM: the \ + rest of the title is gone and the caller sees errors = 0, \ + complete = true" + ), + Ok(Some(_)) => panic!("the truncated skip must not yield a frame"), + }; + assert!(is_mkv_source_invalid(&e), "{e:?}"); + } + + /// The honest path this fix must not break: a skipped element whose declared + /// size is exactly satisfied by the bytes present is still skipped cleanly, + /// and the genuine EOF that follows is still `Ok(None)`. + #[test] + fn a_fully_satisfied_skip_still_ends_at_a_clean_eof() { + let mut cluster = Vec::new(); + ebml::write_id(&mut cluster, ebml::CLUSTER).unwrap(); + ebml::write_unknown_size(&mut cluster).unwrap(); + // A VOID that is fully present. + ebml::write_id(&mut cluster, ebml::VOID).unwrap(); + ebml::write_size(&mut cluster, 8).unwrap(); + cluster.extend_from_slice(&[0u8; 8]); + let block = [0x81u8, 0x00, 0x00, 0x80, 0xAA]; + ebml::write_id(&mut cluster, ebml::SIMPLE_BLOCK).unwrap(); + ebml::write_size(&mut cluster, block.len() as u64).unwrap(); + cluster.extend_from_slice(&block); + + let bytes = mkv_with_track_and_cluster(1, 1, &cluster); + let mut stream = MkvStream::open(Cursor::new(bytes)).unwrap(); + let f = stream.read().unwrap().expect("the frame after the VOID"); + assert_eq!(f.data, vec![0xAA]); + assert!( + stream.read().unwrap().is_none(), + "a genuine EOF at a record boundary is still a clean end" + ); + } + // ============================================================ // Block LACING (RFC 9559 §10.3) and TrackNumber→stream routing // (RFC 9559 §5.1.4.1.1). diff --git a/src/mux/ps.rs b/src/mux/ps.rs index 8315a2a..2902a54 100644 --- a/src/mux/ps.rs +++ b/src/mux/ps.rs @@ -169,6 +169,24 @@ pub struct PsDemuxer { /// stay byte-identical. buffer_base: u64, has_base: bool, + /// Boundary-scan cursor for an unbounded (length-0) PES still waiting for + /// its terminating PS-layer unit: `(buffer offset of the PES start code, + /// buffer offset up to which the search has already proved there is no + /// boundary)`. Both are buffer-relative and are rebased when the buffer + /// drains. + /// + /// Without it, every `feed` re-searches the WHOLE accumulated payload from + /// the PES header: the buffer only stops growing at [`MAX_PS_BUFFER`], so a + /// stream that declares an unbounded PES and then never emits a PS-layer + /// start code (a corrupt or crafted VOB) makes the demuxer scan up to 4 MiB + /// per call, quadratic in the bytes fed. Cleared whenever the PES is + /// emitted, so it can never outlive the packet it describes. + pending_scan: Option<(usize, usize)>, + /// Test-only: total bytes examined by `find_ps_boundary`. Pins the cursor + /// above — the property it exists for is a WORK bound, which no + /// packet-level assertion can observe. + #[cfg(test)] + boundary_bytes_scanned: u64, } impl Default for PsDemuxer { @@ -184,6 +202,9 @@ impl PsDemuxer { buffer: Vec::with_capacity(64 * 1024), buffer_base: 0, has_base: false, + pending_scan: None, + #[cfg(test)] + boundary_bytes_scanned: 0, } } @@ -220,6 +241,8 @@ impl PsDemuxer { // discarded. let packets = self.extract_packets(true); self.buffer.clear(); + // The buffer the cursor indexes into is gone. + self.pending_scan = None; packets } @@ -288,11 +311,29 @@ impl PsDemuxer { // start code — the video ES payload is itself full of // 00 00 01 xx codes that would otherwise cut the PES short. let end = if pes_packet_len == 0 { - match find_ps_boundary(&self.buffer, sc + 4) { - Some(next) => next, + // Resume where the last call stopped searching for + // THIS PES's terminating unit; anything before that is + // already proved boundary-free. + let from = match self.pending_scan { + Some((pes_at, searched_to)) if pes_at == sc => searched_to, + _ => sc + 4, + }; + let (found, searched_to) = find_ps_boundary(&self.buffer, from); + #[cfg(test)] + { + self.boundary_bytes_scanned += searched_to.saturating_sub(from) as u64; + } + match found { + Some(next) => { + self.pending_scan = None; + next + } // At EOF the rest of the buffer is this PES's // payload — emit it. - None if flushing => self.buffer.len(), + None if flushing => { + self.pending_scan = None; + self.buffer.len() + } None => { // No boundary buffered yet. Normally wait for // more data, but a corrupt stream could declare @@ -301,8 +342,10 @@ impl PsDemuxer { // stops untrusted input forcing unbounded // allocation. Past the cap, flush what we have. if self.buffer.len() - sc > MAX_PS_BUFFER { + self.pending_scan = None; self.buffer.len() } else { + self.pending_scan = Some((sc, searched_to)); break; // wait for more data } } @@ -338,6 +381,13 @@ impl PsDemuxer { if self.has_base { self.buffer_base += pos as u64; } + // The cursor is a BUFFER offset, so it moves with the drain. A + // pending PES always starts at or after `pos` (the loop broke on + // it, having already consumed everything before it), so neither + // component can underflow. + self.pending_scan = self + .pending_scan + .map(|(pes_at, searched_to)| (pes_at - pos, searched_to - pos)); } // Trim a start-code-free tail. Every other exit from the loop above @@ -359,6 +409,10 @@ impl PsDemuxer { if self.has_base { self.buffer_base += drop as u64; } + // A pending PES implies a start code IS in the buffer, so this + // branch cannot run while one is open; drop the cursor anyway + // rather than leave a stale offset behind this drain. + self.pending_scan = None; } packets @@ -380,11 +434,19 @@ const START_CODE_PREFIX_KEEP: usize = 2; /// PES inside its own payload and re-scan the discarded video bytes as bogus PS /// units. Restricting the search to PS-layer IDs (>= 0xB9, excluding the video /// ES codes below it) frames the unbounded PES at the right boundary. -fn find_ps_boundary(data: &[u8], from: usize) -> Option<usize> { +/// Returns `(boundary, searched_to)`. `searched_to` is the offset up to which +/// every byte has been PROVED not to begin a PS-layer boundary start code, so +/// a later call over the same buffer (grown at the tail) may resume there +/// instead of re-scanning the payload from the PES header. When the scan runs +/// off the end, the last two bytes are NOT proved: a `00 00 01` prefix can +/// straddle the next feed's boundary by up to two bytes. +fn find_ps_boundary(data: &[u8], from: usize) -> (Option<usize>, usize) { let mut pos = from; while let Some(sc) = find_start_code(data, pos) { if sc + 3 >= data.len() { - return None; + // A start code whose ID byte has not arrived yet: undecided, so + // the next scan must look at it again. + return (None, sc); } let id = data[sc + 3]; if id == PACK_HEADER_ID @@ -392,11 +454,11 @@ fn find_ps_boundary(data: &[u8], from: usize) -> Option<usize> { || id == PROGRAM_END_ID || is_pes_stream_id(id) { - return Some(sc); + return (Some(sc), sc); } pos = sc + 4; } - None + (None, data.len().saturating_sub(2).max(from)) } /// Check whether a start code byte is a valid PES stream ID that carries payload. @@ -1752,7 +1814,80 @@ mod tests { /// after it — exactly the tail a real feed can end on. #[test] fn find_ps_boundary_handles_a_bare_start_code_at_the_buffer_head() { - assert_eq!(find_ps_boundary(&[0x00, 0x00, 0x01], 0), None); + assert_eq!( + find_ps_boundary(&[0x00, 0x00, 0x01], 0), + (None, 0), + "an undecided trailing start code is not proved boundary-free" + ); + } + + /// An unbounded (length-0) PES is terminated by the next PS-LAYER unit, and + /// until one arrives the payload accumulates in the buffer. The search for + /// that unit must not restart at the PES header on every feed: the buffer + /// only stops growing at `MAX_PS_BUFFER` (4 MiB), and a feed is one read + /// batch (at most 510 sectors ≈ 1 MiB, 60 sectors ≈ 120 KiB on an optical + /// drive), so re-scanning from byte 0 costs work quadratic in the bytes + /// fed — up to 4 MiB of scanning per call, for as long as a corrupt or + /// crafted VOB withholds the boundary. A conformant DVD ends every pack + /// within 2048 bytes and never reaches this state. + /// + /// Measured directly, because a work bound has no packet-level shadow: + /// `boundary_bytes_scanned` counts the bytes `find_ps_boundary` examines. + /// 256 chunks x 4 KiB of boundary-free payload is 1 MiB of input; + /// re-scanning from the header on every call examines + /// 4 KiB * 256*257/2 = ~128 MiB. + /// + /// Mutation: drop the `Some((pes_at, searched_to)) if pes_at == sc` arm so + /// `from` is always `sc + 4`. + #[test] + fn an_unterminated_pes_is_not_rescanned_from_its_header_every_feed() { + const CHUNKS: usize = 256; + const CHUNK: usize = 4096; + + let mut demuxer = PsDemuxer::new(); + // Unbounded PES header (length 0), then payload that carries no start + // code at all, so no PS-layer boundary is ever found. + demuxer.feed(&[0x00, 0x00, 0x01, 0xE0, 0x00, 0x00, 0x80, 0x00, 0x00]); + for _ in 0..CHUNKS { + assert!( + demuxer.feed(&[0xFFu8; CHUNK]).is_empty(), + "no boundary yet, so no PES can be emitted" + ); + } + + let fed = (CHUNKS * CHUNK) as u64; + assert!( + demuxer.boundary_bytes_scanned <= 2 * fed, + "boundary search examined {} bytes over {fed} bytes of payload — \ + the scan must advance with the buffer, not restart at the PES header", + demuxer.boundary_bytes_scanned + ); + + // ...and the cursor must not have cost correctness: the PES still ends + // at the pack header that finally arrives, with its whole payload. + let pack = [ + 0x00, + 0x00, + 0x01, + PACK_HEADER_ID, + 0x44, + 0x00, + 0x04, + 0x00, + 0x04, + 0x01, + 0x00, + 0x00, + 0x03, + 0xF8, + ]; + let packets = demuxer.feed(&pack); + assert_eq!(packets.len(), 1, "the pack header terminates the PES"); + assert_eq!( + packets[0].data.len(), + CHUNKS * CHUNK, + "the whole accumulated payload belongs to the PES" + ); } /// The boundary-ID check is a 4-way `||`; a mutant that turns the FIRST @@ -1763,7 +1898,7 @@ mod tests { let data = [0x00, 0x00, 0x01, PACK_HEADER_ID, 0xAA]; assert_eq!( find_ps_boundary(&data, 0), - Some(0), + (Some(0), 0), "a pack header start code alone must register as a PS-layer boundary" ); } diff --git a/src/mux/resolve.rs b/src/mux/resolve.rs index 159b781..71c7679 100644 --- a/src/mux/resolve.rs +++ b/src/mux/resolve.rs @@ -1521,8 +1521,11 @@ fn forensic_clip_extents( tracing::warn!(target: "freemkv::keysource", "fmts: more than one forensic clip on the disc — segment byte space is ambiguous"); return Ok(None); } + // Addressing variant: these extents are a byte-space map for the forensic + // segment table (`clip_byte_to_lba`), not a read plan — an unrecorded + // extent must stay in place here or every later segment offset shifts. let exts: Vec<crate::disc::Extent> = udf - .file_extents(reader, &format!("/BDMV/STREAM/{name}")) + .file_extents_addressing(reader, &format!("/BDMV/STREAM/{name}")) .map_err(io::Error::from)? .into_iter() .filter(|&(lba, sectors)| lba > 0 && sectors > 0) @@ -4440,7 +4443,7 @@ mod tests { "00001.fmts", 20, FMTS_CONTENT_LBA - PART_START, - FMTS_CONTENT_SECTORS * 2048, + u64::from(FMTS_CONTENT_SECTORS) * 2048, true, )], subdirs: Vec::new(), diff --git a/src/sector/prefetched.rs b/src/sector/prefetched.rs index dbfc70b..35b1f19 100644 --- a/src/sector/prefetched.rs +++ b/src/sector/prefetched.rs @@ -85,6 +85,12 @@ pub struct PrefetchedSectorSource { /// /// [`capacity_sectors`]: SectorSource::capacity_sectors total_sectors: u32, + /// Latched the moment a terminal error crosses the channel. The + /// producer NEVER resumes after sending one (every error arm + /// `return`s), so the closed channel that follows is a dead source, + /// not end-of-stream — and `read_sectors` must keep saying so instead + /// of answering `Ok(0)` for the rest of the title. + producer_failed: bool, } impl PrefetchedSectorSource { @@ -300,6 +306,22 @@ impl PrefetchedSectorSource { return; } let sectors_read = (n / 2048) as u32; + // A genuine zero-byte read with no error + // would otherwise spin this loop forever. + // It is not end-of-stream either: the + // extent list still has `remaining` + // sectors to serve, so the inner source + // has quit early. Send a terminal + // sentinel — dropping `tx` here instead + // would reach the consumer as a clean EOF + // and finalize a TRUNCATED title as + // success, exactly as the panic sentinel + // below exists to prevent. + if sectors_read == 0 { + let _ = + tx.send(Err(crate::error::Error::SourceTerminated.into())); + return; + } buf.truncate(n); bytes_read_total = bytes_read_total.saturating_add(n as u64); if let Some(ref f) = event_fn { @@ -313,12 +335,6 @@ impl PrefetchedSectorSource { if tx.send(Ok(buf)).is_err() { return; // consumer dropped } - // A genuine zero-byte read with no error would - // otherwise spin this loop forever; treat it - // as end-of-source. - if sectors_read == 0 { - return; - } offset = offset.saturating_add(sectors_read); } Err(e) => { @@ -344,6 +360,7 @@ impl PrefetchedSectorSource { recycle_tx, producer: Some(producer), total_sectors, + producer_failed: false, }) } @@ -486,8 +503,29 @@ impl SectorSource for PrefetchedSectorSource { let _ = self.recycle_tx.send(filled); Ok(n) } - Ok(Err(e)) => Err(crate::error::Error::IoError { source: e }), - // Channel closed (producer finished or panicked). + // Recover the producer's TYPED error rather than blanket-wrapping + // it as `Error::IoError`. That wrapper is a classification, not a + // container: `is_scsi_transport_failure` matches `IoError`, so a + // wrapped MEDIUM ERROR bad sector reached `fill_extents` looking + // like a wedged bridge and aborted the pass instead of being + // skipped under `skip_errors`. `From<io::Error> for Error` + // downcasts the boxed payload back to the exact variant the + // producer sent (status + sense intact); a genuine OS-level + // `io::Error` — the real dead-bus case — still becomes `IoError`. + Ok(Err(e)) => { + // The producer `return`s after every error it sends, so + // this is also the moment the source dies. Latch it: the + // closed channel that follows must not read as EOF. + self.producer_failed = true; + Err(crate::error::Error::from(e)) + } + // Channel closed. Clean EOF only if the producer never + // signalled a failure — otherwise it exited without + // delivering the rest of the extents, and answering `Ok(0)` + // would let `fill_extents` mistake a dead source for a short + // read, zero-fill every remaining sector of the title and + // still report the pass as complete. + Err(_) if self.producer_failed => Err(crate::error::Error::SourceTerminated), Err(_) => Ok(0), } } @@ -1214,7 +1252,12 @@ mod tests { /// an error (not Ok(0)/EOF), and its ErrorKind must survive the /// round-trip through the channel. Grounding: the producer's /// `Err(e) => tx.send(Err(e.into()))` arm, and `read_sectors`' - /// `Ok(Err(e)) => Err(IoError{source:e})`. + /// `Ok(Err(e)) => { self.producer_failed = true; Err(Error::from(e)) }`. + /// + /// That arm recovers the producer's TYPED error by downcast rather than + /// blanket-wrapping it as `Error::IoError`, so the kind survives; the + /// `producer_failed` latch it also sets is what turns the channel close + /// that follows into `SourceTerminated` instead of a clean EOF. #[test] fn reader_error_propagates_with_kind() { with_watchdog(Duration::from_secs(10), || { @@ -1235,6 +1278,63 @@ mod tests { }); } + /// An inner source that answers a mid-extent read with `Ok(0)` has quit + /// early: the extent list still has sectors to serve. The producer must + /// say so, not simply drop `tx` — a closed channel reads as clean + /// end-of-stream, and `DiscStream::fill_extents` then fabricates zeros + /// for every remaining sector of the title and reports the pass + /// complete. Same rule as the panic sentinel: a truncated title must + /// never be finalized as success. + struct QuitsEarlySource; + impl SectorSource for QuitsEarlySource { + fn read_sectors( + &mut self, + _lba: u32, + _count: u16, + _buf: &mut [u8], + _recovery: bool, + ) -> Result<usize> { + Ok(0) + } + fn capacity_sectors(&self) -> u32 { + 9 + } + } + + #[test] + fn inner_source_quitting_early_is_not_reported_as_end_of_stream() { + with_watchdog(Duration::from_secs(10), || { + let extents = vec![Extent { + start_lba: 0, + sector_count: 9, + }]; + let mut pf = + PrefetchedSectorSource::new(QuitsEarlySource, extents, 3, None).expect("spawn"); + let mut buf = vec![0u8; 3 * 2048]; + let mut last = pf.read_sectors(0, 3, &mut buf, false); + // Whatever the first answer, no call may ever settle on a clean + // `Ok(0)`: 9 sectors were promised and none were delivered. + for _ in 0..4 { + if last.is_err() { + break; + } + assert_eq!( + *last.as_ref().unwrap(), + 0, + "the source delivered no bytes, so nothing can be Ok(n>0)" + ); + last = pf.read_sectors(0, 3, &mut buf, false); + } + let err = + last.expect_err("an undelivered extent list must surface as an error, not as EOF"); + assert!( + err.is_source_terminated(), + "the source is gone for good — retrying or skipping cannot \ + recover anything; got {err:?}" + ); + }); + } + /// A read returning a byte count that is not a whole number of /// sectors (n % 2048 != 0) must be rejected — never truncated and /// advanced, which would split a sector and hand decrypt a partial @@ -1472,4 +1572,95 @@ mod tests { assert_eq!(got.len(), 30 * 2048, "all 10 extents must be drained"); }); } + + /// Source whose every read fails with a `DiscRead` carrying the given + /// SCSI status (and optional sense) — an ordinary MEDIUM ERROR bad + /// sector (0x02 + 03/11/00) or the transport-failure sentinel (0xFF). + struct FailingSource { + status: u8, + sense: Option<crate::scsi::ScsiSense>, + } + + impl SectorSource for FailingSource { + fn capacity_sectors(&self) -> u32 { + 9999 + } + + fn read_sectors( + &mut self, + lba: u32, + _count: u16, + _buf: &mut [u8], + _recovery: bool, + ) -> Result<usize> { + Err(crate::error::Error::DiscRead { + sector: lba as u64, + status: Some(self.status), + sense: self.sense, + }) + } + } + + fn read_one_err(status: u8, sense: Option<crate::scsi::ScsiSense>) -> crate::error::Error { + let extents = vec![Extent { + start_lba: 100, + sector_count: 9, + }]; + let mut pf = PrefetchedSectorSource::new(FailingSource { status, sense }, extents, 3, None) + .expect("spawn"); + let mut buf = vec![0u8; 3 * 2048]; + pf.read_sectors(100, 3, &mut buf, false) + .expect_err("the producer's read failure must surface") + } + + /// REGRESSION: an ordinary MEDIUM ERROR bad sector that crosses the + /// prefetch channel must NOT be classified as a SCSI transport failure. + /// + /// `read_sectors` used to re-wrap EVERY channel error as + /// `Error::IoError { source }`, and `is_scsi_transport_failure` matches + /// `IoError` (the wedged-USB-bridge arm). So a skippable bad sector + /// arrived at `DiscStream::fill_extents` looking like a dead bus and + /// aborted the whole pass instead of honouring `skip_errors`. + #[test] + fn bad_sector_across_channel_is_not_a_transport_failure() { + with_watchdog(Duration::from_secs(10), || { + let sense = crate::scsi::ScsiSense { + sense_key: 0x03, + asc: 0x11, + ascq: 0x00, + }; + let err = read_one_err(crate::scsi::SCSI_STATUS_CHECK_CONDITION, Some(sense)); + assert!( + !err.is_scsi_transport_failure(), + "a MEDIUM ERROR bad sector must stay a bad sector across the \ + prefetch channel, got {err:?}" + ); + // The classification survives because the typed variant does. + assert!( + matches!(err, crate::error::Error::DiscRead { status: Some(s), .. } if s == 0x02), + "expected the producer's DiscRead to survive the channel, got {err:?}" + ); + assert_eq!( + err.scsi_sense().map(|s| (s.sense_key, s.asc, s.ascq)), + Some((0x03, 0x11, 0x00)), + "the drive's sense triple must survive the channel" + ); + }); + } + + /// OPPOSITE-DIRECTION CONTROL: a genuine transport failure (status 0xFF, + /// wedged USB bridge) crossing the same channel MUST still classify as a + /// transport failure, so `fill_extents` / sweep keep aborting the pass + /// instead of zero-filling every read against a dead bus. + #[test] + fn transport_failure_across_channel_still_classifies() { + with_watchdog(Duration::from_secs(10), || { + let err = read_one_err(crate::scsi::SCSI_STATUS_TRANSPORT_FAILURE, None); + assert!( + err.is_scsi_transport_failure(), + "a 0xFF transport failure must remain one across the prefetch \ + channel, got {err:?}" + ); + }); + } } diff --git a/src/udf.rs b/src/udf.rs index a5533d2..ac58b4c 100644 --- a/src/udf.rs +++ b/src/udf.rs @@ -100,6 +100,22 @@ pub struct IcbExtent { pub recorded: bool, } +/// One extent of a file resolved to an ABSOLUTE disc LBA — [`IcbExtent`] with +/// the partition offset already applied. The `recorded` flag travels with it: +/// an unrecorded extent occupies the file's byte space but holds no bytes the +/// file wrote, so a caller must emit `len` zeros for it rather than read those +/// sectors. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct AbsExtent { + /// Absolute disc LBA of the extent. + pub lba: u32, + /// Declared length of the extent in bytes. + pub len: u32, + /// `false` for an ECMA-167 4/14.14.1.1 type-1 (allocated, not recorded) + /// extent — see [`IcbExtent::recorded`]. + pub recorded: bool, +} + /// A directory or file entry. #[derive(Debug, Clone)] pub struct DirEntry { @@ -622,6 +638,11 @@ impl UdfFs { let mut ad_start = ad_offset; let mut ad_bytes = l_ad; const MAX_AD_BLOCKS: usize = 256; + // Set only when a block ends the chain (no continuation pointer). Running + // out of hops instead means the rest of the descriptor list was never + // read, so `extents` describes only PART of the file — see the error's + // own documentation for why handing that back is worse than failing. + let mut chain_ended = false; for _ in 0..MAX_AD_BLOCKS { let num_descriptors = ad_bytes / ad_size; @@ -670,7 +691,16 @@ impl UdfFs { // caller reads those sectors): dropping it slid every later // extent's data down by this hole's length, silently // corrupting the file with no error anywhere. - 1 => extents.push(IcbExtent { + // Type 2 ("not recorded and not allocated") is the other + // sparse-hole encoding and has the SAME byte-space + // semantics: no on-disc data, but the extent is part of the + // file and occupies `data_len` bytes of it. It used to fall + // into the catch-all below, which exits the descriptor loop + // WITHOUT setting a continuation pointer — so `chain_ended` + // was then set true and the function returned Ok with a + // silently truncated list, defeating the UdfAdChainTooLong + // gate and delivering a short file as a complete one. + 1 | 2 => extents.push(IcbExtent { lba: data_lba, len: data_len, recorded: false, @@ -684,6 +714,17 @@ impl UdfFs { } break; } + // Unreachable: `extent_type` is `raw_len >> 30`, a 2-bit + // value, and 0/1/2/3 are now all handled above. Kept as a + // conservative stop rather than a panic, because a library + // must not crash on disc content. + // + // Do not add a new extent type here without handling it + // properly: reaching this `break` leaves `next_block` unset, + // so the loop below takes its `None` arm and sets + // `chain_ended = true` — the list is then returned as Ok, + // silently truncated, with the UdfAdChainTooLong gate + // satisfied. That is exactly how type 2 was being dropped. _ => break, } } @@ -713,10 +754,17 @@ impl UdfFs { ad_start = 24; ad_bytes = aed_l_ad.min(block.len().saturating_sub(24)); } - None => break, + None => { + chain_ended = true; + break; + } } } + if !chain_ended { + return Err(Error::UdfAdChainTooLong); + } + Ok(extents) } @@ -743,12 +791,17 @@ impl UdfFs { /// Unrecorded (ECMA-167 4/14.14.1.1 type-1) extents are included: their /// space is allocated to the file at that location and occupies its byte /// space, so dropping them would slide every later extent's bytes down by - /// the hole's length in a sequential extraction. + /// the hole's length in a sequential extraction. They are RETURNED FLAGGED + /// (`AbsExtent::recorded == false`), because keeping the extent while + /// losing the flag is the same bug the other way round: the caller then + /// reads sectors the file never wrote and ships whatever the media holds + /// there. `read_file_limited` emits zeros for such an extent; every + /// consumer of this list has to be able to do the same. pub fn extents_abs_at( &self, reader: &mut dyn SectorSource, meta_lba: u32, - ) -> Result<Vec<(u32, u32)>> { + ) -> Result<Vec<AbsExtent>> { let alloc = self.read_icb_extents(reader, meta_lba)?; let mut out = Vec::with_capacity(alloc.len()); for ext in alloc { @@ -760,21 +813,67 @@ impl UdfFs { status: None, sense: None, })?; - out.push((abs, ext.len)); + out.push(AbsExtent { + lba: abs, + len: ext.len, + recorded: ext.recorded, + }); } Ok(out) } - /// Get all absolute disc sector extents for a file. - /// Returns Vec of (absolute_lba, sector_count) covering the entire file, - /// including any unrecorded (ECMA-167 4/14.14.1.1 type-1) extent — the - /// space is allocated to the file and occupies its byte space, so omitting - /// it would misplace every later extent. + /// Absolute disc extents `(absolute_lba, sector_count)` for a file, for a + /// caller that will READ those sectors as the file's content — a title's + /// play plan. + /// + /// Refuses, with [`Error::UdfUnrecordedExtent`], a file that contains an + /// unrecorded (ECMA-167 4/14.14.1.1 type-1/type-2) extent. Such an extent + /// occupies the file's byte space but was never written, so the file's + /// true content there is zeros while the media holds something else + /// entirely. A `(lba, sector_count)` pair cannot say "this range is a + /// hole", and both ways of pretending otherwise corrupt the rip: reading + /// it splices undefined sectors into the stream as content, and dropping + /// it slides every later extent's byte space. Until an extent can carry + /// the flag end-to-end, refusing is the only truthful answer at this + /// signature — and a title that is not offered is not a title that was + /// silently mis-ripped. + /// + /// Callers that only need the list as a byte-space ADDRESS MAP (never + /// reading the sectors as content) want + /// [`file_extents_addressing`](Self::file_extents_addressing), which keeps + /// the hole in place so offsets stay correct. pub fn file_extents( &self, reader: &mut dyn SectorSource, path: &str, ) -> Result<Vec<(u32, u32)>> { + let meta_lba = self.entry_at(path)?.meta_lba; + let abs = self.extents_abs_at(reader, meta_lba)?; + // Refuse only a hole that actually OCCUPIES byte space. A zero-length + // unrecorded descriptor displaces nothing: every later extent sits at + // the same offset with or without it, so the read plan is identical to + // the one this resolver would have produced anyway, and the callers' + // `sectors > 0 && lba > 0` filter already discards it. + // + // Refusing on `!recorded` alone dropped whole titles off discs that + // ripped correctly — the reverse of the defect the refusal exists for, + // and just as silent. The hazard is a hole with LENGTH: reading it + // splices undefined sectors into the rip, and dropping it slides every + // later extent's byte space. + if abs.iter().any(|e| !e.recorded && e.len > 0) { + return Err(Error::UdfUnrecordedExtent { + path: path.to_string(), + }); + } + Ok(abs + .iter() + .map(|e| (e.lba, (e.len as u64).div_ceil(2048) as u32)) + .collect()) + } + + /// Resolve `path` to its directory entry. Shared by both extent + /// resolvers so they cannot drift apart on lookup semantics. + fn entry_at(&self, path: &str) -> Result<&DirEntry> { let parts: Vec<&str> = path.trim_matches('/').split('/').collect(); let mut current = &self.root; for part in &parts[..parts.len() - 1] { @@ -794,29 +893,39 @@ impl UdfFs { }); } }; - let entry = current + current .entries .iter() .find(|e| !e.is_dir && e.name.eq_ignore_ascii_case(filename)) .ok_or_else(|| Error::UdfNotFound { path: path.to_string(), - })?; + }) + } - let alloc_extents = self.read_icb_extents(reader, entry.meta_lba)?; - let mut disc_extents = Vec::with_capacity(alloc_extents.len()); - for ext in alloc_extents { - let abs_lba = self - .partition_start - .checked_add(ext.lba) - .ok_or(Error::DiscRead { - sector: self.partition_start as u64, - status: None, - sense: None, - })?; - let sectors = (ext.len as u64).div_ceil(2048) as u32; - disc_extents.push((abs_lba, sectors)); - } - Ok(disc_extents) + /// Absolute disc extents `(absolute_lba, sector_count)` for a file, for a + /// caller that uses the list purely as a BYTE-SPACE ADDRESS MAP — mapping + /// a file offset onto an LBA (see + /// [`crate::aacs::segment::clip_byte_to_lba`]) — and never reads the + /// sectors as the file's content. + /// + /// Unrecorded (ECMA-167 4/14.14.1.1 type-1/type-2) extents are INCLUDED + /// here, unflagged: they occupy the file's byte space, so omitting one + /// would slide every later offset by the hole's length and misaddress the + /// rest of the file. That is the right trade only because nothing on this + /// path treats the returned sectors as stream bytes. Anything that will + /// read them must call [`file_extents`](Self::file_extents), which refuses + /// the file instead. + pub fn file_extents_addressing( + &self, + reader: &mut dyn SectorSource, + path: &str, + ) -> Result<Vec<(u32, u32)>> { + let meta_lba = self.entry_at(path)?.meta_lba; + Ok(self + .extents_abs_at(reader, meta_lba)? + .iter() + .map(|e| (e.lba, (e.len as u64).div_ceil(2048) as u32)) + .collect()) } } @@ -882,7 +991,16 @@ pub fn read_filesystem(reader: &mut dyn SectorSource) -> Result<UdfFs> { let fallback = (VDS_FALLBACK_START, VDS_MAX_SECTORS); let mut candidates = Vec::with_capacity(2); candidates.extend(recorded); - if recorded.map(|(s, _)| s) != Some(fallback.0) { + // Compare the whole extent, not just its start. The two candidates are two + // SWEEPS: an anchor may record the customary LBA and still declare a + // narrower window than the fallback's (16 sectors is the ECMA-167 3/10.2.1 + // minimum, and passes every shape check above). Matching on start alone + // suppressed the fallback as a duplicate and threw the WIDER sweep away, so + // a sequence lying past the declared window — or a Terminating Descriptor + // beyond it — was never reached. `n` is already capped at + // `VDS_MAX_SECTORS`, so tuple equality is exactly "the recorded sweep + // already covers the fallback sweep". + if recorded != Some(fallback) { candidates.push(fallback); } @@ -1199,55 +1317,27 @@ fn read_directory( let tag = u16::from_le_bytes([icb[0], icb[1]]); - // Get allocation extent: where the directory data lives - let (ad_len, ad_pos) = match tag { + // ECMA-167 4/14.6.8 ICB Tag flags: a Uint16 at offset 34 whose low 3 bits + // say how this entry stores its data — 0 short_ad, 1 long_ad, 2 + // extended_ad, 3 EMBEDDED (the data sits in the entry itself, where the + // allocation descriptors would otherwise be). The field is disc-controlled, + // so the layout below is chosen from it rather than assumed; the same flags + // already drive `read_icb_extents` and `read_inline_data`. + let ad_type = u16::from_le_bytes([icb[34], icb[35]]) & 0x07; + + // Where the allocation-descriptor field of this entry begins, and its + // declared length. ECMA-167 4/14.17 (266): L_EA at 208, L_AD at 212, field + // at 216 + L_EA. 4/14.9 (261): L_EA at 168, L_AD at 172, field at 176 + L_EA. + let (ad_off, l_ad) = match tag { 266 => { let l_ea = u32::from_le_bytes([icb[208], icb[209], icb[210], icb[211]]) as usize; - let ad_off = 216 + l_ea; - if ad_off + 8 > icb.len() { - return Err(Error::DiscRead { - sector: icb_abs as u64, - status: None, - sense: None, - }); - } - let len = u32::from_le_bytes([ - icb[ad_off], - icb[ad_off + 1], - icb[ad_off + 2], - icb[ad_off + 3], - ]) & 0x3FFF_FFFF; - let pos = u32::from_le_bytes([ - icb[ad_off + 4], - icb[ad_off + 5], - icb[ad_off + 6], - icb[ad_off + 7], - ]); - (len, pos) + let l_ad = u32::from_le_bytes([icb[212], icb[213], icb[214], icb[215]]) as usize; + (216 + l_ea, l_ad) } 261 => { let l_ea = u32::from_le_bytes([icb[168], icb[169], icb[170], icb[171]]) as usize; - let ad_off = 176 + l_ea; - if ad_off + 8 > icb.len() { - return Err(Error::DiscRead { - sector: icb_abs as u64, - status: None, - sense: None, - }); - } - let len = u32::from_le_bytes([ - icb[ad_off], - icb[ad_off + 1], - icb[ad_off + 2], - icb[ad_off + 3], - ]) & 0x3FFF_FFFF; - let pos = u32::from_le_bytes([ - icb[ad_off + 4], - icb[ad_off + 5], - icb[ad_off + 6], - icb[ad_off + 7], - ]); - (len, pos) + let l_ad = u32::from_le_bytes([icb[172], icb[173], icb[174], icb[175]]) as usize; + (176 + l_ea, l_ad) } // ECMA-167 4/14.9 (File Entry, tag 261) and 4/14.17 (Extended File // Entry, tag 266) are the only descriptors that can be a directory's @@ -1271,39 +1361,89 @@ fn read_directory( } }; - // Reject an oversized directory before allocating: ad_len is the - // disc-controlled 30-bit ICB allocation length, so a corrupt value - // could otherwise force a ~1 GiB zeroed allocation (amplified by - // recursion). Real directories are a few KiB; the 1 MiB cap still - // covers a large STREAM/ dir with thousands of .m2ts FIDs. - if ad_len > MAX_DIR_BYTES { - return Err(Error::DiscRead { + // An EMBEDDED directory (AD type 3) has no out-of-line extent at all: its + // FIDs are the descriptor field itself. Decoding that field as a + // length/LBA pair would read the first FID's own bytes as an extent and + // enumerate an unrelated sector, which yields a silently EMPTY directory + // (no error, no titles). + let (dir_data, ad_len) = if ad_type == 3 { + if ad_off > icb.len() || ad_off + l_ad > icb.len() { + return Err(Error::DiscRead { + sector: icb_abs as u64, + status: None, + sense: None, + }); + } + (icb[ad_off..ad_off + l_ad].to_vec(), l_ad as u32) + } else { + // Out-of-line: read the FIRST allocation descriptor. A short_ad + // (4/14.14.1) and a long_ad (4/14.14.2) both begin with the 4-byte + // extent length followed by the 4-byte logical block number, so one + // decode serves both; an extended_ad (4/14.14.3) puts two further + // length fields first and its lb_num at offset 12. Any other flag + // value is reserved by 4/14.6.8 — decode it as a short_ad, matching + // the documented fallback in `read_icb_extents` rather than failing a + // volume over a byte no real disc sets. + let lba_at = if ad_type == 2 { + ad_off + 12 + } else { + ad_off + 4 + }; + if lba_at + 4 > icb.len() { + return Err(Error::DiscRead { + sector: icb_abs as u64, + status: None, + sense: None, + }); + } + let ad_len = u32::from_le_bytes([ + icb[ad_off], + icb[ad_off + 1], + icb[ad_off + 2], + icb[ad_off + 3], + ]) & 0x3FFF_FFFF; + let ad_pos = u32::from_le_bytes([ + icb[lba_at], + icb[lba_at + 1], + icb[lba_at + 2], + icb[lba_at + 3], + ]); + + // Reject an oversized directory before allocating: ad_len is the + // disc-controlled 30-bit ICB allocation length, so a corrupt value + // could otherwise force a ~1 GiB zeroed allocation (amplified by + // recursion). Real directories are a few KiB; the 1 MiB cap still + // covers a large STREAM/ dir with thousands of .m2ts FIDs. + if ad_len > MAX_DIR_BYTES { + return Err(Error::DiscRead { + sector: meta_start as u64, + status: None, + sense: None, + }); + } + + // Read directory data + let dir_abs = meta_start.checked_add(ad_pos).ok_or(Error::DiscRead { sector: meta_start as u64, status: None, sense: None, - }); - } - - // Read directory data - let dir_abs = meta_start.checked_add(ad_pos).ok_or(Error::DiscRead { - sector: meta_start as u64, - status: None, - sense: None, - })?; - let sector_count = ad_len.div_ceil(2048); - let mut dir_data = vec![0u8; sector_count as usize * 2048]; - for i in 0..sector_count { - let abs = dir_abs.checked_add(i).ok_or(Error::DiscRead { - sector: dir_abs as u64, - status: None, - sense: None, })?; - read_sector( - reader, - abs, - &mut dir_data[(i as usize) * 2048..(i as usize + 1) * 2048], - )?; - } + let sector_count = ad_len.div_ceil(2048); + let mut dir_data = vec![0u8; sector_count as usize * 2048]; + for i in 0..sector_count { + let abs = dir_abs.checked_add(i).ok_or(Error::DiscRead { + sector: dir_abs as u64, + status: None, + sense: None, + })?; + read_sector( + reader, + abs, + &mut dir_data[(i as usize) * 2048..(i as usize + 1) * 2048], + )?; + } + (dir_data, ad_len) + }; // Parse File Identifier Descriptors let mut entries = Vec::new(); @@ -1500,7 +1640,14 @@ pub(crate) fn merge_ranges(ranges: &[(u32, u32)]) -> Vec<(u32, u32)> { // LBAs/lengths, so a corrupt disc could otherwise overflow u32 // (panic in debug, wrap in release). let last_end = last.0.saturating_add(last.1); - if start <= last_end.saturating_add(1) { + // `(start, count)` is HALF-OPEN, so `last_end` is the EXCLUSIVE end and + // two ranges touch exactly when `start == last_end`. Accepting + // `last_end + 1` as well merged across a genuine one-sector hole and + // reported coverage of a sector neither input described — and this + // output is a boundary, not a hint (`disc::merged_extents` -> + // `Disc::encrypted_content_ranges` decides what the decrypting source + // treats as content, where a nav/padding sector must stay outside). + if start <= last_end { // Overlapping or adjacent — extend let new_end = start.saturating_add(count).max(last_end); last.1 = new_end - last.0; @@ -2349,6 +2496,33 @@ mod tests { ); } + /// `merge_ranges` takes HALF-OPEN `(start, count)` ranges, so `start + + /// count` is the EXCLUSIVE end and two ranges touch exactly when the next + /// `start` equals the previous end. A range that starts one sector LATER + /// than that has a genuine, untouched sector between them, and merging + /// across it claims coverage of a sector neither input ever described. + /// + /// That matters because the merged output is a boundary, not a hint: + /// `disc::merged_extents` feeds `Disc::encrypted_content_ranges`, which + /// decides which sectors the decrypting source treats as content. A sector + /// in a real gap belongs to no title extent — it is nav/UDF/padding — and + /// must stay outside the content gate, not be folded into it. + #[test] + fn merge_ranges_keeps_a_genuine_one_sector_gap() { + // [0,5) and [6,9): sector 5 is in neither. The ranges are NOT adjacent. + assert_eq!( + merge_ranges(&[(0, 5), (6, 3)]), + vec![(0, 5), (6, 3)], + "sector 5 is in neither input range, so the two must stay disjoint" + ); + // Exactly touching ([0,5) then [5,3)) still merges — that is adjacency. + assert_eq!( + merge_ranges(&[(0, 5), (5, 3)]), + vec![(0, 8)], + "start == exclusive end is a true touch and must still merge" + ); + } + #[test] fn merge_ranges_saturates_near_u32_max() { // Adjacent ranges near u32::MAX must not panic (debug) or wrap. @@ -2678,6 +2852,156 @@ mod tests { ); } + /// A ZERO-LENGTH unrecorded descriptor must not cost the file its plan. + /// + /// `file_extents` refuses a file whose extent list contains an unrecorded + /// hole, because reading one splices undefined sectors into the rip and + /// dropping one slides every later extent's byte space. Neither is true of + /// a hole with LENGTH ZERO: it displaces nothing, every later extent sits + /// at the same offset with or without it, and the callers' own + /// `sectors > 0 && lba > 0` filter already discards it. + /// + /// Refusing on `!recorded` alone therefore dropped whole titles off discs + /// that ripped correctly before — the reverse of the defect the refusal + /// exists for, and just as silent, since the title simply vanishes. + #[test] + fn file_extents_accepts_a_zero_length_unrecorded_extent() { + // A zero-length type-1 hole, then the file's real 4096 bytes. + let icb = build_efe(4096, &[(1, 0, 4999), (0, 4096, 5000)]); + let mut reader = MapReader::new(); + reader.put(5, icb); + let fs = fs_with( + 0, + 0, + DirEntry { + name: String::new(), + is_dir: true, + meta_lba: 0, + size: 0, + entries: vec![file_entry("ZL", 5, 4096)], + }, + ); + + // Fixture check: the hole really is present and really is zero-length, + // or this test proves nothing. + assert_eq!( + fs.extents_abs_at(&mut reader, 5).expect("extents"), + vec![ + AbsExtent { + lba: 4999, + len: 0, + recorded: false + }, + AbsExtent { + lba: 5000, + len: 4096, + recorded: true + }, + ], + "fixture must present a ZERO-LENGTH unrecorded extent" + ); + + let plan = fs + .file_extents(&mut reader, "/ZL") + .expect("a zero-length hole displaces nothing, so the file is readable"); + assert!( + plan.iter().any(|&(lba, n)| lba == 5000 && n == 2), + "the real 4096 bytes must still be in the read plan; got {plan:?}" + ); + } + + #[test] + fn icb_extents_short_ad_type2_sparse_extent_is_kept_and_flagged_unrecorded() { + // ECMA-167 4/14.14.1.1 extent type 2 = "not recorded and not + // allocated" — the other sparse-hole encoding alongside type 1. It has + // the SAME byte-space semantics as type 1: no on-disc data, but it is + // part of the file and occupies its declared length, so it must be + // returned flagged unrecorded rather than dropped. + // + // Before this was handled it fell into the descriptor loop's catch-all + // `_ => break`, which exits mid-list WITHOUT setting a continuation + // pointer — so `chain_ended` was then set true and the function + // returned Ok with a silently truncated extent list. That defeats the + // very completeness gate (`UdfAdChainTooLong`) added alongside it, and + // delivers a short file as a verified complete extraction. + let icb = build_efe( + 6144, + &[ + (0, 2048, 10), // recorded + (2, 2048, 20), // not recorded, not allocated + (0, 2048, 30), // recorded, after the hole + ], + ); + let mut reader = MapReader::new(); + reader.put(5, icb); + let fs = fs_with(0, 0, file_entry("SP", 5, 6144)); + let extents = fs.read_icb_extents(&mut reader, 5).expect("extents"); + assert_eq!( + extents, + vec![ + IcbExtent { + lba: 10, + len: 2048, + recorded: true + }, + IcbExtent { + lba: 20, + len: 2048, + recorded: false + }, + IcbExtent { + lba: 30, + len: 2048, + recorded: true + }, + ], + "a type-2 hole must not truncate the list: the type-0 extent after \ + it is part of the file" + ); + } + + /// `file_extents` builds the extent list a Blu-ray / HD-DVD TITLE is + /// actually ripped through (`disc/bluray.rs`, `disc/hddvd.rs`, + /// `mux/resolve.rs`). Its `(lba, sector_count)` tuple cannot say "this + /// range is a hole", and both ways of pretending otherwise are wrong: + /// + /// * folding an unrecorded extent in makes the mux read undefined sectors + /// and splice whatever the media holds there into the stream as content; + /// * dropping it slides every later extent's byte space, and that list IS + /// a byte-space map (`aacs::segment::clip_byte_to_lba`). + /// + /// So at this signature the only truthful answer is refusal. + #[test] + fn file_extents_refuses_a_file_with_an_unrecorded_extent() { + let icb = build_efe( + 6144, + &[ + (0, 2048, 10), // recorded + (1, 2048, 20), // allocated, NOT recorded — undefined sectors + (0, 2048, 30), // recorded, after the hole + ], + ); + let mut reader = MapReader::new(); + reader.put(5, icb); + let fs = fs_with( + 0, + 0, + DirEntry { + name: String::new(), + is_dir: true, + meta_lba: 0, + size: 0, + entries: vec![file_entry("SP", 5, 6144)], + }, + ); + let res = fs.file_extents(&mut reader, "/SP"); + assert!( + res.is_err(), + "an unrecorded extent must never reach the title's extent list as \ + readable content; got {res:?}" + ); + } + #[test] fn icb_extents_zero_length_type0_terminates_list() { // ECMA-167: a zero-length type-0 AD terminates the descriptor list. @@ -2703,27 +3027,60 @@ mod tests { ); } + /// Hostile input: a type-3 continuation descriptor whose continuation block + /// points back at itself (a cycle). The `MAX_AD_BLOCKS` bound must make this + /// TERMINATE rather than loop forever — and terminating by exhausting the + /// budget is not the same as reaching the end of the chain. + /// + /// It used to fall through to `Ok(extents)` with the list silently cut short + /// at whatever the 256th hop had collected. `disc/extract.rs` then zero-pads + /// the missing tail out to the entry's declared size, sets `complete = true` + /// with `bytes_unreadable = 0`, and renames off `.partial` — a mostly-zero + /// file delivered as a verified complete extraction. Exhausting the budget + /// means "I do not know the rest of this file", and the only honest answer + /// is an error. #[test] - fn icb_extents_continuation_loop_terminates_without_hang_or_panic() { - // Hostile input: a type-3 continuation descriptor whose continuation - // block points back at itself (a cycle). The MAX_AD_BLOCKS bound must - // make this terminate rather than loop forever. We assert it returns - // a finite Vec and does not panic. The continuation block at meta-rel - // lba 50 contains a recorded extent + a type-3 AD pointing to lba 50. + fn icb_extents_exhausted_continuation_budget_is_an_error_not_a_short_list() { let icb = build_efe(2048, &[(0, 2048, 10), (3, 2048, 50)]); let cont = build_cont_block(&[(0, 2048, 20), (3, 2048, 50)]); let mut reader = MapReader::new(); reader.put(5, icb); reader.put(50, cont); let fs = fs_with(0, 0, file_entry("LOOP", 5, 2048)); - // Must return Ok (bounded), not hang or panic. - let extents = tuples(&fs.read_icb_extents(&mut reader, 5).expect("extents")); - // First block contributes extent (10,2048); each revisit of the - // self-referential cont block adds (20,2048). The hop bound caps the - // total, so the Vec is finite. (256 blocks max → < 600 extents.) - assert!(extents.len() < 1024, "continuation chain must be bounded"); - assert_eq!(extents[0], (10, 2048)); - assert_eq!(extents[1], (20, 2048)); + // Bounded: must not hang or panic. And must not report success. + match fs.read_icb_extents(&mut reader, 5) { + Err(e) => assert!( + matches!(e, Error::UdfAdChainTooLong), + "the hop budget must report itself with its own code: {e:?}" + ), + Ok(extents) => panic!( + "an unterminated AD chain returned Ok with {} extents — a \ + SHORT extent list reported as the whole file; the caller \ + zero-pads the tail and calls the result complete", + extents.len() + ), + } + } + + /// The honest path this fix must not break: a continuation chain that ENDS + /// within the budget still returns its full extent list, in file order. + #[test] + fn a_terminating_continuation_chain_still_returns_every_extent() { + // ICB → cont block at 50 → cont block at 51 → end. + let icb = build_efe(2048, &[(0, 2048, 10), (3, 2048, 50)]); + let cont_a = build_cont_block(&[(0, 2048, 20), (3, 2048, 51)]); + let cont_b = build_cont_block(&[(0, 2048, 30), (0, 4096, 40)]); + let mut reader = MapReader::new(); + reader.put(5, icb); + reader.put(50, cont_a); + reader.put(51, cont_b); + let fs = fs_with(0, 0, file_entry("CHAIN", 5, 2048)); + let got = tuples(&fs.read_icb_extents(&mut reader, 5).expect("extents")); + assert_eq!( + got, + vec![(10, 2048), (20, 2048), (30, 2048), (40, 4096)], + "a chain under the hop budget is a normal file, not an error" + ); } #[test] @@ -3410,17 +3767,37 @@ mod tests { .expect("the plan is built"); // (0, 1004) structure through the end of the metadata partition - // (1010, 5) the ICBs of INDEX.BDMV, EXACT.BIN, HUGE.BIN, SPARSE.BIN, - // merged with the adjacent LBAs between them + // (1010, 1) INDEX.BDMV's ICB, alone — 1011 is NOT collected, so the + // run stops here rather than reaching across the hole + // (1012, 3) the ICBs of EXACT.BIN, HUGE.BIN, SPARSE.BIN — genuinely + // consecutive, so these do merge // (1500, 2) INDEX.BDMV's 4096-byte extent // (1800, 1) EXACT.BIN's extent — at the ceiling, so still cached // (1900, 1) SPARSE.BIN's recorded extent only // Absent: 1011 alone (STREAM is not descended at all, not even for // its ICB), 1600 (00000.M2TS), 1850 (HUGE.BIN's data), 1700 // (SPARSE.BIN's unrecorded extent). + // + // 1011's absence is asserted, not merely commented. This expectation + // used to read `(1010, 5)` — a single run SPANNING 1011 — which + // contradicted the "Absent: 1011" line above it: `merge_ranges` closed + // one-sector holes, so the plan prefetched the very sector the skip + // policy exists to leave alone. assert_eq!( ranges, - vec![(0, 1004), (1010, 5), (1500, 2), (1800, 1), (1900, 1)] + vec![ + (0, 1004), + (1010, 1), + (1012, 3), + (1500, 2), + (1800, 1), + (1900, 1) + ] + ); + assert!( + !ranges.iter().any(|&(s, c)| (s..s + c).contains(&1011)), + "STREAM's ICB at 1011 is deliberately not descended, so no range \ + may cover it: {ranges:?}" ); } @@ -3816,6 +4193,64 @@ mod tests { assert_eq!(fs.root.entries[0].name, "INDEX.BDMV"); } + /// The recorded extent and the customary fallback are two SWEEPS, not two + /// locations: an anchor may point at the customary LBA and still declare a + /// window narrower than the fallback's. De-duplicating them on start LBA + /// alone then throws the wider sweep away and the sequence is never + /// reached, even though it is exactly where the fallback would have looked. + /// + /// Fixture: the anchor records `(LBA 32, 16 sectors)` — the minimum + /// ECMA-167 3/10.2.1 permits, so it passes every shape check — while the + /// Partition and Logical Volume Descriptors sit at 50/51, inside the + /// customary 32-sector window but past the declared 16. No Terminating + /// Descriptor lies in 32..48, or both sweeps would stop at the same sector + /// and the wider one would buy nothing. + #[test] + fn a_recorded_vds_narrower_than_the_customary_window_still_gets_the_wider_sweep() { + use fixture::{DirSpec, MemDisc, PART_START, build_udf_skeleton, file, lay_dir}; + + let mut disc = MemDisc::new(); + build_udf_skeleton(&mut disc, 10); + lay_dir( + &mut disc, + &DirSpec { + name: String::new(), + icb_lba: 10, + dir_data_lba: 11, + files: vec![file("INDEX.BDMV", 12, 13, 2048, false)], + subdirs: Vec::new(), + }, + ); + + // Move the sequence the skeleton wrote at 32/33/34 out to 50/51, past + // the declared 16-sector window. 32..48 is left as zeroed sectors: + // tag 0, no descriptor of any kind, and crucially no tag-8 Terminating + // Descriptor to stop the wider sweep early. + disc.put_bytes(32, &[0u8; 3 * 2048]); + let mut pd = [0u8; 2048]; + pd[0..2].copy_from_slice(&5u16.to_le_bytes()); + pd[188..192].copy_from_slice(&PART_START.to_le_bytes()); + disc.put_bytes(50, &pd); + let mut lvd = [0u8; 2048]; + lvd[0..2].copy_from_slice(&6u16.to_le_bytes()); + lvd[268..272].copy_from_slice(&1u32.to_le_bytes()); + disc.put_bytes(51, &lvd); + + let mut avdp = vec![0u8; 2048]; + avdp[0..2].copy_from_slice(&2u16.to_le_bytes()); + avdp[16..20].copy_from_slice(&(VDS_MIN_SECTORS * 2048).to_le_bytes()); + avdp[20..24].copy_from_slice(&VDS_FALLBACK_START.to_le_bytes()); + disc.put_bytes(256, &avdp); + + let fs = super::read_filesystem(&mut disc).expect( + "an anchor whose window is narrower than the customary one must \ + still get the customary sweep — it is a different sweep, not a \ + duplicate of the one already tried", + ); + assert_eq!(fs.partition_start(), PART_START); + assert_eq!(fs.root.entries[0].name, "INDEX.BDMV"); + } + // ---- UDF 2.50 Metadata Partition coverage. // // Every BD-ROM records its file-system metadata in a Metadata Partition @@ -4144,9 +4579,13 @@ mod tests { meta_start, "the metadata partition begins at the Metadata File's extent" ); + assert_eq!( + spec.meta_bytes, 16_910_336, + "the sector count below is computed from this byte length by hand" + ); assert_eq!( fs.metadata_sectors(), - spec.meta_bytes.div_ceil(2048), + 8257, // 16 910 336 / 2048, exactly "the metadata partition is as long as the Metadata File's extent" ); assert_eq!(fs.volume_id, MV_VOLUME_ID); @@ -4154,6 +4593,38 @@ mod tests { assert_eq!(fs.root.entries[0].size, MV_FILE_SIZE); } + #[test] + fn read_filesystem_counts_a_partial_final_metadata_sector() { + // The Metadata File's extent length is a BYTE count (ECMA-167 4/14.14.1 + // extent_length) and nothing requires it to be a whole multiple of the + // 2048-byte logical sector. `metadata_sectors` is consumed as a sector + // COUNT (`metadata_sector_ranges` ends the UDF-structure range at + // metadata_start + metadata_sectors), so a length with a remainder must + // round UP: truncating instead leaves the final, partly-used sector of + // the metadata partition outside the range, and the AACS/disc-info + // caller then never reads the directory bytes living in it. + // + // Every other fixture on this volume uses an exact multiple, where + // rounding up and truncating agree — this is the only case that tells + // them apart. The expected count is worked out by hand: + // 16 910 337 = 8257 * 2048 + 1 -> 8258 sectors. + let spec = MetaVol { + meta_bytes: 16_910_337, + ..conformant_meta_vol() + }; + let (mut disc, meta_start) = build_meta_vol(&spec); + + let fs = super::read_filesystem(&mut disc) + .expect("a metadata extent that is not a whole number of sectors must still mount"); + + assert_eq!(fs.metadata_start(), meta_start); + assert_eq!( + fs.metadata_sectors(), + 8258, + "the sector holding the final partial 1 byte belongs to the metadata partition" + ); + } + #[test] fn read_filesystem_still_finds_a_metadata_file_entry_recorded_at_block_zero() { // The overwhelmingly common layout: the Metadata File's File Entry is @@ -4238,7 +4709,7 @@ mod tests { let fs = super::read_filesystem(&mut disc) .expect("an allocation descriptor flush with the end of the File Entry is in bounds"); assert_eq!(fs.metadata_start(), meta_start); - assert_eq!(fs.metadata_sectors(), spec.meta_bytes.div_ceil(2048)); + assert_eq!(fs.metadata_sectors(), 8257); // 16 910 336 / 2048, exactly assert_eq!(child_names(&fs.root), vec!["INDEX.BDMV".to_string()]); } @@ -4510,6 +4981,84 @@ mod tests { ); } + /// Build a directory ICB whose ICB Tag flags (ECMA-167 4/14.6.8, Uint16 at + /// offset 34) select `ad_type`, with `body` written where the allocation + /// descriptors live (216 + L_EA for a 266) and L_AD set to its length. + fn build_dir_icb_flagged(ad_type: u16, body: &[u8]) -> [u8; 2048] { + let mut icb = [0u8; 2048]; + icb[0..2].copy_from_slice(&266u16.to_le_bytes()); + icb[34..36].copy_from_slice(&ad_type.to_le_bytes()); + icb[56..64].copy_from_slice(&(body.len() as u64).to_le_bytes()); + icb[208..212].copy_from_slice(&0u32.to_le_bytes()); // L_EA + icb[212..216].copy_from_slice(&(body.len() as u32).to_le_bytes()); // L_AD + icb[216..216 + body.len()].copy_from_slice(body); + icb + } + + #[test] + fn read_directory_reads_a_directory_whose_fids_are_embedded_in_the_icb() { + // ECMA-167 4/14.6.8 ICB Tag flags bits 0-2 select how the entry stores + // its data; 3 means EMBEDDED — the bytes sit in the entry itself, in + // the field where allocation descriptors would otherwise be. There is + // then no out-of-line extent at all. + // + // Decoding that field as a short_ad reads the first FID's own bytes as + // a length/LBA pair: the FID's tag identifier (257) becomes a 257-byte + // "extent" and its tag-header bytes become the LBA, so the walk reads + // an unrelated sector, finds no FID tag there and returns an EMPTY + // directory — a disc whose BDMV enumerates no titles, with no error. + // + // `read_inline_data` already honours these flags for FILES; a + // directory cannot be laxer. + let mut fids = Vec::new(); + push_fid_iu(&mut fids, "", 5, true, true, 0); // parent (..) + push_fid_iu(&mut fids, "INDEX.BDMV", 7, false, false, 0); + + let mut reader = MemReader::new(); + reader.put(5, build_dir_icb_flagged(3, &fids)); + reader.put(7, build_efe_icb(1024, 1024, 0)); + + let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) + .expect("an embedded (AD type 3) directory is valid and must be readable"); + assert_eq!(child_names(&parsed), vec!["INDEX.BDMV".to_string()]); + assert_eq!(parsed.entries[0].size, 1024); + } + + #[test] + fn read_directory_reads_an_extended_ad_directorys_extent_location() { + // ECMA-167 4/14.14.3 extended_ad is 20 bytes: extent_length (4), + // recorded_length (4), information_length (4), then the extent_location + // lb_addr at offset 12 — NOT at offset 4 where a short_ad/long_ad keeps + // it. ICB Tag flags = 2 selects it (4/14.6.8). + // + // Reading the LBA from offset 4 picks up the RECORDED LENGTH instead, + // so the walk reads whatever sector that number names. Here that is + // sector 2048, which holds nothing: the directory comes back empty. + // + // `read_icb_extents` already reads an extended_ad's location at +12; + // this is the same descriptor in the directory path. + let mut fids = Vec::new(); + push_fid_iu(&mut fids, "", 5, true, true, 0); + push_fid_iu(&mut fids, "INDEX.BDMV", 7, false, false, 0); + let mut dir = [0u8; 2048]; + dir[..fids.len()].copy_from_slice(&fids); + + let mut ext_ad = [0u8; 20]; + ext_ad[0..4].copy_from_slice(&(fids.len() as u32).to_le_bytes()); // extent_length + ext_ad[4..8].copy_from_slice(&2048u32.to_le_bytes()); // recorded_length + ext_ad[8..12].copy_from_slice(&(fids.len() as u32).to_le_bytes()); // information_length + ext_ad[12..16].copy_from_slice(&60u32.to_le_bytes()); // extent_location + + let mut reader = MemReader::new(); + reader.put(5, build_dir_icb_flagged(2, &ext_ad)); + reader.put(60, dir); + reader.put(7, build_efe_icb(1024, 1024, 0)); + + let parsed = read_directory(&mut reader, 0, 0, 5, "ROOT", 0, &mut 0, &mut HashSet::new()) + .expect("an extended_ad directory must be readable"); + assert_eq!(child_names(&parsed), vec!["INDEX.BDMV".to_string()]); + } + #[test] fn read_directory_masks_the_extent_type_bits_out_of_the_ad_length() { // ECMA-167 4/14.14.1.1: the 32-bit field at the head of a short_ad is @@ -5122,7 +5671,7 @@ pub(crate) mod fixture { pub(crate) name: String, pub(crate) icb_lba: u32, pub(crate) data_lba: u32, - pub(crate) size: u32, + pub(crate) size: u64, pub(crate) long_ad: bool, pub(crate) contents: Vec<u8>, } @@ -5137,23 +5686,28 @@ pub(crate) mod fixture { } /// Build an Extended File Entry ICB (tag 266) with one allocation descriptor. - pub(crate) fn build_file_icb(size: u32, data_lba: u32, long_ad: bool) -> [u8; 2048] { + pub(crate) fn build_file_icb(size: u64, data_lba: u32, long_ad: bool) -> [u8; 2048] { let mut s = [0u8; 2048]; s[0..2].copy_from_slice(&266u16.to_le_bytes()); // Extended File Entry if long_ad { s[34..36].copy_from_slice(&1u16.to_le_bytes()); // ICB flags → Long AD } - s[56..64].copy_from_slice(&(size as u64).to_le_bytes()); // info_length + s[56..64].copy_from_slice(&size.to_le_bytes()); // info_length s[208..212].copy_from_slice(&0u32.to_le_bytes()); // l_ea let ad_size: u32 = if long_ad { 16 } else { 8 }; s[212..216].copy_from_slice(&ad_size.to_le_bytes()); // l_ad - s[216..220].copy_from_slice(&(size & 0x3FFF_FFFF).to_le_bytes()); + // The allocation descriptor's length field is 32-bit, so a + // declared info_length beyond u32 simply cannot be backed by it — + // which is exactly the disc-vs-reality mismatch a hostile size field + // creates, and what the fixture must be able to express. + let ad_len = (size.min(u32::MAX as u64) as u32) & 0x3FFF_FFFF; + s[216..220].copy_from_slice(&ad_len.to_le_bytes()); s[220..224].copy_from_slice(&data_lba.to_le_bytes()); s } fn build_dir_icb(dir_data_lba: u32, dir_data_len: u32) -> [u8; 2048] { - build_file_icb(dir_data_len, dir_data_lba, false) + build_file_icb(dir_data_len as u64, dir_data_lba, false) } /// Append one File Identifier Descriptor (tag 257) to `buf`. @@ -5245,7 +5799,7 @@ pub(crate) mod fixture { name: &str, icb_lba: u32, data_lba: u32, - size: u32, + size: u64, long_ad: bool, ) -> FileSpec { FileSpec { @@ -5269,7 +5823,7 @@ pub(crate) mod fixture { name: name.to_string(), icb_lba, data_lba, - size: contents.len() as u32, + size: contents.len() as u64, long_ad, contents, }