From 287fce8f9cfc9c501164c6a3bcc2ad0b6c4ef690 Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Sun, 11 Oct 2026 15:38:48 -0700 Subject: [PATCH] M2: bound the total size of page-label prefixes (#1146) Both page-label readers decode each range's prefix into a string of its own: render's inspection for its range table, Organize for the ranges it returns. Ranges may share one prefix object, so a small file could ask for a copy per range: within the 200,000-entry tree limit, about 200 MiB of copies of one 1,024-byte prefix. Decoded prefixes now take at most 4 MiB in all (MAX_PREFIX_TOTAL_BYTES, beside MAX_LABEL_TOTAL_BYTES). Past that the tree is refused before any label is made, as the other tree limits refuse it. Tests: 5,000 ranges sharing a 1,000-byte prefix are refused in both readers (relabelling such a tree changes nothing), and exactly 4 MiB of prefixes is still read. Co-authored-by: Claude Opus 5.5 Co-authored-by: Brandon Thomas --- ROADMAP.md | 1 + crates/cos/src/page_labels.rs | 5 +++++ crates/organize/src/labels.rs | 12 +++++++---- crates/organize/src/tests.rs | 29 +++++++++++++++++++++++++ crates/render/src/inspect.rs | 40 ++++++++++++++++++++++++++++++++++- 5 files changed, 82 insertions(+), 5 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 818c0497..352b0ba2 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -140,6 +140,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md Newest first: the date, what moved, and the new overall estimate. +- **2026-10-11 (M2, page-label prefixes):** The page-label readers bound the total size of decoded prefixes. Each range decodes its own prefix, and ranges may share one prefix object, so a small file could ask for a copy per range: within the 200,000-entry tree limit, about 200 MiB of copies of one 1,024-byte prefix. Inspection and Organize now refuse a tree whose decoded prefixes pass 4 MiB in total (`MAX_PREFIX_TOTAL_BYTES`), before any label is made, as the other tree limits do; relabelling such a tree changes nothing. Synthetic tests share one 1,000-byte prefix across 5,000 ranges and keep exactly 4 MiB of prefixes readable. Overall estimate unchanged. - **2026-10-11 (UI, checkboxes and radio buttons):** Checkboxes no longer look like radio buttons, and on the light theme neither is invisible. With the theme's 6 px widget radius egui drew a checkbox's 14 px box as a circle, so View ▸ Page display's Continuous scrolling and Show cover page read as two more layout choices; on the light theme an unticked box or an unselected radio button was white on white with no edge. Every checkbox (81) and radio button (79) now goes through `widgets::checkbox` / `widgets::radio` / `widgets::radio_value`: a 1 px outline (darker on hover), the plain fill on the light theme as dialogs already had, and for checkboxes a rounded-square box (3 px), keeping what the caller set (dialogs, the search panel). `clippy.toml` disallows `ui.checkbox`, `ui.radio`, `ui.radio_value`, `egui::Checkbox` and `egui::RadioButton` so new ones use them too. Estimates unchanged. - **2026-10-11 (M13, #971):** `text_find` now bounds each page's temporary match-range list to the remaining result limit plus one, which is enough to detect and return a continuation without allocating every match on a dense page. Added limited-search coverage; the existing end-to-end continuation test passes. The text cache byte budget and per-page extraction warnings remain open. - **2026-10-11 (M7, #594):** Ordinary paragraph body drags now use the existing source-preserving object translation path; edge drags still rewrap. An in-progress drag is discarded if the document generation changes, and added-content gestures keep their own edit handler. Added real-shell source regressions for overlapping and identical labels, history, save/reopen, partial overlap, kerning, rotation/crop/UserUnit and atomic refusal. Linux validation passed: focused UI/engine regressions, exact-base source-loss failures, native drag/undo/redo/save/reopen with independent Poppler renders, and all eight CI gates. Text replacement and resizing over unrelated overlapping content remain outside this repair. Estimates unchanged. diff --git a/crates/cos/src/page_labels.rs b/crates/cos/src/page_labels.rs index cf037bf1..557929f8 100644 --- a/crates/cos/src/page_labels.rs +++ b/crates/cos/src/page_labels.rs @@ -7,6 +7,11 @@ pub const MAX_LABEL_BYTES: usize = 1024; pub const MAX_PREFIX_BYTES: usize = 2 * MAX_LABEL_BYTES + 2; /// Sum of the UTF-8 bytes of custom labels for one document. pub const MAX_LABEL_TOTAL_BYTES: usize = 4 << 20; +/// Sum of the UTF-8 bytes of the decoded prefixes of one `/PageLabels` tree. Each range decodes +/// its own prefix, and ranges may share one prefix object, so without this a small file could ask +/// for a copy per range: [`MAX_LABEL_TREE_WORK`] ranges naming one 1,024-byte prefix would hold +/// about 200 MiB. Real documents stay far below it. +pub const MAX_PREFIX_TOTAL_BYTES: usize = 4 << 20; /// The most number-tree entries (nodes, kids and range pairs) read from one `/PageLabels` /// tree. Scaled for real documents: one range per page on a ten-thousand-page PDF happens /// (prefix labels; #307's file has 9,156 pages), so the cap is twenty times that while still diff --git a/crates/organize/src/labels.rs b/crates/organize/src/labels.rs index c351adc0..aa02be42 100644 --- a/crates/organize/src/labels.rs +++ b/crates/organize/src/labels.rs @@ -7,7 +7,7 @@ use std::collections::HashSet; -use pdfcraft_cos::page_labels::{MAX_LABEL_BYTES, MAX_LABEL_TREE_DEPTH, MAX_LABEL_TREE_WORK, MAX_PREFIX_BYTES, alpha, roman}; +use pdfcraft_cos::page_labels::{MAX_LABEL_BYTES, MAX_LABEL_TREE_DEPTH, MAX_LABEL_TREE_WORK, MAX_PREFIX_BYTES, MAX_PREFIX_TOTAL_BYTES, alpha, roman}; use pdfcraft_cos::{Dict, Document, ObjRef, Object, PdfString}; use crate::{OrganizeError, page_count, pages_root}; @@ -146,6 +146,7 @@ fn read_label_node( left: &mut usize, seen: &mut HashSet, entries: &mut Vec, + prefix_left: &mut usize, ) -> Result<(), OrganizeError> { label_work(left, 1)?; if depth > MAX_LABEL_TREE_DEPTH { @@ -179,6 +180,9 @@ fn read_label_node( if prefix.len() > MAX_LABEL_BYTES { return Err(invalid_label("a decoded prefix exceeds 1024 UTF-8 bytes")); } + // A prefix object shared by many ranges is copied into each: count every copy. + *prefix_left = + prefix_left.checked_sub(prefix.len()).ok_or_else(|| invalid_label("decoded prefixes exceed 4 MiB in total"))?; let style = spec.name(b"S").map_or(LabelStyle::None, LabelStyle::from_name); let first = spec.int(b"St").unwrap_or(1).max(1); let first = if style == LabelStyle::None { @@ -203,7 +207,7 @@ fn read_label_node( } // Recursion is bounded before descending, and arrays are borrowed throughout. for kid in kids { - read_label_node(doc, kid, depth + 1, left, seen, entries)?; + read_label_node(doc, kid, depth + 1, left, seen, entries, prefix_left)?; } Ok(()) })?; @@ -217,8 +221,8 @@ fn checked_ranges(doc: &Document) -> Result, OrganizeError> { let catalog = doc.get(root); let Some(tree) = catalog.as_dict().and_then(|c| c.get(b"PageLabels")) else { return Ok(Vec::new()) }; let mut entries = Vec::new(); - let mut left = MAX_LABEL_TREE_WORK; - read_label_node(doc, tree, 0, &mut left, &mut HashSet::new(), &mut entries)?; + let (mut left, mut prefix_left) = (MAX_LABEL_TREE_WORK, MAX_PREFIX_TOTAL_BYTES); + read_label_node(doc, tree, 0, &mut left, &mut HashSet::new(), &mut entries, &mut prefix_left)?; // A start page given twice keeps its later range, as readers that overwrite do (the sort is // stable, so the later one is last among equals). entries.sort_by_key(|r| r.start); diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index 50f35ebe..1014ddba 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -1477,6 +1477,35 @@ fn number_pages_past_four_mib_in_total_is_accepted() { assert_eq!(crate::page_labels(&d).unwrap()[4_999], format!("{}5000", "x".repeat(1000))); } +/// Each range keeps its own decoded prefix, so a small file sharing one prefix object asks for a +/// copy per range (about 200 MiB at the tree-work limit). Past 4 MiB of decoded prefixes in all +/// the tree is refused, and so is relabelling it, which changes nothing. +#[test] +fn page_label_prefixes_are_bounded_in_total() { + // `ranges` ranges naming one shared prefix object of `prefix` bytes, without a style. + let shared = |ranges: i64, prefix: usize| { + let mut d = doc_a(); + let prefix = d.add(Object::String(PdfString::text(&"x".repeat(prefix)))); + let mut spec = Dict::new(); + spec.set(b"P".to_vec(), Object::Ref(prefix)); + let spec = d.add(Object::Dict(spec)); + let mut tree = Dict::new(); + tree.set(b"Nums".to_vec(), Object::Array((0..ranges).flat_map(|i| [Object::Int(i), Object::Ref(spec)]).collect())); + let tree = d.add(Object::Dict(tree)); + d.update_dict(d.root().unwrap(), |c| c.set(b"PageLabels".to_vec(), Object::Ref(tree))).unwrap(); + d + }; + let mut d = shared(5_000, 1_000); + let e = crate::page_labels(&d).unwrap_err().to_string(); + assert!(e.contains("decoded prefixes exceed 4 MiB in total"), "{e}"); + let before = d.modified_objects(); + assert!(crate::number_pages(&mut d, 0, 1, crate::LabelStyle::Decimal, "", 1).is_err()); + assert_eq!(d.modified_objects(), before, "a refused relabelling changes nothing"); + // Exactly 4 MiB of prefixes is still read. + let max = pdfcraft_cos::page_labels::MAX_PREFIX_TOTAL_BYTES / 1024; + assert!(crate::page_labels(&shared(max as i64, 1024)).unwrap().iter().all(|l| l.len() == 1024)); +} + #[test] fn page_boxes_default_inherit_and_set() { let mut doc = open(fixture()); diff --git a/crates/render/src/inspect.rs b/crates/render/src/inspect.rs index 1343c2b5..ebe68762 100644 --- a/crates/render/src/inspect.rs +++ b/crates/render/src/inspect.rs @@ -14,7 +14,9 @@ use hayro::hayro_syntax::DecryptionError; use hayro::hayro_syntax::LoadPdfError; use hayro::hayro_syntax::Pdf; use lopdf::{Dictionary, Document, LoadOptions, Object, ObjectId}; -use pdfcraft_cos::page_labels::{MAX_LABEL_BYTES, MAX_LABEL_TOTAL_BYTES, MAX_LABEL_TREE_DEPTH, MAX_LABEL_TREE_WORK, MAX_PREFIX_BYTES, alpha, roman}; +use pdfcraft_cos::page_labels::{ + MAX_LABEL_BYTES, MAX_LABEL_TOTAL_BYTES, MAX_LABEL_TREE_DEPTH, MAX_LABEL_TREE_WORK, MAX_PREFIX_BYTES, MAX_PREFIX_TOTAL_BYTES, alpha, roman, +}; use crate::OpenError; use crate::structure::{LazyStructure, Structure}; @@ -814,6 +816,7 @@ impl<'a> Inspector<'a> { } // Decode each bounded prefix once, rather than once for every page in its range. let mut specs = BTreeMap::new(); + let mut prefix_total = 0usize; for (start, d) in ranges { let first = d.get(b"St").ok().and_then(|o| self.resolve(o).as_i64().ok()).unwrap_or(1).max(1) as u64; let prefix = match d.get(b"P").ok().map(|o| self.resolve(o)) { @@ -828,6 +831,11 @@ impl<'a> Inspector<'a> { if prefix.len() > MAX_LABEL_BYTES { return Err("a decoded prefix exceeds 1024 UTF-8 bytes"); } + // A prefix object shared by many ranges is decoded once per range: count every copy. + prefix_total = prefix_total.saturating_add(prefix.len()); + if prefix_total > MAX_PREFIX_TOTAL_BYTES { + return Err("decoded prefixes exceed 4 MiB in total"); + } let style = d.get(b"S").ok().and_then(|o| self.resolve(o).as_name().ok()); specs.insert(start, (first, prefix, style)); } @@ -1770,6 +1778,36 @@ mod tests { assert!(pages.iter().all(|p| p.label.len() == MAX_LABEL_BYTES)); } + #[test] + fn page_label_limits_bound_the_total_of_decoded_prefixes() { + // `ranges` ranges naming one shared prefix object of `prefix` bytes, without a style. + let shared = |ranges: i64, prefix: usize| { + let mut doc = Document::with_version("1.7"); + doc.objects.insert((7, 0), Object::String(vec![b'x'; prefix], lopdf::StringFormat::Literal)); + let mut spec = Dictionary::new(); + spec.set("P", Object::Reference((7, 0))); + doc.objects.insert((8, 0), Object::Dictionary(spec)); + let mut tree = Dictionary::new(); + tree.set("Nums", (0..ranges).flat_map(|i| [Object::Integer(i), Object::Reference((8, 0))]).collect::>()); + let mut catalog = Dictionary::new(); + catalog.set("PageLabels", Object::Dictionary(tree)); + (doc, catalog) + }; + let mut pages: Vec<_> = + (1..=3).map(|n| PageInfo { width: 200.0, height: 300.0, crop: [0.0, 0.0, 200.0, 300.0], rotation: 0, label: n.to_string() }).collect(); + // Each range decodes its prefix, so a small file sharing one prefix object asks for a + // copy per range: 5,000 ranges and a 1,000-byte prefix make 5 MB, past the 4 MiB total. + // Three pages make three labels, far below the label total: the prefixes alone refuse it. + let (doc, catalog) = shared(5000, 1000); + let err = Inspector::new(&doc).page_labels(&catalog, &mut pages).unwrap_err(); + assert!(err.contains("prefixes exceed 4 MiB"), "{err}"); + assert!(pages.iter().enumerate().all(|(i, p)| p.label == (i + 1).to_string())); + // Exactly 4 MiB of prefixes is still read. + let (doc, catalog) = shared((MAX_PREFIX_TOTAL_BYTES / MAX_LABEL_BYTES) as i64, MAX_LABEL_BYTES); + Inspector::new(&doc).page_labels(&catalog, &mut pages).unwrap(); + assert!(pages.iter().all(|p| p.label.len() == MAX_LABEL_BYTES)); + } + #[test] fn page_label_limits_handle_prefix_only_and_reference_cycles() { let info = label_info(vec![Object::Integer(0), label_spec("", b"prefix".to_vec(), i64::MAX)], Vec::new());