From d39b55dd6da15f571a86d2ebb4ef0d93299e76f7 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:25:53 +0000 Subject: [PATCH] fix(organize): register imported form fields in linear time Each imported top-level field was checked against every field already in /AcroForm /Fields before it was appended, so a page with many distinct widget fields cost time quadratic in their number. Membership is now kept in a set beside the list. Adds a test-only work counter to the crate, so tests can bound the work a hostile document causes without timing it. --- crates/organize/src/import.rs | 5 ++++- crates/organize/src/lib.rs | 19 +++++++++++++++++++ crates/organize/src/tests.rs | 28 ++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index 80f90fb2a..56625e5d5 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -826,8 +826,11 @@ fn register_fields(dst: &mut Document, fields: &[ObjRef]) -> Result<(), Organize _ => (None, Dict::new()), }; let mut list = form.get(b"Fields").map(|f| dst.resolve(f)).and_then(|f| f.as_array().cloned()).unwrap_or_default(); + let mut listed: HashSet = list.iter().filter_map(Object::as_ref).collect(); for f in fields { - if !list.contains(&Object::Ref(*f)) { + #[cfg(test)] + crate::work(1); + if listed.insert(*f) { list.push(Object::Ref(*f)); } } diff --git a/crates/organize/src/lib.rs b/crates/organize/src/lib.rs index 6a696dd4d..d0e088fd7 100644 --- a/crates/organize/src/lib.rs +++ b/crates/organize/src/lib.rs @@ -739,5 +739,24 @@ pub fn remove_standard_id(doc: &mut Document, standard: Standard) -> Result<(), if removed { Ok(()) } else { Err(OrganizeError::Invalid(format!("the document doesn't declare {}", standard.label()))) } } +#[cfg(test)] +thread_local! { + /// Steps taken on this thread by loops whose cost follows the input's shape, so a test can + /// bound the work a hostile document causes without timing it. + static WORK: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + +/// Count `n` steps of input-shaped work (see [`WORK`]). +#[cfg(test)] +fn work(n: usize) { + WORK.with(|w| w.set(w.get().saturating_add(n))); +} + +/// The steps counted on this thread since the last call. +#[cfg(test)] +fn take_work() -> usize { + WORK.with(|w| w.replace(0)) +} + #[cfg(test)] mod tests; diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index 7a80e38e4..7bb41b42c 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -3132,3 +3132,31 @@ fn page_walk_skips_repeated_and_cyclic_kids_without_copying_attributes() { assert!(walk.next().is_none()); assert_eq!(labels(&doc), ["Page 1", "Page 2", "Page 3"]); } + +/// A one-page document whose page carries `annots` (object bodies 5…), with `extra` bodies after them. +fn page_with_annots(annots: &[String], extra: &[String], catalog_extra: &str) -> Document { + let n = annots.len(); + let refs: String = (0..n).map(|i| format!("{} 0 R ", i + 5)).collect(); + let mut b: Vec = vec![ + format!("<< /Type /Catalog /Pages 2 0 R {catalog_extra} >>"), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >>".into(), + format!("<< /Type /Page /Parent 2 0 R /Contents 4 0 R /Annots [{refs}] >>"), + body("P1"), + ]; + b.extend(annots.iter().cloned()); + b.extend(extra.iter().cloned()); + open(build(&b, "/Root 1 0 R")) +} + +#[test] +fn registering_many_imported_fields_is_linear() { + let n = 2_000; + let widgets: Vec = (0..n).map(|i| format!("<< /Type /Annot /Subtype /Widget /FT /Tx /T (f{i}) /Rect [0 0 10 10] >>")).collect(); + let src = page_with_annots(&widgets, &[], ""); + take_work(); + let out = extract_pages(&src, &[0]).unwrap(); + let work = take_work(); + assert!(work <= 4 * n, "{work} steps registering {n} fields"); + let form = out.resolve(catalog(&out).get(b"AcroForm").unwrap()).as_dict().cloned().unwrap(); + assert_eq!(form.get(b"Fields").and_then(|f| f.as_array()).map(Vec::len), Some(n)); +}