M6: skip forms without a Password field quickly, and find ancestors once (#3926)

Review on #3943, point 1. has_password_values ran a full fields() walk
on every save and recovery snapshot, and clear_password_values walked
every field's ancestors again for each ancestor of each target.

has_password_flag now answers first, reading only /Ff, /Kids and /Parent:
the field tree from /Fields (seen set, 32 levels) and, as fields() does,
the widgets on the pages with the fields above them. A form without such
a field returns before any Field is built. clear_password_values finds
each field's ancestors once.

The test puts a page-only widget, a flag inherited from an unlisted
parent and a nested field whose widget is on no page each alone in a
document. Mutants that drop the page scan, the parent chain, the tree
walk or the descent into kids each fail it, as does treating every
ancestor as exclusive. Dropping the seen set or the early-out changes
only speed (equivalent).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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<Field> {
+    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<usize, FormError> {
     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<ObjRef>)> = 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<usize, FormError> {
             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");

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<String> = 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));
+    }
+}