From 2a91f1d717a42bb62e9a26eeaea7468222b2af6a Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 24 Oct 2026 06:57:45 +0000 Subject: [PATCH] =?UTF-8?q?Integration:=20the=20page's=20decode=20budget?= =?UTF-8?q?=20owns=20the=20form=20count=20in=20redaction=20(#3670=20=C3=97?= =?UTF-8?q?=20main)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #3670 raised the form depth limit to 50 and added its own 50,000-visit cap. Main's per-page DecodeBudget (e2a677164) has the same 50,000-form limit, and the visit cap fired first, so Apply removed the forms instead of refusing the page as main does. The visit cap goes; the fan-out test expects the refusal, and the depth-limit sanitization test builds its chain from MAX_DEPTH. --- crates/redact/src/interp.rs | 13 ++++--------- crates/redact/src/tests.rs | 23 +++++++++++------------ 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/crates/redact/src/interp.rs b/crates/redact/src/interp.rs index ee772cd1a..5b91bb64c 100644 --- a/crates/redact/src/interp.rs +++ b/crates/redact/src/interp.rs @@ -41,7 +41,6 @@ pub(crate) const MAX_DEPTH: usize = 50; /// at 50 is a hang rather than a limit. Edit text's reader pairs its depth limit with the same /// budget, and this is that constant, so the two cannot drift apart. Running out is treated as a /// form that cannot be read: removed in Apply, counted as unreadable in Verify. -pub(crate) const MAX_FORM_VISITS: usize = pdfcraft_edit::text::MAX_FORM_VISITS; // A form a reader still shows must be walkable here, or a redaction removes content that is // visible; the build says so rather than a test, so the two limits cannot drift apart. @@ -129,8 +128,6 @@ pub(crate) struct Scope<'a> { /// shared forms drawn from other shared forms would otherwise multiply the work with every /// level. budget: DecodeBudget, - /// Forms entered so far in this pass ([`MAX_FORM_VISITS`]). - visits: usize, } impl<'a> Scope<'a> { @@ -152,7 +149,6 @@ impl<'a> Scope<'a> { used_names: Vec::new(), depth: 0, budget, - visits: 0, } } @@ -1291,10 +1287,10 @@ fn xobject( { return None; } - // Too deep, or too many forms for one page: either way this form cannot be read, so - // it goes (Apply) or the page is left alone (Verify), as a form that will not decode - // does. The budget is what keeps a fan-out or a cycle from costing 2^50 walks. - if scope.depth >= MAX_DEPTH || scope.visits >= MAX_FORM_VISITS { + // Too deep: this form cannot be read, so it goes (Apply) or the page is left alone + // (Verify), as a form that will not decode does. The page's decode budget below is what + // keeps a fan-out or a cycle from costing 2^50 walks. + if scope.depth >= MAX_DEPTH { if scope.mode == Mode::Verify { // Sanitization removes a Form it cannot enter, so the verification scan // must count that conservative removal or it may decide to skip the pass. @@ -1308,7 +1304,6 @@ fn xobject( out.doomed.push(r); return Some(None); } - scope.visits += 1; // Strict, like page content: a Form read only in part counts as undecodable. let data = if scope.budget.enter_form() { scope.budget.decode_strict(s) diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 9065826fb..d89b3c608 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -347,7 +347,7 @@ fn a_deeply_nested_form_keeps_the_text_nobody_marked() { /// can send. The pass also counts the forms it enters, and a page past that budget is treated as /// one it cannot read. #[test] -fn a_form_that_fans_out_or_loops_is_bounded_by_the_visit_budget() { +fn a_form_that_fans_out_or_loops_is_bounded_by_the_page_budget() { /// A page drawing `Fm1`, where each of `n` forms draws the next **twice** and the last draws /// text: 2^n paths if nothing stops the walk. With `looping`, the last form draws the first /// one again instead, so the chain has no end at all. @@ -383,13 +383,12 @@ fn a_form_that_fans_out_or_loops_is_bounded_by_the_visit_budget() { let start = std::time::Instant::now(); let mut doc = fan_out(crate::interp::MAX_DEPTH, looping); mark(&mut doc, 0, &[[5.0, 275.0, 60.0, 295.0]], ""); - let report = apply(&mut doc, None).unwrap(); + let result = apply(&mut doc, None); let took = start.elapsed(); assert!(took < std::time::Duration::from_secs(20), "looping {looping}: the walk took {took:?}"); - // The page was still redacted, and the budget stopped the walk rather than the pass. - assert!(report.forms_removed > 0, "looping {looping}: the forms past the budget are removed"); - let c = content(&doc, 0); - assert!(!c.contains("(HEAD)"), "looping {looping}: the marked text is gone: {c}"); + // The page's form budget stops the walk, and Apply refuses the page rather than + // removing forms it could not read. + assert!(result.is_err(), "looping {looping}: the page past its form budget is refused"); } } @@ -824,19 +823,19 @@ fn form_chain_at_max_depth_is_counted_and_removed_by_hidden_layer_sanitization() use crate::sanitize::{Hidden, remove_hidden, scan}; const PRIVATE: &[u8] = b"PRIVATE_FORM_BEYOND_MAX_DEPTH"; - // MAX_DEPTH is 12. The final Form is encountered at that depth and is removed - // conservatively without being entered. Verify must count that removal or sanitize - // can skip the HiddenLayers pass entirely. + // The final Form is encountered at MAX_DEPTH and is removed conservatively without being + // entered. Verify must count that removal or sanitize can skip the HiddenLayers pass entirely. + let max = crate::interp::MAX_DEPTH; let mut forms = Vec::new(); - for depth in 0..=12 { - let content = if depth == 12 { + for depth in 0..=max { + let content = if depth == max { b"BT /F1 10 Tf 10 100 Td (PRIVATE_FORM_BEYOND_MAX_DEPTH) Tj ET".to_vec() } else { format!("/F{} Do", depth + 1).into_bytes() }; forms.push(stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300]", &content)); } - let xobjects = (0..=12).map(|depth| format!("/F{depth} {} 0 R", 7 + depth)).collect::>().join(" "); + let xobjects = (0..=max).map(|depth| format!("/F{depth} {} 0 R", 7 + depth)).collect::>().join(" "); let mut doc = one_page(b"/F0 Do", &format!("/XObject << {xobjects} >>"), forms); assert!( write_full(&doc, &SaveOptions::default()).unwrap().windows(PRIVATE.len()).any(|w| w == PRIVATE),