diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 08c9686a0..d892f9296 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -1297,11 +1297,44 @@ pub(crate) fn ask_viewers_to_redraw(doc: &mut Document) -> Result<(), FormError> /// their own) is not empty and not their default value (`/DV`, which the form's author wrote and /// a save restores), or one of whose separate widgets carries a `/V`. fn password_fields(doc: &Document) -> Vec { + if !has_password_flag(doc) { + return Vec::new(); + } 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())); let typed = |f: &Field| f.value.iter().any(|v| !v.is_empty()) && f.value.first() != f.default.first(); fields(doc).into_iter().filter(|f| f.kind == FieldKind::Text && f.has(flags::PASSWORD) && (typed(f) || on_widget(f))).collect() } +/// Whether any field dictionary sets the Password flag (#3943 review). A field that inherits the +/// flag has an ancestor that sets it, so that ancestor is found instead; a kid clearing the flag +/// again only makes the answer cautious. It reads only `/Ff`, `/Kids` and `/Parent`, so a save +/// or recovery snapshot of a form without such a field doesn't build every [`Field`]. Like +/// [`fields`], it looks at the form's field tree and at the widgets on the pages, which may be +/// missing from `/Fields`. Each dictionary is visited once and at most 32 levels deep. +fn has_password_flag(doc: &Document) -> bool { + let sets_it = |r: ObjRef| doc.get(r).as_dict().and_then(|d| field_flags(doc, d)).is_some_and(|ff| ff & flags::PASSWORD != 0); + let top = acroform(doc).and_then(|af| af.get(b"Fields").map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned())).unwrap_or_default(); + let mut stack: Vec<(Object, usize)> = top.into_iter().map(|o| (o, 0)).collect(); + let mut seen = std::collections::HashSet::new(); + while let Some((o, depth)) = stack.pop() { + let Object::Ref(r) = o else { continue }; + if depth > 32 || !seen.insert(r) { + continue; + } + if sets_it(r) { + return true; + } + let kids = doc.get(r).as_dict().and_then(|d| d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned())).unwrap_or_default(); + stack.extend(kids.into_iter().map(|k| (k, depth + 1))); + } + // Widgets on the pages, and the fields above them. + page_refs(doc).into_iter().any(|p| { + let annots = + doc.get(p).as_dict().and_then(|d| d.get(b"Annots").map(|a| doc.resolve(a)).and_then(|a| a.as_array().cloned())).unwrap_or_default(); + annots.iter().filter_map(Object::as_ref).filter(|r| !seen.contains(r)).any(|r| field_chain(doc, r).into_iter().any(sets_it)) + }) +} + /// 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() @@ -1340,8 +1373,10 @@ pub fn clear_password_values(doc: &mut Document) -> Result { if targets.is_empty() { return Ok(0); } - let all = fields(doc); let is_password = |f: &Field| f.kind == FieldKind::Text && f.has(flags::PASSWORD); + // Each field's ancestors, found once: only values change below, never `/Parent` (#3943 + // review: walking them again for every ancestor of every target was quadratic). + let chains: Vec<(bool, Vec)> = fields(doc).iter().map(|g| (is_password(g), field_chain(doc, g.obj))).collect(); for f in &targets { let chain = field_chain(doc, f.obj); for w in &f.widgets { @@ -1359,7 +1394,7 @@ pub fn clear_password_values(doc: &mut Document) -> Result { 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); + let exclusive = chains.iter().filter(|(_, chain)| chain.contains(a)).all(|(password, _)| *password); if exclusive { doc.update_dict(*a, |d| { d.remove(b"V"); diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 29593ddbd..e152b0868 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -3139,3 +3139,44 @@ fn a_cleared_password_field_returns_to_its_default_value() { assert!(!has_password_values(&doc), "a field at its default holds no typed password"); assert_eq!(clear_password_values(&mut doc).unwrap(), 0); } + +/// #3943 review: the quick check that skips forms without a Password field still sees one whose +/// widget is only on the page, not listed in `/Fields`; one that inherits the flag from a parent +/// that is not listed either; and one deeper in the field tree whose widget is on no page. Each +/// is alone in its own document, so each path of the check is needed. +#[test] +fn a_password_field_missing_from_the_field_list_is_still_cleared() { + let form = |fields: &str, annots: &str, rest: &[&str]| { + let mut objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + format!("<< /Type /Page /Parent 2 0 R /Annots [{annots}] >>"), + format!("<< /Fields [{fields}] /DA (/Helv 0 Tf 0 g) >>"), + ]; + objs.extend(rest.iter().map(|o| o.to_string())); + document(&objs) + }; + let page_only = + form("", "5 0 R", &["<< /Type /Annot /Subtype /Widget /FT /Tx /T (pin) /Ff 8192 /V (secret5) /Rect [50 700 150 720] /P 3 0 R >>"]); + let inherited = form( + "", + "5 0 R", + &[ + "<< /Type /Annot /Subtype /Widget /T (kid) /Parent 6 0 R /Rect [50 650 150 670] /P 3 0 R >>", + "<< /FT /Tx /T (acct) /Ff 8192 /V (secret6) /Kids [5 0 R] >>", + ], + ); + let off_page = form( + "5 0 R", + "", + &[ + "<< /T (grp) /Kids [6 0 R] >>", + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (hidden) /Ff 8192 /V (secret7) /Parent 5 0 R /Rect [0 0 0 0] >>", + ], + ); + for (what, mut doc) in [("page only", page_only), ("inherited", inherited), ("on no page", off_page)] { + assert!(has_password_values(&doc), "{what}"); + assert_eq!(clear_password_values(&mut doc).unwrap(), 1, "{what}"); + assert!(fields(&doc).iter().all(|f| f.value.iter().all(|v| v.is_empty())), "{what}: {:?}", fields(&doc)); + } +}