From a50e1491a4d0138fd7fca936407d7716450db6ed Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 30 Oct 2026 08:59:03 +0800 Subject: [PATCH 1/5] M12: a picture an XFA form links is drawn from the PDF's XFAImages name tree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A template's without inline data is now found in the catalog's /Names /XFAImages tree, keyed by the URI the template writes (XFA 3.3 pp. 257-258, "Storing Images Inside PDF"). Only that tree is looked in; a link is never followed outside the file. The tree walk is bounded (depth, nodes, names, a seen-set), and so are the bytes read. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/xfa/src/fixtures.rs | 39 ++++++++ crates/xfa/src/layout/draw.rs | 2 +- crates/xfa/src/layout/field.rs | 2 +- crates/xfa/src/lib.rs | 4 +- crates/xfa/src/linked_images.rs | 158 ++++++++++++++++++++++++++++++++ crates/xfa/src/model.rs | 3 + crates/xfa/src/parse.rs | 3 +- crates/xfa/src/script.rs | 4 +- crates/xfa/src/tests.rs | 57 ++++++++++++ 9 files changed, 267 insertions(+), 5 deletions(-) create mode 100644 crates/xfa/src/linked_images.rs diff --git a/crates/xfa/src/fixtures.rs b/crates/xfa/src/fixtures.rs index 8f6e0f7ee8..3fe12b7af0 100644 --- a/crates/xfa/src/fixtures.rs +++ b/crates/xfa/src/fixtures.rs @@ -739,3 +739,42 @@ if (this.rawValue == "1") { "## .to_string() } + +/// A form whose two draws link their pictures instead of embedding them: `.\logo.png`, which the +/// PDF keeps in its `XFAImages` name tree (a [`tiny_png`], under a `Kids` node, as producers +/// split the tree), and `.\missing.png`, which it doesn't (XFA 3.3 pp. 257–258). +pub fn linked_image_shell() -> Vec { + linked_image_shell_with(b"<< /Kids [7 0 R] >>", br"<< /Limits [(.\\logo.png) (.\\logo.png)] /Names [(.\\logo.png) 8 0 R] >>") +} + +/// [`linked_image_shell`] with its own name tree: `root` is object 6 (the tree the catalog names), +/// `kid` object 7, and object 8 the PNG. +pub fn linked_image_shell_with(root: &[u8], kid: &[u8]) -> Vec { + let xdp = r##" + + + +"##; + let mut png = format!("<< /Length {} >>\nstream\n", TINY_PNG.len()).into_bytes(); + png.extend_from_slice(TINY_PNG); + png.extend_from_slice(b"\nendstream"); + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /NeedsRendering true /AcroForm << /XFA 5 0 R /Fields [] >> /Names << /XFAImages 6 0 R >> >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents 4 0 R >>".to_vec(), + b"<< /Length 44 >>\nstream\nBT /F1 12 Tf 72 700 Td (Please wait...) Tj ET\nendstream".to_vec(), + format!("<< /Length {} >>\nstream\n{xdp}\nendstream", xdp.len()).into_bytes(), + root.to_vec(), + kid.to_vec(), + png, + ]; + assemble(objs) +} diff --git a/crates/xfa/src/layout/draw.rs b/crates/xfa/src/layout/draw.rs index f6520c1628..eb04fd934e 100644 --- a/crates/xfa/src/layout/draw.rs +++ b/crates/xfa/src/layout/draw.rs @@ -233,7 +233,7 @@ impl Layouter<'_> { let inner = rect.inset(&d.common.margin); match &d.value { Value::Text(_) | Value::Rich(_) => self.draw_value_text(&d.value, inner, &ctx.font, &ctx.para), - Value::Image { content_type, data } => self.draw_image(data, content_type, inner), + Value::Image { content_type, data, .. } => self.draw_image(data, content_type, inner), Value::Line { slope_up, edge } => { if !edge.visible() { return Ok(()); diff --git a/crates/xfa/src/layout/field.rs b/crates/xfa/src/layout/field.rs index cc15f16788..991eed63d1 100644 --- a/crates/xfa/src/layout/field.rs +++ b/crates/xfa/src/layout/field.rs @@ -95,7 +95,7 @@ impl Layouter<'_> { let box_ = ui.inset(&f.ui_margin); match (&from_data, &f.value) { (Some(d), _) => self.draw_image(d, "", box_)?, - (None, Value::Image { content_type, data }) => self.draw_image(data, content_type, box_)?, + (None, Value::Image { content_type, data, .. }) => self.draw_image(data, content_type, box_)?, _ => {} } // Then the field itself, which clicking gives a new picture. diff --git a/crates/xfa/src/lib.rs b/crates/xfa/src/lib.rs index 7e63218c90..0927325fd5 100644 --- a/crates/xfa/src/lib.rs +++ b/crates/xfa/src/lib.rs @@ -11,6 +11,7 @@ pub mod data; pub mod fixtures; pub mod image; pub mod layout; +pub mod linked_images; pub mod model; pub mod packets; pub mod parse; @@ -156,7 +157,8 @@ pub fn render_into(doc: &mut Document) -> Result { if packets.xdp.trim().is_empty() { return Err(XfaError::Malformed("the XFA packets hold no template".into())); } - let (tpl, parse_warnings) = parse(&packets.xdp)?; + let (mut tpl, mut parse_warnings) = parse(&packets.xdp)?; + parse_warnings.extend(linked_images::resolve(&mut tpl, doc)); let data = parse_datasets(&packets.xdp); let laid = apply_overrides(&tpl, &overrides(doc)); let mut form = layout(&laid, data.as_ref())?; diff --git a/crates/xfa/src/linked_images.rs b/crates/xfa/src/linked_images.rs new file mode 100644 index 0000000000..8dd66f36ff --- /dev/null +++ b/crates/xfa/src/linked_images.rs @@ -0,0 +1,158 @@ +//! Pictures a template links instead of embedding (``). Since XFA 2.6 a form +//! keeps them in its PDF: each image file is a stream holding the file's bytes, named in the +//! `XFAImages` name tree of the catalog's `Names` dictionary by the URI the template writes (XFA +//! 3.3 pp. 257–258, "Storing Images Inside PDF"). Only that tree is looked in: as the spec warns, +//! a form that isn't trusted must not have its links followed outside the file. + +use std::collections::{HashMap, HashSet}; + +use pdfcraft_cos::{Document, ObjRef, Object}; + +use crate::model::{Field, Node, Subform, Template, Value}; + +/// Most nodes of the name tree visited, and most names kept from it. +const MAX_TREE_NODES: usize = 4096; +const MAX_NAMES: usize = 4096; +/// Deepest the name tree is followed. +const MAX_DEPTH: usize = 32; +/// Largest picture taken from the file, as for a picture written into the template. +const MAX_IMAGE: usize = 32 << 20; +/// Most bytes of pictures taken from the file in all. +const MAX_TOTAL: usize = 128 << 20; +/// Deepest the template is walked. +const MAX_TEMPLATE_DEPTH: usize = 256; + +/// The `XFAImages` name tree: each name, as text, and the object it names. +fn names(doc: &Document) -> HashMap { + let mut out = HashMap::new(); + let Some(root) = doc.root() else { return out }; + let catalog = doc.get(root); + let tree = + catalog.as_dict().and_then(|c| c.get(b"Names")).map(|n| doc.resolve(n)).and_then(|n| n.as_dict().and_then(|d| d.get(b"XFAImages")).cloned()); + let Some(tree) = tree else { return out }; + let mut seen: HashSet = HashSet::new(); + let mut visited = 0usize; + let mut stack = vec![(tree, 0usize)]; + while let Some((node, depth)) = stack.pop() { + if depth > MAX_DEPTH || visited >= MAX_TREE_NODES || out.len() >= MAX_NAMES { + break; + } + visited += 1; + if let Some(r) = node.as_ref() + && !seen.insert(r) + { + continue; + } + let node = doc.resolve(&node); + let Some(d) = node.as_dict() else { continue }; + if let Some(pairs) = d.get(b"Names").map(|n| doc.resolve(n)).and_then(|n| n.as_array().cloned()) { + for pair in pairs.chunks(2) { + if let [key, value] = pair + && let Some(key) = doc.resolve(key).as_string() + { + out.entry(key.to_text()).or_insert_with(|| value.clone()); + if out.len() >= MAX_NAMES { + break; + } + } + } + } + if let Some(kids) = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()) { + stack.extend(kids.into_iter().map(|k| (k, depth + 1))); + } + } + out +} + +/// Pictures found so far by their link, read from the file once each. +struct Found<'a> { + doc: &'a Document, + names: HashMap, + read: HashMap>>, + total: usize, + warnings: Vec, +} + +impl Found<'_> { + /// The bytes of the picture linked as `href`, when the file holds it. + fn get(&mut self, href: &str) -> Option> { + if let Some(data) = self.read.get(href) { + return data.clone(); + } + let data = self.load(href); + self.read.insert(href.to_string(), data.clone()); + data + } + + fn load(&mut self, href: &str) -> Option> { + let object = self.names.get(href)?; + let object = self.doc.resolve(object); + let Object::Stream(s) = &*object else { return None }; + let data = match s.decoded_within(MAX_IMAGE) { + Ok(d) => d, + Err(_) => { + self.warnings.push(format!("a picture the form keeps in the file ({href}) could not be read; it is left out")); + return None; + } + }; + if self.total.saturating_add(data.len()) > MAX_TOTAL { + self.warnings.push("the pictures the form keeps in the file are larger than 128 MB in all; the rest are left out".into()); + return None; + } + self.total = self.total.saturating_add(data.len()); + Some(data) + } + + fn value(&mut self, v: &mut Value) { + if let Value::Image { data, href: Some(href), .. } = v + && data.is_empty() + && let Some(found) = self.get(href) + { + *data = found; + } + } + + fn field(&mut self, f: &mut Field) { + self.value(&mut f.value); + if let Some(c) = f.caption.as_mut() { + self.value(&mut c.value); + } + } + + fn nodes(&mut self, nodes: &mut [Node], depth: usize) { + if depth > MAX_TEMPLATE_DEPTH { + return; + } + for n in nodes { + match n { + Node::Subform(s) => self.subform(s, depth + 1), + Node::Area(a) => self.nodes(&mut a.children, depth + 1), + Node::Field(f) => self.field(f), + Node::Draw(d) => self.value(&mut d.value), + Node::ExclGroup(g) => g.fields.iter_mut().for_each(|f| self.field(f)), + } + } + } + + fn subform(&mut self, s: &mut Subform, depth: usize) { + if let Some(set) = s.page_set.as_mut() { + for area in &mut set.areas { + self.nodes(&mut area.items, depth + 1); + } + } + self.nodes(&mut s.children, depth); + } +} + +/// Give the pictures `tpl` links to (and doesn't embed) the data `doc` keeps for them. A link +/// the file holds nothing for stays empty. What couldn't be read is returned as warnings. +pub fn resolve(tpl: &mut Template, doc: &Document) -> Vec { + let names = names(doc); + if names.is_empty() { + return Vec::new(); + } + let mut found = Found { doc, names, read: HashMap::new(), total: 0, warnings: Vec::new() }; + found.subform(&mut tpl.root, 0); + found.warnings.dedup(); + found.warnings +} diff --git a/crates/xfa/src/model.rs b/crates/xfa/src/model.rs index 0fac8db18e..70db1615af 100644 --- a/crates/xfa/src/model.rs +++ b/crates/xfa/src/model.rs @@ -598,6 +598,9 @@ pub enum Value { Image { content_type: String, data: Vec, + /// Where the template links the picture from (`href`), when it does: its data is then + /// found in the PDF ([`crate::linked_images`]). + href: Option, }, /// A diagonal line through the box: `\` (top-left to bottom-right) or `/`. Line { diff --git a/crates/xfa/src/parse.rs b/crates/xfa/src/parse.rs index 838d1730ea..2527311fd3 100644 --- a/crates/xfa/src/parse.rs +++ b/crates/xfa/src/parse.rs @@ -401,7 +401,8 @@ fn parse_value(n: Option, warnings: &mut Vec) -> Value { warnings.push("an image is larger than 32 MB; it is left out".into()); return Value::Empty; } - return Value::Image { content_type: c.attribute("contentType").unwrap_or("").to_string(), data }; + let href = c.attribute("href").map(str::trim).filter(|h| !h.is_empty()).map(str::to_string); + return Value::Image { content_type: c.attribute("contentType").unwrap_or("").to_string(), data, href }; } "line" => { return Value::Line { slope_up: c.attribute("slope") == Some("/"), edge: child(c, "edge").map(parse_edge).unwrap_or_default() }; diff --git a/crates/xfa/src/script.rs b/crates/xfa/src/script.rs index 6946aa9b1c..005f56a640 100644 --- a/crates/xfa/src/script.rs +++ b/crates/xfa/src/script.rs @@ -656,7 +656,9 @@ pub fn template_of(doc: &Document) -> Result { if packets.xdp.trim().is_empty() { return Err(XfaError::Malformed("the XFA packets hold no template".into())); } - crate::parse(&packets.xdp).map(|(t, _)| t) + let (mut tpl, _) = crate::parse(&packets.xdp)?; + crate::linked_images::resolve(&mut tpl, doc); + Ok(tpl) } /// Whether the template field at `som` is multiline text without a fixed height. diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index e424be5441..bd359d5ac8 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -4145,3 +4145,60 @@ fn clear_password_values_clears_static_xfa_password_fields() { let packets = read_packets(&doc).unwrap().unwrap(); assert!(!packets.xdp.contains("hunter2"), "{}", packets.xdp); } + +/// A picture the template links (`href`) is drawn from the `XFAImages` name tree the PDF keeps +/// it in (XFA 3.3 pp. 257–258), through the tree's `Kids`. A link the file holds nothing for +/// stays out, as before, and is never looked for outside the file. +#[test] +fn a_linked_picture_is_drawn_from_the_pdfs_xfa_images() { + let mut doc = Document::open(Arc::new(crate::fixtures::linked_image_shell())).unwrap(); + let mut tpl = crate::script::template_of(&doc).unwrap(); + let mut pictures = Vec::new(); + let mut walk = |nodes: &[Node]| { + for n in nodes { + if let Node::Subform(s) = n { + for c in &s.children { + if let Node::Draw(d) = c + && let Value::Image { data, href, .. } = &d.value + { + pictures.push((href.clone(), data.len())); + } + } + } + } + }; + walk(&tpl.root.children); + assert_eq!(pictures, vec![(Some(r".\logo.png".into()), crate::fixtures::tiny_png().len()), (Some(r".\missing.png".into()), 0)]); + assert!(crate::linked_images::resolve(&mut tpl, &doc).is_empty(), "nothing to say of a link the file doesn't hold"); + let report = render_into(&mut doc).unwrap(); + let doc = Document::open(Arc::new(write_incremental(&doc, &SaveOptions::default()).unwrap())).unwrap(); + let images = doc + .object_numbers() + .into_iter() + .filter( + |n| matches!(&*doc.get(pdfcraft_cos::ObjRef { num: *n, generation: 0 }), Object::Stream(s) if s.dict.name(b"Subtype") == Some(b"Image")), + ) + .count(); + assert_eq!(images, 2, "the linked PNG and its soft mask"); + assert_eq!(report.warnings.iter().filter(|w| w.contains("not a JPEG, PNG")).count(), 1, "only the missing one: {:?}", report.warnings); +} + +/// A hostile `XFAImages` tree neither hangs nor panics: `Kids` that lead back to themselves, a +/// name that isn't a string, and a name for something that isn't a stream. The picture is then +/// left out, as a link the file holds nothing for. +#[test] +fn a_hostile_xfa_images_tree_is_walked_once_and_gives_nothing() { + for (root, kid) in [ + (&b"<< /Kids [6 0 R 7 0 R] >>"[..], &b"<< /Kids [6 0 R 7 0 R] /Names [42 8 0 R] >>"[..]), + (&b"<< /Names [(.\\logo.png) 7 0 R] >>"[..], &b"<< /Kids [6 0 R] >>"[..]), + (&b"<< /Kids 7 0 R >>"[..], &b"[6 0 R]"[..]), + ] { + let mut doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_with(root, kid))).unwrap(); + let mut tpl = crate::script::template_of(&doc).unwrap(); + assert!(crate::linked_images::resolve(&mut tpl, &doc).is_empty()); + let report = render_into(&mut doc).unwrap(); + assert!(report.warnings.iter().any(|w| w.contains("not a JPEG, PNG")), "{:?}", report.warnings); + let images = doc.object_numbers().into_iter().filter(|n| matches!(&*doc.get(pdfcraft_cos::ObjRef { num: *n, generation: 0 }), Object::Stream(s) if s.dict.name(b"Subtype") == Some(b"Image"))).count(); + assert_eq!(images, 0, "no picture drawn"); + } +} From f75e6d9d5db068f74b5746423f9acf1a6232c607 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 30 Oct 2026 12:30:56 +0800 Subject: [PATCH 2/5] M12: XFAImages: an over-deep branch doesn't end the walk, and each place a picture is used counts - A name-tree branch past the depth limit is skipped; the branches beside it are still walked (before, the walk stopped there). - Every place a picture is put counts against the pictures' total, since each holds its own copy: one large picture used in many places can no longer hold a copy per place without limit. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/xfa/src/linked_images.rs | 35 ++++++++++++++++---------- crates/xfa/src/tests.rs | 44 +++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 13 deletions(-) diff --git a/crates/xfa/src/linked_images.rs b/crates/xfa/src/linked_images.rs index 8dd66f36ff..ff85a1a431 100644 --- a/crates/xfa/src/linked_images.rs +++ b/crates/xfa/src/linked_images.rs @@ -17,8 +17,12 @@ const MAX_NAMES: usize = 4096; const MAX_DEPTH: usize = 32; /// Largest picture taken from the file, as for a picture written into the template. const MAX_IMAGE: usize = 32 << 20; -/// Most bytes of pictures taken from the file in all. +/// Most bytes of pictures taken from the file in all, each place a picture is used counted (a +/// picture is held once per place). Small in tests, so a test can reach it. +#[cfg(not(test))] const MAX_TOTAL: usize = 128 << 20; +#[cfg(test)] +const MAX_TOTAL: usize = 1 << 10; /// Deepest the template is walked. const MAX_TEMPLATE_DEPTH: usize = 256; @@ -34,9 +38,13 @@ fn names(doc: &Document) -> HashMap { let mut visited = 0usize; let mut stack = vec![(tree, 0usize)]; while let Some((node, depth)) = stack.pop() { - if depth > MAX_DEPTH || visited >= MAX_TREE_NODES || out.len() >= MAX_NAMES { + if visited >= MAX_TREE_NODES || out.len() >= MAX_NAMES { break; } + // Too deep: this branch is left, the others are still walked. + if depth > MAX_DEPTH { + continue; + } visited += 1; if let Some(r) = node.as_ref() && !seen.insert(r) @@ -74,14 +82,20 @@ struct Found<'a> { } impl Found<'_> { - /// The bytes of the picture linked as `href`, when the file holds it. + /// A copy of the picture linked as `href` for one more place, when the file holds it and the + /// copies so far leave room for it under [`MAX_TOTAL`]. fn get(&mut self, href: &str) -> Option> { - if let Some(data) = self.read.get(href) { - return data.clone(); + if !self.read.contains_key(href) { + let data = self.load(href); + self.read.insert(href.to_string(), data); } - let data = self.load(href); - self.read.insert(href.to_string(), data.clone()); - data + let len = self.read.get(href).and_then(Option::as_ref).map(Vec::len)?; + if self.total.saturating_add(len) > MAX_TOTAL { + self.warnings.push("the pictures the form keeps in the file take too much memory in all; the rest are left out".into()); + return None; + } + self.total = self.total.saturating_add(len); + self.read.get(href).cloned().flatten() } fn load(&mut self, href: &str) -> Option> { @@ -95,11 +109,6 @@ impl Found<'_> { return None; } }; - if self.total.saturating_add(data.len()) > MAX_TOTAL { - self.warnings.push("the pictures the form keeps in the file are larger than 128 MB in all; the rest are left out".into()); - return None; - } - self.total = self.total.saturating_add(data.len()); Some(data) } diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index bd359d5ac8..282c1bc90e 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -4202,3 +4202,47 @@ fn a_hostile_xfa_images_tree_is_walked_once_and_gives_nothing() { assert_eq!(images, 0, "no picture drawn"); } } + +/// An over-deep branch of the `XFAImages` tree is left on its own: the walk goes on to the +/// branches beside it, and the picture one of them holds is found. +#[test] +fn an_over_deep_xfa_images_branch_does_not_stop_the_walk() { + let mut deep = String::from("<< /Names [(x) 8 0 R] >>"); + for _ in 0..40 { + deep = format!("<< /Kids [{deep}] >>"); + } + // The deep branch is listed last, so it is walked first. + let root = format!("<< /Kids [7 0 R {deep}] >>"); + let doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_with(root.as_bytes(), br"<< /Names [(.\\logo.png) 8 0 R] >>"))).unwrap(); + let tpl = crate::script::template_of(&doc).unwrap(); + let logo = tpl.root.children.iter().find_map(|n| match n { + Node::Subform(s) => s.children.iter().find_map(|c| match c { + Node::Draw(d) if d.common.name.as_deref() == Some("logo") => Some(d.value.clone()), + _ => None, + }), + _ => None, + }); + assert!(matches!(logo, Some(Value::Image { data, .. }) if !data.is_empty()), "the logo is found beside the deep branch"); +} + +/// A picture used in many places is held once per place, so every place counts against the +/// pictures' total (small in tests): past it the rest are left out, with a warning. +#[test] +fn a_linked_picture_used_in_many_places_counts_each_place() { + let doc = Document::open(Arc::new(crate::fixtures::linked_image_shell())).unwrap(); + let draws: String = (0..30) + .map(|i| format!(r#""#)) + .collect(); + let xml = format!( + r#""# + ); + let (mut tpl, _) = crate::parse(&xml).unwrap(); + let warnings = crate::linked_images::resolve(&mut tpl, &doc); + let filled = + tpl.root.children.iter().filter(|n| matches!(n, Node::Draw(d) if matches!(&d.value, Value::Image { data, .. } if !data.is_empty()))).count(); + let one = crate::fixtures::tiny_png().len(); + assert_eq!(filled, (1 << 10) / one, "as many places as the total holds"); + assert!(warnings.iter().any(|w| w.contains("too much memory")), "{warnings:?}"); +} From b1c68aa028c261efb1957bbae1b2ce9e343a02fd Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 30 Oct 2026 12:50:18 +0800 Subject: [PATCH 3/5] M12: XFAImages: pictures kept for reuse count against the total too A picture read from the file is kept for the next place that links it. Those kept bytes now count against MAX_TOTAL as well: past it, a name not read yet is neither read nor kept, so a tree with many names holds no more than the total however many names it has. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/xfa/src/linked_images.rs | 52 +++++++++++++++++++++++++++++---- crates/xfa/src/tests.rs | 21 +++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/crates/xfa/src/linked_images.rs b/crates/xfa/src/linked_images.rs index ff85a1a431..d9c2e2983b 100644 --- a/crates/xfa/src/linked_images.rs +++ b/crates/xfa/src/linked_images.rs @@ -77,27 +77,57 @@ struct Found<'a> { doc: &'a Document, names: HashMap, read: HashMap>>, + /// Bytes of the pictures in `read`, and of the copies put in places: each at most + /// [`MAX_TOTAL`], however many names the tree holds. + kept: usize, total: usize, warnings: Vec, } impl Found<'_> { /// A copy of the picture linked as `href` for one more place, when the file holds it and the - /// copies so far leave room for it under [`MAX_TOTAL`]. + /// pictures kept and the copies so far leave room for it under [`MAX_TOTAL`]. fn get(&mut self, href: &str) -> Option> { if !self.read.contains_key(href) { - let data = self.load(href); + // Past the total, a picture not read yet is neither read nor kept. + let data = if self.kept >= MAX_TOTAL { None } else { self.load(href) }; + let data = match data { + Some(d) if self.kept.saturating_add(d.len()) > MAX_TOTAL => { + self.over_total(); + None + } + Some(d) => { + self.kept = self.kept.saturating_add(d.len()); + Some(d) + } + None => { + if self.kept >= MAX_TOTAL && self.names.contains_key(href) { + self.over_total(); + } + None + } + }; self.read.insert(href.to_string(), data); } let len = self.read.get(href).and_then(Option::as_ref).map(Vec::len)?; if self.total.saturating_add(len) > MAX_TOTAL { - self.warnings.push("the pictures the form keeps in the file take too much memory in all; the rest are left out".into()); + self.over_total(); return None; } self.total = self.total.saturating_add(len); self.read.get(href).cloned().flatten() } + fn over_total(&mut self) { + self.warnings.push("the pictures the form keeps in the file take too much memory in all; the rest are left out".into()); + } + + /// Bytes of the pictures read so far and kept for reuse. + #[cfg(test)] + fn kept(&self) -> usize { + self.read.values().flatten().map(Vec::len).sum() + } + fn load(&mut self, href: &str) -> Option> { let object = self.names.get(href)?; let object = self.doc.resolve(object); @@ -156,12 +186,22 @@ impl Found<'_> { /// Give the pictures `tpl` links to (and doesn't embed) the data `doc` keeps for them. A link /// the file holds nothing for stays empty. What couldn't be read is returned as warnings. pub fn resolve(tpl: &mut Template, doc: &Document) -> Vec { + resolve_found(tpl, doc).map(|f| f.warnings).unwrap_or_default() +} + +fn resolve_found<'a>(tpl: &mut Template, doc: &'a Document) -> Option> { let names = names(doc); if names.is_empty() { - return Vec::new(); + return None; } - let mut found = Found { doc, names, read: HashMap::new(), total: 0, warnings: Vec::new() }; + let mut found = Found { doc, names, read: HashMap::new(), kept: 0, total: 0, warnings: Vec::new() }; found.subform(&mut tpl.root, 0); found.warnings.dedup(); - found.warnings + Some(found) +} + +/// [`resolve`], and the bytes of pictures it held for reuse at the end. +#[cfg(test)] +pub(crate) fn resolve_keeping(tpl: &mut Template, doc: &Document) -> (Vec, usize) { + resolve_found(tpl, doc).map_or((Vec::new(), 0), |f| (f.warnings.clone(), f.kept())) } diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index 282c1bc90e..8038c8d3fa 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -4246,3 +4246,24 @@ fn a_linked_picture_used_in_many_places_counts_each_place() { assert_eq!(filled, (1 << 10) / one, "as many places as the total holds"); assert!(warnings.iter().any(|w| w.contains("too much memory")), "{warnings:?}"); } + +/// Pictures read from the file and kept for reuse count against the total too (small in tests): +/// a tree with many names, each read once, holds no more than the total, however many names +/// there are; past it a name is neither read nor kept. +#[test] +fn pictures_kept_for_reuse_stay_under_the_total_whatever_the_number_of_names() { + let names: String = (0..30).map(|i| format!("(n{i}) 8 0 R ")).collect(); + let kid = format!("<< /Names [{names}] >>"); + let doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_with(b"<< /Kids [7 0 R] >>", kid.as_bytes()))).unwrap(); + let draws: String = + (0..30).map(|i| format!(r#""#)).collect(); + let xml = format!( + r#""# + ); + let (mut tpl, _) = crate::parse(&xml).unwrap(); + let (warnings, kept) = crate::linked_images::resolve_keeping(&mut tpl, &doc); + assert!(kept <= 1 << 10, "kept {kept} bytes"); + assert!(warnings.iter().any(|w| w.contains("too much memory")), "{warnings:?}"); +} From bea66e608170d50c6c1fd79fcc3ba29eec8c1b8b Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 30 Oct 2026 14:29:39 +0800 Subject: [PATCH 4/5] M12: XFAImages: the name tree is walked borrowing each node, not copying its Kids The walk queued every entry of a Kids array before checking the node limit, and resolving a direct node or array deep-copied it (two to four times for a direct tree). It now recurses (depth stays capped at 32), borrowing each node from its resolved owner, and stops at the limit as it goes: only the branch being walked is held. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/xfa/src/linked_images.rs | 101 ++++++++++++++++++++++---------- crates/xfa/src/tests.rs | 21 ++++++- 2 files changed, 88 insertions(+), 34 deletions(-) diff --git a/crates/xfa/src/linked_images.rs b/crates/xfa/src/linked_images.rs index d9c2e2983b..c60d465d52 100644 --- a/crates/xfa/src/linked_images.rs +++ b/crates/xfa/src/linked_images.rs @@ -26,50 +26,87 @@ const MAX_TOTAL: usize = 1 << 10; /// Deepest the template is walked. const MAX_TEMPLATE_DEPTH: usize = 256; +/// `f` of `o`, a reference followed. A direct object is lent as it is, not copied: a tree's +/// arrays can be as long as the file allows. +fn with_resolved(doc: &Document, o: &Object, f: impl FnOnce(&Object) -> R) -> R { + match o { + Object::Ref(r) => f(&doc.get(*r)), + other => f(other), + } +} + /// The `XFAImages` name tree: each name, as text, and the object it names. fn names(doc: &Document) -> HashMap { - let mut out = HashMap::new(); - let Some(root) = doc.root() else { return out }; + let mut walk = Walk { doc, out: HashMap::new(), seen: HashSet::new(), visited: 0 }; + let Some(root) = doc.root() else { return walk.out }; let catalog = doc.get(root); - let tree = - catalog.as_dict().and_then(|c| c.get(b"Names")).map(|n| doc.resolve(n)).and_then(|n| n.as_dict().and_then(|d| d.get(b"XFAImages")).cloned()); - let Some(tree) = tree else { return out }; - let mut seen: HashSet = HashSet::new(); - let mut visited = 0usize; - let mut stack = vec![(tree, 0usize)]; - while let Some((node, depth)) = stack.pop() { - if visited >= MAX_TREE_NODES || out.len() >= MAX_NAMES { - break; - } + if let Some(names) = catalog.as_dict().and_then(|c| c.get(b"Names")) { + with_resolved(doc, names, |n| { + if let Some(tree) = n.as_dict().and_then(|d| d.get(b"XFAImages")) { + walk.node(tree, 0); + } + }); + } + walk.out +} + +/// A walk of the name tree, borrowing each node from its resolved owner: nothing of the tree +/// is copied but the names kept, and only the branch being walked is held. +struct Walk<'a> { + doc: &'a Document, + out: HashMap, + seen: HashSet, + visited: usize, +} + +impl Walk<'_> { + fn full(&self) -> bool { + self.visited >= MAX_TREE_NODES || self.out.len() >= MAX_NAMES + } + + fn node(&mut self, node: &Object, depth: usize) { // Too deep: this branch is left, the others are still walked. - if depth > MAX_DEPTH { - continue; + if self.full() || depth > MAX_DEPTH { + return; } - visited += 1; + self.visited += 1; if let Some(r) = node.as_ref() - && !seen.insert(r) + && !self.seen.insert(r) { - continue; + return; } - let node = doc.resolve(&node); - let Some(d) = node.as_dict() else { continue }; - if let Some(pairs) = d.get(b"Names").map(|n| doc.resolve(n)).and_then(|n| n.as_array().cloned()) { - for pair in pairs.chunks(2) { - if let [key, value] = pair - && let Some(key) = doc.resolve(key).as_string() - { - out.entry(key.to_text()).or_insert_with(|| value.clone()); - if out.len() >= MAX_NAMES { - break; + let doc = self.doc; + with_resolved(doc, node, |node| { + let Some(d) = node.as_dict() else { return }; + if let Some(pairs) = d.get(b"Names") { + with_resolved(doc, pairs, |pairs| self.pairs(pairs)); + } + if let Some(kids) = d.get(b"Kids") { + with_resolved(doc, kids, |kids| { + for kid in kids.as_array().map(Vec::as_slice).unwrap_or_default() { + if self.full() { + break; + } + self.node(kid, depth + 1); } - } + }); + } + }); + } + + fn pairs(&mut self, pairs: &Object) { + let doc = self.doc; + for pair in pairs.as_array().map(Vec::as_slice).unwrap_or_default().chunks(2) { + if self.out.len() >= MAX_NAMES { + break; + } + if let [key, value] = pair + && let Some(key) = with_resolved(doc, key, |k| k.as_string().map(|s| s.to_text())) + { + self.out.entry(key).or_insert_with(|| value.clone()); } } - if let Some(kids) = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()) { - stack.extend(kids.into_iter().map(|k| (k, depth + 1))); - } } - out } /// Pictures found so far by their link, read from the file once each. diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index 8038c8d3fa..a8619b4a07 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -4211,8 +4211,8 @@ fn an_over_deep_xfa_images_branch_does_not_stop_the_walk() { for _ in 0..40 { deep = format!("<< /Kids [{deep}] >>"); } - // The deep branch is listed last, so it is walked first. - let root = format!("<< /Kids [7 0 R {deep}] >>"); + // The deep branch is listed first, so it is walked first. + let root = format!("<< /Kids [{deep} 7 0 R] >>"); let doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_with(root.as_bytes(), br"<< /Names [(.\\logo.png) 8 0 R] >>"))).unwrap(); let tpl = crate::script::template_of(&doc).unwrap(); let logo = tpl.root.children.iter().find_map(|n| match n { @@ -4225,6 +4225,23 @@ fn an_over_deep_xfa_images_branch_does_not_stop_the_walk() { assert!(matches!(logo, Some(Value::Image { data, .. }) if !data.is_empty()), "the logo is found beside the deep branch"); } +/// A `Kids` array far longer than the nodes walked is walked in order, up to the limit, and not +/// held whole: the picture its first kid names is found, however many kids follow. +#[test] +fn a_wide_xfa_images_kids_array_is_walked_in_order_within_the_limit() { + let root = format!("<< /Kids [7 0 R {}] >>", "<< >> ".repeat(10_000)); + let doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_with(root.as_bytes(), br"<< /Names [(.\\logo.png) 8 0 R] >>"))).unwrap(); + let tpl = crate::script::template_of(&doc).unwrap(); + let logo = tpl.root.children.iter().find_map(|n| match n { + Node::Subform(s) => s.children.iter().find_map(|c| match c { + Node::Draw(d) if d.common.name.as_deref() == Some("logo") => Some(d.value.clone()), + _ => None, + }), + _ => None, + }); + assert!(matches!(logo, Some(Value::Image { data, .. }) if !data.is_empty()), "the first kid's logo is found"); +} + /// A picture used in many places is held once per place, so every place counts against the /// pictures' total (small in tests): past it the rest are left out, with a warning. #[test] From 8bb04751c5efa636ef51da552facaf06449d8730 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sun, 1 Nov 2026 14:36:04 +0800 Subject: [PATCH 5/5] M12: XFAImages: every name-and-value pair counts against the walk, and over-long names are left A Names array repeating one name kept the walk going, since only new names counted. Pairs are now counted, repeats included, up to 16,384, and a name over 4,096 bytes is left before it is turned into text. The node limit no longer skips the names of the node the walk has just reached. Tests for a repeated name, an over-long name, the 4,096th node, a linked stream that isn't a picture, and an over-large picture. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/xfa/src/fixtures.rs | 9 ++- crates/xfa/src/linked_images.rs | 26 ++++++-- crates/xfa/src/tests.rs | 102 ++++++++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 7 deletions(-) diff --git a/crates/xfa/src/fixtures.rs b/crates/xfa/src/fixtures.rs index 3fe12b7af0..68b6dabc2e 100644 --- a/crates/xfa/src/fixtures.rs +++ b/crates/xfa/src/fixtures.rs @@ -750,6 +750,11 @@ pub fn linked_image_shell() -> Vec { /// [`linked_image_shell`] with its own name tree: `root` is object 6 (the tree the catalog names), /// `kid` object 7, and object 8 the PNG. pub fn linked_image_shell_with(root: &[u8], kid: &[u8]) -> Vec { + linked_image_shell_holding(root, kid, TINY_PNG) +} + +/// [`linked_image_shell_with`], object 8 a stream holding `picture` (a [`tiny_png`] there). +pub fn linked_image_shell_holding(root: &[u8], kid: &[u8], picture: &[u8]) -> Vec { let xdp = r##" "##; - let mut png = format!("<< /Length {} >>\nstream\n", TINY_PNG.len()).into_bytes(); - png.extend_from_slice(TINY_PNG); + let mut png = format!("<< /Length {} >>\nstream\n", picture.len()).into_bytes(); + png.extend_from_slice(picture); png.extend_from_slice(b"\nendstream"); let objs: Vec> = vec![ b"<< /Type /Catalog /Pages 2 0 R /NeedsRendering true /AcroForm << /XFA 5 0 R /Fields [] >> /Names << /XFAImages 6 0 R >> >>".to_vec(), diff --git a/crates/xfa/src/linked_images.rs b/crates/xfa/src/linked_images.rs index c60d465d52..9fd0e6f789 100644 --- a/crates/xfa/src/linked_images.rs +++ b/crates/xfa/src/linked_images.rs @@ -13,10 +13,20 @@ use crate::model::{Field, Node, Subform, Template, Value}; /// Most nodes of the name tree visited, and most names kept from it. const MAX_TREE_NODES: usize = 4096; const MAX_NAMES: usize = 4096; +/// Most name-and-value pairs looked at, a name repeated counted each time (only a new name is +/// kept, so [`MAX_NAMES`] alone doesn't end a `Names` array that repeats one). +const MAX_PAIRS: usize = 4 * MAX_NAMES; +/// Longest name kept, in bytes: a template's link is a short path or URI, and a longer name +/// could only be matched by an equally long link. +const MAX_NAME_LEN: usize = 4096; /// Deepest the name tree is followed. const MAX_DEPTH: usize = 32; -/// Largest picture taken from the file, as for a picture written into the template. +/// Largest picture taken from the file, as for a picture written into the template. Small in +/// tests, so a test can pass it. +#[cfg(not(test))] const MAX_IMAGE: usize = 32 << 20; +#[cfg(test)] +const MAX_IMAGE: usize = 512; /// Most bytes of pictures taken from the file in all, each place a picture is used counted (a /// picture is held once per place). Small in tests, so a test can reach it. #[cfg(not(test))] @@ -37,7 +47,7 @@ fn with_resolved(doc: &Document, o: &Object, f: impl FnOnce(&Object) -> R) -> /// The `XFAImages` name tree: each name, as text, and the object it names. fn names(doc: &Document) -> HashMap { - let mut walk = Walk { doc, out: HashMap::new(), seen: HashSet::new(), visited: 0 }; + let mut walk = Walk { doc, out: HashMap::new(), seen: HashSet::new(), visited: 0, pairs: 0 }; let Some(root) = doc.root() else { return walk.out }; let catalog = doc.get(root); if let Some(names) = catalog.as_dict().and_then(|c| c.get(b"Names")) { @@ -57,11 +67,13 @@ struct Walk<'a> { out: HashMap, seen: HashSet, visited: usize, + /// Pairs looked at so far, repeats included. + pairs: usize, } impl Walk<'_> { fn full(&self) -> bool { - self.visited >= MAX_TREE_NODES || self.out.len() >= MAX_NAMES + self.visited >= MAX_TREE_NODES || self.out.len() >= MAX_NAMES || self.pairs >= MAX_PAIRS } fn node(&mut self, node: &Object, depth: usize) { @@ -97,11 +109,15 @@ impl Walk<'_> { fn pairs(&mut self, pairs: &Object) { let doc = self.doc; for pair in pairs.as_array().map(Vec::as_slice).unwrap_or_default().chunks(2) { - if self.out.len() >= MAX_NAMES { + // The names and pairs limits only: the node limit ends the walk at the next node, not + // the names of the node it has just come to. + if self.out.len() >= MAX_NAMES || self.pairs >= MAX_PAIRS { break; } + self.pairs += 1; + // An over-long name is left before it is turned into text. if let [key, value] = pair - && let Some(key) = with_resolved(doc, key, |k| k.as_string().map(|s| s.to_text())) + && let Some(key) = with_resolved(doc, key, |k| k.as_string().filter(|s| s.bytes.len() <= MAX_NAME_LEN).map(|s| s.to_text())) { self.out.entry(key).or_insert_with(|| value.clone()); } diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index a8619b4a07..f2aa1dda3d 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -4203,6 +4203,108 @@ fn a_hostile_xfa_images_tree_is_walked_once_and_gives_nothing() { } } +/// The picture data a draw named `name` ends up with, in the template's first subform. +fn linked_picture(tpl: &crate::model::Template, name: &str) -> Option> { + tpl.root.children.iter().find_map(|n| match n { + Node::Subform(s) => s.children.iter().find_map(|c| match c { + Node::Draw(d) if d.common.name.as_deref() == Some(name) => match &d.value { + Value::Image { data, .. } => Some(data.clone()), + _ => None, + }, + _ => None, + }), + _ => None, + }) +} + +/// #5393 review: a `Names` array that repeats one name counts every pair it holds, not only the +/// names kept: the walk stops at its limit however often the name repeats, so the name listed +/// after twenty thousand repeats is never reached. +#[test] +fn a_repeated_xfa_images_name_counts_against_the_walk() { + let kid = format!(r"<< /Names [{}(.\\logo.png) 8 0 R] >>", r"(.\\dup.png) 8 0 R ".repeat(20_000)); + let doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_with(b"<< /Kids [7 0 R] >>", kid.as_bytes()))).unwrap(); + let mut tpl = crate::script::template_of(&doc).unwrap(); + crate::linked_images::resolve(&mut tpl, &doc); + assert_eq!(linked_picture(&tpl, "logo"), Some(Vec::new()), "past the limit the logo's name isn't reached"); +} + +/// The last node the walk reaches, the 4,096th, still gives its names: the node limit stops +/// the walk from going on, not the node it has just come to. +#[test] +fn the_last_xfa_images_node_the_walk_reaches_gives_its_names() { + // The root, 4,094 empty kids, and the kid holding the logo: 4,096 nodes in all. + let root = format!("<< /Kids [{}7 0 R] >>", "<< >> ".repeat(4_094)); + let doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_with(root.as_bytes(), br"<< /Names [(.\\logo.png) 8 0 R] >>"))).unwrap(); + let mut tpl = crate::script::template_of(&doc).unwrap(); + crate::linked_images::resolve(&mut tpl, &doc); + assert_eq!(linked_picture(&tpl, "logo"), Some(crate::fixtures::tiny_png()), "the 4,096th node's name is read"); +} + +/// #5393 review: a name longer than any link is left before it is turned into text; the names +/// beside it are still read. +#[test] +fn an_over_long_xfa_images_name_is_left() { + let long = "L".repeat(5_000); + let kid = format!(r"<< /Names [({long}) 8 0 R (.\\logo.png) 8 0 R] >>"); + let doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_with(b"<< /Kids [7 0 R] >>", kid.as_bytes()))).unwrap(); + let xml = format!( + r#""# + ); + let (mut tpl, _) = crate::parse(&xml).unwrap(); + crate::linked_images::resolve(&mut tpl, &doc); + assert_eq!(linked_picture(&tpl, "long"), Some(Vec::new()), "the over-long name isn't kept"); + assert_eq!(linked_picture(&tpl, "logo"), Some(crate::fixtures::tiny_png()), "the name beside it is"); +} + +/// #5393 review: a name the tree holds whose stream isn't a picture is taken as it is, and +/// drawing it draws nothing and warns, as a picture the template holds that isn't one does. +#[test] +fn a_linked_stream_that_is_not_a_picture_draws_nothing_and_warns() { + let mut doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_holding( + b"<< /Kids [7 0 R] >>", + br"<< /Names [(.\\logo.png) 8 0 R] >>", + b"this is not a picture", + ))) + .unwrap(); + // The stream is found and taken as the logo's data. + let mut tpl = crate::script::template_of(&doc).unwrap(); + assert!(crate::linked_images::resolve(&mut tpl, &doc).is_empty()); + assert_eq!(linked_picture(&tpl, "logo"), Some(b"this is not a picture".to_vec())); + let report = render_into(&mut doc).unwrap(); + // One warning for the logo and the missing picture alike (warnings are said once). + assert!(report.warnings.iter().any(|w| w.contains("not a JPEG, PNG")), "{:?}", report.warnings); + let images = doc + .object_numbers() + .into_iter() + .filter( + |n| matches!(&*doc.get(pdfcraft_cos::ObjRef { num: *n, generation: 0 }), Object::Stream(s) if s.dict.name(b"Subtype") == Some(b"Image")), + ) + .count(); + assert_eq!(images, 0, "no picture drawn"); +} + +/// #5393 review: a picture larger than the limit (small in tests) is left out with a warning, +/// not read whole. +#[test] +fn an_over_large_linked_picture_is_left_with_a_warning() { + let doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_holding( + b"<< /Kids [7 0 R] >>", + br"<< /Names [(.\\logo.png) 8 0 R] >>", + &[7u8; 600], + ))) + .unwrap(); + let mut tpl = crate::script::template_of(&doc).unwrap(); + let warnings = crate::linked_images::resolve(&mut tpl, &doc); + assert!(warnings.iter().any(|w| w.contains("could not be read")), "{warnings:?}"); + assert_eq!(linked_picture(&tpl, "logo"), Some(Vec::new())); +} + /// An over-deep branch of the `XFAImages` tree is left on its own: the walk goes on to the /// branches beside it, and the picture one of them holds is found. #[test]