diff --git a/crates/sign/src/pdf/changes.rs b/crates/sign/src/pdf/changes.rs index 3a10a74e41..de450781ee 100644 --- a/crates/sign/src/pdf/changes.rs +++ b/crates/sign/src/pdf/changes.rs @@ -187,20 +187,9 @@ impl ChangedObjectClassifier<'_> { } fn annotation_array_change(&self, now: &[Object], before: Option<&Object>) -> &'static str { - let was: HashSet = - before.and_then(Object::as_array).map(|items| items.iter().filter_map(Object::as_ref).collect()).unwrap_or_default(); - let kept: HashSet = now.iter().filter_map(Object::as_ref).collect(); - let removed: Vec = was.iter().filter(|reference| !kept.contains(reference)).copied().collect(); - if removed.is_empty() { - let added: Vec = now.iter().filter_map(Object::as_ref).filter(|reference| !was.contains(reference)).collect(); - added_annots(self.doc, &added) - } else if removed.iter().any(|reference| self.old.get(*reference).as_dict().is_some_and(|dict| dict.name(b"Subtype") == Some(b"Widget"))) { - // A form field or signature widget taken off the page: no permission covers it. - kind::OTHER - } else { - // Deleting comments is a comment change (permitted at P 3 only). - kind::COMMENTS - } + let refs_of = |items: &[Object]| items.iter().filter_map(Object::as_ref).collect::>(); + let was = before.and_then(Object::as_array).map(|a| refs_of(a)).unwrap_or_default(); + annot_change(self.doc, self.old, &was, &refs_of(now)) } fn dictionary_change(&self, r: ObjRef, dict: &Dict, before: Option<&Object>) -> &'static str { @@ -391,6 +380,22 @@ fn added_annots(doc: &Document, added: &[ObjRef]) -> &'static str { } } +/// What changing an annotation array from `was` to `now` amounts to, additions and removals judged +/// together. Taking a widget (form field or signature) off a page is covered by no permission. +/// Deleting comments is a comment change, permitted at P 3 only: no weaker than anything added +/// beside it, so it is the verdict whatever else the array gained. +fn annot_change(doc: &Document, old: &Document, was: &[ObjRef], now: &[ObjRef]) -> &'static str { + let mut removed = was.iter().filter(|r| !now.contains(r)).peekable(); + if removed.peek().is_none() { + let added: Vec = now.iter().filter(|r| !was.contains(r)).copied().collect(); + added_annots(doc, &added) + } else if removed.any(|r| old.get(*r).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget"))) { + kind::OTHER + } else { + kind::COMMENTS + } +} + fn refs(doc: &Document, o: Option<&Object>) -> Vec { o.map(|o| doc.resolve(o)).and_then(|a| a.as_array().map(|a| a.iter().filter_map(Object::as_ref).collect())).unwrap_or_default() } @@ -522,11 +527,7 @@ fn signed_page_resources(old: &Document, pages: &HashSet) -> HashMap) -> &'static str { match before { Some(b) if without(d, &[b"Annots"]) != without(b, &[b"Annots"]) => kind::PAGE_CONTENT, - Some(b) => { - let was = refs(old, b.get(b"Annots")); - let added: Vec = refs(doc, d.get(b"Annots")).into_iter().filter(|r| !was.contains(r)).collect(); - added_annots(doc, &added) - } + Some(b) => annot_change(doc, old, &refs(old, b.get(b"Annots")), &refs(doc, d.get(b"Annots"))), None => kind::PAGE_CONTENT, } } @@ -585,11 +586,7 @@ fn change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>) Some(b"Page") => { return match before { Some(b) if without(d, &[b"Annots"]) != without(b, &[b"Annots"]) => kind::PAGE_CONTENT, - Some(b) => { - let was = refs(old, b.get(b"Annots")); - let added: Vec = refs(doc, d.get(b"Annots")).into_iter().filter(|r| !was.contains(r)).collect(); - added_annots(doc, &added) - } + Some(b) => annot_change(doc, old, &refs(old, b.get(b"Annots")), &refs(doc, d.get(b"Annots"))), None => kind::COMMENTS, }; } diff --git a/crates/sign/tests/it/pdf.rs b/crates/sign/tests/it/pdf.rs index 869a4bebd5..0d9f1eb3a4 100644 --- a/crates/sign/tests/it/pdf.rs +++ b/crates/sign/tests/it/pdf.rs @@ -209,6 +209,21 @@ fn an_indirect_annots_array_is_judged_by_the_annotations_it_gains() { assert_eq!((s.status, s.modification), (Status::Invalid, Modification::Disallowed(vec![kind::OTHER]))); } +/// A signature widget taken off the page's inline /Annots array is covered by no permission (the +/// indirect-array case is the test above), and stays a change whatever level the signature has. +#[test] +fn removing_a_widget_from_an_inline_annots_array_is_not_a_comment_change() { + let id = pkcs12::open(&data("rsa-aes.p12"), "test").unwrap(); + let trust = TrustStore { certs: vec![id.certificate.clone()] }; + let page = pdfcraft_cos::ObjRef { num: 3, generation: 0 }; + let signed = pdfcraft_sign::sign(&open(&fixture()), &id, &opts()).unwrap(); + let inline = open(&signed).get(page).as_dict().and_then(|d| d.get(b"Annots").cloned()).unwrap(); + assert!(matches!(inline, Object::Array(ref a) if !a.is_empty()), "the page keeps its widgets inline"); + let stripped = edit_after(&signed, |doc| doc.update_dict(page, |p| p.set(b"Annots".to_vec(), Object::Array(vec![]))).unwrap()); + let s = signatures(&open(&stripped), &stripped, &trust).into_iter().find(|s| s.signed).unwrap(); + assert_eq!((s.status, s.modification), (Status::Invalid, Modification::Disallowed(vec![kind::OTHER]))); +} + /// Issue #1328: a signature field whose /V is null, directly or through a reference, is unsigned: /// it is listed as unsigned and can be signed by name. #[test]