From 0e8c31a9c3ab469fa66a98dd8ff3a729289fddfa Mon Sep 17 00:00:00 2001 From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com> Date: Thu, 6 Aug 2026 08:25:54 -0700 Subject: [PATCH] Round 3: fix three defects introduced by the round-2 fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Auditing my own fixes found all three. None were in the original code. The VTS crack sort was byte-wise case-SENSITIVE while the filters that select those files (vts_group_of / is_title_vob) are case-insensitive. On a case-sensitive volume a set holding vts_01_1.vob beside VTS_01_2.VOB sorted part 2 first, because V (0x56) precedes v (0x76) — reintroducing exactly the budget-exhaustion the ordering exists to prevent. Now sorted on the same uppercase normalisation the filters apply. Refusing a name that round-trips to empty aborted the WHOLE plan. A sidecar folder named with a single emoji made a backup un-rippable that 1.6.0 handled fine, and reported it as a collision with a file that does not exist. It is now skipped with a warning: the entry is unaddressable either way, but one irrelevant file should not cost the user their rip. The mtime check now applies only to files whose CONTENT the plan read — the IFOs, whose bytes 0xC0/0xC4 place every VOB. Everything else is planned from size alone, which is already checked, so comparing mtime there bought nothing and risked a real false positive: disc backups commonly live on exFAT/FAT32, which stores local time, so a long rip spanning a DST transition would see a whole-hour shift on an untouched multi-gigabyte VOB and abort hours in. --- src/dirimage/layout.rs | 20 ++++++++++++++++---- src/dirimage/mod.rs | 19 ++++++++++++++++++- src/disc/extract.rs | 8 +++++++- 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/dirimage/layout.rs b/src/dirimage/layout.rs index a9221f0..29fc571 100644 --- a/src/dirimage/layout.rs +++ b/src/dirimage/layout.rs @@ -233,11 +233,23 @@ fn walk(dir: &Path, disc_path: &str, depth: u32, entries: &mut usize) -> Result< // exit 0. Comparing the raw host names cannot see that; deriving the // key from the same two functions cannot drift from it. let as_read = crate::udf::parse_udf_name(&crate::dirimage::encode::encode_cs0(&name)); - // Nothing left after the reader is done with it: the entry would exist - // in the image and be unaddressable by any name. Refuse rather than - // write something no consumer can reach. + // Nothing left after the reader is done with it — a name made entirely + // of characters `parse_udf_name` drops, e.g. a sidecar folder named + // with a single emoji. The entry would exist in the image and be + // addressable by nothing. + // + // SKIP it, do not fail the plan. Failing turned an irrelevant extra + // file into an un-rippable folder that 1.6.0 handled fine (the + // unreachable entry merely sat there, harming nothing), and reported it + // as a collision with a file that does not exist. It is also not + // pushed to `names`, so it cannot collide with anything either. if as_read.is_empty() { - return Err(Error::DirNameCollision { host: child_path }); + tracing::warn!( + target: "freemkv::dirimage", + path = %child_path, + "name is unrepresentable in UDF after encoding; entry omitted from the image" + ); + continue; } names.push(as_read.to_ascii_uppercase()); if ft.is_dir() { diff --git a/src/dirimage/mod.rs b/src/dirimage/mod.rs index e8abd31..4a3013e 100644 --- a/src/dirimage/mod.rs +++ b/src/dirimage/mod.rs @@ -127,11 +127,28 @@ impl DirImage { let mut files = Vec::with_capacity(nodes.len()); let mut ranges = Vec::new(); for (idx, node) in nodes.iter().enumerate() { + // Carry the plan-time mtime ONLY for files whose CONTENT the plan + // read — the DVD IFOs, whose bytes 0xC0/0xC4 decide where every VOB + // is placed (`layout::place_video_ts` -> `read_head`). + // + // For every other file the plan depends on the SIZE alone, and size + // is already checked. Comparing mtime on those buys nothing and + // costs real false positives: disc backups commonly live on + // exFAT/FAT32, which stores local time, so a long rip spanning a + // DST transition sees a whole-hour shift on a file nobody touched + // and would abort hours in, blaming a change that did not happen. + // The multi-gigabyte VOBs are exactly the files a long rip re-opens + // after the handle cache evicts them. + let content_sensitive = node + .disc_path + .rsplit('.') + .next() + .is_some_and(|e| e.eq_ignore_ascii_case("IFO")); files.push(FileRef { host: node.host.clone(), disc_path: node.disc_path.clone(), size: node.size, - mtime: node.mtime, + mtime: content_sensitive.then_some(node.mtime).flatten(), }); let mut offset = 0u64; for e in &node.extents { diff --git a/src/disc/extract.rs b/src/disc/extract.rs index ab9dd45..73478f2 100644 --- a/src/disc/extract.rs +++ b/src/disc/extract.rs @@ -318,7 +318,13 @@ impl Disc { vts_group_of(&pf.disc_name).as_deref() == Some(vts) && is_title_vob(&pf.disc_name) }) .collect(); - files.sort_by(|a, b| a.disc_name.cmp(&b.disc_name)); + // Case-INSENSITIVE, matching `vts_group_of`/`is_title_vob`, which + // selected these files case-insensitively. A byte-wise sort put + // 'V' (0x56) before 'v' (0x76), so a set holding `vts_01_1.vob` + // alongside `VTS_01_2.VOB` — reachable on a case-sensitive volume — + // started the crack at part 2 and could exhaust the budget in a clear + // run, which is the whole failure this ordering exists to prevent. + files.sort_by_key(|f| f.disc_name.to_ascii_uppercase()); let mut extents: Vec = Vec::new(); for pf in files {