fix(save): validate PDF output before replacing files

Validate serialized PDF bytes and page count before replacing the destination in desktop and automation saves. Reject unreadable or page-losing output before file replacement and saved-state updates.

Keep staging writes synced before publication and retain the regression for a repaired page tree that cannot survive a full rewrite.

Fixes #1698
This commit is contained in:
Marco Arnulfo authored and GitHub committed 2026-10-14 00:25:22 -06:00
1 parent e4028b3c77
commit c1a41cc5c7
4 files changed
+50 -1

No files matched your search

+1
View File
@@ -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)?;
+38
View File
@@ -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.
+10
View File
@@ -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<Vec<u8>>) -> 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<Vec<u8>>, path: Option<String>) -> Result<(), EditError> {
+1 -1
View File
@@ -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())]);