diff --git a/CHANGELOG.md b/CHANGELOG.md index df67b66..7da5baf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,23 @@ ### Fixed +- **A corrupt `mkv://` input is no longer reported as a title worth silently + skipping.** `Error::MkvInvalid` (E6008) carried two unrelated meanings: the + genuine "this title produced no muxable frames" stub — which + `is_skippable_title_stub` classifies as skippable, so an all-titles rip drops + the title and finishes the rest — and *every* malformed-input rejection in the + MKV read path. A truncated file, a bad VINT, a cluster timestamp past + `i64::MAX`, a BlockGroup child overrunning its group: all of them classified as + skippable, so a broken source was passed over by a run that then exited + successfully. The read path now raises `Error::MkvSourceInvalid` + (**E9053**) — the counterpart of `Mp4Invalid` (E9049) — and the writer's + unrepresentable-element-size guards raise `Error::MkvUnencodable` (**E9054**). + Neither is skippable. `E6008` now means only the no-muxable-frames stub (the + mux driver's headers-never-resolved gate and the MKV muxer's zero-frame + `finish()` guard). The `json://` sink's metadata-encoding guard, which also + raised `MkvInvalid`, now raises `NoMetadata` (E9008) like `mux::meta`'s. + Front-ends rendering error strings need entries for E9053 and E9054 (and for + E9051 / E9052, split off `E6008` earlier in this cycle for the same reason). - **The FMTS (AACS 2.1) forensic key resolution now runs once per disc, not once per title.** `Disc::resolve_content_key_map` resolves every title, and the FMTS branch ran ahead of everything else — so each playlist re-walked the UDF diff --git a/src/error.rs b/src/error.rs index c81e05f..e140ea3 100644 --- a/src/error.rs +++ b/src/error.rs @@ -165,6 +165,29 @@ pub const E_MUX_HEADER_BUFFER_EXCEEDED: u16 = 9051; /// successfully — the same conflation [`E_MUX_HEADER_BUFFER_EXCEEDED`] exists to /// avoid. pub const E_MKV_LACING_INVALID: u16 = 9052; +/// An `mkv://` SOURCE file is malformed or truncated — the EBML/Matroska reader +/// rejected it (bad element ID or VINT size, an unknown-size element where a +/// finite one is required, a child overrunning its parent, a truncated element +/// body, a non-UTF-8 string element, an element size above the parser's +/// allocation caps, an out-of-range TimestampScale / cluster timestamp / track +/// number). +/// +/// The read-path counterpart of [`E_MP4_INVALID`], and deliberately NOT +/// [`E_MKV_INVALID`]: [`is_skippable_title_stub`] classifies `E_MKV_INVALID` as +/// a title that yielded no muxable frames, which an all-titles rip may skip +/// while finishing the rest. A corrupt or truncated input file is a FAILURE, not +/// a stub — reporting it as skippable would let a broken source be silently +/// passed over by a run that then exits successfully. The same conflation +/// [`E_MUX_HEADER_BUFFER_EXCEEDED`] and [`E_MKV_LACING_INVALID`] exist to avoid. +pub const E_MKV_SOURCE_INVALID: u16 = 9053; +/// The Matroska WRITER was asked to emit something EBML cannot represent: an +/// element body at or above the 56-bit VINT payload limit (which would encode +/// byte-for-byte as the reserved "unknown size" marker or not fit at all), or a +/// master-element size placeholder that no longer lies inside the buffer being +/// patched. An output-side limit, not a property of any input — so neither +/// [`E_MKV_INVALID`] (a no-frames stub) nor [`E_MKV_SOURCE_INVALID`] (a corrupt +/// source) describes it, and it must not be classified as skippable. +pub const E_MKV_UNENCODABLE: u16 = 9054; pub const E_EXTENT_NOT_UNIT_ALIGNED: u16 = 9030; /// `mp4://` output but the title has no (primary) video track to carry. pub const E_MP4_NO_VIDEO_TRACK: u16 = 9048; @@ -305,7 +328,25 @@ pub enum Error { count: usize, }, IfoParse, + /// A title produced NO muxable frames: the mux driver's pump ended without + /// any video track's `codec_private` resolving, or the MKV muxer reached + /// `finish()` with zero frames written. The canonical case is an empty + /// nav/menu PGC stub, and [`is_skippable_title_stub`] classifies this code as + /// skippable so an all-titles rip drops the stub and finishes the rest. + /// + /// This meaning is EXCLUSIVE. Malformed `mkv://` source input is + /// [`Error::MkvSourceInvalid`]; an unrepresentable element on the write side + /// is [`Error::MkvUnencodable`]. Routing either of those here would report a + /// broken file as a title worth silently skipping. MkvInvalid, + /// An `mkv://` SOURCE file is malformed or truncated — the EBML/Matroska + /// reader rejected it. NOT [`Error::MkvInvalid`]: see + /// [`E_MKV_SOURCE_INVALID`]. + MkvSourceInvalid, + /// The Matroska WRITER cannot encode an element size in EBML (body at or + /// above the 56-bit VINT limit, or a stale master-size placeholder). NOT + /// [`Error::MkvInvalid`]: see [`E_MKV_UNENCODABLE`]. + MkvUnencodable, /// An `mkv://` source Block's lacing header does not describe its payload — /// the frames packed into that Block cannot be separated. NOT /// [`Error::MkvInvalid`]: see [`E_MKV_LACING_INVALID`]. @@ -617,6 +658,8 @@ impl Error { Error::DiscTitleRange { .. } => E_DISC_TITLE_RANGE, Error::IfoParse => E_IFO_PARSE, Error::MkvInvalid => E_MKV_INVALID, + Error::MkvSourceInvalid => E_MKV_SOURCE_INVALID, + Error::MkvUnencodable => E_MKV_UNENCODABLE, Error::MkvLacingInvalid => E_MKV_LACING_INVALID, Error::NoStreams => E_NO_STREAMS, Error::SelectionPidUnknown { .. } => E_SELECTION_PID_UNKNOWN, @@ -912,6 +955,12 @@ impl From for std::io::Error { // 9052 MkvLacingInvalid: a source Block's lacing header does not // describe its own payload — malformed input data. E_MKV_LACING_INVALID => std::io::ErrorKind::InvalidData, + // 9053 MkvSourceInvalid: the mkv:// source file is malformed or + // truncated — invalid input data. + E_MKV_SOURCE_INVALID => std::io::ErrorKind::InvalidData, + // 9054 MkvUnencodable: the writer was asked for an element size EBML + // cannot represent. An output-side limit, not bad input. + E_MKV_UNENCODABLE => std::io::ErrorKind::InvalidData, // mp4:// demux errors: a malformed/truncated source file // (E_MP4_INVALID), or a source whose tracks the mux can't use — no // video track / missing codec-private config. All are invalid data. @@ -970,6 +1019,16 @@ fn io_error_code(e: &std::io::Error) -> Option { /// /// This replaces the CLI's `E7023`/`E6008` string-match with a typed check on /// the [`io::Error`](std::io::Error) `mux_stream` returns. +/// +/// # Not skippable +/// +/// A BROKEN input is not a stub. [`Error::MkvSourceInvalid`] (malformed or +/// truncated `mkv://` source), [`Error::MkvLacingInvalid`], and +/// [`Error::MkvUnencodable`] all used to be raised as [`Error::MkvInvalid`] and +/// therefore landed in this set, so a corrupt source was reported as a title +/// worth silently passing over by a run that then exited successfully. They now +/// carry their own codes and are fatal here — as is +/// [`Error::MuxHeaderBufferExceeded`]. pub fn is_skippable_title_stub(e: &std::io::Error) -> bool { matches!(io_error_code(e), Some(E_MKV_INVALID | E_CSS_KEY_MISSING)) } @@ -1093,14 +1152,38 @@ mod tests { use super::*; #[test] - fn is_skippable_title_stub_matches_only_the_two_stub_codes() { - // The two skippable per-title stub codes, round-tripped through io::Error - // exactly as `mux_stream` returns them. + fn is_skippable_title_stub_excludes_malformed_mkv_input() { + // The two skippable per-title codes, round-tripped through io::Error + // exactly as `mux_stream` returns them. `MkvInvalid` now means ONLY the + // no-muxable-frames stub (the driver's headers-never-resolved gate and the + // MKV muxer's zero-frame `finish()` guard) — the meaning the doc on this + // predicate has always described. let mkv: std::io::Error = Error::MkvInvalid.into(); let css: std::io::Error = Error::CssKeyMissing.into(); assert!(is_skippable_title_stub(&mkv)); assert!(is_skippable_title_stub(&css)); + // A malformed / truncated `mkv://` SOURCE is a FAILURE, not a stub. Every + // read-path rejection in `mux::mkvstream` and `mux::ebml`'s read + // primitives used to be raised as `MkvInvalid`, so a bad VINT, a cluster + // timestamp past i64::MAX or a BlockGroup child overrunning its group all + // landed in the skippable set above: an all-titles rip would pass silently + // over a corrupt input and exit reporting success. Reverting + // `Error::MkvSourceInvalid` back to `Error::MkvInvalid` at those raise + // sites turns this assertion red. + let corrupt: std::io::Error = Error::MkvSourceInvalid.into(); + assert!( + !is_skippable_title_stub(&corrupt), + "a malformed mkv:// source must never classify as a skippable stub, got {corrupt}" + ); + // Same for the write side: an element size EBML cannot represent is an + // output-side limit, not an empty nav/menu stub. + let unencodable: std::io::Error = Error::MkvUnencodable.into(); + assert!(!is_skippable_title_stub(&unencodable)); + // And for the lacing rejection carved out in the same spirit. + let lacing: std::io::Error = Error::MkvLacingInvalid.into(); + assert!(!is_skippable_title_stub(&lacing)); + // A different coded error is NOT skippable (kills a "match anything with // an E-code" mutant). let nostreams: std::io::Error = Error::NoStreams.into(); @@ -1274,6 +1357,19 @@ mod tests { assert_eq!(mapped(Error::M2tsPacketMalformed), ErrorKind::InvalidData); // 9047 DiscCapacityMalformed → InvalidData assert_eq!(mapped(Error::DiscCapacityMalformed), ErrorKind::InvalidData); + // 9053/9054: the mkv:// read-path and write-path rejections split off + // `MkvInvalid`. Both are InvalidData, and both must render as a bare code + // with no English (this crate has none). + assert_eq!(mapped(Error::MkvSourceInvalid), ErrorKind::InvalidData); + assert_eq!(mapped(Error::MkvUnencodable), ErrorKind::InvalidData); + assert_eq!( + Error::MkvSourceInvalid.to_string(), + format!("E{}", E_MKV_SOURCE_INVALID) + ); + assert_eq!( + Error::MkvUnencodable.to_string(), + format!("E{}", E_MKV_UNENCODABLE) + ); } /// `Error::IoError` must round-trip back to the *original* @@ -1409,6 +1505,8 @@ mod tests { E_MUX_EMPTY, E_MUX_HEADER_BUFFER_EXCEEDED, E_MKV_LACING_INVALID, + E_MKV_SOURCE_INVALID, + E_MKV_UNENCODABLE, E_MP4_NO_VIDEO_TRACK, E_MP4_INVALID, E_MP4_MISSING_CODEC_PRIVATE, @@ -1504,6 +1602,8 @@ mod tests { E_MUX_HEADER_BUFFER_EXCEEDED, ), (Error::MkvLacingInvalid, E_MKV_LACING_INVALID), + (Error::MkvSourceInvalid, E_MKV_SOURCE_INVALID), + (Error::MkvUnencodable, E_MKV_UNENCODABLE), (Error::Mp4NoVideoTrack, E_MP4_NO_VIDEO_TRACK), (Error::Mp4Invalid, E_MP4_INVALID), (Error::Mp4MissingCodecPrivate, E_MP4_MISSING_CODEC_PRIVATE), diff --git a/src/mux/driver.rs b/src/mux/driver.rs index 7202bd9..b10209c 100644 --- a/src/mux/driver.rs +++ b/src/mux/driver.rs @@ -80,8 +80,10 @@ fn effective_send_deadline(send_deadline: Option) -> Duration { /// pre-headers buffer without bound (the whole 30-90 GB title, one PES frame at /// a time) until the process is OOM-killed. 512 MiB is far more than any real /// codec-private resolution needs but small enough to fail fast rather than -/// swap the box to death. Once exceeded the mux is refused exactly as the -/// headers-never-resolved gate refuses it (`Error::MkvInvalid`). Mirrors +/// swap the box to death. Once exceeded the mux is refused with +/// `Error::MuxHeaderBufferExceeded` — its OWN code, not the +/// headers-never-resolved gate's `Error::MkvInvalid`, which +/// `error::is_skippable_title_stub` reports as a skippable stub. Mirrors /// autorip's pre-refactor `HEADER_BUFFER_CAP_BYTES`. const HEADER_BUFFER_CAP_BYTES: usize = 512 * 1024 * 1024; @@ -1152,6 +1154,11 @@ mod tests { Duration::from_secs(60), ) .expect_err("unresolved headers must be refused"); + // This gate is the GENUINE stub case — the pump ended without any video + // track's codec_private resolving, so the title produced no muxable + // frames. `MkvInvalid` now means only this (malformed `mkv://` input is + // `MkvSourceInvalid`, E9053), and it must stay skippable so an all-titles + // rip drops the empty nav/menu PGC and finishes the rest. assert!( crate::error::is_skippable_title_stub(&err), "MkvInvalid is a skippable stub, got {err}" diff --git a/src/mux/ebml.rs b/src/mux/ebml.rs index c79382b..8df6e84 100644 --- a/src/mux/ebml.rs +++ b/src/mux/ebml.rs @@ -46,7 +46,7 @@ pub fn write_size(w: &mut impl Write, size: u64) -> io::Result<()> { // "unknown/open-ended" sentinel), and anything larger doesn't fit // the 7-byte payload. Reject so a finite size can never be emitted // as the unknown-size marker. - Err(crate::error::Error::MkvInvalid.into()) + Err(crate::error::Error::MkvUnencodable.into()) } else { // 8-byte size for large elements w.write_all(&[ @@ -211,14 +211,14 @@ pub fn start_master_buf(buf: &mut Vec, id: u32) -> io::Result { pub fn end_master_buf(buf: &mut [u8], size_pos: usize) -> io::Result<()> { let end = buf.len(); let Some(body_start) = size_pos.checked_add(8) else { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvUnencodable.into()); }; if end < body_start { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvUnencodable.into()); } let data_size = (end - body_start) as u64; if data_size >= 0x0100_0000_0000_0000 { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvUnencodable.into()); } buf[size_pos..body_start].copy_from_slice(&[ 0x01, @@ -261,7 +261,7 @@ pub fn read_id(r: &mut impl Read) -> io::Result<(u32, usize)> { 4, )) } else { - Err(crate::error::Error::MkvInvalid.into()) + Err(crate::error::Error::MkvSourceInvalid.into()) } } @@ -362,7 +362,7 @@ pub fn read_size(r: &mut impl Read) -> io::Result<(u64, usize)> { // is a malformed/over-long size field rather than a valid 8-byte // length. Reject it instead of silently building a size from the // following 7 bytes (which would desync the parse). - Err(crate::error::Error::MkvInvalid.into()) + Err(crate::error::Error::MkvSourceInvalid.into()) } } @@ -380,7 +380,7 @@ pub fn read_uint_val(r: &mut impl Read, len: usize) -> io::Result { // (DoS on untrusted input) — reject it at the source so every caller // is safe, not just the ones that pre-check. if len > 8 { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } let mut buf = [0u8; 8]; r.read_exact(&mut buf[..len])?; @@ -392,7 +392,7 @@ pub fn read_uint_val(r: &mut impl Read, len: usize) -> io::Result { } /// Read a float value. EBML floats are exactly 0, 4, or 8 bytes; any other -/// length is rejected as [`Error::MkvInvalid`] and exactly the float width is +/// length is rejected as [`Error::MkvSourceInvalid`] and exactly the float width is /// consumed (so a malformed element never under- or over-reads and desyncs the /// rest of the parent element). pub fn read_float_val(r: &mut impl Read, len: usize) -> io::Result { @@ -408,7 +408,7 @@ pub fn read_float_val(r: &mut impl Read, len: usize) -> io::Result { r.read_exact(&mut buf)?; Ok(f64::from_be_bytes(buf)) } - _ => Err(crate::error::Error::MkvInvalid.into()), + _ => Err(crate::error::Error::MkvSourceInvalid.into()), } } @@ -420,8 +420,8 @@ pub fn read_string_val(r: &mut impl Read, len: usize) -> io::Result { buf.pop(); } // Library rule: errors are numeric variants, never English strings. - // A non-UTF-8 string element is malformed input → MkvInvalid. - String::from_utf8(buf).map_err(|_| crate::error::Error::MkvInvalid.into()) + // A non-UTF-8 string element is malformed input → MkvSourceInvalid. + String::from_utf8(buf).map_err(|_| crate::error::Error::MkvSourceInvalid.into()) } /// Read binary data of `len` bytes. @@ -441,9 +441,9 @@ fn read_exact_bounded(r: &mut impl Read, len: usize) -> io::Result> { let got = r.take(len as u64).read_to_end(&mut buf)?; if got != len { // A truncated element is malformed input. Use the typed crate error - // so callers matching on Error::MkvInvalid catch short reads rather + // so callers matching on Error::MkvSourceInvalid catch short reads rather // than a bare io::ErrorKind that bypasses the numeric-code identity. - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } Ok(buf) } @@ -1186,7 +1186,7 @@ mod tests { #[test] fn read_uint_val_rejects_len_above_8() { // len 9 would index past the [0u8; 8] buffer → OOB/DoS on untrusted - // input. Must be a clean MkvInvalid. + // input. Must be a clean MkvSourceInvalid. let e = read_uint_val(&mut Cursor::new(&[0u8; 16]), 9).unwrap_err(); assert_eq!(e.kind(), io::ErrorKind::InvalidData); } @@ -1227,7 +1227,7 @@ mod tests { #[test] fn read_binary_val_short_read_errors() { - // Declare 100 bytes but supply 4 → MkvInvalid (truncated element). + // Declare 100 bytes but supply 4 → MkvSourceInvalid (truncated element). let e = read_binary_val(&mut Cursor::new(&[1u8, 2, 3, 4]), 100).unwrap_err(); assert_eq!(e.kind(), io::ErrorKind::InvalidData); // Exact-length read returns the bytes verbatim. diff --git a/src/mux/meta_sink.rs b/src/mux/meta_sink.rs index 294619f..d7ece1e 100644 --- a/src/mux/meta_sink.rs +++ b/src/mux/meta_sink.rs @@ -248,8 +248,13 @@ impl JsonSink { // never holds an unencodable value); still, propagate rather than silently // writing "{}" if that ever changes — an empty metadata file must not // masquerade as a successful json:// export. + // + // `NoMetadata` (E9008), matching `mux::meta`'s serialize guard: this is a + // metadata-encoding failure with no MKV involved. It was `MkvInvalid`, + // which `error::is_skippable_title_stub` reports as a skippable empty + // nav/menu stub — a json:// export that failed to encode is not that. let doc = serde_json::to_string_pretty(&title_json(title)) - .map_err(|_| crate::error::Error::MkvInvalid)?; + .map_err(|_| crate::error::Error::NoMetadata)?; let mut f = File::create(path)?; f.write_all(doc.as_bytes())?; f.write_all(b"\n")?; diff --git a/src/mux/mkvstream.rs b/src/mux/mkvstream.rs index 0a487c6..a6d8493 100644 --- a/src/mux/mkvstream.rs +++ b/src/mux/mkvstream.rs @@ -43,7 +43,7 @@ const MAX_UINT_LEN: u64 = 8; /// to allocate or read. Returns the size as `usize` when within bounds. fn checked_size(size: u64, cap: u64) -> io::Result { if size > cap { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } Ok(size as usize) } @@ -670,7 +670,7 @@ impl crate::pes::Stream for MkvStream { // every block PTS in the cluster. Reject it, mirroring the // EBML-size guard in parse_mkv_header. if raw > i64::MAX as u64 { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } rs.cluster_ts_ticks = raw as i64; continue; @@ -699,7 +699,7 @@ impl crate::pes::Stream for MkvStream { // frame so a round-trip through this muxer does not silently // drop those tracks. A non-u64::MAX size bounds the children. if size == u64::MAX { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } let mut remaining = size; let mut block: Option> = None; @@ -716,14 +716,14 @@ impl crate::pes::Stream for MkvStream { while remaining > 0 { let (cid, cs, hlen) = ebml::read_element_header(&mut rs.reader)?; if cs == u64::MAX { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } // A child whose header + body exceeds the bytes left in // the BlockGroup is malformed — reject it rather than // saturating `remaining` to 0 and reading past the group. let consumed = (hlen as u64).saturating_add(cs); if consumed > remaining { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } remaining -= consumed; match cid { @@ -816,7 +816,7 @@ impl crate::pes::Stream for MkvStream { // (take(u64::MAX)) and silently drop all later frames; // reject it like the rest of the parser. if size == u64::MAX { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } skip_bytes(&mut rs.reader, size)?; continue; @@ -944,16 +944,16 @@ fn parse_mkv_header(r: &mut impl Read) -> MkvHeaderResult { let (id, size, _) = ebml::read_element_header(r)?; if id != ebml::EBML { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } if size > i64::MAX as u64 { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } skip_bytes(r, size)?; let (id, _, _) = ebml::read_element_header(r)?; if id != ebml::SEGMENT { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } let (mut got_info, mut got_tracks) = (false, false); @@ -971,10 +971,10 @@ fn parse_mkv_header(r: &mut impl Read) -> MkvHeaderResult { match id { ebml::INFO => { // An unknown-size (u64::MAX) parent would drain children until - // an EOF read error instead of a clean MkvInvalid; reject it for + // an EOF read error instead of a clean MkvSourceInvalid; reject it for // parity with the segment loop guard below. if size == u64::MAX { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } let mut remaining = size; while remaining > 0 { @@ -983,7 +983,7 @@ fn parse_mkv_header(r: &mut impl Read) -> MkvHeaderResult { // would overflow `hlen + cs` (debug panic) and is meaningless // for a sized parent — reject it. if cs == u64::MAX { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } remaining = remaining.saturating_sub(hlen as u64 + cs); match cid { @@ -999,13 +999,13 @@ fn parse_mkv_header(r: &mut impl Read) -> MkvHeaderResult { } ebml::TRACKS => { if size == u64::MAX { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } let mut remaining = size; while remaining > 0 { let (cid, cs, hlen) = ebml::read_element_header(r)?; if cs == u64::MAX { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } remaining = remaining.saturating_sub(hlen as u64 + cs); if cid == ebml::TRACK_ENTRY { @@ -1061,7 +1061,7 @@ fn ts_pid_for_track(tnum: u16) -> io::Result { // MKV track numbers are 1-based; 0 is invalid (and would underflow the // `tnum - 2` below). if tnum == 0 { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } let pid: u32 = if tnum == 1 { 0x1011 @@ -1069,7 +1069,7 @@ fn ts_pid_for_track(tnum: u16) -> io::Result { 0x1100u32 + (tnum as u32 - 2) }; if pid > MAX_TS_PID { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } Ok(pid as u16) } @@ -1100,7 +1100,7 @@ fn parse_track(r: &mut impl Read, size: u64) -> io::Result { while remaining > 0 { let (cid, cs, hlen) = ebml::read_element_header(r)?; if cs == u64::MAX { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } remaining = remaining.saturating_sub(hlen as u64 + cs); match cid { @@ -1110,7 +1110,7 @@ fn parse_track(r: &mut impl Read, size: u64) -> io::Result { // existing small track numbers and corrupt PID/codec lookup). let n = read_uint_bounded(r, cs)?; if n > u16::MAX as u64 { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } tnum = n as u16; } @@ -1134,7 +1134,7 @@ fn parse_track(r: &mut impl Read, size: u64) -> io::Result { while vrem > 0 { let (vid, vs, vhlen) = ebml::read_element_header(r)?; if vs == u64::MAX { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } vrem = vrem.saturating_sub(vhlen as u64 + vs); if vid == ebml::PIXEL_HEIGHT { @@ -1149,7 +1149,7 @@ fn parse_track(r: &mut impl Read, size: u64) -> io::Result { while arem > 0 { let (aid, as_, ahlen) = ebml::read_element_header(r)?; if as_ == u64::MAX { - return Err(crate::error::Error::MkvInvalid.into()); + return Err(crate::error::Error::MkvSourceInvalid.into()); } arem = arem.saturating_sub(ahlen as u64 + as_); match aid { @@ -1527,9 +1527,12 @@ fn parse_block( boundaries are unknowable, so the block is rejected rather than passed \ downstream as one mangled frame" ); - // NOT MkvInvalid: `error::is_skippable_title_stub` classifies that code - // as an empty nav/menu stub, so a real track with unseparable frames - // would be dropped by the caller and the run would still report success. + // Its own code, not the generic `MkvSourceInvalid`: a laced Block names a + // specific RFC 9559 §10.3 feature whose header is self-inconsistent, which + // is a distinct diagnosis from "the container is corrupt somewhere". + // Neither is `MkvInvalid` — `error::is_skippable_title_stub` classifies + // that code as an empty nav/menu stub, so a real track with unseparable + // frames would be dropped by the caller while the run reported success. return Err(crate::error::Error::MkvLacingInvalid.into()); }; @@ -1977,8 +1980,14 @@ mod tests { } } - fn is_mkv_invalid(e: &io::Error) -> bool { - has_code(e, crate::error::E_MKV_INVALID) + /// Whether the error is the read path's malformed-source rejection + /// (`E_MKV_SOURCE_INVALID`). Asserted rather than the historical + /// `E_MKV_INVALID` on purpose: `E_MKV_INVALID` is the no-muxable-frames stub + /// code, and `error::is_skippable_title_stub` classifies it as skippable, so + /// a corrupt source reported under it would be silently passed over by an + /// all-titles rip that then exited successfully. + fn is_mkv_source_invalid(e: &io::Error) -> bool { + has_code(e, crate::error::E_MKV_SOURCE_INVALID) && !crate::error::is_skippable_title_stub(e) } /// Whether an error carries the given numeric code (the crate's errors @@ -1997,11 +2006,13 @@ mod tests { // 0x1100 + (tnum-2) <= 0x1FFF ⇒ tnum <= 0xF01. assert_eq!(ts_pid_for_track(0xF01).unwrap(), 0x1FFF); // One past the edge must be rejected, not wrap u16. - assert!(is_mkv_invalid(&ts_pid_for_track(0xF02).unwrap_err())); + assert!(is_mkv_source_invalid(&ts_pid_for_track(0xF02).unwrap_err())); // Former overflow case (debug panic / release garbage PID) is rejected. - assert!(is_mkv_invalid(&ts_pid_for_track(u16::MAX).unwrap_err())); + assert!(is_mkv_source_invalid( + &ts_pid_for_track(u16::MAX).unwrap_err() + )); // Track 0 is invalid (1-based) and would underflow tnum-2. - assert!(is_mkv_invalid(&ts_pid_for_track(0).unwrap_err())); + assert!(is_mkv_source_invalid(&ts_pid_for_track(0).unwrap_err())); } #[test] @@ -2009,22 +2020,22 @@ mod tests { // Within cap → Ok with usize value. assert_eq!(checked_size(100, 256).unwrap(), 100); assert_eq!(checked_size(256, 256).unwrap(), 256); - // Over cap → MkvInvalid, never a giant allocation. + // Over cap → MkvSourceInvalid, never a giant allocation. let e = checked_size(257, 256).unwrap_err(); - assert!(is_mkv_invalid(&e)); - // A hostile multi-GB block size is rejected as MkvInvalid. + assert!(is_mkv_source_invalid(&e)); + // A hostile multi-GB block size is rejected as MkvSourceInvalid. let e = checked_size(4 * 1024 * 1024 * 1024, MAX_BLOCK_SIZE).unwrap_err(); - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); } #[test] fn read_uint_bounded_rejects_oversized_int() { // size > 8 would index out of the fixed 8-byte buffer in // read_uint_val (panic / OOB). The guard turns it into a clean - // MkvInvalid error instead. + // MkvSourceInvalid error instead. let mut data = Cursor::new(vec![0u8; 16]); let e = read_uint_bounded(&mut data, 9).unwrap_err(); - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); } #[test] @@ -2039,7 +2050,7 @@ mod tests { // Claimed string length far above the cap must not allocate. let mut data = Cursor::new(vec![0u8; 16]); let e = read_string_bounded(&mut data, MAX_STRING_LEN + 1).unwrap_err(); - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); } /// Build a minimal MKV (EBML header + Segment + Info + Tracks) so the @@ -2066,7 +2077,7 @@ mod tests { #[test] fn simple_block_oversized_size_is_rejected() { // Cluster containing a SIMPLE_BLOCK that claims a 2 GiB payload. - // The reader must reject it (MkvInvalid) rather than attempt a + // The reader must reject it (MkvSourceInvalid) rather than attempt a // multi-GB allocation. Header parse stops at CLUSTER, so the // SIMPLE_BLOCK is hit on the first read(). let mut cluster = Vec::new(); @@ -2080,7 +2091,7 @@ mod tests { let mut stream = MkvStream::open(Cursor::new(bytes)).unwrap(); let e = stream.read().unwrap_err(); - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); } #[test] @@ -2126,7 +2137,7 @@ mod tests { #[test] fn truncated_simple_block_body_errors_not_panics() { // A SIMPLE_BLOCK that declares a 64-byte payload but supplies none. - // read_exact_bounded must surface a clean typed MkvInvalid error + // read_exact_bounded must surface a clean typed MkvSourceInvalid error // (a truncated declared element is malformed input), never panic, // and never allocate the full declared size up front. let mut cluster = Vec::new(); @@ -2139,7 +2150,7 @@ mod tests { let mut stream = MkvStream::open(Cursor::new(bytes)).unwrap(); let e = stream.read().unwrap_err(); - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); } /// Build a minimal MKV header + Segment + Info, then a Tracks element with a @@ -2171,7 +2182,7 @@ mod tests { #[test] fn oversized_codec_private_is_rejected() { // A TRACK_ENTRY whose CODEC_PRIVATE declares a payload above - // MAX_CODEC_PRIVATE must be rejected (MkvInvalid) before any + // MAX_CODEC_PRIVATE must be rejected (MkvSourceInvalid) before any // multi-MB allocation, while parsing the header. let mut entry = Vec::new(); ebml::write_uint(&mut entry, ebml::TRACK_NUMBER, 1).unwrap(); @@ -2197,10 +2208,10 @@ mod tests { out.extend_from_slice(&track_entry); let e = match MkvStream::open(Cursor::new(out)) { - Ok(_) => panic!("expected MkvInvalid, got Ok"), + Ok(_) => panic!("expected MkvSourceInvalid, got Ok"), Err(e) => e, }; - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); } #[test] @@ -2444,7 +2455,7 @@ mod tests { // computation would underflow `tnum - 2`). let bytes = mkv_with_track_and_cluster(0, 1, &[]); let e = open_err(MkvStream::open(Cursor::new(bytes))); - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); } #[test] @@ -2452,7 +2463,7 @@ mod tests { // 65536 would truncate to 0 via `as u16` and then underflow. let bytes = mkv_with_track_and_cluster(65536, 1, &[]); let e = open_err(MkvStream::open(Cursor::new(bytes))); - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); } #[test] @@ -2482,7 +2493,7 @@ mod tests { out.extend_from_slice(&tracks); let e = open_err(MkvStream::open(Cursor::new(out))); - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); } #[test] @@ -2503,20 +2514,20 @@ mod tests { out.extend_from_slice(&info); let e = match MkvStream::open(Cursor::new(out)) { - Ok(_) => panic!("expected MkvInvalid, got Ok"), + Ok(_) => panic!("expected MkvSourceInvalid, got Ok"), Err(e) => e, }; - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); } #[test] fn read_uint_val_len_nine_errors_not_panics() { // Direct helper test: an EBML uint cannot exceed 8 bytes. len=9 // would index past the fixed 8-byte stack buffer and panic on - // untrusted input; it must return MkvInvalid instead. + // untrusted input; it must return MkvSourceInvalid instead. let mut data = Cursor::new(vec![0u8; 16]); let e = ebml::read_uint_val(&mut data, 9).unwrap_err(); - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); } #[test] @@ -2525,7 +2536,7 @@ mod tests { // malformed and must error rather than over- or under-read. let mut data = Cursor::new(vec![0u8; 16]); let e = ebml::read_float_val(&mut data, 5).unwrap_err(); - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); // 0/4/8 remain valid widths. let mut z = Cursor::new(vec![0u8; 16]); assert_eq!(ebml::read_float_val(&mut z, 0).unwrap(), 0.0); @@ -2538,11 +2549,11 @@ mod tests { #[test] fn non_utf8_string_element_is_rejected() { // A string element with invalid UTF-8 bytes must surface a numeric - // MkvInvalid error, not an io::Error wrapping the FromUtf8Error + // MkvSourceInvalid error, not an io::Error wrapping the FromUtf8Error // English message (library no-English rule). let mut data = Cursor::new(vec![0xFF, 0xFE, 0xFD, 0xFC]); let e = ebml::read_string_val(&mut data, 4).unwrap_err(); - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); } #[test] @@ -2751,7 +2762,7 @@ mod tests { #[test] fn cluster_timestamp_above_i64_max_is_rejected() { // CLUSTER_TIMESTAMP encoded as an 8-byte uint with the top bit set - // (> i64::MAX). The reader must surface MkvInvalid on read(). + // (> i64::MAX). The reader must surface MkvSourceInvalid on read(). let mut cluster = Vec::new(); ebml::write_id(&mut cluster, ebml::CLUSTER).unwrap(); ebml::write_unknown_size(&mut cluster).unwrap(); @@ -2761,7 +2772,48 @@ mod tests { let bytes = mkv_with_track_and_cluster(1, 1, &cluster); let mut stream = MkvStream::open(Cursor::new(bytes)).unwrap(); let e = stream.read().unwrap_err(); - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); + } + + // ============================================================ + // A malformed mkv:// SOURCE must never be classified as a skippable + // title stub. The read path used to raise `Error::MkvInvalid` for every + // malformed-input rejection, and `error::is_skippable_title_stub` reports + // that code as an empty nav/menu stub — so an all-titles rip silently + // passed over a corrupt input and exited reporting success. + // ============================================================ + + #[test] + fn corrupt_source_is_not_classified_as_a_skippable_title_stub() { + // Same corrupt fixture as above (CLUSTER_TIMESTAMP > i64::MAX) driven + // through the real reader, asserted against the public classifier. + // Mutation: raising `Error::MkvInvalid` instead of + // `Error::MkvSourceInvalid` at that guard turns this red. + let mut cluster = Vec::new(); + ebml::write_id(&mut cluster, ebml::CLUSTER).unwrap(); + ebml::write_unknown_size(&mut cluster).unwrap(); + ebml::write_id(&mut cluster, ebml::CLUSTER_TIMESTAMP).unwrap(); + ebml::write_size(&mut cluster, 8).unwrap(); + cluster.extend_from_slice(&0xFFFF_FFFF_FFFF_FFFFu64.to_be_bytes()); + let bytes = mkv_with_track_and_cluster(1, 1, &cluster); + let mut stream = MkvStream::open(Cursor::new(bytes)).unwrap(); + let e = stream.read().unwrap_err(); + assert!( + !crate::error::is_skippable_title_stub(&e), + "a corrupt mkv:// source must be a failure, not a skippable stub: {e}" + ); + assert_eq!( + e.to_string(), + format!("E{}", crate::error::E_MKV_SOURCE_INVALID) + ); + // A truncated element body (the EBML read primitive) is the same verdict, + // proving the classification is not specific to one guard. + let short = ebml::read_binary_val(&mut Cursor::new(&[1u8, 2, 3, 4]), 100).unwrap_err(); + assert!(!crate::error::is_skippable_title_stub(&short)); + assert_eq!( + short.to_string(), + format!("E{}", crate::error::E_MKV_SOURCE_INVALID) + ); } // ============================================================ @@ -2853,7 +2905,7 @@ mod tests { ebml::write_id(&mut out, ebml::SEGMENT).unwrap(); // wrong first element ebml::write_size(&mut out, 0).unwrap(); let e = open_err(MkvStream::open(Cursor::new(out))); - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); } #[test] @@ -2866,7 +2918,7 @@ mod tests { ebml::write_id(&mut out, ebml::INFO).unwrap(); // not SEGMENT ebml::write_size(&mut out, 0).unwrap(); let e = open_err(MkvStream::open(Cursor::new(out))); - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); } #[test] @@ -2907,7 +2959,7 @@ mod tests { #[test] fn block_group_unknown_size_is_rejected() { // A BLOCK_GROUP declaring unknown size (u64::MAX) would loop draining - // the stream; the reader must reject it as MkvInvalid. + // the stream; the reader must reject it as MkvSourceInvalid. let mut cluster = Vec::new(); ebml::write_id(&mut cluster, ebml::CLUSTER).unwrap(); ebml::write_unknown_size(&mut cluster).unwrap(); @@ -2916,7 +2968,7 @@ mod tests { let bytes = mkv_with_track_and_cluster(1, 1, &cluster); let mut stream = MkvStream::open(Cursor::new(bytes)).unwrap(); let e = stream.read().unwrap_err(); - assert!(is_mkv_invalid(&e)); + assert!(is_mkv_source_invalid(&e)); } #[test]