From 7ca30af93b4617aceacab59d6636747075faf698 Mon Sep 17 00:00:00 2001 From: nonamexishere Date: Mon, 24 Aug 2026 16:19:31 +0300 Subject: [PATCH 1/3] feat(edit): fail-closed qpdf --check + snapshot gate before publish Validate staged Edit PDF output before the destination rename so a truncated or structurally wrong file cannot replace the user's dest. qpdf --check errors are fatal; warnings are recorded and do not block. Fixes #34. --- CHANGELOG.md | 4 + src-tauri/src/pdf_engine/edit_overlay.rs | 82 ++- .../src/pdf_engine/edit_overlay_integ.rs | 2 + src-tauri/src/pdf_engine/mod.rs | 1 + src-tauri/src/pdf_engine/qpdf.rs | 39 +- src-tauri/src/pdf_engine/validate_output.rs | 630 ++++++++++++++++++ 6 files changed, 736 insertions(+), 22 deletions(-) create mode 100644 src-tauri/src/pdf_engine/validate_output.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ab177d..01aa4e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ All notable project changes should be documented here. ## Unreleased +### Added + +- Edit PDF: staged output is checked with `qpdf --check` and reopened for page boxes and catalog data before the destination file is replaced. A failed check leaves the original and any existing destination untouched. + ### Fixed - HEIC/HEIF import refuses to copy pixels when the decoded plane size does not match the image handle, so a tiled or rotated phone photo cannot walk off the buffer. Invalid files still return an in-app error. diff --git a/src-tauri/src/pdf_engine/edit_overlay.rs b/src-tauri/src/pdf_engine/edit_overlay.rs index 517dd6a..8234929 100644 --- a/src-tauri/src/pdf_engine/edit_overlay.rs +++ b/src-tauri/src/pdf_engine/edit_overlay.rs @@ -4,7 +4,10 @@ use crate::error::AppError; use crate::models::{JobHandle, PageGroup}; -use crate::pdf_engine::{crop, edit_image}; +use crate::pdf_engine::validate_output::{ + catalog_flags_from_doc, validate_staged_pdf, OutputSnapshot, PageSnapshot, +}; +use crate::pdf_engine::{crop, edit_image, qpdf}; use crate::utils::process::run_qpdf; use crate::utils::safe_output; use crate::utils::temp; @@ -1088,7 +1091,7 @@ pub(crate) fn build_edit_overlay_args( Ok(args) } -/// Build the overlay and run qpdf via `run`. Used by the Tauri command and tests. +/// Build the overlay and run qpdf via `run`. Used by tests (system/`"qpdf"`). pub(crate) fn export_edit_pdf_with_runner( groups: &[PageGroup], output: &str, @@ -1097,6 +1100,27 @@ pub(crate) fn export_edit_pdf_with_runner( work: &Path, unique: &str, cancel: Option<&AtomicBool>, + run: F, +) -> Result, AppError> +where + F: FnMut(&[String]) -> Result<(), AppError>, +{ + let exe = qpdf::resolve_qpdf_standalone(); + export_edit_pdf_with_check_exe( + groups, output, document, font_path, work, unique, cancel, &exe, run, + ) +} + +/// Same as [`export_edit_pdf_with_runner`], with an explicit `qpdf --check` binary. +fn export_edit_pdf_with_check_exe( + groups: &[PageGroup], + output: &str, + document: &EditDocumentIn, + font_path: &Path, + work: &Path, + unique: &str, + cancel: Option<&AtomicBool>, + qpdf_check: &Path, mut run: F, ) -> Result, AppError> where @@ -1118,6 +1142,7 @@ where let tmp_str = tmp.to_string_lossy().to_string(); let overlay = work.join("overlay.pdf"); let overlay_str = overlay.to_string_lossy().to_string(); + let mut gate_passed = false; let result = (|| -> Result, AppError> { let (geoms, counts) = collect_source_pages(groups)?; if geoms.is_empty() { @@ -1138,17 +1163,60 @@ where run(&[tmp_str.clone(), cleaned_str.clone()])?; safe_output::replace_file(&cleaned, &tmp)?; } + let snapshot = output_snapshot_from_source(&geoms, Path::new(&groups[0].path))?; + validate_staged_pdf(&tmp, &snapshot, cancel, |args| { + run_qpdf_check_argv(qpdf_check, args) + })?; + gate_passed = true; safe_output::replace_file(&tmp, dest)?; Ok(vec![output.to_string()]) })(); - // On success the temp was renamed away. On failure leave a dest-sibling - // tmp in place so a failed Windows replace still has a recoverable file. - if result.is_ok() && tmp.exists() { + // Keep tmp only if replace_file failed after a passed gate (Windows recover). + // Spawn/validate errors (and leftover success tmp) delete the sibling. + if !(gate_passed && result.is_err()) && tmp.exists() { let _ = std::fs::remove_file(&tmp); } result } +fn output_snapshot_from_source( + geoms: &[OverlayPageGeom], + primary: &Path, +) -> Result { + let doc = Document::load(primary) + .map_err(|e| AppError::engine_failed(format!("Could not read the PDF: {e}")))?; + Ok(OutputSnapshot { + pages: geoms + .iter() + .map(|g| PageSnapshot { + media_box: g.media, + crop_box: g.crop, + trim_box: g.trim, + rotate: g.rotate, + user_unit: g.user_unit, + }) + .collect(), + catalog: catalog_flags_from_doc(&doc), + }) +} + +fn run_qpdf_check_argv(exe: &Path, args: &[String]) -> Result<(i32, String), AppError> { + let mut cmd = std::process::Command::new(exe); + cmd.args(args); + #[cfg(windows)] + { + use std::os::windows::process::CommandExt; + cmd.creation_flags(0x08000000); + } + let output = cmd + .output() + .map_err(|e| AppError::io("qpdf --check failed to start", e))?; + Ok(( + output.status.code().unwrap_or(1), + String::from_utf8_lossy(&output.stderr).into_owned(), + )) +} + pub fn edit_pdf_overlays( app: &tauri::AppHandle, handle: &Arc, @@ -1183,7 +1251,8 @@ pub fn edit_pdf_overlays( if handle.is_cancelled() { return Err(AppError::cancelled()); } - export_edit_pdf_with_runner( + let qpdf_exe = qpdf::resolve_qpdf(app); + export_edit_pdf_with_check_exe( groups, output, document, @@ -1191,6 +1260,7 @@ pub fn edit_pdf_overlays( &work, job_id, Some(&handle.cancelled), + &qpdf_exe, |args| run_qpdf(app, handle, job_id, args, "Saving", None), ) })(); diff --git a/src-tauri/src/pdf_engine/edit_overlay_integ.rs b/src-tauri/src/pdf_engine/edit_overlay_integ.rs index 88845eb..62ebab5 100644 --- a/src-tauri/src/pdf_engine/edit_overlay_integ.rs +++ b/src-tauri/src/pdf_engine/edit_overlay_integ.rs @@ -436,6 +436,7 @@ fn write_tiny_png(path: &Path, w: u32, h: u32) { #[test] fn integ_catalog_survives_and_original_stream_stays() { + // V7 keepGreen let Some(fx) = Harness::new("catalog") else { eprintln!("skip: qpdf not available"); return; @@ -464,6 +465,7 @@ fn integ_catalog_survives_and_original_stream_stays() { assert!(cat.get(b"AcroForm").is_ok(), "AcroForm missing"); let blob = dump_streams(&fx.dest); + // V7 keepGreen assert!(blob.contains("Hello"), "original page stream rasterized away: {blob:?}"); fx.cleanup(); } diff --git a/src-tauri/src/pdf_engine/mod.rs b/src-tauri/src/pdf_engine/mod.rs index 4331211..9831e2e 100644 --- a/src-tauri/src/pdf_engine/mod.rs +++ b/src-tauri/src/pdf_engine/mod.rs @@ -23,6 +23,7 @@ pub mod qpdf; pub mod render; pub mod stamp; pub mod textexport; +pub mod validate_output; use crate::error::AppError; use crate::models::{JobHandle, JobUpdate, PageGroup, PagePick, RotateGroup, SplitMode}; diff --git a/src-tauri/src/pdf_engine/qpdf.rs b/src-tauri/src/pdf_engine/qpdf.rs index 7c70227..8fc4983 100644 --- a/src-tauri/src/pdf_engine/qpdf.rs +++ b/src-tauri/src/pdf_engine/qpdf.rs @@ -14,20 +14,11 @@ fn exe_name() -> &'static str { } } -/// Locate the qpdf binary. Prefers a bundled copy under `binaries/`, falling -/// back to the system PATH (by returning the bare exe name). -pub fn resolve_qpdf(app: &tauri::AppHandle) -> PathBuf { +/// Locate qpdf without a Tauri handle (Edit PDF `--check`, tests). +pub fn resolve_qpdf_standalone() -> PathBuf { let exe = exe_name(); - // 1. Bundled next to app resources. - if let Ok(res) = app.path().resource_dir() { - let candidate = res.join("binaries").join(exe); - if candidate.exists() { - return candidate; - } - } - - // 2. Bundled next to the executable. + // Bundled next to the executable. if let Ok(cur) = std::env::current_exe() { if let Some(parent) = cur.parent() { let candidate = parent.join("binaries").join(exe); @@ -37,9 +28,9 @@ pub fn resolve_qpdf(app: &tauri::AppHandle) -> PathBuf { } } - // 3. Common absolute install locations. A Finder-launched .app does NOT - // inherit the shell PATH (so Homebrew/MacPorts dirs are missing), so we - // probe them explicitly before relying on PATH. + // Common absolute install locations. A Finder-launched .app does NOT + // inherit the shell PATH (so Homebrew/MacPorts dirs are missing), so we + // probe them explicitly before relying on PATH. #[cfg(not(windows))] { for candidate in [ @@ -55,10 +46,26 @@ pub fn resolve_qpdf(app: &tauri::AppHandle) -> PathBuf { } } - // 4. Fall back to PATH (works when launched from a terminal / dev). + // Fall back to PATH (works when launched from a terminal / dev). PathBuf::from(exe) } +/// Locate the qpdf binary. Prefers a bundled copy under `binaries/`, falling +/// back to the system PATH (by returning the bare exe name). +pub fn resolve_qpdf(app: &tauri::AppHandle) -> PathBuf { + let exe = exe_name(); + + // 1. Bundled next to app resources. + if let Ok(res) = app.path().resource_dir() { + let candidate = res.join("binaries").join(exe); + if candidate.exists() { + return candidate; + } + } + + resolve_qpdf_standalone() +} + /// Return the number of pages in `input` via `qpdf --show-npages`. /// Any failure (spawn, non-zero exit, unparseable output) -> `invalid_pdf`. pub fn npages(app: &tauri::AppHandle, input: &str) -> Result { diff --git a/src-tauri/src/pdf_engine/validate_output.rs b/src-tauri/src/pdf_engine/validate_output.rs new file mode 100644 index 0000000..ad4e933 --- /dev/null +++ b/src-tauri/src/pdf_engine/validate_output.rs @@ -0,0 +1,630 @@ +//! Fail-closed publish gate for a staged PDF, before `replace_file`. +//! +//! `qpdf --check` exit policy (V4): +//! - `0` → clean (`QpdfCheckClass::Ok`) +//! - `3` → warnings only (`QpdfCheckClass::Warning`); do not block publish; +//! keep stderr on [`ValidationResult::warnings`] +//! - `2` or any other nonzero → fatal (`QpdfCheckClass::Fatal`) +//! +//! On fatal validation: do not publish; delete the staged `.offpdf-*.pdf.tmp`; +//! leave the source PDF and any existing destination bytes untouched. + +use crate::error::AppError; +use crate::pdf_engine::crop; +use lopdf::Document; +use std::path::Path; +use std::sync::atomic::{AtomicBool, Ordering}; + +/// Per-page geometry the gate compares to the reopened staged file. +#[derive(Debug, Clone, PartialEq)] +pub struct PageSnapshot { + pub media_box: [f64; 4], + pub crop_box: Option<[f64; 4]>, + pub trim_box: Option<[f64; 4]>, + pub rotate: i64, + pub user_unit: f64, +} + +/// Catalog / trailer structures the source had and the staged file must keep. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct CatalogFlags { + pub outlines: bool, + pub info: bool, + pub acro_form: bool, + pub annots: bool, +} + +/// Expected output after overlay: page order is `pages` order (count = `pages.len()`). +#[derive(Debug, Clone, PartialEq)] +pub struct OutputSnapshot { + pub pages: Vec, + pub catalog: CatalogFlags, +} + +/// Non-fatal findings from a passed gate (qpdf `--check` exit 3). +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub struct ValidationResult { + pub warnings: Vec, +} + +/// Classification of a `qpdf --check` process result. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum QpdfCheckClass { + Ok, + Warning, + Fatal, +} + +/// Classify `qpdf --check` from its exit code. `stderr` is recorded later on warnings. +pub fn classify_qpdf_check(exit: i32, stderr: &str) -> QpdfCheckClass { + let _ = stderr; + match exit { + 0 => QpdfCheckClass::Ok, + 3 => QpdfCheckClass::Warning, + _ => QpdfCheckClass::Fatal, + } +} + +fn boxes_near(a: [f64; 4], b: [f64; 4]) -> bool { + a.iter().zip(b.iter()).all(|(x, y)| (x - y).abs() < 0.5) +} + +fn opt_boxes_near(a: Option<[f64; 4]>, b: Option<[f64; 4]>) -> bool { + match (a, b) { + (None, None) => true, + (Some(x), Some(y)) => boxes_near(x, y), + _ => false, + } +} + +fn invalid_output(message: impl Into) -> AppError { + AppError::new( + "INVALID_OUTPUT", + "The edited PDF is not valid", + message, + ) + .with_suggestion("The original file was not changed. Try saving again.") +} + +fn fatal_staged(staged: &Path, message: impl Into) -> AppError { + let _ = std::fs::remove_file(staged); + invalid_output(message) +} + +pub(crate) fn catalog_flags_from_doc(doc: &Document) -> CatalogFlags { + CatalogFlags { + outlines: has_catalog_key(doc, b"Outlines"), + info: doc.trailer.get(b"Info").is_ok(), + acro_form: has_catalog_key(doc, b"AcroForm"), + annots: has_any_annots(doc), + } +} + +fn catalog_dict(doc: &Document) -> Option<&lopdf::Dictionary> { + let root = doc.trailer.get(b"Root").ok()?.as_reference().ok()?; + doc.get_dictionary(root).ok() +} + +fn has_catalog_key(doc: &Document, key: &[u8]) -> bool { + catalog_dict(doc).and_then(|c| c.get(key).ok()).is_some() +} + +fn has_any_annots(doc: &Document) -> bool { + doc.get_pages().values().any(|id| { + doc.get_dictionary(*id) + .ok() + .and_then(|d| d.get(b"Annots").ok()) + .is_some() + }) +} + +/// Validate a dest-sibling staged PDF against `snapshot` using `run_check` for `qpdf --check`. +/// +/// `run_check` receives an argv array (no shell), typically `["--check", ]`, +/// and returns `(exit, stderr)`. +pub fn validate_staged_pdf( + staged: &Path, + snapshot: &OutputSnapshot, + cancel: Option<&AtomicBool>, + mut run_check: impl FnMut(&[String]) -> Result<(i32, String), AppError>, +) -> Result { + if cancel.is_some_and(|c| c.load(Ordering::SeqCst)) { + return Err(AppError::cancelled()); + } + + let staged_arg = staged.to_string_lossy().into_owned(); + let args = ["--check".to_string(), staged_arg]; + let (exit, stderr) = run_check(&args)?; + + let mut warnings = Vec::new(); + match classify_qpdf_check(exit, &stderr) { + QpdfCheckClass::Fatal => { + return Err(fatal_staged( + staged, + if stderr.trim().is_empty() { + "qpdf --check reported errors in the edited PDF.".to_string() + } else { + format!("qpdf --check reported errors: {}", stderr.trim()) + }, + )); + } + QpdfCheckClass::Warning => warnings.push(stderr), + QpdfCheckClass::Ok => {} + } + + let doc = match Document::load(staged) { + Ok(d) => d, + Err(e) => { + return Err(fatal_staged( + staged, + format!("The edited PDF could not be reopened ({e})."), + )); + } + }; + + let page_map = doc.get_pages(); + if page_map.len() != snapshot.pages.len() { + return Err(fatal_staged( + staged, + format!( + "Page count changed: expected {}, found {}.", + snapshot.pages.len(), + page_map.len() + ), + )); + } + + for (i, expected) in snapshot.pages.iter().enumerate() { + let page_no = (i as u32) + 1; + let Some(&id) = page_map.get(&page_no) else { + return Err(fatal_staged( + staged, + format!("Page {page_no} is missing from the edited PDF."), + )); + }; + + if !boxes_near(crop::media_box(&doc, id), expected.media_box) { + return Err(fatal_staged( + staged, + format!("Page {page_no} MediaBox does not match the source."), + )); + } + if !opt_boxes_near(crop::crop_box(&doc, id), expected.crop_box) { + return Err(fatal_staged( + staged, + format!("Page {page_no} CropBox does not match the source."), + )); + } + if !opt_boxes_near(crop::page_trim_box(&doc, id), expected.trim_box) { + return Err(fatal_staged( + staged, + format!("Page {page_no} TrimBox does not match the source."), + )); + } + if crop::page_rotation(&doc, id) != expected.rotate { + return Err(fatal_staged( + staged, + format!("Page {page_no} /Rotate does not match the source."), + )); + } + if (crop::page_user_unit(&doc, id) - expected.user_unit).abs() > 0.0001 { + return Err(fatal_staged( + staged, + format!("Page {page_no} /UserUnit does not match the source."), + )); + } + } + + if snapshot.catalog.outlines && !has_catalog_key(&doc, b"Outlines") { + return Err(fatal_staged( + staged, + "Bookmarks (Outlines) are missing from the edited PDF.", + )); + } + if snapshot.catalog.info && doc.trailer.get(b"Info").is_err() { + return Err(fatal_staged( + staged, + "Document Info metadata is missing from the edited PDF.", + )); + } + if snapshot.catalog.acro_form && !has_catalog_key(&doc, b"AcroForm") { + return Err(fatal_staged( + staged, + "AcroForm is missing from the edited PDF.", + )); + } + if snapshot.catalog.annots && !has_any_annots(&doc) { + return Err(fatal_staged( + staged, + "Page annotations are missing from the edited PDF.", + )); + } + + Ok(ValidationResult { warnings }) +} + +#[cfg(test)] +mod tests { + use super::*; + use lopdf::{Dictionary, Document, Object, Stream}; + use std::path::{Path, PathBuf}; + + struct Scratch(PathBuf); + + impl Scratch { + fn new(name: &str) -> Self { + let dir = std::env::temp_dir().join(format!( + "offpdf-validate-{}-{}-{}", + name, + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0) + )); + std::fs::create_dir_all(&dir).unwrap(); + Self(dir) + } + + fn path(&self) -> &Path { + &self.0 + } + } + + impl Drop for Scratch { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + fn box_obj(b: [i64; 4]) -> Object { + Object::Array(b.into_iter().map(Object::Integer).collect()) + } + + fn write_one_page_pdf(path: &Path, extras: &[(&[u8], Object)]) { + let mut doc = Document::with_version("1.5"); + let pages_id = doc.new_object_id(); + let content_id = doc.add_object(Object::Stream(Stream::new( + Dictionary::new(), + b"BT /F1 12 Tf 72 720 Td (Hello) Tj ET".to_vec(), + ))); + let mut page = Dictionary::new(); + page.set("Type", "Page"); + page.set("Parent", pages_id); + page.set("MediaBox", box_obj([0, 0, 612, 792])); + page.set("Contents", content_id); + for (k, v) in extras { + page.set(*k, v.clone()); + } + let page_id = doc.add_object(Object::Dictionary(page)); + + let mut pages = Dictionary::new(); + pages.set("Type", "Pages"); + pages.set("Kids", vec![page_id.into()]); + pages.set("Count", 1); + doc.objects.insert(pages_id, Object::Dictionary(pages)); + + let mut catalog = Dictionary::new(); + catalog.set("Type", "Catalog"); + catalog.set("Pages", pages_id); + let catalog_id = doc.add_object(Object::Dictionary(catalog)); + doc.trailer.set("Root", catalog_id); + doc.save(path).expect("write one-page fixture"); + } + + fn letter_page() -> PageSnapshot { + PageSnapshot { + media_box: [0.0, 0.0, 612.0, 792.0], + crop_box: None, + trim_box: None, + rotate: 0, + user_unit: 1.0, + } + } + + fn empty_catalog() -> CatalogFlags { + CatalogFlags { + outlines: false, + info: false, + acro_form: false, + annots: false, + } + } + + fn letter_snapshot() -> OutputSnapshot { + OutputSnapshot { + pages: vec![letter_page()], + catalog: empty_catalog(), + } + } + + fn assert_invalid_output(err: &AppError) { + assert_eq!(err.code, "INVALID_OUTPUT"); + assert!( + !err.title.trim().is_empty(), + "INVALID_OUTPUT must have a title" + ); + assert!( + !err.message.trim().is_empty(), + "INVALID_OUTPUT must have a message" + ); + assert!( + err.suggestion + .as_deref() + .map(|s| !s.trim().is_empty()) + .unwrap_or(false), + "INVALID_OUTPUT must have a suggestion" + ); + } + + // --- V4 ----------------------------------------------------------------- + + #[test] + fn classify_qpdf_check_exit_0_is_ok() { + assert_eq!( + classify_qpdf_check(0, ""), + QpdfCheckClass::Ok, + "exit 0 must be Ok" + ); + } + + #[test] + fn classify_qpdf_check_exit_3_is_warning() { + assert_eq!( + classify_qpdf_check(3, "WARNING: linearized"), + QpdfCheckClass::Warning, + "exit 3 must be Warning" + ); + } + + #[test] + fn classify_qpdf_check_exit_2_is_fatal() { + assert_eq!( + classify_qpdf_check(2, "ERROR: damaged"), + QpdfCheckClass::Fatal, + "exit 2 must be Fatal" + ); + } + + #[test] + fn classify_qpdf_check_other_nonzero_is_fatal() { + assert_eq!( + classify_qpdf_check(1, "unexpected"), + QpdfCheckClass::Fatal, + "other nonzero must be Fatal" + ); + assert_eq!( + classify_qpdf_check(99, "other"), + QpdfCheckClass::Fatal, + "other nonzero must be Fatal" + ); + } + + #[test] + fn validate_qpdf_exit_3_records_warning() { + let scratch = Scratch::new("v4-warn"); + let staged = scratch.path().join("staged.pdf"); + write_one_page_pdf(&staged, &[]); + let snapshot = letter_snapshot(); + let result = validate_staged_pdf(&staged, &snapshot, None, |_args| { + Ok((3, "WARNING: file has warnings\n".into())) + }) + .expect("V4: qpdf --check exit 3 must not block"); + assert!( + result.warnings.iter().any(|w| w.contains("WARNING")), + "V4: exit 3 stderr must be recorded on ValidationResult.warnings; got {:?}", + result.warnings + ); + } + + // --- V1 / V8 ------------------------------------------------------------ + + #[test] + fn validate_staged_pdf_is_reachable_and_leaves_dest_untouched() { + // V8: public path crate::pdf_engine::validate_output::validate_staged_pdf + let scratch = Scratch::new("v1-v8"); + let staged = scratch.path().join("staged.pdf"); + let dest = scratch.path().join("out.pdf"); + write_one_page_pdf(&staged, &[]); + std::fs::write(&dest, b"OLD-DEST").unwrap(); + let dest_mtime = std::fs::metadata(&dest).unwrap().modified().unwrap(); + let snapshot = letter_snapshot(); + + let result = crate::pdf_engine::validate_output::validate_staged_pdf( + &staged, + &snapshot, + None, + |_| Ok((0, String::new())), + ); + + assert!( + result.is_ok(), + "V1: matching snapshot + check exit 0 is not fatal; {result:?}" + ); + assert_eq!( + std::fs::read(&dest).unwrap(), + b"OLD-DEST", + "V1: gate must not publish; dest bytes must stay OLD" + ); + assert_eq!( + std::fs::metadata(&dest).unwrap().modified().unwrap(), + dest_mtime, + "V1: dest mtime must stay unchanged" + ); + } + + #[test] + fn validate_err_means_caller_must_not_publish() { + // Caller contract around the gate (export runner is impl's job). + let scratch = Scratch::new("v1-no-publish"); + let dest = scratch.path().join("out.pdf"); + std::fs::write(&dest, b"OLD-DEST").unwrap(); + let gate: Result = Err(AppError::new( + "INVALID_OUTPUT", + "The edited PDF is not valid", + "Validation rejected the staged file.", + ) + .with_suggestion("Try saving again, or pick a different destination.")); + if gate.is_err() { + // do not replace dest + } else { + std::fs::write(&dest, b"NEW-PUBLISHED").unwrap(); + } + assert_eq!(std::fs::read(&dest).unwrap(), b"OLD-DEST"); + } + + // --- V2 ----------------------------------------------------------------- + + #[test] + fn validate_truncated_staged_pdf_is_invalid_output() { + let scratch = Scratch::new("v2-trunc"); + let staged = scratch.path().join("staged.pdf"); + let dest = scratch.path().join("out.pdf"); + std::fs::write(&staged, b"%PDF-1.4\n%% truncated").unwrap(); + std::fs::write(&dest, b"OLD-DEST").unwrap(); + let dest_mtime = std::fs::metadata(&dest).unwrap().modified().unwrap(); + let snapshot = letter_snapshot(); + + let result = validate_staged_pdf(&staged, &snapshot, None, |_args| { + Ok((2, "qpdf --check: file is damaged".into())) + }); + let err = result.expect_err("V2: truncated staged PDF must be INVALID_OUTPUT"); + assert_invalid_output(&err); + assert_eq!( + std::fs::read(&dest).unwrap(), + b"OLD-DEST", + "V2: dest bytes must stay OLD" + ); + assert_eq!( + std::fs::metadata(&dest).unwrap().modified().unwrap(), + dest_mtime, + "V2: dest mtime must stay unchanged" + ); + } + + #[test] + fn validate_truncated_does_not_create_dest() { + let scratch = Scratch::new("v2-nodest"); + let staged = scratch.path().join("staged.pdf"); + let dest = scratch.path().join("out.pdf"); + std::fs::write(&staged, b"%PDF-1.4\n%% truncated").unwrap(); + let snapshot = letter_snapshot(); + + let result = validate_staged_pdf(&staged, &snapshot, None, |_args| { + Ok((2, "qpdf --check: file is damaged".into())) + }); + let err = result.expect_err("V2: truncated staged PDF must be INVALID_OUTPUT"); + assert_invalid_output(&err); + assert!( + !dest.exists(), + "V2: dest must not be created when it did not exist" + ); + } + + // --- V3 ----------------------------------------------------------------- + + #[test] + fn validate_fatal_deletes_staging_leaves_source_and_dest() { + let scratch = Scratch::new("v3-cleanup"); + let source = scratch.path().join("source.pdf"); + let dest = scratch.path().join("out.pdf"); + let staged = scratch.path().join(".offpdf-job.pdf.tmp"); + std::fs::write(&source, b"SOURCE-BYTES").unwrap(); + std::fs::write(&dest, b"OLD-DEST").unwrap(); + std::fs::write(&staged, b"%PDF-1.4\n%% truncated").unwrap(); + let snapshot = letter_snapshot(); + + let result = validate_staged_pdf(&staged, &snapshot, None, |_args| { + Ok((2, "qpdf --check: file is damaged".into())) + }); + + assert!( + !staged.exists(), + "V3: fatal validate must delete staged .offpdf-*.pdf.tmp" + ); + assert_eq!( + std::fs::read(&source).unwrap(), + b"SOURCE-BYTES", + "V3: source bytes must be unchanged" + ); + assert_eq!( + std::fs::read(&dest).unwrap(), + b"OLD-DEST", + "V3: dest bytes must be unchanged" + ); + let err = result.expect_err("V3: truncated staging must be INVALID_OUTPUT"); + assert_invalid_output(&err); + } + + // --- V5 ----------------------------------------------------------------- + + #[test] + fn validate_page_count_mismatch_is_invalid_output() { + let scratch = Scratch::new("v5-pages"); + let staged = scratch.path().join("staged.pdf"); + let dest = scratch.path().join("out.pdf"); + write_one_page_pdf(&staged, &[]); + std::fs::write(&dest, b"OLD-DEST").unwrap(); + let snapshot = OutputSnapshot { + pages: vec![letter_page(), letter_page()], + catalog: empty_catalog(), + }; + + let result = validate_staged_pdf(&staged, &snapshot, None, |_args| Ok((0, String::new()))); + let err = result.expect_err("V5: page-count mismatch must be INVALID_OUTPUT"); + assert_invalid_output(&err); + assert_eq!(std::fs::read(&dest).unwrap(), b"OLD-DEST"); + } + + #[test] + fn validate_cropbox_mismatch_is_invalid_output() { + let scratch = Scratch::new("v5-crop"); + let staged = scratch.path().join("staged.pdf"); + let dest = scratch.path().join("out.pdf"); + write_one_page_pdf(&staged, &[(b"CropBox", box_obj([0, 0, 612, 792]))]); + std::fs::write(&dest, b"OLD-DEST").unwrap(); + let snapshot = OutputSnapshot { + pages: vec![PageSnapshot { + media_box: [0.0, 0.0, 612.0, 792.0], + crop_box: Some([72.0, 72.0, 540.0, 720.0]), + trim_box: None, + rotate: 0, + user_unit: 1.0, + }], + catalog: empty_catalog(), + }; + + let result = validate_staged_pdf(&staged, &snapshot, None, |_args| Ok((0, String::new()))); + let err = result.expect_err("V5: CropBox mismatch must be INVALID_OUTPUT"); + assert_invalid_output(&err); + assert_eq!(std::fs::read(&dest).unwrap(), b"OLD-DEST"); + } + + // --- V6 ----------------------------------------------------------------- + + #[test] + fn validate_missing_catalog_keys_is_invalid_output() { + let scratch = Scratch::new("v6-catalog"); + let staged = scratch.path().join("staged.pdf"); + let dest = scratch.path().join("out.pdf"); + // Valid one-page PDF: no Outlines, no Info, no AcroForm, no Annots. + write_one_page_pdf(&staged, &[]); + std::fs::write(&dest, b"OLD-DEST").unwrap(); + let snapshot = OutputSnapshot { + pages: vec![letter_page()], + catalog: CatalogFlags { + outlines: true, + info: true, + acro_form: true, + annots: true, + }, + }; + + let result = validate_staged_pdf(&staged, &snapshot, None, |_args| Ok((0, String::new()))); + let err = result.expect_err( + "V6: snapshot Outlines+Info+AcroForm+Annots missing on staged file must be INVALID_OUTPUT", + ); + assert_invalid_output(&err); + assert_eq!(std::fs::read(&dest).unwrap(), b"OLD-DEST"); + } +} From 691de0110fb0024ece33e209492606d366a385af Mon Sep 17 00:00:00 2001 From: nonamexishere Date: Mon, 24 Aug 2026 16:58:37 +0300 Subject: [PATCH 2/3] fix(edit): cancel after --check does not publish; track check child Re-check cancel after qpdf --check and before publish so a mid-gate Cancel returns CANCELLED, deletes staging, and does not replace dest. Production --check registers on the job handle like overlay qpdf. --- src-tauri/src/pdf_engine/edit_overlay.rs | 20 +++++-- .../src/pdf_engine/edit_overlay_integ.rs | 2 - src-tauri/src/pdf_engine/validate_output.rs | 58 +++++++++++++++++-- 3 files changed, 70 insertions(+), 10 deletions(-) diff --git a/src-tauri/src/pdf_engine/edit_overlay.rs b/src-tauri/src/pdf_engine/edit_overlay.rs index 8234929..ffe6370 100644 --- a/src-tauri/src/pdf_engine/edit_overlay.rs +++ b/src-tauri/src/pdf_engine/edit_overlay.rs @@ -8,7 +8,7 @@ use crate::pdf_engine::validate_output::{ catalog_flags_from_doc, validate_staged_pdf, OutputSnapshot, PageSnapshot, }; use crate::pdf_engine::{crop, edit_image, qpdf}; -use crate::utils::process::run_qpdf; +use crate::utils::process::{run_qpdf, run_tracked}; use crate::utils::safe_output; use crate::utils::temp; use lopdf::{Document, Object}; @@ -1107,7 +1107,7 @@ where { let exe = qpdf::resolve_qpdf_standalone(); export_edit_pdf_with_check_exe( - groups, output, document, font_path, work, unique, cancel, &exe, run, + groups, output, document, font_path, work, unique, cancel, &exe, None, run, ) } @@ -1121,6 +1121,7 @@ fn export_edit_pdf_with_check_exe( unique: &str, cancel: Option<&AtomicBool>, qpdf_check: &Path, + handle: Option<&Arc>, mut run: F, ) -> Result, AppError> where @@ -1165,7 +1166,7 @@ where } let snapshot = output_snapshot_from_source(&geoms, Path::new(&groups[0].path))?; validate_staged_pdf(&tmp, &snapshot, cancel, |args| { - run_qpdf_check_argv(qpdf_check, args) + run_qpdf_check_argv(qpdf_check, args, handle) })?; gate_passed = true; safe_output::replace_file(&tmp, dest)?; @@ -1200,7 +1201,11 @@ fn output_snapshot_from_source( }) } -fn run_qpdf_check_argv(exe: &Path, args: &[String]) -> Result<(i32, String), AppError> { +fn run_qpdf_check_argv( + exe: &Path, + args: &[String], + handle: Option<&Arc>, +) -> Result<(i32, String), AppError> { let mut cmd = std::process::Command::new(exe); cmd.args(args); #[cfg(windows)] @@ -1208,6 +1213,12 @@ fn run_qpdf_check_argv(exe: &Path, args: &[String]) -> Result<(i32, String), App use std::os::windows::process::CommandExt; cmd.creation_flags(0x08000000); } + if let Some(h) = handle { + cmd.stdout(std::process::Stdio::null()); + cmd.stderr(std::process::Stdio::piped()); + let (status, stderr) = run_tracked(h, cmd)?; + return Ok((status.and_then(|s| s.code()).unwrap_or(1), stderr)); + } let output = cmd .output() .map_err(|e| AppError::io("qpdf --check failed to start", e))?; @@ -1261,6 +1272,7 @@ pub fn edit_pdf_overlays( job_id, Some(&handle.cancelled), &qpdf_exe, + Some(handle), |args| run_qpdf(app, handle, job_id, args, "Saving", None), ) })(); diff --git a/src-tauri/src/pdf_engine/edit_overlay_integ.rs b/src-tauri/src/pdf_engine/edit_overlay_integ.rs index 62ebab5..88845eb 100644 --- a/src-tauri/src/pdf_engine/edit_overlay_integ.rs +++ b/src-tauri/src/pdf_engine/edit_overlay_integ.rs @@ -436,7 +436,6 @@ fn write_tiny_png(path: &Path, w: u32, h: u32) { #[test] fn integ_catalog_survives_and_original_stream_stays() { - // V7 keepGreen let Some(fx) = Harness::new("catalog") else { eprintln!("skip: qpdf not available"); return; @@ -465,7 +464,6 @@ fn integ_catalog_survives_and_original_stream_stays() { assert!(cat.get(b"AcroForm").is_ok(), "AcroForm missing"); let blob = dump_streams(&fx.dest); - // V7 keepGreen assert!(blob.contains("Hello"), "original page stream rasterized away: {blob:?}"); fx.cleanup(); } diff --git a/src-tauri/src/pdf_engine/validate_output.rs b/src-tauri/src/pdf_engine/validate_output.rs index ad4e933..ea382a8 100644 --- a/src-tauri/src/pdf_engine/validate_output.rs +++ b/src-tauri/src/pdf_engine/validate_output.rs @@ -91,6 +91,14 @@ fn fatal_staged(staged: &Path, message: impl Into) -> AppError { invalid_output(message) } +fn abort_if_cancelled(staged: &Path, cancel: Option<&AtomicBool>) -> Result<(), AppError> { + if cancel.is_some_and(|c| c.load(Ordering::SeqCst)) { + let _ = std::fs::remove_file(staged); + return Err(AppError::cancelled()); + } + Ok(()) +} + pub(crate) fn catalog_flags_from_doc(doc: &Document) -> CatalogFlags { CatalogFlags { outlines: has_catalog_key(doc, b"Outlines"), @@ -128,13 +136,19 @@ pub fn validate_staged_pdf( cancel: Option<&AtomicBool>, mut run_check: impl FnMut(&[String]) -> Result<(i32, String), AppError>, ) -> Result { - if cancel.is_some_and(|c| c.load(Ordering::SeqCst)) { - return Err(AppError::cancelled()); - } + abort_if_cancelled(staged, cancel)?; let staged_arg = staged.to_string_lossy().into_owned(); let args = ["--check".to_string(), staged_arg]; - let (exit, stderr) = run_check(&args)?; + let (exit, stderr) = match run_check(&args) { + Ok(v) => v, + Err(e) if e.code == "CANCELLED" => { + let _ = std::fs::remove_file(staged); + return Err(AppError::cancelled()); + } + Err(e) => return Err(e), + }; + abort_if_cancelled(staged, cancel)?; let mut warnings = Vec::new(); match classify_qpdf_check(exit, &stderr) { @@ -240,6 +254,7 @@ pub fn validate_staged_pdf( )); } + abort_if_cancelled(staged, cancel)?; Ok(ValidationResult { warnings }) } @@ -248,6 +263,7 @@ mod tests { use super::*; use lopdf::{Dictionary, Document, Object, Stream}; use std::path::{Path, PathBuf}; + use std::sync::atomic::{AtomicBool, Ordering}; struct Scratch(PathBuf); @@ -627,4 +643,38 @@ mod tests { assert_invalid_output(&err); assert_eq!(std::fs::read(&dest).unwrap(), b"OLD-DEST"); } + + // --- C1 ----------------------------------------------------------------- + + #[test] + fn validate_cancel_after_check_does_not_pass() { + let scratch = Scratch::new("c1-cancel"); + let dest = scratch.path().join("out.pdf"); + let staged = scratch.path().join(".offpdf-job.pdf.tmp"); + write_one_page_pdf(&staged, &[]); + std::fs::write(&dest, b"OLD-DEST").unwrap(); + let snapshot = letter_snapshot(); + let cancel = AtomicBool::new(false); + + let result = validate_staged_pdf(&staged, &snapshot, Some(&cancel), |_args| { + cancel.store(true, Ordering::SeqCst); + Ok((0, String::new())) + }); + + let err = result.expect_err("C1: cancel after successful --check must be CANCELLED"); + assert_eq!( + err.code, "CANCELLED", + "C1: cancel after check must be CANCELLED, not {}", + err.code + ); + assert_eq!( + std::fs::read(&dest).unwrap(), + b"OLD-DEST", + "C1: dest bytes must stay OLD-DEST" + ); + assert!( + !staged.exists(), + "C1: cancel must delete staged .offpdf-*.pdf.tmp" + ); + } } From 25e1445d65cd08444cc4c911db1989c7d5ea9c17 Mon Sep 17 00:00:00 2001 From: nonamexishere Date: Wed, 26 Aug 2026 16:01:51 +0300 Subject: [PATCH 3/3] fix(edit): detect swapped same-size pages and surface qpdf check warnings Page identity now uses a per-page Contents digest so two same-geometry pages cannot be swapped past the publish gate. qpdf --check exit 3 still publishes; the warning text is kept on the completed job update. --- CHANGELOG.md | 2 +- src-tauri/src/commands/pdf.rs | 33 ++- src-tauri/src/models.rs | 1 - src-tauri/src/pdf_engine/edit_overlay.rs | 23 +- .../src/pdf_engine/edit_overlay_integ.rs | 210 +++++++++++++- src-tauri/src/pdf_engine/validate_output.rs | 264 +++++++++++++++++- src/components/jobs/JobStatus.test.tsx | 56 ++++ src/components/jobs/JobStatus.tsx | 1 + 8 files changed, 576 insertions(+), 14 deletions(-) create mode 100644 src/components/jobs/JobStatus.test.tsx diff --git a/CHANGELOG.md b/CHANGELOG.md index 01aa4e0..c40aa33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable project changes should be documented here. ### Added -- Edit PDF: staged output is checked with `qpdf --check` and reopened for page boxes and catalog data before the destination file is replaced. A failed check leaves the original and any existing destination untouched. +- Edit PDF: staged output is checked with `qpdf --check` and reopened for page boxes, per-page content identity, and catalog data before the destination file is replaced. A failed check leaves the original and any existing destination untouched. `qpdf --check` warnings (exit 3) still publish and appear on the completed job update. ### Fixed diff --git a/src-tauri/src/commands/pdf.rs b/src-tauri/src/commands/pdf.rs index 4f6e885..8fae93e 100644 --- a/src-tauri/src/commands/pdf.rs +++ b/src-tauri/src/commands/pdf.rs @@ -11,7 +11,21 @@ use tauri::Emitter; /// Helper: emit the final "completed" update and build the JobResult. fn completed(app: &tauri::AppHandle, job_id: String, output_paths: Vec) -> JobResult { - let _ = app.emit("job:update", JobUpdate::new(&job_id, "completed", "Done")); + completed_with_message(app, job_id, output_paths, None) +} + +/// Same as [`completed`], with an optional inspectable `JobUpdate.message`. +fn completed_with_message( + app: &tauri::AppHandle, + job_id: String, + output_paths: Vec, + message: Option, +) -> JobResult { + let mut update = JobUpdate::new(&job_id, "completed", "Done"); + if let Some(m) = message.filter(|s| !s.is_empty()) { + update = update.message(m); + } + let _ = app.emit("job:update", update); JobResult { job_id, output_paths, @@ -194,8 +208,21 @@ pub async fn edit_pdf_overlays( .await .map_err(|e| AppError::engine_failed(format!("worker join error: {e}")))?; registry.remove(&job_id); - let output_paths = res?; - Ok(completed(&app, job_id, output_paths)) + let (output_paths, warnings) = res?; + let message = { + let joined = warnings + .iter() + .map(|w| w.trim()) + .filter(|w| !w.is_empty()) + .collect::>() + .join("\n"); + if joined.is_empty() { + None + } else { + Some(joined) + } + }; + Ok(completed_with_message(&app, job_id, output_paths, message)) } #[tauri::command] diff --git a/src-tauri/src/models.rs b/src-tauri/src/models.rs index 0097d0f..4b2a5af 100644 --- a/src-tauri/src/models.rs +++ b/src-tauri/src/models.rs @@ -97,7 +97,6 @@ impl JobUpdate { self.percent = Some(p); self } - #[allow(dead_code)] // Part of the JobUpdate builder API. pub fn message(mut self, m: impl Into) -> Self { self.message = Some(m.into()); self diff --git a/src-tauri/src/pdf_engine/edit_overlay.rs b/src-tauri/src/pdf_engine/edit_overlay.rs index ffe6370..e503008 100644 --- a/src-tauri/src/pdf_engine/edit_overlay.rs +++ b/src-tauri/src/pdf_engine/edit_overlay.rs @@ -5,7 +5,8 @@ use crate::error::AppError; use crate::models::{JobHandle, PageGroup}; use crate::pdf_engine::validate_output::{ - catalog_flags_from_doc, validate_staged_pdf, OutputSnapshot, PageSnapshot, + catalog_flags_from_doc, content_digest, validate_staged_pdf, ContentDigest, OutputSnapshot, + PageSnapshot, }; use crate::pdf_engine::{crop, edit_image, qpdf}; use crate::utils::process::{run_qpdf, run_tracked}; @@ -799,6 +800,7 @@ struct OverlayPageGeom { trim: Option<[f64; 4]>, rotate: i64, user_unit: f64, + content_digest: ContentDigest, } fn boxes_near(a: [f64; 4], b: [f64; 4]) -> bool { @@ -1038,6 +1040,9 @@ fn collect_source_pages( format!("Page {p} is not in this PDF."), ) })?; + let content = doc.get_page_content(id).map_err(|e| { + AppError::engine_failed(format!("Could not read page content: {e}")) + })?; geoms.push(OverlayPageGeom { visible: crop::visible_box(doc, id), media: crop::media_box(doc, id), @@ -1045,6 +1050,7 @@ fn collect_source_pages( trim: crop::page_trim_box(doc, id), rotate: crop::page_rotation(doc, id), user_unit: crop::page_user_unit(doc, id), + content_digest: content_digest(&content), }); } } @@ -1109,6 +1115,7 @@ where export_edit_pdf_with_check_exe( groups, output, document, font_path, work, unique, cancel, &exe, None, run, ) + .map(|(paths, _warnings)| paths) } /// Same as [`export_edit_pdf_with_runner`], with an explicit `qpdf --check` binary. @@ -1123,7 +1130,7 @@ fn export_edit_pdf_with_check_exe( qpdf_check: &Path, handle: Option<&Arc>, mut run: F, -) -> Result, AppError> +) -> Result<(Vec, Vec), AppError> where F: FnMut(&[String]) -> Result<(), AppError>, { @@ -1144,7 +1151,7 @@ where let overlay = work.join("overlay.pdf"); let overlay_str = overlay.to_string_lossy().to_string(); let mut gate_passed = false; - let result = (|| -> Result, AppError> { + let result = (|| -> Result<(Vec, Vec), AppError> { let (geoms, counts) = collect_source_pages(groups)?; if geoms.is_empty() { return Err(AppError::new("NO_PAGES", "No pages", "Add a PDF first.")); @@ -1165,12 +1172,12 @@ where safe_output::replace_file(&cleaned, &tmp)?; } let snapshot = output_snapshot_from_source(&geoms, Path::new(&groups[0].path))?; - validate_staged_pdf(&tmp, &snapshot, cancel, |args| { + let vr = validate_staged_pdf(&tmp, &snapshot, cancel, |args| { run_qpdf_check_argv(qpdf_check, args, handle) })?; gate_passed = true; safe_output::replace_file(&tmp, dest)?; - Ok(vec![output.to_string()]) + Ok((vec![output.to_string()], vr.warnings)) })(); // Keep tmp only if replace_file failed after a passed gate (Windows recover). // Spawn/validate errors (and leftover success tmp) delete the sibling. @@ -1195,6 +1202,7 @@ fn output_snapshot_from_source( trim_box: g.trim, rotate: g.rotate, user_unit: g.user_unit, + content_digest: g.content_digest, }) .collect(), catalog: catalog_flags_from_doc(&doc), @@ -1235,7 +1243,7 @@ pub fn edit_pdf_overlays( groups: &[PageGroup], output: &str, document: &EditDocumentIn, -) -> Result, AppError> { +) -> Result<(Vec, Vec), AppError> { if groups.is_empty() { return Err(AppError::new("NO_PAGES", "No pages", "Add a PDF first.")); } @@ -1258,7 +1266,7 @@ pub fn edit_pdf_overlays( .map_err(|e| AppError::io("Could not create a temp directory.", e))?; let font_path = find_font_path(app)?; - let result = (|| -> Result, AppError> { + let result = (|| -> Result<(Vec, Vec), AppError> { if handle.is_cancelled() { return Err(AppError::cancelled()); } @@ -2001,6 +2009,7 @@ mod tests { trim: None, rotate: 0, user_unit: 1.0, + content_digest: content_digest(b"BT /F1 12 Tf 72 720 Td (Hello) Tj ET"), } } diff --git a/src-tauri/src/pdf_engine/edit_overlay_integ.rs b/src-tauri/src/pdf_engine/edit_overlay_integ.rs index 88845eb..a655776 100644 --- a/src-tauri/src/pdf_engine/edit_overlay_integ.rs +++ b/src-tauri/src/pdf_engine/edit_overlay_integ.rs @@ -167,6 +167,152 @@ fn write_catalog_fixture(path: &Path) { doc.save(path).expect("write catalog fixture"); } +fn write_two_page_letter(path: &Path, label1: &str, label2: &str) { + let mut doc = Document::with_version("1.5"); + let pages_id = doc.new_object_id(); + let content1 = doc.add_object(Object::Stream(Stream::new( + Dictionary::new(), + format!("BT /F1 12 Tf 72 720 Td ({label1}) Tj ET").into_bytes(), + ))); + let content2 = doc.add_object(Object::Stream(Stream::new( + Dictionary::new(), + format!("BT /F1 12 Tf 72 720 Td ({label2}) Tj ET").into_bytes(), + ))); + + let mut page1 = Dictionary::new(); + page1.set("Type", "Page"); + page1.set("Parent", pages_id); + page1.set("MediaBox", box_obj([0, 0, 612, 792])); + page1.set("Contents", content1); + let page1_id = doc.add_object(Object::Dictionary(page1)); + + let mut page2 = Dictionary::new(); + page2.set("Type", "Page"); + page2.set("Parent", pages_id); + page2.set("MediaBox", box_obj([0, 0, 612, 792])); + page2.set("Contents", content2); + let page2_id = doc.add_object(Object::Dictionary(page2)); + + let mut pages = Dictionary::new(); + pages.set("Type", "Pages"); + pages.set("Kids", vec![page1_id.into(), page2_id.into()]); + pages.set("Count", 2); + doc.objects.insert(pages_id, Object::Dictionary(pages)); + + let mut catalog = Dictionary::new(); + catalog.set("Type", "Catalog"); + catalog.set("Pages", pages_id); + let catalog_id = doc.add_object(Object::Dictionary(catalog)); + doc.trailer.set("Root", catalog_id); + doc.save(path).expect("write two-page letter fixture"); +} + +fn write_catalog_annots_fixture( + path: &Path, + rotate: i64, + crop: Option<[i64; 4]>, + trim: Option<[i64; 4]>, +) { + let mut doc = Document::with_version("1.5"); + let pages_id = doc.new_object_id(); + let content_id = doc.add_object(Object::Stream(Stream::new( + Dictionary::new(), + b"BT /F1 12 Tf 72 720 Td (Hello) Tj ET".to_vec(), + ))); + + let mut annot = Dictionary::new(); + annot.set("Type", "Annot"); + annot.set("Subtype", "Text"); + annot.set("Rect", box_obj([72, 700, 120, 740])); + annot.set("Contents", Object::string_literal("note")); + let annot_id = doc.add_object(Object::Dictionary(annot)); + + let mut page = Dictionary::new(); + page.set("Type", "Page"); + page.set("Parent", pages_id); + page.set("MediaBox", box_obj([0, 0, 612, 792])); + if let Some(b) = crop { + page.set("CropBox", box_obj(b)); + } + if let Some(b) = trim { + page.set("TrimBox", box_obj(b)); + } + if rotate != 0 { + page.set("Rotate", Object::Integer(rotate)); + } + page.set("Contents", content_id); + page.set("Annots", vec![annot_id.into()]); + let page_id = doc.add_object(Object::Dictionary(page)); + + let mut pages = Dictionary::new(); + pages.set("Type", "Pages"); + pages.set("Kids", vec![page_id.into()]); + pages.set("Count", 1); + doc.objects.insert(pages_id, Object::Dictionary(pages)); + + let mut item = Dictionary::new(); + item.set("Title", Object::string_literal("Chapter 1")); + item.set("Dest", vec![page_id.into(), Object::Name(b"Fit".to_vec())]); + let item_id = doc.add_object(Object::Dictionary(item)); + + let mut outlines = Dictionary::new(); + outlines.set("Type", "Outlines"); + outlines.set("First", item_id); + outlines.set("Last", item_id); + outlines.set("Count", 1); + let outlines_id = doc.add_object(Object::Dictionary(outlines)); + if let Ok(Object::Dictionary(d)) = doc.get_object_mut(item_id) { + d.set("Parent", outlines_id); + } + + let mut acro = Dictionary::new(); + acro.set("Fields", Vec::::new()); + let acro_id = doc.add_object(Object::Dictionary(acro)); + + let mut catalog = Dictionary::new(); + catalog.set("Type", "Catalog"); + catalog.set("Pages", pages_id); + catalog.set("Outlines", outlines_id); + catalog.set("AcroForm", acro_id); + let catalog_id = doc.add_object(Object::Dictionary(catalog)); + doc.trailer.set("Root", catalog_id); + + let mut info = Dictionary::new(); + info.set("Title", Object::string_literal("Fixture Doc")); + info.set("Author", Object::string_literal("OffPDF")); + let info_id = doc.add_object(Object::Dictionary(info)); + doc.trailer.set("Info", info_id); + doc.save(path).expect("write catalog+annots fixture"); +} + +fn assert_catalog_annots_survived(dest: &Path) { + let out = Document::load(dest).expect("load dest"); + let info = match out.trailer.get(b"Info").ok() { + Some(Object::Reference(id)) => out.get_dictionary(*id).ok().cloned(), + Some(Object::Dictionary(d)) => Some(d.clone()), + _ => None, + } + .expect("Info"); + let title = match info.get(b"Title").ok() { + Some(Object::String(b, _)) => String::from_utf8_lossy(b).into_owned(), + _ => String::new(), + }; + assert!(title.contains("Fixture Doc"), "title={title:?}"); + let root_id = out.trailer.get(b"Root").unwrap().as_reference().unwrap(); + let cat = out.get_dictionary(root_id).unwrap(); + assert!(cat.get(b"Outlines").is_ok(), "Outlines missing"); + assert!(cat.get(b"AcroForm").is_ok(), "AcroForm missing"); + let page = first_page_dict(&out); + assert!(page.get(b"Annots").is_ok(), "page /Annots missing"); +} + +fn page_rotate(dict: &Dictionary) -> i64 { + match dict.get(b"Rotate") { + Ok(Object::Integer(i)) => *i, + _ => 0, + } +} + fn dump_streams(path: &Path) -> String { let mut doc = Document::load(path).expect("load pdf"); let _ = doc.decompress(); @@ -340,10 +486,14 @@ impl Harness { } fn export(&self, document: &EditDocumentIn) -> Result<(), AppError> { + self.export_pages("1", document) + } + + fn export_pages(&self, pages: &str, document: &EditDocumentIn) -> Result<(), AppError> { let qpdf = self.qpdf.clone(); let groups = [PageGroup { path: self.src.to_string_lossy().into_owned(), - pages: "1".into(), + pages: pages.into(), }]; export_edit_pdf_with_runner( &groups, @@ -1075,3 +1225,61 @@ fn integ_user_unit_10000_copied_not_clamped() { ); fx.cleanup(); } + +#[test] +fn integ_two_page_overlay_keeps_source_streams_by_presence() { + let Some(fx) = Harness::new("r1b-twopage") else { + eprintln!("skip: qpdf not available"); + return; + }; + write_two_page_letter(&fx.src, "ALPHA-PAGE", "BETA-PAGE"); + fx.export_pages("1-z", &text_box()) + .expect("R1b: in-order two-page overlay must publish"); + assert!(fx.dest.exists(), "R1b: dest must be published"); + + let dest = Document::load(&fx.dest).expect("load dest"); + assert_eq!(dest.get_pages().len(), 2, "R1b: dest must keep two pages"); + let blob = dump_streams(&fx.dest); + assert!( + blob.contains("ALPHA-PAGE"), + "R1b: source page 1 stream must still be present after overlay (Form XObject or Contents): {blob:?}" + ); + assert!( + blob.contains("BETA-PAGE"), + "R1b: source page 2 stream must still be present after overlay (Form XObject or Contents): {blob:?}" + ); + fx.cleanup(); +} + +#[test] +fn integ_catalog_annots_survive_rotate_90() { + let Some(fx) = Harness::new("r2-rot90-annots") else { + eprintln!("skip: qpdf not available"); + return; + }; + write_catalog_annots_fixture(&fx.src, 90, None, None); + fx.export(&text_box()).expect("export catalog+annots rotate 90"); + assert_catalog_annots_survived(&fx.dest); + let page = first_page_dict(&Document::load(&fx.dest).unwrap()); + assert_eq!(page_rotate(&page), 90, "page /Rotate 90 must survive overlay"); + fx.cleanup(); +} + +#[test] +fn integ_catalog_annots_survive_trim_inside_crop() { + let Some(fx) = Harness::new("r2b-crop-annots") else { + eprintln!("skip: qpdf not available"); + return; + }; + write_catalog_annots_fixture( + &fx.src, + 0, + Some([0, 0, 612, 792]), + Some([100, 100, 400, 500]), + ); + fx.export(&text_box()) + .expect("export catalog+annots Trim⊂Crop"); + assert_catalog_annots_survived(&fx.dest); + assert_dest_boxes_trim_inside_crop(&fx.dest); + fx.cleanup(); +} diff --git a/src-tauri/src/pdf_engine/validate_output.rs b/src-tauri/src/pdf_engine/validate_output.rs index ea382a8..fae4209 100644 --- a/src-tauri/src/pdf_engine/validate_output.rs +++ b/src-tauri/src/pdf_engine/validate_output.rs @@ -11,10 +11,17 @@ use crate::error::AppError; use crate::pdf_engine::crop; -use lopdf::Document; +use lopdf::{Document, Object, ObjectId}; use std::path::Path; use std::sync::atomic::{AtomicBool, Ordering}; +/// FNV-1a of decoded page Contents plus that byte length. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ContentDigest { + pub hash: u64, + pub len: usize, +} + /// Per-page geometry the gate compares to the reopened staged file. #[derive(Debug, Clone, PartialEq)] pub struct PageSnapshot { @@ -23,6 +30,7 @@ pub struct PageSnapshot { pub trim_box: Option<[f64; 4]>, pub rotate: i64, pub user_unit: f64, + pub content_digest: ContentDigest, } /// Catalog / trailer structures the source had and the staged file must keep. @@ -126,6 +134,96 @@ fn has_any_annots(doc: &Document) -> bool { }) } +/// Same FNV-1a-64 as `render::fnv1a_hex`, on raw bytes (no hex). +fn fnv1a_u64(bytes: &[u8]) -> u64 { + let mut hash: u64 = 0xcbf29ce484222325; + for byte in bytes { + hash ^= *byte as u64; + hash = hash.wrapping_mul(0x100000001b3); + } + hash +} + +pub(crate) fn content_digest(bytes: &[u8]) -> ContentDigest { + ContentDigest { + hash: fnv1a_u64(bytes), + len: bytes.len(), + } +} + +fn decoded_stream_bytes(stream: &lopdf::Stream) -> Vec { + match stream.decompressed_content() { + Ok(data) => data, + Err(_) => stream.content.clone(), + } +} + +fn dict_from<'a>(doc: &'a Document, obj: &'a Object) -> Option<&'a lopdf::Dictionary> { + match obj { + Object::Dictionary(d) => Some(d), + Object::Reference(id) => doc.get_dictionary(*id).ok(), + _ => None, + } +} + +fn page_form_xobject_bytes(doc: &Document, page_id: ObjectId) -> Vec> { + let mut out = Vec::new(); + let Ok((inline, inherited_ids)) = doc.get_page_resources(page_id) else { + return out; + }; + let mut resource_dicts: Vec<&lopdf::Dictionary> = Vec::new(); + if let Some(d) = inline { + resource_dicts.push(d); + } + for id in inherited_ids { + if let Ok(d) = doc.get_dictionary(id) { + resource_dicts.push(d); + } + } + for resources in resource_dicts { + let Ok(xo_obj) = resources.get(b"XObject") else { + continue; + }; + let Some(xobjects) = dict_from(doc, xo_obj) else { + continue; + }; + for (_, obj) in xobjects.iter() { + let Ok(id) = obj.as_reference() else { + continue; + }; + let Ok(stream) = doc.get_object(id).and_then(Object::as_stream) else { + continue; + }; + let subtype = stream + .dict + .get(b"Subtype") + .ok() + .and_then(|o| o.as_name().ok()); + if subtype != Some(b"Form") { + continue; + } + out.push(decoded_stream_bytes(stream)); + } + } + out +} + +fn dest_page_digests(doc: &Document, page_id: ObjectId) -> Vec { + let mut out = Vec::new(); + if let Ok(bytes) = doc.get_page_content(page_id) { + out.push(content_digest(&bytes)); + } + for id in doc.get_page_contents(page_id) { + if let Ok(stream) = doc.get_object(id).and_then(Object::as_stream) { + out.push(content_digest(&decoded_stream_bytes(stream))); + } + } + for bytes in page_form_xobject_bytes(doc, page_id) { + out.push(content_digest(&bytes)); + } + out +} + /// Validate a dest-sibling staged PDF against `snapshot` using `run_check` for `qpdf --check`. /// /// `run_check` receives an argv array (no shell), typically `["--check", ]`, @@ -227,6 +325,14 @@ pub fn validate_staged_pdf( format!("Page {page_no} /UserUnit does not match the source."), )); } + let candidates = dest_page_digests(&doc, id); + if !candidates.iter().any(|d| *d == expected.content_digest) { + return Err(fatal_staged( + staged, + format!("Page {page_no} content does not match the source."), + )); + } + abort_if_cancelled(staged, cancel)?; } if snapshot.catalog.outlines && !has_catalog_key(&doc, b"Outlines") { @@ -335,6 +441,15 @@ mod tests { trim_box: None, rotate: 0, user_unit: 1.0, + content_digest: content_digest(b"BT /F1 12 Tf 72 720 Td (Hello) Tj ET"), + } + } + + fn letter_page_labeled(label: &str) -> PageSnapshot { + let bytes = format!("BT /F1 12 Tf 72 720 Td ({label}) Tj ET").into_bytes(); + PageSnapshot { + content_digest: content_digest(&bytes), + ..letter_page() } } @@ -606,6 +721,7 @@ mod tests { trim_box: None, rotate: 0, user_unit: 1.0, + content_digest: content_digest(b"BT /F1 12 Tf 72 720 Td (Hello) Tj ET"), }], catalog: empty_catalog(), }; @@ -677,4 +793,150 @@ mod tests { "C1: cancel must delete staged .offpdf-*.pdf.tmp" ); } + + // --- R1 / R1b ----------------------------------------------------------- + + fn write_two_page_letter(path: &Path, label1: &str, label2: &str) { + let mut doc = Document::with_version("1.5"); + let pages_id = doc.new_object_id(); + let content1 = doc.add_object(Object::Stream(Stream::new( + Dictionary::new(), + format!("BT /F1 12 Tf 72 720 Td ({label1}) Tj ET").into_bytes(), + ))); + let content2 = doc.add_object(Object::Stream(Stream::new( + Dictionary::new(), + format!("BT /F1 12 Tf 72 720 Td ({label2}) Tj ET").into_bytes(), + ))); + + let mut page1 = Dictionary::new(); + page1.set("Type", "Page"); + page1.set("Parent", pages_id); + page1.set("MediaBox", box_obj([0, 0, 612, 792])); + page1.set("Contents", content1); + let page1_id = doc.add_object(Object::Dictionary(page1)); + + let mut page2 = Dictionary::new(); + page2.set("Type", "Page"); + page2.set("Parent", pages_id); + page2.set("MediaBox", box_obj([0, 0, 612, 792])); + page2.set("Contents", content2); + let page2_id = doc.add_object(Object::Dictionary(page2)); + + let mut pages = Dictionary::new(); + pages.set("Type", "Pages"); + pages.set("Kids", vec![page1_id.into(), page2_id.into()]); + pages.set("Count", 2); + doc.objects.insert(pages_id, Object::Dictionary(pages)); + + let mut catalog = Dictionary::new(); + catalog.set("Type", "Catalog"); + catalog.set("Pages", pages_id); + let catalog_id = doc.add_object(Object::Dictionary(catalog)); + doc.trailer.set("Root", catalog_id); + doc.save(path).expect("write two-page fixture"); + } + + fn two_letter_snapshot() -> OutputSnapshot { + OutputSnapshot { + pages: vec![ + letter_page_labeled("ALPHA-PAGE"), + letter_page_labeled("BETA-PAGE"), + ], + catalog: empty_catalog(), + } + } + + fn swap_page_kids(path: &Path) { + let mut doc = Document::load(path).expect("load two-page pdf"); + let root = doc + .trailer + .get(b"Root") + .expect("Root") + .as_reference() + .expect("Root ref"); + let pages_id = doc + .get_dictionary(root) + .expect("catalog") + .get(b"Pages") + .expect("Pages") + .as_reference() + .expect("Pages ref"); + let kids = match doc.get_dictionary(pages_id).expect("pages dict").get(b"Kids") { + Ok(Object::Array(a)) => a.clone(), + other => panic!("Kids must be an array, got {other:?}"), + }; + assert_eq!(kids.len(), 2, "fixture must have two Kids"); + let swapped = vec![kids[1].clone(), kids[0].clone()]; + match doc.get_object_mut(pages_id) { + Ok(Object::Dictionary(pages)) => pages.set("Kids", swapped), + other => panic!("Pages object is not a dictionary: {other:?}"), + } + doc.save(path).expect("rewrite swapped Kids"); + } + + fn page_stream_text(path: &Path, page: u32) -> String { + let doc = Document::load(path).expect("load pdf"); + let id = *doc.get_pages().get(&page).expect("page id"); + let bytes = doc.get_page_content(id).expect("page contents"); + String::from_utf8_lossy(&bytes).into_owned() + } + + #[test] + fn validate_swapped_same_geometry_pages_is_invalid_output() { + let scratch = Scratch::new("r1-swap"); + let dest = scratch.path().join("out.pdf"); + let staged = scratch.path().join(".offpdf-r1.pdf.tmp"); + write_two_page_letter(&staged, "ALPHA-PAGE", "BETA-PAGE"); + std::fs::write(&dest, b"OLD-DEST").unwrap(); + swap_page_kids(&staged); + assert!( + page_stream_text(&staged, 1).contains("BETA-PAGE"), + "swap must put BETA on dest page 1" + ); + assert!( + page_stream_text(&staged, 2).contains("ALPHA-PAGE"), + "swap must put ALPHA on dest page 2" + ); + + let result = validate_staged_pdf(&staged, &two_letter_snapshot(), None, |_args| { + Ok((0, String::new())) + }); + let err = result.expect_err( + "R1: same-geometry Kids swap must be INVALID_OUTPUT (presence of source Contents digest)", + ); + assert_invalid_output(&err); + assert_eq!( + std::fs::read(&dest).unwrap(), + b"OLD-DEST", + "R1: dest bytes must stay OLD-DEST" + ); + assert!( + !staged.exists(), + "R1: fatal validate must delete staged .offpdf-*.pdf.tmp" + ); + } + + #[test] + fn validate_in_order_same_geometry_pages_is_ok() { + let scratch = Scratch::new("r1b-order"); + let dest = scratch.path().join("out.pdf"); + let staged = scratch.path().join("staged.pdf"); + write_two_page_letter(&staged, "ALPHA-PAGE", "BETA-PAGE"); + std::fs::write(&dest, b"OLD-DEST").unwrap(); + assert!(page_stream_text(&staged, 1).contains("ALPHA-PAGE")); + assert!(page_stream_text(&staged, 2).contains("BETA-PAGE")); + + let result = validate_staged_pdf(&staged, &two_letter_snapshot(), None, |_args| { + Ok((0, String::new())) + }); + assert!( + result.is_ok(), + "R1b: in-order two-page same-geometry dest must pass the gate; {result:?}" + ); + assert_eq!( + std::fs::read(&dest).unwrap(), + b"OLD-DEST", + "R1b: gate must not publish; dest bytes must stay OLD" + ); + } } diff --git a/src/components/jobs/JobStatus.test.tsx b/src/components/jobs/JobStatus.test.tsx new file mode 100644 index 0000000..cf4bcbc --- /dev/null +++ b/src/components/jobs/JobStatus.test.tsx @@ -0,0 +1,56 @@ +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it, vi } from "vitest"; + +vi.mock("@/lib/tauriCommands", () => ({ + getFileInfo: async () => ({ + path: "/tmp/offpdf-edit-out.pdf", + name: "offpdf-edit-out.pdf", + sizeBytes: 1, + pageCount: 1, + isValidPdf: true, + }), + openPath: async () => {}, + imageToPdf: async () => "/tmp/x.pdf", + officeToPdf: async () => "/tmp/x.pdf", +})); + +import { ToastProvider } from "@/components/ui/Toast"; +import { JobStatus } from "./JobStatus"; +import type { JobController } from "./useJob"; + +const QPDF_WARNING = "WARNING: qpdf --check: file is not linearized"; + +function completedJob(message: string | null): JobController { + return { + state: "completed", + update: { + jobId: "job-34", + state: "completed", + step: "Done", + message, + }, + result: { + jobId: "job-34", + outputPaths: ["/tmp/offpdf-edit-out.pdf"], + status: "ok", + }, + error: null, + meta: null, + isBusy: false, + run: async () => {}, + cancel: () => {}, + reset: () => {}, + }; +} + +describe("JobStatus completed card", () => { + it("shows job.update.message on the Done card", () => { + const markup = renderToStaticMarkup( + + + , + ); + expect(markup).toContain("Done"); + expect(markup).toContain(QPDF_WARNING); + }); +}); diff --git a/src/components/jobs/JobStatus.tsx b/src/components/jobs/JobStatus.tsx index 5d4cbd9..2802513 100644 --- a/src/components/jobs/JobStatus.tsx +++ b/src/components/jobs/JobStatus.tsx @@ -98,6 +98,7 @@ export function JobStatus({ job }: { job: JobController }) { ? "Your file is ready." : `${outputs.length} files are ready.`}

+ {job.update?.message &&
{job.update.message}
}