From bfc748738424d2a9c730a8ddae0bd5535af798de Mon Sep 17 00:00:00 2001 From: Hamza Date: Tue, 20 Oct 2026 20:08:41 +0500 Subject: [PATCH 1/2] Tagged PDF: resolve /MarkInfo /Marked when it is an indirect reference Any value in a PDF may be written as an indirect reference, the tagged flag included: `/MarkInfo << /Marked 9 0 R >>` with object 9 holding `true` is valid, and veraPDF's corpus has pass files that write it that way. Both readers of the flag tested the reference itself, so such a document was reported as untagged: doc_info "tagged": false Accessibility Checker "The document isn't marked as tagged (MarkInfo Marked)" Both now resolve the value first. On a pair of otherwise identical tagged pages, one with `/Marked true` and one with `/Marked 8 0 R`: before direct: tagged true indirect: tagged false after direct: tagged true indirect: tagged true and the checker reports the finding for neither. A document with no flag at all still fails the rule, which the test asserts alongside, so this does not turn the rule into one that always passes. Fixes #3671. --- crates/a11y/src/content.rs | 7 +++++-- crates/a11y/src/tests.rs | 20 ++++++++++++++++++++ crates/render/src/inspect.rs | 3 +++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/a11y/src/content.rs b/crates/a11y/src/content.rs index e8a7db377f..d4673f0fe1 100644 --- a/crates/a11y/src/content.rs +++ b/crates/a11y/src/content.rs @@ -167,12 +167,15 @@ 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, so it is resolved + // before the test below, which looks for the boolean itself (#3671). + let marked = + cat.get(b"MarkInfo").map(|m| doc.resolve(m)).and_then(|m| m.as_dict().and_then(|d| d.get(b"Marked").map(|v| doc.resolve(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 diff --git a/crates/a11y/src/tests.rs b/crates/a11y/src/tests.rs index fcd681efc9..cabbf1bcf8 100644 --- a/crates/a11y/src/tests.rs +++ b/crates/a11y/src/tests.rs @@ -192,6 +192,26 @@ 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)); + + // 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 { vec![ diff --git a/crates/render/src/inspect.rs b/crates/render/src/inspect.rs index 0979f3d38a..f7752c4a53 100644 --- a/crates/render/src/inspect.rs +++ b/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) { From 0f5e74101c3799be31ac6d126a62b50205f2cc29 Mon Sep 17 00:00:00 2001 From: Hamza Date: Tue, 20 Oct 2026 21:52:36 +0500 Subject: [PATCH 2/2] Follow a chain of references to the tagged flag, as the inspector does 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. The rule was left holding a reference and reported it untagged, while the render inspector -- which follows chains through lopdf's dereference -- read the same file as tagged. The point of this branch is that the two readers agree, so the chain is followed here too. Bounded at 32 references and cycle-safe: a damaged file can point a reference at itself, and the test covers that case as well as the two-hop chain. Without the fix the chain case reports left: Failed, right: Passed Reported by CodeRabbit on #3686. --- crates/a11y/src/content.rs | 29 ++++++++++++++++++++++++++--- crates/a11y/src/tests.rs | 18 ++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/crates/a11y/src/content.rs b/crates/a11y/src/content.rs index d4673f0fe1..0354d3f084 100644 --- a/crates/a11y/src/content.rs +++ b/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 { + let mut seen: HashSet = 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 { 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,10 +189,11 @@ pub(crate) fn document_rule(doc: &Document, rule: Rule, pages: &[Page], scan: &S } } Rule::TaggedPdf => { - // `/Marked` may be an indirect reference like any other value, so it is resolved - // before the test below, which looks for the boolean itself (#3671). + // `/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| doc.resolve(m)).and_then(|m| m.as_dict().and_then(|d| d.get(b"Marked").map(|v| doc.resolve(v)))); + 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)")); diff --git a/crates/a11y/src/tests.rs b/crates/a11y/src/tests.rs index cabbf1bcf8..2298b7fe93 100644 --- a/crates/a11y/src/tests.rs +++ b/crates/a11y/src/tests.rs @@ -205,6 +205,24 @@ fn marked_as_an_indirect_reference_still_counts_as_tagged() { 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();