From 95729daef0684fe5ccddc6eea60bfa932d006f69 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 20:04:13 +0000 Subject: [PATCH] address review: a pop-up whose parent is a redaction mark is skipped; named-destination lookup is bounded --- crates/organize/src/comments.rs | 35 ++++++++++++++++++++++++++++++++- crates/organize/src/outline.rs | 14 ++++++++++++- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/crates/organize/src/comments.rs b/crates/organize/src/comments.rs index 0fd93a62a0..986492432b 100644 --- a/crates/organize/src/comments.rs +++ b/crates/organize/src/comments.rs @@ -352,7 +352,16 @@ impl PageAnnots { /// Whether `o` is an annotation whose `/Subtype` (resolved: it may be an indirect name) is /// `Popup`. fn is_popup(doc: &Document, o: &Object) -> bool { - doc.resolve(o).as_dict().and_then(|d| d.get(b"Subtype").map(|s| doc.resolve(s))).is_some_and(|s| s.as_name() == Some(b"Popup".as_slice())) + let resolved = doc.resolve(o); + let Some(d) = resolved.as_dict() else { return false }; + if !d.get(b"Subtype").map(|s| doc.resolve(s)).is_some_and(|s| s.as_name() == Some(b"Popup".as_slice())) { + return false; + } + // A pop-up whose `/Parent` is a redaction mark would copy the mark along through it: it is not + // taken (the comment keeps no `/Popup`) rather than copied without its parent. + !d.get(b"Parent").map(|p| doc.resolve(p)).is_some_and(|p| { + p.as_dict().and_then(|d| d.get(b"Subtype").map(|s| doc.resolve(s))).is_some_and(|s| s.as_name() == Some(b"Redact".as_slice())) + }) } #[cfg(test)] @@ -726,6 +735,30 @@ mod tests { assert!(one.iter().any(|(r, _)| *r == popup), "the pop-up with an indirect /Subtype is listed"); } + /// A pop-up whose `/Parent` is a redaction mark is skipped with the link to it, and the mark + /// does not come along through the pop-up. + #[test] + fn a_popup_whose_parent_is_a_redaction_mark_is_skipped() { + let src = build(&[ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >>", + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>", + "<< /Type /Annot /Subtype /Text /Contents (carrier) /Rect [10 10 30 30] /Popup 5 0 R /P 3 0 R >>", + "<< /Type /Annot /Subtype /Popup /Rect [40 40 200 120] /Parent 6 0 R /P 3 0 R >>", + "<< /Type /Annot /Subtype /Redact /Rect [0 200 20 220] /Contents (MARK-SECRET) /P 3 0 R >>", + ]); + let mut doc = build(&[ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >>", + "<< /Type /Page /Parent 2 0 R >>", + ]); + import_comments(&mut doc, &src).unwrap(); + let one = page_annots(&doc, 0); + assert!(by_contents(&one, "carrier").1.get(b"Popup").is_none()); + let all = pdfcraft_cos::write_full(&doc, &pdfcraft_cos::SaveOptions { object_streams: false, ..Default::default() }).unwrap(); + assert!(!all.windows(11).any(|w| w == b"MARK-SECRET"), "the mark was copied"); + } + #[test] fn comment_with_null_irt_and_popup_with_indirect_subtype_are_imported() { let src = build(&[ diff --git a/crates/organize/src/outline.rs b/crates/organize/src/outline.rs index 150402a272..72b1e296bb 100644 --- a/crates/organize/src/outline.rs +++ b/crates/organize/src/outline.rs @@ -23,6 +23,10 @@ pub(crate) const LIST_DEPTH: usize = 32; /// Deepest name-tree node read when looking up named destinations. const NAME_TREE_DEPTH: usize = 32; +/// Most name-tree nodes, entries and key bytes one lookup of named destinations reads. +const MAX_DEST_NODES: usize = 200_000; +const MAX_DEST_ENTRIES: usize = 100_000; +const MAX_DEST_KEY_BYTES: usize = 16 << 20; /// Deepest chain of indirect or named destinations followed for one bookmark. const DEST_DEPTH: u8 = 8; @@ -213,8 +217,12 @@ fn name_tree(doc: &Document) -> HashMap, Object> { true } }; + let (mut visited, mut key_bytes) = (0usize, 0usize); while let Some((raw, depth)) = stack.pop() { - if depth > NAME_TREE_DEPTH { + // Past any of these limits nothing more is read: one bookmark's lookup is bounded, and a + // destination beyond them is not found (the bookmark then has none). + visited += 1; + if depth > NAME_TREE_DEPTH || visited > MAX_DEST_NODES || out.len() >= MAX_DEST_ENTRIES || key_bytes > MAX_DEST_KEY_BYTES { continue; } if let Object::Ref(r) = raw @@ -230,6 +238,10 @@ fn name_tree(doc: &Document) -> HashMap, Object> { if let [k, v] = pair && let Some(key) = name_key(doc, k) { + key_bytes = key_bytes.saturating_add(key.len()); + if out.len() >= MAX_DEST_ENTRIES || key_bytes > MAX_DEST_KEY_BYTES { + break; + } out.entry(key).or_insert_with(|| v.clone()); } }