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 <noreply@anthropic.com>
Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
ymote 2026-10-11 15:38:48 -07:00 committed by GitHub
parent f61a1e5465
commit 287fce8f9c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 82 additions and 5 deletions

View File

@ -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.

View File

@ -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

View File

@ -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<ObjRef>,
entries: &mut Vec<LabelRange>,
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<Vec<LabelRange>, 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);

View File

@ -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());

View File

@ -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::<Vec<_>>());
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());