Integration: the page's decode budget owns the form count in redaction (#3670 × main)

#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.
This commit is contained in:
aiongg
2026-10-24 07:11:16 +00:00
parent 692f34fce2
commit 2a91f1d717
2 changed files with 15 additions and 21 deletions
+4 -9
View File
@@ -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)
+11 -12
View File
@@ -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::<Vec<_>>().join(" ");
let xobjects = (0..=max).map(|depth| format!("/F{depth} {} 0 R", 7 + depth)).collect::<Vec<_>>().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),