From f5e169efb35905738d5587c57551a1d10025abae Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Wed, 29 Jul 2026 22:11:53 -0700 Subject: [PATCH] Stop reporting a corrupt mkv:// source as an empty title MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit E6008 meant two unrelated things: "this title produced no muxable frames", which is a benign stub worth skipping, and "the source file is malformed", which is not. Because a single code carried both, is_skippable_title_stub answered yes to the second one — so feeding a truncated or corrupt mkv:// input made the engine classify it SkippableStub, print a notice saying the title was empty, and exit 0. Silent data loss reported as success. Split into E9053 MkvSourceInvalid for the read path (25 raise sites across mkvstream.rs and ebml.rs's read primitives) and E9054 MkvUnencodable for the four write-side sites, which are the encoder refusing to emit a body at or above the 56-bit VINT limit — an output limit with no input involved, so calling it a corrupt source would be wrong in the other direction. E6008 keeps only the zero-frame guard it was documented to mean. Kept one code for the whole read path rather than one per raise site: nothing a consumer does differs between a bad VINT, a non-UTF-8 string element, a truncated body and a child overrunning its parent. E9052 is the model for when a carve-out earns its keep — laced blocks name one specific RFC 9559 §10.3 feature with its own diagnosis. Also fixed meta_sink.rs raising MkvInvalid for a serde_json encode failure in the json:// sink, where no MKV is involved at all; it now matches the identical guard in mux/meta.rs. Reverting the split at the single code() arm reproduces the old classification: 22 tests fail, including both new assertions. The opposite direction is pinned too — dropping E6008 from the predicate fails the genuine-stub test, which drives the real muxer end to end. --- CHANGELOG.md | 17 +++++ src/error.rs | 106 ++++++++++++++++++++++++++- src/mux/driver.rs | 11 ++- src/mux/ebml.rs | 30 ++++---- src/mux/meta_sink.rs | 7 +- src/mux/mkvstream.rs | 168 ++++++++++++++++++++++++++++--------------- 6 files changed, 260 insertions(+), 79 deletions(-) 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]