From d39b55dd6da15f571a86d2ebb4ef0d93299e76f7 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:25:53 +0000 Subject: [PATCH 01/22] fix(organize): register imported form fields in linear time Each imported top-level field was checked against every field already in /AcroForm /Fields before it was appended, so a page with many distinct widget fields cost time quadratic in their number. Membership is now kept in a set beside the list. Adds a test-only work counter to the crate, so tests can bound the work a hostile document causes without timing it. --- crates/organize/src/import.rs | 5 ++++- crates/organize/src/lib.rs | 19 +++++++++++++++++++ crates/organize/src/tests.rs | 28 ++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index 80f90fb2a4..56625e5d5e 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -826,8 +826,11 @@ fn register_fields(dst: &mut Document, fields: &[ObjRef]) -> Result<(), Organize _ => (None, Dict::new()), }; let mut list = form.get(b"Fields").map(|f| dst.resolve(f)).and_then(|f| f.as_array().cloned()).unwrap_or_default(); + let mut listed: HashSet = list.iter().filter_map(Object::as_ref).collect(); for f in fields { - if !list.contains(&Object::Ref(*f)) { + #[cfg(test)] + crate::work(1); + if listed.insert(*f) { list.push(Object::Ref(*f)); } } diff --git a/crates/organize/src/lib.rs b/crates/organize/src/lib.rs index 6a696dd4db..d0e088fd71 100644 --- a/crates/organize/src/lib.rs +++ b/crates/organize/src/lib.rs @@ -739,5 +739,24 @@ pub fn remove_standard_id(doc: &mut Document, standard: Standard) -> Result<(), if removed { Ok(()) } else { Err(OrganizeError::Invalid(format!("the document doesn't declare {}", standard.label()))) } } +#[cfg(test)] +thread_local! { + /// Steps taken on this thread by loops whose cost follows the input's shape, so a test can + /// bound the work a hostile document causes without timing it. + static WORK: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Count `n` steps of input-shaped work (see [`WORK`]). +#[cfg(test)] +fn work(n: usize) { + WORK.with(|w| w.set(w.get().saturating_add(n))); +} + +/// The steps counted on this thread since the last call. +#[cfg(test)] +fn take_work() -> usize { + WORK.with(|w| w.replace(0)) +} + #[cfg(test)] mod tests; diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index 7a80e38e4c..7bb41b42c6 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -3132,3 +3132,31 @@ fn page_walk_skips_repeated_and_cyclic_kids_without_copying_attributes() { assert!(walk.next().is_none()); assert_eq!(labels(&doc), ["Page 1", "Page 2", "Page 3"]); } + +/// A one-page document whose page carries `annots` (object bodies 5…), with `extra` bodies after them. +fn page_with_annots(annots: &[String], extra: &[String], catalog_extra: &str) -> Document { + let n = annots.len(); + let refs: String = (0..n).map(|i| format!("{} 0 R ", i + 5)).collect(); + let mut b: Vec = vec![ + format!("<< /Type /Catalog /Pages 2 0 R {catalog_extra} >>"), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >>".into(), + format!("<< /Type /Page /Parent 2 0 R /Contents 4 0 R /Annots [{refs}] >>"), + body("P1"), + ]; + b.extend(annots.iter().cloned()); + b.extend(extra.iter().cloned()); + open(build(&b, "/Root 1 0 R")) +} + +#[test] +fn registering_many_imported_fields_is_linear() { + let n = 2_000; + let widgets: Vec = (0..n).map(|i| format!("<< /Type /Annot /Subtype /Widget /FT /Tx /T (f{i}) /Rect [0 0 10 10] >>")).collect(); + let src = page_with_annots(&widgets, &[], ""); + take_work(); + let out = extract_pages(&src, &[0]).unwrap(); + let work = take_work(); + assert!(work <= 4 * n, "{work} steps registering {n} fields"); + let form = out.resolve(catalog(&out).get(b"AcroForm").unwrap()).as_dict().cloned().unwrap(); + assert_eq!(form.get(b"Fields").and_then(|f| f.as_array()).map(Vec::len), Some(n)); +} From da288517e60391328144307076f7516bc3d46c88 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:26:04 +0000 Subject: [PATCH 02/22] fix(organize): resume the field-rename suffix search where it stopped Renaming a colliding field tried Name-1, Name-2, ... from 1 every time, so many fields of one name cost a quadratic number of attempts during import, combine and their conflict previews. Taken names only grow, so the search now resumes after the last suffix handed out for that name; the result is still the first free variant. --- crates/organize/src/import.rs | 27 ++++++++++++++++++++------- crates/organize/src/tests.rs | 15 +++++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index 56625e5d5e..eb617ffedf 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -680,24 +680,32 @@ pub fn combine_field_conflicts(sources: &[&Document]) -> Vec) -> Vec<(ObjRef, String, String, String)> { let mut planned = Vec::new(); - let mut seen = std::collections::HashSet::new(); + let mut walk = RenameWalk::default(); for &root in roots { - plan_renames_under(doc, root, "", taken, &mut seen, 0, &mut planned); + plan_renames_under(doc, root, "", taken, &mut walk, 0, &mut planned); } planned } +/// What one [`plan_renames`] call remembers: the nodes it visited, and for each colliding name +/// the suffix to try next. +#[derive(Default)] +struct RenameWalk { + seen: HashSet, + next: HashMap, +} + /// One node of [`plan_renames`]. fn plan_renames_under( doc: &Document, node: ObjRef, prefix: &str, taken: &mut std::collections::HashSet, - seen: &mut std::collections::HashSet, + walk: &mut RenameWalk, depth: u8, planned: &mut Vec<(ObjRef, String, String, String)>, ) { - if depth > 32 || !seen.insert(node) { + if depth > 32 || !walk.seen.insert(node) { return; } let Some(d) = doc.get(node).as_dict().cloned() else { return }; @@ -705,14 +713,19 @@ fn plan_renames_under( if !name.is_empty() { let fqn = qualified(prefix, &name); if taken.contains(&fqn) { - let mut n = 1u32; + // `taken` only grows, so every suffix below the last one handed out for this name is + // still taken: resume there instead of at 1. + let mut n = walk.next.get(&fqn).copied().unwrap_or(1); let mut variant; while { + #[cfg(test)] + crate::work(1); variant = format!("{name}-{n}"); taken.contains(&qualified(prefix, &variant)) } { - n += 1; + n = n.saturating_add(1); } + walk.next.insert(fqn.clone(), n.saturating_add(1)); let new_fqn = qualified(prefix, &variant); planned.push((node, variant.clone(), fqn, new_fqn.clone())); taken.insert(new_fqn); @@ -724,7 +737,7 @@ fn plan_renames_under( let prefix = qualified_optional(prefix, &name); let kids = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default(); for kid in kids.iter().filter_map(|k| k.as_ref()) { - plan_renames_under(doc, kid, &prefix, taken, seen, depth + 1, planned); + plan_renames_under(doc, kid, &prefix, taken, walk, depth + 1, planned); } } diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index 7bb41b42c6..dcab68a495 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -3160,3 +3160,18 @@ fn registering_many_imported_fields_is_linear() { let form = out.resolve(catalog(&out).get(b"AcroForm").unwrap()).as_dict().cloned().unwrap(); assert_eq!(form.get(b"Fields").and_then(|f| f.as_array()).map(Vec::len), Some(n)); } + +#[test] +fn renaming_many_same_named_fields_resumes_the_suffix_search() { + let n = 2_000; + let widgets: Vec = (0..n).map(|_| "<< /Type /Annot /Subtype /Widget /FT /Tx /T (x) /Rect [0 0 10 10] >>".to_string()).collect(); + let src = page_with_annots(&widgets, &[], ""); + take_work(); + let renames = combine_field_conflicts(&[&src]).remove(0); + let work = take_work(); + assert!(work <= 4 * n, "{work} suffix attempts for {n} fields"); + // Still the first free variant, in order. + assert_eq!(renames.len(), n - 1); + assert_eq!(renames.first(), Some(&("x".to_string(), "x-1".to_string()))); + assert_eq!(renames.last(), Some(&("x".to_string(), format!("x-{}", n - 1)))); +} From 59dbdee4286101c398271a28faf34c73fb1b44e8 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:26:05 +0000 Subject: [PATCH 03/22] fix(organize): copy annotation actions with one shared reference map Fixing up copied annotations cloned the whole source-to-destination map for every non-GoTo action and dropped the copies it made, so many annotations cost quadratic memory and time and an object shared by their actions was copied once per action. The copier now lends its map to the action copy and takes it back with the new entries. --- crates/organize/src/import.rs | 15 +++++++++------ crates/organize/src/tests.rs | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index eb617ffedf..5670f08591 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -154,8 +154,8 @@ impl<'a> Copier<'a> { } /// Rewrite `/P`, `/Dest` and GoTo actions on copied annotations. - fn fix_annotations(&self, dst: &mut Document, annot_pages: &HashMap) -> Result<(), OrganizeError> { - for &a in &self.annots { + fn fix_annotations(&mut self, dst: &mut Document, annot_pages: &HashMap) -> Result<(), OrganizeError> { + for &a in &std::mem::take(&mut self.annots) { let src_dict = dst.get(a).as_dict().cloned(); let Some(d) = src_dict else { continue }; let mut d = d; @@ -188,10 +188,13 @@ impl<'a> Copier<'a> { } } } else { - // Other actions (URI, Launch, JavaScript…) carry no page references. - let mut c = Copier::new(self.src, self.map.clone(), HashMap::new()); - let copied = c.copy_value(dst, &Object::Dict(ad.clone()), None)?; - d.set(b"A".to_vec(), copied); + // Other actions (URI, Launch, JavaScript…) carry no page references. The + // copier lends its map out and takes it back with the new copies in it, + // so actions sharing an object share its one copy. + let mut c = Copier::new(self.src, std::mem::take(&mut self.map), HashMap::new()); + let copied = c.copy_value(dst, &Object::Dict(ad.clone()), None); + self.map = c.map; + d.set(b"A".to_vec(), copied?); } } } diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index dcab68a495..c2bb725d73 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -3175,3 +3175,19 @@ fn renaming_many_same_named_fields_resumes_the_suffix_search() { assert_eq!(renames.first(), Some(&("x".to_string(), "x-1".to_string()))); assert_eq!(renames.last(), Some(&("x".to_string(), format!("x-{}", n - 1)))); } + +#[test] +fn annotation_actions_share_the_objects_they_copy() { + // Fifty links whose URI actions all point at one string object. + let n = 50; + let links: Vec = + (0..n).map(|_| format!("<< /Type /Annot /Subtype /Link /Rect [0 0 10 10] /A << /S /URI /URI {} 0 R >> >>", n + 5)).collect(); + let src = page_with_annots(&links, &["(https://example.com/shared)".into()], ""); + let out = extract_pages(&src, &[0]).unwrap(); + let copies = out + .object_numbers() + .into_iter() + .filter(|&o| out.get(ObjRef::new(o, out.generation(o))).as_string().is_some_and(|s| s.bytes == b"https://example.com/shared")) + .count(); + assert_eq!(copies, 1, "the shared URI is copied once, not once per action"); +} From cab1dc3eb5246292c0e506d1dbda8e207942a54f Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:26:05 +0000 Subject: [PATCH 04/22] fix(organize): read each name-tree node once when importing Looking up a named destination and listing attachments in a source's name trees followed every /Kids entry with only a depth limit, so a node listing the same child twice per level made the walk (and the attachment list) exponential in the depth. Both walks now skip a node already read. --- crates/organize/src/import.rs | 23 ++++++++++++++--------- crates/organize/src/tests.rs | 30 ++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index 5670f08591..03b16ae194 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -544,13 +544,16 @@ fn named_dest(src: &Document, name: &[u8]) -> Option { } let names = src.resolve(catalog.get(b"Names")?); let tree = names.as_dict()?.get(b"Dests")?.clone(); - name_tree_lookup(src, &tree, name, 0) + name_tree_lookup(src, &tree, name, 0, &mut HashSet::new()) } -fn name_tree_lookup(src: &Document, node: &Object, key: &[u8], depth: u8) -> Option { - if depth > 32 { +/// Name trees are read once per node: a node listed twice (or a cycle) is skipped. +fn name_tree_lookup(src: &Document, node: &Object, key: &[u8], depth: u8, seen: &mut HashSet) -> Option { + if depth > 32 || node.as_ref().is_some_and(|r| !seen.insert(r)) { return None; } + #[cfg(test)] + crate::work(1); let node = src.resolve(node); let d = node.as_dict()?; if let Some(names) = d.get(b"Names").map(|n| src.resolve(n)).and_then(|n| n.as_array().cloned()) { @@ -563,14 +566,16 @@ fn name_tree_lookup(src: &Document, node: &Object, key: &[u8], depth: u8) -> Opt } } let kids = d.get(b"Kids").map(|k| src.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default(); - kids.iter().find_map(|k| name_tree_lookup(src, k, key, depth + 1)) + kids.iter().find_map(|k| name_tree_lookup(src, k, key, depth + 1, seen)) } -/// All (key, value) pairs of a name tree, in order. -fn name_tree_entries(src: &Document, node: &Object, depth: u8, out: &mut Vec<(Vec, Object)>) { - if depth > 32 { +/// All (key, value) pairs of a name tree, in order, each node read once. +fn name_tree_entries(src: &Document, node: &Object, depth: u8, seen: &mut HashSet, out: &mut Vec<(Vec, Object)>) { + if depth > 32 || node.as_ref().is_some_and(|r| !seen.insert(r)) { return; } + #[cfg(test)] + crate::work(1); let node = src.resolve(node); let Some(d) = node.as_dict() else { return }; if let Some(names) = d.get(b"Names").map(|n| src.resolve(n)).and_then(|n| n.as_array().cloned()) { @@ -583,7 +588,7 @@ fn name_tree_entries(src: &Document, node: &Object, depth: u8, out: &mut Vec<(Ve } } for k in d.get(b"Kids").map(|k| src.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default() { - name_tree_entries(src, &k, depth + 1, out); + name_tree_entries(src, &k, depth + 1, seen, out); } } @@ -1163,7 +1168,7 @@ fn collect_attachments(dst: &mut Document, src: &Document, out: &mut Vec<(Vec>")); + } + extra.push(if tree == 0 { + "<< /Names [(a.txt) << /Type /Filespec /F (a.txt) >>] >>".into() + } else { + "<< /Names [(other) [3 0 R /Fit]] >>".into() + }); + } + let link = "<< /Type /Annot /Subtype /Link /Rect [0 0 10 10] /Dest (missing) >>".to_string(); + let catalog_extra = format!("/Names << /EmbeddedFiles {first} 0 R /Dests {} 0 R >>", first + depth + 1); + let src = page_with_annots(&[link], &extra, &catalog_extra); + take_work(); + let out = combine(&[("a", &src)]).unwrap(); + let work = take_work(); + assert!(work <= 8 * (depth + 1), "{work} name-tree nodes visited"); + let names = out.resolve(catalog(&out).get(b"Names").unwrap()).as_dict().cloned().unwrap(); + let tree = out.resolve(names.get(b"EmbeddedFiles").unwrap()).as_dict().cloned().unwrap(); + assert_eq!(tree.get(b"Names").and_then(|n| n.as_array()).map(Vec::len), Some(2), "one attachment, listed once"); +} From 1f6ce5ecc707b93a4eb816fd101b0db8be8ab391 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:26:10 +0000 Subject: [PATCH 05/22] fix(organize): analyse each form once when pruning unused XObjects The pruner decoded and walked a form XObject again on every Do that drew it, so a form drawing itself a few times per level was analysed exponentially often within the depth limit. Forms are now remembered with the resources owner they were drawn against, and a repeat records nothing new. --- crates/organize/src/prune.rs | 11 ++++++++++- crates/organize/src/tests.rs | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/crates/organize/src/prune.rs b/crates/organize/src/prune.rs index b5f5e65ffe..9b78a774bc 100644 --- a/crates/organize/src/prune.rs +++ b/crates/organize/src/prune.rs @@ -29,7 +29,7 @@ enum XKey { } /// Who owns the resources currently being walked, for the key drawn names are recorded under. -#[derive(Clone, Copy)] +#[derive(Clone, Copy, PartialEq, Eq, Hash)] enum Owner { /// An indirect resources object (a page's, or a form XObject's own). Resources(ObjRef), @@ -50,6 +50,9 @@ struct Pruner<'a> { glyph_seen: HashSet, /// An indirect font's explicit resources have one owner regardless of which page uses it. glyph_fonts: HashSet, + /// Forms already analysed, with the owner of the resources they drew against: drawing one + /// again records nothing new, and re-reading it on every `Do` is exponential in the nesting. + forms_seen: HashSet<(ObjRef, Owner)>, } impl Pruner<'_> { @@ -211,6 +214,11 @@ impl Pruner<'_> { // A form without its own resources draws against the ones it was invoked with. None => (resources.clone(), owner), }; + if !self.forms_seen.insert((r, inner_owner)) { + continue; + } + #[cfg(test)] + crate::work(1); match s.decoded() { Ok(data) => self.content(&data, &inner, inner_owner, depth + 1), // What the form draws is unknown: keep everything it could reach. @@ -314,6 +322,7 @@ pub(crate) fn prune_unused_xobjects(doc: &mut Document) -> Result = vec![ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >>".into(), + "<< /Type /Page /Parent 2 0 R /Contents 4 0 R /Resources << /XObject << /F 5 0 R /Unused 6 0 R >> >> >>".into(), + "<< /Length 5 >>\nstream\n/F Do\nendstream".into(), + format!( + "<< /Type /XObject /Subtype /Form /BBox [0 0 10 10] /Resources << /XObject << /F 5 0 R >> >> /Length {} >>\nstream\n{form}\nendstream", + form.len() + ), + "<< /Type /XObject /Subtype /Form /BBox [0 0 10 10] /Length 0 >>\nstream\n\nendstream".into(), + ]; + let src = open(build(&b, "/Root 1 0 R")); + take_work(); + let out = extract_pages(&src, &[0]).unwrap(); + let work = take_work(); + assert!(work <= 4, "{work} form analyses"); + let page = out.get(pages(&out).unwrap()[0].obj).as_dict().cloned().unwrap(); + let res = out.resolve(page.get(b"Resources").unwrap()).as_dict().cloned().unwrap(); + let x = out.resolve(res.get(b"XObject").unwrap()).as_dict().cloned().unwrap(); + assert!(x.contains(b"F") && !x.contains(b"Unused"), "pruning still keeps what is drawn and drops the rest"); +} From cb1ddc899bfe475b077500cdc7a0593db2e69be1 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:26:10 +0000 Subject: [PATCH 06/22] fix(organize): find XMP attributes in one pass over the packet Each candidate attribute match searched the whole packet before it for the last < and >, so a long text node repeating the attribute name cost time quadratic in the packet size (up to the 16 MiB decode cap). The scan now keeps the last < and > as it moves forward. --- crates/organize/src/xmp.rs | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/crates/organize/src/xmp.rs b/crates/organize/src/xmp.rs index bbcbbcf83f..cc3a78ccf4 100644 --- a/crates/organize/src/xmp.rs +++ b/crates/organize/src/xmp.rs @@ -172,11 +172,24 @@ fn element_span(xmp: &str, name: &str) -> Option<(usize, usize)> { /// start and end of its value, and its quote character. Only matches inside a start tag. fn attribute_span(xmp: &str, name: &str) -> Option<(usize, usize, usize, char)> { let mut from = 0; + // The last `<` and `>` before the match, found by one pass over the packet rather than by + // searching back from every match. + let (mut scanned, mut last_open, mut last_close) = (0, None, None); while let Some(i) = xmp.get(from..).and_then(|s| s.find(name)).map(|i| i + from) { from = i + name.len(); let before = xmp.get(..i)?; + let gap = xmp.get(scanned..i)?; + #[cfg(test)] + crate::work(gap.len()); + if let Some(at) = gap.rfind('<') { + last_open = Some(scanned + at); + } + if let Some(at) = gap.rfind('>') { + last_close = Some(scanned + at); + } + scanned = i; // Preceded by whitespace, and inside a tag (the last `<` comes after the last `>`). - if !before.ends_with(char::is_whitespace) || before.rfind('<') < before.rfind('>') || before.rfind('<').is_none() { + if !before.ends_with(char::is_whitespace) || last_open < last_close || last_open.is_none() { continue; } let rest = xmp.get(from..)?; @@ -269,4 +282,18 @@ mod tests { } } } + + #[test] + fn attribute_search_scans_the_packet_once() { + // Thousands of attribute-like words in one text node: none is inside a tag. + let packet = format!("{}", " rdf:about".repeat(5_000)); + crate::take_work(); + assert_eq!(attribute_span(&packet, "rdf:about"), None); + let work = crate::take_work(); + assert!(work <= 2 * packet.len(), "{work} bytes scanned in a {}-byte packet", packet.len()); + // A real attribute after them is still found. + let packet = packet.replace("", ""); + let (_, s, e, q) = attribute_span(&packet, "rdf:about").unwrap(); + assert_eq!((packet.get(s..e), q), (Some("u"), '\'')); + } } From f1a89a48423a896511e6709bcf4dad8a213ff36e Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:34:28 +0000 Subject: [PATCH 07/22] fix(edit): wrap added text once per write, not once per line Writing an added text item re-wrapped the whole text inside the loop over its lines to learn the line count, so multiline text cost time and temporary memory quadratic in its length. The wrapped lines are computed once. --- crates/edit/src/added.rs | 12 ++++++++++-- crates/edit/src/tests.rs | 11 +++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/crates/edit/src/added.rs b/crates/edit/src/added.rs index d246df725b..671a72a1b8 100644 --- a/crates/edit/src/added.rs +++ b/crates/edit/src/added.rs @@ -235,9 +235,17 @@ pub fn lines(t: &AddedText) -> Vec { wrapped(t).into_iter().map(|(line, _)| line).collect() } +#[cfg(test)] +thread_local! { + /// How often [`wrapped`] laid a text out on this thread, for the test that keeps it out of per-line loops. + pub(crate) static WRAP_CALLS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// [`lines`], each with whether its paragraph runs right to left (its first strong character /// does). fn wrapped(t: &AddedText) -> Vec<(String, bool)> { + #[cfg(test)] + WRAP_CALLS.with(|c| c.set(c.get().saturating_add(1))); let width = (t.rect[2] - t.rect[0]).max(t.size); let mut out = Vec::new(); for para in t.text.split('\n') { @@ -780,7 +788,8 @@ fn draw(doc: &mut Document, c: &Content, view: [f64; 6], taken: &Dict, existing: let r = text_rect(t); let [cr, cg, cb] = t.color.map(|v| v.clamp(0.0, 1.0)); out.extend(format!("BT /{name} {} Tf {} {} {} rg\n", n(t.size), n(cr), n(cg), n(cb)).bytes()); - for (i, line) in lines(t).iter().enumerate() { + let all = lines(t); + for (i, line) in all.iter().enumerate() { let w = t.family.width(line, t.size, t.bold, t.italic); let x = match t.align { Align::Left | Align::Justify => r[0], @@ -790,7 +799,6 @@ fn draw(doc: &mut Document, c: &Content, view: [f64; 6], taken: &Dict, existing: // Baseline: 0.8 em below the line top. let y = r[3] - (i as f64 * 1.2 + 0.95) * t.size; // Justified: word spacing makes every line but the last fill the box. - let all = lines(t); let spaces = line.matches(' ').count(); let tw = if t.align == Align::Justify && i + 1 < all.len() && spaces > 0 { ((r[2] - r[0]) - w).max(0.0) / spaces as f64 } else { 0.0 }; diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index cf3860237b..73b818be4b 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -6355,3 +6355,14 @@ fn reading_keeps_dotted_leader_entries_whole_and_in_order() { ] ); } + +#[test] +fn added_text_is_wrapped_once_per_write() { + let mut doc = fixture(); + let lines: Vec = (0..40).map(|i| format!("line {i}")).collect(); + let text = AddedText { rect: [72.0, 100.0, 300.0, 700.0], text: lines.join("\n"), size: 10.0, align: Align::Justify, ..AddedText::default() }; + added::WRAP_CALLS.with(|c| c.set(0)); + add_content(&mut doc, 0, &Content::Text(text)).unwrap(); + let calls = added::WRAP_CALLS.with(std::cell::Cell::get); + assert!(calls <= 4, "the text was wrapped {calls} times for 40 lines"); +} From dc7a195dda67813d9c99958d4b602638be51207e Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:34:28 +0000 Subject: [PATCH 08/22] fix(edit): prune each form-field node once after flattening Removing flattened widgets from the field tree recursed into every /Kids entry with only a depth limit, so a node listing its child twice per level (or itself) made the walk exponential. Each node's answer is now remembered; a node met again while it is still being visited survives, as it did when the depth limit stopped a cycle. --- crates/edit/src/flatten.rs | 37 +++++++++++++++++++++++++++++++------ crates/edit/src/tests.rs | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 6 deletions(-) diff --git a/crates/edit/src/flatten.rs b/crates/edit/src/flatten.rs index 3ac5468057..1be77abef7 100644 --- a/crates/edit/src/flatten.rs +++ b/crates/edit/src/flatten.rs @@ -9,7 +9,7 @@ //! form's field tree. An annotation whose appearance cannot be drawn is not flattened: it stays //! as it is, so no comment or field value is lost with nothing in its place. -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use pdfcraft_cos::{Dict, Document, ObjRef, Object}; @@ -295,6 +295,12 @@ fn flatten_which(doc: &mut Document, pages: &[usize], which: Which) -> Result = const { std::cell::Cell::new(0) }; +} + /// Remove flattened widgets from the AcroForm field tree, and fields left without widgets. With /// `on_page`, also the widgets (fields without kids) that are on no page. fn prune_fields(doc: &mut Document, widgets: &HashSet, on_page: Option<&HashSet>) -> Result<(), EditError> { @@ -303,23 +309,42 @@ fn prune_fields(doc: &mut Document, widgets: &HashSet, on_page: Option<& let af_ref = af.as_ref(); let mut af_dict = doc.resolve(&af).as_dict().cloned().unwrap_or_default(); let fields = af_dict.get(b"Fields").map(|f| doc.resolve(f)).and_then(|f| f.as_array().cloned()).unwrap_or_default(); - // Returns whether the node survives. - fn keep(doc: &mut Document, r: ObjRef, widgets: &HashSet, on_page: Option<&HashSet>, depth: usize) -> bool { + // Returns whether the node survives. `known` holds each node's answer once it is visited, so + // a node listed twice is pruned once, and one still being visited (a cycle) survives. + fn keep( + doc: &mut Document, + r: ObjRef, + widgets: &HashSet, + on_page: Option<&HashSet>, + known: &mut HashMap, + depth: usize, + ) -> bool { + #[cfg(test)] + KEEP_CALLS.with(|c| c.set(c.get().saturating_add(1))); if widgets.contains(&r) || depth > 64 { return depth > 64; } + if let Some(&survives) = known.get(&r) { + return survives; + } + known.insert(r, true); let Some(d) = doc.get(r).as_dict().cloned() else { return true }; let Some(kids) = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()) else { - return on_page.is_none_or(|p| p.contains(&r)); + let survives = on_page.is_none_or(|p| p.contains(&r)); + known.insert(r, survives); + return survives; }; let survivors: Vec = - kids.iter().filter(|k| k.as_ref().is_none_or(|kr| keep(doc, kr, widgets, on_page, depth + 1))).cloned().collect(); + kids.iter().filter(|k| k.as_ref().is_none_or(|kr| keep(doc, kr, widgets, on_page, known, depth + 1))).cloned().collect(); if survivors.len() != kids.len() { let _ = doc.update_dict(r, |d| d.set(b"Kids".to_vec(), Object::Array(survivors.clone()))); } + known.insert(r, !survivors.is_empty()); !survivors.is_empty() } - let survivors: Vec = fields.iter().filter(|f| f.as_ref().is_none_or(|r| keep(doc, r, widgets, on_page, 0))).cloned().collect(); + let mut known = HashMap::new(); + let survivors: Vec = + fields.iter().filter(|f| f.as_ref().is_none_or(|r| keep(doc, r, widgets, on_page, &mut known, 0))).cloned().collect(); if survivors.len() == fields.len() && widgets.is_empty() { return Ok(()); // only looked for fields on no page, and there were none } diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 73b818be4b..905c3f4801 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -6366,3 +6366,39 @@ fn added_text_is_wrapped_once_per_write() { let calls = added::WRAP_CALLS.with(std::cell::Cell::get); assert!(calls <= 4, "the text was wrapped {calls} times for 40 lines"); } + +/// A field tree whose every node lists its child twice is pruned in work linear in its nodes, not +/// in its paths, and a node listing itself is visited once. +#[test] +fn flattening_prunes_a_field_tree_with_repeated_kids_once_per_node() { + let levels = 16; + let page = stream("", " "); + // 5: the flattened widget; 6…: the chain; then the hidden widget at its end, on the page. + let leaf = 6 + levels; + let fields = format!("<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [5 0 R 6 0 R {} 0 R] >> >>", leaf + 2); + let annots = format!("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << >> /Contents 4 0 R /Annots [5 0 R {leaf} 0 R] >>"); + let mut objs: Vec = vec![ + fields, + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into(), + annots, + page, + format!("<< /Type /Annot /Subtype /Widget /FT /Tx /T (shown) /Rect [10 10 150 40] /F 4 /AP << /N {} 0 R >> >>", leaf + 1), + ]; + for i in 0..levels { + let next = 7 + i; + objs.push(format!("<< /T (n{i}) /Kids [{next} 0 R {next} 0 R] >>")); + } + objs.push("<< /Type /Annot /Subtype /Widget /FT /Tx /T (hidden) /Rect [10 90 150 120] /F 2 >>".into()); + objs.push("<< /Type /XObject /Subtype /Form /BBox [0 0 140 30] /Length 0 >>\nstream\n\nendstream".into()); + objs.push(format!("<< /T (loop) /Kids [{} 0 R] >>", leaf + 2)); + let objs: Vec<&str> = objs.iter().map(String::as_str).collect(); + let mut doc = build(&objs); + flatten::KEEP_CALLS.with(|c| c.set(0)); + assert_eq!(flatten(&mut doc, &[0], false, true).unwrap(), 1); + let calls = flatten::KEEP_CALLS.with(std::cell::Cell::get); + assert!(calls <= 2 * (levels + 4), "{calls} field-tree visits"); + let root = doc.root().unwrap(); + let af = doc.resolve(doc.get(root).as_dict().unwrap().get(b"AcroForm").unwrap()); + let left = doc.resolve(af.as_dict().unwrap().get(b"Fields").unwrap()).as_array().unwrap().len(); + assert_eq!(left, 2, "the chain to the hidden widget and the self-listing node stay"); +} From c9090b00ef17d9722c6a50530f0376cd9dc31faa Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:34:28 +0000 Subject: [PATCH 09/22] fix(edit): refuse to put content on top when marked-content depth is unknown Past the marked-content depth budget the number of open sequences is unknown, and the mark on top was appended with none of them closed, so the page's own optional-content scopes (possibly hidden) could hide a redaction box, a header or a watermark. Placing on top of such a page is now refused before anything changes; behind it is still possible. --- crates/edit/src/lib.rs | 18 ++++++++++++++---- crates/edit/src/marked_isolation_tests.rs | 15 +++++++++++++-- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/crates/edit/src/lib.rs b/crates/edit/src/lib.rs index bef515f981..af557d7624 100644 --- a/crates/edit/src/lib.rs +++ b/crates/edit/src/lib.rs @@ -492,6 +492,9 @@ enum Isolation { /// the count of `q` left open at the end, when known. `marked` counts known unfinished /// marked-content sequences independently of graphics malformation, or zero when unknown. Unknown { unclosed: usize, marked: usize }, + /// Marked content nests past [`ISOLATION_MAX_DEPTH`]: how many sequences a mark on top would + /// have to close (some may be hidden optional content) is unknown. + TooDeep, } const ISOLATION_MAX_STREAMS: usize = 100_000; @@ -591,7 +594,10 @@ fn isolation(doc: &Document, list: &[Object]) -> Isolation { _ => {} } } - if malformed || text_open || path_open || !stack.is_empty() || !marked_known { + if !marked_known { + return Isolation::TooDeep; + } + if malformed || text_open || path_open || !stack.is_empty() { return Isolation::Unknown { unclosed: if malformed { 0 } else { stack.len() }, marked }; } let comment_open = edges.last().is_some_and(|edge| edge.1); @@ -608,8 +614,8 @@ fn place(doc: &mut Document, page: &pdfcraft_model::Page, kind: MarkKind, conten /// content before leaves set (a `cm`, a clip, text state…) must not reach the mark, including /// content appended after an earlier PdfCraft wrapper by another tool (#3084): the stretch of /// content that leaves state set is wrapped in q/Q, once. Any known unfinished marked-content -/// sequences are closed at the end too, so hidden optional content cannot hide the new mark. -/// Returns the new content stream. +/// sequences are closed at the end too, so hidden optional content cannot hide the new mark; when +/// their number is unknown, nothing is placed on top. Returns the new content stream. fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: &str, content: Vec, behind: bool) -> Result { let mut list = contents(doc, page)?; // The behind-marks at the start: backgrounds in the order added, then watermarks. @@ -624,6 +630,10 @@ fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: &str, cont // behind-marks at the start. A `q` it leaves open is closed at its end, where // closing it changes nothing already drawn. Isolation::Unknown { unclosed, marked } => (behind_marks < list.len()).then_some((behind_marks, unclosed, marked)), + // Content left inside an optional-content sequence it cannot close could be hidden. + Isolation::TooDeep => { + return Err(EditError::Invalid("the page's marked content is nested too deeply to add anything on top of it".into())); + } } }; let mark_ref = doc.add(Object::Stream(Stream::flate(tagged(tag), &content))); @@ -941,7 +951,7 @@ pub fn remove_marks(doc: &mut Document, pages: &[usize], kind: MarkKind) -> Resu if before != kept.len() { let wrappers = match isolation(doc, &kept) { Isolation::Clean { wrappers, .. } => wrappers, - Isolation::Unknown { .. } => { + Isolation::Unknown { .. } | Isolation::TooDeep => { let mut opens = Vec::new(); let mut pairs = Vec::new(); for (i, o) in kept.iter().enumerate() { diff --git a/crates/edit/src/marked_isolation_tests.rs b/crates/edit/src/marked_isolation_tests.rs index 6da8322296..571bd9c242 100644 --- a/crates/edit/src/marked_isolation_tests.rs +++ b/crates/edit/src/marked_isolation_tests.rs @@ -96,7 +96,7 @@ fn recognizes_only_canonical_new_closes_and_retains_legacy_closes() { } #[test] -fn depth_budget_bounds_emitted_closures_and_overflow_stays_unknown() { +fn depth_budget_bounds_emitted_closures_and_overflow_is_refused_on_top() { let maximum = b"/Artifact BMC\n".repeat(ISOLATION_MAX_DEPTH); let mut expected = b"\nEMC".repeat(ISOLATION_MAX_DEPTH); expected.extend_from_slice(WRAP_CLOSE); @@ -114,9 +114,20 @@ fn depth_budget_bounds_emitted_closures_and_overflow_stays_unknown() { place_tagged(&mut doc, &page, MarkKind::HeaderFooter.tag(), b"q Q\n".to_vec(), false).unwrap(); assert_eq!(remove_marks(&mut doc, &[0], MarkKind::HeaderFooter).unwrap(), 1); assert_eq!(contents(&mut doc, &page).unwrap(), original, "maximum combined close can be removed without nesting wrappers"); + // Past the budget the depth is unknown, so nothing can close the page's (possibly hidden) + // optional content before a mark on top: placing one there is refused, and the page is left + // as it was. Behind the content stays possible. for suffix in [b"/Artifact BMC\n".as_slice(), b"/Artifact BMC\nEMC\n", b"/Artifact BMC\nEMC\nEMC\n"] { let mut over = maximum.clone(); over.extend_from_slice(suffix); - assert_eq!(close(&[&over]), WRAP_CLOSE, "overflow cannot resume speculative depth tracking after EMC"); + let mut doc = fixture(&[&over]); + let page = page_list(&doc).remove(0); + let original = contents(&mut doc, &page).unwrap(); + assert!( + matches!(stamp(&mut doc, 0, "Probe", b"q Q\n".to_vec()), Err(EditError::Invalid(_))), + "overflow cannot resume speculative depth tracking after EMC" + ); + assert_eq!(contents(&mut doc, &page).unwrap(), original); + place_tagged(&mut doc, &page, MarkKind::Background.tag(), b"q Q\n".to_vec(), true).unwrap(); } } From fdcfe16b7d7f11020449c40f8e1840d120a3d6d6 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:34:28 +0000 Subject: [PATCH 10/22] fix(edit): bound the text one page's extraction can expand into Text extraction copies a ToUnicode destination for every glyph showing its code, and the page's total text was not bounded, so a small page could expand into gigabytes. One page's extraction (its forms included) stops with an error past 16 MiB of text. --- crates/edit/src/tests.rs | 22 ++++++++++++++++++++++ crates/edit/src/text.rs | 27 +++++++++++++++++++++------ 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 905c3f4801..291f3e26eb 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -6402,3 +6402,25 @@ fn flattening_prunes_a_field_tree_with_repeated_kids_once_per_node() { let left = doc.resolve(af.as_dict().unwrap().get(b"Fields").unwrap()).as_array().unwrap().len(); assert_eq!(left, 2, "the chain to the hidden widget and the self-listing node stay"); } + +/// A ToUnicode destination is repeated for every glyph that shows its code: a page whose text +/// would run past the extraction budget is refused, not built in memory. +#[test] +fn page_text_past_the_budget_is_refused() { + let cmap = stream("", &format!("1 beginbfchar <41> <{}> endbfchar", "0041".repeat(256))); + let page = |shown: &str| { + let content = stream("", &format!("BT /F1 12 Tf 10 10 Td ({shown}) Tj ET")); + build(&[ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << /Font << /F1 5 0 R >> >> /Contents 4 0 R >>", + &content, + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /ToUnicode 6 0 R >>", + &cmap, + ]) + }; + let doc = page(&"A".repeat(100_000)); + assert!(matches!(text_lines(&doc, 0), Err(EditError::Invalid(_))), "25 MB of text from a 100 kB page"); + let doc = page("AA"); + assert_eq!(text_lines(&doc, 0).unwrap()[0].text.len(), 512, "a page within the budget still reads"); +} diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index 93a5eec99e..27e8aa5f1b 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -887,8 +887,15 @@ struct Carry { marked: Vec>, /// For each open marked-content sequence, its optional content group when it is `/OC`. layers: Vec>>, + /// Bytes of glyph text the rest of the page may still produce (see [`MAX_PAGE_TEXT`]). + text_left: usize, } +/// The most glyph text one page's extraction produces, in bytes. A ToUnicode destination is +/// copied for every glyph showing its code, so a small page can otherwise expand into gigabytes; +/// real pages hold far less (a whole novel is about 1 MB). +const MAX_PAGE_TEXT: usize = 16 << 20; + impl Carry { fn new() -> Self { let ts = Ts { @@ -907,7 +914,7 @@ impl Carry { vertical: None, unreadable: None, }; - Carry { ts, stack: Vec::new(), marked: Vec::new(), layers: Vec::new() } + Carry { ts, stack: Vec::new(), marked: Vec::new(), layers: Vec::new(), text_left: MAX_PAGE_TEXT } } } @@ -1119,7 +1126,13 @@ fn interpret( let blank = m.is_space(code, len) || shown.as_deref().is_some_and(|t| !t.is_empty() && t.chars().all(char::is_whitespace)); match shown { - Some(t) => text.push_str(&t), + Some(t) => { + carry.text_left = carry + .text_left + .checked_sub(t.len()) + .ok_or_else(|| EditError::Invalid("the page's text is too large to read".into()))?; + text.push_str(&t); + } None => decodable = false, } let spacing = ts.char_spacing + if m.is_space(code, len) { ts.word_spacing } else { 0.0 }; @@ -1578,7 +1591,7 @@ fn reading_lines(doc: &Document, page: usize) -> Result<(Vec, bool), E let mut forms = invoked.into_iter().peekable(); for s in shown { while let Some((_, name, ts)) = forms.next_if(|f| f.0 < s.op) { - enter(w, resources, &name, ts)?; + enter(w, resources, &name, ts, &mut carry.text_left)?; // Text after a form starts a new line. last = None; } @@ -1590,21 +1603,23 @@ fn reading_lines(doc: &Document, page: usize) -> Result<(Vec, bool), E add_shown(&mut w.lines, &mut last, s, si); } for (_, name, ts) in forms { - enter(w, resources, &name, ts)?; + enter(w, resources, &name, ts, &mut carry.text_left)?; } Ok(()) } - fn enter(w: &mut Walk, resources: &Dict, name: &[u8], ts: Ts) -> Result<(), EditError> { + /// `text_left`: the page's text budget, shared with the forms the page draws. + fn enter(w: &mut Walk, resources: &Dict, name: &[u8], ts: Ts, text_left: &mut usize) -> Result<(), EditError> { let Some(form) = form_call(w.doc, resources, name, ts.ctm, &w.path, &mut w.budget) else { return Ok(()) }; let ops = parse(&form.data).ops; // The form starts with the graphics state at its `Do`, its own matrix applied. - let mut carry = Carry { ts: Ts { ctm: form.ctm, ..ts }, stack: Vec::new(), marked: Vec::new(), layers: Vec::new() }; + let mut carry = Carry { ts: Ts { ctm: form.ctm, ..ts }, stack: Vec::new(), marked: Vec::new(), layers: Vec::new(), text_left: *text_left }; let stream = w.next_stream; w.next_stream = w.next_stream.saturating_add(1); if let Some(r) = form.obj { w.path.push(r); } walk(w, &ops, &form.resources, &mut carry, &|_| stream)?; + *text_left = carry.text_left; if form.obj.is_some() { w.path.pop(); } From b979d4d9c23ee19f507c0e35b5f4c62e7b037b7a Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:34:28 +0000 Subject: [PATCH 11/22] fix(audit): audit the glyphs on each page held, not their ActualText The redaction audit read each page's text through text_lines, which walked the whole page tree again for every page (quadratic in the page count) and let /ActualText replace the glyphs, so an empty or substitute ActualText hid covered text from the audit. Edit gains glyph_text_lines, which reads the page dictionary it is given and ignores ActualText; the audit uses it. --- crates/audit/src/lib.rs | 4 +++- crates/audit/src/tests.rs | 13 +++++++++++++ crates/edit/src/lib.rs | 2 +- crates/edit/src/text.rs | 16 ++++++++++++++-- 4 files changed, 31 insertions(+), 4 deletions(-) diff --git a/crates/audit/src/lib.rs b/crates/audit/src/lib.rs index 365889d61a..cb6e7e7f72 100644 --- a/crates/audit/src/lib.rs +++ b/crates/audit/src/lib.rs @@ -99,7 +99,9 @@ const COVER_CONTRAST: f64 = 3.0; pub fn audit_redactions(doc: &Document) -> Result, AuditError> { let mut out = Vec::new(); for (n, page) in pdfcraft_model::pages(doc).iter().enumerate() { - let lines = pdfcraft_edit::text_lines(doc, n).map_err(|e| AuditError::Text(n, e.to_string()))?; + // The glyphs themselves: `/ActualText` is the file's own claim about them, and an empty + // or substitute one must not hide text a cover sits on. + let lines = pdfcraft_edit::glyph_text_lines(doc, page).map_err(|e| AuditError::Text(n, e.to_string()))?; // 1. Opaque filled rectangles over extractable text. for r in cover::filled_rects(doc, &page.dict)? { let mut covered = String::new(); diff --git a/crates/audit/src/tests.rs b/crates/audit/src/tests.rs index 4ca3b0bf5b..4c36d2821b 100644 --- a/crates/audit/src/tests.rs +++ b/crates/audit/src/tests.rs @@ -592,3 +592,16 @@ fn reading_blocks_respect_stream_comments_and_split_operands() { assert_eq!(text, "Replacement text", "{}", String::from_utf8_lossy(s1)); } } + +/// `/ActualText` is metadata the file's author controls: an empty or substitute replacement must +/// not hide the glyphs a cover sits on. +#[test] +fn a_cover_over_text_with_empty_or_substitute_actual_text_is_found() { + for actual in ["()", "(nothing here)"] { + let content = + format!("/Span << /ActualText {actual} >> BDC\nBT /F1 24 Tf 100 700 Td (SECRET DATA 123) Tj ET\nEMC\n0 g\n100 688 220 32 re f\n"); + let findings = audit_redactions(&open(one_page(content.as_bytes(), ""))).expect("audit must run"); + assert_eq!(findings.len(), 1, "{actual}: {findings:?}"); + assert!(findings[0].covered_text.contains("SECRET"), "{actual}: covered_text was {:?}", findings[0].covered_text); + } +} diff --git a/crates/edit/src/lib.rs b/crates/edit/src/lib.rs index af557d7624..d6861fc0ae 100644 --- a/crates/edit/src/lib.rs +++ b/crates/edit/src/lib.rs @@ -1007,7 +1007,7 @@ mod linebreak; pub mod text; pub use text::{ BlockKey, BlockStyle, LineEdit, MAX_FORM_DEPTH, MAX_FORM_VISITS, RangeStyle, ReadingBlocks, TextBlock, TextLine, TextSession, block_overlapping, - clear_block, reading_blocks, reading_blocks_checked, replace_block, replace_line, rewrite_block, text_blocks, text_lines, + clear_block, glyph_text_lines, reading_blocks, reading_blocks_checked, replace_block, replace_line, rewrite_block, text_blocks, text_lines, }; pub mod added; pub use added::{ diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index 27e8aa5f1b..99e6a3e0f1 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -1290,7 +1290,19 @@ fn interpret( /// The lines of text on a page (0-based), in content order. pub fn text_lines(doc: &Document, page: usize) -> Result, EditError> { - let p = page_dict(doc, page)?; + text_lines_in(doc, &page_dict(doc, page)?, true) +} + +/// The lines `page`'s glyphs show, in content order, without `/ActualText` replacements: what the +/// page draws, not what its marked-content metadata says the glyphs mean. For checks that must +/// not trust that metadata (the redaction audit). Takes the page itself, so a caller going +/// through every page doesn't walk the page tree again for each. +pub fn glyph_text_lines(doc: &Document, page: &pdfcraft_model::Page) -> Result, EditError> { + text_lines_in(doc, page, false) +} + +/// [`text_lines`] of `p`; with `actual_text`, `/ActualText` replaces the glyphs it spans. +fn text_lines_in(doc: &Document, p: &pdfcraft_model::Page, actual_text: bool) -> Result, EditError> { let res = p.dict.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned()).unwrap_or_default(); let mut cache = HashMap::new(); let mut lines: Vec = Vec::new(); @@ -1300,7 +1312,7 @@ pub fn text_lines(doc: &Document, page: usize) -> Result, EditErro let pieces = Pieces::join(&content_streams(doc, &p.dict).into_iter().map(|(_, d)| d).collect::>()); let ops = pieces.parse().ops; let properties = res.get(b"Properties").and_then(|o| doc.dict(o)).unwrap_or_default(); - let shown = interpret(doc, &ops, &res, &mut cache, &mut carry, None, Some(&properties))?; + let shown = interpret(doc, &ops, &res, &mut cache, &mut carry, None, actual_text.then_some(&properties))?; let mut last: Option<(usize, Pen)> = None; // the stream piece and where its last run ended for mut s in shown { // A single RTL glyph's ActualText (e.g. lam-alef) is an inline ligature, not a From 7e9f5a4435bc6d90e85d3dbff0157788fdf070b1 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:37:59 +0000 Subject: [PATCH 12/22] fix(create): bound a TIFF's decoded pages together, and read one when one is wanted A TIFF's pages were each capped at 2 GiB but all kept in memory until embedded, so many small directories sharing one compressed strip could ask for many times that; embedding one image also decoded every page. The pages of one TIFF are now refused past 4 GiB together, and image_xobject reads only the first page. --- crates/create/src/lib.rs | 31 +++++++++++++++++++++++-------- crates/create/src/tests.rs | 25 +++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/crates/create/src/lib.rs b/crates/create/src/lib.rs index 5717977b1e..6e1fdaee69 100644 --- a/crates/create/src/lib.rs +++ b/crates/create/src/lib.rs @@ -402,8 +402,17 @@ fn decoded(name: &str, bytes: &[u8], format: image::ImageFormat) -> Result Result, CreateError> { +/// Most decoded bytes all the pages of one TIFF may take together: its pages stay in memory until +/// they are embedded, and many small directories can share one compressed strip. +const MAX_DECODED_TOTAL: usize = 2 * MAX_DECODED_BYTES; + +/// The first `wanted` pages of a TIFF (multi-page scans become multi-page PDFs). +fn tiff_pages(name: &str, bytes: &[u8], wanted: usize) -> Result, CreateError> { + tiff_pages_within(name, bytes, wanted, MAX_DECODED_TOTAL) +} + +/// [`tiff_pages`], refused once the pages kept take more than `budget` bytes. +fn tiff_pages_within(name: &str, bytes: &[u8], wanted: usize, budget: usize) -> Result, CreateError> { use tiff::ColorType as C; use tiff::decoder::{Decoder, DecodingResult, Limits}; use tiff::tags::Tag; @@ -415,6 +424,7 @@ fn tiff_pages(name: &str, bytes: &[u8]) -> Result, CreateError> { limits.decoding_buffer_size = MAX_DECODED_BYTES; let mut dec = Decoder::new(std::io::Cursor::new(bytes)).map_err(|e| bad(e.to_string()))?.with_limits(limits); let mut out = Vec::new(); + let mut kept = 0usize; loop { let (w, h) = dec.dimensions().map_err(|e| bad(e.to_string()))?; let ct = dec.colortype().map_err(|e| bad(e.to_string()))?; @@ -469,8 +479,12 @@ fn tiff_pages(name: &str, bytes: &[u8]) -> Result, CreateError> { } other => return Err(bad(format!("{other:?} TIFF images aren't supported yet"))), }; + kept = kept.saturating_add(page.data.len()).saturating_add(page.smask.as_ref().map_or(0, |(_, alpha)| alpha.len())); + if kept > budget { + return Err(bad(format!("the TIFF's pages take more than {} MiB together", budget >> 20))); + } out.push(page); - if !dec.more_images() { + if out.len() >= wanted || !dec.more_images() { break; } dec.next_image().map_err(|e| bad(e.to_string()))?; @@ -526,14 +540,15 @@ pub fn source_kind(name: &str, bytes: &[u8]) -> Option { (lower.ends_with(".txt") || lower.ends_with(".text")).then_some(SourceKind::Text) } -/// Detect the image format from its bytes; a TIFF may hold several pages. -fn embed(name: &str, bytes: &[u8]) -> Result, CreateError> { +/// Detect the image format from its bytes; a TIFF may hold several pages, of which the first +/// `wanted` are read. +fn embed(name: &str, bytes: &[u8], wanted: usize) -> Result, CreateError> { if bytes.starts_with(&[0xFF, 0xD8]) { Ok(vec![jpeg(name, &upright_image::upright(bytes))?]) } else if bytes.starts_with(b"\x89PNG\r\n\x1a\n") { Ok(vec![png_image(name, bytes)?]) } else if is_tiff(bytes) { - tiff_pages(name, bytes) + tiff_pages(name, bytes, wanted) } else if bytes.starts_with(b"BM") { Ok(vec![decoded(name, bytes, image::ImageFormat::Bmp)?]) } else if is_gif(bytes) { @@ -548,7 +563,7 @@ fn embed(name: &str, bytes: &[u8]) -> Result, CreateError> { /// Embed an image file (its first page, for TIFFs) as an image XObject in `doc`. Returns the /// object and the image's natural size in points (from its resolution). pub fn image_xobject(doc: &mut Document, name: &str, bytes: &[u8]) -> Result<(ObjRef, (f64, f64)), CreateError> { - let img = embed(name, bytes)?.into_iter().next().ok_or_else(|| CreateError::Image(name.into(), "the file has no image".into()))?; + let img = embed(name, bytes, 1)?.into_iter().next().ok_or_else(|| CreateError::Image(name.into(), "the file has no image".into()))?; let size = (img.px.0 as f64 * 72.0 / img.dpi.0, img.px.1 as f64 * 72.0 / img.dpi.1); Ok((add_image(doc, img, &mut Vec::new()), size)) } @@ -580,7 +595,7 @@ pub fn from_images_with_resolution(images: &[(String, Vec)], resolution: Ima } let mut doc = Document::new_empty(); let mut profiles = Vec::new(); - for img in images.iter().map(|(name, bytes)| embed(name, bytes)).collect::, _>>()?.into_iter().flatten() { + for img in images.iter().map(|(name, bytes)| embed(name, bytes, usize::MAX)).collect::, _>>()?.into_iter().flatten() { let dpi = match resolution { ImageResolution::Embedded => img.dpi, ImageResolution::Dpi(dpi) => (dpi, dpi), diff --git a/crates/create/src/tests.rs b/crates/create/src/tests.rs index a214977675..a352ecbf9a 100644 --- a/crates/create/src/tests.rs +++ b/crates/create/src/tests.rs @@ -1197,3 +1197,28 @@ fn a_cyclic_indexed_colour_space_is_refused() { assert!(out.images.is_empty()); assert_eq!(out.skipped.len(), 1, "{:?}", out.skipped); } + +/// A TIFF of `pages` 64×64 gray pages, then (with `unreadable`) one whose samples Create can't read. +fn gray_tiff(pages: usize, unreadable: bool) -> Vec { + let mut tif = std::io::Cursor::new(Vec::new()); + let mut enc = tiff::encoder::TiffEncoder::new(&mut tif).unwrap(); + for _ in 0..pages { + enc.write_image::(64, 64, &[128; 64 * 64]).unwrap(); + } + if unreadable { + enc.write_image::(1, 1, &[0.5; 3]).unwrap(); + } + tif.into_inner() +} + +/// Many small IFDs can each decode to a large page: the pages a TIFF keeps in memory together +/// are bounded, and embedding one image reads only its first page. +#[test] +fn tiff_pages_are_decoded_within_a_total_budget() { + let tif = gray_tiff(3, false); + assert!(matches!(tiff_pages_within("t.tif", &tif, usize::MAX, 10_000), Err(CreateError::Image(..))), "12 KiB of pages past a 10 KB budget"); + assert_eq!(tiff_pages_within("t.tif", &tif, usize::MAX, 20_000).unwrap().len(), 3); + let tif = gray_tiff(1, true); + let mut doc = Document::new_empty(); + assert!(image_xobject(&mut doc, "t.tif", &tif).is_ok(), "the unreadable last page is never decoded"); +} From cad0fe4526ee6af664ffb47d0e1cd47b1ef173de Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:37:59 +0000 Subject: [PATCH 13/22] fix(create): refuse a GIF screen past 4096x4096 and convert its canvas in place A one-pixel GIF on a huge logical screen decodes to a canvas that size, which the conversion then copied before splitting it into colour and alpha, so a few bytes could hold over a gigabyte. Screens past 16 Mpx are refused before decoding, and the decoded canvas is converted without a copy. --- crates/create/src/lib.rs | 13 ++++++++++++- crates/create/src/tests.rs | 19 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/crates/create/src/lib.rs b/crates/create/src/lib.rs index 6e1fdaee69..33093e74cd 100644 --- a/crates/create/src/lib.rs +++ b/crates/create/src/lib.rs @@ -393,11 +393,22 @@ fn rgba_image(rgba: &[u8], (w, h): (u32, u32), dpi: (f64, f64)) -> Embedded { /// BMP and GIF (the first frame), through the `image` decoders. Their resolution is not read: /// 72 dpi, as for images without one. fn decoded(name: &str, bytes: &[u8], format: image::ImageFormat) -> Result { + // A GIF's first frame is painted on a canvas the size of its logical screen, however small + // the frame: a few bytes can ask for hundreds of MB. + if format == image::ImageFormat::Gif + && let Some(&[w0, w1, h0, h1]) = bytes.get(6..10) + && u64::from(u16::from_le_bytes([w0, w1])) * u64::from(u16::from_le_bytes([h0, h1])) > MAX_GIF_PIXELS + { + return Err(CreateError::Image(name.into(), "the GIF's screen is larger than 4096×4096 pixels".into())); + } let img = image::load_from_memory_with_format(bytes, format).map_err(|e| CreateError::Image(name.into(), e.to_string()))?; - let rgba = img.to_rgba8(); + let rgba = img.into_rgba8(); Ok(rgba_image(rgba.as_raw(), rgba.dimensions(), (72.0, 72.0))) } +/// Largest GIF logical screen, in pixels (4096×4096). +const MAX_GIF_PIXELS: u64 = 1 << 24; + /// Most decoded bytes one TIFF page or PNG may take: room for the 768 MiB CMYK poster in #665, /// twice over. const MAX_DECODED_BYTES: usize = 2 << 30; diff --git a/crates/create/src/tests.rs b/crates/create/src/tests.rs index a352ecbf9a..5c29872b73 100644 --- a/crates/create/src/tests.rs +++ b/crates/create/src/tests.rs @@ -1222,3 +1222,22 @@ fn tiff_pages_are_decoded_within_a_total_budget() { let mut doc = Document::new_empty(); assert!(image_xobject(&mut doc, "t.tif", &tif).is_ok(), "the unreadable last page is never decoded"); } + +/// A one-pixel GIF on a `w`×`h` logical screen: the decoder paints it on a canvas that size. +fn gif_on_screen(w: u16, h: u16) -> Vec { + let mut gif = b"GIF89a".to_vec(); + gif.extend_from_slice(&w.to_le_bytes()); + gif.extend_from_slice(&h.to_le_bytes()); + gif.extend_from_slice(&[0x80, 0, 0, 0xFF, 0xFF, 0xFF, 0, 0, 0]); + gif.extend_from_slice(&[0x2C, 0, 0, 0, 0, 1, 0, 1, 0, 0, 2, 2, 0x44, 1, 0, 0x3B]); + gif +} + +/// The decoder's own limit lets a 74-byte GIF ask for a canvas of hundreds of MB, which the +/// conversion then copies; a screen past [`MAX_GIF_PIXELS`] is refused before decoding. +#[test] +fn a_tiny_gif_on_a_huge_screen_is_refused() { + assert!(matches!(from_images(&[("big.gif".into(), gif_on_screen(4200, 4100))]), Err(CreateError::Image(..)))); + let doc = reopen(&from_images(&[("small.gif".into(), gif_on_screen(40, 30))]).unwrap()); + assert_eq!(media(&pages(&doc)[0]), [0.0, 0.0, 40.0, 30.0]); +} From 9188cf59ebcf1c41ed616e34f096ec38dd19179b Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:37:59 +0000 Subject: [PATCH 14/22] fix(create): don't decode a JPEG carrying thousands of Extended XMP chunks The JPEG decoder re-sorts and rescans every pending Extended XMP chunk after each marker, so a file of many small chunks cost time quadratic in their number before any pixel limit applied. Turning a photo upright now counts the chunks first and leaves a JPEG with more than 1024 as it is. --- crates/create/src/upright_image.rs | 67 +++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/crates/create/src/upright_image.rs b/crates/create/src/upright_image.rs index 9816dfc4b2..b89c517190 100644 --- a/crates/create/src/upright_image.rs +++ b/crates/create/src/upright_image.rs @@ -14,6 +14,12 @@ const QUALITY: u8 = 92; /// Larger photos are embedded as they are: turning them would take hundreds of MB. const MAX_PIXELS: u64 = 64 << 20; +#[cfg(test)] +thread_local! { + /// How many JPEGs [`turned`] handed to the decoder on this thread. + static DECODES: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// A JPEG whose EXIF Orientation is not "as stored" has its pixels turned (and re-encoded); /// everything else, and any JPEG this can't read, is returned unchanged. pub(crate) fn upright(bytes: &[u8]) -> Cow<'_, [u8]> { @@ -24,9 +30,11 @@ pub(crate) fn upright(bytes: &[u8]) -> Cow<'_, [u8]> { } fn turned(bytes: &[u8]) -> Option> { - if !bytes.starts_with(&[0xFF, 0xD8]) { + if !bytes.starts_with(&[0xFF, 0xD8]) || extended_xmp_chunks(bytes) > MAX_EXTENDED_XMP_CHUNKS { return None; } + #[cfg(test)] + DECODES.with(|c| c.set(c.get().saturating_add(1))); let mut decoder = ImageReader::with_format(Cursor::new(bytes), ImageFormat::Jpeg).into_decoder().ok()?; let orientation = decoder.orientation().ok()?; let (w, h) = decoder.dimensions(); @@ -53,6 +61,41 @@ fn turned(bytes: &[u8]) -> Option> { Some(out) } +/// Most Extended XMP chunks a JPEG handed to the decoder may carry (64 MB of metadata). The +/// decoder re-sorts and rescans every chunk so far after each marker, quadratic in their number. +const MAX_EXTENDED_XMP_CHUNKS: usize = 1024; + +/// How many Extended XMP chunks (APP1 segments) come before the image data. +fn extended_xmp_chunks(bytes: &[u8]) -> usize { + const PREFIX: &[u8] = b"http://ns.adobe.com/xmp/extension/\0"; + let (mut i, mut chunks) = (2usize, 0usize); + while let Some(&byte) = bytes.get(i) { + // Stray bytes between segments, which the decoder skips too. + if byte != 0xFF { + i += 1; + continue; + } + let Some(&marker) = bytes.get(i.saturating_add(1)) else { break }; + match marker { + // Fill bytes before a marker. + 0xFF => i += 1, + // Markers without a segment. + 0x00 | 0x01 | 0xD0..=0xD7 => i += 2, + // The image data starts (or ends): no headers after it. + 0xDA | 0xD9 => break, + _ => { + let Some(&[hi, lo]) = bytes.get(i.saturating_add(2)..i.saturating_add(4)) else { break }; + let len = usize::from(u16::from_be_bytes([hi, lo])); + if marker == 0xE1 && bytes.get(i.saturating_add(4)..).is_some_and(|seg| seg.starts_with(PREFIX)) { + chunks += 1; + } + i = i.saturating_add(2).saturating_add(len.max(2)); + } + } + } + chunks +} + /// The resolution in a JFIF header, if the file has one in inches or centimetres. fn jfif_density(bytes: &[u8]) -> Option { let mut i = 2usize; @@ -94,4 +137,26 @@ mod tests { assert!(matches!(upright(b"\x89PNG\r\n\x1a\n"), Cow::Borrowed(_))); assert!(matches!(upright(b"\xff\xd8 not a jpeg"), Cow::Borrowed(_))); } + + #[test] + fn a_jpeg_with_thousands_of_extended_xmp_chunks_is_not_decoded() { + // Empty chunks of one never-complete extended packet: the decoder re-sorts and rescans every + // chunk so far after each marker, quadratic in their number. + let chunk = |n: usize| { + let mut seg = b"http://ns.adobe.com/xmp/extension/\0".to_vec(); + seg.extend_from_slice(&[b'0'; 32]); // the packet's GUID + seg.extend_from_slice(&1000u32.to_be_bytes()); // its full length + seg.extend_from_slice(&0u32.to_be_bytes()); // this chunk's offset + let mut out = vec![0xFF, 0xE1]; + out.extend_from_slice(&u16::try_from(seg.len() + 2).unwrap().to_be_bytes()); + out.extend_from_slice(&seg); + out.repeat(n) + }; + let photo = phone_jpeg(6, 144); + let with = |n: usize| [&photo[..2], b"\0".as_slice(), &chunk(n), &photo[2..]].concat(); + DECODES.with(|c| c.set(0)); + assert!(matches!(upright(&with(4)), Cow::Owned(_)), "a few chunks are an ordinary photo"); + assert!(matches!(upright(&with(5_000)), Cow::Borrowed(_))); + assert_eq!(DECODES.with(std::cell::Cell::get), 1, "only the ordinary photo reached the decoder"); + } } From 6384381127b49b11f1e039a84b6943659f346731 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 21:20:10 +0000 Subject: [PATCH 15/22] fix(forms): read each widget once when enumerating fields A /Kids array listing one widget many times, or shared by several fields, built a Widget for every listing, so a small file expanded into fields times listings widgets. A widget now belongs to the first field that lists it, and a page widget already owned is not adopted again. --- crates/forms/src/lib.rs | 15 +++++++++------ crates/forms/src/tests.rs | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 7bd87a2efc..c8c2ebcb0e 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -590,7 +590,7 @@ fn enumerate(doc: &Document) -> (Vec, usize) { None => Inherited::default(), }; let mut out = Vec::new(); - let mut seen = Seen { fields: std::collections::HashSet::new(), scripts: script_budget() }; + let mut seen = Seen { fields: std::collections::HashSet::new(), widgets: std::collections::HashSet::new(), scripts: script_budget() }; if let Some(af) = &af { for f in af.get(b"Fields").map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned()).unwrap_or_default() { if let Some(r) = f.as_ref() { @@ -623,7 +623,7 @@ fn adopt_page_fields( let Some(annots) = annots.as_array() else { continue }; for e in annots { let Some(r) = e.as_ref() else { continue }; - if seen.fields.contains(&r) { + if seen.fields.contains(&r) || seen.widgets.contains(&r) { continue; } let obj = doc.get(r); @@ -764,15 +764,17 @@ pub fn set_tab_order(doc: &mut Document, pages: &[usize], order: TabOrder) -> Re Ok(()) } -/// Read the field at `r` and the fields under it into `out`, with what they inherit from -/// `parent`: kids with a name are fields of their own, kids without are this field's widgets. -/// What one reading of the form's fields has used: the field dictionaries read, and the script -/// budget their actions share. +/// What one reading of the form's fields has used: the field and widget dictionaries read, and +/// the script budget their actions share. struct Seen { fields: std::collections::HashSet, + /// A widget belongs to the first field that lists it, however often it is listed. + widgets: std::collections::HashSet, scripts: DecodeBudget, } +/// Read the field at `r` and the fields under it into `out`, with what they inherit from +/// `parent`: kids with a name are fields of their own, kids without are this field's widgets. fn walk( doc: &Document, r: ObjRef, @@ -825,6 +827,7 @@ fn walk( return; } let widget_refs: Vec = if d.contains(b"Rect") { vec![r] } else { kids }; + let widget_refs: Vec = widget_refs.into_iter().filter(|w| seen.widgets.insert(*w)).collect(); let actions = actions_of(doc, d, &widget_refs, &mut seen.scripts); let button = (inh.ft.as_deref() == Some(b"Btn") && inh.ff.unwrap_or(0) & flags::PUSH_BUTTON != 0) .then(|| button_action(doc, d, &widget_refs, &mut seen.scripts)) diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index b2b8573ebb..62df8bd872 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -3021,3 +3021,22 @@ fn repeated_script_references_share_one_decode_budget() { let calculate: usize = all.iter().filter_map(|f| f.actions.scripts.calculate.as_ref()).map(String::len).sum(); assert!(calculate > 0 && calculate <= limit, "{calculate} bytes of field scripts"); } + +/// A widget listed many times, by one field's `/Kids` or by a `/Kids` array several fields share, +/// is read once: the first field to list it owns it. +#[test] +fn a_widget_listed_repeatedly_is_read_once() { + let kids = "7 0 R ".repeat(500); + let doc = document(&[ + "<< /Type /Catalog /Pages 4 0 R /AcroForm << /Fields [2 0 R 3 0 R] >> >>".into(), + "<< /FT /Tx /T (a) /Kids 6 0 R >>".into(), + "<< /FT /Tx /T (b) /Kids 6 0 R >>".into(), + "<< /Type /Pages /Kids [5 0 R] /Count 1 >>".into(), + "<< /Type /Page /Parent 4 0 R /MediaBox [0 0 100 100] /Annots [7 0 R] >>".into(), + format!("[{kids}]"), + "<< /Type /Annot /Subtype /Widget /Parent 2 0 R /Rect [0 0 10 10] >>".into(), + ]); + let all = fields(&doc); + let widgets: Vec = all.iter().map(|f| f.widgets.len()).collect(); + assert_eq!(widgets, [1, 0], "{:?}", all.iter().map(|f| &f.name).collect::>()); +} From c704c1089aa417fbc5e95975868fa4c044fc9e32 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 21:55:12 +0000 Subject: [PATCH 16/22] fix(create): keep the TIFF total budget in range on 32-bit targets --- crates/create/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/create/src/lib.rs b/crates/create/src/lib.rs index 33093e74cd..7d1fdfc366 100644 --- a/crates/create/src/lib.rs +++ b/crates/create/src/lib.rs @@ -414,8 +414,9 @@ const MAX_GIF_PIXELS: u64 = 1 << 24; const MAX_DECODED_BYTES: usize = 2 << 30; /// Most decoded bytes all the pages of one TIFF may take together: its pages stay in memory until -/// they are embedded, and many small directories can share one compressed strip. -const MAX_DECODED_TOTAL: usize = 2 * MAX_DECODED_BYTES; +/// they are embedded, and many small directories can share one compressed strip. Saturates to the +/// whole address space on 32-bit targets (wasm32), where 4 GiB doesn't fit in `usize`. +const MAX_DECODED_TOTAL: usize = MAX_DECODED_BYTES.saturating_mul(2); /// The first `wanted` pages of a TIFF (multi-page scans become multi-page PDFs). fn tiff_pages(name: &str, bytes: &[u8], wanted: usize) -> Result, CreateError> { From 7a466ff5fc33b291b71305823e1d8fcf038c73dc Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 23:32:11 +0000 Subject: [PATCH 17/22] fix(edit): prune a field again when a shorter path reaches it past the depth cutoff --- crates/edit/src/flatten.rs | 51 +++++++++++++++++++++++++------------- crates/edit/src/tests.rs | 32 ++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 17 deletions(-) diff --git a/crates/edit/src/flatten.rs b/crates/edit/src/flatten.rs index 1be77abef7..ac90dda113 100644 --- a/crates/edit/src/flatten.rs +++ b/crates/edit/src/flatten.rs @@ -309,42 +309,59 @@ fn prune_fields(doc: &mut Document, widgets: &HashSet, on_page: Option<& let af_ref = af.as_ref(); let mut af_dict = doc.resolve(&af).as_dict().cloned().unwrap_or_default(); let fields = af_dict.get(b"Fields").map(|f| doc.resolve(f)).and_then(|f| f.as_array().cloned()).unwrap_or_default(); - // Returns whether the node survives. `known` holds each node's answer once it is visited, so - // a node listed twice is pruned once, and one still being visited (a cycle) survives. + // Returns whether the node survives, and whether that answer depends on the depth cutoff. + // `known` holds each node's answer once it is visited, so a node listed twice is pruned once, + // and one still being visited (a cycle) survives. An answer the cutoff decided records the depth + // it was reached at, and a shallower path visits the node again: at most once per depth. fn keep( doc: &mut Document, r: ObjRef, widgets: &HashSet, on_page: Option<&HashSet>, - known: &mut HashMap, + known: &mut HashMap)>, depth: usize, - ) -> bool { + ) -> (bool, bool) { #[cfg(test)] KEEP_CALLS.with(|c| c.set(c.get().saturating_add(1))); - if widgets.contains(&r) || depth > 64 { - return depth > 64; + if widgets.contains(&r) { + return (false, false); } - if let Some(&survives) = known.get(&r) { - return survives; + if depth > 64 { + return (true, true); } - known.insert(r, true); - let Some(d) = doc.get(r).as_dict().cloned() else { return true }; + match known.get(&r) { + Some(&(survives, None)) => return (survives, false), + Some(&(survives, Some(at))) if depth >= at => return (survives, true), + _ => {} + } + known.insert(r, (true, None)); + let Some(d) = doc.get(r).as_dict().cloned() else { return (true, false) }; let Some(kids) = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()) else { let survives = on_page.is_none_or(|p| p.contains(&r)); - known.insert(r, survives); - return survives; + known.insert(r, (survives, None)); + return (survives, false); }; - let survivors: Vec = - kids.iter().filter(|k| k.as_ref().is_none_or(|kr| keep(doc, kr, widgets, on_page, known, depth + 1))).cloned().collect(); + let mut cut = false; + let survivors: Vec = kids + .iter() + .filter(|k| { + k.as_ref().is_none_or(|kr| { + let (survives, kid_cut) = keep(doc, kr, widgets, on_page, known, depth + 1); + cut |= kid_cut; + survives + }) + }) + .cloned() + .collect(); if survivors.len() != kids.len() { let _ = doc.update_dict(r, |d| d.set(b"Kids".to_vec(), Object::Array(survivors.clone()))); } - known.insert(r, !survivors.is_empty()); - !survivors.is_empty() + known.insert(r, (!survivors.is_empty(), cut.then_some(depth))); + (!survivors.is_empty(), cut) } let mut known = HashMap::new(); let survivors: Vec = - fields.iter().filter(|f| f.as_ref().is_none_or(|r| keep(doc, r, widgets, on_page, &mut known, 0))).cloned().collect(); + fields.iter().filter(|f| f.as_ref().is_none_or(|r| keep(doc, r, widgets, on_page, &mut known, 0).0)).cloned().collect(); if survivors.len() == fields.len() && widgets.is_empty() { return Ok(()); // only looked for fields on no page, and there were none } diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 69ee2687e3..2b3d474bbf 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -6403,6 +6403,38 @@ fn flattening_prunes_a_field_tree_with_repeated_kids_once_per_node() { assert_eq!(left, 2, "the chain to the hidden widget and the self-listing node stay"); } +/// A field reached first at the end of a long chain has its kids cut off by the depth limit, so +/// they survive there. A shorter path to the same field still looks at its kids: an off-page widget +/// under it is taken out when every field is flattened. +#[test] +fn a_field_reached_again_by_a_shorter_path_is_pruned_there() { + let chain = 64; + let page = stream("", " "); + // 7…: the chain; then the shared field, its off-page widget, and the short path to it. + let shared = 7 + chain; + let fields = format!("<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [5 0 R 7 0 R {} 0 R] >> >>", shared + 2); + let mut objs: Vec = vec![ + fields, + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into(), + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << >> /Contents 4 0 R /Annots [5 0 R] >>".into(), + page, + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (shown) /Rect [10 10 150 40] /F 4 /AP << /N 6 0 R >> >>".into(), + "<< /Type /XObject /Subtype /Form /BBox [0 0 140 30] /Length 0 >>\nstream\n\nendstream".into(), + ]; + for i in 0..chain { + objs.push(format!("<< /T (c{i}) /Kids [{} 0 R] >>", 8 + i)); + } + objs.push(format!("<< /T (shared) /Kids [{} 0 R] >>", shared + 1)); + objs.push("<< /Type /Annot /Subtype /Widget /FT /Tx /T (orphan) /Rect [10 50 150 80] /F 4 >>".into()); + objs.push(format!("<< /T (short) /Kids [{shared} 0 R] >>")); + let objs: Vec<&str> = objs.iter().map(String::as_str).collect(); + let mut doc = build(&objs); + assert_eq!(flatten(&mut doc, &[0], false, true).unwrap(), 1); + let shared_ref = pdfcraft_cos::ObjRef::new(u32::try_from(shared).unwrap(), 0); + let kids = doc.resolve(doc.get(shared_ref).as_dict().unwrap().get(b"Kids").unwrap()).as_array().unwrap().len(); + assert_eq!(kids, 0, "the off-page widget is pruned along the short path"); +} + /// A ToUnicode destination is repeated for every glyph that shows its code: a page whose text /// would run past the extraction budget is refused, not built in memory. #[test] From 54152aed876b272e3556d3d2fa43e55cc72e84c5 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 23:35:40 +0000 Subject: [PATCH 18/22] fix(edit): refuse a mark on top of over-deep content before changing the page --- crates/edit/src/added.rs | 3 +++ crates/edit/src/flatten.rs | 1 + crates/edit/src/lib.rs | 33 ++++++++++++++++++++--- crates/edit/src/marked_isolation_tests.rs | 26 ++++++++++++++++++ 4 files changed, 60 insertions(+), 3 deletions(-) diff --git a/crates/edit/src/added.rs b/crates/edit/src/added.rs index 671a72a1b8..d39bb0fa91 100644 --- a/crates/edit/src/added.rs +++ b/crates/edit/src/added.rs @@ -1050,6 +1050,9 @@ fn write(doc: &mut Document, page: usize, c: &Content, obj: Option) -> R } let taken = pres.get(b"Font").map(|f| doc.resolve(f)).and_then(|f| f.as_dict().cloned()).unwrap_or_default(); let (content, res) = draw(doc, c, view, &taken, obj.is_some())?; + if obj.is_none() { + crate::check_on_top(doc, p)?; + } for (k, v) in res.iter() { let mut sub = pres.get(k).map(|s| doc.resolve(s)).and_then(|s| s.as_dict().cloned()).unwrap_or_default(); for (n2, o) in v.as_dict().into_iter().flat_map(|d| d.iter()) { diff --git a/crates/edit/src/flatten.rs b/crates/edit/src/flatten.rs index ac90dda113..1fe1324b43 100644 --- a/crates/edit/src/flatten.rs +++ b/crates/edit/src/flatten.rs @@ -257,6 +257,7 @@ fn flatten_which(doc: &mut Document, pages: &[usize], which: Which) -> Result (behind_marks < list.len()).then_some((behind_marks, unclosed, marked)), // Content left inside an optional-content sequence it cannot close could be hidden. - Isolation::TooDeep => { - return Err(EditError::Invalid("the page's marked content is nested too deeply to add anything on top of it".into())); - } + Isolation::TooDeep => return Err(too_deep_on_top()), } }; let mark_ref = doc.add(Object::Stream(Stream::flate(tagged(tag), &content))); @@ -661,6 +659,26 @@ fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: &str, cont Ok(mark_ref) } +fn too_deep_on_top() -> EditError { + EditError::Invalid("the page's marked content is nested too deeply to add anything on top of it".into()) +} + +/// The refusal [`place_tagged`] gives for a mark on top of `page`, checked before an edit changes +/// anything (removing old marks, adding resources) so a refused edit leaves the document as it was. +fn check_on_top(doc: &Document, page: &pdfcraft_model::Page) -> Result<(), EditError> { + let obj = doc.get(page.obj); + let list = match obj.as_dict().and_then(|d| d.get(b"Contents")) { + None => return Ok(()), + Some(Object::Ref(r)) => doc.get(*r).as_array().cloned().unwrap_or_else(|| vec![Object::Ref(*r)]), + Some(Object::Array(a)) => a.clone(), + Some(other) => vec![other.clone()], + }; + match isolation(doc, &list) { + Isolation::TooDeep => Err(too_deep_on_top()), + _ => Ok(()), + } +} + /// Put content tagged `tag` first inside the q/Q wrapper, so it applies to the page's original /// content and nothing else (a header's shrink). A page without a wrapper has no original content /// on it and is left alone. @@ -680,6 +698,7 @@ fn place_inside_wrapper(doc: &mut Document, page: &pdfcraft_model::Page, tag: &s pub fn stamp(doc: &mut Document, page: usize, tag: &str, content: Vec) -> Result { let all = page_list(doc); check(&[page], all.len())?; + check_on_top(doc, &all[page])?; add_resources(doc, &all[page], None, Some(&content))?; let p = page_list(doc).swap_remove(page); place_tagged(doc, &p, tag, content, false) @@ -759,6 +778,9 @@ pub fn add_header_footer(doc: &mut Document, pages: &[usize], hf: &HeaderFooter, .iter() .map(|&i| hf.shrink.then(|| shrink_matrix(hf, all[i].display_size(doc), all[i].view_matrix(doc))).transpose()) .collect::, _>>()?; + for &i in pages { + check_on_top(doc, &all[i])?; + } if replace { remove_marks(doc, pages, MarkKind::HeaderFooter)?; } @@ -847,6 +869,11 @@ pub fn add_watermark(doc: &mut Document, pages: &[usize], wm: &Watermark, replac { return Err(EditError::Invalid("invalid watermark settings".into())); } + if !wm.behind { + for &i in pages { + check_on_top(doc, &all[i])?; + } + } if replace { remove_marks(doc, pages, MarkKind::Watermark)?; } diff --git a/crates/edit/src/marked_isolation_tests.rs b/crates/edit/src/marked_isolation_tests.rs index 571bd9c242..227371bcac 100644 --- a/crates/edit/src/marked_isolation_tests.rs +++ b/crates/edit/src/marked_isolation_tests.rs @@ -131,3 +131,29 @@ fn depth_budget_bounds_emitted_closures_and_overflow_is_refused_on_top() { place_tagged(&mut doc, &page, MarkKind::Background.tag(), b"q Q\n".to_vec(), true).unwrap(); } } + +/// Marks refused on top of content nested past the depth budget leave the page as it was: old +/// marks are not removed and no resources are added before the refusal. +#[test] +fn a_mark_refused_on_top_changes_nothing() { + let mut over = b"/Artifact BMC\n".repeat(ISOLATION_MAX_DEPTH); + over.extend_from_slice(b"/Artifact BMC\n"); + let mut doc = fixture(&[&over]); + let behind = Watermark { text: "OLD".into(), behind: true, ..Watermark::default() }; + add_watermark(&mut doc, &[0], &behind, false).unwrap(); + let page = page_list(&doc).remove(0).obj; + let before = (*doc.get(page)).clone(); + let on_top = Watermark { text: "NEW".into(), ..Watermark::default() }; + assert!(matches!(add_watermark(&mut doc, &[0], &on_top, true), Err(EditError::Invalid(_)))); + assert_eq!((*doc.get(page)).clone(), before, "the old watermark stays"); + let hf = HeaderFooter { + text: [String::new(), "Top".into(), String::new(), String::new(), String::new(), String::new()], + font: added::Family::Times, + ..HeaderFooter::default() + }; + assert!(matches!(add_header_footer(&mut doc, &[0], &hf, true, &Context::default()), Err(EditError::Invalid(_)))); + assert_eq!((*doc.get(page)).clone(), before, "no fonts are added"); + let mut stamped = doc.clone(); + assert!(matches!(stamp(&mut stamped, 0, "Probe", b"/PCTimes 9 Tf\n".to_vec()), Err(EditError::Invalid(_)))); + assert_eq!((*stamped.get(page)).clone(), before, "no fonts are added"); +} From 5473d354a9caaf95fb23ea18575dc2db2003459a Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 23:36:31 +0000 Subject: [PATCH 19/22] fix(forms): skip a field of an unread kind before it claims shared widgets --- crates/forms/src/lib.rs | 14 ++++++++------ crates/forms/src/tests.rs | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index c8c2ebcb0e..82c1c8e74e 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -826,12 +826,8 @@ fn walk( } return; } - let widget_refs: Vec = if d.contains(b"Rect") { vec![r] } else { kids }; - let widget_refs: Vec = widget_refs.into_iter().filter(|w| seen.widgets.insert(*w)).collect(); - let actions = actions_of(doc, d, &widget_refs, &mut seen.scripts); - let button = (inh.ft.as_deref() == Some(b"Btn") && inh.ff.unwrap_or(0) & flags::PUSH_BUTTON != 0) - .then(|| button_action(doc, d, &widget_refs, &mut seen.scripts)) - .flatten(); + // A field of no kind this reads is skipped before it claims widgets or scripts, which a later + // field listing them then reads. let ff = inh.ff.unwrap_or(0); let kind = match inh.ft.as_deref() { Some(b"Tx") => FieldKind::Text, @@ -843,6 +839,12 @@ fn walk( Some(b"Sig") => FieldKind::Signature, _ => return, }; + let widget_refs: Vec = if d.contains(b"Rect") { vec![r] } else { kids }; + let widget_refs: Vec = widget_refs.into_iter().filter(|w| seen.widgets.insert(*w)).collect(); + let actions = actions_of(doc, d, &widget_refs, &mut seen.scripts); + let button = (inh.ft.as_deref() == Some(b"Btn") && inh.ff.unwrap_or(0) & flags::PUSH_BUTTON != 0) + .then(|| button_action(doc, d, &widget_refs, &mut seen.scripts)) + .flatten(); let widgets = widget_refs .into_iter() .filter_map(|w| { diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 62df8bd872..4d5a14f578 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -3040,3 +3040,19 @@ fn a_widget_listed_repeatedly_is_read_once() { let widgets: Vec = all.iter().map(|f| f.widgets.len()).collect(); assert_eq!(widgets, [1, 0], "{:?}", all.iter().map(|f| &f.name).collect::>()); } + +/// A field of a kind that isn't read doesn't claim the widget it shares with a field that is. +#[test] +fn a_skipped_field_leaves_its_widget_to_the_next() { + let doc = document(&[ + "<< /Type /Catalog /Pages 4 0 R /AcroForm << /Fields [2 0 R 3 0 R] >> >>".into(), + "<< /FT /Zz /T (odd) /Kids [6 0 R] >>".into(), + "<< /FT /Tx /T (b) /Kids [6 0 R] >>".into(), + "<< /Type /Pages /Kids [5 0 R] /Count 1 >>".into(), + "<< /Type /Page /Parent 4 0 R /MediaBox [0 0 100 100] /Annots [6 0 R] >>".into(), + "<< /Type /Annot /Subtype /Widget /Parent 3 0 R /Rect [0 0 10 10] >>".into(), + ]); + let all = fields(&doc); + let widgets: Vec<(&str, usize)> = all.iter().map(|f| (f.name.as_str(), f.widgets.len())).collect(); + assert_eq!(widgets, [("b", 1)]); +} From 2430d53f9e1955a93b9331e4591320436bb3b0e3 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 23:37:07 +0000 Subject: [PATCH 20/22] fix(print): read the text after a form the budget refuses --- crates/print/src/lib.rs | 2 +- crates/print/src/tests.rs | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/crates/print/src/lib.rs b/crates/print/src/lib.rs index a6337026bd..dc5ac307c1 100644 --- a/crates/print/src/lib.rs +++ b/crates/print/src/lib.rs @@ -362,7 +362,7 @@ fn text_direction(doc: &Document, data: &[u8], res: &Dict, base: Matrix, depth: let m = form.dict.get(b"Matrix").and_then(|m| doc.resolve(m).as_array().and_then(|a| Matrix::from_operands(a))).unwrap_or_default(); let inner = form.dict.get(b"Resources").and_then(|r| doc.resolve(r).as_dict().cloned()).unwrap_or_else(|| res.clone()); if !budget.enter_form() { - return; + continue; } if let Ok(body) = budget.decode(form) { text_direction(doc, &body, &inner, m.then(&ctm), depth + 1, score, budget); diff --git a/crates/print/src/tests.rs b/crates/print/src/tests.rs index fcf1a62689..4cbed38ce8 100644 --- a/crates/print/src/tests.rs +++ b/crates/print/src/tests.rs @@ -916,3 +916,16 @@ fn text_direction_stops_at_the_form_budget() { text_direction(&doc, "/X Do ".repeat(20).as_bytes(), &res, Matrix::IDENTITY, 0, &mut score, &mut DecodeBudget::page()); assert!(score > 0 && score <= pdfcraft_cos::MAX_PAGE_FORMS as i64, "score {score}"); } + +/// A form refused by the spent budget is left out; the text drawn after it still counts. +#[test] +fn text_direction_reads_on_past_a_refused_form() { + let mut doc = Document::new_empty(); + let form = doc.add(Stream::from_raw([(b"Subtype".to_vec(), Object::Name(b"Form".to_vec()))].into_iter().collect(), Vec::new())); + let xobjects: Dict = [(b"X".to_vec(), Object::Ref(form))].into_iter().collect(); + let res: Dict = [(b"XObject".to_vec(), Object::Dict(xobjects))].into_iter().collect(); + let mut score = 0; + let content = b"/X Do BT 0 1 -1 0 0 0 Tm (a) Tj ET"; + text_direction(&doc, content, &res, Matrix::IDENTITY, 0, &mut score, &mut DecodeBudget::new(1 << 20, 0)); + assert_eq!(score, 1); +} From 0d216199b9df68737e9648059f1e5bdf47d670df Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 23:38:39 +0000 Subject: [PATCH 21/22] fix(redact): apply redactions on a copy so a failure leaves the document as it was --- crates/redact/src/lib.rs | 13 ++++++++++--- crates/redact/src/tests.rs | 33 +++++++++++++++++++++++++-------- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index fce9f432a9..00fbbbd171 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -357,10 +357,17 @@ fn localize_resources(doc: &mut Document, selected: &[usize]) -> Result<(), pdfc /// /// # Errors /// -/// This operation is not transactional: an error may leave `doc` partially modified, with -/// sensitive content still present. Call this on a working clone and replace the original only -/// on success. On any error, discard the working clone; do not save or reuse it. +/// On any error `doc` is left as it was: the redaction runs on a copy (cheap: unchanged objects +/// are shared) that replaces `doc` only on success. pub fn apply(doc: &mut Document, pages: Option<&[usize]>) -> Result { + let mut work = doc.clone(); + let report = apply_to(&mut work, pages)?; + *doc = work; + Ok(report) +} + +/// [`apply`] without the copy: an error can leave `doc` part redacted. +fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result { let (all_marks, unreadable) = marks_checked(doc); // A mark with no readable area is not applied, and applying the others would look complete: // refuse before anything changes. Only the pages being applied count. diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 2478e0b31b..e29cd99795 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -1690,8 +1690,8 @@ fn bluefin_survives(doc: &Document) -> bool { /// #3285 review (the maintainer's probes): an element written in place in the parent tree that /// owns the redacted text can't be changed, so applying is refused; the same element as an object -/// of its own is cleaned, and nothing of the word is left in a full rewrite. -/// Refusal does not promise rollback: direct callers discard the failed working copy. +/// of its own is cleaned, and nothing of the word is left in a full rewrite. A refusal leaves the +/// document as it was. #[test] fn an_element_written_in_place_owning_redacted_text_is_refused() { let objs = |element: &str| -> Vec> { @@ -1711,17 +1711,13 @@ fn an_element_written_in_place_owning_redacted_text_is_refused() { let mut doc = pdf(objs("<< /Type /StructElem /S /P /P 6 0 R /Pg 3 0 R /K 0 /ActualText (Codename Bluefin) >>")); mark(&mut doc, 0, &[[10.0, 140.0, 290.0, 170.0]], ""); let before = write_full(&doc, &SaveOptions::default()).unwrap(); - let mut working = doc.clone(); - let error = apply(&mut working, None).unwrap_err(); + let error = apply(&mut doc, None).unwrap_err(); assert!(matches!(error, RedactError::Tags)); assert_eq!( error.to_string(), "the document's tags can't be cleaned completely (an element written in place, or a structure tree too large or too deep to follow); the operation was refused" ); - assert!(!content(&working, 0).contains("Bluefin"), "the failed working copy was partially changed"); - assert!(bluefin_survives(&working), "the failed working copy still holds the word in its tags"); - drop(working); - assert_eq!(write_full(&doc, &SaveOptions::default()).unwrap(), before, "discarding the working copy preserves the original"); + assert_eq!(write_full(&doc, &SaveOptions::default()).unwrap(), before, "the refused document is unchanged"); let mut doc = pdf(objs("7 0 R")); mark(&mut doc, 0, &[[10.0, 140.0, 290.0, 170.0]], ""); let r = apply(&mut doc, None).unwrap(); @@ -2745,3 +2741,24 @@ fn content_and_forms_share_one_budget_per_page() { mark(&mut doc, 0, &[[0.0, 90.0, 50.0, 120.0]], ""); assert!(apply(&mut doc, None).is_err(), "the form past the page's allowance was read"); } + +/// A page that fails to read after another page was already redacted leaves the whole document as +/// it was, the shared resources the first page's rewrite localised included. +#[test] +fn a_failure_on_a_later_page_leaves_the_document_unchanged() { + let mut doc = two_pages_one_resource_dictionary(true, false); + let all = pdfcraft_model::pages(&doc); + let (first, second, parent) = (all[0].obj, all[1].obj, doc.get(all[0].obj).as_dict().unwrap().reference(b"Parent").unwrap()); + let mut dct = Dict::new(); + dct.set(b"Filter".to_vec(), Object::name("DCTDecode")); + let bad = doc.add(Object::Stream(Stream::from_raw(dct, b"garbage".to_vec()))); + doc.update_dict(second, |d| d.set(b"Contents".to_vec(), Object::Ref(bad))).unwrap(); + mark(&mut doc, 0, &[[0.0, 0.0, 300.0, 300.0]], ""); + mark(&mut doc, 1, &[[0.0, 0.0, 50.0, 50.0]], ""); + let before: Vec = [first, second, parent].iter().map(|r| (*doc.get(*r)).clone()).collect(); + let text = content(&doc, 0); + assert_eq!(apply(&mut doc, None), Err(RedactError::Unreadable(2))); + let after: Vec = [first, second, parent].iter().map(|r| (*doc.get(*r)).clone()).collect(); + assert_eq!(after, before); + assert_eq!(content(&doc, 0), text); +} From ba024eb0dd2dfd36f7f1ec2a2344e029a4f8a82b Mon Sep 17 00:00:00 2001 From: aiongg Date: Fri, 23 Oct 2026 00:02:33 +0000 Subject: [PATCH 22/22] fix(edit): flatten on a copy so a page refused part way changes nothing --- crates/edit/src/flatten.rs | 10 +++++++++- crates/edit/src/tests.rs | 22 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/crates/edit/src/flatten.rs b/crates/edit/src/flatten.rs index 1fe1324b43..8aa1b0e320 100644 --- a/crates/edit/src/flatten.rs +++ b/crates/edit/src/flatten.rs @@ -129,7 +129,16 @@ fn undrawn_on(doc: &Document, all: &[pdfcraft_model::Page], pages: &[usize], whi None } +/// Runs [`flatten_in`] on a copy of `doc` (unchanged objects are shared) and keeps it only on +/// success, so a page refused part way leaves the pages before it as they were. fn flatten_which(doc: &mut Document, pages: &[usize], which: Which) -> Result { + let mut work = doc.clone(); + let drawn = flatten_in(&mut work, pages, which)?; + *doc = work; + Ok(drawn) +} + +fn flatten_in(doc: &mut Document, pages: &[usize], which: Which) -> Result { let all = page_list(doc); check(pages, all.len())?; // `?` in an appearance can still be fixed (a field's value, a comment's text are stored as @@ -257,7 +266,6 @@ fn flatten_which(doc: &mut Document, pages: &[usize], which: Which) -> Result>", + "<< /Type /Pages /Kids [3 0 R 6 0 R] /Count 2 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << >> /Contents 4 0 R /Annots [5 0 R] >>", + &stream("", " "), + "<< /Type /Annot /Subtype /Square /Rect [10 10 50 50] /F 4 /AP << /N 9 0 R >> >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << >> /Contents 7 0 R /Annots [8 0 R] >>", + &deep, + "<< /Type /Annot /Subtype /Square /Rect [10 10 50 50] /F 4 /AP << /N 9 0 R >> >>", + "<< /Type /XObject /Subtype /Form /BBox [0 0 40 40] /Length 0 >>\nstream\n\nendstream", + ]); + let first = pdfcraft_cos::ObjRef::new(3, 0); + let before = (*doc.get(first)).clone(); + assert!(matches!(flatten(&mut doc, &[0, 1], true, false), Err(EditError::Invalid(_)))); + assert_eq!((*doc.get(first)).clone(), before, "the first page keeps its comment and content"); +}