mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-24 02:13:12 +00:00
fix(render): read each object stream's index once (vendored hayro-syntax patch 22) (#2427)
* fix(render): read each object stream's index once (vendored hayro-syntax patch 22) `XRef::get_with` parsed an object stream's whole index of `objnum offset` pairs again for every object it fetched from that stream, so reading all of a stream of N objects took N² steps. Firefox keeps every object of a page in one stream: the pdf.js corpus file bug1978317.pdf (65,542 objects in one stream) took 37 s to draw and 40 s to extract its text, and was a known timeout in the robustness baseline. The parsed index is now kept per stream, beside the decoded stream data it indexes (`ObjectStreamIndexes`), and never cleared, as that data isn't. A malformed index is remembered as malformed; `/N` or `/First` that can't be read (a reference that can't be followed while another stream loads) is not remembered. An offset past `usize` is malformed instead of wrapping. A poisoned lock is still used: entries are inserted whole. Release build, Linux: bug1978317.pdf draws in 0.5 s (was 37 s) and extracts its text in 1.9 s (was 40 s), and leaves the baseline's known timeouts; 40,000 pages in one stream open in 0.2 s (was 54 s). `raster::tests::objects_in_one_large_object_stream_are_read_in_linear_time` times out at 20 s with the fix reverted and passes in 0.2 s with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MoEx9Nj8KC4WKDcf5YaokQ * test(render): an object stream offset past usize is malformed, not a panic Review follow-up on #2427. Mutation testing by mtlcl410 found that replacing patch (22)'s `checked_add` in `read_object_stream_index` with a plain `+` survived every test, although in a debug build that `+` panics on the very input it guards against (and wraps in a release build). Their test pins it: it fails with "attempt to add with overflow" under that mutant and passes on this branch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -216,6 +216,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md
|
||||
- **2026-10-12 (M7, picture right-click menu):** The Edit text & images menu of a picture gains Cut Image, Redact Image (a redaction mark over the picture, applied from Redact a PDF), Create Link… and Recognize Text… (for that page), in all 21 catalogs. The menu was drawn only while the pointer was over the picture, so its items outside the picture (most of a logo's menu, and Delete below a page) could not be clicked; an open menu now stays drawn. Still missing from Acrobat's menu: Arrange (bring to front, send backward), Remove crop, Draw freehand and Edit image using… (opening an external editor).
|
||||
- **2026-10-12 (M2, inline-image boundaries):** Unfiltered inline images with known sample layouts keep pixel bytes that look like the `EI` terminator when content is rewritten. Regression tests cover row padding, colour-space and parameter aliases, masks and malformed sizes; classic and packed saved PDFs retain the pixels in independent readers and renderers. Filtered and resource-defined layouts retain the existing tolerant heuristic. Estimates unchanged.
|
||||
- **2026-10-12 (M3, drawing markups):** While a comment is being drawn, the pointer in a band along any edge of the view (or just past it) scrolls the view up, down, left or right, so points beyond what is shown can be reached. A polygon, cloud or run of connected lines takes back its last point with Backspace, Delete or Undo (the first point's removal cancels it). Edge scrolling has unit tests of its maths only; it has not been exercised end to end in the app by an automated test.
|
||||
- **2026-10-15 (M2, large object streams):** An object stream's index of `objnum offset` pairs is read once per stream instead of once per object fetched from it (vendored hayro-syntax patch 22), so reading every object of a stream of N objects no longer takes N² steps. A Firefox page whose 65,542 objects share one stream (pdf.js corpus `bug1978317.pdf`) drew in 37 s and extracted its text in 40 s; it now takes 0.5 s and 1.9 s, and leaves the robustness baseline's known timeouts. 40,000 pages in one stream open in 0.2 s instead of 54 s. The regression fails before the fix. Estimates unchanged.
|
||||
- **2026-10-12 (gap 6, #1319, #1305, #1324):** Editing an XFA form keeps a radio item's stored text when it contains a space, so a later layout still selects it. Replace Pages on a split selection replaces those pages and leaves the gap; editing the dialog range still replaces the span. Marking a figure decorative rewrites only that figure's content stream, so a header, added text and unknown stream entries stay. Estimates unchanged.
|
||||
- **2026-10-15 (M14, gap 24):** `misc.localization` and `misc.installers` move from planned to partial. Localization: 22 catalogs ship, but Chinese shows tofu in releases, Telugu has no release font and Arabic is not mirrored. Installers: macOS and Windows releases are signed, Linux packages are not. The Acrobat menu diff itself remains open. Estimates unchanged.
|
||||
- **2026-10-15 (M14, PNG export):** Export ▸ Image as PNG, `pdfcraft-cli render` and the `page_render` automation/MCP tool write an opaque page as RGB at deflate level 3 instead of RGBA at the default level 6: the files stay the same size and are written 3–5× faster (a 300-dpi scan drawn at 220 dpi: 805 → 167 ms; a text page at 220 dpi: 70 → 40 ms; an annual-report page at 150 dpi: 179 → 64 ms). A page with translucent pixels is still written as RGBA. The automation tools use the engine's encoder instead of a copy of it. Estimates unchanged.
|
||||
|
||||
@@ -4612,6 +4612,88 @@ trailer << /Root 1 0 R >>
|
||||
pdf
|
||||
}
|
||||
|
||||
/// A document whose `pages` page dictionaries all live in one (uncompressed) object stream,
|
||||
/// found through a cross-reference stream: every page shows the content `0 0 1 rg 10 10 80 80
|
||||
/// re f` on a 100 pt square, except the last, which is 50 × 100 pt.
|
||||
fn pages_in_one_object_stream(pages: usize) -> Vec<u8> {
|
||||
// 1 catalog, 2 page tree, 3 content, 4 the object stream, 5.. the pages, then the xref.
|
||||
let page = |n: usize| {
|
||||
let size = if n + 1 == pages { "50 100" } else { "100 100" };
|
||||
format!("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 {size}] /Contents 3 0 R >>\n")
|
||||
};
|
||||
let (mut index, mut body) = (String::new(), String::new());
|
||||
for n in 0..pages {
|
||||
index.push_str(&format!("{} {} ", n + 5, body.len()));
|
||||
body.push_str(&page(n));
|
||||
}
|
||||
index.push('\n');
|
||||
let kids: Vec<String> = (0..pages).map(|n| format!("{} 0 R", n + 5)).collect();
|
||||
let content = "0 0 1 rg 10 10 80 80 re f";
|
||||
let mut pdf = b"%PDF-1.5\n".to_vec();
|
||||
let mut offsets = Vec::new();
|
||||
for (num, obj) in [
|
||||
(1, "<< /Type /Catalog /Pages 2 0 R >>".to_string()),
|
||||
(2, format!("<< /Type /Pages /Kids [{}] /Count {pages} >>", kids.join(" "))),
|
||||
(3, format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len())),
|
||||
(
|
||||
4,
|
||||
format!(
|
||||
"<< /Type /ObjStm /N {pages} /First {} /Length {} >>\nstream\n{index}{body}\nendstream",
|
||||
index.len(),
|
||||
index.len() + body.len()
|
||||
),
|
||||
),
|
||||
] {
|
||||
offsets.push(pdf.len());
|
||||
pdf.extend_from_slice(format!("{num} 0 obj\n{obj}\nendobj\n").as_bytes());
|
||||
}
|
||||
// Cross-reference stream rows of /W [1 4 4]: free 0, objects 1–4 in the file, the pages
|
||||
// at their index in object 4, and the cross-reference stream itself.
|
||||
let xref_num = pages + 5;
|
||||
let xref_at = pdf.len();
|
||||
let mut rows = vec![0u8, 0, 0, 0, 0, 0, 0, 0xff, 0xff];
|
||||
let row = |kind: u8, a: usize, b: usize| [[kind].as_slice(), &(a as u32).to_be_bytes(), &(b as u32).to_be_bytes()].concat();
|
||||
for at in &offsets {
|
||||
rows.extend(row(1, *at, 0));
|
||||
}
|
||||
for n in 0..pages {
|
||||
rows.extend(row(2, 4, n));
|
||||
}
|
||||
rows.extend(row(1, xref_at, 0));
|
||||
pdf.extend_from_slice(
|
||||
format!("{xref_num} 0 obj\n<< /Type /XRef /Size {} /W [1 4 4] /Root 1 0 R /Length {} >>\nstream\n", xref_num + 1, rows.len()).as_bytes(),
|
||||
);
|
||||
pdf.extend_from_slice(&rows);
|
||||
pdf.extend_from_slice(format!("\nendstream\nendobj\nstartxref\n{xref_at}\n%%EOF\n").as_bytes());
|
||||
pdf
|
||||
}
|
||||
|
||||
/// An object stream's index of `objnum offset` pairs is read once, not for every object
|
||||
/// fetched from it (vendored hayro-syntax patch 22). Read again each time, all of a stream of
|
||||
/// N objects took N² steps: a Firefox page keeping 65,542 objects in one stream took 37 s to
|
||||
/// draw and 40 s to extract its text. These 40,000 pages in one stream took about a minute
|
||||
/// to open that way; now a fraction of a second. The last page, at the end of the index,
|
||||
/// still has its own size.
|
||||
#[test]
|
||||
fn objects_in_one_large_object_stream_are_read_in_linear_time() {
|
||||
const PAGES: usize = 40_000;
|
||||
let pdf = pages_in_one_object_stream(PAGES);
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
std::thread::spawn(move || {
|
||||
let mut r = PageRenderer::new(Arc::new(pdf), RenderConfig::default());
|
||||
let first = r.render(RenderRequest { page: 0, kind: RequestKind::Pixels, tile: None, scale: 1.0, tag: 0, preview: false });
|
||||
let last = r.render(RenderRequest { page: PAGES - 1, kind: RequestKind::Pixels, tile: None, scale: 1.0, tag: 0, preview: false });
|
||||
let _ = tx.send((first, last));
|
||||
});
|
||||
let (first, last) =
|
||||
rx.recv_timeout(std::time::Duration::from_secs(20)).expect("reading every object of one object stream must not take quadratic time");
|
||||
assert!(first.error.is_none(), "{:?}", first.error);
|
||||
assert_eq!((first.width, first.height), (100, 100));
|
||||
assert_eq!(rgb_at(&first.rgba, first.width as usize, 50, 50), [0, 0, 255]);
|
||||
assert!(last.error.is_none(), "{:?}", last.error);
|
||||
assert_eq!((last.width, last.height), (50, 100), "the last page is read from its own offset");
|
||||
}
|
||||
|
||||
/// A page whose content expands far beyond its compressed stream stops at the expansion budget,
|
||||
/// which is 64 times the compressed size and at least `MAX_DECODED_STREAM`, and the page says so.
|
||||
#[test]
|
||||
|
||||
@@ -11,6 +11,7 @@
|
||||
> **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M6: AcroForm XML text; estimates unchanged)
|
||||
> **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M6: preserve XML/XFDF attribute whitespace; refreshed upstream; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS)
|
||||
> **Last reviewed:** 2026-10-15 · **Last updated:** 2026-10-15 · **Change:** minor (M1: raw-deflate fallback preserves decoding-limit errors; refreshed upstream; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS)
|
||||
> **Last reviewed:** 2026-10-14 · **Last updated:** 2026-10-15 · **Change:** minor (M2: objects in one large object stream are read in linear time; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS)
|
||||
> **Last reviewed:** 2026-10-13 · **Last updated:** 2026-10-14 · **Change:** minor (M14: the Italian catalog has every entry the other catalogs had and the code looks up, 2,528 entries; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS)
|
||||
> **Last reviewed:** 2026-10-16 | **Last updated:** 2026-10-16 | **Change:** minor (M10 #1201 partial: lossless image Decode; estimates unchanged) | **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS)
|
||||
> **Last reviewed:** 2026-10-12 · **Last updated:** 2026-10-13 · **Change:** minor (M2/M6: spot-colour images convert once per pixel value; widget placement and the Fields panel scale with the field count; estimates unchanged) · **Target:** Adobe Acrobat Pro (Acrobat DC, continuous track 26.002.21931, macOS)
|
||||
@@ -221,6 +222,7 @@ M2 (renderer), M7 (content editing), M10 (OCR and Office), M11 (Preflight and st
|
||||
| 2026-10-12 | minor | M14/#1395: XDG configuration preferences, safe migration and fallback; logs/recovery retain data locations, portable/non-XDG unchanged. Estimates unchanged. |
|
||||
| 2026-10-12 | minor | M6: XFDF field import and data extraction preserve all text around XML comments and processing instructions; synthetic save/reopen regressions. Estimates unchanged. |
|
||||
| 2026-10-12 | minor | M2: preserve whitespace-bounded EI bytes in known unfiltered inline images; row padding, masks and aliases tested, with independent saved-PDF pixel checks. Estimates unchanged. |
|
||||
| 2026-10-15 | minor | M2: an object stream's index is parsed once per stream (hayro-syntax patch 22), not once per object fetched from it; a page whose 65,542 objects share one stream draws in 0.5 s instead of 37 s. Estimates unchanged. |
|
||||
| 2026-10-12 | minor | M1 #1338: a full rewrite keeps unknown trailer entries and objects reachable only from them. Estimates unchanged. |
|
||||
| 2026-10-12 | minor | M7 #1443: adding a word to a multi-line paragraph keeps its line breaks. Composite subset fonts whose `/W` leaves out default-width glyphs (Chrome, Skia) no longer measure those letters as missing, and paragraphs drawn turned on the page group into rows along their own baselines. Estimates unchanged. |
|
||||
| 2026-10-15 | minor | M14: PNG export (Export ▸ Image, `pdfcraft-cli render`, `page_render`) writes opaque pages as RGB at deflate level 3: the same file size, 3–5× faster to write. Estimates unchanged. |
|
||||
|
||||
Vendored
+1
-1
File diff suppressed because one or more lines are too long
Vendored
+102
-17
@@ -289,6 +289,28 @@ impl XRef {
|
||||
Some(f(&mut cache))
|
||||
}
|
||||
|
||||
/// PdfCraft patch: (hayro-syntax (22)) run `f` on this PDF's parsed object stream indexes
|
||||
/// ([`ObjectStreamIndexes`]). `None` when there is none to use (the dummy xref, or a no_std
|
||||
/// cache already borrowed), and the caller then parses the index without it. A poisoned lock
|
||||
/// is still used: entries are complete `Arc`s inserted in one step, so a panic elsewhere
|
||||
/// cannot have left one half-written.
|
||||
fn with_object_stream_indexes<T>(
|
||||
&self,
|
||||
f: impl FnOnce(&mut ObjectStreamIndexes) -> T,
|
||||
) -> Option<T> {
|
||||
let Inner::Some(repr) = &self.0 else {
|
||||
return None;
|
||||
};
|
||||
#[cfg(feature = "std")]
|
||||
let mut indexes = repr
|
||||
.object_stream_indexes
|
||||
.lock()
|
||||
.unwrap_or_else(|poison| poison.into_inner());
|
||||
#[cfg(not(feature = "std"))]
|
||||
let mut indexes = repr.object_stream_indexes.try_borrow_mut().ok()?;
|
||||
Some(f(&mut indexes))
|
||||
}
|
||||
|
||||
fn new(
|
||||
data: PdfData,
|
||||
xref_map: XrefMap,
|
||||
@@ -310,6 +332,7 @@ impl XRef {
|
||||
trailer_data,
|
||||
password: password.to_vec(),
|
||||
dict_cache: Arc::new(Mutex::new(DictCache::default())),
|
||||
object_stream_indexes: Arc::new(Mutex::new(ObjectStreamIndexes::default())),
|
||||
})));
|
||||
|
||||
// We read the trailer twice, once to determine the encryption used and then a second
|
||||
@@ -622,10 +645,35 @@ impl XRef {
|
||||
return None;
|
||||
}
|
||||
|
||||
let stream = self.get_with::<Stream<'_>>(obj_stream_id, &ctx)?;
|
||||
// PdfCraft patch: (hayro-syntax (22)) the stream's index of `objnum offset` pairs
|
||||
// is parsed once and kept, as its decoded data is: parsed again for every object
|
||||
// fetched from it, reading all of a stream of N objects took N² steps (one
|
||||
// Firefox page kept 65,542 objects in one stream and took 37 s to draw).
|
||||
let data = repr.data.get_with(obj_stream_id, &ctx)?;
|
||||
let object_stream = ObjectStream::new(stream, data, &ctx)?;
|
||||
object_stream.get(index)
|
||||
let known = self
|
||||
.with_object_stream_indexes(|indexes| indexes.get(&obj_stream_id).cloned())
|
||||
.flatten();
|
||||
let offsets = match known {
|
||||
// A stream whose pairs don't parse stays unreadable, as it was.
|
||||
Some(offsets) => offsets?,
|
||||
None => {
|
||||
let stream = self.get_with::<Stream<'_>>(obj_stream_id, &ctx)?;
|
||||
// `/N` and `/First` may be references that can't be followed while
|
||||
// another stream loads: reading them fails without being remembered.
|
||||
let num_objects = stream.dict().get::<usize>(N)?;
|
||||
let first_offset = stream.dict().get::<usize>(FIRST)?;
|
||||
let parsed =
|
||||
read_object_stream_index(num_objects, first_offset, data).map(Arc::new);
|
||||
self.with_object_stream_indexes(|indexes| {
|
||||
indexes
|
||||
.entry(obj_stream_id)
|
||||
.or_insert_with(|| parsed.clone())
|
||||
.clone()
|
||||
})
|
||||
.unwrap_or(parsed)?
|
||||
}
|
||||
};
|
||||
ObjectStream::with_offsets(data, &ctx, offsets).get(index)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -698,6 +746,7 @@ impl TrailerData {
|
||||
#[derive(Debug, Clone)]
|
||||
struct SomeRepr {
|
||||
dict_cache: Arc<Mutex<DictCache>>,
|
||||
object_stream_indexes: Arc<Mutex<ObjectStreamIndexes>>,
|
||||
data: Arc<Data>,
|
||||
map: Arc<RwLock<MapRepr>>,
|
||||
metadata: Arc<Metadata>,
|
||||
@@ -1067,34 +1116,62 @@ fn get_decryptor(trailer_dict: &Dict<'_>, password: &[u8]) -> Result<Decryptor,
|
||||
}
|
||||
}
|
||||
|
||||
/// PdfCraft patch: (hayro-syntax (22)) each object stream's parsed `(object number, offset)`
|
||||
/// pairs ([`read_object_stream_index`]) by the stream's identifier, `None` for pairs that don't
|
||||
/// parse. Never cleared, like the decoded stream data they index (`Data`), which a repair keeps
|
||||
/// too. Bounded by that data: every pair takes at least three of its bytes.
|
||||
type ObjectStreamIndexes = FxHashMap<ObjectIdentifier, Option<Arc<Vec<(u32, usize)>>>>;
|
||||
|
||||
/// The `(object number, offset)` pairs at the start of an object stream's decoded `data`, the
|
||||
/// offsets measured from the start of `data`. `None` when the pairs don't parse. Reads no
|
||||
/// further than the data holds, whatever `/N` claims.
|
||||
fn read_object_stream_index(
|
||||
num_objects: usize,
|
||||
first_offset: usize,
|
||||
data: &[u8],
|
||||
) -> Option<Vec<(u32, usize)>> {
|
||||
let mut r = Reader::new(data);
|
||||
|
||||
let mut offsets = vec![];
|
||||
|
||||
for _ in 0..num_objects {
|
||||
r.skip_white_spaces_and_comments();
|
||||
// Skip object number
|
||||
let obj_num = r.read_without_context::<u32>()?;
|
||||
r.skip_white_spaces_and_comments();
|
||||
let relative_offset = r.read_without_context::<usize>()?;
|
||||
// PdfCraft patch: (hayro-syntax (22)) an offset past `usize` is malformed (it wrapped).
|
||||
offsets.push((obj_num, first_offset.checked_add(relative_offset)?));
|
||||
}
|
||||
|
||||
Some(offsets)
|
||||
}
|
||||
|
||||
struct ObjectStream<'a> {
|
||||
data: &'a [u8],
|
||||
ctx: ReaderContext<'a>,
|
||||
offsets: Vec<(u32, usize)>,
|
||||
offsets: Arc<Vec<(u32, usize)>>,
|
||||
}
|
||||
|
||||
impl<'a> ObjectStream<'a> {
|
||||
fn new(inner: Stream<'_>, data: &'a [u8], ctx: &ReaderContext<'a>) -> Option<Self> {
|
||||
let num_objects = inner.dict().get::<usize>(N)?;
|
||||
let first_offset = inner.dict().get::<usize>(FIRST)?;
|
||||
let offsets = read_object_stream_index(num_objects, first_offset, data)?;
|
||||
|
||||
let mut r = Reader::new(data);
|
||||
|
||||
let mut offsets = vec![];
|
||||
|
||||
for _ in 0..num_objects {
|
||||
r.skip_white_spaces_and_comments();
|
||||
// Skip object number
|
||||
let obj_num = r.read_without_context::<u32>()?;
|
||||
r.skip_white_spaces_and_comments();
|
||||
let relative_offset = r.read_without_context::<usize>()?;
|
||||
offsets.push((obj_num, first_offset + relative_offset));
|
||||
}
|
||||
Some(Self::with_offsets(data, ctx, Arc::new(offsets)))
|
||||
}
|
||||
|
||||
/// PdfCraft patch: (hayro-syntax (22)) an object stream whose index is already parsed.
|
||||
fn with_offsets(
|
||||
data: &'a [u8],
|
||||
ctx: &ReaderContext<'a>,
|
||||
offsets: Arc<Vec<(u32, usize)>>,
|
||||
) -> Self {
|
||||
let mut ctx = ctx.clone();
|
||||
ctx.set_in_object_stream(true);
|
||||
|
||||
Some(Self { data, ctx, offsets })
|
||||
Self { data, ctx, offsets }
|
||||
}
|
||||
|
||||
fn get<T>(&self, index: u32) -> Option<T>
|
||||
@@ -1369,4 +1446,12 @@ mod tests {
|
||||
let mut reader = Reader::new(data);
|
||||
assert!(read_xref_table_trailer(&mut reader, &ReaderContext::dummy()).is_none());
|
||||
}
|
||||
|
||||
/// PdfCraft patch: (hayro-syntax (22)) an object's offset past `usize` is malformed: a plain
|
||||
/// `+` there panics in a debug build and wraps in a release one.
|
||||
#[test]
|
||||
fn an_object_stream_offset_past_usize_is_malformed_not_a_panic() {
|
||||
assert_eq!(read_object_stream_index(1, usize::MAX, b"5 1"), None);
|
||||
assert_eq!(read_object_stream_index(1, 10, b"5 1"), Some(vec![(5, 11)]));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,7 +4,6 @@
|
||||
"auth-event-ef-open.pdf": "open-error",
|
||||
"bug1020226.pdf": "open-error",
|
||||
"bug1782186.pdf": "open-error",
|
||||
"bug1978317.pdf": "timeout",
|
||||
"encrypted-attachment.pdf": "open-error",
|
||||
"issue15590.pdf": "open-error",
|
||||
"issue15893_reduced.pdf": "open-error",
|
||||
|
||||
Reference in New Issue
Block a user