From ab7b5848116c3887fecae09d2e7f1791fe5dd1e9 Mon Sep 17 00:00:00 2001 From: Hamza Date: Tue, 20 Oct 2026 19:25:01 +0500 Subject: [PATCH] Redaction: follow nested forms as deep as the readers do A form past the pass's depth limit is removed rather than walked. That is the fail-safe choice for a redaction -- marked content inside a form we cannot read would otherwise survive -- but it also removes whatever else that form drew, marked or not. At a limit of 12, a page with a 13-deep chain lost the text of its innermost form with nothing in it marked. Measured on a page that draws HEAD, a chain of forms whose innermost draws DEEP, then TAIL, with only HEAD marked: chain before after (12) after (50) 1 HEAD DEEP TAIL DEEP TAIL DEEP TAIL 12 HEAD DEEP TAIL DEEP TAIL DEEP TAIL 13 HEAD DEEP TAIL TAIL DEEP TAIL 14 HEAD DEEP TAIL TAIL DEEP TAIL 50 HEAD DEEP TAIL TAIL DEEP TAIL 51 HEAD TAIL TAIL TAIL The limit is now 50, which is what the renderer follows (MAX_NESTED_INTERPRETATION_DEPTH) and what Edit text's reader follows (text::MAX_FORM_DEPTH). So anything a reader can see, this pass can walk. The last row is why 50 and not more: at 51 the renderer does not draw DEEP either, so removing that form costs nothing visible and the fail-safe still holds. Fixes #3645's data loss. The other half of that report -- that nothing tells the user when a form is removed -- is untouched: Report.forms_removed is counted but the engine's edit pipeline does not carry it out, and the one channel that reaches the result (doc.edit_notices) is typed TextNotice, for text that could not be drawn. Widening that type or adding a channel is a design choice for the maintainers rather than something to slip into this fix. --- crates/redact/src/interp.rs | 10 ++++++- crates/redact/src/tests.rs | 53 +++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index 52ca6c276..3d03878e8 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -26,7 +26,15 @@ use pdfcraft_cos::{CosError, Dict, Document, ObjRef, Object, Stream}; use crate::{RedactError, Report, image, page_streams, tags}; use pdfcraft_fonts::pdf::Metrics; -const MAX_DEPTH: usize = 12; +/// How deep this pass follows form XObjects. A form past it is removed rather than walked, which +/// is the fail-safe choice for a redaction -- marked content inside a form we cannot read would +/// otherwise survive -- but it also removes whatever else that form drew, marked or not. So the +/// limit has to reach as deep as anything the reader can see: the renderer follows forms to 50 +/// (`MAX_NESTED_INTERPRETATION_DEPTH`) and so does Edit text's reader (`text::MAX_FORM_DEPTH`). +/// At 12, a page with a 13-deep chain lost the text of its innermost form even with nothing in it +/// marked (#3645); past 50 nothing is drawn by any of the three, so removing it costs nothing +/// visible. +const MAX_DEPTH: usize = 50; #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum Mode { diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index da1ecf12a..9d16fd534 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -288,6 +288,59 @@ fn vectors_are_removed_or_clipped_and_inline_images_go() { assert!(c.contains("150 150 m"), "the line elsewhere stays"); } +/// A chain of nested forms deeper than the pass can walk was removed from the page, taking +/// whatever else it drew with it even when nothing in it was marked. The limit now reaches as +/// deep as the renderer and Edit text read (50), so anything a reader can see can be walked; +/// past that nothing is drawn either, so removing it costs nothing visible (#3645). +#[test] +fn a_deeply_nested_form_keeps_the_text_nobody_marked() { + /// A page that draws `HEAD` at the top, then a chain of `depth` forms whose innermost draws + /// `DEEP`, then `TAIL`. Only `HEAD` is marked. + fn page_with_chain(depth: usize) -> Document { + let first = 7; + let mut objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + format!( + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> /XObject << /Fm1 {first} 0 R >> >> >>" + ) + .into_bytes(), + stream("", b"BT /F1 10 Tf 10 280 Td (HEAD) Tj ET /Fm1 Do BT /F1 10 Tf 10 10 Td (TAIL) Tj ET"), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + ]; + for k in 1..=depth { + let res = if k < depth { + format!("/Resources << /Font << /F1 5 0 R >> /XObject << /Fm{} {} 0 R >> >>", k + 1, first + k) + } else { + "/Resources << /Font << /F1 5 0 R >> >>".to_string() + }; + let body: Vec = if k < depth { format!("/Fm{} Do", k + 1).into_bytes() } else { b"BT /F1 10 Tf 10 150 Td (DEEP) Tj ET".to_vec() }; + objs.push(stream(&format!("/Type /XObject /Subtype /Form /BBox [0 0 300 300] {res}"), &body)); + } + pdf(objs) + } + + // Deeper than the old limit of 12 and within the new one: the innermost text survives. + for depth in [1, 12, 13, 20, 50] { + let mut doc = page_with_chain(depth); + mark(&mut doc, 0, &[[5.0, 275.0, 60.0, 295.0]], ""); + let report = apply(&mut doc, None).unwrap(); + assert_eq!(report.forms_removed, 0, "depth {depth}: a form was removed"); + let c = content(&doc, 0); + assert!(c.contains("(TAIL)"), "depth {depth}: {c}"); + // The innermost form's text is still in the saved file. + let saved = write_incremental(&doc, &SaveOptions::default()).unwrap(); + assert!(saved.windows(6).any(|w| w == b"(DEEP)"), "depth {depth}: the innermost form's text was deleted"); + } + + // Past what any reader follows, the form is still removed: the fail-safe, and nothing the + // renderer draws is lost because it stops at 50 too. + let mut doc = page_with_chain(51); + mark(&mut doc, 0, &[[5.0, 275.0, 60.0, 295.0]], ""); + assert_eq!(apply(&mut doc, None).unwrap().forms_removed, 1, "a chain past 50 is removed"); +} + #[test] fn shared_form_xobjects_are_copied_not_changed() { let form =