From fd653e46ac5e0493df37ea437300ef9fa250bbe5 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 16 Oct 2026 21:31:49 +0800 Subject: [PATCH 01/23] M8: redaction removes Info entries and XMP that repeat the redacted text (#2448) Applying redactions removed the glyphs under the marks but left the same words in the document information dictionary and the XMP metadata. The redaction interpreter now collects the removed glyphs' text in runs by position. After verification, /Info entries containing a run (or one of its longer words) are removed, and the catalog's and pages' XMP streams whose text mentions one are removed whole; unreadable or oversized XMP fails closed. Report::metadata counts what went. Co-Authored-By: Claude Opus 5.5 --- ROADMAP.md | 1 + crates/redact/README.md | 8 +- crates/redact/src/interp.rs | 54 ++++++++- crates/redact/src/lib.rs | 18 ++- crates/redact/src/metadata.rs | 214 ++++++++++++++++++++++++++++++++++ crates/redact/src/tests.rs | 78 +++++++++++++ parity/acrobat-features.toml | 4 +- 7 files changed, 370 insertions(+), 7 deletions(-) create mode 100644 crates/redact/src/metadata.rs diff --git a/ROADMAP.md b/ROADMAP.md index f5de8044e1..3b207390bb 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -324,6 +324,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md - **2026-10-14 (M12, reading colours, #2059):** Pending page backgrounds use the document's reading colour immediately, preventing white flashes while scrolling to uncached pages. A headless canvas regression covers three pending pages in every preset and a custom background. Estimates unchanged. - **2026-10-15 (community, M6, #2608):** In Prepare a form the selected field moves with the arrow keys: a point per press, ten with Shift, as the object nudge and dragging it do. Each press is one undo step ("Change field properties"), and the keys no longer turn the page while a field is selected. Estimates unchanged. - **2026-10-15 (M3, closable community card):** The ArtCraft community card on Home has a close button that hides it for the rest of the run; it returns on the next launch, and its links stay in the top bar's Discord button and the Help menu. A UI regression covers closing and the fresh-launch default. Estimates unchanged. +- **2026-10-15 (M8, #2448):** Applying redactions also removes the document information entries and the document's and pages' XMP metadata that repeat the removed text, so a redacted word can't survive in `/Title`, `/Keywords` or the XMP that mirrors them. Estimates unchanged. - **2026-10-13 (community, M3, #1569 review):** Pixel-only Wayland wheel, wheel-tilt and continuous input use standalone moved events, so Ctrl zoom does not remain latched after the modifier is released (#991); vendored winit patch (13), with a policy test. Estimates unchanged. - **2026-10-15 (M14, #2383 candidate):** Device-limit requests now clamp unsupported maxima and minimum alignments before enabling the adapter's full 2D texture extent. This addresses the reported V3D request of 16 shader variables on an adapter supporting 15. Synthetic regressions fail before the clamp and pass afterward, along with the existing HiDPI case and Windows GPU-required application tests. Raspberry Pi startup remains unverified; milestone and overall estimates are unchanged. - **2026-10-12 (M6):** XFDF field import and data-file extraction join all text nodes in a value, preserving text after XML comments and processing instructions. Synthetic regressions cover Unicode, CDATA, empty values and PDF save/reopen. Estimates unchanged. diff --git a/crates/redact/README.md b/crates/redact/README.md index 34c9f4fe1f..55c0e02f85 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -29,7 +29,13 @@ clear_marks(&mut doc, None)?; // remove marks without app - **annotations** whose rectangle overlaps (with their pop-ups and their replies, `/IRT`), and **form fields** with a widget under a mark; - the marks themselves, replaced by boxes in their fill colour (and overlay text) drawn into the - page as a `/PCMark /Redaction` stream. + page as a `/PCMark /Redaction` stream; +- **metadata** that repeats the removed text (#2448): `/Info` entries containing it, and the + catalog's and pages' XMP streams whose text (element content and attribute values, entities + decoded) contains it, removed whole; unreadable XMP goes too. The removed glyphs' text is + collected in runs by position, and each run (two or more letters or digits) and each of its + words of three or more are matched without regard to case or spacing. `Report::metadata` + counts what went. A verification pass re-reads each redacted page and fails the operation if any glyph or inline image is still under an area. It checks every content stream the page had, whatever its diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index ac69c73acb..42e2045aab 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -46,6 +46,9 @@ pub(crate) const MAX_DEPTH: usize = 50; // A form a reader still shows must be walkable here, or a redaction removes content that is // visible; the build says so rather than a test, so the two limits cannot drift apart. const _: () = assert!(MAX_DEPTH >= pdfcraft_edit::text::MAX_FORM_DEPTH); +/// Bounds on the removed text collected for the metadata check (#2448). +const MAX_RUNS: usize = 100_000; +const MAX_RUN_BYTES: usize = 64 * 1024; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum Mode { @@ -122,6 +125,13 @@ pub(crate) struct Scope<'a> { xobject_oc_cache: HashMap, /// Content removed from hidden layers (blocks and XObjects). pub layer_blocks: usize, + /// Apply: the text of the glyphs removed under the regions, one entry per run of adjacent + /// glyphs (#2448), so the document's metadata can be checked for it. Call [`Scope::flush`] + /// after processing to close the last run. + pub removed_text: Vec, + /// The run being collected, and where its last glyph ended (device space) with its height. + run: String, + run_end: Option<((f64, f64), f64)>, fonts: HashMap, Rc>, used_names: Vec>, depth: usize, @@ -146,6 +156,9 @@ impl<'a> Scope<'a> { ocmd_cache: HashMap::new(), xobject_oc_cache: HashMap::new(), layer_blocks: 0, + removed_text: Vec::new(), + run: String::new(), + run_end: None, fonts: HashMap::new(), used_names: Vec::new(), depth: 0, @@ -153,6 +166,35 @@ impl<'a> Scope<'a> { } } + /// Close the run of removed text being collected. + pub fn flush(&mut self) { + self.run_end = None; + let run = std::mem::take(&mut self.run); + if !run.trim().is_empty() && self.removed_text.len() < MAX_RUNS { + self.removed_text.push(run); + } + } + + /// Note a removed glyph's text: `origin` and `end` are where it starts and ends in device + /// space, `height` its size there. A glyph that starts where the last one ended continues the + /// run, one a little further along the line is a new word of it, and anything else starts a + /// new run, so separate marks don't join their text. + fn removed(&mut self, text: &str, origin: (f64, f64), end: (f64, f64), height: f64) { + if let Some((last, h)) = self.run_end { + let gap = (origin.0 - last.0).hypot(origin.1 - last.1); + let h = h.max(height); + if !(gap.is_finite() && h.is_finite()) || gap > 1.5 * h { + self.flush(); + } else if gap > 0.15 * h && !self.run.ends_with(char::is_whitespace) { + self.run.push(' '); + } + } + if self.run.len().saturating_add(text.len()) <= MAX_RUN_BYTES { + self.run.push_str(text); + } + self.run_end = Some((end, height)); + } + /// Sanitizing visits every form XObject (not only those under a region). fn everywhere(&self) -> bool { self.hidden_text.is_some() || self.layers_enabled @@ -641,7 +683,7 @@ fn clean_replacements(doc: &Document, resources: &Dict, original: &[Op], process /// Lay out a shown string (or `TJ` array) glyph by glyph, advancing `tm`. Returns the `TJ` /// items with the glyphs under a region replaced by displacements, and how many were removed. -fn show(scope: &Scope<'_>, gs: &Gs, tm: &mut Matrix, items: &[Object]) -> (Vec, usize) { +fn show(scope: &mut Scope<'_>, gs: &Gs, tm: &mut Matrix, items: &[Object]) -> (Vec, usize) { let f = &gs.font; let size = gs.size; let mut out: Vec = Vec::new(); @@ -679,6 +721,16 @@ fn show(scope: &Scope<'_>, gs: &Gs, tm: &mut Matrix, items: &[Object]) -> (Vec scope.glyph_hit(b, origin), }; + if scope.mode == Mode::Apply && scope.hidden_text.is_none() { + if hit { + let end = trm.apply(w0, 0.0); + let top = trm.apply(0.0, 1.0); + let height = (top.0 - origin.0).hypot(top.1 - origin.1); + scope.removed(f.text_of(code).as_deref().unwrap_or(""), origin, end, height); + } else { + scope.flush(); + } + } glyphs.push(Glyph { bytes: pos..pos + len, advance, hit }); *tm = Matrix::translate(advance * gs.scale, 0.0).then(tm); pos += len; diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index de9dafe7db..643dc5671e 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -11,6 +11,7 @@ //! //! A verification pass then re-reads every redacted page; if any glyph or inline image is still //! under a region the whole operation fails (callers keep the previous document: fail-closed). +//! Finally, document metadata (`/Info` entries, XMP) that repeats the removed text is removed. #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] @@ -21,6 +22,7 @@ use pdfcraft_cos::{Dict, Document, ObjRef, Object, Stream}; pub mod codes; mod image; mod interp; +mod metadata; mod overlay; pub mod patterns; pub mod sanitize; @@ -149,6 +151,8 @@ pub struct Report { /// 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, + /// Document information entries and XMP streams removed because they held redacted text. + pub metadata: usize, } fn rect_of(doc: &Document, o: Option<&Object>) -> Option<[f64; 4]> { @@ -418,6 +422,7 @@ fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result) -> Result) -> Result 0 { return Err(RedactError::Residue(residue)); } + // 6. The document's metadata must not keep what the pages no longer show (#2448). + report.metadata = metadata::scrub(doc, &removed_text)?; // The previous revision still holds the removed content: the next save must rewrite. doc.require_full_save(); Ok(report) @@ -542,6 +549,8 @@ struct Applying { retired_properties: Vec, widget_owner: Vec<(ObjRef, ObjRef)>, removed_everywhere: HashSet, + /// The text of the removed glyphs, for the metadata check at the end. + removed_text: Vec, } /// Step 1 for one page: rewrites its content so nothing readable stays under a mark, and cleans the tags it no longer shows. @@ -552,14 +561,17 @@ fn redact_page_content( page: &pdfcraft_model::Page, rects: &[[f64; 4]], ) -> Result<(), RedactError> { - let Applying { report, doomed_xobjects, retired_properties, .. } = applying; + let Applying { report, doomed_xobjects, retired_properties, removed_text, .. } = applying; // 1. Content. let mut budget = pdfcraft_cos::DecodeBudget::page(); let (list, data) = page_streams_within(doc, &page.dict, pi, &mut budget)?; let resources = page.dict.get(b"Resources").and_then(|r| doc.resolve(r).as_dict().cloned()).unwrap_or_default(); let out = { let mut scope = Scope::new(rects, Mode::Apply, report, budget); - process(doc, &mut scope, &data, &resources, pdfcraft_content::Matrix::IDENTITY) + let out = process(doc, &mut scope, &data, &resources, pdfcraft_content::Matrix::IDENTITY); + scope.flush(); + removed_text.append(&mut scope.removed_text); + out }; // Fail closed: the interpreter lays vertical-writing text out horizontally, so it cannot // tell whether those glyphs lie under a mark. Rather than report success and possibly diff --git a/crates/redact/src/metadata.rs b/crates/redact/src/metadata.rs new file mode 100644 index 0000000000..821e744b06 --- /dev/null +++ b/crates/redact/src/metadata.rs @@ -0,0 +1,214 @@ +//! Metadata that repeats redacted text (#2448). +//! +//! [`crate::apply`] removes the glyphs under the marks, but the same words are often in the +//! document information dictionary (`/Title`, `/Subject`, `/Keywords`…) and the XMP metadata +//! that mirrors it. Once the page no longer shows them, the metadata must not keep them either. +//! +//! An `/Info` entry whose text contains a redacted term is removed. An XMP stream whose text +//! (element content and attribute values, entities decoded) contains one is removed whole: +//! editing the XML in place could leave it malformed or keep the term in another spelling. +//! Unreadable XMP goes too, and so does text too large to search against every term +//! (fail-closed). Terms are matched without regard to case or spacing. + +use std::collections::HashSet; + +use pdfcraft_cos::{Document, ObjRef, Object}; + +/// Terms longer than this are cut (matching still finds the prefix). +const MAX_TERM_CHARS: usize = 4096; +const MAX_TERMS: usize = 20_000; +/// XMP packets larger than this, decoded, are removed without reading them. +const MAX_XMP_BYTES: usize = 16 * 1024 * 1024; +/// Text whose length times the number of terms exceeds this is treated as matching rather than +/// searched (fail-closed), so a page of removed text against a large packet stays quick. +const MAX_WORK: usize = 1 << 28; + +/// Lower-case `s` with every run of whitespace made one space, trimmed. +fn normalize(s: &str) -> String { + s.split_whitespace().flat_map(|w| w.chars().flat_map(char::to_lowercase).chain(std::iter::once(' '))).collect::().trim_end().to_string() +} + +fn significant(s: &str, min: usize) -> bool { + s.chars().filter(|c| c.is_alphanumeric()).count() >= min +} + +/// The terms to look for: each removed run (at least two letters or digits), and each of its +/// words with at least three, so "Smith, John" in `/Author` is caught when "John Smith" was +/// redacted. Shorter pieces would match nearly any text. +pub(crate) fn terms(removed: &[String]) -> Vec { + let mut seen = HashSet::new(); + let mut out = Vec::new(); + for run in removed { + let run: String = normalize(run).chars().take(MAX_TERM_CHARS).collect(); + let words = run.split(|c: char| !c.is_alphanumeric()).filter(|w| significant(w, 3)).map(str::to_string).collect::>(); + for t in std::iter::once(run.clone()).filter(|r| significant(r, 2)).chain(words) { + if out.len() >= MAX_TERMS { + return out; + } + if seen.insert(t.clone()) { + out.push(t); + } + } + } + out +} + +fn mentions(text: &str, terms: &[String]) -> bool { + let text = normalize(text); + text.len().saturating_mul(terms.len()) > MAX_WORK || terms.iter().any(|t| text.contains(t.as_str())) +} + +/// Decode the five predefined XML entities and character references; anything else stays. +fn unescape(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + let mut rest = s; + while let Some(at) = rest.find('&') { + out.push_str(rest.get(..at).unwrap_or("")); + let tail = rest.get(at..).unwrap_or(""); + let end = tail.find(';').filter(|&e| e <= 12); + let entity = end.and_then(|e| tail.get(1..e)); + let decoded = entity.and_then(|e| match e { + "amp" => Some('&'), + "lt" => Some('<'), + "gt" => Some('>'), + "quot" => Some('"'), + "apos" => Some('\''), + _ => { + let n = match e.strip_prefix("#x").or_else(|| e.strip_prefix("#X")) { + Some(h) => u32::from_str_radix(h, 16).ok(), + None => e.strip_prefix('#').and_then(|d| d.parse().ok()), + }; + n.and_then(char::from_u32) + } + }); + match (decoded, end) { + (Some(c), Some(e)) => { + out.push(c); + rest = tail.get(e + 1..).unwrap_or(""); + } + _ => { + out.push('&'); + rest = tail.get(1..).unwrap_or(""); + } + } + } + out.push_str(rest); + out +} + +/// The text an XML document carries: element content and attribute values (XMP stores simple +/// properties as attributes), each followed by a space, entities decoded. Names, comments and +/// markup are left out, so a term can't match a tag such as `dc:title`. +pub(crate) fn xml_text(xml: &str) -> String { + let mut out = String::new(); + let mut rest = xml; + while !rest.is_empty() { + let Some(open) = rest.find('<') else { + out.push_str(&unescape(rest)); + break; + }; + out.push_str(&unescape(rest.get(..open).unwrap_or(""))); + out.push(' '); + let tag = rest.get(open..).unwrap_or(""); + if let Some(body) = tag.strip_prefix("").unwrap_or(body.len()); + out.push_str(body.get(..end).unwrap_or("")); + out.push(' '); + rest = body.get(end + 3..).unwrap_or(""); + continue; + } + if tag.starts_with("").and_then(|e| tag.get(e + 3..)).unwrap_or(""); + continue; + } + // Attribute values inside the tag, up to its closing `>` (which can't be inside quotes). + let mut chars = tag.char_indices().skip(1); + let mut end = tag.len(); + while let Some((i, c)) = chars.next() { + match c { + '>' => { + end = i + 1; + break; + } + '"' | '\'' => { + let mut value = String::new(); + for (_, d) in chars.by_ref() { + if d == c { + break; + } + value.push(d); + } + out.push_str(&unescape(&value)); + out.push(' '); + } + _ => {} + } + } + rest = tag.get(end..).unwrap_or(""); + } + out +} + +/// The decoded text of an XMP stream (UTF-8, or UTF-16 with a byte-order mark), or `None` when it +/// can't be read. +fn xmp_text(doc: &Document, o: &Object) -> Option { + let Object::Stream(s) = &*doc.resolve(o) else { return None }; + let bytes = s.decoded().ok()?; + if bytes.len() > MAX_XMP_BYTES { + return None; + } + let units = |be: bool| { + bytes.get(2..).unwrap_or(&[]).as_chunks::<2>().0.iter().map(move |&p| if be { u16::from_be_bytes(p) } else { u16::from_le_bytes(p) }) + }; + let text = match bytes.get(..2) { + Some([0xFE, 0xFF]) => char::decode_utf16(units(true)).map(|c| c.unwrap_or('\u{FFFD}')).collect(), + Some([0xFF, 0xFE]) => char::decode_utf16(units(false)).map(|c| c.unwrap_or('\u{FFFD}')).collect(), + _ => String::from_utf8_lossy(&bytes).into_owned(), + }; + Some(xml_text(&text)) +} + +/// Remove the metadata that still holds `removed` text: matching `/Info` entries, and the +/// catalog's and pages' XMP streams that mention it. Returns how many entries and streams went. +pub(crate) fn scrub(doc: &mut Document, removed: &[String]) -> Result { + let terms = terms(removed); + if terms.is_empty() { + return Ok(0); + } + let mut n = 0; + + // The information dictionary, direct in the trailer or an object of its own. + let info_obj = doc.trailer().get(b"Info").cloned(); + if let Some(info) = info_obj.as_ref().and_then(|i| doc.resolve(i).as_dict().cloned()) { + let doomed: Vec> = info + .iter() + .filter(|(_, v)| doc.resolve(v).as_string().is_some_and(|s| mentions(&s.to_text(), &terms))) + .map(|(k, _)| k.to_vec()) + .collect(); + if !doomed.is_empty() { + n += doomed.len(); + let mut kept = info.clone(); + for k in &doomed { + kept.remove(k); + } + match info_obj.as_ref().and_then(Object::as_ref) { + Some(r) => doc.update_dict(r, |d| *d = kept)?, + None => doc.trailer_mut().set(b"Info".to_vec(), Object::Dict(kept)), + } + } + } + + // XMP on the catalog and on every page. + let mut holders: Vec = doc.root().into_iter().collect(); + holders.extend(pdfcraft_model::pages(doc).iter().map(|p| p.obj)); + for holder in holders { + let Some(xmp) = doc.get(holder).as_dict().and_then(|d| d.get(b"Metadata").cloned()) else { continue }; + if xmp_text(doc, &xmp).is_none_or(|t| mentions(&t, &terms)) { + doc.update_dict(holder, |d| { + d.remove(b"Metadata"); + })?; + n += 1; + } + } + Ok(n) +} diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index c072d19db7..a44dd68a63 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -104,6 +104,84 @@ fn a_whole_line_under_a_mark_goes_even_without_widths() { assert_eq!(under(&mut doc, 0, &[[0.0, 0.0, 300.0, 300.0]]), 0, "{}", content(&doc, 0)); } +/// Issue #2448: the redacted words are often in the document information dictionary and the XMP +/// metadata too. Applying removes the entries and streams that repeat them, and keeps the rest. +#[test] +fn redacted_text_does_not_survive_in_info_or_xmp() { + let xmp = |body: &str| { + stream( + "/Type /Metadata /Subtype /XML", + format!( + r#"{body}"# + ) + .as_bytes(), + ) + }; + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /Metadata 7 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 >> >> /Metadata 9 0 R >>".to_vec(), + // "BLUEFIN" spans x 55-90. + stream("", b"BT /F1 10 Tf 10 100 Td (Codename BLUEFIN report) Tj ET"), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + // 7: the document's XMP names it in an attribute, through a character reference. + xmp(r#""#), + // 8: the information dictionary. + b"<< /Title (Codename BLUEFIN report) /Keywords (tuna, bluefin) /Author (Ada Lovelace) /Producer (Test) >>".to_vec(), + // 9: the page's XMP doesn't mention it. + xmp(r#"Annual figures"#), + ]; + let mut doc = Document::open(Arc::new(Pdf::new().objs(objs).trailer_entry("/Info 8 0 R").build())).unwrap(); + mark(&mut doc, 0, &[[55.0, 95.0, 90.0, 110.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!((r.glyphs, r.metadata), (7, 3), "{r:?}"); + let bytes = write_full(&doc, &SaveOptions::default()).unwrap(); + let has = |needle: &[u8]| bytes.windows(needle.len()).any(|w| w.eq_ignore_ascii_case(needle)); + assert!(!has(b"bluefin") && !has(b"BlueFin"), "the redacted word survived in the metadata"); + let saved = Document::open(Arc::new(bytes)).unwrap(); + let info = saved.trailer().get(b"Info").and_then(|i| saved.resolve(i).as_dict().cloned()).unwrap(); + let text = |k: &[u8]| info.get(k).and_then(|v| saved.resolve(v).as_string().map(|s| s.to_text())); + assert_eq!((text(b"Title"), text(b"Keywords")), (None, None)); + assert_eq!((text(b"Author").as_deref(), text(b"Producer").as_deref()), (Some("Ada Lovelace"), Some("Test"))); + let catalog = saved.get(saved.root().unwrap()).as_dict().cloned().unwrap(); + assert!(!catalog.contains(b"Metadata"), "the document XMP named the redacted word"); + assert!(pdfcraft_model::pages(&saved)[0].dict.contains(b"Metadata"), "the page XMP didn't, and stays"); +} + +/// The removed text is collected in runs by position, not by operator: glyphs shown one operator +/// at a time still make one word, a gap makes a space, and separate marks make separate runs. +#[test] +fn removed_text_is_collected_in_runs() { + let mut doc = one_page( + b"BT /F1 10 Tf 10 100 Td (BLU) Tj 15 0 Td (E) Tj 5 0 Td [(F) -300 (IN)] TJ ( 7) Tj ET BT /F1 10 Tf 10 200 Td (Gary) Tj ET", + "", + vec![], + ); + let page = pdfcraft_model::pages(&doc).swap_remove(0); + let (_, data) = page_streams(&doc, &page.dict, 0).unwrap(); + let res = page.dict.get(b"Resources").and_then(|r| doc.resolve(r).as_dict().cloned()).unwrap_or_default(); + let mut rep = Report::default(); + let rects = [[10.0, 95.0, 300.0, 110.0], [10.0, 195.0, 30.0, 210.0]]; + let mut scope = Scope::new(&rects, Mode::Apply, &mut rep, pdfcraft_cos::DecodeBudget::page()); + process(&mut doc, &mut scope, &data, &res, pdfcraft_content::Matrix::IDENTITY); + scope.flush(); + assert_eq!(scope.removed_text, ["BLUEF IN 7", "Gary"]); +} + +#[test] +fn metadata_terms_and_xmp_text() { + // Runs of two letters or digits or more are matched whole, and by their words of three or + // more, ignoring case and spacing. + let t = metadata::terms(&["John Smith".into(), "7".into(), "of".into(), "A-12".into()]); + assert_eq!(t, ["john smith", "john", "smith", "of", "a-12"]); + // XMP text is element content and attribute values, entities decoded; not names or comments. + let x = metadata::xml_text(r#"Résumé ]]>"#); + let words: Vec<&str> = x.split_whitespace().collect(); + assert_eq!(words, ["x", "&", "y", "Résumé", ""]); + assert!(!x.contains("title") && !x.contains("hidden")); +} + #[test] fn added_text_parameters_do_not_keep_redacted_text() { // An Edit ▸ Add content item: its stream dictionary keeps the source text under /PCAdded. diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 3b8dba920c..2cf652df2a 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -5768,8 +5768,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::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" +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", "crates/redact/src/tests.rs::redacted_text_does_not_survive_in_info_or_xmp"] +notes = "confirmation; one undo step; next save is a full rewrite. Document information entries that contain the removed text, and the document's and pages' XMP streams that do, are removed (#2448; matched without regard to case or spacing, by the whole run of removed text and by its words of three letters or digits or more); other objects' XMP, bookmarks and attachments are not searched. 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 cf2207cbea17344d2ce9e531c7b32a27ab586918 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 16 Oct 2026 21:31:49 +0800 Subject: [PATCH 02/23] M8: an Info value that is part of the redacted text goes too (#2448) An /Info value whose words are too short to be terms ("J. Li") survived when the sentence holding it was redacted. A value that is wholly part of a removed run, with three letters or digits at least, now goes as well (the reverse check #2598 proposed). Co-Authored-By: Claude Opus 5.5 --- crates/redact/README.md | 3 ++- crates/redact/src/metadata.rs | 24 ++++++++++++++++++++++-- crates/redact/src/tests.rs | 22 ++++++++++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/crates/redact/README.md b/crates/redact/README.md index 55c0e02f85..3659a8e0e1 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -30,7 +30,8 @@ clear_marks(&mut doc, None)?; // remove marks without app widget under a mark; - the marks themselves, replaced by boxes in their fill colour (and overlay text) drawn into the page as a `/PCMark /Redaction` stream; -- **metadata** that repeats the removed text (#2448): `/Info` entries containing it, and the +- **metadata** that repeats the removed text (#2448): `/Info` entries containing it or wholly + part of it (three letters or digits at least), and the catalog's and pages' XMP streams whose text (element content and attribute values, entities decoded) contains it, removed whole; unreadable XMP goes too. The removed glyphs' text is collected in runs by position, and each run (two or more letters or digits) and each of its diff --git a/crates/redact/src/metadata.rs b/crates/redact/src/metadata.rs index 821e744b06..385f637ee8 100644 --- a/crates/redact/src/metadata.rs +++ b/crates/redact/src/metadata.rs @@ -4,7 +4,9 @@ //! document information dictionary (`/Title`, `/Subject`, `/Keywords`…) and the XMP metadata //! that mirrors it. Once the page no longer shows them, the metadata must not keep them either. //! -//! An `/Info` entry whose text contains a redacted term is removed. An XMP stream whose text +//! An `/Info` entry whose text contains a redacted term is removed, and so is one whose whole +//! value is part of a removed run (an author "J. Li" redacted from a sentence, whose words are too +//! short to be terms; the reverse check #2598 also proposed). An XMP stream whose text //! (element content and attribute values, entities decoded) contains one is removed whole: //! editing the XML in place could leave it malformed or keep the term in another spelling. //! Unreadable XMP goes too, and so does text too large to search against every term @@ -58,6 +60,18 @@ fn mentions(text: &str, terms: &[String]) -> bool { text.len().saturating_mul(terms.len()) > MAX_WORK || terms.iter().any(|t| text.contains(t.as_str())) } +/// Is `value` itself part of one of the removed `runs` (normalized)? It needs three letters or +/// digits, so an entry such as "by" doesn't go with every sentence that contains it. Too much +/// text to search counts as part (fail-closed). +fn within(value: &str, runs: &[String]) -> bool { + let value = normalize(value); + if !significant(&value, 3) { + return false; + } + let total = runs.iter().map(String::len).fold(0usize, usize::saturating_add); + value.len().saturating_mul(total) > MAX_WORK || runs.iter().any(|r| r.contains(value.as_str())) +} + /// Decode the five predefined XML entities and character references; anything else stays. fn unescape(s: &str) -> String { let mut out = String::with_capacity(s.len()); @@ -175,6 +189,7 @@ pub(crate) fn scrub(doc: &mut Document, removed: &[String]) -> Result = removed.iter().take(MAX_TERMS).map(|r| normalize(r).chars().take(MAX_TERM_CHARS).collect()).collect(); let mut n = 0; // The information dictionary, direct in the trailer or an object of its own. @@ -182,7 +197,12 @@ pub(crate) fn scrub(doc: &mut Document, removed: &[String]) -> Result> = info .iter() - .filter(|(_, v)| doc.resolve(v).as_string().is_some_and(|s| mentions(&s.to_text(), &terms))) + .filter(|(_, v)| { + doc.resolve(v).as_string().is_some_and(|s| { + let s = s.to_text(); + mentions(&s, &terms) || within(&s, &runs) + }) + }) .map(|(k, _)| k.to_vec()) .collect(); if !doomed.is_empty() { diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index a44dd68a63..c8459ba4d0 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -149,6 +149,28 @@ fn redacted_text_does_not_survive_in_info_or_xmp() { assert!(pdfcraft_model::pages(&saved)[0].dict.contains(b"Metadata"), "the page XMP didn't, and stays"); } +/// An `/Info` value that is itself part of the removed text goes, though its words are too short +/// to be terms ("J. Li"); a value with fewer than three letters or digits stays. +#[test] +fn an_info_value_inside_the_redacted_text_goes() { + let objs: Vec> = 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 << /Font << /F1 5 0 R >> >> >>".to_vec(), + stream("", b"BT /F1 10 Tf 10 100 Td (Prepared by J. Li) Tj ET"), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + b"<< /Author (J. li) /Subject (by) /Producer (Test) >>".to_vec(), + ]; + let mut doc = Document::open(Arc::new(Pdf::new().objs(objs).trailer_entry("/Info 7 0 R").build())).unwrap(); + mark(&mut doc, 0, &[[5.0, 95.0, 200.0, 110.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!(r.metadata, 1, "{r:?}"); + let info = doc.trailer().get(b"Info").and_then(|i| doc.resolve(i).as_dict().cloned()).unwrap(); + assert!(!info.contains(b"Author"), "the author's name was redacted from the page"); + assert!(info.contains(b"Subject") && info.contains(b"Producer")); +} + /// The removed text is collected in runs by position, not by operator: glyphs shown one operator /// at a time still make one word, a gap makes a space, and separate marks make separate runs. #[test] From 459cf4af5d2559b56302ced43b5606eec3c4c2f0 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 16 Oct 2026 21:31:49 +0800 Subject: [PATCH 03/23] M8: test that a shorter value goes from custom XMP too (#2448) A value shorter than the redacted sentence ("James Bond" from "JAMES BOND IS A SPY") in a custom schema's element or attribute takes its packet with it, as it goes from /Info, while a clean packet and an unrelated entry stay. The regression #2598 suggested keeping whichever fix lands. Co-Authored-By: Claude Opus 5.5 --- crates/redact/src/tests.rs | 45 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index c8459ba4d0..eff0c3d525 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -171,6 +171,51 @@ fn an_info_value_inside_the_redacted_text_goes() { assert!(info.contains(b"Subject") && info.contains(b"Producer")); } +/// A value shorter than the redacted sentence goes from XMP too, in a custom schema's element or +/// attribute, as from `/Info` (the regression #2598 suggested keeping): the packets holding it are +/// removed, one that doesn't stays. +#[test] +fn a_shorter_value_in_custom_xmp_goes() { + let xmp = |body: &str| { + stream( + "/Type /Metadata /Subtype /XML", + format!( + r#"{body}"# + ) + .as_bytes(), + ) + }; + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /Metadata 7 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 >> >> /Metadata 8 0 R >>".to_vec(), + stream("", b"BT /F1 10 Tf 10 100 Td (JAMES BOND IS A SPY) Tj ET"), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + // 7: an element of a custom schema. + xmp(r#"James Bond"#), + // 8: an attribute of a custom schema. + xmp(r#""#), + // 9: a third packet, on no holder yet; swapped onto the page below to show a clean one stays. + xmp(r#"Annual figures"#), + b"<< /Author (James Bond) /Subject (Annual figures) >>".to_vec(), + ]; + for (holder_of_8, expected) in [("8 0 R", 3), ("9 0 R", 2)] { + let objs: Vec> = + objs.iter().map(|o| String::from_utf8_lossy(o).replace("/Metadata 8 0 R", &format!("/Metadata {holder_of_8}")).into_bytes()).collect(); + let mut doc = Document::open(Arc::new(Pdf::new().objs(objs).trailer_entry("/Info 10 0 R").build())).unwrap(); + mark(&mut doc, 0, &[[5.0, 95.0, 200.0, 110.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!(r.metadata, expected, "{holder_of_8}: {r:?}"); + let catalog = doc.get(doc.root().unwrap()).as_dict().cloned().unwrap(); + assert!(!catalog.contains(b"Metadata"), "the custom element named him"); + let page_keeps = pdfcraft_model::pages(&doc)[0].dict.contains(b"Metadata"); + assert_eq!(page_keeps, holder_of_8 == "9 0 R", "the attribute packet goes, a clean one stays"); + let info = doc.trailer().get(b"Info").and_then(|i| doc.resolve(i).as_dict().cloned()).unwrap(); + assert!(!info.contains(b"Author") && info.contains(b"Subject"), "the author goes, the unrelated subject stays"); + } +} + /// The removed text is collected in runs by position, not by operator: glyphs shown one operator /// at a time still make one word, a gap makes a space, and separate marks make separate runs. #[test] From c405d280c6ef3e97df0fb28610a16f761d4b1a57 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 16 Oct 2026 21:31:49 +0800 Subject: [PATCH 04/23] M8: metadata check fails closed and finds letter-spaced and short XMP values (#2448) Review follow-up on #2539: - the glyph end includes character and word spacing, so letter-spaced redacted text is one word and still matches /Title (Bluefin); - an XMP value wholly part of a removed run goes, as an /Info value does (J. Li), compared as whole words (True stays with "untrue"); - when the removed text was cut short (too many runs, a run too long, too many terms) every /Info string and XMP stream goes instead of searching what was kept; - XMP that is not valid UTF-8 or UTF-16 (also UTF-16 without a leading mark) is not searched lossily but removed; - namespace declarations and the xpacket wrapper are not searched; comments and other processing instructions are; - the raw-byte assertion saves without object streams so it can see /Info. Co-Authored-By: Claude Opus 5.5 --- crates/redact/README.md | 17 ++-- crates/redact/src/interp.rs | 19 +++- crates/redact/src/lib.rs | 10 +- crates/redact/src/metadata.rs | 177 ++++++++++++++++++++++------------ crates/redact/src/tests.rs | 117 ++++++++++++++++++++-- parity/acrobat-features.toml | 4 +- 6 files changed, 259 insertions(+), 85 deletions(-) diff --git a/crates/redact/README.md b/crates/redact/README.md index 3659a8e0e1..eb3bce469e 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -31,12 +31,17 @@ clear_marks(&mut doc, None)?; // remove marks without app - the marks themselves, replaced by boxes in their fill colour (and overlay text) drawn into the page as a `/PCMark /Redaction` stream; - **metadata** that repeats the removed text (#2448): `/Info` entries containing it or wholly - part of it (three letters or digits at least), and the - catalog's and pages' XMP streams whose text (element content and attribute values, entities - decoded) contains it, removed whole; unreadable XMP goes too. The removed glyphs' text is - collected in runs by position, and each run (two or more letters or digits) and each of its - words of three or more are matched without regard to case or spacing. `Report::metadata` - counts what went. + part of it as whole words (three letters or digits at least), and the catalog's and pages' + XMP streams whose text (element content, attribute values other than `xmlns` declarations, + comments and processing instructions, entities decoded) contains it or has a value wholly + part of it, removed whole, together with any PDF/A identification in that packet. XMP that + isn't valid UTF-8 or UTF-16 goes too, and when the removed text was too much to collect + every `/Info` string and every XMP stream goes (fail-closed). The removed glyphs' text is + collected in runs by position (character and word spacing included, so letter-spaced text + stays one word), and each run (two or more letters or digits) and each of its words of three + or more are matched without regard to case or spacing. A value that shares only words of one + or two letters with the removed text, and isn't wholly part of it, stays: Sanitize's + metadata category removes the rest. `Report::metadata` counts what went. A verification pass re-reads each redacted page and fails the operation if any glyph or inline image is still under an area. It checks every content stream the page had, whatever its diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index 42e2045aab..6781a4060c 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -129,6 +129,9 @@ pub(crate) struct Scope<'a> { /// glyphs (#2448), so the document's metadata can be checked for it. Call [`Scope::flush`] /// after processing to close the last run. pub removed_text: Vec, + /// Some removed text was not kept (too many runs, or a run too long): the metadata check + /// can't search for it and must fail closed. + pub removed_cut: bool, /// The run being collected, and where its last glyph ended (device space) with its height. run: String, run_end: Option<((f64, f64), f64)>, @@ -157,6 +160,7 @@ impl<'a> Scope<'a> { xobject_oc_cache: HashMap::new(), layer_blocks: 0, removed_text: Vec::new(), + removed_cut: false, run: String::new(), run_end: None, fonts: HashMap::new(), @@ -170,8 +174,12 @@ impl<'a> Scope<'a> { pub fn flush(&mut self) { self.run_end = None; let run = std::mem::take(&mut self.run); - if !run.trim().is_empty() && self.removed_text.len() < MAX_RUNS { - self.removed_text.push(run); + if !run.trim().is_empty() { + if self.removed_text.len() < MAX_RUNS { + self.removed_text.push(run); + } else { + self.removed_cut = true; + } } } @@ -191,6 +199,8 @@ impl<'a> Scope<'a> { } if self.run.len().saturating_add(text.len()) <= MAX_RUN_BYTES { self.run.push_str(text); + } else { + self.removed_cut = true; } self.run_end = Some((end, height)); } @@ -723,7 +733,10 @@ fn show(scope: &mut Scope<'_>, gs: &Gs, tm: &mut Matrix, items: &[Object]) -> (V }; if scope.mode == Mode::Apply && scope.hidden_text.is_none() { if hit { - let end = trm.apply(w0, 0.0); + // The glyph ends after its character and word spacing, in `trm`'s + // units (the size is in `trm`), so letter-spaced text stays one word. + let spaced = if size != 0.0 { w0 + spacing / size } else { w0 }; + let end = trm.apply(spaced, 0.0); let top = trm.apply(0.0, 1.0); let height = (top.0 - origin.0).hypot(top.1 - origin.1); scope.removed(f.text_of(code).as_deref().unwrap_or(""), origin, end, height); diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 643dc5671e..9bc3672027 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -423,6 +423,7 @@ fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result) -> Result) -> Result, /// The text of the removed glyphs, for the metadata check at the end. removed_text: Vec, + /// Some removed text was too long to keep whole for that check. + removed_cut: bool, } /// Step 1 for one page: rewrites its content so nothing readable stays under a mark, and cleans the tags it no longer shows. @@ -561,7 +564,7 @@ fn redact_page_content( page: &pdfcraft_model::Page, rects: &[[f64; 4]], ) -> Result<(), RedactError> { - let Applying { report, doomed_xobjects, retired_properties, removed_text, .. } = applying; + let Applying { report, doomed_xobjects, retired_properties, removed_text, removed_cut, .. } = applying; // 1. Content. let mut budget = pdfcraft_cos::DecodeBudget::page(); let (list, data) = page_streams_within(doc, &page.dict, pi, &mut budget)?; @@ -571,6 +574,7 @@ fn redact_page_content( let out = process(doc, &mut scope, &data, &resources, pdfcraft_content::Matrix::IDENTITY); scope.flush(); removed_text.append(&mut scope.removed_text); + *removed_cut |= scope.removed_cut; out }; // Fail closed: the interpreter lays vertical-writing text out horizontally, so it cannot diff --git a/crates/redact/src/metadata.rs b/crates/redact/src/metadata.rs index 385f637ee8..b6ef956192 100644 --- a/crates/redact/src/metadata.rs +++ b/crates/redact/src/metadata.rs @@ -7,10 +7,15 @@ //! An `/Info` entry whose text contains a redacted term is removed, and so is one whose whole //! value is part of a removed run (an author "J. Li" redacted from a sentence, whose words are too //! short to be terms; the reverse check #2598 also proposed). An XMP stream whose text -//! (element content and attribute values, entities decoded) contains one is removed whole: -//! editing the XML in place could leave it malformed or keep the term in another spelling. -//! Unreadable XMP goes too, and so does text too large to search against every term -//! (fail-closed). Terms are matched without regard to case or spacing. +//! (element content, attribute values other than namespace declarations, comments and +//! processing instructions, entities decoded) contains one, or has a value that is part of a +//! removed run, is removed whole: editing the XML in place could leave it malformed or keep the +//! term in another spelling. A PDF/A identification in that packet goes with it. +//! +//! Fail-closed: XMP that isn't UTF-8 or UTF-16 goes, text too large to search against every term +//! counts as matching, and when the removed text itself was cut short (too many runs or terms, +//! or a run too long to keep) every `/Info` string and every XMP stream goes, since what was +//! never collected can't be searched for. Terms are matched without regard to case or spacing. use std::collections::HashSet; @@ -36,23 +41,28 @@ fn significant(s: &str, min: usize) -> bool { /// The terms to look for: each removed run (at least two letters or digits), and each of its /// words with at least three, so "Smith, John" in `/Author` is caught when "John Smith" was -/// redacted. Shorter pieces would match nearly any text. -pub(crate) fn terms(removed: &[String]) -> Vec { +/// redacted. Shorter pieces would match nearly any text. `None` when a run is longer than +/// [`MAX_TERM_CHARS`] or there are more than [`MAX_TERMS`] terms: the caller then can't search +/// for all of it and must clear the metadata instead. +pub(crate) fn terms(removed: &[String]) -> Option> { let mut seen = HashSet::new(); let mut out = Vec::new(); for run in removed { - let run: String = normalize(run).chars().take(MAX_TERM_CHARS).collect(); + let run = normalize(run); + if run.chars().nth(MAX_TERM_CHARS).is_some() { + return None; + } let words = run.split(|c: char| !c.is_alphanumeric()).filter(|w| significant(w, 3)).map(str::to_string).collect::>(); for t in std::iter::once(run.clone()).filter(|r| significant(r, 2)).chain(words) { - if out.len() >= MAX_TERMS { - return out; - } if seen.insert(t.clone()) { + if out.len() >= MAX_TERMS { + return None; + } out.push(t); } } } - out + Some(out) } fn mentions(text: &str, terms: &[String]) -> bool { @@ -60,8 +70,9 @@ fn mentions(text: &str, terms: &[String]) -> bool { text.len().saturating_mul(terms.len()) > MAX_WORK || terms.iter().any(|t| text.contains(t.as_str())) } -/// Is `value` itself part of one of the removed `runs` (normalized)? It needs three letters or -/// digits, so an entry such as "by" doesn't go with every sentence that contains it. Too much +/// Is `value` itself part of one of the removed `runs` (normalized), as whole words? It needs +/// three letters or digits, so an entry such as "by" doesn't go with every sentence that +/// contains it, and word boundaries, so an XMP value "True" doesn't go with "untrue". Too much /// text to search counts as part (fail-closed). fn within(value: &str, runs: &[String]) -> bool { let value = normalize(value); @@ -69,7 +80,15 @@ fn within(value: &str, runs: &[String]) -> bool { return false; } let total = runs.iter().map(String::len).fold(0usize, usize::saturating_add); - value.len().saturating_mul(total) > MAX_WORK || runs.iter().any(|r| r.contains(value.as_str())) + let word = |c: Option| c.is_some_and(char::is_alphanumeric); + let whole = |r: &String| { + r.match_indices(value.as_str()).any(|(at, m)| { + let before = r.get(..at).and_then(|s| s.chars().next_back()); + let after = r.get(at.saturating_add(m.len())..).and_then(|s| s.chars().next()); + !(word(before) && value.starts_with(char::is_alphanumeric)) && !(word(after) && value.ends_with(char::is_alphanumeric)) + }) + }; + value.len().saturating_mul(total) > MAX_WORK || runs.iter().any(whole) } /// Decode the five predefined XML entities and character references; anything else stays. @@ -110,34 +129,42 @@ fn unescape(s: &str) -> String { out } -/// The text an XML document carries: element content and attribute values (XMP stores simple -/// properties as attributes), each followed by a space, entities decoded. Names, comments and -/// markup are left out, so a term can't match a tag such as `dc:title`. -pub(crate) fn xml_text(xml: &str) -> String { - let mut out = String::new(); +/// The text values an XML document carries, entities decoded: element content, attribute values +/// (XMP stores simple properties as attributes), CDATA, and the text of comments and processing +/// instructions other than the `xpacket` wrapper, which a reader can still see. Names and markup are left out, so a term can't +/// match a tag such as `dc:title`, and so are namespace declarations (`xmlns`), whose URIs +/// would match words such as "adobe" or "application" in every packet. +pub(crate) fn xml_values(xml: &str) -> Vec { + let mut out = Vec::new(); + let mut push = |s: String| { + if !s.trim().is_empty() { + out.push(s); + } + }; let mut rest = xml; while !rest.is_empty() { let Some(open) = rest.find('<') else { - out.push_str(&unescape(rest)); + push(unescape(rest)); break; }; - out.push_str(&unescape(rest.get(..open).unwrap_or(""))); - out.push(' '); + push(unescape(rest.get(..open).unwrap_or(""))); let tag = rest.get(open..).unwrap_or(""); - if let Some(body) = tag.strip_prefix("").unwrap_or(body.len()); - out.push_str(body.get(..end).unwrap_or("")); - out.push(' '); - rest = body.get(end + 3..).unwrap_or(""); - continue; - } - if tag.starts_with("").and_then(|e| tag.get(e + 3..)).unwrap_or(""); + let delimited = [(""), (""), ("")].into_iter().find(|(start, _)| tag.starts_with(start)); + if let Some((start, stop)) = delimited { + let body = tag.get(start.len()..).unwrap_or(""); + let end = body.find(stop).unwrap_or(body.len()); + let text = body.get(..end).unwrap_or(""); + // The packet wrapper's fixed `begin`/`id`/`end` would match words such as "end". + if !(start == "` (which can't be inside quotes). let mut chars = tag.char_indices().skip(1); let mut end = tag.len(); + let mut name = String::new(); while let Some((i, c)) = chars.next() { match c { '>' => { @@ -152,10 +179,15 @@ pub(crate) fn xml_text(xml: &str) -> String { } value.push(d); } - out.push_str(&unescape(&value)); - out.push(' '); + let declaration = name == "xmlns" || name.starts_with("xmlns:"); + if !declaration { + push(unescape(&value)); + } + name.clear(); } - _ => {} + '=' => {} + c if c.is_whitespace() || c == '/' => name.clear(), + c => name.push(c), } } rest = tag.get(end..).unwrap_or(""); @@ -163,48 +195,57 @@ pub(crate) fn xml_text(xml: &str) -> String { out } -/// The decoded text of an XMP stream (UTF-8, or UTF-16 with a byte-order mark), or `None` when it -/// can't be read. -fn xmp_text(doc: &Document, o: &Object) -> Option { +/// Decode an XMP packet: UTF-8 (with or without a byte-order mark), or UTF-16 told by its mark +/// or, as a packet usually begins ` Option { + let utf16 = |body: &[u8], be: bool| -> Option { + let (units, rest) = body.as_chunks::<2>(); + if !rest.is_empty() { + return None; + } + char::decode_utf16(units.iter().map(|&p| if be { u16::from_be_bytes(p) } else { u16::from_le_bytes(p) })).collect::>().ok() + }; + match bytes { + [0xEF, 0xBB, 0xBF, rest @ ..] => std::str::from_utf8(rest).ok().map(str::to_string), + [0xFE, 0xFF, rest @ ..] => utf16(rest, true), + [0xFF, 0xFE, rest @ ..] => utf16(rest, false), + [0, a, ..] if *a != 0 => utf16(bytes, true), + [a, 0, ..] if *a != 0 => utf16(bytes, false), + _ => std::str::from_utf8(bytes).ok().map(str::to_string), + } +} + +/// The text values of an XMP stream, or `None` when it can't be read. +fn xmp_values(doc: &Document, o: &Object) -> Option> { let Object::Stream(s) = &*doc.resolve(o) else { return None }; let bytes = s.decoded().ok()?; if bytes.len() > MAX_XMP_BYTES { return None; } - let units = |be: bool| { - bytes.get(2..).unwrap_or(&[]).as_chunks::<2>().0.iter().map(move |&p| if be { u16::from_be_bytes(p) } else { u16::from_le_bytes(p) }) - }; - let text = match bytes.get(..2) { - Some([0xFE, 0xFF]) => char::decode_utf16(units(true)).map(|c| c.unwrap_or('\u{FFFD}')).collect(), - Some([0xFF, 0xFE]) => char::decode_utf16(units(false)).map(|c| c.unwrap_or('\u{FFFD}')).collect(), - _ => String::from_utf8_lossy(&bytes).into_owned(), - }; - Some(xml_text(&text)) + decode_xmp(&bytes).map(|t| xml_values(&t)) } /// Remove the metadata that still holds `removed` text: matching `/Info` entries, and the -/// catalog's and pages' XMP streams that mention it. Returns how many entries and streams went. -pub(crate) fn scrub(doc: &mut Document, removed: &[String]) -> Result { - let terms = terms(removed); - if terms.is_empty() { - return Ok(0); - } - let runs: Vec = removed.iter().take(MAX_TERMS).map(|r| normalize(r).chars().take(MAX_TERM_CHARS).collect()).collect(); +/// catalog's and pages' XMP streams that mention it. `complete` is false when the collection of +/// removed text was cut short; then, as when the terms themselves exceed their limits, every +/// `/Info` string and every XMP stream goes. Returns how many entries and streams went. +pub(crate) fn scrub(doc: &mut Document, removed: &[String], complete: bool) -> Result { + // `None` matches everything (fail-closed). + let search = match terms(removed).filter(|_| complete) { + Some(t) if t.is_empty() => return Ok(0), + Some(t) => Some((t, removed.iter().map(|r| normalize(r)).collect::>())), + None => None, + }; + let hit = |text: &str| search.as_ref().is_none_or(|(terms, runs)| mentions(text, terms) || within(text, runs)); let mut n = 0; // The information dictionary, direct in the trailer or an object of its own. let info_obj = doc.trailer().get(b"Info").cloned(); if let Some(info) = info_obj.as_ref().and_then(|i| doc.resolve(i).as_dict().cloned()) { - let doomed: Vec> = info - .iter() - .filter(|(_, v)| { - doc.resolve(v).as_string().is_some_and(|s| { - let s = s.to_text(); - mentions(&s, &terms) || within(&s, &runs) - }) - }) - .map(|(k, _)| k.to_vec()) - .collect(); + let doomed: Vec> = + info.iter().filter(|(_, v)| doc.resolve(v).as_string().is_some_and(|s| hit(&s.to_text()))).map(|(k, _)| k.to_vec()).collect(); if !doomed.is_empty() { n += doomed.len(); let mut kept = info.clone(); @@ -223,7 +264,15 @@ pub(crate) fn scrub(doc: &mut Document, removed: &[String]) -> Result true, + Some(values) => { + let words = |(terms, _): &(Vec, Vec)| mentions(&values.join(" "), terms); + let part = |(_, runs): &(Vec, Vec)| values.iter().any(|v| within(v, runs)); + search.as_ref().is_none_or(|s| words(s) || part(s)) + } + }; + if doomed { doc.update_dict(holder, |d| { d.remove(b"Metadata"); })?; diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index eff0c3d525..58194c5cac 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -136,8 +136,10 @@ fn redacted_text_does_not_survive_in_info_or_xmp() { mark(&mut doc, 0, &[[55.0, 95.0, 90.0, 110.0]], ""); let r = apply(&mut doc, None).unwrap(); assert_eq!((r.glyphs, r.metadata), (7, 3), "{r:?}"); - let bytes = write_full(&doc, &SaveOptions::default()).unwrap(); + // Without object streams, so the byte search below can see the `/Info` dictionary. + let bytes = write_full(&doc, &SaveOptions { object_streams: false, ..SaveOptions::default() }).unwrap(); let has = |needle: &[u8]| bytes.windows(needle.len()).any(|w| w.eq_ignore_ascii_case(needle)); + assert!(has(b"Ada Lovelace"), "the kept /Info entry is visible to the search"); assert!(!has(b"bluefin") && !has(b"BlueFin"), "the redacted word survived in the metadata"); let saved = Document::open(Arc::new(bytes)).unwrap(); let info = saved.trailer().get(b"Info").and_then(|i| saved.resolve(i).as_dict().cloned()).unwrap(); @@ -171,6 +173,90 @@ fn an_info_value_inside_the_redacted_text_goes() { assert!(info.contains(b"Subject") && info.contains(b"Producer")); } +/// One page showing `content`, with `/Info` `info` and catalog XMP whose `rdf:RDF` holds +/// `xmp_body`. +fn with_metadata(content: &[u8], info: &str, xmp_body: &str) -> Document { + let xmp = format!( + r#"{xmp_body}"# + ); + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /Metadata 7 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 >> >> >>".to_vec(), + stream("", content), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + stream("/Type /Metadata /Subtype /XML", xmp.as_bytes()), + info.as_bytes().to_vec(), + ]; + Document::open(Arc::new(Pdf::new().objs(objs).trailer_entry("/Info 8 0 R").build())).unwrap() +} + +fn info_keys(doc: &Document) -> Vec { + let info = doc.trailer().get(b"Info").and_then(|i| doc.resolve(i).as_dict().cloned()).unwrap(); + info.iter().map(|(k, _)| String::from_utf8_lossy(k).into_owned()).collect() +} + +fn has_xmp(doc: &Document) -> bool { + doc.get(doc.root().unwrap()).as_dict().is_some_and(|c| c.contains(b"Metadata")) +} + +/// Letter-spaced text (character spacing wider than a word gap between plain glyphs) is still +/// one word, so `/Title (Bluefin)` goes when "B L U E F I N" is redacted (#2539 review). +#[test] +fn letter_spaced_redacted_text_still_matches_metadata() { + let body = r#"Bluefin"#; + let mut doc = with_metadata(b"BT /F1 10 Tf 3 Tc 10 100 Td (BLUEFIN) Tj ET", "<< /Title (Bluefin) /Producer (Test) >>", body); + mark(&mut doc, 0, &[[5.0, 95.0, 200.0, 110.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!(r.metadata, 2, "{r:?}"); + assert_eq!(info_keys(&doc), ["Producer"]); + assert!(!has_xmp(&doc)); +} + +/// An XMP value that is part of the removed text goes, as an `/Info` value does, though its +/// words are too short to be terms ("J. Li"). Values are compared as whole words, so "True" +/// doesn't go with "untrue", and namespace URIs and the `xpacket` wrapper aren't searched, so +/// redacting "Adobe … end" keeps a packet declared under `adobe:ns:meta/` and closed by +/// ``. +#[test] +fn xmp_values_inside_the_redacted_text_go_as_whole_words() { + let creator = r#"J. Li"#; + let mut doc = with_metadata(b"BT /F1 10 Tf 10 100 Td (Prepared by J. Li) Tj ET", "<< /Producer (Test) >>", creator); + mark(&mut doc, 0, &[[5.0, 95.0, 200.0, 110.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!(r.metadata, 1, "{r:?}"); + assert!(!has_xmp(&doc), "the packet named J. Li"); + + let trapped = r#""#; + let mut doc = with_metadata(b"BT /F1 10 Tf 10 100 Td (Adobe said untrue at the end) Tj ET", "<< /Subject (True) /Producer (Test) >>", trapped); + mark(&mut doc, 0, &[[5.0, 95.0, 200.0, 110.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!(r.metadata, 0, "{r:?}"); + assert!(has_xmp(&doc)); + assert_eq!(info_keys(&doc), ["Subject", "Producer"]); +} + +/// When the removed text was too much to collect, what wasn't collected can't be searched for: +/// every `/Info` string and the XMP go (fail-closed), even though the runs that were kept match +/// nothing. +#[test] +fn metadata_is_cleared_when_the_removed_text_was_cut_short() { + // More separate runs of one letter than are kept: each `A` is too far from the last to join. + let mut content = b"BT /F1 10 Tf 10 100 Td".to_vec(); + for _ in 0..50_001 { + content.extend_from_slice(b" (A) Tj 30 0 Td (A) Tj -30 0 Td"); + } + content.extend_from_slice(b" ET"); + let body = r#"Annual figures"#; + let mut doc = with_metadata(&content, "<< /Title (Annual figures) /Producer (Test) /Trapped /False >>", body); + mark(&mut doc, 0, &[[5.0, 95.0, 200.0, 110.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!(r.metadata, 3, "{r:?}"); + assert_eq!(info_keys(&doc), ["Trapped"], "strings go, the name stays"); + assert!(!has_xmp(&doc)); +} + /// A value shorter than the redacted sentence goes from XMP too, in a custom schema's element or /// attribute, as from `/Info` (the regression #2598 suggested keeping): the packets holding it are /// removed, one that doesn't stays. @@ -241,12 +327,29 @@ fn metadata_terms_and_xmp_text() { // Runs of two letters or digits or more are matched whole, and by their words of three or // more, ignoring case and spacing. let t = metadata::terms(&["John Smith".into(), "7".into(), "of".into(), "A-12".into()]); - assert_eq!(t, ["john smith", "john", "smith", "of", "a-12"]); - // XMP text is element content and attribute values, entities decoded; not names or comments. - let x = metadata::xml_text(r#"Résumé ]]>"#); - let words: Vec<&str> = x.split_whitespace().collect(); - assert_eq!(words, ["x", "&", "y", "Résumé", ""]); - assert!(!x.contains("title") && !x.contains("hidden")); + assert_eq!(t.as_deref(), Some(&["john smith", "john", "smith", "of", "a-12"].map(String::from)[..])); + // A run too long to search for whole can't be matched: the caller clears everything. + assert_eq!(metadata::terms(&["x".repeat(5000)]), None); + // XMP values: element content, attribute values, CDATA, comments and processing instructions, + // entities decoded; not names, and not namespace declarations. + let x = metadata::xml_values( + r#"Résumé ]]>"#, + ); + assert_eq!(x, ["audit James Bond", "x & y", " hidden ", "Résumé ", ""]); +} + +/// XMP is decoded from UTF-8 or UTF-16, the latter also without a byte-order mark at the start +/// (a packet's mark sits inside `Bond"; + let utf16 = |be: bool| text.encode_utf16().flat_map(|u| if be { u.to_be_bytes() } else { u.to_le_bytes() }).collect::>(); + assert_eq!(metadata::decode_xmp(text.as_bytes()).as_deref(), Some(text)); + assert_eq!(metadata::decode_xmp(&utf16(true)).as_deref(), Some(text)); + assert_eq!(metadata::decode_xmp(&utf16(false)).as_deref(), Some(text)); + assert_eq!(metadata::decode_xmp(&[[0xFF, 0xFE].as_slice(), &utf16(false)].concat()).as_deref(), Some(text)); + assert_eq!(metadata::decode_xmp(b"Bond \xE9"), None, "Latin-1 isn't searched lossily"); + assert_eq!(metadata::decode_xmp(&utf16(false)[1..]), None, "odd length"); } #[test] diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 2cf652df2a..4fb222a7e0 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -5768,8 +5768,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::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", "crates/redact/src/tests.rs::redacted_text_does_not_survive_in_info_or_xmp"] -notes = "confirmation; one undo step; next save is a full rewrite. Document information entries that contain the removed text, and the document's and pages' XMP streams that do, are removed (#2448; matched without regard to case or spacing, by the whole run of removed text and by its words of three letters or digits or more); other objects' XMP, bookmarks and attachments are not searched. 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" +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", "crates/redact/src/tests.rs::redacted_text_does_not_survive_in_info_or_xmp", "crates/redact/src/tests.rs::metadata_is_cleared_when_the_removed_text_was_cut_short"] +notes = "confirmation; one undo step; next save is a full rewrite. Document information entries that contain the removed text, and the document's and pages' XMP streams that do, are removed (#2448; matched without regard to case or spacing, by the whole run of removed text and by its words of three letters or digits or more, and a value wholly part of a run as whole words; fail-closed when the removed text was too much to collect or the XMP can't be decoded); other objects' XMP, bookmarks and attachments are not searched. 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 ca0cc61b2dcb9077a25e21805f207e01457514cc Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 17 Oct 2026 02:14:24 +0800 Subject: [PATCH 05/23] M8: exact xpacket target, spaced xmlns declarations, XMP decoded within its cap (#2448) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on #2539: - only a processing instruction whose target is exactly `xpacket` is skipped, so `` is searched like any other; - an attribute name is kept across whitespace before `=`, so `xmlns:x = "adobe:ns:meta/"` is still a declaration, not a value; - the XMP stream is decoded with decoded_within(MAX_XMP_BYTES), so a small stream that inflates hugely stops at the cap instead of after it. Co-Authored-By: Claude Opus 5.5 --- crates/redact/src/metadata.rs | 25 ++++++++++++++++++------- crates/redact/src/tests.rs | 6 ++++-- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/crates/redact/src/metadata.rs b/crates/redact/src/metadata.rs index b6ef956192..84500ecc81 100644 --- a/crates/redact/src/metadata.rs +++ b/crates/redact/src/metadata.rs @@ -155,7 +155,7 @@ pub(crate) fn xml_values(xml: &str) -> Vec { let end = body.find(stop).unwrap_or(body.len()); let text = body.get(..end).unwrap_or(""); // The packet wrapper's fixed `begin`/`id`/`end` would match words such as "end". - if !(start == " Vec { // Attribute values inside the tag, up to its closing `>` (which can't be inside quotes). let mut chars = tag.char_indices().skip(1); let mut end = tag.len(); + // The attribute name being read, and the last one read before whitespace: XML allows + // whitespace around `=` (`xmlns:x = "…"`). let mut name = String::new(); + let mut last = String::new(); while let Some((i, c)) = chars.next() { match c { '>' => { @@ -179,14 +182,24 @@ pub(crate) fn xml_values(xml: &str) -> Vec { } value.push(d); } - let declaration = name == "xmlns" || name.starts_with("xmlns:"); + let attr = if name.is_empty() { &last } else { &name }; + let declaration = attr == "xmlns" || attr.starts_with("xmlns:"); if !declaration { push(unescape(&value)); } name.clear(); + last.clear(); } '=' => {} - c if c.is_whitespace() || c == '/' => name.clear(), + '/' => { + name.clear(); + last.clear(); + } + c if c.is_whitespace() => { + if !name.is_empty() { + last = std::mem::take(&mut name); + } + } c => name.push(c), } } @@ -220,10 +233,8 @@ pub(crate) fn decode_xmp(bytes: &[u8]) -> Option { /// The text values of an XMP stream, or `None` when it can't be read. fn xmp_values(doc: &Document, o: &Object) -> Option> { let Object::Stream(s) = &*doc.resolve(o) else { return None }; - let bytes = s.decoded().ok()?; - if bytes.len() > MAX_XMP_BYTES { - return None; - } + // Decoding stops at the cap, so a small stream that inflates hugely costs no more memory. + let bytes = s.decoded_within(MAX_XMP_BYTES).ok()?; decode_xmp(&bytes).map(|t| xml_values(&t)) } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 58194c5cac..19f553332f 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -333,9 +333,11 @@ fn metadata_terms_and_xmp_text() { // XMP values: element content, attribute values, CDATA, comments and processing instructions, // entities decoded; not names, and not namespace declarations. let x = metadata::xml_values( - r#"Résumé ]]>"#, + r#"Résumé ]]>"#, ); - assert_eq!(x, ["audit James Bond", "x & y", " hidden ", "Résumé ", ""]); + // Only the `xpacket` wrapper itself is skipped, and a declaration is one with spaces around + // `=` too. + assert_eq!(x, ["audit James Bond", "xpacketAudit SECRET", "spaced", "x & y", " hidden ", "Résumé ", ""]); } /// XMP is decoded from UTF-8 or UTF-16, the latter also without a byte-order mark at the start From 56ce7661d688d70b66eb394c35febe4fdf282ef8 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 19 Oct 2026 00:44:19 +0800 Subject: [PATCH 06/23] M8: one search budget per metadata scrub, bounded entity scan, fail closed for unread forms (#2448) Review by Losminek on #2539: - The work cap was checked per value, so very many small /Info or XMP values could add up without bound. One budget now covers the whole scrub; past it every further entry or packet counts as matching (fail-closed). Identical removed runs are searched once. - unescape looked for ';' through the rest of the text before checking the 12-byte entity limit, so a packet of bare '&'s was quadratic. It now looks only at the next 13 characters (200,000 '&': 943 ms -> 5 ms). - A form XObject removed whole past the depth limit, or when its stream doesn't decode, never had its text read; both paths now set removed_cut so the metadata check clears everything instead of searching incomplete text. Co-Authored-By: Claude Opus 5.5 --- crates/redact/src/interp.rs | 3 ++ crates/redact/src/metadata.rs | 60 ++++++++++++++++++++++++++++------- crates/redact/src/tests.rs | 57 +++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 11 deletions(-) diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index 6781a4060c..2dcafb339f 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -861,6 +861,8 @@ fn xobject( return None; } scope.report.forms_removed += 1; + // Its text was never read, so the metadata check can't know what went with it. + scope.removed_cut = true; out.replaced.push(name); out.doomed.push(r); return Some(None); @@ -887,6 +889,7 @@ fn xobject( } Err(_) if scope.mode == Mode::Apply => { scope.report.forms_removed += 1; + scope.removed_cut = true; out.replaced.push(name); out.doomed.push(r); return Some(None); diff --git a/crates/redact/src/metadata.rs b/crates/redact/src/metadata.rs index 84500ecc81..c50401ef34 100644 --- a/crates/redact/src/metadata.rs +++ b/crates/redact/src/metadata.rs @@ -26,8 +26,9 @@ const MAX_TERM_CHARS: usize = 4096; const MAX_TERMS: usize = 20_000; /// XMP packets larger than this, decoded, are removed without reading them. const MAX_XMP_BYTES: usize = 16 * 1024 * 1024; -/// Text whose length times the number of terms exceeds this is treated as matching rather than -/// searched (fail-closed), so a page of removed text against a large packet stays quick. +/// The searching (text length times terms, or value length times removed text) one scrub may +/// do in all; past it every entry and packet counts as matching (fail-closed), so a page of +/// removed text against a large packet, or against very many small values, stays quick. const MAX_WORK: usize = 1 << 28; /// Lower-case `s` with every run of whitespace made one space, trimmed. @@ -65,16 +66,41 @@ pub(crate) fn terms(removed: &[String]) -> Option> { Some(out) } -fn mentions(text: &str, terms: &[String]) -> bool { +/// The searching one scrub may do, across every `/Info` entry and XMP value: past it, each +/// further question is answered "yes" (fail-closed), so a packet of many small values can't add +/// up to more than one large one. +struct Work(std::cell::Cell); + +impl Work { + fn new(limit: usize) -> Self { + Self(std::cell::Cell::new(limit)) + } + + /// Take `cost` from what is left; `false` (and nothing left) when it doesn't fit. + fn spend(&self, cost: usize) -> bool { + match self.0.get().checked_sub(cost) { + Some(left) => { + self.0.set(left); + true + } + None => { + self.0.set(0); + false + } + } + } +} + +fn mentions(text: &str, terms: &[String], work: &Work) -> bool { let text = normalize(text); - text.len().saturating_mul(terms.len()) > MAX_WORK || terms.iter().any(|t| text.contains(t.as_str())) + !work.spend(text.len().saturating_mul(terms.len())) || terms.iter().any(|t| text.contains(t.as_str())) } /// Is `value` itself part of one of the removed `runs` (normalized), as whole words? It needs /// three letters or digits, so an entry such as "by" doesn't go with every sentence that /// contains it, and word boundaries, so an XMP value "True" doesn't go with "untrue". Too much /// text to search counts as part (fail-closed). -fn within(value: &str, runs: &[String]) -> bool { +fn within(value: &str, runs: &[String], work: &Work) -> bool { let value = normalize(value); if !significant(&value, 3) { return false; @@ -88,7 +114,7 @@ fn within(value: &str, runs: &[String]) -> bool { !(word(before) && value.starts_with(char::is_alphanumeric)) && !(word(after) && value.ends_with(char::is_alphanumeric)) }) }; - value.len().saturating_mul(total) > MAX_WORK || runs.iter().any(whole) + !work.spend(value.len().saturating_mul(total)) || runs.iter().any(whole) } /// Decode the five predefined XML entities and character references; anything else stays. @@ -98,7 +124,8 @@ fn unescape(s: &str) -> String { while let Some(at) = rest.find('&') { out.push_str(rest.get(..at).unwrap_or("")); let tail = rest.get(at..).unwrap_or(""); - let end = tail.find(';').filter(|&e| e <= 12); + // An entity ends within 12 bytes; looking further made a text of bare `&`s quadratic. + let end = tail.char_indices().take(13).find(|&(_, c)| c == ';').map(|(e, _)| e); let entity = end.and_then(|e| tail.get(1..e)); let decoded = entity.and_then(|e| match e { "amp" => Some('&'), @@ -243,13 +270,24 @@ fn xmp_values(doc: &Document, o: &Object) -> Option> { /// removed text was cut short; then, as when the terms themselves exceed their limits, every /// `/Info` string and every XMP stream goes. Returns how many entries and streams went. pub(crate) fn scrub(doc: &mut Document, removed: &[String], complete: bool) -> Result { + scrub_within(doc, removed, complete, MAX_WORK) +} + +/// [`scrub`] with the searching it may do given, so tests can run out of it. +pub(crate) fn scrub_within(doc: &mut Document, removed: &[String], complete: bool, limit: usize) -> Result { // `None` matches everything (fail-closed). let search = match terms(removed).filter(|_| complete) { Some(t) if t.is_empty() => return Ok(0), - Some(t) => Some((t, removed.iter().map(|r| normalize(r)).collect::>())), + Some(t) => { + // The same run removed many times is searched for once. + let mut seen = HashSet::new(); + let runs: Vec = removed.iter().map(|r| normalize(r)).filter(|r| seen.insert(r.clone())).collect(); + Some((t, runs)) + } None => None, }; - let hit = |text: &str| search.as_ref().is_none_or(|(terms, runs)| mentions(text, terms) || within(text, runs)); + let work = Work::new(limit); + let hit = |text: &str| search.as_ref().is_none_or(|(terms, runs)| mentions(text, terms, &work) || within(text, runs, &work)); let mut n = 0; // The information dictionary, direct in the trailer or an object of its own. @@ -278,8 +316,8 @@ pub(crate) fn scrub(doc: &mut Document, removed: &[String], complete: bool) -> R let doomed = match xmp_values(doc, &xmp) { None => true, Some(values) => { - let words = |(terms, _): &(Vec, Vec)| mentions(&values.join(" "), terms); - let part = |(_, runs): &(Vec, Vec)| values.iter().any(|v| within(v, runs)); + let words = |(terms, _): &(Vec, Vec)| mentions(&values.join(" "), terms, &work); + let part = |(_, runs): &(Vec, Vec)| values.iter().any(|v| within(v, runs, &work)); search.as_ref().is_none_or(|s| words(s) || part(s)) } }; diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 19f553332f..270434aa95 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -257,6 +257,63 @@ fn metadata_is_cleared_when_the_removed_text_was_cut_short() { assert!(!has_xmp(&doc)); } +/// #2539 review: the searching a scrub may do is one budget for all of it, not a check per +/// value: very many small `/Info` values each fit, but together they don't, and past the budget +/// the rest counts as matching (fail-closed). With room to search, nothing that doesn't mention +/// the removed text goes. +#[test] +fn metadata_search_has_one_budget_for_every_value() { + let entries: String = (0..50).map(|i| format!("/K{i} (an unrelated value number {i:02} in the list) ")).collect(); + let info = format!("<< {entries}/Producer (Test) >>"); + let fresh = || with_metadata(b"BT /F1 10 Tf 10 100 Td (Bluefin) Tj ET", &info, ""); + let mut doc = fresh(); + assert_eq!(metadata::scrub_within(&mut doc, &["Bluefin".into()], true, 1 << 28).unwrap(), 0, "room to search: nothing goes"); + let mut doc = fresh(); + // Each value costs about 40; fifty of them don't fit in 1000. + let n = metadata::scrub_within(&mut doc, &["Bluefin".into()], true, 1000).unwrap(); + assert!(n > 20, "past the budget the rest goes ({n} went)"); +} + +/// #2539 review: an entity is looked for only in the 12 bytes after its `&`, so a packet of bare +/// `&` characters is read in linear time, and decoding is unchanged. +#[test] +fn xml_entities_are_found_near_their_ampersand_only() { + assert_eq!(metadata::xml_values("Résumé & AA �"), ["Résumé & AA �"]); + let bare = "&".repeat(200_000); + let started = std::time::Instant::now(); + let values = metadata::xml_values(&format!("{bare}")); + assert_eq!(values.concat().len(), bare.len()); + eprintln!("200000 bare ampersands: {:?}", started.elapsed()); +} + +/// #2539 review: Apply removes a form XObject whole when it is nested past the depth limit (or +/// its stream can't be decoded), without reading its text, so what it held is unknown: the +/// metadata check fails closed. +#[test] +fn a_form_removed_unread_clears_the_metadata() { + // Forms 5..=20 each draw the next; the last one shows text. The page draws form 5. + let mut objs: Vec> = 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 << /XObject << /Fm 5 0 R >> >> >>".to_vec(), + stream("", b"q /Fm Do Q"), + ]; + for n in 5..=20 { + objs.push(stream( + &format!("/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources << /XObject << /Fm {} 0 R >> >>", n + 1), + b"q /Fm Do Q", + )); + } + objs.push(stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300]", b"0 g 10 10 50 50 re f")); + objs.push(b"<< /Title (Annual figures) /Producer (Test) >>".to_vec()); + let info = objs.len(); + let mut doc = Document::open(Arc::new(Pdf::new().objs(objs).trailer_entry(&format!("/Info {info} 0 R")).build())).unwrap(); + mark(&mut doc, 0, &[[0.0, 0.0, 300.0, 300.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert!(r.forms_removed >= 1, "{r:?}"); + assert!(!info_keys(&doc).contains(&"Title".to_string()), "the deep form's text was never read: {:?}", info_keys(&doc)); +} + /// A value shorter than the redacted sentence goes from XMP too, in a custom schema's element or /// attribute, as from `/Info` (the regression #2598 suggested keeping): the packets holding it are /// removed, one that doesn't stays. From d64a363f460c550a5803bbd4277d96f06b71316f Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 19 Oct 2026 01:35:20 +0800 Subject: [PATCH 07/23] M8: metadata search decodes character references with any number of leading zeros (#2448) Review follow-up: `BLUEFIN` hid BLUEFIN from the 13-byte scan. Co-Authored-By: Claude Opus 5.5 --- crates/redact/src/metadata.rs | 21 +++++++++++++++++---- crates/redact/src/tests.rs | 7 ++++++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/crates/redact/src/metadata.rs b/crates/redact/src/metadata.rs index c50401ef34..6fced3ad91 100644 --- a/crates/redact/src/metadata.rs +++ b/crates/redact/src/metadata.rs @@ -124,8 +124,19 @@ fn unescape(s: &str) -> String { while let Some(at) = rest.find('&') { out.push_str(rest.get(..at).unwrap_or("")); let tail = rest.get(at..).unwrap_or(""); - // An entity ends within 12 bytes; looking further made a text of bare `&`s quadratic. - let end = tail.char_indices().take(13).find(|&(_, c)| c == ';').map(|(e, _)| e); + // A named entity ends within 12 bytes; looking further made a text of bare `&`s + // quadratic. A character reference may carry any number of leading zeros, so its digits + // are read to their end: each digit belongs to one `&` only, which keeps it linear. + let end = match tail.strip_prefix("&#") { + Some(digits) => { + let hex = digits.starts_with(['x', 'X']); + let from = 2 + usize::from(hex); + let run = tail.get(from..).unwrap_or("").bytes().take_while(|b| if hex { b.is_ascii_hexdigit() } else { b.is_ascii_digit() }).count(); + let e = from + run; + (tail.as_bytes().get(e) == Some(&b';')).then_some(e) + } + None => tail.char_indices().take(13).find(|&(_, c)| c == ';').map(|(e, _)| e), + }; let entity = end.and_then(|e| tail.get(1..e)); let decoded = entity.and_then(|e| match e { "amp" => Some('&'), @@ -135,8 +146,10 @@ fn unescape(s: &str) -> String { "apos" => Some('\''), _ => { let n = match e.strip_prefix("#x").or_else(|| e.strip_prefix("#X")) { - Some(h) => u32::from_str_radix(h, 16).ok(), - None => e.strip_prefix('#').and_then(|d| d.parse().ok()), + // Leading zeros dropped, so they can't overflow the number; `�` isn't a + // character XML allows, and stays as written. + Some(h) => u32::from_str_radix(h.trim_start_matches('0'), 16).ok(), + None => e.strip_prefix('#').and_then(|d| d.trim_start_matches('0').parse().ok()), }; n.and_then(char::from_u32) } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 270434aa95..9862e9e245 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -279,11 +279,16 @@ fn metadata_search_has_one_budget_for_every_value() { #[test] fn xml_entities_are_found_near_their_ampersand_only() { assert_eq!(metadata::xml_values("Résumé & AA �"), ["Résumé & AA �"]); + // #2539 review: a character reference with many leading zeros still decodes, so it can't + // hide a redacted word from the search; a reference that never ends stays as written. + assert_eq!(metadata::xml_values("BLUEFIN BLUEFIN �"), ["BLUEFIN BLUEFIN �"]); let bare = "&".repeat(200_000); let started = std::time::Instant::now(); let values = metadata::xml_values(&format!("{bare}")); assert_eq!(values.concat().len(), bare.len()); - eprintln!("200000 bare ampersands: {:?}", started.elapsed()); + let zeros = format!("&#{}", "0".repeat(10)).repeat(20_000); + assert_eq!(metadata::xml_values(&format!("{zeros}")).concat().len(), zeros.len()); + eprintln!("200000 bare ampersands, 20000 unended references: {:?}", started.elapsed()); } /// #2539 review: Apply removes a form XObject whole when it is nested past the depth limit (or From 5b2048ef256bf3dd95ac41f8a1d7d2404e80c381 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 19 Oct 2026 11:37:27 +0800 Subject: [PATCH 08/23] M5: a removed glyph whose text isn't known makes the metadata check fail closed (#2448) Review on #2539: a removed glyph with no known text (no /ToUnicode, a glyph name outside the list) went in as an empty string and nothing marked the collection as incomplete, so when every removed glyph was like that the terms were empty and /Info and XMP were left untouched. It now counts as text the check couldn't keep, and every /Info string and XMP stream goes. Co-Authored-By: Claude Opus 5.5 --- crates/redact/src/interp.rs | 9 ++++++++- crates/redact/src/tests.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index 2dcafb339f..b0029ac912 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -739,7 +739,14 @@ fn show(scope: &mut Scope<'_>, gs: &Gs, tm: &mut Matrix, items: &[Object]) -> (V let end = trm.apply(spaced, 0.0); let top = trm.apply(0.0, 1.0); let height = (top.0 - origin.0).hypot(top.1 - origin.1); - scope.removed(f.text_of(code).as_deref().unwrap_or(""), origin, end, height); + let text = f.text_of(code); + // A glyph whose text isn't known could stand for anything in the + // metadata, so the metadata check can't rely on the text it has + // (review on #2539): it fails closed, as for text it couldn't keep. + if text.is_none() { + scope.removed_cut = true; + } + scope.removed(text.as_deref().unwrap_or(""), origin, end, height); } else { scope.flush(); } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 9862e9e245..fef02a85dd 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -173,6 +173,38 @@ fn an_info_value_inside_the_redacted_text_goes() { assert!(info.contains(b"Subject") && info.contains(b"Producer")); } +/// Removed glyphs whose text isn't known (no `/ToUnicode`, glyph names outside the glyph list) +/// leave nothing to search the metadata for, so every `/Info` string goes, as when the collected +/// text was cut short (review on #2539). Glyphs whose text is known keep the targeted check. +#[test] +fn removed_glyphs_with_no_known_text_clear_the_metadata() { + let page = |codes: &[u8], font: &str| -> Document { + let content = [&b"BT /F1 10 Tf 10 100 Td ("[..], codes, b") Tj ET"].concat(); + let objs: Vec> = 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 << /Font << /F1 5 0 R >> >> >>".to_vec(), + stream("", &content), + font.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + b"<< /Title (Project Nightjar) /Producer (Test) >>".to_vec(), + ]; + Document::open(Arc::new(Pdf::new().objs(objs).trailer_entry("/Info 7 0 R").build())).unwrap() + }; + let unnamed = FONT.replace(" /Widths", " /Encoding << /Differences [65 /g17 /g18 /g19] >> /Widths"); + let mut doc = page(b"ABC", &unnamed); + mark(&mut doc, 0, &[[5.0, 95.0, 200.0, 110.0]], ""); + let r = apply(&mut doc, None).unwrap(); + let info = doc.trailer().get(b"Info").and_then(|i| doc.resolve(i).as_dict().cloned()).unwrap(); + assert!(!info.contains(b"Title") && !info.contains(b"Producer"), "fail closed: {r:?}"); + // Control: the same glyphs with known text don't touch unrelated metadata. + let mut doc = page(b"ABC", FONT); + mark(&mut doc, 0, &[[5.0, 95.0, 200.0, 110.0]], ""); + apply(&mut doc, None).unwrap(); + let info = doc.trailer().get(b"Info").and_then(|i| doc.resolve(i).as_dict().cloned()).unwrap(); + assert!(info.contains(b"Title") && info.contains(b"Producer")); +} + /// One page showing `content`, with `/Info` `info` and catalog XMP whose `rdf:RDF` holds /// `xmp_body`. fn with_metadata(content: &[u8], info: &str, xmp_body: &str) -> Document { From aed4599ed13bd20f52554cdb452c05814ca118c9 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sun, 25 Oct 2026 02:37:36 +0800 Subject: [PATCH 09/23] M8: the unread-form metadata test nests past main's form depth limit (#2448) main raised the redaction pass's form depth limit from 12 to 50 (#3645), so the 16-deep chain was walked and the test's form was no longer removed unread. The chain is now built from MAX_DEPTH, one level past it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_0175FWj4xFhMhSH3UpMU8DSC --- crates/redact/src/tests.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index fef02a85dd..e873a03b0e 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -328,14 +328,16 @@ fn xml_entities_are_found_near_their_ampersand_only() { /// metadata check fails closed. #[test] fn a_form_removed_unread_clears_the_metadata() { - // Forms 5..=20 each draw the next; the last one shows text. The page draws form 5. + // Forms 5, 6, ... each draw the next, one more than the depth limit deep, and the last one + // paints. The page draws form 5. + let last = 5 + crate::interp::MAX_DEPTH; let mut objs: Vec> = 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 << /XObject << /Fm 5 0 R >> >> >>".to_vec(), stream("", b"q /Fm Do Q"), ]; - for n in 5..=20 { + for n in 5..=last { objs.push(stream( &format!("/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources << /XObject << /Fm {} 0 R >> >>", n + 1), b"q /Fm Do Q", From 409477067ac3cec662746e696fcf47208b6321ef Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 31 Oct 2026 08:05:51 +0800 Subject: [PATCH 10/23] M8: the redact README lists every fail-closed trigger and the metadata it doesn't scrub (#2448) Review on #2539 (nitpick): the README named only the collection limit and unreadable XMP as fail-closed cases. It now lists all of them (the run and run-length limits, a removed glyph with unknown text, a form XObject removed unread past the depth limit or unreadable, the term limits, the search work limit, oversized or undecodable XMP) and the metadata the check leaves to Sanitize: XMP on objects other than the catalog and pages, bookmark titles, and attachments. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/redact/README.md | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/crates/redact/README.md b/crates/redact/README.md index eb3bce469e..0db0e18c49 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -34,14 +34,24 @@ clear_marks(&mut doc, None)?; // remove marks without app part of it as whole words (three letters or digits at least), and the catalog's and pages' XMP streams whose text (element content, attribute values other than `xmlns` declarations, comments and processing instructions, entities decoded) contains it or has a value wholly - part of it, removed whole, together with any PDF/A identification in that packet. XMP that - isn't valid UTF-8 or UTF-16 goes too, and when the removed text was too much to collect - every `/Info` string and every XMP stream goes (fail-closed). The removed glyphs' text is - collected in runs by position (character and word spacing included, so letter-spaced text - stays one word), and each run (two or more letters or digits) and each of its words of three - or more are matched without regard to case or spacing. A value that shares only words of one - or two letters with the removed text, and isn't wholly part of it, stays: Sanitize's - metadata category removes the rest. `Report::metadata` counts what went. + part of it, removed whole, together with any PDF/A identification in that packet. The removed + glyphs' text is collected in runs by position (character and word spacing included, so + letter-spaced text stays one word), and each run (two or more letters or digits) and each of + its words of three or more are matched without regard to case or spacing. A value that shares + only words of one or two letters with the removed text, and isn't wholly part of it, stays: + Sanitize's metadata category removes the rest. `Report::metadata` counts what went. + - **Fail-closed.** When the check can't know what the removed text was, every `/Info` + string and every catalog and page XMP stream goes: more than 100,000 runs or a run over + 64 KiB removed (the collection limit); a removed glyph whose text isn't known (no + `/ToUnicode` or encoding gives it); a form XObject removed without being read, because it + is nested past the depth limit (`MAX_DEPTH`, 50) or its content can't be read; or more + than 20,000 terms, or a run longer than 4,096 characters, to search for. Once the + search's work limit (2^28 steps) runs out, every entry and packet not yet checked goes + too. An XMP packet that isn't valid UTF-8 or UTF-16, can't be decoded, or decodes to + more than 16 MiB goes without being read. + - **Not scrubbed** (Sanitize's categories cover them): XMP streams on anything but the + catalog and the pages (images, fonts, form XObjects, annotations and other objects), + bookmark titles, and attachments (embedded files, their names and descriptions). A verification pass re-reads each redacted page and fails the operation if any glyph or inline image is still under an area. It checks every content stream the page had, whatever its From ca4ddcfb90c1ac4ced6c8ac8ca620443a765c9e9 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sun, 1 Nov 2026 17:46:19 +0800 Subject: [PATCH 11/23] M8: a test for each way the metadata check fails closed; XMP is decoded strictly (#2448) End-to-end tests for a removed run over 64 KiB, more than 20,000 terms, a run over 4,096 characters, and XMP that is corrupt, cut short, behind two filters, over 16 MiB or in another encoding. Each was seen failing with its guard removed or loosened. The corrupt and cut-short cases showed that the tolerant decoder returned the part of the packet before the damage, which was then searched as if it were the whole packet. XMP is now decoded strictly, and a packet behind more than one filter goes unread. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/redact/README.md | 5 +- crates/redact/src/metadata.rs | 15 +- crates/redact/src/tests.rs | 1 + .../redact/src/tests/metadata_fail_closed.rs | 157 ++++++++++++++++++ 4 files changed, 172 insertions(+), 6 deletions(-) create mode 100644 crates/redact/src/tests/metadata_fail_closed.rs diff --git a/crates/redact/README.md b/crates/redact/README.md index 0db0e18c49..ec73af0ae3 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -47,8 +47,9 @@ clear_marks(&mut doc, None)?; // remove marks without app is nested past the depth limit (`MAX_DEPTH`, 50) or its content can't be read; or more than 20,000 terms, or a run longer than 4,096 characters, to search for. Once the search's work limit (2^28 steps) runs out, every entry and packet not yet checked goes - too. An XMP packet that isn't valid UTF-8 or UTF-16, can't be decoded, or decodes to - more than 16 MiB goes without being read. + too. An XMP packet that isn't valid UTF-8 or UTF-16, can't be decoded strictly (damaged or + cut-short filter data, or more than one filter), or decodes to more than + 16 MiB goes without being read. - **Not scrubbed** (Sanitize's categories cover them): XMP streams on anything but the catalog and the pages (images, fonts, form XObjects, annotations and other objects), bookmark titles, and attachments (embedded files, their names and descriptions). diff --git a/crates/redact/src/metadata.rs b/crates/redact/src/metadata.rs index 6fced3ad91..8946cf46f6 100644 --- a/crates/redact/src/metadata.rs +++ b/crates/redact/src/metadata.rs @@ -12,8 +12,9 @@ //! removed run, is removed whole: editing the XML in place could leave it malformed or keep the //! term in another spelling. A PDF/A identification in that packet goes with it. //! -//! Fail-closed: XMP that isn't UTF-8 or UTF-16 goes, text too large to search against every term -//! counts as matching, and when the removed text itself was cut short (too many runs or terms, +//! Fail-closed: XMP that isn't UTF-8 or UTF-16, or whose filter data is damaged or behind more than +//! one filter (it is decoded strictly, so nothing after damage goes unsearched), goes; text too +//! large to search against every term counts as matching, and when the removed text itself was cut short (too many runs or terms, //! or a run too long to keep) every `/Info` string and every XMP stream goes, since what was //! never collected can't be searched for. Terms are matched without regard to case or spacing. @@ -273,8 +274,14 @@ pub(crate) fn decode_xmp(bytes: &[u8]) -> Option { /// The text values of an XMP stream, or `None` when it can't be read. fn xmp_values(doc: &Document, o: &Object) -> Option> { let Object::Stream(s) = &*doc.resolve(o) else { return None }; - // Decoding stops at the cap, so a small stream that inflates hugely costs no more memory. - let bytes = s.decoded_within(MAX_XMP_BYTES).ok()?; + // Decoded strictly: a tolerant decode returns whatever came before the damage, and the part + // after it would go unsearched. A chain of more than one filter has no strict decoder here, + // so it isn't read either (fail-closed; XMP is rarely filtered at all). Decoding stops at the + // cap, so a small stream that inflates hugely costs no more memory. + if s.filters().len() > 1 { + return None; + } + let bytes = s.decoded_first_filter_strict(MAX_XMP_BYTES).ok()?; decode_xmp(&bytes).map(|t| xml_values(&t)) } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index e873a03b0e..b810043279 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -3040,6 +3040,7 @@ fn unused_descriptions_in_shared_or_inherited_resources_are_retired() { mod descriptions; mod inline_images; mod limits; +mod metadata_fail_closed; mod property_retirement; mod surviving_references; mod text_state; diff --git a/crates/redact/src/tests/metadata_fail_closed.rs b/crates/redact/src/tests/metadata_fail_closed.rs new file mode 100644 index 0000000000..0a958ccd88 --- /dev/null +++ b/crates/redact/src/tests/metadata_fail_closed.rs @@ -0,0 +1,157 @@ +//! #2539 review: each way the metadata check fails closed, end to end. When the removed text +//! can't be known in full, every `/Info` string and both the catalog's and the page's XMP go; +//! when one XMP packet can't be read, that packet goes and the rest of the metadata is checked +//! as usual. + +use super::*; + +const CLEAN_XMP: &str = r#"Annual figures"#; + +/// One page showing `content`, with `/Info` (two strings and a name), catalog XMP (object 7) +/// and page XMP (object 8). None of the metadata mentions anything the tests remove. +fn page_with_metadata(content: &[u8], catalog_xmp: Vec) -> Document { + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /Metadata 7 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 >> >> /Metadata 8 0 R >>".to_vec(), + stream("", content), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + catalog_xmp, + stream("/Type /Metadata /Subtype /XML", CLEAN_XMP.as_bytes()), + b"<< /Title (Annual figures) /Producer (Test) /Trapped /False >>".to_vec(), + ]; + Document::open(Arc::new(Pdf::new().objs(objs).trailer_entry("/Info 9 0 R").build())).unwrap() +} + +fn clean_xmp() -> Vec { + stream("/Type /Metadata /Subtype /XML", CLEAN_XMP.as_bytes()) +} + +fn catalog_has_xmp(doc: &Document) -> bool { + doc.get(doc.root().unwrap()).as_dict().is_some_and(|c| c.contains(b"Metadata")) +} + +fn page_has_xmp(doc: &Document) -> bool { + pdfcraft_model::pages(doc)[0].dict.contains(b"Metadata") +} + +/// Mark the line at y 100 across the page and apply. +fn redact_line(doc: &mut Document) -> Report { + mark(doc, 0, &[[0.0, 95.0, 300.0, 110.0]], ""); + apply(doc, None).unwrap() +} + +/// Every `/Info` string and both XMP streams went, though none of them mentions what was removed. +fn assert_cleared(doc: &Document, r: &Report) { + assert_eq!(r.metadata, 4, "{r:?}"); + assert_eq!(info_keys(doc), ["Trapped"], "strings go, the name stays"); + assert!(!catalog_has_xmp(doc) && !page_has_xmp(doc), "both XMP streams go"); +} + +/// Control for the tests below: removing a short word that the metadata doesn't mention keeps +/// all of it. +#[test] +fn metadata_that_does_not_mention_the_removed_text_stays() { + let mut doc = page_with_metadata(b"BT /F1 10 Tf 10 100 Td (Bluefin) Tj ET", clean_xmp()); + let r = redact_line(&mut doc); + assert_eq!(r.metadata, 0, "{r:?}"); + assert_eq!(info_keys(&doc), ["Title", "Producer", "Trapped"]); + assert!(catalog_has_xmp(&doc) && page_has_xmp(&doc)); +} + +/// A single removed run longer than the collection keeps (64 KiB): the part past the limit is +/// never seen. The run is one letter, 70,000 spaces (one space once normalized) and another +/// letter, so what would be collected without the limit is short enough to search, and only +/// the limit itself clears the metadata. +#[test] +fn a_removed_run_over_64_kib_clears_the_metadata() { + // At 0.004 pt each glyph advances 0.002 pt, so all 70,002 fit in 141 pt of one line. + let text = format!("A{}B", " ".repeat(70_000)); + let content = format!("BT /F1 0.004 Tf 10 100 Td ({text}) Tj ET"); + let mut doc = page_with_metadata(content.as_bytes(), clean_xmp()); + let r = redact_line(&mut doc); + assert_eq!(r.glyphs, 70_002, "{r:?}"); + assert_cleared(&doc, &r); +} + +/// More than 20,000 distinct terms: each removed run is one word, far enough from the last to +/// start a run of its own, and each word is different. +#[test] +fn more_than_20000_terms_clear_the_metadata() { + let mut content = b"BT /F1 10 Tf 10 100 Td".to_vec(); + // Words alternate between x 10 and x 70; each is 30 pt wide, so the gap between two is + // 30 pt, more than a run spans. + for i in 0..20_001u32 { + let x = if i % 2 == 0 { 60 } else { -60 }; + content.extend_from_slice(format!(" (q{i:05}) Tj {x} 0 Td").as_bytes()); + } + content.extend_from_slice(b" ET"); + let mut doc = page_with_metadata(&content, clean_xmp()); + let r = redact_line(&mut doc); + assert_eq!(r.glyphs, 20_001 * 6, "{r:?}"); + assert_cleared(&doc, &r); +} + +/// A removed run longer than 4,096 characters can't be searched for whole. +#[test] +fn a_removed_run_over_4096_characters_clears_the_metadata() { + // 4,100 letters at 0.05 pt advance 0.025 pt each: 103 pt in all, one run, 4 KiB. + let content = format!("BT /F1 0.05 Tf 10 100 Td ({}) Tj ET", "Z".repeat(4_100)); + let mut doc = page_with_metadata(content.as_bytes(), clean_xmp()); + let r = redact_line(&mut doc); + assert_eq!(r.glyphs, 4_100, "{r:?}"); + assert_cleared(&doc, &r); +} + +/// The catalog's packet can't be read: that packet goes, unread, while the page's packet and +/// `/Info`, which don't mention the removed word, stay. +fn assert_only_the_catalog_packet_went(catalog_xmp: Vec) { + let mut doc = page_with_metadata(b"BT /F1 10 Tf 10 100 Td (Bluefin) Tj ET", catalog_xmp); + let r = redact_line(&mut doc); + assert_eq!(r.metadata, 1, "{r:?}"); + assert!(!catalog_has_xmp(&doc), "the packet that couldn't be read goes"); + assert!(page_has_xmp(&doc), "the readable packet is checked, and stays"); + assert_eq!(info_keys(&doc), ["Title", "Producer", "Trapped"]); +} + +/// XMP whose Flate data is corrupt can't be decoded, so it can't be searched. +#[test] +fn an_xmp_stream_whose_filter_data_is_corrupt_goes() { + assert_only_the_catalog_packet_went(stream("/Type /Metadata /Subtype /XML /Filter /FlateDecode", b"this is not deflate data at all")); +} + +/// A valid Flate stream cut off half way decodes, leniently, to the first part of the packet; +/// the rest was never seen, so the packet isn't searched but goes. +#[test] +fn an_xmp_stream_whose_filter_data_is_cut_short_goes() { + let full = Stream::flate(Dict::new(), CLEAN_XMP.as_bytes()); + let cut = full.raw.get(..full.raw.len() / 2).unwrap(); + assert_only_the_catalog_packet_went(stream("/Type /Metadata /Subtype /XML /Filter /FlateDecode", cut)); +} + +/// XMP that decodes to more than 16 MiB isn't read at all. +#[test] +fn an_xmp_stream_over_16_mib_goes() { + // A valid packet, padded with whitespace past the limit. + let padded = CLEAN_XMP.replace("", &format!("{}", " ".repeat(16 * 1024 * 1024))); + assert_only_the_catalog_packet_went(stream("/Type /Metadata /Subtype /XML", padded.as_bytes())); +} + +/// XMP that is neither UTF-8 nor UTF-16 (here Latin-1) isn't searched lossily. +#[test] +fn an_xmp_stream_in_another_encoding_goes() { + let latin1 = CLEAN_XMP.replace("Annual figures", "Ann\u{e9}e").replace('\u{e9}', "\u{0}"); + let bytes: Vec = latin1.bytes().map(|b| if b == 0 { 0xE9 } else { b }).collect(); + assert!(std::str::from_utf8(&bytes).is_err()); + assert_only_the_catalog_packet_went(stream("/Type /Metadata /Subtype /XML", &bytes)); +} + +/// XMP behind two filters has no strict decoder here, so it isn't read either. The first filter +/// alone decodes to valid ASCII (the packet in hex), which must not be searched as the packet. +#[test] +fn an_xmp_stream_behind_two_filters_goes() { + let hex: String = CLEAN_XMP.bytes().map(|b| format!("{b:02x}")).chain(std::iter::once(">".to_string())).collect(); + let Stream { raw, .. } = Stream::flate(Dict::new(), hex.as_bytes()); + assert_only_the_catalog_packet_went(stream("/Type /Metadata /Subtype /XML /Filter [/FlateDecode /ASCIIHexDecode]", &raw)); +} From 81d629d1ff41943031db1c0270c6426d2c6741a4 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sun, 1 Nov 2026 17:51:37 +0800 Subject: [PATCH 12/23] M8: applying redactions counts the bookmarks, attachments, comments and XMP that still hold the removed text (#2448) Report::still_holding (Elsewhere) counts the places apply leaves as they are that still hold the removed text: bookmark titles, attachment names, file names and descriptions, the text and authors of the comments that remain, and XMP on objects other than the catalog and the pages. Nothing is changed; Remove hidden information removes them. The text is matched as the metadata check matches it (one Search type now serves both), with its own work limit. When the removed text couldn't be searched for, every place holding any text counts and `unsearched` says so. The walks read each outline item and name-tree node once, within caps. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/redact/README.md | 16 +- crates/redact/src/elsewhere.rs | 219 +++++++++++++++++++++++++++ crates/redact/src/lib.rs | 11 ++ crates/redact/src/metadata.rs | 113 ++++++++++---- crates/redact/src/sanitize.rs | 41 +++++ crates/redact/src/tests.rs | 1 + crates/redact/src/tests/elsewhere.rs | 113 ++++++++++++++ 7 files changed, 484 insertions(+), 30 deletions(-) create mode 100644 crates/redact/src/elsewhere.rs create mode 100644 crates/redact/src/tests/elsewhere.rs diff --git a/crates/redact/README.md b/crates/redact/README.md index ec73af0ae3..0bb6241715 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -50,9 +50,19 @@ clear_marks(&mut doc, None)?; // remove marks without app too. An XMP packet that isn't valid UTF-8 or UTF-16, can't be decoded strictly (damaged or cut-short filter data, or more than one filter), or decodes to more than 16 MiB goes without being read. - - **Not scrubbed** (Sanitize's categories cover them): XMP streams on anything but the - catalog and the pages (images, fonts, form XObjects, annotations and other objects), - bookmark titles, and attachments (embedded files, their names and descriptions). + - **Not scrubbed, but reported** (Remove hidden information's categories remove them): bookmark + titles, attachments (names in the document's list of embedded files, file names and + descriptions, and the files of file attachment comments), the comments that remain (their + text, rich text, subject and author; not form fields or pop-ups), and XMP streams on anything + but the catalog and the pages (images, fonts, form XObjects, annotations and other objects). + `apply` changes none of them, but counts those that still hold the removed text, matched as + above, in `Report::still_holding` (`Elsewhere`), so the interface and the `redact_apply` tool + can say where it is; this search has a work limit of its own, past which every place counts. + When the removed text couldn't be searched for (the cases above in which every `/Info` + string goes), every one of them that holds any text is counted and `unsearched` is set; an XMP + stream or rich text stream that can't be read counts as holding it. The walks are bounded: at + most 100,000 bookmarks and 100,000 name-tree nodes (32 deep) are read, each once, and a walk + cut short counts one more place. A verification pass re-reads each redacted page and fails the operation if any glyph or inline image is still under an area. It checks every content stream the page had, whatever its diff --git a/crates/redact/src/elsewhere.rs b/crates/redact/src/elsewhere.rs new file mode 100644 index 0000000000..901775ac1b --- /dev/null +++ b/crates/redact/src/elsewhere.rs @@ -0,0 +1,219 @@ +//! Places [`crate::apply`] leaves as they are that may still hold the removed text (#2448 +//! review): bookmark titles, attachments, the comments that remain, and XMP on objects other than +//! the catalog and the pages. Removing them is Remove hidden information's job (its bookmarks, +//! attachments, comments and metadata categories); applying redactions only counts them, so the +//! user can be told. Nothing here changes the document. +//! +//! The text is matched as the metadata check matches it ([`crate::metadata::Search`]), with a +//! work limit of its own. Every walk is bounded: outline items and name-tree nodes are each read +//! once (a seen-set for indirect ones) up to a cap, and the XMP walk is Remove hidden +//! information's own. + +use std::collections::HashSet; + +use pdfcraft_cos::{Document, ObjRef, Object}; + +use crate::metadata::Search; + +/// Places that may still hold the removed text after [`crate::apply`], which it doesn't change. +/// Remove hidden information removes them. Each count is of places, not of occurrences. +/// +/// When the removed text couldn't be searched for (`unsearched`: the text of a removed glyph +/// isn't known, a form was removed unread, or there was too much removed text to collect or +/// search for, as for the metadata check), every place holding any text is counted, since any +/// of them may hold it. A count may also include places that couldn't be read or that lie past +/// a walk's cap (one for each walk cut short), which count as holding it, and, once the search's +/// work limit runs out, every place not yet checked that holds any text. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct Elsewhere { + /// Bookmarks (outline items) whose title holds it. + pub bookmarks: usize, + /// Embedded files whose name in the document's list of attachments (`/EmbeddedFiles`), + /// file name (`/F`, `/UF`) or description (`/Desc`) holds it, and file attachment + /// annotations whose file's name or description does. + pub attachments: usize, + /// Annotations left on the pages (not form field widgets or pop-ups) whose text + /// (`/Contents`, rich text `/RC`, subject `/Subj`) or author (`/T`) holds it. + pub comments: usize, + /// XMP streams on objects other than the catalog and the pages (images, fonts, form + /// XObjects, annotations…) whose text holds it, or that can't be read. + pub xmp: usize, + /// The removed text couldn't be searched for, so the counts are of every place that holds + /// any text. + pub unsearched: bool, +} + +impl Elsewhere { + /// All the places counted. + pub fn total(&self) -> usize { + self.bookmarks.saturating_add(self.attachments).saturating_add(self.comments).saturating_add(self.xmp) + } +} + +/// The most outline items read. +const MAX_OUTLINE_ITEMS: usize = 100_000; +/// The most name-tree nodes read, and how deep the tree is followed. +const MAX_NAME_TREE_NODES: usize = 100_000; +const MAX_NAME_TREE_DEPTH: usize = 32; + +/// Count the places that hold the `removed` text (see [`Elsewhere`]). `complete` is false when +/// the collection of removed text was cut short. +pub(crate) fn find(doc: &Document, removed: &[String], complete: bool) -> Elsewhere { + let search = Search::of(removed, complete); + if search.is_nothing() { + return Elsewhere::default(); + } + // A blank value holds nothing, even when everything counts. + let hit = + |o: Option<&Object>| o.and_then(|o| doc.resolve(o).as_string().map(|s| s.to_text())).is_some_and(|t| !t.trim().is_empty() && search.text(&t)); + Elsewhere { + bookmarks: bookmarks(doc, &hit), + attachments: attachments(doc, &hit), + comments: comments(doc, &search, &hit), + xmp: xmp(doc, &search), + unsearched: search.is_everything(), + } +} + +/// Outline items whose `/Title` holds the text. +fn bookmarks(doc: &Document, hit: &dyn Fn(Option<&Object>) -> bool) -> usize { + let Some(root) = doc.root() else { return 0 }; + let Some(first) = + doc.get(root).as_dict().and_then(|c| c.get(b"Outlines").cloned()).and_then(|o| doc.resolve(&o).as_dict()?.get(b"First").cloned()) + else { + return 0; + }; + let (mut n, mut read) = (0usize, 0usize); + let mut seen = HashSet::new(); + let mut next = vec![first]; + while let Some(o) = next.pop() { + if let Object::Ref(r) = &o + && !seen.insert(*r) + { + continue; + } + let Some(item) = doc.resolve(&o).as_dict().cloned() else { continue }; + if read >= MAX_OUTLINE_ITEMS { + // Items past the cap aren't read: count one for them. + return n.saturating_add(1); + } + read += 1; + n += usize::from(hit(item.get(b"Title"))); + next.extend([b"Next".as_slice(), b"First"].into_iter().filter_map(|k| item.get(k).cloned())); + } + n +} + +/// Embedded files in the `/EmbeddedFiles` name tree whose name, file name or description holds +/// the text, and file attachment annotations whose file's do. +fn attachments(doc: &Document, hit: &dyn Fn(Option<&Object>) -> bool) -> usize { + let file = |spec: Option<&Object>| { + spec.and_then(|s| doc.resolve(s).as_dict().cloned()).is_some_and(|d| [b"F".as_slice(), b"UF", b"Desc"].iter().any(|k| hit(d.get(k)))) + }; + let mut n = 0usize; + let tree = doc + .root() + .and_then(|r| doc.get(r).as_dict()?.get(b"Names").cloned()) + .and_then(|names| doc.resolve(&names).as_dict()?.get(b"EmbeddedFiles").cloned()); + if let Some(tree) = tree { + let mut seen = HashSet::new(); + let mut next = vec![(tree, 0usize)]; + let mut read = 0usize; + while let Some((o, depth)) = next.pop() { + if let Object::Ref(r) = &o + && !seen.insert(*r) + { + continue; + } + let Some(node) = doc.resolve(&o).as_dict().cloned() else { continue }; + if read >= MAX_NAME_TREE_NODES || depth > MAX_NAME_TREE_DEPTH { + // A node past the cap isn't read: count one for what it holds. + n = n.saturating_add(1); + break; + } + read += 1; + if let Some(leaves) = node.get(b"Names").map(|l| doc.resolve(l)).and_then(|l| l.as_array().cloned()) { + for pair in leaves.chunks(2) { + n += usize::from(hit(pair.first()) || file(pair.get(1))); + } + } + if let Some(kids) = node.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()) { + next.extend(kids.into_iter().map(|k| (k, depth.saturating_add(1)))); + } + } + } + for page in pdfcraft_model::pages(doc) { + for a in crate::annots_of(doc, &page.dict) { + let Some(a) = doc.resolve(&a).as_dict().cloned() else { continue }; + if a.name(b"Subtype") == Some(b"FileAttachment".as_slice()) { + n += usize::from(file(a.get(b"FS"))); + } + } + } + n +} + +/// Annotations on the pages, other than form field widgets and pop-ups (which show their +/// parent's text), whose text or author holds it. +fn comments(doc: &Document, search: &Search, hit: &dyn Fn(Option<&Object>) -> bool) -> usize { + let mut seen = HashSet::new(); + let mut n = 0usize; + for page in pdfcraft_model::pages(doc) { + for a in crate::annots_of(doc, &page.dict) { + if let Some(r) = a.as_ref() + && !seen.insert(r) + { + continue; + } + let Some(a) = doc.resolve(&a).as_dict().cloned() else { continue }; + if matches!(a.name(b"Subtype"), Some(b"Widget" | b"Popup")) { + continue; + } + // Rich text is XHTML: its text is searched, not its markup. A rich text stream that + // can't be read counts as holding it. + let rich = a.get(b"RC").is_some_and(|o| rich_text(doc, o).is_none_or(|t| !t.trim().is_empty() && search.text(&t))); + n += usize::from(rich || [b"Contents".as_slice(), b"T", b"Subj"].iter().any(|k| hit(a.get(k)))); + } + } + n +} + +/// The text of a rich text string or stream (`/RC`), markup left out; `None` when a stream +/// can't be read (strictly, as UTF-8, within the XMP limit). +fn rich_text(doc: &Document, o: &Object) -> Option { + let xml = match &*doc.resolve(o) { + Object::String(s) => s.to_text(), + Object::Stream(s) if s.filters().len() <= 1 => String::from_utf8(s.decoded_first_filter_strict(crate::metadata::MAX_XMP_BYTES).ok()?).ok()?, + Object::Stream(_) => return None, + _ => return Some(String::new()), + }; + Some(crate::metadata::xml_values(&xml).join(" ")) +} + +/// XMP streams on anything reachable from the catalog other than the catalog's and the pages' +/// own, each counted once, that hold the text or can't be read. +fn xmp(doc: &Document, search: &Search) -> usize { + let scrubbed: HashSet = crate::metadata::catalog_and_pages(doc).into_iter().collect(); + let Ok(values) = crate::sanitize::owned_values(doc, b"Metadata") else { + // A graph too deep to walk: some XMP may be there, unread. + return 1; + }; + let mut seen = HashSet::new(); + let mut n = 0usize; + for (holder, own, v) in values { + if own && scrubbed.contains(&holder) { + continue; + } + if let Some(r) = v.as_ref() + && !seen.insert(r) + { + continue; + } + // An entry that isn't a stream holds no XMP. + if !matches!(&*doc.resolve(&v), Object::Stream(_)) { + continue; + } + n += usize::from(search.xmp(doc, &v)); + } + n +} diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 9bc3672027..4d6bd676e0 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -12,6 +12,8 @@ //! A verification pass then re-reads every redacted page; if any glyph or inline image is still //! under a region the whole operation fails (callers keep the previous document: fail-closed). //! Finally, document metadata (`/Info` entries, XMP) that repeats the removed text is removed. +//! Places it leaves as they are (bookmark titles, attachments, remaining comments, other XMP) +//! are only counted when they still hold that text ([`Report::still_holding`]). #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] @@ -20,6 +22,7 @@ use std::collections::{HashMap, HashSet}; use pdfcraft_cos::{Dict, Document, ObjRef, Object, Stream}; pub mod codes; +mod elsewhere; mod image; mod interp; mod metadata; @@ -32,6 +35,7 @@ mod tests; pub(crate) mod type3; pub(crate) mod type3_tables; +pub use elsewhere::Elsewhere; use interp::{Mode, Scope, process}; pub type Rgb = [f64; 3]; @@ -153,6 +157,11 @@ pub struct Report { pub overlays: Vec, /// Document information entries and XMP streams removed because they held redacted text. pub metadata: usize, + /// Places this apply leaves as they are that still hold the removed text (bookmark titles, + /// attachments, the comments that remain, XMP on other objects), counted so the user can be + /// told; Remove hidden information removes them. See [`Elsewhere`] for when the counts are + /// of every place holding any text. + pub still_holding: Elsewhere, } fn rect_of(doc: &Document, o: Option<&Object>) -> Option<[f64; 4]> { @@ -536,6 +545,8 @@ fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result Option> { decode_xmp(&bytes).map(|t| xml_values(&t)) } +/// What the removed text is looked for with, and the searching left to do it. +pub(crate) struct Search { + lookup: Lookup, + work: Work, +} + +enum Lookup { + /// Nothing with text was removed: nothing can hold it. + Nothing, + /// The removed text couldn't be collected or searched for in full: everything counts as + /// holding it (fail-closed). + Everything, + /// The terms ([`terms`]) and the distinct runs, normalized. + Terms(Vec, Vec), +} + +impl Search { + /// The search for `removed` text, with `limit` steps of searching ([`MAX_WORK`] outside + /// tests). `complete` is false when the collection of removed text was cut short; then, as + /// when the terms exceed their limits, everything matches. + pub(crate) fn new(removed: &[String], complete: bool, limit: usize) -> Self { + let lookup = match terms(removed).filter(|_| complete) { + Some(t) if t.is_empty() => Lookup::Nothing, + Some(t) => { + // The same run removed many times is searched for once. + let mut seen = HashSet::new(); + let runs: Vec = removed.iter().map(|r| normalize(r)).filter(|r| seen.insert(r.clone())).collect(); + Lookup::Terms(t, runs) + } + None => Lookup::Everything, + }; + Self { lookup, work: Work::new(limit) } + } + + /// [`Search::new`] with the whole work limit. + pub(crate) fn of(removed: &[String], complete: bool) -> Self { + Self::new(removed, complete, MAX_WORK) + } + + /// Nothing with text was removed. + pub(crate) fn is_nothing(&self) -> bool { + matches!(self.lookup, Lookup::Nothing) + } + + /// The removed text couldn't be searched for: everything matches. + pub(crate) fn is_everything(&self) -> bool { + matches!(self.lookup, Lookup::Everything) + } + + /// Does `text` hold the removed text: a term, or wholly part of a removed run? Past the + /// work limit, yes (fail-closed). + pub(crate) fn text(&self, text: &str) -> bool { + match &self.lookup { + Lookup::Nothing => false, + Lookup::Everything => true, + Lookup::Terms(terms, runs) => mentions(text, terms, &self.work) || within(text, runs, &self.work), + } + } + + /// Does the XMP stream `o` hold the removed text? A stream that can't be read does + /// (fail-closed), unless nothing with text was removed. + pub(crate) fn xmp(&self, doc: &Document, o: &Object) -> bool { + match &self.lookup { + Lookup::Nothing => false, + Lookup::Everything => true, + Lookup::Terms(terms, runs) => xmp_values(doc, o) + .is_none_or(|values| mentions(&values.join(" "), terms, &self.work) || values.iter().any(|v| within(v, runs, &self.work))), + } + } +} + /// Remove the metadata that still holds `removed` text: matching `/Info` entries, and the /// catalog's and pages' XMP streams that mention it. `complete` is false when the collection of /// removed text was cut short; then, as when the terms themselves exceed their limits, every @@ -295,26 +366,17 @@ pub(crate) fn scrub(doc: &mut Document, removed: &[String], complete: bool) -> R /// [`scrub`] with the searching it may do given, so tests can run out of it. pub(crate) fn scrub_within(doc: &mut Document, removed: &[String], complete: bool, limit: usize) -> Result { - // `None` matches everything (fail-closed). - let search = match terms(removed).filter(|_| complete) { - Some(t) if t.is_empty() => return Ok(0), - Some(t) => { - // The same run removed many times is searched for once. - let mut seen = HashSet::new(); - let runs: Vec = removed.iter().map(|r| normalize(r)).filter(|r| seen.insert(r.clone())).collect(); - Some((t, runs)) - } - None => None, - }; - let work = Work::new(limit); - let hit = |text: &str| search.as_ref().is_none_or(|(terms, runs)| mentions(text, terms, &work) || within(text, runs, &work)); + let search = Search::new(removed, complete, limit); + if search.is_nothing() { + return Ok(0); + } let mut n = 0; // The information dictionary, direct in the trailer or an object of its own. let info_obj = doc.trailer().get(b"Info").cloned(); if let Some(info) = info_obj.as_ref().and_then(|i| doc.resolve(i).as_dict().cloned()) { let doomed: Vec> = - info.iter().filter(|(_, v)| doc.resolve(v).as_string().is_some_and(|s| hit(&s.to_text()))).map(|(k, _)| k.to_vec()).collect(); + info.iter().filter(|(_, v)| doc.resolve(v).as_string().is_some_and(|s| search.text(&s.to_text()))).map(|(k, _)| k.to_vec()).collect(); if !doomed.is_empty() { n += doomed.len(); let mut kept = info.clone(); @@ -329,19 +391,9 @@ pub(crate) fn scrub_within(doc: &mut Document, removed: &[String], complete: boo } // XMP on the catalog and on every page. - let mut holders: Vec = doc.root().into_iter().collect(); - holders.extend(pdfcraft_model::pages(doc).iter().map(|p| p.obj)); - for holder in holders { + for holder in catalog_and_pages(doc) { let Some(xmp) = doc.get(holder).as_dict().and_then(|d| d.get(b"Metadata").cloned()) else { continue }; - let doomed = match xmp_values(doc, &xmp) { - None => true, - Some(values) => { - let words = |(terms, _): &(Vec, Vec)| mentions(&values.join(" "), terms, &work); - let part = |(_, runs): &(Vec, Vec)| values.iter().any(|v| within(v, runs, &work)); - search.as_ref().is_none_or(|s| words(s) || part(s)) - } - }; - if doomed { + if search.xmp(doc, &xmp) { doc.update_dict(holder, |d| { d.remove(b"Metadata"); })?; @@ -350,3 +402,10 @@ pub(crate) fn scrub_within(doc: &mut Document, removed: &[String], complete: boo } Ok(n) } + +/// The catalog and the pages: the objects whose XMP [`scrub`] checks. +pub(crate) fn catalog_and_pages(doc: &Document) -> Vec { + let mut holders: Vec = doc.root().into_iter().collect(); + holders.extend(pdfcraft_model::pages(doc).iter().map(|p| p.obj)); + holders +} diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index 98703dfbf2..b6cf65a1c8 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -318,6 +318,47 @@ fn strip_owned(o: &mut Object, role: Held, keys: &[&[u8]], depth: usize) { } } +/// Every value of `key` held outside a name map, as [`owners`] finds them: with the indirect +/// object that holds it and whether it is that object's own entry (rather than one in a +/// dictionary written inside it). Fails as [`owners`] does for a graph nested too deeply. +pub(crate) fn owned_values(doc: &Document, key: &[u8]) -> Result, RedactError> { + let (_, holders) = owners(doc, &[key])?; + let mut out = Vec::new(); + for (r, role) in holders { + collect_owned(&doc.get(r), role, key, 0, &mut |own, v| out.push((r, own, v.clone()))); + } + Ok(out) +} + +/// Calls `found` with each value of `key` in `o` and what it holds in place, outside name maps +/// (the reading counterpart of [`strip_owned`]), and whether it is `o`'s own entry. +fn collect_owned(o: &Object, role: Held, key: &[u8], depth: usize, found: &mut dyn FnMut(bool, &Object)) { + if depth > MAX_DIRECT_DEPTH { + return; + } + let (d, role) = match o { + Object::Array(a) => { + for v in a { + collect_owned(v, Held::Owner, key, depth + 1, found); + } + return; + } + Object::Dict(d) => (d, role), + Object::Stream(s) => (&s.dict, Held::Owner), + _ => return, + }; + if role != Held::Map + && let Some(v) = d.get(key) + { + found(depth == 0, v); + } + for (k, v) in d.iter() { + if matches!(v, Object::Array(_) | Object::Dict(_)) { + collect_owned(v, role.child(k), key, depth + 1, found); + } + } +} + /// A comment as the comments list sees it, less file attachments (removed with Attachments) and /// redaction marks. fn is_comment(subtype: &[u8]) -> bool { diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index b810043279..49c74e64bf 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -3038,6 +3038,7 @@ fn unused_descriptions_in_shared_or_inherited_resources_are_retired() { } mod descriptions; +mod elsewhere; mod inline_images; mod limits; mod metadata_fail_closed; diff --git a/crates/redact/src/tests/elsewhere.rs b/crates/redact/src/tests/elsewhere.rs new file mode 100644 index 0000000000..7379af19cc --- /dev/null +++ b/crates/redact/src/tests/elsewhere.rs @@ -0,0 +1,113 @@ +//! #2539 review: what applying leaves as it is but still holds the removed text is counted in +//! [`Report::still_holding`], and nothing of it is changed. + +use super::*; + +const XMP_BLUEFIN: &str = r#"Photo for Bluefin"#; +const XMP_OTHER: &str = r#"Annual figures"#; + +/// The objects whose text the check reads, by number. +const HELD: [u32; 20] = [7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26]; + +/// A page showing "Codename BLUEFIN report" (BLUEFIN at x 55–90) with `font`, and the word, or +/// other text, in a bookmark, attachments, comments and XMP the mark doesn't touch: +/// - bookmarks: "Bluefin plans", "Annual figures", and under it "BLUEFIN appendix"; +/// - attachments: one named `bluefin.txt`, one described "About Bluefin", one with neither, and +/// a file attachment annotation for `Bluefin.pdf`; +/// - comments: one whose text mentions it, one by "Bluefin Team", one whose rich text does, one +/// that doesn't, and a pop-up repeating it (pop-ups aren't counted); +/// - XMP: an image's mentions it, a font's doesn't. +fn page_with_places(font: &str) -> Document { + let annot = |rest: &str| format!("<< /Type /Annot /Rect [200 200 220 220] {rest} >>").into_bytes(); + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /Outlines 7 0 R /Names << /EmbeddedFiles 11 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 >> /XObject << /Im 24 0 R >> >> /Annots [16 0 R 17 0 R 18 0 R 19 0 R 20 0 R 21 0 R] >>".to_vec(), + stream("", b"BT /F1 10 Tf 10 100 Td (Codename BLUEFIN report) Tj ET q 10 0 0 10 200 250 cm /Im Do Q"), + format!("{} /Metadata 26 0 R >>", font.replace("95 0 R", "6 0 R").strip_suffix(">>").unwrap()).into_bytes(), + widths(), + // 7–10: the outline. + b"<< /Type /Outlines /First 8 0 R /Last 9 0 R /Count 3 >>".to_vec(), + b"<< /Title (Bluefin plans) /Parent 7 0 R /Next 9 0 R >>".to_vec(), + b"<< /Title (Annual figures) /Parent 7 0 R /Prev 8 0 R /First 10 0 R /Last 10 0 R /Count 1 >>".to_vec(), + b"<< /Title (BLUEFIN appendix) /Parent 9 0 R >>".to_vec(), + // 11–15: the attachments' name tree and file specifications. + b"<< /Kids [12 0 R] >>".to_vec(), + b"<< /Limits [(a.txt) (bluefin.txt)] /Names [(a.txt) 13 0 R (b.txt) 14 0 R (bluefin.txt) 15 0 R] >>".to_vec(), + b"<< /Type /Filespec /F (a.txt) /UF (a.txt) >>".to_vec(), + b"<< /Type /Filespec /F (b.txt) /Desc (About Bluefin) >>".to_vec(), + b"<< /Type /Filespec /F (c.txt) >>".to_vec(), + // 16–21: the annotations. + annot("/Subtype /Text /Contents (Ask about bluefin) /T (Ada)"), + annot("/Subtype /Text /Contents (Seen) /T (Bluefin Team)"), + annot("/Subtype /FreeText /Contents (plain) /RC (

BLUEFIN

) /DA (/F1 10 Tf)"), + annot("/Subtype /Text /Contents (Looks good) /T (Ada) /Popup 21 0 R"), + annot("/Subtype /FileAttachment /Contents (Attached) /T (Ada) /FS << /Type /Filespec /UF (Bluefin.pdf) >>"), + annot("/Subtype /Popup /Parent 19 0 R /Contents (bluefin)"), + // 22–23: unused. + b"null".to_vec(), + b"null".to_vec(), + // 24–26: an image with XMP that mentions it, and the two packets. + stream("/Type /XObject /Subtype /Image /Width 1 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8 /Metadata 25 0 R", b"\x80"), + stream("/Type /Metadata /Subtype /XML", XMP_BLUEFIN.as_bytes()), + stream("/Type /Metadata /Subtype /XML", XMP_OTHER.as_bytes()), + ]; + pdf(objs) +} + +fn held(doc: &Document) -> Vec { + HELD.iter().map(|&n| (*doc.get(ObjRef::new(n, 0))).clone()).collect() +} + +#[test] +fn places_apply_leaves_that_still_hold_the_removed_text_are_counted_and_kept() { + let mut doc = page_with_places(FONT); + let before = held(&doc); + mark(&mut doc, 0, &[[55.0, 95.0, 90.0, 110.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!(r.glyphs, 7, "{r:?}"); + assert_eq!(r.still_holding, Elsewhere { bookmarks: 2, attachments: 3, comments: 3, xmp: 1, unsearched: false }, "{r:?}"); + assert_eq!(r.still_holding.total(), 9); + assert_eq!(held(&doc), before, "counting changes nothing"); + let catalog = doc.get(doc.root().unwrap()).as_dict().cloned().unwrap(); + assert!(catalog.contains(b"Outlines") && catalog.contains(b"Names")); + assert_eq!(annots_of(&doc, &pdfcraft_model::pages(&doc)[0].dict).len(), 6, "the comments stay"); +} + +/// Removed text that is unrelated to every place counts nothing. +#[test] +fn places_without_the_removed_text_are_not_counted() { + let mut doc = page_with_places(FONT); + // "Codename" alone, at x 10–50. + mark(&mut doc, 0, &[[8.0, 95.0, 50.0, 110.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!(r.glyphs, 8, "{r:?}"); + assert_eq!(r.still_holding, Elsewhere::default(), "{r:?}"); +} + +/// When the removed text can't be known (a removed glyph whose text isn't known), every place +/// that holds any text counts, and says why. +#[test] +fn places_count_whenever_they_hold_text_if_the_removed_text_is_unknown() { + let unnamed = FONT.replace(" /Widths", " /Encoding << /Differences [66 /g17 /g18 /g19 /g20 /g21 /g22 /g23 /g24 /g25 /g26 /g27 /g28 /g29 /g30 /g31 /g32 /g33 /g34 /g35 /g36 /g37 /g38 /g39 /g40 /g41 /g42] >> /Widths"); + let mut doc = page_with_places(&unnamed); + let before = held(&doc); + mark(&mut doc, 0, &[[55.0, 95.0, 90.0, 110.0]], ""); + let r = apply(&mut doc, None).unwrap(); + // Every bookmark title, every attachment (each has a name), every comment but the pop-up, + // and both XMP packets (the font's too). + assert_eq!(r.still_holding, Elsewhere { bookmarks: 3, attachments: 4, comments: 5, xmp: 2, unsearched: true }, "{r:?}"); + assert_eq!(held(&doc), before); +} + +/// A bookmark list that loops back on itself is walked once. +#[test] +fn a_looping_outline_is_read_once() { + let mut doc = page_with_places(FONT); + // The last item's /Next goes back to the first. + doc.update_dict(ObjRef::new(9, 0), |d| d.set(b"Next".to_vec(), Object::Ref(ObjRef::new(8, 0)))).unwrap(); + doc.update_dict(ObjRef::new(10, 0), |d| d.set(b"First".to_vec(), Object::Ref(ObjRef::new(7, 0)))).unwrap(); + mark(&mut doc, 0, &[[55.0, 95.0, 90.0, 110.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!(r.still_holding.bookmarks, 2, "{r:?}"); +} From 1fd2ff31bbd219cfa9e7dd16f4c899a942d098ea Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sun, 1 Nov 2026 18:09:13 +0800 Subject: [PATCH 13/23] M8: test that a form whose content can't be decoded clears the metadata (#2448) The unread-form branch was tested for a form nested past the depth limit only; a form under a mark whose filter can't be decoded is removed unread too, and now has an end-to-end test, seen failing with that branch's flag removed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- .../redact/src/tests/metadata_fail_closed.rs | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/crates/redact/src/tests/metadata_fail_closed.rs b/crates/redact/src/tests/metadata_fail_closed.rs index 0a958ccd88..2735b302ba 100644 --- a/crates/redact/src/tests/metadata_fail_closed.rs +++ b/crates/redact/src/tests/metadata_fail_closed.rs @@ -155,3 +155,26 @@ fn an_xmp_stream_behind_two_filters_goes() { let Stream { raw, .. } = Stream::flate(Dict::new(), hex.as_bytes()); assert_only_the_catalog_packet_went(stream("/Type /Metadata /Subtype /XML /Filter [/FlateDecode /ASCIIHexDecode]", &raw)); } + +/// A form XObject under a mark whose content can't be decoded is removed without being read, so +/// what it showed is unknown (the deep-form case is `a_form_removed_unread_clears_the_metadata`). +#[test] +fn a_form_whose_content_cant_be_read_clears_the_metadata() { + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /Metadata 7 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 >> /XObject << /Fm 9 0 R >> >> /Metadata 8 0 R >>".to_vec(), + stream("", b"q 1 0 0 1 0 0 cm /Fm Do Q"), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + clean_xmp(), + clean_xmp(), + // 9: a form whose data no filter here decodes. + stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Filter /NoSuchDecode", b"BT /F1 10 Tf 10 100 Td (Bluefin) Tj ET"), + b"<< /Title (Annual figures) /Producer (Test) /Trapped /False >>".to_vec(), + ]; + let mut doc = Document::open(Arc::new(Pdf::new().objs(objs).trailer_entry("/Info 10 0 R").build())).unwrap(); + let r = redact_line(&mut doc); + assert_eq!(r.forms_removed, 1, "{r:?}"); + assert_cleared(&doc, &r); +} From d37bde8c3b43beda006a6909d90f4e8d68322905 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sun, 1 Nov 2026 18:31:52 +0800 Subject: [PATCH 14/23] M8: Apply redactions passes on where the removed text is still held; redact_apply returns it (#2448) The document keeps the last apply's Report::still_holding as redaction_elsewhere (re-exported as RedactionElsewhere), cleared by the next edit, undo or redo as edit_notices is. redact_apply returns it as still_holding with the four counts, their total and unsearched. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/automation/src/redact.rs | 10 ++++++++++ crates/automation/src/tools.rs | 2 +- crates/automation/tests/it/automation.rs | 25 ++++++++++++++++++++++++ crates/engine/src/document.rs | 5 +++++ crates/engine/src/edit/run.rs | 7 ++++++- crates/engine/src/lib.rs | 1 + crates/engine/src/open.rs | 1 + crates/engine/src/session/editing.rs | 4 ++++ crates/engine/src/tests.rs | 19 ++++++++++++++++++ 9 files changed, 72 insertions(+), 2 deletions(-) diff --git a/crates/automation/src/redact.rs b/crates/automation/src/redact.rs index e662bcac19..baea3d75c5 100644 --- a/crates/automation/src/redact.rs +++ b/crates/automation/src/redact.rs @@ -165,6 +165,16 @@ impl Automation { let after = self.doc(a)?.redaction_marks(); out["applied"] = json!(before - after); out["marks_pending"] = json!(after); + // Where the removed text is still held that applying leaves as it is. + let left = self.doc(a)?.redaction_elsewhere.unwrap_or_default(); + out["still_holding"] = json!({ + "bookmarks": left.bookmarks, + "attachments": left.attachments, + "comments": left.comments, + "xmp": left.xmp, + "total": left.total(), + "unsearched": left.unsearched, + }); Ok(out) } diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index 53c4582af9..2d4d597a75 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -544,7 +544,7 @@ pub fn tools() -> Vec { t( "redact_apply", "Apply redactions", - "Apply the redaction marks (all, or those on pages): text, images, vectors, comments and form fields under them are removed for good and boxes are drawn in their place. A verification pass fails the operation if anything readable remains. Undoable until saved; the saved file no longer contains the content.", + "Apply the redaction marks (all, or those on pages): text, images, vectors, comments and form fields under them are removed for good and boxes are drawn in their place. A verification pass fails the operation if anything readable remains. Document metadata (Info entries, the document's and pages' XMP) that repeats the removed text is removed too. still_holding counts the places applying leaves as they are that still hold the removed text: bookmarks (titles), attachments (names, file names, descriptions), comments (text, rich text, subject or author of those that remain) and xmp (XMP on images, fonts, forms, annotations and other objects), with their total; doc_remove_hidden removes them. unsearched: true means the removed text couldn't be searched for, so every such place holding any text is counted. Undoable until saved; the saved file no longer contains the content.", ) .destructive() .with(schema(json!({ "doc": doc(), "pages": pages("whose marks to apply (default: all)") }), &["doc"])), diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index 01d0d1eb24..412bc9f80e 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -4289,6 +4289,31 @@ fn redacting_through_tools() { assert!(matches!(a.call("redact_mark", &json!({ "doc": doc })), Err(ToolError::InvalidArgs(_)))); } +/// #2539 review: `redact_apply` says where the removed text is still held that applying leaves as +/// it is (a bookmark title and a note's text here), and changes neither. +#[test] +fn redact_apply_reports_places_that_still_hold_the_removed_text() { + let dir = workdir("redact-still-holding"); + std::fs::write(dir.join("memo.txt"), "Codename Bluefin report\nPublic line").unwrap(); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_create", json!({ "from": "text", "path": "memo.txt" }))["doc"].as_u64().unwrap(); + ok(&mut a, "bookmark_add", json!({ "doc": doc, "title": "Bluefin plans", "page": 1 })); + ok(&mut a, "bookmark_add", json!({ "doc": doc, "title": "Annual figures", "page": 1 })); + ok(&mut a, "comment_add", json!({ "doc": doc, "page": 1, "type": "note", "at": [300, 400], "contents": "Ask about bluefin" })); + ok(&mut a, "comment_add", json!({ "doc": doc, "page": 1, "type": "note", "at": [300, 500], "contents": "Looks good" })); + ok(&mut a, "redact_mark", json!({ "doc": doc, "find": "Bluefin" })); + let r = ok(&mut a, "redact_apply", json!({ "doc": doc })); + assert_eq!(r["still_holding"], json!({ "bookmarks": 1, "attachments": 0, "comments": 1, "xmp": 0, "total": 2, "unsearched": false }), "{r}"); + assert!(!page_text(&mut a, doc)[0].contains("Bluefin")); + let titles: Vec = ok(&mut a, "bookmark_list", json!({ "doc": doc }))["bookmarks"] + .as_array() + .unwrap() + .iter() + .map(|b| b["title"].as_str().unwrap().to_string()) + .collect(); + assert_eq!(titles, ["Bluefin plans", "Annual figures"], "reported, not changed"); +} + #[test] /// Mark, apply and save remove the complete international number while retaining labels and controls. fn redacting_italian_international_slash_numbers_through_tools() { diff --git a/crates/engine/src/document.rs b/crates/engine/src/document.rs index eaa8f70807..7ad7fffec9 100644 --- a/crates/engine/src/document.rs +++ b/crates/engine/src/document.rs @@ -62,6 +62,11 @@ pub struct Document { /// What the last successful [`Session::apply`](crate::Session::apply) wrote that its fonts can't draw: kept as typed, /// drawn with `?` in its place (see [`TextNotice`]). The interface shows it; tools return it. pub edit_notices: Vec, + /// What the last successful [`Session::apply`](crate::Session::apply), when it applied redactions, + /// found still holding the removed text in places applying leaves as they are (bookmark titles, + /// attachments, remaining comments, other XMP; see [`RedactionElsewhere`](crate::RedactionElsewhere)). + /// `None` after any other edit, an undo or a redo. The interface says so; tools return it. + pub redaction_elsewhere: Option, } /// Text an edit stored as typed but drew with `?` in place of `ch`, because the font it is drawn diff --git a/crates/engine/src/edit/run.rs b/crates/engine/src/edit/run.rs index 32dc7942ab..af49c1db00 100644 --- a/crates/engine/src/edit/run.rs +++ b/crates/engine/src/edit/run.rs @@ -183,6 +183,8 @@ pub(crate) struct EditCtx { /// The overlay streams the edit's applied redactions added, which alone the renderer's check /// leaves out. pub(crate) redaction_overlays: Vec, + /// What the edit's applied redactions left as it was that still holds the removed text. + pub(crate) redaction_elsewhere: Option, } impl EditCtx { @@ -211,6 +213,7 @@ impl EditCtx { count: 0, renamed_fields: Vec::new(), redaction_overlays: Vec::new(), + redaction_elsewhere: None, } } @@ -648,7 +651,9 @@ pub(crate) fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut E pdfcraft_edit::add_content(doc, *page, &content)?; } Edit::ApplyRedactions { pages } => { - cx.redaction_overlays = pdfcraft_redact::apply(doc, pages.as_deref())?.overlays; + let report = pdfcraft_redact::apply(doc, pages.as_deref())?; + cx.redaction_overlays = report.overlays; + cx.redaction_elsewhere = Some(report.still_holding); } Edit::ClearRedactions => { pdfcraft_redact::clear_marks(doc, None)?; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index c41c0466c4..c168cd2bf9 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -112,6 +112,7 @@ pub use pdfcraft_forms::detect; pub use pdfcraft_optimize as optimize; pub use pdfcraft_preflight as pdfa; pub use pdfcraft_print as print; +pub use pdfcraft_redact::Elsewhere as RedactionElsewhere; pub use pdfcraft_redact::codes::{CODE_SETS as REDACTION_CODE_SETS, CodeSet as RedactionCodeSet}; pub use pdfcraft_redact::patterns::{ MAX_WORDS as REDACT_MAX_WORDS, PATTERNS as REDACT_PATTERNS, Pattern as RedactPattern, find as find_pattern, word_list as redact_word_list, diff --git a/crates/engine/src/open.rs b/crates/engine/src/open.rs index 58bc8327e2..84c9472a20 100644 --- a/crates/engine/src/open.rs +++ b/crates/engine/src/open.rs @@ -352,6 +352,7 @@ impl Session { fonts_stale: false, xfa_warnings: Vec::new(), edit_notices: Vec::new(), + redaction_elsewhere: None, } } } diff --git a/crates/engine/src/session/editing.rs b/crates/engine/src/session/editing.rs index f8d5648a2d..675597a565 100644 --- a/crates/engine/src/session/editing.rs +++ b/crates/engine/src/session/editing.rs @@ -16,6 +16,7 @@ impl Session { let xfa_time_scale = self.xfa_time_scale.unwrap_or(1); let doc = self.doc_mut(id)?; doc.edit_notices.clear(); + doc.redaction_elsewhere = None; let name = doc.name.clone(); let is_xfa = doc.info.xfa.is_some(); let mut scope = scope_of(&edit); @@ -201,6 +202,7 @@ impl Session { doc.dirty = true; doc.generation += 1; doc.edit_notices = notices; + doc.redaction_elsewhere = cx.redaction_elsewhere; if !cx.xfa_removed.is_empty() { let removed: Vec = cx.xfa_removed.iter().map(|n| format!("{XFA_REMOVED_PREFIX}{n}")).collect(); doc.xfa_warnings.retain(|w| w.starts_with(XFA_REMOVED_PREFIX)); @@ -246,6 +248,7 @@ impl Session { return Err(e); } doc.edit_notices.clear(); + doc.redaction_elsewhere = None; Self::sync_dirty(doc); doc.generation += 1; Ok(label) @@ -275,6 +278,7 @@ impl Session { return Err(e); } doc.edit_notices.clear(); + doc.redaction_elsewhere = None; Self::sync_dirty(doc); doc.generation += 1; Ok(label) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 6a68916dff..16a612339c 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2829,6 +2829,25 @@ fn text_after_an_inline_image_is_redacted_where_the_page_shows_it() { ); } +/// #2539 review: Apply redactions tells the caller where the removed text is still held that +/// it leaves as it is (here a bookmark title), and the next edit, or an undo, forgets it. +#[test] +fn applied_redactions_report_a_bookmark_that_still_holds_the_removed_text() { + let mut s = Session::new().with_clock(|| 1_700_000_000); + let page = one_line_page("/Font << /F1 4 0 R >>", "BT /F1 10 Tf 20 150 Td (Codename BLUEFIN report) Tj ET"); + let id = s.open("bookmark.pdf", None, Arc::new(page), None).unwrap(); + s.apply(id, Edit::AddBookmark { parent: vec![], index: 0, title: "Bluefin plans".into(), page: 0, left: None, top: None }).unwrap(); + s.apply(id, Edit::AddBookmark { parent: vec![], index: 1, title: "Annual figures".into(), page: 0, left: None, top: None }).unwrap(); + mark_shown_text(&mut s, id, "BLUEFIN"); + assert_eq!(s.get(id).unwrap().redaction_elsewhere, None, "only applying redactions reports it"); + s.apply(id, Edit::ApplyRedactions { pages: None }).unwrap(); + let found = s.get(id).unwrap().redaction_elsewhere.expect("applying reports what it left"); + assert_eq!(found, crate::RedactionElsewhere { bookmarks: 1, ..Default::default() }); + assert_eq!(outline_titles(&s.get(id).unwrap().info.outline), ["Bluefin plans→1", "Annual figures→1"], "the bookmark is reported, not changed"); + s.undo(id).unwrap(); + assert_eq!(s.get(id).unwrap().redaction_elsewhere, None, "an undo forgets it"); +} + /// 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] From 4dbc4fac696c9d9b27711eb989a25001b4e4c094 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sun, 1 Nov 2026 18:31:52 +0800 Subject: [PATCH 15/23] M8: the apply notification says where the removed text is still held (#2448) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After "Applied N redaction marks…", the same notification names the bookmark titles, attachments, comments and other metadata streams that still hold the redacted text, and points to Remove hidden information. New English templates; untranslated text shows in English. The protect.apply-redactions parity note and evidence say what is reported and what is still left. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/ui-egui/src/dialogs.rs | 41 +++++++++++++++++++-- crates/ui-egui/tests/it/redact.rs | 60 +++++++++++++++++++++++++++++++ parity/acrobat-features.toml | 4 +-- 3 files changed, 100 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index e57299fdcb..046f6a87af 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1711,10 +1711,17 @@ fn finish_redactions_and_fields(app: &mut PdfCraftApp, req: &mut Pending) { Some(Dialog::RedactApply) => { let marks = app.active_ids().and_then(|(_, id)| app.session.get(id)).map_or(0, |d| d.redaction_marks()); if app.apply_edit(Edit::ApplyRedactions { pages: None }) { - if marks == 1 { - app.notify_tr("Applied 1 redaction mark. Save to remove the content from the file."); + let applied = if marks == 1 { + tl!("Applied 1 redaction mark. Save to remove the content from the file.").to_string() } else { - app.notify_fmt("Applied {n} redaction marks. Save to remove the content from the file.", &[("n", &marks.to_string())]); + crate::i18n::fmt(tl!("Applied {n} redaction marks. Save to remove the content from the file."), &[("n", &marks.to_string())]) + }; + // Where the removed text is still held that applying leaves as it is: said in the + // same notification, so it can't replace the first sentence. + let left = app.active_ids().and_then(|(_, id)| app.session.get(id)).and_then(|d| d.redaction_elsewhere); + match left.and_then(|e| still_holding_note(&e)) { + Some(note) => app.notify(format!("{applied} {note}")), + None => app.notify(applied), } } } @@ -1760,6 +1767,34 @@ fn finish_redactions_and_fields(app: &mut PdfCraftApp, req: &mut Pending) { } } +/// The sentence telling the user where the removed text is still held that applying redactions +/// leaves as it is, or `None` when it is nowhere else. When the text couldn't be searched for, +/// the places are those holding any text, and the sentence says they may hold it. +fn still_holding_note(e: &pdfcraft_engine::RedactionElsewhere) -> Option { + let count = |n: usize, one: &str, many: &str| match n { + 0 => None, + 1 => Some(tl!(one).to_string()), + n => Some(crate::i18n::fmt(tl!(many), &[("n", &n.to_string())])), + }; + let places: Vec = [ + count(e.bookmarks, "1 bookmark title", "{n} bookmark titles"), + count(e.attachments, "1 attachment", "{n} attachments"), + count(e.comments, "1 comment", "{n} comments"), + count(e.xmp, "1 other metadata stream", "{n} other metadata streams"), + ] + .into_iter() + .flatten() + .collect(); + let (last, rest) = places.split_last()?; + let list = if rest.is_empty() { last.clone() } else { crate::i18n::fmt(tl!("{list} and {last}"), &[("list", &rest.join(", ")), ("last", last)]) }; + let template = if e.unsearched { + "The redacted text couldn't be searched for, so {places} may still hold it. Remove hidden information removes them." + } else { + "The redacted text also appears in {places}, which redaction doesn't change. Remove hidden information removes them." + }; + Some(crate::i18n::fmt(tl!(template), &[("places", &list)])) +} + /// Edits built from the dialog's drafts. fn finish_edits(app: &mut PdfCraftApp, dialog: Dialog, req: &mut Pending) { if req.props_now diff --git a/crates/ui-egui/tests/it/redact.rs b/crates/ui-egui/tests/it/redact.rs index 3e11861cbc..f32341adf6 100644 --- a/crates/ui-egui/tests/it/redact.rs +++ b/crates/ui-egui/tests/it/redact.rs @@ -265,3 +265,63 @@ fn removing_hidden_information_and_sanitizing() { let s = h.state(); assert_eq!(s.session.get(s.views[0].id).unwrap().can_undo(), Some("Sanitize document")); } + +/// A one-page PDF showing "Codename BLUEFIN report", with bookmarks "Bluefin plans" and +/// "Annual figures". +fn bookmarked_pdf() -> Vec { + let content = "BT /F1 18 Tf 20 150 Td (Codename BLUEFIN report) Tj ET"; + let objs = [ + "<< /Type /Catalog /Pages 2 0 R /Outlines 6 0 R >>".to_string(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_string(), + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Resources << /Font << /F1 4 0 R >> >> /Contents 5 0 R >>".to_string(), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".to_string(), + format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()), + "<< /Type /Outlines /First 7 0 R /Last 8 0 R /Count 2 >>".to_string(), + "<< /Title (Bluefin plans) /Parent 6 0 R /Next 8 0 R /Dest [3 0 R /Fit] >>".to_string(), + "<< /Title (Annual figures) /Parent 6 0 R /Prev 7 0 R /Dest [3 0 R /Fit] >>".to_string(), + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + out +} + +/// #2539 review: applying redactions says when the removed text is still in a place it leaves +/// as it is (here a bookmark title), in the same notification as the count of marks applied. +#[test] +fn applying_says_where_the_removed_text_is_still_held() { + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("bluefin.pdf", None, bookmarked_pdf()).unwrap(); + app + }); + crate::support::settle(&mut h); + assert!(h.state_mut().execute("redact.mark")); + h.run_steps(2); + // Redact pages ▸ current page, then Redact all ▸ Apply. + h.state_mut().execute("redact.pages"); + h.run_steps(2); + h.get_by_label("OK").click(); + h.run_steps(3); + assert_eq!(marks(&h), 1); + h.get_by_label("Redact all").click(); + h.run_steps(3); + h.get_by_label("Apply").click(); + h.run_steps(4); + assert_eq!(marks(&h), 0); + let toast = h.state().toast.as_ref().map(|(m, _)| m.clone()).unwrap_or_default(); + assert_eq!( + toast, + "Applied 1 redaction mark. Save to remove the content from the file. The redacted text also appears in 1 bookmark title, which redaction doesn't change. Remove hidden information removes them." + ); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 4fb222a7e0..cedd69b876 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -5768,8 +5768,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::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", "crates/redact/src/tests.rs::redacted_text_does_not_survive_in_info_or_xmp", "crates/redact/src/tests.rs::metadata_is_cleared_when_the_removed_text_was_cut_short"] -notes = "confirmation; one undo step; next save is a full rewrite. Document information entries that contain the removed text, and the document's and pages' XMP streams that do, are removed (#2448; matched without regard to case or spacing, by the whole run of removed text and by its words of three letters or digits or more, and a value wholly part of a run as whole words; fail-closed when the removed text was too much to collect or the XMP can't be decoded); other objects' XMP, bookmarks and attachments are not searched. 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" +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", "crates/redact/src/tests.rs::redacted_text_does_not_survive_in_info_or_xmp", "crates/redact/src/tests.rs::metadata_is_cleared_when_the_removed_text_was_cut_short", "crates/redact/src/tests/metadata_fail_closed.rs::an_xmp_stream_whose_filter_data_is_corrupt_goes", "crates/redact/src/tests/elsewhere.rs::places_apply_leaves_that_still_hold_the_removed_text_are_counted_and_kept", "crates/engine/src/tests.rs::applied_redactions_report_a_bookmark_that_still_holds_the_removed_text", "crates/automation/tests/it/automation.rs::redact_apply_reports_places_that_still_hold_the_removed_text", "crates/ui-egui/tests/it/redact.rs::applying_says_where_the_removed_text_is_still_held"] +notes = "confirmation; one undo step; next save is a full rewrite. Document information entries that contain the removed text, and the document's and pages' XMP streams that do, are removed (#2448; matched without regard to case or spacing, by the whole run of removed text and by its words of three letters or digits or more, and a value wholly part of a run as whole words; fail-closed: all of it goes when the removed text can't be known in full (too much to collect or search for, a glyph whose text isn't known, a form removed unread), and an XMP packet goes when it can't be decoded strictly, is over 16 MiB or isn't UTF-8 or UTF-16). Bookmark titles, attachments (names, file names, descriptions), the comments that remain and other objects' XMP are not changed: applying counts those that still hold the removed text, and the notification and redact_apply's still_holding say where (Remove hidden information removes them); when the removed text couldn't be searched for, every such place holding any text is counted. 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 3e025b91436c45d5b515a2de058d44fb2dcb3733 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 2 Nov 2026 00:44:54 +0800 Subject: [PATCH 16/23] M8: the places applied redactions leave holding the removed text are an EditNotice (#2448) Main's `EditNotice` is "something the last edit did that the user should hear about", set by `Session::apply` and cleared by the next edit, undo and redo, which is what the PR's separate `redaction_elsewhere` field did by hand. It is now `EditNotice::RedactionElsewhere(Elsewhere)`, pushed when a place is counted or the text couldn't be searched for: tools list it in `warnings` with the other notices, the interface says it through the same path, and `Document::redaction_elsewhere()` gives `redact_apply` and the Apply dialog the counts. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/automation/src/redact.rs | 2 +- crates/automation/tests/it/automation.rs | 8 ++++ crates/engine/src/document.rs | 54 +++++++++++++++++++++--- crates/engine/src/open.rs | 1 - crates/engine/src/session/editing.rs | 8 ++-- crates/engine/src/tests.rs | 9 ++-- crates/ui-egui/src/dialogs.rs | 4 +- crates/ui-egui/src/editing.rs | 6 ++- 8 files changed, 71 insertions(+), 21 deletions(-) diff --git a/crates/automation/src/redact.rs b/crates/automation/src/redact.rs index baea3d75c5..4b95736613 100644 --- a/crates/automation/src/redact.rs +++ b/crates/automation/src/redact.rs @@ -166,7 +166,7 @@ impl Automation { out["applied"] = json!(before - after); out["marks_pending"] = json!(after); // Where the removed text is still held that applying leaves as it is. - let left = self.doc(a)?.redaction_elsewhere.unwrap_or_default(); + let left = self.doc(a)?.redaction_elsewhere().unwrap_or_default(); out["still_holding"] = json!({ "bookmarks": left.bookmarks, "attachments": left.attachments, diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index bf2d1b3ceb..653c18c4bf 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -4365,6 +4365,14 @@ fn redact_apply_reports_places_that_still_hold_the_removed_text() { ok(&mut a, "redact_mark", json!({ "doc": doc, "find": "Bluefin" })); let r = ok(&mut a, "redact_apply", json!({ "doc": doc })); assert_eq!(r["still_holding"], json!({ "bookmarks": 1, "attachments": 0, "comments": 1, "xmp": 0, "total": 2, "unsearched": false }), "{r}"); + // Said in the warnings too, with the other edit notices. + assert_eq!( + r["warnings"], + json!([ + "the redacted text also appears in 1 bookmark title and 1 comment, which redaction doesn't change (Remove hidden information removes them)" + ]), + "{r}" + ); assert!(!page_text(&mut a, doc)[0].contains("Bluefin")); let titles: Vec = ok(&mut a, "bookmark_list", json!({ "doc": doc }))["bookmarks"] .as_array() diff --git a/crates/engine/src/document.rs b/crates/engine/src/document.rs index fa0bedafad..863f370a72 100644 --- a/crates/engine/src/document.rs +++ b/crates/engine/src/document.rs @@ -60,14 +60,10 @@ pub struct Document { /// (also in `info.warnings`, kept there when the document is re-read). pub xfa_warnings: Vec, /// What the last successful [`Session::apply`](crate::Session::apply) wants said (see - /// [`EditNotice`]): text its fonts can't draw, pages whose own backdrop stays off. The - /// interface shows it; tools return it. + /// [`EditNotice`]): text its fonts can't draw, pages whose own backdrop stays off, places + /// applied redactions left that still hold the removed text. The interface shows it; tools + /// return it. pub edit_notices: Vec, - /// What the last successful [`Session::apply`](crate::Session::apply), when it applied redactions, - /// found still holding the removed text in places applying leaves as they are (bookmark titles, - /// attachments, remaining comments, other XMP; see [`RedactionElsewhere`](crate::RedactionElsewhere)). - /// `None` after any other edit, an undo or a redo. The interface says so; tools return it. - pub redaction_elsewhere: Option, } /// Something the last successful edit did that the user should hear about, never left silent @@ -81,6 +77,11 @@ pub enum EditNotice { /// turning it on could paint over something (`pdfcraft_edit::backdrops_left_off`). They look /// the same on white paper. BackdropsLeftOff { pages: Vec }, + /// Applying redactions left these places as they were, and they still hold the removed text + /// (bookmark titles, attachments, the comments that remain, XMP on other objects; see + /// [`RedactionElsewhere`](crate::RedactionElsewhere)). Only when one is counted, or the text + /// couldn't be searched for. + RedactionElsewhere(pdfcraft_redact::Elsewhere), } impl std::fmt::Display for EditNotice { @@ -95,10 +96,39 @@ impl std::fmt::Display for EditNotice { page_runs(pages) ) } + EditNotice::RedactionElsewhere(e) => elsewhere_text(e, f), } } } +/// [`EditNotice::RedactionElsewhere`] in words (English; the interface has its own wording). +fn elsewhere_text(e: &pdfcraft_redact::Elsewhere, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let count = |n: usize, one: &str, many: &str| match n { + 0 => None, + 1 => Some(format!("1 {one}")), + n => Some(format!("{n} {many}")), + }; + let places: Vec = [ + count(e.bookmarks, "bookmark title", "bookmark titles"), + count(e.attachments, "attachment", "attachments"), + count(e.comments, "comment", "comments"), + count(e.xmp, "other metadata stream", "other metadata streams"), + ] + .into_iter() + .flatten() + .collect(); + let list = match places.split_last() { + None => "no place counted".to_string(), + Some((last, [])) => last.clone(), + Some((last, rest)) => format!("{} and {last}", rest.join(", ")), + }; + if e.unsearched { + write!(f, "the redacted text couldn't be searched for, so {list} may still hold it (Remove hidden information removes them)") + } else { + write!(f, "the redacted text also appears in {list}, which redaction doesn't change (Remove hidden information removes them)") + } +} + /// 0-based pages as 1-based runs: `[0, 1, 2, 5]` is "1-3, 6". pub(crate) fn page_runs(pages: &[usize]) -> String { let mut runs: Vec<(usize, usize)> = Vec::new(); @@ -185,6 +215,16 @@ pub(crate) fn undrawn_notices(prev: &pdfcraft_cos::Document, next: &pdfcraft_cos } impl Document { + /// The places the last edit's applied redactions left that still hold the removed text + /// ([`EditNotice::RedactionElsewhere`]), or `None` when there are none or the last edit + /// applied none. + pub fn redaction_elsewhere(&self) -> Option { + self.edit_notices.iter().find_map(|n| match n { + EditNotice::RedactionElsewhere(e) => Some(*e), + _ => None, + }) + } + /// How the document opens (Document Properties ▸ Initial View). /// Accessibility ▸ Check for accessibility: the full check (`None` if the document can't be /// read for editing). diff --git a/crates/engine/src/open.rs b/crates/engine/src/open.rs index c43ac354d1..32db07abee 100644 --- a/crates/engine/src/open.rs +++ b/crates/engine/src/open.rs @@ -352,7 +352,6 @@ impl Session { fonts_stale: false, xfa_warnings: Vec::new(), edit_notices: Vec::new(), - redaction_elsewhere: None, } } } diff --git a/crates/engine/src/session/editing.rs b/crates/engine/src/session/editing.rs index d7e84f09f6..023ce138ab 100644 --- a/crates/engine/src/session/editing.rs +++ b/crates/engine/src/session/editing.rs @@ -26,7 +26,6 @@ impl Session { let xfa_time_scale = self.xfa_time_scale.unwrap_or(1); let doc = self.doc_mut(id)?; doc.edit_notices.clear(); - doc.redaction_elsewhere = None; let name = doc.name.clone(); let is_xfa = doc.info.xfa.is_some(); let mut scope = scope_of(&edit); @@ -182,7 +181,9 @@ impl Session { if signed && ((rewrites(&next) && !rewrites(&editor.cos)) || (next.is_modified() && next.revisions().is_empty())) { return Err(EditError::SignedRewrite(edit.label().to_string())); } - let notices = edit_notices(&edit, &editor.cos, &next); + let mut notices = edit_notices(&edit, &editor.cos, &next); + // Places applied redactions left that still hold the removed text: said, not silent. + notices.extend(cx.redaction_elsewhere.filter(|e| e.total() > 0 || e.unsearched).map(EditNotice::RedactionElsewhere)); let previous = std::mem::replace(&mut editor.cos, next); let keys = keys_after(&edit).unwrap_or_else(|| editor.keys.clone()); let previous_keys = std::mem::replace(&mut editor.keys, keys); @@ -211,7 +212,6 @@ impl Session { doc.dirty = true; doc.generation += 1; doc.edit_notices = notices; - doc.redaction_elsewhere = cx.redaction_elsewhere; if !cx.xfa_removed.is_empty() { let removed: Vec = cx.xfa_removed.iter().map(|n| format!("{XFA_REMOVED_PREFIX}{n}")).collect(); doc.xfa_warnings.retain(|w| w.starts_with(XFA_REMOVED_PREFIX)); @@ -257,7 +257,6 @@ impl Session { return Err(e); } doc.edit_notices.clear(); - doc.redaction_elsewhere = None; Self::sync_dirty(doc); doc.generation += 1; Ok(label) @@ -287,7 +286,6 @@ impl Session { return Err(e); } doc.edit_notices.clear(); - doc.redaction_elsewhere = None; Self::sync_dirty(doc); doc.generation += 1; Ok(label) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 8925bff358..91ad63e783 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2992,13 +2992,16 @@ fn applied_redactions_report_a_bookmark_that_still_holds_the_removed_text() { s.apply(id, Edit::AddBookmark { parent: vec![], index: 0, title: "Bluefin plans".into(), page: 0, left: None, top: None }).unwrap(); s.apply(id, Edit::AddBookmark { parent: vec![], index: 1, title: "Annual figures".into(), page: 0, left: None, top: None }).unwrap(); mark_shown_text(&mut s, id, "BLUEFIN"); - assert_eq!(s.get(id).unwrap().redaction_elsewhere, None, "only applying redactions reports it"); + assert_eq!(s.get(id).unwrap().redaction_elsewhere(), None, "only applying redactions reports it"); s.apply(id, Edit::ApplyRedactions { pages: None }).unwrap(); - let found = s.get(id).unwrap().redaction_elsewhere.expect("applying reports what it left"); + let found = s.get(id).unwrap().redaction_elsewhere().expect("applying reports what it left"); assert_eq!(found, crate::RedactionElsewhere { bookmarks: 1, ..Default::default() }); + // One of the edit notices, so tools and the interface say it with the others. + let said: Vec = s.get(id).unwrap().edit_notices.iter().map(ToString::to_string).collect(); + assert_eq!(said, ["the redacted text also appears in 1 bookmark title, which redaction doesn't change (Remove hidden information removes them)"]); assert_eq!(outline_titles(&s.get(id).unwrap().info.outline), ["Bluefin plans→1", "Annual figures→1"], "the bookmark is reported, not changed"); s.undo(id).unwrap(); - assert_eq!(s.get(id).unwrap().redaction_elsewhere, None, "an undo forgets it"); + assert_eq!(s.get(id).unwrap().redaction_elsewhere(), None, "an undo forgets it"); } /// A word whose font and size an ExtGState /Font entry (`gs`) sets is placed where the renderer diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index dc89a87a12..75f6e1463c 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1726,7 +1726,7 @@ fn finish_redactions_and_fields(app: &mut PdfCraftApp, req: &mut Pending) { }; // Where the removed text is still held that applying leaves as it is: said in the // same notification, so it can't replace the first sentence. - let left = app.active_ids().and_then(|(_, id)| app.session.get(id)).and_then(|d| d.redaction_elsewhere); + let left = app.active_ids().and_then(|(_, id)| app.session.get(id)).and_then(|d| d.redaction_elsewhere()); match left.and_then(|e| still_holding_note(&e)) { Some(note) => app.notify(format!("{applied} {note}")), None => app.notify(applied), @@ -1782,7 +1782,7 @@ fn finish_redactions_and_fields(app: &mut PdfCraftApp, req: &mut Pending) { /// The sentence telling the user where the removed text is still held that applying redactions /// leaves as it is, or `None` when it is nowhere else. When the text couldn't be searched for, /// the places are those holding any text, and the sentence says they may hold it. -fn still_holding_note(e: &pdfcraft_engine::RedactionElsewhere) -> Option { +pub(crate) fn still_holding_note(e: &pdfcraft_engine::RedactionElsewhere) -> Option { let count = |n: usize, one: &str, many: &str| match n { 0 => None, 1 => Some(tl!(one).to_string()), diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 8def569f58..517609d0fa 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -171,12 +171,14 @@ impl PdfCraftApp { self.quick_tool = crate::QuickTool::Select; } // Text saved as typed but drawn with `?` (its font can't draw it), or pages whose - // own backdrop stays off: said, not silent. Before the scripts' output, so an alert - // they raise still shows. + // own backdrop stays off, or places applied redactions left holding the removed text + // (the Apply redactions dialog then says it after its count): said, not silent. + // Before the scripts' output, so an alert they raise still shows. if let Some(n) = self.session.get(id).and_then(|d| d.edit_notices.first()).cloned() { self.notify(match &n { pdfcraft_engine::EditNotice::Undrawn(t) => crate::content_ui::undrawn_message(t), pdfcraft_engine::EditNotice::BackdropsLeftOff { pages } => crate::marks_ui::backdrops_left_off_message(pages), + pdfcraft_engine::EditNotice::RedactionElsewhere(e) => crate::dialogs::still_holding_note(e).unwrap_or_else(|| n.to_string()), }); } let out = self.session.take_js_output(id); From fdcc569094520e737c8a5d5b50826bc0890c7730 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 2 Nov 2026 00:51:48 +0800 Subject: [PATCH 17/23] M8: the metadata check also compares letters and digits alone, so spacing and TJ tracking can't hide the removed text (#2448) #2539 review: a heading tracked with TJ numbers ([(B) -300 (L) -300 ...] TJ) was collected as "B L U E F I N", which neither held "Bluefin" nor had a word of three letters, and a value spelled "Blue fin" or "BLUE-FIN" didn't hold the term "bluefin". A TJ gap can't be told from a word space, so the glyph ends stay as they are; instead every term with three letters or digits is also looked for by its letters and digits alone, and a value is part of a run when its letters and digits are, starting and ending at the run's word edges ("True" still isn't part of "untrue"). It over-matches only towards removal. Tests: a_heading_tracked_with_tj_numbers_matches_the_metadata, a_value_spaced_or_hyphenated_otherwise_matches (both failed before: /Title kept, metadata 0). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/redact/README.md | 8 +- crates/redact/src/metadata.rs | 107 +++++++++++++++---- crates/redact/src/tests.rs | 1 + crates/redact/src/tests/metadata_matching.rs | 37 +++++++ 4 files changed, 130 insertions(+), 23 deletions(-) create mode 100644 crates/redact/src/tests/metadata_matching.rs diff --git a/crates/redact/README.md b/crates/redact/README.md index 8c742f6cfe..fef032fa1b 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -42,7 +42,13 @@ clear_marks(&mut doc, None)?; // remove marks without app part of it, removed whole, together with any PDF/A identification in that packet. The removed glyphs' text is collected in runs by position (character and word spacing included, so letter-spaced text stays one word), and each run (two or more letters or digits) and each of - its words of three or more are matched without regard to case or spacing. A value that shares + its words of three or more are matched without regard to case or spacing, and also by their + letters and digits alone when they have three or more: "Blue fin" and "BLUE-FIN" hold + "Bluefin", and so does a heading whose letters `TJ` numbers spread apart (collected as + "B L U E F I N", since a `TJ` gap can't be told from a word space). A value is wholly part of + a run when its letters and digits are, starting and ending where the run's words do, so + "True" isn't part of "untrue". This over-matches only towards removal: "is a" also holds + "Lisa". A value that shares only words of one or two letters with the removed text, and isn't wholly part of it, stays: Sanitize's metadata category removes the rest. `Report::metadata` counts what went. - **Fail-closed.** When the check can't know what the removed text was, every `/Info` diff --git a/crates/redact/src/metadata.rs b/crates/redact/src/metadata.rs index 38dca290fd..2eafdb63c9 100644 --- a/crates/redact/src/metadata.rs +++ b/crates/redact/src/metadata.rs @@ -16,7 +16,9 @@ //! one filter (it is decoded strictly, so nothing after damage goes unsearched), goes; text too //! large to search against every term counts as matching, and when the removed text itself was cut short (too many runs or terms, //! or a run too long to keep) every `/Info` string and every XMP stream goes, since what was -//! never collected can't be searched for. Terms are matched without regard to case or spacing. +//! never collected can't be searched for. Terms are matched without regard to case or spacing, +//! and by their letters and digits alone, so "Blue fin", "BLUE-FIN" and a tracked heading +//! collected as "B L U E F I N" all match "Bluefin" (#2539 review). use std::collections::HashSet; @@ -41,6 +43,50 @@ fn significant(s: &str, min: usize) -> bool { s.chars().filter(|c| c.is_alphanumeric()).count() >= min } +/// The letters and digits of `s` alone: "blue-fin" and "b l u e f i n" are both "bluefin". +fn squeeze(s: &str) -> String { + s.chars().filter(|c| c.is_alphanumeric()).collect() +} + +/// A removed run (normalized), and its letters and digits alone with where its words begin and +/// end among them, so a value can be found in it whatever its spacing but still as whole words. +struct Run { + text: String, + letters: String, + /// `edges[i]`: byte `i` of `letters` begins a word (something other than a letter or digit + /// comes before it in the run, or nothing does), or `i` is the end. One entry per byte of + /// `letters`, and one for the end. + edges: Vec, +} + +impl Run { + fn new(text: String) -> Self { + let mut letters = String::new(); + let mut edges = Vec::new(); + let mut gap = true; + for c in text.chars() { + if c.is_alphanumeric() { + edges.resize(letters.len(), false); + edges.push(gap); + letters.push(c); + gap = false; + } else { + gap = true; + } + } + edges.resize(letters.len(), false); + edges.push(true); + Self { text, letters, edges } + } + + /// Are the letters and digits `value` (squeezed) among the run's, beginning and ending at + /// word edges? "bluefin" is in "b l u e f i n" and "bluef in"; "true" isn't in "untrue". + fn holds_letters(&self, value: &str) -> bool { + let edge = |i: usize| self.edges.get(i).copied().unwrap_or(false); + !value.is_empty() && self.letters.match_indices(value).any(|(at, m)| edge(at) && edge(at.saturating_add(m.len()))) + } +} + /// The terms to look for: each removed run (at least two letters or digits), and each of its /// words with at least three, so "Smith, John" in `/Author` is caught when "John Smith" was /// redacted. Shorter pieces would match nearly any text. `None` when a run is longer than @@ -67,6 +113,14 @@ pub(crate) fn terms(removed: &[String]) -> Option> { Some(out) } +/// The terms ([`terms`]) with three letters or digits or more, by their letters and digits +/// alone, each once: "blue fin" holds the term "bluefin" this way, and "bluefin" the run +/// "b l u e f i n". +fn squeezed_terms(terms: &[String]) -> Vec { + let mut seen = HashSet::new(); + terms.iter().filter(|t| significant(t, 3)).map(|t| squeeze(t)).filter(|t| seen.insert(t.clone())).collect() +} + /// The searching one scrub may do, across every `/Info` entry and XMP value: past it, each /// further question is answered "yes" (fail-closed), so a packet of many small values can't add /// up to more than one large one. @@ -92,28 +146,35 @@ impl Work { } } -fn mentions(text: &str, terms: &[String], work: &Work) -> bool { +/// Does `text` hold a term, as written or, for `squeezed` terms, by its letters and digits alone? +fn mentions(text: &str, terms: &[String], squeezed: &[String], work: &Work) -> bool { let text = normalize(text); - !work.spend(text.len().saturating_mul(terms.len())) || terms.iter().any(|t| text.contains(t.as_str())) + let letters = squeeze(&text); + !work.spend(text.len().saturating_mul(terms.len().saturating_add(squeezed.len()))) + || terms.iter().any(|t| text.contains(t.as_str())) + || squeezed.iter().any(|t| letters.contains(t.as_str())) } -/// Is `value` itself part of one of the removed `runs` (normalized), as whole words? It needs -/// three letters or digits, so an entry such as "by" doesn't go with every sentence that -/// contains it, and word boundaries, so an XMP value "True" doesn't go with "untrue". Too much -/// text to search counts as part (fail-closed). -fn within(value: &str, runs: &[String], work: &Work) -> bool { +/// Is `value` itself part of one of the removed `runs`, as whole words, compared as written or by +/// letters and digits alone ("Bluefin" is part of "b l u e f i n")? It needs three letters or +/// digits, so an entry such as "by" doesn't go with every sentence that contains it, and word +/// edges, so an XMP value "True" doesn't go with "untrue". Too much text to search counts as +/// part (fail-closed). +fn within(value: &str, runs: &[Run], work: &Work) -> bool { let value = normalize(value); if !significant(&value, 3) { return false; } - let total = runs.iter().map(String::len).fold(0usize, usize::saturating_add); + let letters = squeeze(&value); + let total = runs.iter().map(|r| r.text.len().saturating_add(r.letters.len())).fold(0usize, usize::saturating_add); let word = |c: Option| c.is_some_and(char::is_alphanumeric); - let whole = |r: &String| { - r.match_indices(value.as_str()).any(|(at, m)| { - let before = r.get(..at).and_then(|s| s.chars().next_back()); - let after = r.get(at.saturating_add(m.len())..).and_then(|s| s.chars().next()); - !(word(before) && value.starts_with(char::is_alphanumeric)) && !(word(after) && value.ends_with(char::is_alphanumeric)) - }) + let whole = |r: &Run| { + r.holds_letters(&letters) + || r.text.match_indices(value.as_str()).any(|(at, m)| { + let before = r.text.get(..at).and_then(|s| s.chars().next_back()); + let after = r.text.get(at.saturating_add(m.len())..).and_then(|s| s.chars().next()); + !(word(before) && value.starts_with(char::is_alphanumeric)) && !(word(after) && value.ends_with(char::is_alphanumeric)) + }) }; !work.spend(value.len().saturating_mul(total)) || runs.iter().any(whole) } @@ -297,8 +358,9 @@ enum Lookup { /// The removed text couldn't be collected or searched for in full: everything counts as /// holding it (fail-closed). Everything, - /// The terms ([`terms`]) and the distinct runs, normalized. - Terms(Vec, Vec), + /// The terms ([`terms`]), the same by their letters and digits alone ([`squeezed_terms`]), + /// and the distinct runs, normalized. + Terms(Vec, Vec, Vec), } impl Search { @@ -311,8 +373,9 @@ impl Search { Some(t) => { // The same run removed many times is searched for once. let mut seen = HashSet::new(); - let runs: Vec = removed.iter().map(|r| normalize(r)).filter(|r| seen.insert(r.clone())).collect(); - Lookup::Terms(t, runs) + let runs: Vec = removed.iter().map(|r| normalize(r)).filter(|r| seen.insert(r.clone())).map(Run::new).collect(); + let squeezed = squeezed_terms(&t); + Lookup::Terms(t, squeezed, runs) } None => Lookup::Everything, }; @@ -340,7 +403,7 @@ impl Search { match &self.lookup { Lookup::Nothing => false, Lookup::Everything => true, - Lookup::Terms(terms, runs) => mentions(text, terms, &self.work) || within(text, runs, &self.work), + Lookup::Terms(terms, squeezed, runs) => mentions(text, terms, squeezed, &self.work) || within(text, runs, &self.work), } } @@ -350,8 +413,8 @@ impl Search { match &self.lookup { Lookup::Nothing => false, Lookup::Everything => true, - Lookup::Terms(terms, runs) => xmp_values(doc, o) - .is_none_or(|values| mentions(&values.join(" "), terms, &self.work) || values.iter().any(|v| within(v, runs, &self.work))), + Lookup::Terms(terms, squeezed, runs) => xmp_values(doc, o) + .is_none_or(|values| mentions(&values.join(" "), terms, squeezed, &self.work) || values.iter().any(|v| within(v, runs, &self.work))), } } } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index c7e5334b4d..24fd6eeba8 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -3229,6 +3229,7 @@ mod elsewhere; mod inline_images; mod limits; mod metadata_fail_closed; +mod metadata_matching; mod property_retirement; mod surviving_references; mod text_state; diff --git a/crates/redact/src/tests/metadata_matching.rs b/crates/redact/src/tests/metadata_matching.rs new file mode 100644 index 0000000000..b29ed51893 --- /dev/null +++ b/crates/redact/src/tests/metadata_matching.rs @@ -0,0 +1,37 @@ +//! #2539 review (second round): spellings of the removed text the metadata check has to match. + +use super::*; + +fn dc_title(title: &str) -> String { + format!(r#"{title}"#) +} + +/// A tracked heading, its letters spread by `TJ` numbers rather than character spacing, is +/// collected as "B L U E F I N": its letters and digits still match `/Title (Bluefin)` and the +/// XMP title, while `/Producer` stays. +#[test] +fn a_heading_tracked_with_tj_numbers_matches_the_metadata() { + let content = b"BT /F1 10 Tf 10 100 Td [(B) -300 (L) -300 (U) -300 (E) -300 (F) -300 (I) -300 (N)] TJ ET"; + let mut doc = with_metadata(content, "<< /Title (Bluefin) /Producer (Test) >>", &dc_title("Bluefin")); + mark(&mut doc, 0, &[[5.0, 95.0, 200.0, 110.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!(r.glyphs, 7, "{r:?}"); + assert_eq!(info_keys(&doc), ["Producer"], "{r:?}"); + assert!(!has_xmp(&doc), "the XMP title named it"); + assert_eq!(r.metadata, 2, "{r:?}"); +} + +/// A value that spells the removed word with a space or a hyphen in it still holds it: "Blue fin" +/// and "BLUE-FIN" go when "BLUEFIN" is redacted, while "Bluefish", which only shares letters +/// with it, stays. +#[test] +fn a_value_spaced_or_hyphenated_otherwise_matches() { + let info = "<< /Title (Blue fin) /Keywords (tuna, BLUE-FIN) /Subject (Bluefish) /Producer (Test) >>"; + let mut doc = with_metadata(b"BT /F1 10 Tf 10 100 Td (Codename BLUEFIN report) Tj ET", info, &dc_title("Blue Fin")); + // "BLUEFIN" spans x 55-90. + mark(&mut doc, 0, &[[55.0, 95.0, 90.0, 110.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!(info_keys(&doc), ["Subject", "Producer"], "{r:?}"); + assert!(!has_xmp(&doc), "the XMP title spelled it with a space"); + assert_eq!(r.metadata, 3, "{r:?}"); +} From 38030a7d54513f632899cf1106ab90504d937cdf Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 2 Nov 2026 00:54:23 +0800 Subject: [PATCH 18/23] M8: the metadata check compares NFKC, lower-case text, so Unicode forms can't hide the removed text (#2448) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #2539 review: a decomposed "Müller" in a UTF-16BE /Title, a U+FB01 ligature from a ToUnicode map and full-width letters never matched their plain forms. Both sides are now normalized to NFKC and lower case (unicode-normalization, already in the build through pdfcraft-crypt's stringprep, so no new crate). Normalized text more than four times its input's length (U+FDFA and the like) isn't searched: it counts as holding the text. Tests: unicode_forms_of_the_removed_text_match (failed before: Title, Subject and Keywords kept), a_value_that_normalizes_to_far_more_text_counts_as_holding_it (failed with the growth limit raised to 100: 0 removed). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- Cargo.lock | 1 + crates/redact/Cargo.toml | 2 + crates/redact/README.md | 9 ++-- crates/redact/src/metadata.rs | 51 +++++++++++++++----- crates/redact/src/tests/metadata_matching.rs | 33 +++++++++++++ 5 files changed, 81 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fd186d995c..3cc808ab1c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4586,6 +4586,7 @@ dependencies = [ "pdfcraft-testkit", "pdfcraft-xfa", "thiserror 2.0.21", + "unicode-normalization", ] [[package]] diff --git a/crates/redact/Cargo.toml b/crates/redact/Cargo.toml index 8d1515d8a4..10cb97ccde 100644 --- a/crates/redact/Cargo.toml +++ b/crates/redact/Cargo.toml @@ -18,6 +18,8 @@ pdfcraft-edit = { workspace = true } pdfcraft-annot = { workspace = true } pdfcraft-xfa = { workspace = true } thiserror = { workspace = true } +# NFKC for the metadata check; already in the build through pdfcraft-crypt (stringprep). +unicode-normalization = "0.1" [dev-dependencies] pdfcraft-filters = { workspace = true } diff --git a/crates/redact/README.md b/crates/redact/README.md index fef032fa1b..129602287c 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -48,8 +48,10 @@ clear_marks(&mut doc, None)?; // remove marks without app "B L U E F I N", since a `TJ` gap can't be told from a word space). A value is wholly part of a run when its letters and digits are, starting and ending where the run's words do, so "True" isn't part of "untrue". This over-matches only towards removal: "is a" also holds - "Lisa". A value that shares - only words of one or two letters with the removed text, and isn't wholly part of it, stays: + "Lisa". Both sides are compared in Unicode compatibility form (NFKC) and lower case, so a + decomposed "Müller" (u and U+0308), full-width letters and a "fi" ligature from a + `ToUnicode` map match their plain forms. A value that shares only words of one or two + letters with the removed text, and isn't wholly part of it, stays: Sanitize's metadata category removes the rest. `Report::metadata` counts what went. - **Fail-closed.** When the check can't know what the removed text was, every `/Info` string and every catalog and page XMP stream goes: more than 100,000 runs or a run over @@ -60,7 +62,8 @@ clear_marks(&mut doc, None)?; // remove marks without app search's work limit (2^28 steps) runs out, every entry and packet not yet checked goes too. An XMP packet that isn't valid UTF-8 or UTF-16, can't be decoded strictly (damaged or cut-short filter data, or more than one filter), or decodes to more than - 16 MiB goes without being read. + 16 MiB goes without being read, and a value whose normalized form would be more than four + times as long (U+FDFA alone is 18 letters) counts as holding the text. - **Not scrubbed, but reported** (Remove hidden information's categories remove them): bookmark titles, attachments (names in the document's list of embedded files, file names and descriptions, and the files of file attachment comments), the comments that remain (their diff --git a/crates/redact/src/metadata.rs b/crates/redact/src/metadata.rs index 2eafdb63c9..9c6d435d27 100644 --- a/crates/redact/src/metadata.rs +++ b/crates/redact/src/metadata.rs @@ -23,6 +23,7 @@ use std::collections::HashSet; use pdfcraft_cos::{Document, ObjRef, Object}; +use unicode_normalization::UnicodeNormalization; /// Terms longer than this are cut (matching still finds the prefix). const MAX_TERM_CHARS: usize = 4096; @@ -33,10 +34,34 @@ pub(crate) const MAX_XMP_BYTES: usize = 16 * 1024 * 1024; /// do in all; past it every entry and packet counts as matching (fail-closed), so a page of /// removed text against a large packet, or against very many small values, stays quick. const MAX_WORK: usize = 1 << 28; +/// Normalized text longer than this many times its input (plus 64 bytes) isn't searched: it +/// counts as matching. Ordinary text barely changes length; a few compatibility characters +/// (U+FDFA, the square katakana words) decompose to many. +const MAX_GROWTH: usize = 4; -/// Lower-case `s` with every run of whitespace made one space, trimmed. -fn normalize(s: &str) -> String { - s.split_whitespace().flat_map(|w| w.chars().flat_map(char::to_lowercase).chain(std::iter::once(' '))).collect::().trim_end().to_string() +/// `s` in Unicode compatibility form (NFKC), lower case, every run of whitespace made one space, +/// trimmed: "Mu\u{308}ller" (decomposed), "ＭÜＬＬＥＲ" (full width) and "Müller" are all +/// "müller", and the ligature "fi" is "fi" (#2539 review). `None` when it would grow past +/// [`MAX_GROWTH`] times `s`. +fn normalize(s: &str) -> Option { + let cap = s.len().saturating_mul(MAX_GROWTH).saturating_add(64); + let mut out = String::new(); + let mut space = false; + for c in s.nfkc().flat_map(char::to_lowercase).nfkc() { + if c.is_whitespace() { + space = !out.is_empty(); + continue; + } + if space { + out.push(' '); + space = false; + } + out.push(c); + if out.len() > cap { + return None; + } + } + Some(out) } fn significant(s: &str, min: usize) -> bool { @@ -90,13 +115,14 @@ impl Run { /// The terms to look for: each removed run (at least two letters or digits), and each of its /// words with at least three, so "Smith, John" in `/Author` is caught when "John Smith" was /// redacted. Shorter pieces would match nearly any text. `None` when a run is longer than -/// [`MAX_TERM_CHARS`] or there are more than [`MAX_TERMS`] terms: the caller then can't search +/// [`MAX_TERM_CHARS`] (or grows past [`MAX_GROWTH`] when normalized) or there are more than +/// [`MAX_TERMS`] terms: the caller then can't search /// for all of it and must clear the metadata instead. pub(crate) fn terms(removed: &[String]) -> Option> { let mut seen = HashSet::new(); let mut out = Vec::new(); for run in removed { - let run = normalize(run); + let run = normalize(run)?; if run.chars().nth(MAX_TERM_CHARS).is_some() { return None; } @@ -148,7 +174,7 @@ impl Work { /// Does `text` hold a term, as written or, for `squeezed` terms, by its letters and digits alone? fn mentions(text: &str, terms: &[String], squeezed: &[String], work: &Work) -> bool { - let text = normalize(text); + let Some(text) = normalize(text) else { return true }; let letters = squeeze(&text); !work.spend(text.len().saturating_mul(terms.len().saturating_add(squeezed.len()))) || terms.iter().any(|t| text.contains(t.as_str())) @@ -161,7 +187,7 @@ fn mentions(text: &str, terms: &[String], squeezed: &[String], work: &Work) -> b /// edges, so an XMP value "True" doesn't go with "untrue". Too much text to search counts as /// part (fail-closed). fn within(value: &str, runs: &[Run], work: &Work) -> bool { - let value = normalize(value); + let Some(value) = normalize(value) else { return true }; if !significant(&value, 3) { return false; } @@ -368,16 +394,17 @@ impl Search { /// tests). `complete` is false when the collection of removed text was cut short; then, as /// when the terms exceed their limits, everything matches. pub(crate) fn new(removed: &[String], complete: bool, limit: usize) -> Self { - let lookup = match terms(removed).filter(|_| complete) { - Some(t) if t.is_empty() => Lookup::Nothing, - Some(t) => { + let runs: Option> = removed.iter().map(|r| normalize(r)).collect(); + let lookup = match (terms(removed).filter(|_| complete), runs) { + (Some(t), _) if t.is_empty() => Lookup::Nothing, + (Some(t), Some(runs)) => { // The same run removed many times is searched for once. let mut seen = HashSet::new(); - let runs: Vec = removed.iter().map(|r| normalize(r)).filter(|r| seen.insert(r.clone())).map(Run::new).collect(); + let runs: Vec = runs.into_iter().filter(|r| seen.insert(r.clone())).map(Run::new).collect(); let squeezed = squeezed_terms(&t); Lookup::Terms(t, squeezed, runs) } - None => Lookup::Everything, + _ => Lookup::Everything, }; Self { lookup, work: Work::new(limit) } } diff --git a/crates/redact/src/tests/metadata_matching.rs b/crates/redact/src/tests/metadata_matching.rs index b29ed51893..d944dac766 100644 --- a/crates/redact/src/tests/metadata_matching.rs +++ b/crates/redact/src/tests/metadata_matching.rs @@ -35,3 +35,36 @@ fn a_value_spaced_or_hyphenated_otherwise_matches() { assert!(!has_xmp(&doc), "the XMP title spelled it with a space"); assert_eq!(r.metadata, 3, "{r:?}"); } + +/// `s` as a UTF-16BE text string in hex, byte order mark first. +fn utf16be(s: &str) -> String { + format!("", s.encode_utf16().map(|u| format!("{u:04X}")).collect::()) +} + +/// The removed text and the metadata are compared after Unicode compatibility normalization +/// (NFKC) and lower case on both sides: a decomposed "Müller" (u + U+0308) in a UTF-16BE `/Title` +/// matches the composed one removed from the page, a "fi" ligature from a `ToUnicode` map matches +/// "fi", and full-width letters match their ASCII forms. +#[test] +fn unicode_forms_of_the_removed_text_match() { + let info = format!( + "<< /Title {} /Subject (Finance plan) /Keywords {} /Author (Ada Lovelace) /Producer (Test) >>", + utf16be("Mu\u{308}ller"), + utf16be("\u{FF22}\u{FF2C}\u{FF35}\u{FF25}\u{FF26}\u{FF29}\u{FF2E}") + ); + let mut doc = with_metadata(b"BT /F1 10 Tf 10 100 Td (x) Tj ET", &info, ""); + let removed = ["M\u{FC}ller".to_string(), "\u{FB01}nance".to_string(), "Bluefin".to_string()]; + let n = metadata::scrub_within(&mut doc, &removed, true, 1 << 28).unwrap(); + assert_eq!(info_keys(&doc), ["Author", "Producer"]); + assert_eq!(n, 3); +} + +/// A value whose normalized form would be far longer than itself (U+FDFA is 18 letters in NFKC) +/// isn't normalized in full: it counts as holding the removed text (fail-closed). +#[test] +fn a_value_that_normalizes_to_far_more_text_counts_as_holding_it() { + let info = format!("<< /Subject {} /Producer (Test) >>", utf16be(&"\u{FDFA}".repeat(50))); + let mut doc = with_metadata(b"BT /F1 10 Tf 10 100 Td (x) Tj ET", &info, ""); + assert_eq!(metadata::scrub_within(&mut doc, &["Bluefin".into()], true, 1 << 28).unwrap(), 1); + assert_eq!(info_keys(&doc), ["Producer"]); +} From 17a934506452e3c2f88e72be8ddba309fbcd8eb9 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 2 Nov 2026 00:56:20 +0800 Subject: [PATCH 19/23] M8: an FF FE text string is searched as UTF-16LE, in /Info and the places applying leaves (#2448) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #2539 review: ISO 32000-2 §7.9.2.2.1 defines text strings in PDFDocEncoding, UTF-16BE (FE FF) and UTF-8 (EF BB BF) only, so an FF FE string was read through PDFDocEncoding and Cyrillic or CJK text in it could never match. It is now read as UTF-16LE for the search (fail-closed), in the /Info scrub and in the bookmark, attachment and comment counts. Test: a_utf16le_string_is_searched_as_utf16le (failed before: 0 /Info entries removed; with only the count reverted, bookmarks 0 instead of 1). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/redact/README.md | 4 ++- crates/redact/src/elsewhere.rs | 7 ++--- crates/redact/src/metadata.rs | 17 +++++++++++- crates/redact/src/tests/metadata_matching.rs | 28 ++++++++++++++++++++ 4 files changed, 51 insertions(+), 5 deletions(-) diff --git a/crates/redact/README.md b/crates/redact/README.md index 129602287c..2cba145390 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -50,7 +50,9 @@ clear_marks(&mut doc, None)?; // remove marks without app "True" isn't part of "untrue". This over-matches only towards removal: "is a" also holds "Lisa". Both sides are compared in Unicode compatibility form (NFKC) and lower case, so a decomposed "Müller" (u and U+0308), full-width letters and a "fi" ligature from a - `ToUnicode` map match their plain forms. A value that shares only words of one or two + `ToUnicode` map match their plain forms. A string behind an `FF FE` mark, which ISO 32000-2 + §7.9.2.2.1 doesn't define, is read as UTF-16LE, here and in the places reported below, rather + than through PDFDocEncoding, in which it could never match. A value that shares only words of one or two letters with the removed text, and isn't wholly part of it, stays: Sanitize's metadata category removes the rest. `Report::metadata` counts what went. - **Fail-closed.** When the check can't know what the removed text was, every `/Info` diff --git a/crates/redact/src/elsewhere.rs b/crates/redact/src/elsewhere.rs index 901775ac1b..4ebac7bad9 100644 --- a/crates/redact/src/elsewhere.rs +++ b/crates/redact/src/elsewhere.rs @@ -64,8 +64,9 @@ pub(crate) fn find(doc: &Document, removed: &[String], complete: bool) -> Elsewh return Elsewhere::default(); } // A blank value holds nothing, even when everything counts. - let hit = - |o: Option<&Object>| o.and_then(|o| doc.resolve(o).as_string().map(|s| s.to_text())).is_some_and(|t| !t.trim().is_empty() && search.text(&t)); + let hit = |o: Option<&Object>| { + o.and_then(|o| doc.resolve(o).as_string().map(crate::metadata::text_string)).is_some_and(|t| !t.trim().is_empty() && search.text(&t)) + }; Elsewhere { bookmarks: bookmarks(doc, &hit), attachments: attachments(doc, &hit), @@ -182,7 +183,7 @@ fn comments(doc: &Document, search: &Search, hit: &dyn Fn(Option<&Object>) -> bo /// can't be read (strictly, as UTF-8, within the XMP limit). fn rich_text(doc: &Document, o: &Object) -> Option { let xml = match &*doc.resolve(o) { - Object::String(s) => s.to_text(), + Object::String(s) => crate::metadata::text_string(s), Object::Stream(s) if s.filters().len() <= 1 => String::from_utf8(s.decoded_first_filter_strict(crate::metadata::MAX_XMP_BYTES).ok()?).ok()?, Object::Stream(_) => return None, _ => return Some(String::new()), diff --git a/crates/redact/src/metadata.rs b/crates/redact/src/metadata.rs index 9c6d435d27..18cff41907 100644 --- a/crates/redact/src/metadata.rs +++ b/crates/redact/src/metadata.rs @@ -358,6 +358,21 @@ pub(crate) fn decode_xmp(bytes: &[u8]) -> Option { } } +/// The text of a text string, as searched for the removed text. ISO 32000-2 §7.9.2.2.1 defines +/// PDFDocEncoding, UTF-16BE (`FE FF`) and UTF-8 (`EF BB BF`) only, but a string behind an `FF FE` +/// mark is UTF-16LE to any reader that takes it as such, and read through PDFDocEncoding it +/// could never match: it is read as UTF-16LE (an odd last byte is left out, a lone surrogate +/// becomes U+FFFD), so the search stays fail-closed (#2539 review). +pub(crate) fn text_string(s: &pdfcraft_cos::PdfString) -> String { + match s.bytes.as_slice() { + [0xFF, 0xFE, rest @ ..] => { + let units = rest.as_chunks::<2>().0.iter().map(|&p| u16::from_le_bytes(p)); + char::decode_utf16(units).map(|c| c.unwrap_or(char::REPLACEMENT_CHARACTER)).collect() + } + _ => s.to_text(), + } +} + /// The text values of an XMP stream, or `None` when it can't be read. fn xmp_values(doc: &Document, o: &Object) -> Option> { let Object::Stream(s) = &*doc.resolve(o) else { return None }; @@ -466,7 +481,7 @@ pub(crate) fn scrub_within(doc: &mut Document, removed: &[String], complete: boo let info_obj = doc.trailer().get(b"Info").cloned(); if let Some(info) = info_obj.as_ref().and_then(|i| doc.resolve(i).as_dict().cloned()) { let doomed: Vec> = - info.iter().filter(|(_, v)| doc.resolve(v).as_string().is_some_and(|s| search.text(&s.to_text()))).map(|(k, _)| k.to_vec()).collect(); + info.iter().filter(|(_, v)| doc.resolve(v).as_string().is_some_and(|s| search.text(&text_string(s)))).map(|(k, _)| k.to_vec()).collect(); if !doomed.is_empty() { n += doomed.len(); let mut kept = info.clone(); diff --git a/crates/redact/src/tests/metadata_matching.rs b/crates/redact/src/tests/metadata_matching.rs index d944dac766..441ad104a3 100644 --- a/crates/redact/src/tests/metadata_matching.rs +++ b/crates/redact/src/tests/metadata_matching.rs @@ -68,3 +68,31 @@ fn a_value_that_normalizes_to_far_more_text_counts_as_holding_it() { assert_eq!(metadata::scrub_within(&mut doc, &["Bluefin".into()], true, 1 << 28).unwrap(), 1); assert_eq!(info_keys(&doc), ["Producer"]); } + +/// `s` as UTF-16LE bytes in hex, behind an `FF FE` byte order mark. +fn utf16le(s: &str) -> String { + format!("", s.encode_utf16().map(|u| format!("{:02X}{:02X}", u & 0xFF, u >> 8)).collect::()) +} + +/// ISO 32000-2 §7.9.2.2.1 defines text strings in PDFDocEncoding, UTF-16BE (`FE FF`) and UTF-8 +/// (`EF BB BF`) only, but a string behind an `FF FE` mark is UTF-16LE to anyone who reads it as +/// such; read through PDFDocEncoding, Cyrillic or CJK text in it could never match. It is +/// searched as UTF-16LE in `/Info` and in the places applying leaves (here a bookmark title). +#[test] +fn a_utf16le_string_is_searched_as_utf16le() { + let info = format!("<< /Title {} /Subject {} /Producer (Test) >>", utf16le("Синий план"), utf16le("Годовой отчёт")); + let mut doc = with_metadata(b"BT /F1 10 Tf 10 100 Td (x) Tj ET", &info, ""); + assert_eq!(metadata::scrub_within(&mut doc, &["СИНИЙ".into()], true, 1 << 28).unwrap(), 1); + assert_eq!(info_keys(&doc), ["Subject", "Producer"]); + + // A bookmark titled the same way is counted as still holding it. + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /Outlines 4 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] >>".to_vec(), + b"<< /Type /Outlines /First 5 0 R /Last 5 0 R /Count 1 >>".to_vec(), + format!("<< /Title {} /Parent 4 0 R /Dest [3 0 R /Fit] >>", utf16le("Синий план")).into_bytes(), + ]; + let doc = Document::open(Arc::new(Pdf::new().objs(objs).build())).unwrap(); + assert_eq!(crate::elsewhere::find(&doc, &["СИНИЙ".into()], true), Elsewhere { bookmarks: 1, ..Default::default() }); +} From 6ce5b5fe4312e67283812e3224d1df9d8a7315c6 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 2 Nov 2026 01:16:34 +0800 Subject: [PATCH 20/23] M8: a removed document XMP packet's PDF/A and PDF/UA identification is written back, or reported lost (#2448) #2539 review: the scrub removes the catalog's XMP packet whole when it holds the removed text, and with it the file's PDF/A and PDF/UA claims, which Report didn't say. The identification (pdfaid part, conformance, amd, rev; pdfuaid part, rev) is now read by namespace (roxmltree, already in the build through pdfcraft-xfa), each value kept only when it is the short code it must be and doesn't hold the removed text (one-character codes always), and written back in a packet holding nothing else, with a fixed pdfuaid extension schema under PDF/A-1 to -3 (left out, with the PDF/UA claim, if the redacted words are among its words). Report::identification says, per standard, None / Kept / Lost; the engine passes it on as EditNotice::RedactionIdentification, the Apply notification says it after the count, and redact_apply returns identification { pdfa, pdfua }. Tests (redact): the_identification_of_a_removed_packet_is_written_back, a_packet_without_identification_is_only_removed, an_identification_that_cannot_be_kept_is_reported_lost, unsearched_text_keeps_only_one_character_codes, the_extension_schema_is_left_out_when_it_uses_a_redacted_word; engine applied_redactions_report_the_pdfa_identification_written_back; automation redact_apply_reports_the_identification_written_back; ui-egui applying_says_the_pdfa_identification_was_written_back. With the write-back replaced by "nothing kept", the redact tests fail (Kept expected, None found); with the notice not pushed, the engine, automation and UI tests fail. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- Cargo.lock | 1 + crates/automation/src/redact.rs | 8 + crates/automation/src/tools.rs | 2 +- crates/automation/tests/it/automation.rs | 34 ++++ crates/engine/src/document.rs | 31 +++ crates/engine/src/edit/run.rs | 5 + crates/engine/src/lib.rs | 1 + crates/engine/src/session/editing.rs | 6 +- crates/engine/src/tests.rs | 32 +++ crates/redact/Cargo.toml | 2 + crates/redact/README.md | 12 +- crates/redact/src/identification.rs | 204 +++++++++++++++++++ crates/redact/src/lib.rs | 9 +- crates/redact/src/metadata.rs | 46 ++++- crates/redact/src/tests.rs | 5 +- crates/redact/src/tests/identification.rs | 123 +++++++++++ crates/redact/src/tests/metadata_matching.rs | 6 +- crates/ui-egui/src/dialogs.rs | 42 +++- crates/ui-egui/src/editing.rs | 3 + crates/ui-egui/tests/it/redact.rs | 53 +++++ 20 files changed, 601 insertions(+), 24 deletions(-) create mode 100644 crates/redact/src/identification.rs create mode 100644 crates/redact/src/tests/identification.rs diff --git a/Cargo.lock b/Cargo.lock index 3cc808ab1c..846b85418e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4585,6 +4585,7 @@ dependencies = [ "pdfcraft-model", "pdfcraft-testkit", "pdfcraft-xfa", + "roxmltree 0.21.2", "thiserror 2.0.21", "unicode-normalization", ] diff --git a/crates/automation/src/redact.rs b/crates/automation/src/redact.rs index 4b95736613..70e4cb73b2 100644 --- a/crates/automation/src/redact.rs +++ b/crates/automation/src/redact.rs @@ -175,6 +175,14 @@ impl Automation { "total": left.total(), "unsearched": left.unsearched, }); + // What became of a PDF/A or PDF/UA claim in the document XMP the scrub removed. + let claim = |c: pdfcraft_engine::RedactionClaim| match c { + pdfcraft_engine::RedactionClaim::None => "none", + pdfcraft_engine::RedactionClaim::Kept => "kept", + pdfcraft_engine::RedactionClaim::Lost => "lost", + }; + let id = self.doc(a)?.redaction_identification().unwrap_or_default(); + out["identification"] = json!({ "pdfa": claim(id.pdfa), "pdfua": claim(id.pdfua) }); Ok(out) } diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index 0f1b14b9a3..aa30801bfe 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -545,7 +545,7 @@ pub fn tools() -> Vec { t( "redact_apply", "Apply redactions", - "Apply the redaction marks (all, or those on pages): text, images, vectors, comments and form fields under them are removed for good and boxes are drawn in their place. A verification pass fails the operation if anything readable remains. Document metadata (Info entries, the document's and pages' XMP) that repeats the removed text is removed too. still_holding counts the places applying leaves as they are that still hold the removed text: bookmarks (titles), attachments (names, file names, descriptions), comments (text, rich text, subject or author of those that remain) and xmp (XMP on images, fonts, forms, annotations and other objects), with their total; doc_remove_hidden removes them. unsearched: true means the removed text couldn't be searched for, so every such place holding any text is counted. Undoable until saved; the saved file no longer contains the content.", + "Apply the redaction marks (all, or those on pages): text, images, vectors, comments and form fields under them are removed for good and boxes are drawn in their place. A verification pass fails the operation if anything readable remains. Document metadata (Info entries, the document's and pages' XMP) that repeats the removed text is removed too; when the document XMP goes, identification says what became of its PDF/A and PDF/UA claims (pdfa, pdfua: none, kept when written back in a packet holding only the identification, or lost). still_holding counts the places applying leaves as they are that still hold the removed text: bookmarks (titles), attachments (names, file names, descriptions), comments (text, rich text, subject or author of those that remain) and xmp (XMP on images, fonts, forms, annotations and other objects), with their total; doc_remove_hidden removes them. unsearched: true means the removed text couldn't be searched for, so every such place holding any text is counted. Undoable until saved; the saved file no longer contains the content.", ) .destructive() .with(schema(json!({ "doc": doc(), "pages": pages("whose marks to apply (default: all)") }), &["doc"])), diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index 653c18c4bf..133e31022b 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -4381,6 +4381,40 @@ fn redact_apply_reports_places_that_still_hold_the_removed_text() { .map(|b| b["title"].as_str().unwrap().to_string()) .collect(); assert_eq!(titles, ["Bluefin plans", "Annual figures"], "reported, not changed"); + assert_eq!(r["identification"], json!({ "pdfa": "none", "pdfua": "none" }), "{r}"); +} + +/// #2539 review: when `redact_apply` removes the document's XMP packet because it names the +/// redacted word, `identification` says the PDF/A-2b and PDF/UA-1 identification it held was +/// written back, and the warnings say it too. +#[test] +fn redact_apply_reports_the_identification_written_back() { + let dir = workdir("redact-identification"); + let xmp = r#"Bluefin report"#; + let body = "BT /F1 18 Tf 20 150 Td (Codename BLUEFIN report) Tj ET"; + let content = format!("<< /Length {} >>\nstream\n{body}\nendstream", body.len()); + let metadata = format!("<< /Type /Metadata /Subtype /XML /Length {} >>\nstream\n{xmp}\nendstream", xmp.len()); + let objs = [ + "<< /Type /Catalog /Pages 2 0 R /Metadata 6 0 R >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Resources << /Font << /F1 4 0 R >> >> /Contents 5 0 R >>", + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>", + &content, + &metadata, + ]; + std::fs::write(dir.join("pdfa.pdf"), pdf_of(&objs)).unwrap(); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({ "path": "pdfa.pdf" }))["doc"].as_u64().unwrap(); + ok(&mut a, "redact_mark", json!({ "doc": doc, "find": "BLUEFIN" })); + let r = ok(&mut a, "redact_apply", json!({ "doc": doc })); + assert_eq!(r["identification"], json!({ "pdfa": "kept", "pdfua": "kept" }), "{r}"); + assert_eq!( + r["warnings"], + json!([ + "the document's XMP metadata held the redacted text and was removed; its PDF/A and PDF/UA identification was written back on its own" + ]), + "{r}" + ); } #[test] diff --git a/crates/engine/src/document.rs b/crates/engine/src/document.rs index 863f370a72..da921f7ccc 100644 --- a/crates/engine/src/document.rs +++ b/crates/engine/src/document.rs @@ -82,6 +82,16 @@ pub enum EditNotice { /// [`RedactionElsewhere`](crate::RedactionElsewhere)). Only when one is counted, or the text /// couldn't be searched for. RedactionElsewhere(pdfcraft_redact::Elsewhere), + /// Applying redactions removed the document's XMP packet, which held the removed text and + /// said the file conforms to PDF/A or PDF/UA: each claim was written back in a packet of its + /// own, or lost (see [`RedactionIdentification`](crate::RedactionIdentification)). + RedactionIdentification(pdfcraft_redact::Identification), +} + +/// The standards whose identification `id` has as `claim`: "PDF/A", "PDF/UA" or both, joined. +pub(crate) fn standards_with(id: &pdfcraft_redact::Identification, claim: pdfcraft_redact::Claim) -> Option { + let names: Vec<&str> = [("PDF/A", id.pdfa), ("PDF/UA", id.pdfua)].into_iter().filter(|(_, c)| *c == claim).map(|(n, _)| n).collect(); + (!names.is_empty()).then(|| names.join(" and ")) } impl std::fmt::Display for EditNotice { @@ -97,6 +107,17 @@ impl std::fmt::Display for EditNotice { ) } EditNotice::RedactionElsewhere(e) => elsewhere_text(e, f), + EditNotice::RedactionIdentification(id) => { + use pdfcraft_redact::Claim; + write!(f, "the document's XMP metadata held the redacted text and was removed")?; + if let Some(kept) = standards_with(id, Claim::Kept) { + write!(f, "; its {kept} identification was written back on its own")?; + } + match standards_with(id, Claim::Lost) { + Some(lost) => write!(f, "; its {lost} identification couldn't be kept, so the file no longer says it conforms to {lost}"), + None => Ok(()), + } + } } } } @@ -225,6 +246,16 @@ impl Document { }) } + /// What became of the PDF/A and PDF/UA identification when the last edit's applied + /// redactions removed the document's XMP packet ([`EditNotice::RedactionIdentification`]), + /// or `None` when no packet that made such a claim was removed. + pub fn redaction_identification(&self) -> Option { + self.edit_notices.iter().find_map(|n| match n { + EditNotice::RedactionIdentification(id) => Some(*id), + _ => None, + }) + } + /// How the document opens (Document Properties ▸ Initial View). /// Accessibility ▸ Check for accessibility: the full check (`None` if the document can't be /// read for editing). diff --git a/crates/engine/src/edit/run.rs b/crates/engine/src/edit/run.rs index 89e3e7b176..17841128d7 100644 --- a/crates/engine/src/edit/run.rs +++ b/crates/engine/src/edit/run.rs @@ -187,6 +187,9 @@ pub(crate) struct EditCtx { pub(crate) redaction_overlays: Vec, /// What the edit's applied redactions left as it was that still holds the removed text. pub(crate) redaction_elsewhere: Option, + /// What became of the PDF/A and PDF/UA identification when the edit's applied redactions + /// removed the catalog's XMP packet. + pub(crate) redaction_identification: pdfcraft_redact::Identification, } impl EditCtx { @@ -217,6 +220,7 @@ impl EditCtx { renamed_fields: Vec::new(), redaction_overlays: Vec::new(), redaction_elsewhere: None, + redaction_identification: pdfcraft_redact::Identification::default(), } } @@ -653,6 +657,7 @@ pub(crate) fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut E let report = pdfcraft_redact::apply(doc, pages.as_deref())?; cx.redaction_overlays = report.overlays; cx.redaction_elsewhere = Some(report.still_holding); + cx.redaction_identification = report.identification; } Edit::ClearRedactions => { pdfcraft_redact::clear_marks(doc, None)?; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 200a79dc9a..69036d9d52 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -119,6 +119,7 @@ pub use pdfcraft_redact::patterns::{ MAX_WORDS as REDACT_MAX_WORDS, PATTERNS as REDACT_PATTERNS, Pattern as RedactPattern, find as find_pattern, word_list as redact_word_list, }; pub use pdfcraft_redact::sanitize::{HIDDEN, Hidden}; +pub use pdfcraft_redact::{Claim as RedactionClaim, Identification as RedactionIdentification}; pub use pdfcraft_sign as sign; pub use pdfcraft_sign::{SignOptions, SignatureInfo, Status as SignatureStatus, TrustStore}; pub use pdfcraft_xfdf::Format as DataFormat; diff --git a/crates/engine/src/session/editing.rs b/crates/engine/src/session/editing.rs index 023ce138ab..a8b07c73e1 100644 --- a/crates/engine/src/session/editing.rs +++ b/crates/engine/src/session/editing.rs @@ -182,8 +182,12 @@ impl Session { return Err(EditError::SignedRewrite(edit.label().to_string())); } let mut notices = edit_notices(&edit, &editor.cos, &next); - // Places applied redactions left that still hold the removed text: said, not silent. + // Places applied redactions left that still hold the removed text, and what became of + // the PDF/A or PDF/UA identification in the XMP they removed: said, not silent. notices.extend(cx.redaction_elsewhere.filter(|e| e.total() > 0 || e.unsearched).map(EditNotice::RedactionElsewhere)); + if cx.redaction_identification != pdfcraft_redact::Identification::default() { + notices.push(EditNotice::RedactionIdentification(cx.redaction_identification)); + } let previous = std::mem::replace(&mut editor.cos, next); let keys = keys_after(&edit).unwrap_or_else(|| editor.keys.clone()); let previous_keys = std::mem::replace(&mut editor.keys, keys); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 91ad63e783..71a212671c 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2903,6 +2903,11 @@ fn one_line_page_with(resources: &str, stream_entries: &str, body: &str) -> Vec< "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".to_string(), format!("<< /Length {} {stream_entries} >>\nstream\n{body}\nendstream", body.len()), ]; + pdf_of(&objs) +} + +/// A file of `objs`, numbered from 1, the first the catalog. +fn pdf_of(objs: &[String]) -> Vec { let mut out = b"%PDF-1.7\n".to_vec(); let mut offsets = Vec::new(); for (i, o) in objs.iter().enumerate() { @@ -3004,6 +3009,33 @@ fn applied_redactions_report_a_bookmark_that_still_holds_the_removed_text() { assert_eq!(s.get(id).unwrap().redaction_elsewhere(), None, "an undo forgets it"); } +/// #2539 review: applying redactions that remove the document's XMP packet, which named the +/// redacted word and said the file conforms to PDF/A-2b, writes the identification back and +/// says so; tools list it with the other edit notices, and an undo forgets it. +#[test] +fn applied_redactions_report_the_pdfa_identification_written_back() { + let xmp = r#"Bluefin report"#; + let body = "BT /F1 10 Tf 20 150 Td (Codename BLUEFIN report) Tj ET"; + let objs = [ + "<< /Type /Catalog /Pages 2 0 R /Metadata 6 0 R >>".to_string(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 300] >>".to_string(), + "<< /Type /Page /Parent 2 0 R /Resources << /Font << /F1 4 0 R >> >> /Contents 5 0 R >>".to_string(), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".to_string(), + format!("<< /Length {} >>\nstream\n{body}\nendstream", body.len()), + format!("<< /Type /Metadata /Subtype /XML /Length {} >>\nstream\n{xmp}\nendstream", xmp.len()), + ]; + let mut s = Session::new().with_clock(|| 1_700_000_000); + let id = s.open("pdfa.pdf", None, Arc::new(pdf_of(&objs)), None).unwrap(); + mark_shown_text(&mut s, id, "BLUEFIN"); + s.apply(id, Edit::ApplyRedactions { pages: None }).unwrap(); + let kept = crate::RedactionIdentification { pdfa: crate::RedactionClaim::Kept, pdfua: crate::RedactionClaim::None }; + assert_eq!(s.get(id).unwrap().redaction_identification(), Some(kept)); + let said: Vec = s.get(id).unwrap().edit_notices.iter().map(ToString::to_string).collect(); + assert_eq!(said, ["the document's XMP metadata held the redacted text and was removed; its PDF/A identification was written back on its own"]); + s.undo(id).unwrap(); + assert_eq!(s.get(id).unwrap().redaction_identification(), None, "an undo forgets it"); +} + /// 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/Cargo.toml b/crates/redact/Cargo.toml index 10cb97ccde..984e0ea34e 100644 --- a/crates/redact/Cargo.toml +++ b/crates/redact/Cargo.toml @@ -20,6 +20,8 @@ pdfcraft-xfa = { workspace = true } thiserror = { workspace = true } # NFKC for the metadata check; already in the build through pdfcraft-crypt (stringprep). unicode-normalization = "0.1" +# Reads the PDF/A and PDF/UA identification by namespace; already in the build through pdfcraft-xfa. +roxmltree = "0.21" [dev-dependencies] pdfcraft-filters = { workspace = true } diff --git a/crates/redact/README.md b/crates/redact/README.md index 2cba145390..8063f1a9f4 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -39,7 +39,17 @@ clear_marks(&mut doc, None)?; // remove marks without app part of it as whole words (three letters or digits at least), and the catalog's and pages' XMP streams whose text (element content, attribute values other than `xmlns` declarations, comments and processing instructions, entities decoded) contains it or has a value wholly - part of it, removed whole, together with any PDF/A identification in that packet. The removed + part of it, removed whole. When the catalog's packet goes, its PDF/A (`pdfaid`) and PDF/UA + (`pdfuaid`) identification, read by namespace, is written back in a packet holding nothing + else: the part number, the PDF/A conformance letter, amendment and revision, and PDF/UA's + part and revision, each kept only if it is the short code it must be and doesn't itself hold + the removed text (a one-character code can't, so it is kept even when the text couldn't be + searched for), plus a fixed extension schema for `pdfuaid` under PDF/A-1 to -3, left out + with the PDF/UA claim if the redacted words are among its words. `Report::identification` + says, for each, whether the claim was kept or lost (a value that isn't a valid code or holds + the text, or a packet that can't be read); the notification and `redact_apply` say it too. + `/Info` entries that remain aren't mirrored into the new packet, so a PDF/A validator may + still report them. The removed glyphs' text is collected in runs by position (character and word spacing included, so letter-spaced text stays one word), and each run (two or more letters or digits) and each of its words of three or more are matched without regard to case or spacing, and also by their diff --git a/crates/redact/src/identification.rs b/crates/redact/src/identification.rs new file mode 100644 index 0000000000..b509481adf --- /dev/null +++ b/crates/redact/src/identification.rs @@ -0,0 +1,204 @@ +//! The PDF/A and PDF/UA identification of a document XMP packet the metadata check removes +//! (#2539 review). +//! +//! The scrub removes the catalog's XMP packet whole when it holds the removed text, and the +//! packet is where a file says it conforms to PDF/A (ISO 19005, the `pdfaid` schema) or PDF/UA +//! (ISO 14289, `pdfuaid`). Rather than drop that claim silently, the scrub writes back a packet +//! holding only those identification properties, read by namespace (not by prefix), each +//! checked to be the short code it must be (a part number, a conformance letter, a year), and +//! none of them holding the removed text. Nothing else of the old packet is copied. What became +//! of each claim is reported ([`Identification`]). + +use pdfcraft_cos::{Dict, Document, Object, Stream}; + +use crate::metadata::Search; + +const PDFA_ID: &str = "http://www.aiim.org/pdfa/ns/id/"; +const PDFUA_ID: &str = "http://www.aiim.org/pdfua/ns/id/"; +/// Packets with more `=` than this (attributes, roughly) aren't parsed: the parser checks each +/// element's attributes against each other, so a huge attribute list would be slow. +const MAX_EQUALS: usize = 4096; +/// The most XML nodes parsed. +const MAX_NODES: u32 = 100_000; +/// The PDF/A extension schema describing `pdfuaid:part`, fixed text. +const PDFUA_EXTENSION: &str = concat!( + r#""#, + r#""#, + r#"PDF/UA identification schemahttp://www.aiim.org/pdfua/ns/id/pdfuaid"#, + r#""#, + r#"partIntegerinternalThe part of ISO 14289 the file conforms to"#, + r#""#, +); + +/// What became of a conformance claim when the packet holding it was removed. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub enum Claim { + /// No removed packet made the claim. + #[default] + None, + /// The packet written in its place makes it again. + Kept, + /// The removed packet made it (or, when it couldn't be read, seems to have), and it couldn't + /// be written back: a property wasn't a valid code, held the removed text, or the packet + /// couldn't be read. + Lost, +} + +/// What became of the document's PDF/A and PDF/UA identification when the metadata check +/// removed the catalog's XMP packet. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct Identification { + /// PDF/A (`pdfaid`: part, conformance, and amendment and revision when present). + pub pdfa: Claim, + /// PDF/UA (`pdfuaid`: part, and revision when present). + pub pdfua: Claim, +} + +impl Identification { + /// A claim was lost. + pub fn lost(&self) -> bool { + self.pdfa == Claim::Lost || self.pdfua == Claim::Lost + } +} + +/// The identification of the removed catalog packet `xmp`, and the packet to write in its place +/// (`None` when nothing is kept). +pub(crate) fn keep(doc: &Document, xmp: &Object, search: &Search) -> (Identification, Option) { + let Some(text) = crate::metadata::xmp_text(doc, xmp) else { + // Unreadable: a claim it seems to make, by its namespace or usual prefix in the raw + // bytes, is lost. + let seen = |raw: &[u8], ns: &str, prefix: &[u8]| { + let has = |n: &[u8]| raw.windows(n.len()).any(|w| w == n); + if has(ns.as_bytes()) || has(prefix) { Claim::Lost } else { Claim::None } + }; + let id = match &*doc.resolve(xmp) { + Object::Stream(s) => Identification { pdfa: seen(&s.raw, PDFA_ID, b"pdfaid:"), pdfua: seen(&s.raw, PDFUA_ID, b"pdfuaid:") }, + _ => Identification::default(), + }; + return (id, None); + }; + let Some(found) = read(&text) else { + let seen = |ns: &str, prefix: &str| if text.contains(ns) || text.contains(prefix) { Claim::Lost } else { Claim::None }; + return (Identification { pdfa: seen(PDFA_ID, "pdfaid:"), pdfua: seen(PDFUA_ID, "pdfuaid:") }, None); + }; + // A value is kept when it is a valid code and doesn't hold the removed text. One letter or + // digit can't be a term, so it is kept even when everything counts as holding the text. + let ok = |v: &Option, valid: fn(&str) -> bool| -> Option { + let v = v.as_deref()?; + let short = v.chars().filter(|c| c.is_alphanumeric()).count() < 2; + (valid(v) && (short || !search.text(v))).then(|| v.to_string()) + }; + let mut id = Identification::default(); + let mut body = String::new(); + + let a = &found.pdfa; + if a.any() { + let part = ok(&a.part, part); + let conformance = ok(&a.conformance, |v| v.len() == 1 && v.bytes().all(|b| b.is_ascii_alphabetic())); + // PDF/A-1 to -3 require a conformance level; PDF/A-4 has one only for -4e and -4f. + if part.is_some() && (conformance.is_some() || a.conformance.is_none()) { + id.pdfa = Claim::Kept; + body.push_str(r#""#); + let props = [("part", part), ("conformance", conformance), ("amd", ok(&a.amd, amendment)), ("rev", ok(&a.rev, year))]; + for (name, v) in props { + if let Some(v) = v { + body.push_str(&format!("{v}")); + } + } + body.push_str(""); + } else { + id.pdfa = Claim::Lost; + } + } + if found.pdfua.any() { + // PDF/A-1 to -3 accept only the schemas they define or that the packet describes in an + // extension schema, which pdfuaid needs there. Its fixed text is ours, but a file whose + // redacted words it happens to use doesn't get it (nor, then, the PDF/UA claim). + let extension = id.pdfa == Claim::Kept && a.part.as_deref().is_some_and(|p| matches!(p, "1" | "2" | "3")); + let extension_ok = !extension || !crate::metadata::xml_values(PDFUA_EXTENSION).iter().any(|v| search.text(v)); + match ok(&found.pdfua.part, part).filter(|_| extension_ok) { + Some(p) => { + id.pdfua = Claim::Kept; + // PDF/UA-2 adds a revision year (PDF 2.0, so never beside PDF/A-1 to -3). + let rev = ok(&found.pdfua.rev, year).map(|r| format!("{r}")).unwrap_or_default(); + body.push_str(&format!(r#"{p}{rev}"#)); + if extension { + body.push_str(PDFUA_EXTENSION); + } + } + None => id.pdfua = Claim::Lost, + } + } + if body.is_empty() { + return (id, None); + } + let packet = format!( + "\n{body}\n" + ); + let mut dict = Dict::new(); + dict.set(b"Type".to_vec(), Object::name("Metadata")); + dict.set(b"Subtype".to_vec(), Object::name("XML")); + // Unfiltered, as PDF/A requires of metadata streams. + (id, Some(Stream::from_raw(dict, packet.into_bytes()))) +} + +/// A part number: one or two digits. +fn part(v: &str) -> bool { + (1..=2).contains(&v.len()) && v.bytes().all(|b| b.is_ascii_digit()) +} + +/// An amendment identifier, such as "2005" or "1:2005". +fn amendment(v: &str) -> bool { + (1..=16).contains(&v.len()) && v.bytes().all(|b| b.is_ascii_digit() || b == b':' || b == b'.' || b == b'-') +} + +/// A revision year. +fn year(v: &str) -> bool { + v.len() == 4 && v.bytes().all(|b| b.is_ascii_digit()) +} + +#[derive(Default)] +struct PdfaId { + part: Option, + conformance: Option, + amd: Option, + rev: Option, +} + +impl PdfaId { + fn any(&self) -> bool { + self.part.is_some() || self.conformance.is_some() || self.amd.is_some() || self.rev.is_some() + } +} + +#[derive(Default)] +struct Found { + pdfa: PdfaId, + pdfua: PdfaId, +} + +/// The identification properties of a packet, by namespace, written as attributes or as +/// elements; the first of each wins. `None` when the packet can't be parsed. +fn read(text: &str) -> Option { + if text.bytes().filter(|&b| b == b'=').count() > MAX_EQUALS { + return None; + } + let options = roxmltree::ParsingOptions { allow_dtd: false, nodes_limit: MAX_NODES, ..Default::default() }; + let xml = roxmltree::Document::parse_with_options(text, options).ok()?; + let mut found = Found::default(); + for node in xml.descendants().filter(roxmltree::Node::is_element) { + for (ns, id) in [(PDFA_ID, &mut found.pdfa), (PDFUA_ID, &mut found.pdfua)] { + for (local, slot) in [("part", &mut id.part), ("conformance", &mut id.conformance), ("amd", &mut id.amd), ("rev", &mut id.rev)] { + if slot.is_some() { + continue; + } + if let Some(v) = node.attribute((ns, local)) { + *slot = Some(v.trim().to_string()); + } else if node.tag_name().namespace() == Some(ns) && node.tag_name().name() == local { + *slot = Some(node.text().unwrap_or("").trim().to_string()); + } + } + } + } + Some(found) +} diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 4d6bd676e0..5925878430 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -23,6 +23,7 @@ use pdfcraft_cos::{Dict, Document, ObjRef, Object, Stream}; pub mod codes; mod elsewhere; +mod identification; mod image; mod interp; mod metadata; @@ -36,6 +37,7 @@ pub(crate) mod type3; pub(crate) mod type3_tables; pub use elsewhere::Elsewhere; +pub use identification::{Claim, Identification}; use interp::{Mode, Scope, process}; pub type Rgb = [f64; 3]; @@ -157,6 +159,9 @@ pub struct Report { pub overlays: Vec, /// Document information entries and XMP streams removed because they held redacted text. pub metadata: usize, + /// What became of the PDF/A and PDF/UA identification when the catalog's XMP packet was + /// removed: written back in a packet of its own, or lost (see [`Identification`]). + pub identification: Identification, /// Places this apply leaves as they are that still hold the removed text (bookmark titles, /// attachments, the comments that remain, XMP on other objects), counted so the user can be /// told; Remove hidden information removes them. See [`Elsewhere`] for when the counts are @@ -544,7 +549,9 @@ fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result String { /// The text values of an XMP stream, or `None` when it can't be read. fn xmp_values(doc: &Document, o: &Object) -> Option> { + xmp_text(doc, o).map(|t| xml_values(&t)) +} + +/// The decoded text of an XMP stream, or `None` when it can't be read. +pub(crate) fn xmp_text(doc: &Document, o: &Object) -> Option { let Object::Stream(s) = &*doc.resolve(o) else { return None }; // Decoded strictly: a tolerant decode returns whatever came before the damage, and the part // after it would go unsearched. A chain of more than one filter has no strict decoder here, @@ -384,7 +389,7 @@ fn xmp_values(doc: &Document, o: &Object) -> Option> { return None; } let bytes = s.decoded_first_filter_strict(MAX_XMP_BYTES).ok()?; - decode_xmp(&bytes).map(|t| xml_values(&t)) + decode_xmp(&bytes) } /// What the removed text is looked for with, and the searching left to do it. @@ -461,19 +466,30 @@ impl Search { } } +/// What [`scrub`] did. +#[derive(Debug, Default)] +pub(crate) struct Scrubbed { + /// `/Info` entries and XMP streams removed. + pub(crate) removed: usize, + /// What became of the PDF/A and PDF/UA identification of a removed catalog packet. + pub(crate) identification: crate::Identification, +} + /// Remove the metadata that still holds `removed` text: matching `/Info` entries, and the /// catalog's and pages' XMP streams that mention it. `complete` is false when the collection of /// removed text was cut short; then, as when the terms themselves exceed their limits, every -/// `/Info` string and every XMP stream goes. Returns how many entries and streams went. -pub(crate) fn scrub(doc: &mut Document, removed: &[String], complete: bool) -> Result { +/// `/Info` string and every XMP stream goes. A removed catalog packet's PDF/A and PDF/UA +/// identification is written back in a packet of its own ([`crate::identification`]). +pub(crate) fn scrub(doc: &mut Document, removed: &[String], complete: bool) -> Result { scrub_within(doc, removed, complete, MAX_WORK) } /// [`scrub`] with the searching it may do given, so tests can run out of it. -pub(crate) fn scrub_within(doc: &mut Document, removed: &[String], complete: bool, limit: usize) -> Result { +pub(crate) fn scrub_within(doc: &mut Document, removed: &[String], complete: bool, limit: usize) -> Result { let search = Search::new(removed, complete, limit); + let mut out = Scrubbed::default(); if search.is_nothing() { - return Ok(0); + return Ok(out); } let mut n = 0; @@ -496,16 +512,30 @@ pub(crate) fn scrub_within(doc: &mut Document, removed: &[String], complete: boo } // XMP on the catalog and on every page. + let root = doc.root(); for holder in catalog_and_pages(doc) { let Some(xmp) = doc.get(holder).as_dict().and_then(|d| d.get(b"Metadata").cloned()) else { continue }; if search.xmp(doc, &xmp) { - doc.update_dict(holder, |d| { - d.remove(b"Metadata"); + // The catalog's packet says whether the file conforms to PDF/A or PDF/UA: that much + // is written back, in a packet of its own. + let replacement = if Some(holder) == root { + let (id, packet) = crate::identification::keep(doc, &xmp, &search); + out.identification = id; + packet.map(|p| doc.add(Object::Stream(p))) + } else { + None + }; + doc.update_dict(holder, |d| match replacement { + Some(r) => d.set(b"Metadata".to_vec(), Object::Ref(r)), + None => { + d.remove(b"Metadata"); + } })?; n += 1; } } - Ok(n) + out.removed = n; + Ok(out) } /// The catalog and the pages: the objects whose XMP [`scrub`] checks. diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 24fd6eeba8..7a19040e52 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -299,10 +299,10 @@ fn metadata_search_has_one_budget_for_every_value() { let info = format!("<< {entries}/Producer (Test) >>"); let fresh = || with_metadata(b"BT /F1 10 Tf 10 100 Td (Bluefin) Tj ET", &info, ""); let mut doc = fresh(); - assert_eq!(metadata::scrub_within(&mut doc, &["Bluefin".into()], true, 1 << 28).unwrap(), 0, "room to search: nothing goes"); + assert_eq!(metadata::scrub_within(&mut doc, &["Bluefin".into()], true, 1 << 28).unwrap().removed, 0, "room to search: nothing goes"); let mut doc = fresh(); // Each value costs about 40; fifty of them don't fit in 1000. - let n = metadata::scrub_within(&mut doc, &["Bluefin".into()], true, 1000).unwrap(); + let n = metadata::scrub_within(&mut doc, &["Bluefin".into()], true, 1000).unwrap().removed; assert!(n > 20, "past the budget the rest goes ({n} went)"); } @@ -3226,6 +3226,7 @@ fn unused_descriptions_in_shared_or_inherited_resources_are_retired() { mod descriptions; mod elsewhere; +mod identification; mod inline_images; mod limits; mod metadata_fail_closed; diff --git a/crates/redact/src/tests/identification.rs b/crates/redact/src/tests/identification.rs new file mode 100644 index 0000000000..c896e335fe --- /dev/null +++ b/crates/redact/src/tests/identification.rs @@ -0,0 +1,123 @@ +//! #2539 review: the PDF/A and PDF/UA identification of a catalog XMP packet the metadata check +//! removes is written back in a packet of its own, or reported lost. + +use super::*; +use crate::{Claim, Identification}; + +const DOC_XMP_HEAD: &str = r#""#; +const DOC_XMP_TAIL: &str = r#""#; + +/// One page showing "Codename BLUEFIN report", whose catalog XMP is `xmp` (raw bytes). +fn page(xmp: &[u8]) -> Document { + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /Metadata 7 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 >> >> >>".to_vec(), + stream("", b"BT /F1 10 Tf 10 100 Td (Codename BLUEFIN report) Tj ET"), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + stream("/Type /Metadata /Subtype /XML", xmp), + b"<< /Producer (Test) >>".to_vec(), + ]; + Document::open(Arc::new(Pdf::new().objs(objs).trailer_entry("/Info 8 0 R").build())).unwrap() +} + +/// Apply a mark over "BLUEFIN" and return the report and the catalog's XMP text afterwards. +fn redact(doc: &mut Document) -> (Report, Option) { + mark(doc, 0, &[[55.0, 95.0, 90.0, 110.0]], ""); + let r = apply(doc, None).unwrap(); + let xmp = doc.get(doc.root().unwrap()).as_dict().and_then(|c| c.get(b"Metadata").cloned()); + let text = xmp.and_then(|x| crate::metadata::xmp_text(doc, &x)); + (r, text) +} + +/// The packet that names the redacted word goes, and one holding only its PDF/A and PDF/UA +/// identification takes its place: the PDF/A properties written as attributes, PDF/UA's as an +/// element under a prefix of its own, both found by namespace. Nothing else is copied. +#[test] +fn the_identification_of_a_removed_packet_is_written_back() { + let body = r#"1Bluefin reportAda Lovelace"#; + let mut doc = page(format!("{DOC_XMP_HEAD}{body}{DOC_XMP_TAIL}").as_bytes()); + let (r, text) = redact(&mut doc); + assert_eq!(r.identification, Identification { pdfa: Claim::Kept, pdfua: Claim::Kept }, "{r:?}"); + assert_eq!(r.metadata, 1, "{r:?}"); + let text = text.expect("a packet is written back"); + assert!(!text.to_lowercase().contains("bluefin") && !text.contains("Lovelace"), "{text}"); + let xml = roxmltree::Document::parse(&text).expect("well-formed"); + let value = |ns: &str, local: &str| { + xml.descendants().find(|n| n.tag_name().namespace() == Some(ns) && n.tag_name().name() == local).and_then(|n| n.text()).map(str::to_string) + }; + let a = "http://www.aiim.org/pdfa/ns/id/"; + let ua = "http://www.aiim.org/pdfua/ns/id/"; + assert_eq!((value(a, "part").as_deref(), value(a, "conformance").as_deref(), value(ua, "part").as_deref()), (Some("2"), Some("B"), Some("1"))); + // PDF/A-2 accepts pdfuaid only through an extension schema describing it. + assert_eq!(value("http://www.aiim.org/pdfa/ns/schema#", "prefix").as_deref(), Some("pdfuaid"), "{text}"); +} + +/// A packet without identification is removed as before, and nothing is said about it. +#[test] +fn a_packet_without_identification_is_only_removed() { + let body = r#"Bluefin report"#; + let mut doc = page(format!("{DOC_XMP_HEAD}{body}{DOC_XMP_TAIL}").as_bytes()); + let (r, text) = redact(&mut doc); + assert_eq!((r.identification, text), (Identification::default(), None), "{r:?}"); +} + +/// The identification is lost, and said to be, when its value isn't the code it must be (a +/// conformance level that is a word) or when the packet can't be read (here it isn't UTF-8). +#[test] +fn an_identification_that_cannot_be_kept_is_reported_lost() { + let wordy = r#""#; + let mut doc = page(format!("{DOC_XMP_HEAD}{wordy}{DOC_XMP_TAIL}").as_bytes()); + let (r, text) = redact(&mut doc); + assert_eq!((r.identification, text), (Identification { pdfa: Claim::Lost, pdfua: Claim::None }, None), "{r:?}"); + + let latin1 = [ + DOC_XMP_HEAD.as_bytes(), + br#""#, + DOC_XMP_TAIL.as_bytes(), + ] + .concat(); + let mut doc = page(&latin1); + let (r, text) = redact(&mut doc); + assert_eq!((r.identification, text), (Identification { pdfa: Claim::None, pdfua: Claim::Lost }, None), "{r:?}"); + assert!(r.identification.lost()); +} + +/// When the removed text couldn't be searched for, every catalog packet goes, but a part number +/// and a conformance letter, one letter or digit each, can't hold it and are written back; a +/// revision year could, and isn't. +#[test] +fn unsearched_text_keeps_only_one_character_codes() { + let body = r#"Annual figures"#; + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /Metadata 7 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 >> >> >>".to_vec(), + stream("", b"BT /F1 10 Tf 10 100 Td (ABC) Tj ET"), + // Glyph names outside the glyph list: the removed text isn't known. + FONT.replace("95 0 R", "6 0 R").replace(" /Widths", " /Encoding << /Differences [65 /g17 /g18 /g19] >> /Widths").into_bytes(), + widths(), + stream("/Type /Metadata /Subtype /XML", format!("{DOC_XMP_HEAD}{body}{DOC_XMP_TAIL}").as_bytes()), + ]; + let mut doc = Document::open(Arc::new(Pdf::new().objs(objs).build())).unwrap(); + mark(&mut doc, 0, &[[5.0, 95.0, 200.0, 110.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!(r.identification, Identification { pdfa: Claim::Kept, pdfua: Claim::None }, "{r:?}"); + let xmp = doc.get(doc.root().unwrap()).as_dict().and_then(|c| c.get(b"Metadata").cloned()).unwrap(); + let text = crate::metadata::xmp_text(&doc, &xmp).unwrap(); + assert!(text.contains("4") && !text.contains("2020") && !text.contains("Annual"), "{text}"); +} + +/// The extension schema PDF/A-2 needs for pdfuaid is fixed text, but if the redacted words are +/// among its words ("schema" here) it isn't written, and the PDF/UA claim is reported lost. +#[test] +fn the_extension_schema_is_left_out_when_it_uses_a_redacted_word() { + let body = r#"Schema notes"#; + let mut doc = page(format!("{DOC_XMP_HEAD}{body}{DOC_XMP_TAIL}").as_bytes()); + let s = crate::metadata::scrub_within(&mut doc, &["SCHEMA".into()], true, 1 << 28).unwrap(); + assert_eq!(s.identification, Identification { pdfa: Claim::Kept, pdfua: Claim::Lost }); + let xmp = doc.get(doc.root().unwrap()).as_dict().and_then(|c| c.get(b"Metadata").cloned()).unwrap(); + let text = crate::metadata::xmp_text(&doc, &xmp).unwrap(); + assert!(!text.to_lowercase().contains("schema") && text.contains("U"), "{text}"); +} diff --git a/crates/redact/src/tests/metadata_matching.rs b/crates/redact/src/tests/metadata_matching.rs index 441ad104a3..8cab62b0ce 100644 --- a/crates/redact/src/tests/metadata_matching.rs +++ b/crates/redact/src/tests/metadata_matching.rs @@ -54,7 +54,7 @@ fn unicode_forms_of_the_removed_text_match() { ); let mut doc = with_metadata(b"BT /F1 10 Tf 10 100 Td (x) Tj ET", &info, ""); let removed = ["M\u{FC}ller".to_string(), "\u{FB01}nance".to_string(), "Bluefin".to_string()]; - let n = metadata::scrub_within(&mut doc, &removed, true, 1 << 28).unwrap(); + let n = metadata::scrub_within(&mut doc, &removed, true, 1 << 28).unwrap().removed; assert_eq!(info_keys(&doc), ["Author", "Producer"]); assert_eq!(n, 3); } @@ -65,7 +65,7 @@ fn unicode_forms_of_the_removed_text_match() { fn a_value_that_normalizes_to_far_more_text_counts_as_holding_it() { let info = format!("<< /Subject {} /Producer (Test) >>", utf16be(&"\u{FDFA}".repeat(50))); let mut doc = with_metadata(b"BT /F1 10 Tf 10 100 Td (x) Tj ET", &info, ""); - assert_eq!(metadata::scrub_within(&mut doc, &["Bluefin".into()], true, 1 << 28).unwrap(), 1); + assert_eq!(metadata::scrub_within(&mut doc, &["Bluefin".into()], true, 1 << 28).unwrap().removed, 1); assert_eq!(info_keys(&doc), ["Producer"]); } @@ -82,7 +82,7 @@ fn utf16le(s: &str) -> String { fn a_utf16le_string_is_searched_as_utf16le() { let info = format!("<< /Title {} /Subject {} /Producer (Test) >>", utf16le("Синий план"), utf16le("Годовой отчёт")); let mut doc = with_metadata(b"BT /F1 10 Tf 10 100 Td (x) Tj ET", &info, ""); - assert_eq!(metadata::scrub_within(&mut doc, &["СИНИЙ".into()], true, 1 << 28).unwrap(), 1); + assert_eq!(metadata::scrub_within(&mut doc, &["СИНИЙ".into()], true, 1 << 28).unwrap().removed, 1); assert_eq!(info_keys(&doc), ["Subject", "Producer"]); // A bookmark titled the same way is counted as still holding it. diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 75f6e1463c..0c4c5c81c8 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1724,13 +1724,14 @@ fn finish_redactions_and_fields(app: &mut PdfCraftApp, req: &mut Pending) { } else { crate::i18n::fmt(tl!("Applied {n} redaction marks. Save to remove the content from the file."), &[("n", &marks.to_string())]) }; - // Where the removed text is still held that applying leaves as it is: said in the - // same notification, so it can't replace the first sentence. - let left = app.active_ids().and_then(|(_, id)| app.session.get(id)).and_then(|d| d.redaction_elsewhere()); - match left.and_then(|e| still_holding_note(&e)) { - Some(note) => app.notify(format!("{applied} {note}")), - None => app.notify(applied), - } + // Where the removed text is still held that applying leaves as it is, and what + // became of a PDF/A or PDF/UA claim in the document XMP it removed: said in the + // same notification, so neither can replace the first sentence. + let doc = app.active_ids().and_then(|(_, id)| app.session.get(id)); + let left = doc.and_then(|d| d.redaction_elsewhere()).and_then(|e| still_holding_note(&e)); + let id = doc.and_then(|d| d.redaction_identification()).and_then(|id| identification_note(&id)); + let message = [Some(applied), left, id].into_iter().flatten().collect::>().join(" "); + app.notify(message); } } _ => {} @@ -1807,6 +1808,33 @@ pub(crate) fn still_holding_note(e: &pdfcraft_engine::RedactionElsewhere) -> Opt Some(crate::i18n::fmt(tl!(template), &[("places", &list)])) } +/// The sentence telling the user what became of the document's PDF/A or PDF/UA identification +/// when applying redactions removed the XMP packet that held it (and the redacted text), or +/// `None` when no such packet was removed. +pub(crate) fn identification_note(id: &pdfcraft_engine::RedactionIdentification) -> Option { + use pdfcraft_engine::RedactionClaim as Claim; + let named = |claim: Claim| -> Option { + match (id.pdfa == claim, id.pdfua == claim) { + (true, true) => Some(tl!("PDF/A and PDF/UA").to_string()), + (true, false) => Some("PDF/A".to_string()), + (false, true) => Some("PDF/UA".to_string()), + (false, false) => None, + } + }; + let kept = named(Claim::Kept).map(|s| crate::i18n::fmt(tl!("Its {standard} identification was written back on its own."), &[("standard", &s)])); + let lost = named(Claim::Lost).map(|s| { + crate::i18n::fmt( + tl!("Its {standard} identification couldn't be kept, so the file no longer says it conforms to {standard}."), + &[("standard", &s)], + ) + }); + if kept.is_none() && lost.is_none() { + return None; + } + let head = tl!("The document's XMP metadata held the redacted text and was removed.").to_string(); + Some([Some(head), kept, lost].into_iter().flatten().collect::>().join(" ")) +} + /// Edits built from the dialog's drafts. fn finish_edits(app: &mut PdfCraftApp, dialog: Dialog, req: &mut Pending) { if req.props_now diff --git a/crates/ui-egui/src/editing.rs b/crates/ui-egui/src/editing.rs index 517609d0fa..53d4257f01 100644 --- a/crates/ui-egui/src/editing.rs +++ b/crates/ui-egui/src/editing.rs @@ -179,6 +179,9 @@ impl PdfCraftApp { pdfcraft_engine::EditNotice::Undrawn(t) => crate::content_ui::undrawn_message(t), pdfcraft_engine::EditNotice::BackdropsLeftOff { pages } => crate::marks_ui::backdrops_left_off_message(pages), pdfcraft_engine::EditNotice::RedactionElsewhere(e) => crate::dialogs::still_holding_note(e).unwrap_or_else(|| n.to_string()), + pdfcraft_engine::EditNotice::RedactionIdentification(id) => { + crate::dialogs::identification_note(id).unwrap_or_else(|| n.to_string()) + } }); } let out = self.session.take_js_output(id); diff --git a/crates/ui-egui/tests/it/redact.rs b/crates/ui-egui/tests/it/redact.rs index f32341adf6..8f75b44985 100644 --- a/crates/ui-egui/tests/it/redact.rs +++ b/crates/ui-egui/tests/it/redact.rs @@ -325,3 +325,56 @@ fn applying_says_where_the_removed_text_is_still_held() { "Applied 1 redaction mark. Save to remove the content from the file. The redacted text also appears in 1 bookmark title, which redaction doesn't change. Remove hidden information removes them." ); } + +/// #2539 review: applying redactions that remove the document's XMP packet, which named the +/// redacted word and said the file conforms to PDF/A, says that the identification was written +/// back, in the same notification as the count of marks applied. +#[test] +fn applying_says_the_pdfa_identification_was_written_back() { + let xmp = r#"Bluefin report"#; + let content = "BT /F1 18 Tf 20 150 Td (Codename BLUEFIN report) Tj ET"; + let objs = [ + "<< /Type /Catalog /Pages 2 0 R /Metadata 6 0 R >>".to_string(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_string(), + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Resources << /Font << /F1 4 0 R >> >> /Contents 5 0 R >>".to_string(), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".to_string(), + format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()), + format!("<< /Type /Metadata /Subtype /XML /Length {} >>\nstream\n{xmp}\nendstream", xmp.len()), + ]; + let mut bytes = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(bytes.len()); + bytes.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = bytes.len(); + bytes.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + bytes.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + bytes.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("pdfa.pdf", None, bytes).unwrap(); + app + }); + crate::support::settle(&mut h); + assert!(h.state_mut().execute("redact.mark")); + h.run_steps(2); + h.state_mut().execute("redact.pages"); + h.run_steps(2); + h.get_by_label("OK").click(); + h.run_steps(3); + assert_eq!(marks(&h), 1); + h.get_by_label("Redact all").click(); + h.run_steps(3); + h.get_by_label("Apply").click(); + h.run_steps(4); + assert_eq!(marks(&h), 0); + let toast = h.state().toast.as_ref().map(|(m, _)| m.clone()).unwrap_or_default(); + assert_eq!( + toast, + "Applied 1 redaction mark. Save to remove the content from the file. The document's XMP metadata held the redacted text and was removed. Its PDF/A identification was written back on its own." + ); +} From 4bfea3ade6bc27a9f4abe0552e941b326b472ae8 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 2 Nov 2026 01:17:24 +0800 Subject: [PATCH 21/23] M8: protect.apply-redactions notes and evidence cover the second review round (#2448) Letters-and-digits matching, NFKC, FF FE strings as UTF-16LE, and the PDF/A / PDF/UA identification written back or reported lost; what still isn't done (remaining /Info entries aren't mirrored into the new packet). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- 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 016d69b49d..846eddbe18 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -5780,8 +5780,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::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", "crates/redact/src/tests.rs::redacted_text_does_not_survive_in_info_or_xmp", "crates/redact/src/tests.rs::metadata_is_cleared_when_the_removed_text_was_cut_short", "crates/redact/src/tests/metadata_fail_closed.rs::an_xmp_stream_whose_filter_data_is_corrupt_goes", "crates/redact/src/tests/elsewhere.rs::places_apply_leaves_that_still_hold_the_removed_text_are_counted_and_kept", "crates/engine/src/tests.rs::applied_redactions_report_a_bookmark_that_still_holds_the_removed_text", "crates/automation/tests/it/automation.rs::redact_apply_reports_places_that_still_hold_the_removed_text", "crates/ui-egui/tests/it/redact.rs::applying_says_where_the_removed_text_is_still_held"] -notes = "confirmation; one undo step; next save is a full rewrite. Document information entries that contain the removed text, and the document's and pages' XMP streams that do, are removed (#2448; matched without regard to case or spacing, by the whole run of removed text and by its words of three letters or digits or more, and a value wholly part of a run as whole words; fail-closed: all of it goes when the removed text can't be known in full (too much to collect or search for, a glyph whose text isn't known, a form removed unread), and an XMP packet goes when it can't be decoded strictly, is over 16 MiB or isn't UTF-8 or UTF-16). Bookmark titles, attachments (names, file names, descriptions), the comments that remain and other objects' XMP are not changed: applying counts those that still hold the removed text, and the notification and redact_apply's still_holding say where (Remove hidden information removes them); when the removed text couldn't be searched for, every such place holding any text is counted. 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" +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", "crates/redact/src/tests.rs::redacted_text_does_not_survive_in_info_or_xmp", "crates/redact/src/tests.rs::metadata_is_cleared_when_the_removed_text_was_cut_short", "crates/redact/src/tests/metadata_fail_closed.rs::an_xmp_stream_whose_filter_data_is_corrupt_goes", "crates/redact/src/tests/elsewhere.rs::places_apply_leaves_that_still_hold_the_removed_text_are_counted_and_kept", "crates/engine/src/tests.rs::applied_redactions_report_a_bookmark_that_still_holds_the_removed_text", "crates/automation/tests/it/automation.rs::redact_apply_reports_places_that_still_hold_the_removed_text", "crates/ui-egui/tests/it/redact.rs::applying_says_where_the_removed_text_is_still_held", "crates/redact/src/tests/metadata_matching.rs::a_heading_tracked_with_tj_numbers_matches_the_metadata", "crates/redact/src/tests/metadata_matching.rs::unicode_forms_of_the_removed_text_match", "crates/redact/src/tests/metadata_matching.rs::a_utf16le_string_is_searched_as_utf16le", "crates/redact/src/tests/identification.rs::the_identification_of_a_removed_packet_is_written_back", "crates/automation/tests/it/automation.rs::redact_apply_reports_the_identification_written_back", "crates/ui-egui/tests/it/redact.rs::applying_says_the_pdfa_identification_was_written_back"] +notes = "confirmation; one undo step; next save is a full rewrite. Document information entries that contain the removed text, and the document's and pages' XMP streams that do, are removed (#2448; matched without regard to case or spacing, by the whole run of removed text and by its words of three letters or digits or more, and a value wholly part of a run as whole words, each also compared by letters and digits alone (so spacing, hyphens and TJ tracking don't hide it), after NFKC and lower case, with FF FE strings read as UTF-16LE; fail-closed: all of it goes when the removed text can't be known in full (too much to collect or search for, a glyph whose text isn't known, a form removed unread), and an XMP packet goes when it can't be decoded strictly, is over 16 MiB or isn't UTF-8 or UTF-16). When the document's XMP goes, its PDF/A and PDF/UA identification is written back in a packet holding only that, value by value when each is a valid code that doesn't hold the removed text, else reported lost (Report::identification, the notification, redact_apply's identification); /Info entries that remain aren't mirrored into that packet. Bookmark titles, attachments (names, file names, descriptions), the comments that remain and other objects' XMP are not changed: applying counts those that still hold the removed text, and the notification and redact_apply's still_holding say where (Remove hidden information removes them); when the removed text couldn't be searched for, every such place holding any text is counted. 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 dcef0aebff204dd86670eb48cefc72ae365df688 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 2 Nov 2026 23:36:21 +0800 Subject: [PATCH 22/23] M8: bound the removed text kept for the metadata check per document (#2448) The run and byte limits were per page, and a glyph's /ToUnicode text can be 256 UTF-16 units, so a short content stream could keep gigabytes of removed text across pages. The document now keeps at most 16 MiB; past that the rest is not kept and the metadata check fails closed, as for any other text it couldn't keep (CodeRabbit's retained concern on #2539). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyG234xeYkJfr41E6iP --- crates/redact/src/interp.rs | 19 +++++++++++++- crates/redact/src/lib.rs | 9 +++++-- crates/redact/src/tests.rs | 49 +++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 3 deletions(-) diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index b0029ac912..540ee703a9 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -49,6 +49,11 @@ const _: () = assert!(MAX_DEPTH >= pdfcraft_edit::text::MAX_FORM_DEPTH); /// Bounds on the removed text collected for the metadata check (#2448). const MAX_RUNS: usize = 100_000; const MAX_RUN_BYTES: usize = 64 * 1024; +/// The removed text kept for the whole document, in bytes. A glyph's `/ToUnicode` text has no +/// length limit, so a short content stream can make many runs of nearly [`MAX_RUN_BYTES`] each, +/// on every page; past this the rest is not kept and the metadata check fails closed, as for +/// any other text it couldn't keep (review on #2539). +pub(crate) const MAX_REMOVED_BYTES: usize = 16 * 1024 * 1024; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum Mode { @@ -132,6 +137,9 @@ pub(crate) struct Scope<'a> { /// Some removed text was not kept (too many runs, or a run too long): the metadata check /// can't search for it and must fail closed. pub removed_cut: bool, + /// Bytes of removed text this scope may still keep: what is left of the document's + /// [`MAX_REMOVED_BYTES`] (see [`Scope::with_removed_room`]). + pub removed_room: usize, /// The run being collected, and where its last glyph ended (device space) with its height. run: String, run_end: Option<((f64, f64), f64)>, @@ -161,6 +169,7 @@ impl<'a> Scope<'a> { layer_blocks: 0, removed_text: Vec::new(), removed_cut: false, + removed_room: MAX_REMOVED_BYTES, run: String::new(), run_end: None, fonts: HashMap::new(), @@ -170,12 +179,20 @@ impl<'a> Scope<'a> { } } + /// Keep at most `room` bytes of removed text: what earlier pages left of the document's + /// [`MAX_REMOVED_BYTES`]. + pub fn with_removed_room(mut self, room: usize) -> Self { + self.removed_room = room; + self + } + /// Close the run of removed text being collected. pub fn flush(&mut self) { self.run_end = None; let run = std::mem::take(&mut self.run); if !run.trim().is_empty() { - if self.removed_text.len() < MAX_RUNS { + if self.removed_text.len() < MAX_RUNS && run.len() <= self.removed_room { + self.removed_room -= run.len(); self.removed_text.push(run); } else { self.removed_cut = true; diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 5925878430..61030a2bd5 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -438,6 +438,7 @@ fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result, /// Some removed text was too long to keep whole for that check. removed_cut: bool, + /// Bytes of removed text later pages may still keep (`interp::MAX_REMOVED_BYTES` for the + /// document). + removed_room: usize, } /// Step 1 for one page: rewrites its content so nothing readable stays under a mark, and cleans the tags it no longer shows. @@ -582,17 +586,18 @@ fn redact_page_content( page: &pdfcraft_model::Page, rects: &[[f64; 4]], ) -> Result<(), RedactError> { - let Applying { report, doomed_xobjects, retired_properties, removed_text, removed_cut, .. } = applying; + let Applying { report, doomed_xobjects, retired_properties, removed_text, removed_cut, removed_room, .. } = applying; // 1. Content. let mut budget = pdfcraft_cos::DecodeBudget::page(); let (list, data) = page_streams_within(doc, &page.dict, pi, &mut budget)?; let resources = page.dict.get(b"Resources").and_then(|r| doc.resolve(r).as_dict().cloned()).unwrap_or_default(); let out = { - let mut scope = Scope::new(rects, Mode::Apply, report, budget); + let mut scope = Scope::new(rects, Mode::Apply, report, budget).with_removed_room(*removed_room); let out = process(doc, &mut scope, &data, &resources, pdfcraft_content::Matrix::IDENTITY); scope.flush(); removed_text.append(&mut scope.removed_text); *removed_cut |= scope.removed_cut; + *removed_room = scope.removed_room; out }; // Fail closed: the interpreter lays vertical-writing text out horizontally, so it cannot diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 7a19040e52..1f0826db32 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -289,6 +289,55 @@ fn metadata_is_cleared_when_the_removed_text_was_cut_short() { assert!(!has_xmp(&doc)); } +/// #2539 review: the removed text kept for the metadata check is bounded for the whole document, +/// not per page. One glyph code can stand for 256 UTF-16 units of `/ToUnicode` text (768 bytes of +/// UTF-8), so here each page makes 150 runs of 80 such glyphs (9.2 MB, under the page's run and +/// byte limits); two pages pass `MAX_REMOVED_BYTES`, so the rest isn't kept and the check fails +/// closed. +#[test] +fn removed_text_kept_for_the_metadata_check_is_bounded_per_document() { + let long = "4E00".repeat(256); + let to_unicode = format!("/CIDInit /ProcSet findresource begin 1 beginbfchar <41> <{long}> endbfchar end"); + let run = "A".repeat(80); + let mut content = b"BT /F1 1 Tf 10 100 Td".to_vec(); + for _ in 0..75 { + content.extend_from_slice(format!(" ({run}) Tj 30 0 Td ({run}) Tj -30 0 Td").as_bytes()); + } + content.extend_from_slice(b" ET"); + let page = "/Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >>"; + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R 8 0 R] /Count 2 >>".to_vec(), + format!("<< {page} >>").into_bytes(), + stream("", &content), + FONT.replace("95 0 R", "6 0 R").replace(" /Widths", " /ToUnicode 7 0 R /Widths").into_bytes(), + widths(), + stream("", to_unicode.as_bytes()), + format!("<< {page} >>").into_bytes(), + ]; + let mut doc = pdf(objs); + let mut applying = Applying { + report: Report::default(), + doomed_fields: Vec::new(), + doomed_xobjects: Vec::new(), + retired_properties: Vec::new(), + widget_owner: Vec::new(), + removed_text: Vec::new(), + removed_cut: false, + removed_room: interp::MAX_REMOVED_BYTES, + removed_everywhere: HashSet::new(), + }; + let rects = [[5.0, 99.0, 200.0, 102.0]]; + let kept = |a: &Applying| a.removed_text.iter().map(String::len).sum::(); + let first = pdfcraft_model::pages(&doc).swap_remove(0); + redact_page_content(&mut doc, &mut applying, 0, &first, &rects).unwrap(); + assert_eq!((kept(&applying), applying.removed_cut), (9_216_000, false), "one page fits"); + let second = pdfcraft_model::pages(&doc).swap_remove(1); + redact_page_content(&mut doc, &mut applying, 1, &second, &rects).unwrap(); + assert!(applying.removed_cut, "the second page passes the document's limit"); + assert!(kept(&applying) <= interp::MAX_REMOVED_BYTES, "kept {} bytes", kept(&applying)); +} + /// #2539 review: the searching a scrub may do is one budget for all of it, not a check per /// value: very many small `/Info` values each fit, but together they don't, and past the budget /// the rest counts as matching (fail-closed). With room to search, nothing that doesn't mention From 0c6860e2582c94a1be4b8cd58cf23b011e69cbc4 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Mon, 2 Nov 2026 23:36:53 +0800 Subject: [PATCH 23/23] M8: the redact README lists the per-document collection limit (#2448) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyG234xeYkJfr41E6iP --- crates/redact/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/redact/README.md b/crates/redact/README.md index 8063f1a9f4..2a8f3a879e 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -66,8 +66,8 @@ clear_marks(&mut doc, None)?; // remove marks without app letters with the removed text, and isn't wholly part of it, stays: Sanitize's metadata category removes the rest. `Report::metadata` counts what went. - **Fail-closed.** When the check can't know what the removed text was, every `/Info` - string and every catalog and page XMP stream goes: more than 100,000 runs or a run over - 64 KiB removed (the collection limit); a removed glyph whose text isn't known (no + string and every catalog and page XMP stream goes: more than 100,000 runs on a page, a run + over 64 KiB, or more than 16 MiB in the whole document removed (the collection limits); a removed glyph whose text isn't known (no `/ToUnicode` or encoding gives it); a form XObject removed without being read, because it is nested past the depth limit (`MAX_DEPTH`, 50) or its content can't be read; or more than 20,000 terms, or a run longer than 4,096 characters, to search for. Once the