diff --git a/crates/a11y/src/content.rs b/crates/a11y/src/content.rs index f72c40416e..38d05220d9 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,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 diff --git a/crates/a11y/src/tests.rs b/crates/a11y/src/tests.rs index fcd681efc9..2298b7fe93 100644 --- a/crates/a11y/src/tests.rs +++ b/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 { 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) {