diff --git a/crates/cos/src/writer.rs b/crates/cos/src/writer.rs index b151399fef..61d14475b4 100644 --- a/crates/cos/src/writer.rs +++ b/crates/cos/src/writer.rs @@ -2,17 +2,20 @@ //! //! * `write_incremental` appends the edited objects, a cross-reference section in the same style //! as the file's last one (table or stream) and a trailer with `/Prev`. The original bytes are -//! never modified, which keeps digital signatures over earlier revisions valid. +//! never modified, which keeps digital signatures over earlier revisions valid. With +//! `SaveOptions::drop_unlinked`, objects added since the file was read that nothing refers to +//! any more are left out (written as free). //! * `write_full` writes only objects reachable from the trailer, renumbered densely, with a //! cross-reference table or stream — the "Save As" / garbage-collecting path. Unknown trailer //! entries are copied, including objects reachable only from them. use std::borrow::Cow; -use std::collections::{BTreeMap, HashMap, VecDeque}; +use std::collections::{BTreeMap, HashMap, HashSet, VecDeque}; use std::io::Write; +use std::sync::Arc; use crate::CosError; -use crate::document::{Document, ObjectReader}; +use crate::document::{Document, ObjectReader, XrefEntry}; use crate::object::{Dict, ObjRef, Object, PdfString, Stream}; /// Serialize one object. Deterministic: same object → same bytes. @@ -156,11 +159,16 @@ pub struct SaveOptions { /// Full saves: pack non-stream objects into compressed object streams with a cross-reference /// stream (PDF 1.5+, §7.5.7–7.5.8). Off writes a classic table that any reader accepts. pub object_streams: bool, + /// 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. + pub drop_unlinked: bool, } impl Default for SaveOptions { fn default() -> Self { - Self { mod_date: None, id_seed: 0x5052_494E_5443_5241, object_streams: true } + Self { mod_date: None, id_seed: 0x5052_494E_5443_5241, object_streams: true, drop_unlinked: false } } } @@ -209,8 +217,14 @@ pub fn write_incremental(doc: &Document, opts: &SaveOptions) -> Result, let base = (original.len() + usize::from(newline)).saturating_sub(shift) as u64; let mut out = Vec::new(); let mut offsets: BTreeMap = BTreeMap::new(); // num → (offset, gen, in use) + let unlinked = if opts.drop_unlinked { unlinked_additions(&doc) } else { HashSet::new() }; for (num, generation, obj) in doc.overlay_entries() { match obj { + // Written as free, so `/Size` still covers every number this document handed out. The + // generation stays: the number was never in use, and an undo can bring it back as is. + Some(_) if unlinked.contains(&num) => { + offsets.insert(num, (0, generation, false)); + } Some(o) => { offsets.insert(num, (base + out.len() as u64, generation, true)); write_indirect(num, generation, &prepared(&doc, num, num, generation, &o), &mut out); @@ -387,7 +401,7 @@ pub fn write_full(doc: &Document, opts: &SaveOptions) -> Result, CosErro /// Object `r` read again for writing. The traversal read it once and only kept its number, so /// a second read that fails, finds nothing or finds a different kind of object must fail the /// save rather than write `null` in its place. -fn reread(reader: &mut ObjectReader, r: ObjRef, is_stream: bool) -> Result, CosError> { +fn reread(reader: &mut ObjectReader, r: ObjRef, is_stream: bool) -> Result, CosError> { let o = reader.try_get(r)?; if matches!(*o, Object::Null) || matches!(*o, Object::Stream(_)) != is_stream { return Err(CosError::Syntax { offset: 0, detail: format!("object {} read differently while saving; nothing was written", r.num) }); @@ -439,6 +453,37 @@ fn output_number(index: usize) -> Result { u32::try_from(index).ok().and_then(|n| n.checked_add(1)).ok_or_else(|| CosError::Syntax { offset: 0, detail: "too many objects to save".into() }) } +/// Objects added since the file was read (their numbers are not in use in it) that nothing an +/// incremental update writes refers to: what one edit added and a later one unlinked, such as a +/// deleted comment or a cleared redaction mark. The file never held them, so leaving them out +/// loses nothing, while writing them would keep what the user removed. Only the update is walked: +/// 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 { + 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(); + if pending.is_empty() { + return HashSet::new(); + } + let mut stack: Vec = Vec::new(); + doc.trailer().iter().for_each(|(_, v)| collect_refs(v, &mut |r| stack.push(r))); + for (num, _, o) in doc.overlay_entries() { + if let Some(o) = o + && !added(num) + { + collect_refs(&o, &mut |r| stack.push(r)); + } + } + while let Some(r) = stack.pop() { + if let Some(o) = pending.remove(&r.num) { + collect_refs(&o, &mut |r| stack.push(r)); + } + } + pending.into_keys().collect() +} + fn collect_refs(o: &Object, f: &mut impl FnMut(ObjRef)) { match o { Object::Ref(r) => f(*r), diff --git a/crates/cos/tests/it/writer.rs b/crates/cos/tests/it/writer.rs index ac04be2e94..f71bdac47c 100644 --- a/crates/cos/tests/it/writer.rs +++ b/crates/cos/tests/it/writer.rs @@ -228,6 +228,54 @@ fn incremental_save_keeps_a_trailer_only_edit() { assert_qpdf_clean(&saved); } +/// What one edit added and a later one unlinked (a deleted comment, a cleared redaction mark) is +/// not written: the file never held it, and writing it would keep what was removed. Additions the +/// update still refers to, from the trailer or a changed object and through other additions, are. +#[test] +fn incremental_save_leaves_out_additions_nothing_refers_to() { + let mut doc = Document::open(Arc::new(classic_table())).unwrap(); + let text = |s: &str| Object::String(pdfcraft_cos::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 page_obj = ObjRef::new(3, 0); + let leaf = doc.add(text("kept leaf")); + let kept = doc.add(dict(&[("Leaf", Object::Ref(leaf))])); + doc.update_dict(page_obj, |d| d.set(b"PieceInfo".to_vec(), Object::Ref(kept))).unwrap(); + let by_trailer = doc.add(text("kept by the trailer")); + doc.trailer_mut().set(b"Custom".to_vec(), Object::Ref(by_trailer)); + // Linked from the page and unlinked again, as clearing a mark does. + let dropped_leaf = doc.add(text("dropped leaf")); + let dropped = doc.add(dict(&[("Leaf", Object::Ref(dropped_leaf))])); + doc.update_dict(page_obj, |d| d.set(b"Annots".to_vec(), Object::Array(vec![Object::Ref(dropped)]))).unwrap(); + doc.update_dict(page_obj, |d| { + d.remove(b"Annots"); + }) + .unwrap(); + // Unlinked additions that refer to each other. + let first = doc.add(Object::Null); + let second = doc.add(dict(&[("Next", Object::Ref(first)), ("Note", text("dropped cycle"))])); + doc.set(first, dict(&[("Next", Object::Ref(second))])); + + // Off by default: a writer may add what only it finds again, by scanning. + let kept_all = write_incremental(&doc, &SaveOptions::default()).unwrap(); + assert!(kept_all.windows(12).any(|w| w == b"dropped leaf")); + let saved = write_incremental(&doc, &SaveOptions { drop_unlinked: true, ..SaveOptions::default() }).unwrap(); + let holds = |s: &str| saved.windows(s.len()).any(|w| w == s.as_bytes()); + assert!(holds("kept leaf") && holds("kept by the trailer")); + assert!(!holds("dropped leaf") && !holds("dropped cycle"), "unlinked additions are written"); + let back = Document::open(Arc::new(saved.clone())).unwrap(); + assert!(back.repair_log().is_empty(), "{:?}", back.repair_log()); + assert_eq!(back.get(leaf).as_string().map(|s| s.bytes.as_slice()), Some(b"kept leaf".as_slice())); + for gone in [dropped, dropped_leaf, first, second] { + assert_eq!(*back.get(gone), Object::Null); + } + assert!(!page(&back).contains(b"Annots")); + assert_qpdf_clean(&saved); +} + #[test] fn qpdf_accepts_object_stream_output() { let qpdf = pdfcraft_oracle::or_skip!(pdfcraft_oracle::qpdf()); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 68fc076b97..da31821990 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -4310,7 +4310,8 @@ impl Session { if !editor.cos.is_modified() && on_saved && (!editor.cos.reconstructed() || doc.is_signed()) { return Ok(editor.saved.clone()); } - let opts = SaveOptions { mod_date: self.now().map(pdfcraft_cos::pdf_date), ..SaveOptions::default() }; + // What the user removed again before saving (a cleared redaction mark) stays out of the file. + let opts = SaveOptions { mod_date: self.now().map(pdfcraft_cos::pdf_date), drop_unlinked: true, ..SaveOptions::default() }; // A restored snapshot can be based on an earlier revision. Append to the last saved // file so that revision stays in the chain. A full rewrite (redaction, new encryption, // a reconstructed file) still writes the current graph on its own. diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ed4e6efa1c..70165d4f64 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2490,6 +2490,55 @@ fn redaction_marks_apply_for_good_and_undo() { assert!(matches!(s.apply(id, Edit::ApplyRedactions { pages: None }), Err(EditError::Redact(_)))); } +/// 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. +#[test] +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(); + 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}"); + let mut reopened = Session::new(); + let back = reopened.open("saved.pdf", None, saved.clone(), None).unwrap(); + assert_eq!(page_texts(&reopened, back), ["Page 1"]); + let page = pdfcraft_model::pages(&reopened.get(back).unwrap().editor.as_ref().unwrap().cos).swap_remove(0); + assert!(!page.dict.contains(b"Annots"), "clear: {clear}"); + + // Undo past that save brings the mark back, under the number the saved file left free. + s.mark_saved(id, saved, None).unwrap(); + s.undo(id).unwrap(); + let again = s.save_bytes(id).unwrap(); + let mut reopened = Session::new(); + let back = reopened.open("again.pdf", None, again.clone(), None).unwrap(); + let d = reopened.get(back).unwrap(); + assert!(d.info.warnings.is_empty() && d.editor.as_ref().unwrap().cos.repair_log().is_empty(), "{:?}", d.info.warnings); + assert_eq!(d.redaction_marks(), 1, "clear: {clear}"); + let marks = pdfcraft_redact::marks(&d.editor.as_ref().unwrap().cos); + assert_eq!(marks.first().map(|m| m.overlay.as_str()), Some("MARKED OVERLAY")); + } +} + #[test] fn oversized_redaction_overlay_preserves_the_document_and_undo() { let (mut s, id) = session_with(1);