fix(a11y): splice named and multi-stream marked content in mark_tag_artifact (#5263)

Share the stream-splicing path between mark_decorative and mark_tag_artifact so mark_tag_artifact resolves named property lists in /Resources /Properties, resolves indirect /MCR references in /K, leaves untouched and shared content streams intact, and validates the tag parent before mutating /Contents or /ParentTree.
This commit is contained in:
jonathanwxh-cell
2026-11-02 23:38:55 -05:00
committed by GitHub
parent 97260514e6
commit 49d5997e18
3 changed files with 162 additions and 100 deletions
+78 -92
View File
@@ -125,9 +125,9 @@ pub(crate) fn elements(doc: &Document) -> Vec<Element> {
}
/// The marked-content ids an element owns directly.
fn mcids(d: &Dict) -> Vec<i64> {
fn mcids(doc: &Document, d: &Dict) -> Vec<i64> {
let mut out = Vec::new();
let mut push = |o: &Object| match o {
let mut push = |o: &Object| match &*doc.resolve(o) {
Object::Int(n) => out.push(*n),
Object::Dict(m) if m.name(b"Type") == Some(b"MCR") => {
if let Some(n) = m.get(b"MCID").and_then(Object::as_int) {
@@ -323,7 +323,7 @@ pub fn figures(doc: &Document) -> Vec<Figure> {
let mut wanted: HashMap<usize, HashSet<i64>> = HashMap::new();
for (e, page) in &figures {
if let Some(p) = page {
wanted.entry(*p).or_default().extend(mcids(&e.dict));
wanted.entry(*p).or_default().extend(mcids(doc, &e.dict));
}
}
let bounds: HashMap<usize, HashMap<i64, [f64; 4]>> =
@@ -333,7 +333,7 @@ pub fn figures(doc: &Document) -> Vec<Figure> {
.map(|(e, page)| {
let alt = e.dict.get(b"Alt").map(|o| doc.resolve(o)).and_then(|o| o.as_string().map(|s| s.to_text()));
let on_page = page.and_then(|p| bounds.get(&p));
let bbox = on_page.and_then(|b| mcids(&e.dict).iter().filter_map(|id| b.get(id)).fold(None, |acc, b| Some(union(acc, *b))));
let bbox = on_page.and_then(|b| mcids(doc, &e.dict).iter().filter_map(|id| b.get(id)).fold(None, |acc, b| Some(union(acc, *b))));
Figure { obj: e.object, page, alt, bbox }
})
.collect()
@@ -363,52 +363,73 @@ pub fn set_alt(doc: &mut Document, obj: ObjRef, alt: Option<&str>) -> Result<(),
}
/// The page's content entries: one stream, or the elements of a contents array.
fn content_entries(doc: &Document, page: &Dict) -> Option<Vec<Object>> {
let c = page.get(b"Contents")?;
Some(match &*doc.resolve(c) {
fn content_entries(doc: &Document, page: &Dict) -> Vec<Object> {
let Some(c) = page.get(b"Contents") else { return Vec::new() };
match &*doc.resolve(c) {
Object::Array(a) => a.clone(),
Object::Stream(_) => vec![c.clone()],
_ => return None,
})
_ => Vec::new(),
}
}
fn marked(doc: &Document, props: &Dict, op: &pdfcraft_content::Op, ids: &HashSet<i64>) -> bool {
op.op == b"BDC" && bdc_mcid(doc, props, op).is_some_and(|n| ids.contains(&n))
}
type DecodedStream = (Object, Option<(Stream, Vec<u8>)>);
/// Turn the figure's marked-content blocks into artifacts, rewriting only the streams that
/// contain them. Other content streams, and unknown entries on a rewritten stream, stay.
fn artifact_marked_content(doc: &mut Document, page: &pdfcraft_model::Page, ids: &HashSet<i64>) -> Result<bool, AltError> {
let Some(entries) = content_entries(doc, &page.dict) else { return Ok(false) };
let props = page_properties(doc, &page.dict);
let mut streams: Vec<Option<(Stream, Vec<u8>)>> = Vec::with_capacity(entries.len());
/// Decode the page's content streams within the page decode budget. `None` when a stream is
/// rejected because the budget is exhausted.
fn decode_page_streams(doc: &Document, page: &Dict) -> Option<Vec<DecodedStream>> {
let entries = content_entries(doc, page);
let mut streams = Vec::with_capacity(entries.len());
let mut budget = DecodeBudget::page();
for entry in &entries {
match &*doc.resolve(entry) {
for entry in entries {
let decoded = match &*doc.resolve(&entry) {
Object::Stream(s) => {
let data = match budget.decode(s) {
Ok(data) => data,
// Content past the budget may hold more of the figure: editing the rest
// would leave that part marked as a figure the tree no longer has.
Err(_) if budget.exhausted() => return Err(AltError::TooLarge),
// Content past the budget may hold more of the tag: editing the rest
// would leave that part marked as a tag the tree no longer has.
Err(_) if budget.exhausted() => return None,
Err(_) => Vec::new(),
};
streams.push(Some((s.clone(), data)));
Some((s.clone(), data))
}
_ => streams.push(None),
_ => None,
};
streams.push((entry, decoded));
}
Some(streams)
}
fn marked_id(doc: &Document, props: &Dict, op: &pdfcraft_content::Op, ids: &HashSet<i64>) -> Option<i64> {
(op.op == b"BDC").then(|| bdc_mcid(doc, props, op)).flatten().filter(|n| ids.contains(n))
}
/// Turn the tag's marked-content blocks into artifacts, rewriting only the streams that
/// contain them. Other content streams, and unknown entries on a rewritten stream, stay.
/// With `require_all`, every id in `ids` must appear on the page before any stream is rewritten.
fn artifact_marked_content(
doc: &mut Document,
page: &pdfcraft_model::Page,
streams: Vec<DecodedStream>,
ids: &HashSet<i64>,
require_all: bool,
) -> Result<bool, pdfcraft_cos::CosError> {
let props = page_properties(doc, &page.dict);
let decoded: Vec<&[u8]> = streams.iter().filter_map(|(_, s)| s.as_ref().map(|(_, bytes)| bytes.as_slice())).collect();
let pieces = pdfcraft_content::Pieces::join(&decoded);
let mut found = HashSet::new();
let mut edits = Vec::new();
for op in pieces.parse().ops {
if let Some(id) = marked_id(doc, &props, &op, ids) {
found.insert(id);
edits.push((op.span, b"/Artifact BMC".to_vec()));
}
}
let decoded: Vec<&[u8]> = streams.iter().flatten().map(|(_, bytes)| bytes.as_slice()).collect();
let pieces = pdfcraft_content::Pieces::join(&decoded);
let edits: Vec<_> =
pieces.parse().ops.into_iter().filter(|op| marked(doc, &props, op, ids)).map(|op| (op.span, b"/Artifact BMC".to_vec())).collect();
if edits.is_empty() {
if if require_all { found != *ids } else { found.is_empty() } {
return Ok(false);
}
let mut spliced = pieces.splice(edits).into_iter();
let mut next = Vec::with_capacity(entries.len());
let mut changed = false;
for (entry, stream) in entries.into_iter().zip(streams) {
let mut next = Vec::with_capacity(streams.len());
for (entry, stream) in streams {
let Some((stream, old)) = stream else {
next.push(entry);
continue;
@@ -421,7 +442,6 @@ fn artifact_marked_content(doc: &mut Document, page: &pdfcraft_model::Page, ids:
next.push(entry);
continue;
}
changed = true;
let mut dict = stream.dict;
dict.remove(b"Length");
dict.remove(b"Filter");
@@ -431,28 +451,27 @@ fn artifact_marked_content(doc: &mut Document, page: &pdfcraft_model::Page, ids:
// appear elsewhere in this array or on another page, where it must stay unchanged.
next.push(Object::Ref(doc.add(stream)));
}
if changed {
let contents = match next.as_slice() {
[one] => one.clone(),
_ => Object::Array(next),
};
doc.update_dict(page.obj, |p| p.set(b"Contents".to_vec(), contents))?;
}
Ok(changed)
let contents = match next.as_slice() {
[one] => one.clone(),
_ => Object::Array(next),
};
doc.update_dict(page.obj, |p| p.set(b"Contents".to_vec(), contents))?;
Ok(true)
}
/// Decorative figure: its marked content on the page becomes an artifact, and the element
/// leaves the structure tree (and the parent tree).
pub fn mark_decorative(doc: &mut Document, obj: ObjRef) -> Result<(), AltError> {
let d = check_figure(doc, obj)?;
let ids: HashSet<i64> = mcids(&d).into_iter().collect();
let ids: HashSet<i64> = mcids(doc, &d).into_iter().collect();
let pages = pdfcraft_model::pages(doc);
let pg = d.get(b"Pg").and_then(Object::as_ref).or_else(|| elements(doc).into_iter().find(|e| e.object == obj).and_then(|e| e.page));
if let Some(page) = pg.and_then(|p| pages.iter().find(|x| x.obj == p)).filter(|_| !ids.is_empty())
&& artifact_marked_content(doc, page, &ids)?
{
if let Some(page) = pg.and_then(|p| pages.iter().find(|x| x.obj == p)).filter(|_| !ids.is_empty()) {
let streams = decode_page_streams(doc, &page.dict).ok_or(AltError::TooLarge)?;
// The parent tree no longer points those ids at the figure.
if let Some(key) = page.dict.get(b"StructParents").and_then(Object::as_int) {
if artifact_marked_content(doc, page, streams, &ids, false)?
&& let Some(key) = page.dict.get(b"StructParents").and_then(Object::as_int)
{
clear_parent_tree(doc, key, obj)?;
}
}
@@ -484,54 +503,23 @@ pub fn mark_tag_artifact(doc: &mut Document, obj: ObjRef) -> Result<(), Artifact
if has_child_structure(doc, &element.dict) {
return Err(ArtifactError::HasChildren);
}
let ids: HashSet<i64> = mcids(&element.dict).into_iter().collect();
let ids: HashSet<i64> = mcids(doc, &element.dict).into_iter().collect();
if ids.is_empty() {
return Err(ArtifactError::NoMarkedContent);
}
let pages = pdfcraft_model::pages(doc);
let page_ref = element.page.ok_or(ArtifactError::NoPage)?;
let page = pages.iter().find(|page| page.obj == page_ref).ok_or(ArtifactError::NoPage)?;
let mut content = Vec::new();
if let Some(c) = page.dict.get(b"Contents") {
let mut budget = DecodeBudget::page();
for data in budget.decode_contents(doc, c) {
match data {
Ok(data) => {
content.extend_from_slice(&data);
content.push(b' ');
}
// The page's /Contents is replaced with what was decoded: content past the
// budget would be dropped from the page.
Err(_) if budget.exhausted() => return Err(ArtifactError::TooLarge),
Err(_) => {}
}
}
}
let mut ops = pdfcraft_content::parse(&content).ops;
let mut found = HashSet::new();
for op in &mut ops {
let id = (op.op == b"BDC")
.then(|| match op.operands.get(1) {
Some(Object::Dict(mark)) => mark.get(b"MCID").and_then(Object::as_int),
_ => None,
})
.flatten()
.filter(|id| ids.contains(id));
if let Some(id) = id {
*op = pdfcraft_content::Op::new("BMC", vec![Object::name("Artifact")]);
found.insert(id);
}
}
if found != ids {
let parent = element.dict.get(b"P").and_then(Object::as_ref).ok_or(ArtifactError::NoParent)?;
let replacement = replacement_kids(doc, parent, obj)?;
let streams = decode_page_streams(doc, &page.dict).ok_or(ArtifactError::TooLarge)?;
if !artifact_marked_content(doc, page, streams, &ids, true)? {
return Err(ArtifactError::ContentNotFound);
}
let stream = doc.add(Object::Stream(Stream::flate(Dict::new(), &pdfcraft_content::serialize_ops(&ops))));
doc.update_dict(page.obj, |p| p.set(b"Contents".to_vec(), Object::Ref(stream)))?;
if let Some(key) = page.dict.get(b"StructParents").and_then(Object::as_int) {
clear_parent_tree(doc, key, obj).map_err(|e| ArtifactError::ParentTree(e.to_string()))?;
}
let parent = element.dict.get(b"P").and_then(Object::as_ref).ok_or(ArtifactError::NoParent)?;
remove_from_parent(doc, parent, obj)?;
doc.update_dict(parent, |p| p.set(b"K".to_vec(), replacement))?;
// Keep the retired object valid if another object holds an out-of-tree reference to it.
doc.update_dict(obj, |d| {
d.remove(b"P");
@@ -551,20 +539,18 @@ fn has_child_structure(doc: &Document, tag: &Dict) -> bool {
tag.get(b"K").is_some_and(|items| child(doc, items))
}
fn remove_from_parent(doc: &mut Document, parent: ObjRef, obj: ObjRef) -> Result<(), ArtifactError> {
fn replacement_kids(doc: &Document, parent: ObjRef, obj: ObjRef) -> Result<Object, ArtifactError> {
let kids = doc.get(parent).as_dict().and_then(|p| p.get(b"K").cloned()).ok_or(ArtifactError::NoParent)?;
let replacement = match kids {
match kids {
Object::Array(items) => {
if items.iter().filter(|item| item.as_ref() == Some(obj)).count() != 1 {
return Err(ArtifactError::AmbiguousParent);
}
Object::Array(items.into_iter().filter(|item| item.as_ref() != Some(obj)).collect())
Ok(Object::Array(items.into_iter().filter(|item| item.as_ref() != Some(obj)).collect()))
}
Object::Ref(r) if r == obj => Object::Array(Vec::new()),
_ => return Err(ArtifactError::NoParent),
};
doc.update_dict(parent, |p| p.set(b"K".to_vec(), replacement))?;
Ok(())
Object::Ref(r) if r == obj => Ok(Object::Array(Vec::new())),
_ => Err(ArtifactError::NoParent),
}
}
/// Replace `obj` by null in the page's parent-tree array (`key`), in a flat /Nums tree.
+70
View File
@@ -1017,6 +1017,76 @@ fn named_property_lists_are_found_and_marked_decorative_like_inline_ones() {
assert!(props.contains(b"P0") && props.contains(b"P1") && props.contains(b"Other"), "resources are left as they were");
}
#[test]
fn mark_tag_artifact_finds_named_property_lists_and_indirect_mcrs() {
use pdfcraft_cos::{Dict, ObjRef, Object};
let mut doc = named_figures_doc();
let mcr = doc.add(Object::Dict([(b"Type".to_vec(), Object::name("MCR")), (b"MCID".to_vec(), Object::Int(0))].into_iter().collect::<Dict>()));
doc.update_dict(ObjRef::new(7, 0), |d| d.set(b"K".to_vec(), Object::Ref(mcr))).unwrap();
assert_eq!(figures(&doc)[0].bbox, Some([10.0, 150.0, 50.0, 170.0]), "indirect MCR is resolved for figure bounds");
for (tag, gone) in [(ObjRef::new(7, 0), "/P0"), (ObjRef::new(8, 0), "/P1")] {
mark_tag_artifact(&mut doc, tag).unwrap();
let pages = pdfcraft_model::pages(&doc);
let c = doc.resolve(pages[0].dict.get(b"Contents").unwrap());
let Object::Stream(c) = &*c else { panic!("one content stream") };
let text = String::from_utf8(c.decoded().unwrap()).unwrap();
assert!(text.contains("/Artifact BMC") && !text.contains(gone), "{text}");
}
assert_eq!(tags(&doc).into_iter().map(|t| t.object.num).collect::<Vec<_>>(), [6]);
let nums = doc.get(ObjRef::new(9, 0)).as_dict().and_then(|d| d.get(b"Nums").cloned());
assert_eq!(nums, Some(Object::Array(vec![Object::Int(0), Object::Array(vec![Object::Null, Object::Null])])));
let r = check(&doc, &Options { rules: [Rule::FiguresAltText, Rule::TaggedContent].into(), pages: None });
assert_eq!(failed(&r), Vec::<Rule>::new(), "{:?}", r.results);
}
#[test]
fn mark_tag_artifact_validates_parent_and_all_mcids_before_mutating_document() {
use pdfcraft_cos::{ObjRef, Object};
// Missing /P on the tag must fail before touching /Contents or /ParentTree.
let mut no_parent = figures_doc();
let before_contents = no_parent.get(ObjRef::new(3, 0)).as_dict().unwrap().get(b"Contents").cloned();
let before_pt = no_parent.get(ObjRef::new(9, 0)).as_ref().clone();
no_parent
.update_dict(ObjRef::new(7, 0), |d| {
d.remove(b"P");
})
.unwrap();
assert_eq!(mark_tag_artifact(&mut no_parent, ObjRef::new(7, 0)), Err(ArtifactError::NoParent));
assert_eq!(no_parent.get(ObjRef::new(3, 0)).as_dict().unwrap().get(b"Contents").cloned(), before_contents);
assert_eq!(*no_parent.get(ObjRef::new(9, 0)), before_pt);
// Duplicate entry in parent's /K must fail with AmbiguousParent before touching /Contents or /ParentTree.
let mut dup_parent = figures_doc();
dup_parent
.update_dict(ObjRef::new(6, 0), |d| {
d.set(b"K".to_vec(), Object::Array(vec![Object::Ref(ObjRef::new(7, 0)), Object::Ref(ObjRef::new(8, 0)), Object::Ref(ObjRef::new(7, 0))]));
})
.unwrap();
assert_eq!(mark_tag_artifact(&mut dup_parent, ObjRef::new(7, 0)), Err(ArtifactError::AmbiguousParent));
assert_eq!(dup_parent.get(ObjRef::new(3, 0)).as_dict().unwrap().get(b"Contents").cloned(), before_contents);
assert_eq!(*dup_parent.get(ObjRef::new(9, 0)), before_pt);
// Partial MCID match (MCID 0 present, MCID 99 absent) must fail with ContentNotFound in
// mark_tag_artifact without mutating /Contents, while mark_decorative still converts MCID 0.
let mut partial = figures_doc();
partial.update_dict(ObjRef::new(7, 0), |d| d.set(b"K".to_vec(), Object::Array(vec![Object::Int(0), Object::Int(99)]))).unwrap();
assert_eq!(mark_tag_artifact(&mut partial, ObjRef::new(7, 0)), Err(ArtifactError::ContentNotFound));
assert_eq!(partial.get(ObjRef::new(3, 0)).as_dict().unwrap().get(b"Contents").cloned(), before_contents);
assert_eq!(*partial.get(ObjRef::new(9, 0)), before_pt);
mark_decorative(&mut partial, ObjRef::new(7, 0)).unwrap();
assert_ne!(partial.get(ObjRef::new(3, 0)).as_dict().unwrap().get(b"Contents").cloned(), before_contents);
assert_ne!(*partial.get(ObjRef::new(9, 0)), before_pt);
// When none of a figure's MCIDs appear on the page, mark_decorative leaves /Contents and /ParentTree untouched.
let mut absent = figures_doc();
absent.update_dict(ObjRef::new(7, 0), |d| d.set(b"K".to_vec(), Object::Int(99))).unwrap();
mark_decorative(&mut absent, ObjRef::new(7, 0)).unwrap();
assert_eq!(absent.get(ObjRef::new(3, 0)).as_dict().unwrap().get(b"Contents").cloned(), before_contents);
assert_eq!(*absent.get(ObjRef::new(9, 0)), before_pt);
}
fn ref_(n: u32) -> Option<pdfcraft_cos::ObjRef> {
Some(pdfcraft_cos::ObjRef::new(n, 0))
}
+14 -8
View File
@@ -99,14 +99,20 @@ fn shown_strings(ops: &[Op]) -> Vec<String> {
#[test]
fn mark_decorative_leaves_the_other_stream_untouched() {
let mut doc = tagged_figure_two_streams("0 0 1 rg 300 600 100 50 re f", "BT /F1 12 Tf 72 500 Td (After) Tj ET");
let before = doc.get(ObjRef::new(11, 0)).clone();
mark_decorative(&mut doc, ObjRef::new(8, 0)).expect("mark decorative succeeds");
assert!(figures(&doc).is_empty());
assert_eq!(*doc.get(ObjRef::new(11, 0)), *before, "the second stream holds no figure and keeps its object");
let page = doc.get(ObjRef::new(3, 0));
let contents = page.as_dict().unwrap().get(b"Contents").unwrap().as_array().unwrap();
assert_eq!(contents[1].as_ref(), Some(ObjRef::new(11, 0)), "the untouched stream must not be replaced by a re-encoded copy");
for use_artifact in [false, true] {
let mut doc = tagged_figure_two_streams("0 0 1 rg 300 600 100 50 re f", "BT /F1 12 Tf 72 500 Td (After) Tj ET");
let before = doc.get(ObjRef::new(11, 0)).clone();
if use_artifact {
mark_tag_artifact(&mut doc, ObjRef::new(8, 0)).expect("mark tag artifact succeeds");
} else {
mark_decorative(&mut doc, ObjRef::new(8, 0)).expect("mark decorative succeeds");
}
assert!(figures(&doc).is_empty());
assert_eq!(*doc.get(ObjRef::new(11, 0)), *before, "the second stream holds no figure and keeps its object");
let page = doc.get(ObjRef::new(3, 0));
let contents = page.as_dict().unwrap().get(b"Contents").unwrap().as_array().unwrap();
assert_eq!(contents[1].as_ref(), Some(ObjRef::new(11, 0)), "the untouched stream must not be replaced by a re-encoded copy");
}
}
/// A and B supply different operands to R's BDC. Reusing R must not let the first