Merge PR #3686: fix(a11y): resolve /MarkInfo /Marked when it is an indirect reference (#3671)

crates/a11y/src/content.rs

@@ -143,6 +143,28 @@ fn catalog(doc: &Document) -> Dict {
     doc.root().and_then(|r| doc.get(r).as_dict().cloned()).unwrap_or_default()
 }
 
+/// The most references one value may be written through. A chain is unusual but legal, and the
+/// render inspector follows them (lopdf's `dereference` has a limit of its own), so the two
+/// readers of a flag must agree about one.
+const MAX_REFERENCE_CHAIN: usize = 32;
+
+/// The value a chain of indirect references ends at. [`Document::resolve`] follows one reference,
+/// and an indirect object's value may itself be a reference (ISO 32000-2 §7.3.10): `/Marked
+/// 8 0 R` with object 8 holding `9 0 R` holding `true` is a tagged document, which this rule read
+/// as untagged while `doc_info` read it as tagged. Bounded and cycle-safe, since a damaged file
+/// can point a reference at itself; a chain longer than that, or one that loops, is no value.
+fn resolved(doc: &Document, o: &Object) -> std::sync::Arc<Object> {
+    let mut seen: HashSet<ObjRef> = HashSet::new();
+    let mut value = doc.resolve(o);
+    while let Object::Ref(r) = &*value {
+        if seen.len() >= MAX_REFERENCE_CHAIN || !seen.insert(*r) {
+            return std::sync::Arc::new(Object::Null);
+        }
+        value = doc.get(*r);
+    }
+    value
+}
+
 fn text_of(doc: &Document, d: &Dict, key: &[u8]) -> Option<String> {
     d.get(key).map(|o| doc.resolve(o)).and_then(|o| o.as_string().map(|s| s.to_text())).map(|s| s.trim().to_owned()).filter(|s| !s.is_empty())
 }
@@ -167,12 +189,16 @@ pub(crate) fn document_rule(doc: &Document, rule: Rule, pages: &[Page], scan: &S
             }
         }
         Rule::TaggedPdf => {
-            let marked = cat.get(b"MarkInfo").map(|m| doc.resolve(m)).and_then(|m| m.as_dict().and_then(|d| d.get(b"Marked").cloned()));
+            // `/Marked` may be an indirect reference like any other value -- or a chain of
+            // them -- so it is followed to the value before the test below, which looks for the
+            // boolean itself (#3671).
+            let marked =
+                cat.get(b"MarkInfo").map(|m| resolved(doc, m)).and_then(|m| m.as_dict().and_then(|d| d.get(b"Marked").map(|v| resolved(doc, v))));
             let mut out = Vec::new();
             if !cat.contains(b"StructTreeRoot") {
                 out.push(Finding::plain(None, "The document has no tags (structure tree)"));
             }
-            if !matches!(marked, Some(Object::Bool(true))) {
+            if !matches!(marked.as_deref(), Some(Object::Bool(true))) {
                 out.push(Finding::plain(None, "The document isn't marked as tagged (MarkInfo Marked)"));
             }
             out

crates/a11y/src/tests.rs

@@ -192,6 +192,44 @@ fn an_untagged_document_fails_the_document_rules() {
     assert_eq!((only.count(Status::Failed), only.count(Status::Skipped)), (1, 31));
 }
 
+/// `/MarkInfo /Marked` may be written as an indirect reference, like any value. The rule read the
+/// reference itself and so reported a tagged document as untagged (#3671).
+#[test]
+fn marked_as_an_indirect_reference_still_counts_as_tagged() {
+    let mut bodies = good();
+    // The catalog is object 1; point /Marked at a new object holding `true`.
+    let last = bodies.len() + 1;
+    bodies[0] = bodies[0].replace("/MarkInfo << /Marked true >>", &format!("/MarkInfo << /Marked {last} 0 R >>"));
+    bodies.push("true".into());
+    let doc = pdf(&bodies, "");
+    let report = check(&doc, &Options { rules: [Rule::TaggedPdf].into(), pages: None });
+    assert_eq!(status(&report, Rule::TaggedPdf), Status::Passed, "{:?}", failed(&report));
+
+    // A reference to a reference is followed too: the render inspector follows chains, so the
+    // two readers of the flag would otherwise disagree about one file.
+    let mut chain = good();
+    let first = chain.len() + 1;
+    chain[0] = chain[0].replace("/MarkInfo << /Marked true >>", &format!("/MarkInfo << /Marked {first} 0 R >>"));
+    chain.push(format!("{} 0 R", first + 1));
+    chain.push("true".into());
+    let report = check(&pdf(&chain, ""), &Options { rules: [Rule::TaggedPdf].into(), pages: None });
+    assert_eq!(status(&report, Rule::TaggedPdf), Status::Passed, "{:?}", failed(&report));
+
+    // A reference that points at itself is no value, not a hang.
+    let mut loops = good();
+    let at = loops.len() + 1;
+    loops[0] = loops[0].replace("/MarkInfo << /Marked true >>", &format!("/MarkInfo << /Marked {at} 0 R >>"));
+    loops.push(format!("{at} 0 R"));
+    let report = check(&pdf(&loops, ""), &Options { rules: [Rule::TaggedPdf].into(), pages: None });
+    assert_eq!(status(&report, Rule::TaggedPdf), Status::Failed);
+
+    // The direct form is unchanged, and a document with no flag at all still fails.
+    assert_eq!(status(&check(&pdf(&good(), ""), &Options { rules: [Rule::TaggedPdf].into(), pages: None }), Rule::TaggedPdf), Status::Passed);
+    let mut none = good();
+    none[0] = none[0].replace("/MarkInfo << /Marked true >>", "");
+    assert_eq!(status(&check(&pdf(&none, ""), &Options { rules: [Rule::TaggedPdf].into(), pages: None }), Rule::TaggedPdf), Status::Failed);
+}
+
 /// A tagged document that passes every automatic rule.
 fn good() -> Vec<String> {
     vec![

crates/render/src/inspect.rs

@@ -643,6 +643,9 @@ impl<'a> Inspector<'a> {
             .ok()
             .and_then(|o| self.dict(o))
             .and_then(|m| m.get(b"Marked").ok())
+            // Any value may be written as an indirect reference, `/Marked 9 0 R` included, so the
+            // flag is resolved before it is read as a boolean (#3671).
+            .map(|o| self.resolve(o))
             .and_then(|o| o.as_bool().ok())
             .unwrap_or(false);
         match self.page_labels(catalog, &mut info.pages) {