diff --git a/Cargo.lock b/Cargo.lock index 73d2844..e73e462 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1952,6 +1952,7 @@ name = "nzb-postproc" version = "0.2.6" dependencies = [ "anyhow", + "md-5 0.10.6", "nzb-core", "opentelemetry", "rust-par2", diff --git a/crates/nzb-postproc/Cargo.toml b/crates/nzb-postproc/Cargo.toml index edc7f97..c69d3c6 100644 --- a/crates/nzb-postproc/Cargo.toml +++ b/crates/nzb-postproc/Cargo.toml @@ -24,6 +24,11 @@ zip = "8" [dev-dependencies] tempfile = "3" +# Integration tests synthesise real PAR2 index files (see +# `tests/support/par2_fixture.rs`), which requires writing spec-correct +# packet MD5s. Version tracks what `rust-par2` itself parses with. +md-5 = "0.10" +tokio = { version = "1", features = ["macros", "rt-multi-thread"] } [lints.clippy] all = { level = "warn", priority = -1 } diff --git a/crates/nzb-postproc/src/detect.rs b/crates/nzb-postproc/src/detect.rs index 5d842d5..5ce4ba6 100644 --- a/crates/nzb-postproc/src/detect.rs +++ b/crates/nzb-postproc/src/detect.rs @@ -3,10 +3,58 @@ //! Scans a completed download directory to find par2 files, RAR archives, //! 7z archives, ZIP archives, and cleanup candidates. +use std::collections::BTreeMap; +use std::io::Read; use std::path::{Path, PathBuf}; use walkdir::WalkDir; +/// RAR 4.x volume signature. +const RAR4_SIGNATURE: &[u8] = b"Rar!\x1a\x07\x00"; +/// RAR 5.x volume signature. +const RAR5_SIGNATURE: &[u8] = b"Rar!\x1a\x07\x01\x00"; + +/// Returns true if the file begins with a RAR volume signature. +/// +/// Obfuscated posts strip every naming cue, so content is the only reliable +/// evidence that a `.NN` file is an archive volume. Extension matching +/// alone would misclassify unrelated numeric-suffixed files. +pub fn has_rar_signature(path: &Path) -> bool { + let Ok(mut file) = std::fs::File::open(path) else { + return false; + }; + // Read up to the longest signature; short files simply cannot match. + let mut header = [0u8; RAR5_SIGNATURE.len()]; + let mut filled = 0; + while filled < header.len() { + match file.read(&mut header[filled..]) { + Ok(0) => break, + Ok(n) => filled += n, + Err(_) => return false, + } + } + let head = &header[..filled]; + head.starts_with(RAR4_SIGNATURE) || head.starts_with(RAR5_SIGNATURE) +} + +/// Split a bare numeric extension: `"cfd4be79….45"` → `("cfd4be79…", 45)`. +/// +/// Returns `None` for anything with a non-numeric extension, and for split 7z +/// volumes (`archive.7z.001`) — those are numeric too, but they belong to the +/// 7z path and must not be reclassified as RAR. +fn split_numeric_volume(filename: &str) -> Option<(&str, u32)> { + let dot = filename.rfind('.')?; + let ext = &filename[dot + 1..]; + if ext.is_empty() || !ext.chars().all(|c| c.is_ascii_digit()) { + return None; + } + let stem = &filename[..dot]; + if stem.to_ascii_lowercase().ends_with(".7z") { + return None; + } + ext.parse::().ok().map(|num| (stem, num)) +} + /// Parsed RAR volume information: set name and volume number. #[derive(Debug, Clone, PartialEq, Eq)] pub struct RarVolumeInfo { @@ -77,6 +125,32 @@ pub fn parse_rar_volume(filename: &str) -> Option { None } +/// Parse a RAR volume from a file on disk, falling back to content inspection. +/// +/// [`parse_rar_volume`] can only judge names, so it cannot recognise an +/// obfuscated volume like `cfd4be79….45`. This variant additionally accepts a +/// bare numeric extension when the file actually begins with a RAR signature — +/// evidence a filename cannot provide. Prefer it wherever a path is available. +/// +/// The volume number for an obfuscated set is the numeric extension itself, so +/// volumes order correctly relative to one another within the set. It is not +/// comparable with the numbering [`parse_rar_volume`] assigns to conventional +/// sets, which is anchored to `.rar` = 0. +pub fn parse_rar_volume_at(path: &Path) -> Option { + let name = path.file_name().and_then(|n| n.to_str())?; + if let Some(info) = parse_rar_volume(name) { + return Some(info); + } + let (set_name, volume_number) = split_numeric_volume(name)?; + if !has_rar_signature(path) { + return None; + } + Some(RarVolumeInfo { + set_name: set_name.to_string(), + volume_number, + }) +} + /// The type of archive detected. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ArchiveType { @@ -159,6 +233,8 @@ fn is_par2_volume(name_lower: &str) -> bool { /// `unrar x`). pub fn find_rar_files(dir: &Path) -> Vec { let mut first_volumes: Vec = Vec::new(); + // Obfuscated sets keyed by set name → (lowest volume seen, its path). + let mut numeric_sets: BTreeMap = BTreeMap::new(); for entry in WalkDir::new(dir).into_iter().flatten() { let path = entry.path(); @@ -191,11 +267,34 @@ pub fn find_rar_files(dir: &Path) -> Vec { } // Plain .rar with no .partNNN — this is the first volume in old-style first_volumes.push(path.to_path_buf()); + continue; } // Old-style: .r00, .r01, etc. — we do NOT add these; the .rar file // is the first volume in old-style sets. + if parse_rar_volume(&name_lower).is_some() { + continue; + } + + // Obfuscated set: `.NN` with no recognisable extension. Only + // files that actually begin with a RAR signature qualify, so unrelated + // numeric-suffixed files (`Concert.Recording.1987`) are left alone. + // The lowest-numbered volume is treated as the one to hand to unrar. + if let Some((set_name, volume_number)) = split_numeric_volume(&name_lower) + && has_rar_signature(path) + { + numeric_sets + .entry(set_name.to_string()) + .and_modify(|(lowest, lowest_path)| { + if volume_number < *lowest { + *lowest = volume_number; + *lowest_path = path.to_path_buf(); + } + }) + .or_insert_with(|| (volume_number, path.to_path_buf())); + } } + first_volumes.extend(numeric_sets.into_values().map(|(_, path)| path)); first_volumes.sort(); first_volumes } @@ -251,7 +350,7 @@ pub fn find_cleanup_files(dir: &Path) -> Vec { None => continue, }; - if is_cleanup_candidate(&name) { + if is_cleanup_candidate_at(path, &name) { cleanup.push(path.to_path_buf()); } } @@ -273,7 +372,7 @@ pub fn has_usable_output(dir: &Path) -> std::io::Result { let Some(name) = path.file_name().and_then(|name| name.to_str()) else { continue; }; - if !is_cleanup_candidate(&name.to_ascii_lowercase()) { + if !is_cleanup_candidate_at(path, &name.to_ascii_lowercase()) { return Ok(true); } } @@ -307,8 +406,30 @@ fn is_split_7z_volume(name_lower: &str) -> bool { false } -/// Determine whether a file (by its lowercased name) is safe to clean up +/// Determine whether a file on disk is safe to clean up after successful +/// extraction, using its name and — where the name is uninformative — its +/// content. +/// +/// `name_lower` must be the lowercased file name of `path`. +/// +/// An obfuscated `.NN` volume is indistinguishable by name from an +/// ordinary file that happens to end in digits, so the RAR signature is what +/// separates junk to delete from payload to keep. Getting this wrong in either +/// direction is costly: treating payload as junk deletes it, and treating an +/// archive volume as payload lets a job report Completed with nothing usable +/// in it (issue #87). +fn is_cleanup_candidate_at(path: &Path, name_lower: &str) -> bool { + if is_cleanup_candidate(name_lower) { + return true; + } + split_numeric_volume(name_lower).is_some() && has_rar_signature(path) +} + +/// Determine whether a file (by its lowercased name alone) is safe to clean up /// after successful extraction. +/// +/// Name-only: prefer [`is_cleanup_candidate_at`] wherever a path is available, +/// so obfuscated volumes are caught too. fn is_cleanup_candidate(name: &str) -> bool { // Par2 files: .par2 if name.ends_with(".par2") || name.ends_with(".zip") || name.ends_with(".7z") { diff --git a/crates/nzb-postproc/src/lib.rs b/crates/nzb-postproc/src/lib.rs index 8a35c5a..0db76ca 100644 --- a/crates/nzb-postproc/src/lib.rs +++ b/crates/nzb-postproc/src/lib.rs @@ -16,7 +16,10 @@ pub mod unpack; // need nzb-postproc as a single dependency. pub use nzb_core; -pub use detect::{ArchiveType, RarVolumeInfo, has_usable_output, parse_rar_volume}; +pub use detect::{ + ArchiveType, RarVolumeInfo, has_rar_signature, has_usable_output, parse_rar_volume, + parse_rar_volume_at, +}; pub use par2::recovery_can_cover; pub use pipeline::{PostProcConfig, PostProcResult, run_pipeline, run_pipeline_with_resources}; pub use resources::{PostProcLimits, PostProcResourcePool, PostProcResourceSnapshot}; diff --git a/crates/nzb-postproc/src/pipeline.rs b/crates/nzb-postproc/src/pipeline.rs index 99452ce..7f75c6c 100644 --- a/crates/nzb-postproc/src/pipeline.rs +++ b/crates/nzb-postproc/src/pipeline.rs @@ -160,12 +160,34 @@ pub async fn run_pipeline_with_resources( }); } } else if config.articles_failed == 0 { + // Files are known-good from CRC checks during yEnc decode, so the + // expensive MD5 verification pass is skipped. + // + // PAR2-guided deobfuscation still has to run. Obfuscated posts arrive + // with meaningless filenames whether or not an article failed, and the + // PAR2 metadata is the only record of the real names. While this + // rename lived inside the verify branch below, a *clean* download of + // an obfuscated post was never deobfuscated: no archive was found, the + // Extract stage reported "No archives found", and the job completed + // with raw volumes on disk. A damaged download self-healed; a healthy + // one did not (issue #87). info!("Skipping PAR2 verification — zero article failures (CRC-verified)"); + let start = Instant::now(); + let message = match rust_par2::parse(&par2_files[0]) { + Ok(file_set) => { + rename_to_par2_names(&file_set, job_dir); + "Skipped — zero article failures (PAR2-guided rename applied)".to_string() + } + Err(e) => { + debug!(error = %e, "PAR2 parse failed; skipping PAR2-guided deobfuscation"); + format!("Skipped — zero article failures (PAR2 parse failed: {e})") + } + }; stages.push(StageResult { name: "Verify".to_string(), status: StageStatus::Skipped, - message: Some("Skipped — zero article failures".to_string()), - duration_secs: 0.0, + message: Some(message), + duration_secs: start.elapsed().as_secs_f64(), }); } else { let verify_start = Instant::now(); @@ -182,7 +204,9 @@ pub async fn run_pipeline_with_resources( // PAR2 expected names (common with obfuscated posts where // NZB subjects have readable names but PAR2 references // the original obfuscated filenames), rename them using - // MD5-16k hash matching before verification runs. + // MD5-16k hash matching before verification runs. The + // zero-failure branch above runs this too — verification is + // skipped there, but deobfuscation must not be. rename_to_par2_names(&file_set, job_dir); // Run verify (and repair if needed) in a single spawn_blocking call. diff --git a/crates/nzb-postproc/tests/obfuscated_rar_volumes.rs b/crates/nzb-postproc/tests/obfuscated_rar_volumes.rs new file mode 100644 index 0000000..5f29501 --- /dev/null +++ b/crates/nzb-postproc/tests/obfuscated_rar_volumes.rs @@ -0,0 +1,303 @@ +//! Detection-layer regression suite for issue #87: obfuscated multi-volume RAR +//! sets named `<32 hex digits>.NN` were never detected, never extracted, and +//! still let the job report Completed. +//! +//! The suite is deliberately paired: +//! +//! * `neg_*` — **negative controls**. Conventional naming (`.rar`/`.r00`, +//! `.partNNN.rar`, `.7z.001`) and true payload. These passed before the fix +//! as well as after, so a `pos_*` failure is about the *name*, not the +//! plumbing. They are the guard against the fix breaking ordinary sets. +//! +//! * `pos_*` — the obfuscated `.NN` set. Each of these failed before the +//! fix, one per broken layer. +//! +//! * `guard_*` — **false-positive guards**. A naive fix ("treat any numeric +//! extension as a RAR volume") breaks these; they are why detection reads +//! the RAR signature instead of trusting the extension. +//! +//! Fixtures carry real RAR signature bytes because detection now sniffs them. + +use std::fs; +use std::path::Path; + +use nzb_postproc::ArchiveType; +use nzb_postproc::detect::{ + find_archives, find_cleanup_files, has_usable_output, parse_rar_volume, parse_rar_volume_at, +}; + +/// RAR 4.x volume signature. +const RAR4_MAGIC: &[u8] = b"Rar!\x1a\x07\x00"; +/// RAR 5.x volume signature. +const RAR5_MAGIC: &[u8] = b"Rar!\x1a\x07\x01\x00"; + +/// The obfuscated set name from the issue report. +const HASH: &str = "cfd4be79c0fb01d429c52a9f8551ee79"; + +/// Write `files` (name, contents) into a fresh temp dir. +fn make_dir(files: &[(&str, &[u8])]) -> tempfile::TempDir { + let dir = tempfile::tempdir().unwrap(); + for (name, contents) in files { + fs::write(dir.path().join(name), contents).unwrap(); + } + dir +} + +/// A RAR volume body: signature + filler so the file is not empty. +fn rar_volume(magic: &[u8]) -> Vec { + let mut body = magic.to_vec(); + body.extend_from_slice(&[0u8; 64]); + body +} + +/// Count the RAR entries `find_archives` reported. +fn rar_archives(dir: &Path) -> Vec { + find_archives(dir) + .into_iter() + .filter(|(kind, _)| *kind == ArchiveType::Rar) + .map(|(_, path)| path.file_name().unwrap().to_string_lossy().into_owned()) + .collect() +} + +/// Cleanup candidates as bare filenames. +fn cleanup_names(dir: &Path) -> Vec { + find_cleanup_files(dir) + .into_iter() + .map(|path| path.file_name().unwrap().to_string_lossy().into_owned()) + .collect() +} + +// --------------------------------------------------------------------------- +// Negative controls — conventional naming. Must stay green. +// --------------------------------------------------------------------------- + +#[test] +fn neg_old_style_rar_set_is_detected_extracted_and_cleaned() { + let body = rar_volume(RAR4_MAGIC); + let dir = make_dir(&[ + ("movie.rar", &body), + ("movie.r00", &body), + ("movie.r01", &body), + ("movie.par2", b""), + ]); + + // Detected, and only the first volume is offered for extraction. + assert_eq!( + rar_archives(dir.path()), + vec!["movie.rar".to_string()], + "old-style set should yield exactly the first volume" + ); + + // Every volume is a cleanup candidate. + let cleanup = cleanup_names(dir.path()); + for name in ["movie.rar", "movie.r00", "movie.r01", "movie.par2"] { + assert!( + cleanup.contains(&name.to_string()), + "{name} should be cleanup" + ); + } + + // Raw archives alone are not a usable completion. + assert!( + !has_usable_output(dir.path()).unwrap(), + "a directory of raw volumes must not count as usable output" + ); + + // Volume numbering is understood. + assert_eq!(parse_rar_volume("movie.rar").unwrap().volume_number, 0); + assert_eq!(parse_rar_volume("movie.r00").unwrap().volume_number, 1); + assert_eq!(parse_rar_volume("movie.r01").unwrap().volume_number, 2); +} + +#[test] +fn neg_new_style_part_set_is_detected() { + let body = rar_volume(RAR5_MAGIC); + let dir = make_dir(&[ + ("movie.part001.rar", &body), + ("movie.part002.rar", &body), + ("movie.part003.rar", &body), + ]); + + assert_eq!( + rar_archives(dir.path()), + vec!["movie.part001.rar".to_string()], + "new-style set should yield exactly part001" + ); + assert!(!has_usable_output(dir.path()).unwrap()); + + let parsed = parse_rar_volume("movie.part002.rar").unwrap(); + assert_eq!(parsed.set_name, "movie"); + assert_eq!(parsed.volume_number, 1); +} + +#[test] +fn neg_split_7z_set_is_detected() { + let dir = make_dir(&[ + ("archive.7z.001", b"7z\xbc\xaf\x27\x1c"), + ("archive.7z.002", b"data"), + ]); + + let archives = find_archives(dir.path()); + assert_eq!(archives.len(), 1, "only the .001 volume starts extraction"); + assert_eq!(archives[0].0, ArchiveType::SevenZip); + assert!(!has_usable_output(dir.path()).unwrap()); +} + +#[test] +fn neg_real_payload_counts_as_usable_output() { + let dir = make_dir(&[("Movie.2024.1080p.mkv", b"\x1a\x45\xdf\xa3matroska")]); + assert!( + has_usable_output(dir.path()).unwrap(), + "extracted media must count as usable output" + ); + assert!( + cleanup_names(dir.path()).is_empty(), + "payload must never be a cleanup candidate" + ); +} + +// --------------------------------------------------------------------------- +// The obfuscated `.NN` set — each of these failed before the fix. +// --------------------------------------------------------------------------- + +/// Build the obfuscated set from the issue: `.45`, `.46`, `.47` + par2. +fn obfuscated_dir() -> tempfile::TempDir { + let body = rar_volume(RAR4_MAGIC); + let dir = tempfile::tempdir().unwrap(); + for n in 45..=47u32 { + fs::write(dir.path().join(format!("{HASH}.{n}")), &body).unwrap(); + } + fs::write(dir.path().join(format!("{HASH}.par2")), b"").unwrap(); + dir +} + +#[test] +fn pos_obfuscated_numeric_volumes_are_detected_as_rar() { + let dir = obfuscated_dir(); + let archives = rar_archives(dir.path()); + assert!( + !archives.is_empty(), + "obfuscated RAR set went undetected; find_archives returned nothing, \ + so the Extract stage reports \"No archives found\" and skips" + ); + assert_eq!( + archives.len(), + 1, + "exactly one volume should start extraction, got {archives:?}" + ); +} + +#[test] +fn pos_obfuscated_volumes_are_not_usable_output() { + let dir = obfuscated_dir(); + assert!( + !has_usable_output(dir.path()).unwrap(), + "raw .NN volumes were reported as usable payload, which is what \ + lets move_to_history stamp the job Completed" + ); +} + +#[test] +fn pos_obfuscated_volumes_are_cleanup_candidates() { + let dir = obfuscated_dir(); + let cleanup = cleanup_names(dir.path()); + for n in 45..=47u32 { + let name = format!("{HASH}.{n}"); + assert!( + cleanup.contains(&name), + "{name} should be a cleanup candidate; got {cleanup:?}" + ); + } +} + +#[test] +fn pos_obfuscated_volume_numbers_are_parsed() { + // Name alone cannot distinguish an obfuscated volume from any other file + // ending in digits, so numbering is resolved through the content-aware + // entry point. `parse_rar_volume` deliberately still rejects these. + let dir = obfuscated_dir(); + assert!( + parse_rar_volume(&format!("{HASH}.45")).is_none(), + "the name-only parser must not claim bare numeric extensions" + ); + + let parsed = parse_rar_volume_at(&dir.path().join(format!("{HASH}.45"))) + .expect("bare numeric RAR volume should parse from disk"); + assert_eq!(parsed.set_name, HASH); + + let next = parse_rar_volume_at(&dir.path().join(format!("{HASH}.46"))) + .expect("bare numeric RAR volume should parse from disk"); + assert_eq!( + next.volume_number, + parsed.volume_number + 1, + "consecutive numeric volumes must order consecutively" + ); +} + +#[test] +fn guard_numeric_file_without_signature_is_not_a_volume() { + // The content check is what keeps `parse_rar_volume_at` honest. + let dir = make_dir(&[("logfile.01", b"2026-08-11 INFO started")]); + assert!( + parse_rar_volume_at(&dir.path().join("logfile.01")).is_none(), + "a numeric-suffixed file with no RAR signature is not a volume" + ); +} + +#[test] +fn pos_obfuscated_rar5_volumes_are_detected() { + let body = rar_volume(RAR5_MAGIC); + let dir = tempfile::tempdir().unwrap(); + for n in 1..=3u32 { + fs::write(dir.path().join(format!("{HASH}.{n:02}")), &body).unwrap(); + } + assert!( + !rar_archives(dir.path()).is_empty(), + "RAR5 obfuscated set went undetected" + ); +} + +// --------------------------------------------------------------------------- +// False-positive guards — a naive "any numeric extension is a RAR volume" fix +// breaks these. They are why detection sniffs the RAR signature. +// --------------------------------------------------------------------------- + +#[test] +fn guard_numeric_suffix_without_rar_signature_is_not_an_archive() { + // Looks like `.NN` but holds no RAR signature — e.g. a stray data + // file. Detection must not claim it. + let dir = make_dir(&[ + ( + "Concert.Recording.1987", + b"plain text notes, not an archive", + ), + ("logfile.01", b"2026-08-11 INFO started"), + ]); + assert!( + rar_archives(dir.path()).is_empty(), + "non-RAR numeric-suffixed files must not be detected as archives" + ); +} + +#[test] +fn guard_split_7z_volumes_are_not_reclassified_as_rar() { + // `.001` is numeric too. It must stay a 7z volume, not become a RAR set, + // or extraction hands the wrong file to the wrong extractor. + let dir = make_dir(&[ + ("archive.7z.001", b"7z\xbc\xaf\x27\x1c"), + ("archive.7z.002", b"more data"), + ]); + assert!( + rar_archives(dir.path()).is_empty(), + "split 7z volumes must not be classified as RAR" + ); + assert_eq!(find_archives(dir.path()).len(), 1); +} + +#[test] +fn guard_media_file_with_year_suffix_stays_payload() { + // `has_usable_output` must not start discarding real payload just because + // the name ends in digits. + let dir = make_dir(&[("Movie.Title.2024.mkv", b"\x1a\x45\xdf\xa3matroska")]); + assert!(has_usable_output(dir.path()).unwrap()); +} diff --git a/crates/nzb-postproc/tests/par2_deobfuscation_gate.rs b/crates/nzb-postproc/tests/par2_deobfuscation_gate.rs new file mode 100644 index 0000000..d45bfb1 --- /dev/null +++ b/crates/nzb-postproc/tests/par2_deobfuscation_gate.rs @@ -0,0 +1,211 @@ +//! Regression suite for the second half of issue #87: the PAR2-guided +//! deobfuscation that fixes obfuscated names used to be skipped on a healthy +//! download. +//! +//! `rename_to_par2_names` (pipeline.rs) recovers original filenames by matching +//! each file's first-16K MD5 against the PAR2 metadata — exactly what an +//! obfuscated `.NN` set needs. It used to be called only from inside the +//! verify branch, and verification is skipped outright when +//! `articles_failed == 0` ("files are known-good from CRC checks"), so the one +//! mechanism that can deobfuscate the set only ran when the download was +//! *damaged*. +//! +//! The fix runs the rename whenever the PAR2 index parses, independent of the +//! verify decision. These tests pin that: the pairing mirrors +//! `obfuscated_rar_volumes.rs`: +//! +//! * `neg_*` — the damaged path, where the rename always ran. Passed before +//! the fix too; proves the rename machinery itself was never the problem. +//! * `pos_*` — the healthy path. Same files, same PAR2, only +//! `articles_failed` differs. This is the test that failed before the fix. +//! * `guard_*` — the rename must stay targeted now that it runs on every job. +//! +//! Extraction and cleanup are disabled throughout so the assertions are about +//! filenames on disk and nothing else — these tests need no `unrar` binary. + +mod support; + +use std::fs; +use std::path::Path; + +use nzb_postproc::{PostProcConfig, run_pipeline}; +use support::par2_fixture::Par2Fixture; + +/// The obfuscated set name from the issue report. +const HASH: &str = "cfd4be79c0fb01d429c52a9f8551ee79"; + +/// Canonical names PAR2 knows the volumes by. +const CANONICAL: [&str; 3] = [ + "Movie.Title.2024.part001.rar", + "Movie.Title.2024.part002.rar", + "Movie.Title.2024.part003.rar", +]; + +/// Volume bodies: real RAR signature plus per-volume filler so each file has a +/// distinct 16K hash and matching is unambiguous. +fn volume_bodies() -> Vec> { + (0..3u8) + .map(|n| { + let mut body = b"Rar!\x1a\x07\x00".to_vec(); + body.extend_from_slice(&[n; 512]); + body + }) + .collect() +} + +/// A job directory holding the three volumes under `names`, plus a PAR2 index +/// that records them under their canonical names. +fn job_dir(names: &[String]) -> tempfile::TempDir { + let dir = tempfile::tempdir().unwrap(); + let bodies = volume_bodies(); + + let mut fixture = Par2Fixture::new(); + for (canonical, body) in CANONICAL.iter().zip(&bodies) { + fixture = fixture.add_file(canonical, body); + } + fixture.write_index(&dir.path().join("Movie.Title.2024.par2")); + + for (name, body) in names.iter().zip(&bodies) { + fs::write(dir.path().join(name), body).unwrap(); + } + dir +} + +/// The obfuscated layout: `.45`, `.46`, `.47`. +fn obfuscated_names() -> Vec { + (45..=47u32).map(|n| format!("{HASH}.{n}")).collect() +} + +/// Pipeline config that isolates the verify/rename stage. +fn config(articles_failed: usize) -> PostProcConfig { + PostProcConfig { + articles_failed, + skip_extract: true, + cleanup_after_extract: false, + ..Default::default() + } +} + +/// Sorted non-par2 filenames in `dir`. +fn names_on_disk(dir: &Path) -> Vec { + let mut names: Vec = fs::read_dir(dir) + .unwrap() + .filter_map(|e| e.ok()) + .map(|e| e.file_name().to_string_lossy().into_owned()) + .filter(|n| !n.to_lowercase().ends_with(".par2")) + .collect(); + names.sort(); + names +} + +// --------------------------------------------------------------------------- +// Fixture self-test — if this breaks, every result below is meaningless. +// --------------------------------------------------------------------------- + +#[test] +fn neg_par2_fixture_is_parseable_and_names_the_files() { + let dir = job_dir(&obfuscated_names()); + let index = dir.path().join("Movie.Title.2024.par2"); + + let file_set = rust_par2::parse(&index).expect("synthesised PAR2 index must parse"); + let mut names: Vec = file_set + .files + .values() + .map(|f| f.filename.clone()) + .collect(); + names.sort(); + assert_eq!( + names, + CANONICAL.iter().map(|s| s.to_string()).collect::>(), + "synthesised PAR2 index must report the canonical filenames" + ); + + // The 16K hashes must match what the rename path computes, or matching + // would silently never fire and the gate tests would prove nothing. + for (canonical, body) in CANONICAL.iter().zip(volume_bodies()) { + let probe = dir.path().join("probe.bin"); + fs::write(&probe, &body).unwrap(); + let hash = rust_par2::compute_hash_16k(&probe).unwrap(); + assert!( + file_set + .files + .values() + .any(|f| f.filename == *canonical && f.hash_16k == hash), + "16K hash recorded for {canonical} must match compute_hash_16k" + ); + fs::remove_file(&probe).unwrap(); + } +} + +// --------------------------------------------------------------------------- +// Negative control — damaged download. The rename always ran here. +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn neg_damaged_download_deobfuscates_via_par2() { + let dir = job_dir(&obfuscated_names()); + + // articles_failed > 0 → verify branch runs → rename_to_par2_names runs. + let _ = run_pipeline(dir.path(), &config(1)).await; + + assert_eq!( + names_on_disk(dir.path()), + CANONICAL.iter().map(|s| s.to_string()).collect::>(), + "with articles_failed > 0 the PAR2-guided rename should restore the \ + canonical names" + ); +} + +// --------------------------------------------------------------------------- +// Healthy download — this is the case that failed before the fix. +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn pos_healthy_download_deobfuscates_via_par2() { + let dir = job_dir(&obfuscated_names()); + + // Identical inputs to the negative control; only articles_failed differs. + let _ = run_pipeline(dir.path(), &config(0)).await; + + assert_eq!( + names_on_disk(dir.path()), + CANONICAL.iter().map(|s| s.to_string()).collect::>(), + "a clean download of an obfuscated post kept its .NN names — the \ + PAR2-guided rename has been re-gated behind the verify branch, which \ + articles_failed == 0 skips, so the set is never deobfuscated and the \ + Extract stage finds no archives (issue #87)" + ); +} + +// --------------------------------------------------------------------------- +// Guards — the rename must stay targeted now that it runs on every job. +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn guard_correctly_named_files_are_left_alone() { + let canonical: Vec = CANONICAL.iter().map(|s| s.to_string()).collect(); + let dir = job_dir(&canonical); + + let _ = run_pipeline(dir.path(), &config(1)).await; + + assert_eq!( + names_on_disk(dir.path()), + canonical, + "files already matching PAR2 must not be renamed" + ); +} + +#[tokio::test] +async fn guard_unrelated_files_are_not_renamed() { + // A file PAR2 knows nothing about must keep its name — lifting the gate + // must not turn the rename into a free-for-all over the job directory. + let dir = job_dir(&obfuscated_names()); + fs::write(dir.path().join("readme.nfo"), b"release notes").unwrap(); + + let _ = run_pipeline(dir.path(), &config(1)).await; + + assert!( + dir.path().join("readme.nfo").exists(), + "a file absent from the PAR2 set must be left untouched" + ); +} diff --git a/crates/nzb-postproc/tests/support/mod.rs b/crates/nzb-postproc/tests/support/mod.rs new file mode 100644 index 0000000..c226ad9 --- /dev/null +++ b/crates/nzb-postproc/tests/support/mod.rs @@ -0,0 +1,3 @@ +//! Shared test support for `nzb-postproc` integration tests. + +pub mod par2_fixture; diff --git a/crates/nzb-postproc/tests/support/par2_fixture.rs b/crates/nzb-postproc/tests/support/par2_fixture.rs new file mode 100644 index 0000000..73e85da --- /dev/null +++ b/crates/nzb-postproc/tests/support/par2_fixture.rs @@ -0,0 +1,155 @@ +//! Synthesise real, parseable PAR2 index files for integration tests. +//! +//! The repo has no `.par2` fixtures and no `par2create` binary in CI, which +//! previously made the PAR2-guided deobfuscation path (`rename_to_par2_names`) +//! untestable end-to-end. This builder writes spec-correct PAR2 packets so +//! `rust_par2::parse` accepts them and the pipeline exercises the real code +//! path rather than a mock. +//! +//! Only the packets the pipeline actually reads are emitted: +//! +//! * **Main** — carries the slice size; `parse` returns `NoMainPacket` without it. +//! * **FileDesc** — one per file: file ID, full-file MD5, first-16K MD5, size, +//! and the *expected* filename. The 16K hash is what `rename_to_par2_names` +//! matches obfuscated files against. +//! +//! Recovery (`RecvSlic`) and slice-checksum (`IFSC`) packets are omitted: they +//! only matter for actual repair, which these tests never reach. Verification +//! will therefore report files as damaged — that is fine and expected, because +//! the assertions are about *filenames on disk*, not repair outcomes. +//! +//! Packet layout implemented here (little-endian), per the PAR 2.0 spec: +//! +//! ```text +//! 0..8 magic "PAR2\0PKT" +//! 8..16 packet length (u64, whole packet, multiple of 4) +//! 16..32 MD5 of everything from offset 32 onward +//! 32..48 recovery set ID +//! 48..64 packet type +//! 64.. body +//! ``` + +use std::path::Path; + +use md5::{Digest, Md5}; + +const MAGIC: &[u8; 8] = b"PAR2\x00PKT"; +const TYPE_MAIN: &[u8; 16] = b"PAR 2.0\x00Main\x00\x00\x00\x00"; +const TYPE_FILE_DESC: &[u8; 16] = b"PAR 2.0\x00FileDesc"; + +/// One file recorded in the recovery set. +struct FileEntry { + /// The name PAR2 considers canonical — what a rename should restore. + expected_name: String, + /// File ID (spec: MD5 of hash_16k + size + name). + file_id: [u8; 16], + hash: [u8; 16], + hash_16k: [u8; 16], + size: u64, +} + +/// Builds a PAR2 index file describing a set of files by content. +pub struct Par2Fixture { + slice_size: u64, + recovery_set_id: [u8; 16], + files: Vec, +} + +impl Par2Fixture { + /// Start a new recovery set. `slice_size` must be a multiple of 4. + pub fn new() -> Self { + Self { + slice_size: 4096, + // Fixed, not random: tests must be deterministic. + recovery_set_id: *b"rustnzbfixture01", + files: Vec::new(), + } + } + + /// Record `contents` under the canonical name PAR2 should report. + /// + /// This does not write the file to disk — the test decides whether to + /// place it under its canonical name or an obfuscated one. + pub fn add_file(mut self, expected_name: &str, contents: &[u8]) -> Self { + let hash: [u8; 16] = Md5::digest(contents).into(); + // Mirrors `rust_par2::compute_hash_16k`: MD5 of the first 16 KiB. + let head = &contents[..contents.len().min(16384)]; + let hash_16k: [u8; 16] = Md5::digest(head).into(); + let size = contents.len() as u64; + + // Spec: File ID = MD5(hash_16k || size_le || name). + let mut id_input = Vec::new(); + id_input.extend_from_slice(&hash_16k); + id_input.extend_from_slice(&size.to_le_bytes()); + id_input.extend_from_slice(expected_name.as_bytes()); + let file_id: [u8; 16] = Md5::digest(&id_input).into(); + + self.files.push(FileEntry { + expected_name: expected_name.to_string(), + file_id, + hash, + hash_16k, + size, + }); + self + } + + /// Write the index `.par2` file to `path`. + pub fn write_index(&self, path: &Path) { + let mut out = Vec::new(); + + // Main packet: slice size, file count, then the file IDs in order. + let mut main_body = Vec::new(); + main_body.extend_from_slice(&self.slice_size.to_le_bytes()); + main_body.extend_from_slice(&(self.files.len() as u32).to_le_bytes()); + for file in &self.files { + main_body.extend_from_slice(&file.file_id); + } + out.extend_from_slice(&self.packet(TYPE_MAIN, &main_body)); + + // One FileDesc packet per file. + for file in &self.files { + let mut body = Vec::new(); + body.extend_from_slice(&file.file_id); + body.extend_from_slice(&file.hash); + body.extend_from_slice(&file.hash_16k); + body.extend_from_slice(&file.size.to_le_bytes()); + body.extend_from_slice(file.expected_name.as_bytes()); + // Filename is null-padded to a multiple of 4 so the packet length + // stays 4-aligned; the parser rejects lengths that are not. + while body.len() % 4 != 0 { + body.push(0); + } + out.extend_from_slice(&self.packet(TYPE_FILE_DESC, &body)); + } + + std::fs::write(path, &out).unwrap(); + } + + /// Frame `body` as a PAR2 packet with a correct length and MD5. + /// + /// The parser recomputes the MD5 over everything from offset 32 and skips + /// packets that don't match, so this must be exact — a wrong hash makes + /// the fixture silently empty rather than failing loudly. + fn packet(&self, packet_type: &[u8; 16], body: &[u8]) -> Vec { + // `data` is what the MD5 covers: set ID + type + body. + let mut data = Vec::with_capacity(32 + body.len()); + data.extend_from_slice(&self.recovery_set_id); + data.extend_from_slice(packet_type); + data.extend_from_slice(body); + + let packet_len = (32 + data.len()) as u64; + assert!( + packet_len % 4 == 0, + "PAR2 packet length must be 4-aligned, got {packet_len}" + ); + let md5: [u8; 16] = Md5::digest(&data).into(); + + let mut packet = Vec::with_capacity(packet_len as usize); + packet.extend_from_slice(MAGIC); + packet.extend_from_slice(&packet_len.to_le_bytes()); + packet.extend_from_slice(&md5); + packet.extend_from_slice(&data); + packet + } +}