From cb3722b94cb17c72ccd841dac83479ae137f55bf Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 05:30:11 +0000 Subject: [PATCH] address review: refuse a batch that deletes a marked page while adding pages --- crates/engine/src/session/editing.rs | 10 ++++++++++ crates/engine/src/tests.rs | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/crates/engine/src/session/editing.rs b/crates/engine/src/session/editing.rs index a4e40750d2..f1149da021 100644 --- a/crates/engine/src/session/editing.rs +++ b/crates/engine/src/session/editing.rs @@ -77,6 +77,16 @@ impl Session { // over the page as the renderer shows it, so the renderer has the last word: on every // apply the edit ran, a batch's included. if !cx.redaction_marks.is_empty() { + // A batch that deletes a marked page and adds pages cannot be tied back to the marks: + // a copy of a page may carry the text the redaction removed from the original only. + // Fail closed. + let before: std::collections::HashSet<_> = pdfcraft_model::pages(&editor.cos).iter().map(|p| p.obj).collect(); + let after: std::collections::HashSet<_> = pdfcraft_model::pages(&next).iter().map(|p| p.obj).collect(); + if cx.redaction_marks.iter().any(|(o, _)| !after.contains(o)) && after.iter().any(|o| !before.contains(o)) { + return Err(EditError::Invalid( + "a batch that deletes a page with a redaction mark and adds pages can't be verified; the document was not changed".into(), + )); + } guard(|| shown_under_redactions(&next, &cx.redaction_marks, &cx.redaction_overlays, doc.password.as_deref())) .unwrap_or_else(|m| Err(EditError::Invalid(format!("{} failed unexpectedly ({m}); the document was not changed", edit.label()))))?; } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 274a918cdf..ed66114a15 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -10274,3 +10274,22 @@ fn redactions_applied_in_a_batch_get_the_renderer_check() { assert!(!page_texts(&s, id)[0].contains("SECRET42"), "{:?}", page_texts(&s, id)); assert_eq!(s.get(id).unwrap().redaction_marks(), 0); } + +/// A batch that deletes a page holding a redaction mark and adds pages is refused whole: the +/// marks cannot be tied to copies, so redaction fails closed. Without added pages it goes through. +#[test] +fn a_batch_that_deletes_a_marked_page_and_adds_pages_is_refused() { + let mut s = Session::new().with_clock(|| 1_700_000_000); + let page = one_line_page_with("/Font << /F1 4 0 R >>", "", "BT /F1 16 Tf 20 150 Td (Public words then SECRET42) Tj ET"); + let id = s.open("batch.pdf", None, Arc::new(page), None).unwrap(); + mark_shown_text(&mut s, id, "SECRET42"); + let edits = vec![ + Edit::ApplyRedactions { pages: None }, + Edit::InsertBlankPage { at: 1, width: 612.0, height: 792.0 }, + Edit::DeletePages { pages: vec![0] }, + ]; + let refused = s.apply(id, Edit::Batch { label: "x".into(), edits }); + assert!(matches!(refused, Err(EditError::Invalid(_))), "{refused:?}"); + assert!(page_texts(&s, id)[0].contains("SECRET42"), "nothing changed"); + assert_eq!(s.get(id).unwrap().redaction_marks(), 1); +}