From c2c4db7eca99386ee0c90d2bf7dee2ae2f45cf89 Mon Sep 17 00:00:00 2001 From: eliets2 <287273517+eliets2@users.noreply.github.com> Date: Tue, 20 Oct 2026 17:31:48 +0300 Subject: [PATCH 1/2] redact: count shared outline items once so the walk ends An outline whose items name shared children (/First and /Next pointing at nodes that other items also name) was counted by following every path. With 60 levels of two items each, a walk that follows every path takes 2^60 steps. The outline count never finished, and Remove Hidden Information and Sanitize both run that count first. The count now keeps the set of items it has reached. An item reached a second time is not counted again, and the level stops there: the walk that reached the item first goes on to count the rest of that level. A total budget of 100,000 distinct items bounds the work for an outline with very many items. The per-level cap is unchanged. Ordinary outlines count as before. The existing hidden-information test (two bookmarks) passes unchanged, and a new nested outline test (five items) pins the count for nesting and siblings. Tests added in crates/redact/src/tests.rs: - an_outline_whose_items_share_children_is_counted_without_spinning - removing_bookmarks_from_an_outline_whose_items_share_children_finishes - an_outline_counts_no_more_items_than_its_total_budget - a_nested_outline_is_counted_item_by_item The first, second and third were seen to fail on the code before this change: the first two timed out after 10 seconds, and the third read 120000 where 100000 is expected. The fourth passes before and after the change and guards the ordinary count. The spin tests run the walk on a thread with a 10 second bound, so a regression fails the test instead of hanging the run. Reverting only the shared-item check, or only the budget check, fails the tests they pin. Checked with cargo fmt --all -- --check, clippy with -D warnings on pdfcraft-redact, and cargo test for pdfcraft-redact. --- crates/redact/src/sanitize.rs | 21 ++++++- crates/redact/src/tests.rs | 108 ++++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+), 2 deletions(-) diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index 1940e376a8..a56118da68 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -97,17 +97,34 @@ fn name_tree_len(doc: &Document, node: &Dict, depth: usize) -> usize { leaves + kids.iter().filter_map(|k| doc.resolve(k).as_dict().cloned()).map(|k| name_tree_len(doc, &k, depth + 1)).sum::() } +/// The most distinct outline items one count reads, over all levels together. +const OUTLINE_ITEM_BUDGET: usize = 100_000; + fn outline_len(doc: &Document, first: Option<&Object>, depth: usize) -> usize { + outline_len_seen(doc, first, depth, &mut std::collections::HashSet::new()) +} + +/// Counts an outline level and everything below it, each item once. `seen` holds the items already +/// counted. An item that two others name (as `/First` or `/Next`) is counted where the walk first +/// reaches it, and a level stops at an item already seen, because the walk that reached that item +/// counts the rest of its level. A hostile outline therefore costs one step per item, not 2^n steps +/// for n levels of shared children. +fn outline_len_seen(doc: &Document, first: Option<&Object>, depth: usize, seen: &mut std::collections::HashSet) -> usize { let mut n = 0; let mut cur = first.cloned(); let mut guard = 0; while let Some(o) = cur { guard += 1; - if guard > 100_000 || depth > 64 { + if guard > 100_000 || depth > 64 || seen.len() >= OUTLINE_ITEM_BUDGET { + break; + } + if let Object::Ref(r) = &o + && !seen.insert(*r) + { break; } let Some(d) = doc.resolve(&o).as_dict().cloned() else { break }; - n += 1 + outline_len(doc, d.get(b"First"), depth + 1); + n += 1 + outline_len_seen(doc, d.get(b"First"), depth + 1, seen); cur = d.get(b"Next").cloned(); } n diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index da1ecf12ab..ee76cc0863 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -727,6 +727,114 @@ fn hidden_information_is_counted_and_removed() { assert!(shown, "the field's value stays visible as page content"); } +/// A one-page document whose catalog names an outline root (object 5). `items` are the outline +/// items, objects 6 onward. +fn outlined_document(items: Vec>) -> Document { + let mut objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /Outlines 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"), + b"<< /Type /Outlines /First 6 0 R >>".to_vec(), + ]; + objs.extend(items); + pdf(objs) +} + +/// An outline of `levels` levels with two items each. Both items of a level name the first item +/// of the next level as their /First, so each of those items has two parents, and a walk that +/// follows every path takes 2^levels steps. Items a and b of level k are objects 6 + 2k and 7 + 2k. +fn hostile_outline(levels: usize) -> Document { + let mut items = Vec::new(); + for k in 0..levels { + let (a, b) = (6 + 2 * k, 7 + 2 * k); + let child = if k + 1 < levels { format!(" /First {} 0 R", a + 2) } else { String::new() }; + items.push(format!("<< /Title (a) /Next {b} 0 R{child} >>").into_bytes()); + items.push(format!("<< /Title (b){child} /Parent {a} 0 R >>").into_bytes()); + } + outlined_document(items) +} + +/// Runs `work` on another thread and returns its result. A walk that spins fails the test after +/// `bound` instead of hanging the whole run; the thread stays behind and ends with the process. +fn finishes_within(bound: std::time::Duration, work: impl FnOnce() -> T + Send + 'static) -> T { + let (tx, rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + let _ = tx.send(work()); + }); + match rx.recv_timeout(bound) { + Ok(out) => out, + Err(std::sync::mpsc::RecvTimeoutError::Timeout) => { + panic!("the walk did not finish within {bound:?}: it is spinning on shared outline items") + } + Err(std::sync::mpsc::RecvTimeoutError::Disconnected) => panic!("the walk panicked"), + } +} + +/// Generous for a slow machine: the bounded walk takes a few milliseconds. +const WALK_BOUND: std::time::Duration = std::time::Duration::from_secs(10); + +/// An outline whose items name shared children is counted item by item, each item once, and the +/// walk ends. Counting along every path instead takes 2^60 steps. +#[test] +fn an_outline_whose_items_share_children_is_counted_without_spinning() { + use crate::sanitize::{Hidden, scan}; + let doc = hostile_outline(60); + let counts = finishes_within(WALK_BOUND, move || scan(&doc)); + assert_eq!(counts.iter().find(|c| c.0 == Hidden::Bookmarks), Some(&(Hidden::Bookmarks, 120)), "60 levels of two distinct items"); +} + +/// Removing the bookmarks counts them through the same walk first, so it must finish too, and +/// it must take the outline out of the catalog. +#[test] +fn removing_bookmarks_from_an_outline_whose_items_share_children_finishes() { + use crate::sanitize::{Hidden, remove_hidden}; + let mut doc = hostile_outline(60); + let (done, outlines_left) = finishes_within(WALK_BOUND, move || { + let done = remove_hidden(&mut doc, &[Hidden::Bookmarks]); + let root = doc.root().unwrap(); + let left = doc.get(root).as_dict().is_some_and(|c| c.contains(b"Outlines")); + (done, left) + }); + assert_eq!(done.unwrap(), [(Hidden::Bookmarks, 120)]); + assert!(!outlines_left, "the outline is removed from the catalog"); +} + +/// Nesting and siblings: every item of an ordinary outline is counted once, as before. +#[test] +fn a_nested_outline_is_counted_item_by_item() { + use crate::sanitize::{Hidden, scan}; + let doc = outlined_document(vec![ + b"<< /Title (Chapter 1) /Parent 5 0 R /First 7 0 R /Last 8 0 R /Next 9 0 R >>".to_vec(), + b"<< /Title (1.1) /Parent 6 0 R /Next 8 0 R >>".to_vec(), + b"<< /Title (1.2) /Parent 6 0 R /Prev 7 0 R >>".to_vec(), + b"<< /Title (Chapter 2) /Parent 5 0 R /Prev 6 0 R /First 10 0 R >>".to_vec(), + b"<< /Title (2.1) /Parent 9 0 R >>".to_vec(), + ]); + assert_eq!(scan(&doc).iter().find(|c| c.0 == Hidden::Bookmarks), Some(&(Hidden::Bookmarks, 5))); +} + +/// Two levels of 60,000 distinct items: each level is under the per-level cap, so only the total +/// cap stops the count at 100,000 (without it the count would be 120,000). +#[test] +fn an_outline_counts_no_more_items_than_its_total_budget() { + use crate::sanitize::{Hidden, scan}; + let per_level = 60_000; + let mut items = Vec::with_capacity(2 * per_level); + // Item j is object 6 + j. Level one is items 0..per_level; its first item leads to level two. + for i in 0..per_level { + let next = if i + 1 < per_level { format!(" /Next {} 0 R", 7 + i) } else { String::new() }; + let first = if i == 0 { format!(" /First {} 0 R", 6 + per_level) } else { String::new() }; + items.push(format!("<< /Title (a){next}{first} >>").into_bytes()); + } + for i in 0..per_level { + let next = if i + 1 < per_level { format!(" /Next {} 0 R", 7 + per_level + i) } else { String::new() }; + items.push(format!("<< /Title (b){next} >>").into_bytes()); + } + let doc = outlined_document(items); + assert_eq!(scan(&doc).iter().find(|c| c.0 == Hidden::Bookmarks), Some(&(Hidden::Bookmarks, 100_000))); +} + /// Issue #2447: in a tagged PDF the structure tree points at the link and the comment it tags /// (`/OBJR`). Removing them must drop those references too, or the full rewrite keeps the /// annotations, the URL and the comment text included, while the page no longer lists them. From b97b38323272d44e506acf4966a7a8ea8fe9cb0f Mon Sep 17 00:00:00 2001 From: eliets2 <287273517+eliets2@users.noreply.github.com> Date: Tue, 20 Oct 2026 21:04:06 +0300 Subject: [PATCH 2/2] redact: count inline outline items against the total budget The total budget of the outline count (100,000 items) was checked against the number of indirect items the walk had reached. Items written inline, as a direct dictionary, were counted in the result but never entered that set, so an outline of 60,000 indirect siblings, each with one inline child, counted 120,000 items. The walk now keeps a separate running total of every item it counts, inline items included, and stops when that total reaches the budget. The set of indirect items is still used to count shared items once. Ordinary outlines count as before. The doc comment of the walk now states the depth limit in one sentence: an item first reached through a deeper path is not visited again from a shallower one, so items nested past depth 64 may stay uncounted. The depth behaviour is unchanged. Test added in crates/redact/src/tests.rs: inline_outline_children_count_towards_the_total_budget. It reads 120000 on the previous head and 100000 now. Reverting the budget check to the indirect-only count fails it; reverting the increment fails it and the existing total budget test. The other outline tests pass unchanged. Checked with cargo fmt --all -- --check, clippy with -D warnings on pdfcraft-redact, and cargo test for pdfcraft-redact (85 passed, 0 failed). --- crates/redact/src/sanitize.rs | 29 ++++++++++++++++++++--------- crates/redact/src/tests.rs | 16 ++++++++++++++++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index a56118da68..ef1a44c91d 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -101,21 +101,31 @@ fn name_tree_len(doc: &Document, node: &Dict, depth: usize) -> usize { const OUTLINE_ITEM_BUDGET: usize = 100_000; fn outline_len(doc: &Document, first: Option<&Object>, depth: usize) -> usize { - outline_len_seen(doc, first, depth, &mut std::collections::HashSet::new()) + let (mut seen, mut counted) = (std::collections::HashSet::new(), 0); + outline_len_seen(doc, first, depth, &mut seen, &mut counted) } -/// Counts an outline level and everything below it, each item once. `seen` holds the items already -/// counted. An item that two others name (as `/First` or `/Next`) is counted where the walk first -/// reaches it, and a level stops at an item already seen, because the walk that reached that item -/// counts the rest of its level. A hostile outline therefore costs one step per item, not 2^n steps -/// for n levels of shared children. -fn outline_len_seen(doc: &Document, first: Option<&Object>, depth: usize, seen: &mut std::collections::HashSet) -> usize { +/// Counts an outline level and everything below it, each item once. `seen` holds the indirect items +/// already counted, and `counted` every item counted so far, inline ones included, which the total +/// budget applies to. An item that two others name (as `/First` or `/Next`) is counted where the walk +/// first reaches it, and a level stops at an item already seen, because the walk that reached that +/// item counts the rest of its level. A hostile outline therefore costs one step per item, not 2^n +/// steps for n levels of shared children. The depth limit of 64 only matters for outlines nested that +/// deep: an item first reached through a deeper path is not visited again from a shallower one, so +/// items nested beyond the limit may stay uncounted. +fn outline_len_seen( + doc: &Document, + first: Option<&Object>, + depth: usize, + seen: &mut std::collections::HashSet, + counted: &mut usize, +) -> usize { let mut n = 0; let mut cur = first.cloned(); let mut guard = 0; while let Some(o) = cur { guard += 1; - if guard > 100_000 || depth > 64 || seen.len() >= OUTLINE_ITEM_BUDGET { + if guard > 100_000 || depth > 64 || *counted >= OUTLINE_ITEM_BUDGET { break; } if let Object::Ref(r) = &o @@ -124,7 +134,8 @@ fn outline_len_seen(doc: &Document, first: Option<&Object>, depth: usize, seen: break; } let Some(d) = doc.resolve(&o).as_dict().cloned() else { break }; - n += 1 + outline_len_seen(doc, d.get(b"First"), depth + 1, seen); + *counted += 1; + n += 1 + outline_len_seen(doc, d.get(b"First"), depth + 1, seen, counted); cur = d.get(b"Next").cloned(); } n diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index ee76cc0863..feb21c1660 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -835,6 +835,22 @@ fn an_outline_counts_no_more_items_than_its_total_budget() { assert_eq!(scan(&doc).iter().find(|c| c.0 == Hidden::Bookmarks), Some(&(Hidden::Bookmarks, 100_000))); } +/// Items written inline (not as indirect objects) count towards the total budget too. Each of +/// 60,000 indirect siblings has one inline child, so 120,000 items are reached, and the total must +/// still stop at 100,000. +#[test] +fn inline_outline_children_count_towards_the_total_budget() { + use crate::sanitize::{Hidden, scan}; + let siblings = 60_000; + let mut items = Vec::with_capacity(siblings); + for i in 0..siblings { + let next = if i + 1 < siblings { format!(" /Next {} 0 R", 7 + i) } else { String::new() }; + items.push(format!("<< /Title (a){next} /First << /Title (c) >> >>").into_bytes()); + } + let doc = outlined_document(items); + assert_eq!(scan(&doc).iter().find(|c| c.0 == Hidden::Bookmarks), Some(&(Hidden::Bookmarks, 100_000))); +} + /// Issue #2447: in a tagged PDF the structure tree points at the link and the comment it tags /// (`/OBJR`). Removing them must drop those references too, or the full rewrite keeps the /// annotations, the URL and the comment text included, while the page no longer lists them.