From 85df8ee66817a726af56a7101bab81a2a791f823 Mon Sep 17 00:00:00 2001 From: KenM76 Date: Mon, 2 Nov 2026 17:08:05 -0500 Subject: [PATCH 1/4] feat(cos): list the objects that could not be read in doc_info MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An object whose load fails reads as null (ISO 32000-2 §7.3.10) and the file opens, but nothing said so: the repair log records only document-level repairs, so a file with an unparseable /Info showed no repair and no reason for its missing title. pdfcraft-cos Document now records each object whose load fails, with its generation and the reason, in a set shared by clones and scans like the object cache: at most 100, keyed by number, first reason kept, cut to 160 characters, built only for an object not yet listed. It is reset where the caches are (authenticate, reconstruct); edited objects drop off a document's list. Document::unreadable_objects() gives neutral lines ("object 4 0 could not be read: ..."), the engine passes them on, and automation doc_info adds an unreadable_objects array, kept apart from repairs so the "has been repaired" notice and the clean-file checks keep their meaning. Tests: a cos unit test for listing, dedup, clones, scans and edits; a cos test for the 100 cap and the no-readable-copy reason; an end-to-end doc_info test on a synthetic broken file. Each fails with the fix reverted. The core.repair-log parity entry cites them and says what is not listed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9 --- crates/automation/src/json.rs | 3 + crates/automation/src/tools.rs | 2 +- crates/automation/tests/it/main.rs | 1 + .../automation/tests/it/unreadable_objects.rs | 64 +++++++++++++++++++ crates/cos/src/document.rs | 5 +- crates/cos/src/document/objects.rs | 48 +++++++++++++- crates/cos/src/document/open.rs | 3 + crates/cos/src/document/reader.rs | 3 + crates/cos/src/document/state.rs | 48 ++++++++++++++ crates/cos/src/document/tests.rs | 58 +++++++++++++++++ crates/engine/src/document.rs | 6 ++ parity/acrobat-features.toml | 4 +- 12 files changed, 238 insertions(+), 7 deletions(-) create mode 100644 crates/automation/tests/it/unreadable_objects.rs diff --git a/crates/automation/src/json.rs b/crates/automation/src/json.rs index 9e93d4c347..c950e72f05 100644 --- a/crates/automation/src/json.rs +++ b/crates/automation/src/json.rs @@ -375,6 +375,9 @@ pub(crate) fn info(d: &Document) -> Value { })).collect::>(), "warnings": i.warnings, "repairs": d.repair_log(), + // Objects that could not be read (each reads as null): nothing was repaired, what they held is + // lost. Last, so it includes what reading the fields above failed on. + "unreadable_objects": d.unreadable_objects(), }) } diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index 7cafcb4a37..06927dee90 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -142,7 +142,7 @@ pub fn tools() -> Vec { .cmd("file.open") .with(schema(json!({ "path": { "type": "string" }, "password": { "type": "string", "description": "User or owner password for encrypted files." } }), &["path"])), t("doc_list", "List open documents", "List the open documents with their ids, page counts and unsaved state.").ro().with(schema(json!({}), &[])), - t("doc_info", "Inspect a document", "Metadata, page sizes and labels, bookmarks, annotations, form fields, links, layers, page transitions and display times, the destination it opens at, attachments, fonts, security and repair notes.") + t("doc_info", "Inspect a document", "Metadata, page sizes and labels, bookmarks, annotations, form fields, links, layers, page transitions and display times, the destination it opens at, attachments, fonts, security, repair notes and the objects that could not be read.") .ro() .cmd("file.properties") .with(schema(json!({ "doc": doc() }), &["doc"])), diff --git a/crates/automation/tests/it/main.rs b/crates/automation/tests/it/main.rs index d36dbe93a9..14e485c9f3 100644 --- a/crates/automation/tests/it/main.rs +++ b/crates/automation/tests/it/main.rs @@ -12,3 +12,4 @@ mod optimize_content_boundaries; mod redact_after_stray_q_form; mod redact_text_after_q; mod render_warnings; +mod unreadable_objects; diff --git a/crates/automation/tests/it/unreadable_objects.rs b/crates/automation/tests/it/unreadable_objects.rs new file mode 100644 index 0000000000..f19a19078a --- /dev/null +++ b/crates/automation/tests/it/unreadable_objects.rs @@ -0,0 +1,64 @@ +//! `doc_info` lists the objects of a file that could not be read: each reads as null, so the file +//! opens, but what it held is lost and nothing was repaired. + +use pdfcraft_automation::{Automation, Content}; +use serde_json::{Value, json}; + +/// A one-page PDF whose document information dictionary (object 4) is `info`. +fn with_info(info: &str) -> Vec { + let bodies = [ + "<< /Type /Catalog /Pages 2 0 R >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] >>", + info, + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, body) in bodies.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{body}\nendobj\n", i + 1).as_bytes()); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", bodies.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R /Info 4 0 R >>\nstartxref\n{xref}\n%%EOF\n", bodies.len() + 1).as_bytes()); + out +} + +fn info_of(a: &mut Automation, path: &str) -> Value { + let doc = match a.call("doc_open", &json!({ "path": path })).unwrap().remove(0) { + Content::Json(v) => v["doc"].as_u64().unwrap(), + other => panic!("expected JSON: {other:?}"), + }; + match a.call("doc_info", &json!({ "doc": doc })).unwrap().remove(0) { + Content::Json(v) => v, + other => panic!("expected JSON: {other:?}"), + } +} + +/// An unparseable `/Info` is listed with its number and why, the rest of the file reads, and +/// nothing is reported as repaired. +#[test] +fn doc_info_lists_an_object_that_could_not_be_read() { + let dir = std::env::temp_dir().join(format!("pdfcraft-unreadable-objects-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("broken.pdf"), with_info("<< /Title (Report) /Author ) >>")).unwrap(); + std::fs::write(dir.join("sound.pdf"), with_info("<< /Title (Report) >>")).unwrap(); + let mut a = Automation::new().with_root(&dir).unwrap(); + + let info = info_of(&mut a, "broken.pdf"); + let lines = info["unreadable_objects"].as_array().unwrap(); + assert_eq!(lines.len(), 1, "{info}"); + let line = lines[0].as_str().unwrap(); + assert!(line.starts_with("object 4 0 could not be read: syntax error"), "{line}"); + assert_eq!(info["repairs"], json!([]), "nothing was repaired"); + assert_eq!(info["pages"].as_array().unwrap().len(), 1, "the rest of the file reads"); + + let info = info_of(&mut a, "sound.pdf"); + assert_eq!(info["unreadable_objects"], json!([]), "{info}"); + assert_eq!(info["title"], "Report"); + let _ = std::fs::remove_dir_all(dir); +} diff --git a/crates/cos/src/document.rs b/crates/cos/src/document.rs index 082d1910d1..81eb1fd4d9 100644 --- a/crates/cos/src/document.rs +++ b/crates/cos/src/document.rs @@ -24,7 +24,7 @@ pub(crate) use reader::find; #[cfg(test)] use reader::rfind; pub(crate) use state::ObjectReader; -use state::{ObjStm, Slot}; +use state::{ObjStm, Slot, Unreadable}; pub use state::{ReadState, SourceIdentity}; #[cfg(test)] use state::{SECTION_READS, objstm_member, owned_bytes}; @@ -49,6 +49,9 @@ pub struct Document { repair_log: Arc>, cache: Arc>>>, objstms: Arc>>>, + /// See [`Self::unreadable_objects`]. Shared by clones, like the caches: it describes the + /// source, and is reset wherever they are. + unreadable: Arc>, /// 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`). jbig2_globals: Arc>>>>, diff --git a/crates/cos/src/document/objects.rs b/crates/cos/src/document/objects.rs index 2ac18cff03..fdda5f5cf5 100644 --- a/crates/cos/src/document/objects.rs +++ b/crates/cos/src/document/objects.rs @@ -86,6 +86,38 @@ impl Document { &self.repair_log } + /// Objects of the file that could not be read, lowest number first, as lines such as + /// "object 11 0 could not be read: syntax error at byte 512: ...". [`Self::get`] reads each + /// as null (ISO 32000-2 §7.3.10), so a document with one opens, but what it held is lost: + /// unlike [`Self::repair_log`], nothing was repaired. Lists the objects read so far, by this + /// document, its clones or a scan of it, that have not since been edited; past 100, a last + /// line says there were more. Opening again, authenticating or rebuilding the index starts + /// the list afresh, as it does the object cache. + pub fn unreadable_objects(&self) -> Vec { + let Ok(unreadable) = self.unreadable.lock() else { return Vec::new() }; + unreadable.lines(|num| self.overlay.contains_key(&num)) + } + + /// Record `num` as unreadable. `reason` is built only for an object not yet listed, so a file + /// whose broken object is asked for again and again pays for it once. + fn note_unreadable(&self, num: u32, reason: impl FnOnce() -> String) { + // A poisoned lock only means the object goes unlisted; the read itself still fails. + let Ok(mut unreadable) = self.unreadable.lock() else { return }; + if unreadable.wants(num) { + unreadable.note(num, self.generation(num), &reason()); + } + } + + /// Why an object listed at `offset` could not be read: what parsing it there, without resolving + /// an indirect `/Length`, found. A best account, not the only one: the reader also tried + /// elsewhere. + fn why_unreadable(&self, offset: usize) -> String { + match parse_indirect_shared(&self.data, offset, &|_| None, &self.endstreams) { + Err(e) => e.to_string(), + Ok(_) => "no readable copy was found at its cross-reference offset or elsewhere in the file".into(), + } + } + /// Add a line to [`Self::repair_log`]: a leniency applied while reading that the user should /// know about (fidelity: never silent). pub fn note_repair(&mut self, line: String) { @@ -395,8 +427,15 @@ impl Document { // cycles such as `6 0 obj << /Length 6 0 R >>`. Bound the nesting on every path. let _guard = LoadGuard::enter(num).ok_or_else(|| CosError::Syntax { offset: 0, detail: format!("reference cycle while loading object {num}") })?; - let obj = self.load(num, 0)?; - let obj = Arc::new(obj); + let obj = match self.load(num, 0) { + Ok(obj) => Arc::new(obj), + Err(e) => { + if !matches!(e, CosError::Poisoned) { + self.note_unreadable(num, || e.to_string()); + } + return Err(e); + } + }; // 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()); @@ -554,7 +593,10 @@ impl Document { .or_else(|| if self.header_offset == 0 { None } else { at(off.saturating_add(self.header_offset)) }) .or_else(|| self.located.get(&num).and_then(|(o, _)| usize::try_from(*o).ok()).and_then(at)) .or_else(|| self.scan_for(num).map(|(id, o)| self.decrypted(id, o))) - .ok_or(CosError::MissingObject(num)) + .ok_or_else(|| { + self.note_unreadable(num, || self.why_unreadable(off)); + CosError::MissingObject(num) + }) } Some(XrefEntry::InStream { stream, index }) => { let stm = self.objstm(stream)?; diff --git a/crates/cos/src/document/open.rs b/crates/cos/src/document/open.rs index 395424b771..21c0dd83ba 100644 --- a/crates/cos/src/document/open.rs +++ b/crates/cos/src/document/open.rs @@ -19,6 +19,7 @@ impl Document { repair_log: Arc::new(Vec::new()), cache: Arc::default(), objstms: Arc::default(), + unreadable: Arc::default(), jbig2_globals: Arc::default(), stream_limit: crate::object::MAX_DECODED, overlay: BTreeMap::new(), @@ -103,6 +104,7 @@ impl Document { repair_log: Arc::new(Vec::new()), cache: Arc::default(), objstms: Arc::default(), + unreadable: Arc::default(), jbig2_globals: Arc::default(), stream_limit: stream_limit.min(crate::object::MAX_DECODED), overlay: BTreeMap::new(), @@ -200,6 +202,7 @@ impl Document { // Objects read while locating the catalog were read before decryption was possible. self.cache = Arc::default(); self.objstms = Arc::default(); + self.unreadable = Arc::default(); Ok(()) } diff --git a/crates/cos/src/document/reader.rs b/crates/cos/src/document/reader.rs index d21bf9403f..1f944fd870 100644 --- a/crates/cos/src/document/reader.rs +++ b/crates/cos/src/document/reader.rs @@ -1,5 +1,6 @@ #[cfg(test)] use super::state::SECTION_READS; +use super::state::Unreadable; use super::xref::{ MAX_COMPACT_XREF_BYTES, MAX_COMPACT_XREF_ROWS, Revision, XrefEntry, XrefIndex, XrefRange, XrefRevisionSection, XrefSection, XrefSectionData, }; @@ -633,6 +634,7 @@ impl Document { } self.entries = Arc::new(XrefIndex::Materialized(Arc::new(entries.clone()))); self.cache.lock().map_err(|_| CosError::Poisoned)?.clear(); + *self.unreadable.lock().map_err(|_| CosError::Poisoned)? = Unreadable::default(); // Objects inside object streams. for num in objstms { if let Ok(o) = self.try_get(num) @@ -650,6 +652,7 @@ impl Document { let object_count = entries.len(); self.entries = Arc::new(XrefIndex::Materialized(Arc::new(entries))); self.cache.lock().map_err(|_| CosError::Poisoned)?.clear(); + *self.unreadable.lock().map_err(|_| CosError::Poisoned)? = Unreadable::default(); // Trailer: the last `trailer` dictionary or cross-reference stream dictionary with a // usable /Root (last in the file, whichever kind), else the catalog itself. One whose // /Root is a catalog (as `root_is_catalog` judges it) beats one whose /Root is another diff --git a/crates/cos/src/document/state.rs b/crates/cos/src/document/state.rs index b4323f982e..8dbba4a93a 100644 --- a/crates/cos/src/document/state.rs +++ b/crates/cos/src/document/state.rs @@ -61,6 +61,54 @@ pub(super) fn objstm_member(num: i64, offset: i64, first: Option, len: us (start < len).then_some((num, start)) } +/// Objects the reader could not read, each with the reason (see `Document::unreadable_objects`). +/// Bounded: past [`Unreadable::MAX`] objects only the fact that there were more is kept. +#[derive(Debug, Default)] +pub(super) struct Unreadable { + pub(super) objects: BTreeMap, + pub(super) more: bool, +} + +impl Unreadable { + pub(super) const MAX: usize = 100; + /// Characters of a reason kept: a parser's message can quote the bytes it stopped at. + const REASON: usize = 160; + + /// Whether `num` is to be recorded: not listed yet and there is room. A full list notes that + /// there were more, so a caller asks this before building the reason. + pub(super) fn wants(&mut self, num: u32) -> bool { + if self.objects.contains_key(&num) { + return false; + } + if self.objects.len() >= Self::MAX { + self.more = true; + return false; + } + true + } + + /// Record `num` once; the first reason given is kept. + pub(super) fn note(&mut self, num: u32, generation: u16, reason: &str) { + if self.wants(num) { + self.objects.insert(num, (generation, reason.chars().take(Self::REASON).collect())); + } + } + + /// One line per recorded object not `hidden`, then one saying there were more, if there were. + pub(super) fn lines(&self, hidden: impl Fn(u32) -> bool) -> Vec { + let mut lines: Vec = self + .objects + .iter() + .filter(|(num, _)| !hidden(**num)) + .map(|(num, (generation, reason))| format!("object {num} {generation} could not be read: {reason}")) + .collect(); + if self.more { + lines.push(format!("more objects could not be read; only the first {} are listed", Self::MAX)); + } + lines + } +} + /// Where an overlay slot came from. #[derive(Clone, Debug, PartialEq)] pub(super) enum Slot { diff --git a/crates/cos/src/document/tests.rs b/crates/cos/src/document/tests.rs index cca4b87326..c400e029d2 100644 --- a/crates/cos/src/document/tests.rs +++ b/crates/cos/src/document/tests.rs @@ -1701,3 +1701,61 @@ 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"); } + +/// A file whose object 3 (its `/Info`) can't be parsed: the rest reads. +fn with_unreadable_info() -> Vec { + build( + &["<< /Type /Catalog /Pages 2 0 R >>", "<< /Type /Pages /Kids [] /Count 0 >>", "<< /Title (Report) /Author ) >>"], + "/Root 1 0 R /Info 3 0 R", + ) +} + +/// An object that can't be parsed reads as null and is listed once, by number and generation, +/// with why. Clones and scans share the list, an edit takes the object off it (in that document +/// only), and a sound file lists nothing. +#[test] +fn unreadable_objects_are_listed_with_the_reason() { + let doc = Document::open(Arc::new(with_unreadable_info())).unwrap(); + assert!(doc.unreadable_objects().is_empty(), "nothing has read object 3 yet"); + assert_eq!(*doc.get(ObjRef::new(3, 0)), Object::Null); + assert!(doc.try_get(3).is_err()); + let lines = doc.unreadable_objects(); + assert_eq!(lines.len(), 1, "read twice, listed once: {lines:?}"); + assert!(lines[0].starts_with("object 3 0 could not be read: syntax error"), "{lines:?}"); + let scanned = doc.clone(); + assert_eq!(scanned.scan_objects_checked().filter(|(_, o)| o.is_err()).count(), 1); + assert_eq!(doc.unreadable_objects(), lines, "the scan found the same object"); + let mut edited = doc.clone(); + edited.set(ObjRef::new(3, 0), Object::Dict(Dict::new())); + assert!(edited.unreadable_objects().is_empty(), "the edit replaced it"); + assert_eq!(doc.unreadable_objects(), lines); + let sound = + build(&["<< /Type /Catalog /Pages 2 0 R >>", "<< /Type /Pages /Kids [] /Count 0 >>", "<< /Title (Report) >>"], "/Root 1 0 R /Info 3 0 R"); + let sound = Document::open(Arc::new(sound)).unwrap(); + assert_eq!(sound.scan_objects_checked().filter(|(_, o)| o.is_err()).count(), 0); + assert!(sound.unreadable_objects().is_empty()); +} + +/// The list keeps at most 100 objects and then says there were more; an object whose +/// cross-reference offset holds no object is listed with that as its reason. +#[test] +fn the_unreadable_object_list_is_bounded() { + let mut bodies = vec!["<< /Type /Catalog /Pages 2 0 R >>", "<< /Type /Pages /Kids [] /Count 0 >>"]; + bodies.extend(std::iter::repeat_n("<< /A ) >>", 120)); + let doc = Document::open(Arc::new(build(&bodies, "/Root 1 0 R"))).unwrap(); + assert_eq!(doc.scan_objects_checked().filter(|(_, o)| o.is_err()).count(), 120); + let lines = doc.unreadable_objects(); + assert_eq!(lines.len(), 101, "{:?}", lines.last()); + assert!(lines[0].starts_with("object 3 0 could not be read"), "{}", lines[0]); + assert_eq!(lines[100], "more objects could not be read; only the first 100 are listed"); + + let base = build(&["<< /Type /Catalog /Pages 2 0 R >>", "<< /Type /Pages /Kids [] /Count 0 >>", "(three)"], "/Root 1 0 R"); + let text = String::from_utf8_lossy(&base).into_owned(); + let gone = text.replacen("3 0 obj\n(three)", "9 0 obj\n(three)", 1); + let doc = Document::open(Arc::new(gone.into_bytes())).unwrap(); + assert!(doc.try_get(3).is_err()); + assert_eq!( + doc.unreadable_objects(), + ["object 3 0 could not be read: no readable copy was found at its cross-reference offset or elsewhere in the file"] + ); +} diff --git a/crates/engine/src/document.rs b/crates/engine/src/document.rs index fa98a18417..b3e93cab0b 100644 --- a/crates/engine/src/document.rs +++ b/crates/engine/src/document.rs @@ -631,6 +631,12 @@ impl Document { pub fn repair_log(&self) -> Vec { self.editor.as_ref().map(|e| e.cos.repair_log().to_vec()).unwrap_or_default() } + + /// Objects of the file that could not be read and read as null, one line each (see + /// [`pdfcraft_cos::Document::unreadable_objects`]): only those read so far. + pub fn unreadable_objects(&self) -> Vec { + self.editor.as_ref().map(|e| e.cos.unreadable_objects()).unwrap_or_default() + } } /// A title that names no document: what a web browser writes when it saves a blank page or a diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 290a410e14..451e84f4aa 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -70,8 +70,8 @@ milestone = "M1" status = "shipped" commands = [] tools = ["doc_info"] -evidence = ["crates/ui-egui/tests/it/viewer.rs::damaged_files_say_they_were_repaired"] -notes = "a notice says the file was repaired; Document Properties ▸ Advanced lists the repairs; doc_info reports them" +evidence = ["crates/ui-egui/tests/it/viewer.rs::damaged_files_say_they_were_repaired", "crates/cos/src/document/tests.rs::unreadable_objects_are_listed_with_the_reason", "crates/automation/tests/it/unreadable_objects.rs::doc_info_lists_an_object_that_could_not_be_read"] +notes = "a notice says the file was repaired; Document Properties ▸ Advanced lists the repairs; doc_info reports them, and lists apart (unreadable_objects) each object that could not be read and reads as null, with its number and why: only objects read so far, including any object a scan, save or audit read, at most 100; a stream whose own data fails to decode is not listed, but the objects of an object stream that cannot be decoded are" [[feature]] id = "core.xref-streams" From 649e97930f0f516ac684393f9b16e5c3c00120a6 Mon Sep 17 00:00:00 2001 From: KenM76 Date: Mon, 2 Nov 2026 18:03:00 -0500 Subject: [PATCH 2/4] fix(cos): a rebuilt index keeps the read failures its scan found Rebuilding the cross-reference index from object headers reset the unreadable-object list a second time, after adding the object-stream members, so an object the header scan failed to read dropped off unreadable_objects() unless something read it again. Those failures still stand against the full index: the added members leave every scanned object's entry as it was, and a /Length found only then changes how much of a stream is read, never whether it reads. The list is now reset once, before the scan; the object cache is still cleared after it. Test: a_rebuilt_file_lists_the_objects_its_scan_could_not_read fails without the fix (the list is empty) and passes with it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9 --- crates/cos/src/document/reader.rs | 3 ++- crates/cos/src/document/tests.rs | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/cos/src/document/reader.rs b/crates/cos/src/document/reader.rs index 1f944fd870..d556c1423b 100644 --- a/crates/cos/src/document/reader.rs +++ b/crates/cos/src/document/reader.rs @@ -651,8 +651,9 @@ impl Document { } let object_count = entries.len(); self.entries = Arc::new(XrefIndex::Materialized(Arc::new(entries))); + // Failures from the scan stand: the members added change no scanned entry, and a + // `/Length` found only now cannot make a stream unreadable. self.cache.lock().map_err(|_| CosError::Poisoned)?.clear(); - *self.unreadable.lock().map_err(|_| CosError::Poisoned)? = Unreadable::default(); // Trailer: the last `trailer` dictionary or cross-reference stream dictionary with a // usable /Root (last in the file, whichever kind), else the catalog itself. One whose // /Root is a catalog (as `root_is_catalog` judges it) beats one whose /Root is another diff --git a/crates/cos/src/document/tests.rs b/crates/cos/src/document/tests.rs index c400e029d2..b587471c62 100644 --- a/crates/cos/src/document/tests.rs +++ b/crates/cos/src/document/tests.rs @@ -1736,6 +1736,17 @@ fn unreadable_objects_are_listed_with_the_reason() { assert!(sound.unreadable_objects().is_empty()); } +/// A file rebuilt from its object headers still lists an object the scan could not read. +#[test] +fn a_rebuilt_file_lists_the_objects_its_scan_could_not_read() { + let text = "%PDF-1.7\n1 0 obj\n<< /Type /Catalog /Pages 2 0 R >>\nendobj\n2 0 obj\n<< /Type /Pages /Kids [] /Count 0 >>\nendobj\n3 0 obj\n<< /A ) >>\nendobj\n%%EOF\n"; + let doc = Document::open(Arc::new(text.as_bytes().to_vec())).unwrap(); + assert!(doc.repair_log().iter().any(|l| l.starts_with("rebuilt cross-reference index")), "{:?}", doc.repair_log()); + let lines = doc.unreadable_objects(); + assert_eq!(lines.len(), 1, "{lines:?}"); + assert!(lines[0].starts_with("object 3 0 could not be read: syntax error"), "{lines:?}"); +} + /// The list keeps at most 100 objects and then says there were more; an object whose /// cross-reference offset holds no object is listed with that as its reason. #[test] From 57ca07dea3b56bfb678f7831bbc11767c03b476f Mon Sep 17 00:00:00 2001 From: KenM76 Date: Tue, 3 Nov 2026 12:51:45 -0500 Subject: [PATCH 3/4] cos: test that a clone's scan reaches an unread original The clone-scan check ran after the original had already listed the object, so it passed whether or not scan findings were shared (CodeRabbit on #5816). A fresh document is now cloned before anything reads it, only the clone is scanned, and the original must list the object. Seen failing with the scan's list unshared. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9 --- crates/cos/src/document/tests.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/cos/src/document/tests.rs b/crates/cos/src/document/tests.rs index 50a69159f1..fda2128ebc 100644 --- a/crates/cos/src/document/tests.rs +++ b/crates/cos/src/document/tests.rs @@ -1727,6 +1727,10 @@ fn unreadable_objects_are_listed_with_the_reason() { let scanned = doc.clone(); assert_eq!(scanned.scan_objects_checked().filter(|(_, o)| o.is_err()).count(), 1); assert_eq!(doc.unreadable_objects(), lines, "the scan found the same object"); + let unread = Document::open(Arc::new(with_unreadable_info())).unwrap(); + let scanned = unread.clone(); + assert_eq!(scanned.scan_objects_checked().filter(|(_, o)| o.is_err()).count(), 1); + assert_eq!(unread.unreadable_objects(), lines, "a clone's scan reaches the original"); let mut edited = doc.clone(); edited.set(ObjRef::new(3, 0), Object::Dict(Dict::new())); assert!(edited.unreadable_objects().is_empty(), "the edit replaced it"); From 7971fb73f2794cd9089838dd07365cfa9f41ca5a Mon Sep 17 00:00:00 2001 From: KenM76 Date: Wed, 4 Nov 2026 11:04:00 -0500 Subject: [PATCH 4/4] fix(cos): a clone's edited objects don't use up its unreadable-object allowance The unreadable list is shared by clones, and each one hides the entries it has edited. The 100-entry cap counted the whole shared list, so a clone that edited every listed object recorded none of its own failures and listed only "more objects could not be read". The cap now counts the entries the asking document still shows. Each document still lists at most 100 entries, then the "more" line. Test: objects_a_clone_edited_do_not_use_up_its_unreadable_allowance. It was seen failing with each half reverted (left: 1, right: 10; then left: 110, right: 101). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9 --- crates/cos/src/document/objects.rs | 5 +++-- crates/cos/src/document/state.rs | 30 ++++++++++++++++++------------ crates/cos/src/document/tests.rs | 26 ++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 14 deletions(-) diff --git a/crates/cos/src/document/objects.rs b/crates/cos/src/document/objects.rs index f85b7b9191..6cf1df9b1e 100644 --- a/crates/cos/src/document/objects.rs +++ b/crates/cos/src/document/objects.rs @@ -103,8 +103,9 @@ impl Document { fn note_unreadable(&self, num: u32, reason: impl FnOnce() -> String) { // A poisoned lock only means the object goes unlisted; the read itself still fails. let Ok(mut unreadable) = self.unreadable.lock() else { return }; - if unreadable.wants(num) { - unreadable.note(num, self.generation(num), &reason()); + let hidden = |n: u32| self.overlay.contains_key(&n); + if unreadable.wants(num, hidden) { + unreadable.note(num, self.generation(num), &reason(), hidden); } } diff --git a/crates/cos/src/document/state.rs b/crates/cos/src/document/state.rs index a6171479ca..3627280960 100644 --- a/crates/cos/src/document/state.rs +++ b/crates/cos/src/document/state.rs @@ -62,7 +62,9 @@ pub(super) fn objstm_member(num: i64, offset: i64, first: Option, len: us } /// Objects the reader could not read, each with the reason (see `Document::unreadable_objects`). -/// Bounded: past [`Unreadable::MAX`] objects only the fact that there were more is kept. +/// Bounded: a document records past [`Unreadable::MAX`] of the objects it shows only the fact that +/// there were more. Entries a clone has edited are hidden from it and do not count, so the whole +/// list holds at most `MAX` plus the objects edited in clones. #[derive(Debug, Default)] pub(super) struct Unreadable { pub(super) objects: BTreeMap, @@ -74,13 +76,15 @@ impl Unreadable { /// Characters of a reason kept: a parser's message can quote the bytes it stopped at. const REASON: usize = 160; - /// Whether `num` is to be recorded: not listed yet and there is room. A full list notes that - /// there were more, so a caller asks this before building the reason. - pub(super) fn wants(&mut self, num: u32) -> bool { + /// Whether `num` is to be recorded: not listed yet and there is room among the entries the + /// asking document still shows (`hidden` are the ones it has edited, as in [`Self::lines`]), + /// so a clone's edits never use up its own allowance. A full list notes that there were more, + /// so a caller asks this before building the reason. + pub(super) fn wants(&mut self, num: u32, hidden: impl Fn(u32) -> bool) -> bool { if self.objects.contains_key(&num) { return false; } - if self.objects.len() >= Self::MAX { + if self.objects.keys().filter(|n| !hidden(**n)).count() >= Self::MAX { self.more = true; return false; } @@ -88,21 +92,23 @@ impl Unreadable { } /// Record `num` once; the first reason given is kept. - pub(super) fn note(&mut self, num: u32, generation: u16, reason: &str) { - if self.wants(num) { + pub(super) fn note(&mut self, num: u32, generation: u16, reason: &str, hidden: impl Fn(u32) -> bool) { + if self.wants(num, hidden) { self.objects.insert(num, (generation, reason.chars().take(Self::REASON).collect())); } } /// One line per recorded object not `hidden`, then one saying there were more, if there were. pub(super) fn lines(&self, hidden: impl Fn(u32) -> bool) -> Vec { - let mut lines: Vec = self - .objects - .iter() - .filter(|(num, _)| !hidden(**num)) + // Clones share the list and each hides its own edits, so the list may hold more than + // `MAX` entries in all; each document still shows at most `MAX`. + let mut shown = self.objects.iter().filter(|(num, _)| !hidden(**num)); + let mut lines: Vec = shown + .by_ref() + .take(Self::MAX) .map(|(num, (generation, reason))| format!("object {num} {generation} could not be read: {reason}")) .collect(); - if self.more { + if self.more || shown.next().is_some() { lines.push(format!("more objects could not be read; only the first {} are listed", Self::MAX)); } lines diff --git a/crates/cos/src/document/tests.rs b/crates/cos/src/document/tests.rs index 2f40b25b07..4dbc99bb4e 100644 --- a/crates/cos/src/document/tests.rs +++ b/crates/cos/src/document/tests.rs @@ -1919,6 +1919,32 @@ fn the_unreadable_object_list_is_bounded() { ); } +/// The unreadable list is shared by clones, and each hides what it has edited. A clone that edited +/// every listed object still lists its own failures: hidden entries don't use up its allowance. +#[test] +fn objects_a_clone_edited_do_not_use_up_its_unreadable_allowance() { + let mut bodies = vec!["<< /Type /Catalog /Pages 2 0 R >>", "<< /Type /Pages /Kids [] /Count 0 >>"]; + bodies.extend(std::iter::repeat_n("<< /A ) >>", 110)); + let doc = Document::open(Arc::new(build(&bodies, "/Root 1 0 R"))).unwrap(); + for num in 3..103 { + assert!(doc.try_get(num).is_err()); + } + assert_eq!(doc.unreadable_objects().len(), 100); + let mut edited = doc.clone(); + for num in 3..103 { + edited.set(ObjRef::new(num, 0), Object::Null); + } + for num in 103..113 { + assert!(edited.try_get(num).is_err()); + } + let lines = edited.unreadable_objects(); + assert_eq!(lines.len(), 10, "{lines:?}"); + assert!(lines[0].starts_with("object 103 0 could not be read"), "{}", lines[0]); + let original = doc.unreadable_objects(); + assert_eq!(original.len(), 101, "the original still shows at most 100, then says there were more"); + assert_eq!(original[100], "more objects could not be read; only the first 100 are listed"); +} + /// 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]