mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-08 03:54:35 +00:00
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9
This commit is contained in:
1 parent
02dbc21207
commit
7971fb73f2
3 files changed
+47
-14
No files matched your search
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -62,7 +62,9 @@ pub(super) fn objstm_member(num: i64, offset: i64, first: Option<usize>, 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<u32, (u16, String)>,
|
||||
@@ -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<String> {
|
||||
let mut lines: Vec<String> = 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<String> = 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
|
||||
|
||||
@@ -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]
|
||||
|
||||
Reference in new issue
Block a user