diff --git a/crates/cos/src/document.rs b/crates/cos/src/document.rs index 892c8e04d..5fc8c6212 100644 --- a/crates/cos/src/document.rs +++ b/crates/cos/src/document.rs @@ -1312,6 +1312,17 @@ impl Document { self.overlay.insert(r.num, Slot::Freed(r.generation.saturating_add(1))); } + /// Free what was added since the file was read and nothing refers to any more: the objects + /// [`SaveOptions::drop_unlinked`](crate::SaveOptions::drop_unlinked) leaves out of an + /// incremental save, freed as it writes them (the generation stays). For a writer about to + /// add an object that nothing refers to either, which that option would leave out too. + pub fn free_unlinked_additions(&mut self) { + for num in crate::writer::unlinked_additions(self) { + let generation = self.generation(num); + self.overlay.insert(num, Slot::Freed(generation)); + } + } + /// Record `current`'s object graph as unsaved edits of this document. /// /// Used when the working snapshot was restored from an earlier revision and the next diff --git a/crates/cos/src/writer.rs b/crates/cos/src/writer.rs index 61d14475b..627f71649 100644 --- a/crates/cos/src/writer.rs +++ b/crates/cos/src/writer.rs @@ -162,7 +162,9 @@ pub struct SaveOptions { /// Incremental saves: leave out what was added since the file was read and nothing refers to /// any more, such as a comment added and deleted again or a redaction mark added and cleared /// (written as free). Off by default, because a writer may add an object only it finds again - /// by scanning, as a standalone document timestamp is. Full saves always leave these out. + /// by scanning, as a standalone document timestamp is; such a writer frees the leftovers + /// before adding it, with [`Document::free_unlinked_additions`]. Full saves always leave + /// these out. pub drop_unlinked: bool, } @@ -460,7 +462,7 @@ fn output_number(index: usize) -> Result { /// from the trailer and the file's changed objects through additions. An unchanged object of the /// file is not followed: a reference it holds to such a number already reads as null in the file /// (a damaged file's dangling reference), and it still does. -fn unlinked_additions(doc: &Document) -> HashSet { +pub(crate) fn unlinked_additions(doc: &Document) -> HashSet { let added = |num: u32| matches!(doc.xref_entry(num), None | Some(XrefEntry::Free { .. })); let mut pending: HashMap> = doc.overlay_entries().filter_map(|(num, _, o)| Some((num, o?))).filter(|(num, _)| added(*num)).collect(); diff --git a/crates/cos/tests/it/writer.rs b/crates/cos/tests/it/writer.rs index f71bdac47..1b9866425 100644 --- a/crates/cos/tests/it/writer.rs +++ b/crates/cos/tests/it/writer.rs @@ -274,6 +274,15 @@ fn incremental_save_leaves_out_additions_nothing_refers_to() { } assert!(!page(&back).contains(b"Annots")); assert_qpdf_clean(&saved); + + // Freed beforehand, they are written the same way without the option, and what is added + // after that, nothing referring to it either (a document timestamp), is kept. + let mut freed = doc.clone(); + freed.free_unlinked_additions(); + assert_eq!(write_incremental(&freed, &SaveOptions::default()).unwrap(), saved, "the same objects, freed the same way"); + freed.add(text("added after")); + let stamped = write_incremental(&freed, &SaveOptions::default()).unwrap(); + assert!(stamped.windows(11).any(|w| w == b"added after") && !stamped.windows(12).any(|w| w == b"dropped leaf")); } #[test] diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 70165d4f6..e4165a4bf 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2490,6 +2490,28 @@ fn redaction_marks_apply_for_good_and_undo() { assert!(matches!(s.apply(id, Edit::ApplyRedactions { pages: None }), Err(EditError::Redact(_)))); } +/// What [`mark_with_reply`] writes that must not reach a file once the mark is removed. +const MARK_TRACES: [&str; 5] = ["/Redact", "MARKED OVERLAY", "Mark Author", "REPLY TO THE MARK", "Replier"]; + +/// A redaction mark on page 1 (the first annotation there) and a reply to it. +fn mark_with_reply(s: &mut Session, id: DocId) { + let shape = Shape::Redact { + quads: vec![pdfcraft_annot::rect_quad([80.0, 140.0, 100.0, 180.0])], + overlay: "MARKED OVERLAY".into(), + look: Default::default(), + }; + let mark = Edit::AddAnnotation(NewAnnotation { + page: 0, + style: Style::default_for(&shape), + shape, + contents: String::new(), + author: "Mark Author".into(), + }); + s.apply(id, mark).unwrap(); + let reply = Edit::ReplyToAnnotation { page: 0, index: 0, text: "REPLY TO THE MARK".into(), author: "Replier".into() }; + s.apply(id, reply).unwrap(); +} + /// Marks removed before they are applied, all at once or one by one, are not saved, nor are /// replies to them: the file would otherwise record where they were, their overlay text and who /// made them, and the page is left as it was. @@ -2497,28 +2519,14 @@ fn redaction_marks_apply_for_good_and_undo() { fn removed_redaction_marks_are_not_saved() { for clear in [true, false] { let (mut s, id) = session_with(1); - let shape = Shape::Redact { - quads: vec![pdfcraft_annot::rect_quad([80.0, 140.0, 100.0, 180.0])], - overlay: "MARKED OVERLAY".into(), - look: Default::default(), - }; - let mark = Edit::AddAnnotation(NewAnnotation { - page: 0, - style: Style::default_for(&shape), - shape, - contents: String::new(), - author: "Mark Author".into(), - }); - s.apply(id, mark).unwrap(); - let reply = Edit::ReplyToAnnotation { page: 0, index: 0, text: "REPLY TO THE MARK".into(), author: "Replier".into() }; - s.apply(id, reply).unwrap(); + mark_with_reply(&mut s, id); let removal = if clear { Edit::ClearRedactions } else { Edit::DeleteAnnotation { page: 0, index: 0 } }; s.apply(id, removal).unwrap(); assert_eq!(s.get(id).unwrap().redaction_marks(), 0); let saved = s.save_bytes(id).unwrap(); - let holds = |s: &[u8]| saved.windows(s.len()).any(|w| w == s); - assert!(!holds(b"/Redact") && !holds(b"MARKED OVERLAY") && !holds(b"Mark Author"), "clear: {clear}"); - assert!(!holds(b"REPLY TO THE MARK") && !holds(b"Replier"), "clear: {clear}"); + for trace in MARK_TRACES { + assert!(!saved.windows(trace.len()).any(|w| w == trace.as_bytes()), "clear: {clear}, {trace:?}"); + } let mut reopened = Session::new(); let back = reopened.open("saved.pdf", None, saved.clone(), None).unwrap(); assert_eq!(page_texts(&reopened, back), ["Page 1"]); @@ -2539,6 +2547,27 @@ fn removed_redaction_marks_are_not_saved() { } } +/// Removed marks are not in a signed file either, nor in one with revocation evidence embedded: +/// no later save could take them out of those. +#[test] +fn removed_redaction_marks_are_not_signed() { + let p12 = std::fs::read(concat!(env!("CARGO_MANIFEST_DIR"), "/../sign/tests/data/ec-p256.p12")).unwrap(); + let digital_id = sign::pkcs12::open(&p12, "test").unwrap(); + // After the test certificate's start date (2026-10-02). + let mut s = Session::new().with_clock(|| 1_800_000_000); + let id = s.open("fixture.pdf", None, Arc::new(fixture(1)), None).unwrap(); + mark_with_reply(&mut s, id); + s.apply(id, Edit::ClearRedactions).unwrap(); + let signed = s.sign(id, &digital_id, SignOptions { rect: None, ..SignOptions::default() }).unwrap(); + let evidence = sign::dss::Evidence { certs: vec![digital_id.certificate.raw.clone()], ocsps: Vec::new(), crls: Vec::new() }; + let ltv = s.embed_ltv(id, &evidence).unwrap(); + for (what, out) in [("signed", signed), ("LTV", ltv)] { + for trace in MARK_TRACES { + assert!(!out.windows(trace.len()).any(|w| w == trace.as_bytes()), "{what}: {trace:?}"); + } + } +} + #[test] fn oversized_redaction_overlay_preserves_the_document_and_undo() { let (mut s, id) = session_with(1); diff --git a/crates/sign/src/dss.rs b/crates/sign/src/dss.rs index e095ec749..0d82c797a 100644 --- a/crates/sign/src/dss.rs +++ b/crates/sign/src/dss.rs @@ -93,7 +93,9 @@ pub fn embed(doc: &Document, evidence: &Evidence) -> Result, SignError> } let dss_ref = doc.add(Object::Dict(dss)); doc.update_dict(root, |c| c.set(b"DSS".to_vec(), Object::Ref(dss_ref)))?; - Ok(pdfcraft_cos::write_incremental(&doc, &SaveOptions::default())?) + // What the user removed again before this (a cleared redaction mark) stays out of the file, + // as in a save; the store and everything in it are reachable from the catalog. + Ok(pdfcraft_cos::write_incremental(&doc, &SaveOptions { drop_unlinked: true, ..SaveOptions::default() })?) } /// Store one DER blob as a `/Type /Embed` stream. diff --git a/crates/sign/src/pdf.rs b/crates/sign/src/pdf.rs index d9669f08e..19120c248 100644 --- a/crates/sign/src/pdf.rs +++ b/crates/sign/src/pdf.rs @@ -2163,8 +2163,11 @@ fn sign_inner( let mut apd = Dict::new(); apd.set(b"N".to_vec(), Object::Ref(ap_ref)); doc.update_dict(widget, |w| w.set(b"AP".to_vec(), Object::Dict(apd)))?; - // Write with the placeholders, then patch them. - let save = SaveOptions { mod_date: Some(opts.date.clone()), object_streams: false, ..SaveOptions::default() }; + // Write with the placeholders, then patch them. What the user removed again before signing + // (a cleared redaction mark) stays out of the file, as in a save; what signing added is + // reachable (the signature through the field's `/V`), and the placeholders are found in the + // written bytes. + let save = SaveOptions { mod_date: Some(opts.date.clone()), object_streams: false, drop_unlinked: true, ..SaveOptions::default() }; let mut out = pdfcraft_cos::write_incremental(&doc, &save)?; let (br_at, gap) = locate(&out, reserve)?; let (start, end) = gap; @@ -2213,6 +2216,10 @@ pub fn timestamp_document(doc: &Document, tsa: &dyn crate::timestamp::TimestampA )); } let mut doc = doc.clone(); + // What the user removed again before this (a cleared redaction mark) stays out of the file, + // as in a save. Nothing refers to the timestamp either, so the leftovers go first and the + // save keeps everything. + doc.free_unlinked_additions(); let mut v = Dict::new(); v.set(b"Type".to_vec(), Object::name("DocTimeStamp")); v.set(b"Filter".to_vec(), Object::name("Adobe.PPKLite")); diff --git a/crates/sign/tests/it/pdf.rs b/crates/sign/tests/it/pdf.rs index 55aa732d7..f1a0218c6 100644 --- a/crates/sign/tests/it/pdf.rs +++ b/crates/sign/tests/it/pdf.rs @@ -2,7 +2,7 @@ use std::sync::Arc; -use pdfcraft_cos::{Document, Object, PdfString, SaveOptions, write_incremental}; +use pdfcraft_cos::{Dict, Document, ObjRef, Object, PdfString, SaveOptions, write_incremental}; use pdfcraft_sign::{ Certificate, Detail, DigitalId, Modification, Name, SignError, SignOptions, Status, Time, TimestampAuthority, TrustStore, WORDINGS, der, in_english, kind, pkcs12, signatures, @@ -721,6 +721,71 @@ fn sign_then_ltv_then_timestamp_makes_a_b_lta_file() { assert_eq!(stamp.status, Status::Valid, "{:?}", stamp.details); } +/// A redaction mark and a reply to it, added to the page and removed again before signing (what +/// clearing a mark leaves behind), are in none of the signing outputs: a signature, a timestamped +/// one, embedded revocation evidence or a document timestamp. Nothing can take them out of a +/// signed revision afterwards. Each output still validates. +#[test] +fn what_was_removed_before_signing_is_not_signed() { + use pdfcraft_sign::dss::{self, Evidence}; + let id = pkcs12::open(&data("ec-p256.p12"), "test").unwrap(); + let tsa = TestTsa { id: tsa_id(), time: Time { year: 2026, month: 10, day: 6, hour: 12, minute: 0, second: 0 } }; + let anchor = id.chain.first().cloned().unwrap_or_else(|| id.certificate.clone()); + let trust = TrustStore { certs: vec![anchor, tsa.id.certificate.clone()] }; + let text = |s: &str| Object::String(PdfString::literal(s.as_bytes().to_vec())); + let dict = |entries: &[(&str, Object)]| { + let mut d = Dict::new(); + entries.iter().for_each(|(k, v)| d.set(k.as_bytes().to_vec(), v.clone())); + Object::Dict(d) + }; + let cleared = |bytes: &[u8]| { + let mut doc = open(bytes); + let page = ObjRef::new(3, 0); + let annots = doc.get(page).as_dict().and_then(|d| d.get(b"Annots").cloned()).unwrap(); + let mark = doc.add(dict(&[ + ("Type", Object::name("Annot")), + ("Subtype", Object::name("Redact")), + ("Rect", Object::Array([80, 140, 100, 180].map(Object::Int).to_vec())), + ("OverlayText", text("MARKED OVERLAY")), + ("T", text("Mark Author")), + ("P", Object::Ref(page)), + ])); + let reply = doc.add(dict(&[ + ("Type", Object::name("Annot")), + ("Subtype", Object::name("Text")), + ("IRT", Object::Ref(mark)), + ("Contents", text("REPLY TO THE MARK")), + ("T", text("Replier")), + ])); + let mut with = annots.as_array().cloned().unwrap(); + with.extend([Object::Ref(mark), Object::Ref(reply)]); + doc.update_dict(page, |d| d.set(b"Annots".to_vec(), Object::Array(with))).unwrap(); + doc.update_dict(page, |d| d.set(b"Annots".to_vec(), annots.clone())).unwrap(); + doc + }; + let unsigned = fixture(); + let signed = pdfcraft_sign::sign(&open(&unsigned), &id, &opts()).unwrap(); + let evidence = Evidence { certs: vec![id.certificate.raw.clone()], ocsps: Vec::new(), crls: Vec::new() }; + let outputs = [ + ("sign", &unsigned, pdfcraft_sign::sign(&cleared(&unsigned), &id, &opts()).unwrap()), + ("sign_with_timestamp", &unsigned, pdfcraft_sign::sign_with_timestamp(&cleared(&unsigned), &id, &opts(), &tsa).unwrap()), + ("dss::embed", &signed, dss::embed(&cleared(&signed), &evidence).unwrap()), + ("timestamp_document", &unsigned, pdfcraft_sign::timestamp_document(&cleared(&unsigned), &tsa, "D:20261006120000Z").unwrap()), + ]; + for (what, input, out) in outputs { + assert!(out.starts_with(input), "{what}: an incremental update"); + let holds = |s: &str| out.windows(s.len()).any(|w| w == s.as_bytes()); + for trace in ["/Redact", "MARKED OVERLAY", "Mark Author", "REPLY TO THE MARK", "Replier"] { + assert!(!holds(trace), "{what} wrote {trace:?}"); + } + let doc = open(&out); + assert!(doc.repair_log().is_empty(), "{what}: {:?}", doc.repair_log()); + let all: Vec<_> = signatures(&doc, &out, &trust).into_iter().filter(|s| s.signed).collect(); + assert_eq!(all.iter().filter(|s| s.doc_timestamp).count(), usize::from(what == "timestamp_document"), "{what}"); + assert!(!all.is_empty() && all.iter().all(|s| s.status == Status::Valid), "{what}: {all:?}"); + } +} + #[test] fn an_embedded_verified_revocation_invalidates_the_signature() { use pdfcraft_sign::dss::{self, Evidence};