From 509b5d5cf350682d1b4476dc79d9c75d94928db3 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Thu, 22 Oct 2026 06:54:56 +0800 Subject: [PATCH] M6: clear Password values inherited from an ancestor or held by a widget (#3926) Review follow-up: the helpers checked only the field dictionary's own /V. A value inherited through /Parent or carried by a separate widget was missed, so the document could be written as it was. Detection now uses the resolved value and the widgets; clearing removes /V from the field, its widgets and every ancestor only password fields inherit from, and gives the field an empty value of its own where a shared ancestor value has to stay. Co-Authored-By: Claude Opus 5.5 --- crates/forms/src/lib.rs | 88 ++++++++++++++++++++++++++++++++------- crates/forms/src/tests.rs | 34 +++++++++++++++ 2 files changed, 106 insertions(+), 16 deletions(-) diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index fe04f0fba4..e5ce6a00ea 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -1293,28 +1293,84 @@ pub(crate) fn ask_viewers_to_redraw(doc: &mut Document) -> Result<(), FormError> Ok(()) } -/// Whether any text field with the Password flag holds a value (`/V`) in `doc`. -pub fn has_password_values(doc: &Document) -> bool { - fields(doc).iter().any(|f| f.kind == FieldKind::Text && f.has(flags::PASSWORD) && doc.get(f.obj).as_dict().is_some_and(|d| d.get(b"V").is_some())) +/// Text fields with the Password flag (inherited or their own) whose value (`/V`, inherited or +/// their own) is not empty, or one of whose separate widgets carries a `/V`. +fn password_fields(doc: &Document) -> Vec { + let on_widget = |f: &Field| f.widgets.iter().any(|w| w.obj != f.obj && doc.get(w.obj).as_dict().is_some_and(|d| d.get(b"V").is_some())); + fields(doc) + .into_iter() + .filter(|f| f.kind == FieldKind::Text && f.has(flags::PASSWORD) && (f.value.iter().any(|v| !v.is_empty()) || on_widget(f))) + .collect() } -/// Remove the value (`/V`) of every text field with the Password flag, for a copy of the -/// document about to be written to a file (#3926). ISO 32000-2 §12.7.5.3, Table 231, Password -/// flag, note: "it is imperative that PDF processors never store the value of the text field in -/// the PDF file if this flag is set". The appearance keeps its masking characters. Returns how -/// many values were removed. +/// Whether any text field with the Password flag holds a value in `doc`. +pub fn has_password_values(doc: &Document) -> bool { + !password_fields(doc).is_empty() +} + +/// `r` and its field ancestors (`/Parent`), nearest first, at most 32 deep and without repeats. +fn field_chain(doc: &Document, r: ObjRef) -> Vec { + let mut chain = vec![r]; + let mut cur = r; + while chain.len() < 32 { + let Some(Object::Ref(parent)) = doc.get(cur).as_dict().and_then(|d| d.get(b"Parent").cloned()) else { break }; + if chain.contains(&parent) { + break; + } + chain.push(parent); + cur = parent; + } + chain +} + +/// Remove the values of the text fields with the Password flag, for a copy of the document about +/// to be written to a file (#3926). ISO 32000-2 §12.7.5.3, Table 231, Password flag, note: "it is +/// imperative that PDF processors never store the value of the text field in the PDF file if +/// this flag is set". The appearance keeps its masking characters. +/// +/// A value can sit on the field, on one of its widgets, or on an ancestor it inherits from +/// (`/V` is inheritable). Each is removed; an ancestor's only when every field inheriting from it +/// is such a password field. Where an ancestor's value is shared with other fields and stays, the +/// password field gets an empty value of its own so it no longer inherits it. Returns how many +/// fields were cleared. pub fn clear_password_values(doc: &mut Document) -> Result { - let holders: Vec = fields(doc) - .into_iter() - .filter(|f| f.kind == FieldKind::Text && f.has(flags::PASSWORD) && doc.get(f.obj).as_dict().is_some_and(|d| d.get(b"V").is_some())) - .map(|f| f.obj) - .collect(); - for r in &holders { - doc.update_dict(*r, |d| { + let targets = password_fields(doc); + if targets.is_empty() { + return Ok(0); + } + let all = fields(doc); + let is_password = |f: &Field| f.kind == FieldKind::Text && f.has(flags::PASSWORD); + for f in &targets { + let chain = field_chain(doc, f.obj); + for w in &f.widgets { + if w.obj != f.obj { + doc.update_dict(w.obj, |d| { + d.remove(b"V"); + })?; + } + } + doc.update_dict(f.obj, |d| { d.remove(b"V"); })?; + let mut shared_value_kept = false; + for a in chain.iter().skip(1) { + if doc.get(*a).as_dict().is_none_or(|d| d.get(b"V").is_none()) { + continue; + } + let exclusive = all.iter().filter(|g| field_chain(doc, g.obj).contains(a)).all(is_password); + if exclusive { + doc.update_dict(*a, |d| { + d.remove(b"V"); + })?; + } else { + shared_value_kept = true; + } + } + if shared_value_kept { + doc.update_dict(f.obj, |d| d.set(b"V".to_vec(), PdfString::text("")))?; + } } - Ok(holders.len()) + Ok(targets.len()) } /// Whether the form asks viewers to draw its fields' appearances themselves (`/NeedAppearances`). diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 05b14d9338..d72c0da0e1 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -3073,3 +3073,37 @@ fn a_skipped_field_leaves_its_widget_to_the_next() { let widgets: Vec<(&str, usize)> = all.iter().map(|f| (f.name.as_str(), f.widgets.len())).collect(); assert_eq!(widgets, [("b", 1)]); } + +/// #3926 / #3943 review: a password value can be inherited from an ancestor field or sit on a +/// separate widget. Clearing removes both; an ancestor's value shared with a non-password field +/// stays for that field, and the password field gets an empty value of its own instead. +#[test] +fn inherited_and_widget_password_values_are_cleared() { + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), // 1 + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), // 2 + "<< /Type /Page /Parent 2 0 R /Annots [6 0 R 7 0 R 9 0 R 11 0 R] >>".into(), // 3 + "<< /Fields [5 0 R 8 0 R 10 0 R] /DA (/Helv 0 Tf 0 g) >>".into(), // 4 + "<< /T (acct) /FT /Tx /Ff 8192 /V (secret1) /Kids [6 0 R] >>".into(), // 5 flag and value inherited + "<< /Type /Annot /Subtype /Widget /T (pin) /Parent 5 0 R /Rect [50 700 150 720] /P 3 0 R >>".into(), // 6 + "<< /Type /Annot /Subtype /Widget /Parent 8 0 R /V (secret3) /Rect [50 650 150 670] /P 3 0 R >>".into(), // 7 value on the widget + "<< /FT /Tx /T (code) /Ff 8192 /Kids [7 0 R] >>".into(), // 8 + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (pw) /Ff 8192 /Parent 10 0 R /Rect [50 600 150 620] /P 3 0 R >>".into(), // 9 + "<< /T (grp) /V (shared2) /Kids [9 0 R 11 0 R] >>".into(), // 10 shared with a plain field + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (plain) /Parent 10 0 R /Rect [50 550 150 570] /P 3 0 R >>".into(), // 11 + ]; + let mut doc = document(&objs); + assert!(has_password_values(&doc)); + let value = |doc: &Document, n: &str| fields(doc).into_iter().find(|f| f.name == n).map(|f| f.value).unwrap_or_default(); + assert_eq!(value(&doc, "acct.pin"), ["secret1"], "inherited before"); + assert_eq!(clear_password_values(&mut doc).unwrap(), 3); + assert!(!has_password_values(&doc)); + for n in ["acct.pin", "code", "grp.pw"] { + assert!(value(&doc, n).iter().all(|v| v.is_empty()), "{n} cleared"); + } + assert_eq!(value(&doc, "grp.plain"), ["shared2"], "the shared value stays for the plain field"); + let has_v = |doc: &Document, n: u32| doc.get(pdfcraft_cos::ObjRef { num: n, generation: 0 }).as_dict().is_some_and(|d| d.get(b"V").is_some()); + assert!(!has_v(&doc, 5), "an ancestor only password fields inherit from loses its value"); + assert!(!has_v(&doc, 7), "a widget's value is removed"); + assert!(has_v(&doc, 10), "a shared ancestor keeps its value"); +}