diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index 6e631b708..4a34909de 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -879,6 +879,7 @@ impl Automation { // A save that adds or removes encryption, or must not keep the old revision (Sanitize, // redaction), is rewritten in full even when an incremental save was asked for. let incremental = !full && bytes.starts_with(&self.doc(a)?.saved_bytes()); + self.session.check_save(id, &bytes).map_err(failed)?; write(&target, &bytes)?; let path = target.to_string_lossy().into_owned(); self.session.mark_saved(id, bytes.clone(), Some(path.clone())).map_err(failed)?; diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index 5b9050dbd..e788d9610 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -524,6 +524,44 @@ fn an_in_place_save_reports_a_required_rewrite_as_not_incremental() { } } +/// Issue #1698: a save whose bytes don't reopen is refused before it touches the file. The +/// catalog's `/Pages` is not a page tree, so the reader finds the page by scanning, while a +/// full rewrite keeps only what the catalog reaches and drops it. +#[test] +fn a_save_that_does_not_reopen_leaves_the_file_as_it_was() { + let dir = workdir("save-verify-before-write"); + let objs: [&[u8]; 4] = [ + b"<< /Type /Catalog /Pages 3 0 R >>", + b"<< /Type /Page /MediaBox [0 0 200 300] /Contents 4 0 R >>", + b"[ 0 ]", + b"<< /Length 18 >>\nstream\n0 0 m 100 100 l S\nendstream", + ]; + let mut original = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(original.len()); + original.extend_from_slice(format!("{} 0 obj\n", i + 1).as_bytes()); + original.extend_from_slice(o); + original.extend_from_slice(b"\nendobj\n"); + } + let xref = original.len(); + original.extend_from_slice(b"xref\n0 5\n0000000000 65535 f \n"); + for o in offsets { + original.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + original.extend_from_slice(format!("trailer\n<< /Size 5 /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n").as_bytes()); + std::fs::write(dir.join("damaged.pdf"), &original).unwrap(); + + let mut a = auto(&dir); + let opened = ok(&mut a, "doc_open", json!({ "path": "damaged.pdf" })); + assert_eq!(opened["pages"], 1, "{opened}"); + let doc = opened["doc"].as_u64().unwrap(); + let e = err(&mut a, "doc_save", json!({ "doc": doc, "full": true })); + assert!(e.to_string().contains("could not be reopened"), "{e}"); + let on_disk = std::fs::read(dir.join("damaged.pdf")).unwrap(); + assert!(on_disk == original, "the file was replaced by {} bytes that don't reopen", on_disk.len()); +} + /// organize.field-name-conflicts: combining files whose forms share a field name keeps the /// fields separate by renaming, says so in the result, and can be told to merge instead; any /// other policy is an argument error. diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 15c0551b1..fbe62b2c2 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -3741,6 +3741,16 @@ impl Session { guard(|| write_full(&editor.cos, &opts)).map_err(EditError::Write)?.map(Arc::new).map_err(|e| EditError::Write(e.to_string())) } + /// Check that `bytes`, from [`Self::save_bytes`] or [`Self::save_full_bytes`], reopen as this + /// document before they are written (#1698): [`Self::mark_saved`] only finds out after the + /// file has been replaced. Same error as `mark_saved`. + pub fn check_save(&self, id: DocId, bytes: &Arc>) -> Result<(), EditError> { + let doc = self.get(id).ok_or(EditError::NoDocument)?; + let Some(editor) = doc.editor.as_ref() else { return Ok(()) }; + pdfcraft_cos::Document::open_with_password(bytes.clone(), editor.keys.reopen.as_deref()).map_err(|e| EditError::Reopen(e.to_string()))?; + pdfcraft_render::check_pages(bytes.clone(), doc.password.as_deref(), doc.info.pages.len()).map_err(|e| EditError::Reopen(e.to_string())) + } + /// Record a successful save of `bytes` (to `path`, if any): rebase editing on the saved file /// so the next save appends only newer edits. Undo history is kept. pub fn mark_saved(&mut self, id: DocId, bytes: Arc>, path: Option) -> Result<(), EditError> { diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 9b1b35b19..dc7226ef6 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -664,7 +664,7 @@ impl PdfCraftApp { self.notify_tr("The document was closed before it could be saved."); return false; }; - let bytes = match self.session.save_bytes(id) { + let bytes = match self.session.save_bytes(id).and_then(|b| self.session.check_save(id, &b).map(|()| b)) { Ok(b) => b, Err(e) => { self.notify_fmt("Couldn't save {name}: {e}", &[("name", &name), ("e", &e.to_string())]);