From 237838102eb2736707b61299a95d129259b9ece6 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 9 Nov 2026 05:19:46 -0500 Subject: [PATCH 1/8] fix(xfa): existing-field traversal shares a field's name among its queued children (#6080) --- crates/xfa/src/pdf.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/xfa/src/pdf.rs b/crates/xfa/src/pdf.rs index b8377cee9f..4e87da5a7a 100644 --- a/crates/xfa/src/pdf.rs +++ b/crates/xfa/src/pdf.rs @@ -110,8 +110,9 @@ fn existing_fields(doc: &Document, catalog: &Dict) -> (HashMap, let mut names = HashSet::new(); let Some(acro) = catalog.get(b"AcroForm").map(|a| doc.resolve(a)) else { return (out, names) }; let Some(fields) = acro.as_dict().and_then(|a| a.get(b"Fields")).map(|f| doc.resolve(f)) else { return (out, names) }; - let mut stack: Vec<(Object, String, bool, usize)> = - fields.as_array().into_iter().flatten().map(|f| (f.clone(), String::new(), false, 0)).collect(); + // The accumulated name is shared by the children it is queued for, not copied into each. + let mut stack: Vec<(Object, std::rc::Rc, bool, usize)> = + fields.as_array().into_iter().flatten().map(|f| (f.clone(), "".into(), false, 0)).collect(); let mut seen = 0usize; while let Some((field, prefix, parent_is_sig, depth)) = stack.pop() { seen += 1; @@ -124,7 +125,7 @@ fn existing_fields(doc: &Document, catalog: &Dict) -> (HashMap, // Generated to name a signature field: only what it holds is looked at. if depth < MAX_FIELD_DEPTH { let kids = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default(); - stack.extend(kids.into_iter().map(|k| (k, String::new(), false, depth + 1))); + stack.extend(kids.into_iter().map(|k| (k, "".into(), false, depth + 1))); } continue; } @@ -139,12 +140,12 @@ fn existing_fields(doc: &Document, catalog: &Dict) -> (HashMap, continue; } let name = match d.get(b"T").map(|t| doc.resolve(t)).and_then(|t| t.as_string().map(|t| t.to_text())) { - Some(t) if prefix.is_empty() => t, - Some(t) => format!("{prefix}.{t}"), + Some(t) if prefix.is_empty() => t.into(), + Some(t) => format!("{prefix}.{t}").into(), None => prefix, }; if !name.is_empty() { - names.insert(name.clone()); + names.insert(name.to_string()); } let is_sig = match d.name(b"FT") { Some(ft) => ft == b"Sig", @@ -154,7 +155,7 @@ fn existing_fields(doc: &Document, catalog: &Dict) -> (HashMap, if is_sig && let Some(value) = d.get(b"V").filter(|v| doc.resolve(v).as_dict().is_some()) { // The appearance is on the field when it is its own widget, or on its one widget. let appearance = d.get(b"AP").cloned().or_else(|| kids.iter().find_map(|k| doc.resolve(k).as_dict().and_then(|k| k.get(b"AP").cloned()))); - out.entry(name.clone()).or_insert(Signed { value: value.clone(), appearance }); + out.entry(name.to_string()).or_insert(Signed { value: value.clone(), appearance }); } if depth < MAX_FIELD_DEPTH { stack.extend(kids.into_iter().map(|k| (k, name.clone(), is_sig, depth + 1))); From 01c132a6bf65ed96632206b475d3bc8903902db4 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 9 Nov 2026 05:20:02 -0500 Subject: [PATCH 2/8] fix(xfa): strip reads a shared indirect Kids array once (#6081) --- crates/xfa/src/pdf/strip.rs | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/crates/xfa/src/pdf/strip.rs b/crates/xfa/src/pdf/strip.rs index 1d62f28b18..93f1a65307 100644 --- a/crates/xfa/src/pdf/strip.rs +++ b/crates/xfa/src/pdf/strip.rs @@ -151,6 +151,7 @@ pub fn strip(doc: &mut Document) -> Result { let fields = acro.get(b"Fields").map(|f| doc.resolve(f)).and_then(|f| f.as_array().cloned()).unwrap_or_default(); let mut seen = std::collections::HashSet::new(); let mut todo: Vec = Vec::new(); + let mut kids_seen = std::collections::HashSet::new(); for r in fields.iter().filter_map(Object::as_ref) { if seen.insert(r) && seen.len() <= MAX_STRIP_FIELDS { todo.push(r); @@ -177,7 +178,9 @@ pub fn strip(doc: &mut Document) -> Result { } })?; } - if let Some(kids) = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()) { + // An indirect /Kids array several fields share is read once: its entries are already queued. + let shared_seen = d.get(b"Kids").and_then(Object::as_ref).is_some_and(|k| !kids_seen.insert(k)); + if !shared_seen && let Some(kids) = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()) { for k in kids.iter().filter_map(Object::as_ref) { if seen.insert(k) && seen.len() <= MAX_STRIP_FIELDS { todo.push(k); @@ -283,3 +286,34 @@ pub fn existing_layout(doc: &Document) -> Option { warnings, }) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn fields_sharing_a_kids_array_still_lose_their_private_keys_once() { + let mut doc = Document::new_empty(); + let mut kid = Dict::new(); + kid.set(CLICK_KEY.to_vec(), Object::Int(1)); + let kid = doc.add(Object::Dict(kid)); + let kids = doc.add(Object::Array(vec![Object::Ref(kid)])); + let parents: Vec = (0..3) + .map(|_| { + let mut p = Dict::new(); + p.set(b"Kids".to_vec(), Object::Ref(kids)); + Object::Ref(doc.add(Object::Dict(p))) + }) + .collect(); + let mut acro = Dict::new(); + acro.set(b"XFA".to_vec(), Object::Array(Vec::new())); + acro.set(b"Fields".to_vec(), Object::Array(parents)); + let root = doc.root().unwrap(); + let mut catalog = doc.get(root).as_dict().cloned().unwrap(); + catalog.set(b"AcroForm".to_vec(), Object::Dict(acro)); + doc.set(root, Object::Dict(catalog)); + let out = strip(&mut doc).unwrap(); + assert_eq!(out.script_buttons, 1); + assert!(!doc.get(kid).as_dict().unwrap().contains(CLICK_KEY)); + } +} From 14ae96a2aca29d93f710c95895495237e0772d9a Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 9 Nov 2026 05:20:18 -0500 Subject: [PATCH 3/8] fix(print): a printed page's form shares the page's Resources when it adds no appearance (#6089) --- crates/print/src/imposition.rs | 16 ++++++++++++---- crates/print/src/tests.rs | 12 ++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/crates/print/src/imposition.rs b/crates/print/src/imposition.rs index 348ae151ba..19a75af32c 100644 --- a/crates/print/src/imposition.rs +++ b/crates/print/src/imposition.rs @@ -219,14 +219,22 @@ pub(crate) fn page_form( body.extend(format!("q {} {} {} {} {} {} cm /{name} Do Q\n", n(a0), n(b0), n(c0), n(d0), n(e0), n(f0)).bytes()); xobjects.set(name.into_bytes(), Object::Ref(ap)); } - if !xobjects.is_empty() { - res.set(b"XObject".to_vec(), Object::Dict(xobjects)); - } + // With no appearance added the form keeps the page's own Resources, shared by reference, + // rather than a deep copy per printed page. + let resources = match page.dict.get(b"Resources") { + Some(r) if k == 0 && doc.resolve(r).as_dict().is_some() => r.clone(), + _ => { + if !xobjects.is_empty() { + res.set(b"XObject".to_vec(), Object::Dict(xobjects)); + } + Object::Dict(res) + } + }; let mut d = Dict::new(); d.set(b"Type".to_vec(), Object::name("XObject")); d.set(b"Subtype".to_vec(), Object::name("Form")); d.set(b"BBox".to_vec(), Object::Array(crop.iter().map(|v| Object::Real(*v)).collect())); - d.set(b"Resources".to_vec(), Object::Dict(res)); + d.set(b"Resources".to_vec(), resources); if let Some(g) = page.dict.get(b"Group") { d.set(b"Group".to_vec(), g.clone()); } diff --git a/crates/print/src/tests.rs b/crates/print/src/tests.rs index 63d8a351e7..5b8954fee9 100644 --- a/crates/print/src/tests.rs +++ b/crates/print/src/tests.rs @@ -1285,3 +1285,15 @@ fn contents_pieces_join_with_a_space_across_a_trailing_comment() { let printed_ctrl = impose(&with_newline, &settings(vec![0], Layout::Size(SizeMode::Actual))).unwrap(); assert_eq!(form_strings(printed_ctrl), [b"HIDDEN".to_vec(), b"SHOWN".to_vec()]); } + +#[test] +fn a_page_form_without_annotations_shares_the_pages_indirect_resources() { + let mut doc = fixture(2); + let pages = pdfcraft_model::pages(&doc); + let res = doc.add(Object::Dict([(b"Font".to_vec(), Object::Dict(Dict::new()))].into_iter().collect())); + doc.update_dict(pages[1].obj, |d| d.set(b"Resources".to_vec(), Object::Ref(res))).unwrap(); + let pages = pdfcraft_model::pages(&doc); + let form = imposition::page_form(&mut doc, &pages, 1, Content::default(), &mut Default::default()).unwrap(); + let Object::Stream(s) = &*doc.get(form) else { panic!("form stream") }; + assert_eq!(s.dict.get(b"Resources"), Some(&Object::Ref(res))); +} From 3a900b8bd21de13f9de841796b452f716103eb3d Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 9 Nov 2026 05:20:34 -0500 Subject: [PATCH 4/8] fix(a11y): auto-tagging bounds the content it plans across all pages (#6090) --- crates/a11y/src/autotag.rs | 10 ++++++++++ crates/a11y/src/tests.rs | 17 +++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/crates/a11y/src/autotag.rs b/crates/a11y/src/autotag.rs index 5c0dc2bd19..1b898fdfda 100644 --- a/crates/a11y/src/autotag.rs +++ b/crates/a11y/src/autotag.rs @@ -9,6 +9,10 @@ use pdfcraft_cos::{DecodeBudget, Dict, Document, ObjRef, Object, Stream}; const MAX_PAGES: usize = 1_000; const MAX_PAGE_BYTES: usize = 16 * 1024 * 1024; const MAX_OPERATORS: usize = 100_000; +/// What all pages together may decode and keep: pages can share one stream, so the per-page limits +/// alone let a small file retain a plan for every page. Real documents stay far under these. +const MAX_TOTAL_BYTES: usize = 128 * 1024 * 1024; +const MAX_TOTAL_OPERATORS: usize = 2_000_000; const MAX_TEXT_OBJECTS: usize = 4_096; #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -186,6 +190,7 @@ fn plan(doc: &Document) -> Result, AutoTagError> { } let mut out = Vec::with_capacity(pages.len()); let mut total = 0usize; + let (mut total_bytes, mut total_ops) = (0usize, 0usize); for (index, page) in pages.iter().enumerate() { if page.dict.contains(b"StructParents") { return Err(unsupported(index, "page already has a structure parent key")); @@ -197,10 +202,15 @@ fn plan(doc: &Document) -> Result, AutoTagError> { return Err(unsupported(index, "annotations need separate structure tags")); } let bytes = content_bytes(doc, page, index)?; + total_bytes = total_bytes.saturating_add(bytes.len()); let parsed = parse(&bytes); if parsed.skipped > 0 || parsed.ops.len() > MAX_OPERATORS { return Err(unsupported(index, "content has malformed or too many operators")); } + total_ops = total_ops.saturating_add(parsed.ops.len()); + if total_bytes > MAX_TOTAL_BYTES || total_ops > MAX_TOTAL_OPERATORS { + return Err(unsupported(index, "the document has too much content to auto-tag in one edit")); + } let spans = text_spans(&parsed.ops, index)?; total = total.saturating_add(spans.len()); out.push(Plan { page: page.obj, ops: parsed.ops, spans }); diff --git a/crates/a11y/src/tests.rs b/crates/a11y/src/tests.rs index 66ae06fce4..e5bc3e425a 100644 --- a/crates/a11y/src/tests.rs +++ b/crates/a11y/src/tests.rs @@ -1763,3 +1763,20 @@ fn marking_a_tag_artifact_refuses_content_past_the_decode_budget() { assert_eq!(mark_tag_artifact(&mut doc, ObjRef::new(7, 0)), Err(ArtifactError::TooLarge)); assert_eq!(doc.get(ObjRef::new(3, 0)).as_dict().unwrap().get(b"Contents"), Some(&contents), "the page keeps its content"); } + +#[test] +fn autotag_text_refuses_pages_that_share_content_past_the_total_operator_allowance() { + // Each page is just under the per-page limit; sharing one stream makes the total 21 times it. + let content = format!("BT (a) Tj ET {}", "q Q ".repeat(49_990)); + let pages = 21; + let kids: Vec = (0..pages).map(|i| format!("{} 0 R", 4 + i)).collect(); + let mut bodies = vec![ + "<< /Type /Catalog /Pages 2 0 R >>".to_string(), + format!("<< /Type /Pages /Kids [{}] /Count {pages} /MediaBox [0 0 200 200] >>", kids.join(" ")), + stream(&content), + ]; + bodies.extend((0..pages).map(|_| "<< /Type /Page /Parent 2 0 R /Contents 3 0 R >>".to_string())); + let mut doc = pdf(&bodies, ""); + let Err(AutoTagError::Unsupported { reason, .. }) = autotag_text(&mut doc) else { panic!("expected a refusal") }; + assert!(reason.contains("too much content"), "{reason}"); +} From ce3c1ef805c093691b8bf2d6a917e92d43c6f911 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 9 Nov 2026 05:20:57 -0500 Subject: [PATCH 5/8] fix(sign): chain building hashes a certificate once for all candidate issuers (#6096) --- crates/sign/src/x509.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/sign/src/x509.rs b/crates/sign/src/x509.rs index 5dd03d086d..1cf586b3e8 100644 --- a/crates/sign/src/x509.rs +++ b/crates/sign/src/x509.rs @@ -415,10 +415,19 @@ impl Certificate { /// Whether `key` verifies this certificate's signature. pub fn signed_by(&self, key: &PublicKey) -> bool { + self.signed_by_cached(key, &mut None) + } + + /// [`Self::signed_by`] keeping the digest of the signed part in `digest`, so checking one + /// certificate against many candidate keys hashes it once. + fn signed_by_cached(&self, key: &PublicKey, digest: &mut Option>) -> bool { let Ok(alg) = Tlv::parse_all(&self.sig_alg) else { return false }; match keys::signature_algorithm(&alg) { Ok((scheme @ keys::Scheme::Ed25519, _)) => key.verify_message(scheme, &self.tbs, &self.signature).unwrap_or(false), - Ok((scheme, Some(digest))) => key.verify(scheme, digest, &digest.digest(&[&self.tbs]), &self.signature).unwrap_or(false), + Ok((scheme, Some(d))) => { + let hash = digest.get_or_insert_with(|| d.digest(&[&self.tbs])); + key.verify(scheme, d, hash, &self.signature).unwrap_or(false) + } _ => false, } } @@ -533,7 +542,8 @@ pub fn build_chain_noted<'a>(leaf: &'a Certificate, pool: &'a [Certificate], at: // CA certificates between `last` and the leaf: what a candidate's pathLenConstraint limits. let below = chain.len() - 1; let mut found = None; - for c in pool.iter().filter(|c| c.subject.raw == last.issuer.raw && !chain.contains(c) && last.signed_by(&c.public_key)) { + let mut digest = None; + for c in pool.iter().filter(|c| c.subject.raw == last.issuer.raw && !chain.contains(c) && last.signed_by_cached(&c.public_key, &mut digest)) { // Each reason is a whole sentence, not a phrase stitched into one: a language puts the // two names where it needs them. let why = if !c.may_issue() { From 910b3267740b5ff8963d761f599060524949faf5 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Mon, 9 Nov 2026 05:21:13 -0500 Subject: [PATCH 6/8] fix(render): the named-destination index spends key bytes from one allowance (#6097) --- crates/render/src/inspect/core.rs | 3 ++- crates/render/src/inspect/navigation.rs | 29 +++++++++++++++++++++---- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/crates/render/src/inspect/core.rs b/crates/render/src/inspect/core.rs index c5df792f23..16de483e04 100644 --- a/crates/render/src/inspect/core.rs +++ b/crates/render/src/inspect/core.rs @@ -169,7 +169,8 @@ impl<'a> Inspector<'a> { if let Some(tree) = doc.catalog().ok().and_then(|c| c.get(b"Names").ok()).and_then(|o| me.dict(o)).and_then(|n| n.get(b"Dests").ok()).and_then(|o| me.dict(o)) { - me.name_tree_raw(tree, &mut HashSet::new(), 0, &mut named); + let mut key_budget = MAX_LINK_ATTACHMENT_TEXT_BYTES; + me.name_tree_raw(tree, &mut HashSet::new(), 0, &mut named, &mut key_budget); } me.named = named; me diff --git a/crates/render/src/inspect/navigation.rs b/crates/render/src/inspect/navigation.rs index fb19526875..4c6e88400e 100644 --- a/crates/render/src/inspect/navigation.rs +++ b/crates/render/src/inspect/navigation.rs @@ -79,6 +79,7 @@ impl<'a> Inspector<'a> { seen: &mut HashSet<*const Dictionary>, depth: u32, out: &mut HashMap, &'a Object>, + key_budget: &mut usize, ) { if depth > 32 || !seen.insert(node as *const _) { return; @@ -87,18 +88,25 @@ impl<'a> Inspector<'a> { for pair in pairs.chunks(2) { if let [k, v] = pair { let key = match self.resolve(k) { - Object::String(s, _) => s.clone(), - Object::Name(n) => n.clone(), + Object::String(s, _) => s, + Object::Name(n) => n, _ => continue, }; - out.entry(key).or_insert(v); + // Every key is hashed, so its bytes are spent from one allowance for the whole + // tree (a key repeated by reference is charged each time); one past it is + // not indexed. Only a first occurrence is copied. + let Some(left) = key_budget.checked_sub(key.len()) else { continue }; + *key_budget = left; + if !out.contains_key(key.as_slice()) { + out.insert(key.clone(), v); + } } } } if let Ok(Object::Array(kids)) = node.get(b"Kids").map(|o| self.resolve(o)) { for k in kids { if let Some(d) = self.dict(k) { - self.name_tree_raw(d, seen, depth + 1, out); + self.name_tree_raw(d, seen, depth + 1, out, key_budget); } } } @@ -559,4 +567,17 @@ mod open_dest_tests { let shallow = ["<< /Kids [7 0 R] >>".to_string(), "<< /Kids [8 0 R] >>".into(), "<< /Names [(x) [4 0 R /Fit]] >>".into()]; assert_eq!(open_dest("/OpenAction (x) /Names << /Dests 6 0 R >>", &shallow), Some((1, DestView::Fit))); } + + #[test] + fn a_key_repeated_by_reference_is_charged_each_time_against_the_name_tree_allowance() { + let big = format!("<{}>", "41".repeat(1 << 20)); + let tree = |repeats: usize| { + let names = format!("{}(x) [4 0 R /Fit]", "7 0 R [4 0 R /Fit] ".repeat(repeats)); + [format!("<< /Names [{names}] >>"), big.clone()] + }; + let catalog = "/OpenAction (x) /Names << /Dests 6 0 R >>"; + assert_eq!(open_dest(catalog, &tree(2)), Some((1, DestView::Fit))); + // 17 MiB of repeats of one reference spend the 16 MiB allowance before the real key. + assert_eq!(open_dest(catalog, &tree(17)), None); + } } From e3f2f2ebf1a0ca78496b88fb84f59a86dc184a4a Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Mon, 9 Nov 2026 13:21:47 +0300 Subject: [PATCH 7/8] export: the Office zip writer outgrows the classic records with ZIP64 (#2554) --- crates/export/src/zip.rs | 316 ++++++++++++++++++++++++++++++++++----- 1 file changed, 277 insertions(+), 39 deletions(-) diff --git a/crates/export/src/zip.rs b/crates/export/src/zip.rs index a89d08cb7b..e15966c119 100644 --- a/crates/export/src/zip.rs +++ b/crates/export/src/zip.rs @@ -1,4 +1,6 @@ -//! A minimal zip writer (deflate or store, no zip64) for Office files. +//! A minimal zip writer (deflate or store) for Office files. Archives that outgrow the classic +//! end-of-central-directory record's 16-bit entry count or 32-bit sizes are written with the +//! ZIP64 records (APPNOTE 4.4), which every reader understands; small ones are unchanged. use std::io::Write; @@ -22,12 +24,102 @@ fn crc32(data: &[u8]) -> u32 { c ^ 0xFFFF_FFFF } -/// Builds a zip archive in memory. +/// The largest entry count the classic end-of-central-directory record holds (APPNOTE 4.3.12). +const MAX_CLASSIC_COUNT: u64 = u16::MAX as u64; +/// The largest size or offset the classic records hold: past it the field is a sentinel and the +/// ZIP64 record carries the real value (APPNOTE 4.4.1). +const MAX_CLASSIC_U32: u64 = u32::MAX as u64; + +/// The ZIP64 extended-information extra field (APPNOTE 4.5.3) holding `fields` — the values that +/// no longer fit their classic fields — in specification order. Empty when everything fits. +fn zip64_extra(fields: &[u64]) -> Vec { + let mut e = Vec::with_capacity(4 + 8 * fields.len()); + if !fields.is_empty() { + e.extend(0x0001u16.to_le_bytes()); + e.extend((8 * fields.len() as u16).to_le_bytes()); + for &f in fields { + e.extend(f.to_le_bytes()); + } + } + e +} + +/// The classic 32-bit field value for `value`: the value itself, or the 0xFFFFFFFF sentinel when it +/// no longer fits — including when it equals the sentinel, which a reader cannot tell from a real +/// value — in which case the ZIP64 extra field carries the real value (APPNOTE 4.4.3). +fn classic_u32(value: u64) -> u32 { + if value >= MAX_CLASSIC_U32 { u32::MAX } else { value as u32 } +} + +/// Builds one member's local file header and central directory entry. The sizes and `offset` are +/// parameters, not read from the body, so tests can inject values past the classic fields without +/// allocating the data. +fn entry_headers(name: &str, crc: u32, method: u16, compressed: u64, uncompressed: u64, offset: u64) -> (Vec, Vec) { + // A value at or above the sentinel cannot be held by a classic field, so it is offloaded. + let size_is_zip64 = uncompressed >= MAX_CLASSIC_U32 || compressed >= MAX_CLASSIC_U32; + // A local header that carries ZIP64 sizes carries both, original then compressed, even when only + // one overflowed (APPNOTE 4.5.3). A central entry carries one value for each field it set to its + // sentinel, and only those, so a reader walking the fields by their classic ones stays in step. + let local_extra = if size_is_zip64 { zip64_extra(&[uncompressed, compressed]) } else { Vec::new() }; + let mut central_fields = Vec::new(); + if uncompressed >= MAX_CLASSIC_U32 { + central_fields.push(uncompressed); + } + if compressed >= MAX_CLASSIC_U32 { + central_fields.push(compressed); + } + if offset >= MAX_CLASSIC_U32 { + central_fields.push(offset); + } + let central_extra = zip64_extra(¢ral_fields); + // An entry that carries ZIP64 fields needs version 4.5 to extract (APPNOTE 4.4.3); the version + // in the local header and the central entry agree. + let version_needed = if size_is_zip64 || offset >= MAX_CLASSIC_U32 { 45u16 } else { 20u16 }; + let header = |sig: u32, central: bool| { + let mut h = Vec::new(); + h.extend(sig.to_le_bytes()); + if central { + h.extend(20u16.to_le_bytes()); // made by + } + h.extend(version_needed.to_le_bytes()); // version needed + h.extend(0x0800u16.to_le_bytes()); // UTF-8 names + h.extend(method.to_le_bytes()); + h.extend([0, 0, 0x21, 0]); // 00:00, 1980-01-01 (a zero date is not a valid DOS date) + h.extend(crc.to_le_bytes()); + // A local ZIP64 extra carries both sizes, so both corresponding classic fields are + // sentinels even when only one size needs offloading (APPNOTE 4.5.3). Central entries + // remain selective and use a sentinel only for fields represented in their extra. + let (compressed_field, uncompressed_field) = + if !central && size_is_zip64 { (u32::MAX, u32::MAX) } else { (classic_u32(compressed), classic_u32(uncompressed)) }; + h.extend(compressed_field.to_le_bytes()); + h.extend(uncompressed_field.to_le_bytes()); + h.extend((name.len() as u16).to_le_bytes()); + let extra = if central { ¢ral_extra } else { &local_extra }; + h.extend((extra.len() as u16).to_le_bytes()); // extra + if central { + h.extend(0u16.to_le_bytes()); // comment + h.extend(0u16.to_le_bytes()); // disk + h.extend(0u16.to_le_bytes()); // internal attributes + h.extend(0u32.to_le_bytes()); // external attributes + h.extend(classic_u32(offset).to_le_bytes()); + } + h.extend(name.as_bytes()); + h.extend(extra); + h + }; + (header(0x0403_4b50, false), header(0x0201_4b50, true)) +} + +/// Builds a zip archive in memory. An archive past the classic record's 16-bit entry count or +/// 32-bit sizes and offsets is written with a ZIP64 end-of-central-directory record: a +/// PowerPoint export writes two entries per page plus ten fixed parts, so a document with +/// 32 763 pages already passes 65 535 entries, and without the ZIP64 record the count wraps +/// and readers see a central directory that does not describe the archive. #[derive(Default)] pub struct Zip { out: Vec, central: Vec, - count: u16, + count: usize, } impl Zip { @@ -41,49 +133,44 @@ impl Zip { } else { (0u16, data.to_vec()) }; - let offset = self.out.len() as u32; - let header = |sig: u32, central: bool| { - let mut h = Vec::new(); - h.extend(sig.to_le_bytes()); - if central { - h.extend(20u16.to_le_bytes()); // made by - } - h.extend(20u16.to_le_bytes()); // version needed - h.extend(0x0800u16.to_le_bytes()); // UTF-8 names - h.extend(method.to_le_bytes()); - h.extend([0, 0, 0x21, 0]); // 00:00, 1980-01-01 (a zero date is not a valid DOS date) - h.extend(crc.to_le_bytes()); - h.extend((body.len() as u32).to_le_bytes()); - h.extend((data.len() as u32).to_le_bytes()); - h.extend((name.len() as u16).to_le_bytes()); - h.extend(0u16.to_le_bytes()); // extra - if central { - h.extend(0u16.to_le_bytes()); // comment - h.extend(0u16.to_le_bytes()); // disk - h.extend(0u16.to_le_bytes()); // internal attributes - h.extend(0u32.to_le_bytes()); // external attributes - h.extend(offset.to_le_bytes()); - } - h.extend(name.as_bytes()); - h - }; - self.out.extend(header(0x0403_4b50, false)); + let offset = self.out.len() as u64; + let (local, central) = entry_headers(name, crc, method, body.len() as u64, data.len() as u64, offset); + self.out.extend(local); self.out.extend(&body); - self.central.extend(header(0x0201_4b50, true)); + self.central.extend(central); self.count += 1; } pub fn finish(mut self) -> Vec { - let start = self.out.len() as u32; - let size = self.central.len() as u32; + let start = self.out.len() as u64; + let size = self.central.len() as u64; + let count = self.count as u64; self.out.append(&mut self.central); + let classic_count = if count > MAX_CLASSIC_COUNT { u16::MAX } else { count as u16 }; + if count > MAX_CLASSIC_COUNT || size >= MAX_CLASSIC_U32 || start >= MAX_CLASSIC_U32 { + let eocd64_at = self.out.len() as u64; + self.out.extend(0x0606_4b50u32.to_le_bytes()); // ZIP64 end of central directory record + self.out.extend(44u64.to_le_bytes()); // size of the rest of this record + self.out.extend(45u16.to_le_bytes()); // version made by (4.5, DOS) + self.out.extend(45u16.to_le_bytes()); // version needed to extract + self.out.extend(0u32.to_le_bytes()); // number of this disk + self.out.extend(0u32.to_le_bytes()); // disk with the start of the central directory + self.out.extend(count.to_le_bytes()); // entries on this disk + self.out.extend(count.to_le_bytes()); // entries in total + self.out.extend(size.to_le_bytes()); // size of the central directory + self.out.extend(start.to_le_bytes()); // offset of the central directory + self.out.extend(0x0706_4b50u32.to_le_bytes()); // ZIP64 end of central directory locator + self.out.extend(0u32.to_le_bytes()); // disk with the ZIP64 record + self.out.extend(eocd64_at.to_le_bytes()); // offset of the ZIP64 record + self.out.extend(1u32.to_le_bytes()); // total number of disks + } self.out.extend(0x0605_4b50u32.to_le_bytes()); - self.out.extend([0u8; 4]); - self.out.extend(self.count.to_le_bytes()); - self.out.extend(self.count.to_le_bytes()); - self.out.extend(size.to_le_bytes()); - self.out.extend(start.to_le_bytes()); - self.out.extend(0u16.to_le_bytes()); + self.out.extend([0u8; 4]); // number of this disk, disk with the central directory + self.out.extend(classic_count.to_le_bytes()); // entries on this disk + self.out.extend(classic_count.to_le_bytes()); // entries in total + self.out.extend(classic_u32(size).to_le_bytes()); + self.out.extend(classic_u32(start).to_le_bytes()); + self.out.extend(0u16.to_le_bytes()); // comment length self.out } } @@ -107,4 +194,155 @@ mod tests { assert_eq!(b.windows(4).filter(|w| *w == b"PK\x01\x02").count(), 2); assert_eq!(&b[b.len() - 22..b.len() - 18], b"PK\x05\x06"); } + + #[test] + fn more_entries_than_the_classic_record_holds_writes_zip64() { + // A PowerPoint export writes two entries per page plus ten fixed parts, so a + // document with 32 763 pages already passes the classic record's 16-bit entry + // count. The count must not wrap: it overflows in a debug build, and in a release + // one it silently writes a central directory readers cannot match to the archive. + let mut z = Zip::default(); + for i in 0..u16::MAX as usize + 1 { + z.add(&format!("ppt/slides/slide{i}.xml"), b"x", true); + z.add(&format!("ppt/slides/_rels/slide{i}.xml.rels"), b"y", true); + } + let b = z.finish(); + let entries = 2 * (u16::MAX as usize + 1); + // The classic end-of-central-directory record holds sentinels… + let eocd = &b[b.len() - 22..]; + assert_eq!(&eocd[..4], &0x0605_4b50u32.to_le_bytes()); + assert_eq!(&eocd[8..10], &u16::MAX.to_le_bytes(), "entries on this disk"); + assert_eq!(&eocd[10..12], &u16::MAX.to_le_bytes(), "entries in total"); + // …and the ZIP64 locator and record carry the truth. + let locator = &b[b.len() - 42..b.len() - 22]; + assert_eq!(&locator[..4], &0x0706_4b50u32.to_le_bytes()); + let at = u64::from_le_bytes(locator[8..16].try_into().unwrap_or([0; 8])) as usize; + assert_eq!(&b[at..at + 4], &0x0606_4b50u32.to_le_bytes()); + assert_eq!(u64::from_le_bytes(b[at + 24..at + 32].try_into().unwrap_or([0; 8])), entries as u64, "entries on this disk"); + assert_eq!(u64::from_le_bytes(b[at + 32..at + 40].try_into().unwrap_or([0; 8])), entries as u64, "entries in total"); + } + + /// The extra field of a local file header (APPNOTE 4.3.7), which follows the file name. + fn local_extra(h: &[u8]) -> &[u8] { + let le = |i: usize, n: usize| h[i..i + n].iter().rev().fold(0usize, |v, b| v << 8 | *b as usize); + let (name_len, extra_len) = (le(26, 2), le(28, 2)); + &h[30 + name_len..30 + name_len + extra_len] + } + + /// The extra field of a central directory entry (APPNOTE 4.3.12), which follows the file name. + fn central_extra(h: &[u8]) -> &[u8] { + let le = |i: usize, n: usize| h[i..i + n].iter().rev().fold(0usize, |v, b| v << 8 | *b as usize); + let (name_len, extra_len) = (le(28, 2), le(30, 2)); + &h[46 + name_len..46 + name_len + extra_len] + } + + /// The classic compressed and original size fields of a local file header (`at` 18) or a + /// central directory entry (`at` 20). + fn classic_sizes(h: &[u8], at: usize) -> (u32, u32) { + let field = |i: usize| u32::from_le_bytes(h[i..i + 4].try_into().unwrap_or([0; 4])); + (field(at), field(at + 4)) + } + + /// Asserts the local header carries both sizes whenever either needs ZIP64, with both classic + /// size fields set to the sentinel (APPNOTE 4.5.3: a ZIP64 value appears only for a field set to + /// its sentinel, and the local entry must carry both sizes). The central entry stays selective: + /// only the fields that themselves overflow get a value, in specification order. + fn assert_local_pair_and_central_selectivity(compressed: u64, uncompressed: u64) { + let at_limit = |v: u64| v >= u32::MAX as u64; + let (local, central) = entry_headers("a", 0, 0, compressed, uncompressed, 0); + assert_eq!(classic_sizes(&local, 18), (u32::MAX, u32::MAX), "both local size fields are sentinels for ({compressed}, {uncompressed})"); + let extra = local_extra(&local); + assert_eq!( + u16::from_le_bytes(extra[2..4].try_into().unwrap_or([0; 2])), + 16, + "both local sizes are present for ({compressed}, {uncompressed})" + ); + assert_eq!(u64::from_le_bytes(extra[4..12].try_into().unwrap_or([0; 8])), uncompressed, "original size first"); + assert_eq!(u64::from_le_bytes(extra[12..20].try_into().unwrap_or([0; 8])), compressed, "compressed size second"); + let expected = + (if at_limit(compressed) { u32::MAX } else { compressed as u32 }, if at_limit(uncompressed) { u32::MAX } else { uncompressed as u32 }); + assert_eq!( + classic_sizes(¢ral, 20), + expected, + "the central entry offloads only the fields that overflow for ({compressed}, {uncompressed})" + ); + let fields = usize::from(at_limit(uncompressed)) + usize::from(at_limit(compressed)); + let extra = central_extra(¢ral); + assert_eq!( + u16::from_le_bytes(extra[2..4].try_into().unwrap_or([0; 2])), + 8 * fields as u16, + "one central value per sentinel field for ({compressed}, {uncompressed})" + ); + if at_limit(uncompressed) { + assert_eq!(u64::from_le_bytes(extra[4..12].try_into().unwrap_or([0; 8])), uncompressed, "original size first"); + } + if at_limit(compressed) { + let at = if at_limit(uncompressed) { 12 } else { 4 }; + assert_eq!(u64::from_le_bytes(extra[at..at + 8].try_into().unwrap_or([0; 8])), compressed, "compressed size in specification order"); + } + } + + #[test] + fn both_local_sizes_are_sentinels_when_only_one_needs_zip64() { + // A local ZIP64 extra must carry both sizes, and its values correspond to classic fields set + // to their sentinel — so a small classic size next to an overflowing one is wrong even + // though the extra holds the truth. Both orders of which size overflows. + assert_local_pair_and_central_selectivity(100, u32::MAX as u64 + 1); + assert_local_pair_and_central_selectivity(u32::MAX as u64 + 1, 100); + } + + #[test] + fn both_local_sizes_are_sentinels_when_one_equals_the_sentinel() { + // A size of exactly 0xFFFFFFFF is indistinguishable from the sentinel, so it needs ZIP64 + // just like a larger one; the other local size must then become a sentinel too. Both orders. + assert_local_pair_and_central_selectivity(100, u32::MAX as u64); + assert_local_pair_and_central_selectivity(u32::MAX as u64, 100); + } + + #[test] + fn local_zip64_extra_carries_both_sizes() { + // When only one size overflows the u32 limit, the local header still carries both sizes — + // original then compressed — because a local ZIP64 extra that carries either must carry + // both (APPNOTE 4.5.3). Either size may be the one past the limit. + for (compressed, uncompressed) in [(100u64, u32::MAX as u64 + 1), (u32::MAX as u64 + 1, 100)] { + let (local, _) = entry_headers("a", 0, 0, compressed, uncompressed, 0); + let extra = local_extra(&local); + assert_eq!(u16::from_le_bytes(extra[..2].try_into().unwrap_or([0; 2])), 0x0001, "ZIP64 extra id"); + assert_eq!(u16::from_le_bytes(extra[2..4].try_into().unwrap_or([0; 2])), 16, "both sizes are present"); + assert_eq!(u64::from_le_bytes(extra[4..12].try_into().unwrap_or([0; 8])), uncompressed, "original size first"); + assert_eq!(u64::from_le_bytes(extra[12..20].try_into().unwrap_or([0; 8])), compressed, "compressed size second"); + } + } + + #[test] + fn zip64_entries_need_version_4_5() { + // An entry that carries ZIP64 fields must advertise version 4.5 to extract, in both the + // local file header and the central directory entry (APPNOTE 4.4.3). An entry that fits + // every classic field stays at 2.0, so small archives are unchanged. + let (local, central) = entry_headers("a", 0, 0, u32::MAX as u64 + 1, u32::MAX as u64 + 1, 0); + assert_eq!(u16::from_le_bytes(local[4..6].try_into().unwrap_or([0; 2])), 45, "local version needed"); + assert_eq!(u16::from_le_bytes(central[6..8].try_into().unwrap_or([0; 2])), 45, "central version needed"); + let (local, central) = entry_headers("a", 0, 0, 4, 8, 0); + assert_eq!(u16::from_le_bytes(local[4..6].try_into().unwrap_or([0; 2])), 20, "local version needed"); + assert_eq!(u16::from_le_bytes(central[6..8].try_into().unwrap_or([0; 2])), 20, "central version needed"); + } + + #[test] + fn size_equal_to_the_sentinel_is_offloaded() { + // A real size of exactly u32::MAX is indistinguishable from the 0xFFFFFFFF sentinel, so it + // too must be offloaded: every classic field set to the sentinel needs its 8-byte value in + // the central extra, or a reader such as Python's zipfile reports "Compress size not + // found". Both orders of which size is the MAX+1 one must work. + for (compressed, uncompressed) in [(u32::MAX as u64, u32::MAX as u64 + 1), (u32::MAX as u64 + 1, u32::MAX as u64)] { + let (local, central) = entry_headers("a", 0, 0, compressed, uncompressed, 0); + assert_eq!(u32::from_le_bytes(local[18..22].try_into().unwrap_or([0; 4])), u32::MAX, "local compressed sentinel"); + assert_eq!(u32::from_le_bytes(local[22..26].try_into().unwrap_or([0; 4])), u32::MAX, "local original sentinel"); + assert_eq!(u32::from_le_bytes(central[20..24].try_into().unwrap_or([0; 4])), u32::MAX, "central compressed sentinel"); + assert_eq!(u32::from_le_bytes(central[24..28].try_into().unwrap_or([0; 4])), u32::MAX, "central original sentinel"); + let extra = central_extra(¢ral); + assert_eq!(u16::from_le_bytes(extra[2..4].try_into().unwrap_or([0; 2])), 16, "a value for each sentinel field"); + assert_eq!(u64::from_le_bytes(extra[4..12].try_into().unwrap_or([0; 8])), uncompressed, "original size first"); + assert_eq!(u64::from_le_bytes(extra[12..20].try_into().unwrap_or([0; 8])), compressed, "compressed size second"); + } + } } From 938f56989fb33c3abb37e9bb4ed5272ad1dd5b0f Mon Sep 17 00:00:00 2001 From: jonathanwxh-cell Date: Mon, 9 Nov 2026 18:22:09 +0800 Subject: [PATCH 8/8] fix(organize): preserve page labels across extract, split, and combine (#2670) (#2799) --- crates/organize/src/import/combine.rs | 28 ++++- crates/organize/src/labels.rs | 123 +++++++++++++++++-- crates/organize/src/tests.rs | 170 ++++++++++++++++++++++++++ 3 files changed, 311 insertions(+), 10 deletions(-) diff --git a/crates/organize/src/import/combine.rs b/crates/organize/src/import/combine.rs index 7782e5de18..c1a1fed520 100644 --- a/crates/organize/src/import/combine.rs +++ b/crates/organize/src/import/combine.rs @@ -7,9 +7,13 @@ use super::{Dict, Document, FieldConflicts, HashMap, HashSet, ObjRef, Object, Or /// go to the copied pages. pub fn extract_pages(src: &Document, pages: &[usize]) -> Result { let mut out = Document::new_empty(); + let mut labels = Vec::new(); + let mut has_labels = false; + crate::labels::collect_page_labels(src, pages, 0, &mut labels, &mut has_labels)?; // One source: nothing to deduplicate. Its fields keep their names (a source with two // same-named field roots stays one field, as it was), so Extract and Split never rename. let imported = import_pages_mapped(&mut out, src, pages, 0, FieldConflicts::Merge)?; + crate::labels::finish_page_labels(&mut out, &labels, has_labels)?; // The bookmarks of these pages come along, mapped to the copies (#4020). let outline = kept_bookmarks(source_outline(src, imported.page_map)); if !outline.is_empty() { @@ -119,6 +123,8 @@ pub struct Combiner { out: Document, marks: Vec, attachments: Vec<(Vec, Object)>, + labels: Vec, + has_labels: bool, /// The first source's print standard, and what its pages brought along. standard: Option, /// Whether every source so far declares the first source's print standard. @@ -156,6 +162,8 @@ impl Combiner { out: Document::new_empty(), marks: Vec::new(), attachments: Vec::new(), + labels: Vec::new(), + has_labels: false, standard: None, agreed: true, field_conflicts, @@ -182,7 +190,12 @@ impl Combiner { None => (0..n).collect(), }; let at = crate::page_count(&self.out)?; + let mut labels = self.labels.clone(); + let mut has_labels = self.has_labels; + crate::labels::collect_page_labels(src, &pages, at, &mut labels, &mut has_labels)?; let imported = import_pages_mapped(&mut self.out, src, &pages, at, self.field_conflicts)?; + self.labels = labels; + self.has_labels = has_labels; self.renamed_fields.extend(imported.renamed_fields); if let Some(first) = imported.pages.first() { let outline = source_outline(src, imported.page_map); @@ -212,13 +225,14 @@ impl Combiner { /// [`Combiner::finish`] reporting what the combine renamed. pub fn finish_with_renames(self) -> Result { - let Combiner { mut out, marks, attachments, standard, agreed, renamed_fields, .. } = self; + let Combiner { mut out, marks, attachments, labels, has_labels, standard, agreed, renamed_fields, .. } = self; if let Some(first) = standard.filter(|_| agreed) { crate::pdfx::carry(&mut out, first.source.as_ref(), &first.declared, first.objects)?; } // Sources often share fonts, images and profiles (or are the same file): store them once. let all: Vec = out.object_numbers().into_iter().map(|n| ObjRef::new(n, out.generation(n))).collect(); crate::dedupe::dedupe_resources(&mut out, &all, false); + crate::labels::finish_page_labels(&mut out, &labels, has_labels)?; add_outline(&mut out, &marks)?; set_attachments(&mut out, attachments)?; Ok(CombineOutcome { doc: out, renamed_fields }) @@ -299,9 +313,18 @@ pub fn combine_grouped_with(runs: &[Run<'_>], conflicts: FieldConflicts) -> Resu // The pages in the runs' order. A file's first run comes before its others, and its pages // were copied in run order, so its bookmark (its first copied page) is its first page shown. let mut order: Vec<(ObjRef, Dict)> = Vec::new(); + let mut labels = Vec::new(); + let mut has_labels = false; for (file, start, len) in slices { - let run = copies.get(file).and_then(|c| c.get(start..start.saturating_add(len))).unwrap_or_default(); + let end = start.saturating_add(len); + let at = order.len(); + let run = copies.get(file).and_then(|c| c.get(start..end)).unwrap_or_default(); order.extend(run.iter().map(|r| (*r, Dict::new()))); + if let Some((_, f)) = files.get(file) + && let Some(pages) = f.pages.get(start..end) + { + crate::labels::collect_page_labels(f.src, pages, at, &mut labels, &mut has_labels)?; + } } rebuild(&mut out, &order)?; if let Some((declared, src, objects)) = standard.filter(|_| agreed) { @@ -309,6 +332,7 @@ pub fn combine_grouped_with(runs: &[Run<'_>], conflicts: FieldConflicts) -> Resu } let all: Vec = out.object_numbers().into_iter().map(|n| ObjRef::new(n, out.generation(n))).collect(); crate::dedupe::dedupe_resources(&mut out, &all, false); + crate::labels::finish_page_labels(&mut out, &labels, has_labels)?; add_outline(&mut out, &marks)?; set_attachments(&mut out, attachments)?; Ok(CombineOutcome { doc: out, renamed_fields }) diff --git a/crates/organize/src/labels.rs b/crates/organize/src/labels.rs index 8ee04bed53..e296148ebd 100644 --- a/crates/organize/src/labels.rs +++ b/crates/organize/src/labels.rs @@ -360,11 +360,13 @@ pub fn set_page_label_ranges(doc: &mut Document, ranges: &[LabelRange]) -> Resul return Err(invalid_label("the number tree has too many entries")); } // Validate every range and all effective labels before changing any document object. + let mut prefix_left = MAX_PREFIX_TOTAL_BYTES; for r in ranges { if r.first == 0 || i64::try_from(r.start).is_err() { return Err(invalid_label("range starts and first numbers must be valid positive PDF integers")); } r.label(r.start)?; + prefix_left = prefix_left.checked_sub(r.prefix.len()).ok_or_else(|| invalid_label("decoded prefixes exceed 4 MiB in total"))?; } let mut sorted = ranges.to_vec(); sorted.sort_by_key(|r| r.start); @@ -450,15 +452,9 @@ pub fn number_pages(doc: &mut Document, from: usize, to: usize, style: LabelStyl } // Drop ranges that only continue the previous one. let mut merged: Vec = Vec::new(); + let mut prefix_left = MAX_PREFIX_TOTAL_BYTES; for r in ranges { - let continues = merged.last().is_some_and(|p| { - p.style == r.style - && p.prefix == r.prefix - && r.start.checked_sub(p.start).and_then(|n| u32::try_from(n).ok()).and_then(|n| p.first.checked_add(n)) == Some(r.first) - }); - if !continues { - merged.push(r); - } + push_merged_range(&mut merged, &mut prefix_left, r)?; } // Plain 1, 2, 3 everywhere is what a document without labels shows: remove them. if merged.len() == 1 && merged[0] == default_range() { @@ -467,6 +463,117 @@ pub fn number_pages(doc: &mut Document, from: usize, to: usize, style: LabelStyl set_page_label_ranges(doc, &merged) } +fn push_merged_range(merged: &mut Vec, prefix_left: &mut usize, r: LabelRange) -> Result<(), OrganizeError> { + let continues = merged.last().is_some_and(|p| { + p.style == r.style + && p.prefix == r.prefix + && match r.style { + LabelStyle::None => p.first == r.first, + _ => r.start.checked_sub(p.start).and_then(|n| u32::try_from(n).ok()).and_then(|n| p.first.checked_add(n)) == Some(r.first), + } + }); + if !continues { + if merged.len() + 1 >= MAX_LABEL_TREE_WORK { + return Err(invalid_label("the number tree has too many entries")); + } + *prefix_left = prefix_left.checked_sub(r.prefix.len()).ok_or_else(|| invalid_label("decoded prefixes exceed 4 MiB in total"))?; + merged.push(r); + } + Ok(()) +} + +/// Decimal numbering that shows the source page's physical number (`src_page` is 0-based). +fn source_physical_range(dst_start: usize, src_page: usize) -> LabelRange { + LabelRange { start: dst_start, style: LabelStyle::Decimal, prefix: String::new(), first: u32::try_from(src_page.saturating_add(1)).unwrap_or(1) } +} + +/// Whether `set_page_label_ranges` can store `r` while it covers `r.start..end` (`end` exclusive). +/// Entry-count and aggregate-prefix limits stay hard errors in [`push_merged_range`]. +fn range_writable(r: &LabelRange, end: usize) -> bool { + r.label(r.start).is_ok() && check_spans(std::slice::from_ref(r), end).is_ok() +} + +/// Append the label ranges that `pages` (0-based indices in `src`) carry when placed at +/// `dst_start` in an output document. Sets `has_labels` when `src` has `/PageLabels`. +pub(crate) fn collect_page_labels( + src: &Document, + pages: &[usize], + dst_start: usize, + out: &mut Vec, + has_labels: &mut bool, +) -> Result<(), OrganizeError> { + if pages.is_empty() { + return Ok(()); + } + let mut src_ranges = checked_ranges(src).unwrap_or_default(); + if !src_ranges.is_empty() { + *has_labels = true; + } + for r in &mut src_ranges { + // An empty prefix-only range shows each page's physical number in the source. + if r.style == LabelStyle::None && r.prefix.is_empty() { + r.style = LabelStyle::Decimal; + r.first = u32::try_from(r.start.saturating_add(1)).unwrap_or(1); + } + } + if src_ranges.first().is_none_or(|r| r.start > 0) { + src_ranges.insert(0, default_range()); + } + let mut prefix_left = MAX_PREFIX_TOTAL_BYTES.saturating_sub(out.iter().map(|r| r.prefix.len()).sum::()); + let mut i = 0; + while i < pages.len() { + let src_start = pages[i]; + let mut len = 1; + while i + len < pages.len() && src_start.checked_add(len) == Some(pages[i + len]) { + len += 1; + } + let run_dst = dst_start + i; + let src_end = src_start.saturating_add(len); + let run_end = run_dst.saturating_add(len); + let idx = src_ranges.partition_point(|r| r.start <= src_start).saturating_sub(1); + let span_end = |following: Option| { + following + .filter(|start| *start > src_start && *start < src_end) + .and_then(|start| start.checked_sub(src_start).and_then(|delta| run_dst.checked_add(delta))) + .unwrap_or(run_end) + }; + if let Some(first_range) = src_ranges.get(idx) { + let mapped = match first_range.number(src_start) { + Ok(first) => LabelRange { start: run_dst, style: first_range.style, prefix: first_range.prefix.clone(), first }, + Err(_) => source_physical_range(run_dst, src_start), + }; + let mapped = if range_writable(&mapped, span_end(src_ranges.get(idx + 1).map(|r| r.start))) { + mapped + } else { + source_physical_range(run_dst, src_start) + }; + push_merged_range(out, &mut prefix_left, mapped)?; + } + let tail = src_ranges.get(idx + 1..).unwrap_or_default(); + for (n, r) in tail.iter().enumerate() { + if r.start >= src_end { + break; + } + let Some(delta) = r.start.checked_sub(src_start) else { continue }; + let Some(start) = run_dst.checked_add(delta) else { continue }; + let inner = LabelRange { start, style: r.style, prefix: r.prefix.clone(), first: r.first }; + let following = tail.get(n + 1).map(|next| next.start); + let inner = if range_writable(&inner, span_end(following)) { inner } else { source_physical_range(start, r.start) }; + push_merged_range(out, &mut prefix_left, inner)?; + } + i += len; + } + Ok(()) +} + +/// Write collected page label ranges to `dst` when at least one source carried `/PageLabels`. +pub(crate) fn finish_page_labels(dst: &mut Document, ranges: &[LabelRange], has_labels: bool) -> Result<(), OrganizeError> { + if has_labels && !ranges.is_empty() { + set_page_label_ranges(dst, ranges)?; + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index c79c3d8667..0aa6d149d9 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -2223,6 +2223,176 @@ fn deleting_pages_keeps_the_label_ranges_on_their_pages() { } } +#[test] +fn extracting_splitting_and_combining_preserve_page_labels() { + use crate::{ + LabelRange, + LabelStyle::{Decimal, LowerRoman, UpperAlpha}, + SplitBy, + }; + let ten = || { + let mut d = doc_a(); + for at in 3..10 { + crate::insert_blank_page(&mut d, at, 300.0, 400.0).unwrap(); + } + let ranges = [ + LabelRange { start: 0, style: LowerRoman, prefix: String::new(), first: 1 }, + LabelRange { start: 3, style: Decimal, prefix: String::new(), first: 1 }, + LabelRange { start: 7, style: Decimal, prefix: "A-".into(), first: 1 }, + ]; + crate::set_page_label_ranges(&mut d, &ranges).unwrap(); + d + }; + let d = ten(); + + // Extracting non-contiguous pages (e.g. first front-matter page and first appendix page) + // keeps their labels and writes /PageLabels (#2670). + let pair = full_roundtrip(&extract_pages(&d, &[0, 7]).unwrap()); + assert_eq!(crate::page_labels(&pair).unwrap(), ["i", "A-1"]); + assert_eq!( + crate::page_label_ranges(&pair), + [ + LabelRange { start: 0, style: LowerRoman, prefix: String::new(), first: 1 }, + LabelRange { start: 1, style: Decimal, prefix: "A-".into(), first: 1 }, + ] + ); + + // Contiguous runs and out-of-order picks keep each page's label and merge continuations. + let picked = full_roundtrip(&extract_pages(&d, &[1, 2, 8, 9, 0]).unwrap()); + assert_eq!(crate::page_labels(&picked).unwrap(), ["ii", "iii", "A-2", "A-3", "i"]); + assert_eq!( + crate::page_label_ranges(&picked), + [ + LabelRange { start: 0, style: LowerRoman, prefix: String::new(), first: 2 }, + LabelRange { start: 2, style: Decimal, prefix: "A-".into(), first: 2 }, + LabelRange { start: 4, style: LowerRoman, prefix: String::new(), first: 1 }, + ] + ); + + // An unlabelled source stays unlabelled after extraction. + let unlabelled = full_roundtrip(&extract_pages(&doc_a(), &[2, 0]).unwrap()); + assert!(crate::page_label_ranges(&unlabelled).is_empty()); + + // Splitting preserves each part's labels, including mid-range starting numbers. + let parts = crate::split(&d, &SplitBy::PageCount(4)).unwrap(); + let part_labels: Vec> = parts.iter().map(|p| crate::page_labels(&full_roundtrip(p)).unwrap()).collect(); + assert_eq!(part_labels, vec![vec!["i", "ii", "iii", "1"], vec!["2", "3", "4", "A-1"], vec!["A-2", "A-3"],]); + + // Combining whole files, selected pages, and grouped runs preserves labels from each source. + let mut b = doc_b(); + crate::set_page_label_ranges(&mut b, &[LabelRange { start: 0, style: UpperAlpha, prefix: "B-".into(), first: 1 }]).unwrap(); + let combined = full_roundtrip(&combine_selected(&[("D", &d, Some(&[0, 1, 7])), ("Plain", &doc_a(), Some(&[0, 1])), ("B", &b, None)]).unwrap()); + assert_eq!(crate::page_labels(&combined).unwrap(), ["i", "ii", "A-1", "1", "2", "B-A", "B-B"]); + + let grouped = + full_roundtrip(&crate::combine_grouped(&[(0, "D", &d, Some(&[0, 1])), (1, "B", &b, Some(&[1])), (0, "D", &d, Some(&[2, 7]))]).unwrap()); + assert_eq!(crate::page_labels(&grouped).unwrap(), ["i", "ii", "B-B", "iii", "A-1"]); + + // Ranges starting after page 0 and prefix-only ranges (LabelStyle::None) merge and carry over. + let mut late = doc_a(); + crate::set_page_label_ranges( + &mut late, + &[ + LabelRange { start: 1, style: crate::LabelStyle::None, prefix: "Cover".into(), first: 1 }, + LabelRange { start: 2, style: LowerRoman, prefix: String::new(), first: 4 }, + ], + ) + .unwrap(); + let from_late = full_roundtrip(&extract_pages(&late, &[0, 1, 1, 2]).unwrap()); + assert_eq!(crate::page_labels(&from_late).unwrap(), ["1", "Cover", "Cover", "iv"]); + assert_eq!( + crate::page_label_ranges(&from_late), + [ + LabelRange { start: 0, style: Decimal, prefix: String::new(), first: 1 }, + LabelRange { start: 1, style: crate::LabelStyle::None, prefix: "Cover".into(), first: 1 }, + LabelRange { start: 3, style: LowerRoman, prefix: String::new(), first: 4 }, + ] + ); + + // An empty selection from a labelled source in combine_selected does not add /PageLabels. + let empty_pick = full_roundtrip(&combine_selected(&[("D", &d, Some(&[])), ("Plain", &doc_a(), Some(&[0, 1]))]).unwrap()); + assert!(crate::page_label_ranges(&empty_pick).is_empty()); + + // A damaged (cyclic) /PageLabels tree does not prevent extracting pages. + let mut cyclic = doc_a(); + let tree = cyclic.add(Object::Null); + let mut dict = Dict::new(); + dict.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(tree)])); + cyclic.set(tree, Object::Dict(dict)); + cyclic.update_dict(cyclic.root().unwrap(), |c| c.set(b"PageLabels".to_vec(), Object::Ref(tree))).unwrap(); + let extracted_cyclic = full_roundtrip(&extract_pages(&cyclic, &[0, 2]).unwrap()); + assert_eq!(crate::page_labels(&extracted_cyclic).unwrap(), ["1", "2"]); + + // An empty prefix-only range falls back to physical page numbers in the source; extracting a + // later page covered by it preserves that physical page number across save/reopen. + let mut empty_prefix = doc_a(); + crate::set_page_label_ranges( + &mut empty_prefix, + &[ + LabelRange { start: 0, style: crate::LabelStyle::None, prefix: String::new(), first: 1 }, + LabelRange { start: 2, style: LowerRoman, prefix: String::new(), first: 1 }, + ], + ) + .unwrap(); + assert_eq!(crate::PageLabels::new(&empty_prefix).unwrap().label(1).unwrap(), "2"); + let extracted_empty_prefix = full_roundtrip(&extract_pages(&empty_prefix, &[1]).unwrap()); + assert_eq!(crate::PageLabels::new(&extracted_empty_prefix).unwrap().label(0).unwrap(), "2"); + + // Repeated non-continuing ranges that exceed the 4 MiB total-prefix limit or the range-count + // limit are refused rather than writing an unreadable tree or silently dropping labels. + let mut one = flat_doc(1); + crate::number_pages(&mut one, 0, 0, Decimal, &"x".repeat(1_000), 1).unwrap(); + let err = extract_pages(&one, &vec![0; 4_195]).unwrap_err().to_string(); + assert!(err.contains("decoded prefixes exceed 4 MiB in total"), "{err}"); + + crate::number_pages(&mut one, 0, 0, LowerRoman, "", 2).unwrap(); + let err = extract_pages(&one, &vec![0; pdfcraft_cos::page_labels::MAX_LABEL_TREE_WORK]).unwrap_err().to_string(); + assert!(err.contains("the number tree has too many entries"), "{err}"); +} + +/// A range the reader accepts but the writer cannot store (a Roman numeral past 1,024 bytes, or a +/// decimal span whose labels grow past that) degrades to the source physical numbers. Extract +/// still succeeds, and a neighbouring range that does fit is kept. +#[test] +fn extracting_unrepresentable_labels_uses_physical_numbers() { + let mut roman = doc_a(); + let mut spec = Dict::new(); + spec.set(b"S".to_vec(), Object::name("R")); + spec.set(b"St".to_vec(), Object::Int(4_000_000)); + let mut tree = Dict::new(); + tree.set(b"Nums".to_vec(), Object::Array(vec![Object::Int(0), Object::Dict(spec)])); + let tree = roman.add(Object::Dict(tree)); + roman.update_dict(roman.root().unwrap(), |c| c.set(b"PageLabels".to_vec(), Object::Ref(tree))).unwrap(); + let extracted = full_roundtrip(&extract_pages(&roman, &[0, 2]).unwrap()); + assert_eq!(crate::page_labels(&extracted).unwrap(), ["1", "3"]); + + let mut wide = flat_doc(5); + let mut spec = Dict::new(); + spec.set(b"P".to_vec(), Object::String(PdfString::text(&"p".repeat(1_020)))); + spec.set(b"S".to_vec(), Object::name("D")); + spec.set(b"St".to_vec(), Object::Int(9_996)); + let mut tree = Dict::new(); + tree.set(b"Nums".to_vec(), Object::Array(vec![Object::Int(0), Object::Dict(spec)])); + let tree = wide.add(Object::Dict(tree)); + wide.update_dict(wide.root().unwrap(), |c| c.set(b"PageLabels".to_vec(), Object::Ref(tree))).unwrap(); + let extracted = full_roundtrip(&extract_pages(&wide, &[0, 1, 2, 3, 4]).unwrap()); + assert_eq!(crate::page_labels(&extracted).unwrap(), ["1", "2", "3", "4", "5"]); + + let mut mixed = doc_a(); + let mut ok = Dict::new(); + ok.set(b"P".to_vec(), Object::String(PdfString::text("ok-"))); + ok.set(b"S".to_vec(), Object::name("D")); + let mut huge = Dict::new(); + huge.set(b"S".to_vec(), Object::name("R")); + huge.set(b"St".to_vec(), Object::Int(4_000_000)); + let mut tree = Dict::new(); + tree.set(b"Nums".to_vec(), Object::Array(vec![Object::Int(0), Object::Dict(ok), Object::Int(1), Object::Dict(huge)])); + let tree = mixed.add(Object::Dict(tree)); + mixed.update_dict(mixed.root().unwrap(), |c| c.set(b"PageLabels".to_vec(), Object::Ref(tree))).unwrap(); + let extracted = full_roundtrip(&extract_pages(&mixed, &[0, 1]).unwrap()); + assert_eq!(crate::page_labels(&extracted).unwrap(), ["ok-1", "2"]); +} + #[test] fn page_label_limits_reject_unsupported_edits_before_mutating() { use crate::LabelStyle::*;