diff --git a/src/disc/bluray.rs b/src/disc/bluray.rs index c466990..8841833 100644 --- a/src/disc/bluray.rs +++ b/src/disc/bluray.rs @@ -138,21 +138,49 @@ impl Disc { // 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; + // + // ABSENCE is the only benign failure. A missing `.ssif` is + // the ordinary case (every 2D disc), and the extension + // fallback below exists precisely for it. Every OTHER + // error means the bytes this title needs could not be + // resolved: a scratched sector under the clip's ICB + // (DiscRead), an allocation-descriptor chain that never + // terminated (UdfAdChainTooLong), a file whose data is + // embedded rather than extent-mapped (UdfEmbeddedData). + // Those used to fall through to the not-found path, so the + // clip contributed zero extents while `total_size` and the + // play-item timing still counted it — a title advertising + // its full runtime with a piece silently missing, and no + // log line anywhere. + // + // `Halted` is NOT a disc defect and must not be treated as + // one. It is the operator cancelling: once the flag is + // set, EVERY drive command returns it, so classifying it + // here would drop each remaining playlist in turn and hand + // back a truncated title list at success — the same shape + // this refusal exists to prevent, wearing a cancel. This + // function returns Option and has no channel to propagate + // a halt, so the pre-existing behaviour is preserved + // deliberately rather than made worse. + let mut unresolved: Option = None; + let mut note = |e: &Error| { + if !matches!(e, Error::UdfNotFound { .. } | Error::Halted) { + unresolved.get_or_insert(e.code()); + } + }; let file_exts = match udf_fs.file_extents(reader, &ssif) { Ok(exts) => { is_3d = true; Some(exts) } Err(e) => { - unrecorded |= matches!(e, Error::UdfUnrecordedExtent { .. }); + note(&e); 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 { .. }); + note(&e); None } } @@ -165,12 +193,16 @@ impl Disc { // 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 { + if let (None, Some(code)) = (&file_exts, unresolved) { + // The REAL code, not a fixed one. A scratched sector + // (E6000) and an over-long AD chain (E6016) logged as + // E6017 would send anyone triaging them after authoring + // holes and hide the population that actually exists. tracing::warn!( target: "freemkv::disc", playlist = ?filename, clip = ?play_item.clip_id, - "E{}", crate::error::E_UDF_UNRECORDED_EXTENT + "E{}", code ); return None; } @@ -1169,6 +1201,91 @@ mod tests { ); } + /// An unrecorded extent was never the only way a clip fails to resolve. + /// + /// RED BEFORE GREEN: this fixture gives the .m2ts an ICB whose descriptor + /// tag is neither 261 nor 266, so `file_extents` returns `DiscRead` — the + /// same variant a SCRATCHED SECTOR under a real clip's ICB produces, which + /// is the ordinary way this happens on real media. Before the fix only + /// `UdfUnrecordedExtent` set the drop flag, so this fell through to the + /// "file absent" path and `parse_playlist` returned a title: full declared + /// duration from the play item, `total_size` already counted from the + /// .clpi, and ZERO extents — a movie advertising its runtime with the + /// content missing, and not one log line. The clip must drop the title + /// exactly as an unrecorded extent does. + #[test] + fn parse_playlist_unreadable_clip_icb_yields_no_title() { + let mut disc = MemDisc::new(); + let udf = { + let bdmv = DirSpec { + name: "BDMV".to_string(), + icb_lba: 20, + dir_data_lba: 21, + files: Vec::new(), + subdirs: vec![ + DirSpec { + name: "STREAM".to_string(), + icb_lba: 22, + dir_data_lba: 23, + files: vec![file("00001.m2ts", 100, 5000, 4096, false)], + subdirs: vec![], + }, + DirSpec { + name: "CLIPINF".to_string(), + icb_lba: 24, + dir_data_lba: 25, + files: vec![file_with("00001.clpi", 102, 8000, build_clpi(4000), false)], + subdirs: vec![], + }, + ], + }; + let root = DirSpec { + name: String::new(), + icb_lba: 10, + dir_data_lba: 11, + files: Vec::new(), + subdirs: vec![bdmv], + }; + build_udf_skeleton(&mut disc, 10); + lay_dir(&mut disc, &root); + // Corrupt ONLY the descriptor tag, leaving a structurally valid + // ICB behind it. That is what an unreadable/garbled sector looks + // like to the parser, and it is deliberately NOT an unrecorded + // extent — the point is the error class the old code ignored. + let mut icb = build_file_icb(4096, 5000, false); + icb[0..2].copy_from_slice(&999u16.to_le_bytes()); + disc.put_bytes(PART_START + 100, &icb); + udf::read_filesystem(&mut disc).expect("fs") + }; + // The fixture must really produce a non-unrecorded error, or the + // behaviour under test is never reached. + assert!( + matches!( + udf.file_extents(&mut disc, "/BDMV/STREAM/00001.m2ts"), + Err(Error::DiscRead { .. }) + ), + "fixture must fail with DiscRead, not UdfUnrecordedExtent" + ); + let mpls = build_mpls( + &[PiSpec { + clip_id: *b"00001", + in_time: 0, + out_time: 60 * 45000, + }], + (0, 0, 0, 0, 0, 0, 0, 0), + &[], + &[], + ); + let t = Disc::parse_playlist(&mut disc, &udf, "00001.mpls", &mpls); + assert!( + t.is_none(), + "a clip whose extents could not be resolved must drop the title, \ + not yield one that counts the clip's runtime and ships none of \ + its bytes; got {:?}", + t.map(|t| (t.size_bytes, t.extents)) + ); + } + // --------------------------------------------------------------- // Tests: STN stream mapping // --------------------------------------------------------------- diff --git a/src/disc/hddvd.rs b/src/disc/hddvd.rs index b66bac3..c83c088 100644 --- a/src/disc/hddvd.rs +++ b/src/disc/hddvd.rs @@ -957,7 +957,31 @@ impl Disc { unusable.insert(name.to_ascii_lowercase()); } Err(crate::error::Error::Halted) => return Err(crate::error::Error::Halted), - Err(_) => {} + // EVERY other failure means the same thing: no truthful read + // plan for this clip. A scratched sector under its ICB + // (DiscRead), an allocation-descriptor chain that never + // terminated (UdfAdChainTooLong), a file whose data is + // embedded rather than extent-mapped (UdfEmbeddedData) — all + // of them used to land in a bare `Err(_) => {}`: no log, and + // the clip NOT marked unusable, so a split feature still + // composed from FEATURE_1 alone and presented half a movie as + // a whole one. That is the very outcome the arm above was + // written to prevent, reachable through every error but one. + // + // The code logged is the error's own. Reusing 6017 here would + // account a scratched disc as an authoring hole. + // + // (`UdfNotFound` needs no special case: these names came from + // `ts_dir.entries`, so the lookup cannot miss.) + Err(e) => { + tracing::warn!( + target: "freemkv::disc", + clip = ?name, + code = e.code(), + "clip extents could not be resolved; dropping every title that names it" + ); + unusable.insert(name.to_ascii_lowercase()); + } } if !extents.is_empty() { clip_extents.insert(name.to_ascii_lowercase(), (name.clone(), *size, extents)); @@ -2848,6 +2872,90 @@ mod tests { ); } + /// A clip whose extents cannot be resolved must drop the SPLIT FEATURE. + /// + /// Catches reverting the `Err(e) => { warn; unusable.insert(..) }` arm in + /// `Disc::scan_hddvd_titles` back to the bare `Err(_) => {}` it replaced. + /// + /// RED BEFORE GREEN, and TWO earlier attempts at this test did NOT go red. + /// The first asserted "no title names the broken clip" — that passes either + /// way, because a clip that resolves to no extents is never inserted into + /// `clip_extents` and so yields no per-clip title regardless. The second + /// fixed that but shipped no `.vti`: `order` is built solely from + /// `parse_vti_clip_order` of the navigation file, so with no `.vti` it is + /// EMPTY, `feature` is empty, and no composed title is ever built — the + /// assertion held vacuously with the fix reverted. Hence the synthetic + /// `HVA00001.VTI` below: it is what makes the composer run at all. + /// + /// The defect lives one level up from the per-clip titles, in the composed + /// feature: `unusable` is what tells the composer that a part is MISSING + /// rather than merely absent, and the old bare `Err(_) => {}` populated it + /// for nothing but an unrecorded extent. So a scratched sector under + /// FEATURE_2's ICB (`Error::DiscRead`) left FEATURE_1 composing a title + /// named "FEATURE" by itself — half a movie offered as the whole one. + #[test] + fn scan_hddvd_titles_drops_a_split_feature_whose_part_cannot_be_read() { + let mut disc = MemDisc::new(); + // The VTI clip table is the ONLY source of authored order, and the + // composed feature title exists only for clips it names. Without it + // this test cannot distinguish the fix from its absence. + let vti_bytes = synthetic_vti(&["FEATURE_1.EVO", "FEATURE_2.EVO"]); + let files = vec![ + file_with("HVA00001.VTI", 90, 20000, vti_bytes, true), + file("FEATURE_1.EVO", 100, 5000, 4 * 2048, true), + file("FEATURE_2.EVO", 101, 9000, 4 * 2048, 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); + // Blank FEATURE_2's ICB (laid at PART_START + 101). Its descriptor tag + // is then 0, neither 261 nor 266 — what the parser sees when the sector + // holding an ICB cannot be read back intact. Deliberately NOT an + // unrecorded extent: that class was already handled. + disc.put_bytes(PART_START + 101, &[0u8; 2048]); + let udf = crate::udf::read_filesystem(&mut disc).expect("fs"); + assert!( + matches!( + udf.file_extents(&mut disc, "/HVDVD_TS/FEATURE_2.EVO"), + Err(crate::error::Error::DiscRead { .. }) + ), + "fixture must fail with DiscRead, not UdfUnrecordedExtent" + ); + // Guard the guard: if the VTI ever stopped parsing, `order` would be + // empty and the assertion below would hold for the wrong reason. + assert_eq!( + parse_vti_clip_order(&synthetic_vti(&["FEATURE_1.EVO", "FEATURE_2.EVO"])), + vec!["FEATURE_1.EVO".to_string(), "FEATURE_2.EVO".to_string()], + "fixture VTI must yield both feature parts in authored order" + ); + + let titles = Disc::scan_hddvd_titles(&mut disc, &udf, None).expect("scan"); + // FEATURE_1 alone must not be offered as the feature. It may still + // appear as its own standalone clip title — that stands alone and is + // honest — but nothing may present it as the composed whole. + let composed: Vec<_> = titles + .iter() + .filter(|t| t.clips.len() > 1 || t.playlist.eq_ignore_ascii_case("FEATURE")) + .map(|t| &t.playlist) + .collect(); + assert!( + composed.is_empty(), + "a split feature missing one part must not compose; got {composed:?}" + ); + } + /// A clip whose file has a zero-byte size (a degenerate/empty allocation: /// its ICB's allocation descriptor has `data_len == 0`, the UDF AD-list /// terminator, so `file_extents` yields no extent at all) must not diff --git a/src/error.rs b/src/error.rs index 47658fe..84c829e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -66,6 +66,7 @@ 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; +pub const E_UDF_EMBEDDED_DATA: u16 = 6018; // AACS (7xxx) pub const E_AACS_NO_KEYS: u16 = 7000; @@ -430,6 +431,22 @@ pub enum Error { /// the declared size and report a mostly-empty file as a complete /// extraction, so the read fails instead. UdfAdChainTooLong, + /// A file's ICB declares its data EMBEDDED inline (ECMA-167 4/14.6.8 + /// allocation-descriptor type 3), so it has no out-of-line extents at all. + /// + /// Returned only when a caller asked for a read plan over such a file. + /// The bytes in the allocation-descriptor field are then the file's own + /// CONTENT, not descriptors, so decoding them as (length, LBA) pairs + /// manufactures extents out of arbitrary data and points the reader at + /// unrelated sectors — a rip that completes at rc=0 carrying whatever + /// happened to be there. `read_directory` already refuses the same shape + /// for directories; this is the file half of that decision. + /// + /// A file that legitimately stores its data this way is tiny (an ICB caps + /// it at well under 2 KiB — the AACS `*.inf` key files are the usual + /// case), and the callers that expect one read it via `read_inline_data` + /// long before extents are ever requested. A stream file cannot be one. + UdfEmbeddedData, DiscTitleRange { index: usize, count: usize, @@ -898,6 +915,7 @@ impl Error { Error::UdfNotFilesystem => E_UDF_NOT_FILESYSTEM, Error::UdfBufferTooSmall => E_UDF_BUFFER_TOO_SMALL, Error::UdfAdChainTooLong => E_UDF_AD_CHAIN_TOO_LONG, + Error::UdfEmbeddedData => E_UDF_EMBEDDED_DATA, Error::DiscTitleRange { .. } => E_DISC_TITLE_RANGE, Error::ShortImageRead { .. } => E_SHORT_IMAGE_READ, Error::EmptyImage => E_EMPTY_IMAGE, @@ -1918,6 +1936,13 @@ mod tests { E_IMAGE_TRUNCATED, E_UDF_BUFFER_TOO_SMALL, E_UDF_NOT_FILESYSTEM, + // These four were absent, so the "every published code is unique" + // claim above did not actually cover them: a new variant reusing + // 6014, 6016 or 6017 would have passed this test. + E_SELECTION_PID_UNKNOWN, + E_UDF_AD_CHAIN_TOO_LONG, + E_UDF_UNRECORDED_EXTENT, + E_UDF_EMBEDDED_DATA, E_AACS_NO_KEYS, E_AACS_CERT_SHORT, E_AACS_AGID_ALLOC, diff --git a/src/mux/codec/ac3.rs b/src/mux/codec/ac3.rs index 7f95d10..0837b5d 100644 --- a/src/mux/codec/ac3.rs +++ b/src/mux/codec/ac3.rs @@ -2482,4 +2482,88 @@ mod tests { "the unit belongs to the packet its FIRST byte came from" ); } + /// A track that becomes POISONED while an access unit is held open across a + /// PES boundary must not emit that access unit when it resumes. + /// + /// Mutation this catches: deleting (or inverting) the resume-path re-check + /// `if drop_reason.is_none() && self.tally.is_poisoned()` at the top of + /// `scan_access_units`. The verdict on a held access unit is frozen at the + /// moment it was OPENED, and `ac3_drop_reason` reads the tally BEFORE the + /// previous access unit is closed — so the very drop that crosses the + /// poison threshold lands after the next unit's verdict was already taken + /// as `None`. Without the re-check that unit is emitted as an ordinary + /// frame after the track has been judged too damaged to mux: corrupt audio + /// passed through as success, and worse, it is the ONE frame that escapes + /// a whole-track fallback whose entire point is that nothing after the + /// verdict ships. + /// + /// Neither existing held-AU test reaches this: both keep a pristine tally. + /// + /// The fixture drives the exact interleaving above. `DropTally` poisons + /// once `verified_dropped * 2 > kept + dropped` past the 200-AU gate, so + /// one PES carries 200 CRC-failing `substreamid`-0 syncframes followed by a + /// clean one. Closing corrupt unit #200 (which happens only when the clean + /// frame is reached) latches the poison — after that clean frame's own + /// verdict was computed. Being an E-AC-3 unit that can still gain + /// substreams, it is then HELD across the boundary with `drop_reason: + /// None`, which is precisely the state the re-check exists for. + #[test] + fn a_held_access_unit_is_dropped_when_the_track_poisons_before_it_resumes() { + // One more than the verdict gate: the Nth close is what poisons. + const CORRUPT_AUS: usize = 200; + + let mut parser = Ac3Parser::new(); + let mut data = Vec::new(); + for _ in 0..CORRUPT_AUS { + let mut f = eac3_substream_frame(0, 0); + // Corrupt a payload byte AFTER the CRC was finalized: the header + // (sizing, strmtyp/substreamid, bsid) stays intact so the frame is + // still parsed as a whole access unit and fails only the CRC — + // a VERIFIED drop, the only kind that feeds the poison verdict. + f[100] ^= 0xFF; + assert!(!frame_crc_ok(&f), "the fixture frame must fail its CRC"); + data.extend_from_slice(&f); + } + // The clean access unit. Last in the PES, so it is held open. + let clean = eac3_substream_frame(0, 0); + assert!( + frame_crc_ok(&clean), + "the held unit is individually decodable" + ); + data.extend_from_slice(&clean); + + let emitted = parser.parse(&make_eac3_pes(data)); + assert!( + emitted.is_empty(), + "every corrupt unit is dropped and the clean one is held; got {} frame(s)", + emitted.len() + ); + // The state the re-check depends on: the track IS poisoned, and the + // held unit was opened before that verdict existed. + assert!( + parser.tally.is_poisoned(), + "fixture must actually cross the whole-track poison threshold" + ); + + // Resume. The next PES opens a new unit, which closes the held one. + let out = parser.parse(&make_eac3_pes(eac3_substream_frame(0, 0))); + assert!( + out.is_empty(), + "an access unit held across the boundary must not be emitted once \ + the track is poisoned; got {} frame(s) totalling {} bytes", + out.len(), + out.iter().map(|f| f.data.len()).sum::() + ); + // ...and nothing may leak at end of stream either. + let tail = parser.flush(); + assert!( + tail.is_empty(), + "a poisoned track emits nothing at EOS; got {} frame(s)", + tail.len() + ); + assert!( + parser.dropped_frames() > CORRUPT_AUS as u64, + "the held unit must be ACCOUNTED as a drop, not silently discarded" + ); + } } diff --git a/src/mux/ps.rs b/src/mux/ps.rs index 2902a54..a70903b 100644 --- a/src/mux/ps.rs +++ b/src/mux/ps.rs @@ -1890,6 +1890,117 @@ mod tests { ); } + /// The resume cursor is a BUFFER offset, so it must be rebased when the + /// buffer drains — and this is the only test in which a drain actually + /// happens while a cursor is live. + /// + /// Mutations this catches, both halves of + /// `self.pending_scan.map(|(pes_at, searched_to)| (pes_at - pos, searched_to - pos))` + /// in `extract_packets`: + /// * `searched_to - pos` -> `searched_to`: the next call resumes `pos` + /// bytes PAST where the previous scan actually stopped, so that window + /// is never examined. Here the terminating pack header lands inside it + /// and is missed outright — the unbounded PES runs on past its real + /// end, swallowing the following unit. That is a CORRECTNESS failure, + /// not a slow path, and the assertion on the emitted packet catches it. + /// * `pes_at - pos` -> `pes_at`: the stale offset no longer equals the + /// PES's post-drain `sc`, the resume arm stops matching and the search + /// restarts at the PES header. Caught by `boundary_bytes_scanned`, + /// which is why the fixture puts 64 KiB of payload in the SAME chunk + /// that opens the PES: that is exactly the span a restart re-examines, + /// so the mutant roughly doubles the bytes scanned. + /// + /// `an_unterminated_pes_is_not_rescanned_from_its_header_every_feed` cannot + /// reach either: it opens the unbounded PES as the very FIRST bytes of the + /// very first feed, so nothing ever drains ahead of it, `pos` stays 0 and + /// the subtraction is a no-op. The comment above it asserts neither + /// component can underflow; nothing exercised the arithmetic at all. + /// + /// So this fixture puts COMPLETE PS units — a pack header and a + /// length-bounded PES — ahead of the unbounded video PES *in the same + /// chunk*. The loop consumes them, breaks on the unbounded PES, and drains + /// `pos` bytes with `pending_scan` live: exactly the real DVD shape, where + /// a video PES opens partway through a read batch. + #[test] + fn a_resume_cursor_survives_the_drain_of_units_ahead_of_the_unbounded_pes() { + // Payload fed in the SAME chunk that opens the PES. Large enough that + // re-scanning it is unmistakable in `boundary_bytes_scanned`, and it is + // the exact span the un-rebased `pes_at` mutant re-examines. + const PAYLOAD: usize = 64 * 1024; + + // A 14-byte MPEG-2 pack header with pack_stuffing_length 0. + const PACK: [u8; 14] = [ + 0x00, + 0x00, + 0x01, + PACK_HEADER_ID, + 0x44, + 0x00, + 0x04, + 0x00, + 0x04, + 0x01, + 0x00, + 0x00, + 0x03, + 0xF8, + ]; + // A length-BOUNDED PES — a complete unit, so the loop consumes it and + // `pos` advances past it before breaking on the unbounded PES. + const BOUNDED_PES: [u8; 11] = [ + 0x00, 0x00, 0x01, 0xE0, 0x00, 0x05, 0x80, 0x00, 0x00, 0xAA, 0xBB, + ]; + // The unbounded (length-0) video PES whose scan must be resumed. + const OPEN_PES: [u8; 9] = [0x00, 0x00, 0x01, 0xE0, 0x00, 0x00, 0x80, 0x00, 0x00]; + // Bytes drained ahead of the unbounded PES on the first feed — the + // `pos` the cursor must be rebased by. + const DRAINED: usize = PACK.len() + BOUNDED_PES.len(); + + let mut demuxer = PsDemuxer::new(); + let mut first = PACK.to_vec(); + first.extend_from_slice(&BOUNDED_PES); + first.extend_from_slice(&OPEN_PES); + first.extend_from_slice(&[0xFFu8; PAYLOAD]); + let head = demuxer.feed(&first); + assert_eq!( + head.len(), + 1, + "the bounded PES ahead of the open one is emitted immediately, \ + which is what makes the buffer drain with a cursor live" + ); + + // The terminating pack arrives at the head of the next feed — i.e. + // within `DRAINED` bytes of where the previous scan stopped, which is + // precisely the window an un-rebased `searched_to` skips over. + assert!( + PACK.len() <= DRAINED, + "the terminating pack must fit inside the window a stale \ + `searched_to` would skip, or the mutant survives" + ); + let packets = demuxer.feed(&PACK); + assert_eq!( + packets.len(), + 1, + "the pack header terminates the open PES; a scan resumed past it \ + never sees it and the PES runs on" + ); + assert_eq!( + packets[0].data.len(), + PAYLOAD, + "exactly the payload fed belongs to the PES" + ); + + // Work bound: the payload is proved boundary-free ONCE. Re-scanning it + // after the drain roughly doubles this. + assert!( + demuxer.boundary_bytes_scanned <= (PAYLOAD + 1024) as u64, + "boundary search examined {} bytes over {PAYLOAD} bytes of payload — \ + a cursor left un-rebased across the drain never matches the PES's \ + new offset, so the scan restarts at the header", + demuxer.boundary_bytes_scanned + ); + } + /// The boundary-ID check is a 4-way `||`; a mutant that turns the FIRST /// `||` into `&&` makes a lone pack-header start code (which can never /// also equal `SYSTEM_HEADER_ID`) fail to register as a boundary at all. diff --git a/src/udf.rs b/src/udf.rs index ac58b4c..bdf3abc 100644 --- a/src/udf.rs +++ b/src/udf.rs @@ -106,7 +106,12 @@ pub struct IcbExtent { /// 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 { +/// `pub(crate)`: reachable only through `extents_abs_at`, which is itself +/// internal. The crate's public surface is Drive / Disc / ScanOptions / +/// input() / output() / numeric errors, and leaking this type would make any +/// future reshape of its fields a breaking change for consumers that never +/// needed it. +pub(crate) struct AbsExtent { /// Absolute disc LBA of the extent. pub lba: u32, /// Declared length of the extent in bytes. @@ -616,14 +621,38 @@ impl UdfFs { // every multi-extent title at its first extent (~973 MB-1 GiB). let icb_flags = u16::from_le_bytes([icb[34], icb[35]]); let ad_type = (icb_flags & 0x07) as usize; + // Type 3 is EMBEDDED data: the descriptor field holds the file's own + // content, not allocation descriptors. Reading it as a run of + // (length, LBA) pairs manufactures extents out of arbitrary file + // bytes and sends the reader to unrelated sectors, which a rip then + // emits as this title's stream at rc=0. `read_directory` refuses + // exactly this shape a few hundred lines below; this is the file half + // of that same decision, and it must be an ERROR rather than an empty + // list — an empty list reaches the caller as a clip that contributed + // nothing while its declared duration and size still count it, which + // is the silent loss pointed the other way. + // + // A real disc CAN store a file this way, but only a tiny one: an ICB + // caps embedded content at well under 2 KiB, and the callers that + // expect such a file (the AACS `*.inf` key files) read it through + // `read_inline_data` before any extent is requested. No .m2ts, .ssif + // or .evo can physically fit, so no retail stream file reaches here. + if ad_type == 3 { + // ...except a zero-length file, which is legally encodable as + // embedded-with-nothing-embedded. It has no extents and no + // content, so it is not a failure — and returning the empty list + // matches what a zero-length file already produces today. + if l_ad == 0 { + return Ok(Vec::new()); + } + return Err(Error::UdfEmbeddedData); + } let ad_size: usize = match ad_type { 0 => 8, // Short AD 1 => 16, // Long AD 2 => 20, // Extended AD - // 3 = inline/embedded data (no out-of-line extents) — never - // used for large stream files. Anything else is unexpected; - // fall back to the historical 8-byte stride rather than fail - // the whole title. + // Anything else is reserved by 4/14.6.8; fall back to the + // historical 8-byte stride rather than fail the whole title. _ => 8, }; @@ -797,7 +826,10 @@ impl UdfFs { /// 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( + /// `pub(crate)`: returns `AbsExtent`, which is internal — a public fn + /// handing back a crate-private type would not compile, and neither is + /// part of the crate's published surface. + pub(crate) fn extents_abs_at( &self, reader: &mut dyn SectorSource, meta_lba: u32, @@ -915,7 +947,12 @@ impl UdfFs { /// 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( + /// `pub(crate)`: this returns unrecorded (never-written) extents + /// UNFLAGGED, in a shape identical to `file_extents`'s safe return. An + /// external consumer reaching for the more general-sounding name would + /// silently obtain a read plan over a hole. The doc below says so, but a + /// doc comment is not a guard — scoping makes the misuse a compile error. + pub(crate) fn file_extents_addressing( &self, reader: &mut dyn SectorSource, path: &str, @@ -2852,6 +2889,52 @@ mod tests { ); } + /// AD type 3 is EMBEDDED data, not a descriptor list. + /// + /// RED BEFORE GREEN: with the old `_ => 8` fallback this returned + /// `Ok([IcbExtent { lba: 999, len: 2048 }])` — an extent decoded out of + /// the file's own CONTENT bytes. A rip would then read sector 999, which + /// holds something else entirely, and emit it as this title's stream at + /// rc=0. That is the failure this refusal exists to prevent, so the + /// fixture deliberately makes the content decode as a PLAUSIBLE extent + /// rather than as garbage: garbage would have been caught by the + /// terminator check anyway, and would prove nothing. + #[test] + fn icb_extents_embedded_data_is_refused_not_decoded_as_descriptors() { + let mut icb = build_efe(2048, &[(0, 2048, 999)]); + // ICB Tag flags at abs offset 34: low 3 bits = 3 (embedded). + icb[34..36].copy_from_slice(&3u16.to_le_bytes()); + let mut reader = MapReader::new(); + reader.put(5, icb); + let fs = fs_with(0, 0, file_entry("EMB", 5, 2048)); + let err = fs + .read_icb_extents(&mut reader, 5) + .expect_err("an embedded ICB has no extents to hand out"); + assert!( + matches!(err, Error::UdfEmbeddedData), + "expected UdfEmbeddedData, got {err:?}" + ); + assert_eq!(err.code(), crate::error::E_UDF_EMBEDDED_DATA); + } + + /// ...but a ZERO-LENGTH file may legally be encoded as embedded-with- + /// nothing-embedded. It has no content and no extents, so it is not a + /// failure — refusing it would fail a rip over a legal empty file. + #[test] + fn icb_extents_embedded_but_empty_is_not_an_error() { + let mut icb = build_efe(0, &[]); + icb[34..36].copy_from_slice(&3u16.to_le_bytes()); + icb[212..216].copy_from_slice(&0u32.to_le_bytes()); // l_ad = 0 + let mut reader = MapReader::new(); + reader.put(5, icb); + let fs = fs_with(0, 0, file_entry("EMPTY", 5, 0)); + assert_eq!( + fs.read_icb_extents(&mut reader, 5) + .expect("legal empty file"), + vec![] + ); + } + /// A ZERO-LENGTH unrecorded descriptor must not cost the file its plan. /// /// `file_extents` refuses a file whose extent list contains an unrecorded