fix(parser): recover root-bearing linearized xrefs - #435
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Shadow auto-approve: would not auto-approve because issues were found.
Fix all with cubic | Re-trigger cubic
Guard short inputs, parse trailer dictionaries without mistaking comments or nested keys for /Root, bound trailer searches to the next xref table, and preserve amended /Prev chains.
Only retain the newest rootless table when its top-level /Prev offset points backward, and cover forward malformed chains with a regression test.
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
Ignore comment text when locating the standalone trailer keyword and add a regression test for a root-bearing trailer following a commented placeholder.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/lib.rs">
<violation number="1" location="src/lib.rs:4229">
P3: The new `find_standalone_keyword` reimplements the exact whitespace/`%`-comment skipping that `skip_pdf_whitespace_and_comments` (defined a few lines above, in the same file) already provides, with identical match arms (`is_ascii_whitespace`, then `%` → skip to `\n`/`\r`). Two separate copies of the PDF comment semantics now have to be kept in sync. The standalone keyword scan can reuse the existing helper: since whitespace/comments are skipped before inspecting the next byte, the preceding byte is always whitespace (or `pos == 0`), so the `before_ok` check becomes implied and the loop simplifies.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| while pos < end { | ||
| match buf[pos] { | ||
| byte if byte.is_ascii_whitespace() => pos += 1, | ||
| b'%' => { |
There was a problem hiding this comment.
P3: The new find_standalone_keyword reimplements the exact whitespace/%-comment skipping that skip_pdf_whitespace_and_comments (defined a few lines above, in the same file) already provides, with identical match arms (is_ascii_whitespace, then % → skip to \n/\r). Two separate copies of the PDF comment semantics now have to be kept in sync. The standalone keyword scan can reuse the existing helper: since whitespace/comments are skipped before inspecting the next byte, the preceding byte is always whitespace (or pos == 0), so the before_ok check becomes implied and the loop simplifies.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/lib.rs, line 4229:
<comment>The new `find_standalone_keyword` reimplements the exact whitespace/`%`-comment skipping that `skip_pdf_whitespace_and_comments` (defined a few lines above, in the same file) already provides, with identical match arms (`is_ascii_whitespace`, then `%` → skip to `\n`/`\r`). Two separate copies of the PDF comment semantics now have to be kept in sync. The standalone keyword scan can reuse the existing helper: since whitespace/comments are skipped before inspecting the next byte, the preceding byte is always whitespace (or `pos == 0`), so the `before_ok` check becomes implied and the loop simplifies.</comment>
<file context>
@@ -4222,16 +4222,29 @@ fn find_standalone_keyword(buf: &[u8], start: usize, end: usize, keyword: &[u8])
+ while pos < end {
+ match buf[pos] {
+ byte if byte.is_ascii_whitespace() => pos += 1,
+ b'%' => {
+ while pos < end && !matches!(buf[pos], b'\n' | b'\r') {
+ pos += 1;
</file context>
Problem
Issue #345's scanned PDF reports
page_count: 0and rejects conversion withOCR is required. The file is linearized and has more than 512 bytes of padding after its final%%EOF.The document has two classic xref tables:
/Rootand/Prev;/Rootin its own trailer.lopdf cannot find the final EOF through the padding, so container repair runs. The old repair selected the newest classic xref without inspecting its trailer. That loads the rootless final table, leaves the catalog unreachable, and produces a zero-page result even though the root-bearing table and its
/Prevchain are intact.Change
/Root, which also recovers the rest of the chain through/Prev.page_count == 0and now returns one text page containingHello World.The sample from the issue was also checked locally: it now reports four scanned pages (still requiring OCR) instead of zero pages.
Tests
cargo test --lib— 987 passed.cargo test --test integration_tests test_process_pdf_repairs_padded_linearized_xref_chain -- --nocapture— 1 passed.cargo clippy --lib -- -D warnings— clean.cargo fmt— clean.Fixes #345
Summary by cubic
Recover linearized PDFs with padded EOF by preferring a root-bearing classic xref and validating /Prev direction. Previously repair always chose the newest classic xref and could pick a rootless final table, producing page_count: 0; now it selects the newest xref whose trailer has /Root, or keeps the newest rootless table only when its /Prev points backward, preserving revisions and restoring a reachable catalog.
Fixes #345.
Written for commit bc1e19e. Summary will update on new commits.