Open: measure cached objects once and by what they hold, read plain reals without a copy, and keep no /PieceInfo in inspection (#5873) (#5958)

crates/cos/src/document.rs

@@ -27,7 +27,7 @@ pub(crate) use state::ObjectReader;
 use state::{ObjStm, Slot};
 pub use state::{ReadState, SourceIdentity};
 #[cfg(test)]
-use state::{SECTION_READS, objstm_member, owned_bytes};
+use state::{SECTION_READS, held_bytes, objstm_member};
 use xref::XrefIndex;
 pub use xref::{Revision, XrefEntry};
 
@@ -47,7 +47,7 @@ pub struct Document {
     /// `fill_xref_gaps`.
     located: Arc<BTreeMap<u32, (u64, u16)>>,
     repair_log: Arc<Vec<String>>,
-    cache: Arc<Mutex<HashMap<u32, Arc<Object>>>>,
+    cache: Arc<Mutex<state::ObjectCache>>,
     objstms: Arc<Mutex<HashMap<u32, Arc<ObjStm>>>>,
     /// What `/JBIG2Globals` streams of the file decode to, by object number: the images of a scan
     /// share them, and each would otherwise decode them again (see `Document::jbig2_globals`).

crates/cos/src/document/objects.rs

@@ -1,5 +1,5 @@
 use super::reader::find;
-use super::state::{LENGTHS, LoadGuard, ObjStm, ObjectReader, ReadState, Slot, SourceIdentity, objstm_member, owned_bytes};
+use super::state::{LENGTHS, LoadGuard, ObjStm, ObjectReader, ReadState, Slot, SourceIdentity, held_bytes, objstm_member};
 use super::xref::{Revision, XrefEntry};
 use super::{
     Arc, CosError, Cow, Dict, Document, Lexer, MAX_DECODED, MAX_JBIG2_GLOBALS, ObjRef, Object, Stream, is_whitespace, parse_indirect_shared,
@@ -399,8 +399,9 @@ impl Document {
         let obj = Arc::new(obj);
         // The editing cache is bounded like the scan reader's: re-parsing a file-backed view
         // is cheap, and returned objects outlive eviction.
-        self.cache.lock().map_err(|_| CosError::Poisoned)?.insert(num, obj.clone());
-        self.evict_editor_cache_if_over_budget(num, &obj)?;
+        let owned = held_bytes(&obj, &self.data, 0);
+        self.cache.lock().map_err(|_| CosError::Poisoned)?.insert(num, obj.clone(), owned);
+        self.evict_editor_cache_if_over_budget(num)?;
         Ok(obj)
     }
 
@@ -409,10 +410,10 @@ impl Document {
     /// document cannot pin its whole object graph. Only owned (decrypted/copied) bytes
     /// count; file-backed views re-parse for free.
     ///
-    /// Worst case per document: input bytes + up to `EDITOR_CACHE_OBJECTS` ×
-    /// `EDITOR_LARGE_OBJECT` (1024 × 256 KB = 256 MB) of small objects between full
-    /// accounting passes + `EDITOR_OBJSTM_BYTES` (32 MB) + unsaved `overlay` edits.
-    /// The count cap always holds; byte accounting between passes is approximate.
+    /// Worst case per document: input bytes + `EDITOR_CACHE_BYTES` (32 MB) of owned object
+    /// data + `EDITOR_OBJSTM_BYTES` (32 MB) + unsaved `overlay` edits. Each object is measured
+    /// once, as it is cached ([`ObjectCache`](super::state::ObjectCache)), so both caps hold
+    /// after every insertion.
     /// Clones share these caches, so one clone's eviction drops another's hot set;
     /// that only costs re-parsing, never correctness. Eviction keeps just the newcomer:
     /// file-backed views re-parse cheaply and editor working sets stay far below 1024.
@@ -425,37 +426,13 @@ impl Document {
 
     const EDITOR_OBJSTM_BYTES: usize = 32 * 1024 * 1024;
 
-    /// Newcomers above this force a full accounting pass even below the count cap:
-    /// `EDITOR_CACHE_BYTES / 128`, so 128 large newcomers always trigger a pass before the
-    /// byte budget can double.
-    const EDITOR_LARGE_OBJECT: usize = 256 * 1024;
-
-    fn evict_editor_cache_if_over_budget(&self, fresh_num: u32, fresh: &Object) -> Result<(), CosError> {
-        // Fast path: the full fold is O(cache) per insert, so skip it for a small newcomer
-        // below the count cap. `len` is copied out first; nothing here holds the lock.
-        let len = self.cache.lock().map_err(|_| CosError::Poisoned)?.len();
-        if len <= Self::EDITOR_CACHE_OBJECTS && owned_bytes(fresh, &self.data, 0) <= Self::EDITOR_LARGE_OBJECT {
-            return Ok(());
-        }
-        // Walk a snapshot off-lock; re-lock only to evict.
-        let live: Vec<(u32, Arc<Object>)> = self.cache.lock().map_err(|_| CosError::Poisoned)?.iter().map(|(n, o)| (*n, o.clone())).collect();
-        let data = &self.data;
-        let owned: usize = live.iter().fold(0usize, |n, (_, o)| n.saturating_add(owned_bytes(o, data, 0)));
-        if live.len() <= Self::EDITOR_CACHE_OBJECTS && owned <= Self::EDITOR_CACHE_BYTES {
-            return Ok(());
-        }
-        // Re-check under the final lock: another thread may have inserted or evicted since
-        // the snapshot. Keep its newcomers, and skip the eviction if back within budget.
-        let snapshot: std::collections::HashSet<u32> = live.into_iter().map(|(n, _)| n).collect();
+    fn evict_editor_cache_if_over_budget(&self, fresh_num: u32) -> Result<(), CosError> {
         let mut cache = self.cache.lock().map_err(|_| CosError::Poisoned)?;
-        if cache.len() <= Self::EDITOR_CACHE_OBJECTS {
-            let data = &self.data;
-            let owned: usize = cache.values().fold(0usize, |n, o| n.saturating_add(owned_bytes(o, data, 0)));
-            if owned <= Self::EDITOR_CACHE_BYTES {
-                return Ok(());
-            }
+        if cache.len() <= Self::EDITOR_CACHE_OBJECTS && cache.owned() <= Self::EDITOR_CACHE_BYTES {
+            return Ok(());
         }
-        cache.retain(|n, _| *n == fresh_num || !snapshot.contains(n));
+        // Eviction keeps just the newcomer: file-backed views re-parse cheaply.
+        cache.retain(|n| *n == fresh_num);
         Ok(())
     }
 

crates/cos/src/document/state.rs

@@ -163,11 +163,10 @@ impl ObjectReader {
         // input bytes, which the document keeps anyway, so they don't count; decrypted
         // streams and strings do. Checked after failed loads too: resolving a /Length may
         // have cached large objects.
-        if let Ok(mut cache) = self.document.cache.lock() {
-            let data = &self.document.data;
-            if cache.len() >= Self::OBJECTS || cache.values().fold(0usize, |n, o| n.saturating_add(owned_bytes(o, data, 0))) > Self::STREAM_BYTES {
-                cache.clear();
-            }
+        if let Ok(mut cache) = self.document.cache.lock()
+            && (cache.len() >= Self::OBJECTS || cache.owned() > Self::STREAM_BYTES)
+        {
+            cache.clear();
         }
         object
     }
@@ -179,26 +178,96 @@ thread_local! {
     pub(super) static SECTION_READS: std::cell::Cell<usize> = const { std::cell::Cell::new(0) };
 }
 
-/// Nesting [`owned_bytes`] walks: past the parser's own nesting limit, so every parsed object
+/// Parsed objects by number, each with what it holds ([`held_bytes`]) measured once, when it goes
+/// in, and their sum kept beside them.
+///
+/// Both caches are bounded by those bytes. Summing them on every read walked every cached object
+/// whole each time: a file whose page dictionaries carry kilobytes of private data (InDesign's
+/// `/PieceInfo`) took seconds to open, almost all of it re-measuring objects already measured
+/// (#5873).
+#[derive(Default)]
+pub(crate) struct ObjectCache {
+    entries: HashMap<u32, (Arc<Object>, usize)>,
+    owned: usize,
+}
+
+impl ObjectCache {
+    pub(crate) fn get(&self, num: &u32) -> Option<&Arc<Object>> {
+        self.entries.get(num).map(|(object, _)| object)
+    }
+
+    #[cfg(test)]
+    pub(crate) fn contains_key(&self, num: &u32) -> bool {
+        self.entries.contains_key(num)
+    }
+
+    /// Keep `object` as `num`, which holds `owned` bytes (what [`held_bytes`] says of it).
+    pub(crate) fn insert(&mut self, num: u32, object: Arc<Object>, owned: usize) {
+        if let Some((_, before)) = self.entries.insert(num, (object, owned)) {
+            self.owned = self.owned.saturating_sub(before);
+        }
+        self.owned = self.owned.saturating_add(owned);
+    }
+
+    pub(crate) fn len(&self) -> usize {
+        self.entries.len()
+    }
+
+    #[cfg(test)]
+    pub(crate) fn is_empty(&self) -> bool {
+        self.entries.is_empty()
+    }
+
+    /// The bytes the cached objects hold between them.
+    pub(crate) fn owned(&self) -> usize {
+        self.owned
+    }
+
+    pub(crate) fn clear(&mut self) {
+        self.entries.clear();
+        self.owned = 0;
+    }
+
+    pub(crate) fn retain(&mut self, mut keep: impl FnMut(&u32) -> bool) {
+        self.entries.retain(|num, _| keep(num));
+        self.owned = self.entries.values().fold(0usize, |n, (_, owned)| n.saturating_add(*owned));
+    }
+
+    #[cfg(test)]
+    pub(crate) fn values(&self) -> impl Iterator<Item = &Arc<Object>> {
+        self.entries.values().map(|(object, _)| object)
+    }
+}
+
+/// Nesting [`held_bytes`] walks: past the parser's own nesting limit, so every parsed object
 /// is measured in full.
 const OWNED_BYTES_DEPTH: usize = 256;
 
-/// Bytes `object` holds of its own: string and stream data that is not a view into `data`
-/// (the input file). An object nested deeper than [`OWNED_BYTES_DEPTH`] cannot be measured
-/// and counts as too large to keep.
-pub(super) fn owned_bytes(object: &Object, data: &Arc<Vec<u8>>, depth: usize) -> usize {
+/// What keeping `object` parsed costs: string and stream data that is not a view into `data`
+/// (the input file), and the parsed tree around it -- an [`Object`] per value and a key per
+/// dictionary entry. An object nested deeper than [`OWNED_BYTES_DEPTH`] cannot be measured and
+/// counts as too large to keep.
+///
+/// The tree counts. Measured by its strings and streams alone, a page dictionary carrying
+/// InDesign's `/PieceInfo` -- thousands of small numeric arrays, parsed into tens of thousands of
+/// values -- weighed nothing, so the caches kept a thousand of them: a 35 MB file held 1.5 GB
+/// (#5873).
+pub(super) fn held_bytes(object: &Object, data: &Arc<Vec<u8>>, depth: usize) -> usize {
     if depth > OWNED_BYTES_DEPTH {
         return usize::MAX;
     }
+    let node = std::mem::size_of::<Object>();
+    // A key, its slot beside the value, and room in the index a large dictionary keeps.
+    let entry = |key: &[u8]| key.len().saturating_add(std::mem::size_of::<(crate::Name, Object)>()).saturating_add(16);
+    let dict =
+        |d: &crate::Dict, start: usize| d.iter().fold(start, |n, (k, v)| n.saturating_add(entry(k)).saturating_add(held_bytes(v, data, depth + 1)));
     match object {
-        Object::String(s) => s.bytes.len(),
-        Object::Stream(s) => {
-            let own = if s.raw.shares(data) { 0 } else { s.raw.len() };
-            s.dict.iter().fold(own, |n, (_, v)| n.saturating_add(owned_bytes(v, data, depth + 1)))
-        }
-        Object::Array(a) => a.iter().fold(0usize, |n, v| n.saturating_add(owned_bytes(v, data, depth + 1))),
-        Object::Dict(d) => d.iter().fold(0usize, |n, (_, v)| n.saturating_add(owned_bytes(v, data, depth + 1))),
-        _ => 0,
+        Object::String(s) => node.saturating_add(s.bytes.len()),
+        Object::Name(n) => node.saturating_add(n.len()),
+        Object::Stream(s) => dict(&s.dict, node.saturating_add(if s.raw.shares(data) { 0 } else { s.raw.len() })),
+        Object::Array(a) => a.iter().fold(node, |n, v| n.saturating_add(held_bytes(v, data, depth + 1))),
+        Object::Dict(d) => dict(d, node),
+        _ => node,
     }
 }
 

crates/cos/src/document/tests.rs

@@ -1109,7 +1109,8 @@ fn temporary_reader_keeps_file_views_and_evicts_data_of_its_own() {
     // Data of its own (decrypted streams, strings) counts: past the budget the cache is
     // cleared after the next read, including a failed one.
     let owned = Object::Stream(crate::Stream::from_raw(Dict::new(), vec![b'y'; ObjectReader::STREAM_BYTES + 1]));
-    reader.document.cache.lock().unwrap().insert(3, Arc::new(owned));
+    let size = held_bytes(&owned, &reader.document.data, 0);
+    reader.document.cache.lock().unwrap().insert(3, Arc::new(owned), size);
     assert_eq!(*reader.get(ObjRef::new(99, 0)), Object::Null);
     assert!(reader.document.cache.lock().unwrap().is_empty(), "owned data over budget is evicted");
 }
@@ -1192,13 +1193,14 @@ fn editor_cache_counts_deeply_nested_strings() {
     source.update_dict(source.root().unwrap(), |d| d.set(b"Extension".to_vec(), Object::Array(objects))).unwrap();
     let bytes = Arc::new(crate::write_full(&source, &crate::SaveOptions::default()).unwrap());
     let doc = Document::open(bytes).unwrap();
-    assert_eq!(owned_bytes(&nested, &doc.data, 0), 1 << 20);
+    let held = held_bytes(&nested, &doc.data, 0);
+    assert!((1 << 20..(1 << 20) + 4096).contains(&held), "the string and a little tree: {held}");
     let root = doc.get(doc.root().unwrap());
     for reference in root.as_dict().unwrap().get(b"Extension").unwrap().as_array().unwrap() {
         let Object::Ref(reference) = reference else { panic!("indirect object") };
         drop(doc.get(*reference));
         let cache = doc.cache.lock().unwrap();
-        let owned = cache.values().fold(0usize, |n, o| n.saturating_add(owned_bytes(o, &doc.data, 0)));
+        let owned = cache.values().fold(0usize, |n, o| n.saturating_add(held_bytes(o, &doc.data, 0)));
         let strings = cache.values().filter(|o| o.as_array().is_some()).count();
         assert!(strings << 20 <= Document::EDITOR_CACHE_BYTES + (1 << 20), "{strings} MiB strings cached, {owned} bytes counted");
     }
@@ -1240,8 +1242,8 @@ fn editor_cache_stays_bounded() {
 
 #[test]
 fn editor_cache_byte_bound_fires() {
-    // 40 × 1 MB strings: far below the count cap, above the 32 MB byte budget. Each
-    // newcomer exceeds EDITOR_LARGE_OBJECT, so every insert takes a full accounting pass.
+    // 40 × 1 MB strings: far below the count cap, above the 32 MB byte budget, which is
+    // checked after every insertion.
     let mut source = Document::new_empty();
     let objects: Vec<_> = (0..40)
         .map(|i| {
@@ -1701,3 +1703,50 @@ fn a_stream_whose_length_is_itself_and_never_ends_loads_quickly() {
     });
     rx.recv_timeout(std::time::Duration::from_secs(20)).expect("loading must not retry exponentially");
 }
+
+/// The cache keeps the sum of what its objects hold as they come and go, so neither reader has
+/// to measure the cache again on every read (#5873).
+#[test]
+fn the_object_cache_keeps_its_total_as_objects_come_and_go() {
+    let data = Arc::new(Vec::new());
+    let one = Arc::new(Object::String(crate::PdfString::literal(vec![b'x'; 100])));
+    let two = Arc::new(Object::Array(vec![Object::Int(1); 10]));
+    let (a, b) = (held_bytes(&one, &data, 0), held_bytes(&two, &data, 0));
+    let mut cache = super::state::ObjectCache::default();
+    cache.insert(1, one.clone(), a);
+    cache.insert(2, two, b);
+    assert_eq!(cache.owned(), a + b);
+    cache.insert(2, one, a);
+    assert_eq!(cache.owned(), a + a, "a replaced object no longer counts");
+    cache.retain(|n| *n == 1);
+    assert_eq!((cache.len(), cache.owned()), (1, a));
+    cache.clear();
+    assert_eq!((cache.len(), cache.owned()), (0, 0));
+}
+
+/// A dictionary of numbers owns no string or stream bytes, but parsed it is tens of thousands
+/// of values: InDesign's per-page `/PieceInfo`. Counting only strings and streams, the editing
+/// cache kept a thousand such pages and a 35 MB book held 1.5 GB (#5873). What the parsed tree
+/// holds counts now, so the byte budget bounds them.
+#[test]
+fn the_editor_cache_charges_a_dictionary_for_the_values_it_holds() {
+    let entries: String = (0..2000).map(|i| format!("/{i}[{i} 141 3 116.22 -375.993 1.0]")).collect();
+    let dict = format!("<< /PieceInfo << /InDesign << /Map << {entries} >> >> >> >>");
+    let count = 300;
+    let refs: String = (0..count).map(|i| format!("{} 0 R ", i + 2)).collect();
+    let mut bodies = vec![format!("<< /Type /Catalog /Extension [{refs}] >>")];
+    bodies.extend((0..count).map(|_| dict.clone()));
+    let bodies: Vec<&str> = bodies.iter().map(String::as_str).collect();
+    let doc = Document::open(Arc::new(build(&bodies, "/Root 1 0 R"))).unwrap();
+    let one = doc.get(crate::ObjRef::new(2, 0));
+    let held = held_bytes(&one, &doc.data, 0);
+    assert!(held > 2000 * std::mem::size_of::<Object>(), "the parsed tree counts: {held} bytes");
+    for i in 0..count {
+        drop(doc.get(crate::ObjRef::new(i as u32 + 2, 0)));
+    }
+    let cache = doc.cache.lock().unwrap();
+    assert!(cache.owned() <= Document::EDITOR_CACHE_BYTES, "{} bytes held", cache.owned());
+    assert!(cache.len() < count, "the byte budget evicted some, below the count cap: {} kept", cache.len());
+    let sum = cache.values().fold(0usize, |n, o| n.saturating_add(held_bytes(o, &doc.data, 0)));
+    assert_eq!(cache.owned(), sum, "the kept total is the cache's own");
+}

crates/cos/src/parser.rs

@@ -341,7 +341,11 @@ impl<'a> Lexer<'a> {
         let s = std::str::from_utf8(tok).map_err(|_| self.err("bad number"))?;
         if s.contains('.') {
             // Tolerate forms like "--5", "5-" or "1.2.3" by parsing the longest valid prefix.
-            let cleaned: String = s.trim_start_matches('+').replacen("--", "-", 1);
+            // Only a doubled sign needs a cleaned copy: a page of InDesign `/PieceInfo` is
+            // thousands of plain reals, and building a copy and a substring search for each
+            // was a large part of reading one (#5873).
+            let t = s.trim_start_matches('+');
+            let cleaned = if t.as_bytes().windows(2).any(|w| w == b"--") { std::borrow::Cow::Owned(t.replacen("--", "-", 1)) } else { t.into() };
             let v = cleaned.get(..real_prefix_len(cleaned.as_bytes())).and_then(|p| parse_counted::<f64>(p).ok());
             return Ok(Object::Real(v.unwrap_or(0.0)));
         }
@@ -707,4 +711,13 @@ mod tests {
         let mut lx = Lexer::new(two.as_bytes(), 0);
         assert!(lx.object().is_ok() && lx.object().is_ok());
     }
+
+    /// A plain real is read without a cleaned copy of its token; the forms that need one read
+    /// as they did (#5873).
+    #[test]
+    fn reals_read_the_same_with_and_without_a_doubled_sign() {
+        for (token, want) in [("1.5", 1.5), ("-375.993", -375.993), ("+2.25", 2.25), ("--5.5", -5.5), ("1.2.3", 1.2), ("-.5", -0.5), ("0.0", 0.0)] {
+            assert_eq!(Lexer::new(token.as_bytes(), 0).object().unwrap(), Object::Real(want), "{token}");
+        }
+    }
 }

crates/render/src/inspect/tests.rs

@@ -1870,3 +1870,45 @@ trailer << /Root 1 0 R >>
     assert!((p.width - 3874.96).abs() < 0.1, "width expected ~3874.96, got {}", p.width);
     assert!((p.height - 5493.54).abs() < 0.1, "height expected ~5493.54, got {}", p.height);
 }
+
+/// Inspection keeps every object it reads, converted, for as long as it runs, and nothing it
+/// reports comes from `/PieceInfo`. InDesign writes kilobytes of that private data into every
+/// page, and a 1,412-page book held over a gigabyte of it (#5873). It is left out of what
+/// inspection keeps, and what inspection reports is the same as for the page without it.
+#[test]
+fn inspection_keeps_no_private_page_data() {
+    use lopdf::dictionary;
+    let make = |private: bool| -> Vec<u8> {
+        let mut doc = Document::with_version("1.7");
+        let pages_id = doc.new_object_id();
+        let mut page = dictionary! {
+            "Type" => "Page",
+            "Parent" => pages_id,
+            "MediaBox" => vec![0.into(), 0.into(), 200.into(), 300.into()],
+            "Rotate" => 90,
+        };
+        if private {
+            let map: Dictionary = (0..50).map(|i| (i.to_string(), Object::Array(vec![Object::Integer(i), Object::Real(116.22)]))).collect();
+            page.set("PieceInfo", dictionary! { "InDesign" => dictionary! { "PageItemUIDToLocationDataMap" => map } });
+        }
+        let page_id = doc.add_object(page);
+        doc.objects.insert(pages_id, dictionary! { "Type" => "Pages", "Kids" => vec![page_id.into()], "Count" => 1 }.into());
+        let catalog = doc.add_object(dictionary! { "Type" => "Catalog", "Pages" => pages_id });
+        doc.trailer.set("Root", catalog);
+        let mut out = Vec::new();
+        doc.save_to(&mut out).unwrap();
+        out
+    };
+    let (with, without) = (make(true), make(false));
+    let lazy = LazyStructure::new(Arc::new(with.clone()), None).unwrap();
+    let page = lazy.get_object((2, 0)).unwrap().as_dict().unwrap();
+    assert!(!page.has(b"PieceInfo"), "the private data is not kept");
+    assert!(page.has(b"MediaBox") && page.has(b"Rotate") && page.has(b"Parent"), "the rest of the page is");
+
+    let report = |bytes: Vec<u8>| {
+        let mut info = inspect(Arc::new(bytes), None).unwrap();
+        info.file_size = 0;
+        format!("{info:?}")
+    };
+    assert_eq!(report(with), report(without), "what inspection reports does not change");
+}

crates/render/src/structure.rs

@@ -187,9 +187,20 @@ fn convert_dict(dict: &pdfcraft_cos::Dict, depth: usize) -> Result<Dictionary> {
     if depth > 100 {
         return Err(Error::RecursionLimit);
     }
-    dict.iter().map(|(key, value)| Ok((key.clone(), convert(value, depth + 1)?))).collect::<Result<Vec<_>>>().map(|items| items.into_iter().collect())
+    dict.iter()
+        .filter(|(key, _)| key.as_slice() != PRIVATE_DATA)
+        .map(|(key, value)| Ok((key.clone(), convert(value, depth + 1)?)))
+        .collect::<Result<Vec<_>>>()
+        .map(|items| items.into_iter().collect())
 }
 
+/// `/PieceInfo`: an application's private data about a page, form or document (ISO 32000-1
+/// §14.5), left out of what inspection keeps. Nothing here reads it, and every object read is
+/// kept converted for the inspection's lifetime: InDesign writes kilobytes of it into every page,
+/// and a 1,412-page book held over a gigabyte of it (#5873). The file and the editing document are
+/// untouched; this is the read-only view inspection walks.
+const PRIVATE_DATA: &[u8] = b"PieceInfo";
+
 fn convert(object: &CosObject, depth: usize) -> Result<Object> {
     if depth > 100 {
         return Err(Error::RecursionLimit);