diff --git a/crates/audit/src/lib.rs b/crates/audit/src/lib.rs index 98efe2fed..f1f135098 100644 --- a/crates/audit/src/lib.rs +++ b/crates/audit/src/lib.rs @@ -182,7 +182,9 @@ fn audit(doc: &Document, limits: Limits) -> Result, AuditError let mut budget = Budget { limits, comparisons: 0, page_cost: 0, covered_text: 0 }; let mut out = Vec::new(); for (n, page) in pdfcraft_model::pages(doc).iter().enumerate() { - let lines = pdfcraft_edit::text_lines(doc, n).map_err(|e| AuditError::Text(n, e.to_string()))?; + // The glyphs themselves: `/ActualText` is the file's own claim about them, and an empty + // or substitute one must not hide text a cover sits on. + let lines = pdfcraft_edit::glyph_text_lines(doc, page).map_err(|e| AuditError::Text(n, e.to_string()))?; budget.start_page(&lines); // 1. Opaque filled rectangles over extractable text. for r in cover::filled_rects(doc, &page.dict)? { diff --git a/crates/audit/src/tests.rs b/crates/audit/src/tests.rs index 3b5c5c8c7..3aca406e7 100644 --- a/crates/audit/src/tests.rs +++ b/crates/audit/src/tests.rs @@ -725,6 +725,19 @@ fn withheld_covered_text_is_not_assembled() { assert_eq!(copied.as_deref(), Some("SECRET DATA 123")); } +/// `/ActualText` is metadata the file's author controls: an empty or substitute replacement must +/// not hide the glyphs a cover sits on. +#[test] +fn a_cover_over_text_with_empty_or_substitute_actual_text_is_found() { + for actual in ["()", "(nothing here)"] { + let content = + format!("/Span << /ActualText {actual} >> BDC\nBT /F1 24 Tf 100 700 Td (SECRET DATA 123) Tj ET\nEMC\n0 g\n100 688 220 32 re f\n"); + let findings = audit_redactions(&open(one_page(content.as_bytes(), ""))).expect("audit must run"); + assert_eq!(findings.len(), 1, "{actual}: {findings:?}"); + assert!(findings[0].covered_text.contains("SECRET"), "{actual}: covered_text was {:?}", findings[0].covered_text); + } +} + #[test] fn repeated_stream_references_share_one_decode_budget() { use pdfcraft_cos::{Dict, Document, Object, Stream}; diff --git a/crates/create/src/lib.rs b/crates/create/src/lib.rs index 5717977b1..7d1fdfc36 100644 --- a/crates/create/src/lib.rs +++ b/crates/create/src/lib.rs @@ -393,17 +393,38 @@ fn rgba_image(rgba: &[u8], (w, h): (u32, u32), dpi: (f64, f64)) -> Embedded { /// BMP and GIF (the first frame), through the `image` decoders. Their resolution is not read: /// 72 dpi, as for images without one. fn decoded(name: &str, bytes: &[u8], format: image::ImageFormat) -> Result { + // A GIF's first frame is painted on a canvas the size of its logical screen, however small + // the frame: a few bytes can ask for hundreds of MB. + if format == image::ImageFormat::Gif + && let Some(&[w0, w1, h0, h1]) = bytes.get(6..10) + && u64::from(u16::from_le_bytes([w0, w1])) * u64::from(u16::from_le_bytes([h0, h1])) > MAX_GIF_PIXELS + { + return Err(CreateError::Image(name.into(), "the GIF's screen is larger than 4096×4096 pixels".into())); + } let img = image::load_from_memory_with_format(bytes, format).map_err(|e| CreateError::Image(name.into(), e.to_string()))?; - let rgba = img.to_rgba8(); + let rgba = img.into_rgba8(); Ok(rgba_image(rgba.as_raw(), rgba.dimensions(), (72.0, 72.0))) } +/// Largest GIF logical screen, in pixels (4096×4096). +const MAX_GIF_PIXELS: u64 = 1 << 24; + /// Most decoded bytes one TIFF page or PNG may take: room for the 768 MiB CMYK poster in #665, /// twice over. const MAX_DECODED_BYTES: usize = 2 << 30; -/// Every page of a TIFF (multi-page scans become multi-page PDFs). -fn tiff_pages(name: &str, bytes: &[u8]) -> Result, CreateError> { +/// Most decoded bytes all the pages of one TIFF may take together: its pages stay in memory until +/// they are embedded, and many small directories can share one compressed strip. Saturates to the +/// whole address space on 32-bit targets (wasm32), where 4 GiB doesn't fit in `usize`. +const MAX_DECODED_TOTAL: usize = MAX_DECODED_BYTES.saturating_mul(2); + +/// The first `wanted` pages of a TIFF (multi-page scans become multi-page PDFs). +fn tiff_pages(name: &str, bytes: &[u8], wanted: usize) -> Result, CreateError> { + tiff_pages_within(name, bytes, wanted, MAX_DECODED_TOTAL) +} + +/// [`tiff_pages`], refused once the pages kept take more than `budget` bytes. +fn tiff_pages_within(name: &str, bytes: &[u8], wanted: usize, budget: usize) -> Result, CreateError> { use tiff::ColorType as C; use tiff::decoder::{Decoder, DecodingResult, Limits}; use tiff::tags::Tag; @@ -415,6 +436,7 @@ fn tiff_pages(name: &str, bytes: &[u8]) -> Result, CreateError> { limits.decoding_buffer_size = MAX_DECODED_BYTES; let mut dec = Decoder::new(std::io::Cursor::new(bytes)).map_err(|e| bad(e.to_string()))?.with_limits(limits); let mut out = Vec::new(); + let mut kept = 0usize; loop { let (w, h) = dec.dimensions().map_err(|e| bad(e.to_string()))?; let ct = dec.colortype().map_err(|e| bad(e.to_string()))?; @@ -469,8 +491,12 @@ fn tiff_pages(name: &str, bytes: &[u8]) -> Result, CreateError> { } other => return Err(bad(format!("{other:?} TIFF images aren't supported yet"))), }; + kept = kept.saturating_add(page.data.len()).saturating_add(page.smask.as_ref().map_or(0, |(_, alpha)| alpha.len())); + if kept > budget { + return Err(bad(format!("the TIFF's pages take more than {} MiB together", budget >> 20))); + } out.push(page); - if !dec.more_images() { + if out.len() >= wanted || !dec.more_images() { break; } dec.next_image().map_err(|e| bad(e.to_string()))?; @@ -526,14 +552,15 @@ pub fn source_kind(name: &str, bytes: &[u8]) -> Option { (lower.ends_with(".txt") || lower.ends_with(".text")).then_some(SourceKind::Text) } -/// Detect the image format from its bytes; a TIFF may hold several pages. -fn embed(name: &str, bytes: &[u8]) -> Result, CreateError> { +/// Detect the image format from its bytes; a TIFF may hold several pages, of which the first +/// `wanted` are read. +fn embed(name: &str, bytes: &[u8], wanted: usize) -> Result, CreateError> { if bytes.starts_with(&[0xFF, 0xD8]) { Ok(vec![jpeg(name, &upright_image::upright(bytes))?]) } else if bytes.starts_with(b"\x89PNG\r\n\x1a\n") { Ok(vec![png_image(name, bytes)?]) } else if is_tiff(bytes) { - tiff_pages(name, bytes) + tiff_pages(name, bytes, wanted) } else if bytes.starts_with(b"BM") { Ok(vec![decoded(name, bytes, image::ImageFormat::Bmp)?]) } else if is_gif(bytes) { @@ -548,7 +575,7 @@ fn embed(name: &str, bytes: &[u8]) -> Result, CreateError> { /// Embed an image file (its first page, for TIFFs) as an image XObject in `doc`. Returns the /// object and the image's natural size in points (from its resolution). pub fn image_xobject(doc: &mut Document, name: &str, bytes: &[u8]) -> Result<(ObjRef, (f64, f64)), CreateError> { - let img = embed(name, bytes)?.into_iter().next().ok_or_else(|| CreateError::Image(name.into(), "the file has no image".into()))?; + let img = embed(name, bytes, 1)?.into_iter().next().ok_or_else(|| CreateError::Image(name.into(), "the file has no image".into()))?; let size = (img.px.0 as f64 * 72.0 / img.dpi.0, img.px.1 as f64 * 72.0 / img.dpi.1); Ok((add_image(doc, img, &mut Vec::new()), size)) } @@ -580,7 +607,7 @@ pub fn from_images_with_resolution(images: &[(String, Vec)], resolution: Ima } let mut doc = Document::new_empty(); let mut profiles = Vec::new(); - for img in images.iter().map(|(name, bytes)| embed(name, bytes)).collect::, _>>()?.into_iter().flatten() { + for img in images.iter().map(|(name, bytes)| embed(name, bytes, usize::MAX)).collect::, _>>()?.into_iter().flatten() { let dpi = match resolution { ImageResolution::Embedded => img.dpi, ImageResolution::Dpi(dpi) => (dpi, dpi), diff --git a/crates/create/src/tests.rs b/crates/create/src/tests.rs index 2007932f7..b6c7969c2 100644 --- a/crates/create/src/tests.rs +++ b/crates/create/src/tests.rs @@ -1206,3 +1206,47 @@ fn a_cyclic_indexed_colour_space_is_refused() { assert!(out.images.is_empty()); assert_eq!(out.skipped.len(), 1, "{:?}", out.skipped); } + +/// A TIFF of `pages` 64×64 gray pages, then (with `unreadable`) one whose samples Create can't read. +fn gray_tiff(pages: usize, unreadable: bool) -> Vec { + let mut tif = std::io::Cursor::new(Vec::new()); + let mut enc = tiff::encoder::TiffEncoder::new(&mut tif).unwrap(); + for _ in 0..pages { + enc.write_image::(64, 64, &[128; 64 * 64]).unwrap(); + } + if unreadable { + enc.write_image::(1, 1, &[0.5; 3]).unwrap(); + } + tif.into_inner() +} + +/// Many small IFDs can each decode to a large page: the pages a TIFF keeps in memory together +/// are bounded, and embedding one image reads only its first page. +#[test] +fn tiff_pages_are_decoded_within_a_total_budget() { + let tif = gray_tiff(3, false); + assert!(matches!(tiff_pages_within("t.tif", &tif, usize::MAX, 10_000), Err(CreateError::Image(..))), "12 KiB of pages past a 10 KB budget"); + assert_eq!(tiff_pages_within("t.tif", &tif, usize::MAX, 20_000).unwrap().len(), 3); + let tif = gray_tiff(1, true); + let mut doc = Document::new_empty(); + assert!(image_xobject(&mut doc, "t.tif", &tif).is_ok(), "the unreadable last page is never decoded"); +} + +/// A one-pixel GIF on a `w`×`h` logical screen: the decoder paints it on a canvas that size. +fn gif_on_screen(w: u16, h: u16) -> Vec { + let mut gif = b"GIF89a".to_vec(); + gif.extend_from_slice(&w.to_le_bytes()); + gif.extend_from_slice(&h.to_le_bytes()); + gif.extend_from_slice(&[0x80, 0, 0, 0xFF, 0xFF, 0xFF, 0, 0, 0]); + gif.extend_from_slice(&[0x2C, 0, 0, 0, 0, 1, 0, 1, 0, 0, 2, 2, 0x44, 1, 0, 0x3B]); + gif +} + +/// The decoder's own limit lets a 74-byte GIF ask for a canvas of hundreds of MB, which the +/// conversion then copies; a screen past [`MAX_GIF_PIXELS`] is refused before decoding. +#[test] +fn a_tiny_gif_on_a_huge_screen_is_refused() { + assert!(matches!(from_images(&[("big.gif".into(), gif_on_screen(4200, 4100))]), Err(CreateError::Image(..)))); + let doc = reopen(&from_images(&[("small.gif".into(), gif_on_screen(40, 30))]).unwrap()); + assert_eq!(media(&pages(&doc)[0]), [0.0, 0.0, 40.0, 30.0]); +} diff --git a/crates/create/src/upright_image.rs b/crates/create/src/upright_image.rs index 9816dfc4b..b89c51719 100644 --- a/crates/create/src/upright_image.rs +++ b/crates/create/src/upright_image.rs @@ -14,6 +14,12 @@ const QUALITY: u8 = 92; /// Larger photos are embedded as they are: turning them would take hundreds of MB. const MAX_PIXELS: u64 = 64 << 20; +#[cfg(test)] +thread_local! { + /// How many JPEGs [`turned`] handed to the decoder on this thread. + static DECODES: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// A JPEG whose EXIF Orientation is not "as stored" has its pixels turned (and re-encoded); /// everything else, and any JPEG this can't read, is returned unchanged. pub(crate) fn upright(bytes: &[u8]) -> Cow<'_, [u8]> { @@ -24,9 +30,11 @@ pub(crate) fn upright(bytes: &[u8]) -> Cow<'_, [u8]> { } fn turned(bytes: &[u8]) -> Option> { - if !bytes.starts_with(&[0xFF, 0xD8]) { + if !bytes.starts_with(&[0xFF, 0xD8]) || extended_xmp_chunks(bytes) > MAX_EXTENDED_XMP_CHUNKS { return None; } + #[cfg(test)] + DECODES.with(|c| c.set(c.get().saturating_add(1))); let mut decoder = ImageReader::with_format(Cursor::new(bytes), ImageFormat::Jpeg).into_decoder().ok()?; let orientation = decoder.orientation().ok()?; let (w, h) = decoder.dimensions(); @@ -53,6 +61,41 @@ fn turned(bytes: &[u8]) -> Option> { Some(out) } +/// Most Extended XMP chunks a JPEG handed to the decoder may carry (64 MB of metadata). The +/// decoder re-sorts and rescans every chunk so far after each marker, quadratic in their number. +const MAX_EXTENDED_XMP_CHUNKS: usize = 1024; + +/// How many Extended XMP chunks (APP1 segments) come before the image data. +fn extended_xmp_chunks(bytes: &[u8]) -> usize { + const PREFIX: &[u8] = b"http://ns.adobe.com/xmp/extension/\0"; + let (mut i, mut chunks) = (2usize, 0usize); + while let Some(&byte) = bytes.get(i) { + // Stray bytes between segments, which the decoder skips too. + if byte != 0xFF { + i += 1; + continue; + } + let Some(&marker) = bytes.get(i.saturating_add(1)) else { break }; + match marker { + // Fill bytes before a marker. + 0xFF => i += 1, + // Markers without a segment. + 0x00 | 0x01 | 0xD0..=0xD7 => i += 2, + // The image data starts (or ends): no headers after it. + 0xDA | 0xD9 => break, + _ => { + let Some(&[hi, lo]) = bytes.get(i.saturating_add(2)..i.saturating_add(4)) else { break }; + let len = usize::from(u16::from_be_bytes([hi, lo])); + if marker == 0xE1 && bytes.get(i.saturating_add(4)..).is_some_and(|seg| seg.starts_with(PREFIX)) { + chunks += 1; + } + i = i.saturating_add(2).saturating_add(len.max(2)); + } + } + } + chunks +} + /// The resolution in a JFIF header, if the file has one in inches or centimetres. fn jfif_density(bytes: &[u8]) -> Option { let mut i = 2usize; @@ -94,4 +137,26 @@ mod tests { assert!(matches!(upright(b"\x89PNG\r\n\x1a\n"), Cow::Borrowed(_))); assert!(matches!(upright(b"\xff\xd8 not a jpeg"), Cow::Borrowed(_))); } + + #[test] + fn a_jpeg_with_thousands_of_extended_xmp_chunks_is_not_decoded() { + // Empty chunks of one never-complete extended packet: the decoder re-sorts and rescans every + // chunk so far after each marker, quadratic in their number. + let chunk = |n: usize| { + let mut seg = b"http://ns.adobe.com/xmp/extension/\0".to_vec(); + seg.extend_from_slice(&[b'0'; 32]); // the packet's GUID + seg.extend_from_slice(&1000u32.to_be_bytes()); // its full length + seg.extend_from_slice(&0u32.to_be_bytes()); // this chunk's offset + let mut out = vec![0xFF, 0xE1]; + out.extend_from_slice(&u16::try_from(seg.len() + 2).unwrap().to_be_bytes()); + out.extend_from_slice(&seg); + out.repeat(n) + }; + let photo = phone_jpeg(6, 144); + let with = |n: usize| [&photo[..2], b"\0".as_slice(), &chunk(n), &photo[2..]].concat(); + DECODES.with(|c| c.set(0)); + assert!(matches!(upright(&with(4)), Cow::Owned(_)), "a few chunks are an ordinary photo"); + assert!(matches!(upright(&with(5_000)), Cow::Borrowed(_))); + assert_eq!(DECODES.with(std::cell::Cell::get), 1, "only the ordinary photo reached the decoder"); + } } diff --git a/crates/edit/src/added.rs b/crates/edit/src/added.rs index d246df725..d39bb0fa9 100644 --- a/crates/edit/src/added.rs +++ b/crates/edit/src/added.rs @@ -235,9 +235,17 @@ pub fn lines(t: &AddedText) -> Vec { wrapped(t).into_iter().map(|(line, _)| line).collect() } +#[cfg(test)] +thread_local! { + /// How often [`wrapped`] laid a text out on this thread, for the test that keeps it out of per-line loops. + pub(crate) static WRAP_CALLS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// [`lines`], each with whether its paragraph runs right to left (its first strong character /// does). fn wrapped(t: &AddedText) -> Vec<(String, bool)> { + #[cfg(test)] + WRAP_CALLS.with(|c| c.set(c.get().saturating_add(1))); let width = (t.rect[2] - t.rect[0]).max(t.size); let mut out = Vec::new(); for para in t.text.split('\n') { @@ -780,7 +788,8 @@ fn draw(doc: &mut Document, c: &Content, view: [f64; 6], taken: &Dict, existing: let r = text_rect(t); let [cr, cg, cb] = t.color.map(|v| v.clamp(0.0, 1.0)); out.extend(format!("BT /{name} {} Tf {} {} {} rg\n", n(t.size), n(cr), n(cg), n(cb)).bytes()); - for (i, line) in lines(t).iter().enumerate() { + let all = lines(t); + for (i, line) in all.iter().enumerate() { let w = t.family.width(line, t.size, t.bold, t.italic); let x = match t.align { Align::Left | Align::Justify => r[0], @@ -790,7 +799,6 @@ fn draw(doc: &mut Document, c: &Content, view: [f64; 6], taken: &Dict, existing: // Baseline: 0.8 em below the line top. let y = r[3] - (i as f64 * 1.2 + 0.95) * t.size; // Justified: word spacing makes every line but the last fill the box. - let all = lines(t); let spaces = line.matches(' ').count(); let tw = if t.align == Align::Justify && i + 1 < all.len() && spaces > 0 { ((r[2] - r[0]) - w).max(0.0) / spaces as f64 } else { 0.0 }; @@ -1042,6 +1050,9 @@ fn write(doc: &mut Document, page: usize, c: &Content, obj: Option) -> R } let taken = pres.get(b"Font").map(|f| doc.resolve(f)).and_then(|f| f.as_dict().cloned()).unwrap_or_default(); let (content, res) = draw(doc, c, view, &taken, obj.is_some())?; + if obj.is_none() { + crate::check_on_top(doc, p)?; + } for (k, v) in res.iter() { let mut sub = pres.get(k).map(|s| doc.resolve(s)).and_then(|s| s.as_dict().cloned()).unwrap_or_default(); for (n2, o) in v.as_dict().into_iter().flat_map(|d| d.iter()) { diff --git a/crates/edit/src/flatten.rs b/crates/edit/src/flatten.rs index 3ac546805..8aa1b0e32 100644 --- a/crates/edit/src/flatten.rs +++ b/crates/edit/src/flatten.rs @@ -9,7 +9,7 @@ //! form's field tree. An annotation whose appearance cannot be drawn is not flattened: it stays //! as it is, so no comment or field value is lost with nothing in its place. -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use pdfcraft_cos::{Dict, Document, ObjRef, Object}; @@ -129,7 +129,16 @@ fn undrawn_on(doc: &Document, all: &[pdfcraft_model::Page], pages: &[usize], whi None } +/// Runs [`flatten_in`] on a copy of `doc` (unchanged objects are shared) and keeps it only on +/// success, so a page refused part way leaves the pages before it as they were. fn flatten_which(doc: &mut Document, pages: &[usize], which: Which) -> Result { + let mut work = doc.clone(); + let drawn = flatten_in(&mut work, pages, which)?; + *doc = work; + Ok(drawn) +} + +fn flatten_in(doc: &mut Document, pages: &[usize], which: Which) -> Result { let all = page_list(doc); check(pages, all.len())?; // `?` in an appearance can still be fixed (a field's value, a comment's text are stored as @@ -295,6 +304,12 @@ fn flatten_which(doc: &mut Document, pages: &[usize], which: Which) -> Result = const { std::cell::Cell::new(0) }; +} + /// Remove flattened widgets from the AcroForm field tree, and fields left without widgets. With /// `on_page`, also the widgets (fields without kids) that are on no page. fn prune_fields(doc: &mut Document, widgets: &HashSet, on_page: Option<&HashSet>) -> Result<(), EditError> { @@ -303,23 +318,59 @@ fn prune_fields(doc: &mut Document, widgets: &HashSet, on_page: Option<& let af_ref = af.as_ref(); let mut af_dict = doc.resolve(&af).as_dict().cloned().unwrap_or_default(); let fields = af_dict.get(b"Fields").map(|f| doc.resolve(f)).and_then(|f| f.as_array().cloned()).unwrap_or_default(); - // Returns whether the node survives. - fn keep(doc: &mut Document, r: ObjRef, widgets: &HashSet, on_page: Option<&HashSet>, depth: usize) -> bool { - if widgets.contains(&r) || depth > 64 { - return depth > 64; + // Returns whether the node survives, and whether that answer depends on the depth cutoff. + // `known` holds each node's answer once it is visited, so a node listed twice is pruned once, + // and one still being visited (a cycle) survives. An answer the cutoff decided records the depth + // it was reached at, and a shallower path visits the node again: at most once per depth. + fn keep( + doc: &mut Document, + r: ObjRef, + widgets: &HashSet, + on_page: Option<&HashSet>, + known: &mut HashMap)>, + depth: usize, + ) -> (bool, bool) { + #[cfg(test)] + KEEP_CALLS.with(|c| c.set(c.get().saturating_add(1))); + if widgets.contains(&r) { + return (false, false); } - let Some(d) = doc.get(r).as_dict().cloned() else { return true }; + if depth > 64 { + return (true, true); + } + match known.get(&r) { + Some(&(survives, None)) => return (survives, false), + Some(&(survives, Some(at))) if depth >= at => return (survives, true), + _ => {} + } + known.insert(r, (true, None)); + let Some(d) = doc.get(r).as_dict().cloned() else { return (true, false) }; let Some(kids) = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()) else { - return on_page.is_none_or(|p| p.contains(&r)); + let survives = on_page.is_none_or(|p| p.contains(&r)); + known.insert(r, (survives, None)); + return (survives, false); }; - let survivors: Vec = - kids.iter().filter(|k| k.as_ref().is_none_or(|kr| keep(doc, kr, widgets, on_page, depth + 1))).cloned().collect(); + let mut cut = false; + let survivors: Vec = kids + .iter() + .filter(|k| { + k.as_ref().is_none_or(|kr| { + let (survives, kid_cut) = keep(doc, kr, widgets, on_page, known, depth + 1); + cut |= kid_cut; + survives + }) + }) + .cloned() + .collect(); if survivors.len() != kids.len() { let _ = doc.update_dict(r, |d| d.set(b"Kids".to_vec(), Object::Array(survivors.clone()))); } - !survivors.is_empty() + known.insert(r, (!survivors.is_empty(), cut.then_some(depth))); + (!survivors.is_empty(), cut) } - let survivors: Vec = fields.iter().filter(|f| f.as_ref().is_none_or(|r| keep(doc, r, widgets, on_page, 0))).cloned().collect(); + let mut known = HashMap::new(); + let survivors: Vec = + fields.iter().filter(|f| f.as_ref().is_none_or(|r| keep(doc, r, widgets, on_page, &mut known, 0).0)).cloned().collect(); if survivors.len() == fields.len() && widgets.is_empty() { return Ok(()); // only looked for fields on no page, and there were none } diff --git a/crates/edit/src/lib.rs b/crates/edit/src/lib.rs index bef515f98..630f12550 100644 --- a/crates/edit/src/lib.rs +++ b/crates/edit/src/lib.rs @@ -492,6 +492,9 @@ enum Isolation { /// the count of `q` left open at the end, when known. `marked` counts known unfinished /// marked-content sequences independently of graphics malformation, or zero when unknown. Unknown { unclosed: usize, marked: usize }, + /// Marked content nests past [`ISOLATION_MAX_DEPTH`]: how many sequences a mark on top would + /// have to close (some may be hidden optional content) is unknown. + TooDeep, } const ISOLATION_MAX_STREAMS: usize = 100_000; @@ -591,7 +594,10 @@ fn isolation(doc: &Document, list: &[Object]) -> Isolation { _ => {} } } - if malformed || text_open || path_open || !stack.is_empty() || !marked_known { + if !marked_known { + return Isolation::TooDeep; + } + if malformed || text_open || path_open || !stack.is_empty() { return Isolation::Unknown { unclosed: if malformed { 0 } else { stack.len() }, marked }; } let comment_open = edges.last().is_some_and(|edge| edge.1); @@ -608,8 +614,8 @@ fn place(doc: &mut Document, page: &pdfcraft_model::Page, kind: MarkKind, conten /// content before leaves set (a `cm`, a clip, text state…) must not reach the mark, including /// content appended after an earlier PdfCraft wrapper by another tool (#3084): the stretch of /// content that leaves state set is wrapped in q/Q, once. Any known unfinished marked-content -/// sequences are closed at the end too, so hidden optional content cannot hide the new mark. -/// Returns the new content stream. +/// sequences are closed at the end too, so hidden optional content cannot hide the new mark; when +/// their number is unknown, nothing is placed on top. Returns the new content stream. fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: &str, content: Vec, behind: bool) -> Result { let mut list = contents(doc, page)?; // The behind-marks at the start: backgrounds in the order added, then watermarks. @@ -624,6 +630,8 @@ fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: &str, cont // behind-marks at the start. A `q` it leaves open is closed at its end, where // closing it changes nothing already drawn. Isolation::Unknown { unclosed, marked } => (behind_marks < list.len()).then_some((behind_marks, unclosed, marked)), + // Content left inside an optional-content sequence it cannot close could be hidden. + Isolation::TooDeep => return Err(too_deep_on_top()), } }; let mark_ref = doc.add(Object::Stream(Stream::flate(tagged(tag), &content))); @@ -651,6 +659,26 @@ fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: &str, cont Ok(mark_ref) } +fn too_deep_on_top() -> EditError { + EditError::Invalid("the page's marked content is nested too deeply to add anything on top of it".into()) +} + +/// The refusal [`place_tagged`] gives for a mark on top of `page`, checked before an edit changes +/// anything (removing old marks, adding resources) so a refused edit leaves the document as it was. +fn check_on_top(doc: &Document, page: &pdfcraft_model::Page) -> Result<(), EditError> { + let obj = doc.get(page.obj); + let list = match obj.as_dict().and_then(|d| d.get(b"Contents")) { + None => return Ok(()), + Some(Object::Ref(r)) => doc.get(*r).as_array().cloned().unwrap_or_else(|| vec![Object::Ref(*r)]), + Some(Object::Array(a)) => a.clone(), + Some(other) => vec![other.clone()], + }; + match isolation(doc, &list) { + Isolation::TooDeep => Err(too_deep_on_top()), + _ => Ok(()), + } +} + /// Put content tagged `tag` first inside the q/Q wrapper, so it applies to the page's original /// content and nothing else (a header's shrink). A page without a wrapper has no original content /// on it and is left alone. @@ -670,6 +698,7 @@ fn place_inside_wrapper(doc: &mut Document, page: &pdfcraft_model::Page, tag: &s pub fn stamp(doc: &mut Document, page: usize, tag: &str, content: Vec) -> Result { let all = page_list(doc); check(&[page], all.len())?; + check_on_top(doc, &all[page])?; add_resources(doc, &all[page], None, Some(&content))?; let p = page_list(doc).swap_remove(page); place_tagged(doc, &p, tag, content, false) @@ -749,6 +778,9 @@ pub fn add_header_footer(doc: &mut Document, pages: &[usize], hf: &HeaderFooter, .iter() .map(|&i| hf.shrink.then(|| shrink_matrix(hf, all[i].display_size(doc), all[i].view_matrix(doc))).transpose()) .collect::, _>>()?; + for &i in pages { + check_on_top(doc, &all[i])?; + } if replace { remove_marks(doc, pages, MarkKind::HeaderFooter)?; } @@ -837,6 +869,11 @@ pub fn add_watermark(doc: &mut Document, pages: &[usize], wm: &Watermark, replac { return Err(EditError::Invalid("invalid watermark settings".into())); } + if !wm.behind { + for &i in pages { + check_on_top(doc, &all[i])?; + } + } if replace { remove_marks(doc, pages, MarkKind::Watermark)?; } @@ -941,7 +978,7 @@ pub fn remove_marks(doc: &mut Document, pages: &[usize], kind: MarkKind) -> Resu if before != kept.len() { let wrappers = match isolation(doc, &kept) { Isolation::Clean { wrappers, .. } => wrappers, - Isolation::Unknown { .. } => { + Isolation::Unknown { .. } | Isolation::TooDeep => { let mut opens = Vec::new(); let mut pairs = Vec::new(); for (i, o) in kept.iter().enumerate() { @@ -997,7 +1034,7 @@ mod linebreak; pub mod text; pub use text::{ BlockKey, BlockStyle, LineEdit, MAX_FORM_DEPTH, MAX_FORM_VISITS, RangeStyle, ReadingBlocks, TextBlock, TextLine, TextSession, block_overlapping, - clear_block, reading_blocks, reading_blocks_checked, replace_block, replace_line, rewrite_block, text_blocks, text_lines, + clear_block, glyph_text_lines, reading_blocks, reading_blocks_checked, replace_block, replace_line, rewrite_block, text_blocks, text_lines, }; pub mod added; pub use added::{ diff --git a/crates/edit/src/marked_isolation_tests.rs b/crates/edit/src/marked_isolation_tests.rs index 6da832229..227371bca 100644 --- a/crates/edit/src/marked_isolation_tests.rs +++ b/crates/edit/src/marked_isolation_tests.rs @@ -96,7 +96,7 @@ fn recognizes_only_canonical_new_closes_and_retains_legacy_closes() { } #[test] -fn depth_budget_bounds_emitted_closures_and_overflow_stays_unknown() { +fn depth_budget_bounds_emitted_closures_and_overflow_is_refused_on_top() { let maximum = b"/Artifact BMC\n".repeat(ISOLATION_MAX_DEPTH); let mut expected = b"\nEMC".repeat(ISOLATION_MAX_DEPTH); expected.extend_from_slice(WRAP_CLOSE); @@ -114,9 +114,46 @@ fn depth_budget_bounds_emitted_closures_and_overflow_stays_unknown() { place_tagged(&mut doc, &page, MarkKind::HeaderFooter.tag(), b"q Q\n".to_vec(), false).unwrap(); assert_eq!(remove_marks(&mut doc, &[0], MarkKind::HeaderFooter).unwrap(), 1); assert_eq!(contents(&mut doc, &page).unwrap(), original, "maximum combined close can be removed without nesting wrappers"); + // Past the budget the depth is unknown, so nothing can close the page's (possibly hidden) + // optional content before a mark on top: placing one there is refused, and the page is left + // as it was. Behind the content stays possible. for suffix in [b"/Artifact BMC\n".as_slice(), b"/Artifact BMC\nEMC\n", b"/Artifact BMC\nEMC\nEMC\n"] { let mut over = maximum.clone(); over.extend_from_slice(suffix); - assert_eq!(close(&[&over]), WRAP_CLOSE, "overflow cannot resume speculative depth tracking after EMC"); + let mut doc = fixture(&[&over]); + let page = page_list(&doc).remove(0); + let original = contents(&mut doc, &page).unwrap(); + assert!( + matches!(stamp(&mut doc, 0, "Probe", b"q Q\n".to_vec()), Err(EditError::Invalid(_))), + "overflow cannot resume speculative depth tracking after EMC" + ); + assert_eq!(contents(&mut doc, &page).unwrap(), original); + place_tagged(&mut doc, &page, MarkKind::Background.tag(), b"q Q\n".to_vec(), true).unwrap(); } } + +/// Marks refused on top of content nested past the depth budget leave the page as it was: old +/// marks are not removed and no resources are added before the refusal. +#[test] +fn a_mark_refused_on_top_changes_nothing() { + let mut over = b"/Artifact BMC\n".repeat(ISOLATION_MAX_DEPTH); + over.extend_from_slice(b"/Artifact BMC\n"); + let mut doc = fixture(&[&over]); + let behind = Watermark { text: "OLD".into(), behind: true, ..Watermark::default() }; + add_watermark(&mut doc, &[0], &behind, false).unwrap(); + let page = page_list(&doc).remove(0).obj; + let before = (*doc.get(page)).clone(); + let on_top = Watermark { text: "NEW".into(), ..Watermark::default() }; + assert!(matches!(add_watermark(&mut doc, &[0], &on_top, true), Err(EditError::Invalid(_)))); + assert_eq!((*doc.get(page)).clone(), before, "the old watermark stays"); + let hf = HeaderFooter { + text: [String::new(), "Top".into(), String::new(), String::new(), String::new(), String::new()], + font: added::Family::Times, + ..HeaderFooter::default() + }; + assert!(matches!(add_header_footer(&mut doc, &[0], &hf, true, &Context::default()), Err(EditError::Invalid(_)))); + assert_eq!((*doc.get(page)).clone(), before, "no fonts are added"); + let mut stamped = doc.clone(); + assert!(matches!(stamp(&mut stamped, 0, "Probe", b"/PCTimes 9 Tf\n".to_vec()), Err(EditError::Invalid(_)))); + assert_eq!((*stamped.get(page)).clone(), before, "no fonts are added"); +} diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 1395fb1c3..b1c463a6a 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -6356,6 +6356,107 @@ fn reading_keeps_dotted_leader_entries_whole_and_in_order() { ); } +#[test] +fn added_text_is_wrapped_once_per_write() { + let mut doc = fixture(); + let lines: Vec = (0..40).map(|i| format!("line {i}")).collect(); + let text = AddedText { rect: [72.0, 100.0, 300.0, 700.0], text: lines.join("\n"), size: 10.0, align: Align::Justify, ..AddedText::default() }; + added::WRAP_CALLS.with(|c| c.set(0)); + add_content(&mut doc, 0, &Content::Text(text)).unwrap(); + let calls = added::WRAP_CALLS.with(std::cell::Cell::get); + assert!(calls <= 4, "the text was wrapped {calls} times for 40 lines"); +} + +/// A field tree whose every node lists its child twice is pruned in work linear in its nodes, not +/// in its paths, and a node listing itself is visited once. +#[test] +fn flattening_prunes_a_field_tree_with_repeated_kids_once_per_node() { + let levels = 16; + let page = stream("", " "); + // 5: the flattened widget; 6…: the chain; then the hidden widget at its end, on the page. + let leaf = 6 + levels; + let fields = format!("<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [5 0 R 6 0 R {} 0 R] >> >>", leaf + 2); + let annots = format!("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << >> /Contents 4 0 R /Annots [5 0 R {leaf} 0 R] >>"); + let mut objs: Vec = vec![ + fields, + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into(), + annots, + page, + format!("<< /Type /Annot /Subtype /Widget /FT /Tx /T (shown) /Rect [10 10 150 40] /F 4 /AP << /N {} 0 R >> >>", leaf + 1), + ]; + for i in 0..levels { + let next = 7 + i; + objs.push(format!("<< /T (n{i}) /Kids [{next} 0 R {next} 0 R] >>")); + } + objs.push("<< /Type /Annot /Subtype /Widget /FT /Tx /T (hidden) /Rect [10 90 150 120] /F 2 >>".into()); + objs.push("<< /Type /XObject /Subtype /Form /BBox [0 0 140 30] /Length 0 >>\nstream\n\nendstream".into()); + objs.push(format!("<< /T (loop) /Kids [{} 0 R] >>", leaf + 2)); + let objs: Vec<&str> = objs.iter().map(String::as_str).collect(); + let mut doc = build(&objs); + flatten::KEEP_CALLS.with(|c| c.set(0)); + assert_eq!(flatten(&mut doc, &[0], false, true).unwrap(), 1); + let calls = flatten::KEEP_CALLS.with(std::cell::Cell::get); + assert!(calls <= 2 * (levels + 4), "{calls} field-tree visits"); + let root = doc.root().unwrap(); + let af = doc.resolve(doc.get(root).as_dict().unwrap().get(b"AcroForm").unwrap()); + let left = doc.resolve(af.as_dict().unwrap().get(b"Fields").unwrap()).as_array().unwrap().len(); + assert_eq!(left, 2, "the chain to the hidden widget and the self-listing node stay"); +} + +/// A field reached first at the end of a long chain has its kids cut off by the depth limit, so +/// they survive there. A shorter path to the same field still looks at its kids: an off-page widget +/// under it is taken out when every field is flattened. +#[test] +fn a_field_reached_again_by_a_shorter_path_is_pruned_there() { + let chain = 64; + let page = stream("", " "); + // 7…: the chain; then the shared field, its off-page widget, and the short path to it. + let shared = 7 + chain; + let fields = format!("<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [5 0 R 7 0 R {} 0 R] >> >>", shared + 2); + let mut objs: Vec = vec![ + fields, + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into(), + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << >> /Contents 4 0 R /Annots [5 0 R] >>".into(), + page, + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (shown) /Rect [10 10 150 40] /F 4 /AP << /N 6 0 R >> >>".into(), + "<< /Type /XObject /Subtype /Form /BBox [0 0 140 30] /Length 0 >>\nstream\n\nendstream".into(), + ]; + for i in 0..chain { + objs.push(format!("<< /T (c{i}) /Kids [{} 0 R] >>", 8 + i)); + } + objs.push(format!("<< /T (shared) /Kids [{} 0 R] >>", shared + 1)); + objs.push("<< /Type /Annot /Subtype /Widget /FT /Tx /T (orphan) /Rect [10 50 150 80] /F 4 >>".into()); + objs.push(format!("<< /T (short) /Kids [{shared} 0 R] >>")); + let objs: Vec<&str> = objs.iter().map(String::as_str).collect(); + let mut doc = build(&objs); + assert_eq!(flatten(&mut doc, &[0], false, true).unwrap(), 1); + let shared_ref = pdfcraft_cos::ObjRef::new(u32::try_from(shared).unwrap(), 0); + let kids = doc.resolve(doc.get(shared_ref).as_dict().unwrap().get(b"Kids").unwrap()).as_array().unwrap().len(); + assert_eq!(kids, 0, "the off-page widget is pruned along the short path"); +} + +/// A ToUnicode destination is repeated for every glyph that shows its code: a page whose text +/// would run past the extraction budget is refused, not built in memory. +#[test] +fn page_text_past_the_budget_is_refused() { + let cmap = stream("", &format!("1 beginbfchar <41> <{}> endbfchar", "0041".repeat(256))); + let page = |shown: &str| { + let content = stream("", &format!("BT /F1 12 Tf 10 10 Td ({shown}) Tj ET")); + build(&[ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << /Font << /F1 5 0 R >> >> /Contents 4 0 R >>", + &content, + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /ToUnicode 6 0 R >>", + &cmap, + ]) + }; + let doc = page(&"A".repeat(100_000)); + assert!(matches!(text_lines(&doc, 0), Err(EditError::Invalid(_))), "25 MB of text from a 100 kB page"); + let doc = page("AA"); + assert_eq!(text_lines(&doc, 0).unwrap()[0].text.len(), 512, "a page within the budget still reads"); +} + /// A one-page document from the page's /Contents and /Resources, built in code. fn page_with(build: impl FnOnce(&mut Document) -> (Object, Dict)) -> Document { let mut doc = Document::new_empty(); @@ -6448,3 +6549,25 @@ fn reading_spends_one_decode_budget_on_content_and_forms() { }); assert!(crate::text::reading_blocks_checked(&doc, 0).unwrap().truncated); } + +/// A page that can't take flattened content on top (marked content nested past the depth budget) +/// refuses the whole flatten: the pages before it keep their comments and content. +#[test] +fn a_flatten_refused_on_a_later_page_changes_nothing() { + let deep = stream("", &"/Artifact BMC\n".repeat(10_001)); + let mut doc = build(&[ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R 6 0 R] /Count 2 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << >> /Contents 4 0 R /Annots [5 0 R] >>", + &stream("", " "), + "<< /Type /Annot /Subtype /Square /Rect [10 10 50 50] /F 4 /AP << /N 9 0 R >> >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Resources << >> /Contents 7 0 R /Annots [8 0 R] >>", + &deep, + "<< /Type /Annot /Subtype /Square /Rect [10 10 50 50] /F 4 /AP << /N 9 0 R >> >>", + "<< /Type /XObject /Subtype /Form /BBox [0 0 40 40] /Length 0 >>\nstream\n\nendstream", + ]); + let first = pdfcraft_cos::ObjRef::new(3, 0); + let before = (*doc.get(first)).clone(); + assert!(matches!(flatten(&mut doc, &[0, 1], true, false), Err(EditError::Invalid(_)))); + assert_eq!((*doc.get(first)).clone(), before, "the first page keeps its comment and content"); +} diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index e15a2491a..29ac90abc 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -903,8 +903,15 @@ struct Carry { marked: Vec>, /// For each open marked-content sequence, its optional content group when it is `/OC`. layers: Vec>>, + /// Bytes of glyph text the rest of the page may still produce (see [`MAX_PAGE_TEXT`]). + text_left: usize, } +/// The most glyph text one page's extraction produces, in bytes. A ToUnicode destination is +/// copied for every glyph showing its code, so a small page can otherwise expand into gigabytes; +/// real pages hold far less (a whole novel is about 1 MB). +const MAX_PAGE_TEXT: usize = 16 << 20; + impl Carry { fn new() -> Self { let ts = Ts { @@ -923,7 +930,7 @@ impl Carry { vertical: None, unreadable: None, }; - Carry { ts, stack: Vec::new(), marked: Vec::new(), layers: Vec::new() } + Carry { ts, stack: Vec::new(), marked: Vec::new(), layers: Vec::new(), text_left: MAX_PAGE_TEXT } } } @@ -1135,7 +1142,13 @@ fn interpret( let blank = m.is_space(code, len) || shown.as_deref().is_some_and(|t| !t.is_empty() && t.chars().all(char::is_whitespace)); match shown { - Some(t) => text.push_str(&t), + Some(t) => { + carry.text_left = carry + .text_left + .checked_sub(t.len()) + .ok_or_else(|| EditError::Invalid("the page's text is too large to read".into()))?; + text.push_str(&t); + } None => decodable = false, } let spacing = ts.char_spacing + if m.is_space(code, len) { ts.word_spacing } else { 0.0 }; @@ -1293,7 +1306,19 @@ fn interpret( /// The lines of text on a page (0-based), in content order. pub fn text_lines(doc: &Document, page: usize) -> Result, EditError> { - let p = page_dict(doc, page)?; + text_lines_in(doc, &page_dict(doc, page)?, true) +} + +/// The lines `page`'s glyphs show, in content order, without `/ActualText` replacements: what the +/// page draws, not what its marked-content metadata says the glyphs mean. For checks that must +/// not trust that metadata (the redaction audit). Takes the page itself, so a caller going +/// through every page doesn't walk the page tree again for each. +pub fn glyph_text_lines(doc: &Document, page: &pdfcraft_model::Page) -> Result, EditError> { + text_lines_in(doc, page, false) +} + +/// [`text_lines`] of `p`; with `actual_text`, `/ActualText` replaces the glyphs it spans. +fn text_lines_in(doc: &Document, p: &pdfcraft_model::Page, actual_text: bool) -> Result, EditError> { let res = p.dict.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned()).unwrap_or_default(); let mut cache = HashMap::new(); let mut lines: Vec = Vec::new(); @@ -1303,7 +1328,7 @@ pub fn text_lines(doc: &Document, page: usize) -> Result, EditErro let pieces = Pieces::join(&content_streams(doc, &p.dict)?.into_iter().map(|(_, d)| d).collect::>()); let ops = pieces.parse().ops; let properties = res.get(b"Properties").and_then(|o| doc.dict(o)).unwrap_or_default(); - let shown = interpret(doc, &ops, &res, &mut cache, &mut carry, None, Some(&properties))?; + let shown = interpret(doc, &ops, &res, &mut cache, &mut carry, None, actual_text.then_some(&properties))?; let mut last: Option<(usize, Pen)> = None; // the stream piece and where its last run ended for mut s in shown { // A single RTL glyph's ActualText (e.g. lam-alef) is an inline ligature, not a @@ -1605,7 +1630,7 @@ fn reading_lines(doc: &Document, page: usize) -> Result<(Vec, bool), E let mut forms = invoked.into_iter().peekable(); for s in shown { while let Some((_, name, ts)) = forms.next_if(|f| f.0 < s.op) { - enter(w, resources, &name, ts)?; + enter(w, resources, &name, ts, &mut carry.text_left)?; // Text after a form starts a new line. last = None; } @@ -1617,21 +1642,23 @@ fn reading_lines(doc: &Document, page: usize) -> Result<(Vec, bool), E add_shown(&mut w.lines, &mut last, s, si); } for (_, name, ts) in forms { - enter(w, resources, &name, ts)?; + enter(w, resources, &name, ts, &mut carry.text_left)?; } Ok(()) } - fn enter(w: &mut Walk, resources: &Dict, name: &[u8], ts: Ts) -> Result<(), EditError> { + /// `text_left`: the page's text budget, shared with the forms the page draws. + fn enter(w: &mut Walk, resources: &Dict, name: &[u8], ts: Ts, text_left: &mut usize) -> Result<(), EditError> { let Some(form) = form_call(w.doc, resources, name, ts.ctm, &w.path, &mut w.budget) else { return Ok(()) }; let ops = parse(&form.data).ops; // The form starts with the graphics state at its `Do`, its own matrix applied. - let mut carry = Carry { ts: Ts { ctm: form.ctm, ..ts }, stack: Vec::new(), marked: Vec::new(), layers: Vec::new() }; + let mut carry = Carry { ts: Ts { ctm: form.ctm, ..ts }, stack: Vec::new(), marked: Vec::new(), layers: Vec::new(), text_left: *text_left }; let stream = w.next_stream; w.next_stream = w.next_stream.saturating_add(1); if let Some(r) = form.obj { w.path.push(r); } walk(w, &ops, &form.resources, &mut carry, &|_| stream)?; + *text_left = carry.text_left; if form.obj.is_some() { w.path.pop(); } diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 7bd87a2ef..82c1c8e74 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -590,7 +590,7 @@ fn enumerate(doc: &Document) -> (Vec, usize) { None => Inherited::default(), }; let mut out = Vec::new(); - let mut seen = Seen { fields: std::collections::HashSet::new(), scripts: script_budget() }; + let mut seen = Seen { fields: std::collections::HashSet::new(), widgets: std::collections::HashSet::new(), scripts: script_budget() }; if let Some(af) = &af { for f in af.get(b"Fields").map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned()).unwrap_or_default() { if let Some(r) = f.as_ref() { @@ -623,7 +623,7 @@ fn adopt_page_fields( let Some(annots) = annots.as_array() else { continue }; for e in annots { let Some(r) = e.as_ref() else { continue }; - if seen.fields.contains(&r) { + if seen.fields.contains(&r) || seen.widgets.contains(&r) { continue; } let obj = doc.get(r); @@ -764,15 +764,17 @@ pub fn set_tab_order(doc: &mut Document, pages: &[usize], order: TabOrder) -> Re Ok(()) } -/// Read the field at `r` and the fields under it into `out`, with what they inherit from -/// `parent`: kids with a name are fields of their own, kids without are this field's widgets. -/// What one reading of the form's fields has used: the field dictionaries read, and the script -/// budget their actions share. +/// What one reading of the form's fields has used: the field and widget dictionaries read, and +/// the script budget their actions share. struct Seen { fields: std::collections::HashSet, + /// A widget belongs to the first field that lists it, however often it is listed. + widgets: std::collections::HashSet, scripts: DecodeBudget, } +/// Read the field at `r` and the fields under it into `out`, with what they inherit from +/// `parent`: kids with a name are fields of their own, kids without are this field's widgets. fn walk( doc: &Document, r: ObjRef, @@ -824,11 +826,8 @@ fn walk( } return; } - let widget_refs: Vec = if d.contains(b"Rect") { vec![r] } else { kids }; - let actions = actions_of(doc, d, &widget_refs, &mut seen.scripts); - let button = (inh.ft.as_deref() == Some(b"Btn") && inh.ff.unwrap_or(0) & flags::PUSH_BUTTON != 0) - .then(|| button_action(doc, d, &widget_refs, &mut seen.scripts)) - .flatten(); + // A field of no kind this reads is skipped before it claims widgets or scripts, which a later + // field listing them then reads. let ff = inh.ff.unwrap_or(0); let kind = match inh.ft.as_deref() { Some(b"Tx") => FieldKind::Text, @@ -840,6 +839,12 @@ fn walk( Some(b"Sig") => FieldKind::Signature, _ => return, }; + let widget_refs: Vec = if d.contains(b"Rect") { vec![r] } else { kids }; + let widget_refs: Vec = widget_refs.into_iter().filter(|w| seen.widgets.insert(*w)).collect(); + let actions = actions_of(doc, d, &widget_refs, &mut seen.scripts); + let button = (inh.ft.as_deref() == Some(b"Btn") && inh.ff.unwrap_or(0) & flags::PUSH_BUTTON != 0) + .then(|| button_action(doc, d, &widget_refs, &mut seen.scripts)) + .flatten(); let widgets = widget_refs .into_iter() .filter_map(|w| { diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index b2b8573eb..4d5a14f57 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -3021,3 +3021,38 @@ fn repeated_script_references_share_one_decode_budget() { let calculate: usize = all.iter().filter_map(|f| f.actions.scripts.calculate.as_ref()).map(String::len).sum(); assert!(calculate > 0 && calculate <= limit, "{calculate} bytes of field scripts"); } + +/// A widget listed many times, by one field's `/Kids` or by a `/Kids` array several fields share, +/// is read once: the first field to list it owns it. +#[test] +fn a_widget_listed_repeatedly_is_read_once() { + let kids = "7 0 R ".repeat(500); + let doc = document(&[ + "<< /Type /Catalog /Pages 4 0 R /AcroForm << /Fields [2 0 R 3 0 R] >> >>".into(), + "<< /FT /Tx /T (a) /Kids 6 0 R >>".into(), + "<< /FT /Tx /T (b) /Kids 6 0 R >>".into(), + "<< /Type /Pages /Kids [5 0 R] /Count 1 >>".into(), + "<< /Type /Page /Parent 4 0 R /MediaBox [0 0 100 100] /Annots [7 0 R] >>".into(), + format!("[{kids}]"), + "<< /Type /Annot /Subtype /Widget /Parent 2 0 R /Rect [0 0 10 10] >>".into(), + ]); + let all = fields(&doc); + let widgets: Vec = all.iter().map(|f| f.widgets.len()).collect(); + assert_eq!(widgets, [1, 0], "{:?}", all.iter().map(|f| &f.name).collect::>()); +} + +/// A field of a kind that isn't read doesn't claim the widget it shares with a field that is. +#[test] +fn a_skipped_field_leaves_its_widget_to_the_next() { + let doc = document(&[ + "<< /Type /Catalog /Pages 4 0 R /AcroForm << /Fields [2 0 R 3 0 R] >> >>".into(), + "<< /FT /Zz /T (odd) /Kids [6 0 R] >>".into(), + "<< /FT /Tx /T (b) /Kids [6 0 R] >>".into(), + "<< /Type /Pages /Kids [5 0 R] /Count 1 >>".into(), + "<< /Type /Page /Parent 4 0 R /MediaBox [0 0 100 100] /Annots [6 0 R] >>".into(), + "<< /Type /Annot /Subtype /Widget /Parent 3 0 R /Rect [0 0 10 10] >>".into(), + ]); + let all = fields(&doc); + let widgets: Vec<(&str, usize)> = all.iter().map(|f| (f.name.as_str(), f.widgets.len())).collect(); + assert_eq!(widgets, [("b", 1)]); +} diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index 024dfa5c3..1162a810c 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -154,8 +154,8 @@ impl<'a> Copier<'a> { } /// Rewrite `/P`, `/Dest` and GoTo actions on copied annotations. - fn fix_annotations(&self, dst: &mut Document, annot_pages: &HashMap) -> Result<(), OrganizeError> { - for &a in &self.annots { + fn fix_annotations(&mut self, dst: &mut Document, annot_pages: &HashMap) -> Result<(), OrganizeError> { + for &a in &std::mem::take(&mut self.annots) { let src_dict = dst.get(a).as_dict().cloned(); let Some(d) = src_dict else { continue }; let mut d = d; @@ -188,10 +188,13 @@ impl<'a> Copier<'a> { } } } else { - // Other actions (URI, Launch, JavaScript…) carry no page references. - let mut c = Copier::new(self.src, self.map.clone(), HashMap::new()); - let copied = c.copy_value(dst, &Object::Dict(ad.clone()), None)?; - d.set(b"A".to_vec(), copied); + // Other actions (URI, Launch, JavaScript…) carry no page references. The + // copier lends its map out and takes it back with the new copies in it, + // so actions sharing an object share its one copy. + let mut c = Copier::new(self.src, std::mem::take(&mut self.map), HashMap::new()); + let copied = c.copy_value(dst, &Object::Dict(ad.clone()), None); + self.map = c.map; + d.set(b"A".to_vec(), copied?); } } } @@ -541,13 +544,16 @@ fn named_dest(src: &Document, name: &[u8]) -> Option { } let names = src.resolve(catalog.get(b"Names")?); let tree = names.as_dict()?.get(b"Dests")?.clone(); - name_tree_lookup(src, &tree, name, 0) + name_tree_lookup(src, &tree, name, 0, &mut HashSet::new()) } -fn name_tree_lookup(src: &Document, node: &Object, key: &[u8], depth: u8) -> Option { - if depth > 32 { +/// Name trees are read once per node: a node listed twice (or a cycle) is skipped. +fn name_tree_lookup(src: &Document, node: &Object, key: &[u8], depth: u8, seen: &mut HashSet) -> Option { + if depth > 32 || node.as_ref().is_some_and(|r| !seen.insert(r)) { return None; } + #[cfg(test)] + crate::work(1); let node = src.resolve(node); let d = node.as_dict()?; if let Some(names) = d.get(b"Names").map(|n| src.resolve(n)).and_then(|n| n.as_array().cloned()) { @@ -560,14 +566,16 @@ fn name_tree_lookup(src: &Document, node: &Object, key: &[u8], depth: u8) -> Opt } } let kids = d.get(b"Kids").map(|k| src.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default(); - kids.iter().find_map(|k| name_tree_lookup(src, k, key, depth + 1)) + kids.iter().find_map(|k| name_tree_lookup(src, k, key, depth + 1, seen)) } -/// All (key, value) pairs of a name tree, in order. -fn name_tree_entries(src: &Document, node: &Object, depth: u8, out: &mut Vec<(Vec, Object)>) { - if depth > 32 { +/// All (key, value) pairs of a name tree, in order, each node read once. +fn name_tree_entries(src: &Document, node: &Object, depth: u8, seen: &mut HashSet, out: &mut Vec<(Vec, Object)>) { + if depth > 32 || node.as_ref().is_some_and(|r| !seen.insert(r)) { return; } + #[cfg(test)] + crate::work(1); let node = src.resolve(node); let Some(d) = node.as_dict() else { return }; if let Some(names) = d.get(b"Names").map(|n| src.resolve(n)).and_then(|n| n.as_array().cloned()) { @@ -580,7 +588,7 @@ fn name_tree_entries(src: &Document, node: &Object, depth: u8, out: &mut Vec<(Ve } } for k in d.get(b"Kids").map(|k| src.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default() { - name_tree_entries(src, &k, depth + 1, out); + name_tree_entries(src, &k, depth + 1, seen, out); } } @@ -680,24 +688,32 @@ pub fn combine_field_conflicts(sources: &[&Document]) -> Vec) -> Vec<(ObjRef, String, String, String)> { let mut planned = Vec::new(); - let mut seen = std::collections::HashSet::new(); + let mut walk = RenameWalk::default(); for &root in roots { - plan_renames_under(doc, root, "", taken, &mut seen, 0, &mut planned); + plan_renames_under(doc, root, "", taken, &mut walk, 0, &mut planned); } planned } +/// What one [`plan_renames`] call remembers: the nodes it visited, and for each colliding name +/// the suffix to try next. +#[derive(Default)] +struct RenameWalk { + seen: HashSet, + next: HashMap, +} + /// One node of [`plan_renames`]. fn plan_renames_under( doc: &Document, node: ObjRef, prefix: &str, taken: &mut std::collections::HashSet, - seen: &mut std::collections::HashSet, + walk: &mut RenameWalk, depth: u8, planned: &mut Vec<(ObjRef, String, String, String)>, ) { - if depth > 32 || !seen.insert(node) { + if depth > 32 || !walk.seen.insert(node) { return; } let Some(d) = doc.get(node).as_dict().cloned() else { return }; @@ -705,14 +721,19 @@ fn plan_renames_under( if !name.is_empty() { let fqn = qualified(prefix, &name); if taken.contains(&fqn) { - let mut n = 1u32; + // `taken` only grows, so every suffix below the last one handed out for this name is + // still taken: resume there instead of at 1. + let mut n = walk.next.get(&fqn).copied().unwrap_or(1); let mut variant; while { + #[cfg(test)] + crate::work(1); variant = format!("{name}-{n}"); taken.contains(&qualified(prefix, &variant)) } { - n += 1; + n = n.saturating_add(1); } + walk.next.insert(fqn.clone(), n.saturating_add(1)); let new_fqn = qualified(prefix, &variant); planned.push((node, variant.clone(), fqn, new_fqn.clone())); taken.insert(new_fqn); @@ -724,7 +745,7 @@ fn plan_renames_under( let prefix = qualified_optional(prefix, &name); let kids = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default(); for kid in kids.iter().filter_map(|k| k.as_ref()) { - plan_renames_under(doc, kid, &prefix, taken, seen, depth + 1, planned); + plan_renames_under(doc, kid, &prefix, taken, walk, depth + 1, planned); } } @@ -826,8 +847,11 @@ fn register_fields(dst: &mut Document, fields: &[ObjRef]) -> Result<(), Organize _ => (None, Dict::new()), }; let mut list = form.get(b"Fields").map(|f| dst.resolve(f)).and_then(|f| f.as_array().cloned()).unwrap_or_default(); + let mut listed: HashSet = list.iter().filter_map(Object::as_ref).collect(); for f in fields { - if !list.contains(&Object::Ref(*f)) { + #[cfg(test)] + crate::work(1); + if listed.insert(*f) { list.push(Object::Ref(*f)); } } @@ -1144,7 +1168,7 @@ fn collect_attachments(dst: &mut Document, src: &Document, out: &mut Vec<(Vec Result<(), if removed { Ok(()) } else { Err(OrganizeError::Invalid(format!("the document doesn't declare {}", standard.label()))) } } +#[cfg(test)] +thread_local! { + /// Steps taken on this thread by loops whose cost follows the input's shape, so a test can + /// bound the work a hostile document causes without timing it. + static WORK: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Count `n` steps of input-shaped work (see [`WORK`]). +#[cfg(test)] +fn work(n: usize) { + WORK.with(|w| w.set(w.get().saturating_add(n))); +} + +/// The steps counted on this thread since the last call. +#[cfg(test)] +fn take_work() -> usize { + WORK.with(|w| w.replace(0)) +} + #[cfg(test)] mod tests; diff --git a/crates/organize/src/prune.rs b/crates/organize/src/prune.rs index a42c34299..d8db2f7ac 100644 --- a/crates/organize/src/prune.rs +++ b/crates/organize/src/prune.rs @@ -29,7 +29,7 @@ enum XKey { } /// Who owns the resources currently being walked, for the key drawn names are recorded under. -#[derive(Clone, Copy)] +#[derive(Clone, Copy, PartialEq, Eq, Hash)] enum Owner { /// An indirect resources object (a page's, or a form XObject's own). Resources(ObjRef), @@ -50,6 +50,9 @@ struct Pruner<'a> { glyph_seen: HashSet, /// An indirect font's explicit resources have one owner regardless of which page uses it. glyph_fonts: HashSet, + /// Forms already analysed, with the owner of the resources they drew against: drawing one + /// again records nothing new, and re-reading it on every `Do` is exponential in the nesting. + forms_seen: HashSet<(ObjRef, Owner)>, } impl Pruner<'_> { @@ -211,6 +214,11 @@ impl Pruner<'_> { // A form without its own resources draws against the ones it was invoked with. None => (resources.clone(), owner), }; + if !self.forms_seen.insert((r, inner_owner)) { + continue; + } + #[cfg(test)] + crate::work(1); match s.decoded() { Ok(data) => self.content(&data, &inner, inner_owner, depth + 1), // What the form draws is unknown: keep everything it could reach. @@ -307,6 +315,7 @@ pub(crate) fn prune_unused_xobjects(doc: &mut Document) -> Result Document { + let n = annots.len(); + let refs: String = (0..n).map(|i| format!("{} 0 R ", i + 5)).collect(); + let mut b: Vec = vec![ + format!("<< /Type /Catalog /Pages 2 0 R {catalog_extra} >>"), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >>".into(), + format!("<< /Type /Page /Parent 2 0 R /Contents 4 0 R /Annots [{refs}] >>"), + body("P1"), + ]; + b.extend(annots.iter().cloned()); + b.extend(extra.iter().cloned()); + open(build(&b, "/Root 1 0 R")) +} + +#[test] +fn registering_many_imported_fields_is_linear() { + let n = 2_000; + let widgets: Vec = (0..n).map(|i| format!("<< /Type /Annot /Subtype /Widget /FT /Tx /T (f{i}) /Rect [0 0 10 10] >>")).collect(); + let src = page_with_annots(&widgets, &[], ""); + take_work(); + let out = extract_pages(&src, &[0]).unwrap(); + let work = take_work(); + assert!(work <= 4 * n, "{work} steps registering {n} fields"); + let form = out.resolve(catalog(&out).get(b"AcroForm").unwrap()).as_dict().cloned().unwrap(); + assert_eq!(form.get(b"Fields").and_then(|f| f.as_array()).map(Vec::len), Some(n)); +} + +#[test] +fn renaming_many_same_named_fields_resumes_the_suffix_search() { + let n = 2_000; + let widgets: Vec = (0..n).map(|_| "<< /Type /Annot /Subtype /Widget /FT /Tx /T (x) /Rect [0 0 10 10] >>".to_string()).collect(); + let src = page_with_annots(&widgets, &[], ""); + take_work(); + let renames = combine_field_conflicts(&[&src]).remove(0); + let work = take_work(); + assert!(work <= 4 * n, "{work} suffix attempts for {n} fields"); + // Still the first free variant, in order. + assert_eq!(renames.len(), n - 1); + assert_eq!(renames.first(), Some(&("x".to_string(), "x-1".to_string()))); + assert_eq!(renames.last(), Some(&("x".to_string(), format!("x-{}", n - 1)))); +} + +#[test] +fn annotation_actions_share_the_objects_they_copy() { + // Fifty links whose URI actions all point at one string object. + let n = 50; + let links: Vec = + (0..n).map(|_| format!("<< /Type /Annot /Subtype /Link /Rect [0 0 10 10] /A << /S /URI /URI {} 0 R >> >>", n + 5)).collect(); + let src = page_with_annots(&links, &["(https://example.com/shared)".into()], ""); + let out = extract_pages(&src, &[0]).unwrap(); + let copies = out + .object_numbers() + .into_iter() + .filter(|&o| out.get(ObjRef::new(o, out.generation(o))).as_string().is_some_and(|s| s.bytes == b"https://example.com/shared")) + .count(); + assert_eq!(copies, 1, "the shared URI is copied once, not once per action"); +} + +#[test] +fn name_trees_with_shared_kids_are_walked_once_per_node() { + // Each level lists the next one twice: 2^depth paths through `depth` nodes. + let depth = 10; + let first = 6; // after the catalog, pages, page, content and the link + let mut extra = Vec::new(); + for tree in 0..2 { + let base = first + tree * (depth + 1); + for level in 0..depth { + let next = base + level + 1; + extra.push(format!("<< /Kids [{next} 0 R {next} 0 R] >>")); + } + extra.push(if tree == 0 { + "<< /Names [(a.txt) << /Type /Filespec /F (a.txt) >>] >>".into() + } else { + "<< /Names [(other) [3 0 R /Fit]] >>".into() + }); + } + let link = "<< /Type /Annot /Subtype /Link /Rect [0 0 10 10] /Dest (missing) >>".to_string(); + let catalog_extra = format!("/Names << /EmbeddedFiles {first} 0 R /Dests {} 0 R >>", first + depth + 1); + let src = page_with_annots(&[link], &extra, &catalog_extra); + take_work(); + let out = combine(&[("a", &src)]).unwrap(); + let work = take_work(); + assert!(work <= 8 * (depth + 1), "{work} name-tree nodes visited"); + let names = out.resolve(catalog(&out).get(b"Names").unwrap()).as_dict().cloned().unwrap(); + let tree = out.resolve(names.get(b"EmbeddedFiles").unwrap()).as_dict().cloned().unwrap(); + assert_eq!(tree.get(b"Names").and_then(|n| n.as_array()).map(Vec::len), Some(2), "one attachment, listed once"); +} + +#[test] +fn a_form_drawing_itself_repeatedly_is_analysed_once() { + let form = "/F Do /F Do /F Do /F Do"; + let b: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >>".into(), + "<< /Type /Page /Parent 2 0 R /Contents 4 0 R /Resources << /XObject << /F 5 0 R /Unused 6 0 R >> >> >>".into(), + "<< /Length 5 >>\nstream\n/F Do\nendstream".into(), + format!( + "<< /Type /XObject /Subtype /Form /BBox [0 0 10 10] /Resources << /XObject << /F 5 0 R >> >> /Length {} >>\nstream\n{form}\nendstream", + form.len() + ), + "<< /Type /XObject /Subtype /Form /BBox [0 0 10 10] /Length 0 >>\nstream\n\nendstream".into(), + ]; + let src = open(build(&b, "/Root 1 0 R")); + take_work(); + let out = extract_pages(&src, &[0]).unwrap(); + let work = take_work(); + assert!(work <= 4, "{work} form analyses"); + let page = out.get(pages(&out).unwrap()[0].obj).as_dict().cloned().unwrap(); + let res = out.resolve(page.get(b"Resources").unwrap()).as_dict().cloned().unwrap(); + let x = out.resolve(res.get(b"XObject").unwrap()).as_dict().cloned().unwrap(); + assert!(x.contains(b"F") && !x.contains(b"Unused"), "pruning still keeps what is drawn and drops the rest"); +} + /// One page whose /Contents names the same 1 MiB stream 300 times (a few kilobytes on disk). fn repeated_contents() -> Document { use pdfcraft_cos::{Dict, Stream}; diff --git a/crates/organize/src/xmp.rs b/crates/organize/src/xmp.rs index bbcbbcf83..cc3a78ccf 100644 --- a/crates/organize/src/xmp.rs +++ b/crates/organize/src/xmp.rs @@ -172,11 +172,24 @@ fn element_span(xmp: &str, name: &str) -> Option<(usize, usize)> { /// start and end of its value, and its quote character. Only matches inside a start tag. fn attribute_span(xmp: &str, name: &str) -> Option<(usize, usize, usize, char)> { let mut from = 0; + // The last `<` and `>` before the match, found by one pass over the packet rather than by + // searching back from every match. + let (mut scanned, mut last_open, mut last_close) = (0, None, None); while let Some(i) = xmp.get(from..).and_then(|s| s.find(name)).map(|i| i + from) { from = i + name.len(); let before = xmp.get(..i)?; + let gap = xmp.get(scanned..i)?; + #[cfg(test)] + crate::work(gap.len()); + if let Some(at) = gap.rfind('<') { + last_open = Some(scanned + at); + } + if let Some(at) = gap.rfind('>') { + last_close = Some(scanned + at); + } + scanned = i; // Preceded by whitespace, and inside a tag (the last `<` comes after the last `>`). - if !before.ends_with(char::is_whitespace) || before.rfind('<') < before.rfind('>') || before.rfind('<').is_none() { + if !before.ends_with(char::is_whitespace) || last_open < last_close || last_open.is_none() { continue; } let rest = xmp.get(from..)?; @@ -269,4 +282,18 @@ mod tests { } } } + + #[test] + fn attribute_search_scans_the_packet_once() { + // Thousands of attribute-like words in one text node: none is inside a tag. + let packet = format!("{}", " rdf:about".repeat(5_000)); + crate::take_work(); + assert_eq!(attribute_span(&packet, "rdf:about"), None); + let work = crate::take_work(); + assert!(work <= 2 * packet.len(), "{work} bytes scanned in a {}-byte packet", packet.len()); + // A real attribute after them is still found. + let packet = packet.replace("", ""); + let (_, s, e, q) = attribute_span(&packet, "rdf:about").unwrap(); + assert_eq!((packet.get(s..e), q), (Some("u"), '\'')); + } } diff --git a/crates/print/src/lib.rs b/crates/print/src/lib.rs index 9d47eb1bf..db65492c1 100644 --- a/crates/print/src/lib.rs +++ b/crates/print/src/lib.rs @@ -362,7 +362,7 @@ fn text_direction(doc: &Document, data: &[u8], res: &Dict, base: Matrix, depth: let m = form.dict.get(b"Matrix").and_then(|m| doc.resolve(m).as_array().and_then(|a| Matrix::from_operands(a))).unwrap_or_default(); let inner = form.dict.get(b"Resources").and_then(|r| doc.resolve(r).as_dict().cloned()).unwrap_or_else(|| res.clone()); if !budget.enter_form() { - return; + continue; } if let Ok(body) = budget.decode(form) { text_direction(doc, &body, &inner, m.then(&ctm), depth + 1, score, budget); diff --git a/crates/print/src/tests.rs b/crates/print/src/tests.rs index fe3a1e29a..fabe19a13 100644 --- a/crates/print/src/tests.rs +++ b/crates/print/src/tests.rs @@ -927,3 +927,16 @@ fn text_direction_stops_at_the_form_budget() { text_direction(&doc, "/X Do ".repeat(20).as_bytes(), &res, Matrix::IDENTITY, 0, &mut score, &mut DecodeBudget::page()); assert!(score > 0 && score <= pdfcraft_cos::MAX_PAGE_FORMS as i64, "score {score}"); } + +/// A form refused by the spent budget is left out; the text drawn after it still counts. +#[test] +fn text_direction_reads_on_past_a_refused_form() { + let mut doc = Document::new_empty(); + let form = doc.add(Stream::from_raw([(b"Subtype".to_vec(), Object::Name(b"Form".to_vec()))].into_iter().collect(), Vec::new())); + let xobjects: Dict = [(b"X".to_vec(), Object::Ref(form))].into_iter().collect(); + let res: Dict = [(b"XObject".to_vec(), Object::Dict(xobjects))].into_iter().collect(); + let mut score = 0; + let content = b"/X Do BT 0 1 -1 0 0 0 Tm (a) Tj ET"; + text_direction(&doc, content, &res, Matrix::IDENTITY, 0, &mut score, &mut DecodeBudget::new(1 << 20, 0)); + assert_eq!(score, 1); +} diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index fce9f432a..00fbbbd17 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -357,10 +357,17 @@ fn localize_resources(doc: &mut Document, selected: &[usize]) -> Result<(), pdfc /// /// # Errors /// -/// This operation is not transactional: an error may leave `doc` partially modified, with -/// sensitive content still present. Call this on a working clone and replace the original only -/// on success. On any error, discard the working clone; do not save or reuse it. +/// On any error `doc` is left as it was: the redaction runs on a copy (cheap: unchanged objects +/// are shared) that replaces `doc` only on success. pub fn apply(doc: &mut Document, pages: Option<&[usize]>) -> Result { + let mut work = doc.clone(); + let report = apply_to(&mut work, pages)?; + *doc = work; + Ok(report) +} + +/// [`apply`] without the copy: an error can leave `doc` part redacted. +fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result { let (all_marks, unreadable) = marks_checked(doc); // A mark with no readable area is not applied, and applying the others would look complete: // refuse before anything changes. Only the pages being applied count. diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 2478e0b31..e29cd9979 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -1690,8 +1690,8 @@ fn bluefin_survives(doc: &Document) -> bool { /// #3285 review (the maintainer's probes): an element written in place in the parent tree that /// owns the redacted text can't be changed, so applying is refused; the same element as an object -/// of its own is cleaned, and nothing of the word is left in a full rewrite. -/// Refusal does not promise rollback: direct callers discard the failed working copy. +/// of its own is cleaned, and nothing of the word is left in a full rewrite. A refusal leaves the +/// document as it was. #[test] fn an_element_written_in_place_owning_redacted_text_is_refused() { let objs = |element: &str| -> Vec> { @@ -1711,17 +1711,13 @@ fn an_element_written_in_place_owning_redacted_text_is_refused() { let mut doc = pdf(objs("<< /Type /StructElem /S /P /P 6 0 R /Pg 3 0 R /K 0 /ActualText (Codename Bluefin) >>")); mark(&mut doc, 0, &[[10.0, 140.0, 290.0, 170.0]], ""); let before = write_full(&doc, &SaveOptions::default()).unwrap(); - let mut working = doc.clone(); - let error = apply(&mut working, None).unwrap_err(); + let error = apply(&mut doc, None).unwrap_err(); assert!(matches!(error, RedactError::Tags)); assert_eq!( error.to_string(), "the document's tags can't be cleaned completely (an element written in place, or a structure tree too large or too deep to follow); the operation was refused" ); - assert!(!content(&working, 0).contains("Bluefin"), "the failed working copy was partially changed"); - assert!(bluefin_survives(&working), "the failed working copy still holds the word in its tags"); - drop(working); - assert_eq!(write_full(&doc, &SaveOptions::default()).unwrap(), before, "discarding the working copy preserves the original"); + assert_eq!(write_full(&doc, &SaveOptions::default()).unwrap(), before, "the refused document is unchanged"); let mut doc = pdf(objs("7 0 R")); mark(&mut doc, 0, &[[10.0, 140.0, 290.0, 170.0]], ""); let r = apply(&mut doc, None).unwrap(); @@ -2745,3 +2741,24 @@ fn content_and_forms_share_one_budget_per_page() { mark(&mut doc, 0, &[[0.0, 90.0, 50.0, 120.0]], ""); assert!(apply(&mut doc, None).is_err(), "the form past the page's allowance was read"); } + +/// A page that fails to read after another page was already redacted leaves the whole document as +/// it was, the shared resources the first page's rewrite localised included. +#[test] +fn a_failure_on_a_later_page_leaves_the_document_unchanged() { + let mut doc = two_pages_one_resource_dictionary(true, false); + let all = pdfcraft_model::pages(&doc); + let (first, second, parent) = (all[0].obj, all[1].obj, doc.get(all[0].obj).as_dict().unwrap().reference(b"Parent").unwrap()); + let mut dct = Dict::new(); + dct.set(b"Filter".to_vec(), Object::name("DCTDecode")); + let bad = doc.add(Object::Stream(Stream::from_raw(dct, b"garbage".to_vec()))); + doc.update_dict(second, |d| d.set(b"Contents".to_vec(), Object::Ref(bad))).unwrap(); + mark(&mut doc, 0, &[[0.0, 0.0, 300.0, 300.0]], ""); + mark(&mut doc, 1, &[[0.0, 0.0, 50.0, 50.0]], ""); + let before: Vec = [first, second, parent].iter().map(|r| (*doc.get(*r)).clone()).collect(); + let text = content(&doc, 0); + assert_eq!(apply(&mut doc, None), Err(RedactError::Unreadable(2))); + let after: Vec = [first, second, parent].iter().map(|r| (*doc.get(*r)).clone()).collect(); + assert_eq!(after, before); + assert_eq!(content(&doc, 0), text); +}