diff --git a/crates/engine/src/session/save.rs b/crates/engine/src/session/save.rs index 1e6918f6a6..c8fa0d703a 100644 --- a/crates/engine/src/session/save.rs +++ b/crates/engine/src/session/save.rs @@ -276,7 +276,7 @@ impl Session { opts.date = self.signing_date(); } // A typed Password-field value is not signed into the file (#3926), as it isn't saved. - let cos = writable(&editor.cos)?; + let cos = drawn_for_first_signature(writable(&editor.cos)?); Ok(Arc::new(pdfcraft_sign::sign(&cos, id, &opts)?)) } @@ -294,7 +294,7 @@ impl Session { if opts.date.is_empty() { opts.date = self.signing_date(); } - let cos = writable(&editor.cos)?; + let cos = drawn_for_first_signature(writable(&editor.cos)?); let signed = Arc::new(pdfcraft_sign::sign_with_timestamp(&cos, id, &opts, tsa)?); // Checked like a save before anyone writes it (#2166). self.check_save(doc, &signed)?; @@ -336,3 +336,47 @@ impl Session { Ok(()) } } + +/// `cos` with its form fields drawn, when the signature about to be made is the document's first +/// and the form sets `/NeedAppearances` (#5939). That signature removes the flag, after which +/// viewers show each field's stored appearance; a filled field whose stored appearance draws no +/// text (an empty placeholder such as `/Tx BMC EMC`, or a stale look) would go blank. So every +/// text and choice field is drawn from its value first, as the document is displayed +/// ([`pdfcraft_forms::with_field_appearances`]), and the signature covers that look. +/// +/// One exception: where PdfCraft's drawing would show a character as `?` (the field's font can't +/// hold it) and the field's own appearance draws text, the field keeps its own appearance, which +/// the program that filled it drew with a font that has the character. A field whose own +/// appearance draws no text gets PdfCraft's drawing, `?` and all. Later signatures change +/// nothing here: redrawing would count as a change to the earlier signatures. +fn drawn_for_first_signature(cos: std::borrow::Cow<'_, pdfcraft_cos::Document>) -> std::borrow::Cow<'_, pdfcraft_cos::Document> { + use pdfcraft_forms::FieldKind; + if !pdfcraft_forms::need_appearances(&cos) || pdfcraft_sign::has_signatures(&cos) { + return cos; + } + let Some(mut drawn) = pdfcraft_forms::with_field_appearances(&cos) else { return cos }; + for f in pdfcraft_forms::fields(&cos) { + if !matches!(f.kind, FieldKind::Text | FieldKind::Combo | FieldKind::List) { + continue; + } + for w in &f.widgets { + let own = cos.get(w.obj).as_dict().filter(|d| draws_text(&cos, d)).and_then(|d| d.get(b"AP").cloned()); + // `.1`: the first character the drawing showed as `?`. + if let Some(ap) = own + && pdfcraft_forms::appearance::field_appearance_drawn(&cos, &f, w, &f.value, true).1.is_some() + { + drawn.update_dict(w.obj, |d| d.set(b"AP".to_vec(), ap)).ok(); + } + } + } + std::borrow::Cow::Owned(drawn) +} + +/// Whether the widget's normal appearance stream (`/AP /N`) shows text, itself or through a +/// form XObject it paints (counted as text without looking inside). +fn draws_text(cos: &pdfcraft_cos::Document, widget: &pdfcraft_cos::Dict) -> bool { + let ap = widget.get(b"AP").map(|ap| cos.resolve(ap)); + let normal = ap.as_deref().and_then(|ap| ap.as_dict()).and_then(|ap| ap.get(b"N")).map(|n| cos.resolve(n)); + let Some(pdfcraft_cos::Object::Stream(s)) = normal.as_deref() else { return false }; + s.decoded().is_ok_and(|data| pdfcraft_content::ops(&data).any(|op| matches!(op.op.as_slice(), b"Tj" | b"TJ" | b"'" | b"\"" | b"Do"))) +} diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index c109a0bc80..55c57c2f29 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2699,6 +2699,111 @@ fn a_signed_file_carries_no_password_field_value() { assert!(!appears_in_pdf(&signed, "*******"), "nor the typed length"); } +/// A form with `/NeedAppearances true` whose filled fields have stored appearances that draw no +/// text: `name` an empty placeholder, `town` one too with a value Helvetica can't hold (`ň`), +/// `city` such a value with its own appearance that draws it (#5939). +fn need_appearances_form() -> Vec { + let stream = + |content: &str| format!("<< /Type /XObject /Subtype /Form /BBox [0 0 160 20] /Length {} >>\nstream\n{content}\nendstream", content.len()); + let objs = [ + "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R 5 0 R 6 0 R] /NeedAppearances true /DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 7 0 R >> >> >> >>".to_string(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 300] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R 5 0 R 6 0 R] >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (name) /V (Ada Lovelace) /Rect [20 250 180 270] /P 3 0 R /AP << /N 8 0 R >> >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (town) /V /Rect [20 200 180 220] /P 3 0 R /AP << /N 9 0 R >> >>".into(), + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (city) /V /Rect [20 150 180 170] /P 3 0 R /AP << /N 10 0 R >> >>".into(), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".into(), + stream("/Tx BMC EMC"), + stream("/Tx BMC EMC"), + stream("/Tx BMC BT /Helv 10 Tf 2 5 Td (Brno r) Tj ET EMC"), + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + out +} + +/// The decoded normal appearance of the widget `num` in `doc`. +fn normal_appearance(doc: &pdfcraft_cos::Document, num: u32) -> String { + let widget = doc.get(pdfcraft_cos::ObjRef::new(num, 0)); + let ap = doc.resolve(widget.as_dict().unwrap().get(b"AP").unwrap()); + let n = doc.resolve(ap.as_dict().unwrap().get(b"N").unwrap()); + let pdfcraft_cos::Object::Stream(s) = &*n else { panic!("{n:?}") }; + String::from_utf8_lossy(&s.decoded().unwrap()).into_owned() +} + +/// #5939 review: the first signature removes `/NeedAppearances`, so the fields are drawn from +/// their values first; without that, `name` and `town` would show their empty placeholders. A +/// field whose own appearance draws its value keeps it rather than get a `?` for `ř`. +#[test] +fn the_first_signature_draws_the_fields_need_appearances_left_to_the_viewer() { + 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("form.pdf", None, Arc::new(need_appearances_form()), None).unwrap(); + let signed = s.sign(id, &digital_id, SignOptions { rect: None, ..SignOptions::default() }).unwrap(); + let doc = pdfcraft_cos::Document::open(signed.clone()).unwrap(); + assert!(!pdfcraft_forms::need_appearances(&doc)); + assert!(normal_appearance(&doc, 4).contains("(Ada Lovelace) Tj"), "{}", normal_appearance(&doc, 4)); + assert!(normal_appearance(&doc, 5).contains("(Plze?) Tj"), "{}", normal_appearance(&doc, 5)); + assert!(normal_appearance(&doc, 6).contains("(Brno r) Tj"), "{}", normal_appearance(&doc, 6)); + let city = |d: &pdfcraft_cos::Document| d.get(pdfcraft_cos::ObjRef::new(6, 0)).as_dict().unwrap().get(b"AP").cloned(); + let original = pdfcraft_cos::Document::open(Arc::new(need_appearances_form())).unwrap(); + assert_eq!(city(&doc), city(&original), "city keeps its own appearance"); + s.mark_signed(id, signed, None).unwrap(); + s.set_trusted_certificates(vec![digital_id.certificate.clone()]); + let sig = &s.get(id).unwrap().signatures[0]; + assert_eq!((sig.signed, sig.status, sig.modification.clone()), (true, SignatureStatus::Valid, sign::Modification::None)); +} + +/// A later signature draws nothing and leaves the flag: either would be a change to what the +/// first signature covers. +#[test] +fn a_later_signature_leaves_need_appearances_forms_alone() { + 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(); + let mut s = Session::new().with_clock(|| 1_800_000_000); + let id = s.open("form.pdf", None, Arc::new(need_appearances_form()), None).unwrap(); + let first = s.sign(id, &digital_id, SignOptions { rect: None, ..SignOptions::default() }).unwrap(); + // The flag set again after the first signature, and `name` back to its empty placeholder. + let mut cos = pdfcraft_cos::Document::open(first).unwrap(); + let root = cos.root().unwrap(); + cos.update_dict(root, |c| { + if let Some(pdfcraft_cos::Object::Dict(af)) = c.get_mut(b"AcroForm") { + af.set(b"NeedAppearances".to_vec(), pdfcraft_cos::Object::Bool(true)); + } + }) + .unwrap(); + let placeholder = cos.add(pdfcraft_cos::Object::Stream(pdfcraft_cos::Stream::from_raw(Default::default(), b"/Tx BMC EMC".to_vec()))); + cos.update_dict(pdfcraft_cos::ObjRef::new(4, 0), |w| { + let mut ap = pdfcraft_cos::Dict::new(); + ap.set(b"N".to_vec(), pdfcraft_cos::Object::Ref(placeholder)); + w.set(b"AP".to_vec(), pdfcraft_cos::Object::Dict(ap)); + }) + .unwrap(); + let flagged = + Arc::new(pdfcraft_cos::write_incremental(&cos, &pdfcraft_cos::SaveOptions { object_streams: false, ..Default::default() }).unwrap()); + let id = s.open("flagged.pdf", None, flagged.clone(), None).unwrap(); + let second = s.sign(id, &digital_id, SignOptions { rect: None, ..SignOptions::default() }).unwrap(); + let (before, after) = (pdfcraft_cos::Document::open(flagged).unwrap(), pdfcraft_cos::Document::open(second.clone()).unwrap()); + assert!(pdfcraft_forms::need_appearances(&after)); + let widget = |d: &pdfcraft_cos::Document| d.get(pdfcraft_cos::ObjRef::new(4, 0)); + assert_eq!(widget(&after), widget(&before)); + assert_eq!(normal_appearance(&after, 4), "/Tx BMC EMC"); + s.mark_signed(id, second, None).unwrap(); + assert_eq!(s.get(id).unwrap().signatures.iter().filter(|x| x.signed).count(), 2); +} + /// #3944: a value typed into an XFA `` field stays in the open document (both the /// widget and the live XFA datasets, even across multiple edits and across Save + `mark_saved`), /// but is written to no file — neither on the widget nor in the XFA datasets stream. diff --git a/crates/sign/src/lib.rs b/crates/sign/src/lib.rs index 2954de288d..a0aeadfdee 100644 --- a/crates/sign/src/lib.rs +++ b/crates/sign/src/lib.rs @@ -29,7 +29,7 @@ pub use graphic::Graphic; pub use keys::{DigestAlg, PrivateKey, PublicKey}; pub use pdf::{ Appearance, AppearanceLabels, Detail, DigestCache, Fill, Modification, SignOptions, SignatureInfo, Status, TrustStore, WORDINGS, Words, - in_english, kind, list as signatures, sign, sign_with_timestamp, timestamp_document, validate, + has_signatures, in_english, kind, list as signatures, sign, sign_with_timestamp, timestamp_document, validate, }; pub use pkcs12::DigitalId; pub use timestamp::{TimestampAuthority, TimestampQuery, TimestampToken}; diff --git a/crates/sign/src/pdf.rs b/crates/sign/src/pdf.rs index 8fafff7f01..c9d224fea4 100644 --- a/crates/sign/src/pdf.rs +++ b/crates/sign/src/pdf.rs @@ -35,7 +35,7 @@ use changes::only_grew; use listing::{DISCOVERY_CACHE_LIMIT, is_document_timestamp}; pub use listing::{DigestCache, list, list_cached}; pub(crate) use signing::fmt; -pub use signing::{Appearance, AppearanceLabels, SignOptions, display_date, sign, sign_with_timestamp, timestamp_document}; +pub use signing::{Appearance, AppearanceLabels, SignOptions, display_date, has_signatures, sign, sign_with_timestamp, timestamp_document}; pub use types::{Detail, Fill, Modification, Status, TrustStore, WORDINGS, Words, in_english, kind}; pub(crate) use validate::signature_contents; pub use validate::validate; diff --git a/crates/sign/src/pdf/signing.rs b/crates/sign/src/pdf/signing.rs index e5cc6a97f8..efb5960ee5 100644 --- a/crates/sign/src/pdf/signing.rs +++ b/crates/sign/src/pdf/signing.rs @@ -119,12 +119,28 @@ fn check_signing_permissions(doc: &Document, opts: &SignOptions) -> Result<(), S /// Placeholder `/ByteRange` values: fixed width, patched after writing. const BR_MARK: [i64; 3] = [1_111_111_111, 2_222_222_222, 3_333_333_333]; +/// Whether `doc` already holds a signature or a document timestamp: a signature dictionary whose +/// `/Contents` is not empty (a field whose `/V` has an empty `/Contents` is not signed yet). A +/// signature field without a name (`/T`) counts too: indirect signature dictionaries are found by +/// their `/ByteRange`, not through the field tree. +pub fn has_signatures(doc: &Document) -> bool { + let signed = |d: &Dict| { + let contents = d.get(b"Contents").map(|c| doc.resolve(c)); + contents.as_deref().and_then(Object::as_string).is_some_and(|s| !s.bytes.is_empty()) + }; + super::validate::signature_contents(doc).iter().any(|der| !der.is_empty()) + || doc.scan_objects().any(|(_, o)| o.as_dict().is_some_and(|d| d.contains(b"ByteRange") && signed(d))) +} + /// Sign `doc` with `id`; returns the signed file (an incremental update when possible). /// -/// The document's first signature removes the AcroForm's `/NeedAppearances` flag: Acrobat does -/// not recognise a signature while it is set. Later signatures leave the flag as it is. -/// A field that has a value but no appearance stream of its own then shows empty in viewers that -/// only drew it because of the flag. +/// The document's first signature (no [`has_signatures`]) removes the AcroForm's +/// `/NeedAppearances` flag: Acrobat does not recognise a signature while it is set. Later +/// signatures leave the flag as it is. This function draws no field appearances: once the flag is +/// gone, viewers show each field's stored normal appearance (`/AP /N`), so a filled field whose +/// stored appearance draws no text (an empty placeholder such as `/Tx BMC EMC`, or a look left +/// from an earlier value) shows that instead of its value. A caller that wants the values kept +/// draws the fields first, as `Session::sign` in the engine does. pub fn sign(doc: &Document, id: &DigitalId, opts: &SignOptions) -> Result, SignError> { sign_inner(doc, id, opts, None) } @@ -159,8 +175,7 @@ fn sign_inner( } check_signing_permissions(doc, opts)?; let mut doc = doc.clone(); - // A signature field whose /V holds an empty /Contents is not a signature yet. - let earlier_signatures = super::validate::signature_contents(&doc).iter().any(|der| !der.is_empty()); + let earlier_signatures = has_signatures(&doc); let root = doc.root().ok_or_else(|| SignError::Pdf("the document has no catalog".into()))?; let alg = id.key.preferred_digest(); let reserve = @@ -304,8 +319,9 @@ fn sign_inner( // Acrobat does not recognise a signature in a form that asks viewers to redraw its fields // (`/NeedAppearances true`): redrawing would change what the signature covers. The first // signature drops the flag; a later one leaves it, as removing it would count as a change - // to the earlier signatures. The cost: a field with a value but no appearance stream of its - // own, which viewers only drew because of the flag, now shows empty in them. + // to the earlier signatures. Without the flag viewers draw each field's stored `/AP /N`, so + // a filled field whose stored appearance draws no text (an empty placeholder, or a stale + // look) shows that instead of its value unless the caller drew the fields first (see `sign`). if !earlier_signatures { if let Some(Object::Ref(r)) = doc.get(root).as_dict().and_then(|c| c.get(b"AcroForm").cloned()) { doc.update_dict(r, |af| { diff --git a/crates/sign/tests/it/pdf.rs b/crates/sign/tests/it/pdf.rs index 0213b53441..b327c104ee 100644 --- a/crates/sign/tests/it/pdf.rs +++ b/crates/sign/tests/it/pdf.rs @@ -162,10 +162,28 @@ fn an_empty_signature_value_does_not_count_as_an_earlier_signature() { b"<< /Type /Sig /Filter /Adobe.PPKLite /SubFilter /ETSI.CAdES.detached /Contents <> >>", ], ); + assert!(!pdfcraft_sign::has_signatures(&open(&bytes))); let signed = pdfcraft_sign::sign(&open(&bytes), &id, &SignOptions { field: Some("Approval".into()), ..opts() }).unwrap(); assert!(form_of(&signed).get(b"NeedAppearances").is_none()); } +/// A signed field without a name (`/T` is optional) is an earlier signature too: signing another +/// field leaves `/NeedAppearances` as it is. +#[test] +fn an_unnamed_signed_field_counts_as_an_earlier_signature() { + let id = pkcs12::open(&data("ec-p256.p12"), "test").unwrap(); + let bytes = fixture_with( + b"<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [6 0 R 7 0 R] /NeedAppearances true >> >>", + &[ + b"<< /Type /Annot /Subtype /Widget /FT /Sig /Rect [10 20 100 70] /P 3 0 R /F 4 /V 8 0 R >>", + b"<< /Type /Sig /Filter /Adobe.PPKLite /SubFilter /ETSI.CAdES.detached /ByteRange [0 10 20 10] /Contents <3080> >>", + ], + ); + assert!(pdfcraft_sign::has_signatures(&open(&bytes))); + let signed = pdfcraft_sign::sign(&open(&bytes), &id, &SignOptions { field: Some("Approval".into()), ..opts() }).unwrap(); + assert_eq!(form_of(&signed).get(b"NeedAppearances"), Some(&Object::Bool(true))); +} + /// A second signer leaves the form's flags alone: changing them would count as a change to /// what the first signature covered. #[test]