Merge pull request #6502 from mtlcl410/fix/sign-string-bytes-not-spelling

M9: a signed string respelled in hexadecimal is no change after signing (sign.changes-after-signing)

crates/sign/src/pdf/changes.rs

@@ -168,7 +168,7 @@ pub(super) fn classify_changes(doc: &Document, old: Option<Document>, p: Option<
         let r = ObjRef { num, generation: doc.generation(num) };
         let new = doc.get(r);
         let before = old.try_get(num).ok().filter(|o| !matches!(**o, Object::Null));
-        if before.as_deref() == Some(&*new) {
+        if before.as_deref().is_some_and(|b| same_object(b, &new)) {
             // The same content at a new place. That is true for the renderer as well only when
             // the entry locates the object itself: where this reader had to search for the
             // object, another reader may arrive at a different one.
@@ -238,7 +238,7 @@ fn only_validation_data(allowed: &mut Vec<&'static str>, disallowed: &mut Vec<&'
     // Compared by content: an indirect /Perms rewritten in place keeps its reference (and, shared
     // with the trailer's /Info, would otherwise pass as a metadata change).
     let perms = |d: &Document| d.root().and_then(|r| d.get(r).as_dict().and_then(|c| c.get(b"Perms").map(|p| (*d.resolve(p)).clone())));
-    if perms(doc) != perms(old) && !disallowed.contains(&kind::STRUCTURE) {
+    if !same_entry(perms(doc).as_ref(), perms(old).as_ref()) && !disallowed.contains(&kind::STRUCTURE) {
         disallowed.push(kind::STRUCTURE);
     }
 }
@@ -269,7 +269,7 @@ fn only_invisible_timestamps_added(doc: &Document, old: &Document) -> bool {
             continue;
         }
         // Unchanged since signing: the certification already covered it.
-        if old.try_get(num).ok().filter(|o| !matches!(**o, Object::Null)).as_deref() == Some(&*new) {
+        if old.try_get(num).ok().filter(|o| !matches!(**o, Object::Null)).is_some_and(|o| same_object(&o, &new)) {
             continue;
         }
         let value = if is_value { Some(d.clone()) } else { d.get(b"V").map(|v| doc.resolve(v)).and_then(|v| v.as_dict().cloned()) };
@@ -330,7 +330,7 @@ impl ChangedObjectClassifier<'_> {
     }
 
     fn keeps_previous_entries(&self, dict: &Dict, before: Option<&Object>) -> bool {
-        before.and_then(Object::as_dict).is_some_and(|previous| previous.iter().all(|(key, value)| dict.get(key) == Some(value)))
+        before.and_then(Object::as_dict).is_some_and(|previous| previous.iter().all(|(key, value)| same_entry(dict.get(key), Some(value))))
     }
 
     fn is_signed_annotation_array(&self, r: ObjRef, before: Option<&Object>) -> bool {
@@ -402,7 +402,7 @@ impl ChangedObjectClassifier<'_> {
     /// `/Type`) is a page content change.
     fn page_change_kind(&self, d: &Dict, before: Option<&Dict>) -> Option<&'static str> {
         match before {
-            Some(b) if without(d, &[b"Annots"]) != without(b, &[b"Annots"]) => Some(kind::PAGE_CONTENT),
+            Some(b) if !same_but_annots(d, b) => Some(kind::PAGE_CONTENT),
             Some(b) => match (annot_refs(self.old, b.get(b"Annots"), self.annot_budget), annot_refs(self.doc, d.get(b"Annots"), self.annot_budget)) {
                 (Some(was), Some(now)) => self.annots_change(&was, &now),
                 _ => Some(kind::OTHER),
@@ -520,14 +520,14 @@ pub(crate) fn only_grew(doc: &Document, old: &Document, before: &Object, now: &O
             let refs = |a: &[Object]| a.iter().map(Object::as_ref).collect::<Option<HashSet<ObjRef>>>();
             match (refs(b), refs(n)) {
                 (Some(b), Some(n)) => b.is_subset(&n),
-                _ => b.len() <= MAX_COMPARED && n.len() <= MAX_COMPARED && b.iter().all(|x| n.contains(x)),
+                _ => b.len() <= MAX_COMPARED && n.len() <= MAX_COMPARED && b.iter().all(|x| n.iter().any(|y| same_object(x, y))),
             }
         }
         (Object::Dict(b), Object::Dict(n)) => b.iter().all(|(k, v)| match n.get(k) {
-            Some(nv) => nv == v || (depth < 4 && only_grew(doc, old, &old.resolve(v), &doc.resolve(nv), depth + 1)),
+            Some(nv) => same_object(nv, v) || (depth < 4 && only_grew(doc, old, &old.resolve(v), &doc.resolve(nv), depth + 1)),
             None => false,
         }),
-        _ => before == now,
+        _ => same_object(before, now),
     }
 }
 
@@ -651,13 +651,7 @@ fn acroform_kind(doc: &Document, old: &Document, now: &Dict, before: Option<&Dic
     if lost_entries(doc, old, before.get(b"Fields"), now.get(b"Fields")) {
         return kind::OTHER;
     }
-    let strip = |d: &Dict| -> Vec<(Vec<u8>, Object)> {
-        let mut v: Vec<(Vec<u8>, Object)> =
-            d.iter().filter(|(k, _)| !matches!(k.as_slice(), b"Fields" | b"SigFlags")).map(|(k, o)| (k.to_vec(), o.clone())).collect();
-        v.sort_by(|a, b| a.0.cmp(&b.0));
-        v
-    };
-    if strip(now) != strip(before) {
+    if !same_entries(now, before, &[b"Fields", b"SigFlags"]) {
         return kind::FORM_FILL;
     }
     let old_fields = refs(old, before.get(b"Fields"));
@@ -665,11 +659,53 @@ fn acroform_kind(doc: &Document, old: &Document, now: &Dict, before: Option<&Dic
     if added.is_empty() || added_annots(doc, &added) == kind::SIGNATURE { kind::SIGNATURE } else { kind::FORM_FILL }
 }
 
-/// Dictionary entries in a stable order, without `keys`.
-fn without(x: &Dict, keys: &[&[u8]]) -> Vec<(Vec<u8>, Object)> {
-    let mut v: Vec<(Vec<u8>, Object)> = x.iter().filter(|(k, _)| !keys.contains(&k.as_slice())).map(|(k, o)| (k.to_vec(), o.clone())).collect();
-    v.sort_by(|a, b| a.0.cmp(&b.0));
-    v
+/// Whether two page dictionaries are the same apart from `/Annots`. Dictionaries are compared
+/// without regard to key order at every depth: a writer may re-serialise a page's inline
+/// resources in another order without changing what the page shows.
+fn same_but_annots(d: &Dict, b: &Dict) -> bool {
+    same_entries(d, b, &[b"Annots"])
+}
+
+/// Whether two dictionaries hold the same entries apart from `skip`: both sorted by key, then
+/// compared pairwise, so neither side can hide a key the other lacks.
+fn same_entries(a: &Dict, b: &Dict, skip: &[&[u8]]) -> bool {
+    fn sorted<'a>(x: &'a Dict, skip: &[&[u8]]) -> Vec<(&'a [u8], &'a Object)> {
+        let mut v: Vec<(&[u8], &Object)> = x.iter().map(|(k, o)| (k.as_slice(), o)).filter(|(k, _)| !skip.contains(k)).collect();
+        v.sort_by(|p, q| p.0.cmp(q.0));
+        v
+    }
+    let (x, y) = (sorted(a, skip), sorted(b, skip));
+    x.len() == y.len() && x.iter().zip(&y).all(|((k, v), (l, w))| k == l && same_object(v, w))
+}
+
+/// Object equality where dictionaries (and stream dictionaries) ignore key order; arrays keep it.
+/// A string is its bytes (ISO 32000-2 §7.3.4.1): written as a literal or as hexadecimal, it is the
+/// same string, so a later save that respells one changes nothing it says. Likewise a number is
+/// its value (§7.3.3: "wherever a real number is expected, an integer may be used instead"), so a
+/// signed `/Rect [20.0 20.0 120.0 40.0]` saved again as `[20 20 120 40]` is the same rectangle.
+/// References compare by object and generation, and stream data byte for byte as stored. The
+/// recursion follows the parsed nesting, which the parser bounds.
+fn same_object(a: &Object, b: &Object) -> bool {
+    match (a, b) {
+        (Object::String(a), Object::String(b)) => a.bytes == b.bytes,
+        // Compared in i128, where every i64 and every whole real near its range is exact (the cast
+        // saturates far beyond it): no fractional real, NaN or infinity (whose fract is NaN)
+        // equals an integer, nor does the real 2^63 pass for i64::MAX.
+        (Object::Int(i), Object::Real(r)) | (Object::Real(r), Object::Int(i)) => r.fract() == 0.0 && *r as i128 == i128::from(*i),
+        (Object::Dict(a), Object::Dict(b)) => same_entries(a, b, &[]),
+        (Object::Stream(a), Object::Stream(b)) => a.raw == b.raw && same_entries(&a.dict, &b.dict, &[]),
+        (Object::Array(a), Object::Array(b)) => a.len() == b.len() && a.iter().zip(b).all(|(v, w)| same_object(v, w)),
+        _ => a == b,
+    }
+}
+
+/// [`same_object`] for an entry that may be absent: both absent, or both present and the same.
+fn same_entry(a: Option<&Object>, b: Option<&Object>) -> bool {
+    match (a, b) {
+        (Some(a), Some(b)) => same_object(a, b),
+        (None, None) => true,
+        _ => false,
+    }
 }
 
 /// Whether a dictionary was a signature object: a signature or timestamp value, a signature
@@ -769,7 +805,7 @@ fn extensions_kept(doc: &Document, old: &Document, d: &Dict, b: &Dict) -> bool {
     let was = b.get(b"Extensions").map(|e| old.resolve(e)).and_then(|e| e.as_dict().cloned());
     match (was, now) {
         (None, _) => true,
-        (Some(was), Some(now)) => was.iter().all(|(k, v)| now.get(k) == Some(v)),
+        (Some(was), Some(now)) => was.iter().all(|(k, v)| same_entry(now.get(k), Some(v))),
         (Some(_), None) => false,
     }
 }
@@ -777,18 +813,18 @@ fn extensions_kept(doc: &Document, old: &Document, d: &Dict, b: &Dict) -> bool {
 /// A change to the signed catalog `b`, now `d`.
 fn catalog_kind(doc: &Document, old: &Document, d: &Dict, b: &Dict) -> &'static str {
     let keys: &[&[u8]] = &[b"AcroForm", b"DSS", b"Perms", b"Metadata", b"NeedsRendering", b"Extensions"];
-    if without(d, keys) != without(b, keys) {
+    if !same_entries(d, b, keys) {
         kind::STRUCTURE
     } else if !extensions_kept(doc, old, d, b) {
         // Developer extensions (ISO 32000-2 §7.12) only declare what the file uses. A writer
         // adding a /DSS also declares the ESIC extension it belongs to, so a declaration may be
         // added, but none that was signed may change or go.
         kind::STRUCTURE
-    } else if d.get(b"AcroForm") != b.get(b"AcroForm") {
+    } else if !same_entry(d.get(b"AcroForm"), b.get(b"AcroForm")) {
         // Compare the forms themselves, inline or not.
         let now = acroform(doc).unwrap_or_default();
         acroform_kind(doc, old, &now, None)
-    } else if b.get(b"Perms") != d.get(b"Perms") {
+    } else if !same_entry(b.get(b"Perms"), d.get(b"Perms")) {
         kind::SIGNATURE
     } else {
         kind::METADATA
@@ -840,7 +876,7 @@ fn change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>)
     // so relabelling the widget now does not change that.
     if before.is_some_and(|b| {
         b.name(b"Subtype") == Some(b"Widget") && b.name(b"FT") == Some(b"Sig") && b.get(b"V").is_some_and(|v| old.resolve(v).as_dict().is_some())
-    }) && before != Some(d)
+    }) && !before.is_some_and(|b| same_entries(d, b, &[]))
     {
         return kind::PAGE_CONTENT;
     }
@@ -854,7 +890,9 @@ fn change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>)
     }
     // Where a signed widget sits and whether it shows are what the page shows: filling the form
     // changes its value and appearance, never its rectangle or its flags.
-    if before.is_some_and(|b| b.name(b"Subtype") == Some(b"Widget") && (d.get(b"Rect") != b.get(b"Rect") || d.get(b"F") != b.get(b"F"))) {
+    if before.is_some_and(|b| {
+        b.name(b"Subtype") == Some(b"Widget") && (!same_entry(d.get(b"Rect"), b.get(b"Rect")) || !same_entry(d.get(b"F"), b.get(b"F")))
+    }) {
         return kind::PAGE_CONTENT;
     }
     match ty {
@@ -863,7 +901,7 @@ fn change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>)
         Some(b"Catalog") => return before.map_or(kind::FORM_FILL, |b| catalog_kind(doc, old, d, b)),
         Some(b"Page") => {
             return match before {
-                Some(b) if without(d, &[b"Annots"]) != without(b, &[b"Annots"]) => kind::PAGE_CONTENT,
+                Some(b) if !same_but_annots(d, b) => kind::PAGE_CONTENT,
                 Some(b) => {
                     let was = refs(old, b.get(b"Annots"));
                     let added: Vec<ObjRef> = refs(doc, d.get(b"Annots")).into_iter().filter(|r| !was.contains(r)).collect();
@@ -915,6 +953,154 @@ mod tests {
     use super::*;
     use std::sync::Arc;
 
+    /// A signed page's change, judged by [`ChangedObjectClassifier::page_change_kind`] with no
+    /// other signed roles in play; `None` when nothing the page shows changed.
+    fn page_change_kind(doc: &Document, old: &Document, d: &Dict, before: Option<&Dict>) -> Option<&'static str> {
+        let (none, budget, resources) = (HashSet::new(), Cell::new(MAX_ANNOT_ENTRIES), HashMap::new());
+        let classifier = ChangedObjectClassifier {
+            doc,
+            old,
+            old_content: &none,
+            old_metadata: None,
+            old_xfa_data: &none,
+            signed_pages: &none,
+            signed_annots: &none,
+            page_nodes: &none,
+            kids_arrays: &none,
+            placed_annots: &none,
+            annot_budget: &budget,
+            signed_resources: &resources,
+            old_info: None,
+            old_form: None,
+            dss: &none,
+        };
+        classifier.page_change_kind(d, before)
+    }
+
+    fn font(order: &[&str], base: &str) -> Object {
+        let mut d = Dict::new();
+        for k in order {
+            match *k {
+                "Type" => d.set(b"Type".to_vec(), Object::name("Font")),
+                "Subtype" => d.set(b"Subtype".to_vec(), Object::name("Type1")),
+                "BaseFont" => d.set(b"BaseFont".to_vec(), Object::name(base)),
+                _ => {
+                    let mut w = Dict::new();
+                    w.set(b"Differences".to_vec(), Object::Array(vec![Object::Int(32), Object::name("space")]));
+                    w.set(b"Type".to_vec(), Object::name("Encoding"));
+                    d.set(b"Encoding".to_vec(), Object::Dict(w));
+                }
+            }
+        }
+        Object::Dict(d)
+    }
+
+    fn page(fonts: Vec<(&str, Object)>, annots: Vec<Object>) -> Dict {
+        let mut f = Dict::new();
+        fonts.into_iter().for_each(|(k, v)| f.set(k.as_bytes().to_vec(), v));
+        let mut r = Dict::new();
+        r.set(b"Font".to_vec(), Object::Dict(f));
+        let mut p = Dict::new();
+        p.set(b"Type".to_vec(), Object::name("Page"));
+        p.set(b"Resources".to_vec(), Object::Dict(r));
+        p.set(b"Annots".to_vec(), Object::Array(annots));
+        p
+    }
+
+    #[test]
+    fn reordered_inline_resources_are_not_page_content() {
+        let doc = Document::new_empty();
+        let order = ["Type", "Subtype", "BaseFont", "Encoding"];
+        let before = page(vec![("F1", font(&order, "Times-Roman")), ("F2", font(&order, "Helvetica"))], vec![]);
+        let unchanged = page_change_kind(&doc, &doc, &before, Some(&before));
+        let reordered = page(
+            vec![
+                ("F2", font(&["Encoding", "BaseFont", "Type", "Subtype"], "Helvetica")),
+                ("F1", font(&["BaseFont", "Subtype", "Encoding", "Type"], "Times-Roman")),
+            ],
+            vec![],
+        );
+        assert_eq!(page_change_kind(&doc, &doc, &reordered, Some(&before)), unchanged);
+        assert_eq!(change_kind(&doc, &doc, &reordered, Some(&before)), change_kind(&doc, &doc, &before, Some(&before)));
+
+        // A changed value inside the nested dictionary still is a page content change.
+        let changed = page(vec![("F2", font(&order, "Helvetica")), ("F1", font(&order, "Courier"))], vec![]);
+        assert_eq!(page_change_kind(&doc, &doc, &changed, Some(&before)), Some(kind::PAGE_CONTENT));
+        assert_eq!(change_kind(&doc, &doc, &changed, Some(&before)), kind::PAGE_CONTENT);
+    }
+
+    /// The page as signed and the same page with its inline resources reordered are each no
+    /// page change at all, stated as `None` rather than only equal to each other, and so is a page
+    /// whose numbers and strings are respelled as well (§7.3.3, §7.3.4.1).
+    #[test]
+    fn reordered_and_respelled_pages_are_no_page_change() {
+        let doc = Document::new_empty();
+        let order = ["Type", "Subtype", "BaseFont", "Encoding"];
+        let mut before = page(vec![("F1", font(&order, "Times-Roman")), ("F2", font(&order, "Helvetica"))], vec![]);
+        before.set(b"MediaBox".to_vec(), Object::Array(vec![Object::Int(0), Object::Int(0), Object::Real(612.0), Object::Real(792.0)]));
+        before.set(
+            b"PieceInfo".to_vec(),
+            Object::Dict(Dict::from_iter([(b"Note".to_vec(), Object::String(pdfcraft_cos::PdfString::literal("signed")))])),
+        );
+        let mut reordered = page(
+            vec![
+                ("F2", font(&["Encoding", "BaseFont", "Type", "Subtype"], "Helvetica")),
+                ("F1", font(&["BaseFont", "Subtype", "Encoding", "Type"], "Times-Roman")),
+            ],
+            vec![],
+        );
+        reordered.set(b"MediaBox".to_vec(), Object::Array(vec![Object::Real(0.0), Object::Int(0), Object::Int(612), Object::Int(792)]));
+        reordered.set(
+            b"PieceInfo".to_vec(),
+            Object::Dict(Dict::from_iter([(b"Note".to_vec(), Object::String(pdfcraft_cos::PdfString { bytes: b"signed".to_vec(), hex: true }))])),
+        );
+        assert_eq!(page_change_kind(&doc, &doc, &before, Some(&before)), None);
+        assert_eq!(page_change_kind(&doc, &doc, &reordered, Some(&before)), None);
+        assert_eq!(page_change_kind(&doc, &doc, &before, Some(&reordered)), None);
+
+        // A real that is not the integer it replaced is a moved box.
+        let mut moved = reordered.clone();
+        moved.set(b"MediaBox".to_vec(), Object::Array(vec![Object::Int(0), Object::Int(0), Object::Real(612.5), Object::Int(792)]));
+        assert_eq!(page_change_kind(&doc, &doc, &moved, Some(&before)), Some(kind::PAGE_CONTENT));
+    }
+
+    /// Numbers compare by value across integer and real, and only when the values are equal.
+    #[test]
+    fn integers_and_reals_compare_by_value() {
+        let same = |a: Object, b: Object| same_object(&a, &b) && same_object(&b, &a);
+        assert!(same(Object::Int(20), Object::Real(20.0)));
+        assert!(same(Object::Int(0), Object::Real(-0.0)));
+        assert!(same(Object::Int(-7), Object::Real(-7.0)));
+        assert!(!same(Object::Int(20), Object::Real(20.5)));
+        assert!(!same(Object::Int(20), Object::Real(f64::NAN)));
+        assert!(!same(Object::Int(i64::MAX), Object::Real(f64::INFINITY)));
+        assert!(!same(Object::Int(i64::MAX), Object::Real(9_223_372_036_854_775_808.0)));
+        assert!(!same(Object::Int(1), Object::Bool(true)));
+    }
+
+    #[test]
+    fn a_swapped_nested_key_is_page_content_both_ways() {
+        let doc = Document::new_empty();
+        let order = ["Type", "Subtype", "BaseFont"];
+        let signed = page(vec![("F1", font(&order, "Times-Roman"))], vec![]);
+        // Same entry count, but /Subtype was dropped and /Encoding added in its place.
+        let swapped = page(vec![("F1", font(&["Type", "Encoding", "BaseFont"], "Times-Roman"))], vec![]);
+        for (now, then) in [(&swapped, &signed), (&signed, &swapped)] {
+            assert_eq!(page_change_kind(&doc, &doc, now, Some(then)), Some(kind::PAGE_CONTENT));
+            assert_eq!(change_kind(&doc, &doc, now, Some(then)), kind::PAGE_CONTENT);
+        }
+    }
+
+    #[test]
+    fn reordered_arrays_are_still_page_content() {
+        let doc = Document::new_empty();
+        let mut before = page(vec![], vec![]);
+        before.set(b"MediaBox".to_vec(), Object::Array(vec![Object::Int(0), Object::Int(0), Object::Int(612), Object::Int(792)]));
+        let mut after = before.clone();
+        after.set(b"MediaBox".to_vec(), Object::Array(vec![Object::Int(0), Object::Int(0), Object::Int(792), Object::Int(612)]));
+        assert_eq!(page_change_kind(&doc, &doc, &after, Some(&before)), Some(kind::PAGE_CONTENT));
+    }
+
     /// A PDF of `objs` (numbered from 1) whose trailer also holds `trailer_extra`.
     fn pdf(objs: &[&str], trailer_extra: &str) -> Vec<u8> {
         let mut out = b"%PDF-1.7\n".to_vec();

crates/sign/src/pdf/revision_classification_tests.rs

@@ -282,3 +282,118 @@ fn a_repaired_slot_rewritten_cleanly_is_judged_by_its_references() {
     let now = signed.clone().revision(&[(4, Row::Obj(0, 0, &filled))]);
     assert!(matches!(verdict(&now.open(), signed.open(), Some(2)), Modification::Disallowed(_)));
 }
+
+/// A signed catalog and text field whose strings a later save respells, given the catalog `extra`
+/// entries and the field's `/T` as written in each revision.
+fn respelled(signed_extra: &str, now_extra: &str, signed_t: &str, now_t: &str) -> (Document, Document) {
+    let catalog = |extra: &str| format!("<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [5 0 R] /DA (/Helv 0 Tf 0 g) >> {extra} >>");
+    let field = |t: &str| format!("<< /Type /Annot /Subtype /Widget /FT /Tx /T {t} /V (Ann) /Rect [20 20 120 40] /P 3 0 R >>");
+    let signed = File::new().revision(&[
+        (1, Row::Obj(0, 0, &catalog(signed_extra))),
+        (2, Row::Obj(0, 0, PAGES)),
+        (3, Row::Obj(0, 0, &page("/Annots [5 0 R]"))),
+        (4, Row::Obj(0, 0, &page("/Annots []"))),
+        (5, Row::Obj(0, 0, &field(signed_t))),
+    ]);
+    let now = signed.clone().revision(&[(1, Row::Obj(0, 0, &catalog(now_extra))), (5, Row::Obj(0, 0, &field(now_t)))]);
+    (now.open(), signed.open())
+}
+
+/// A string is its bytes (ISO 32000-2 §7.3.4.1): a later save that writes the catalog's /Lang in
+/// hexadecimal instead of as a literal changes nothing, under every permission level.
+#[test]
+fn a_string_respelled_in_hex_is_no_change() {
+    for p in [Some(1), Some(2), Some(3), None] {
+        let (doc, old) = respelled("/Lang (en-US)", "/Lang <656E2D5553>", "(Name)", "(Name)");
+        assert_eq!(verdict(&doc, old, p), Modification::Allowed(Vec::new()), "P={p:?}");
+    }
+}
+
+/// The same holds for strings nested in dictionaries and arrays (a name tree's keys), with the
+/// entries also in another order, and for a signed field's /T.
+#[test]
+fn nested_strings_respelled_and_reordered_are_no_change() {
+    let (doc, old) = respelled(
+        "/Names << /Dests << /Names [(intro) [3 0 R /Fit]] >> >> /Lang (en-US)",
+        "/Lang <656e2d5553> /Names << /Dests << /Names [<696E74726F> [3 0 R /Fit]] >> >>",
+        "(Name)",
+        "<4E616D65>",
+    );
+    assert_eq!(verdict(&doc, old, Some(1)), Modification::Allowed(Vec::new()));
+}
+
+/// Different bytes are a different string however they are written: a catalog whose /Lang (or a
+/// string deep in its name tree) now says something else is still a structure change.
+#[test]
+fn a_string_whose_bytes_change_is_still_a_change() {
+    for (signed, now) in [
+        ("/Lang (en-US)", "/Lang <64652D4445>"),
+        ("/Names << /Dests << /Names [(intro) [3 0 R /Fit]] >> >>", "/Names << /Dests << /Names [<6F75747472> [3 0 R /Fit]] >> >>"),
+    ] {
+        let (doc, old) = respelled(signed, now, "(Name)", "(Name)");
+        assert_eq!(verdict(&doc, old, Some(2)), Modification::Disallowed(vec![super::kind::STRUCTURE]), "{now}");
+    }
+}
+
+/// A save that adds a signature field and respells the catalog's and the form's strings on the
+/// way is judged by the field it added: the respelled /Lang and /DA are not a structure or form
+/// change.
+#[test]
+fn a_signature_added_with_respelled_strings_is_signing() {
+    let signed = File::new().revision(&[
+        (1, Row::Obj(0, 0, "<< /Type /Catalog /Pages 2 0 R /Lang (en-US) /AcroForm << /Fields [5 0 R] /DA (/Helv 0 Tf 0 g) >> >>")),
+        (2, Row::Obj(0, 0, PAGES)),
+        (3, Row::Obj(0, 0, &page("/Annots [5 0 R]"))),
+        (4, Row::Obj(0, 0, &page("/Annots []"))),
+        (5, Row::Obj(0, 0, TEXT_FIELD)),
+    ]);
+    let catalog =
+        |lang: &str, da: &str| format!("<< /Type /Catalog /Pages 2 0 R /Lang {lang} /AcroForm << /DA {da} /Fields [5 0 R 6 0 R] /SigFlags 3 >> >>");
+    let sig = "<< /Type /Annot /Subtype /Widget /FT /Sig /T (Sig1) /Rect [0 0 0 0] /P 4 0 R >>";
+    let now = |lang: &str, da: &str| {
+        signed.clone().revision(&[(1, Row::Obj(0, 0, &catalog(lang, da))), (4, Row::Obj(0, 0, &page("/Annots [6 0 R]"))), (6, Row::Obj(0, 0, sig))])
+    };
+    let respelled = now("<656E2D5553>", "<2F48656C76203020546620302067>");
+    assert_eq!(verdict(&respelled.open(), signed.open(), Some(2)), Modification::Allowed(vec![super::kind::SIGNATURE]));
+    // A /DA that now says something else is a form change, however it is spelled.
+    let restyled = now("<656E2D5553>", "<2F48656C7620313220546620302067>");
+    assert_eq!(verdict(&restyled.open(), signed.open(), None), Modification::Allowed(vec![super::kind::FORM_FILL, super::kind::SIGNATURE]));
+}
+
+/// A signed widget whose /Rect was written with reals, filled and saved by PdfCraft's own writer,
+/// which writes a whole real as an integer: the rectangle is the same numbers (ISO 32000-2
+/// §7.3.3, "wherever a real number is expected, an integer may be used instead"), so the save is a
+/// form fill. A rectangle whose numbers did change is still a moved widget.
+#[test]
+fn a_widget_rect_saved_back_as_integers_is_a_form_fill() {
+    let field = "<< /Type /Annot /Subtype /Widget /FT /Tx /T (Name) /V (Ann) /Rect [20.0 20.0 120.0 40.0] /P 3 0 R >>";
+    let signed = File::new().revision(&[
+        (1, Row::Obj(0, 0, "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [5 0 R] >> >>")),
+        (2, Row::Obj(0, 0, PAGES)),
+        (3, Row::Obj(0, 0, &page("/Annots [5 0 R]"))),
+        (4, Row::Obj(0, 0, &page("/Annots []"))),
+        (5, Row::Obj(0, 0, field)),
+    ]);
+    let save = |rect: Option<f64>| {
+        let mut doc = signed.open();
+        doc.update_dict(r(5, 0), |w| {
+            w.set(b"V".to_vec(), Object::String(pdfcraft_cos::PdfString::literal("Bob")));
+            if let Some(x) = rect {
+                w.set(b"Rect".to_vec(), Object::Array(vec![Object::Real(x), Object::Real(20.0), Object::Real(120.0), Object::Real(40.0)]));
+            }
+        })
+        .unwrap();
+        let bytes = pdfcraft_cos::write_incremental(&doc, &pdfcraft_cos::SaveOptions { object_streams: false, ..Default::default() }).unwrap();
+        Document::open(Arc::new(bytes)).unwrap()
+    };
+    let filled = save(None);
+    let rect = filled.resolve(&Object::Ref(r(5, 0))).as_dict().and_then(|w| w.get(b"Rect").cloned());
+    assert_eq!(
+        rect,
+        Some(Object::Array(vec![Object::Int(20), Object::Int(20), Object::Int(120), Object::Int(40)])),
+        "the writer respells whole reals"
+    );
+    assert_eq!(verdict(&filled, signed.open(), Some(2)), Modification::Allowed(vec![super::kind::FORM_FILL]));
+    let moved = save(Some(20.5));
+    assert_eq!(verdict(&moved, signed.open(), Some(2)), Modification::Disallowed(vec![super::kind::PAGE_CONTENT]));
+}

parity/acrobat-features.toml

@@ -6547,8 +6547,8 @@ milestone = "M9"
 status = "shipped"
 commands = ["sign.panel"]
 tools = ["sign_list"]
-evidence = ["crates/sign/tests/it/pdf.rs::later_changes_are_classified_under_the_signature_permissions", "crates/sign/tests/it/pdf.rs::signing_an_existing_field_and_counter_signing", "crates/engine/src/tests.rs::signing_saving_trusting_and_commenting_afterwards", "crates/sign/tests/it/pdf.rs::validation_data_and_xmp_added_after_signing_are_not_tampering", "crates/sign/tests/it/pdf.rs::a_dss_entry_does_not_make_the_object_it_names_part_of_the_store", "crates/sign/tests/it/pdf.rs::an_existing_dictionary_is_not_a_store_part_because_the_store_names_it", "crates/sign/tests/it/pdf.rs::an_existing_array_stays_what_it_is_when_the_store_points_at_it", "crates/sign/tests/it/pdf.rs::a_signed_store_may_grow_but_not_lose_or_swap_entries", "crates/sign/tests/it/pdf.rs::a_dictionary_labelled_metadata_is_not_a_metadata_change", "crates/sign/tests/it/pdf.rs::a_stream_already_labelled_metadata_when_signed_is_not_metadata_unless_the_catalog_names_it", "crates/sign/tests/it/pdf.rs::the_catalogs_own_xmp_stream_may_still_be_rewritten_after_signing", "crates/sign/tests/it/pdf.rs::rewritten_page_resources_are_page_content_whatever_keys_they_carry", "crates/sign/tests/it/pdf.rs::rewriting_a_certified_forms_xfa_data_is_a_form_fill", "crates/sign/tests/it/pdf.rs::the_catalogs_perms_do_not_change_under_no_changes"]
-notes = "revision diff classified as signing, form fill, comments, metadata, page content or structure. Later revisions that add DSS data (new /Certs, /CRLs, /OCSPs, /VRI objects, or ones of the signed store that only grew) or rewrite the catalog's XMP are permitted changes; a /DSS entry that names other objects, or a /Type /DSS or /Type /Metadata label on an existing dictionary or stream, does not excuse changing them (negative tests at every DocMDP level). A signed page's resources (fonts, XObjects, graphics states and what they reach) are page content by the role they held when signed, whatever keys a later revision writes into them (#1787). Rewriting the XFA `datasets` or `form` packet the signed revision named is form fill, as a Reader Extensions (UR3) save of a certified form does (#2759); the packets' roles come from the signed revision. Under P=1 a change to the catalog's /Perms breaks the certification."
+evidence = ["crates/sign/tests/it/pdf.rs::later_changes_are_classified_under_the_signature_permissions", "crates/sign/tests/it/pdf.rs::signing_an_existing_field_and_counter_signing", "crates/engine/src/tests.rs::signing_saving_trusting_and_commenting_afterwards", "crates/sign/tests/it/pdf.rs::validation_data_and_xmp_added_after_signing_are_not_tampering", "crates/sign/tests/it/pdf.rs::a_dss_entry_does_not_make_the_object_it_names_part_of_the_store", "crates/sign/tests/it/pdf.rs::an_existing_dictionary_is_not_a_store_part_because_the_store_names_it", "crates/sign/tests/it/pdf.rs::an_existing_array_stays_what_it_is_when_the_store_points_at_it", "crates/sign/tests/it/pdf.rs::a_signed_store_may_grow_but_not_lose_or_swap_entries", "crates/sign/tests/it/pdf.rs::a_dictionary_labelled_metadata_is_not_a_metadata_change", "crates/sign/tests/it/pdf.rs::a_stream_already_labelled_metadata_when_signed_is_not_metadata_unless_the_catalog_names_it", "crates/sign/tests/it/pdf.rs::the_catalogs_own_xmp_stream_may_still_be_rewritten_after_signing", "crates/sign/tests/it/pdf.rs::rewritten_page_resources_are_page_content_whatever_keys_they_carry", "crates/sign/tests/it/pdf.rs::rewriting_a_certified_forms_xfa_data_is_a_form_fill", "crates/sign/tests/it/pdf.rs::the_catalogs_perms_do_not_change_under_no_changes", "crates/sign/src/pdf/revision_classification_tests.rs::a_string_respelled_in_hex_is_no_change", "crates/sign/src/pdf/revision_classification_tests.rs::a_string_whose_bytes_change_is_still_a_change", "crates/sign/src/pdf/revision_classification_tests.rs::a_widget_rect_saved_back_as_integers_is_a_form_fill"]
+notes = "revision diff classified as signing, form fill, comments, metadata, page content or structure. Later revisions that add DSS data (new /Certs, /CRLs, /OCSPs, /VRI objects, or ones of the signed store that only grew) or rewrite the catalog's XMP are permitted changes; a /DSS entry that names other objects, or a /Type /DSS or /Type /Metadata label on an existing dictionary or stream, does not excuse changing them (negative tests at every DocMDP level). A signed page's resources (fonts, XObjects, graphics states and what they reach) are page content by the role they held when signed, whatever keys a later revision writes into them (#1787). Rewriting the XFA `datasets` or `form` packet the signed revision named is form fill, as a Reader Extensions (UR3) save of a certified form does (#2759); the packets' roles come from the signed revision. Under P=1 a change to the catalog's /Perms breaks the certification. Signed objects are compared by value: a string is its bytes, so one a later save rewrites from literal to hexadecimal (or back) is unchanged (ISO 32000-2 §7.3.4.1), and a number is its value, so a signed widget /Rect or page /MediaBox of whole reals saved back as integers is unchanged (§7.3.3)."
 
 [[feature]]
 id = "sign.view-signed-version"