From a525cf1d424de0cdf1dc50e34f1b6cfedcdda24e Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:12:51 +0000 Subject: [PATCH 01/42] fix(js): refuse code compiled from strings that skipped the script checks eval and the Function constructors compiled code built at run time, which never passed the size and nesting checks a script passes before it runs, so a short form script could hand boa's recursive parser a deeply nested string and overflow the stack. Form scripts now refuse to compile code from strings. XFA scripts, which the runner itself starts through eval, hold such code to the same checks as the script. --- crates/js/src/lib.rs | 42 +++++++++++++++++++++++++++++++++++++++++- crates/js/src/tests.rs | 33 +++++++++++++++++++++++++++++++++ crates/js/src/xfa.rs | 5 ++++- 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/crates/js/src/lib.rs b/crates/js/src/lib.rs index 785345c672..ae94feb0aa 100644 --- a/crates/js/src/lib.rs +++ b/crates/js/src/lib.rs @@ -27,9 +27,11 @@ use std::cell::RefCell; use std::collections::BTreeSet; use std::sync::Arc; +use boa_engine::context::HostHooks; use boa_engine::object::builtins::{JsArray, JsFunction}; use boa_engine::object::{FunctionObjectBuilder, ObjectInitializer}; use boa_engine::property::Attribute; +use boa_engine::realm::Realm; use boa_engine::{Context, JsError, JsNativeError, JsObject, JsResult, JsString, JsValue, NativeFunction, Source, js_string}; /// The kinds of field, as `field.type` names them. @@ -1128,8 +1130,46 @@ pub fn run_shared(script: &str, event: &Event, doc: &DocInfo, fields: Arc JsResult<()> { + Err(JsNativeError::typ().with_message("scripts can't compile code from strings (eval, Function)").into()) + } +} + +/// Host hooks that hold code compiled from strings to the checks a script passes ([`refuse`]), +/// for runners that start the script itself through `eval`. +pub(crate) struct CheckedCodeFromStrings; + +impl HostHooks for CheckedCodeFromStrings { + fn ensure_can_compile_strings(&self, _: Realm, parameters: &[JsString], body: &JsString, _: bool, _: &mut Context) -> JsResult<()> { + let units = parameters.iter().chain(std::iter::once(body)).fold(0usize, |n, s| n.saturating_add(s.len())); + if units > MAX_SCRIPT_BYTES { + return Err(JsNativeError::range().with_message("the code is too long to compile").into()); + } + // As one text: brackets left open in one parameter carry into the next. + let mut source = parameters.iter().map(JsString::to_std_string_lossy).collect::>().join(","); + source.push('\n'); + source.push_str(&body.to_std_string_lossy()); + match refuse(&source) { + Some(why) => Err(JsNativeError::range().with_message(why).into()), + None => Ok(()), + } + } +} + fn run_here(script: &str, event: &Event, doc: &DocInfo, fields: Arc>, doc_scripts: &[String], limits: Limits) -> Outcome { - let mut ctx = Context::default(); + let mut ctx = match Context::builder().host_hooks(std::rc::Rc::new(NoCodeFromStrings)).build() { + Ok(ctx) => ctx, + Err(e) => { + let error = Some(format!("the script engine could not start: {e}")); + return Outcome { rc: true, value: event.value.clone(), change: event.change.clone(), error, ..Default::default() }; + } + }; ctx.runtime_limits_mut().set_loop_iteration_limit(limits.loop_iterations); ctx.runtime_limits_mut().set_recursion_limit(limits.recursion); ctx.insert_data(RefCell::new(Host { diff --git a/crates/js/src/tests.rs b/crates/js/src/tests.rs index 7a9efb5dfd..5b6adef53c 100644 --- a/crates/js/src/tests.rs +++ b/crates/js/src/tests.rs @@ -313,6 +313,23 @@ fn ordinary_nesting_still_runs() { assert_eq!(go(&s, &Event::field("Calculate", "a", "")).value, "1000"); } +/// Code built from strings at run time would reach the parser without the size and nesting +/// checks the script itself passed, so `eval` and the `Function` constructors refuse it. +#[test] +fn scripts_cannot_compile_code_from_strings() { + for s in [ + "event.value = eval('1 + 1');", + "event.value = (0, eval)('1 + 1');", + "event.value = new Function('return 2')();", + "event.value = Function('a', 'return a')(2);", + "event.value = (function* () {}).constructor('yield 2')().next().value;", + "event.value = new Array(1001).join('(') + '1' + new Array(1001).join(')'); event.value = eval(event.value);", + ] { + let o = go(s, &Event::field("Calculate", "a", "")); + assert!(o.error.as_deref().is_some_and(|e| e.contains("compile code from strings")), "{s}: {o:?}"); + } +} + mod xfa_model { use crate::Limits; use crate::xfa::*; @@ -398,6 +415,22 @@ mod xfa_model { assert_eq!(o.effects, vec![XfaEffect::SetValue { som: "form1[0].page1[0].total[0]".into(), value: "5".into() }]); } + /// The script runs through `eval`, so code built from strings at run time is held to the + /// script's own size and nesting checks instead of being refused outright. + #[test] + fn code_built_from_strings_is_checked_like_the_script() { + let total = "form1[0].page1[0].total[0]"; + assert_eq!(go("eval('qty.rawValue * 2')", total, "calculate").result.as_deref(), Some("8")); + assert_eq!(go("new Function('return 3')()", total, "calculate").result.as_deref(), Some("3")); + for s in [ + "eval(new Array(1001).join('(') + '1' + new Array(1001).join(')'))", + "new Function(new Array(1001).join('[') + new Array(1001).join(']'))", + ] { + let o = go(s, total, "calculate"); + assert!(o.error.as_deref().is_some_and(|e| e.contains("nested too deeply")), "{s}: {o:?}"); + } + } + /// A form whose one field `d` holds the ISO date 2026-10-20, shown as 20261020. pub(super) fn dated_form() -> XfaNode { let d = XfaNode { formatted: Some("20261020".into()), ..field("d", "form1[0].page1[0].d[0]", "2026-10-20", false) }; diff --git a/crates/js/src/xfa.rs b/crates/js/src/xfa.rs index 5a693db9f0..490408a106 100644 --- a/crates/js/src/xfa.rs +++ b/crates/js/src/xfa.rs @@ -1473,7 +1473,10 @@ pub(crate) fn warm_up() { /// `started` is called once the engine is ready: a time limit on the script runs from then. pub(crate) fn run_here(script: &str, event: &XfaEvent, doc: &XfaDoc, root: XfaNode, limits: Limits, started: &dyn Fn()) -> XfaOutcome { warm_up(); - let mut ctx = Context::default(); + let mut ctx = match Context::builder().host_hooks(std::rc::Rc::new(crate::CheckedCodeFromStrings)).build() { + Ok(ctx) => ctx, + Err(e) => return XfaOutcome { error: Some(format!("the script engine could not start: {e}")), ..Default::default() }, + }; ctx.runtime_limits_mut().set_loop_iteration_limit(limits.loop_iterations); ctx.runtime_limits_mut().set_recursion_limit(limits.recursion); let host_state = XHost::new(root, &event.target, doc); From 5dc64face0131a62712d8f414352787e4ef92617 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:23:04 +0000 Subject: [PATCH 02/42] fix(automation): keep page text from tools when copying is not allowed text_lines returned a copy-restricted document's lines, ocr_recognize returned the words it recognised, and text_edit on a paragraph echoed the paragraph's own text, all without checking the copying permission that the other text tools check. text_lines is refused like them; recognising and restyling still work with permission to change the document, and leave the text out. --- crates/automation/src/a11y.rs | 8 +++-- crates/automation/src/lib.rs | 13 ++++++-- crates/automation/src/tools.rs | 4 +-- crates/automation/tests/it/automation.rs | 38 ++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/crates/automation/src/a11y.rs b/crates/automation/src/a11y.rs index 2124549731..21bde353e9 100644 --- a/crates/automation/src/a11y.rs +++ b/crates/automation/src/a11y.rs @@ -626,13 +626,17 @@ impl Automation { pub(crate) fn ocr_recognize(&mut self, a: &Args) -> Result { let settings = self.ocr_settings(a)?; let pages = if a.opt_ints("pages")?.is_some() { self.pages(a, "pages")? } else { Vec::new() }; - let id = self.doc(a)?.id; + let doc = self.doc(a)?; + // Recognizing needs only permission to change the document; the words it found are + // returned only when copying is allowed too. + let (id, copy) = (doc.id, doc.allows_copying()); let found = self.session.recognize_text(id, &pages, settings).map_err(failed)?; let list: Vec = found .iter() .map(|p| match &p.skipped { Some(why) => json!({ "page": p.page + 1, "skipped": why }), - None => json!({ "page": p.page + 1, "words": p.words.len(), "already_on_page": p.already_on_page, "text": p.text() }), + None if copy => json!({ "page": p.page + 1, "words": p.words.len(), "already_on_page": p.already_on_page, "text": p.text() }), + None => json!({ "page": p.page + 1, "words": p.words.len(), "already_on_page": p.already_on_page }), }) .collect(); Ok(json!({ "words": found.iter().map(|p| p.words.len()).sum::(), "pages": list })) diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index c90240875b..522705bb15 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -435,7 +435,7 @@ impl Automation { "doc_revisions" => self.doc_revisions(&a)?, "text_lines" => { let page = self.page(&a)?; - let doc = self.doc(&a)?; + let doc = self.doc_to_copy(&a)?; let info = &doc.info.pages[page]; let r = |x: f32| (x as f64 * 100.0).round() / 100.0; let lines: Vec = doc @@ -709,7 +709,11 @@ impl Automation { let moved = [block.rect[0] + dx, block.rect[1] + dy, block.rect[2] + dx, block.rect[3] + dy]; let wanted: String = text.split_whitespace().collect(); let mut out = self.apply(&a, Edit::EditTextBlock { page, block: index, text, style })?; - let after = self.doc(&a)?.text_blocks(page); + let doc = self.doc(&a)?; + let after = doc.text_blocks(page); + // Without `text` the paragraph keeps the document's own words: not echoed when + // the document doesn't allow copying them. + let copy = doc.allows_copying(); // Only a paragraph that now shows exactly the requested text is the edited one. A // neighbour that overlaps the old box (overlaid text, a text layer) must not take // its number, and neither must a paragraph the edit merged it into: rewriting that @@ -717,7 +721,10 @@ impl Automation { let edited = block_overlapping(&after, moved) .filter(|&i| !wanted.is_empty() && after.get(i).is_some_and(|b| b.text.split_whitespace().collect::() == wanted)); if let Some(b) = edited.and_then(|i| after.get(i)) { - out["paragraph"] = json!({ "text": b.text, "lines": b.lines.len(), "font": b.base_font }); + out["paragraph"] = json!({ "lines": b.lines.len(), "font": b.base_font }); + if copy { + out["paragraph"]["text"] = json!(b.text); + } } let stale = self.follow_edit(id, page, k, edited.and_then(|i| after.get(i)), &after); if !stale.is_empty() { diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index 9ce6b12d4f..4fff59c07d 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -1172,7 +1172,7 @@ pub fn tools() -> Vec { )), t("js_enabled", "JavaScript on or off", "Preferences ▸ JavaScript ▸ Enable Acrobat JavaScript: set it with `enabled`, or read it. With JavaScript off, field scripts other than Acrobat's AF calls don't run.") .with(schema(json!({ "enabled": { "type": "boolean" } }), &[])), - t("ocr_recognize", "Recognize text (OCR)", "Scan & OCR ▸ Recognize text: render pages, read the words in them and add them as invisible text over the page image, so scanned pages become searchable and selectable (a searchable image; the image is not changed). Pages that already have text are skipped unless skip_text_pages is false; then pictures on those pages are read too, and recognised words that the page's own text already has (an earlier OCR layer included) are left out and counted as already_on_page. Returns each page's recognised text, word count or why it was skipped. Needs the OCR models (ocr_status). Undoable as one step.") + t("ocr_recognize", "Recognize text (OCR)", "Scan & OCR ▸ Recognize text: render pages, read the words in them and add them as invisible text over the page image, so scanned pages become searchable and selectable (a searchable image; the image is not changed). Pages that already have text are skipped unless skip_text_pages is false; then pictures on those pages are read too, and recognised words that the page's own text already has (an earlier OCR layer included) are left out and counted as already_on_page. Returns each page's recognised text (left out when the document's security doesn't allow copying), word count or why it was skipped. Needs the OCR models (ocr_status). Undoable as one step.") .cmd("ocr.recognize") .with(schema( json!({ @@ -1332,7 +1332,7 @@ pub fn tools() -> Vec { .ro() .cmd("edit.edit_text") .with(schema(json!({ "doc": doc(), "page": { "type": "integer", "minimum": 1 }, "paragraph": { "type": "integer", "minimum": 1 }, "text": { "type": "string" }, "size": { "type": "number" }, "width": { "type": "number" }, "font_resource":{"type":"string"}, "ranges":{"type":"array","maxItems":1024,"items":{"type":"object"}} }), &["doc", "page", "paragraph"])), - t("text_edit", "Edit text", "Replace the text of one paragraph (paragraph, from text_paragraphs: rewrapped to the paragraph's width with its line spacing) or one line (line, from text_lines) in place, keeping position, size and colour. For a mixed-style region, editor_text supplies visual row breaks; surviving text keeps its source-run styles and inserted text inherits the surrounding run. For a paragraph, also change its formatting: font (helvetica, times, courier) with bold/italic, size (points), color (#rrggbb), align (left, center, right, justify), underline, line_spacing (× size), char_spacing (points) and scale (horizontal, percent), or move it (dx, dy in points; up is +dy) and rewrap it to a new width (points); text may then be omitted. Its own font is reused when it can show every character and no font/weight change is requested; otherwise it uses a standard font matching the source style or, for text no standard font can show, real fallback outlines from PdfCraft's own faces, from the one that has every character: the craft-fonts Japanese faces (Mincho/Gothic matching the requested/source family and available weight) or, behind them, the bundled Inter face, which carries the Latin, Greek and Cyrillic letters they lack (Uzbek қ, Kazakh ә). The result shows the font used. Missing Japanese weights use Regular, not synthetic bold; the small web build has only Gothic Regular. Paragraph bold may be set without font to keep the source family. Text that no available font can show is refused. A paragraph number is refused when an earlier edit merged that paragraph into a neighbour (list the page again); the result's stale_paragraphs names the numbers gone. Undoable.") + t("text_edit", "Edit text", "Replace the text of one paragraph (paragraph, from text_paragraphs: rewrapped to the paragraph's width with its line spacing) or one line (line, from text_lines) in place, keeping position, size and colour. For a mixed-style region, editor_text supplies visual row breaks; surviving text keeps its source-run styles and inserted text inherits the surrounding run. For a paragraph, also change its formatting: font (helvetica, times, courier) with bold/italic, size (points), color (#rrggbb), align (left, center, right, justify), underline, line_spacing (× size), char_spacing (points) and scale (horizontal, percent), or move it (dx, dy in points; up is +dy) and rewrap it to a new width (points); text may then be omitted. Its own font is reused when it can show every character and no font/weight change is requested; otherwise it uses a standard font matching the source style or, for text no standard font can show, real fallback outlines from PdfCraft's own faces, from the one that has every character: the craft-fonts Japanese faces (Mincho/Gothic matching the requested/source family and available weight) or, behind them, the bundled Inter face, which carries the Latin, Greek and Cyrillic letters they lack (Uzbek қ, Kazakh ә). The result shows the font used, and for a paragraph its text unless the document's security doesn't allow copying. Missing Japanese weights use Regular, not synthetic bold; the small web build has only Gothic Regular. Paragraph bold may be set without font to keep the source family. Text that no available font can show is refused. A paragraph number is refused when an earlier edit merged that paragraph into a neighbour (list the page again); the result's stale_paragraphs names the numbers gone. Undoable.") .cmd("edit.edit_text") .with(schema( json!({ "doc": doc(), "page": { "type": "integer", "minimum": 1 }, "line": { "type": "integer", "minimum": 1 }, diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index 0ebf3a9bc0..778deaaec4 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -2130,6 +2130,44 @@ fn protecting_through_tools() { ok(&mut c, "doc_open", json!({ "path": "open.pdf" })); } +/// A document that allows changes but not copying: the tools that return a page's text refuse, +/// or leave the text out where the tool also changes the page (a restyled paragraph, recognized +/// words). +#[test] +fn copy_restricted_text_stays_in_the_document() { + let dir = workdir("copy-restricted-text"); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap(); + let locked = json!({ "open_password": "open", "permissions_password": "boss", "changes": "any-except-extract" }); + let mut protect = locked.clone(); + protect["doc"] = json!(doc); + ok(&mut a, "doc_protect", protect); + ok(&mut a, "doc_save", json!({ "doc": doc, "path": "locked.pdf" })); + let re = ok(&mut a, "doc_open", json!({ "path": "locked.pdf", "password": "open" }))["doc"].as_u64().unwrap(); + match a.call("text_lines", &json!({ "doc": re, "page": 1 })) { + Err(ToolError::Failed(m)) => assert!(m.contains("don't allow copying"), "{m}"), + other => panic!("text_lines is refused: {other:?}"), + } + let restyled = ok(&mut a, "text_edit", json!({ "doc": re, "page": 1, "paragraph": 1, "size": 30 })); + assert!(!restyled.to_string().contains("Page"), "{restyled}"); + if ok(&mut a, "ocr_status", json!({}))["available"] != true { + eprintln!("skipped the OCR part: OCR models not installed"); + return; + } + let text = ok(&mut a, "doc_create", json!({ "from": "text", "text": "Recognised words stay private" }))["doc"].as_u64().unwrap(); + ok(&mut a, "doc_export_images", json!({ "doc": text, "folder": "scan", "dpi": 150, "pages": [1] })); + let png = std::fs::read_dir(dir.join("scan")).unwrap().next().unwrap().unwrap().path(); + let scan = ok(&mut a, "doc_create", json!({ "from": "images", "paths": [png.to_string_lossy()] }))["doc"].as_u64().unwrap(); + let mut protect = locked; + protect["doc"] = json!(scan); + ok(&mut a, "doc_protect", protect); + ok(&mut a, "doc_save", json!({ "doc": scan, "path": "scan.pdf" })); + let scan = ok(&mut a, "doc_open", json!({ "path": "scan.pdf", "password": "open" }))["doc"].as_u64().unwrap(); + let r = ok(&mut a, "ocr_recognize", json!({ "doc": scan })); + assert!(r["words"].as_u64().unwrap() >= 3, "{r}"); + assert!(r["pages"][0].get("text").is_none() && !r.to_string().to_lowercase().contains("private"), "{r}"); +} + /// A form secured with "fill-sign" changes and no open password (how secured forms are usually /// distributed): its existing signature field can be signed without the permissions password, /// the update keeps the encryption, and the signature validates after reopening. A new field From 30300ae71be58cc5e1800cdbd19c9ec432f7a0ee Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:37:46 +0000 Subject: [PATCH 03/42] fix(engine): compare only documents that allow copying their text Compare read both documents' words out with no check of the copying permission, and its result, report and difference comments carry that text, so a copy-restricted document's text could be taken out through a comparison. Both documents must now allow copying. --- crates/engine/src/compare.rs | 5 ++++- crates/engine/src/tests.rs | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/compare.rs b/crates/engine/src/compare.rs index 50508e20ea..c23db91f1f 100644 --- a/crates/engine/src/compare.rs +++ b/crates/engine/src/compare.rs @@ -46,10 +46,13 @@ impl crate::Document { } impl Session { - /// Compare files: the text differences from document `old` to document `new`. + /// Compare files: the text differences from document `old` to document `new`. The result + /// carries both documents' text, so both must allow copying it. pub fn compare(&self, old: DocId, new: DocId) -> Result { let a = self.get(old).ok_or(EditError::NoDocument)?; let b = self.get(new).ok_or(EditError::NoDocument)?; + a.check_copying()?; + b.check_copying()?; Ok(pdfcraft_compare::compare(&a.words(), &b.words())) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 0df176226a..f93e765260 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1311,6 +1311,12 @@ fn copy_restricted_documents_refuse_extraction_unless_opened_by_the_owner() { let oid = s.open("r.pdf", None, bytes, Some("owner")).unwrap(); assert!(s.get(oid).unwrap().allows_copying()); assert!(extracting.iter().all(|c| enabled(&s, oid, c))); + // Comparing reads both documents' text out, into the result, the report and the comments. + let refused = || Err(EditError::NotPermitted("copying text and images")); + assert_eq!((s.compare(id, oid).map(|_| ()), s.compare(oid, id).map(|_| ())), (refused(), refused())); + assert_eq!(s.compare_report(oid, id).map(|_| ()), refused()); + assert!(s.compare(oid, oid).is_ok()); + assert_eq!(s.mark_differences(id, oid).map(|_| ()), refused()); // Unencrypted documents allow everything. let (s, plain) = session_with(1); assert!(s.get(plain).unwrap().check_copying().is_ok() && extracting.iter().all(|c| enabled(&s, plain, c))); From 28ab616eb62a7ddc13b32895cd2215146f7ff812 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:37:55 +0000 Subject: [PATCH 04/42] fix(engine): print as low-resolution pictures when only that is allowed Printing checked the print permission but not the separate high-quality one (Table 22, bit 12), so a document limited to low-resolution printing came out as its original vector pages, text included. Such a document now always prints as pictures of at most 150 dpi, whatever the settings ask for. --- crates/engine/src/lib.rs | 12 +++++++++++- crates/engine/src/tests.rs | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index e20cbc596e..fc1a8d2074 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -232,6 +232,10 @@ pub struct DocId(pub u64); /// Undo/redo depth. Snapshots share unchanged data, so this is memory-cheap. const MAX_UNDO: usize = 100; +/// The highest resolution a document that allows only low-resolution printing (Table 22, bit 12 +/// clear) is printed at. +const LOW_RESOLUTION_PRINT_DPI: f64 = 150.0; + /// The passwords that go with one state of a document (protection can change them). #[derive(Clone, Debug, Default, PartialEq)] struct Keys { @@ -4522,9 +4526,15 @@ impl Session { if !doc.allows_printing() { return Err(EditError::NotPermitted("printing")); } + // Table 22, bit 12 clear: only a low-level representation may be printed, so the pages + // always go as pictures, at no more than the low resolution. + let low_res = doc.permissions().is_some_and(|p| !p.print_high_quality()).then(|| print::Raster { + dpi: settings.raster.map_or(LOW_RESOLUTION_PRINT_DPI, |r| r.dpi.min(LOW_RESOLUTION_PRINT_DPI)), + grayscale: settings.raster.is_some_and(|r| r.grayscale), + }); // Print as image: the pages used become pictures first, then the picture document is // imposed in their place (its page k is the k-th distinct source page used). - if let Some(raster) = &settings.raster { + if let Some(raster) = low_res.as_ref().or(settings.raster.as_ref()) { let mut distinct: Vec = Vec::new(); for &p in &settings.pages { if !distinct.contains(&p) { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index f93e765260..2d67305445 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1340,6 +1340,38 @@ fn print_restricted_documents_disable_print_unless_opened_by_the_owner() { assert!(enabled(&s, low, "print.dialog"), "low-resolution printing is printing"); } +/// Table 22, bit 12 clear: printing is limited to a low-level representation. Whatever the +/// settings ask for, the print-ready PDF carries each page as a picture of at most 150 dpi, with +/// no text or vector artwork left; the owner password lifts the limit. +#[test] +fn low_resolution_printing_prints_pictures_of_at_most_150_dpi() { + let bytes = protected("", "owner", -1 ^ (1 << 11)); + let mut s = Session::new(); + let low = s.open("low.pdf", None, bytes.clone(), None).unwrap(); + let width = f64::from(s.get(low).unwrap().info.pages[0].width); + let actual = print::Settings { pages: vec![0], layout: print::Layout::Size(print::SizeMode::Actual), ..Default::default() }; + for raster in [None, Some(print::Raster { dpi: 600.0, grayscale: false })] { + let printed = s.print_pdf(low, &print::Settings { raster, ..actual.clone() }).unwrap(); + let mut s2 = Session::new(); + let id2 = s2.open("print.pdf", None, Arc::new(printed.clone()), None).unwrap(); + assert!(page_texts(&s2, id2).iter().all(|t| t.trim().is_empty()), "{raster:?}: no text is left"); + let doc = pdfcraft_cos::Document::open(Arc::new(printed)).unwrap(); + let widths: Vec = doc + .scan_objects() + .filter_map(|(_, o)| match &*o { + pdfcraft_cos::Object::Stream(st) if st.dict.name(b"Subtype") == Some(b"Image") => st.dict.int(b"Width"), + _ => None, + }) + .collect(); + assert!(!widths.is_empty() && widths.iter().all(|&w| w as f64 <= width * 150.0 / 72.0 + 1.0), "{raster:?}: {widths:?} px for {width} pt"); + } + let owner = s.open("low.pdf", None, bytes, Some("owner")).unwrap(); + let printed = s.print_pdf(owner, &actual).unwrap(); + let mut s2 = Session::new(); + let id2 = s2.open("print.pdf", None, Arc::new(printed), None).unwrap(); + assert!(page_texts(&s2, id2).iter().any(|t| !t.trim().is_empty()), "the owner prints the pages themselves"); +} + #[test] fn the_permissions_password_unlocks_an_open_document() { // Only a permissions password (#995, #1077): opens without asking. Changes are allowed, From 73f11b2a29075094381d0a9d4c1bbcdb90627c8e Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:37:55 +0000 Subject: [PATCH 05/42] fix(engine): fail an action whose information-removal step fails A step that applies an edit logged any error and carried on, which is right for a flatten with nothing to flatten but not for removing hidden information or sanitizing: on a signed file the removal was refused, and the unchanged file was written out as the action's successful result. Those steps now fail the action unless there was nothing to remove. --- crates/engine/src/actions.rs | 6 +++++- crates/engine/src/tests.rs | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/actions.rs b/crates/engine/src/actions.rs index f679cf87cd..f7d11d47ed 100644 --- a/crates/engine/src/actions.rs +++ b/crates/engine/src/actions.rs @@ -191,9 +191,13 @@ fn apply_step(s: &mut Session, id: DocId, step: &Step, log: &mut Vec) -> return Ok(None); } }; - // Steps that find nothing to do (no comments to flatten, …) aren't failures. + // Steps that find nothing to do (no comments to flatten, …) aren't failures. A removal step + // that didn't remove is: the file would be passed on as cleaned with the data still in it. + let removes = matches!(step, Step::RemoveHiddenInformation | Step::Sanitize); match s.apply(id, edit) { Ok(()) => log.push(step.label().to_string()), + Err(EditError::Redact(pdfcraft_redact::RedactError::NothingToApply)) if removes => log.push(format!("{}: nothing to remove", step.label())), + Err(e) if removes => return Err(e), Err(e) => log.push(format!("{}: {e}", step.label())), } Ok(None) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2d67305445..217d8d166b 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -4274,6 +4274,24 @@ fn actions_run_their_steps_on_files() { assert!(actions::run_on(&custom, "bad.pdf", Arc::new(b"nope".to_vec()), |_, _| {}).is_err()); } +/// A removal step that fails (here: a signed file can't be rewritten without its hidden data) +/// fails the action, so the file is not passed on as cleaned. +#[test] +fn actions_fail_when_hidden_information_could_not_be_removed() { + let p12 = std::fs::read(concat!(env!("CARGO_MANIFEST_DIR"), "/../sign/tests/data/ec-p256.p12")).unwrap(); + let digital_id = sign::pkcs12::open(&p12, "test").unwrap(); + let mut s = Session::new().with_clock(|| 1_800_000_000); + let id = s.open("fixture.pdf", None, Arc::new(fixture(2)), None).unwrap(); + s.apply(id, Edit::SetInfo { key: "Author".into(), value: "Private Name".into() }).unwrap(); + let saved = s.save_bytes(id).unwrap(); + s.mark_saved(id, saved, None).unwrap(); + let signed = s.sign(id, &digital_id, SignOptions { page: 1, ..SignOptions::default() }).unwrap(); + let built = actions::builtin(); + let sanitize = built.iter().find(|a| a.name == "Sanitize Documents").unwrap(); + let e = actions::run_on(sanitize, "signed.pdf", signed, |_, _| {}).map(|r| r.log).unwrap_err(); + assert!(e.starts_with("Sanitize document:"), "{e}"); +} + #[test] fn exporting_office_files_keeps_images() { let mut s = Session::new(); From c3bc881607b71be14cdccbba50fc5e81eb54247d Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:39:40 +0000 Subject: [PATCH 06/42] fix(platform): a saved file keeps its group along with its mode The atomic writer gave the new file the replaced file's mode but left it in the saver's own group, so a file readable by one group became readable by another. The staging file joins the replaced file's group before it gets the mode and before anything is written; where it can't, group and others get no access. --- crates/platform/src/staging.rs | 69 +++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 9 deletions(-) diff --git a/crates/platform/src/staging.rs b/crates/platform/src/staging.rs index 0b727449cb..f6a98c4fe4 100644 --- a/crates/platform/src/staging.rs +++ b/crates/platform/src/staging.rs @@ -31,16 +31,16 @@ pub fn create_staging( create_staging_as(dir, name, form, suffixes, None) } -/// [`create_staging`], giving the new file the permissions `keep` on Unix. It is created readable -/// and writable by its owner only and then given `keep`, so its mode is never more open than that -/// of the file it will replace, not even while it is still empty (a handle opened then would keep -/// reading what is written later). Only the mode is copied: the owner and group are the saver's. +/// [`create_staging`], giving the new file the mode and group of the file `keep` describes on +/// Unix. It is created readable and writable by its owner only and then given them, so its mode +/// is never more open than that of the file it will replace, not even while it is still empty (a +/// handle opened then would keep reading what is written later). The owner is the saver's. fn create_staging_as( dir: &Path, name: &str, form: StagingName, suffixes: impl IntoIterator, - keep: Option<&std::fs::Permissions>, + keep: Option<&std::fs::Metadata>, ) -> std::io::Result<(PathBuf, std::fs::File)> { // At most 128 bytes of the target's name, cut between characters, so the staging name fits // the 255-byte (Linux, macOS) and 255-unit (Windows) limits however long that name is. @@ -68,7 +68,7 @@ fn create_staging_as( if let Some(keep) = keep { // A volume that can't change modes (FAT, some network shares) leaves the file // owner-only: stricter than the file it replaces, never looser. - let _ = file.set_permissions(keep.clone()); + let _ = file.set_permissions(kept_permissions(&file, keep)); } return Ok((tmp, file)); } @@ -81,6 +81,26 @@ fn create_staging_as( Err(std::io::Error::new(std::io::ErrorKind::AlreadyExists, "every temporary file name tried is taken")) } +/// The permissions a staging file gets to replace the file `keep` describes. The mode's group bits +/// are meant for that file's group, so the staging file joins it first. Where it can't (the saver +/// isn't a member), group and others get nothing: someone the group bits shut out would +/// otherwise read the file as one of the others. +#[cfg(unix)] +fn kept_permissions(file: &std::fs::File, keep: &std::fs::Metadata) -> std::fs::Permissions { + use std::os::unix::fs::{MetadataExt as _, PermissionsExt as _}; + let mut mode = keep.permissions(); + let joined = file.metadata().is_ok_and(|m| m.gid() == keep.gid()) || std::os::unix::fs::fchown(file, None, Some(keep.gid())).is_ok(); + if !joined { + mode.set_mode(mode.mode() & !0o077); + } + mode +} + +#[cfg(not(unix))] +fn kept_permissions(_: &std::fs::File, keep: &std::fs::Metadata) -> std::fs::Permissions { + keep.permissions() +} + /// Unpredictable staging-name suffixes, so a name can't be planted in advance. Each is the /// standard library's keyed hash of a counter (`RandomState::hash_one`, currently SipHash-1-3), /// keyed from the operating system's random source. That is all a staging name needs, but it is @@ -98,7 +118,7 @@ pub fn staging_suffixes() -> impl Iterator { /// and the target is untouched until the new file is complete. The target's folder must exist; /// a folder at the target is refused by the rename. Flushing is the staging file's own /// (`sync_all`); parent-directory durability after a power loss is not promised. On Unix a file -/// that is replaced keeps its permissions, so a private file stays private; a new file gets the +/// that is replaced keeps its permissions and group, so a private file stays private; a new file gets the /// usual mode for a new file. On Windows and macOS a replaced file keeps its creation date, where /// the volume stores one. /// On macOS, accessible extended attributes of an existing regular destination are copied @@ -126,9 +146,9 @@ pub fn write_atomically_with( } let dir = path.parent().filter(|d| !d.as_os_str().is_empty()).unwrap_or(Path::new(".")); let name = path.file_name().and_then(|n| n.to_str()).unwrap_or("save"); - // A file that is replaced keeps its permissions (#1790). Unix modes only: Windows is as before. + // A file that is replaced keeps its permissions (#1790) and group. Unix only: Windows is as before. #[cfg(unix)] - let keep = std::fs::metadata(path).ok().map(|m| m.permissions()); + let keep = std::fs::metadata(path).ok(); #[cfg(not(unix))] let keep = None; // A saved file is the same document: it keeps its creation date (#2658). @@ -583,6 +603,37 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// A replaced file keeps its group as well as its mode, so group permissions never reach a + /// group that couldn't read the file before (the saver's own group, by default). + #[cfg(target_os = "linux")] + #[test] + fn a_replaced_file_keeps_its_group() { + use std::os::unix::fs::{MetadataExt as _, PermissionsExt as _}; + let dir = test_dir("group"); + let target = dir.join("out.pdf"); + std::fs::write(&target, b"OLD").unwrap(); + let created = std::fs::metadata(&target).unwrap().gid(); + let status = std::fs::read_to_string("/proc/self/status").unwrap(); + let groups = status.lines().find_map(|l| l.strip_prefix("Groups:")).unwrap_or_default(); + let Some(other) = groups.split_whitespace().filter_map(|g| g.parse::().ok()).find(|&g| g != created) else { + eprintln!("skipped: the test needs a second group"); + return; + }; + std::os::unix::fs::chown(&target, None, Some(other)).unwrap(); + std::fs::set_permissions(&target, std::fs::Permissions::from_mode(0o640)).unwrap(); + let mut staged = (0, 0); + write_atomically_with(&target, b"NEW", StagingName::SuffixThenTag, [7], |f| { + let m = f.metadata()?; + staged = (m.gid(), m.permissions().mode() & 0o777); + Ok(()) + }) + .unwrap(); + assert_eq!(staged, (other, 0o640), "the staging file has the target's group before it is flushed"); + let m = std::fs::metadata(&target).unwrap(); + assert_eq!((m.gid(), m.permissions().mode() & 0o777), (other, 0o640)); + let _ = std::fs::remove_dir_all(&dir); + } + /// #2658: a save replaces the file, but the document is the same one: it keeps its creation /// date where the system lets a program set it. #[cfg(any(windows, target_os = "macos"))] From 77480498b25f34028198c32f15afb50516664a73 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 17:04:07 +0000 Subject: [PATCH 07/42] fix(engine): low-resolution printing prints only the chosen content Pages printed as pictures were drawn with everything on them, so form fields only printed the page content, and the document with stamps printed every comment. The pictures are drawn from a copy holding only what the vector print draws. --- crates/engine/src/lib.rs | 3 +- crates/engine/src/print_raster.rs | 26 +++++++---- crates/engine/src/tests.rs | 77 +++++++++++++++++++++++++++++++ crates/print/src/lib.rs | 19 ++++++++ 4 files changed, 114 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index fc1a8d2074..817eb79a96 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -4541,8 +4541,7 @@ impl Session { distinct.push(p); } } - let hide_comments = matches!(settings.content, print::Content::Document | print::Content::FormFieldsOnly); - let bytes = print_raster::rasterize(doc, &distinct, raster, hide_comments).map_err(EditError::Print)?; + let bytes = print_raster::rasterize(doc, &distinct, raster, settings.content).map_err(EditError::Print)?; let pictures = pdfcraft_cos::Document::open(Arc::new(bytes)).map_err(|e| EditError::Print(e.to_string()))?; let remapped = print::Settings { pages: settings.pages.iter().map(|p| distinct.iter().position(|d| d == p).unwrap_or(0)).collect(), diff --git a/crates/engine/src/print_raster.rs b/crates/engine/src/print_raster.rs index 117c8e14cc..4ed5895860 100644 --- a/crates/engine/src/print_raster.rs +++ b/crates/engine/src/print_raster.rs @@ -1,25 +1,33 @@ //! Print as image (Acrobat's Advanced Print Setup ▸ Print As Image): every page //! sent to the printer as a picture at a chosen resolution, for printers that skip text, //! shading or transparency. The pages are rendered the way the viewer shows them (reading -//! colours off, the print's own colours), made into an image-only document, and that document -//! is imposed on the paper like any other. The print is no longer searchable text. +//! colours off, the print's own colours) with only what the print's content choice keeps, made +//! into an image-only document, and that document is imposed on the paper like any other. The +//! print is no longer searchable text. +use pdfcraft_cos::{SaveOptions, write_incremental}; use pdfcraft_create::ImageResolution; -use pdfcraft_print::Raster; +use pdfcraft_print::{Content, Raster}; use pdfcraft_render::{PRINT_TAG, PageRenderer, RenderRequest}; -use crate::Document; use crate::export::encode_png; +use crate::{Document, guard}; /// `doc`'s pages `pages` (0-based, each once, in the order given) as an image-only PDF, one -/// page per entry, the size of the source page as displayed. Pages too large for the renderer -/// at `raster.dpi` render at the largest resolution it allows. -pub fn rasterize(doc: &Document, pages: &[usize], raster: &Raster, hide_comments: bool) -> Result, String> { +/// page per entry, the size of the source page as displayed, drawing what `content` prints. +/// Pages too large for the renderer at `raster.dpi` render at the largest resolution it allows. +pub fn rasterize(doc: &Document, pages: &[usize], raster: &Raster, content: Content) -> Result, String> { let dpi = raster.dpi.clamp(72.0, 600.0); let src = doc.export_source(); let mut config = src.config.clone(); - config.hide_comments = hide_comments; - let mut renderer = PageRenderer::new(src.bytes, config); + // The copy below holds only what prints, so the viewer's own comment hiding plays no part. + config.hide_comments = false; + let printed = guard(|| -> Result, String> { + let mut cos = pdfcraft_cos::Document::open_with_password(src.bytes.clone(), config.password.as_deref()).map_err(|e| e.to_string())?; + pdfcraft_print::keep_printed(&mut cos, content); + write_incremental(&cos, &SaveOptions::default()).map_err(|e| e.to_string()) + })??; + let mut renderer = PageRenderer::new(std::sync::Arc::new(printed), config); let mut images: Vec<(String, Vec)> = Vec::with_capacity(pages.len()); let mut used: Option = None; for &page in pages { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 217d8d166b..3d347e6492 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1372,6 +1372,83 @@ fn low_resolution_printing_prints_pictures_of_at_most_150_dpi() { assert!(page_texts(&s2, id2).iter().any(|t| !t.trim().is_empty()), "the owner prints the pages themselves"); } +/// A 100 × 400 pt page with one band per kind of thing that prints, top to bottom: page content +/// (blue), a filled text field (green), a stamp (red) and a square comment (yellow). +fn print_content_pdf() -> Vec { + let content = "0 0 1 rg 0 300 100 100 re f"; + let ap = |colour: &str| { + let ops = format!("{colour} rg 0 0 100 100 re f"); + format!("<< /Type /XObject /Subtype /Form /BBox [0 0 100 100] /Length {} >> stream\n{ops}\nendstream", ops.len()) + }; + format!( + "%PDF-1.7\n\ + 1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [5 0 R] >> >> endobj\n\ + 2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n\ + 3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 100 400] /Contents 4 0 R /Annots [5 0 R 6 0 R 7 0 R] >> endobj\n\ + 4 0 obj << /Length {} >> stream\n{content}\nendstream endobj\n\ + 5 0 obj << /Type /Annot /Subtype /Widget /FT /Tx /T (Name) /V (Filled) /F 4 /P 3 0 R /Rect [0 200 100 300] /AP << /N 8 0 R >> >> endobj\n\ + 6 0 obj << /Type /Annot /Subtype /Stamp /F 4 /P 3 0 R /Rect [0 100 100 200] /AP << /N 9 0 R >> >> endobj\n\ + 7 0 obj << /Type /Annot /Subtype /Square /F 4 /P 3 0 R /Rect [0 0 100 100] /AP << /N 10 0 R >> >> endobj\n\ + 8 0 obj {} endobj\n\ + 9 0 obj {} endobj\n\ + 10 0 obj {} endobj\n\ + trailer << /Root 1 0 R >>\n%%EOF", + content.len(), + ap("0 1 0"), + ap("1 0 0"), + ap("1 1 0"), + ) + .into_bytes() +} + +/// Low-resolution printing prints what the chosen content prints when the pages themselves are +/// printed: form fields only leaves out the page content and every comment, the document alone +/// leaves out the comments, and the document with stamps keeps only the stamps. +#[test] +fn low_resolution_printing_prints_only_the_chosen_content() { + let mut cos = pdfcraft_cos::Document::open(Arc::new(print_content_pdf())).unwrap(); + cos.set_encryption(&pdfcraft_cos::NewEncryption { + algorithm: pdfcraft_cos::Algorithm::Aes256, + user_password: "", + owner_password: "owner", + permissions: -1 ^ (1 << 11), + encrypt_metadata: true, + seed: [7; 32], + }) + .unwrap(); + let bytes = Arc::new(pdfcraft_cos::write_full(&cos, &Default::default()).unwrap()); + let mut s = Session::new(); + let low = s.open("low.pdf", None, bytes.clone(), None).unwrap(); + let owner = s.open("low.pdf", None, bytes, Some("owner")).unwrap(); + // Each band's centre, in pixels from the top of the sheet at 72 dpi, and its colour. + let bands = [("content", 50, [0, 0, 255]), ("field", 150, [0, 255, 0]), ("stamp", 250, [255, 0, 0]), ("comment", 350, [255, 255, 0])]; + let all: &[&str] = &["content", "field", "stamp", "comment"]; + for (content, shown) in [ + (print::Content::DocumentAndMarkups, all), + (print::Content::Document, &["content", "field"][..]), + (print::Content::DocumentAndStamps, &["content", "field", "stamp"][..]), + (print::Content::FormFieldsOnly, &["field"][..]), + ] { + let settings = print::Settings { + pages: vec![0], + paper: (100.0, 400.0), + layout: print::Layout::Size(print::SizeMode::Actual), + content, + ..Default::default() + }; + for (id, how) in [(owner, "pages"), (low, "pictures")] { + let (w, h, rgba) = printed_pixels(s.print_pdf(id, &settings).unwrap()); + assert_eq!((w, h), (100, 400), "{content:?} as {how}"); + for (band, y, colour) in bands { + let px = rgba.get((y * w as usize + 50) * 4..).and_then(|p| p.get(..3)).unwrap(); + let near = |c: [u8; 3]| px.iter().zip(c).all(|(&a, b)| a.abs_diff(b) < 40); + let expected = shown.contains(&band); + assert!(if expected { near(colour) } else { near([255; 3]) }, "{content:?} as {how}: {band} drawn {expected} expected, pixel {px:?}"); + } + } + } +} + #[test] fn the_permissions_password_unlocks_an_open_document() { // Only a permissions password (#995, #1077): opens without asking. Changes are allowed, diff --git a/crates/print/src/lib.rs b/crates/print/src/lib.rs index e935cb938e..6839e5f6b0 100644 --- a/crates/print/src/lib.rs +++ b/crates/print/src/lib.rs @@ -696,6 +696,25 @@ fn decoded_contents(doc: &Document, page: &Dict) -> Vec { out } +/// Reduce `doc`'s pages to what [`impose`] draws of them under `content`: the annotations that +/// don't print are dropped, and with [`Content::FormFieldsOnly`] the page content too. For +/// printing pages as pictures, where a renderer draws each page as it stands. +pub fn keep_printed(doc: &mut Document, content: Content) { + for page in pdfcraft_model::pages(doc) { + let annots = page.dict.get(b"Annots").and_then(|a| doc.resolve(a).as_array().cloned()).unwrap_or_default(); + let kept: Vec = annots.into_iter().filter(|a| doc.resolve(a).as_dict().is_some_and(|d| prints(doc, d, content))).collect(); + // Fails only when the page object isn't a dictionary, and such a page draws nothing. + let _ = doc.update_dict(page.obj, |d| { + if content == Content::FormFieldsOnly { + d.remove(b"Contents"); + } + if d.contains(b"Annots") { + d.set(b"Annots".to_vec(), Object::Array(kept)); + } + }); + } +} + /// Does annotation `d` print under `content`? fn prints(doc: &Document, d: &Dict, content: Content) -> bool { const HIDDEN: i64 = 2; From 4aad36cf199aa7d5dd568a695dd810e6dc665a43 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:05:41 +0000 Subject: [PATCH 08/42] fix(create): refuse an indexed colour space whose base is indexed before following it Image export parsed an Indexed colour space's base before checking whether that base was itself Indexed. A base that refers back to the same array recursed until the stack overflowed. The base is now checked first; since Indexed is the only colour space that recurses, this also ends any cycle. --- crates/create/src/extract.rs | 11 +++++++++-- crates/create/src/tests.rs | 16 ++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/crates/create/src/extract.rs b/crates/create/src/extract.rs index 768da4d1f1..bd5ab5271c 100644 --- a/crates/create/src/extract.rs +++ b/crates/create/src/extract.rs @@ -383,10 +383,17 @@ fn space(doc: &Document, cs: Option<&Object>) -> Result { Some(b"CalGray") => Ok(Space::Gray), Some(b"CalRGB") => Ok(Space::Rgb), Some(b"Indexed" | b"I") => { - let base = space(doc, a.get(1))?; - if matches!(base, Space::Indexed(..)) { + // Checked before recursing: Indexed is the only space that recurses, so refusing + // an Indexed base here also ends a cyclic base reference. + let nested = match a.get(1).map(|b| doc.resolve(b)).as_deref() { + Some(Object::Array(b)) => matches!(b.first().and_then(Object::as_name), Some(b"Indexed" | b"I")), + Some(Object::Name(n)) => matches!(n.as_slice(), b"Indexed" | b"I"), + _ => false, + }; + if nested { return Err("nested indexed colour spaces aren't valid".into()); } + let base = space(doc, a.get(1))?; let hival = match a.get(2).map(|value| doc.resolve(value)).as_deref() { Some(Object::Int(value)) if (0..=255).contains(value) => *value as u8, _ => return Err("an indexed colour space with an invalid hival".into()), diff --git a/crates/create/src/tests.rs b/crates/create/src/tests.rs index 6356105699..a214977675 100644 --- a/crates/create/src/tests.rs +++ b/crates/create/src/tests.rs @@ -1181,3 +1181,19 @@ fn a_jpeg_with_an_exif_orientation_becomes_an_upright_page() { let img = image_of(&doc, 1); assert_eq!((img.int(b"Width"), img.int(b"Height")), (Some(60), Some(20))); } + +#[test] +fn a_cyclic_indexed_colour_space_is_refused() { + let mut doc = from_images(&[("flat.png".into(), png_bytes(false))]).unwrap(); + // 5 0 obj [/Indexed 5 0 R 255 <00>]: the base is the indexed space itself. + let cs = doc.add(Object::Null); + doc.set(cs, Object::Array(vec![Object::name("Indexed"), Object::Ref(cs), Object::Int(255), Object::String(PdfString::literal(vec![0]))])); + with_image(&mut doc, |_| { + let mut dict = image_dict(1, 1, "DeviceGray", 8); + dict.set(b"ColorSpace".to_vec(), Object::Ref(cs)); + Stream::from_raw(dict, vec![0]) + }); + let out = extract_images(&doc, &[0], 0); + assert!(out.images.is_empty()); + assert_eq!(out.skipped.len(), 1, "{:?}", out.skipped); +} From 78a562bc081b8b2afad687e2399071c003ae9a19 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:08:32 +0000 Subject: [PATCH 09/42] fix(optimize): bound a merged fragment image's height before sizing its buffer The fragment merge multiplied the merged width by a height cast from a float without checking either fit. A tiny fragment rectangle gives a huge height: the multiplication overflowed (a panic in debug builds, a wrap past the pixel limit in release builds) and the u32 cast saturated, sizing an enormous allocation. The height must now fit u32 before the pixel limit is checked, and the buffer size uses checked multiplication. --- crates/optimize/src/fragments.rs | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/crates/optimize/src/fragments.rs b/crates/optimize/src/fragments.rs index b5967eee09..caf617f717 100644 --- a/crates/optimize/src/fragments.rs +++ b/crates/optimize/src/fragments.rs @@ -143,12 +143,16 @@ fn merge(doc: &Document, run: &[Fragment], n: usize) -> Option<(Vec, u32, u3 // Pixel rows per unit of height: the finest fragment's. let density = run.iter().map(|f| f.height as f64 / (f.rect[3] - f.rect[1])).fold(0.0, f64::max); let height = ((top - bottom) * density).round(); - if !(height.is_finite() && height >= 1.0) || (width as u64) * (height as u64) > MAX_PIXELS { + if !(height.is_finite() && height >= 1.0 && height <= f64::from(u32::MAX)) { return None; } + // In range and whole, so the cast is exact. let height = height as u32; - let row = width as usize * n; - let mut out = vec![255u8; row * height as usize]; + if u64::from(width) * u64::from(height) > MAX_PIXELS { + return None; + } + let row = (width as usize).checked_mul(n)?; + let mut out = vec![255u8; row.checked_mul(height as usize)?]; for f in run { let r0 = (((top - f.rect[3]) * density).round().max(0.0) as u32).min(height); let r1 = (((top - f.rect[1]) * density).round().max(0.0) as u32).min(height); @@ -297,3 +301,21 @@ pub(crate) fn run(doc: &mut Document, pages: &[ObjRef], report: &mut Report, pro } Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_merged_height_beyond_u32_is_refused_before_allocating() { + // Two fragments, one pixel row each, the top one 2^-48 high: the merged image would be + // 2^16 × 2^48 pixels, whose product wraps to 0 in u64. + let image = ObjRef { num: 1, generation: 0 }; + let edge = 1.0 - (-48f64).exp2(); + let run = [ + Fragment { image, rect: [0.0, edge, 1.0, 1.0], width: 1 << 16, height: 1 }, + Fragment { image, rect: [0.0, 0.0, 1.0, edge], width: 1 << 16, height: 1 }, + ]; + assert!(merge(&Document::new_empty(), &run, 3).is_none()); + } +} From f0e3880c35327859f82228caaa8219d86b8955c0 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:12:52 +0000 Subject: [PATCH 10/42] fix(xfdf): never let a same-named imported comment replace a form widget Comment import replaces an existing annotation with the same /NM in place. The check for annotations it must keep covered locked comments and redaction marks but not widgets or field dictionaries, so an imported comment could overwrite a field's widget object, setting its value and validation entries without the form value setter while the AcroForm field list still pointed at it. Widgets and field dictionaries are now kept, and the imported comment is added beside them under no name. --- crates/xfdf/src/lib.rs | 11 ++++++----- crates/xfdf/src/tests.rs | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index ab258990e2..1e037b6795 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -1017,10 +1017,11 @@ const REDACT_UNSEEN: i64 = 1 | 2 | 32; /// `/F` Locked (ISO 32000-2 Table 167): the user protected this annotation from change. const LOCKED: i64 = 128; -/// An existing annotation an import must never replace, move or unlock: one the user locked, or a -/// redaction mark (replacing it would drop the region the user marked to remove). +/// An existing annotation an import must never replace, move or unlock: one the user locked, a +/// redaction mark (replacing it would drop the region the user marked to remove), or a form +/// widget or field (replacing it would set the field's value and policy around the form setter). fn import_protected(x: &Dict) -> bool { - x.int(b"F").unwrap_or(0) & LOCKED != 0 || x.name(b"Subtype") == Some(b"Redact".as_slice()) + x.int(b"F").unwrap_or(0) & LOCKED != 0 || matches!(x.name(b"Subtype"), Some(b"Redact" | b"Widget")) || x.contains(b"FT") } fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result { @@ -1042,8 +1043,8 @@ fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result = Vec::new(); - // Never let a same-name import replace a locked comment or a redaction mark: keep it, and add - // the imported dict beside it under no name of its own. + // Never let a same-name import replace a protected annotation (`import_protected`): keep it, + // and add the imported dict beside it under no name of its own. let mut clash = false; for a in annots { let existing = a.as_ref().and_then(|r| doc.get(r).as_dict().cloned()); diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index 6c9824a245..57155e4b7a 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -1660,3 +1660,22 @@ fn tabular_import_keeps_tsv_quotes_literal_and_csv_value_tabs() { import(&mut doc, b"Name,Other\r\n\"Ada\tLovelace\",other field\r\n").unwrap(); assert_eq!(fields(&doc).into_iter().find(|f| f.name == "Name").unwrap().value, vec!["Ada\tLovelace"]); } + +#[test] +fn a_same_named_comment_import_never_replaces_a_form_widget() { + let mut doc = blank(); + let page = pdfcraft_model::pages(&doc)[0].clone(); + let widget = doc + .resolve(page.dict.get(b"Annots").unwrap()) + .as_array() + .unwrap() + .iter() + .filter_map(Object::as_ref) + .find(|r| doc.get(*r).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget".as_slice()))) + .unwrap(); + doc.update_dict(widget, |d| d.set(b"NM".to_vec(), PdfString::text("w1"))).unwrap(); + let before = doc.get(widget); + let fdf = b"%FDF-1.2\n1 0 obj\n<< /FDF << /Annots [<< /Subtype /Text /NM (w1) /Page 0 /Rect [0 0 1 1] /FT /Tx /T (Name) /V (forged) >>] >> >>\nendobj\ntrailer\n<< /Root 1 0 R >>\n%%EOF\n"; + assert_eq!(import(&mut doc, fdf).unwrap().comments, 1); + assert_eq!(doc.get(widget), before, "the widget was overwritten by an imported comment"); +} From d266222339adf378f785c98fdd3ae65eb8831867 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:22:02 +0000 Subject: [PATCH 11/42] fix(xfdf): draw every imported redaction mark's review outline, opaque A redaction mark's outline is drawn from its /QuadPoints, but applying the mark falls back to its /Rect when it has none. An imported mark with only a /Rect, or with quads the outline can't draw, therefore had no visible outline while still removing content when applied, and an imported opacity of 0 hid the outline too. Imported marks now get quads from their /Rect when their own are unusable (or none, so they remove nothing), and their /CA and /ca are dropped. --- crates/xfdf/src/lib.rs | 21 +++++++++++++++++++++ crates/xfdf/src/tests.rs | 23 +++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index 1e037b6795..80c37b5e86 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -1032,6 +1032,27 @@ fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result { + let (l, r, b, t) = (x0.min(x1), x0.max(x1), y0.min(y1), y0.max(y1)); + d.set(b"QuadPoints".to_vec(), arr(&[l, t, r, t, l, b, r, b])); + } + _ => { + d.remove(b"QuadPoints"); + } + } + } } let nm = d.get(b"NM").and_then(|n| n.as_string().map(|s| s.to_text())); let annots: Vec = doc diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index 57155e4b7a..7951bef4dd 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -1679,3 +1679,26 @@ fn a_same_named_comment_import_never_replaces_a_form_widget() { assert_eq!(import(&mut doc, fdf).unwrap().comments, 1); assert_eq!(doc.get(widget), before, "the widget was overwritten by an imported comment"); } + +#[test] +fn an_imported_rect_only_or_transparent_redaction_mark_is_drawn_for_review() { + let xfdf = br#""#; + let fdf = b"%FDF-1.2\n1 0 obj\n<< /FDF << /Annots [<< /Subtype /Redact /NM (r1) /Page 0 /Rect [60 600 560 620] /CA 0 /ca 0 >>] >> >>\nendobj\ntrailer\n<< /Root 1 0 R >>\n%%EOF\n"; + for data in [xfdf.as_slice(), fdf.as_slice()] { + let mut doc = blank(); + assert_eq!(import(&mut doc, data).unwrap().comments, 1); + let page = pdfcraft_model::pages(&doc)[0].clone(); + let mark = doc + .resolve(page.dict.get(b"Annots").unwrap()) + .as_array() + .unwrap() + .iter() + .filter_map(|a| a.as_ref().and_then(|r| doc.get(r).as_dict().cloned())) + .find(|d| d.name(b"Subtype") == Some(b"Redact".as_slice())) + .unwrap(); + assert_eq!(nums_of(&doc, mark.get(b"QuadPoints")), vec![60.0, 620.0, 560.0, 620.0, 60.0, 600.0, 560.0, 600.0]); + let ap = mark.get(b"AP").map(|a| doc.resolve(a)).and_then(|a| a.as_dict().cloned()); + assert!(ap.is_some_and(|ap| ap.get(b"N").is_some()), "the mark has no review appearance: {mark:?}"); + assert!(mark.get(b"CA").is_none() && mark.get(b"ca").is_none(), "the mark can be drawn transparent: {mark:?}"); + } +} From 8a8dab6c00ad27e9d3e1b66e42442c1fbd930ba7 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:37:40 +0000 Subject: [PATCH 12/42] fix(xfdf): keep merged spreadsheet cells from starting a formula Merge Data Files into Spreadsheet writes field names and values from other people's files as CSV cells, escaping only the delimiters. A cell starting with =, +, - or @ is evaluated as a formula when the CSV is opened in a spreadsheet program. Such cells, after any leading whitespace, are now written with a leading apostrophe; finite numbers such as -12.5 are left as they are. --- crates/xfdf/src/lib.rs | 18 +++++++++++++++++- crates/xfdf/src/tests.rs | 15 +++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index 80c37b5e86..1eac1f96c5 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -1287,6 +1287,10 @@ pub fn data_values(bytes: &[u8]) -> Result)>, DataError /// Merge Data Files into Spreadsheet: one CSV row per file, one column per field name (in the /// order they first appear). +/// +/// The names and values come from other people's files, and spreadsheet programs evaluate a cell +/// that starts with `=`, `+`, `-` or `@` (after any leading whitespace) as a formula. Such a cell +/// is written with a leading `'`, which keeps it text; a plain number such as `-12.5` is not. pub fn merge_csv(files: &[Vec<(String, Vec)>]) -> String { let mut columns: Vec<&str> = Vec::new(); for f in files { @@ -1296,7 +1300,19 @@ pub fn merge_csv(files: &[Vec<(String, Vec)>]) -> String { } } } - let q = |s: &str| if s.contains([',', '"', '\n', '\r', '\t']) { format!("\"{}\"", s.replace('"', "\"\"")) } else { s.to_string() }; + let literal = |s: &str| { + let t = s.trim_start(); + let formula = match t.chars().next() { + Some('=' | '@') => true, + Some('+' | '-') => !t.parse::().is_ok_and(f64::is_finite), + _ => false, + }; + if formula { format!("'{s}") } else { s.to_string() } + }; + let q = |s: &str| { + let s = literal(s); + if s.contains([',', '"', '\n', '\r', '\t']) { format!("\"{}\"", s.replace('"', "\"\"")) } else { s } + }; let mut out = columns.iter().map(|c| q(c)).collect::>().join(","); out.push('\n'); for f in files { diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index 7951bef4dd..3e6dc58a5d 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -1702,3 +1702,18 @@ fn an_imported_rect_only_or_transparent_redaction_mark_is_drawn_for_review() { assert!(mark.get(b"CA").is_none() && mark.get(b"ca").is_none(), "the mark can be drawn transparent: {mark:?}"); } } + +#[test] +fn merged_spreadsheet_cells_never_start_a_formula() { + let row = vec![ + ("=HYPERLINK(\"http://x\")".to_string(), vec!["@SUM(A1)".to_string()]), + ("Total".to_string(), vec!["-2+3".to_string()]), + ("Plus".to_string(), vec!["\t+1+cmd".to_string()]), + ("Balance".to_string(), vec!["-12.5".to_string()]), + ("\tName".to_string(), vec!["Ada".to_string()]), + ]; + let csv = merge_csv(&[row]); + let lines: Vec<&str> = csv.lines().collect(); + assert_eq!(lines[0], "\"'=HYPERLINK(\"\"http://x\"\")\",Total,Plus,Balance,\"\tName\""); + assert_eq!(lines[1], "'@SUM(A1),'-2+3,\"'\t+1+cmd\",-12.5,Ada"); +} From 63f7932d06e19b1b9f9a1cb4baaab3550578bb4a Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 17:00:43 +0000 Subject: [PATCH 13/42] fix(xfdf): fall back to the /Rect when an imported redaction mark has a zero-area quad A quad whose corners all lie on one line (or on one point) encloses nothing, yet such /QuadPoints counted as drawable, replaced the mark's /Rect and left an imported redaction that covers nothing. Any quad without area now makes the mark's quads unusable, so its /Rect is used instead. --- crates/xfdf/src/lib.rs | 22 ++++++++++++++++------ crates/xfdf/src/tests.rs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index 1eac1f96c5..9582298b34 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -246,6 +246,14 @@ fn nums_of(doc: &Document, o: Option<&Object>) -> Vec { o.map(|o| doc.resolve(o)).and_then(|a| a.as_array().map(|a| a.iter().filter_map(|x| doc.resolve(x).as_f64()).collect())).unwrap_or_default() } +/// Whether a quad's four corners enclose any area: some three of them are not collinear. +fn quad_has_area(q: &[f64; 8]) -> bool { + let [ax, ay, bx, by, cx, cy, dx, dy] = *q; + let cross = |(px, py): (f64, f64), (qx, qy): (f64, f64), (rx, ry): (f64, f64)| (qx - px) * (ry - py) - (qy - py) * (rx - px); + let (a, b, c, d) = ((ax, ay), (bx, by), (cx, cy), (dx, dy)); + [cross(a, b, c), cross(a, b, d), cross(a, c, d), cross(b, c, d)].iter().any(|v| v.abs() > 0.0) +} + fn text_of(doc: &Document, d: &Dict, k: &[u8]) -> Option { d.get(k).and_then(|o| match &*doc.resolve(o) { Object::String(s) => Some(s.to_text()), @@ -1036,13 +1044,15 @@ fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result = q.iter().filter_map(Object::as_f64).filter(|v| v.is_finite()).collect(); + let (quads, rest) = v.as_chunks::<8>(); + v.len() == q.len() && rest.is_empty() && !quads.is_empty() && quads.iter().all(quad_has_area) + }); if !quads_drawable { - // Quads the outline can't draw are replaced by the /Rect's, or dropped when there is - // no usable /Rect either; such a mark then removes nothing. + // Quads the outline can't draw, or any quad that encloses no area, are replaced by the + // /Rect's, or dropped when there is no usable /Rect either; such a mark then removes + // nothing. match nums_of(doc, d.get(b"Rect"))[..] { [x0, y0, x1, y1] if [x0, y0, x1, y1].iter().all(|v| v.is_finite()) => { let (l, r, b, t) = (x0.min(x1), x0.max(x1), y0.min(y1), y0.max(y1)); diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index 3e6dc58a5d..de880fe664 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -1717,3 +1717,37 @@ fn merged_spreadsheet_cells_never_start_a_formula() { assert_eq!(lines[0], "\"'=HYPERLINK(\"\"http://x\"\")\",Total,Plus,Balance,\"\tName\""); assert_eq!(lines[1], "'@SUM(A1),'-2+3,\"'\t+1+cmd\",-12.5,Ada"); } + +fn imported_redact_mark(data: &[u8]) -> Dict { + let mut doc = blank(); + assert_eq!(import(&mut doc, data).unwrap().comments, 1); + let page = pdfcraft_model::pages(&doc)[0].clone(); + doc.resolve(page.dict.get(b"Annots").unwrap()) + .as_array() + .unwrap() + .iter() + .filter_map(|a| a.as_ref().and_then(|r| doc.get(r).as_dict().cloned())) + .find(|d| d.name(b"Subtype") == Some(b"Redact".as_slice())) + .unwrap() +} + +#[test] +fn an_imported_redaction_mark_with_zero_area_quads_falls_back_to_its_rect() { + let rect_quads = vec![60.0, 620.0, 560.0, 620.0, 60.0, 600.0, 560.0, 600.0]; + let fdf = |quads: &str| { + format!( + "%FDF-1.2\n1 0 obj\n<< /FDF << /Annots [<< /Subtype /Redact /NM (r1) /Page 0 /Rect [60 600 560 620] /QuadPoints [{quads}] >>] >> >>\nendobj\ntrailer\n<< /Root 1 0 R >>\n%%EOF\n" + ) + }; + let cases = [ + fdf("100 100 100 100 100 100 100 100").into_bytes(), + // A good quad beside one that is a line (all four corners collinear). + fdf("60 620 560 620 60 600 560 600 10 10 20 20 30 30 40 40").into_bytes(), + br#""#.to_vec(), + ]; + for data in cases { + let mark = imported_redact_mark(&data); + let quads: Vec = mark.get(b"QuadPoints").and_then(Object::as_array).unwrap().iter().filter_map(Object::as_f64).collect(); + assert_eq!(quads, rect_quads, "zero-area quads were kept: {mark:?}"); + } +} From 256097023c57cc0574287d338051c88676b72096 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 17:01:55 +0000 Subject: [PATCH 14/42] fix(xfdf): use an imported redaction mark's /Rect only when it is four finite numbers The /Rect fallback dropped non-numbers before checking the length, so a malformed /Rect such as [0 0 /Bad 100 100] read as a valid rectangle and gave the mark quads. The array itself must now hold exactly four finite numbers. --- crates/xfdf/src/lib.rs | 13 +++++++++++-- crates/xfdf/src/tests.rs | 7 +++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index 9582298b34..219b0de81b 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -246,6 +246,15 @@ fn nums_of(doc: &Document, o: Option<&Object>) -> Vec { o.map(|o| doc.resolve(o)).and_then(|a| a.as_array().map(|a| a.iter().filter_map(|x| doc.resolve(x).as_f64()).collect())).unwrap_or_default() } +/// A `/Rect` of exactly four finite numbers; any other entry makes the whole rectangle unusable. +fn rect_strict(doc: &Document, o: Option<&Object>) -> Option<[f64; 4]> { + let num = |x: &Object| doc.resolve(x).as_f64().filter(|v| v.is_finite()); + match doc.resolve(o?).as_array()?.as_slice() { + [x0, y0, x1, y1] => Some([num(x0)?, num(y0)?, num(x1)?, num(y1)?]), + _ => None, + } +} + /// Whether a quad's four corners enclose any area: some three of them are not collinear. fn quad_has_area(q: &[f64; 8]) -> bool { let [ax, ay, bx, by, cx, cy, dx, dy] = *q; @@ -1053,8 +1062,8 @@ fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result { + match rect_strict(doc, d.get(b"Rect")) { + Some([x0, y0, x1, y1]) => { let (l, r, b, t) = (x0.min(x1), x0.max(x1), y0.min(y1), y0.max(y1)); d.set(b"QuadPoints".to_vec(), arr(&[l, t, r, t, l, b, r, b])); } diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index de880fe664..c98fab06ac 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -1751,3 +1751,10 @@ fn an_imported_redaction_mark_with_zero_area_quads_falls_back_to_its_rect() { assert_eq!(quads, rect_quads, "zero-area quads were kept: {mark:?}"); } } + +#[test] +fn an_imported_redaction_rect_with_a_non_number_is_not_used() { + let fdf = b"%FDF-1.2\n1 0 obj\n<< /FDF << /Annots [<< /Subtype /Redact /NM (r1) /Page 0 /Rect [0 0 /Bad 100 100] >>] >> >>\nendobj\ntrailer\n<< /Root 1 0 R >>\n%%EOF\n"; + let mark = imported_redact_mark(fdf); + assert!(mark.get(b"QuadPoints").is_none(), "quads were drawn from a malformed /Rect: {mark:?}"); +} From f1c56ef86534ec182cde58cc827a700e0a693f51 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:45:44 +0000 Subject: [PATCH 15/42] fix(redact): count a name tree's shared nodes once The hidden-information count followed every /Kids entry of the attachment and document-script name trees with only a depth limit, so a node listed twice by each level was read once per path: a few hundred bytes made the scan take 2^32 steps. Indirect nodes are now read once each, under a total node budget. --- crates/redact/src/sanitize.rs | 34 ++++++++++++++++++++++++++++------ crates/redact/src/tests.rs | 22 ++++++++++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index 83db13d728..c9c2b182aa 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -87,14 +87,36 @@ fn sub(doc: &Document, d: &Dict, key: &[u8]) -> Option { d.get(key).and_then(|o| doc.resolve(o).as_dict().cloned()) } -/// Entries of a name tree (`/Names` leaves, `/Kids` recursion), bounded against cycles. -fn name_tree_len(doc: &Document, node: &Dict, depth: usize) -> usize { - if depth > 32 { +/// The most name-tree nodes one count reads. +const NAME_TREE_NODE_BUDGET: usize = 100_000; + +/// Entries of a name tree (`/Names` leaves, `/Kids` recursion). Each indirect node is read once, +/// however many `/Kids` arrays list it, so shared or cyclic kids cost one step per node rather +/// than one per path; depth and a total node budget bound the rest. +fn name_tree_len(doc: &Document, node: &Dict) -> usize { + let (mut seen, mut visited) = (std::collections::HashSet::new(), 0); + name_tree_len_seen(doc, node, 0, &mut seen, &mut visited) +} + +fn name_tree_len_seen(doc: &Document, node: &Dict, depth: usize, seen: &mut std::collections::HashSet, visited: &mut usize) -> usize { + *visited += 1; + if depth > 32 || *visited > NAME_TREE_NODE_BUDGET { return 0; } let leaves = node.get(b"Names").and_then(|n| doc.resolve(n).as_array().map(|a| a.len() / 2)).unwrap_or(0); let kids = node.get(b"Kids").and_then(|k| doc.resolve(k).as_array().cloned()).unwrap_or_default(); - leaves + kids.iter().filter_map(|k| doc.resolve(k).as_dict().cloned()).map(|k| name_tree_len(doc, &k, depth + 1)).sum::() + let mut n = leaves; + for k in &kids { + if let Object::Ref(r) = k + && !seen.insert(*r) + { + continue; + } + if let Some(kid) = doc.resolve(k).as_dict() { + n = n.saturating_add(name_tree_len_seen(doc, kid, depth + 1, seen, visited)); + } + } + n } /// The most distinct outline items one count reads, over all levels together. @@ -270,7 +292,7 @@ pub fn scan(doc: &Document) -> Vec<(Hidden, usize)> { let xmp = usize::from(cat.contains(b"Metadata")) + pages.iter().filter(|p| p.dict.contains(b"Metadata")).count(); info + xmp } - Hidden::Attachments => sub(doc, &names, b"EmbeddedFiles").map_or(0, |t| name_tree_len(doc, &t, 0)) + count_sub(b"FileAttachment"), + Hidden::Attachments => sub(doc, &names, b"EmbeddedFiles").map_or(0, |t| name_tree_len(doc, &t)) + count_sub(b"FileAttachment"), Hidden::Comments => annots.iter().filter(|a| is_comment(a.name(b"Subtype").unwrap_or(b""))).count(), Hidden::FormFields => pdfcraft_forms::fields(doc).len(), Hidden::HiddenText => content_pass(&mut doc2, true, false, false).map(|(n, _)| n).unwrap_or(0), @@ -285,7 +307,7 @@ pub fn scan(doc: &Document) -> Vec<(Hidden, usize)> { + usize::from(cat.contains(b"AA")) + pages.iter().filter(|p| p.dict.contains(b"AA")).count() + annots.iter().filter(|a| a.contains(b"AA") || (a.name(b"Subtype") == Some(b"Widget") && a.contains(b"A"))).count() - + sub(doc, &names, b"JavaScript").map_or(0, |t| name_tree_len(doc, &t, 0)) + + sub(doc, &names, b"JavaScript").map_or(0, |t| name_tree_len(doc, &t)) } Hidden::PrivateData => usize::from(cat.contains(b"PieceInfo")) + pages.iter().filter(|p| p.dict.contains(b"PieceInfo")).count(), }; diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 286667937d..2a19014a9c 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2472,3 +2472,25 @@ fn a_tj_surplus_arrays_undrawn_glyphs_are_not_seen_under_a_mark() { assert_eq!(under(&mut doc, 0, &[[45.0, 95.0, 50.0, 110.0]]), 0, "nothing is drawn past LASTARR's 35pt width"); assert_eq!(under(&mut doc, 0, &[[10.0, 95.0, 45.0, 110.0]]), 7, "LASTARR's 7 glyphs, the one the page draws"); } + +/// A name tree whose levels each list the next level twice: a walk that follows every path +/// visits 2^levels nodes. +#[test] +fn a_name_tree_whose_nodes_share_kids_counts_each_entry_once() { + use crate::sanitize::{Hidden, scan}; + let levels = 12; + let mut objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /Names << /EmbeddedFiles 5 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 >>".to_vec(), + stream("", b"BT ET"), + ]; + for k in 0..levels { + let next = 6 + k; + objs.push(format!("<< /Kids [{next} 0 R {next} 0 R] >>").into_bytes()); + } + objs.push(b"<< /Names [(a.txt) << /Type /Filespec /F (a.txt) >>] >>".to_vec()); + let doc = pdf(objs); + let counts: std::collections::HashMap = scan(&doc).into_iter().collect(); + assert_eq!(counts[&Hidden::Attachments], 1, "one entry, reached through 2^{levels} paths"); +} From ee010fbbe5af7d7efcc8859b976a34121f3225d0 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:46:18 +0000 Subject: [PATCH 16/42] fix(redact): read content strictly before sanitizing or redacting it Page and Form content was decoded tolerantly, keeping what came before a corrupt spot and dropping the rest without saying so. A page whose content broke at its first bytes therefore read as empty: the hidden-text scan counted nothing, the removal was skipped and the original stream, which other viewers still draw, stayed in the file. Content is now decoded strictly; an unreadable page counts as something to remove, so the removal runs and refuses the page, and a Form read only in part is treated like an undecodable one. --- crates/redact/src/interp.rs | 3 ++- crates/redact/src/lib.rs | 4 +++- crates/redact/src/sanitize.rs | 3 +++ crates/redact/src/tests.rs | 20 ++++++++++++++++++++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index c9a404b935..a608568f38 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -1201,7 +1201,8 @@ fn xobject( out.doomed.push(r); return Some(None); } - let data = match s.decoded() { + // Strict, like page content: a Form read only in part counts as undecodable. + let data = match s.decoded_strict() { Ok(d) => d, Err(_) if scope.mode == Mode::Apply => { scope.report.forms_removed += 1; diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 6df5102afa..364f7cc193 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -231,7 +231,9 @@ pub(crate) fn page_streams(doc: &Document, page: &Dict, index: usize) -> Result< let mut data = Vec::new(); for o in &list { match &*doc.resolve(o) { - Object::Stream(s) => data.push(s.decoded().map_err(|_| RedactError::Unreadable(index + 1))?), + // Strict: a tolerant decode keeps what precedes corruption and drops the rest, which + // still draws in viewers that read further; anything not read can't be cleaned. + Object::Stream(s) => data.push(s.decoded_strict().map_err(|_| RedactError::Unreadable(index + 1))?), Object::Null => data.push(Vec::new()), _ => return Err(RedactError::Unreadable(index + 1)), } diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index c9c2b182aa..f4c880e30b 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -205,6 +205,9 @@ fn content_pass(doc: &mut Document, text: bool, layers: bool, write: bool) -> Re if write { return Err(RedactError::Unreadable(pi + 1)); } + // Content that can't be read may hide anything: count it so the removal runs and + // refuses the page, instead of a zero count skipping it. + total += 1; continue; }; let resources = page.dict.get(b"Resources").and_then(|r| doc.resolve(r).as_dict().cloned()).unwrap_or_default(); diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 2a19014a9c..a0d68af2d7 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2494,3 +2494,23 @@ fn a_name_tree_whose_nodes_share_kids_counts_each_entry_once() { let counts: std::collections::HashMap = scan(&doc).into_iter().collect(); assert_eq!(counts[&Hidden::Attachments], 1, "one entry, reached through 2^{levels} paths"); } + +/// Page content that stops decoding at its start (`z` inside an ASCII85 group) while the rest of +/// the stream draws hidden text. A tolerant decode reads nothing, counts nothing and keeps the +/// stream; sanitizing must refuse instead. +#[test] +fn hidden_text_in_content_that_stops_decoding_is_refused() { + use crate::sanitize::{Hidden, remove_hidden, sanitize}; + let encoded: &[u8] = br"!!z6<#'\7PQ#?0Ha>,+>Y->EZd(j+>PW)+B2ko-rWQ%;FOP6+B3(u78s~>"; + let fresh = || { + let mut doc = one_page(b"", "", vec![stream("/Filter /ASCII85Decode", encoded)]); + doc.update_dict(ObjRef::new(3, 0), |d| d.set(b"Contents".to_vec(), Object::Ref(ObjRef::new(7, 0)))).unwrap(); + doc + }; + let mut doc = fresh(); + let out = remove_hidden(&mut doc, &[Hidden::HiddenText]); + assert!(matches!(out, Err(RedactError::Unreadable(1))), "{out:?}"); + let mut doc = fresh(); + let out = sanitize(&mut doc); + assert!(matches!(out, Err(RedactError::Unreadable(1))), "{out:?}"); +} From 3fe9fe218ecbc4fb6ab2f9eba346115001bf0368 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:46:38 +0000 Subject: [PATCH 17/42] fix(redact): remove the widgets flattening keeps when removing form fields Removing form fields flattens them and then drops the interactive form, but flattening keeps hidden and no-view widgets (they draw nothing) and widgets with no appearance. The pages still listed them, so their values and their parent fields' values survived the full rewrite. Every widget left after flattening now leaves the pages and the structure tree, inline annotations included. --- crates/redact/src/sanitize.rs | 11 ++++++++--- crates/redact/src/tests.rs | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index f4c880e30b..41ef96cccf 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -341,7 +341,8 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result 0 { + let fields = which.contains(&Hidden::FormFields) && count(Hidden::FormFields) > 0; + if fields { // Fields without appearances get one first, so their values stay visible. for f in pdfcraft_forms::fields(doc) { if f.widgets.iter().any(|w| !doc.get(w.obj).as_dict().is_some_and(|d| d.contains(b"AP"))) { @@ -354,10 +355,12 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result bool { let s = a.name(b"Subtype").unwrap_or(b""); (which.contains(&Hidden::Comments) && is_comment(s)) + || (fields && s == b"Widget") || (which.contains(&Hidden::Attachments) && s == b"FileAttachment") || (which.contains(&Hidden::LinksActionsScripts) && s == b"Link") }; @@ -374,7 +377,9 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result Object { + Object::Array(nums.iter().map(|&n| Object::Ref(ObjRef::new(n, 0))).collect()) +} + +/// An interactive form dictionary listing fields `nums`. +fn fields_dict(nums: &[u32]) -> Object { + let mut d = Dict::new(); + d.set(b"Fields".to_vec(), ref_array(nums)); + Object::Dict(d) +} + +/// Flattening keeps hidden widgets (they draw nothing); removing form fields must still take them +/// and their values, and their parent field's value, out of the file. +#[test] +fn removing_form_fields_takes_hidden_widgets_and_their_values_out_of_the_file() { + use crate::sanitize::{Hidden, remove_hidden}; + let mut doc = one_page( + b"BT /F1 10 Tf 10 100 Td (Visible) Tj ET", + "", + vec![ + // 7 a hidden terminal field merged with its widget, 8 a parent field, 9 its hidden widget + b"<< /Type /Annot /Subtype /Widget /FT /Tx /T (own) /V (LEAK_WIDGET_VALUE) /F 2 /Rect [10 200 150 220] /AP << /N 10 0 R >> >>".to_vec(), + b"<< /FT /Tx /T (parent) /V (LEAK_PARENT_VALUE) /Kids [9 0 R] >>".to_vec(), + b"<< /Type /Annot /Subtype /Widget /Parent 8 0 R /F 32 /Rect [10 230 150 250] /AP << /N 10 0 R >> >>".to_vec(), + stream("/Type /XObject /Subtype /Form /BBox [0 0 140 20]", b"0 g"), + ], + ); + doc.update_dict(doc.root().unwrap(), |d| d.set(b"AcroForm".to_vec(), fields_dict(&[7, 8]))).unwrap(); + doc.update_dict(ObjRef::new(3, 0), |d| d.set(b"Annots".to_vec(), ref_array(&[7, 9]))).unwrap(); + remove_hidden(&mut doc, &[Hidden::FormFields]).unwrap(); + assert_no_secret(&doc, b"LEAK_"); + assert!(annots_of(&doc, &pdfcraft_model::pages(&doc).swap_remove(0).dict).is_empty()); +} From 750bbe27f1864bcb42b1cc1745256ae91d6db729 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:47:21 +0000 Subject: [PATCH 18/42] fix(redact): clean the structure tree after removing hidden content Removing hidden text or hidden layers rewrote page and Form content but left the structure tree alone, unlike applying redactions. An element's ActualText, Alt or expansion text still described the removed content, and a marked-content reference into a Form kept the original Form, with the removed text, in the full rewrite. The hidden-content pass now cleans the tags of what each page no longer shows and moves Form references to the rewritten copy, refusing when the tree can't be cleaned completely. --- crates/redact/src/sanitize.rs | 13 ++++++++++++- crates/redact/src/tests.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index 41ef96cccf..fea0449512 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -254,7 +254,7 @@ fn content_pass(doc: &mut Document, text: bool, layers: bool, write: bool) -> Re let mut cleaned_page = page.dict.clone(); cleaned_page.set(b"Contents".to_vec(), Object::Array(new_list.clone())); let (_, page_content) = page_streams(doc, &cleaned_page, pi)?; - let mut res = resources; + let mut res = resources.clone(); if !out.xobjects.is_empty() || !out.replaced.is_empty() { let mut xo = res.get(b"XObject").and_then(|x| doc.resolve(x).as_dict().cloned()).unwrap_or_default(); // A full save keeps whatever is still referenced: unlink the originals this page no @@ -270,6 +270,17 @@ fn content_pass(doc: &mut Document, text: bool, layers: bool, write: bool) -> Re d.set(b"Contents".to_vec(), Object::Array(new_list)); d.set(b"Resources".to_vec(), Object::Dict(res)); })?; + // Tags must not describe or point at what the page no longer shows. + let (changed_ids, empty_ids) = crate::tags::touched( + doc, + &resources, + pdfcraft_content::Pieces::join(&data).data(), + pdfcraft_content::Pieces::join(&page_content).data(), + ); + crate::tags::clean(doc, page.obj, None, &changed_ids, &empty_ids)?; + } + for (old, new, changed, empty) in out.tag_changes { + crate::tags::clean(doc, page.obj, Some((old, new)), &changed, &empty)?; } } // What a page no longer draws must not stay reachable through an inherited or shared diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 4d2cc76017..5c0eece859 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2549,3 +2549,31 @@ fn removing_form_fields_takes_hidden_widgets_and_their_values_out_of_the_file() assert_no_secret(&doc, b"LEAK_"); assert!(annots_of(&doc, &pdfcraft_model::pages(&doc).swap_remove(0).dict).is_empty()); } + +/// Removing hidden text cleans the structure tree as redaction does: the elements describing the +/// removed text lose their descriptions, and a reference into a rewritten Form points at the copy. +#[test] +fn removing_hidden_text_cleans_the_structure_tree() { + use crate::sanitize::{Hidden, remove_hidden}; + let mut doc = one_page( + b"/P <> BDC BT /F1 10 Tf 3 Tr 10 200 Td (SECRET) Tj ET EMC /P <> BDC BT /F1 10 Tf 0 Tr 10 100 Td (Public) Tj ET EMC /Fm Do", + "/XObject << /Fm 7 0 R >>", + vec![ + stream( + "/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources << /Font << /F1 5 0 R >> >>", + b"/P <> BDC BT /F1 10 Tf 3 Tr 10 150 Td (FORMSECRET) Tj ET EMC", + ), + b"<< /Type /StructTreeRoot /K 9 0 R >>".to_vec(), + b"<< /S /Document /P 8 0 R /K [10 0 R 11 0 R 12 0 R] >>".to_vec(), + b"<< /S /P /P 9 0 R /Pg 3 0 R /ActualText (LEAK_PAGE_TAG) /K 0 >>".to_vec(), + b"<< /S /P /P 9 0 R /Pg 3 0 R /ActualText (Public) /K 1 >>".to_vec(), + b"<< /S /P /P 9 0 R /Pg 3 0 R /Alt (LEAK_FORM_TAG) /K << /Type /MCR /Pg 3 0 R /Stm 7 0 R /MCID 0 >> >>".to_vec(), + ], + ); + doc.update_dict(doc.root().unwrap(), |d| d.set(b"StructTreeRoot".to_vec(), Object::Ref(ObjRef::new(8, 0)))).unwrap(); + doc.update_dict(ObjRef::new(3, 0), |d| d.set(b"StructParents".to_vec(), Object::Int(0))).unwrap(); + remove_hidden(&mut doc, &[Hidden::HiddenText]).unwrap(); + assert_no_secret(&doc, b"LEAK_"); + assert_no_secret(&doc, b"SECRET"); + assert!(doc.get(ObjRef::new(11, 0)).as_dict().unwrap().contains(b"ActualText"), "untouched content keeps its tags"); +} From 822179ccf2334418c8a711b993528da31d0977ed Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:51:06 +0000 Subject: [PATCH 19/42] fix(redact): remove hidden data from every object that owns it Metadata, private application data, associated files and additional actions were removed from the catalog and pages only. The same entries on an image or font (XMP), a Form XObject (PieceInfo), a page or annotation (/AF with its embedded file) or a form field (/AA scripts) survived the full rewrite, and the scan did not count them, so removing only such a category reported nothing to remove. A walk of every object reachable from the catalog now counts and strips these entries wherever they are, leaving name maps such as resource dictionaries alone (their keys are names a producer chose), and refuses a graph nested too deeply to walk in full. --- crates/redact/src/lib.rs | 4 + crates/redact/src/sanitize.rs | 193 +++++++++++++++++++++++++++++----- crates/redact/src/tests.rs | 94 +++++++++++++++++ 3 files changed, 265 insertions(+), 26 deletions(-) diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 364f7cc193..968773b91d 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -64,6 +64,10 @@ pub enum RedactError { "the document's tags can't be cleaned completely (an element written in place, or a structure tree too large or too deep to follow); the operation was refused" )] Tags, + /// Objects written in place nest deeper than hidden information is followed, so its removal + /// could not be shown complete. + #[error("the document's objects nest too deeply to remove hidden information completely; the operation was refused")] + TooDeep, #[error(transparent)] Edit(#[from] pdfcraft_edit::EditError), #[error(transparent)] diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index fea0449512..d9edfbde95 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -15,7 +15,7 @@ use crate::{RedactError, Report, annots_of, listed_annotations, page_streams}; pub enum Hidden { /// Document information (`/Info`) and XMP metadata streams. Metadata, - /// Embedded files and file attachment annotations. + /// Embedded files, associated files (`/AF`) and file attachment annotations. Attachments, /// Comments and markup annotations (with their pop-ups). Comments, @@ -163,6 +163,148 @@ fn outline_len_seen( n } +/// Entries that hold a category's data wherever they appear, not only on the catalog and pages: +/// XMP on images, fonts and Forms, private data on Forms, associated files on pages and +/// annotations, additional actions on fields. +const OWNED: [(Hidden, &[u8]); 4] = + [(Hidden::Metadata, b"Metadata"), (Hidden::PrivateData, b"PieceInfo"), (Hidden::Attachments, b"AF"), (Hidden::LinksActionsScripts, b"AA")]; + +/// Keys whose dictionary values map names a producer chooses to objects (resources, glyph +/// procedures, named destinations, role and class maps, private data by application). A name +/// there may be spelt like an owned entry (a font called `/AF`), so these maps are walked but +/// never stripped. +const NAME_MAPS: [&[u8]; 14] = [ + b"Font", + b"XObject", + b"ExtGState", + b"ColorSpace", + b"Pattern", + b"Shading", + b"Properties", + b"CharProcs", + b"Dests", + b"RoleMap", + b"ClassMap", + b"Colorants", + b"Schema", + b"PieceInfo", +]; + +/// How deep the owner sweep follows dictionaries and arrays written in place. +const MAX_DIRECT_DEPTH: usize = 64; + +/// What a dictionary is to the owner sweep. +#[derive(Clone, Copy, PartialEq, Eq, Hash)] +enum Held { + /// An object that may own an entry of [`OWNED`]. + Owner, + /// A [`NAME_MAPS`] map: its keys are names, its values owners. + Map, + /// An appearance dictionary (`/AP`): its dictionary values map appearance states. + Appearances, +} + +impl Held { + fn child(self, key: &[u8]) -> Held { + match self { + Held::Owner if NAME_MAPS.contains(&key) => Held::Map, + Held::Owner if key == b"AP" => Held::Appearances, + Held::Appearances => Held::Map, + _ => Held::Owner, + } + } +} + +/// The objects reachable from the catalog that hold one of `keys` outside a name map, and how +/// many dictionaries hold each key. Each object is read at most once per [`Held`] role, so a +/// shared or cyclic graph costs one step per object. +fn owners(doc: &Document, keys: &[&[u8]]) -> Result<(Vec, Vec<(ObjRef, Held)>), RedactError> { + let mut held = vec![0; keys.len()]; + let mut holders = Vec::new(); + let Some(root) = doc.root() else { return Ok((held, holders)) }; + let mut next = vec![(root, Held::Owner)]; + let mut seen = std::collections::HashSet::new(); + while let Some((r, role)) = next.pop() { + if !seen.insert((r, role)) { + continue; + } + if find_owned(&doc.get(r), role, keys, &mut held, &mut next, 0)? { + holders.push((r, role)); + } + } + Ok((held, holders)) +} + +/// Counts the dictionaries in `o` (and what it holds in place) that hold one of `keys`, and +/// queues the indirect objects it names. Returns whether any key was found. +fn find_owned(o: &Object, role: Held, keys: &[&[u8]], held: &mut [usize], next: &mut Vec<(ObjRef, Held)>, depth: usize) -> Result { + if depth > MAX_DIRECT_DEPTH { + return Err(RedactError::TooDeep); + } + let (d, role) = match o { + Object::Ref(r) => { + next.push((*r, role)); + return Ok(false); + } + Object::Array(a) => { + let mut found = false; + for v in a { + found |= find_owned(v, Held::Owner, keys, held, next, depth + 1)?; + } + return Ok(found); + } + Object::Dict(d) => (d, role), + // A stream's dictionary is never a name map. + Object::Stream(s) => (&s.dict, Held::Owner), + _ => return Ok(false), + }; + let mut found = false; + if role != Held::Map { + for (i, k) in keys.iter().enumerate() { + if d.contains(k) + && let Some(n) = held.get_mut(i) + { + *n = n.saturating_add(1); + found = true; + } + } + } + for (k, v) in d.iter() { + found |= find_owned(v, role.child(k), keys, held, next, depth + 1)?; + } + Ok(found) +} + +/// Removes `keys` from `o` and from what it holds in place, outside name maps. [`owners`] has +/// already refused anything nested past the depth limit. +fn strip_owned(o: &mut Object, role: Held, keys: &[&[u8]], depth: usize) { + if depth > MAX_DIRECT_DEPTH { + return; + } + let (d, role) = match o { + Object::Array(a) => { + for v in a { + strip_owned(v, Held::Owner, keys, depth + 1); + } + return; + } + Object::Dict(d) => (d, role), + Object::Stream(s) => (&mut s.dict, Held::Owner), + _ => return, + }; + if role != Held::Map { + for k in keys { + d.remove(k); + } + } + let names: Vec> = d.iter().filter(|(_, v)| matches!(v, Object::Array(_) | Object::Dict(_))).map(|(k, _)| k.clone()).collect(); + for k in names { + if let Some(v) = d.get_mut(&k) { + strip_owned(v, role.child(&k), keys, depth + 1); + } + } +} + /// A comment as the comments list sees it, less file attachments (removed with Attachments) and /// redaction marks. fn is_comment(subtype: &[u8]) -> bool { @@ -296,6 +438,11 @@ pub fn scan(doc: &Document) -> Vec<(Hidden, usize)> { let annots: Vec = pages.iter().flat_map(|p| annots_of(doc, &p.dict)).filter_map(|a| doc.resolve(&a).as_dict().cloned()).collect(); let count_sub = |s: &[u8]| annots.iter().filter(|a| a.name(b"Subtype") == Some(s)).count(); let names = sub(doc, &cat, b"Names").unwrap_or_default(); + let owned_keys: Vec<&[u8]> = OWNED.iter().map(|o| o.1).collect(); + // A graph nested too deeply to walk counts as holding everything, so the removal runs and + // refuses it. + let held = owners(doc, &owned_keys).map_or_else(|_| vec![1; OWNED.len()], |o| o.0); + let owned = |h: Hidden| OWNED.iter().position(|o| o.0 == h).and_then(|i| held.get(i)).copied().unwrap_or(0); let mut doc2 = doc.clone(); HIDDEN .into_iter() @@ -303,10 +450,13 @@ pub fn scan(doc: &Document) -> Vec<(Hidden, usize)> { let n = match h { Hidden::Metadata => { let info = doc.trailer().get(b"Info").and_then(|i| doc.resolve(i).as_dict().map(|d| d.len())).unwrap_or(0); - let xmp = usize::from(cat.contains(b"Metadata")) + pages.iter().filter(|p| p.dict.contains(b"Metadata")).count(); - info + xmp + info + owned(Hidden::Metadata) + } + Hidden::Attachments => { + sub(doc, &names, b"EmbeddedFiles").map_or(0, |t| name_tree_len(doc, &t)) + + count_sub(b"FileAttachment") + + owned(Hidden::Attachments) } - Hidden::Attachments => sub(doc, &names, b"EmbeddedFiles").map_or(0, |t| name_tree_len(doc, &t)) + count_sub(b"FileAttachment"), Hidden::Comments => annots.iter().filter(|a| is_comment(a.name(b"Subtype").unwrap_or(b""))).count(), Hidden::FormFields => pdfcraft_forms::fields(doc).len(), Hidden::HiddenText => content_pass(&mut doc2, true, false, false).map(|(n, _)| n).unwrap_or(0), @@ -318,12 +468,11 @@ pub fn scan(doc: &Document) -> Vec<(Hidden, usize)> { Hidden::LinksActionsScripts => { count_sub(b"Link") + usize::from(cat.get(b"OpenAction").is_some_and(|a| doc.resolve(a).as_dict().is_some())) - + usize::from(cat.contains(b"AA")) - + pages.iter().filter(|p| p.dict.contains(b"AA")).count() - + annots.iter().filter(|a| a.contains(b"AA") || (a.name(b"Subtype") == Some(b"Widget") && a.contains(b"A"))).count() + + owned(Hidden::LinksActionsScripts) + + annots.iter().filter(|a| !a.contains(b"AA") && a.name(b"Subtype") == Some(b"Widget") && a.contains(b"A")).count() + sub(doc, &names, b"JavaScript").map_or(0, |t| name_tree_len(doc, &t)) } - Hidden::PrivateData => usize::from(cat.contains(b"PieceInfo")) + pages.iter().filter(|p| p.dict.contains(b"PieceInfo")).count(), + Hidden::PrivateData => owned(Hidden::PrivateData), }; (h, n) }) @@ -406,24 +555,12 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result Result { - cat.remove(b"Metadata"); doc.trailer_mut().remove(b"Info"); } Hidden::Attachments => { if let Some(n) = names.as_mut() { n.remove(b"EmbeddedFiles"); } - cat.remove(b"AF"); } Hidden::Bookmarks => { cat.remove(b"Outlines"); @@ -457,14 +592,10 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result { - cat.remove(b"PieceInfo"); - } Hidden::HiddenLayers if !off.is_empty() => { // The off layers leave the configuration (their content is gone already). if let Some(mut oc) = sub(doc, &cat, b"OCProperties") { @@ -494,6 +625,16 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result = OWNED.iter().filter(|o| which.contains(&o.0)).map(|o| o.1).collect(); + if !keys.is_empty() { + for (r, role) in owners(doc, &keys)?.1 { + let mut o = (*doc.get(r)).clone(); + strip_owned(&mut o, role, &keys, 0); + doc.set(r, o); + } + } if done.is_empty() { return Err(RedactError::NothingToApply); } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 5c0eece859..603fede2c7 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2577,3 +2577,97 @@ fn removing_hidden_text_cleans_the_structure_tree() { assert_no_secret(&doc, b"SECRET"); assert!(doc.get(ObjRef::new(11, 0)).as_dict().unwrap().contains(b"ActualText"), "untouched content keeps its tags"); } + +/// One page drawing image /Im (object 7) and form /Fm (object 8), with `extra` objects from 9 on. +fn owners_fixture(image: &str, form: &str, extra: Vec>) -> Document { + let mut objs = vec![ + stream(&format!("/Type /XObject /Subtype /Image /Width 1 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8 {image}"), b"\x80"), + stream(&format!("/Type /XObject /Subtype /Form /BBox [0 0 10 10] {form}"), b"0 0 5 5 re f"), + ]; + objs.extend(extra); + one_page(b"BT /F1 10 Tf 10 100 Td (Visible) Tj ET q 10 0 0 10 0 0 cm /Im Do Q /Fm Do", "/XObject << /Im 7 0 R /Fm 8 0 R >>", objs) +} + +/// Removing one category and saving leaves none of `secret` in the file, and the scan counted it. +fn removes_from_every_owner(mut doc: Document, h: crate::sanitize::Hidden, secret: &[u8]) { + use crate::sanitize::{remove_hidden, scan}; + let counts: std::collections::HashMap<_, usize> = scan(&doc).into_iter().collect(); + assert!(counts[&h] > 0, "{h:?} is counted"); + remove_hidden(&mut doc, &[h]).unwrap(); + assert_no_secret(&doc, secret); + assert!(scan(&reopen(&doc)).into_iter().all(|(k, n)| k != h || n == 0 || h == crate::sanitize::Hidden::Metadata)); +} + +#[test] +fn private_data_of_a_form_xobject_is_removed() { + let doc = owners_fixture("", "/PieceInfo << /App << /Private (LEAK_PRIVATE) >> >>", vec![]); + removes_from_every_owner(doc, crate::sanitize::Hidden::PrivateData, b"LEAK_"); +} + +#[test] +fn metadata_of_an_image_is_removed() { + let doc = owners_fixture("/Metadata 9 0 R", "", vec![stream("/Type /Metadata /Subtype /XML", b"LEAK_XMP")]); + removes_from_every_owner(doc, crate::sanitize::Hidden::Metadata, b"LEAK_"); +} + +#[test] +fn a_pages_associated_files_are_removed_with_attachments() { + let mut doc = owners_fixture( + "", + "", + vec![b"<< /Type /Filespec /F (a.txt) /AFRelationship /Data /EF << /F 10 0 R >> >>".to_vec(), stream("/Type /EmbeddedFile", b"LEAK_ATTACHED")], + ); + doc.update_dict(ObjRef::new(3, 0), |d| d.set(b"AF".to_vec(), ref_array(&[9]))).unwrap(); + removes_from_every_owner(doc, crate::sanitize::Hidden::Attachments, b"LEAK_"); +} + +#[test] +fn a_fields_own_actions_are_removed_with_links_actions_and_scripts() { + let mut doc = owners_fixture( + "", + "", + vec![ + b"<< /FT /Tx /T (total) /AA << /C << /S /JavaScript /JS (LEAK_SCRIPT) >> >> /Kids [10 0 R] >>".to_vec(), + b"<< /Type /Annot /Subtype /Widget /Parent 9 0 R /Rect [10 200 150 220] >>".to_vec(), + ], + ); + doc.update_dict(doc.root().unwrap(), |d| d.set(b"AcroForm".to_vec(), fields_dict(&[9]))).unwrap(); + doc.update_dict(ObjRef::new(3, 0), |d| d.set(b"Annots".to_vec(), ref_array(&[10]))).unwrap(); + removes_from_every_owner(doc, crate::sanitize::Hidden::LinksActionsScripts, b"LEAK_"); +} + +/// Resource names are chosen by the producer: a font named like an owned entry stays. +#[test] +fn a_resource_named_like_an_owned_entry_is_kept() { + let mut doc = one_page( + b"BT /AF 10 Tf 10 100 Td (Visible) Tj ET", + "", + vec![b"<< /Type /Filespec /F (a.txt) /EF << /F 8 0 R >> >>".to_vec(), stream("/Type /EmbeddedFile", b"LEAK_ATTACHED")], + ); + doc.update_dict(ObjRef::new(3, 0), |d| { + let mut fonts = Dict::new(); + fonts.set(b"AF".to_vec(), Object::Ref(ObjRef::new(5, 0))); + let mut res = Dict::new(); + res.set(b"Font".to_vec(), Object::Dict(fonts)); + d.set(b"Resources".to_vec(), Object::Dict(res)); + d.set(b"AF".to_vec(), ref_array(&[7])); + }) + .unwrap(); + removes_from_every_owner(doc.clone(), crate::sanitize::Hidden::Attachments, b"LEAK_"); + crate::sanitize::remove_hidden(&mut doc, &[crate::sanitize::Hidden::Attachments]).unwrap(); + let page = pdfcraft_model::pages(&doc).swap_remove(0); + let res = doc.resolve(page.dict.get(b"Resources").unwrap()).as_dict().cloned().unwrap(); + assert!(res.get(b"Font").and_then(|f| f.as_dict()).is_some_and(|f| f.contains(b"AF"))); + assert!(!page.dict.contains(b"AF")); +} + +/// Owners written in place deeper than the sweep follows are refused, not skipped. +#[test] +fn owners_nested_too_deeply_are_refused() { + use crate::sanitize::{Hidden, remove_hidden}; + let nested = format!("{}<< /AA << /O << /S /JavaScript /JS (LEAK_DEEP) >> >> >>{}", "[".repeat(100), "]".repeat(100)); + let mut doc = one_page(b"BT ET", "", vec![format!("<< /Deep {nested} >>").into_bytes()]); + doc.update_dict(ObjRef::new(3, 0), |d| d.set(b"PieceInfo".to_vec(), Object::Ref(ObjRef::new(7, 0)))).unwrap(); + let out = remove_hidden(&mut doc, &[Hidden::LinksActionsScripts]); + assert!(matches!(out, Err(RedactError::TooDeep)), "{out:?}"); +} From 94563649e604acbd3fb2a8c3cca0185b63dd2493 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:22:48 +0000 Subject: [PATCH 20/42] fix(redact): name the owner sweep's holder list for clippy --- crates/redact/src/sanitize.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index d9edfbde95..7349c433ae 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -215,10 +215,13 @@ impl Held { } } +/// Indirect objects that hold an owned entry, each with the role it was reached in. +type Holders = Vec<(ObjRef, Held)>; + /// The objects reachable from the catalog that hold one of `keys` outside a name map, and how /// many dictionaries hold each key. Each object is read at most once per [`Held`] role, so a /// shared or cyclic graph costs one step per object. -fn owners(doc: &Document, keys: &[&[u8]]) -> Result<(Vec, Vec<(ObjRef, Held)>), RedactError> { +fn owners(doc: &Document, keys: &[&[u8]]) -> Result<(Vec, Holders), RedactError> { let mut held = vec![0; keys.len()]; let mut holders = Vec::new(); let Some(root) = doc.root() else { return Ok((held, holders)) }; From 4d64ca8e2da819dff1d0e08c63939560d3b62a78 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:23:26 +0000 Subject: [PATCH 21/42] docs(redact): say which objects hidden-information removal reaches --- crates/redact/README.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/redact/README.md b/crates/redact/README.md index 9c2793e248..2caf067b5b 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -71,7 +71,11 @@ sanitize::sanitize(&mut doc)?; // all of them Categories: metadata (`/Info`, XMP), file attachments, comments, form fields (flattened so the values stay visible), hidden text (render modes 3 and 7, or wholly off the page), hidden layers (content of off optional-content groups, then the groups), bookmarks, links/actions/JavaScript, -and `/PieceInfo` private data. Both make the next save a full rewrite. +and `/PieceInfo` private data. XMP metadata, private data, associated files (`/AF`) and +additional actions (`/AA`) are removed from every object reachable from the catalog that owns +them (images, fonts, Forms, annotations and fields as well as the catalog and pages), never from +name maps such as resource dictionaries. Content is read strictly: a page whose content can't be +read in full is refused, not cleaned in part. Both make the next save a full rewrite. `patterns` finds phone numbers, e-mail addresses, credit card numbers (Luhn-checked), US Social Security numbers and dates for Search & Redact. From 241c5512d646e9be2964ccba96a69be5ab579bb5 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 17:04:40 +0000 Subject: [PATCH 22/42] fix(redact): charge repeated name-tree kids to the node budget The name-tree count cloned each node's /Kids array and skipped repeated references without charging the budget, so a node listing the same kid many times cost work proportional to every entry while the budget saw one step. The walk now borrows the array, charges every entry it does not descend into, and stops once the budget is spent. --- crates/redact/src/sanitize.rs | 30 ++++++++++++++++++++---------- crates/redact/src/tests.rs | 20 ++++++++++++++++++++ 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index 7349c433ae..5b2918f01c 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -92,28 +92,38 @@ const NAME_TREE_NODE_BUDGET: usize = 100_000; /// Entries of a name tree (`/Names` leaves, `/Kids` recursion). Each indirect node is read once, /// however many `/Kids` arrays list it, so shared or cyclic kids cost one step per node rather -/// than one per path; depth and a total node budget bound the rest. +/// than one per path. Every `/Kids` entry the walk does not descend into (a repeated reference or +/// a non-dictionary) costs a step as well, so a long `/Kids` array of repeats cannot outrun the +/// budget. Depth and a total step budget bound the rest. fn name_tree_len(doc: &Document, node: &Dict) -> usize { let (mut seen, mut visited) = (std::collections::HashSet::new(), 0); name_tree_len_seen(doc, node, 0, &mut seen, &mut visited) } fn name_tree_len_seen(doc: &Document, node: &Dict, depth: usize, seen: &mut std::collections::HashSet, visited: &mut usize) -> usize { - *visited += 1; + *visited = visited.saturating_add(1); if depth > 32 || *visited > NAME_TREE_NODE_BUDGET { return 0; } let leaves = node.get(b"Names").and_then(|n| doc.resolve(n).as_array().map(|a| a.len() / 2)).unwrap_or(0); - let kids = node.get(b"Kids").and_then(|k| doc.resolve(k).as_array().cloned()).unwrap_or_default(); let mut n = leaves; - for k in &kids { - if let Object::Ref(r) = k - && !seen.insert(*r) - { - continue; + let kids = node.get(b"Kids").map(|k| doc.resolve(k)); + for k in kids.as_deref().and_then(Object::as_array).map_or(&[][..], Vec::as_slice) { + if *visited > NAME_TREE_NODE_BUDGET { + break; } - if let Some(kid) = doc.resolve(k).as_dict() { - n = n.saturating_add(name_tree_len_seen(doc, kid, depth + 1, seen, visited)); + let held; + let kid = match k { + Object::Ref(r) if !seen.insert(*r) => None, + Object::Ref(r) => { + held = doc.get(*r); + held.as_dict() + } + other => other.as_dict(), + }; + match kid { + Some(kid) => n = n.saturating_add(name_tree_len_seen(doc, kid, depth + 1, seen, visited)), + None => *visited = visited.saturating_add(1), } } n diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 603fede2c7..a9902495df 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2671,3 +2671,23 @@ fn owners_nested_too_deeply_are_refused() { let out = remove_hidden(&mut doc, &[Hidden::LinksActionsScripts]); assert!(matches!(out, Err(RedactError::TooDeep)), "{out:?}"); } + +/// A name tree node that lists the same kid more times than the node budget: each repeat costs a +/// step, so the walk stops at the budget instead of scanning every repeat, and the distinct kid +/// listed after them is never reached. +#[test] +fn repeated_name_tree_kids_count_towards_the_node_budget() { + use crate::sanitize::{Hidden, scan}; + let repeats = "6 0 R ".repeat(100_010); + let doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R /Names << /EmbeddedFiles 5 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 >>".to_vec(), + stream("", b"BT ET"), + format!("<< /Kids [{repeats}7 0 R] >>").into_bytes(), + b"<< /Names [(a.txt) << /Type /Filespec /F (a.txt) >>] >>".to_vec(), + b"<< /Names [(b.txt) << /Type /Filespec /F (b.txt) >>] >>".to_vec(), + ]); + let counts: std::collections::HashMap = scan(&doc).into_iter().collect(); + assert_eq!(counts[&Hidden::Attachments], 1, "the walk went past its node budget to the last kid"); +} From 110b7c678a1658c8b880be1d5f6d255c8f2e2248 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:29:44 +0000 Subject: [PATCH 23/42] fix(a11y): read the enclosing marked content in one step per operator Marked-text extraction and figure bounds searched the whole marked-content stack before every operator, so deeply nested unmarked BMC blocks made a page quadratic. The text stack now holds each level's effective id (its own, else the enclosing one's), and the bounds pass counts the open sequences that match. --- crates/a11y/src/alt.rs | 17 ++++++++++++---- crates/a11y/src/headings.rs | 21 +++++++++++-------- crates/a11y/src/lib.rs | 14 +++++++++++++ crates/a11y/src/tests.rs | 40 +++++++++++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 12 deletions(-) diff --git a/crates/a11y/src/alt.rs b/crates/a11y/src/alt.rs index e4755ab385..ed91f9e50f 100644 --- a/crates/a11y/src/alt.rs +++ b/crates/a11y/src/alt.rs @@ -183,16 +183,19 @@ fn union(a: Option<[f64; 4]>, b: [f64; 4]) -> [f64; 4] { } /// Where the marked content `ids` is drawn on a page (images, forms and paths). -fn content_bbox(doc: &Document, page: &Dict, ids: &HashSet) -> Option<[f64; 4]> { +pub(crate) fn content_bbox(doc: &Document, page: &Dict, ids: &HashSet) -> Option<[f64; 4]> { let res = page.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned()).unwrap_or_default(); let xobjects = res.get(b"XObject").map(|x| doc.resolve(x)).and_then(|x| x.as_dict().cloned()).unwrap_or_default(); let props = page_properties(doc, page); let mut ctm = vec![Matrix([1.0, 0.0, 0.0, 1.0, 0.0, 0.0])]; + // Whether each open marked-content sequence is one of `ids`, and how many of them are. let mut marks: Vec = Vec::new(); + let mut matching = 0usize; let mut bbox = None; let mut path: Vec<(f64, f64)> = Vec::new(); for op in pdfcraft_content::parse(&page_content(doc, page)).ops { - let inside = marks.iter().any(|m| *m); + crate::count_mark_reads(1); + let inside = matching > 0; let top = ctm.last().copied().unwrap_or(Matrix([1.0, 0.0, 0.0, 1.0, 0.0, 0.0])); match op.op.as_slice() { b"q" => ctm.push(top), @@ -207,9 +210,15 @@ fn content_bbox(doc: &Document, page: &Dict, ids: &HashSet) -> Option<[f64; } } b"BMC" => marks.push(false), - b"BDC" => marks.push(bdc_mcid(doc, &props, &op).is_some_and(|n| ids.contains(&n))), + b"BDC" => { + let wanted = bdc_mcid(doc, &props, &op).is_some_and(|n| ids.contains(&n)); + matching += usize::from(wanted); + marks.push(wanted); + } b"EMC" => { - marks.pop(); + if marks.pop() == Some(true) { + matching = matching.saturating_sub(1); + } } b"m" | b"l" => { if let Some([x, y]) = op.nums::<2>() { diff --git a/crates/a11y/src/headings.rs b/crates/a11y/src/headings.rs index 537ed908d7..d3a478cb95 100644 --- a/crates/a11y/src/headings.rs +++ b/crates/a11y/src/headings.rs @@ -182,6 +182,7 @@ pub(crate) fn mcid_texts(doc: &Document, page: &Dict) -> HashMap { let res = sub(doc, page, b"Resources"); let (fonts, props) = (sub(doc, &res, b"Font"), sub(doc, &res, b"Properties")); let mut metrics: HashMap, Metrics> = HashMap::new(); + // The id each open marked-content sequence is in: its own, else the enclosing one's. let mut stack: Vec> = Vec::new(); let mut out: HashMap = HashMap::new(); // Where each id's last glyph ended, in user space. @@ -191,16 +192,20 @@ pub(crate) fn mcid_texts(doc: &Document, page: &Dict) -> HashMap { let mut unsaved = 0usize; let (mut tm, mut tlm) = (Matrix::IDENTITY, Matrix::IDENTITY); for op in pdfcraft_content::parse(&page_content(doc, page)).ops { - let id = stack.iter().rev().find_map(|m| *m); + crate::count_mark_reads(1); + let id = stack.last().copied().flatten(); match op.op.as_slice() { - b"BMC" => stack.push(None), - b"BDC" => stack.push(match op.operands.get(1) { - Some(Object::Dict(d)) => d.get(b"MCID").and_then(Object::as_int), - Some(Object::Name(n)) => { - props.get(n).map(|x| doc.resolve(x)).and_then(|x| x.as_dict().and_then(|d| d.get(b"MCID").and_then(Object::as_int))) + b"BMC" => stack.push(id), + b"BDC" => stack.push( + match op.operands.get(1) { + Some(Object::Dict(d)) => d.get(b"MCID").and_then(Object::as_int), + Some(Object::Name(n)) => { + props.get(n).map(|x| doc.resolve(x)).and_then(|x| x.as_dict().and_then(|d| d.get(b"MCID").and_then(Object::as_int))) + } + _ => None, } - _ => None, - }), + .or(id), + ), b"EMC" => { stack.pop(); } diff --git a/crates/a11y/src/lib.rs b/crates/a11y/src/lib.rs index ff81cc4c97..22fac756ed 100644 --- a/crates/a11y/src/lib.rs +++ b/crates/a11y/src/lib.rs @@ -21,6 +21,20 @@ mod report; mod structure; mod tags; +#[cfg(test)] +thread_local! { + /// Marked-content stack entries read to decide which marks enclose an operator, for the tests + /// that keep that constant per operator. + static MARK_READS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Counts `n` marked-content stack reads (tests only). +#[inline] +fn count_mark_reads(_n: usize) { + #[cfg(test)] + MARK_READS.with(|c| c.set(c.get().saturating_add(_n))); +} + pub use alt::{AltError, ArtifactError, Figure, figures, mark_decorative, mark_tag_artifact, set_alt}; pub use autotag::{AutoTagError, AutoTagResult, autotag_text}; pub use autotag_fields::tag_form_fields; diff --git a/crates/a11y/src/tests.rs b/crates/a11y/src/tests.rs index 15ca643e83..b5eb547420 100644 --- a/crates/a11y/src/tests.rs +++ b/crates/a11y/src/tests.rs @@ -1023,6 +1023,46 @@ fn glyph_by_tm(text: &str, size: f64, y: f64) -> String { ops + " ET" } +/// Marked-content stack reads while `f` runs. +fn mark_reads(f: impl FnOnce()) -> usize { + MARK_READS.with(|c| c.set(0)); + f(); + MARK_READS.with(|c| c.get()) +} + +/// Page content nesting `depth` unmarked `BMC` blocks around a marked one, with `ops` operators inside. +fn deeply_nested_page(doc: &mut Document, depth: usize, ops: usize) -> pdfcraft_cos::Dict { + use pdfcraft_cos::{Dict, Stream}; + let mut data = "/Figure << /MCID 0 >> BDC ".repeat(1); + data.push_str(&"/X BMC ".repeat(depth)); + data.push_str(&"0 0 1 1 re f ".repeat(ops)); + data.push_str(&"EMC ".repeat(depth + 1)); + let content = doc.add(Stream::flate(Dict::new(), data.as_bytes())); + [(b"Contents".to_vec(), Object::Ref(content))].into_iter().collect() +} + +#[test] +fn marked_text_reads_its_enclosing_id_in_constant_steps() { + let mut doc = Document::new_empty(); + let page = deeply_nested_page(&mut doc, 5_000, 5_000); + let reads = mark_reads(|| { + crate::headings::mcid_texts(&doc, &page); + }); + // About 15 000 operators; a scan of the stack per operator reads millions of entries. + assert!(reads <= 4 * 15_000, "{reads} stack reads"); +} + +#[test] +fn figure_bounds_read_the_marked_content_state_in_constant_steps() { + let mut doc = Document::new_empty(); + let page = deeply_nested_page(&mut doc, 5_000, 5_000); + // No mark encloses figure 7, so a scan of the stack per operator reads all of it. + let reads = mark_reads(|| { + crate::alt::content_bbox(&doc, &page, &std::collections::HashSet::from([7])); + }); + assert!(reads <= 4 * 15_000, "{reads} stack reads"); +} + /// One tagged page whose H1 headings show `contents` (one marked-content sequence each). fn heading_page(contents: &[String]) -> Document { let marked: Vec = contents.iter().enumerate().map(|(i, c)| format!("/H1 << /MCID {i} >> BDC {c} EMC")).collect(); From 38211588c754da1a7319dc5fa0f2b0bf54069815 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:32:02 +0000 Subject: [PATCH 24/42] fix(a11y): find every figure's bounds on a page in one pass Figure listing decoded and parsed a page's content once per figure on it, so many small Figure elements pointing at one page multiplied the work. The figures are grouped by page and one pass gathers the bounds of every marked-content id they own; what a sequence draws also counts for the sequences around it, as before. --- crates/a11y/src/alt.rs | 86 ++++++++++++++++++++++++++++++---------- crates/a11y/src/lib.rs | 2 + crates/a11y/src/tests.rs | 23 ++++++++++- 3 files changed, 89 insertions(+), 22 deletions(-) diff --git a/crates/a11y/src/alt.rs b/crates/a11y/src/alt.rs index ed91f9e50f..fe8eb292c7 100644 --- a/crates/a11y/src/alt.rs +++ b/crates/a11y/src/alt.rs @@ -182,16 +182,47 @@ fn union(a: Option<[f64; 4]>, b: [f64; 4]) -> [f64; 4] { } } -/// Where the marked content `ids` is drawn on a page (images, forms and paths). -pub(crate) fn content_bbox(doc: &Document, page: &Dict, ids: &HashSet) -> Option<[f64; 4]> { +/// An open marked-content sequence while bounds are gathered: its id, when it is one of those asked +/// for, and the bounds of what has been drawn in it so far. +struct Mark { + id: Option, + drawn: Option<[f64; 4]>, +} + +/// Adds `b` to what the innermost open sequence has drawn. +fn draw(marks: &mut [Mark], b: [f64; 4]) { + if let Some(m) = marks.last_mut() { + m.drawn = Some(union(m.drawn, b)); + } +} + +/// Closes the innermost open sequence: what it drew counts for its id and for the sequence around it. +fn close(marks: &mut Vec, matching: &mut usize, out: &mut HashMap) { + let Some(m) = marks.pop() else { return }; + if m.id.is_some() { + *matching = matching.saturating_sub(1); + } + let Some(b) = m.drawn else { return }; + if let Some(id) = m.id { + let e = out.entry(id).or_insert(b); + *e = union(Some(*e), b); + } + draw(marks, b); +} + +/// Where each of the marked content `ids` is drawn on a page (images, forms and paths), in one pass +/// over the page. What a sequence draws counts for every sequence of `ids` around it too. +pub(crate) fn content_bboxes(doc: &Document, page: &Dict, ids: &HashSet) -> HashMap { let res = page.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned()).unwrap_or_default(); let xobjects = res.get(b"XObject").map(|x| doc.resolve(x)).and_then(|x| x.as_dict().cloned()).unwrap_or_default(); let props = page_properties(doc, page); + #[cfg(test)] + crate::BOUNDS_PASSES.with(|c| c.set(c.get() + 1)); let mut ctm = vec![Matrix([1.0, 0.0, 0.0, 1.0, 0.0, 0.0])]; - // Whether each open marked-content sequence is one of `ids`, and how many of them are. - let mut marks: Vec = Vec::new(); + // The open marked-content sequences, and how many of them are one of `ids`. + let mut marks: Vec = Vec::new(); let mut matching = 0usize; - let mut bbox = None; + let mut out = HashMap::new(); let mut path: Vec<(f64, f64)> = Vec::new(); for op in pdfcraft_content::parse(&page_content(doc, page)).ops { crate::count_mark_reads(1); @@ -209,17 +240,13 @@ pub(crate) fn content_bbox(doc: &Document, page: &Dict, ids: &HashSet) -> O *t = m.then(&top); } } - b"BMC" => marks.push(false), + b"BMC" => marks.push(Mark { id: None, drawn: None }), b"BDC" => { - let wanted = bdc_mcid(doc, &props, &op).is_some_and(|n| ids.contains(&n)); - matching += usize::from(wanted); - marks.push(wanted); - } - b"EMC" => { - if marks.pop() == Some(true) { - matching = matching.saturating_sub(1); - } + let id = bdc_mcid(doc, &props, &op).filter(|n| ids.contains(n)); + matching += usize::from(id.is_some()); + marks.push(Mark { id, drawn: None }); } + b"EMC" => close(&mut marks, &mut matching, &mut out), b"m" | b"l" => { if let Some([x, y]) = op.nums::<2>() { path.push(top.apply(x, y)); @@ -240,7 +267,7 @@ pub(crate) fn content_bbox(doc: &Document, page: &Dict, ids: &HashSet) -> O b"f" | b"F" | b"f*" | b"B" | b"B*" | b"b" | b"b*" | b"S" | b"s" | b"n" => { if inside && !matches!(op.op.as_slice(), b"n") { for (x, y) in &path { - bbox = Some(union(bbox, [*x, *y, *x, *y])); + draw(&mut marks, [*x, *y, *x, *y]); } } path.clear(); @@ -260,13 +287,16 @@ pub(crate) fn content_bbox(doc: &Document, page: &Dict, ids: &HashSet) -> O } _ => top.bbox([0.0, 0.0, 1.0, 1.0]), }; - bbox = Some(union(bbox, unit)); + draw(&mut marks, unit); } - b"BI" if inside => bbox = Some(union(bbox, top.bbox([0.0, 0.0, 1.0, 1.0]))), + b"BI" if inside => draw(&mut marks, top.bbox([0.0, 0.0, 1.0, 1.0])), _ => {} } } - bbox + while !marks.is_empty() { + close(&mut marks, &mut matching, &mut out); + } + out } /// The figures, in document order. @@ -275,14 +305,28 @@ pub fn figures(doc: &Document) -> Vec
{ let role_map = root.get(b"RoleMap").map(|o| doc.resolve(o)).and_then(|o| o.as_dict().cloned()).unwrap_or_default(); let pages = pdfcraft_model::pages(doc); let index: HashMap = pages.iter().enumerate().map(|(i, p)| (p.obj, i)).collect(); - elements(doc) + let figures: Vec<(Element, Option)> = elements(doc) .into_iter() .filter(|e| e.dict.name(b"S").is_some_and(|s| standard(doc, &role_map, s) == b"Figure")) .map(|e| { let page = e.page.and_then(|p| index.get(&p).copied()); + (e, page) + }) + .collect(); + let mut wanted: HashMap> = HashMap::new(); + for (e, page) in &figures { + if let Some(p) = page { + wanted.entry(*p).or_default().extend(mcids(&e.dict)); + } + } + let bounds: HashMap> = + wanted.into_iter().filter_map(|(p, ids)| Some((p, content_bboxes(doc, &pages.get(p)?.dict, &ids)))).collect(); + figures + .into_iter() + .map(|(e, page)| { let alt = e.dict.get(b"Alt").map(|o| doc.resolve(o)).and_then(|o| o.as_string().map(|s| s.to_text())); - let ids: HashSet = mcids(&e.dict).into_iter().collect(); - let bbox = page.and_then(|p| content_bbox(doc, &pages[p].dict, &ids)); + let on_page = page.and_then(|p| bounds.get(&p)); + let bbox = on_page.and_then(|b| mcids(&e.dict).iter().filter_map(|id| b.get(id)).fold(None, |acc, b| Some(union(acc, *b)))); Figure { obj: e.object, page, alt, bbox } }) .collect() diff --git a/crates/a11y/src/lib.rs b/crates/a11y/src/lib.rs index 22fac756ed..21d8ea5820 100644 --- a/crates/a11y/src/lib.rs +++ b/crates/a11y/src/lib.rs @@ -26,6 +26,8 @@ thread_local! { /// Marked-content stack entries read to decide which marks enclose an operator, for the tests /// that keep that constant per operator. static MARK_READS: std::cell::Cell = const { std::cell::Cell::new(0) }; + /// Page content passes made to find figure bounds. + static BOUNDS_PASSES: std::cell::Cell = const { std::cell::Cell::new(0) }; } /// Counts `n` marked-content stack reads (tests only). diff --git a/crates/a11y/src/tests.rs b/crates/a11y/src/tests.rs index b5eb547420..a24f59edb2 100644 --- a/crates/a11y/src/tests.rs +++ b/crates/a11y/src/tests.rs @@ -1058,11 +1058,32 @@ fn figure_bounds_read_the_marked_content_state_in_constant_steps() { let page = deeply_nested_page(&mut doc, 5_000, 5_000); // No mark encloses figure 7, so a scan of the stack per operator reads all of it. let reads = mark_reads(|| { - crate::alt::content_bbox(&doc, &page, &std::collections::HashSet::from([7])); + crate::alt::content_bboxes(&doc, &page, &std::collections::HashSet::from([7])); }); assert!(reads <= 4 * 15_000, "{reads} stack reads"); } +#[test] +fn figure_bounds_take_one_pass_over_each_page() { + let n = 50; + let content: Vec = (0..n).map(|i| format!("/Figure << /MCID {i} >> BDC {i} 0 1 1 re f EMC")).collect(); + let kids: Vec = (0..n).map(|i| format!("{} 0 R", 6 + i)).collect(); + let mut objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /StructTreeRoot 5 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into(), + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Contents 4 0 R >>".into(), + stream(&content.join(" ")), + format!("<< /Type /StructTreeRoot /K [{}] >>", kids.join(" ")), + ]; + objs.extend((0..n).map(|i| format!("<< /S /Figure /Pg 3 0 R /K {i} >>"))); + let doc = pdf(&objs, ""); + BOUNDS_PASSES.with(|c| c.set(0)); + let f = figures(&doc); + assert_eq!(BOUNDS_PASSES.with(|c| c.get()), 1, "page passes"); + assert_eq!(f.len(), n); + assert!(f.iter().enumerate().all(|(i, f)| f.bbox == Some([i as f64, 0.0, i as f64 + 1.0, 1.0]))); +} + /// One tagged page whose H1 headings show `contents` (one marked-content sequence each). fn heading_page(contents: &[String]) -> Document { let marked: Vec = contents.iter().enumerate().map(|(i, c)| format!("/H1 << /MCID {i} >> BDC {c} EMC")).collect(); From 1cc392e5b04ada01b130236c6d101636e8bb62da Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:33:28 +0000 Subject: [PATCH 25/42] fix(fonts): cut ToUnicode destinations to the CMap limit A sequential bfrange stores one destination and builds every covered glyph's text from all of it, so one huge destination and many repeats of its codes made text extraction copy megabytes per glyph. Destinations keep at most 256 UTF-16 units (the format's 512 bytes), and marked-text extraction stops asking for a glyph's text once its id has all the text it keeps. --- crates/a11y/src/headings.rs | 12 ++++++------ crates/fonts/src/pdf.rs | 22 ++++++++++++++++++++-- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/crates/a11y/src/headings.rs b/crates/a11y/src/headings.rs index d3a478cb95..41c749ed3c 100644 --- a/crates/a11y/src/headings.rs +++ b/crates/a11y/src/headings.rs @@ -285,17 +285,17 @@ pub(crate) fn mcid_texts(doc: &Document, page: &Dict) -> HashMap { }; for (code, len) in m.codes(&s.bytes) { let trm = tm.then(&gs.ctm); + // A glyph's text is only built while its id still takes more. if let Some(id) = id + && out.get(&id).is_none_or(|t| t.len() < MAX_TITLE * 4) && let Some(glyph) = m.text_of(code) { let text = out.entry(id).or_default(); - if text.len() < MAX_TITLE * 4 { - let breaks = ends.get(&id).is_some_and(|end| new_word(&trm, *end, gs.size, gs.scale)); - if breaks && !text.is_empty() && !text.ends_with(' ') && !glyph.starts_with(' ') { - text.push(' '); - } - text.push_str(&glyph); + let breaks = ends.get(&id).is_some_and(|end| new_word(&trm, *end, gs.size, gs.scale)); + if breaks && !text.is_empty() && !text.ends_with(' ') && !glyph.starts_with(' ') { + text.push(' '); } + text.push_str(&glyph); } let advance = m.width(code) * gs.size + gs.char_spacing + if m.is_space(code, len) { gs.word_spacing } else { 0.0 }; tm = Matrix::translate(advance * gs.scale, 0.0).then(&tm); diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index 0aaed6939e..0badcfc3a9 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -905,9 +905,13 @@ impl Metrics { } } -/// The UTF-16 code units of CMap hex bytes. +/// Most UTF-16 units kept from one ToUnicode destination: the CMap format allows destinations of up +/// to 512 bytes, and every glyph a sequential range covers builds its text from the whole of one. +const MAX_DESTINATION_UNITS: usize = 256; + +/// The UTF-16 code units of CMap hex bytes, at most [`MAX_DESTINATION_UNITS`] of them. fn code_units(b: &[u8]) -> Vec { - b.chunks(2).map(|c| u16::from_be_bytes([c[0], *c.get(1).unwrap_or(&0)])).collect() + b.chunks(2).take(MAX_DESTINATION_UNITS).map(|c| u16::from_be_bytes([c[0], *c.get(1).unwrap_or(&0)])).collect() } /// UTF-16BE (with surrogates) from CMap hex bytes. @@ -1262,6 +1266,20 @@ mod tests { use super::*; + /// Every glyph of a sequential range builds its text from the destination, so a long one is cut + /// to the CMap limit once, when it is read. + #[test] + fn unicode_destinations_are_cut_to_the_cmap_limit() { + let long = "0041".repeat(4096); + let cmap = format!("2 beginbfrange <0010> <0020> <{long}> <0030> <0030> [<{long}>] endbfrange 1 beginbfchar <0040> <{long}> endbfchar"); + let mut u = Unicode::default(); + parse_to_unicode(cmap.as_bytes(), &mut u); + for code in [0x10, 0x20, 0x30, 0x40] { + let text = u.text(code).unwrap_or_default(); + assert!(!text.is_empty() && text.len() <= MAX_DESTINATION_UNITS * 3, "code {code:#x}: {} bytes", text.len()); + } + } + /// A font whose ranges cover a few low codes keeps tables that size, not a fixed 64k-entry one. #[test] fn low_code_tables_end_with_the_last_covered_code() { From 75c4275810edafaa3b607d136b18569d18c797cd Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:35:00 +0000 Subject: [PATCH 26/42] fix(fonts): look up CID and width ranges by binary search Code-to-CID and /W range lookups scanned every record in declaration order, once per glyph and once per explicit ToUnicode entry while a font was read, so a few thousand records multiplied by as many codes took billions of steps. Both tables are indexed once into stretches whose first-declared covering record wins, the precedence the scan had, and the /W ranges stop at the CMap entry limit. --- crates/fonts/src/pdf.rs | 157 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 141 insertions(+), 16 deletions(-) diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index 0badcfc3a9..4a118a07d3 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -33,9 +33,9 @@ pub struct Metrics { widths: Vec, /// Composite fonts: CID → width, and CID ranges with one width. cid_widths: HashMap, - cid_ranges: Vec<(u32, u32, f64)>, + cid_ranges: Spans, /// Code → CID for embedded non-identity CMaps (`cidrange` / `cidchar`). - cid_map: Vec<(u32, u32, u32)>, + cid_map: Spans, default: f64, /// Composite fonts with an explicit `/DW`: `/W` may leave out the glyphs that have that /// width (Chrome and Skia do), so a CID missing from `/W` is not a missing glyph. @@ -598,8 +598,8 @@ impl Metrics { first: 0, widths: Vec::new(), cid_widths: HashMap::new(), - cid_ranges: Vec::new(), - cid_map: Vec::new(), + cid_ranges: Spans::default(), + cid_map: Spans::default(), default: 500.0, explicit_default: false, std14: Some(crate::Std14::Helvetica), @@ -685,7 +685,7 @@ impl Metrics { if !spaces.is_empty() { m.codes = Codes::Ranges(spaces); } - m.cid_map = cids; + m.cid_map = Spans::new(cids); } let desc = font .get(b"DescendantFonts") @@ -698,6 +698,7 @@ impl Metrics { m.explicit_default = true; } if let Some(w) = desc.get(b"W").map(|w| doc.resolve(w)).and_then(|w| w.as_array().cloned()) { + let mut ranges = Vec::new(); let mut i = 0; while i < w.len() { let Some(c0) = doc.resolve(&w[i]).as_f64().map(|v| v as u32) else { break }; @@ -714,13 +715,16 @@ impl Metrics { } Some(c1) => { if let (Some(c1), Some(v)) = (c1.as_f64(), w.get(i + 2).and_then(|x| doc.resolve(x).as_f64())) { - m.cid_ranges.push((c0, c1 as u32, v)); + if ranges.len() < MAX_CMAP_ENTRIES { + ranges.push((c0, c1 as u32, v)); + } } i += 3; } None => break, } } + m.cid_ranges = Spans::new(ranges); } descriptor = dict(doc, desc.get(b"FontDescriptor")); } else { @@ -791,14 +795,14 @@ impl Metrics { if self.cid_map.is_empty() { return code; } - self.cid_map.iter().find(|(lo, hi, _)| code >= *lo && code <= *hi).map_or(0, |(lo, _, c)| c.saturating_add(code - lo)) + self.cid_map.find(code).map_or(0, |(lo, _, c)| c.saturating_add(code.saturating_sub(*lo))) } /// The advance of `code` in text space per unit of font size. pub fn width(&self, code: u32) -> f64 { if self.composite { let cid = self.cid(code); - let w = self.cid_widths.get(&cid).copied().or_else(|| self.cid_ranges.iter().find(|(a, b, _)| cid >= *a && cid <= *b).map(|r| r.2)); + let w = self.cid_widths.get(&cid).copied().or_else(|| self.cid_ranges.find(cid).map(|r| r.2)); return w.unwrap_or(self.default) * self.scale; } if let Some(w) = code.checked_sub(self.first).and_then(|i| self.widths.get(i as usize)) { @@ -858,7 +862,7 @@ impl Metrics { // A CID /W leaves out takes /DW, and Skia (Chrome) leaves out every CID of that width: for // those, only the subset's program says whether the glyph is there. return self.cid_widths.contains_key(&cid) - || self.cid_ranges.iter().any(|(a, b, _)| cid >= *a && cid <= *b) + || self.cid_ranges.find(cid).is_some() || !self.subset || self.outlined_cids.get(cid as usize / 64).is_some_and(|w| (*w >> (cid % 64)) & 1 == 1) // No readable TrueType program to ask (e.g. CFF or missing): an explicit /DW @@ -958,16 +962,69 @@ fn add_range(out: &mut Unicode, budget: &mut usize, seq: &mut u32, lo: u32, hi: true } +#[cfg(test)] +thread_local! { + /// Code span records read by CID and width lookups, for the tests that keep a lookup logarithmic. + static SPAN_READS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Counts one span record read (tests only). +#[inline] +fn count_span_reads() { + #[cfg(test)] + SPAN_READS.with(|c| c.set(c.get().saturating_add(1))); +} + +/// Code spans `(lo, hi, value)` in declaration order, where the first-declared span covering a code +/// wins, indexed so that a lookup is one binary search. +#[derive(Clone, Debug, Default, PartialEq)] +struct Spans { + spans: Vec<(u32, u32, T)>, + /// The span covering each stretch of codes; see [`stretches`]. + cover: Vec<(u64, Option)>, +} + +impl Spans { + fn new(spans: Vec<(u32, u32, T)>) -> Self { + let bounds: Vec<(u32, u32)> = spans.iter().map(|s| (s.0, s.1)).collect(); + Spans { cover: stretches(&bounds, false), spans } + } + + fn is_empty(&self) -> bool { + self.spans.is_empty() + } + + /// The first-declared span covering `code`. + fn find(&self, code: u32) -> Option<&(u32, u32, T)> { + let step = self.cover.partition_point(|&(start, _)| { + count_span_reads(); + start <= u64::from(code) + }); + let &(_, record) = self.cover.get(step.checked_sub(1)?)?; + self.spans.get(record?) + } +} + /// For each stretch of codes, from its first code on, the latest-declared range covering it (`None` /// where no range does), sorted by code; a lookup is one binary search over it. fn cover_index(ranges: &[BfRange]) -> Vec<(u64, Option)> { + let spans: Vec<(u32, u32)> = ranges.iter().map(|r| (r.lo, r.hi)).collect(); + stretches(&spans, true) +} + +/// For each stretch of codes, from its first code on, the span `(lo, hi)` covering it (`None` where +/// none does), sorted by code: the latest-declared one when `later_wins`, else the first-declared. +/// At most `u32::MAX` spans are indexed. +fn stretches(spans: &[(u32, u32)], later_wins: bool) -> Vec<(u64, Option)> { + // Heap keys, largest for the winning record; the mapping is its own inverse. + let key = |record: u32| if later_wins { record } else { !record }; // Openings (first code, record) and closings (one past the last code), each in code order. The cover // can only change at one of their codes, so the sweep visits those codes in order. - let mut opens: Vec<(u32, u32)> = ranges.iter().enumerate().map(|(i, r)| (r.lo, i as u32)).collect(); + let mut opens: Vec<(u32, u32)> = spans.iter().zip(0..u32::MAX).map(|(&(lo, _), i)| (lo, i)).collect(); opens.sort_unstable(); - let mut closes: Vec = ranges.iter().map(|r| u64::from(r.hi) + 1).collect(); + let mut closes: Vec = spans.iter().take(opens.len()).map(|&(_, hi)| u64::from(hi) + 1).collect(); closes.sort_unstable(); - // Records that have opened and not yet closed; the top is the latest declared one. + // Keys of the records that have opened and not yet closed; the top is the winning one. let mut open: BinaryHeap = BinaryHeap::new(); let mut cover: Vec<(u64, Option)> = Vec::new(); let (mut next_open, mut next_close) = (0, 0); @@ -981,16 +1038,16 @@ fn cover_index(ranges: &[BfRange]) -> Vec<(u64, Option)> { while let Some(&(lo, record)) = opens.get(next_open) && u64::from(lo) <= p { - open.push(record); + open.push(key(record)); next_open += 1; } while closes.get(next_close).is_some_and(|&end| end <= p) { next_close += 1; } - while open.peek().is_some_and(|&top| ranges.get(top as usize).is_none_or(|r| u64::from(r.hi) < p)) { + while open.peek().is_some_and(|&top| spans.get(key(top) as usize).is_none_or(|&(_, hi)| u64::from(hi) < p)) { open.pop(); } - let winner = open.peek().map(|&top| top as usize); + let winner = open.peek().map(|&top| key(top) as usize); if cover.last().map(|&(_, w)| w) != Some(winner) { cover.push((p, winner)); } @@ -1266,6 +1323,74 @@ mod tests { use super::*; + /// Span records read while `f` runs. + fn span_reads(f: impl FnOnce()) -> usize { + SPAN_READS.with(|c| c.set(0)); + f(); + SPAN_READS.with(|c| c.get()) + } + + /// A composite font with `encoding` as its CMap, `w` as its widths and `to_unicode`. + fn composite(doc: &mut Document, encoding: &str, w: Vec, to_unicode: &str) -> Dict { + let mut desc = Dict::new(); + desc.set(b"Subtype".to_vec(), Object::name("CIDFontType2")); + desc.set(b"W".to_vec(), Object::Array(w)); + let enc = Object::Ref(doc.add(Object::Stream(Stream::flate(Dict::new(), encoding.as_bytes())))); + let tu = Object::Ref(doc.add(Object::Stream(Stream::flate(Dict::new(), to_unicode.as_bytes())))); + font( + doc, + vec![ + ("Subtype", Object::name("Type0")), + ("Encoding", enc), + ("DescendantFonts", Object::Array(vec![Object::Dict(desc)])), + ("ToUnicode", tu), + ], + ) + } + + /// Many `/W` ranges that leave a code out cost a glyph of it a binary search, not a scan of them all. + #[test] + fn widths_of_a_code_no_range_covers_take_few_steps() { + let mut doc = Document::new_empty(); + let w: Vec = (1..=10_000i64).flat_map(|k| [Object::Int(2 * k), Object::Int(2 * k), Object::Int(500)]).collect(); + let f = composite(&mut doc, "", w, ""); + let m = Metrics::from_dict(&doc, &f); + assert_eq!(m.width(4), 0.5); + let reads = span_reads(|| { + for _ in 0..1000 { + assert_eq!(m.width(3), 1.0); + } + }); + assert!(reads <= 1000 * 64, "{reads} span reads"); + } + + /// Where spans overlap, the first-declared one wins, as a scan in declaration order finds it. + #[test] + fn overlapping_spans_resolve_to_the_first_declared() { + let spans = Spans::new(vec![(0, 10, 'a'), (5, 20, 'b'), (3, 3, 'c'), (30, 25, 'd'), (u32::MAX, u32::MAX, 'e')]); + let found = |code| spans.find(code).map(|s| s.2); + let want = [(0, Some('a')), (3, Some('a')), (10, Some('a')), (11, Some('b')), (20, Some('b')), (21, None), (27, None), (u32::MAX, Some('e'))]; + for (code, value) in want { + assert_eq!(found(code), value, "code {code}"); + } + } + + /// Checking the glyph of every explicit ToUnicode entry looks its code up once in the CID map. + #[test] + fn many_unicode_entries_look_up_their_cids_in_few_steps() { + let n = 2000; + let cidchars: String = (0..n).map(|k| format!("<{:04X}> {} ", 0x8000 + k, k + 1)).collect(); + let encoding = format!("1 begincodespacerange <0000> endcodespacerange {n} begincidchar {cidchars} endcidchar"); + let bfchars: String = (0..n).map(|k| format!("<{:04X}> <{:04X}> ", 0x1000 + k, 0x4E00 + k)).collect(); + let to_unicode = format!("{n} beginbfchar {bfchars} endbfchar"); + let mut doc = Document::new_empty(); + let f = composite(&mut doc, &encoding, Vec::new(), &to_unicode); + let reads = span_reads(|| { + Metrics::from_dict(&doc, &f); + }); + assert!(reads <= n * 64, "{reads} span reads"); + } + /// Every glyph of a sequential range builds its text from the destination, so a long one is cut /// to the CMap limit once, when it is read. #[test] @@ -1736,7 +1861,7 @@ mod tests { let map = to_unicode(b"1 beginbfrange <0001> <0003> [<0041> <0043>] endbfrange"); assert_eq!((map.get(&2), map.get(&3).map(String::as_str)), (None, Some("C"))); let mut m = Metrics::fallback(); - (_, m.cid_map) = parse_cmap(b"1 begincidrange <0000> 4294967295 endcidrange"); + m.cid_map = Spans::new(parse_cmap(b"1 begincidrange <0000> 4294967295 endcidrange").1); assert_eq!(m.cid(5), u32::MAX); let mut doc = Document::new_empty(); let mut enc = Dict::new(); From 5609bcbe37728d65186303b4c8817dbdb8e8c3b8 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:36:06 +0000 Subject: [PATCH 27/42] fix(fonts): wrap text measuring each word once Line wrapping built and measured the whole candidate line for every word, so a long field value or annotation text at a tiny font size, with many words on one line, took quadratic time and copying. The wrapper keeps the line's width as it goes and measures each word, space and broken character once. --- crates/fonts/src/lib.rs | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/crates/fonts/src/lib.rs b/crates/fonts/src/lib.rs index ec103ed5f0..37a2ea74d8 100644 --- a/crates/fonts/src/lib.rs +++ b/crates/fonts/src/lib.rs @@ -88,17 +88,28 @@ pub fn wrap(text: &str, size: f64, width: f64) -> Vec { } /// [`wrap`], measuring with `measure` (for example [`cjk_width`]). +/// +/// A line measures as the sum of its words, the spaces between them and the characters of a broken +/// word, each measured once, so the work stays linear in `text` however much fits on a line. pub fn wrap_with(text: &str, size: f64, width: f64, measure: impl Fn(&str, f64) -> f64) -> Vec { let mut lines = Vec::new(); + let space = measure(" ", size); + let mut buf = [0u8; 4]; // A CRLF pair is one break, a lone CR or LF is one break each. for para in text.replace("\r\n", "\n").split(['\n', '\r']) { let mut line = String::new(); + let mut line_width = 0.0; // No word is on the line yet; an empty word (a leading space) still starts it. let mut fresh = true; for word in para.split(' ') { - let candidate = if fresh { word.to_string() } else { format!("{line} {word}") }; - if measure(&candidate, size) <= width || fresh && measure(word, size) <= width { - line = candidate; + let word_width = measure(word, size); + let candidate = if fresh { word_width } else { line_width + space + word_width }; + if candidate <= width { + if !fresh { + line.push(' '); + } + line.push_str(word); + line_width = candidate; fresh = false; continue; } @@ -108,11 +119,15 @@ pub fn wrap_with(text: &str, size: f64, width: f64, measure: impl Fn(&str, f64) } else { lines.push(std::mem::take(&mut line)); } + line_width = 0.0; for ch in word.chars() { - if !line.is_empty() && measure(&format!("{line}{ch}"), size) > width { + let ch_width = measure(ch.encode_utf8(&mut buf), size); + if !line.is_empty() && line_width + ch_width > width { lines.push(std::mem::take(&mut line)); + line_width = 0.0; } line.push(ch); + line_width += ch_width; } fresh = line.is_empty(); } @@ -442,4 +457,20 @@ mod tests { let lines = wrap_with("日本語のテキストは文字単位で折り返されます", 10.0, 45.0, cjk_width); assert!(lines.len() > 3 && lines.iter().all(|l| cjk_width(l, 10.0) <= 45.0), "{lines:?}"); } + + /// Wrapping measures each word and character a bounded number of times, however many fit on a line. + #[test] + fn wrapping_measures_each_word_a_few_times() { + let measured = std::cell::Cell::new(0usize); + let measure = |s: &str, size: f64| { + measured.set(measured.get() + s.len()); + helvetica_width(s, size) + }; + let text = format!("{} {}", "word ".repeat(5_000), "x".repeat(5_000)); + let lines = wrap_with(&text, 1.0, 13_000.0, measure); + assert_eq!(lines.len(), 2, "the words fit on one line, the long word on another"); + assert!(measured.get() <= 4 * text.len(), "{} bytes measured", measured.get()); + let narrow = wrap_with(&text, 10.0, 100.0, helvetica_width); + assert!(narrow.iter().all(|l| helvetica_width(l, 10.0) <= 100.0), "{narrow:?}"); + } } From fcedb156893891c718fca03dd27c9a04ba5ae110 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:36:48 +0000 Subject: [PATCH 28/42] fix(export): find font size and colour slots through a map The body-size tally and the RTF colour table searched every slot seen so far for each block, and the RTF writer searched the colour table again for every paragraph and cell, so a document with many distinct sizes or colours made export quadratic. Both keep their first-seen order and look slots up in a hash map. --- crates/export/src/lib.rs | 75 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 5 deletions(-) diff --git a/crates/export/src/lib.rs b/crates/export/src/lib.rs index f3030ad58d..8c3999712f 100644 --- a/crates/export/src/lib.rs +++ b/crates/export/src/lib.rs @@ -8,6 +8,8 @@ #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] +use std::collections::HashMap; + mod pptx; mod zip; @@ -85,14 +87,33 @@ enum Item<'a> { PageBreak, } +#[cfg(test)] +thread_local! { + /// Entries read to find a font size's or a colour's slot, for the tests that keep it constant. + static LOOKUP_STEPS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Counts `n` entries read by a lookup (tests only). +#[inline] +fn count_lookup_steps(_n: usize) { + #[cfg(test)] + LOOKUP_STEPS.with(|c| c.set(c.get().saturating_add(_n))); +} + /// The body text size: the size most of the document's characters are set in. fn body_size(pages: &[Page]) -> f64 { + // Sizes in first-seen order (which breaks ties), and where each one is in it. let mut by: Vec<(i64, usize)> = Vec::new(); + let mut slot: HashMap = HashMap::new(); for b in pages.iter().flat_map(|p| &p.blocks) { let k = (b.size * 2.0).round() as i64; - match by.iter_mut().find(|e| e.0 == k) { - Some(e) => e.1 += b.text.len(), - None => by.push((k, b.text.len())), + count_lookup_steps(1); + let i = *slot.entry(k).or_insert_with(|| { + by.push((k, 0)); + by.len() - 1 + }); + if let Some(e) = by.get_mut(i) { + e.1 = e.1.saturating_add(b.text.len()); } } by.into_iter().max_by_key(|e| e.1).map_or(11.0, |e| e.0 as f64 / 2.0) @@ -831,9 +852,12 @@ pub fn rtf(pages: &[Page]) -> String { let items = items(pages); // The colour table: entry 0 is the default (auto) colour, then each non-black colour once. let mut colors: Vec<[u8; 3]> = Vec::new(); + let mut slot: HashMap<[u8; 3], usize> = HashMap::new(); let mut note = |c: [f64; 3]| { let c = rgb8(c); - if c != [0, 0, 0] && !colors.contains(&c) { + count_lookup_steps(1); + if c != [0, 0, 0] && !slot.contains_key(&c) { + slot.insert(c, colors.len()); colors.push(c); } }; @@ -847,7 +871,8 @@ pub fn rtf(pages: &[Page]) -> String { // `\cfN` for a colour (nothing for black, which keeps the default). let cf = |c: [f64; 3]| { let c = rgb8(c); - colors.iter().position(|e| *e == c).map_or(String::new(), |i| format!("\\cf{}", i + 1)) + count_lookup_steps(1); + slot.get(&c).map_or(String::new(), |i| format!("\\cf{}", i + 1)) }; let mut s = String::from("{\\rtf1\\ansi\\deff0{\\fonttbl{\\f0 Helvetica;}}\n"); if !colors.is_empty() { @@ -911,6 +936,46 @@ pub fn rtf(pages: &[Page]) -> String { mod tests { use super::*; + /// A page of `n` one-word blocks, each in its own size and colour. + fn varied_page(n: usize) -> Page { + let blocks = (0..n) + .map(|i| Block { + text: "word".into(), + rect: [72.0, i as f64, 100.0, i as f64 + 1.0], + size: 4.0 + i as f64 * 0.5, + bold: false, + italic: false, + color: [(i % 256) as f64 / 255.0, (i / 256 % 256) as f64 / 255.0, 0.5], + rotation: 0.0, + }) + .collect(); + Page { width: 612.0, height: 792.0, blocks, images: Vec::new(), background: None } + } + + fn lookup_steps(f: impl FnOnce()) -> usize { + LOOKUP_STEPS.with(|c| c.set(0)); + f(); + LOOKUP_STEPS.with(|c| c.get()) + } + + #[test] + fn many_distinct_sizes_are_counted_in_constant_steps_each() { + let pages = [varied_page(3000)]; + let steps = lookup_steps(|| { + body_size(&pages); + }); + assert!(steps <= 2 * 3000, "{steps} steps"); + } + + #[test] + fn many_distinct_colours_take_constant_steps_each() { + let pages = [varied_page(3000)]; + let mut out = String::new(); + let steps = lookup_steps(|| out = rtf(&pages)); + assert!(steps <= 4 * 3000, "{steps} steps"); + assert!(out.contains("\\cf3000 ") || out.contains("\\cf3000\\"), "the last colour has the last slot"); + } + fn page() -> Page { let b = |t: &str, y: f64, size: f64, bold: bool| Block { text: t.into(), From dba51fc8237dd04b5cd594d5fa56106e902d74b8 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:37:39 +0000 Subject: [PATCH 29/42] fix(optimize): refuse a JPEG whose own size differs from its dictionary's The pixel reader capped the image size the dictionary gives, then decoded the JPEG at the size its header gives and compared afterwards, so a small image dictionary over a huge JPEG allocated the huge one first. The reader takes the size from the JPEG header and refuses a mismatch before decoding. --- crates/optimize/src/images.rs | 40 ++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/crates/optimize/src/images.rs b/crates/optimize/src/images.rs index 6182b08922..1d8a59e253 100644 --- a/crates/optimize/src/images.rs +++ b/crates/optimize/src/images.rs @@ -227,6 +227,19 @@ fn is_dct(s: &Stream) -> Option { } } +#[cfg(test)] +thread_local! { + /// Pixels decoded from JPEG data, for the test that keeps a mismatched JPEG from decoding. + static DECODED_PIXELS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Counts `n` decoded pixels (tests only). +#[inline] +fn count_decoded(_n: u64) { + #[cfg(test)] + DECODED_PIXELS.with(|c| c.set(c.get().saturating_add(_n))); +} + /// 8-bit samples of an image (`n` components), decoded. pub(crate) fn pixels(s: &Stream, n: usize) -> Option> { let (w, h) = (u32::try_from(s.dict.int(b"Width")?).ok()?, u32::try_from(s.dict.int(b"Height")?).ok()?); @@ -235,7 +248,15 @@ pub(crate) fn pixels(s: &Stream, n: usize) -> Option> { } let bytes = usize::try_from(w).ok()?.checked_mul(usize::try_from(h).ok()?)?.checked_mul(n)?; if is_dct(s)? { - let img = image::load_from_memory_with_format(&s.raw, image::ImageFormat::Jpeg).ok()?; + use image::ImageDecoder as _; + // The JPEG's own size is what gets decoded: one other than the dictionary's is refused from + // its header, before any pixels are. + let decoder = image::codecs::jpeg::JpegDecoder::new(std::io::Cursor::new(&s.raw[..])).ok()?; + if decoder.dimensions() != (w, h) { + return None; + } + let img = image::DynamicImage::from_decoder(decoder).ok()?; + count_decoded(u64::from(img.width()) * u64::from(img.height())); // Adobe CMYK/YCCK JPEGs aren't handled (the colour space check excludes them). let v = if n == 1 { img.into_luma8().into_raw() } else { img.into_rgb8().into_raw() }; (v.len() == bytes).then_some(v) @@ -432,4 +453,21 @@ mod dimension_tests { let stream = Stream::from_raw(dict, vec![1, 2, 3, 4, 5]); assert_eq!(pixels(&stream, 1), Some(vec![1, 2, 3, 4])); } + + /// A JPEG larger than its dictionary says is refused from its header, before its pixels are decoded. + #[test] + fn a_jpeg_of_another_size_than_its_dictionary_is_not_decoded() { + let data = jpeg(&vec![128; 1000 * 1000], 1, 1000, 1000, 50).unwrap_or_default(); + let stream = |w: i64, h: i64| { + let mut dict = Dict::new(); + dict.set(b"Width".to_vec(), Object::Int(w)); + dict.set(b"Height".to_vec(), Object::Int(h)); + dict.set(b"Filter".to_vec(), Object::name("DCTDecode")); + Stream::from_raw(dict, data.clone()) + }; + DECODED_PIXELS.with(|c| c.set(0)); + assert!(pixels(&stream(10, 10), 1).is_none()); + assert_eq!(DECODED_PIXELS.with(|c| c.get()), 0, "pixels decoded"); + assert_eq!(pixels(&stream(1000, 1000), 1).map(|p| p.len()), Some(1000 * 1000)); + } } From 6b62585339b351d1bf8827377aa01117d5a1b4f7 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:39:09 +0000 Subject: [PATCH 30/42] fix(optimize): bound the search for fragment runs on a page A run of images that is not one picture was searched again from each of its groups, collecting the rest of the run every time, so many images drawn over one another made a page quadratic. The search reads at most 16 groups per content operator (at least 65 536) and leaves the rest of the page as it is once that is spent. --- crates/optimize/src/fragments.rs | 55 ++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/crates/optimize/src/fragments.rs b/crates/optimize/src/fragments.rs index caf617f717..44e1655460 100644 --- a/crates/optimize/src/fragments.rs +++ b/crates/optimize/src/fragments.rs @@ -29,6 +29,19 @@ pub const MIN_FRAGMENTS: usize = 4; /// Most pixels a merged image may have (memory). const MAX_PIXELS: u64 = 80_000_000; +/// Fragment groups a page's search for runs may read per content operator, and at least. A run +/// that is not one picture is searched again from each of its groups, so without a limit images +/// drawn over one another take time quadratic in their number; past it, the rest of the page is +/// left as it is. +const SCAN_PER_OP: usize = 16; +const MIN_SCAN: usize = 1 << 16; + +#[cfg(test)] +thread_local! { + /// Fragment groups read, for the test that keeps a page's search linear in its content. + static GROUP_READS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// One fragment: its image, its box in the coordinates the run is drawn in, and its size. struct Fragment { image: ObjRef, @@ -40,6 +53,8 @@ struct Fragment { /// The ops `q`, one or more `cm`, `Do`, `Q` starting at `i`: the index after them, the matrix /// and the name drawn. fn group_at(ops: &[Op], i: usize) -> Option<(usize, Matrix, Vec)> { + #[cfg(test)] + GROUP_READS.with(|c| c.set(c.get().saturating_add(1))); if !ops.get(i)?.is("q") { return None; } @@ -226,7 +241,13 @@ fn merge_page(doc: &mut Document, page: ObjRef, report: &mut Report, progress: & let ops = parsed.ops; let mut out: Vec = Vec::with_capacity(ops.len()); let (mut i, mut merged, mut k) = (0, 0, 0); + let budget = ops.len().saturating_mul(SCAN_PER_OP).max(MIN_SCAN); + let mut scanned = 0usize; while i < ops.len() { + if scanned > budget { + out.extend(ops.get(i..).unwrap_or_default().iter().cloned()); + break; + } // The longest run of fragment groups from here with one colour space. let mut run = Vec::new(); let mut kind: Option<(Object, usize)> = None; @@ -245,10 +266,12 @@ fn merge_page(doc: &mut Document, page: ObjRef, report: &mut Report, progress: & run.push(Fragment { image, rect: [e, f, e + a, f + d], width, height }); j = next; } + scanned = scanned.saturating_add(run.len()).saturating_add(1); // Not one picture as a whole: the rows drawn one under the other from the start may be. if run.len() >= MIN_FRAGMENTS && !stacked(&mut run) { run = drawn_in_sequence(&ops, i, &xobjects, doc); j = run_end(&ops, i, run.len()); + scanned = scanned.saturating_add(run.len().saturating_mul(2)).saturating_add(2); } let Some((cs, n)) = kind.filter(|_| run.len() >= MIN_FRAGMENTS && stacked(&mut run)) else { // Not a run: keep this op and look again from the next one. @@ -304,6 +327,8 @@ pub(crate) fn run(doc: &mut Document, pages: &[ObjRef], report: &mut Report, pro #[cfg(test)] mod tests { + use pdfcraft_cos::{Dict, Document, Object, Stream}; + use super::*; #[test] @@ -318,4 +343,34 @@ mod tests { ]; assert!(merge(&Document::new_empty(), &run, 3).is_none()); } + + /// Many images drawn on top of each other are not one picture; looking for runs among them reads + /// a bounded number of groups per operator, not the rest of the page from every group. + #[test] + fn rejected_runs_read_a_bounded_number_of_groups() { + let mut doc = Document::new_empty(); + let mut image = Dict::new(); + for (k, v) in + [("Subtype", Object::name("Image")), ("Width", Object::Int(1)), ("Height", Object::Int(1)), ("BitsPerComponent", Object::Int(8))] + { + image.set(k.as_bytes().to_vec(), v); + } + image.set(b"ColorSpace".to_vec(), Object::name("DeviceGray")); + let image = doc.add(Object::Stream(Stream::from_raw(image, vec![0]))); + let groups = 4000; + let content = doc.add(Object::Stream(Stream::flate(Dict::new(), "q 10 0 0 10 0 0 cm /Im Do Q\n".repeat(groups).as_bytes()))); + let mut xobjects = Dict::new(); + xobjects.set(b"Im".to_vec(), Object::Ref(image)); + let mut resources = Dict::new(); + resources.set(b"XObject".to_vec(), Object::Dict(xobjects)); + let mut page = Dict::new(); + page.set(b"Contents".to_vec(), Object::Ref(content)); + page.set(b"Resources".to_vec(), Object::Dict(resources)); + let page = doc.add(Object::Dict(page)); + GROUP_READS.with(|c| c.set(0)); + let merged = merge_page(&mut doc, page, &mut Report::default(), &mut |_| true); + assert_eq!(merged.ok(), Some(0)); + let reads = GROUP_READS.with(|c| c.get()); + assert!(reads <= 40 * 4 * groups, "{reads} group reads"); + } } From 3e968c8aa24adfb729c6d45a3c98abf5fed07027 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 17:36:35 +0000 Subject: [PATCH 31/42] chore(a11y, fonts): satisfy clippy in the CID width ranges and a test helper --- crates/a11y/src/tests.rs | 2 +- crates/fonts/src/pdf.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/a11y/src/tests.rs b/crates/a11y/src/tests.rs index a24f59edb2..ad57477364 100644 --- a/crates/a11y/src/tests.rs +++ b/crates/a11y/src/tests.rs @@ -1033,7 +1033,7 @@ fn mark_reads(f: impl FnOnce()) -> usize { /// Page content nesting `depth` unmarked `BMC` blocks around a marked one, with `ops` operators inside. fn deeply_nested_page(doc: &mut Document, depth: usize, ops: usize) -> pdfcraft_cos::Dict { use pdfcraft_cos::{Dict, Stream}; - let mut data = "/Figure << /MCID 0 >> BDC ".repeat(1); + let mut data = "/Figure << /MCID 0 >> BDC ".to_string(); data.push_str(&"/X BMC ".repeat(depth)); data.push_str(&"0 0 1 1 re f ".repeat(ops)); data.push_str(&"EMC ".repeat(depth + 1)); diff --git a/crates/fonts/src/pdf.rs b/crates/fonts/src/pdf.rs index 4a118a07d3..3ddd16a9b4 100644 --- a/crates/fonts/src/pdf.rs +++ b/crates/fonts/src/pdf.rs @@ -714,10 +714,10 @@ impl Metrics { i += 2; } Some(c1) => { - if let (Some(c1), Some(v)) = (c1.as_f64(), w.get(i + 2).and_then(|x| doc.resolve(x).as_f64())) { - if ranges.len() < MAX_CMAP_ENTRIES { - ranges.push((c0, c1 as u32, v)); - } + if let (Some(c1), Some(v)) = (c1.as_f64(), w.get(i + 2).and_then(|x| doc.resolve(x).as_f64())) + && ranges.len() < MAX_CMAP_ENTRIES + { + ranges.push((c0, c1 as u32, v)); } i += 3; } From f42fb0ad49bc729fbfa8ec492dae592f8ec89bed Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:43:01 +0000 Subject: [PATCH 32/42] fix(forms): a deleted field leaves the calculation order Deleting a field took it off its pages and out of the field tree but left its entry in the form's /CO array. That reference kept the field dictionary, its value and its widgets reachable, so a full save after applying redactions over the field still wrote the redacted value. --- crates/forms/src/author.rs | 25 +++++++++++++++++ crates/redact/src/tests.rs | 1 + .../redact/src/tests/surviving_references.rs | 28 +++++++++++++++++++ 3 files changed, 54 insertions(+) create mode 100644 crates/redact/src/tests/surviving_references.rs diff --git a/crates/forms/src/author.rs b/crates/forms/src/author.rs index f5178a6879..544d0a7f8b 100644 --- a/crates/forms/src/author.rs +++ b/crates/forms/src/author.rs @@ -1280,6 +1280,31 @@ pub fn delete_field(doc: &mut Document, name: &str) -> Result<(), FormError> { } } } + // Out of the calculation order too: a `/CO` entry would keep the field, its value and its + // widgets reachable for a full save. + drop_from_calculation_order(doc, |r| r == f.obj || widgets.contains(&r)) +} + +/// Remove the `/CO` entries `gone` matches, whether the form and the array are held directly or +/// by reference. +fn drop_from_calculation_order(doc: &mut Document, gone: impl Fn(ObjRef) -> bool) -> Result<(), FormError> { + let Some(root) = doc.root() else { return Ok(()) }; + let Some(af_obj) = doc.get(root).as_dict().and_then(|d| d.get(b"AcroForm").cloned()) else { return Ok(()) }; + let Some(mut af) = doc.resolve(&af_obj).as_dict().cloned() else { return Ok(()) }; + let Some(co_obj) = af.get(b"CO").cloned() else { return Ok(()) }; + let co = doc.resolve(&co_obj).as_array().cloned().unwrap_or_default(); + let kept: Vec = co.iter().filter(|o| !o.as_ref().is_some_and(&gone)).cloned().collect(); + if kept.len() == co.len() { + return Ok(()); + } + match (co_obj.as_ref(), af_obj.as_ref()) { + (Some(r), _) if doc.get(r).as_array().is_some() => doc.set(r, Object::Array(kept)), + (_, Some(af_ref)) => doc.update_dict(af_ref, |d| d.set(b"CO".to_vec(), Object::Array(kept)))?, + (_, None) => { + af.set(b"CO".to_vec(), Object::Array(kept)); + doc.update_dict(root, |d| d.set(b"AcroForm".to_vec(), Object::Dict(af)))?; + } + } Ok(()) } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index a9902495df..52d80eb26e 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2462,6 +2462,7 @@ fn unused_descriptions_in_shared_or_inherited_resources_are_retired() { } mod property_retirement; +mod surviving_references; /// `[(FIRSTARR)] [(LASTARR)] TJ` is malformed (a surplus array operand), but the operator still /// draws only the one immediately before it (LASTARR, 7 glyphs, 35 pt wide from x = 10) -- not diff --git a/crates/redact/src/tests/surviving_references.rs b/crates/redact/src/tests/surviving_references.rs new file mode 100644 index 0000000000..cfe6d87bbe --- /dev/null +++ b/crates/redact/src/tests/surviving_references.rs @@ -0,0 +1,28 @@ +//! Redacted content must leave the file: no other reference may keep it reachable by a full save. + +use super::*; + +/// Whether `secret` survives a full save of `doc`, in the file's bytes or in a decoded stream. +fn full_save_holds(doc: &Document, secret: &[u8]) -> bool { + let bytes = write_full(doc, &SaveOptions::default()).unwrap(); + let found = |data: &[u8]| data.windows(secret.len()).any(|w| w == secret); + if found(&bytes) { + return true; + } + let saved = Document::open(Arc::new(bytes)).unwrap(); + saved.scan_objects().into_iter().any(|(_, o)| matches!(&*o, Object::Stream(s) if s.decoded().is_ok_and(|d| found(&d)))) +} + +#[test] +fn a_redacted_field_listed_in_the_calculation_order_leaves_the_saved_file() { + let mut doc = one_page(b"", "", vec![]); + pdfcraft_forms::add_field(&mut doc, 0, [10.0, 50.0, 100.0, 70.0], &pdfcraft_forms::NewField::Text { multiline: false }, Some("ssn")).unwrap(); + pdfcraft_forms::set_value(&mut doc, "ssn", &pdfcraft_forms::FieldValue::Text("SSN-078-05-1120".into())).unwrap(); + let field = pdfcraft_forms::fields(&doc).swap_remove(0).obj; + let root = doc.root().unwrap(); + let af = doc.get(root).as_dict().unwrap().reference(b"AcroForm").unwrap(); + doc.update_dict(af, |d| d.set(b"CO".to_vec(), Object::Array(vec![Object::Ref(field)]))).unwrap(); + mark(&mut doc, 0, &[[0.0, 40.0, 120.0, 80.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().fields, 1); + assert!(!full_save_holds(&doc, b"SSN-078-05-1120"), "the field's value is still reachable"); +} From 6ca4431b50a21760c7c9c9378b9d01a05acf140f Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:46:23 +0000 Subject: [PATCH 33/42] fix(redact): check every input content stream, whatever it calls itself Both checks for text left under applied redaction marks (redaction's own verify pass and the engine's renderer cross-check) skipped any page content stream whose dictionary said /PCMark /Redaction, so that the overlay's label would not count. An input file could put that label on its own content stream and keep it out of both checks. Applying redactions now records the overlay streams it adds (stamp returns the new stream), and only those are skipped. --- crates/edit/src/lib.rs | 15 +++++++++------ crates/engine/src/lib.rs | 15 ++++++++++----- crates/engine/src/tests.rs | 34 +++++++++++++++++++++++++++++++++- crates/redact/src/lib.rs | 16 ++++++++-------- 4 files changed, 60 insertions(+), 20 deletions(-) diff --git a/crates/edit/src/lib.rs b/crates/edit/src/lib.rs index d3db00fe30..bef515f981 100644 --- a/crates/edit/src/lib.rs +++ b/crates/edit/src/lib.rs @@ -17,7 +17,7 @@ #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] -use pdfcraft_cos::{Dict, Document, Object, Stream}; +use pdfcraft_cos::{Dict, Document, ObjRef, Object, Stream}; use pdfcraft_fonts::{helvetica_width, literal, win_ansi}; #[derive(Debug, thiserror::Error, PartialEq)] @@ -601,7 +601,7 @@ fn isolation(doc: &Document, list: &[Object]) -> Isolation { /// Put a mark's content on a page: behind (with the behind-marks at the start) or on top /// (appended, after wrapping the original content in q/Q). fn place(doc: &mut Document, page: &pdfcraft_model::Page, kind: MarkKind, content: Vec, behind: bool) -> Result<(), EditError> { - place_tagged(doc, page, kind.tag(), content, behind) + place_tagged(doc, page, kind.tag(), content, behind).map(drop) } /// Put content on a page, tagged `tag` (see [`place`]). On top, whatever graphics state the @@ -609,7 +609,8 @@ fn place(doc: &mut Document, page: &pdfcraft_model::Page, kind: MarkKind, conten /// content appended after an earlier PdfCraft wrapper by another tool (#3084): the stretch of /// content that leaves state set is wrapped in q/Q, once. Any known unfinished marked-content /// sequences are closed at the end too, so hidden optional content cannot hide the new mark. -fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: &str, content: Vec, behind: bool) -> Result<(), EditError> { +/// Returns the new content stream. +fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: &str, content: Vec, behind: bool) -> Result { let mut list = contents(doc, page)?; // The behind-marks at the start: backgrounds in the order added, then watermarks. let behind_kinds = [MarkKind::Background.tag(), MarkKind::Watermark.tag()]; @@ -625,7 +626,8 @@ fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: &str, cont Isolation::Unknown { unclosed, marked } => (behind_marks < list.len()).then_some((behind_marks, unclosed, marked)), } }; - let mark = Object::Ref(doc.add(Object::Stream(Stream::flate(tagged(tag), &content)))); + let mark_ref = doc.add(Object::Stream(Stream::flate(tagged(tag), &content))); + let mark = Object::Ref(mark_ref); if behind { // Right after the backgrounds already there, so a new background is painted over the // older ones (#3591) and a watermark over all of them. Not after the watermarks: on a @@ -646,7 +648,7 @@ fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: &str, cont list.push(mark); } doc.update_dict(page.obj, |d| d.set(b"Contents".to_vec(), Object::Array(list)))?; - Ok(()) + Ok(mark_ref) } /// Put content tagged `tag` first inside the q/Q wrapper, so it applies to the page's original @@ -664,7 +666,8 @@ fn place_inside_wrapper(doc: &mut Document, page: &pdfcraft_model::Page, tag: &s /// Draw `content` on top of a page (0-based) as permanent page content, with standard Helvetica /// available as `/PCHelv`. The stream is tagged `tag` (not a mark kind, so Remove never takes /// it away); the original content is wrapped in q/Q first. Used for applied redaction boxes. -pub fn stamp(doc: &mut Document, page: usize, tag: &str, content: Vec) -> Result<(), EditError> { +/// Returns the new content stream. +pub fn stamp(doc: &mut Document, page: usize, tag: &str, content: Vec) -> Result { let all = page_list(doc); check(&[page], all.len())?; add_resources(doc, &all[page], None, Some(&content))?; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 817eb79a96..32d3a1f15b 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -989,12 +989,13 @@ fn recovered_render_password(cos: &pdfcraft_cos::Document) -> Option { /// that it reads differently), a mark drawn over the text as displayed lies over glyphs the /// interpreter believes are elsewhere: they stay in the file and nothing notices. The renderer is /// what the user saw when placing the marks, so it is asked. `after` is written as a save would -/// write it, without the marks' own overlay (the box and its label are drawn on purpose), its +/// write it, without the marks' own `overlays` (the box and its label are drawn on purpose), its /// text is extracted, and a glyph still mostly inside a mark fails the edit: the caller keeps /// the document as it was. fn shown_under_redactions( after: &pdfcraft_cos::Document, marks: &[pdfcraft_redact::Mark], + overlays: &[pdfcraft_cos::ObjRef], pages: &[pdfcraft_render::PageInfo], password: Option<&str>, ) -> Result<(), EditError> { @@ -1011,8 +1012,7 @@ fn shown_under_redactions( for &pi in &marked { let Some(page) = model.get(pi) else { continue }; let Some(Object::Array(list)) = page.dict.get(b"Contents").map(|c| (*shown.resolve(c)).clone()) else { continue }; - let kept: Vec = - list.into_iter().filter(|o| shown.resolve(o).as_dict().and_then(|d| d.name(b"PCMark")) != Some(b"Redaction")).collect(); + let kept: Vec = list.into_iter().filter(|o| !o.as_ref().is_some_and(|r| overlays.contains(&r))).collect(); shown.update_dict(page.obj, |d| d.set(b"Contents".to_vec(), Object::Array(kept))).map_err(|e| EditError::Write(e.to_string()))?; } let bytes = write_incremental(&shown, &SaveOptions::default()).map_err(|e| EditError::Write(e.to_string()))?; @@ -2356,6 +2356,9 @@ struct EditCtx { /// Fields the edit renamed to keep imported names separate (old → new), for the repair /// log and the undo label. renamed_fields: Vec<(String, String)>, + /// The overlay streams the edit's applied redactions added, which alone the renderer's check + /// leaves out. + redaction_overlays: Vec, } impl EditCtx { @@ -2382,6 +2385,7 @@ impl EditCtx { seed, count: 0, renamed_fields: Vec::new(), + redaction_overlays: Vec::new(), } } @@ -2736,7 +2740,7 @@ fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut EditCtx) -> pdfcraft_edit::update_content(doc, *page, *index, &AddedContent::Image(pdfcraft_edit::AddedImage { image, ..old }))?; } Edit::ApplyRedactions { pages } => { - pdfcraft_redact::apply(doc, pages.as_deref())?; + cx.redaction_overlays = pdfcraft_redact::apply(doc, pages.as_deref())?.overlays; } Edit::ClearRedactions => { pdfcraft_redact::clear_marks(doc, None)?; @@ -3798,6 +3802,7 @@ impl Session { cx.js = Some(js::JsRunner::new(&next, &name)); } cx.renamed_fields.clear(); + cx.redaction_overlays.clear(); // `next` is a copy: if the edit fails or crashes, the document is unchanged. guard(|| run_edit(&mut next, &edit, &mut cx)) .unwrap_or_else(|m| Err(EditError::Invalid(format!("{} failed unexpectedly ({m}); the document was not changed", edit.label()))))?; @@ -3806,7 +3811,7 @@ impl Session { if let Edit::ApplyRedactions { pages } = &edit { let marks: Vec = pdfcraft_redact::marks(&editor.cos).into_iter().filter(|m| pages.as_ref().is_none_or(|p| p.contains(&m.page))).collect(); - guard(|| shown_under_redactions(&next, &marks, &doc.info.pages, doc.password.as_deref())) + guard(|| shown_under_redactions(&next, &marks, &cx.redaction_overlays, &doc.info.pages, doc.password.as_deref())) .unwrap_or_else(|m| Err(EditError::Invalid(format!("{} failed unexpectedly ({m}); the document was not changed", edit.label()))))?; } // Renamed fields are said, not slipped in: a repair-log note (with the caveat that diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 3d347e6492..958204bc53 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2253,12 +2253,17 @@ fn setting_the_calculation_order_recalculates_and_undoes() { /// One 612×300 page with the content stream `body`. Object 4 is Helvetica; `resources` is what /// the page's /Resources holds. fn one_line_page(resources: &str, body: &str) -> Vec { + one_line_page_with(resources, "", body) +} + +/// [`one_line_page`] with `stream_entries` added to the content stream's dictionary. +fn one_line_page_with(resources: &str, stream_entries: &str, body: &str) -> Vec { let objs = [ "<< /Type /Catalog /Pages 2 0 R >>".to_string(), "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 300] >>".to_string(), format!("<< /Type /Page /Parent 2 0 R /Resources << {resources} >> /Contents 5 0 R >>"), "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".to_string(), - format!("<< /Length {} >>\nstream\n{body}\nendstream", body.len()), + format!("<< /Length {} {stream_entries} >>\nstream\n{body}\nendstream", body.len()), ]; let mut out = b"%PDF-1.7\n".to_vec(); let mut offsets = Vec::new(); @@ -2316,6 +2321,33 @@ fn redaction_is_refused_when_the_page_still_shows_text_under_a_mark() { assert!(shown.starts_with("Public words then") && shown.ends_with("and more") && !shown.contains("SECRET42"), "{shown}"); } +/// Only the overlay a redaction adds is left out of the renderer's check. A page whose own content +/// stream calls itself a redaction overlay (`/PCMark /Redaction`) is checked like any other. +#[test] +fn a_content_stream_labelled_as_a_redaction_overlay_is_still_checked() { + let line = "Public words then SECRET42 and more"; + let mut s = Session::new().with_clock(|| 1_700_000_000); + let forged = one_line_page_with( + "/ExtGState << /GS1 << /Type /ExtGState /Font [4 0 R 16] >> >>", + "/PCMark /Redaction", + &format!("BT /GS1 gs 20 150 Td ({line}) Tj ET"), + ); + let id = s.open("forged.pdf", None, Arc::new(forged), None).unwrap(); + mark_shown_text(&mut s, id, "SECRET42"); + let applied = s.apply(id, Edit::ApplyRedactions { pages: None }); + if applied.is_ok() { + assert!(!page_texts(&s, id).concat().contains("SECRET42"), "the page still shows the redacted word"); + let saved = s.save_bytes(id).unwrap(); + let held = |d: &[u8]| d.windows(8).any(|w| w == b"SECRET42"); + assert!(!held(&saved), "the saved file holds the redacted word"); + let reopened = pdfcraft_cos::Document::open(saved).unwrap(); + assert!( + !reopened.scan_objects().any(|(_, o)| matches!(&*o, pdfcraft_cos::Object::Stream(st) if st.decoded().is_ok_and(|d| held(&d)))), + "a saved stream holds the redacted word" + ); + } +} + #[test] fn redaction_marks_apply_for_good_and_undo() { let (mut s, id) = session_with(2); diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 968773b91d..97d5bcd9fe 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -143,6 +143,10 @@ pub struct Report { /// The form was an AcroForm+XFA form and its `/XFA` packets (which still held redacted field /// values, and from which a relayout could rebuild them) were removed. pub xfa_removed: bool, + /// The content streams this apply added to draw the marks' boxes and labels. They are the + /// only streams a check for text left under the marks may skip: an input stream labelled + /// like an overlay (`/PCMark /Redaction`) is checked like any other. + pub overlays: Vec, } fn rect_of(doc: &Document, o: Option<&Object>) -> Option<[f64; 4]> { @@ -576,7 +580,7 @@ pub fn apply(doc: &mut Document, pages: Option<&[usize]>) -> Result) -> Result = page_marks.iter().flat_map(|m| m.rects.iter().copied()).collect(); let (list, data) = page_streams(doc, &page.dict, pi)?; - // The overlay stream (last) draws text of its own: the overlay label is allowed. - let original: Vec> = list - .iter() - .zip(data) - .filter(|(o, _)| doc.resolve(o).as_dict().and_then(|d| d.name(b"PCMark")) != Some(b"Redaction")) - .map(|(_, d)| d) - .collect(); + // The overlay stream draws text of its own: the overlay label is allowed. + let original: Vec> = + list.iter().zip(data).filter(|(o, _)| !o.as_ref().is_some_and(|r| report.overlays.contains(&r))).map(|(_, d)| d).collect(); let resources = page.dict.get(b"Resources").and_then(|r| doc.resolve(r).as_dict().cloned()).unwrap_or_default(); let mut scratch = Report::default(); let mut scope = Scope::new(&rects, Mode::Verify, &mut scratch); From 22aa6d5f9afb7339a19fa55b792b96f7c7910c37 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:50:59 +0000 Subject: [PATCH 34/42] fix(redact): place glyphs with the inherited and ExtGState-set text state The redaction interpreter started every form XObject with an empty text state (no font, size 0, no spacing) although a form inherits its caller's graphics state, and it ignored the /Font entry of an ExtGState set by gs. Text shown that way was placed at the wrong positions, so removal and the verify pass could both miss glyphs under a mark. Forms now start from the caller's state, and gs sets the font and size as Tf does. The engine's renderer cross-check test used the gs case as its example of interpreter/renderer disagreement; it now runs the check directly on an unredacted page, which also covers an input stream labelled as a redaction overlay. --- crates/engine/src/lib.rs | 2 +- crates/engine/src/tests.rs | 78 +++++++++++++-------------- crates/redact/src/interp.rs | 51 ++++++++++++++++-- crates/redact/src/tests.rs | 1 + crates/redact/src/tests/text_state.rs | 27 ++++++++++ 5 files changed, 111 insertions(+), 48 deletions(-) create mode 100644 crates/redact/src/tests/text_state.rs diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 32d3a1f15b..f257865d3a 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -985,7 +985,7 @@ fn recovered_render_password(cos: &pdfcraft_cos::Document) -> Option { /// /// Redaction decides what lies under a mark from the positions its own interpreter computes, and /// its verify pass reuses them. Where they differ from the renderer's (a standard font without -/// /Widths, whose advances the interpreter approximates; a CMap, or a font set outside `Tf`, +/// /Widths, whose advances the interpreter approximates; a CMap /// that it reads differently), a mark drawn over the text as displayed lies over glyphs the /// interpreter believes are elsewhere: they stay in the file and nothing notices. The renderer is /// what the user saw when placing the marks, so it is asked. `after` is written as a save would diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 958204bc53..8937cf6780 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2294,58 +2294,52 @@ fn mark_shown_text(s: &mut Session, id: DocId, needle: &str) { s.apply(id, mark).unwrap(); } -/// Redaction removes text by the positions its own interpreter works out. Here the font and size -/// come from an ExtGState /Font entry (`gs`), which the renderer honours and the interpreter does -/// not model, so it places no glyph under the mark the user drew over the word as displayed: it -/// used to report success and leave the word in the file. The renderer is asked now, and the edit -/// is refused. +/// Redaction removes text by the positions its own interpreter works out, and the renderer has +/// the last word: where the two disagree, a page that still shows a glyph mostly under a mark +/// fails the edit. The check is run here on a page nothing was removed from, as if the +/// interpreter had placed the word elsewhere. Only the overlay streams an apply added are left +/// out: a content stream of the input labelled like one (`/PCMark /Redaction`) is checked too. #[test] -fn redaction_is_refused_when_the_page_still_shows_text_under_a_mark() { +fn the_renderer_check_refuses_text_still_shown_under_a_mark() { + let mut s = Session::new().with_clock(|| 1_700_000_000); + let page = one_line_page_with("/Font << /F1 4 0 R >>", "/PCMark /Redaction", "BT /F1 16 Tf 20 150 Td (Public words then SECRET42) Tj ET"); + let id = s.open("shown.pdf", None, Arc::new(page), None).unwrap(); + mark_shown_text(&mut s, id, "SECRET42"); + let doc = s.get(id).unwrap(); + let mut cos = doc.editor.as_ref().unwrap().cos.clone(); + let marks = pdfcraft_redact::marks(&cos); + let refused = |cos: &pdfcraft_cos::Document| { + let shown = shown_under_redactions(cos, &marks, &[], &doc.info.pages, None); + matches!(shown, Err(EditError::Redact(pdfcraft_redact::RedactError::Residue(n))) if n > 0) + }; + assert!(refused(&cos)); + // The page's contents as an array, as they are once an apply has added its overlay. + let page = pdfcraft_model::pages(&cos).swap_remove(0); + let contents = page.dict.get(b"Contents").cloned().unwrap(); + cos.update_dict(page.obj, |d| d.set(b"Contents".to_vec(), pdfcraft_cos::Object::Array(vec![contents]))).unwrap(); + assert!(refused(&cos), "the labelled input stream was not checked"); +} + +/// A word whose font and size an ExtGState /Font entry (`gs`) sets is placed where the renderer +/// draws it, so a mark drawn over it as displayed takes it out of the file. +#[test] +fn a_word_in_a_font_set_by_an_extgstate_is_redacted() { let line = "Public words then SECRET42 and more"; let mut s = Session::new().with_clock(|| 1_700_000_000); let by_gs = one_line_page("/ExtGState << /GS1 << /Type /ExtGState /Font [4 0 R 16] >> >>", &format!("BT /GS1 gs 20 150 Td ({line}) Tj ET")); let id = s.open("gs.pdf", None, Arc::new(by_gs), None).unwrap(); mark_shown_text(&mut s, id, "SECRET42"); - let refused = s.apply(id, Edit::ApplyRedactions { pages: None }); - assert!(matches!(refused, Err(EditError::Redact(pdfcraft_redact::RedactError::Residue(n))) if n > 0), "{refused:?}"); - // Nothing was changed: the mark is still pending and the page still shows the line. - assert_eq!(s.get(id).unwrap().redaction_marks(), 1); - assert_eq!(page_texts(&s, id), [line]); - - // The same line with its font set by `Tf`: the mark is applied and the word is gone. - let by_tf = one_line_page("/Font << /F1 4 0 R >>", &format!("BT /F1 16 Tf 20 150 Td ({line}) Tj ET")); - let id = s.open("tf.pdf", None, Arc::new(by_tf), None).unwrap(); - mark_shown_text(&mut s, id, "SECRET42"); s.apply(id, Edit::ApplyRedactions { pages: None }).unwrap(); let shown = page_texts(&s, id).swap_remove(0); assert!(shown.starts_with("Public words then") && shown.ends_with("and more") && !shown.contains("SECRET42"), "{shown}"); -} - -/// Only the overlay a redaction adds is left out of the renderer's check. A page whose own content -/// stream calls itself a redaction overlay (`/PCMark /Redaction`) is checked like any other. -#[test] -fn a_content_stream_labelled_as_a_redaction_overlay_is_still_checked() { - let line = "Public words then SECRET42 and more"; - let mut s = Session::new().with_clock(|| 1_700_000_000); - let forged = one_line_page_with( - "/ExtGState << /GS1 << /Type /ExtGState /Font [4 0 R 16] >> >>", - "/PCMark /Redaction", - &format!("BT /GS1 gs 20 150 Td ({line}) Tj ET"), + let saved = s.save_bytes(id).unwrap(); + let held = |d: &[u8]| d.windows(8).any(|w| w == b"SECRET42"); + assert!(!held(&saved), "the saved file holds the redacted word"); + let reopened = pdfcraft_cos::Document::open(saved).unwrap(); + assert!( + !reopened.scan_objects().any(|(_, o)| matches!(&*o, pdfcraft_cos::Object::Stream(st) if st.decoded().is_ok_and(|d| held(&d)))), + "a saved stream holds the redacted word" ); - let id = s.open("forged.pdf", None, Arc::new(forged), None).unwrap(); - mark_shown_text(&mut s, id, "SECRET42"); - let applied = s.apply(id, Edit::ApplyRedactions { pages: None }); - if applied.is_ok() { - assert!(!page_texts(&s, id).concat().contains("SECRET42"), "the page still shows the redacted word"); - let saved = s.save_bytes(id).unwrap(); - let held = |d: &[u8]| d.windows(8).any(|w| w == b"SECRET42"); - assert!(!held(&saved), "the saved file holds the redacted word"); - let reopened = pdfcraft_cos::Document::open(saved).unwrap(); - assert!( - !reopened.scan_objects().any(|(_, o)| matches!(&*o, pdfcraft_cos::Object::Stream(st) if st.decoded().is_ok_and(|d| held(&d)))), - "a saved stream holds the redacted word" - ); - } } #[test] diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index a608568f38..14dedbb344 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -690,6 +690,34 @@ fn prune_form_resources(doc: &Document, res: &mut Dict, content: &[Vec]) -> Ok(()) } +/// The font, writing direction and size an ExtGState's `/Font [font size]` entry sets, as `Tf` +/// would (ISO 32000-2 §8.4.5); `None` when `name` sets no font. The metrics are cached with the +/// fonts `Tf` names, under a key no resource name can take (a name cannot hold a null byte). +fn extgstate_font( + doc: &Document, + scope: &mut Scope<'_>, + ext_gstates: &Dict, + name: &[u8], + fallback: &Rc, +) -> Option<(Rc, bool, f64)> { + let state = doc.resolve(ext_gstates.get(name)?).as_dict().cloned()?; + let entry = doc.resolve(state.get(b"Font")?).as_array().cloned()?; + let [font, size] = entry.as_slice() else { return None }; + let size = doc.resolve(size).as_f64()?; + let font_dict = doc.resolve(font).as_dict().cloned(); + let vertical = font_dict.as_ref().is_some_and(|d| is_vertical(doc, d)); + let key = [b"\0".as_slice(), name].concat(); + let metrics = match scope.fonts.get(&key) { + Some(m) => m.clone(), + None => { + let m = font_dict.as_ref().map(|d| Rc::new(Metrics::from_dict(doc, d))).unwrap_or_else(|| fallback.clone()); + scope.fonts.insert(key, m.clone()); + m + } + }; + Some((metrics, vertical, size)) +} + /// One glyph of a shown string. struct Glyph { bytes: std::ops::Range, @@ -700,11 +728,9 @@ struct Glyph { /// Process a scope's content streams (state flows from one stream to the next). pub(crate) fn process(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec], resources: &Dict, ctm: Matrix) -> Output { - let mut out = Output::default(); - let fallback = Rc::new(Metrics::fallback()); - let mut gs = Gs { + let gs = Gs { ctm, - font: fallback.clone(), + font: Rc::new(Metrics::fallback()), vertical: false, size: 0.0, char_spacing: 0.0, @@ -714,9 +740,19 @@ pub(crate) fn process(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec< leading: 0.0, rise: 0.0, }; + process_from(doc, scope, streams, resources, gs) +} + +/// [`process`] from graphics state `gs`: a form XObject starts with its caller's, text state +/// included (ISO 32000-2 §8.10.1), so text it shows without setting a font of its own is placed +/// with the caller's font, size and spacing, as the renderer places it. +fn process_from(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec], resources: &Dict, mut gs: Gs) -> Output { + let mut out = Output::default(); + let fallback = Rc::new(Metrics::fallback()); let mut stack: Vec = Vec::new(); let (mut tm, mut tlm) = (Matrix::IDENTITY, Matrix::IDENTITY); let fonts_res = res_dict(doc, resources, b"Font"); + let ext_gstates = res_dict(doc, resources, b"ExtGState"); let mut xobjects = res_dict(doc, resources, b"XObject"); // The current path: its operators, its bounding box in user space, and whether it clips. let mut path: Vec = Vec::new(); @@ -865,6 +901,11 @@ pub(crate) fn process(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec< }; } } + b"gs" => { + if let Some((font, vertical, size)) = op.name(0).and_then(|n| extgstate_font(doc, scope, &ext_gstates, n, &fallback)) { + (gs.font, gs.vertical, gs.size) = (font, vertical, size); + } + } b"Tc" => gs.char_spacing = op.num(0).unwrap_or(0.0), b"Tw" => gs.word_spacing = op.num(0).unwrap_or(0.0), b"Tz" => gs.scale = op.num(0).unwrap_or(100.0) / 100.0, @@ -1223,7 +1264,7 @@ fn xobject( let res = own.clone().unwrap_or_else(|| resources.clone()); let saved_fonts = std::mem::take(&mut scope.fonts); scope.depth += 1; - let inner = process(doc, scope, std::slice::from_ref(&data), &res, fctm); + let inner = process_from(doc, scope, std::slice::from_ref(&data), &res, Gs { ctm: fctm, ..gs.clone() }); scope.depth -= 1; scope.fonts = saved_fonts; out.residue += inner.residue; diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 52d80eb26e..882cc50aeb 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2463,6 +2463,7 @@ fn unused_descriptions_in_shared_or_inherited_resources_are_retired() { mod property_retirement; mod surviving_references; +mod text_state; /// `[(FIRSTARR)] [(LASTARR)] TJ` is malformed (a surplus array operand), but the operator still /// draws only the one immediately before it (LASTARR, 7 glyphs, 35 pt wide from x = 10) -- not diff --git a/crates/redact/src/tests/text_state.rs b/crates/redact/src/tests/text_state.rs new file mode 100644 index 0000000000..4e888f262c --- /dev/null +++ b/crates/redact/src/tests/text_state.rs @@ -0,0 +1,27 @@ +//! Glyphs are placed with the whole text state, wherever it was set. + +use super::*; + +const SECRET_AT_10_100: [f64; 4] = [5.0, 95.0, 45.0, 110.0]; + +#[test] +fn a_font_set_by_an_extgstate_places_its_glyphs() { + let mut doc = one_page(b"BT /GS1 gs 10 100 Td (SECRET) Tj ET", "/ExtGState << /GS1 << /Type /ExtGState /Font [5 0 R 10] >> >>", vec![]); + // "ET" spans x 30–40. + assert_eq!(under(&mut doc, 0, &[[30.0, 95.0, 40.0, 110.0]]), 2); + mark(&mut doc, 0, &[SECRET_AT_10_100], ""); + assert_eq!(apply(&mut doc, None).unwrap().glyphs, 6); + assert!(!content(&doc, 0).contains("SECRET")); +} + +#[test] +fn a_form_inherits_the_text_state_of_the_page_that_draws_it() { + // The form shows text with the font, size and spacing the page set before `Do`. + let form = stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300]", b"BT 10 100 Td (SECRET) Tj ET"); + let mut doc = one_page(b"/F1 10 Tf 1 Tc /Fm Do", "/XObject << /Fm 7 0 R >>", vec![form]); + // Glyphs 5 pt wide, 1 pt apart: the last one spans x 40–45. + assert_eq!(under(&mut doc, 0, &[[40.0, 95.0, 46.0, 110.0]]), 1); + mark(&mut doc, 0, &[SECRET_AT_10_100], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!((r.glyphs, r.forms_rewritten), (6, 1), "{r:?}"); +} From be64a60c876402f43ffef893f1fd87ac77786981 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:52:12 +0000 Subject: [PATCH 35/42] fix(redact): unlink removed XObjects from surviving forms' resources After applying redactions, removed images and forms were unlinked from page-tree resource dictionaries but not from the resources of forms the page still draws. A form sharing the page's original resource dictionary (a common producer layout) kept the removed image reachable, so a full save wrote it. The walk over surviving forms now drops XObject entries for removed objects that the form's own content does not draw, and fails closed when that cannot be determined. --- crates/redact/src/interp.rs | 34 +++++++++++++--- .../redact/src/tests/surviving_references.rs | 40 +++++++++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index 14dedbb344..fca3a1da88 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -529,8 +529,9 @@ pub(crate) fn unlink_doomed(doc: &mut Document, doomed: &[ObjRef], retired_prope } } // A surviving form can own the same old resource dictionary as a rewritten page. Its - // unused descriptions remain reachable too, even after every page holder is cleaned. - unlink_form_properties(doc, &holder_nodes, retired_properties)?; + // unused descriptions and doomed XObjects remain reachable too, even after every page + // holder is cleaned. + unlink_from_forms(doc, &holder_nodes, &doomed, retired_properties)?; Ok(()) } @@ -567,8 +568,11 @@ fn prune_retired_properties(doc: &Document, res: &mut Dict, names: &[Vec], c Ok(pruned) } -fn unlink_form_properties(doc: &mut Document, holders: &[ObjRef], retired: &[Object]) -> Result<(), CosError> { - if retired.is_empty() { +/// Unlink retired properties and `doomed` XObjects from the resources of the forms reachable from +/// the page-tree `holders`, each unless the form's own content (or a form inheriting its +/// resources) still uses the name. Fails closed when that cannot be determined. +fn unlink_from_forms(doc: &mut Document, holders: &[ObjRef], doomed: &HashSet, retired: &[Object]) -> Result<(), CosError> { + if retired.is_empty() && doomed.is_empty() { return Ok(()); } let mut pending: Vec = holders @@ -593,7 +597,27 @@ fn unlink_form_properties(doc: &mut Document, holders: &[ObjRef], retired: &[Obj let Some(mut res) = form.dict.get(b"Resources").and_then(|r| doc.resolve(r).as_dict().cloned()) else { continue }; let names: Vec> = res_dict(doc, &res, b"Properties").iter().filter(|(_, value)| retired.contains(value)).map(|(name, _)| name.to_vec()).collect(); - if !names.is_empty() && prune_retired_properties(doc, &mut res, &names, &[vec![form.decoded()?]])? { + let mut xo = res_dict(doc, &res, b"XObject"); + let doomed_names: Vec> = + xo.iter().filter(|(_, v)| v.as_ref().is_some_and(|r| doomed.contains(&r))).map(|(n, _)| n.to_vec()).collect(); + if names.is_empty() && doomed_names.is_empty() { + pending.push(res); + continue; + } + let data = form.decoded()?; + let mut pruned = false; + for name in doomed_names { + match content_draws(doc, std::slice::from_ref(&data), &xo, &name, 0, &mut HashSet::new()) { + Some(true) => {} + Some(false) => pruned |= xo.remove(&name).is_some(), + None => return Err(CosError::Syntax { offset: 0, detail: "redaction could not fully inspect shared form resources".into() }), + } + } + if pruned { + res.set(b"XObject".to_vec(), Object::Dict(xo)); + } + pruned |= prune_retired_properties(doc, &mut res, &names, &[vec![data]])?; + if pruned { doc.update_dict(reference, |d| d.set(b"Resources".to_vec(), Object::Dict(res.clone())))?; } pending.push(res); diff --git a/crates/redact/src/tests/surviving_references.rs b/crates/redact/src/tests/surviving_references.rs index cfe6d87bbe..2c8bdb09c2 100644 --- a/crates/redact/src/tests/surviving_references.rs +++ b/crates/redact/src/tests/surviving_references.rs @@ -26,3 +26,43 @@ fn a_redacted_field_listed_in_the_calculation_order_leaves_the_saved_file() { assert_eq!(apply(&mut doc, None).unwrap().fields, 1); assert!(!full_save_holds(&doc, b"SSN-078-05-1120"), "the field's value is still reachable"); } + +/// A surviving form shares the page's resource dictionary, which lists the image the mark +/// removes; the form never draws it. +#[test] +fn a_redacted_image_does_not_survive_in_a_surviving_forms_resources() { + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources 7 0 R >>".to_vec(), + stream("", b"q 100 0 0 100 0 0 cm /Im Do Q /Fm Do"), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + b"<< /Font << /F1 5 0 R >> /XObject << /Im 8 0 R /Fm 9 0 R >> >>".to_vec(), + stream("/Type /XObject /Subtype /Image /Width 11 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8", b"IMAGESECRET"), + stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources 7 0 R", b"BT /F1 10 Tf 10 200 Td (Public) Tj ET"), + ]); + mark(&mut doc, 0, &[[0.0, 0.0, 120.0, 120.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().images_removed, 1); + assert!(content(&doc, 0).contains("Do"), "the form is still drawn"); + assert!(!full_save_holds(&doc, b"IMAGESECRET"), "the removed image is still reachable"); +} + +/// The same form drawing the image itself keeps it: the page still shows it there. +#[test] +fn an_image_a_surviving_form_draws_is_kept() { + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources 7 0 R >>".to_vec(), + stream("", b"q 100 0 0 100 0 0 cm /Im Do Q /Fm Do"), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + b"<< /Font << /F1 5 0 R >> /XObject << /Im 8 0 R /Fm 9 0 R >> >>".to_vec(), + stream("/Type /XObject /Subtype /Image /Width 11 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8", b"IMAGESECRET"), + stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources 7 0 R", b"q 50 0 0 50 200 200 cm /Im Do Q"), + ]); + mark(&mut doc, 0, &[[0.0, 0.0, 120.0, 120.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().images_removed, 1); + assert!(full_save_holds(&doc, b"IMAGESECRET"), "the image the form still draws outside the mark"); +} From cccd435a5308bd628976e0f0d9923f1eebf90e36 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:53:24 +0000 Subject: [PATCH 36/42] fix(redact): bound image decoding by the declared raster size Clearing the pixels of a partly covered image decoded its stream with the global 1 GiB allowance and only then cut it to the raster length its Width, Height and bit depth declare. A small Flate stream could make every such image allocate and inflate far more than it can use. Decoding now stops at twice the raster plus a byte per row (filter-chain and predictor overhead) and a little slack; an image past that is removed whole, as an undecodable one already is. --- crates/redact/src/image.rs | 6 +++++- crates/redact/src/tests.rs | 1 + crates/redact/src/tests/limits.rs | 25 +++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) create mode 100644 crates/redact/src/tests/limits.rs diff --git a/crates/redact/src/image.rs b/crates/redact/src/image.rs index 9a19ba2f0b..601700f257 100644 --- a/crates/redact/src/image.rs +++ b/crates/redact/src/image.rs @@ -77,7 +77,11 @@ fn decode_and_clear(doc: &Document, s: &Stream, ctm: &Matrix, rects: &[[f64; 4]] let bytes = row.checked_mul(h)?; let row_bits = row.checked_mul(8)?; bytes.checked_mul(8)?; - let mut data = s.decoded().ok()?; + // Decode no further than the raster can need: a filter stage may legitimately produce up to + // twice it (ASCIIHex text under Flate) plus a PNG predictor's byte per row. Data past that + // can only be a decompression bomb; the image is then removed whole. + let allowance = bytes.checked_add(h)?.checked_mul(2)?.checked_add(4096)?; + let mut data = s.decoded_within(allowance).ok()?; if data.len() < bytes { return None; } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 882cc50aeb..94081f9e35 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2461,6 +2461,7 @@ fn unused_descriptions_in_shared_or_inherited_resources_are_retired() { } } +mod limits; mod property_retirement; mod surviving_references; mod text_state; diff --git a/crates/redact/src/tests/limits.rs b/crates/redact/src/tests/limits.rs new file mode 100644 index 0000000000..6bee96aed4 --- /dev/null +++ b/crates/redact/src/tests/limits.rs @@ -0,0 +1,25 @@ +//! Work and memory stay bounded by what the page declares, whatever the file holds. + +use super::*; + +/// A half-covered 4×1 grey image whose Flate data decodes to `decoded`. +fn half_covered_image(decoded: &[u8]) -> Report { + let dict = "/Type /XObject /Subtype /Image /Width 4 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8"; + let mut doc = one_page(b"q 40 0 0 10 100 100 cm /Im Do Q", "/XObject << /Im 7 0 R >>", vec![stream(dict, b"")]); + let Object::Stream(s) = &*doc.get(ObjRef::new(7, 0)) else { panic!() }; + doc.set(ObjRef::new(7, 0), Object::Stream(Stream::flate(s.dict.clone(), decoded))); + mark(&mut doc, 0, &[[95.0, 95.0, 120.0, 115.0]], ""); + apply(&mut doc, None).unwrap() +} + +#[test] +fn an_image_decoding_far_past_its_raster_is_removed_without_decoding_it_all() { + let r = half_covered_image(&[255; 4]); + assert_eq!((r.images_removed, r.images_cleared), (0, 1), "{r:?}"); + // 4 bytes of raster, then 4 MiB the image never uses: decoding stops at a bound derived from + // the raster, and the image goes whole. + let mut bomb = vec![255; 4]; + bomb.resize(4 << 20, 0); + let r = half_covered_image(&bomb); + assert_eq!((r.images_removed, r.images_cleared), (1, 0), "{r:?}"); +} From 2cdb39503ef303b5cc66703abda470d044896690 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:55:22 +0000 Subject: [PATCH 37/42] fix(redact): compare marked content without a copy per enclosing MCID Finding which marked content a redaction changed copied every operator into the buffer of each MCID enclosing it, so content nested n deep was held n times: a modest stream of nested BDCs could exhaust memory. Each operator inside marked content is now serialized once, and each MCID keeps the ranges its sequences span. The bytes compared are capped; MCIDs past the cap count as changed, so their descriptions are cleared rather than kept. --- crates/redact/src/tags.rs | 88 +++++++++++++++++++++++++------ crates/redact/src/tests/limits.rs | 30 +++++++++++ 2 files changed, 101 insertions(+), 17 deletions(-) diff --git a/crates/redact/src/tags.rs b/crates/redact/src/tags.rs index ab5735e065..8548810600 100644 --- a/crates/redact/src/tags.rs +++ b/crates/redact/src/tags.rs @@ -6,15 +6,49 @@ //! references to marked content that is now empty are dropped from the element. use std::collections::{HashMap, HashSet}; +use std::ops::Range; use pdfcraft_cos::{Dict, Document, ObjRef, Object}; use crate::RedactError; -/// Each MCID's operators (serialized), and whether anything is still drawn inside it. -fn by_mcid(doc: &Document, resources: &Dict, data: &[u8]) -> HashMap, bool)> { - let mut out: HashMap, bool)> = HashMap::new(); - let mut stack: Vec> = Vec::new(); +/// The marked content of one content stream. Every operator inside some MCID is serialized once +/// into `ops`, and each MCID holds the ranges of `ops` its sequences span (marked-content +/// operators are not written, so a sequence is one range however deeply it nests), with whether +/// anything is drawn inside it. Copying the operators into every enclosing MCID instead would +/// hold `depth` copies of nested content. +#[derive(Default)] +struct Marked { + ops: Vec, + ids: HashMap>, bool)>, +} + +impl Marked { + /// The bytes of `ranges`, which are ranges of `ops`. + fn bytes<'a>(&'a self, ranges: &'a [Range]) -> impl Iterator + 'a { + ranges.iter().flat_map(|r| self.ops.get(r.clone()).unwrap_or_default()) + } +} + +/// Most bytes [`touched`] compares for one stream. Nested sequences share their operators, so the +/// comparisons can add up to the content's size times its nesting depth; past this, the MCIDs +/// left are taken as changed (their descriptions go) rather than compared. +const MAX_COMPARED: usize = 1 << 28; + +fn by_mcid(doc: &Document, resources: &Dict, data: &[u8]) -> Marked { + let mut out = Marked::default(); + // Open sequences: the MCID with where its content starts in `ops` and the drawing operators + // written before it, or `None` for a sequence without one. + let mut stack: Vec> = Vec::new(); + let (mut open_ids, mut drawn) = (0usize, 0usize); + let close = |out: &mut Marked, open: Option<(i64, usize, usize)>, drawn: usize| { + if let Some((id, start, drawn_before)) = open { + let end = out.ops.len(); + let e = out.ids.entry(id).or_default(); + e.0.push(start..end); + e.1 |= drawn > drawn_before; + } + }; for op in pdfcraft_content::parse(data).ops { match op.op.as_slice() { b"BDC" => { @@ -28,9 +62,10 @@ fn by_mcid(doc: &Document, resources: &Dict, data: &[u8]) -> HashMap None, }; if let Some(id) = id { - out.entry(id).or_default(); + out.ids.entry(id).or_default(); + open_ids += 1; } - stack.push(id); + stack.push(id.map(|id| (id, out.ops.len(), drawn))); continue; } b"BMC" => { @@ -38,11 +73,17 @@ fn by_mcid(doc: &Document, resources: &Dict, data: &[u8]) -> HashMap { - stack.pop(); + if let Some(open) = stack.pop() { + open_ids = open_ids.saturating_sub(usize::from(open.is_some())); + close(&mut out, open, drawn); + } continue; } _ => {} } + if open_ids == 0 { + continue; + } let draws = match op.op.as_slice() { b"Tj" | b"'" | b"\"" => op.operands.last().and_then(Object::as_string).is_some_and(|s| !s.bytes.is_empty()), b"TJ" => { @@ -51,13 +92,12 @@ fn by_mcid(doc: &Document, resources: &Dict, data: &[u8]) -> HashMap true, _ => false, }; - let mut bytes = Vec::new(); - pdfcraft_content::write_op(&op, &mut bytes); - for id in stack.iter().flatten() { - let e = out.entry(*id).or_default(); - e.0.extend_from_slice(&bytes); - e.1 |= draws; - } + pdfcraft_content::write_op(&op, &mut out.ops); + drawn += usize::from(draws); + } + // Sequences the stream leaves open run to its end. + while let Some(open) = stack.pop() { + close(&mut out, open, drawn); } out } @@ -67,10 +107,17 @@ pub(crate) fn touched(doc: &Document, resources: &Dict, before: &[u8], after: &[ let (b, a) = (by_mcid(doc, resources, before), by_mcid(doc, resources, after)); let mut changed = HashSet::new(); let mut empty = HashSet::new(); - for (id, (ops, _)) in &b { - match a.get(id) { + let mut budget = MAX_COMPARED; + let len = |ranges: &[Range]| ranges.iter().map(|r| r.len()).fold(0usize, usize::saturating_add); + for (id, (old, _)) in &b.ids { + match a.ids.get(id) { Some((new, draws)) => { - if new != ops { + let n = len(old); + let same = n == len(new) && n <= budget && { + budget -= n; + b.bytes(old).eq(a.bytes(new)) + }; + if !same { changed.insert(*id); } if !draws { @@ -413,3 +460,10 @@ pub(crate) fn drop_object_refs_within(doc: &mut Document, gone: &HashSet } Ok(n) } + +/// Bytes held for comparing marked content, for tests that bound it. +#[cfg(test)] +pub(crate) fn retained_for_comparison(doc: &Document, resources: &Dict, data: &[u8]) -> usize { + let marked = by_mcid(doc, resources, data); + marked.ops.len() + marked.ids.values().map(|(ranges, _)| ranges.len() * std::mem::size_of::>()).sum::() +} diff --git a/crates/redact/src/tests/limits.rs b/crates/redact/src/tests/limits.rs index 6bee96aed4..8519e08608 100644 --- a/crates/redact/src/tests/limits.rs +++ b/crates/redact/src/tests/limits.rs @@ -23,3 +23,33 @@ fn an_image_decoding_far_past_its_raster_is_removed_without_decoding_it_all() { let r = half_covered_image(&bomb); assert_eq!((r.images_removed, r.images_cleared), (1, 0), "{r:?}"); } + +/// Marked content nested `depth` deep, each level with its own MCID, around `depth` drawing +/// operators. +fn nested_marked_content(depth: usize) -> Vec { + let mut data = Vec::new(); + for id in 0..depth { + data.extend_from_slice(format!("/P <> BDC\n").as_bytes()); + } + data.extend(b"0 0 1 1 re f\n".repeat(depth)); + data.extend(b"EMC\n".repeat(depth)); + data +} + +#[test] +fn comparing_deeply_nested_marked_content_holds_each_operator_once() { + let doc = one_page(b"", "", vec![]); + let data = nested_marked_content(2000); + let held = tags::retained_for_comparison(&doc, &Dict::new(), &data); + assert!(held <= 2 * data.len(), "{held} bytes held for {} bytes of content", data.len()); +} + +#[test] +fn a_change_inside_nested_marked_content_touches_every_enclosing_mcid() { + let doc = one_page(b"", "", vec![]); + let before = b"/P <> BDC /P <> BDC BT (A) Tj ET EMC 0 0 1 1 re f EMC /P <> BDC BT (B) Tj ET EMC"; + let after = b"/P <> BDC /P <> BDC BT ET EMC 0 0 1 1 re f EMC /P <> BDC BT (B) Tj ET EMC"; + let (changed, empty) = tags::touched(&doc, &Dict::new(), before, after); + assert_eq!(changed, HashSet::from([0, 1])); + assert_eq!(empty, HashSet::from([1])); +} From 7db37911ed7f22f623f5a9416aa1e83ebe053adc Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:57:39 +0000 Subject: [PATCH 38/42] fix(redact): clear descriptions above removed content and annotations Structure cleanup cleared alternate text, actual text and expansions only on the element that directly owned changed marked content, and an element that lost an annotation reference kept its descriptions unless it lost every kid. An ancestor's /ActualText covering a redacted span, or a link element's /Alt repeating the address of a removed link, stayed in the saved file. Elements that lose an annotation now always lose their descriptions, and every ancestor of an affected element (found through both /P and the tree's /K, which may disagree) loses its own. An element written in place above one with a description to clear refuses the operation, as other in-place cases already do. --- crates/redact/src/tags.rs | 135 ++++++++++++++---- crates/redact/src/tests.rs | 1 + crates/redact/src/tests/descriptions.rs | 64 +++++++++ .../redact/src/tests/surviving_references.rs | 2 +- 4 files changed, 177 insertions(+), 25 deletions(-) create mode 100644 crates/redact/src/tests/descriptions.rs diff --git a/crates/redact/src/tags.rs b/crates/redact/src/tags.rs index 8548810600..60c158b180 100644 --- a/crates/redact/src/tags.rs +++ b/crates/redact/src/tags.rs @@ -183,20 +183,21 @@ pub(crate) fn clean_within( // also from the parent tree and the ID tree: one listed only there still owns marked content // (#3302). The stack pops /K first, so an element reached through /K gets the page it inherits // from its ancestors before a tree reaches it without one. - let mut todo: Vec<(Object, Option)> = - [&b"IDTree"[..], b"ParentTree", b"K"].iter().filter_map(|key| root.get(key).cloned()).map(|k| (k, None)).collect(); + let mut todo: Vec<(Object, Option, Above)> = + [&b"IDTree"[..], b"ParentTree", b"K"].iter().filter_map(|key| root.get(key).cloned()).map(|k| (k, None, Above::default())).collect(); let mut seen = HashSet::new(); let mut pages = HashMap::new(); + let mut parents = Parents::new(); let mut hits: Vec<(ObjRef, Dict, Option, bool)> = Vec::new(); // Whether marked content `x` (a kid on page `pg`) is one this cleanup must change. let touches = |doc: &Document, x: &Object, pg: Option| { mcid_of(doc, x, pg, page, stream).is_some_and(|m| stream.is_some() || changed.contains(&m) || empty.contains(&m)) }; - while let Some((k, pg)) = todo.pop() { + while let Some((k, pg, above)) = todo.pop() { match k { - Object::Array(a) => todo.extend(a.into_iter().map(|x| (x, pg))), + Object::Array(a) => todo.extend(a.into_iter().map(|x| (x, pg, above))), // A number or name tree node written in place. - Object::Dict(d) if !d.contains(b"S") => todo.extend(tree_node_entries(&d).into_iter().map(|x| (x, pg))), + Object::Dict(d) if !d.contains(b"S") => todo.extend(tree_node_entries(&d).into_iter().map(|x| (x, pg, above))), // An element written in place: its kids are followed, but if it owns marked content // this cleanup changes, only rewriting what holds it could change it. Object::Dict(d) => { @@ -208,9 +209,11 @@ pub(crate) fn clean_within( if kids.iter().any(|x| touches(doc, x, pg)) { return Err(RedactError::Tags); } - todo.extend(kids.into_iter().map(|x| (x, pg))); + let above = above.through(&d); + todo.extend(kids.into_iter().map(|x| (x, pg, above))); } Object::Ref(r) => { + parents.record(r, above); if !seen.insert(r) { continue; } @@ -219,13 +222,13 @@ pub(crate) fn clean_within( } let object = doc.get(r); if let Object::Array(a) = &*object { - todo.extend(a.iter().cloned().map(|x| (x, pg))); + todo.extend(a.iter().cloned().map(|x| (x, pg, above))); continue; } let Some(d) = object.as_dict().cloned() else { continue }; if !d.contains(b"S") { // A parent tree or ID tree node: its values may be elements. - todo.extend(tree_node_entries(&d).into_iter().map(|x| (x, pg))); + todo.extend(tree_node_entries(&d).into_iter().map(|x| (x, pg, above))); continue; } let pg = match d.get(b"Pg").and_then(Object::as_ref).or(pg) { @@ -238,11 +241,12 @@ pub(crate) fn clean_within( if sensitive || remap { hits.push((r, d.clone(), pg, sensitive)); } - todo.extend(kids.into_iter().map(|x| (x, pg))); + todo.extend(kids.into_iter().map(|x| (x, pg, Above::element(r)))); } _ => {} } } + let sensitive: Vec = hits.iter().filter(|h| h.3).map(|h| h.0).collect(); let mut n = 0; for (r, d, pg, sensitive) in hits { let kids = d.get(b"K").map(|k| doc.resolve(k)); @@ -291,9 +295,88 @@ pub(crate) fn clean_within( })?; n += usize::from(sensitive && doc.get(r).as_dict() != Some(&d)); } + // An element above may describe the changed content too (`/ActualText` on a span's parent + // covers the span, §14.9.4). + n += clear_ancestors(doc, &sensitive, &parents, limit)?; Ok(n) } +/// Where the walk reached an object from: the nearest element above it that is an object of its +/// own, and whether an element written in place between them carries a description (which only +/// rewriting the object holding it could remove). +#[derive(Clone, Copy, Default)] +struct Above { + element: Option, + described_in_place: bool, +} + +impl Above { + fn element(r: ObjRef) -> Self { + Self { element: Some(r), described_in_place: false } + } + + /// Below an element written in place. + fn through(self, d: &Dict) -> Self { + Self { described_in_place: self.described_in_place || DESCRIPTIONS.iter().any(|k| d.contains(k)), ..self } + } +} + +/// The keys of an element that describe its content. +const DESCRIPTIONS: [&[u8]; 3] = [b"Alt", b"ActualText", b"E"]; + +/// Each element's parents as the walk found them: through `/K`, which `/P` may not agree with. +struct Parents(HashMap>); + +impl Parents { + fn new() -> Self { + Self(HashMap::new()) + } + + fn record(&mut self, r: ObjRef, above: Above) { + if above.element.is_some() || above.described_in_place { + self.0.entry(r).or_default().push(above); + } + } +} + +/// Remove the descriptions of every element above `elements`, through both `/P` and the walk's +/// `parents`; returns how many elements changed. [`RedactError::Tags`] when an element written in +/// place above them carries one, or when there are more than `limit` ancestors. +fn clear_ancestors(doc: &mut Document, elements: &[ObjRef], parents: &Parents, limit: usize) -> Result { + let mut todo: Vec = elements.to_vec(); + let mut seen: HashSet = elements.iter().copied().collect(); + let mut cleared = 0; + while let Some(r) = todo.pop() { + let mut up: Vec = doc.get(r).as_dict().and_then(|d| d.get(b"P")).and_then(Object::as_ref).into_iter().collect(); + for above in parents.0.get(&r).into_iter().flatten() { + if above.described_in_place { + return Err(RedactError::Tags); + } + up.extend(above.element); + } + for a in up { + if !seen.insert(a) { + continue; + } + if seen.len() > limit { + return Err(RedactError::Tags); + } + // The structure tree root (no /S) ends the climb. + let Some(d) = doc.get(a).as_dict().filter(|d| d.contains(b"S")).cloned() else { continue }; + if DESCRIPTIONS.iter().any(|k| d.contains(k)) { + doc.update_dict(a, |e| { + for key in DESCRIPTIONS { + e.remove(key); + } + })?; + cleared += 1; + } + todo.push(a); + } + } + Ok(cleared) +} + /// The page an element inherits (§14.7.4.4): the nearest `/Pg` among its ancestors, following /// `/P`. `Ok(None)` when none has one; [`RedactError::Tags`] when the lookup gives up (more than /// [`MAX_PARENTS`] ancestors, or a loop), which isn't the same as having no page. `pages` keeps @@ -384,12 +467,14 @@ pub(crate) fn drop_object_refs_within(doc: &mut Document, gone: &HashSet }; // Elements are reached from the root's /K, and also from the parent tree and the ID tree: // one listed only there (left behind by an editor, say) still points at its annotation. - let mut todo: Vec = [&b"K"[..], b"ParentTree", b"IDTree"].iter().filter_map(|key| root.get(key).cloned()).collect(); + let mut todo: Vec<(Object, Above)> = + [&b"K"[..], b"ParentTree", b"IDTree"].iter().filter_map(|key| root.get(key).cloned()).map(|k| (k, Above::default())).collect(); let mut seen = HashSet::new(); + let mut parents = Parents::new(); let mut hits: Vec = Vec::new(); - while let Some(k) = todo.pop() { + while let Some((k, above)) = todo.pop() { match k { - Object::Array(a) => todo.extend(a), + Object::Array(a) => todo.extend(a.into_iter().map(|x| (x, above))), // A structure element written in place: its kids are followed; one that tags a // removed annotation can't be changed here (see above). Object::Dict(d) if d.contains(b"S") => { @@ -397,11 +482,13 @@ pub(crate) fn drop_object_refs_within(doc: &mut Document, gone: &HashSet if kids.iter().any(|x| refers_to(doc, x, gone)) { return Err(RedactError::Tags); } - todo.extend(kids); + let above = above.through(&d); + todo.extend(kids.into_iter().map(|x| (x, above))); } // A number or name tree node written in place (§7.9.6, §7.9.7): its values and kids. - Object::Dict(d) => todo.extend(tree_node_entries(&d)), + Object::Dict(d) => todo.extend(tree_node_entries(&d).into_iter().map(|x| (x, above))), Object::Ref(r) => { + parents.record(r, above); if !seen.insert(r) { continue; } @@ -410,26 +497,26 @@ pub(crate) fn drop_object_refs_within(doc: &mut Document, gone: &HashSet } let object = doc.get(r); if let Object::Array(a) = &*object { - todo.extend(a.iter().cloned()); + todo.extend(a.iter().cloned().map(|x| (x, above))); continue; } let Some(d) = object.as_dict() else { continue }; if !d.contains(b"S") { // A parent tree or ID tree node: its values may be elements. - todo.extend(tree_node_entries(d)); + todo.extend(tree_node_entries(d).into_iter().map(|x| (x, above))); continue; } let kids = kids_of(doc, d); if kids.iter().any(|x| refers_to(doc, x, gone)) { hits.push(r); } - todo.extend(kids); + todo.extend(kids.into_iter().map(|x| (x, Above::element(r)))); } _ => {} } } let mut n = 0; - for r in hits { + for &r in &hits { let Some(d) = doc.get(r).as_dict().cloned() else { continue }; let kids = d.get(b"K").map(|k| doc.resolve(k)); let array = kids.as_ref().is_some_and(|k| matches!(&**k, Object::Array(_))); @@ -449,15 +536,15 @@ pub(crate) fn drop_object_refs_within(doc: &mut Document, gone: &HashSet } else { e.remove(b"K"); } - // An element left with nothing to tag keeps no text describing what it tagged: - // a link's alternate text often repeats the URL, an annotation's its comment. - if keep.is_empty() { - for key in [&b"Alt"[..], b"ActualText", b"T", b"E"] { - e.remove(key); - } + // An element that lost an annotation keeps no text describing it, even with other + // kids left: a link's alternate text often repeats the URL, an annotation's its + // comment. Elements above it may describe it too. + for key in [&b"Alt"[..], b"ActualText", b"T", b"E"] { + e.remove(key); } })?; } + clear_ancestors(doc, &hits, &parents, limit)?; Ok(n) } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 94081f9e35..ebc95f4519 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -2461,6 +2461,7 @@ fn unused_descriptions_in_shared_or_inherited_resources_are_retired() { } } +mod descriptions; mod limits; mod property_retirement; mod surviving_references; diff --git a/crates/redact/src/tests/descriptions.rs b/crates/redact/src/tests/descriptions.rs new file mode 100644 index 0000000000..df6b8555dc --- /dev/null +++ b/crates/redact/src/tests/descriptions.rs @@ -0,0 +1,64 @@ +//! Descriptions in the structure tree (alternate text, actual text, expansions) go with the +//! content they describe, on the element that owns it and on every element above it. + +use super::surviving_references::full_save_holds; +use super::*; + +/// A tagged page: "SECRET" (MCID 0) inside a section whose descriptions repeat it, and "Public" +/// (MCID 1). `paragraph_parent` is the secret paragraph's /P. +fn sectioned(paragraph_parent: &str) -> Document { + let content = b"/P <> BDC BT /F1 10 Tf 10 200 Td (SECRET) Tj ET EMC /P <> BDC BT /F1 10 Tf 10 100 Td (Public) Tj ET EMC"; + pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R /StructTreeRoot 7 0 R /MarkInfo << /Marked true >> >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /StructParents 0 /Resources << /Font << /F1 5 0 R >> >> >>".to_vec(), + stream("", content), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + b"<< /Type /StructTreeRoot /K 8 0 R >>".to_vec(), + b"<< /S /Document /P 7 0 R /K [11 0 R 10 0 R] >>".to_vec(), + format!("<< /S /P /P {paragraph_parent} /Pg 3 0 R /K 0 >>").into_bytes(), + b"<< /S /P /P 8 0 R /Pg 3 0 R /ActualText (Public) /K 1 >>".to_vec(), + b"<< /S /Sect /P 8 0 R /ActualText (LEAK_ACTUAL) /Alt (LEAK_ALT) /E (LEAK_EXPANSION) /K 9 0 R >>".to_vec(), + ]) +} + +#[test] +fn an_ancestor_describing_redacted_content_loses_its_descriptions() { + // The paragraph names its section as parent, or (wrongly) the document: the section is found + // either way, by its /K. + for parent in ["11 0 R", "8 0 R"] { + let mut doc = sectioned(parent); + mark(&mut doc, 0, &[[5.0, 195.0, 60.0, 212.0]], ""); + apply(&mut doc, None).unwrap(); + assert!(!full_save_holds(&doc, b"LEAK_"), "a description of the redacted word survived (paragraph /P {parent})"); + let public = doc.get(ObjRef::new(10, 0)).as_dict().cloned().unwrap(); + assert!(public.contains(b"ActualText"), "an element beside it keeps its own"); + } +} + +#[test] +fn a_link_element_that_keeps_its_text_loses_the_description_of_its_removed_annotation() { + use crate::sanitize::{Hidden, remove_hidden}; + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R /MarkInfo << /Marked true >> /StructTreeRoot 8 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> /Annots [6 0 R] /StructParents 0 >>" + .to_vec(), + stream("", b"/Link << /MCID 0 >> BDC BT /F1 10 Tf 10 230 Td (See the site) Tj ET EMC"), + FONT.replace("95 0 R", "12 0 R").into_bytes(), + b"<< /Type /Annot /Subtype /Link /Rect [10 228 80 240] /StructParent 1 /A << /S /URI /URI (https://example.org/) >> >>".to_vec(), + b"null".to_vec(), + b"<< /Type /StructTreeRoot /K 9 0 R /ParentTree << /Nums [0 [11 0 R] 1 11 0 R] >> /ParentTreeNextKey 2 >>".to_vec(), + b"<< /Type /StructElem /S /Document /P 8 0 R /K 10 0 R >>".to_vec(), + b"<< /Type /StructElem /S /P /P 9 0 R /ActualText (See LEAK_PARENT) /K 11 0 R >>".to_vec(), + // The link element tags its text and the annotation; its descriptions repeat the address. + b"<< /Type /StructElem /S /Link /P 10 0 R /Pg 3 0 R /Alt (LEAK_ALT) /ActualText (LEAK_ACTUAL) /E (LEAK_E) /T (LEAK_T) /K [0 << /Type /OBJR /Obj 6 0 R /Pg 3 0 R >>] >>".to_vec(), + widths(), + ]); + let done = remove_hidden(&mut doc, &[Hidden::LinksActionsScripts]).unwrap(); + assert!(done.contains(&(Hidden::LinksActionsScripts, 1)), "{done:?}"); + assert!(!full_save_holds(&doc, b"LEAK_"), "a description of the removed link survived"); + let link = doc.get(ObjRef::new(11, 0)).as_dict().cloned().unwrap(); + assert_eq!(link.get(b"K"), Some(&Object::Array(vec![Object::Int(0)])), "the element keeps its text"); +} diff --git a/crates/redact/src/tests/surviving_references.rs b/crates/redact/src/tests/surviving_references.rs index 2c8bdb09c2..d3aa98af67 100644 --- a/crates/redact/src/tests/surviving_references.rs +++ b/crates/redact/src/tests/surviving_references.rs @@ -3,7 +3,7 @@ use super::*; /// Whether `secret` survives a full save of `doc`, in the file's bytes or in a decoded stream. -fn full_save_holds(doc: &Document, secret: &[u8]) -> bool { +pub(super) fn full_save_holds(doc: &Document, secret: &[u8]) -> bool { let bytes = write_full(doc, &SaveOptions::default()).unwrap(); let found = |data: &[u8]| data.windows(secret.len()).any(|w| w == secret); if found(&bytes) { From b72078871a307ad8d28b9e3f861885435fe10834 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:09:57 +0000 Subject: [PATCH 39/42] fix(content): end inline images where the renderer ends them Filtered inline image data has no length, so its end is an EI chosen among candidates, and the content parser chose by a different rule than the renderer's tokenizer (hayro-syntax): the renderer also takes an EI with no whitespace before it, and checks a candidate by reading what follows as operators. With image data such as "7FEI 1 0 0 1 200 0 cm", the renderer ran a cm that redaction read as image data, so text was drawn 200 pt from where removal and its verify pass looked. The parser now takes each inline image's end from the renderer's tokenizer (already in the tree through pdfcraft-cos), so both read the same operators after the image; a test runs both on the same fixtures. Only where that tokenizer reads no image (no EI it accepts, or parameters it can't read) does the parser fall back to its own rules, counting the image in Parsed::ambiguous_images, and applying redactions refuses such a page as unreadable. --- Cargo.lock | 1 + crates/content/Cargo.toml | 3 + crates/content/src/lib.rs | 120 ++++++++++++++++++----- crates/content/src/tests.rs | 58 +++++++++-- crates/engine/src/tests.rs | 24 +++++ crates/redact/src/interp.rs | 6 ++ crates/redact/src/lib.rs | 6 ++ crates/redact/src/tests.rs | 1 + crates/redact/src/tests/inline_images.rs | 34 +++++++ 9 files changed, 224 insertions(+), 29 deletions(-) create mode 100644 crates/redact/src/tests/inline_images.rs diff --git a/Cargo.lock b/Cargo.lock index a950149243..e318b7a77c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4042,6 +4042,7 @@ version = "0.11.0" name = "pdfcraft-content" version = "0.11.0" dependencies = [ + "hayro-syntax", "pdfcraft-cos", ] diff --git a/crates/content/Cargo.toml b/crates/content/Cargo.toml index c553df0b8c..e6b4c28a42 100644 --- a/crates/content/Cargo.toml +++ b/crates/content/Cargo.toml @@ -10,3 +10,6 @@ workspace = true [dependencies] pdfcraft-cos = { workspace = true } +# The renderer's content tokenizer, already in the tree through pdfcraft-cos: inline images end +# where the renderer ends them. +hayro-syntax = { workspace = true } diff --git a/crates/content/src/lib.rs b/crates/content/src/lib.rs index eb126ffa21..097ec26897 100644 --- a/crates/content/src/lib.rs +++ b/crates/content/src/lib.rs @@ -60,6 +60,10 @@ pub struct Parsed { pub ops: Vec, /// Bytes that could not be read as operands or operators. pub skipped: usize, + /// Inline images the renderer's tokenizer reads no image for (it stops, or finds no `EI` it + /// accepts). Their end here is a guess, so the operators after them may not be the ones the + /// page draws. + pub ambiguous_images: usize, /// For [`Pieces::parse`]: per piece, whether a comment runs into it from the piece before /// and whether one runs out of it into the next. Only boundaries are recorded, so the size /// depends on the number of pieces, not of comments. @@ -142,19 +146,27 @@ fn parse_pieces(data: &[u8], ends: &[usize]) -> Parsed { start.get_or_insert(at); operands.push(Object::Null); } - b"BI" => match inline_image(data, lx.pos) { - Ok((dict, img, end)) => { - lx.pos = end; - out.ops.push(Op { op: b"BI".to_vec(), operands: Vec::new(), inline: Some((dict, img)), span: start.take().unwrap_or(at)..end }); - operands.clear(); + b"BI" => { + let image = inline_image(data, at, lx.pos); + out.ambiguous_images += usize::from(image.ambiguous); + let end = image.end; + lx.pos = end; + match image.image { + Some((dict, img)) => { + out.ops.push(Op { + op: b"BI".to_vec(), + operands: Vec::new(), + inline: Some((dict, img)), + span: start.take().unwrap_or(at)..end, + }); + } + None => { + out.skipped += end.saturating_sub(at); + start = None; + } } - Err(end) => { - out.skipped += end.saturating_sub(at); - lx.pos = end; - start = None; - operands.clear(); - } - }, + operands.clear(); + } _ => { let s = start.take().unwrap_or(at); out.ops.push(Op { op: kw.to_vec(), operands: std::mem::take(&mut operands), inline: None, span: s..lx.pos }); @@ -198,11 +210,29 @@ fn comment_edges(data: &[u8], trivia: std::ops::Range, ends: &[usize], bo } } -/// Read an inline image starting after `BI`: (parameters, data, position after `EI`). `Err` holds -/// the position after the `EI` (or the end of the stream) of an image whose parameters can't be read. -fn inline_image(data: &[u8], pos: usize) -> Result<(Dict, Vec, usize), usize> { +/// An inline image as [`inline_image`] reads it. +struct Inline { + /// Its parameters and data; `None` when the parameters can't be read. + image: Option<(Dict, Vec)>, + /// The position after its `EI` (or the end of the stream). + end: usize, + /// The renderer's tokenizer reads no image here, so `end` is this parser's guess. + ambiguous: bool, +} + +/// Read the inline image whose `BI` keyword starts at `bi` (its parameters start at `pos`). +/// +/// Filtered image data has no length, and `EI` can occur inside it, so where it ends is a +/// heuristic choice among `EI` candidates. The renderer's choice decides which operators the +/// page draws after the image, and redaction must see the same ones, so the end is taken from +/// the renderer's tokenizer (hayro-syntax) rather than worked out here by another rule. Only +/// when that tokenizer reads no image does this parser fall back to its own rules, and the +/// image is marked ambiguous. +fn inline_image(data: &[u8], bi: usize, pos: usize) -> Inline { let mut lx = Lexer::new(data, pos); let mut dict = Dict::new(); + let mut readable = true; + let mut stop = pos; loop { lx.skip_ws(); if lx.eat_keyword(b"ID") { @@ -216,22 +246,68 @@ fn inline_image(data: &[u8], pos: usize) -> Result<(Dict, Vec, usize), usize let Some((k, v)) = entry else { // A stream that ends inside the parameters is cut short, not unreadable: keep what was read. if at >= data.len() { - return Ok((dict, Vec::new(), data.len())); + return Inline { image: Some((dict, Vec::new())), end: data.len(), ambiguous: false }; } - // Without an `EI` after it, the image runs to the end of the stream. - return Err(end_of_image(data, at, b"").map_or(data.len(), |(_, end)| end)); + readable = false; + stop = at; + break; }; dict.set(k, v); } + if let Some((start, end_data, end)) = renderer_image_end(data, bi) { + // Where the renderer ends the data at an `EI` found by its heuristic, one whitespace byte + // before that `EI` separates it from the data. + let mut samples = data.get(start..end_data).unwrap_or_default(); + if data.get(end_data..).is_some_and(|rest| rest.starts_with(b"EI")) + && let Some((last, rest)) = samples.split_last() + && is_ws(*last) + { + samples = rest; + } + return Inline { image: readable.then(|| (dict, samples.to_vec())), end, ambiguous: false }; + } + if !readable { + // Without an `EI` after it, the image runs to the end of the stream. + return Inline { image: None, end: end_of_image(data, stop, b"").map_or(data.len(), |(_, end)| end), ambiguous: true }; + } // One whitespace byte separates ID from the data. let begin = (lx.pos + 1).min(data.len()); let end = unfiltered_image_end(data, begin, &dict) .or_else(|| declared_image_end(data, begin, &dict)) .or_else(|| image_data_end(data, begin, filter_eod(&dict))); - Ok(match end { - Some((end_data, end)) => (dict, data[begin..end_data].to_vec(), end), - None => (dict, data[begin..].to_vec(), data.len()), - }) + let (image, end) = match end { + Some((end_data, end)) => (data.get(begin..end_data).unwrap_or_default().to_vec(), end), + None => (data.get(begin..).unwrap_or_default().to_vec(), data.len()), + }; + Inline { image: Some((dict, image)), end, ambiguous: true } +} + +/// Where the renderer's tokenizer ends the inline image whose `BI` starts at `bi`: the start and +/// end of its data and the position after its `EI`. `None` when it reads no image there (its +/// parameters don't read, or no `EI` passes its checks). +fn renderer_image_end(data: &[u8], bi: usize) -> Option<(usize, usize, usize)> { + use hayro_syntax::content::TypedIter; + use hayro_syntax::content::ops::{InlineImage, TypedInstruction}; + let tail = data.get(bi..)?; + let mut iter = TypedIter::new(tail); + let (start, len) = match iter.next()? { + TypedInstruction::InlineImage(InlineImage(stream)) => match stream.raw_data() { + // The data is a slice of `tail`: its offset is where it starts. + std::borrow::Cow::Borrowed(samples) => ((samples.as_ptr() as usize).checked_sub(tail.as_ptr() as usize)?, samples.len()), + std::borrow::Cow::Owned(_) => return None, + }, + _ => return None, + }; + let end_data = start.checked_add(len)?; + // `committed` is past the whitespace after `EI`; the image ends at the `EI`. + let mut end = iter.committed(); + while end > end_data && tail.get(end - 1).is_some_and(|b| is_ws(*b)) { + end -= 1; + } + if end < end_data.checked_add(2)? || tail.get(end - 2..end) != Some(b"EI") { + return None; + } + Some((bi.checked_add(start)?, bi.checked_add(end_data)?, bi.checked_add(end)?)) } /// The end-of-data marker (`>` for ASCIIHex, `~>` for ASCII85) of the outer stream filter, if any. diff --git a/crates/content/src/tests.rs b/crates/content/src/tests.rs index 094a0665df..c9abef8349 100644 --- a/crates/content/src/tests.rs +++ b/crates/content/src/tests.rs @@ -102,10 +102,11 @@ fn unfiltered_inline_masks_default_to_one_bit_and_indexed_images_have_one_compon let parsed = inline_pixels(parameters.as_bytes(), b" EI "); assert_eq!(parsed.ops[1].inline.as_ref().unwrap().1, b" EI "); assert_eq!(parsed.ops.len(), 3); - // Indexed samples permit at most 8 bits, even though device spaces permit 16. + // Indexed samples permit at most 8 bits, even though device spaces permit 16, so the + // length is unknown, and the renderer takes the second `EI`. let invalid = parameters.replace("/W 4", "/W 2").replace("/BPC 8", "/BPC 16"); let parsed = inline_pixels(invalid.as_bytes(), b" EI "); - assert!(parsed.ops[1].inline.as_ref().unwrap().1.is_empty()); + assert_eq!(parsed.ops[1].inline.as_ref().unwrap().1, b" EI "); } } @@ -127,7 +128,7 @@ fn unfiltered_inline_image_terminator_must_follow_its_samples() { } #[test] -fn filtered_unknown_and_damaged_inline_images_keep_the_existing_fallback() { +fn filtered_unknown_and_damaged_inline_images_end_where_the_renderer_ends_them() { for parameters in [ b"/W 4 /H 1 /BPC 8 /CS /G /F /A85".as_slice(), b"/W 4 /H 1 /BPC 8 /CS /ResourceColor", @@ -142,12 +143,20 @@ fn filtered_unknown_and_damaged_inline_images_keep_the_existing_fallback() { b"/W 9223372036854775807 /H 9223372036854775807 /BPC 16 /CS /CMYK", b"/W 9223372036854775807 /H 2 /BPC 8 /CS /G", ] { + // What follows the first `EI` (`!!~>`) does not read as content, so the renderer takes + // the later one. let parsed = inline_pixels(parameters, b"! EI !!~>"); - assert_eq!(parsed.ops[1].inline.as_ref().unwrap().1, b"!", "{}", String::from_utf8_lossy(parameters)); + let label = String::from_utf8_lossy(parameters); + assert_eq!(parsed.ops[1].inline.as_ref().unwrap().1, b"! EI !!~>", "{label}"); + assert_eq!(parsed.ops.iter().map(|op| op.op.as_slice()).collect::>(), [b"q".as_slice(), b"BI", b"Q"], "{label}"); + assert_eq!(parsed.ambiguous_images, 0, "{label}"); } - // A short payload still uses the tolerant recovery, including when there is no EI. - for src in [b"BI /W 4 /H 1 /BPC 8 /CS /G ID abc\nEI Q".as_slice(), b"BI /W 4 /H 1 /BPC 8 /CS /G ID abc"] { - assert_eq!(parse(src).ops[0].inline.as_ref().unwrap().1, b"abc"); + // A short payload is read whole; without an `EI`, the renderer reads no image and this + // parser's end is a guess. + for (src, ambiguous) in [(b"BI /W 4 /H 1 /BPC 8 /CS /G ID abc\nEI Q".as_slice(), 0), (b"BI /W 4 /H 1 /BPC 8 /CS /G ID abc", 1)] { + let parsed = parse(src); + assert_eq!(parsed.ops[0].inline.as_ref().unwrap().1, b"abc"); + assert_eq!(parsed.ambiguous_images, ambiguous); } } @@ -459,3 +468,38 @@ fn ascii85_inline_image_does_not_terminate_at_gt_ei_inside_payload() { assert_eq!(img, b"!!!!>EI !!~>"); } } + +/// The operators the renderer's tokenizer reads from `data`. +fn renderer_operators(data: &[u8]) -> Vec> { + let mut iter = hayro_syntax::content::UntypedIter::new(data); + let mut out = Vec::new(); + while let Some(instruction) = iter.next() { + out.push(instruction.operator.as_ref().to_vec()); + } + out +} + +/// Inline images end where the renderer ends them, so both read the same operators after one: +/// an `EI` without whitespace before it in the data, upper-case text ending in "EI" after the +/// image, and an `EI` inside compressed bytes (#3579). +#[test] +fn inline_images_end_where_the_renderer_ends_them() { + let flate: Vec = (0..FLATE_WITH_EI.len()).step_by(2).map(|i| u8::from_str_radix(&FLATE_WITH_EI[i..i + 2], 16).unwrap()).collect(); + let mut stored = b"q BI /W 64 /H 1 /BPC 8 /CS /G /F /Fl ID ".to_vec(); + stored.extend_from_slice(&flate); + stored.extend_from_slice(b"\nEI Q BT /F1 24 Tf 72 200 Td (AFTER) Tj ET"); + let fixtures: [&[u8]; 6] = [ + b"BI /W 1 /H 1 /CS /G /BPC 8 /F /AHx ID 7FEI 1 0 0 1 200 0 cm EI BT /F1 10 Tf 10 100 Td (SECRET) Tj ET", + b"BI /W 1 /H 1 /CS /G /BPC 8 /F /AHx ID 7F> EI BT /F1 10 Tf 10 100 Td (ZWEI) Tj (EIN FREI BEI) Tj ET", + b"q BI /W 2 /H 1 /CS /G /BPC 8 /F /AHx ID 7F7F> EI Q BT (DREI) Tj ET BI /W 1 /H 1 /CS /G /BPC 8 /F /AHx ID 00> EI", + b"BI /W 4 /H 1 /BPC 8 /CS /G /F /A85 ID ! EI !!~>\nEI Q", + b"q BI /W 4 /H 1 /BPC 8 /CS /G ID EI \nEI Q", + &stored, + ]; + for data in fixtures { + let parsed = parse(data); + let ours: Vec> = parsed.ops.iter().map(|op| op.op.clone()).collect(); + assert_eq!(ours, renderer_operators(data), "{}", String::from_utf8_lossy(data)); + assert_eq!((parsed.skipped, parsed.ambiguous_images), (0, 0), "{}", String::from_utf8_lossy(data)); + } +} diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 8937cf6780..3012a1f3fe 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2320,6 +2320,30 @@ fn the_renderer_check_refuses_text_still_shown_under_a_mark() { assert!(refused(&cos), "the labelled input stream was not checked"); } +/// Inline image data `7FEI 1 0 0 1 200 0 cm` holds an `EI` the renderer ends the image at, so it +/// runs the `cm` and shows SECRET 200 pt to the right. Redaction reads the image the same way, so +/// a mark drawn over the word as shown takes it out of the file. +#[test] +fn text_after_an_inline_image_is_redacted_where_the_page_shows_it() { + let mut s = Session::new().with_clock(|| 1_700_000_000); + let page = one_line_page( + "/Font << /F1 4 0 R >>", + "BI /W 1 /H 1 /CS /G /BPC 8 /F /AHx ID 7FEI 1 0 0 1 200 0 cm EI BT /F1 10 Tf 10 100 Td (SECRET) Tj ET", + ); + let id = s.open("inline.pdf", None, Arc::new(page), None).unwrap(); + mark_shown_text(&mut s, id, "SECRET"); + s.apply(id, Edit::ApplyRedactions { pages: None }).unwrap(); + assert!(!page_texts(&s, id).concat().contains("SECRET")); + let saved = s.save_bytes(id).unwrap(); + let held = |d: &[u8]| d.windows(6).any(|w| w == b"SECRET"); + assert!(!held(&saved), "the saved file holds the redacted word"); + let reopened = pdfcraft_cos::Document::open(saved).unwrap(); + assert!( + !reopened.scan_objects().any(|(_, o)| matches!(&*o, pdfcraft_cos::Object::Stream(st) if st.decoded().is_ok_and(|d| held(&d)))), + "a saved stream holds the redacted word" + ); +} + /// A word whose font and size an ExtGState /Font entry (`gs`) sets is placed where the renderer /// draws it, so a mark drawn over it as displayed takes it out of the file. #[test] diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index fca3a1da88..b4186fd4e9 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -74,6 +74,10 @@ pub(crate) struct Output { /// Apply mode met vertical-writing text (which it cannot remove) while a redaction region was /// in force: the operation must fail closed rather than leave that text under the mark. pub vertical: bool, + /// The content holds an inline image the renderer reads no image for + /// ([`pdfcraft_content::Parsed::ambiguous_images`]): what this scope placed after it may not + /// be what the page shows, so a redaction must not trust it. + pub ambiguous: bool, pub tag_changes: Vec<(ObjRef, ObjRef, HashSet, HashSet)>, pub error: Option, } @@ -789,6 +793,7 @@ fn process_from(doc: &mut Document, scope: &mut Scope<'_>, streams: &[Vec], // next. Parse them joined, then give each operator back to the piece its keyword is in. let joined = Pieces::join(streams); let mut parsed = joined.parse(); + out.ambiguous = parsed.ambiguous_images > 0; // A rewritten piece loses its comments, so keep where they began and ended: a comment that // ran on into the next piece still hides that piece's first line. let comment_edges = joined.comment_edges(&parsed); @@ -1293,6 +1298,7 @@ fn xobject( scope.fonts = saved_fonts; out.residue += inner.residue; out.vertical |= inner.vertical; + out.ambiguous |= inner.ambiguous; out.doomed.extend_from_slice(&inner.doomed); out.retired_properties.extend(inner.retired_properties); out.tag_changes.extend(inner.tag_changes); diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 97d5bcd9fe..e67b373a11 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -411,6 +411,12 @@ pub fn apply(doc: &mut Document, pages: Option<&[usize]>) -> Result EI BT /F1 10 Tf 10 100 Td (SECRET) Tj ET", "", vec![]); + mark(&mut doc, 0, &[[5.0, 95.0, 45.0, 110.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().glyphs, 6); + assert!(!content(&doc, 0).contains("SECRET")); +} + +/// Without an `EI` the renderer accepts, where the operators resume is a guess: refused. +#[test] +fn a_page_whose_inline_image_the_renderer_cannot_end_is_refused() { + let mut doc = one_page(b"BT /F1 10 Tf 10 100 Td (SECRET) Tj ET BI /W 1 /H 1 /CS /G /BPC 8 /F /AHx ID 7F", "", vec![]); + mark(&mut doc, 0, &[[5.0, 95.0, 45.0, 110.0]], ""); + assert_eq!(apply(&mut doc, None), Err(RedactError::Unreadable(1))); +} From 01aa6bcc17b09ad294777d7df49f6b52d6c275fe Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:31:38 +0000 Subject: [PATCH 40/42] docs(redact): describe the overlay, text state, description and inline image rules --- crates/content/README.md | 11 +++++++---- crates/redact/README.md | 22 ++++++++++++++++------ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/crates/content/README.md b/crates/content/README.md index 6ab984dd49..fedeadc56b 100644 --- a/crates/content/README.md +++ b/crates/content/README.md @@ -13,7 +13,10 @@ The parser is tolerant (unreadable bytes are skipped and counted, never fatal) a panics. `redact` and (later) `edit` build their interpreters on it; the graphics-state tracker and the `Patch` API of architecture §6 grow here. -Unfiltered inline images with built-in or Indexed colour spaces, and image masks, use their -sample layout (including per-row byte padding) to locate `EI`. This preserves pixel bytes -that look like the terminator. Filtered images and resource-defined colour spaces retain -the tolerant delimiter heuristic; damaged or truncated layouts fall back to it too. +An inline image ends where the renderer's tokenizer (`hayro-syntax`) ends it, so the parser +and the renderer read the same operators after the image, and redaction removes what the page +actually draws. Only where that tokenizer reads no image (its parameters don't read, or no `EI` +passes its checks) does the parser locate `EI` by its own rules: unfiltered images with built-in +or Indexed colour spaces, and image masks, by their sample layout (including per-row byte +padding), other images by the tolerant delimiter heuristic. Such an image is counted in +`Parsed::ambiguous_images`, because the operators after it may not be the ones the page draws. diff --git a/crates/redact/README.md b/crates/redact/README.md index 2caf067b5b..cca3d51fdc 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -20,7 +20,8 @@ clear_marks(&mut doc, None)?; // remove marks without app - **images**: fully covered → removed; partly covered → a copy with the covered pixels cleared (8/16-bit and 1/2/4-bit images, image masks; Flate/LZW/RL/A85/AHx). Codecs PdfCraft can't re-encode (DCT, JPX, JBIG2, CCITT) are removed whole (fail-closed); inline images under an - area are removed. Images with separate `/SMask` or `/Mask` data are removed whole; + area are removed. Images with separate `/SMask` or `/Mask` data, and images whose data decodes + to more than twice their declared raster (plus a byte per row and 4 KiB), are removed whole; cleared copies drop `/Alternates` so alternate pixels cannot survive; - **vectors**: covered paths are removed; partly covered paths and shadings are clipped so nothing paints inside the areas; @@ -31,8 +32,13 @@ clear_marks(&mut doc, None)?; // remove marks without app page as a `/PCMark /Redaction` stream. A verification pass re-reads each redacted page and fails the operation if any glyph or inline -image is still under an area. Content that can't be decoded makes the operation fail rather than -leave it unredacted. A Redact annotation with neither a usable `/QuadPoints` array nor a usable +image is still under an area. It checks every content stream the page had, whatever its +dictionary calls it; only the overlay streams this `apply` added (`Report::overlays`) are skipped. +Text is placed with the font and size set by `Tf` or by an ExtGState's `/Font` through `gs`, and a +form XObject starts from the text state of the content that draws it. Content that can't be +decoded makes the operation fail rather than leave it unredacted, and so does an inline image the +renderer's tokenizer reads no image for (`RedactError::Unreadable`): where the page's operators +resume after it would be a guess. A Redact annotation with neither a usable `/QuadPoints` array nor a usable `/Rect` makes it fail with `RedactError::UnreadableMark` before anything changes, instead of skipping that mark. The count covers the marks on the pages being applied. @@ -50,15 +56,19 @@ as Sanitize drops them, so a full rewrite of a tagged PDF can't keep a removed c A tree where that can't be done (an element written in place, or one too large to walk) makes `apply` fail with `RedactError::Tags` rather than leave the annotation reachable. -Replacement descriptions are removed only from marked-content spans changed by cleanup. -Unchanged spans retain their descriptions; unused property entries are retired during redaction +Replacement descriptions (`/Alt`, `/ActualText`, `/E`) are removed from marked-content spans +changed by cleanup, from every structure element above them (found through both `/P` and `/K`), +and from elements that lose an annotation reference, which also lose `/T`. An element written in +place above one whose descriptions go makes `apply` fail with `RedactError::Tags`. Unchanged +spans retain their descriptions; unused property entries are retired during redaction and hidden-content cleanup, including aliases in shared/inherited resources. Descriptions still used by surviving pages or forms are kept. A redacted page that inherits `/Resources` from the page tree gets them pinned (by reference; a direct inherited dictionary is made one shared object first) before it is rewritten. The parent's `/Resources` is then retired unless another page still inherits it, so it cannot keep a removed -XObject reachable. Pages that aren't redacted are left untouched. +XObject reachable. Surviving form XObjects lose the resource entries for removed XObjects their own +content doesn't draw, for the same reason. Pages that aren't redacted are left untouched. ## Remove hidden information / Sanitize From 8555d6f159a7b2d7d420185642b8901719b457a6 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:43:38 +0000 Subject: [PATCH 41/42] fix(parity): cite the renamed renderer-check test and drop fixed redaction gaps --- parity/acrobat-features.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 362c2aa776..45dda39822 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -5709,8 +5709,8 @@ milestone = "M8" status = "shipped" commands = ["redact.apply"] tools = ["redact_apply"] -evidence = ["crates/ui-egui/tests/it/redact.rs::marking_applying_and_clearing", "crates/engine/src/tests.rs::redaction_marks_apply_for_good_and_undo", "crates/automation/tests/it/automation.rs::redacting_through_tools", "crates/engine/src/tests.rs::redaction_is_refused_when_the_page_still_shows_text_under_a_mark", "crates/redact/src/tests.rs::redacted_xobjects_do_not_survive_through_inherited_or_shared_resources"] -notes = "confirmation; one undo step; next save is a full rewrite. Refused, with nothing changed, when the page still shows text under a mark afterwards: text whose font is set through an ExtGState or taken by a form from its caller, some predefined CMaps and rotated Type 3 fonts are not removed yet, so those documents cannot be redacted. Removed images and forms are unlinked from page-tree resources (own, inherited or shared), not yet from a form's own resources, annotation appearances or patterns" +evidence = ["crates/ui-egui/tests/it/redact.rs::marking_applying_and_clearing", "crates/engine/src/tests.rs::redaction_marks_apply_for_good_and_undo", "crates/automation/tests/it/automation.rs::redacting_through_tools", "crates/engine/src/tests.rs::the_renderer_check_refuses_text_still_shown_under_a_mark", "crates/redact/src/tests.rs::redacted_xobjects_do_not_survive_through_inherited_or_shared_resources"] +notes = "confirmation; one undo step; next save is a full rewrite. Refused, with nothing changed, when the page still shows text under a mark afterwards: text in some predefined CMaps and rotated Type 3 fonts is not removed yet, so those documents cannot be redacted. Also refused on a page with an inline image the renderer reads no image for, since the operators after it can't be known. Removed images and forms are unlinked from page-tree resources (own, inherited or shared) and from surviving forms' resources, not yet from annotation appearances or patterns" [[feature]] id = "protect.redact-glyph-removal" From 39844f06b58bc96bae72d3fb56faaa3b6201c945 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Thu, 22 Oct 2026 15:39:48 -0400 Subject: [PATCH 42/42] Release 0.12.0 Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 72 +++++++++++++++++++++++++++--------------------------- Cargo.toml | 68 +++++++++++++++++++++++++-------------------------- 2 files changed, 70 insertions(+), 70 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e318b7a77c..6518430017 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3936,7 +3936,7 @@ dependencies = [ [[package]] name = "pdfcraft" -version = "0.11.0" +version = "0.12.0" dependencies = [ "chrono", "craft-quit", @@ -3962,7 +3962,7 @@ dependencies = [ [[package]] name = "pdfcraft-a11y" -version = "0.11.0" +version = "0.12.0" dependencies = [ "pdfcraft-content", "pdfcraft-cos", @@ -3974,7 +3974,7 @@ dependencies = [ [[package]] name = "pdfcraft-annot" -version = "0.11.0" +version = "0.12.0" dependencies = [ "hayro-syntax", "pdfcraft-cos", @@ -3985,7 +3985,7 @@ dependencies = [ [[package]] name = "pdfcraft-audit" -version = "0.11.0" +version = "0.12.0" dependencies = [ "pdfcraft-content", "pdfcraft-cos", @@ -3996,7 +3996,7 @@ dependencies = [ [[package]] name = "pdfcraft-automation" -version = "0.11.0" +version = "0.12.0" dependencies = [ "base64 0.22.1", "flate2", @@ -4018,7 +4018,7 @@ dependencies = [ [[package]] name = "pdfcraft-cli" -version = "0.11.0" +version = "0.12.0" dependencies = [ "base64 0.22.1", "pdfcraft-audit", @@ -4036,11 +4036,11 @@ dependencies = [ [[package]] name = "pdfcraft-compare" -version = "0.11.0" +version = "0.12.0" [[package]] name = "pdfcraft-content" -version = "0.11.0" +version = "0.12.0" dependencies = [ "hayro-syntax", "pdfcraft-cos", @@ -4048,7 +4048,7 @@ dependencies = [ [[package]] name = "pdfcraft-cos" -version = "0.11.0" +version = "0.12.0" dependencies = [ "chrono", "hayro-syntax", @@ -4063,7 +4063,7 @@ dependencies = [ [[package]] name = "pdfcraft-create" -version = "0.11.0" +version = "0.12.0" dependencies = [ "encoding_rs", "hayro-syntax", @@ -4079,7 +4079,7 @@ dependencies = [ [[package]] name = "pdfcraft-crypt" -version = "0.11.0" +version = "0.12.0" dependencies = [ "aes", "md-5", @@ -4091,7 +4091,7 @@ dependencies = [ [[package]] name = "pdfcraft-edit" -version = "0.11.0" +version = "0.12.0" dependencies = [ "hayro-syntax", "kurbo 0.13.1", @@ -4106,7 +4106,7 @@ dependencies = [ [[package]] name = "pdfcraft-engine" -version = "0.11.0" +version = "0.12.0" dependencies = [ "chrono", "craft-docpath", @@ -4147,14 +4147,14 @@ dependencies = [ [[package]] name = "pdfcraft-export" -version = "0.11.0" +version = "0.12.0" dependencies = [ "flate2", ] [[package]] name = "pdfcraft-filters" -version = "0.11.0" +version = "0.12.0" dependencies = [ "flate2", "proptest", @@ -4164,7 +4164,7 @@ dependencies = [ [[package]] name = "pdfcraft-fonts" -version = "0.11.0" +version = "0.12.0" dependencies = [ "harfrust", "pdfcraft-cos", @@ -4174,7 +4174,7 @@ dependencies = [ [[package]] name = "pdfcraft-forms" -version = "0.11.0" +version = "0.12.0" dependencies = [ "hayro-syntax", "pdfcraft-content", @@ -4187,11 +4187,11 @@ dependencies = [ [[package]] name = "pdfcraft-geom" -version = "0.11.0" +version = "0.12.0" [[package]] name = "pdfcraft-js" -version = "0.11.0" +version = "0.12.0" dependencies = [ "boa_engine", "chrono", @@ -4199,7 +4199,7 @@ dependencies = [ [[package]] name = "pdfcraft-measure" -version = "0.11.0" +version = "0.12.0" dependencies = [ "pdfcraft-annot", "pdfcraft-content", @@ -4212,14 +4212,14 @@ dependencies = [ [[package]] name = "pdfcraft-model" -version = "0.11.0" +version = "0.12.0" dependencies = [ "pdfcraft-cos", ] [[package]] name = "pdfcraft-ocr" -version = "0.11.0" +version = "0.12.0" dependencies = [ "ocrs", "pdfcraft-fonts", @@ -4229,7 +4229,7 @@ dependencies = [ [[package]] name = "pdfcraft-optimize" -version = "0.11.0" +version = "0.12.0" dependencies = [ "image", "pdfcraft-annot", @@ -4242,11 +4242,11 @@ dependencies = [ [[package]] name = "pdfcraft-oracle" -version = "0.11.0" +version = "0.12.0" [[package]] name = "pdfcraft-organize" -version = "0.11.0" +version = "0.12.0" dependencies = [ "hayro-syntax", "pdfcraft-content", @@ -4257,7 +4257,7 @@ dependencies = [ [[package]] name = "pdfcraft-platform" -version = "0.11.0" +version = "0.12.0" dependencies = [ "async-channel", "futures-lite", @@ -4267,7 +4267,7 @@ dependencies = [ [[package]] name = "pdfcraft-preflight" -version = "0.11.0" +version = "0.12.0" dependencies = [ "flate2", "pdfcraft-content", @@ -4278,7 +4278,7 @@ dependencies = [ [[package]] name = "pdfcraft-print" -version = "0.11.0" +version = "0.12.0" dependencies = [ "craft-print", "pdfcraft-content", @@ -4290,7 +4290,7 @@ dependencies = [ [[package]] name = "pdfcraft-redact" -version = "0.11.0" +version = "0.12.0" dependencies = [ "pdfcraft-annot", "pdfcraft-content", @@ -4306,7 +4306,7 @@ dependencies = [ [[package]] name = "pdfcraft-render" -version = "0.11.0" +version = "0.12.0" dependencies = [ "bytemuck", "flate2", @@ -4325,7 +4325,7 @@ dependencies = [ [[package]] name = "pdfcraft-sign" -version = "0.11.0" +version = "0.12.0" dependencies = [ "aes", "aws-lc-rs", @@ -4360,11 +4360,11 @@ dependencies = [ [[package]] name = "pdfcraft-testkit" -version = "0.11.0" +version = "0.12.0" [[package]] name = "pdfcraft-ui-egui" -version = "0.11.0" +version = "0.12.0" dependencies = [ "arboard", "base64 0.22.1", @@ -4409,7 +4409,7 @@ dependencies = [ [[package]] name = "pdfcraft-web" -version = "0.11.0" +version = "0.12.0" dependencies = [ "eframe", "egui", @@ -4421,7 +4421,7 @@ dependencies = [ [[package]] name = "pdfcraft-xfa" -version = "0.11.0" +version = "0.12.0" dependencies = [ "base64 0.22.1", "image", @@ -4435,7 +4435,7 @@ dependencies = [ [[package]] name = "pdfcraft-xfdf" -version = "0.11.0" +version = "0.12.0" dependencies = [ "pdfcraft-annot", "pdfcraft-cos", diff --git a/Cargo.toml b/Cargo.toml index 2012403144..5841b50c68 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ exclude = ["vendor"] default-members = ["crates/*", "apps/pdfcraft", "apps/pdfcraft-cli", "xtask"] [workspace.package] -version = "0.11.0" +version = "0.12.0" edition = "2024" license = "MIT OR Apache-2.0" rust-version = "1.95" @@ -21,39 +21,39 @@ craft-print = { git = "https://github.com/storytold/craft-libs", version = "0.1. # Whether a path stored in a document may be opened (GoToR link targets). craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "82a24b006eefa036b43c09a67fb9a3fbd17ffe63" } # internal -pdfcraft-geom = { path = "crates/geom", version = "0.11.0" } -pdfcraft-filters = { path = "crates/filters", version = "0.11.0" } -pdfcraft-cos = { path = "crates/cos", version = "0.11.0" } -pdfcraft-crypt = { path = "crates/crypt", version = "0.11.0" } -pdfcraft-organize = { path = "crates/organize", version = "0.11.0" } -pdfcraft-measure = { path = "crates/measure", version = "0.11.0" } -pdfcraft-annot = { path = "crates/annot", version = "0.11.0" } -pdfcraft-audit = { path = "crates/audit", version = "0.11.0" } -pdfcraft-fonts = { path = "crates/fonts", version = "0.11.0" } -pdfcraft-model = { path = "crates/model", version = "0.11.0" } -pdfcraft-content = { path = "crates/content", version = "0.11.0" } -pdfcraft-redact = { path = "crates/redact", version = "0.11.0" } -pdfcraft-print = { path = "crates/print", version = "0.11.0" } -pdfcraft-xfdf = { path = "crates/xfdf", version = "0.11.0" } -pdfcraft-sign = { path = "crates/sign", version = "0.11.0" } -pdfcraft-optimize = { path = "crates/optimize", version = "0.11.0" } -pdfcraft-a11y = { path = "crates/a11y", version = "0.11.0" } -pdfcraft-oracle = { path = "crates/oracle", version = "0.11.0" } -pdfcraft-testkit = { path = "crates/testkit", version = "0.11.0" } -pdfcraft-ocr = { path = "crates/ocr", version = "0.11.0" } -pdfcraft-js = { path = "crates/js", version = "0.11.0" } -pdfcraft-compare = { path = "crates/compare", version = "0.11.0" } -pdfcraft-preflight = { path = "crates/preflight", version = "0.11.0" } -pdfcraft-export = { path = "crates/export", version = "0.11.0" } -pdfcraft-edit = { path = "crates/edit", version = "0.11.0" } -pdfcraft-create = { path = "crates/create", version = "0.11.0" } -pdfcraft-forms = { path = "crates/forms", version = "0.11.0" } -pdfcraft-xfa = { path = "crates/xfa", version = "0.11.0" } -pdfcraft-render = { path = "crates/render", version = "0.11.0" } -pdfcraft-engine = { path = "crates/engine", version = "0.11.0" } -pdfcraft-platform = { path = "crates/platform", version = "0.11.0" } -pdfcraft-ui-egui = { path = "crates/ui-egui", version = "0.11.0" } -pdfcraft-automation = { path = "crates/automation", version = "0.11.0", default-features = false } +pdfcraft-geom = { path = "crates/geom", version = "0.12.0" } +pdfcraft-filters = { path = "crates/filters", version = "0.12.0" } +pdfcraft-cos = { path = "crates/cos", version = "0.12.0" } +pdfcraft-crypt = { path = "crates/crypt", version = "0.12.0" } +pdfcraft-organize = { path = "crates/organize", version = "0.12.0" } +pdfcraft-measure = { path = "crates/measure", version = "0.12.0" } +pdfcraft-annot = { path = "crates/annot", version = "0.12.0" } +pdfcraft-audit = { path = "crates/audit", version = "0.12.0" } +pdfcraft-fonts = { path = "crates/fonts", version = "0.12.0" } +pdfcraft-model = { path = "crates/model", version = "0.12.0" } +pdfcraft-content = { path = "crates/content", version = "0.12.0" } +pdfcraft-redact = { path = "crates/redact", version = "0.12.0" } +pdfcraft-print = { path = "crates/print", version = "0.12.0" } +pdfcraft-xfdf = { path = "crates/xfdf", version = "0.12.0" } +pdfcraft-sign = { path = "crates/sign", version = "0.12.0" } +pdfcraft-optimize = { path = "crates/optimize", version = "0.12.0" } +pdfcraft-a11y = { path = "crates/a11y", version = "0.12.0" } +pdfcraft-oracle = { path = "crates/oracle", version = "0.12.0" } +pdfcraft-testkit = { path = "crates/testkit", version = "0.12.0" } +pdfcraft-ocr = { path = "crates/ocr", version = "0.12.0" } +pdfcraft-js = { path = "crates/js", version = "0.12.0" } +pdfcraft-compare = { path = "crates/compare", version = "0.12.0" } +pdfcraft-preflight = { path = "crates/preflight", version = "0.12.0" } +pdfcraft-export = { path = "crates/export", version = "0.12.0" } +pdfcraft-edit = { path = "crates/edit", version = "0.12.0" } +pdfcraft-create = { path = "crates/create", version = "0.12.0" } +pdfcraft-forms = { path = "crates/forms", version = "0.12.0" } +pdfcraft-xfa = { path = "crates/xfa", version = "0.12.0" } +pdfcraft-render = { path = "crates/render", version = "0.12.0" } +pdfcraft-engine = { path = "crates/engine", version = "0.12.0" } +pdfcraft-platform = { path = "crates/platform", version = "0.12.0" } +pdfcraft-ui-egui = { path = "crates/ui-egui", version = "0.12.0" } +pdfcraft-automation = { path = "crates/automation", version = "0.12.0", default-features = false } # pdf hayro = "0.7.1"