From 110b7c678a1658c8b880be1d5f6d255c8f2e2248 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:29:44 +0000 Subject: [PATCH 01/20] fix(a11y): read the enclosing marked content in one step per operator Marked-text extraction and figure bounds searched the whole marked-content stack before every operator, so deeply nested unmarked BMC blocks made a page quadratic. The text stack now holds each level's effective id (its own, else the enclosing one's), and the bounds pass counts the open sequences that match. --- crates/a11y/src/alt.rs | 17 ++++++++++++---- crates/a11y/src/headings.rs | 21 +++++++++++-------- crates/a11y/src/lib.rs | 14 +++++++++++++ crates/a11y/src/tests.rs | 40 +++++++++++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 12 deletions(-) diff --git a/crates/a11y/src/alt.rs b/crates/a11y/src/alt.rs index e4755ab38..ed91f9e50 100644 --- a/crates/a11y/src/alt.rs +++ b/crates/a11y/src/alt.rs @@ -183,16 +183,19 @@ fn union(a: Option<[f64; 4]>, b: [f64; 4]) -> [f64; 4] { } /// Where the marked content `ids` is drawn on a page (images, forms and paths). -fn content_bbox(doc: &Document, page: &Dict, ids: &HashSet) -> Option<[f64; 4]> { +pub(crate) fn content_bbox(doc: &Document, page: &Dict, ids: &HashSet) -> Option<[f64; 4]> { let res = page.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned()).unwrap_or_default(); let xobjects = res.get(b"XObject").map(|x| doc.resolve(x)).and_then(|x| x.as_dict().cloned()).unwrap_or_default(); let props = page_properties(doc, page); let mut ctm = vec![Matrix([1.0, 0.0, 0.0, 1.0, 0.0, 0.0])]; + // Whether each open marked-content sequence is one of `ids`, and how many of them are. let mut marks: Vec = Vec::new(); + let mut matching = 0usize; let mut bbox = None; let mut path: Vec<(f64, f64)> = Vec::new(); for op in pdfcraft_content::parse(&page_content(doc, page)).ops { - let inside = marks.iter().any(|m| *m); + crate::count_mark_reads(1); + let inside = matching > 0; let top = ctm.last().copied().unwrap_or(Matrix([1.0, 0.0, 0.0, 1.0, 0.0, 0.0])); match op.op.as_slice() { b"q" => ctm.push(top), @@ -207,9 +210,15 @@ fn content_bbox(doc: &Document, page: &Dict, ids: &HashSet) -> Option<[f64; } } b"BMC" => marks.push(false), - b"BDC" => marks.push(bdc_mcid(doc, &props, &op).is_some_and(|n| ids.contains(&n))), + b"BDC" => { + let wanted = bdc_mcid(doc, &props, &op).is_some_and(|n| ids.contains(&n)); + matching += usize::from(wanted); + marks.push(wanted); + } b"EMC" => { - marks.pop(); + if marks.pop() == Some(true) { + matching = matching.saturating_sub(1); + } } b"m" | b"l" => { if let Some([x, y]) = op.nums::<2>() { diff --git a/crates/a11y/src/headings.rs b/crates/a11y/src/headings.rs index 537ed908d..d3a478cb9 100644 --- a/crates/a11y/src/headings.rs +++ b/crates/a11y/src/headings.rs @@ -182,6 +182,7 @@ pub(crate) fn mcid_texts(doc: &Document, page: &Dict) -> HashMap { let res = sub(doc, page, b"Resources"); let (fonts, props) = (sub(doc, &res, b"Font"), sub(doc, &res, b"Properties")); let mut metrics: HashMap, Metrics> = HashMap::new(); + // The id each open marked-content sequence is in: its own, else the enclosing one's. let mut stack: Vec> = Vec::new(); let mut out: HashMap = HashMap::new(); // Where each id's last glyph ended, in user space. @@ -191,16 +192,20 @@ pub(crate) fn mcid_texts(doc: &Document, page: &Dict) -> HashMap { let mut unsaved = 0usize; let (mut tm, mut tlm) = (Matrix::IDENTITY, Matrix::IDENTITY); for op in pdfcraft_content::parse(&page_content(doc, page)).ops { - let id = stack.iter().rev().find_map(|m| *m); + crate::count_mark_reads(1); + let id = stack.last().copied().flatten(); match op.op.as_slice() { - b"BMC" => stack.push(None), - b"BDC" => stack.push(match op.operands.get(1) { - Some(Object::Dict(d)) => d.get(b"MCID").and_then(Object::as_int), - Some(Object::Name(n)) => { - props.get(n).map(|x| doc.resolve(x)).and_then(|x| x.as_dict().and_then(|d| d.get(b"MCID").and_then(Object::as_int))) + b"BMC" => stack.push(id), + b"BDC" => stack.push( + match op.operands.get(1) { + Some(Object::Dict(d)) => d.get(b"MCID").and_then(Object::as_int), + Some(Object::Name(n)) => { + props.get(n).map(|x| doc.resolve(x)).and_then(|x| x.as_dict().and_then(|d| d.get(b"MCID").and_then(Object::as_int))) + } + _ => None, } - _ => None, - }), + .or(id), + ), b"EMC" => { stack.pop(); } diff --git a/crates/a11y/src/lib.rs b/crates/a11y/src/lib.rs index ff81cc4c9..22fac756e 100644 --- a/crates/a11y/src/lib.rs +++ b/crates/a11y/src/lib.rs @@ -21,6 +21,20 @@ mod report; mod structure; mod tags; +#[cfg(test)] +thread_local! { + /// Marked-content stack entries read to decide which marks enclose an operator, for the tests + /// that keep that constant per operator. + static MARK_READS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Counts `n` marked-content stack reads (tests only). +#[inline] +fn count_mark_reads(_n: usize) { + #[cfg(test)] + MARK_READS.with(|c| c.set(c.get().saturating_add(_n))); +} + pub use alt::{AltError, ArtifactError, Figure, figures, mark_decorative, mark_tag_artifact, set_alt}; pub use autotag::{AutoTagError, AutoTagResult, autotag_text}; pub use autotag_fields::tag_form_fields; diff --git a/crates/a11y/src/tests.rs b/crates/a11y/src/tests.rs index 15ca643e8..b5eb54742 100644 --- a/crates/a11y/src/tests.rs +++ b/crates/a11y/src/tests.rs @@ -1023,6 +1023,46 @@ fn glyph_by_tm(text: &str, size: f64, y: f64) -> String { ops + " ET" } +/// Marked-content stack reads while `f` runs. +fn mark_reads(f: impl FnOnce()) -> usize { + MARK_READS.with(|c| c.set(0)); + f(); + MARK_READS.with(|c| c.get()) +} + +/// Page content nesting `depth` unmarked `BMC` blocks around a marked one, with `ops` operators inside. +fn deeply_nested_page(doc: &mut Document, depth: usize, ops: usize) -> pdfcraft_cos::Dict { + use pdfcraft_cos::{Dict, Stream}; + let mut data = "/Figure << /MCID 0 >> BDC ".repeat(1); + data.push_str(&"/X BMC ".repeat(depth)); + data.push_str(&"0 0 1 1 re f ".repeat(ops)); + data.push_str(&"EMC ".repeat(depth + 1)); + let content = doc.add(Stream::flate(Dict::new(), data.as_bytes())); + [(b"Contents".to_vec(), Object::Ref(content))].into_iter().collect() +} + +#[test] +fn marked_text_reads_its_enclosing_id_in_constant_steps() { + let mut doc = Document::new_empty(); + let page = deeply_nested_page(&mut doc, 5_000, 5_000); + let reads = mark_reads(|| { + crate::headings::mcid_texts(&doc, &page); + }); + // About 15 000 operators; a scan of the stack per operator reads millions of entries. + assert!(reads <= 4 * 15_000, "{reads} stack reads"); +} + +#[test] +fn figure_bounds_read_the_marked_content_state_in_constant_steps() { + let mut doc = Document::new_empty(); + let page = deeply_nested_page(&mut doc, 5_000, 5_000); + // No mark encloses figure 7, so a scan of the stack per operator reads all of it. + let reads = mark_reads(|| { + crate::alt::content_bbox(&doc, &page, &std::collections::HashSet::from([7])); + }); + assert!(reads <= 4 * 15_000, "{reads} stack reads"); +} + /// One tagged page whose H1 headings show `contents` (one marked-content sequence each). fn heading_page(contents: &[String]) -> Document { let marked: Vec = contents.iter().enumerate().map(|(i, c)| format!("/H1 << /MCID {i} >> BDC {c} EMC")).collect(); From 38211588c754da1a7319dc5fa0f2b0bf54069815 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:32:02 +0000 Subject: [PATCH 02/20] fix(a11y): find every figure's bounds on a page in one pass Figure listing decoded and parsed a page's content once per figure on it, so many small Figure elements pointing at one page multiplied the work. The figures are grouped by page and one pass gathers the bounds of every marked-content id they own; what a sequence draws also counts for the sequences around it, as before. --- crates/a11y/src/alt.rs | 86 ++++++++++++++++++++++++++++++---------- crates/a11y/src/lib.rs | 2 + crates/a11y/src/tests.rs | 23 ++++++++++- 3 files changed, 89 insertions(+), 22 deletions(-) diff --git a/crates/a11y/src/alt.rs b/crates/a11y/src/alt.rs index ed91f9e50..fe8eb292c 100644 --- a/crates/a11y/src/alt.rs +++ b/crates/a11y/src/alt.rs @@ -182,16 +182,47 @@ fn union(a: Option<[f64; 4]>, b: [f64; 4]) -> [f64; 4] { } } -/// Where the marked content `ids` is drawn on a page (images, forms and paths). -pub(crate) fn content_bbox(doc: &Document, page: &Dict, ids: &HashSet) -> Option<[f64; 4]> { +/// An open marked-content sequence while bounds are gathered: its id, when it is one of those asked +/// for, and the bounds of what has been drawn in it so far. +struct Mark { + id: Option, + drawn: Option<[f64; 4]>, +} + +/// Adds `b` to what the innermost open sequence has drawn. +fn draw(marks: &mut [Mark], b: [f64; 4]) { + if let Some(m) = marks.last_mut() { + m.drawn = Some(union(m.drawn, b)); + } +} + +/// Closes the innermost open sequence: what it drew counts for its id and for the sequence around it. +fn close(marks: &mut Vec, matching: &mut usize, out: &mut HashMap) { + let Some(m) = marks.pop() else { return }; + if m.id.is_some() { + *matching = matching.saturating_sub(1); + } + let Some(b) = m.drawn else { return }; + if let Some(id) = m.id { + let e = out.entry(id).or_insert(b); + *e = union(Some(*e), b); + } + draw(marks, b); +} + +/// Where each of the marked content `ids` is drawn on a page (images, forms and paths), in one pass +/// over the page. What a sequence draws counts for every sequence of `ids` around it too. +pub(crate) fn content_bboxes(doc: &Document, page: &Dict, ids: &HashSet) -> HashMap { let res = page.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned()).unwrap_or_default(); let xobjects = res.get(b"XObject").map(|x| doc.resolve(x)).and_then(|x| x.as_dict().cloned()).unwrap_or_default(); let props = page_properties(doc, page); + #[cfg(test)] + crate::BOUNDS_PASSES.with(|c| c.set(c.get() + 1)); let mut ctm = vec![Matrix([1.0, 0.0, 0.0, 1.0, 0.0, 0.0])]; - // Whether each open marked-content sequence is one of `ids`, and how many of them are. - let mut marks: Vec = Vec::new(); + // The open marked-content sequences, and how many of them are one of `ids`. + let mut marks: Vec = Vec::new(); let mut matching = 0usize; - let mut bbox = None; + let mut out = HashMap::new(); let mut path: Vec<(f64, f64)> = Vec::new(); for op in pdfcraft_content::parse(&page_content(doc, page)).ops { crate::count_mark_reads(1); @@ -209,17 +240,13 @@ pub(crate) fn content_bbox(doc: &Document, page: &Dict, ids: &HashSet) -> O *t = m.then(&top); } } - b"BMC" => marks.push(false), + b"BMC" => marks.push(Mark { id: None, drawn: None }), b"BDC" => { - let wanted = bdc_mcid(doc, &props, &op).is_some_and(|n| ids.contains(&n)); - matching += usize::from(wanted); - marks.push(wanted); - } - b"EMC" => { - if marks.pop() == Some(true) { - matching = matching.saturating_sub(1); - } + let id = bdc_mcid(doc, &props, &op).filter(|n| ids.contains(n)); + matching += usize::from(id.is_some()); + marks.push(Mark { id, drawn: None }); } + b"EMC" => close(&mut marks, &mut matching, &mut out), b"m" | b"l" => { if let Some([x, y]) = op.nums::<2>() { path.push(top.apply(x, y)); @@ -240,7 +267,7 @@ pub(crate) fn content_bbox(doc: &Document, page: &Dict, ids: &HashSet) -> O b"f" | b"F" | b"f*" | b"B" | b"B*" | b"b" | b"b*" | b"S" | b"s" | b"n" => { if inside && !matches!(op.op.as_slice(), b"n") { for (x, y) in &path { - bbox = Some(union(bbox, [*x, *y, *x, *y])); + draw(&mut marks, [*x, *y, *x, *y]); } } path.clear(); @@ -260,13 +287,16 @@ pub(crate) fn content_bbox(doc: &Document, page: &Dict, ids: &HashSet) -> O } _ => top.bbox([0.0, 0.0, 1.0, 1.0]), }; - bbox = Some(union(bbox, unit)); + draw(&mut marks, unit); } - b"BI" if inside => bbox = Some(union(bbox, top.bbox([0.0, 0.0, 1.0, 1.0]))), + b"BI" if inside => draw(&mut marks, top.bbox([0.0, 0.0, 1.0, 1.0])), _ => {} } } - bbox + while !marks.is_empty() { + close(&mut marks, &mut matching, &mut out); + } + out } /// The figures, in document order. @@ -275,14 +305,28 @@ pub fn figures(doc: &Document) -> Vec
{ let role_map = root.get(b"RoleMap").map(|o| doc.resolve(o)).and_then(|o| o.as_dict().cloned()).unwrap_or_default(); let pages = pdfcraft_model::pages(doc); let index: HashMap = pages.iter().enumerate().map(|(i, p)| (p.obj, i)).collect(); - elements(doc) + let figures: Vec<(Element, Option)> = elements(doc) .into_iter() .filter(|e| e.dict.name(b"S").is_some_and(|s| standard(doc, &role_map, s) == b"Figure")) .map(|e| { let page = e.page.and_then(|p| index.get(&p).copied()); + (e, page) + }) + .collect(); + let mut wanted: HashMap> = HashMap::new(); + for (e, page) in &figures { + if let Some(p) = page { + wanted.entry(*p).or_default().extend(mcids(&e.dict)); + } + } + let bounds: HashMap> = + wanted.into_iter().filter_map(|(p, ids)| Some((p, content_bboxes(doc, &pages.get(p)?.dict, &ids)))).collect(); + figures + .into_iter() + .map(|(e, page)| { let alt = e.dict.get(b"Alt").map(|o| doc.resolve(o)).and_then(|o| o.as_string().map(|s| s.to_text())); - let ids: HashSet = mcids(&e.dict).into_iter().collect(); - let bbox = page.and_then(|p| content_bbox(doc, &pages[p].dict, &ids)); + let on_page = page.and_then(|p| bounds.get(&p)); + let bbox = on_page.and_then(|b| mcids(&e.dict).iter().filter_map(|id| b.get(id)).fold(None, |acc, b| Some(union(acc, *b)))); Figure { obj: e.object, page, alt, bbox } }) .collect() diff --git a/crates/a11y/src/lib.rs b/crates/a11y/src/lib.rs index 22fac756e..21d8ea582 100644 --- a/crates/a11y/src/lib.rs +++ b/crates/a11y/src/lib.rs @@ -26,6 +26,8 @@ thread_local! { /// Marked-content stack entries read to decide which marks enclose an operator, for the tests /// that keep that constant per operator. static MARK_READS: std::cell::Cell = const { std::cell::Cell::new(0) }; + /// Page content passes made to find figure bounds. + static BOUNDS_PASSES: std::cell::Cell = const { std::cell::Cell::new(0) }; } /// Counts `n` marked-content stack reads (tests only). diff --git a/crates/a11y/src/tests.rs b/crates/a11y/src/tests.rs index b5eb54742..a24f59edb 100644 --- a/crates/a11y/src/tests.rs +++ b/crates/a11y/src/tests.rs @@ -1058,11 +1058,32 @@ fn figure_bounds_read_the_marked_content_state_in_constant_steps() { let page = deeply_nested_page(&mut doc, 5_000, 5_000); // No mark encloses figure 7, so a scan of the stack per operator reads all of it. let reads = mark_reads(|| { - crate::alt::content_bbox(&doc, &page, &std::collections::HashSet::from([7])); + crate::alt::content_bboxes(&doc, &page, &std::collections::HashSet::from([7])); }); assert!(reads <= 4 * 15_000, "{reads} stack reads"); } +#[test] +fn figure_bounds_take_one_pass_over_each_page() { + let n = 50; + let content: Vec = (0..n).map(|i| format!("/Figure << /MCID {i} >> BDC {i} 0 1 1 re f EMC")).collect(); + let kids: Vec = (0..n).map(|i| format!("{} 0 R", 6 + i)).collect(); + let mut objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /StructTreeRoot 5 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into(), + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Contents 4 0 R >>".into(), + stream(&content.join(" ")), + format!("<< /Type /StructTreeRoot /K [{}] >>", kids.join(" ")), + ]; + objs.extend((0..n).map(|i| format!("<< /S /Figure /Pg 3 0 R /K {i} >>"))); + let doc = pdf(&objs, ""); + BOUNDS_PASSES.with(|c| c.set(0)); + let f = figures(&doc); + assert_eq!(BOUNDS_PASSES.with(|c| c.get()), 1, "page passes"); + assert_eq!(f.len(), n); + assert!(f.iter().enumerate().all(|(i, f)| f.bbox == Some([i as f64, 0.0, i as f64 + 1.0, 1.0]))); +} + /// One tagged page whose H1 headings show `contents` (one marked-content sequence each). fn heading_page(contents: &[String]) -> Document { let marked: Vec = contents.iter().enumerate().map(|(i, c)| format!("/H1 << /MCID {i} >> BDC {c} EMC")).collect(); From 1cc392e5b04ada01b130236c6d101636e8bb62da Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:33:28 +0000 Subject: [PATCH 03/20] fix(fonts): cut ToUnicode destinations to the CMap limit A sequential bfrange stores one destination and builds every covered glyph's text from all of it, so one huge destination and many repeats of its codes made text extraction copy megabytes per glyph. Destinations keep at most 256 UTF-16 units (the format's 512 bytes), and marked-text extraction stops asking for a glyph's text once its id has all the text it keeps. --- crates/a11y/src/headings.rs | 12 ++++++------ crates/fonts/src/pdf.rs | 22 ++++++++++++++++++++-- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/crates/a11y/src/headings.rs b/crates/a11y/src/headings.rs index d3a478cb9..41c749ed3 100644 --- a/crates/a11y/src/headings.rs +++ b/crates/a11y/src/headings.rs @@ -285,17 +285,17 @@ pub(crate) fn mcid_texts(doc: &Document, page: &Dict) -> HashMap { }; for (code, len) in m.codes(&s.bytes) { let trm = tm.then(&gs.ctm); + // A glyph's text is only built while its id still takes more. if let Some(id) = id + && out.get(&id).is_none_or(|t| t.len() < MAX_TITLE * 4) && let Some(glyph) = m.text_of(code) { let text = out.entry(id).or_default(); - if text.len() < MAX_TITLE * 4 { - let breaks = ends.get(&id).is_some_and(|end| new_word(&trm, *end, gs.size, gs.scale)); - if breaks && !text.is_empty() && !text.ends_with(' ') && !glyph.starts_with(' ') { - text.push(' '); - } - text.push_str(&glyph); + let breaks = ends.get(&id).is_some_and(|end| new_word(&trm, *end, gs.size, gs.scale)); + if breaks && !text.is_empty() && !text.ends_with(' ') && !glyph.starts_with(' ') { + text.push(' '); } + text.push_str(&glyph); } let advance = m.width(code) * gs.size + gs.char_spacing + if m.is_space(code, len) { gs.word_spacing } else { 0.0 }; tm = Matrix::translate(advance * gs.scale, 0.0).then(&tm); diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index 0aaed6939..0badcfc3a 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -905,9 +905,13 @@ impl Metrics { } } -/// The UTF-16 code units of CMap hex bytes. +/// Most UTF-16 units kept from one ToUnicode destination: the CMap format allows destinations of up +/// to 512 bytes, and every glyph a sequential range covers builds its text from the whole of one. +const MAX_DESTINATION_UNITS: usize = 256; + +/// The UTF-16 code units of CMap hex bytes, at most [`MAX_DESTINATION_UNITS`] of them. fn code_units(b: &[u8]) -> Vec { - b.chunks(2).map(|c| u16::from_be_bytes([c[0], *c.get(1).unwrap_or(&0)])).collect() + b.chunks(2).take(MAX_DESTINATION_UNITS).map(|c| u16::from_be_bytes([c[0], *c.get(1).unwrap_or(&0)])).collect() } /// UTF-16BE (with surrogates) from CMap hex bytes. @@ -1262,6 +1266,20 @@ mod tests { use super::*; + /// Every glyph of a sequential range builds its text from the destination, so a long one is cut + /// to the CMap limit once, when it is read. + #[test] + fn unicode_destinations_are_cut_to_the_cmap_limit() { + let long = "0041".repeat(4096); + let cmap = format!("2 beginbfrange <0010> <0020> <{long}> <0030> <0030> [<{long}>] endbfrange 1 beginbfchar <0040> <{long}> endbfchar"); + let mut u = Unicode::default(); + parse_to_unicode(cmap.as_bytes(), &mut u); + for code in [0x10, 0x20, 0x30, 0x40] { + let text = u.text(code).unwrap_or_default(); + assert!(!text.is_empty() && text.len() <= MAX_DESTINATION_UNITS * 3, "code {code:#x}: {} bytes", text.len()); + } + } + /// A font whose ranges cover a few low codes keeps tables that size, not a fixed 64k-entry one. #[test] fn low_code_tables_end_with_the_last_covered_code() { From 75c4275810edafaa3b607d136b18569d18c797cd Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:35:00 +0000 Subject: [PATCH 04/20] fix(fonts): look up CID and width ranges by binary search Code-to-CID and /W range lookups scanned every record in declaration order, once per glyph and once per explicit ToUnicode entry while a font was read, so a few thousand records multiplied by as many codes took billions of steps. Both tables are indexed once into stretches whose first-declared covering record wins, the precedence the scan had, and the /W ranges stop at the CMap entry limit. --- crates/fonts/src/pdf.rs | 157 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 141 insertions(+), 16 deletions(-) diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index 0badcfc3a..4a118a07d 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -33,9 +33,9 @@ pub struct Metrics { widths: Vec, /// Composite fonts: CID → width, and CID ranges with one width. cid_widths: HashMap, - cid_ranges: Vec<(u32, u32, f64)>, + cid_ranges: Spans, /// Code → CID for embedded non-identity CMaps (`cidrange` / `cidchar`). - cid_map: Vec<(u32, u32, u32)>, + cid_map: Spans, default: f64, /// Composite fonts with an explicit `/DW`: `/W` may leave out the glyphs that have that /// width (Chrome and Skia do), so a CID missing from `/W` is not a missing glyph. @@ -598,8 +598,8 @@ impl Metrics { first: 0, widths: Vec::new(), cid_widths: HashMap::new(), - cid_ranges: Vec::new(), - cid_map: Vec::new(), + cid_ranges: Spans::default(), + cid_map: Spans::default(), default: 500.0, explicit_default: false, std14: Some(crate::Std14::Helvetica), @@ -685,7 +685,7 @@ impl Metrics { if !spaces.is_empty() { m.codes = Codes::Ranges(spaces); } - m.cid_map = cids; + m.cid_map = Spans::new(cids); } let desc = font .get(b"DescendantFonts") @@ -698,6 +698,7 @@ impl Metrics { m.explicit_default = true; } if let Some(w) = desc.get(b"W").map(|w| doc.resolve(w)).and_then(|w| w.as_array().cloned()) { + let mut ranges = Vec::new(); let mut i = 0; while i < w.len() { let Some(c0) = doc.resolve(&w[i]).as_f64().map(|v| v as u32) else { break }; @@ -714,13 +715,16 @@ impl Metrics { } Some(c1) => { if let (Some(c1), Some(v)) = (c1.as_f64(), w.get(i + 2).and_then(|x| doc.resolve(x).as_f64())) { - m.cid_ranges.push((c0, c1 as u32, v)); + if ranges.len() < MAX_CMAP_ENTRIES { + ranges.push((c0, c1 as u32, v)); + } } i += 3; } None => break, } } + m.cid_ranges = Spans::new(ranges); } descriptor = dict(doc, desc.get(b"FontDescriptor")); } else { @@ -791,14 +795,14 @@ impl Metrics { if self.cid_map.is_empty() { return code; } - self.cid_map.iter().find(|(lo, hi, _)| code >= *lo && code <= *hi).map_or(0, |(lo, _, c)| c.saturating_add(code - lo)) + self.cid_map.find(code).map_or(0, |(lo, _, c)| c.saturating_add(code.saturating_sub(*lo))) } /// The advance of `code` in text space per unit of font size. pub fn width(&self, code: u32) -> f64 { if self.composite { let cid = self.cid(code); - let w = self.cid_widths.get(&cid).copied().or_else(|| self.cid_ranges.iter().find(|(a, b, _)| cid >= *a && cid <= *b).map(|r| r.2)); + let w = self.cid_widths.get(&cid).copied().or_else(|| self.cid_ranges.find(cid).map(|r| r.2)); return w.unwrap_or(self.default) * self.scale; } if let Some(w) = code.checked_sub(self.first).and_then(|i| self.widths.get(i as usize)) { @@ -858,7 +862,7 @@ impl Metrics { // A CID /W leaves out takes /DW, and Skia (Chrome) leaves out every CID of that width: for // those, only the subset's program says whether the glyph is there. return self.cid_widths.contains_key(&cid) - || self.cid_ranges.iter().any(|(a, b, _)| cid >= *a && cid <= *b) + || self.cid_ranges.find(cid).is_some() || !self.subset || self.outlined_cids.get(cid as usize / 64).is_some_and(|w| (*w >> (cid % 64)) & 1 == 1) // No readable TrueType program to ask (e.g. CFF or missing): an explicit /DW @@ -958,16 +962,69 @@ fn add_range(out: &mut Unicode, budget: &mut usize, seq: &mut u32, lo: u32, hi: true } +#[cfg(test)] +thread_local! { + /// Code span records read by CID and width lookups, for the tests that keep a lookup logarithmic. + static SPAN_READS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Counts one span record read (tests only). +#[inline] +fn count_span_reads() { + #[cfg(test)] + SPAN_READS.with(|c| c.set(c.get().saturating_add(1))); +} + +/// Code spans `(lo, hi, value)` in declaration order, where the first-declared span covering a code +/// wins, indexed so that a lookup is one binary search. +#[derive(Clone, Debug, Default, PartialEq)] +struct Spans { + spans: Vec<(u32, u32, T)>, + /// The span covering each stretch of codes; see [`stretches`]. + cover: Vec<(u64, Option)>, +} + +impl Spans { + fn new(spans: Vec<(u32, u32, T)>) -> Self { + let bounds: Vec<(u32, u32)> = spans.iter().map(|s| (s.0, s.1)).collect(); + Spans { cover: stretches(&bounds, false), spans } + } + + fn is_empty(&self) -> bool { + self.spans.is_empty() + } + + /// The first-declared span covering `code`. + fn find(&self, code: u32) -> Option<&(u32, u32, T)> { + let step = self.cover.partition_point(|&(start, _)| { + count_span_reads(); + start <= u64::from(code) + }); + let &(_, record) = self.cover.get(step.checked_sub(1)?)?; + self.spans.get(record?) + } +} + /// For each stretch of codes, from its first code on, the latest-declared range covering it (`None` /// where no range does), sorted by code; a lookup is one binary search over it. fn cover_index(ranges: &[BfRange]) -> Vec<(u64, Option)> { + let spans: Vec<(u32, u32)> = ranges.iter().map(|r| (r.lo, r.hi)).collect(); + stretches(&spans, true) +} + +/// For each stretch of codes, from its first code on, the span `(lo, hi)` covering it (`None` where +/// none does), sorted by code: the latest-declared one when `later_wins`, else the first-declared. +/// At most `u32::MAX` spans are indexed. +fn stretches(spans: &[(u32, u32)], later_wins: bool) -> Vec<(u64, Option)> { + // Heap keys, largest for the winning record; the mapping is its own inverse. + let key = |record: u32| if later_wins { record } else { !record }; // Openings (first code, record) and closings (one past the last code), each in code order. The cover // can only change at one of their codes, so the sweep visits those codes in order. - let mut opens: Vec<(u32, u32)> = ranges.iter().enumerate().map(|(i, r)| (r.lo, i as u32)).collect(); + let mut opens: Vec<(u32, u32)> = spans.iter().zip(0..u32::MAX).map(|(&(lo, _), i)| (lo, i)).collect(); opens.sort_unstable(); - let mut closes: Vec = ranges.iter().map(|r| u64::from(r.hi) + 1).collect(); + let mut closes: Vec = spans.iter().take(opens.len()).map(|&(_, hi)| u64::from(hi) + 1).collect(); closes.sort_unstable(); - // Records that have opened and not yet closed; the top is the latest declared one. + // Keys of the records that have opened and not yet closed; the top is the winning one. let mut open: BinaryHeap = BinaryHeap::new(); let mut cover: Vec<(u64, Option)> = Vec::new(); let (mut next_open, mut next_close) = (0, 0); @@ -981,16 +1038,16 @@ fn cover_index(ranges: &[BfRange]) -> Vec<(u64, Option)> { while let Some(&(lo, record)) = opens.get(next_open) && u64::from(lo) <= p { - open.push(record); + open.push(key(record)); next_open += 1; } while closes.get(next_close).is_some_and(|&end| end <= p) { next_close += 1; } - while open.peek().is_some_and(|&top| ranges.get(top as usize).is_none_or(|r| u64::from(r.hi) < p)) { + while open.peek().is_some_and(|&top| spans.get(key(top) as usize).is_none_or(|&(_, hi)| u64::from(hi) < p)) { open.pop(); } - let winner = open.peek().map(|&top| top as usize); + let winner = open.peek().map(|&top| key(top) as usize); if cover.last().map(|&(_, w)| w) != Some(winner) { cover.push((p, winner)); } @@ -1266,6 +1323,74 @@ mod tests { use super::*; + /// Span records read while `f` runs. + fn span_reads(f: impl FnOnce()) -> usize { + SPAN_READS.with(|c| c.set(0)); + f(); + SPAN_READS.with(|c| c.get()) + } + + /// A composite font with `encoding` as its CMap, `w` as its widths and `to_unicode`. + fn composite(doc: &mut Document, encoding: &str, w: Vec, to_unicode: &str) -> Dict { + let mut desc = Dict::new(); + desc.set(b"Subtype".to_vec(), Object::name("CIDFontType2")); + desc.set(b"W".to_vec(), Object::Array(w)); + let enc = Object::Ref(doc.add(Object::Stream(Stream::flate(Dict::new(), encoding.as_bytes())))); + let tu = Object::Ref(doc.add(Object::Stream(Stream::flate(Dict::new(), to_unicode.as_bytes())))); + font( + doc, + vec![ + ("Subtype", Object::name("Type0")), + ("Encoding", enc), + ("DescendantFonts", Object::Array(vec![Object::Dict(desc)])), + ("ToUnicode", tu), + ], + ) + } + + /// Many `/W` ranges that leave a code out cost a glyph of it a binary search, not a scan of them all. + #[test] + fn widths_of_a_code_no_range_covers_take_few_steps() { + let mut doc = Document::new_empty(); + let w: Vec = (1..=10_000i64).flat_map(|k| [Object::Int(2 * k), Object::Int(2 * k), Object::Int(500)]).collect(); + let f = composite(&mut doc, "", w, ""); + let m = Metrics::from_dict(&doc, &f); + assert_eq!(m.width(4), 0.5); + let reads = span_reads(|| { + for _ in 0..1000 { + assert_eq!(m.width(3), 1.0); + } + }); + assert!(reads <= 1000 * 64, "{reads} span reads"); + } + + /// Where spans overlap, the first-declared one wins, as a scan in declaration order finds it. + #[test] + fn overlapping_spans_resolve_to_the_first_declared() { + let spans = Spans::new(vec![(0, 10, 'a'), (5, 20, 'b'), (3, 3, 'c'), (30, 25, 'd'), (u32::MAX, u32::MAX, 'e')]); + let found = |code| spans.find(code).map(|s| s.2); + let want = [(0, Some('a')), (3, Some('a')), (10, Some('a')), (11, Some('b')), (20, Some('b')), (21, None), (27, None), (u32::MAX, Some('e'))]; + for (code, value) in want { + assert_eq!(found(code), value, "code {code}"); + } + } + + /// Checking the glyph of every explicit ToUnicode entry looks its code up once in the CID map. + #[test] + fn many_unicode_entries_look_up_their_cids_in_few_steps() { + let n = 2000; + let cidchars: String = (0..n).map(|k| format!("<{:04X}> {} ", 0x8000 + k, k + 1)).collect(); + let encoding = format!("1 begincodespacerange <0000> endcodespacerange {n} begincidchar {cidchars} endcidchar"); + let bfchars: String = (0..n).map(|k| format!("<{:04X}> <{:04X}> ", 0x1000 + k, 0x4E00 + k)).collect(); + let to_unicode = format!("{n} beginbfchar {bfchars} endbfchar"); + let mut doc = Document::new_empty(); + let f = composite(&mut doc, &encoding, Vec::new(), &to_unicode); + let reads = span_reads(|| { + Metrics::from_dict(&doc, &f); + }); + assert!(reads <= n * 64, "{reads} span reads"); + } + /// Every glyph of a sequential range builds its text from the destination, so a long one is cut /// to the CMap limit once, when it is read. #[test] @@ -1736,7 +1861,7 @@ mod tests { let map = to_unicode(b"1 beginbfrange <0001> <0003> [<0041> <0043>] endbfrange"); assert_eq!((map.get(&2), map.get(&3).map(String::as_str)), (None, Some("C"))); let mut m = Metrics::fallback(); - (_, m.cid_map) = parse_cmap(b"1 begincidrange <0000> 4294967295 endcidrange"); + m.cid_map = Spans::new(parse_cmap(b"1 begincidrange <0000> 4294967295 endcidrange").1); assert_eq!(m.cid(5), u32::MAX); let mut doc = Document::new_empty(); let mut enc = Dict::new(); From 5609bcbe37728d65186303b4c8817dbdb8e8c3b8 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:36:06 +0000 Subject: [PATCH 05/20] fix(fonts): wrap text measuring each word once Line wrapping built and measured the whole candidate line for every word, so a long field value or annotation text at a tiny font size, with many words on one line, took quadratic time and copying. The wrapper keeps the line's width as it goes and measures each word, space and broken character once. --- crates/fonts/src/lib.rs | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/crates/fonts/src/lib.rs b/crates/fonts/src/lib.rs index ec103ed5f..37a2ea74d 100644 --- a/crates/fonts/src/lib.rs +++ b/crates/fonts/src/lib.rs @@ -88,17 +88,28 @@ pub fn wrap(text: &str, size: f64, width: f64) -> Vec { } /// [`wrap`], measuring with `measure` (for example [`cjk_width`]). +/// +/// A line measures as the sum of its words, the spaces between them and the characters of a broken +/// word, each measured once, so the work stays linear in `text` however much fits on a line. pub fn wrap_with(text: &str, size: f64, width: f64, measure: impl Fn(&str, f64) -> f64) -> Vec { let mut lines = Vec::new(); + let space = measure(" ", size); + let mut buf = [0u8; 4]; // A CRLF pair is one break, a lone CR or LF is one break each. for para in text.replace("\r\n", "\n").split(['\n', '\r']) { let mut line = String::new(); + let mut line_width = 0.0; // No word is on the line yet; an empty word (a leading space) still starts it. let mut fresh = true; for word in para.split(' ') { - let candidate = if fresh { word.to_string() } else { format!("{line} {word}") }; - if measure(&candidate, size) <= width || fresh && measure(word, size) <= width { - line = candidate; + let word_width = measure(word, size); + let candidate = if fresh { word_width } else { line_width + space + word_width }; + if candidate <= width { + if !fresh { + line.push(' '); + } + line.push_str(word); + line_width = candidate; fresh = false; continue; } @@ -108,11 +119,15 @@ pub fn wrap_with(text: &str, size: f64, width: f64, measure: impl Fn(&str, f64) } else { lines.push(std::mem::take(&mut line)); } + line_width = 0.0; for ch in word.chars() { - if !line.is_empty() && measure(&format!("{line}{ch}"), size) > width { + let ch_width = measure(ch.encode_utf8(&mut buf), size); + if !line.is_empty() && line_width + ch_width > width { lines.push(std::mem::take(&mut line)); + line_width = 0.0; } line.push(ch); + line_width += ch_width; } fresh = line.is_empty(); } @@ -442,4 +457,20 @@ mod tests { let lines = wrap_with("日本語のテキストは文字単位で折り返されます", 10.0, 45.0, cjk_width); assert!(lines.len() > 3 && lines.iter().all(|l| cjk_width(l, 10.0) <= 45.0), "{lines:?}"); } + + /// Wrapping measures each word and character a bounded number of times, however many fit on a line. + #[test] + fn wrapping_measures_each_word_a_few_times() { + let measured = std::cell::Cell::new(0usize); + let measure = |s: &str, size: f64| { + measured.set(measured.get() + s.len()); + helvetica_width(s, size) + }; + let text = format!("{} {}", "word ".repeat(5_000), "x".repeat(5_000)); + let lines = wrap_with(&text, 1.0, 13_000.0, measure); + assert_eq!(lines.len(), 2, "the words fit on one line, the long word on another"); + assert!(measured.get() <= 4 * text.len(), "{} bytes measured", measured.get()); + let narrow = wrap_with(&text, 10.0, 100.0, helvetica_width); + assert!(narrow.iter().all(|l| helvetica_width(l, 10.0) <= 100.0), "{narrow:?}"); + } } From fcedb156893891c718fca03dd27c9a04ba5ae110 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:36:48 +0000 Subject: [PATCH 06/20] fix(export): find font size and colour slots through a map The body-size tally and the RTF colour table searched every slot seen so far for each block, and the RTF writer searched the colour table again for every paragraph and cell, so a document with many distinct sizes or colours made export quadratic. Both keep their first-seen order and look slots up in a hash map. --- crates/export/src/lib.rs | 75 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 5 deletions(-) diff --git a/crates/export/src/lib.rs b/crates/export/src/lib.rs index f3030ad58..8c3999712 100644 --- a/crates/export/src/lib.rs +++ b/crates/export/src/lib.rs @@ -8,6 +8,8 @@ #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] +use std::collections::HashMap; + mod pptx; mod zip; @@ -85,14 +87,33 @@ enum Item<'a> { PageBreak, } +#[cfg(test)] +thread_local! { + /// Entries read to find a font size's or a colour's slot, for the tests that keep it constant. + static LOOKUP_STEPS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Counts `n` entries read by a lookup (tests only). +#[inline] +fn count_lookup_steps(_n: usize) { + #[cfg(test)] + LOOKUP_STEPS.with(|c| c.set(c.get().saturating_add(_n))); +} + /// The body text size: the size most of the document's characters are set in. fn body_size(pages: &[Page]) -> f64 { + // Sizes in first-seen order (which breaks ties), and where each one is in it. let mut by: Vec<(i64, usize)> = Vec::new(); + let mut slot: HashMap = HashMap::new(); for b in pages.iter().flat_map(|p| &p.blocks) { let k = (b.size * 2.0).round() as i64; - match by.iter_mut().find(|e| e.0 == k) { - Some(e) => e.1 += b.text.len(), - None => by.push((k, b.text.len())), + count_lookup_steps(1); + let i = *slot.entry(k).or_insert_with(|| { + by.push((k, 0)); + by.len() - 1 + }); + if let Some(e) = by.get_mut(i) { + e.1 = e.1.saturating_add(b.text.len()); } } by.into_iter().max_by_key(|e| e.1).map_or(11.0, |e| e.0 as f64 / 2.0) @@ -831,9 +852,12 @@ pub fn rtf(pages: &[Page]) -> String { let items = items(pages); // The colour table: entry 0 is the default (auto) colour, then each non-black colour once. let mut colors: Vec<[u8; 3]> = Vec::new(); + let mut slot: HashMap<[u8; 3], usize> = HashMap::new(); let mut note = |c: [f64; 3]| { let c = rgb8(c); - if c != [0, 0, 0] && !colors.contains(&c) { + count_lookup_steps(1); + if c != [0, 0, 0] && !slot.contains_key(&c) { + slot.insert(c, colors.len()); colors.push(c); } }; @@ -847,7 +871,8 @@ pub fn rtf(pages: &[Page]) -> String { // `\cfN` for a colour (nothing for black, which keeps the default). let cf = |c: [f64; 3]| { let c = rgb8(c); - colors.iter().position(|e| *e == c).map_or(String::new(), |i| format!("\\cf{}", i + 1)) + count_lookup_steps(1); + slot.get(&c).map_or(String::new(), |i| format!("\\cf{}", i + 1)) }; let mut s = String::from("{\\rtf1\\ansi\\deff0{\\fonttbl{\\f0 Helvetica;}}\n"); if !colors.is_empty() { @@ -911,6 +936,46 @@ pub fn rtf(pages: &[Page]) -> String { mod tests { use super::*; + /// A page of `n` one-word blocks, each in its own size and colour. + fn varied_page(n: usize) -> Page { + let blocks = (0..n) + .map(|i| Block { + text: "word".into(), + rect: [72.0, i as f64, 100.0, i as f64 + 1.0], + size: 4.0 + i as f64 * 0.5, + bold: false, + italic: false, + color: [(i % 256) as f64 / 255.0, (i / 256 % 256) as f64 / 255.0, 0.5], + rotation: 0.0, + }) + .collect(); + Page { width: 612.0, height: 792.0, blocks, images: Vec::new(), background: None } + } + + fn lookup_steps(f: impl FnOnce()) -> usize { + LOOKUP_STEPS.with(|c| c.set(0)); + f(); + LOOKUP_STEPS.with(|c| c.get()) + } + + #[test] + fn many_distinct_sizes_are_counted_in_constant_steps_each() { + let pages = [varied_page(3000)]; + let steps = lookup_steps(|| { + body_size(&pages); + }); + assert!(steps <= 2 * 3000, "{steps} steps"); + } + + #[test] + fn many_distinct_colours_take_constant_steps_each() { + let pages = [varied_page(3000)]; + let mut out = String::new(); + let steps = lookup_steps(|| out = rtf(&pages)); + assert!(steps <= 4 * 3000, "{steps} steps"); + assert!(out.contains("\\cf3000 ") || out.contains("\\cf3000\\"), "the last colour has the last slot"); + } + fn page() -> Page { let b = |t: &str, y: f64, size: f64, bold: bool| Block { text: t.into(), From dba51fc8237dd04b5cd594d5fa56106e902d74b8 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:37:39 +0000 Subject: [PATCH 07/20] fix(optimize): refuse a JPEG whose own size differs from its dictionary's The pixel reader capped the image size the dictionary gives, then decoded the JPEG at the size its header gives and compared afterwards, so a small image dictionary over a huge JPEG allocated the huge one first. The reader takes the size from the JPEG header and refuses a mismatch before decoding. --- crates/optimize/src/images.rs | 40 ++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/crates/optimize/src/images.rs b/crates/optimize/src/images.rs index 6182b0892..1d8a59e25 100644 --- a/crates/optimize/src/images.rs +++ b/crates/optimize/src/images.rs @@ -227,6 +227,19 @@ fn is_dct(s: &Stream) -> Option { } } +#[cfg(test)] +thread_local! { + /// Pixels decoded from JPEG data, for the test that keeps a mismatched JPEG from decoding. + static DECODED_PIXELS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Counts `n` decoded pixels (tests only). +#[inline] +fn count_decoded(_n: u64) { + #[cfg(test)] + DECODED_PIXELS.with(|c| c.set(c.get().saturating_add(_n))); +} + /// 8-bit samples of an image (`n` components), decoded. pub(crate) fn pixels(s: &Stream, n: usize) -> Option> { let (w, h) = (u32::try_from(s.dict.int(b"Width")?).ok()?, u32::try_from(s.dict.int(b"Height")?).ok()?); @@ -235,7 +248,15 @@ pub(crate) fn pixels(s: &Stream, n: usize) -> Option> { } let bytes = usize::try_from(w).ok()?.checked_mul(usize::try_from(h).ok()?)?.checked_mul(n)?; if is_dct(s)? { - let img = image::load_from_memory_with_format(&s.raw, image::ImageFormat::Jpeg).ok()?; + use image::ImageDecoder as _; + // The JPEG's own size is what gets decoded: one other than the dictionary's is refused from + // its header, before any pixels are. + let decoder = image::codecs::jpeg::JpegDecoder::new(std::io::Cursor::new(&s.raw[..])).ok()?; + if decoder.dimensions() != (w, h) { + return None; + } + let img = image::DynamicImage::from_decoder(decoder).ok()?; + count_decoded(u64::from(img.width()) * u64::from(img.height())); // Adobe CMYK/YCCK JPEGs aren't handled (the colour space check excludes them). let v = if n == 1 { img.into_luma8().into_raw() } else { img.into_rgb8().into_raw() }; (v.len() == bytes).then_some(v) @@ -432,4 +453,21 @@ mod dimension_tests { let stream = Stream::from_raw(dict, vec![1, 2, 3, 4, 5]); assert_eq!(pixels(&stream, 1), Some(vec![1, 2, 3, 4])); } + + /// A JPEG larger than its dictionary says is refused from its header, before its pixels are decoded. + #[test] + fn a_jpeg_of_another_size_than_its_dictionary_is_not_decoded() { + let data = jpeg(&vec![128; 1000 * 1000], 1, 1000, 1000, 50).unwrap_or_default(); + let stream = |w: i64, h: i64| { + let mut dict = Dict::new(); + dict.set(b"Width".to_vec(), Object::Int(w)); + dict.set(b"Height".to_vec(), Object::Int(h)); + dict.set(b"Filter".to_vec(), Object::name("DCTDecode")); + Stream::from_raw(dict, data.clone()) + }; + DECODED_PIXELS.with(|c| c.set(0)); + assert!(pixels(&stream(10, 10), 1).is_none()); + assert_eq!(DECODED_PIXELS.with(|c| c.get()), 0, "pixels decoded"); + assert_eq!(pixels(&stream(1000, 1000), 1).map(|p| p.len()), Some(1000 * 1000)); + } } From 6b62585339b351d1bf8827377aa01117d5a1b4f7 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:39:09 +0000 Subject: [PATCH 08/20] fix(optimize): bound the search for fragment runs on a page A run of images that is not one picture was searched again from each of its groups, collecting the rest of the run every time, so many images drawn over one another made a page quadratic. The search reads at most 16 groups per content operator (at least 65 536) and leaves the rest of the page as it is once that is spent. --- crates/optimize/src/fragments.rs | 55 ++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/crates/optimize/src/fragments.rs b/crates/optimize/src/fragments.rs index caf617f71..44e165546 100644 --- a/crates/optimize/src/fragments.rs +++ b/crates/optimize/src/fragments.rs @@ -29,6 +29,19 @@ pub const MIN_FRAGMENTS: usize = 4; /// Most pixels a merged image may have (memory). const MAX_PIXELS: u64 = 80_000_000; +/// Fragment groups a page's search for runs may read per content operator, and at least. A run +/// that is not one picture is searched again from each of its groups, so without a limit images +/// drawn over one another take time quadratic in their number; past it, the rest of the page is +/// left as it is. +const SCAN_PER_OP: usize = 16; +const MIN_SCAN: usize = 1 << 16; + +#[cfg(test)] +thread_local! { + /// Fragment groups read, for the test that keeps a page's search linear in its content. + static GROUP_READS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// One fragment: its image, its box in the coordinates the run is drawn in, and its size. struct Fragment { image: ObjRef, @@ -40,6 +53,8 @@ struct Fragment { /// The ops `q`, one or more `cm`, `Do`, `Q` starting at `i`: the index after them, the matrix /// and the name drawn. fn group_at(ops: &[Op], i: usize) -> Option<(usize, Matrix, Vec)> { + #[cfg(test)] + GROUP_READS.with(|c| c.set(c.get().saturating_add(1))); if !ops.get(i)?.is("q") { return None; } @@ -226,7 +241,13 @@ fn merge_page(doc: &mut Document, page: ObjRef, report: &mut Report, progress: & let ops = parsed.ops; let mut out: Vec = Vec::with_capacity(ops.len()); let (mut i, mut merged, mut k) = (0, 0, 0); + let budget = ops.len().saturating_mul(SCAN_PER_OP).max(MIN_SCAN); + let mut scanned = 0usize; while i < ops.len() { + if scanned > budget { + out.extend(ops.get(i..).unwrap_or_default().iter().cloned()); + break; + } // The longest run of fragment groups from here with one colour space. let mut run = Vec::new(); let mut kind: Option<(Object, usize)> = None; @@ -245,10 +266,12 @@ fn merge_page(doc: &mut Document, page: ObjRef, report: &mut Report, progress: & run.push(Fragment { image, rect: [e, f, e + a, f + d], width, height }); j = next; } + scanned = scanned.saturating_add(run.len()).saturating_add(1); // Not one picture as a whole: the rows drawn one under the other from the start may be. if run.len() >= MIN_FRAGMENTS && !stacked(&mut run) { run = drawn_in_sequence(&ops, i, &xobjects, doc); j = run_end(&ops, i, run.len()); + scanned = scanned.saturating_add(run.len().saturating_mul(2)).saturating_add(2); } let Some((cs, n)) = kind.filter(|_| run.len() >= MIN_FRAGMENTS && stacked(&mut run)) else { // Not a run: keep this op and look again from the next one. @@ -304,6 +327,8 @@ pub(crate) fn run(doc: &mut Document, pages: &[ObjRef], report: &mut Report, pro #[cfg(test)] mod tests { + use pdfcraft_cos::{Dict, Document, Object, Stream}; + use super::*; #[test] @@ -318,4 +343,34 @@ mod tests { ]; assert!(merge(&Document::new_empty(), &run, 3).is_none()); } + + /// Many images drawn on top of each other are not one picture; looking for runs among them reads + /// a bounded number of groups per operator, not the rest of the page from every group. + #[test] + fn rejected_runs_read_a_bounded_number_of_groups() { + let mut doc = Document::new_empty(); + let mut image = Dict::new(); + for (k, v) in + [("Subtype", Object::name("Image")), ("Width", Object::Int(1)), ("Height", Object::Int(1)), ("BitsPerComponent", Object::Int(8))] + { + image.set(k.as_bytes().to_vec(), v); + } + image.set(b"ColorSpace".to_vec(), Object::name("DeviceGray")); + let image = doc.add(Object::Stream(Stream::from_raw(image, vec![0]))); + let groups = 4000; + let content = doc.add(Object::Stream(Stream::flate(Dict::new(), "q 10 0 0 10 0 0 cm /Im Do Q\n".repeat(groups).as_bytes()))); + let mut xobjects = Dict::new(); + xobjects.set(b"Im".to_vec(), Object::Ref(image)); + let mut resources = Dict::new(); + resources.set(b"XObject".to_vec(), Object::Dict(xobjects)); + let mut page = Dict::new(); + page.set(b"Contents".to_vec(), Object::Ref(content)); + page.set(b"Resources".to_vec(), Object::Dict(resources)); + let page = doc.add(Object::Dict(page)); + GROUP_READS.with(|c| c.set(0)); + let merged = merge_page(&mut doc, page, &mut Report::default(), &mut |_| true); + assert_eq!(merged.ok(), Some(0)); + let reads = GROUP_READS.with(|c| c.get()); + assert!(reads <= 40 * 4 * groups, "{reads} group reads"); + } } From 3e968c8aa24adfb729c6d45a3c98abf5fed07027 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 17:36:35 +0000 Subject: [PATCH 09/20] chore(a11y, fonts): satisfy clippy in the CID width ranges and a test helper --- crates/a11y/src/tests.rs | 2 +- crates/fonts/src/pdf.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/a11y/src/tests.rs b/crates/a11y/src/tests.rs index a24f59edb..ad5747736 100644 --- a/crates/a11y/src/tests.rs +++ b/crates/a11y/src/tests.rs @@ -1033,7 +1033,7 @@ fn mark_reads(f: impl FnOnce()) -> usize { /// Page content nesting `depth` unmarked `BMC` blocks around a marked one, with `ops` operators inside. fn deeply_nested_page(doc: &mut Document, depth: usize, ops: usize) -> pdfcraft_cos::Dict { use pdfcraft_cos::{Dict, Stream}; - let mut data = "/Figure << /MCID 0 >> BDC ".repeat(1); + let mut data = "/Figure << /MCID 0 >> BDC ".to_string(); data.push_str(&"/X BMC ".repeat(depth)); data.push_str(&"0 0 1 1 re f ".repeat(ops)); data.push_str(&"EMC ".repeat(depth + 1)); diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index 4a118a07d..3ddd16a9b 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -714,10 +714,10 @@ impl Metrics { i += 2; } Some(c1) => { - if let (Some(c1), Some(v)) = (c1.as_f64(), w.get(i + 2).and_then(|x| doc.resolve(x).as_f64())) { - if ranges.len() < MAX_CMAP_ENTRIES { - ranges.push((c0, c1 as u32, v)); - } + if let (Some(c1), Some(v)) = (c1.as_f64(), w.get(i + 2).and_then(|x| doc.resolve(x).as_f64())) + && ranges.len() < MAX_CMAP_ENTRIES + { + ranges.push((c0, c1 as u32, v)); } i += 3; } From f42fb0ad49bc729fbfa8ec492dae592f8ec89bed Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:43:01 +0000 Subject: [PATCH 10/20] fix(forms): a deleted field leaves the calculation order Deleting a field took it off its pages and out of the field tree but left its entry in the form's /CO array. That reference kept the field dictionary, its value and its widgets reachable, so a full save after applying redactions over the field still wrote the redacted value. --- crates/forms/src/author.rs | 25 +++++++++++++++++ crates/redact/src/tests.rs | 1 + .../redact/src/tests/surviving_references.rs | 28 +++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 crates/redact/src/tests/surviving_references.rs diff --git a/crates/forms/src/author.rs b/crates/forms/src/author.rs index f5178a687..544d0a7f8 100644 --- a/crates/forms/src/author.rs +++ b/crates/forms/src/author.rs @@ -1280,6 +1280,31 @@ pub fn delete_field(doc: &mut Document, name: &str) -> Result<(), FormError> { } } } + // Out of the calculation order too: a `/CO` entry would keep the field, its value and its + // widgets reachable for a full save. + drop_from_calculation_order(doc, |r| r == f.obj || widgets.contains(&r)) +} + +/// Remove the `/CO` entries `gone` matches, whether the form and the array are held directly or +/// by reference. +fn drop_from_calculation_order(doc: &mut Document, gone: impl Fn(ObjRef) -> bool) -> Result<(), FormError> { + let Some(root) = doc.root() else { return Ok(()) }; + let Some(af_obj) = doc.get(root).as_dict().and_then(|d| d.get(b"AcroForm").cloned()) else { return Ok(()) }; + let Some(mut af) = doc.resolve(&af_obj).as_dict().cloned() else { return Ok(()) }; + let Some(co_obj) = af.get(b"CO").cloned() else { return Ok(()) }; + let co = doc.resolve(&co_obj).as_array().cloned().unwrap_or_default(); + let kept: Vec = co.iter().filter(|o| !o.as_ref().is_some_and(&gone)).cloned().collect(); + if kept.len() == co.len() { + return Ok(()); + } + match (co_obj.as_ref(), af_obj.as_ref()) { + (Some(r), _) if doc.get(r).as_array().is_some() => doc.set(r, Object::Array(kept)), + (_, Some(af_ref)) => doc.update_dict(af_ref, |d| d.set(b"CO".to_vec(), Object::Array(kept)))?, + (_, None) => { + af.set(b"CO".to_vec(), Object::Array(kept)); + doc.update_dict(root, |d| d.set(b"AcroForm".to_vec(), Object::Dict(af)))?; + } + } Ok(()) } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index a9902495d..52d80eb26 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2462,6 +2462,7 @@ fn unused_descriptions_in_shared_or_inherited_resources_are_retired() { } mod property_retirement; +mod surviving_references; /// `[(FIRSTARR)] [(LASTARR)] TJ` is malformed (a surplus array operand), but the operator still /// draws only the one immediately before it (LASTARR, 7 glyphs, 35 pt wide from x = 10) -- not diff --git a/crates/redact/src/tests/surviving_references.rs b/crates/redact/src/tests/surviving_references.rs new file mode 100644 index 000000000..cfe6d87bb --- /dev/null +++ b/crates/redact/src/tests/surviving_references.rs @@ -0,0 +1,28 @@ +//! Redacted content must leave the file: no other reference may keep it reachable by a full save. + +use super::*; + +/// Whether `secret` survives a full save of `doc`, in the file's bytes or in a decoded stream. +fn full_save_holds(doc: &Document, secret: &[u8]) -> bool { + let bytes = write_full(doc, &SaveOptions::default()).unwrap(); + let found = |data: &[u8]| data.windows(secret.len()).any(|w| w == secret); + if found(&bytes) { + return true; + } + let saved = Document::open(Arc::new(bytes)).unwrap(); + saved.scan_objects().into_iter().any(|(_, o)| matches!(&*o, Object::Stream(s) if s.decoded().is_ok_and(|d| found(&d)))) +} + +#[test] +fn a_redacted_field_listed_in_the_calculation_order_leaves_the_saved_file() { + let mut doc = one_page(b"", "", vec![]); + pdfcraft_forms::add_field(&mut doc, 0, [10.0, 50.0, 100.0, 70.0], &pdfcraft_forms::NewField::Text { multiline: false }, Some("ssn")).unwrap(); + pdfcraft_forms::set_value(&mut doc, "ssn", &pdfcraft_forms::FieldValue::Text("SSN-078-05-1120".into())).unwrap(); + let field = pdfcraft_forms::fields(&doc).swap_remove(0).obj; + let root = doc.root().unwrap(); + let af = doc.get(root).as_dict().unwrap().reference(b"AcroForm").unwrap(); + doc.update_dict(af, |d| d.set(b"CO".to_vec(), Object::Array(vec![Object::Ref(field)]))).unwrap(); + mark(&mut doc, 0, &[[0.0, 40.0, 120.0, 80.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().fields, 1); + assert!(!full_save_holds(&doc, b"SSN-078-05-1120"), "the field's value is still reachable"); +} From 6ca4431b50a21760c7c9c9378b9d01a05acf140f Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:46:23 +0000 Subject: [PATCH 11/20] fix(redact): check every input content stream, whatever it calls itself Both checks for text left under applied redaction marks (redaction's own verify pass and the engine's renderer cross-check) skipped any page content stream whose dictionary said /PCMark /Redaction, so that the overlay's label would not count. An input file could put that label on its own content stream and keep it out of both checks. Applying redactions now records the overlay streams it adds (stamp returns the new stream), and only those are skipped. --- crates/edit/src/lib.rs | 15 +++++++++------ crates/engine/src/lib.rs | 15 ++++++++++----- crates/engine/src/tests.rs | 34 +++++++++++++++++++++++++++++++++- crates/redact/src/lib.rs | 16 ++++++++-------- 4 files changed, 60 insertions(+), 20 deletions(-) diff --git a/crates/edit/src/lib.rs b/crates/edit/src/lib.rs index d3db00fe3..bef515f98 100644 --- a/crates/edit/src/lib.rs +++ b/crates/edit/src/lib.rs @@ -17,7 +17,7 @@ #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] -use pdfcraft_cos::{Dict, Document, Object, Stream}; +use pdfcraft_cos::{Dict, Document, ObjRef, Object, Stream}; use pdfcraft_fonts::{helvetica_width, literal, win_ansi}; #[derive(Debug, thiserror::Error, PartialEq)] @@ -601,7 +601,7 @@ fn isolation(doc: &Document, list: &[Object]) -> Isolation { /// Put a mark's content on a page: behind (with the behind-marks at the start) or on top /// (appended, after wrapping the original content in q/Q). fn place(doc: &mut Document, page: &pdfcraft_model::Page, kind: MarkKind, content: Vec, behind: bool) -> Result<(), EditError> { - place_tagged(doc, page, kind.tag(), content, behind) + place_tagged(doc, page, kind.tag(), content, behind).map(drop) } /// Put content on a page, tagged `tag` (see [`place`]). On top, whatever graphics state the @@ -609,7 +609,8 @@ fn place(doc: &mut Document, page: &pdfcraft_model::Page, kind: MarkKind, conten /// 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. -fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: &str, content: Vec, behind: bool) -> Result<(), EditError> { +/// 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. let behind_kinds = [MarkKind::Background.tag(), MarkKind::Watermark.tag()]; @@ -625,7 +626,8 @@ fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: &str, cont Isolation::Unknown { unclosed, marked } => (behind_marks < list.len()).then_some((behind_marks, unclosed, marked)), } }; - let mark = Object::Ref(doc.add(Object::Stream(Stream::flate(tagged(tag), &content)))); + let mark_ref = doc.add(Object::Stream(Stream::flate(tagged(tag), &content))); + let mark = Object::Ref(mark_ref); if behind { // Right after the backgrounds already there, so a new background is painted over the // older ones (#3591) and a watermark over all of them. Not after the watermarks: on a @@ -646,7 +648,7 @@ fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: &str, cont list.push(mark); } doc.update_dict(page.obj, |d| d.set(b"Contents".to_vec(), Object::Array(list)))?; - Ok(()) + Ok(mark_ref) } /// Put content tagged `tag` first inside the q/Q wrapper, so it applies to the page's original @@ -664,7 +666,8 @@ fn place_inside_wrapper(doc: &mut Document, page: &pdfcraft_model::Page, tag: &s /// Draw `content` on top of a page (0-based) as permanent page content, with standard Helvetica /// available as `/PCHelv`. The stream is tagged `tag` (not a mark kind, so Remove never takes /// it away); the original content is wrapped in q/Q first. Used for applied redaction boxes. -pub fn stamp(doc: &mut Document, page: usize, tag: &str, content: Vec) -> Result<(), EditError> { +/// Returns the new content stream. +pub fn stamp(doc: &mut Document, page: usize, tag: &str, content: Vec) -> Result { let all = page_list(doc); check(&[page], all.len())?; add_resources(doc, &all[page], None, Some(&content))?; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 817eb79a9..32d3a1f15 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -989,12 +989,13 @@ fn recovered_render_password(cos: &pdfcraft_cos::Document) -> Option { /// that it reads differently), a mark drawn over the text as displayed lies over glyphs the /// interpreter believes are elsewhere: they stay in the file and nothing notices. The renderer is /// what the user saw when placing the marks, so it is asked. `after` is written as a save would -/// write it, without the marks' own overlay (the box and its label are drawn on purpose), its +/// write it, without the marks' own `overlays` (the box and its label are drawn on purpose), its /// text is extracted, and a glyph still mostly inside a mark fails the edit: the caller keeps /// the document as it was. fn shown_under_redactions( after: &pdfcraft_cos::Document, marks: &[pdfcraft_redact::Mark], + overlays: &[pdfcraft_cos::ObjRef], pages: &[pdfcraft_render::PageInfo], password: Option<&str>, ) -> Result<(), EditError> { @@ -1011,8 +1012,7 @@ fn shown_under_redactions( for &pi in &marked { let Some(page) = model.get(pi) else { continue }; let Some(Object::Array(list)) = page.dict.get(b"Contents").map(|c| (*shown.resolve(c)).clone()) else { continue }; - let kept: Vec = - list.into_iter().filter(|o| shown.resolve(o).as_dict().and_then(|d| d.name(b"PCMark")) != Some(b"Redaction")).collect(); + let kept: Vec = list.into_iter().filter(|o| !o.as_ref().is_some_and(|r| overlays.contains(&r))).collect(); shown.update_dict(page.obj, |d| d.set(b"Contents".to_vec(), Object::Array(kept))).map_err(|e| EditError::Write(e.to_string()))?; } let bytes = write_incremental(&shown, &SaveOptions::default()).map_err(|e| EditError::Write(e.to_string()))?; @@ -2356,6 +2356,9 @@ struct EditCtx { /// Fields the edit renamed to keep imported names separate (old → new), for the repair /// log and the undo label. renamed_fields: Vec<(String, String)>, + /// The overlay streams the edit's applied redactions added, which alone the renderer's check + /// leaves out. + redaction_overlays: Vec, } impl EditCtx { @@ -2382,6 +2385,7 @@ impl EditCtx { seed, count: 0, renamed_fields: Vec::new(), + redaction_overlays: Vec::new(), } } @@ -2736,7 +2740,7 @@ fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut EditCtx) -> pdfcraft_edit::update_content(doc, *page, *index, &AddedContent::Image(pdfcraft_edit::AddedImage { image, ..old }))?; } Edit::ApplyRedactions { pages } => { - pdfcraft_redact::apply(doc, pages.as_deref())?; + cx.redaction_overlays = pdfcraft_redact::apply(doc, pages.as_deref())?.overlays; } Edit::ClearRedactions => { pdfcraft_redact::clear_marks(doc, None)?; @@ -3798,6 +3802,7 @@ impl Session { cx.js = Some(js::JsRunner::new(&next, &name)); } cx.renamed_fields.clear(); + cx.redaction_overlays.clear(); // `next` is a copy: if the edit fails or crashes, the document is unchanged. guard(|| run_edit(&mut next, &edit, &mut cx)) .unwrap_or_else(|m| Err(EditError::Invalid(format!("{} failed unexpectedly ({m}); the document was not changed", edit.label()))))?; @@ -3806,7 +3811,7 @@ impl Session { if let Edit::ApplyRedactions { pages } = &edit { let marks: Vec = pdfcraft_redact::marks(&editor.cos).into_iter().filter(|m| pages.as_ref().is_none_or(|p| p.contains(&m.page))).collect(); - guard(|| shown_under_redactions(&next, &marks, &doc.info.pages, doc.password.as_deref())) + guard(|| shown_under_redactions(&next, &marks, &cx.redaction_overlays, &doc.info.pages, doc.password.as_deref())) .unwrap_or_else(|m| Err(EditError::Invalid(format!("{} failed unexpectedly ({m}); the document was not changed", edit.label()))))?; } // Renamed fields are said, not slipped in: a repair-log note (with the caveat that diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 3d347e649..958204bc5 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2253,12 +2253,17 @@ fn setting_the_calculation_order_recalculates_and_undoes() { /// One 612×300 page with the content stream `body`. Object 4 is Helvetica; `resources` is what /// the page's /Resources holds. fn one_line_page(resources: &str, body: &str) -> Vec { + one_line_page_with(resources, "", body) +} + +/// [`one_line_page`] with `stream_entries` added to the content stream's dictionary. +fn one_line_page_with(resources: &str, stream_entries: &str, body: &str) -> Vec { let objs = [ "<< /Type /Catalog /Pages 2 0 R >>".to_string(), "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 300] >>".to_string(), format!("<< /Type /Page /Parent 2 0 R /Resources << {resources} >> /Contents 5 0 R >>"), "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".to_string(), - format!("<< /Length {} >>\nstream\n{body}\nendstream", body.len()), + format!("<< /Length {} {stream_entries} >>\nstream\n{body}\nendstream", body.len()), ]; let mut out = b"%PDF-1.7\n".to_vec(); let mut offsets = Vec::new(); @@ -2316,6 +2321,33 @@ fn redaction_is_refused_when_the_page_still_shows_text_under_a_mark() { assert!(shown.starts_with("Public words then") && shown.ends_with("and more") && !shown.contains("SECRET42"), "{shown}"); } +/// Only the overlay a redaction adds is left out of the renderer's check. A page whose own content +/// stream calls itself a redaction overlay (`/PCMark /Redaction`) is checked like any other. +#[test] +fn a_content_stream_labelled_as_a_redaction_overlay_is_still_checked() { + let line = "Public words then SECRET42 and more"; + let mut s = Session::new().with_clock(|| 1_700_000_000); + let forged = one_line_page_with( + "/ExtGState << /GS1 << /Type /ExtGState /Font [4 0 R 16] >> >>", + "/PCMark /Redaction", + &format!("BT /GS1 gs 20 150 Td ({line}) Tj ET"), + ); + let id = s.open("forged.pdf", None, Arc::new(forged), None).unwrap(); + mark_shown_text(&mut s, id, "SECRET42"); + let applied = s.apply(id, Edit::ApplyRedactions { pages: None }); + if applied.is_ok() { + assert!(!page_texts(&s, id).concat().contains("SECRET42"), "the page still shows the redacted word"); + let saved = s.save_bytes(id).unwrap(); + let held = |d: &[u8]| d.windows(8).any(|w| w == b"SECRET42"); + assert!(!held(&saved), "the saved file holds the redacted word"); + let reopened = pdfcraft_cos::Document::open(saved).unwrap(); + assert!( + !reopened.scan_objects().any(|(_, o)| matches!(&*o, pdfcraft_cos::Object::Stream(st) if st.decoded().is_ok_and(|d| held(&d)))), + "a saved stream holds the redacted word" + ); + } +} + #[test] fn redaction_marks_apply_for_good_and_undo() { let (mut s, id) = session_with(2); diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 968773b91..97d5bcd9f 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -143,6 +143,10 @@ pub struct Report { /// The form was an AcroForm+XFA form and its `/XFA` packets (which still held redacted field /// values, and from which a relayout could rebuild them) were removed. pub xfa_removed: bool, + /// The content streams this apply added to draw the marks' boxes and labels. They are the + /// only streams a check for text left under the marks may skip: an input stream labelled + /// like an overlay (`/PCMark /Redaction`) is checked like any other. + pub overlays: Vec, } fn rect_of(doc: &Document, o: Option<&Object>) -> Option<[f64; 4]> { @@ -576,7 +580,7 @@ pub fn apply(doc: &mut Document, pages: Option<&[usize]>) -> Result) -> Result = page_marks.iter().flat_map(|m| m.rects.iter().copied()).collect(); let (list, data) = page_streams(doc, &page.dict, pi)?; - // The overlay stream (last) draws text of its own: the overlay label is allowed. - let original: Vec> = list - .iter() - .zip(data) - .filter(|(o, _)| doc.resolve(o).as_dict().and_then(|d| d.name(b"PCMark")) != Some(b"Redaction")) - .map(|(_, d)| d) - .collect(); + // The overlay stream draws text of its own: the overlay label is allowed. + let original: Vec> = + list.iter().zip(data).filter(|(o, _)| !o.as_ref().is_some_and(|r| report.overlays.contains(&r))).map(|(_, d)| d).collect(); let resources = page.dict.get(b"Resources").and_then(|r| doc.resolve(r).as_dict().cloned()).unwrap_or_default(); let mut scratch = Report::default(); let mut scope = Scope::new(&rects, Mode::Verify, &mut scratch); From 22aa6d5f9afb7339a19fa55b792b96f7c7910c37 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:50:59 +0000 Subject: [PATCH 12/20] fix(redact): place glyphs with the inherited and ExtGState-set text state The redaction interpreter started every form XObject with an empty text state (no font, size 0, no spacing) although a form inherits its caller's graphics state, and it ignored the /Font entry of an ExtGState set by gs. Text shown that way was placed at the wrong positions, so removal and the verify pass could both miss glyphs under a mark. Forms now start from the caller's state, and gs sets the font and size as Tf does. The engine's renderer cross-check test used the gs case as its example of interpreter/renderer disagreement; it now runs the check directly on an unredacted page, which also covers an input stream labelled as a redaction overlay. --- crates/engine/src/lib.rs | 2 +- crates/engine/src/tests.rs | 78 +++++++++++++-------------- crates/redact/src/interp.rs | 51 ++++++++++++++++-- crates/redact/src/tests.rs | 1 + crates/redact/src/tests/text_state.rs | 27 ++++++++++ 5 files changed, 111 insertions(+), 48 deletions(-) create mode 100644 crates/redact/src/tests/text_state.rs diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 32d3a1f15..f257865d3 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -985,7 +985,7 @@ fn recovered_render_password(cos: &pdfcraft_cos::Document) -> Option { /// /// Redaction decides what lies under a mark from the positions its own interpreter computes, and /// its verify pass reuses them. Where they differ from the renderer's (a standard font without -/// /Widths, whose advances the interpreter approximates; a CMap, or a font set outside `Tf`, +/// /Widths, whose advances the interpreter approximates; a CMap /// that it reads differently), a mark drawn over the text as displayed lies over glyphs the /// interpreter believes are elsewhere: they stay in the file and nothing notices. The renderer is /// what the user saw when placing the marks, so it is asked. `after` is written as a save would diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 958204bc5..8937cf678 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2294,58 +2294,52 @@ fn mark_shown_text(s: &mut Session, id: DocId, needle: &str) { s.apply(id, mark).unwrap(); } -/// Redaction removes text by the positions its own interpreter works out. Here the font and size -/// come from an ExtGState /Font entry (`gs`), which the renderer honours and the interpreter does -/// not model, so it places no glyph under the mark the user drew over the word as displayed: it -/// used to report success and leave the word in the file. The renderer is asked now, and the edit -/// is refused. +/// Redaction removes text by the positions its own interpreter works out, and the renderer has +/// the last word: where the two disagree, a page that still shows a glyph mostly under a mark +/// fails the edit. The check is run here on a page nothing was removed from, as if the +/// interpreter had placed the word elsewhere. Only the overlay streams an apply added are left +/// out: a content stream of the input labelled like one (`/PCMark /Redaction`) is checked too. #[test] -fn redaction_is_refused_when_the_page_still_shows_text_under_a_mark() { +fn the_renderer_check_refuses_text_still_shown_under_a_mark() { + let mut s = Session::new().with_clock(|| 1_700_000_000); + let page = one_line_page_with("/Font << /F1 4 0 R >>", "/PCMark /Redaction", "BT /F1 16 Tf 20 150 Td (Public words then SECRET42) Tj ET"); + let id = s.open("shown.pdf", None, Arc::new(page), None).unwrap(); + mark_shown_text(&mut s, id, "SECRET42"); + let doc = s.get(id).unwrap(); + let mut cos = doc.editor.as_ref().unwrap().cos.clone(); + let marks = pdfcraft_redact::marks(&cos); + let refused = |cos: &pdfcraft_cos::Document| { + let shown = shown_under_redactions(cos, &marks, &[], &doc.info.pages, None); + matches!(shown, Err(EditError::Redact(pdfcraft_redact::RedactError::Residue(n))) if n > 0) + }; + assert!(refused(&cos)); + // The page's contents as an array, as they are once an apply has added its overlay. + let page = pdfcraft_model::pages(&cos).swap_remove(0); + let contents = page.dict.get(b"Contents").cloned().unwrap(); + cos.update_dict(page.obj, |d| d.set(b"Contents".to_vec(), pdfcraft_cos::Object::Array(vec![contents]))).unwrap(); + assert!(refused(&cos), "the labelled input stream was not checked"); +} + +/// A word whose font and size an ExtGState /Font entry (`gs`) sets is placed where the renderer +/// draws it, so a mark drawn over it as displayed takes it out of the file. +#[test] +fn a_word_in_a_font_set_by_an_extgstate_is_redacted() { let line = "Public words then SECRET42 and more"; let mut s = Session::new().with_clock(|| 1_700_000_000); let by_gs = one_line_page("/ExtGState << /GS1 << /Type /ExtGState /Font [4 0 R 16] >> >>", &format!("BT /GS1 gs 20 150 Td ({line}) Tj ET")); let id = s.open("gs.pdf", None, Arc::new(by_gs), None).unwrap(); mark_shown_text(&mut s, id, "SECRET42"); - let refused = s.apply(id, Edit::ApplyRedactions { pages: None }); - assert!(matches!(refused, Err(EditError::Redact(pdfcraft_redact::RedactError::Residue(n))) if n > 0), "{refused:?}"); - // Nothing was changed: the mark is still pending and the page still shows the line. - assert_eq!(s.get(id).unwrap().redaction_marks(), 1); - assert_eq!(page_texts(&s, id), [line]); - - // The same line with its font set by `Tf`: the mark is applied and the word is gone. - let by_tf = one_line_page("/Font << /F1 4 0 R >>", &format!("BT /F1 16 Tf 20 150 Td ({line}) Tj ET")); - let id = s.open("tf.pdf", None, Arc::new(by_tf), None).unwrap(); - mark_shown_text(&mut s, id, "SECRET42"); s.apply(id, Edit::ApplyRedactions { pages: None }).unwrap(); let shown = page_texts(&s, id).swap_remove(0); assert!(shown.starts_with("Public words then") && shown.ends_with("and more") && !shown.contains("SECRET42"), "{shown}"); -} - -/// Only the overlay a redaction adds is left out of the renderer's check. A page whose own content -/// stream calls itself a redaction overlay (`/PCMark /Redaction`) is checked like any other. -#[test] -fn a_content_stream_labelled_as_a_redaction_overlay_is_still_checked() { - let line = "Public words then SECRET42 and more"; - let mut s = Session::new().with_clock(|| 1_700_000_000); - let forged = one_line_page_with( - "/ExtGState << /GS1 << /Type /ExtGState /Font [4 0 R 16] >> >>", - "/PCMark /Redaction", - &format!("BT /GS1 gs 20 150 Td ({line}) Tj ET"), + let saved = s.save_bytes(id).unwrap(); + let held = |d: &[u8]| d.windows(8).any(|w| w == b"SECRET42"); + assert!(!held(&saved), "the saved file holds the redacted word"); + let reopened = pdfcraft_cos::Document::open(saved).unwrap(); + assert!( + !reopened.scan_objects().any(|(_, o)| matches!(&*o, pdfcraft_cos::Object::Stream(st) if st.decoded().is_ok_and(|d| held(&d)))), + "a saved stream holds the redacted word" ); - let id = s.open("forged.pdf", None, Arc::new(forged), None).unwrap(); - mark_shown_text(&mut s, id, "SECRET42"); - let applied = s.apply(id, Edit::ApplyRedactions { pages: None }); - if applied.is_ok() { - assert!(!page_texts(&s, id).concat().contains("SECRET42"), "the page still shows the redacted word"); - let saved = s.save_bytes(id).unwrap(); - let held = |d: &[u8]| d.windows(8).any(|w| w == b"SECRET42"); - assert!(!held(&saved), "the saved file holds the redacted word"); - let reopened = pdfcraft_cos::Document::open(saved).unwrap(); - assert!( - !reopened.scan_objects().any(|(_, o)| matches!(&*o, pdfcraft_cos::Object::Stream(st) if st.decoded().is_ok_and(|d| held(&d)))), - "a saved stream holds the redacted word" - ); - } } #[test] diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index a608568f3..14dedbb34 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -690,6 +690,34 @@ fn prune_form_resources(doc: &Document, res: &mut Dict, content: &[Vec]) -> Ok(()) } +/// The font, writing direction and size an ExtGState's `/Font [font size]` entry sets, as `Tf` +/// would (ISO 32000-2 §8.4.5); `None` when `name` sets no font. The metrics are cached with the +/// fonts `Tf` names, under a key no resource name can take (a name cannot hold a null byte). +fn extgstate_font( + doc: &Document, + scope: &mut Scope<'_>, + ext_gstates: &Dict, + name: &[u8], + fallback: &Rc, +) -> Option<(Rc, bool, f64)> { + let state = doc.resolve(ext_gstates.get(name)?).as_dict().cloned()?; + let entry = doc.resolve(state.get(b"Font")?).as_array().cloned()?; + let [font, size] = entry.as_slice() else { return None }; + let size = doc.resolve(size).as_f64()?; + let font_dict = doc.resolve(font).as_dict().cloned(); + let vertical = font_dict.as_ref().is_some_and(|d| is_vertical(doc, d)); + let key = [b"\0".as_slice(), name].concat(); + let metrics = match scope.fonts.get(&key) { + Some(m) => m.clone(), + None => { + let m = font_dict.as_ref().map(|d| Rc::new(Metrics::from_dict(doc, d))).unwrap_or_else(|| fallback.clone()); + scope.fonts.insert(key, m.clone()); + m + } + }; + Some((metrics, vertical, size)) +} + /// One glyph of a shown string. struct Glyph { bytes: std::ops::Range, @@ -700,11 +728,9 @@ struct Glyph { /// Process a scope's content streams (state flows from one stream to the next). pub(crate) fn process(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec], resources: &Dict, ctm: Matrix) -> Output { - let mut out = Output::default(); - let fallback = Rc::new(Metrics::fallback()); - let mut gs = Gs { + let gs = Gs { ctm, - font: fallback.clone(), + font: Rc::new(Metrics::fallback()), vertical: false, size: 0.0, char_spacing: 0.0, @@ -714,9 +740,19 @@ pub(crate) fn process(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec< leading: 0.0, rise: 0.0, }; + process_from(doc, scope, streams, resources, gs) +} + +/// [`process`] from graphics state `gs`: a form XObject starts with its caller's, text state +/// included (ISO 32000-2 §8.10.1), so text it shows without setting a font of its own is placed +/// with the caller's font, size and spacing, as the renderer places it. +fn process_from(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec], resources: &Dict, mut gs: Gs) -> Output { + let mut out = Output::default(); + let fallback = Rc::new(Metrics::fallback()); let mut stack: Vec = Vec::new(); let (mut tm, mut tlm) = (Matrix::IDENTITY, Matrix::IDENTITY); let fonts_res = res_dict(doc, resources, b"Font"); + let ext_gstates = res_dict(doc, resources, b"ExtGState"); let mut xobjects = res_dict(doc, resources, b"XObject"); // The current path: its operators, its bounding box in user space, and whether it clips. let mut path: Vec = Vec::new(); @@ -865,6 +901,11 @@ pub(crate) fn process(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec< }; } } + b"gs" => { + if let Some((font, vertical, size)) = op.name(0).and_then(|n| extgstate_font(doc, scope, &ext_gstates, n, &fallback)) { + (gs.font, gs.vertical, gs.size) = (font, vertical, size); + } + } b"Tc" => gs.char_spacing = op.num(0).unwrap_or(0.0), b"Tw" => gs.word_spacing = op.num(0).unwrap_or(0.0), b"Tz" => gs.scale = op.num(0).unwrap_or(100.0) / 100.0, @@ -1223,7 +1264,7 @@ fn xobject( let res = own.clone().unwrap_or_else(|| resources.clone()); let saved_fonts = std::mem::take(&mut scope.fonts); scope.depth += 1; - let inner = process(doc, scope, std::slice::from_ref(&data), &res, fctm); + let inner = process_from(doc, scope, std::slice::from_ref(&data), &res, Gs { ctm: fctm, ..gs.clone() }); scope.depth -= 1; scope.fonts = saved_fonts; out.residue += inner.residue; diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 52d80eb26..882cc50ae 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2463,6 +2463,7 @@ fn unused_descriptions_in_shared_or_inherited_resources_are_retired() { mod property_retirement; mod surviving_references; +mod text_state; /// `[(FIRSTARR)] [(LASTARR)] TJ` is malformed (a surplus array operand), but the operator still /// draws only the one immediately before it (LASTARR, 7 glyphs, 35 pt wide from x = 10) -- not diff --git a/crates/redact/src/tests/text_state.rs b/crates/redact/src/tests/text_state.rs new file mode 100644 index 000000000..4e888f262 --- /dev/null +++ b/crates/redact/src/tests/text_state.rs @@ -0,0 +1,27 @@ +//! Glyphs are placed with the whole text state, wherever it was set. + +use super::*; + +const SECRET_AT_10_100: [f64; 4] = [5.0, 95.0, 45.0, 110.0]; + +#[test] +fn a_font_set_by_an_extgstate_places_its_glyphs() { + let mut doc = one_page(b"BT /GS1 gs 10 100 Td (SECRET) Tj ET", "/ExtGState << /GS1 << /Type /ExtGState /Font [5 0 R 10] >> >>", vec![]); + // "ET" spans x 30–40. + assert_eq!(under(&mut doc, 0, &[[30.0, 95.0, 40.0, 110.0]]), 2); + mark(&mut doc, 0, &[SECRET_AT_10_100], ""); + assert_eq!(apply(&mut doc, None).unwrap().glyphs, 6); + assert!(!content(&doc, 0).contains("SECRET")); +} + +#[test] +fn a_form_inherits_the_text_state_of_the_page_that_draws_it() { + // The form shows text with the font, size and spacing the page set before `Do`. + let form = stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300]", b"BT 10 100 Td (SECRET) Tj ET"); + let mut doc = one_page(b"/F1 10 Tf 1 Tc /Fm Do", "/XObject << /Fm 7 0 R >>", vec![form]); + // Glyphs 5 pt wide, 1 pt apart: the last one spans x 40–45. + assert_eq!(under(&mut doc, 0, &[[40.0, 95.0, 46.0, 110.0]]), 1); + mark(&mut doc, 0, &[SECRET_AT_10_100], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!((r.glyphs, r.forms_rewritten), (6, 1), "{r:?}"); +} From be64a60c876402f43ffef893f1fd87ac77786981 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:52:12 +0000 Subject: [PATCH 13/20] fix(redact): unlink removed XObjects from surviving forms' resources After applying redactions, removed images and forms were unlinked from page-tree resource dictionaries but not from the resources of forms the page still draws. A form sharing the page's original resource dictionary (a common producer layout) kept the removed image reachable, so a full save wrote it. The walk over surviving forms now drops XObject entries for removed objects that the form's own content does not draw, and fails closed when that cannot be determined. --- crates/redact/src/interp.rs | 34 +++++++++++++--- .../redact/src/tests/surviving_references.rs | 40 +++++++++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index 14dedbb34..fca3a1da8 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -529,8 +529,9 @@ pub(crate) fn unlink_doomed(doc: &mut Document, doomed: &[ObjRef], retired_prope } } // A surviving form can own the same old resource dictionary as a rewritten page. Its - // unused descriptions remain reachable too, even after every page holder is cleaned. - unlink_form_properties(doc, &holder_nodes, retired_properties)?; + // unused descriptions and doomed XObjects remain reachable too, even after every page + // holder is cleaned. + unlink_from_forms(doc, &holder_nodes, &doomed, retired_properties)?; Ok(()) } @@ -567,8 +568,11 @@ fn prune_retired_properties(doc: &Document, res: &mut Dict, names: &[Vec], c Ok(pruned) } -fn unlink_form_properties(doc: &mut Document, holders: &[ObjRef], retired: &[Object]) -> Result<(), CosError> { - if retired.is_empty() { +/// Unlink retired properties and `doomed` XObjects from the resources of the forms reachable from +/// the page-tree `holders`, each unless the form's own content (or a form inheriting its +/// resources) still uses the name. Fails closed when that cannot be determined. +fn unlink_from_forms(doc: &mut Document, holders: &[ObjRef], doomed: &HashSet, retired: &[Object]) -> Result<(), CosError> { + if retired.is_empty() && doomed.is_empty() { return Ok(()); } let mut pending: Vec = holders @@ -593,7 +597,27 @@ fn unlink_form_properties(doc: &mut Document, holders: &[ObjRef], retired: &[Obj let Some(mut res) = form.dict.get(b"Resources").and_then(|r| doc.resolve(r).as_dict().cloned()) else { continue }; let names: Vec> = res_dict(doc, &res, b"Properties").iter().filter(|(_, value)| retired.contains(value)).map(|(name, _)| name.to_vec()).collect(); - if !names.is_empty() && prune_retired_properties(doc, &mut res, &names, &[vec![form.decoded()?]])? { + let mut xo = res_dict(doc, &res, b"XObject"); + let doomed_names: Vec> = + xo.iter().filter(|(_, v)| v.as_ref().is_some_and(|r| doomed.contains(&r))).map(|(n, _)| n.to_vec()).collect(); + if names.is_empty() && doomed_names.is_empty() { + pending.push(res); + continue; + } + let data = form.decoded()?; + let mut pruned = false; + for name in doomed_names { + match content_draws(doc, std::slice::from_ref(&data), &xo, &name, 0, &mut HashSet::new()) { + Some(true) => {} + Some(false) => pruned |= xo.remove(&name).is_some(), + None => return Err(CosError::Syntax { offset: 0, detail: "redaction could not fully inspect shared form resources".into() }), + } + } + if pruned { + res.set(b"XObject".to_vec(), Object::Dict(xo)); + } + pruned |= prune_retired_properties(doc, &mut res, &names, &[vec![data]])?; + if pruned { doc.update_dict(reference, |d| d.set(b"Resources".to_vec(), Object::Dict(res.clone())))?; } pending.push(res); diff --git a/crates/redact/src/tests/surviving_references.rs b/crates/redact/src/tests/surviving_references.rs index cfe6d87bb..2c8bdb09c 100644 --- a/crates/redact/src/tests/surviving_references.rs +++ b/crates/redact/src/tests/surviving_references.rs @@ -26,3 +26,43 @@ fn a_redacted_field_listed_in_the_calculation_order_leaves_the_saved_file() { assert_eq!(apply(&mut doc, None).unwrap().fields, 1); assert!(!full_save_holds(&doc, b"SSN-078-05-1120"), "the field's value is still reachable"); } + +/// A surviving form shares the page's resource dictionary, which lists the image the mark +/// removes; the form never draws it. +#[test] +fn a_redacted_image_does_not_survive_in_a_surviving_forms_resources() { + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 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 300 300] /Contents 4 0 R /Resources 7 0 R >>".to_vec(), + stream("", b"q 100 0 0 100 0 0 cm /Im Do Q /Fm Do"), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + b"<< /Font << /F1 5 0 R >> /XObject << /Im 8 0 R /Fm 9 0 R >> >>".to_vec(), + stream("/Type /XObject /Subtype /Image /Width 11 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8", b"IMAGESECRET"), + stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources 7 0 R", b"BT /F1 10 Tf 10 200 Td (Public) Tj ET"), + ]); + mark(&mut doc, 0, &[[0.0, 0.0, 120.0, 120.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().images_removed, 1); + assert!(content(&doc, 0).contains("Do"), "the form is still drawn"); + assert!(!full_save_holds(&doc, b"IMAGESECRET"), "the removed image is still reachable"); +} + +/// The same form drawing the image itself keeps it: the page still shows it there. +#[test] +fn an_image_a_surviving_form_draws_is_kept() { + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 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 300 300] /Contents 4 0 R /Resources 7 0 R >>".to_vec(), + stream("", b"q 100 0 0 100 0 0 cm /Im Do Q /Fm Do"), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + b"<< /Font << /F1 5 0 R >> /XObject << /Im 8 0 R /Fm 9 0 R >> >>".to_vec(), + stream("/Type /XObject /Subtype /Image /Width 11 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8", b"IMAGESECRET"), + stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources 7 0 R", b"q 50 0 0 50 200 200 cm /Im Do Q"), + ]); + mark(&mut doc, 0, &[[0.0, 0.0, 120.0, 120.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().images_removed, 1); + assert!(full_save_holds(&doc, b"IMAGESECRET"), "the image the form still draws outside the mark"); +} From cccd435a5308bd628976e0f0d9923f1eebf90e36 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:53:24 +0000 Subject: [PATCH 14/20] fix(redact): bound image decoding by the declared raster size Clearing the pixels of a partly covered image decoded its stream with the global 1 GiB allowance and only then cut it to the raster length its Width, Height and bit depth declare. A small Flate stream could make every such image allocate and inflate far more than it can use. Decoding now stops at twice the raster plus a byte per row (filter-chain and predictor overhead) and a little slack; an image past that is removed whole, as an undecodable one already is. --- crates/redact/src/image.rs | 6 +++++- crates/redact/src/tests.rs | 1 + crates/redact/src/tests/limits.rs | 25 +++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 crates/redact/src/tests/limits.rs diff --git a/crates/redact/src/image.rs b/crates/redact/src/image.rs index 9a19ba2f0..601700f25 100644 --- a/crates/redact/src/image.rs +++ b/crates/redact/src/image.rs @@ -77,7 +77,11 @@ fn decode_and_clear(doc: &Document, s: &Stream, ctm: &Matrix, rects: &[[f64; 4]] let bytes = row.checked_mul(h)?; let row_bits = row.checked_mul(8)?; bytes.checked_mul(8)?; - let mut data = s.decoded().ok()?; + // Decode no further than the raster can need: a filter stage may legitimately produce up to + // twice it (ASCIIHex text under Flate) plus a PNG predictor's byte per row. Data past that + // can only be a decompression bomb; the image is then removed whole. + let allowance = bytes.checked_add(h)?.checked_mul(2)?.checked_add(4096)?; + let mut data = s.decoded_within(allowance).ok()?; if data.len() < bytes { return None; } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 882cc50ae..94081f9e3 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2461,6 +2461,7 @@ fn unused_descriptions_in_shared_or_inherited_resources_are_retired() { } } +mod limits; mod property_retirement; mod surviving_references; mod text_state; diff --git a/crates/redact/src/tests/limits.rs b/crates/redact/src/tests/limits.rs new file mode 100644 index 000000000..6bee96aed --- /dev/null +++ b/crates/redact/src/tests/limits.rs @@ -0,0 +1,25 @@ +//! Work and memory stay bounded by what the page declares, whatever the file holds. + +use super::*; + +/// A half-covered 4×1 grey image whose Flate data decodes to `decoded`. +fn half_covered_image(decoded: &[u8]) -> Report { + let dict = "/Type /XObject /Subtype /Image /Width 4 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8"; + let mut doc = one_page(b"q 40 0 0 10 100 100 cm /Im Do Q", "/XObject << /Im 7 0 R >>", vec![stream(dict, b"")]); + let Object::Stream(s) = &*doc.get(ObjRef::new(7, 0)) else { panic!() }; + doc.set(ObjRef::new(7, 0), Object::Stream(Stream::flate(s.dict.clone(), decoded))); + mark(&mut doc, 0, &[[95.0, 95.0, 120.0, 115.0]], ""); + apply(&mut doc, None).unwrap() +} + +#[test] +fn an_image_decoding_far_past_its_raster_is_removed_without_decoding_it_all() { + let r = half_covered_image(&[255; 4]); + assert_eq!((r.images_removed, r.images_cleared), (0, 1), "{r:?}"); + // 4 bytes of raster, then 4 MiB the image never uses: decoding stops at a bound derived from + // the raster, and the image goes whole. + let mut bomb = vec![255; 4]; + bomb.resize(4 << 20, 0); + let r = half_covered_image(&bomb); + assert_eq!((r.images_removed, r.images_cleared), (1, 0), "{r:?}"); +} From 2cdb39503ef303b5cc66703abda470d044896690 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:55:22 +0000 Subject: [PATCH 15/20] fix(redact): compare marked content without a copy per enclosing MCID Finding which marked content a redaction changed copied every operator into the buffer of each MCID enclosing it, so content nested n deep was held n times: a modest stream of nested BDCs could exhaust memory. Each operator inside marked content is now serialized once, and each MCID keeps the ranges its sequences span. The bytes compared are capped; MCIDs past the cap count as changed, so their descriptions are cleared rather than kept. --- crates/redact/src/tags.rs | 88 +++++++++++++++++++++++++------ crates/redact/src/tests/limits.rs | 30 +++++++++++ 2 files changed, 101 insertions(+), 17 deletions(-) diff --git a/crates/redact/src/tags.rs b/crates/redact/src/tags.rs index ab5735e06..854881060 100644 --- a/crates/redact/src/tags.rs +++ b/crates/redact/src/tags.rs @@ -6,15 +6,49 @@ //! references to marked content that is now empty are dropped from the element. use std::collections::{HashMap, HashSet}; +use std::ops::Range; use pdfcraft_cos::{Dict, Document, ObjRef, Object}; use crate::RedactError; -/// Each MCID's operators (serialized), and whether anything is still drawn inside it. -fn by_mcid(doc: &Document, resources: &Dict, data: &[u8]) -> HashMap, bool)> { - let mut out: HashMap, bool)> = HashMap::new(); - let mut stack: Vec> = Vec::new(); +/// The marked content of one content stream. Every operator inside some MCID is serialized once +/// into `ops`, and each MCID holds the ranges of `ops` its sequences span (marked-content +/// operators are not written, so a sequence is one range however deeply it nests), with whether +/// anything is drawn inside it. Copying the operators into every enclosing MCID instead would +/// hold `depth` copies of nested content. +#[derive(Default)] +struct Marked { + ops: Vec, + ids: HashMap>, bool)>, +} + +impl Marked { + /// The bytes of `ranges`, which are ranges of `ops`. + fn bytes<'a>(&'a self, ranges: &'a [Range]) -> impl Iterator + 'a { + ranges.iter().flat_map(|r| self.ops.get(r.clone()).unwrap_or_default()) + } +} + +/// Most bytes [`touched`] compares for one stream. Nested sequences share their operators, so the +/// comparisons can add up to the content's size times its nesting depth; past this, the MCIDs +/// left are taken as changed (their descriptions go) rather than compared. +const MAX_COMPARED: usize = 1 << 28; + +fn by_mcid(doc: &Document, resources: &Dict, data: &[u8]) -> Marked { + let mut out = Marked::default(); + // Open sequences: the MCID with where its content starts in `ops` and the drawing operators + // written before it, or `None` for a sequence without one. + let mut stack: Vec> = Vec::new(); + let (mut open_ids, mut drawn) = (0usize, 0usize); + let close = |out: &mut Marked, open: Option<(i64, usize, usize)>, drawn: usize| { + if let Some((id, start, drawn_before)) = open { + let end = out.ops.len(); + let e = out.ids.entry(id).or_default(); + e.0.push(start..end); + e.1 |= drawn > drawn_before; + } + }; for op in pdfcraft_content::parse(data).ops { match op.op.as_slice() { b"BDC" => { @@ -28,9 +62,10 @@ fn by_mcid(doc: &Document, resources: &Dict, data: &[u8]) -> HashMap None, }; if let Some(id) = id { - out.entry(id).or_default(); + out.ids.entry(id).or_default(); + open_ids += 1; } - stack.push(id); + stack.push(id.map(|id| (id, out.ops.len(), drawn))); continue; } b"BMC" => { @@ -38,11 +73,17 @@ fn by_mcid(doc: &Document, resources: &Dict, data: &[u8]) -> HashMap { - stack.pop(); + if let Some(open) = stack.pop() { + open_ids = open_ids.saturating_sub(usize::from(open.is_some())); + close(&mut out, open, drawn); + } continue; } _ => {} } + if open_ids == 0 { + continue; + } let draws = match op.op.as_slice() { b"Tj" | b"'" | b"\"" => op.operands.last().and_then(Object::as_string).is_some_and(|s| !s.bytes.is_empty()), b"TJ" => { @@ -51,13 +92,12 @@ fn by_mcid(doc: &Document, resources: &Dict, data: &[u8]) -> HashMap true, _ => false, }; - let mut bytes = Vec::new(); - pdfcraft_content::write_op(&op, &mut bytes); - for id in stack.iter().flatten() { - let e = out.entry(*id).or_default(); - e.0.extend_from_slice(&bytes); - e.1 |= draws; - } + pdfcraft_content::write_op(&op, &mut out.ops); + drawn += usize::from(draws); + } + // Sequences the stream leaves open run to its end. + while let Some(open) = stack.pop() { + close(&mut out, open, drawn); } out } @@ -67,10 +107,17 @@ pub(crate) fn touched(doc: &Document, resources: &Dict, before: &[u8], after: &[ let (b, a) = (by_mcid(doc, resources, before), by_mcid(doc, resources, after)); let mut changed = HashSet::new(); let mut empty = HashSet::new(); - for (id, (ops, _)) in &b { - match a.get(id) { + let mut budget = MAX_COMPARED; + let len = |ranges: &[Range]| ranges.iter().map(|r| r.len()).fold(0usize, usize::saturating_add); + for (id, (old, _)) in &b.ids { + match a.ids.get(id) { Some((new, draws)) => { - if new != ops { + let n = len(old); + let same = n == len(new) && n <= budget && { + budget -= n; + b.bytes(old).eq(a.bytes(new)) + }; + if !same { changed.insert(*id); } if !draws { @@ -413,3 +460,10 @@ pub(crate) fn drop_object_refs_within(doc: &mut Document, gone: &HashSet } Ok(n) } + +/// Bytes held for comparing marked content, for tests that bound it. +#[cfg(test)] +pub(crate) fn retained_for_comparison(doc: &Document, resources: &Dict, data: &[u8]) -> usize { + let marked = by_mcid(doc, resources, data); + marked.ops.len() + marked.ids.values().map(|(ranges, _)| ranges.len() * std::mem::size_of::>()).sum::() +} diff --git a/crates/redact/src/tests/limits.rs b/crates/redact/src/tests/limits.rs index 6bee96aed..8519e0860 100644 --- a/crates/redact/src/tests/limits.rs +++ b/crates/redact/src/tests/limits.rs @@ -23,3 +23,33 @@ fn an_image_decoding_far_past_its_raster_is_removed_without_decoding_it_all() { let r = half_covered_image(&bomb); assert_eq!((r.images_removed, r.images_cleared), (1, 0), "{r:?}"); } + +/// Marked content nested `depth` deep, each level with its own MCID, around `depth` drawing +/// operators. +fn nested_marked_content(depth: usize) -> Vec { + let mut data = Vec::new(); + for id in 0..depth { + data.extend_from_slice(format!("/P <> BDC\n").as_bytes()); + } + data.extend(b"0 0 1 1 re f\n".repeat(depth)); + data.extend(b"EMC\n".repeat(depth)); + data +} + +#[test] +fn comparing_deeply_nested_marked_content_holds_each_operator_once() { + let doc = one_page(b"", "", vec![]); + let data = nested_marked_content(2000); + let held = tags::retained_for_comparison(&doc, &Dict::new(), &data); + assert!(held <= 2 * data.len(), "{held} bytes held for {} bytes of content", data.len()); +} + +#[test] +fn a_change_inside_nested_marked_content_touches_every_enclosing_mcid() { + let doc = one_page(b"", "", vec![]); + let before = b"/P <> BDC /P <> BDC BT (A) Tj ET EMC 0 0 1 1 re f EMC /P <> BDC BT (B) Tj ET EMC"; + let after = b"/P <> BDC /P <> BDC BT ET EMC 0 0 1 1 re f EMC /P <> BDC BT (B) Tj ET EMC"; + let (changed, empty) = tags::touched(&doc, &Dict::new(), before, after); + assert_eq!(changed, HashSet::from([0, 1])); + assert_eq!(empty, HashSet::from([1])); +} From 7db37911ed7f22f623f5a9416aa1e83ebe053adc Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:57:39 +0000 Subject: [PATCH 16/20] fix(redact): clear descriptions above removed content and annotations Structure cleanup cleared alternate text, actual text and expansions only on the element that directly owned changed marked content, and an element that lost an annotation reference kept its descriptions unless it lost every kid. An ancestor's /ActualText covering a redacted span, or a link element's /Alt repeating the address of a removed link, stayed in the saved file. Elements that lose an annotation now always lose their descriptions, and every ancestor of an affected element (found through both /P and the tree's /K, which may disagree) loses its own. An element written in place above one with a description to clear refuses the operation, as other in-place cases already do. --- crates/redact/src/tags.rs | 135 ++++++++++++++---- crates/redact/src/tests.rs | 1 + crates/redact/src/tests/descriptions.rs | 64 +++++++++ .../redact/src/tests/surviving_references.rs | 2 +- 4 files changed, 177 insertions(+), 25 deletions(-) create mode 100644 crates/redact/src/tests/descriptions.rs diff --git a/crates/redact/src/tags.rs b/crates/redact/src/tags.rs index 854881060..60c158b18 100644 --- a/crates/redact/src/tags.rs +++ b/crates/redact/src/tags.rs @@ -183,20 +183,21 @@ pub(crate) fn clean_within( // also from the parent tree and the ID tree: one listed only there still owns marked content // (#3302). The stack pops /K first, so an element reached through /K gets the page it inherits // from its ancestors before a tree reaches it without one. - let mut todo: Vec<(Object, Option)> = - [&b"IDTree"[..], b"ParentTree", b"K"].iter().filter_map(|key| root.get(key).cloned()).map(|k| (k, None)).collect(); + let mut todo: Vec<(Object, Option, Above)> = + [&b"IDTree"[..], b"ParentTree", b"K"].iter().filter_map(|key| root.get(key).cloned()).map(|k| (k, None, Above::default())).collect(); let mut seen = HashSet::new(); let mut pages = HashMap::new(); + let mut parents = Parents::new(); let mut hits: Vec<(ObjRef, Dict, Option, bool)> = Vec::new(); // Whether marked content `x` (a kid on page `pg`) is one this cleanup must change. let touches = |doc: &Document, x: &Object, pg: Option| { mcid_of(doc, x, pg, page, stream).is_some_and(|m| stream.is_some() || changed.contains(&m) || empty.contains(&m)) }; - while let Some((k, pg)) = todo.pop() { + while let Some((k, pg, above)) = todo.pop() { match k { - Object::Array(a) => todo.extend(a.into_iter().map(|x| (x, pg))), + Object::Array(a) => todo.extend(a.into_iter().map(|x| (x, pg, above))), // A number or name tree node written in place. - Object::Dict(d) if !d.contains(b"S") => todo.extend(tree_node_entries(&d).into_iter().map(|x| (x, pg))), + Object::Dict(d) if !d.contains(b"S") => todo.extend(tree_node_entries(&d).into_iter().map(|x| (x, pg, above))), // An element written in place: its kids are followed, but if it owns marked content // this cleanup changes, only rewriting what holds it could change it. Object::Dict(d) => { @@ -208,9 +209,11 @@ pub(crate) fn clean_within( if kids.iter().any(|x| touches(doc, x, pg)) { return Err(RedactError::Tags); } - todo.extend(kids.into_iter().map(|x| (x, pg))); + let above = above.through(&d); + todo.extend(kids.into_iter().map(|x| (x, pg, above))); } Object::Ref(r) => { + parents.record(r, above); if !seen.insert(r) { continue; } @@ -219,13 +222,13 @@ pub(crate) fn clean_within( } let object = doc.get(r); if let Object::Array(a) = &*object { - todo.extend(a.iter().cloned().map(|x| (x, pg))); + todo.extend(a.iter().cloned().map(|x| (x, pg, above))); continue; } let Some(d) = object.as_dict().cloned() else { continue }; if !d.contains(b"S") { // A parent tree or ID tree node: its values may be elements. - todo.extend(tree_node_entries(&d).into_iter().map(|x| (x, pg))); + todo.extend(tree_node_entries(&d).into_iter().map(|x| (x, pg, above))); continue; } let pg = match d.get(b"Pg").and_then(Object::as_ref).or(pg) { @@ -238,11 +241,12 @@ pub(crate) fn clean_within( if sensitive || remap { hits.push((r, d.clone(), pg, sensitive)); } - todo.extend(kids.into_iter().map(|x| (x, pg))); + todo.extend(kids.into_iter().map(|x| (x, pg, Above::element(r)))); } _ => {} } } + let sensitive: Vec = hits.iter().filter(|h| h.3).map(|h| h.0).collect(); let mut n = 0; for (r, d, pg, sensitive) in hits { let kids = d.get(b"K").map(|k| doc.resolve(k)); @@ -291,9 +295,88 @@ pub(crate) fn clean_within( })?; n += usize::from(sensitive && doc.get(r).as_dict() != Some(&d)); } + // An element above may describe the changed content too (`/ActualText` on a span's parent + // covers the span, §14.9.4). + n += clear_ancestors(doc, &sensitive, &parents, limit)?; Ok(n) } +/// Where the walk reached an object from: the nearest element above it that is an object of its +/// own, and whether an element written in place between them carries a description (which only +/// rewriting the object holding it could remove). +#[derive(Clone, Copy, Default)] +struct Above { + element: Option, + described_in_place: bool, +} + +impl Above { + fn element(r: ObjRef) -> Self { + Self { element: Some(r), described_in_place: false } + } + + /// Below an element written in place. + fn through(self, d: &Dict) -> Self { + Self { described_in_place: self.described_in_place || DESCRIPTIONS.iter().any(|k| d.contains(k)), ..self } + } +} + +/// The keys of an element that describe its content. +const DESCRIPTIONS: [&[u8]; 3] = [b"Alt", b"ActualText", b"E"]; + +/// Each element's parents as the walk found them: through `/K`, which `/P` may not agree with. +struct Parents(HashMap>); + +impl Parents { + fn new() -> Self { + Self(HashMap::new()) + } + + fn record(&mut self, r: ObjRef, above: Above) { + if above.element.is_some() || above.described_in_place { + self.0.entry(r).or_default().push(above); + } + } +} + +/// Remove the descriptions of every element above `elements`, through both `/P` and the walk's +/// `parents`; returns how many elements changed. [`RedactError::Tags`] when an element written in +/// place above them carries one, or when there are more than `limit` ancestors. +fn clear_ancestors(doc: &mut Document, elements: &[ObjRef], parents: &Parents, limit: usize) -> Result { + let mut todo: Vec = elements.to_vec(); + let mut seen: HashSet = elements.iter().copied().collect(); + let mut cleared = 0; + while let Some(r) = todo.pop() { + let mut up: Vec = doc.get(r).as_dict().and_then(|d| d.get(b"P")).and_then(Object::as_ref).into_iter().collect(); + for above in parents.0.get(&r).into_iter().flatten() { + if above.described_in_place { + return Err(RedactError::Tags); + } + up.extend(above.element); + } + for a in up { + if !seen.insert(a) { + continue; + } + if seen.len() > limit { + return Err(RedactError::Tags); + } + // The structure tree root (no /S) ends the climb. + let Some(d) = doc.get(a).as_dict().filter(|d| d.contains(b"S")).cloned() else { continue }; + if DESCRIPTIONS.iter().any(|k| d.contains(k)) { + doc.update_dict(a, |e| { + for key in DESCRIPTIONS { + e.remove(key); + } + })?; + cleared += 1; + } + todo.push(a); + } + } + Ok(cleared) +} + /// The page an element inherits (§14.7.4.4): the nearest `/Pg` among its ancestors, following /// `/P`. `Ok(None)` when none has one; [`RedactError::Tags`] when the lookup gives up (more than /// [`MAX_PARENTS`] ancestors, or a loop), which isn't the same as having no page. `pages` keeps @@ -384,12 +467,14 @@ pub(crate) fn drop_object_refs_within(doc: &mut Document, gone: &HashSet }; // Elements are reached from the root's /K, and also from the parent tree and the ID tree: // one listed only there (left behind by an editor, say) still points at its annotation. - let mut todo: Vec = [&b"K"[..], b"ParentTree", b"IDTree"].iter().filter_map(|key| root.get(key).cloned()).collect(); + let mut todo: Vec<(Object, Above)> = + [&b"K"[..], b"ParentTree", b"IDTree"].iter().filter_map(|key| root.get(key).cloned()).map(|k| (k, Above::default())).collect(); let mut seen = HashSet::new(); + let mut parents = Parents::new(); let mut hits: Vec = Vec::new(); - while let Some(k) = todo.pop() { + while let Some((k, above)) = todo.pop() { match k { - Object::Array(a) => todo.extend(a), + Object::Array(a) => todo.extend(a.into_iter().map(|x| (x, above))), // A structure element written in place: its kids are followed; one that tags a // removed annotation can't be changed here (see above). Object::Dict(d) if d.contains(b"S") => { @@ -397,11 +482,13 @@ pub(crate) fn drop_object_refs_within(doc: &mut Document, gone: &HashSet if kids.iter().any(|x| refers_to(doc, x, gone)) { return Err(RedactError::Tags); } - todo.extend(kids); + let above = above.through(&d); + todo.extend(kids.into_iter().map(|x| (x, above))); } // A number or name tree node written in place (§7.9.6, §7.9.7): its values and kids. - Object::Dict(d) => todo.extend(tree_node_entries(&d)), + Object::Dict(d) => todo.extend(tree_node_entries(&d).into_iter().map(|x| (x, above))), Object::Ref(r) => { + parents.record(r, above); if !seen.insert(r) { continue; } @@ -410,26 +497,26 @@ pub(crate) fn drop_object_refs_within(doc: &mut Document, gone: &HashSet } let object = doc.get(r); if let Object::Array(a) = &*object { - todo.extend(a.iter().cloned()); + todo.extend(a.iter().cloned().map(|x| (x, above))); continue; } let Some(d) = object.as_dict() else { continue }; if !d.contains(b"S") { // A parent tree or ID tree node: its values may be elements. - todo.extend(tree_node_entries(d)); + todo.extend(tree_node_entries(d).into_iter().map(|x| (x, above))); continue; } let kids = kids_of(doc, d); if kids.iter().any(|x| refers_to(doc, x, gone)) { hits.push(r); } - todo.extend(kids); + todo.extend(kids.into_iter().map(|x| (x, Above::element(r)))); } _ => {} } } let mut n = 0; - for r in hits { + for &r in &hits { let Some(d) = doc.get(r).as_dict().cloned() else { continue }; let kids = d.get(b"K").map(|k| doc.resolve(k)); let array = kids.as_ref().is_some_and(|k| matches!(&**k, Object::Array(_))); @@ -449,15 +536,15 @@ pub(crate) fn drop_object_refs_within(doc: &mut Document, gone: &HashSet } else { e.remove(b"K"); } - // An element left with nothing to tag keeps no text describing what it tagged: - // a link's alternate text often repeats the URL, an annotation's its comment. - if keep.is_empty() { - for key in [&b"Alt"[..], b"ActualText", b"T", b"E"] { - e.remove(key); - } + // An element that lost an annotation keeps no text describing it, even with other + // kids left: a link's alternate text often repeats the URL, an annotation's its + // comment. Elements above it may describe it too. + for key in [&b"Alt"[..], b"ActualText", b"T", b"E"] { + e.remove(key); } })?; } + clear_ancestors(doc, &hits, &parents, limit)?; Ok(n) } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 94081f9e3..ebc95f451 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2461,6 +2461,7 @@ fn unused_descriptions_in_shared_or_inherited_resources_are_retired() { } } +mod descriptions; mod limits; mod property_retirement; mod surviving_references; diff --git a/crates/redact/src/tests/descriptions.rs b/crates/redact/src/tests/descriptions.rs new file mode 100644 index 000000000..df6b8555d --- /dev/null +++ b/crates/redact/src/tests/descriptions.rs @@ -0,0 +1,64 @@ +//! Descriptions in the structure tree (alternate text, actual text, expansions) go with the +//! content they describe, on the element that owns it and on every element above it. + +use super::surviving_references::full_save_holds; +use super::*; + +/// A tagged page: "SECRET" (MCID 0) inside a section whose descriptions repeat it, and "Public" +/// (MCID 1). `paragraph_parent` is the secret paragraph's /P. +fn sectioned(paragraph_parent: &str) -> Document { + let content = b"/P <> BDC BT /F1 10 Tf 10 200 Td (SECRET) Tj ET EMC /P <> BDC BT /F1 10 Tf 10 100 Td (Public) Tj ET EMC"; + pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R /StructTreeRoot 7 0 R /MarkInfo << /Marked true >> >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /StructParents 0 /Resources << /Font << /F1 5 0 R >> >> >>".to_vec(), + stream("", content), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + b"<< /Type /StructTreeRoot /K 8 0 R >>".to_vec(), + b"<< /S /Document /P 7 0 R /K [11 0 R 10 0 R] >>".to_vec(), + format!("<< /S /P /P {paragraph_parent} /Pg 3 0 R /K 0 >>").into_bytes(), + b"<< /S /P /P 8 0 R /Pg 3 0 R /ActualText (Public) /K 1 >>".to_vec(), + b"<< /S /Sect /P 8 0 R /ActualText (LEAK_ACTUAL) /Alt (LEAK_ALT) /E (LEAK_EXPANSION) /K 9 0 R >>".to_vec(), + ]) +} + +#[test] +fn an_ancestor_describing_redacted_content_loses_its_descriptions() { + // The paragraph names its section as parent, or (wrongly) the document: the section is found + // either way, by its /K. + for parent in ["11 0 R", "8 0 R"] { + let mut doc = sectioned(parent); + mark(&mut doc, 0, &[[5.0, 195.0, 60.0, 212.0]], ""); + apply(&mut doc, None).unwrap(); + assert!(!full_save_holds(&doc, b"LEAK_"), "a description of the redacted word survived (paragraph /P {parent})"); + let public = doc.get(ObjRef::new(10, 0)).as_dict().cloned().unwrap(); + assert!(public.contains(b"ActualText"), "an element beside it keeps its own"); + } +} + +#[test] +fn a_link_element_that_keeps_its_text_loses_the_description_of_its_removed_annotation() { + use crate::sanitize::{Hidden, remove_hidden}; + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R /MarkInfo << /Marked true >> /StructTreeRoot 8 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 300 300] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> /Annots [6 0 R] /StructParents 0 >>" + .to_vec(), + stream("", b"/Link << /MCID 0 >> BDC BT /F1 10 Tf 10 230 Td (See the site) Tj ET EMC"), + FONT.replace("95 0 R", "12 0 R").into_bytes(), + b"<< /Type /Annot /Subtype /Link /Rect [10 228 80 240] /StructParent 1 /A << /S /URI /URI (https://example.org/) >> >>".to_vec(), + b"null".to_vec(), + b"<< /Type /StructTreeRoot /K 9 0 R /ParentTree << /Nums [0 [11 0 R] 1 11 0 R] >> /ParentTreeNextKey 2 >>".to_vec(), + b"<< /Type /StructElem /S /Document /P 8 0 R /K 10 0 R >>".to_vec(), + b"<< /Type /StructElem /S /P /P 9 0 R /ActualText (See LEAK_PARENT) /K 11 0 R >>".to_vec(), + // The link element tags its text and the annotation; its descriptions repeat the address. + b"<< /Type /StructElem /S /Link /P 10 0 R /Pg 3 0 R /Alt (LEAK_ALT) /ActualText (LEAK_ACTUAL) /E (LEAK_E) /T (LEAK_T) /K [0 << /Type /OBJR /Obj 6 0 R /Pg 3 0 R >>] >>".to_vec(), + widths(), + ]); + let done = remove_hidden(&mut doc, &[Hidden::LinksActionsScripts]).unwrap(); + assert!(done.contains(&(Hidden::LinksActionsScripts, 1)), "{done:?}"); + assert!(!full_save_holds(&doc, b"LEAK_"), "a description of the removed link survived"); + let link = doc.get(ObjRef::new(11, 0)).as_dict().cloned().unwrap(); + assert_eq!(link.get(b"K"), Some(&Object::Array(vec![Object::Int(0)])), "the element keeps its text"); +} diff --git a/crates/redact/src/tests/surviving_references.rs b/crates/redact/src/tests/surviving_references.rs index 2c8bdb09c..d3aa98af6 100644 --- a/crates/redact/src/tests/surviving_references.rs +++ b/crates/redact/src/tests/surviving_references.rs @@ -3,7 +3,7 @@ use super::*; /// Whether `secret` survives a full save of `doc`, in the file's bytes or in a decoded stream. -fn full_save_holds(doc: &Document, secret: &[u8]) -> bool { +pub(super) fn full_save_holds(doc: &Document, secret: &[u8]) -> bool { let bytes = write_full(doc, &SaveOptions::default()).unwrap(); let found = |data: &[u8]| data.windows(secret.len()).any(|w| w == secret); if found(&bytes) { From b72078871a307ad8d28b9e3f861885435fe10834 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:09:57 +0000 Subject: [PATCH 17/20] fix(content): end inline images where the renderer ends them Filtered inline image data has no length, so its end is an EI chosen among candidates, and the content parser chose by a different rule than the renderer's tokenizer (hayro-syntax): the renderer also takes an EI with no whitespace before it, and checks a candidate by reading what follows as operators. With image data such as "7FEI 1 0 0 1 200 0 cm", the renderer ran a cm that redaction read as image data, so text was drawn 200 pt from where removal and its verify pass looked. The parser now takes each inline image's end from the renderer's tokenizer (already in the tree through pdfcraft-cos), so both read the same operators after the image; a test runs both on the same fixtures. Only where that tokenizer reads no image (no EI it accepts, or parameters it can't read) does the parser fall back to its own rules, counting the image in Parsed::ambiguous_images, and applying redactions refuses such a page as unreadable. --- Cargo.lock | 1 + crates/content/Cargo.toml | 3 + crates/content/src/lib.rs | 120 ++++++++++++++++++----- crates/content/src/tests.rs | 58 +++++++++-- crates/engine/src/tests.rs | 24 +++++ crates/redact/src/interp.rs | 6 ++ crates/redact/src/lib.rs | 6 ++ crates/redact/src/tests.rs | 1 + crates/redact/src/tests/inline_images.rs | 34 +++++++ 9 files changed, 224 insertions(+), 29 deletions(-) create mode 100644 crates/redact/src/tests/inline_images.rs diff --git a/Cargo.lock b/Cargo.lock index a95014924..e318b7a77 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4042,6 +4042,7 @@ version = "0.11.0" name = "pdfcraft-content" version = "0.11.0" dependencies = [ + "hayro-syntax", "pdfcraft-cos", ] diff --git a/crates/content/Cargo.toml b/crates/content/Cargo.toml index c553df0b8..e6b4c28a4 100644 --- a/crates/content/Cargo.toml +++ b/crates/content/Cargo.toml @@ -10,3 +10,6 @@ workspace = true [dependencies] pdfcraft-cos = { workspace = true } +# The renderer's content tokenizer, already in the tree through pdfcraft-cos: inline images end +# where the renderer ends them. +hayro-syntax = { workspace = true } diff --git a/crates/content/src/lib.rs b/crates/content/src/lib.rs index eb126ffa2..097ec2689 100644 --- a/crates/content/src/lib.rs +++ b/crates/content/src/lib.rs @@ -60,6 +60,10 @@ pub struct Parsed { pub ops: Vec, /// Bytes that could not be read as operands or operators. pub skipped: usize, + /// Inline images the renderer's tokenizer reads no image for (it stops, or finds no `EI` it + /// accepts). Their end here is a guess, so the operators after them may not be the ones the + /// page draws. + pub ambiguous_images: usize, /// For [`Pieces::parse`]: per piece, whether a comment runs into it from the piece before /// and whether one runs out of it into the next. Only boundaries are recorded, so the size /// depends on the number of pieces, not of comments. @@ -142,19 +146,27 @@ fn parse_pieces(data: &[u8], ends: &[usize]) -> Parsed { start.get_or_insert(at); operands.push(Object::Null); } - b"BI" => match inline_image(data, lx.pos) { - Ok((dict, img, end)) => { - lx.pos = end; - out.ops.push(Op { op: b"BI".to_vec(), operands: Vec::new(), inline: Some((dict, img)), span: start.take().unwrap_or(at)..end }); - operands.clear(); + b"BI" => { + let image = inline_image(data, at, lx.pos); + out.ambiguous_images += usize::from(image.ambiguous); + let end = image.end; + lx.pos = end; + match image.image { + Some((dict, img)) => { + out.ops.push(Op { + op: b"BI".to_vec(), + operands: Vec::new(), + inline: Some((dict, img)), + span: start.take().unwrap_or(at)..end, + }); + } + None => { + out.skipped += end.saturating_sub(at); + start = None; + } } - Err(end) => { - out.skipped += end.saturating_sub(at); - lx.pos = end; - start = None; - operands.clear(); - } - }, + operands.clear(); + } _ => { let s = start.take().unwrap_or(at); out.ops.push(Op { op: kw.to_vec(), operands: std::mem::take(&mut operands), inline: None, span: s..lx.pos }); @@ -198,11 +210,29 @@ fn comment_edges(data: &[u8], trivia: std::ops::Range, ends: &[usize], bo } } -/// Read an inline image starting after `BI`: (parameters, data, position after `EI`). `Err` holds -/// the position after the `EI` (or the end of the stream) of an image whose parameters can't be read. -fn inline_image(data: &[u8], pos: usize) -> Result<(Dict, Vec, usize), usize> { +/// An inline image as [`inline_image`] reads it. +struct Inline { + /// Its parameters and data; `None` when the parameters can't be read. + image: Option<(Dict, Vec)>, + /// The position after its `EI` (or the end of the stream). + end: usize, + /// The renderer's tokenizer reads no image here, so `end` is this parser's guess. + ambiguous: bool, +} + +/// Read the inline image whose `BI` keyword starts at `bi` (its parameters start at `pos`). +/// +/// Filtered image data has no length, and `EI` can occur inside it, so where it ends is a +/// heuristic choice among `EI` candidates. The renderer's choice decides which operators the +/// page draws after the image, and redaction must see the same ones, so the end is taken from +/// the renderer's tokenizer (hayro-syntax) rather than worked out here by another rule. Only +/// when that tokenizer reads no image does this parser fall back to its own rules, and the +/// image is marked ambiguous. +fn inline_image(data: &[u8], bi: usize, pos: usize) -> Inline { let mut lx = Lexer::new(data, pos); let mut dict = Dict::new(); + let mut readable = true; + let mut stop = pos; loop { lx.skip_ws(); if lx.eat_keyword(b"ID") { @@ -216,22 +246,68 @@ fn inline_image(data: &[u8], pos: usize) -> Result<(Dict, Vec, usize), usize let Some((k, v)) = entry else { // A stream that ends inside the parameters is cut short, not unreadable: keep what was read. if at >= data.len() { - return Ok((dict, Vec::new(), data.len())); + return Inline { image: Some((dict, Vec::new())), end: data.len(), ambiguous: false }; } - // Without an `EI` after it, the image runs to the end of the stream. - return Err(end_of_image(data, at, b"").map_or(data.len(), |(_, end)| end)); + readable = false; + stop = at; + break; }; dict.set(k, v); } + if let Some((start, end_data, end)) = renderer_image_end(data, bi) { + // Where the renderer ends the data at an `EI` found by its heuristic, one whitespace byte + // before that `EI` separates it from the data. + let mut samples = data.get(start..end_data).unwrap_or_default(); + if data.get(end_data..).is_some_and(|rest| rest.starts_with(b"EI")) + && let Some((last, rest)) = samples.split_last() + && is_ws(*last) + { + samples = rest; + } + return Inline { image: readable.then(|| (dict, samples.to_vec())), end, ambiguous: false }; + } + if !readable { + // Without an `EI` after it, the image runs to the end of the stream. + return Inline { image: None, end: end_of_image(data, stop, b"").map_or(data.len(), |(_, end)| end), ambiguous: true }; + } // One whitespace byte separates ID from the data. let begin = (lx.pos + 1).min(data.len()); let end = unfiltered_image_end(data, begin, &dict) .or_else(|| declared_image_end(data, begin, &dict)) .or_else(|| image_data_end(data, begin, filter_eod(&dict))); - Ok(match end { - Some((end_data, end)) => (dict, data[begin..end_data].to_vec(), end), - None => (dict, data[begin..].to_vec(), data.len()), - }) + let (image, end) = match end { + Some((end_data, end)) => (data.get(begin..end_data).unwrap_or_default().to_vec(), end), + None => (data.get(begin..).unwrap_or_default().to_vec(), data.len()), + }; + Inline { image: Some((dict, image)), end, ambiguous: true } +} + +/// Where the renderer's tokenizer ends the inline image whose `BI` starts at `bi`: the start and +/// end of its data and the position after its `EI`. `None` when it reads no image there (its +/// parameters don't read, or no `EI` passes its checks). +fn renderer_image_end(data: &[u8], bi: usize) -> Option<(usize, usize, usize)> { + use hayro_syntax::content::TypedIter; + use hayro_syntax::content::ops::{InlineImage, TypedInstruction}; + let tail = data.get(bi..)?; + let mut iter = TypedIter::new(tail); + let (start, len) = match iter.next()? { + TypedInstruction::InlineImage(InlineImage(stream)) => match stream.raw_data() { + // The data is a slice of `tail`: its offset is where it starts. + std::borrow::Cow::Borrowed(samples) => ((samples.as_ptr() as usize).checked_sub(tail.as_ptr() as usize)?, samples.len()), + std::borrow::Cow::Owned(_) => return None, + }, + _ => return None, + }; + let end_data = start.checked_add(len)?; + // `committed` is past the whitespace after `EI`; the image ends at the `EI`. + let mut end = iter.committed(); + while end > end_data && tail.get(end - 1).is_some_and(|b| is_ws(*b)) { + end -= 1; + } + if end < end_data.checked_add(2)? || tail.get(end - 2..end) != Some(b"EI") { + return None; + } + Some((bi.checked_add(start)?, bi.checked_add(end_data)?, bi.checked_add(end)?)) } /// The end-of-data marker (`>` for ASCIIHex, `~>` for ASCII85) of the outer stream filter, if any. diff --git a/crates/content/src/tests.rs b/crates/content/src/tests.rs index 094a0665d..c9abef834 100644 --- a/crates/content/src/tests.rs +++ b/crates/content/src/tests.rs @@ -102,10 +102,11 @@ fn unfiltered_inline_masks_default_to_one_bit_and_indexed_images_have_one_compon let parsed = inline_pixels(parameters.as_bytes(), b" EI "); assert_eq!(parsed.ops[1].inline.as_ref().unwrap().1, b" EI "); assert_eq!(parsed.ops.len(), 3); - // Indexed samples permit at most 8 bits, even though device spaces permit 16. + // Indexed samples permit at most 8 bits, even though device spaces permit 16, so the + // length is unknown, and the renderer takes the second `EI`. let invalid = parameters.replace("/W 4", "/W 2").replace("/BPC 8", "/BPC 16"); let parsed = inline_pixels(invalid.as_bytes(), b" EI "); - assert!(parsed.ops[1].inline.as_ref().unwrap().1.is_empty()); + assert_eq!(parsed.ops[1].inline.as_ref().unwrap().1, b" EI "); } } @@ -127,7 +128,7 @@ fn unfiltered_inline_image_terminator_must_follow_its_samples() { } #[test] -fn filtered_unknown_and_damaged_inline_images_keep_the_existing_fallback() { +fn filtered_unknown_and_damaged_inline_images_end_where_the_renderer_ends_them() { for parameters in [ b"/W 4 /H 1 /BPC 8 /CS /G /F /A85".as_slice(), b"/W 4 /H 1 /BPC 8 /CS /ResourceColor", @@ -142,12 +143,20 @@ fn filtered_unknown_and_damaged_inline_images_keep_the_existing_fallback() { b"/W 9223372036854775807 /H 9223372036854775807 /BPC 16 /CS /CMYK", b"/W 9223372036854775807 /H 2 /BPC 8 /CS /G", ] { + // What follows the first `EI` (`!!~>`) does not read as content, so the renderer takes + // the later one. let parsed = inline_pixels(parameters, b"! EI !!~>"); - assert_eq!(parsed.ops[1].inline.as_ref().unwrap().1, b"!", "{}", String::from_utf8_lossy(parameters)); + let label = String::from_utf8_lossy(parameters); + assert_eq!(parsed.ops[1].inline.as_ref().unwrap().1, b"! EI !!~>", "{label}"); + assert_eq!(parsed.ops.iter().map(|op| op.op.as_slice()).collect::>(), [b"q".as_slice(), b"BI", b"Q"], "{label}"); + assert_eq!(parsed.ambiguous_images, 0, "{label}"); } - // A short payload still uses the tolerant recovery, including when there is no EI. - for src in [b"BI /W 4 /H 1 /BPC 8 /CS /G ID abc\nEI Q".as_slice(), b"BI /W 4 /H 1 /BPC 8 /CS /G ID abc"] { - assert_eq!(parse(src).ops[0].inline.as_ref().unwrap().1, b"abc"); + // A short payload is read whole; without an `EI`, the renderer reads no image and this + // parser's end is a guess. + for (src, ambiguous) in [(b"BI /W 4 /H 1 /BPC 8 /CS /G ID abc\nEI Q".as_slice(), 0), (b"BI /W 4 /H 1 /BPC 8 /CS /G ID abc", 1)] { + let parsed = parse(src); + assert_eq!(parsed.ops[0].inline.as_ref().unwrap().1, b"abc"); + assert_eq!(parsed.ambiguous_images, ambiguous); } } @@ -459,3 +468,38 @@ fn ascii85_inline_image_does_not_terminate_at_gt_ei_inside_payload() { assert_eq!(img, b"!!!!>EI !!~>"); } } + +/// The operators the renderer's tokenizer reads from `data`. +fn renderer_operators(data: &[u8]) -> Vec> { + let mut iter = hayro_syntax::content::UntypedIter::new(data); + let mut out = Vec::new(); + while let Some(instruction) = iter.next() { + out.push(instruction.operator.as_ref().to_vec()); + } + out +} + +/// Inline images end where the renderer ends them, so both read the same operators after one: +/// an `EI` without whitespace before it in the data, upper-case text ending in "EI" after the +/// image, and an `EI` inside compressed bytes (#3579). +#[test] +fn inline_images_end_where_the_renderer_ends_them() { + let flate: Vec = (0..FLATE_WITH_EI.len()).step_by(2).map(|i| u8::from_str_radix(&FLATE_WITH_EI[i..i + 2], 16).unwrap()).collect(); + let mut stored = b"q BI /W 64 /H 1 /BPC 8 /CS /G /F /Fl ID ".to_vec(); + stored.extend_from_slice(&flate); + stored.extend_from_slice(b"\nEI Q BT /F1 24 Tf 72 200 Td (AFTER) Tj ET"); + let fixtures: [&[u8]; 6] = [ + b"BI /W 1 /H 1 /CS /G /BPC 8 /F /AHx ID 7FEI 1 0 0 1 200 0 cm EI BT /F1 10 Tf 10 100 Td (SECRET) Tj ET", + b"BI /W 1 /H 1 /CS /G /BPC 8 /F /AHx ID 7F> EI BT /F1 10 Tf 10 100 Td (ZWEI) Tj (EIN FREI BEI) Tj ET", + b"q BI /W 2 /H 1 /CS /G /BPC 8 /F /AHx ID 7F7F> EI Q BT (DREI) Tj ET BI /W 1 /H 1 /CS /G /BPC 8 /F /AHx ID 00> EI", + b"BI /W 4 /H 1 /BPC 8 /CS /G /F /A85 ID ! EI !!~>\nEI Q", + b"q BI /W 4 /H 1 /BPC 8 /CS /G ID EI \nEI Q", + &stored, + ]; + for data in fixtures { + let parsed = parse(data); + let ours: Vec> = parsed.ops.iter().map(|op| op.op.clone()).collect(); + assert_eq!(ours, renderer_operators(data), "{}", String::from_utf8_lossy(data)); + assert_eq!((parsed.skipped, parsed.ambiguous_images), (0, 0), "{}", String::from_utf8_lossy(data)); + } +} diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 8937cf678..3012a1f3f 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2320,6 +2320,30 @@ fn the_renderer_check_refuses_text_still_shown_under_a_mark() { assert!(refused(&cos), "the labelled input stream was not checked"); } +/// Inline image data `7FEI 1 0 0 1 200 0 cm` holds an `EI` the renderer ends the image at, so it +/// runs the `cm` and shows SECRET 200 pt to the right. Redaction reads the image the same way, so +/// a mark drawn over the word as shown takes it out of the file. +#[test] +fn text_after_an_inline_image_is_redacted_where_the_page_shows_it() { + let mut s = Session::new().with_clock(|| 1_700_000_000); + let page = one_line_page( + "/Font << /F1 4 0 R >>", + "BI /W 1 /H 1 /CS /G /BPC 8 /F /AHx ID 7FEI 1 0 0 1 200 0 cm EI BT /F1 10 Tf 10 100 Td (SECRET) Tj ET", + ); + let id = s.open("inline.pdf", None, Arc::new(page), None).unwrap(); + mark_shown_text(&mut s, id, "SECRET"); + s.apply(id, Edit::ApplyRedactions { pages: None }).unwrap(); + assert!(!page_texts(&s, id).concat().contains("SECRET")); + let saved = s.save_bytes(id).unwrap(); + let held = |d: &[u8]| d.windows(6).any(|w| w == b"SECRET"); + assert!(!held(&saved), "the saved file holds the redacted word"); + let reopened = pdfcraft_cos::Document::open(saved).unwrap(); + assert!( + !reopened.scan_objects().any(|(_, o)| matches!(&*o, pdfcraft_cos::Object::Stream(st) if st.decoded().is_ok_and(|d| held(&d)))), + "a saved stream holds the redacted word" + ); +} + /// A word whose font and size an ExtGState /Font entry (`gs`) sets is placed where the renderer /// draws it, so a mark drawn over it as displayed takes it out of the file. #[test] diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index fca3a1da8..b4186fd4e 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -74,6 +74,10 @@ pub(crate) struct Output { /// Apply mode met vertical-writing text (which it cannot remove) while a redaction region was /// in force: the operation must fail closed rather than leave that text under the mark. pub vertical: bool, + /// The content holds an inline image the renderer reads no image for + /// ([`pdfcraft_content::Parsed::ambiguous_images`]): what this scope placed after it may not + /// be what the page shows, so a redaction must not trust it. + pub ambiguous: bool, pub tag_changes: Vec<(ObjRef, ObjRef, HashSet, HashSet)>, pub error: Option, } @@ -789,6 +793,7 @@ fn process_from(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec], // next. Parse them joined, then give each operator back to the piece its keyword is in. let joined = Pieces::join(streams); let mut parsed = joined.parse(); + out.ambiguous = parsed.ambiguous_images > 0; // A rewritten piece loses its comments, so keep where they began and ended: a comment that // ran on into the next piece still hides that piece's first line. let comment_edges = joined.comment_edges(&parsed); @@ -1293,6 +1298,7 @@ fn xobject( scope.fonts = saved_fonts; out.residue += inner.residue; out.vertical |= inner.vertical; + out.ambiguous |= inner.ambiguous; out.doomed.extend_from_slice(&inner.doomed); out.retired_properties.extend(inner.retired_properties); out.tag_changes.extend(inner.tag_changes); diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 97d5bcd9f..e67b373a1 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -411,6 +411,12 @@ pub fn apply(doc: &mut Document, pages: Option<&[usize]>) -> Result EI BT /F1 10 Tf 10 100 Td (SECRET) Tj ET", "", vec![]); + mark(&mut doc, 0, &[[5.0, 95.0, 45.0, 110.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().glyphs, 6); + assert!(!content(&doc, 0).contains("SECRET")); +} + +/// Without an `EI` the renderer accepts, where the operators resume is a guess: refused. +#[test] +fn a_page_whose_inline_image_the_renderer_cannot_end_is_refused() { + let mut doc = one_page(b"BT /F1 10 Tf 10 100 Td (SECRET) Tj ET BI /W 1 /H 1 /CS /G /BPC 8 /F /AHx ID 7F", "", vec![]); + mark(&mut doc, 0, &[[5.0, 95.0, 45.0, 110.0]], ""); + assert_eq!(apply(&mut doc, None), Err(RedactError::Unreadable(1))); +} From 01aa6bcc17b09ad294777d7df49f6b52d6c275fe Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:31:38 +0000 Subject: [PATCH 18/20] docs(redact): describe the overlay, text state, description and inline image rules --- crates/content/README.md | 11 +++++++---- crates/redact/README.md | 22 ++++++++++++++++------ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/crates/content/README.md b/crates/content/README.md index 6ab984dd4..fedeadc56 100644 --- a/crates/content/README.md +++ b/crates/content/README.md @@ -13,7 +13,10 @@ The parser is tolerant (unreadable bytes are skipped and counted, never fatal) a panics. `redact` and (later) `edit` build their interpreters on it; the graphics-state tracker and the `Patch` API of architecture §6 grow here. -Unfiltered inline images with built-in or Indexed colour spaces, and image masks, use their -sample layout (including per-row byte padding) to locate `EI`. This preserves pixel bytes -that look like the terminator. Filtered images and resource-defined colour spaces retain -the tolerant delimiter heuristic; damaged or truncated layouts fall back to it too. +An inline image ends where the renderer's tokenizer (`hayro-syntax`) ends it, so the parser +and the renderer read the same operators after the image, and redaction removes what the page +actually draws. Only where that tokenizer reads no image (its parameters don't read, or no `EI` +passes its checks) does the parser locate `EI` by its own rules: unfiltered images with built-in +or Indexed colour spaces, and image masks, by their sample layout (including per-row byte +padding), other images by the tolerant delimiter heuristic. Such an image is counted in +`Parsed::ambiguous_images`, because the operators after it may not be the ones the page draws. diff --git a/crates/redact/README.md b/crates/redact/README.md index 2caf067b5..cca3d51fd 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -20,7 +20,8 @@ clear_marks(&mut doc, None)?; // remove marks without app - **images**: fully covered → removed; partly covered → a copy with the covered pixels cleared (8/16-bit and 1/2/4-bit images, image masks; Flate/LZW/RL/A85/AHx). Codecs PdfCraft can't re-encode (DCT, JPX, JBIG2, CCITT) are removed whole (fail-closed); inline images under an - area are removed. Images with separate `/SMask` or `/Mask` data are removed whole; + area are removed. Images with separate `/SMask` or `/Mask` data, and images whose data decodes + to more than twice their declared raster (plus a byte per row and 4 KiB), are removed whole; cleared copies drop `/Alternates` so alternate pixels cannot survive; - **vectors**: covered paths are removed; partly covered paths and shadings are clipped so nothing paints inside the areas; @@ -31,8 +32,13 @@ clear_marks(&mut doc, None)?; // remove marks without app page as a `/PCMark /Redaction` stream. A verification pass re-reads each redacted page and fails the operation if any glyph or inline -image is still under an area. Content that can't be decoded makes the operation fail rather than -leave it unredacted. A Redact annotation with neither a usable `/QuadPoints` array nor a usable +image is still under an area. It checks every content stream the page had, whatever its +dictionary calls it; only the overlay streams this `apply` added (`Report::overlays`) are skipped. +Text is placed with the font and size set by `Tf` or by an ExtGState's `/Font` through `gs`, and a +form XObject starts from the text state of the content that draws it. Content that can't be +decoded makes the operation fail rather than leave it unredacted, and so does an inline image the +renderer's tokenizer reads no image for (`RedactError::Unreadable`): where the page's operators +resume after it would be a guess. A Redact annotation with neither a usable `/QuadPoints` array nor a usable `/Rect` makes it fail with `RedactError::UnreadableMark` before anything changes, instead of skipping that mark. The count covers the marks on the pages being applied. @@ -50,15 +56,19 @@ as Sanitize drops them, so a full rewrite of a tagged PDF can't keep a removed c A tree where that can't be done (an element written in place, or one too large to walk) makes `apply` fail with `RedactError::Tags` rather than leave the annotation reachable. -Replacement descriptions are removed only from marked-content spans changed by cleanup. -Unchanged spans retain their descriptions; unused property entries are retired during redaction +Replacement descriptions (`/Alt`, `/ActualText`, `/E`) are removed from marked-content spans +changed by cleanup, from every structure element above them (found through both `/P` and `/K`), +and from elements that lose an annotation reference, which also lose `/T`. An element written in +place above one whose descriptions go makes `apply` fail with `RedactError::Tags`. Unchanged +spans retain their descriptions; unused property entries are retired during redaction and hidden-content cleanup, including aliases in shared/inherited resources. Descriptions still used by surviving pages or forms are kept. A redacted page that inherits `/Resources` from the page tree gets them pinned (by reference; a direct inherited dictionary is made one shared object first) before it is rewritten. The parent's `/Resources` is then retired unless another page still inherits it, so it cannot keep a removed -XObject reachable. Pages that aren't redacted are left untouched. +XObject reachable. Surviving form XObjects lose the resource entries for removed XObjects their own +content doesn't draw, for the same reason. Pages that aren't redacted are left untouched. ## Remove hidden information / Sanitize From 8555d6f159a7b2d7d420185642b8901719b457a6 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:43:38 +0000 Subject: [PATCH 19/20] fix(parity): cite the renamed renderer-check test and drop fixed redaction gaps --- parity/acrobat-features.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 362c2aa77..45dda3982 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -5709,8 +5709,8 @@ milestone = "M8" status = "shipped" commands = ["redact.apply"] tools = ["redact_apply"] -evidence = ["crates/ui-egui/tests/it/redact.rs::marking_applying_and_clearing", "crates/engine/src/tests.rs::redaction_marks_apply_for_good_and_undo", "crates/automation/tests/it/automation.rs::redacting_through_tools", "crates/engine/src/tests.rs::redaction_is_refused_when_the_page_still_shows_text_under_a_mark", "crates/redact/src/tests.rs::redacted_xobjects_do_not_survive_through_inherited_or_shared_resources"] -notes = "confirmation; one undo step; next save is a full rewrite. Refused, with nothing changed, when the page still shows text under a mark afterwards: text whose font is set through an ExtGState or taken by a form from its caller, some predefined CMaps and rotated Type 3 fonts are not removed yet, so those documents cannot be redacted. Removed images and forms are unlinked from page-tree resources (own, inherited or shared), not yet from a form's own resources, annotation appearances or patterns" +evidence = ["crates/ui-egui/tests/it/redact.rs::marking_applying_and_clearing", "crates/engine/src/tests.rs::redaction_marks_apply_for_good_and_undo", "crates/automation/tests/it/automation.rs::redacting_through_tools", "crates/engine/src/tests.rs::the_renderer_check_refuses_text_still_shown_under_a_mark", "crates/redact/src/tests.rs::redacted_xobjects_do_not_survive_through_inherited_or_shared_resources"] +notes = "confirmation; one undo step; next save is a full rewrite. Refused, with nothing changed, when the page still shows text under a mark afterwards: text in some predefined CMaps and rotated Type 3 fonts is not removed yet, so those documents cannot be redacted. Also refused on a page with an inline image the renderer reads no image for, since the operators after it can't be known. Removed images and forms are unlinked from page-tree resources (own, inherited or shared) and from surviving forms' resources, not yet from annotation appearances or patterns" [[feature]] id = "protect.redact-glyph-removal" From 39844f06b58bc96bae72d3fb56faaa3b6201c945 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Thu, 22 Oct 2026 15:39:48 -0400 Subject: [PATCH 20/20] Release 0.12.0 Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 72 +++++++++++++++++++++++++++--------------------------- Cargo.toml | 68 +++++++++++++++++++++++++-------------------------- 2 files changed, 70 insertions(+), 70 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e318b7a77..651843001 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3936,7 +3936,7 @@ dependencies = [ [[package]] name = "pdfcraft" -version = "0.11.0" +version = "0.12.0" dependencies = [ "chrono", "craft-quit", @@ -3962,7 +3962,7 @@ dependencies = [ [[package]] name = "pdfcraft-a11y" -version = "0.11.0" +version = "0.12.0" dependencies = [ "pdfcraft-content", "pdfcraft-cos", @@ -3974,7 +3974,7 @@ dependencies = [ [[package]] name = "pdfcraft-annot" -version = "0.11.0" +version = "0.12.0" dependencies = [ "hayro-syntax", "pdfcraft-cos", @@ -3985,7 +3985,7 @@ dependencies = [ [[package]] name = "pdfcraft-audit" -version = "0.11.0" +version = "0.12.0" dependencies = [ "pdfcraft-content", "pdfcraft-cos", @@ -3996,7 +3996,7 @@ dependencies = [ [[package]] name = "pdfcraft-automation" -version = "0.11.0" +version = "0.12.0" dependencies = [ "base64 0.22.1", "flate2", @@ -4018,7 +4018,7 @@ dependencies = [ [[package]] name = "pdfcraft-cli" -version = "0.11.0" +version = "0.12.0" dependencies = [ "base64 0.22.1", "pdfcraft-audit", @@ -4036,11 +4036,11 @@ dependencies = [ [[package]] name = "pdfcraft-compare" -version = "0.11.0" +version = "0.12.0" [[package]] name = "pdfcraft-content" -version = "0.11.0" +version = "0.12.0" dependencies = [ "hayro-syntax", "pdfcraft-cos", @@ -4048,7 +4048,7 @@ dependencies = [ [[package]] name = "pdfcraft-cos" -version = "0.11.0" +version = "0.12.0" dependencies = [ "chrono", "hayro-syntax", @@ -4063,7 +4063,7 @@ dependencies = [ [[package]] name = "pdfcraft-create" -version = "0.11.0" +version = "0.12.0" dependencies = [ "encoding_rs", "hayro-syntax", @@ -4079,7 +4079,7 @@ dependencies = [ [[package]] name = "pdfcraft-crypt" -version = "0.11.0" +version = "0.12.0" dependencies = [ "aes", "md-5", @@ -4091,7 +4091,7 @@ dependencies = [ [[package]] name = "pdfcraft-edit" -version = "0.11.0" +version = "0.12.0" dependencies = [ "hayro-syntax", "kurbo 0.13.1", @@ -4106,7 +4106,7 @@ dependencies = [ [[package]] name = "pdfcraft-engine" -version = "0.11.0" +version = "0.12.0" dependencies = [ "chrono", "craft-docpath", @@ -4147,14 +4147,14 @@ dependencies = [ [[package]] name = "pdfcraft-export" -version = "0.11.0" +version = "0.12.0" dependencies = [ "flate2", ] [[package]] name = "pdfcraft-filters" -version = "0.11.0" +version = "0.12.0" dependencies = [ "flate2", "proptest", @@ -4164,7 +4164,7 @@ dependencies = [ [[package]] name = "pdfcraft-fonts" -version = "0.11.0" +version = "0.12.0" dependencies = [ "harfrust", "pdfcraft-cos", @@ -4174,7 +4174,7 @@ dependencies = [ [[package]] name = "pdfcraft-forms" -version = "0.11.0" +version = "0.12.0" dependencies = [ "hayro-syntax", "pdfcraft-content", @@ -4187,11 +4187,11 @@ dependencies = [ [[package]] name = "pdfcraft-geom" -version = "0.11.0" +version = "0.12.0" [[package]] name = "pdfcraft-js" -version = "0.11.0" +version = "0.12.0" dependencies = [ "boa_engine", "chrono", @@ -4199,7 +4199,7 @@ dependencies = [ [[package]] name = "pdfcraft-measure" -version = "0.11.0" +version = "0.12.0" dependencies = [ "pdfcraft-annot", "pdfcraft-content", @@ -4212,14 +4212,14 @@ dependencies = [ [[package]] name = "pdfcraft-model" -version = "0.11.0" +version = "0.12.0" dependencies = [ "pdfcraft-cos", ] [[package]] name = "pdfcraft-ocr" -version = "0.11.0" +version = "0.12.0" dependencies = [ "ocrs", "pdfcraft-fonts", @@ -4229,7 +4229,7 @@ dependencies = [ [[package]] name = "pdfcraft-optimize" -version = "0.11.0" +version = "0.12.0" dependencies = [ "image", "pdfcraft-annot", @@ -4242,11 +4242,11 @@ dependencies = [ [[package]] name = "pdfcraft-oracle" -version = "0.11.0" +version = "0.12.0" [[package]] name = "pdfcraft-organize" -version = "0.11.0" +version = "0.12.0" dependencies = [ "hayro-syntax", "pdfcraft-content", @@ -4257,7 +4257,7 @@ dependencies = [ [[package]] name = "pdfcraft-platform" -version = "0.11.0" +version = "0.12.0" dependencies = [ "async-channel", "futures-lite", @@ -4267,7 +4267,7 @@ dependencies = [ [[package]] name = "pdfcraft-preflight" -version = "0.11.0" +version = "0.12.0" dependencies = [ "flate2", "pdfcraft-content", @@ -4278,7 +4278,7 @@ dependencies = [ [[package]] name = "pdfcraft-print" -version = "0.11.0" +version = "0.12.0" dependencies = [ "craft-print", "pdfcraft-content", @@ -4290,7 +4290,7 @@ dependencies = [ [[package]] name = "pdfcraft-redact" -version = "0.11.0" +version = "0.12.0" dependencies = [ "pdfcraft-annot", "pdfcraft-content", @@ -4306,7 +4306,7 @@ dependencies = [ [[package]] name = "pdfcraft-render" -version = "0.11.0" +version = "0.12.0" dependencies = [ "bytemuck", "flate2", @@ -4325,7 +4325,7 @@ dependencies = [ [[package]] name = "pdfcraft-sign" -version = "0.11.0" +version = "0.12.0" dependencies = [ "aes", "aws-lc-rs", @@ -4360,11 +4360,11 @@ dependencies = [ [[package]] name = "pdfcraft-testkit" -version = "0.11.0" +version = "0.12.0" [[package]] name = "pdfcraft-ui-egui" -version = "0.11.0" +version = "0.12.0" dependencies = [ "arboard", "base64 0.22.1", @@ -4409,7 +4409,7 @@ dependencies = [ [[package]] name = "pdfcraft-web" -version = "0.11.0" +version = "0.12.0" dependencies = [ "eframe", "egui", @@ -4421,7 +4421,7 @@ dependencies = [ [[package]] name = "pdfcraft-xfa" -version = "0.11.0" +version = "0.12.0" dependencies = [ "base64 0.22.1", "image", @@ -4435,7 +4435,7 @@ dependencies = [ [[package]] name = "pdfcraft-xfdf" -version = "0.11.0" +version = "0.12.0" dependencies = [ "pdfcraft-annot", "pdfcraft-cos", diff --git a/Cargo.toml b/Cargo.toml index 201240314..5841b50c6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ exclude = ["vendor"] default-members = ["crates/*", "apps/pdfcraft", "apps/pdfcraft-cli", "xtask"] [workspace.package] -version = "0.11.0" +version = "0.12.0" edition = "2024" license = "MIT OR Apache-2.0" rust-version = "1.95" @@ -21,39 +21,39 @@ craft-print = { git = "https://github.com/storytold/craft-libs", version = "0.1. # Whether a path stored in a document may be opened (GoToR link targets). craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "82a24b006eefa036b43c09a67fb9a3fbd17ffe63" } # internal -pdfcraft-geom = { path = "crates/geom", version = "0.11.0" } -pdfcraft-filters = { path = "crates/filters", version = "0.11.0" } -pdfcraft-cos = { path = "crates/cos", version = "0.11.0" } -pdfcraft-crypt = { path = "crates/crypt", version = "0.11.0" } -pdfcraft-organize = { path = "crates/organize", version = "0.11.0" } -pdfcraft-measure = { path = "crates/measure", version = "0.11.0" } -pdfcraft-annot = { path = "crates/annot", version = "0.11.0" } -pdfcraft-audit = { path = "crates/audit", version = "0.11.0" } -pdfcraft-fonts = { path = "crates/fonts", version = "0.11.0" } -pdfcraft-model = { path = "crates/model", version = "0.11.0" } -pdfcraft-content = { path = "crates/content", version = "0.11.0" } -pdfcraft-redact = { path = "crates/redact", version = "0.11.0" } -pdfcraft-print = { path = "crates/print", version = "0.11.0" } -pdfcraft-xfdf = { path = "crates/xfdf", version = "0.11.0" } -pdfcraft-sign = { path = "crates/sign", version = "0.11.0" } -pdfcraft-optimize = { path = "crates/optimize", version = "0.11.0" } -pdfcraft-a11y = { path = "crates/a11y", version = "0.11.0" } -pdfcraft-oracle = { path = "crates/oracle", version = "0.11.0" } -pdfcraft-testkit = { path = "crates/testkit", version = "0.11.0" } -pdfcraft-ocr = { path = "crates/ocr", version = "0.11.0" } -pdfcraft-js = { path = "crates/js", version = "0.11.0" } -pdfcraft-compare = { path = "crates/compare", version = "0.11.0" } -pdfcraft-preflight = { path = "crates/preflight", version = "0.11.0" } -pdfcraft-export = { path = "crates/export", version = "0.11.0" } -pdfcraft-edit = { path = "crates/edit", version = "0.11.0" } -pdfcraft-create = { path = "crates/create", version = "0.11.0" } -pdfcraft-forms = { path = "crates/forms", version = "0.11.0" } -pdfcraft-xfa = { path = "crates/xfa", version = "0.11.0" } -pdfcraft-render = { path = "crates/render", version = "0.11.0" } -pdfcraft-engine = { path = "crates/engine", version = "0.11.0" } -pdfcraft-platform = { path = "crates/platform", version = "0.11.0" } -pdfcraft-ui-egui = { path = "crates/ui-egui", version = "0.11.0" } -pdfcraft-automation = { path = "crates/automation", version = "0.11.0", default-features = false } +pdfcraft-geom = { path = "crates/geom", version = "0.12.0" } +pdfcraft-filters = { path = "crates/filters", version = "0.12.0" } +pdfcraft-cos = { path = "crates/cos", version = "0.12.0" } +pdfcraft-crypt = { path = "crates/crypt", version = "0.12.0" } +pdfcraft-organize = { path = "crates/organize", version = "0.12.0" } +pdfcraft-measure = { path = "crates/measure", version = "0.12.0" } +pdfcraft-annot = { path = "crates/annot", version = "0.12.0" } +pdfcraft-audit = { path = "crates/audit", version = "0.12.0" } +pdfcraft-fonts = { path = "crates/fonts", version = "0.12.0" } +pdfcraft-model = { path = "crates/model", version = "0.12.0" } +pdfcraft-content = { path = "crates/content", version = "0.12.0" } +pdfcraft-redact = { path = "crates/redact", version = "0.12.0" } +pdfcraft-print = { path = "crates/print", version = "0.12.0" } +pdfcraft-xfdf = { path = "crates/xfdf", version = "0.12.0" } +pdfcraft-sign = { path = "crates/sign", version = "0.12.0" } +pdfcraft-optimize = { path = "crates/optimize", version = "0.12.0" } +pdfcraft-a11y = { path = "crates/a11y", version = "0.12.0" } +pdfcraft-oracle = { path = "crates/oracle", version = "0.12.0" } +pdfcraft-testkit = { path = "crates/testkit", version = "0.12.0" } +pdfcraft-ocr = { path = "crates/ocr", version = "0.12.0" } +pdfcraft-js = { path = "crates/js", version = "0.12.0" } +pdfcraft-compare = { path = "crates/compare", version = "0.12.0" } +pdfcraft-preflight = { path = "crates/preflight", version = "0.12.0" } +pdfcraft-export = { path = "crates/export", version = "0.12.0" } +pdfcraft-edit = { path = "crates/edit", version = "0.12.0" } +pdfcraft-create = { path = "crates/create", version = "0.12.0" } +pdfcraft-forms = { path = "crates/forms", version = "0.12.0" } +pdfcraft-xfa = { path = "crates/xfa", version = "0.12.0" } +pdfcraft-render = { path = "crates/render", version = "0.12.0" } +pdfcraft-engine = { path = "crates/engine", version = "0.12.0" } +pdfcraft-platform = { path = "crates/platform", version = "0.12.0" } +pdfcraft-ui-egui = { path = "crates/ui-egui", version = "0.12.0" } +pdfcraft-automation = { path = "crates/automation", version = "0.12.0", default-features = false } # pdf hayro = "0.7.1"