diff --git a/src/lib.rs b/src/lib.rs index 769cda4..0c9f504 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3553,6 +3553,7 @@ fn repair_pdf_container_candidates(buf: &[u8]) -> Vec> { let mut candidates = Vec::new(); add_repair_candidate(&mut candidates, append_missing_eof_marker(buf), buf); + add_repair_candidate(&mut candidates, recover_startxref_pointer(buf), buf); let stripped = strip_leading_pdf_container_bytes(buf); if let Some(stripped_buf) = stripped.as_deref() { @@ -3562,11 +3563,112 @@ fn repair_pdf_container_candidates(buf: &[u8]) -> Vec> { append_missing_eof_marker(stripped_buf), buf, ); + add_repair_candidate( + &mut candidates, + recover_startxref_pointer(stripped_buf), + buf, + ); } candidates } +/// Some PDF writers emit a `startxref` pointer that doesn't actually point +/// at the cross-reference table — a single corrupted byte in the offset is +/// enough. lopdf trusts that pointer outright and fails to load rather than +/// searching for the real table, unlike pypdf/pdfium which both recover by +/// locating it directly. This finds the real (classic, non-stream) `xref` +/// table by scanning for the keyword — validating that a plausible +/// subsection header follows, not just any standalone "xref" token, since +/// this crate processes untrusted input and a coincidental match inside +/// unrelated stream/string content must not get "repaired" against a bogus +/// offset (lopdf would then load successfully against garbage instead of +/// returning a clean error) — and appends a corrected trailing +/// `startxref`/`%%EOF` block. lopdf's own `get_xref_start` always uses the +/// *last* `%%EOF` in the final 512 bytes of the buffer, so ours +/// transparently supersedes the broken one without needing to touch +/// anything already in the file. +/// +/// Doesn't cover cross-reference *streams* (`N 0 obj << /Type /XRef ...`, +/// used by some PDF 1.5+ writers instead of a classic table) — recovering +/// those needs the containing object's number, not just a byte offset. +fn recover_startxref_pointer(buf: &[u8]) -> Option> { + let xref_pos = find_last_valid_xref_table_start(buf)?; + + let mut repaired = Vec::with_capacity(buf.len() + 32); + repaired.extend_from_slice(buf); + if !repaired.ends_with(b"\n") { + repaired.push(b'\n'); + } + repaired.extend_from_slice(format!("startxref\n{xref_pos}\n%%EOF\n").as_bytes()); + Some(repaired) +} + +/// Finds the last standalone `xref` token in `buf` that is immediately +/// followed by a plausible classic cross-reference subsection header +/// (` `, e.g. "0 6") — the shape every real classic xref +/// table starts with. A single reverse byte scan: O(n) even on a +/// pathological buffer with many non-matching or non-standalone "xref" +/// occurrences, unlike repeatedly re-searching a shrinking prefix. +fn find_last_valid_xref_table_start(buf: &[u8]) -> Option { + const KEYWORD: &[u8] = b"xref"; + if buf.len() < KEYWORD.len() { + return None; + } + let mut pos = buf.len() - KEYWORD.len(); + loop { + if &buf[pos..pos + KEYWORD.len()] == KEYWORD { + let before_ok = pos == 0 || buf[pos - 1].is_ascii_whitespace(); + let after_ok = buf + .get(pos + KEYWORD.len()) + .is_none_or(|c| c.is_ascii_whitespace()); + if before_ok && after_ok && looks_like_xref_subsection_header(buf, pos + KEYWORD.len()) + { + return Some(pos); + } + } + if pos == 0 { + return None; + } + pos -= 1; + } +} + +/// Checks that `buf[pos..]` starts (after whitespace) with two +/// whitespace-separated runs of ASCII digits — ` `, the +/// first subsection header of a classic PDF cross-reference table. +fn looks_like_xref_subsection_header(buf: &[u8], pos: usize) -> bool { + fn skip_ws(buf: &[u8], mut pos: usize) -> usize { + while buf.get(pos).is_some_and(u8::is_ascii_whitespace) { + pos += 1; + } + pos + } + fn skip_digits(buf: &[u8], mut pos: usize) -> usize { + while buf.get(pos).is_some_and(u8::is_ascii_digit) { + pos += 1; + } + pos + } + + let pos = skip_ws(buf, pos); + let after_first_digits = skip_digits(buf, pos); + if after_first_digits == pos { + return false; // no start-id + } + let sep = skip_ws(buf, after_first_digits); + if sep == after_first_digits { + return false; // start-id and count must be whitespace-separated + } + let after_count = skip_digits(buf, sep); + if after_count == sep { + return false; // no count + } + // The count run must end at whitespace/buffer-end, not run into trailing + // garbage (e.g. a coincidental "xref\n0 6garbage" in stream content). + buf.get(after_count).is_none_or(u8::is_ascii_whitespace) +} + fn add_repair_candidate( candidates: &mut Vec>, candidate: Option>, @@ -7099,4 +7201,65 @@ mod tests { // Pre-filled cell was not touched. assert_eq!(cells[1].text, "Pre-filled"); } + + // -- recover_startxref_pointer / find_last_valid_xref_table_start ------ + // + // Direct unit tests on the byte-level scan, addressing review feedback + // on #230: a coincidental standalone "xref" token that isn't actually + // followed by a subsection header (start-id + count) must not be + // treated as a real table — accepting it would let lopdf "succeed" + // against a bogus offset and silently return garbled/empty content + // instead of a clean error. + + #[test] + fn find_xref_rejects_standalone_token_without_subsection_header() { + // "xref" appears as a real standalone word, but nothing that looks + // like " " follows it. + let buf = b"Please refer to the xref appendix for details."; + assert_eq!(find_last_valid_xref_table_start(buf), None); + } + + #[test] + fn find_xref_accepts_real_classic_table_header() { + let buf = b"garbage\nxref\n0 6\n0000000000 65535 f \n%%EOF"; + let pos = find_last_valid_xref_table_start(buf).expect("should find the real table"); + assert_eq!(&buf[pos..pos + 4], b"xref"); + assert_eq!(&buf[pos..], b"xref\n0 6\n0000000000 65535 f \n%%EOF"); + } + + #[test] + fn find_xref_skips_coincidental_match_and_finds_real_table_before_it() { + // A coincidental "xref" (no subsection header) appears *after* the + // real table in the buffer — the scan must not stop at the first + // (rightmost) standalone token it finds; it must keep looking + // backward until one actually validates. + let buf = b"xref\n0 3\n0000000000 65535 f \ntrailer\nsee the xref\n"; + let pos = find_last_valid_xref_table_start(buf).expect("should find the real table"); + assert_eq!(pos, 0); + } + + #[test] + fn find_xref_rejects_substring_of_startxref() { + // "xref" is a substring of "startxref" but isn't a standalone + // token there (not preceded by whitespace) — must not match, even + // though a number immediately follows it. + let buf = b"startxref\n1234\n%%EOF"; + assert_eq!(find_last_valid_xref_table_start(buf), None); + } + + #[test] + fn find_xref_rejects_count_run_with_trailing_garbage() { + // "xref\n0 6garbage" has the right shape (digits, whitespace, + // digits) but the count run doesn't end at whitespace/EOF — it + // runs straight into non-digit garbage, so this must not be + // accepted as a real subsection header. + let buf = b"xref\n0 6garbage\n%%EOF"; + assert_eq!(find_last_valid_xref_table_start(buf), None); + } + + #[test] + fn recover_startxref_pointer_returns_none_without_a_valid_table() { + let buf = b"Please refer to the xref appendix for details."; + assert!(recover_startxref_pointer(buf).is_none()); + } } diff --git a/tests/fixtures/broken_startxref_pointer.pdf b/tests/fixtures/broken_startxref_pointer.pdf new file mode 100644 index 0000000..7522df5 --- /dev/null +++ b/tests/fixtures/broken_startxref_pointer.pdf @@ -0,0 +1,68 @@ +%PDF-1.3 +%“Œ‹ž ReportLab Generated PDF document (opensource) +1 0 obj +<< +/F1 2 0 R +>> +endobj +2 0 obj +<< +/BaseFont /Helvetica /Encoding /WinAnsiEncoding /Name /F1 /Subtype /Type1 /Type /Font +>> +endobj +3 0 obj +<< +/Contents 7 0 R /MediaBox [ 0 0 612 792 ] /Parent 6 0 R /Resources << +/Font 1 0 R /ProcSet [ /PDF /Text /ImageB /ImageC /ImageI ] +>> /Rotate 0 /Trans << + +>> + /Type /Page +>> +endobj +4 0 obj +<< +/PageMode /UseNone /Pages 6 0 R /Type /Catalog +>> +endobj +5 0 obj +<< +/Author (anonymous) /CreationDate (D:20260803112923+00'00') /Creator (anonymous) /Keywords () /ModDate (D:20260803112923+00'00') /Producer (ReportLab PDF Library - \(opensource\)) + /Subject (unspecified) /Title (untitled) /Trapped /False +>> +endobj +6 0 obj +<< +/Count 1 /Kids [ 3 0 R ] /Type /Pages +>> +endobj +7 0 obj +<< +/Filter [ /ASCII85Decode /FlateDecode ] /Length 202 +>> +stream +GarW05mr9@&;9NOME,dW.,B;'jAjYq0S4Z`*D9aMA;]$5J)A/$3lESen1?F)ZJsa4$4&N%-%cs)#qW5EVhhbPiRDrAV>MC%.spto@CU"ZdipR'TtFiMR_%m*Hm$N%qL7a"ckkp9T/s[N2"Og377mP*M^akb2XQZ@'l*qT(9bVtDb5+)S&Q.#%E)<]Ao`TSk2AE'/E\fn~>endstream +endobj +xref +0 8 +0000000000 65535 f +0000000061 00000 n +0000000092 00000 n +0000000199 00000 n +0000000392 00000 n +0000000460 00000 n +0000000721 00000 n +0000000780 00000 n +trailer +<< +/ID +[<6d7ea1213c5974c78613d5d2a08423b5><6d7ea1213c5974c78613d5d2a08423b5>] +% ReportLab generated PDF document -- digest (opensource) + +/Info 5 0 R +/Root 4 0 R +/Size 8 +>> +startxref +9072 +%%EOF diff --git a/tests/integration_tests.rs b/tests/integration_tests.rs index 3cfc06e..0506291 100644 --- a/tests/integration_tests.rs +++ b/tests/integration_tests.rs @@ -3996,6 +3996,29 @@ fn pdf_options_debug_redacts_password() { assert!(dbg.contains("REDACTED"), "expected redaction marker: {dbg}"); } +/// Regression for #228: a `startxref` pointer corrupted to point at the +/// wrong byte offset (a single flipped digit — a real, common writer bug) +/// must not make the whole file unprocessable. The real classic xref table +/// is still present and findable by scanning for the `xref` keyword; both +/// pypdf and pdfium recover the same way. Before this fix, every entry +/// point raised "Invalid PDF structure" on a file whose object data was +/// otherwise completely intact. +#[test] +fn test_process_pdf_recovers_corrupted_startxref_pointer() { + let result = process_pdf_with_options( + "tests/fixtures/broken_startxref_pointer.pdf", + PdfOptions::new(), + ) + .expect("a corrupted startxref pointer should be recoverable, like pypdf/pdfium"); + + assert_eq!(result.page_count, 1); + let md = result.markdown.unwrap_or_default(); + assert!( + md.contains("Order Detail Report by Account") && md.contains("WIDGET ASSEMBLY"), + "recovered document should extract its real text, got: {md:?}" + ); +} + /// Regression for #227: `extract_pages_markdown`'s per-page `needs_ocr` /// must agree with `classify_pdf`/`detect_pdf_type` on the same page. The /// fixture is a full-page raster "scan" with a single line of genuine