Catch names the reader cannot tell apart, and in-place content changes
Two dirimage findings, both silent-wrong-output. The per-directory uniqueness check compared raw host names, but the reader does not see raw host names: parse_udf_name trims leading and trailing whitespace and drops any code unit char::from_u32 rejects — which is every half of the surrogate pairs the encoder emits for non-BMP characters. So " 00000.m2ts" and "00000.m2ts", or "A<astral>.m2ts" and "A.m2ts", were two entries at plan time and ONE name at read time. find/read_file take the first match, so a title resolved to the wrong file extents and muxed the wrong bytes at exit 0 — the exact shadowing DirNameCollision exists to prevent. The key is now derived by round-tripping the name through the very encoder and parser that will be used, so it cannot drift from them. A name that survives that round trip as empty is refused outright: it would exist in the image and be addressable by nothing. The new test builds a real folder, calls plan, and was confirmed to FAIL with the fix reverted. Separately, the plan-vs-read revalidation compared file LENGTH only, while the plan depends on CONTENT: a DVD VOB placement comes from bytes 0xC0/0xC4 of its IFO, and IFOs occupy a whole number of sectors so an in-place rewrite keeps the length. A re-authoring tool touching the folder mid-rip would pass the size check while every title extent pointed at stale sectors. mtime is now compared alongside size, and only when both sides report one, so a filesystem without timestamps falls back to the old behaviour rather than failing every read.
This commit is contained in:
+21
-2
@@ -79,6 +79,9 @@ struct FileRef {
|
||||
host: PathBuf,
|
||||
disc_path: String,
|
||||
size: u64,
|
||||
/// Host mtime at plan time — see `layout::FileNode::mtime` for why size
|
||||
/// alone is not enough.
|
||||
mtime: Option<std::time::SystemTime>,
|
||||
}
|
||||
|
||||
/// A synthesized UDF disc image over a host directory.
|
||||
@@ -128,6 +131,7 @@ impl DirImage {
|
||||
host: node.host.clone(),
|
||||
disc_path: node.disc_path.clone(),
|
||||
size: node.size,
|
||||
mtime: node.mtime,
|
||||
});
|
||||
let mut offset = 0u64;
|
||||
for e in &node.extents {
|
||||
@@ -206,8 +210,23 @@ impl DirImage {
|
||||
return Ok(&mut self.open[0].1);
|
||||
}
|
||||
let f = File::open(&self.files[file].host).map_err(Error::from)?;
|
||||
let live = f.metadata().map_err(Error::from)?.len();
|
||||
if live != self.files[file].size {
|
||||
let md = f.metadata().map_err(Error::from)?;
|
||||
// Size AND mtime. Size alone is content-blind, and this plan depends on
|
||||
// content: a DVD's VOB placement comes from bytes 0xC0/0xC4 of its IFO,
|
||||
// and an IFO rewritten in place keeps its length because IFOs occupy a
|
||||
// whole number of sectors. The size check would pass while every title
|
||||
// extent pointed at the wrong sectors — corrupt video behind an intact
|
||||
// structure, reported complete at exit 0.
|
||||
//
|
||||
// Only compared when both sides have a timestamp; a platform or
|
||||
// filesystem that reports none simply falls back to the size check
|
||||
// rather than failing every read.
|
||||
let changed_size = md.len() != self.files[file].size;
|
||||
let changed_mtime = match (self.files[file].mtime, md.modified().ok()) {
|
||||
(Some(planned), Some(live)) => planned != live,
|
||||
_ => false,
|
||||
};
|
||||
if changed_size || changed_mtime {
|
||||
return Err(Error::DirImageFileChanged {
|
||||
path: self.files[file].disc_path.clone(),
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user