comment-guard: trim over-cap comment blocks to caps (comments only, no code change)

Bring every flagged comment block within its cap (inline // <=3 lines,
/// item-doc prose <=8): tighten wording, or convert an over-long inline
block that documents the item below it into a /// doc, preserving all
technical facts. No code, string literals, or logic changed.
This commit is contained in:
Matthew Jackson
2026-09-17 22:47:32 -07:00
parent 041ac5876f
commit 5c9e2fefe5
41 changed files with 335 additions and 466 deletions
+3 -4
View File
@@ -306,10 +306,9 @@ pub(crate) fn resolve_dk_node(
uv: u32,
u_mask_shift: u8,
) -> Option<DeviceKey> {
// `u_mask_shift` is a disc/keydb-controlled u8: bit positions 32..=255 do not
// exist in a u32, and `1u32 << b` with b >= 32 panics (debug) / wraps (release).
// Cap the search at the 32 real bit positions — same >= 32 guard the mask shifts
// above use — so a bad disc can never trip the shift.
// `u_mask_shift` is disc/keydb-controlled: `1u32 << b` with b >= 32 panics
// (debug) / wraps (release), so cap the search at the 32 real u32 bit positions
// (same >= 32 guard the mask shifts above use) — a bad disc can't trip the shift.
for b in 0..u_mask_shift.min(32) {
let dk = DeviceKey {
key: *key,
+9 -10
View File
@@ -105,16 +105,15 @@ pub(crate) fn role_paths(udf: &crate::udf::UdfFs, role: AacsRole) -> Vec<String>
v
}
// Walk an AACS role's candidate paths, returning the first that reads.
//
// AACS ships a `/AACS/DUPLICATE/` copy of every managed file precisely so a
// bad read on the primary can fall through to the backup. A missing candidate
// (`UdfNotFound`) and a failed read (`DiscRead`) are therefore both retriable:
// the walk moves on to the next candidate rather than aborting. Only after
// every candidate has been tried does a remembered `DiscRead` propagate (more
// informative than `AacsNoKeys`); if every candidate was merely absent, the
// result is `AacsNoKeys`. Any other error is a genuine hard failure (parse,
// corruption) and propagates immediately.
/// Walk an AACS role's candidate paths, returning the first that reads.
///
/// AACS ships a `/AACS/DUPLICATE/` copy of every managed file so a bad primary
/// read can fall through to the backup. A missing candidate (`UdfNotFound`) and a
/// failed read (`DiscRead`) are both retriable: the walk moves on rather than
/// aborting. After all candidates are tried, a remembered `DiscRead` propagates
/// (more informative than `AacsNoKeys`); if every candidate was merely absent, the
/// result is `AacsNoKeys`. Any other error is a hard failure (parse, corruption)
/// and propagates immediately.
pub(crate) fn read_first<S, F>(candidates: &[S], mut read: F) -> crate::error::Result<Vec<u8>>
where
S: AsRef<str>,
+6 -11
View File
@@ -178,11 +178,9 @@ pub fn fmts_key_ranges(
) else {
continue;
};
// start_byte = start_spn*192 is generally NOT 2048-aligned, so the LBA
// carry is floor((end_byte-1)/2048) - floor(start_byte/2048), NOT
// (end_byte-1-start_byte)/2048 — the latter is only correct when
// start_byte is sector-aligned and otherwise mismatches `b - a`, wrongly
// dropping a legitimate non-aligned segment back to the Unit Key.
// start_byte = start_spn*192 is generally NOT 2048-aligned, so the LBA carry
// is floor((end_byte-1)/2048) - floor(start_byte/2048); the naive (end_byte-1
// -start_byte)/2048 matches `b-a` only when sector-aligned, else wrongly drops the non-aligned segment back to the Unit Key.
let sector = crate::consts::SECTOR_BYTES as u64;
let expected_delta = (end_byte - 1) / sector - start_byte / sector;
if b >= a && (b - a) as u64 == expected_delta {
@@ -272,12 +270,9 @@ mod tests {
start_lba: 1000,
sector_count: 1_000_000,
}];
// start_spn=1 → start_byte = 192, which is NOT 2048-aligned. end_spn=10 →
// end_byte-1 = 2111. The correct LBA carry is
// floor(2111/2048) - floor(192/2048) = 1 - 0 = 1, so the segment maps to
// LBA 1000..=1001. The old aligned formula computed
// (2111 - 192)/2048 = 0, which mismatched b-a = 1 and DROPPED the segment
// (falling back to the Unit Key) — this test caught that as an empty map.
// start_spn=1 → start_byte=192 (NOT 2048-aligned); end_spn=10 → end_byte-1=2111.
// Correct carry floor(2111/2048)-floor(192/2048)=1 maps to LBA 1000..=1001; the
// old aligned (2111-192)/2048=0 mismatched b-a=1 and DROPPED it to the Unit Key (empty map).
let segs = vec![Segment {
index: 5,
start_spn: 1,
+3 -5
View File
@@ -143,12 +143,10 @@ fn recover_title_key_from_plain(
}
}
/// Recover the CSS title key from a scrambled sector using a known plaintext
/// for the encrypted region.
/// Recover the CSS title key from a scrambled sector via known plaintext.
///
/// `plain` is the expected plaintext at byte 0x80 (at least 10 bytes).
/// Returns the recovered key only if it actually descrambles the sector back
/// to `plain` — guarding against the rare spurious LFSR-seed match.
/// `plain` is the expected plaintext at byte 0x80 (≥10 bytes); returns the key only
/// if it descrambles the sector back to `plain`, guarding a rare spurious LFSR match.
///
/// TEST-ONLY: the production crack path uses [`crack_title_key`], which derives
/// its own crib. `recover_title_key` is a known-plaintext helper exercised only
+8 -17
View File
@@ -379,24 +379,15 @@ pub fn has_scramble_flag_bits(sector: &[u8]) -> bool {
/// untouched, so this signature survives scrambling.
pub(crate) const PACK_START: [u8; 4] = [0x00, 0x00, 0x01, 0xBA];
/// Check if a sector is a CSS-scrambled DVD **video pack** — the HARDENED
/// test the crack scan uses for its `saw_scrambled` flag, and the same gate
/// [`descramble_sector`] / [`descramble_region`] use.
/// Check if a sector is a CSS-scrambled DVD **video pack** — the HARDENED gate the
/// crack scan's `saw_scrambled` flag and [`descramble_sector`]/[`descramble_region`] share.
///
/// Requires BOTH the MPEG-PS pack-start code AND the 0x14 scramble bits —
/// [`has_scramble_flag_bits`] alone is not enough. Also excludes
/// structural/nav `stream_id`s at offset 0x11 (`0xBB`/`0xBE`/`0xBF`) that CSS
/// never scrambles, so a decrypted HD-DVD's RDI nav packs can't falsely trip
/// it. See docs/css-mod.md — `is_scrambled_pack`.
///
/// NOTE: the `stream_id` sits at 0x11 only when the pack header carries ZERO
/// stuffing bytes (14-byte pack header, then `00 00 01 <stream_id>`); the low
/// 3 bits of byte 0x0D are `pack_stuffing_length` and would shift it right by
/// that many bytes. DVD-Video VOB packs are authored with no pack stuffing, so
/// 0x11 is correct for the sectors this gate ever sees. A stuffed pack would
/// read a 0xFF stuffing byte here — which is not one of the excluded nav ids,
/// so the exclusion simply doesn't fire (the safe direction: the 0x14 scramble
/// bits, still the real gate, decide it).
/// Requires BOTH the MPEG-PS pack-start code AND the 0x14 scramble bits
/// ([`has_scramble_flag_bits`] alone is not enough), and excludes structural/nav
/// `stream_id`s at 0x11 (`0xBB`/`0xBE`/`0xBF`) that CSS never scrambles so a decrypted
/// HD-DVD's RDI nav packs can't falsely trip it. 0x11 holds the `stream_id` only for
/// zero-stuffing headers (always true for DVD-Video VOB packs); a stuffed pack reads
/// 0xFF there, harmless. See docs/css-mod.md — `is_scrambled_pack`.
pub fn is_scrambled_pack(sector: &[u8]) -> bool {
use crate::consts::pes_stream_id::{PADDING_STREAM, PRIVATE_STREAM_2, SYSTEM_HEADER};
sector.len() >= 2048
+6 -8
View File
@@ -1522,10 +1522,9 @@ mod tests {
}
}
// `with_content_ranges` contract: an encrypted unit whose LBA is OUTSIDE the
// disc's content extents (clear filesystem / BDMV nav) must be passed through
// untouched — never decrypted, verified, or counted as loss. Before the fix the
// content map was ignored, so this unit was decrypted (its bytes mangled).
// `with_content_ranges` contract: an encrypted unit whose LBA is OUTSIDE the disc's
// content extents (clear filesystem / BDMV nav) must pass through untouched — never
// decrypted, verified, or counted as loss. Pre-fix the content map was ignored, so this unit was decrypted (mangled).
#[test]
fn mapped_decrypt_skips_units_outside_content_ranges() {
let unit_key = [0x33u8; 16];
@@ -1560,10 +1559,9 @@ mod tests {
);
}
// Exercise the rayon parallel branch of `apply_aacs_map`: a buffer of far more
// than `PARALLEL_MIN_UNITS` units, decrypted on the multi-thread path, must
// recover EVERY unit byte-for-byte — i.e. the `par_chunks_mut` fan-out keeps the
// per-unit `idx_in_buf` correct (a race or off-by-one would corrupt some units).
// Exercise the rayon parallel branch of `apply_aacs_map`: a buffer of far more than
// `PARALLEL_MIN_UNITS` units on the multi-thread path must recover EVERY unit byte-for-
// byte — i.e. `par_chunks_mut` keeps each `idx_in_buf` correct (a race/off-by-one corrupts units).
#[test]
fn mapped_decrypt_parallel_path_recovers_every_unit() {
use crate::disc::ContentFormat;
+8 -10
View File
@@ -600,11 +600,9 @@ pub(super) fn plan(root: &Path) -> Result<Layout> {
if total_sectors > MAX_IMAGE_SECTORS {
return Err(Error::DirImageTooLarge);
}
// Metadata is materialized up front and held for the life of the image, so
// its size is bounded here rather than discovered when memory runs out.
// Count the FULL footprint — File Entries AND each directory's FID-list
// sectors — not just one File Entry per node: a wide, shallow tree's
// directory data alone can dwarf the per-node entries.
// Metadata is materialized up front and held for the image's life, so bound its
// size here rather than at OOM. Count the FULL footprint — File Entries AND each
// directory's FID-list sectors (a wide, shallow tree's dir data can dwarf them).
let meta_bytes = metadata_block_count(&tree).saturating_mul(SECTOR as u64);
if meta_bytes > MAX_META_BYTES {
return Err(Error::DirImageTooLarge);
@@ -694,11 +692,11 @@ mod tests {
);
}
// The metadata ceiling must account for each directory's FID-list sectors,
// not just one File Entry per node: a wide directory's FID list can span
// many sectors that the old (dir_count + file_count) count ignored, so an
// oversized folder could slip past the guard and be materialized in full.
// See docs/dirimage.md — metadata_ceiling_counts_fid_lists.
/// The metadata ceiling must account for each directory's FID-list sectors,
/// not just one File Entry per node: a wide directory's FID list can span
/// many sectors that the old (dir_count + file_count) count ignored, so an
/// oversized folder could slip past the guard and be materialized in full.
/// See docs/dirimage.md — metadata_ceiling_counts_fid_lists.
#[test]
fn metadata_block_count_includes_the_fid_list_sectors() {
let files: Vec<FileNode> = (0..300)
+6 -6
View File
@@ -259,12 +259,9 @@ impl SectorSource for DirImage {
if buf.len() < need {
return Err(Error::UdfBufferTooSmall);
}
// A read that BEGINS at or beyond the image's own capacity addresses a
// sector that cannot exist (an oversized/crafted offset): refuse it, as a
// real file/ISO source's read_exact does past EOF, rather than fabricate
// zeros and report success. A batch that starts IN range and overshoots
// the end is still fine — its tail past capacity zero-pads below, which
// the mux's final batch and the FS scan's fixed-LBA probes both expect.
// A read that BEGINS at/beyond the image's own capacity addresses a sector
// that cannot exist (an oversized/crafted offset): refuse it, as a real
// file/ISO source's read_exact does past EOF, not fabricated zeros+success.
if count > 0 && lba >= self.total_sectors {
return Err(Error::DiscRead {
sector: lba as u64,
@@ -272,6 +269,9 @@ impl SectorSource for DirImage {
sense: None,
});
}
// A batch that starts IN range but overshoots the end is fine: this zero-pads
// its tail past capacity, which the mux's final batch and the FS scan's
// fixed-LBA probes both expect.
buf[..need].fill(0);
// Walk the request in RUNS, not sector by sector: a mux batch (8192
// sectors) almost always lands in one extent, so per-sector seek+read
+3 -8
View File
@@ -130,14 +130,9 @@ impl AacsCertUnlocker<'_> {
// AACS-specific "why the read_data_key read failed" diagnostic does
// not cross the seam. The bus-key gate keys off presence, not cause.
read_data_key_err: None,
// Always false on the cert route: `run_bus` dispatches ONLY the disc-
// keyed unlockers (`disc_unlockers` = AACS, DVD), so `matched` is only
// ever "AACS"/"DVD"/"" — never a firmware/drive unlocker (freemkv, LD,
// Renesas). The disc-keyed cert route removes bus encryption via the
// read_data_key (AKE), which the gate credits through
// `read_data_key.is_some()`. A genuine DRIVE unlock cannot surface here;
// it arrives via the OEM-VID short-circuit in `do_handshake_cert`, which
// sets `drive_unlocked: true` for exactly that (rdk-less) case.
// Always false on the cert route: `run_bus` dispatches ONLY disc-keyed unlockers
// (AACS, DVD), so `matched` is never a firmware/drive unlocker (freemkv/LD/Renesas);
// a genuine DRIVE unlock instead sets `drive_unlocked: true` via the OEM-VID short-circuit in `do_handshake_cert` (the rdk-less case, credited by the gate like a cert `read_data_key`).
drive_unlocked: false,
})
}
+12 -18
View File
@@ -354,9 +354,6 @@ impl SectorSource for Borrowed<'_> {
}
}
/// Recursively plan the host tree: collect directories to create and files to
/// extract, sanitizing each component and detecting host-path collisions.
/// Skips the top-level `AACS/` and `CERTIFICATE/` directories (§7).
/// Whether `dir` lives on a case-INSENSITIVE filesystem (macOS APFS/HFS+ and
/// Windows NTFS default). Probes the real target: create a lowercase marker
/// and test whether its uppercase spelling resolves to the same file. On a
@@ -366,12 +363,9 @@ impl SectorSource for Borrowed<'_> {
/// can't run (e.g. a read-only dir), assume case-insensitive — the conservative
/// choice that never MISSES a real overwrite collision.
fn dir_is_case_insensitive(dir: &Path) -> bool {
// Unique per attempt: a FIXED probe name (`.fmkv_case_probe`) let two
// concurrent extracts into the same dir race — one's `remove_file` could
// delete the other's marker between its create and its `exists()` check,
// flipping the result (a TOCTOU collision). A per-call token makes each
// probe's lower/upper pair private, so concurrent probes never touch the
// same path.
// Unique per attempt: a FIXED name let concurrent extracts race — one's
// `remove_file` could delete another's marker between create and `exists()`
// (a TOCTOU flip). A per-call token keeps each probe's pair on private paths.
let token = unique_probe_token();
let lower_name = format!(".fmkv_case_probe_{token}");
let lower = dir.join(&lower_name);
@@ -403,6 +397,9 @@ fn unique_probe_token() -> String {
format!("{pid:x}_{nanos:x}_{n:x}")
}
/// Recursively plan the host tree: collect directories to create and files to
/// extract, sanitizing each component and detecting host-path collisions.
/// Skips the top-level `AACS/` and `CERTIFICATE/` directories (§7).
#[allow(clippy::too_many_arguments)]
fn plan_tree(
reader: &mut dyn SectorSource,
@@ -432,11 +429,9 @@ fn plan_tree(
let safe = sanitize_component(&entry.name)?;
let child_rel = host_rel.join(&safe);
let child_disc = format!("{disc_path}/{}", entry.name);
// Collision: two distinct disc paths → same host FILE. On a case-
// insensitive host that folds `Movie`/`movie` together; on a case-
// sensitive host only an EXACT match collides (case-differing names are
// distinct files that coexist). Either way `.partial` shares the
// final-name namespace so `X`/`X.partial` collide too.
// Collision: two distinct disc paths → one host FILE. Case-insensitive folds
// `Movie`/`movie`; case-sensitive collides only on EXACT match (else names
// coexist). `.partial` shares the final-name namespace, so `X`/`X.partial` collide.
let mut register = |key: PathBuf| -> Result<()> {
let folded = if case_insensitive {
key.to_string_lossy().to_lowercase()
@@ -1956,10 +1951,9 @@ mod tests {
fn scrambled_vob(title_key: [u8; 5], marker: u8) -> (Vec<u8>, Vec<u8>) {
let seed = [0x11u8, 0x22, 0x33, 0x44, marker];
let mut plain = vec![0u8; 2048];
// The crack scan's `is_scrambled_pack` gate requires the MPEG-PS pack-
// start signature (a real scrambled sector is an MPEG-2 PS pack) before
// attempting a crack; without it, `resolve_vts_key` falls back to
// `base_keys` for both groups, masking this regression.
// The crack scan's `is_scrambled_pack` gate needs the MPEG-PS pack-start
// signature (real scrambled sector = MPEG-2 PS pack) before cracking; without it
// `resolve_vts_key` falls back to `base_keys` for both groups, masking the regression.
plain[0x00..0x04].copy_from_slice(&crate::css::PACK_START);
plain[0x14] = 0x10; // scramble flag
let pat: Vec<u8> = (0..8)
+11 -18
View File
@@ -1607,13 +1607,9 @@ impl Disc {
/// Read UDF filesystem and set up buffered reader with metadata prefetched.
/// Shared setup for both identify() and scan().
fn read_udf(session: &mut Drive) -> Result<(u32, udf::BufferedSectorReader<'_>, udf::UdfFs)> {
// READ CAPACITY is the sole authoritative whole-disc size (the UDF
// partition is a subset — measured 288 sectors short on a real BD, which
// would truncate the backup anchor). Its field failures are sporadic
// (5/5 reliable on the target drive), so retry rather than substitute a
// short value. On persistent failure capacity is left 0: the shared
// scan/identify/MKV path stays lenient (titles read by extent), and only
// image output hard-errors, via `image_read_sectors`.
// READ CAPACITY is the sole authoritative whole-disc size (the UDF partition
// is a subset — 288 sectors short on a real BD, truncating the backup anchor).
// Its sporadic failures are ridden out by `read_capacity_retrying` (0 on hard fail).
let capacity = Self::read_capacity_retrying(session);
let batch = detect_max_batch_sectors(session.device_path());
let mut buffered = udf::BufferedSectorReader::new(session, batch);
@@ -1641,17 +1637,14 @@ impl Disc {
/// comfortably inside a cold spin-up + one re-enumeration.
const READ_CAPACITY_BACKOFF_CAP: std::time::Duration = std::time::Duration::from_secs(2);
/// READ CAPACITY with retry. Returns the hardware sector count, or 0 if
/// every attempt failed. READ CAPACITY is the only authoritative whole-disc
/// size for an image and its failures are sporadic (a transient USB/UAS +
/// spin-up fault), so ride one out with exponential backoff instead of
/// substituting a UDF partition size (a subset that would truncate the
/// image). No readiness poke: the only ready helper (`Drive::wait_ready`) is
/// a ~30s TUR loop that would blow this budget, and inventing a cheap
/// single-shot TUR is out of scope — a plain backoff is sufficient. A final
/// 0 keeps the shared scan/identify/MKV path lenient (those read titles by
/// extent); image output turns the 0 into a hard [`Error::EmptyImage`] via
/// `image_read_sectors`.
/// READ CAPACITY with retry. Returns the hardware sector count, or 0 if every
/// attempt failed. It is the only authoritative whole-disc size for an image and its
/// failures are sporadic (a transient USB/UAS + spin-up fault), so ride one out with
/// exponential backoff rather than substitute a UDF partition size (a subset that
/// would truncate the image). No readiness poke: the only ready helper
/// (`Drive::wait_ready`) is a ~30s TUR loop that would blow this budget, and a cheap
/// single-shot TUR is out of scope. A final 0 keeps the shared scan/identify/MKV path
/// lenient (titles read by extent); image output makes it a hard [`Error::EmptyImage`] via `image_read_sectors`.
fn read_capacity_retrying(session: &mut Drive) -> u32 {
let mut backoff = Self::READ_CAPACITY_BACKOFF_START;
for attempt in 1..=Self::READ_CAPACITY_ATTEMPTS {
+3 -4
View File
@@ -313,10 +313,9 @@ impl Drive {
return Ok(());
}
Err(Error::Halted) => return Err(Error::Halted),
// A dead bus (transport failure / disconnected drive) will never
// spin up — surface it immediately instead of polling a phantom
// for the full 30 s. A plain not-ready TUR failure falls through
// and keeps the loop going.
// A dead bus (transport failure / disconnected drive) will never spin
// up — surface it immediately instead of polling a phantom for 30 s. A
// plain not-ready TUR failure falls through and keeps the loop going.
Err(e) if e.is_scsi_transport_failure() => return Err(e),
Err(_) => {}
}
+8 -9
View File
@@ -64,11 +64,11 @@ pub fn parse_hex_u8(s: &str) -> Option<u8> {
u8::from_str_radix(hex_int_body(s)?, 16).ok()
}
// Trim, strip the optional `0x`/`0X` prefix, and reject a leading `+` sign.
// `from_str_radix` accepts a leading `+` (`+10` → 16), but hex key material
// never carries a sign — the byte parsers already reject it via `byte()`, so
// the integer parsers must too, or the same value parses in one path and drops
// in another. (A bare `-` already fails on the unsigned parse.)
/// Trim, strip the optional `0x`/`0X` prefix, and reject a leading `+` sign.
/// `from_str_radix` accepts a leading `+` (`+10` → 16), but hex key material
/// never carries a sign — the byte parsers already reject it via `byte()`, so
/// the integer parsers must too, or the same value parses in one path and drops
/// in another. (A bare `-` already fails on the unsigned parse.)
fn hex_int_body(s: &str) -> Option<&str> {
let body = strip_hex_prefix(s.trim());
if body.starts_with('+') {
@@ -145,10 +145,9 @@ mod tests {
assert_eq!(parse_hex_u16("0xzz"), None);
}
// `from_str_radix` accepts a leading `+` (`+10` → 16), but hex key material
// is never signed and the byte parsers reject it — so the integer parsers
// must reject it too. Red-before-green: before the guard, each of these
// returned `Some(..)` instead of `None`.
// `from_str_radix` accepts a leading `+` (`+10` → 16), but hex key material is
// never signed and the byte parsers reject it, so the integer parsers must too.
// Red-before-green: before the guard, each of these returned `Some(..)` not `None`.
#[test]
fn hex_ints_reject_leading_plus_sign() {
assert_eq!(parse_hex_u16("+10"), None);
+3 -5
View File
@@ -942,11 +942,9 @@ fn parse_pgc(data: &[u8], pgc_offset: usize, chapters: u16) -> Result<DvdTitle>
times
};
// Extract subtitle palette at PGC offset 0xA4: 16 colors × 4 bytes
// [padding, Y, Cr, Cb]. Chroma order is Cr (byte 2) BEFORE Cb (byte 3),
// fixed by the DVD-Video PGC CLUT format — NOT Cb before Cr. The consumer
// `mux::codec::dvdsub::ycbcr_to_rgb` reads it the same way; keep them in
// lockstep or subtitle colours channel-swap.
// Subtitle palette at PGC offset 0xA4: 16 colors × 4 bytes [padding, Y, Cr, Cb].
// Chroma order is Cr (byte 2) BEFORE Cb (byte 3) per the DVD-Video PGC CLUT format,
// NOT Cb first; consumer `mux::codec::dvdsub::ycbcr_to_rgb` must stay in lockstep or subtitle colours channel-swap.
let palette = if pgc_offset + 0xA4 + 64 <= data.len() {
let mut colors = Vec::with_capacity(16);
for i in 0..16 {
+8 -8
View File
@@ -145,14 +145,14 @@ pub(crate) fn parse_bdmt_xml(xml_text: &str) -> Option<BdmtFields> {
Some((title, description, disc_set))
}
// Reject description candidates that are themselves XML fragments (e.g.
// only <di:thumbnail/> children, no prose) OR mixed content — prose with an
// embedded child element, which `xml::text` returns verbatim (tags and all).
// The old `starts_with('<')` only caught fragments that LED with a tag, so a
// description like `Real prose <di:thumbnail/>` leaked its markup through. Any
// `<` that begins an element / close tag / comment / PI marks the value as
// XML-tainted; a bare `<` used as prose (e.g. `a < b`) is left alone.
// See docs/bdmt.md — looks_like_xml.
/// Reject description candidates that are themselves XML fragments (e.g.
/// only <di:thumbnail/> children, no prose) OR mixed content — prose with an
/// embedded child element, which `xml::text` returns verbatim (tags and all).
/// The old `starts_with('<')` only caught fragments that LED with a tag, so a
/// description like `Real prose <di:thumbnail/>` leaked its markup through. Any
/// `<` that begins an element / close tag / comment / PI marks the value as
/// XML-tainted; a bare `<` used as prose (e.g. `a < b`) is left alone.
/// See docs/bdmt.md — looks_like_xml.
fn looks_like_xml(s: &str) -> bool {
let bytes = s.as_bytes();
bytes.iter().enumerate().any(|(i, &b)| {
+8 -9
View File
@@ -595,10 +595,9 @@ impl<'a> Iterator for Instructions<'a> {
let pc = self.pos;
let opcode = self.code[pc];
let size = instruction_size(self.code, pc)?;
// `size` can be as large as `usize::MAX` on a 32-bit target (saturated
// switch-table math on adversarial bytecode), so `pc + size` must be a
// CHECKED add — a wrapping add could produce a small `end` that passes
// the bounds check below and then slices out of range / panics.
// `size` can reach `usize::MAX` on a 32-bit target (saturated switch-table
// math on adversarial bytecode), so `pc + size` must be a CHECKED add: a
// wrapping add could yield a small `end` that passes the bounds check below, then slices out of range / panics.
let end = pc.checked_add(size)?;
if end > self.code.len() {
return None;
@@ -1167,11 +1166,11 @@ mod tests {
assert_eq!(count, 0);
}
// A `tableswitch` whose declared jump table spans the full i32 index range
// drives `instruction_size` to a saturated, buffer-exceeding size. `next()`
// must add `pc + size` with a CHECKED add (no wrap on a 32-bit `usize`, no
// panic on any target) and simply stop the walk — malformed disc bytecode
// yields no instruction, never a panic.
/// A `tableswitch` whose declared jump table spans the full i32 index range
/// drives `instruction_size` to a saturated, buffer-exceeding size. `next()`
/// must add `pc + size` with a CHECKED add (no wrap on a 32-bit `usize`, no
/// panic on any target) and simply stop the walk — malformed disc bytecode
/// yields no instruction, never a panic.
#[test]
fn instructions_iter_rejects_oversized_switch_without_panicking() {
let mut code = vec![TABLESWITCH, 0, 0, 0]; // opcode + 3 pad bytes
+5 -8
View File
@@ -96,11 +96,9 @@ fn assign_stream_numbers(
let mut audio_idx: u32 = 1;
let mut sub_idx: u32 = 1;
// Numbers already EMITTED per type. A playbackconfig that maps two distinct
// StreamInfo_IDs to the SAME stream number (a duplicate) must NOT emit that
// number twice — the second occurrence is demoted to a synthesized free
// number instead of colliding. A HashSet keeps the skip loop O(1) so the
// full 65535-stream space stays cheap to walk.
// Numbers already EMITTED per type. When a playbackconfig maps two distinct
// StreamInfo_IDs to the SAME number (a duplicate), the second is demoted to a
// synthesized free number, not emitted twice; the HashSet keeps the skip loop O(1) across the full 65535-stream space.
let mut used_audio: HashSet<u16> = HashSet::new();
let mut used_sub: HashSet<u16> = HashSet::new();
let mut out = Vec::with_capacity(infos.len());
@@ -117,9 +115,8 @@ fn assign_stream_numbers(
Some(n) if !used.contains(&n) => n,
_ => {
// Advance past any number already claimed via the map OR already
// emitted (dedup). The counter strictly increases and
// NUMBER_SPACE_END is fixed, so this terminates in at most 65535
// steps for any input.
// emitted (dedup). The counter strictly increases and NUMBER_SPACE_END
// is fixed, so this terminates in at most 65535 steps for any input.
while *idx < NUMBER_SPACE_END
&& (taken.contains(&(*idx as u16)) || used.contains(&(*idx as u16)))
{
+10 -12
View File
@@ -147,10 +147,8 @@ fn parse_studio_attr(xml: &[u8]) -> Option<String> {
let text = std::str::from_utf8(xml).ok()?;
let bytes = text.as_bytes();
// Anchor `studio` as a WHOLE attribute name, not a bare substring: the old
// `find("studio")` also fired on `substudio=`, `studioId=`, or the literal
// word "studio" inside some OTHER attribute's value, then read a later
// attribute's quotes as the studio value. Require a word boundary before it
// and an `=` (after optional whitespace) immediately after.
// `find("studio")` also fired on `substudio=`, `studioId=`, or "studio" inside
// another attribute's value. Require a word boundary before it and an `=` (after optional whitespace) immediately after.
let mut from = 0;
while let Some(rel) = text[from..].find("studio") {
let start = from + rel;
@@ -2990,10 +2988,10 @@ mod tests {
assert_eq!(parse_studio_attr(&[0xff, 0xfe, 0x00]), None);
}
// The `studio` anchor is a WHOLE attribute name, not a bare substring: a
// longer attribute that merely CONTAINS "studio", or the word inside some
// other attribute's value, must not be read as the studio — and the real
// `studio="..."` on the same tag must still win.
/// The `studio` anchor is a WHOLE attribute name, not a bare substring: a
/// longer attribute that merely CONTAINS "studio", or the word inside some
/// other attribute's value, must not be read as the studio — and the real
/// `studio="..."` on the same tag must still win.
#[test]
fn studio_anchor_is_a_whole_attribute_not_a_substring() {
// `substudio` / `studioId` are not `studio`.
@@ -3011,10 +3009,10 @@ mod tests {
assert_eq!(parse_studio_attr(b"<C title=\"studio ghibli\">"), None);
}
// A malformed `studio=` candidate whose quote is never closed must be
// SKIPPED, not abort the scan: a later well-formed `studio="..."` still
// resolves. Pre-fix the `?` on the missing close-quote returned None for the
// whole document, dropping the real studio.
/// A malformed `studio=` candidate whose quote is never closed must be
/// SKIPPED, not abort the scan: a later well-formed `studio="..."` still
/// resolves. Pre-fix the `?` on the missing close-quote returned None for the
/// whole document, dropping the real studio.
#[test]
fn unterminated_studio_quote_is_skipped_and_a_later_studio_resolves() {
// First `studio="` opens a double-quoted value with NO other `"` until
+6 -6
View File
@@ -70,12 +70,12 @@ pub fn parse(reader: &mut dyn SectorSource, udf: &UdfFs) -> Option<ParseResult>
Some(result)
}
// The feature playlist hint for a `dcx.xml` doc. Derives the numeric id and the
// filename from ONE parsed number so they can never disagree: `format!("{id}")`
// on a 5-digit id above u16::MAX (65536..=99999) left playlist_id `None` while
// the filename stayed `Some`, a half-hint whose two fields named different
// things. Require a valid u16 playlist number, then format the canonical
// 5-digit `NNNNN.mpls` from it.
/// The feature playlist hint for a `dcx.xml` doc. Derives the numeric id and the
/// filename from ONE parsed number so they can never disagree: `format!("{id}")`
/// on a 5-digit id above u16::MAX (65536..=99999) left playlist_id `None` while
/// the filename stayed `Some`, a half-hint whose two fields named different
/// things. Require a valid u16 playlist number, then format the canonical
/// 5-digit `NNNNN.mpls` from it.
pub(crate) fn feature_hint(text: &str) -> Option<super::FeaturePlaylistHint> {
let id = feature_playlist_id(text)?;
let playlist_id = id.parse::<u16>().ok()?;
+7 -7
View File
@@ -48,13 +48,13 @@ pub fn parse(reader: &mut dyn SectorSource, udf: &UdfFs) -> Option<ParseResult>
Some(result)
}
// The feature playlist hint for a `playlists.xml` `<playlist>` element. Surface
// its identity so title selection can prefer it over a size-inflated decoy.
// Derive the numeric id and the filename from ONE parsed number so they cannot
// disagree: keep only the digits (resisting a stray quote/space), require a
// valid u16 playlist number, and format the canonical 5-digit `NNNNN.mpls`.
// (`format!("{digits}.mpls")` on a 5-digit id above u16::MAX left playlist_id
// None while the filename stayed Some — a half-hint.)
/// The feature playlist hint for a `playlists.xml` `<playlist>` element. Surface
/// its identity so title selection can prefer it over a size-inflated decoy.
/// Derive the numeric id and the filename from ONE parsed number so they cannot
/// disagree: keep only the digits (resisting a stray quote/space), require a
/// valid u16 playlist number, and format the canonical 5-digit `NNNNN.mpls`.
/// (`format!("{digits}.mpls")` on a 5-digit id above u16::MAX left playlist_id
/// None while the filename stayed Some — a half-hint.)
fn feature_hint(feature: &str) -> Option<super::FeaturePlaylistHint> {
let id = super::xml::attr(feature, "id")?;
let digits: String = id.chars().filter(|c| c.is_ascii_digit()).collect();
+6 -8
View File
@@ -154,10 +154,9 @@ pub fn parse(data: &[u8]) -> Result<Playlist> {
// UO_mask_table(8) + misc flags(1) + still_mode(1) + still_time(2), then
// STN_table starting at STN_OFFSET.
const STN_OFFSET: usize = 32;
// `>=`, not `>`: the 16-byte STN header spans item[STN_OFFSET..STN_OFFSET+16],
// so item.len() == STN_OFFSET + 16 already holds every header byte. The
// strict `>` demanded one extra byte and skipped a play item whose STN
// header ends exactly at the item boundary (a stream-less STN table).
// `>=`, not `>`: the 16-byte STN header spans item[STN_OFFSET..STN_OFFSET+16], so
// item.len() == STN_OFFSET + 16 already holds every header byte. Strict `>` demanded
// one extra byte, skipping a play item whose STN header ends exactly at the boundary (stream-less STN table).
if item_idx == 0 && item.len() >= STN_OFFSET + 16 {
// STN header: length(2) + reserved(2) + counts(8) + reserved(4) = 16 bytes
let n_video = item[STN_OFFSET + 4] as usize;
@@ -739,10 +738,9 @@ mod tests {
assert!(!s.secondary);
}
// The STN gate is inclusive of the boundary: a first play item whose STN
// table is JUST the 16-byte header (no stream entries) has item.len() ==
// STN_OFFSET(32) + 16 == 48 exactly, and must be parsed, not skipped by a
// strict `>` that demands one extra byte.
// The STN gate is inclusive of the boundary: a first play item whose STN table is
// JUST the 16-byte header (no stream entries) has item.len() == STN_OFFSET(32)+16 ==
// 48 exactly, and must be parsed, not skipped by a strict `>` demanding one extra byte.
#[test]
fn stn_header_ending_exactly_at_the_item_boundary_is_parsed() {
let data = build_mpls(&[(b"00001", 1, 0, 9000000)], (0, 0, 0, 0, 0, 0, 0, 0), &[]);
+7 -7
View File
@@ -199,13 +199,13 @@ mod tests {
assert_eq!(t.dropped_frames(), TRACK_VERDICT_MIN_AUS * 3);
}
// Drop-gate mismatch: the two counters disagree. A MAJORITY of the track's
// AUs are dropped (>50%), yet only a MINORITY are individually verified
// undecodable — the rest are collateral. The poison gate keys on the
// VERIFIED subset, so this track must survive. The collateral bulk is
// recorded FIRST, so if the gate mistakenly judged on the raw `dropped`
// count it would poison as soon as a verified drop pushed it past the
// minimum; keying on `verified_dropped` it never does.
/// Drop-gate mismatch: the two counters disagree. A MAJORITY of the track's
/// AUs are dropped (>50%), yet only a MINORITY are individually verified
/// undecodable — the rest are collateral. The poison gate keys on the
/// VERIFIED subset, so this track must survive. The collateral bulk is
/// recorded FIRST, so if the gate mistakenly judged on the raw `dropped`
/// count it would poison as soon as a verified drop pushed it past the
/// minimum; keying on `verified_dropped` it never does.
#[test]
fn a_verified_minority_amid_a_dropped_majority_does_not_poison() {
let mut t = DropTally::new("test");
+19 -25
View File
@@ -316,12 +316,9 @@ impl CodecParser for DtsParser {
let au_src = self.front_source();
self.emit_or_drop(au, au_pts, dur_ns, au_src, &mut frames);
self.drain_front(au_end);
// After draining, the marker covering the new front (if any) carries
// the next AU's PTS; `pending_pts` is only the fallback when no
// marker survives. Track it so the fallback stays sensible. A fully
// drained buffer has no live front, so the mark still "covering"
// offset 0 belongs to the AU just emitted — invalidate rather than
// keep that stale PTS as a fallback for a later PTS-less PES.
// After draining, the mark over the new front carries the next AU's PTS;
// `pending_pts` is the fallback when none survives. A fully drained buffer
// has no live front, so its offset-0 mark is the just-emitted AU's — invalidate rather than keep that stale PTS as a fallback for a later PTS-less PES.
self.pending_pts = if self.acc.is_empty() {
PTS_UNSET
} else {
@@ -373,10 +370,9 @@ impl DtsParser {
self.acc.clear();
return Vec::new();
}
// A trailing core sync at EOS opens a NEW access unit whose frame never
// completed — a complete next core would have closed this AU during
// parse(), so anything left here is truncated. Drop it rather than
// splicing it onto the final AU; real trailing extensions are kept.
// A trailing core sync at EOS opens a NEW access unit that never completed
// (a complete next core would have closed this AU during parse()), so it is
// truncated — drop it rather than splice onto the final AU; real trailing extensions are kept.
let emit_end = final_au_end(self.acc.as_slice(), core_size);
// The final AU's PTS is the PES covering the buffer front (its core's
// PES). Fall back to pending_pts, clamping the sentinel to 0.
@@ -391,12 +387,12 @@ impl DtsParser {
}
}
// Offset where the FINAL access unit ends at end-of-stream, dropping a
// truncated trailing core. Trailing DTS-HD extension substreams belong to this
// AU and are skipped precisely (kept); a trailing CORE sync — complete or
// truncated — begins a NEW AU that never closed during `parse()`, so the AU
// ends there. A tail too short to identify, or non-sync garbage, is kept with
// the AU. See docs/dts.md.
/// Offset where the FINAL access unit ends at end-of-stream, dropping a
/// truncated trailing core. Trailing DTS-HD extension substreams belong to this
/// AU and are skipped precisely (kept); a trailing CORE sync — complete or
/// truncated — begins a NEW AU that never closed during `parse()`, so the AU
/// ends there. A tail too short to identify, or non-sync garbage, is kept with
/// the AU. See docs/dts.md.
fn final_au_end(buf: &[u8], core_size: usize) -> usize {
let mut pos = core_size;
loop {
@@ -749,10 +745,10 @@ mod tests {
}
}
// A PTS-less PES must continue the timeline from the most recent known base,
// never snap it back to 0. `pending_pts` going PTS_UNSET (e.g. after a forced
// flush) previously fell straight to 0; the fallback must instead reach for
// the projected next-AU start, then the last PES front.
/// A PTS-less PES must continue the timeline from the most recent known base,
/// never snap it back to 0. `pending_pts` going PTS_UNSET (e.g. after a forced
/// flush) previously fell straight to 0; the fallback must instead reach for
/// the projected next-AU start, then the last PES front.
#[test]
fn continuation_base_prefers_a_known_projection_over_zero() {
let mut parser = DtsParser::new();
@@ -960,11 +956,9 @@ mod tests {
#[test]
fn truncated_trailing_core_is_dropped_at_eos() {
// A complete core is held awaiting a next core, but the stream cuts off
// mid-way into that next core (only its sync + a couple bytes arrived —
// too few to size, so it's held, not emitted, during parse). At EOS the
// truncated trailing core must be DROPPED, not spliced onto the final
// AU: the emitted unit is exactly the complete 512-byte core.
// A complete core is held awaiting a next core, but the stream cuts off mid-way
// into that next core (only sync + 2 bytes arrived — too few to size, so held not
// emitted during parse). At EOS that truncated trailing core is DROPPED, not spliced onto the final AU: the emitted unit is exactly the complete 512-byte core.
let mut parser = DtsParser::new();
let mut stream = make_dts_core(512);
// Truncated trailing core: sync + 2 bytes (< CORE_HEADER_MIN_BYTES).
+9 -18
View File
@@ -208,11 +208,9 @@ impl CodecParser for H264Parser {
let mut keyframe = false;
// Picture coding type, MEASURED from the first coded slice's header.
let mut coding_type: Option<CodingType> = None;
// Open-GOP promotion needs EVERY VCL slice to be intra, not just the
// first: a picture whose first slice is I but whose later slices are
// P/B is not a clean random-access point. `all_vcl_intra` stays true
// only while every slice header parses AND codes as I/SI; `saw_vcl`
// guards against promoting a param-set-only access unit.
// Open-GOP promotion needs EVERY VCL slice intra, not just the first: a picture
// whose first slice is I but later slices are P/B is not a clean random-access
// point. `all_vcl_intra` holds only while every slice header parses AND codes as I/SI; `saw_vcl` guards against promoting a param-set-only access unit.
let mut all_vcl_intra = true;
let mut saw_vcl = false;
// Did this access unit already carry each param-set type in-band?
@@ -250,11 +248,9 @@ impl CodecParser for H264Parser {
if nal_type == NAL_SLICE_IDR {
keyframe = true;
}
// Measure coding type from each slice header (§7.3.3:
// first_mb_in_slice, slice_type, both ue(v)) after unescaping
// EBSP (§7.3.1) — large first_mb_in_slice needs escaped 0x00 0x00.
// The FIRST slice sets the picture's `coding_type`; EVERY slice
// feeds `all_vcl_intra` for the open-GOP promotion below.
// Measure coding type from each slice header (§7.3.3: first_mb_in_slice,
// slice_type, both ue(v)) after unescaping EBSP (§7.3.1) — large
// first_mb_in_slice needs escaped 0x00 0x00. The FIRST slice sets the picture's `coding_type`; EVERY slice feeds `all_vcl_intra` for the open-GOP promotion below.
if nal_type == NAL_SLICE_NON_IDR || nal_type == NAL_SLICE_IDR {
saw_vcl = true;
let header = unescape_ebsp_prefix(&nal[1..]);
@@ -288,14 +284,9 @@ impl CodecParser for H264Parser {
return Vec::new();
}
// Open-GOP resync anchor: BD titles use open GOPs whose random-access
// point is a non-IDR I-frame. H.264 signals such a recovery point with a
// recovery_point SEI, which this parser does NOT read; instead it infers
// the anchor from the slices themselves. Without treating it as a
// keyframe, the resync gate (`mux/resync.rs`) can miss it and drop frames
// to EOF. Promote only when EVERY VCL slice is intra (a picture with a
// P/B slice is not a random-access point) and only on the base view
// (`!mvc`): a dependent MVC view is never an independent anchor.
// Open-GOP resync anchor: BD titles use open GOPs whose random-access point is a
// non-IDR I-frame. H.264 signals it with a recovery_point SEI, which this parser
// does NOT read — it infers the anchor from the slices. Without treating it as a keyframe the resync gate (`mux/resync.rs`) can miss it and drop frames to EOF. Promote only when EVERY VCL slice is intra (a picture with a P/B slice is not a random-access point) and only on the base view (`!mvc`): a dependent MVC view is never an independent anchor.
if saw_vcl && all_vcl_intra && !mvc {
keyframe = true;
}
+15 -15
View File
@@ -67,12 +67,12 @@ fn hevc_pps_id(pps_nal: &[u8]) -> Option<u32> {
BitReader::new(pps_nal.get(2..)?).read_ue()
}
// Measures the coding type from the FIRST coded slice of an access unit
// (H.265 §7.3.6.1). `None` for a non-first slice or on truncation — never a
// guess. `resolve_num_extra` maps the slice's OWN `slice_pic_parameter_set_id`
// to that PPS's `num_extra_slice_header_bits`, so the `slice_type` bit offset is
// taken from the PPS the slice references — not merely the last-active one. See
// docs/hevc.md — hevc_first_slice_coding_type.
/// Measures the coding type from the FIRST coded slice of an access unit
/// (H.265 §7.3.6.1). `None` for a non-first slice or on truncation — never a
/// guess. `resolve_num_extra` maps the slice's OWN `slice_pic_parameter_set_id`
/// to that PPS's `num_extra_slice_header_bits`, so the `slice_type` bit offset is
/// taken from the PPS the slice references — not merely the last-active one. See
/// docs/hevc.md — hevc_first_slice_coding_type.
fn hevc_first_slice_coding_type(
nal: &[u8],
nal_type: u8,
@@ -112,10 +112,10 @@ pub struct HevcParser {
cur_vps: Option<Vec<u8>>,
cur_sps: Option<Vec<u8>>,
cur_pps: Option<Vec<u8>>,
// Every PPS seen, keyed by its `pps_pic_parameter_set_id`, so a slice's
// coding type is measured with the `num_extra_slice_header_bits` of the PPS
// it REFERENCES rather than the last-active one (multiple PPS with differing
// values would otherwise shift the slice_type bit offset).
/// Every PPS seen, keyed by its `pps_pic_parameter_set_id`, so a slice's
/// coding type is measured with the `num_extra_slice_header_bits` of the PPS
/// it REFERENCES rather than the last-active one (multiple PPS with differing
/// values would otherwise shift the slice_type bit offset).
pps_by_id: std::collections::HashMap<u32, Vec<u8>>,
// Splice-aware CRA→BLA rewrite for a non-seamless BD clip boundary (first
// CRA_NUT -> BLA_W_LP so NoRaslOutput discards dangling RASL). Armed by
@@ -1350,11 +1350,11 @@ mod tests {
);
}
// A slice must be measured with the num_extra_slice_header_bits of the PPS
// it REFERENCES (slice_pic_parameter_set_id), not the last-active PPS. Two
// PPS with different num_extra: the slice points at the FIRST while the
// second is active. Assuming the active PPS reads slice_type at the wrong
// offset (None here); resolving by the slice's own pps id reads it right.
/// A slice must be measured with the num_extra_slice_header_bits of the PPS
/// it REFERENCES (slice_pic_parameter_set_id), not the last-active PPS. Two
/// PPS with different num_extra: the slice points at the FIRST while the
/// second is active. Assuming the active PPS reads slice_type at the wrong
/// offset (None here); resolving by the slice's own pps id reads it right.
#[test]
#[allow(clippy::unusual_byte_groupings)]
fn slice_coding_type_uses_the_pps_the_slice_references_not_the_active_one() {
+3 -4
View File
@@ -277,10 +277,9 @@ mod tests {
#[test]
fn pts_less_pes_carries_last_timestamp_and_propagates_discontinuity() {
// A PES with no PTS (legal for audio, e.g. after a discontinuity) must
// carry the last known timestamp forward — resetting to 0 would corrupt
// A/V sync — and the emitted unit must reflect the PES discontinuity
// flag rather than a hardcoded false. Mirrors the adts/flac guards.
// A PES with no PTS (legal for audio, e.g. after a discontinuity) must carry the
// last known timestamp forward — resetting to 0 would corrupt A/V sync — and the
// emitted unit must reflect the PES discontinuity flag, not a hardcoded false. Mirrors the adts/flac guards.
let mut parser = LpcmParser::new();
let header = vec![0x00, 0x01, 0x00, 0b1001_0001];
let pcm = vec![0x11, 0x22, 0x33, 0x44];
+20 -30
View File
@@ -652,9 +652,8 @@ pub struct DemuxSink {
/// flows through the timeline but does NOT count here. The gate in `finish()`
/// pairs this with `timeline.dropped_for(persisted_tracks)` so numerator and
/// denominator cover the SAME set of tracks: were it "frames the timeline
/// placed" instead, a filtered-out track's clip-join drops would be measured
/// against a written count that never included them, and a legitimate
/// filtered export could wrongly trip `SeamPlanDroppedMost`/`SinkWroteNothing`.
/// placed", a filtered-out track's clip-join drops would measure against a
/// count that never included them, wrongly tripping `SeamPlanDroppedMost`/`SinkWroteNothing`.
frames_mapped: u64,
}
@@ -876,11 +875,9 @@ impl Stream for DemuxSink {
self.ref_first_pts_ns.get_or_insert(pts);
}
if let Some(Some(t)) = self.tracks.get_mut(frame.track) {
// Count the denominator only for frames that are actually persisted.
// A kind-filtered frame (e.g. video during an `audio://` export)
// maps successfully but writes to no track file, so counting it
// here would mask a seam plan that dropped every *persisted* frame
// and let the sink ship zero-byte files at exit 0.
// Denominator = persisted frames only. A kind-filtered frame (video
// in an `audio://` export) maps but writes no file; counting it masks
// a seam plan that dropped every *persisted* frame → zero-byte exit 0.
self.frames_mapped = self.frames_mapped.saturating_add(1);
t.first_pts_ns.get_or_insert(pts);
t.writer.write_frame(&mut t.w, frame, pts)?;
@@ -893,16 +890,9 @@ impl Stream for DemuxSink {
return Ok(());
}
self.finished = true;
// Same reporting as the MKV muxer's finish: an unexpected drop volume is
// how a demux quietly comes up short, so surface it here too.
//
// Count drops ONLY for tracks this export actually persists, matching the
// `frames_mapped` denominator (which counts persisted frames only). A
// filtered export (`audio://`/`sub://`, or a `--select` subset) drops the
// video track's clip-join tails through the timeline without writing a
// file for it; folding those into the numerator via `dropped_total()`
// wrongly tripped `SeamPlanDroppedMost`/`SinkWroteNothing` on an
// otherwise-complete rip.
// Same as the MKV muxer's finish: surface unexpected drop volume. Count
// drops ONLY for persisted tracks (matches the `frames_mapped` denominator;
// see its doc for why filtered exports must not fold non-persisted drops in).
let persisted_tracks: Vec<usize> = self
.tracks
.iter()
@@ -1725,11 +1715,11 @@ mod tests {
let _ = std::fs::remove_dir_all(&dir);
}
// A demux export where the seam plan drops MORE frames than it persists
// must report SeamPlanDroppedMost — even though at least one frame did
// persist (frames_mapped > 0). The denominator counts persisted frames
// only, so a mostly-emptied export can no longer slip past as success.
// See docs/demux-sink.md — a_demux_export_the_seam_plan_mostly_emptied_fails.
/// A demux export where the seam plan drops MORE frames than it persists
/// must report SeamPlanDroppedMost — even though at least one frame did
/// persist (frames_mapped > 0). The denominator counts persisted frames
/// only, so a mostly-emptied export can no longer slip past as success.
/// See docs/demux-sink.md — a_demux_export_the_seam_plan_mostly_emptied_fails.
#[test]
fn a_demux_export_the_seam_plan_mostly_emptied_fails() {
let dir = tempdir();
@@ -1789,13 +1779,13 @@ mod tests {
let _ = std::fs::remove_dir_all(&dir);
}
// A FILTERED export (`audio://`) must NOT trip the drop gates on drops that
// belong to a track it never persists. The video track is dropped hard at
// the clip join (2 frames, all outside the marks), while the persisted audio
// track loses nothing. `dropped_total()` would fold the video drops (2) into
// the numerator and, exceeding the persisted count (1), wrongly report
// SeamPlanDroppedMost; `dropped_for(persisted_tracks)` sees 0 and passes.
// See docs/demux-sink.md — a_filtered_export_ignores_drops_on_filtered_tracks.
/// A FILTERED export (`audio://`) must NOT trip the drop gates on drops that
/// belong to a track it never persists. The video track is dropped hard at
/// the clip join (2 frames, all outside the marks), while the persisted audio
/// track loses nothing. `dropped_total()` would fold the video drops (2) into
/// the numerator and, exceeding the persisted count (1), wrongly report
/// SeamPlanDroppedMost; `dropped_for(persisted_tracks)` sees 0 and passes.
/// See docs/demux-sink.md — a_filtered_export_ignores_drops_on_filtered_tracks.
#[test]
fn a_filtered_export_ignores_drops_on_non_persisted_tracks() {
let dir = tempdir();
+6 -10
View File
@@ -232,11 +232,9 @@ impl<W: Write> M2tsMux<W> {
if self.audio.is_none() {
return Ok(());
}
// Audio seeds the shared origin too. Seeding only on video meant a leading
// audio frame (audio before the first video) fell back to `base =
// unwrap_or(raw_90k)` — its OWN pts — collapsing every pre-video audio frame
// onto PTS 0 and losing the spacing between them. `get_or_insert` still lets
// the FIRST frame (audio or video) fix the origin, so nothing is pulled up.
// Audio seeds the shared origin too. Video-only seeding made a leading audio
// frame fall back to `base = unwrap_or(raw_90k)` (its OWN pts), collapsing all
// pre-video audio onto PTS 0; `get_or_insert` still lets the FIRST frame fix it.
let pts_90k = self.base_relative_pts(pts_ns, /* may_seed_base */ true);
let pes = build_audio_pes(pts_90k, data);
self.write_pes(PID_AUDIO, &pes, None, false)
@@ -1431,11 +1429,9 @@ mod tests {
#[test]
fn pre_video_audio_frames_keep_their_spacing() {
// Two audio frames arrive before any video. The FIRST frame (audio) seeds
// the shared origin, so the second audio frame keeps its 1s offset. The OLD
// code seeded the origin only on video, so every pre-video audio frame fell
// back to `base = unwrap_or(raw_90k)` — its own pts — collapsing BOTH onto
// PTS 0 and losing the 1s spacing.
// Two audio frames before any video. The FIRST (audio) seeds the shared origin,
// so the second keeps its 1s offset. OLD code seeded only on video → both fell
// back to `base = unwrap_or(raw_90k)` (own pts), collapsing onto PTS 0, losing 1s.
let decode_audio_pts = |pkt: &[u8]| -> u64 {
let afc = (pkt[3] >> 4) & 0x03;
let pes_start = if afc & 0b10 != 0 {
+6 -11
View File
@@ -109,22 +109,17 @@ impl Stream for ChaptersSink {
// See docs/meta-sink.md — title_json schema.
pub(crate) fn title_json(title: &DiscTitle) -> serde_json::Value {
use serde_json::json;
// `index`/`is_main` are disc-level facts (a title's position in the sorted
// title list, and whether it is the selected main feature). A `json://` sink
// is handed ONE `DiscTitle` with no view of the disc, so neither is knowable
// here: this document describes a single title in isolation. They are fixed
// at `0`/`false` deliberately — the authoritative, derived values live on the
// disc-level `DiscProfile::from_disc` path, not on this per-title export.
// `index`/`is_main` are disc-level (title's position in the sorted list, whether
// it's the selected main feature). A `json://` sink sees ONE `DiscTitle` with no
// disc view, so both are fixed `0`/`false`; authoritative values live on `DiscProfile::from_disc`.
let profile = TitleProfile::from_title(title, 0, false);
// Serializing a plain-scalar struct is infallible; fall back to an empty
// object rather than panic if that ever changes (the create() path then
// surfaces the empty doc as a NoMetadata error).
let mut doc = serde_json::to_value(&profile).unwrap_or_else(|_| json!({}));
// Restore the per-stream detail the normalized `TitleProfile` omits.
// `TitleProfile` is a deliberately flat VIEW with no field for pid, colour
// signalling, sample rate, raw purpose/qualifier, etc.; rather than widen
// that shared type, enrich this document straight from the source Title,
// walking each kind in the same order `from_title` split them.
// Restore per-stream detail the flat `TitleProfile` VIEW omits (pid, colour
// signalling, sample rate, raw purpose/qualifier, etc.). Rather than widen that
// shared type, enrich straight from the source Title, in `from_title`'s kind order.
enrich_streams(&mut doc, title);
doc["format"] = json!(format!("{:?}", title.content_format));
doc["playlist_id"] = json!(title.playlist_id);
+4 -4
View File
@@ -2481,10 +2481,10 @@ mod tests {
);
}
// FlagInterlaced is written up-front from the FIRST picture, then corrected
// at finish() to the WHOLE-STREAM scan majority. A progressive leader on an
// interlaced feature must not misrepresent the title: the header byte is
// rewritten from progressive (2) to interlaced (1).
/// FlagInterlaced is written up-front from the FIRST picture, then corrected
/// at finish() to the WHOLE-STREAM scan majority. A progressive leader on an
/// interlaced feature must not misrepresent the title: the header byte is
/// rewritten from progressive (2) to interlaced (1).
#[test]
fn flag_interlaced_is_patched_to_the_whole_stream_scan_majority() {
use std::sync::{Arc, Mutex};
+3 -5
View File
@@ -2953,11 +2953,9 @@ mod tests {
#[test]
fn info_child_size_exceeding_remaining_is_rejected() {
// A crafted INFO whose sole child (TITLE) declares a 1000-byte body while
// the INFO parent is sized to just the child header: header+body overruns
// the parent's `remaining`. The old code saturated `remaining` to 0 and
// fell through to read the oversized child (an EOF/garbage read); the guard
// must instead reject it as MkvSourceInvalid.
// Crafted INFO whose sole child (TITLE) declares a 1000-byte body while the INFO
// parent is sized to just the child header, so header+body overruns `remaining`.
// Old code saturated it to 0 and read the oversized child (EOF/garbage); the guard rejects as MkvSourceInvalid.
let mut info = Vec::new();
ebml::write_id(&mut info, ebml::TITLE).unwrap();
ebml::write_size(&mut info, 1000).unwrap(); // declares 1000 bytes, provides none
+18 -23
View File
@@ -103,11 +103,9 @@ impl<R: Read + Seek> Mp4Reader<R> {
// (8-byte) trak headers can't force the scan to materialize a Vec far
// larger than the moov payload before the per-track cap below ever runs.
for trak in find_boxes_capped(&moov, b"trak", MAX_TRACKS) {
// `find_boxes_capped` already yields at most MAX_TRACKS matches, so
// `track_idx` can never reach MAX_TRACKS inside this loop — the old
// runtime `break` was unreachable. Assert the invariant instead (a lower
// cap slipping in would trip this in debug) so the per-track PID stays
// within u16 without dead control flow.
// `find_boxes_capped` yields at most MAX_TRACKS matches, so `track_idx`
// never reaches MAX_TRACKS here (old runtime `break` was dead). Assert the
// invariant instead so the per-track PID stays within u16 without dead flow.
debug_assert!(track_idx < MAX_TRACKS, "trak scan exceeded MAX_TRACKS");
let Some(mdia) = find_box(trak, b"mdia") else {
tracing::warn!(track = track_idx, "mp4: trak has no mdia, dropping track");
@@ -471,11 +469,9 @@ fn find_boxes_capped<'a>(payload: &'a [u8], want: &[u8; 4], cap: usize) -> Vec<&
0 => (payload.len() - pos, 8usize),
n => (n, 8usize),
};
// checked_add: `box_size` is an attacker-controlled 64-bit largesize cast
// to usize, so a value near usize::MAX at a nonzero `pos` would wrap past
// the guard on a plain `pos + box_size` — producing a release panic on the
// slice or an infinite loop on the advance. Compute the end once and reuse
// it for the bounds check, the slice, and the advance.
// checked_add: `box_size` is an attacker-controlled 64-bit largesize cast to
// usize; near usize::MAX at nonzero `pos`, plain `pos + box_size` wraps past the
// guard (release panic on slice / infinite advance). Compute `end` once, reuse it.
let end = match pos.checked_add(box_size) {
Some(end) if box_size >= header_len && end <= payload.len() => end,
_ => break,
@@ -738,10 +734,9 @@ fn parse_stsd(b: &[u8]) -> Option<StsdInfo> {
// its AudioSpecificConfig in an `esds` box — the MKV CodecPrivate for
// A_AAC. AC-3/DTS are self-describing in-band (None).
let channels = if body.len() >= 28 { be16(body, 16) } else { 2 };
// The real audio sample rate lives in the AudioSampleEntry 16.16
// samplerate field at body offset 24 (ISO/IEC 14496-12 §12.2.3); the
// integer rate is the high 16 bits. The mdhd media timescale is only a
// fallback (it is USUALLY the sample rate, but need not be).
// Real audio sample rate: AudioSampleEntry 16.16 samplerate field at body
// offset 24 (ISO/IEC 14496-12 §12.2.3), integer rate = high 16 bits. The mdhd
// media timescale is only a fallback (USUALLY the rate, but need not be).
let sample_rate = if body.len() >= 28 {
be16(body, 24) as u32
} else {
@@ -2257,11 +2252,11 @@ mod tests {
);
}
// Regression: audio sample rate must come from the AudioSampleEntry 16.16
// samplerate field, NOT the mdhd media timescale. Here the two DIFFER
// (mdhd timescale = 90_000, AudioSampleEntry samplerate = 48_000); the
// entry value must win. Old code read the rate from the timescale, so a
// 90 kHz mdhd yielded SampleRate::Unknown, dropping the real 48 kHz.
/// Regression: audio sample rate must come from the AudioSampleEntry 16.16
/// samplerate field, NOT the mdhd media timescale. Here the two DIFFER
/// (mdhd timescale = 90_000, AudioSampleEntry samplerate = 48_000); the
/// entry value must win. Old code read the rate from the timescale, so a
/// 90 kHz mdhd yielded SampleRate::Unknown, dropping the real 48 kHz.
#[test]
fn audio_sample_entry_samplerate_wins_over_mdhd_timescale() {
use std::io::Cursor;
@@ -3156,10 +3151,10 @@ mod tests {
);
}
// A crafted 64-bit `largesize` near u64::MAX at a nonzero `pos` must not
// wrap `pos + box_size` past the length guard: the scan must reject it and
// return without a release panic (slice) or an infinite loop (advance).
// See docs/mp4-read.md — find_boxes_capped_rejects_a_hostile_largesize.
/// A crafted 64-bit `largesize` near u64::MAX at a nonzero `pos` must not
/// wrap `pos + box_size` past the length guard: the scan must reject it and
/// return without a release panic (slice) or an infinite loop (advance).
/// See docs/mp4-read.md — find_boxes_capped_rejects_a_hostile_largesize.
#[test]
fn find_boxes_capped_rejects_a_hostile_largesize_near_u64_max() {
let mut payload = Vec::new();
+8 -9
View File
@@ -42,10 +42,9 @@ pub(crate) fn is_blocked_ip(ip: IpAddr) -> bool {
}
IpAddr::V6(v6) => {
let seg = v6.segments();
// 6to4 (2002::/16) embeds an IPv4 in segments[1..3]; Teredo
// (2001:0000::/32) embeds the client IPv4 in the last two segments,
// each XOR 0xffff. Both must be re-checked as their embedded IPv4 or
// an internal target slips through the tunnel.
// 6to4 (2002::/16) embeds an IPv4 in segments[1..3]; Teredo (2001:0000::/32)
// embeds the client IPv4 in the last two segments, each XOR 0xffff. Re-check
// both as their embedded IPv4 or an internal target slips through the tunnel.
let sixtofour = (seg[0] == 0x2002)
.then(|| std::net::Ipv4Addr::from(((seg[1] as u32) << 16) | (seg[2] as u32)));
let teredo = (seg[0] == 0x2001 && seg[1] == 0x0000).then(|| {
@@ -378,11 +377,11 @@ mod tests {
))));
}
// The NAT64 well-known prefix 64:ff9b::/96 (RFC 6052) embeds an IPv4 in its
// last 32 bits; the guard must decode and re-check that embedded IPv4 so an
// internal target does not slip through a NAT64 translator — parity with
// keysources. A NAT64 address embedding a private/special IPv4 must classify
// exactly as that IPv4 does directly.
/// The NAT64 well-known prefix 64:ff9b::/96 (RFC 6052) embeds an IPv4 in its
/// last 32 bits; the guard must decode and re-check that embedded IPv4 so an
/// internal target does not slip through a NAT64 translator — parity with
/// keysources. A NAT64 address embedding a private/special IPv4 must classify
/// exactly as that IPv4 does directly.
#[test]
fn ssrf_guard_blocks_embedded_ipv4_via_nat64() {
use std::net::{Ipv4Addr, Ipv6Addr};
+3 -5
View File
@@ -532,11 +532,9 @@ where
// Bounded, file-backed probe; no live drive here.
None,
)
// Non-fatal: a failed key-map resolution leaves the probe
// without FMTS ranges, so TrueHD channel correction is
// skipped (MPLS 7.1/Atmos stays understated as 5.1). Log the
// actual error — same level/target as the sibling re-open
// failure below — instead of swallowing it with a bare `.ok()`.
// Non-fatal: a failed key-map resolution leaves the probe without
// FMTS ranges, so TrueHD channel correction is skipped (MPLS 7.1/Atmos
// stays understated as 5.1). Log the actual error (parity with the sibling re-open failure below), not a bare `.ok()`.
.inspect_err(|e| {
tracing::debug!(
target: "mux",
+17 -17
View File
@@ -432,17 +432,17 @@ pub(crate) struct TimelineContinuity {
// (distinct from the shared frontier). See
// docs/mux-timeline.md#timelinecontinuitylast_raw_ns.
last_raw_ns: Vec<Option<i64>>,
// Per-track provisional offset for frames arriving before the video
// frame that opens their epoch, tagged with the epoch sequence it was
// captured in (see `epoch_seq`). Never written to offset_ns/high_ns and
// never retires an epoch. See docs/mux-timeline.md#timelinecontinuityprovisional.
/// Per-track provisional offset for frames arriving before the video
/// frame that opens their epoch, tagged with the epoch sequence it was
/// captured in (see `epoch_seq`). Never written to offset_ns/high_ns and
/// never retires an epoch. See docs/mux-timeline.md#timelinecontinuityprovisional.
provisional: Vec<Option<(u64, i64)>>,
// Monotonic epoch counter, bumped once per `open_epoch`. Used as the epoch
// IDENTITY a provisional is tagged with, instead of `epoch_offsets.len()`:
// that length is capped at MAX_EPOCHS, so past the cap it stops changing and
// a provisional captured then would compare equal forever and never retire —
// outliving its epoch and mis-offsetting later passive frames. A monotonic
// counter always advances. (Same root as the disc.rs slice-identity finding.)
/// Monotonic epoch counter, bumped once per `open_epoch`. Used as the epoch
/// IDENTITY a provisional is tagged with, instead of `epoch_offsets.len()`:
/// that length is capped at MAX_EPOCHS, so past the cap it stops changing and
/// a provisional captured then would compare equal forever and never retire —
/// outliving its epoch and mis-offsetting later passive frames. A monotonic
/// counter always advances. (Same root as the disc.rs slice-identity finding.)
epoch_seq: u64,
}
@@ -495,13 +495,13 @@ impl TimelineContinuity {
self.seams.as_ref().map_or(0, |p| p.dropped_total())
}
// Frames dropped for falling outside the playlist's clip marks, counted
// ONLY over `tracks` — the numerator/denominator alignment a filtered sink
// needs. A `demux://` variant that persists a subset of tracks (e.g.
// `audio://`) counts its written denominator over just those tracks, so the
// drop count it gates on must cover the SAME set: a video track dropped at a
// clip join during an `audio://` export is not evidence the audio files came
// up short. Zero without a seam plan.
/// Frames dropped for falling outside the playlist's clip marks, counted
/// ONLY over `tracks` — the numerator/denominator alignment a filtered sink
/// needs. A `demux://` variant that persists a subset of tracks (e.g.
/// `audio://`) counts its written denominator over just those tracks, so the
/// drop count it gates on must cover the SAME set: a video track dropped at a
/// clip join during an `audio://` export is not evidence the audio files came
/// up short. Zero without a seam plan.
pub(crate) fn dropped_for(&self, tracks: &[usize]) -> u64 {
self.seams.as_ref().map_or(0, |p| {
tracks
+12 -20
View File
@@ -158,11 +158,9 @@ impl<W: Write> TsMuxer<W> {
return Ok(());
}
// Seed the global PTS origin from the FIRST frame of ANY kind, so a
// single fixed origin rebases every frame and the audio/video offset is
// preserved. Seeding on video ONLY meant each pre-video audio frame fell
// back to `base = unwrap_or(pts_ns)` — i.e. its OWN pts — collapsing every
// leading audio frame onto PTS 0 and destroying the spacing between them.
// Seed the global PTS origin from the FIRST frame of ANY kind, so one fixed
// origin rebases every frame and the a/v offset is preserved. Video-ONLY seeding
// made pre-video audio fall back to `base = unwrap_or(pts_ns)` (own pts) → PTS 0, spacing lost.
let base = *self.base_pts_ns.get_or_insert(pts_ns);
let pts_ns = pts_ns.saturating_sub(base);
@@ -754,12 +752,9 @@ mod tests {
#[test]
fn av_offset_preserved_with_audio_before_first_video() {
// Audio at t=0 arrives before the first video keyframe at t=1s. The FIRST
// frame of any kind fixes the origin, so the leading audio frame becomes
// the origin (PTS 0) and the video keyframe lands 1s = 90_000 ticks later.
// The 1s audio→video offset must survive. The OLD code seeded the origin
// from video only, so the pre-video audio fell back to its OWN pts and BOTH
// collapsed to 0, destroying the offset.
// Audio at t=0 before the first video keyframe at t=1s. The FIRST frame of any
// kind fixes the origin, so audio becomes origin (PTS 0), video lands 1s=90_000
// ticks later. OLD video-only seeding made both fall back to own pts → collapse to 0, offset lost.
let mut sink: Vec<u8> = Vec::new();
{
let mut mux = TsMuxer::new(&mut sink, &[VIDEO_PID, AUDIO_PID]);
@@ -1093,10 +1088,9 @@ mod tests {
#[test]
fn negative_relative_pts_saturates_to_zero() {
// A frame earlier than the origin (negative relative PTS) must encode
// PTS 0, never an underflowed huge value. Video keyframe at t=2s seeds
// the origin; a later-written audio frame at t=0 is 2s BEFORE it → -2s,
// which must floor to 0.
// A frame earlier than the origin (negative relative PTS) must encode PTS 0, not
// an underflowed huge value. Video keyframe at t=2s seeds the origin; a later
// audio frame at t=0 is 2s BEFORE it → -2s, which must floor to 0.
let mut sink: Vec<u8> = Vec::new();
{
let mut mux = TsMuxer::new(&mut sink, &[VIDEO_PID, AUDIO_PID]);
@@ -1117,11 +1111,9 @@ mod tests {
#[test]
fn audio_only_stream_preserves_frame_spacing() {
// Regression: an audio-only stream (no video to seed the origin) must keep
// the spacing between its frames. The FIRST audio frame seeds the origin;
// later frames rebase on it. The OLD code seeded only on video, so EVERY
// audio-only frame fell back to `base = unwrap_or(pts_ns)` — its own pts —
// and flat-lined every frame onto PTS 0.
// Regression: an audio-only stream (no video to seed the origin) must keep its
// frame spacing. The FIRST audio frame seeds the origin; later frames rebase on
// it. OLD video-only seeding made EVERY frame fall back to own pts → all on PTS 0.
let mut sink: Vec<u8> = Vec::new();
{
let mut mux = TsMuxer::new(&mut sink, &[AUDIO_PID]);
+3 -7
View File
@@ -141,13 +141,9 @@ impl PesFrame {
if len > MAX_FRAME_SIZE {
return Err(crate::error::Error::PesFrameTooLarge { size: len }.into());
}
// Grow the buffer INCREMENTALLY instead of `vec![0u8; len]`: the header's
// `len` is attacker-controlled up to MAX_FRAME_SIZE (256 MiB), so a
// truncated stream claiming a huge frame must not pre-allocate the whole
// max before we learn the bytes aren't there. Read through a bounded
// scratch chunk and append, so memory tracks bytes actually delivered; a
// short read still surfaces as UnexpectedEof, and a full valid frame
// assembles byte-for-byte.
// Grow the buffer INCREMENTALLY (not `vec![0u8; len]`): `len` is attacker-controlled
// up to MAX_FRAME_SIZE (256 MiB), so a truncated stream claiming a huge frame must not
// pre-allocate the max; a bounded scratch chunk tracks delivered bytes, short reads still surface UnexpectedEof, valid frames assemble byte-for-byte.
const GROW_CHUNK: usize = 1024 * 1024; // 1 MiB scratch, heap-allocated
let mut data: Vec<u8> = Vec::with_capacity(len.min(GROW_CHUNK));
let mut chunk = vec![0u8; GROW_CHUNK.min(len).max(1)];
+9 -17
View File
@@ -242,12 +242,9 @@ impl<S: SectorSource> SectorSource for DecryptingSectorSource<S> {
let content = self.content_ranges.clone();
let content_ref = content.as_deref();
// Defense-in-depth: AACS units are 3-sector aligned; misalignment would
// silently mis-decrypt, so reject loud (DecryptFailed) before reading.
// Gated relative to `unit_base` (per-extent), not raw `lba % 3`. Only
// enforced when the span actually TOUCHES encrypted content — a clear
// UDF/nav read outside every content range is pass-through, so a
// non-aligned filesystem LBA must not fail the whole-disc reader.
// Defense-in-depth: AACS units are 3-sector aligned; misalignment silently mis-
// decrypts, so reject loud (DecryptFailed) before reading — gated vs `unit_base`
// (not raw `lba % 3`), only when the span TOUCHES content (else UDF/nav passes through).
if matches!(self.keys, DecryptKeys::Aacs { .. })
&& span_touches_content(content_ref, lba, count)
&& !crate::aacs::content::is_unit_aligned(lba, self.unit_base)
@@ -258,10 +255,9 @@ impl<S: SectorSource> SectorSource for DecryptingSectorSource<S> {
.inner
.read_sectors_fua(lba, count, buf, recovery, fua)?;
// Proactive map path (storm-free mux): keys were resolved per unit up
// front, so decrypt with the mapped key and trust it, no per-unit
// `is_clean` check. A resolver gap fails loud; bad TS passes through.
// Units outside the content extents (when set) pass through untouched.
// Proactive map path (storm-free mux): keys were resolved per unit up front,
// so decrypt with the mapped key and trust it (no per-unit `is_clean`). A
// resolver gap fails loud; bad TS and units outside content extents pass through.
if let Some(map) = self.key_map.clone() {
crate::decrypt::decrypt_sectors_mapped_in_content(
&mut buf[..n],
@@ -273,13 +269,9 @@ impl<S: SectorSource> SectorSource for DecryptingSectorSource<S> {
return Ok(n);
}
// Decrypt `buf` in place (None / CSS / AACS); with a content map, units
// outside the encrypted extents pass through untouched. A can't-decrypt
// fails loud; broken-TS output is the muxer's concern, not a read failure.
// No map installed: CSS self-descramble / clear pass-through. A can't-
// decrypt (misalignment, or mapless AACS reaching here — a bug) fails
// loud; otherwise units pass through, same as above.
// No map installed: `decrypt_buf` does CSS self-descramble / clear pass-through
// (units outside the content extents pass through untouched). A can't-decrypt
// (misalignment, or mapless AACS here — a bug) fails loud; broken TS is the muxer's concern.
Self::decrypt_buf(&mut buf[..n], &mut self.keys, lba, content_ref)?;
Ok(n)
}
+3 -6
View File
@@ -157,12 +157,9 @@ impl PrefetchedSectorSource {
offset = 0;
continue;
}
// AACS units are SECTOR_ALIGNMENT (3) sectors; decrypt only
// processes full units, leaving a short tail encrypted. If the
// tail can't fill a unit, error instead of emitting encrypted bytes.
// `remaining > 0` is already guaranteed above, so a value below one
// unit IS the short-tail case — the `!is_multiple_of` clause was
// redundant (0 < remaining < align is never a multiple of align).
// AACS units are SECTOR_ALIGNMENT (3) sectors; decrypt processes only
// full units, so a tail below one unit can't decrypt — error, don't emit
// encrypted bytes. (`remaining > 0` above, so `< align` IS the short tail; `!is_multiple_of` was redundant.)
if remaining < unit_align as u32 {
let _ = tx.send(Err(crate::error::Error::ExtentNotUnitAligned.into()));
return;
+14 -25
View File
@@ -1125,13 +1125,9 @@ fn read_directory(
}
(icb[ad_off..ad_off + l_ad].to_vec(), l_ad as u32)
} else {
// Out-of-line: the FID list may span MULTIPLE allocation descriptors,
// and only extent_type 0 is recorded on-disc. Reading just the first AD
// (and trusting it recorded) truncated a directory whose FIDs cross an
// extent boundary, and could read a sparse/continuation descriptor's
// (length, LBA) as if it were FID data. Walk the descriptor list —
// following a type-3 continuation block just like `read_icb_extents` —
// gathering every recorded extent's sectors into one buffer.
// Out-of-line: the FID list may span MULTIPLE ADs (only extent_type 0 is FID data);
// reading just the first AD truncated dirs whose FIDs cross an extent boundary, or
// misread a sparse/type-3 descriptor's (length,LBA) as FIDs. Walk the list, gathering every extent's sectors into one buffer, following type-3 continuations like `read_icb_extents`.
let ad_size: usize = match ad_type {
0 => 8, // short_ad
1 => 16, // long_ad
@@ -1187,11 +1183,9 @@ fn read_directory(
]);
if extent_type == 3 {
// Continuation (ECMA-167 4/14.14.1.1 type 3): the REST of the
// ADs live in the block at data_lba, NOT FID data — reading
// its (length, LBA) as an extent, as the old first-AD-only
// path did, enumerates an unrelated sector. Follow the
// pointer instead (bounded by MAX_AD_BLOCKS).
// Continuation (ECMA-167 4/14.14.1.1 type 3): the REST of the ADs live
// in the block at data_lba, NOT FID data — reading its (length,LBA) as an
// extent (the old first-AD-only bug) enumerates an unrelated sector. Follow the pointer instead (bounded by MAX_AD_BLOCKS).
if data_len > 0 {
next_block = Some(data_lba);
}
@@ -1263,11 +1257,9 @@ fn read_directory(
let mut entries = Vec::new();
let mut pos = 0;
// `<=`, not `<`: a FID's fixed 38-byte header that ends exactly at the
// declared boundary (pos + 38 == dir_end) is fully present and must be
// read; the strict `<` dropped it. The name, which may run past the header,
// is bounded against `dir_end` below so a FID whose header fits but whose
// NAME spills past the declared length is still rejected.
// `<=`, not `<`: a FID's fixed 38-byte header ending exactly at the declared boundary
// (pos + 38 == dir_end) is fully present and must be read (strict `<` dropped it). The
// name may run past the header but is bounded against `dir_end` below, so a FID whose header fits but whose NAME spills past the length is still rejected.
while pos + 38 <= dir_end {
let fid_tag = u16::from_le_bytes([dir_data[pos], dir_data[pos + 1]]);
if fid_tag != 257 {
@@ -4434,11 +4426,9 @@ mod tests {
#[test]
fn read_directory_reads_fids_spanning_multiple_allocation_descriptors() {
// A directory's FID list can span MORE than one allocation descriptor
// (ECMA-167 4/14.14.1). Reading only the first AD truncated the list at
// the extent boundary; every recorded extent must be gathered and walked.
// Extent 0 (sector 60) is packed to exactly 2048 bytes so the walk flows
// straight into extent 1 (sector 61) where the second file's FID lives.
// A directory's FID list can span MORE than one AD (ECMA-167 4/14.14.1); reading
// only the first AD truncated it at the extent boundary, so every recorded extent
// must be gathered and walked. Extent 0 (sector 60) is packed to exactly 2048 bytes so the walk flows into extent 1 (sector 61), where the second file's FID lives.
let mut e0 = Vec::new();
push_fid_iu(&mut e0, "", 5, true, true, 0); // parent (..), 40 bytes
// Pad FIRST.CLPI's implementation-use area so e0 ends exactly at 2048.
@@ -4510,9 +4500,8 @@ mod tests {
#[test]
fn read_directory_reads_a_long_ad_directorys_extent() {
// ICB Tag flags = 1 selects long_ad (16 bytes): extent_length(4) +
// extent_location{ lba(4) + part_ref(2) } + impl_use(6). The LBA is at
// offset 4 (like short_ad), NOT offset 12 like extended_ad — reading the
// wrong offset walks an empty sector and the directory comes back empty.
// extent_location{ lba(4) + part_ref(2) } + impl_use(6). The LBA is at offset 4
// (like short_ad), NOT offset 12 like extended_ad — the wrong offset walks an empty sector and the directory comes back empty.
let mut fids = Vec::new();
push_fid_iu(&mut fids, "", 5, true, true, 0);
push_fid_iu(&mut fids, "INDEX.BDMV", 7, false, false, 0);
+17 -17
View File
@@ -129,10 +129,10 @@ pub(crate) fn run_features(
run(firmware_unlockers(), scsi, drive_id, fu::DiscKind::Unknown)
}
// The FIRMWARE/drive unlockers, in dispatch order. These key off the DRIVE and
// remove bus encryption AT THE DRIVE, so a VID from one of them means the drive
// is already unlocked (see `is_drive_unlocker`). Single constructor so both the
// dispatch and the name/classification helpers share one source of truth.
/// The FIRMWARE/drive unlockers, in dispatch order. These key off the DRIVE and
/// remove bus encryption AT THE DRIVE, so a VID from one of them means the drive
/// is already unlocked (see `is_drive_unlocker`). Single constructor so both the
/// dispatch and the name/classification helpers share one source of truth.
fn firmware_unlockers() -> Vec<Box<dyn fu::Unlocker>> {
vec![
Box::new(fu::FreemkvUnlocker::new()),
@@ -151,15 +151,15 @@ fn disc_unlockers(host_certs: Vec<fu::HostCert>) -> Vec<Box<dyn fu::Unlocker>> {
]
}
// Whether `name` (a matched unlocker's `.name()`) is a firmware/drive unlocker,
// i.e. one whose success means bus encryption is already removed at the drive.
// Derived from the real unlocker set, never a hardcoded name list.
//
// Test-only: the disc-keyed cert route (`run_bus`) can only ever match the disc
// unlockers (AACS/DVD), so no production path classifies a `matched` name through
// here — the AACS cert handshake credits bus removal via its `read_data_key`, and a
// genuine firmware drive-unlock is surfaced separately by the OEM-VID short-circuit.
// Retained so the disjointness invariant stays under test.
/// Whether `name` (a matched unlocker's `.name()`) is a firmware/drive unlocker,
/// i.e. one whose success means bus encryption is already removed at the drive.
/// Derived from the real unlocker set, never a hardcoded name list.
///
/// Test-only: the disc-keyed cert route (`run_bus`) can only match the disc
/// unlockers (AACS/DVD), so no production path classifies a `matched` name here —
/// the AACS cert handshake credits bus removal via its `read_data_key`, and a
/// genuine firmware drive-unlock is surfaced by the OEM-VID short-circuit. Retained
/// so the disjointness invariant stays under test.
#[cfg(test)]
pub(crate) fn is_drive_unlocker(name: &str) -> bool {
firmware_unlockers().iter().any(|u| u.name() == name)
@@ -218,10 +218,10 @@ mod tests {
.expect_err("error path")
}
// The user-facing unlocker matrix must be DERIVED from the real unlocker
// instances, never a hand-maintained list that can silently drift. Pin the
// dispatch order (firmware set, then disc set) and cross-check that every
// name comes from an actual `.name()`.
/// The user-facing unlocker matrix must be DERIVED from the real unlocker
/// instances, never a hand-maintained list that can silently drift. Pin the
/// dispatch order (firmware set, then disc set) and cross-check that every
/// name comes from an actual `.name()`.
#[test]
fn unlocker_names_are_derived_from_the_real_unlockers() {
assert_eq!(