mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-26 19:19:10 -06:00
fix(sign,cos): signing leaves out what was added and removed again
The signing saves wrote with default options, so a redaction mark added and cleared in the session (or a comment added and deleted) reached the file through a signature, a timestamped signature, embedded LTV evidence or a document timestamp, with its QuadPoints, overlay text and authors. A signed revision can't be changed afterwards, so nothing could take it out again. `sign`, `sign_with_timestamp` and `dss::embed` now save with `drop_unlinked`: everything they add is reachable (the signature through the field's `/V`, the store from the catalog), and the ByteRange placeholder is found in the written bytes. `timestamp_document` can't: its `/DocTimeStamp` dictionary has no field pointing to it, so the option would drop it and every document timestamp would fail with "ByteRange placeholder not found". It frees the leftovers first with the new `Document::free_unlinked_additions`, which frees exactly what the option leaves out, and then adds the dictionary. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
195a2c9bab
commit
ba33257fa3
@@ -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
|
||||
|
||||
@@ -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<u32, CosError> {
|
||||
/// 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<u32> {
|
||||
pub(crate) fn unlinked_additions(doc: &Document) -> HashSet<u32> {
|
||||
let added = |num: u32| matches!(doc.xref_entry(num), None | Some(XrefEntry::Free { .. }));
|
||||
let mut pending: HashMap<u32, Arc<Object>> =
|
||||
doc.overlay_entries().filter_map(|(num, _, o)| Some((num, o?))).filter(|(num, _)| added(*num)).collect();
|
||||
|
||||
@@ -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]
|
||||
|
||||
+47
-18
@@ -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);
|
||||
|
||||
@@ -93,7 +93,9 @@ pub fn embed(doc: &Document, evidence: &Evidence) -> Result<Vec<u8>, 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.
|
||||
|
||||
@@ -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"));
|
||||
|
||||
@@ -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};
|
||||
|
||||
Reference in New Issue
Block a user