Fix tokens split across content streams - #76
Conversation
# Conflicts: # ps.go
|
Thanks for this fix — the root-cause analysis and the synthetic regression test made absorbing it a pleasure. Since this PR has been waiting for a while and upstream looks inactive, I'm maintaining an actively developed continuation of this library at https://github.com/chappihappymeal/pdf (semver-tagged, CI, every fix with regression tests). Your commit was absorbed there with your authorship preserved (chappihappymeal/pdf@d26fd47) and shipped in v0.2.0. One follow-up you might find interesting: while reviewing it we found that concatenating the raw streams can glue adjacent tokens together when a stream doesn't end with whitespace — two numeric operands merge ( If you run into other issues in the library, I'd be glad to hear about them over there. |
|
Thanks for picking this up, and for preserving the authorship. Really nice to see the contribution actually make it into a release. Also, huge respect for taking the initiative to keep the project alive and actively maintained. Setting up CI, adding regression tests, doing proper releases, and generally running with it is a lot of work, and it’s great to see someone giving the library that kind of attention. Nice catch on the stream-boundary issue too. That’s a subtle one. Best of luck with the fork, and I’ll definitely keep it in mind if I run into anything else. |
…c#78) Follow-ups on top of gage-marshall's hardening commits, each verified to hang, crash, allocate without bound or misbehave before its fix, and each pinned by a test in hardening_test.go. Hangs and crashes: - /Prev chains in readXrefStream and readXrefTable were the one traversal left without a cycle guard; a repeated offset now ends the chain (readPrevSection, shared by both paths). - An outline entry whose /First and /Next both point back at itself multiplied the depth and sibling caps combinatorially. The tree now has one node budget, charged before any other check, and references already followed are not followed again. - The object-stream nesting cap restarted at zero whenever a stream's own header (/N, /First, /Length, /Extends) was read through Value.Key, so a reference there back into the stream overflowed the goroutine stack, which is fatal. The depth now travels on the Value. - A run of unclosed array or dictionary openers recursed through readObject until the stack was gone; nesting is bounded at 512. - A spec-valid odd-length hex string such as <F> panicked in readHexString (PDF 32000-1, 7.3.4.3). - A page whose /Contents is an empty array, a valid blank page, panicked in Interpret sizing the reader list at 2*0-1 (from the ledongthuc#76 absorption). Allocation and CPU bounds: - /Index [8388600 1] in a 223-byte file allocated over a gigabyte growing the cross-reference slice to the named object number. The table is now a dense slice that grows in proportion to the entries stored, with far off numbers kept in a map. - /W [0 0 0] read no bytes per entry, so /Size alone drove 8 million stores from an empty stream; a zero total width is rejected. - /Count far beyond the pages present had GetPlainText and GetStyledTexts walk the tree once per claimed page; extraction stops after 64 consecutive missing pages. Behaviour kept from master: - A ToUnicode block whose declared count exceeds the pairs present keeps the pairs (operandCount) instead of discarding the whole cmap; a block closed without being opened still discards it, as before. - A ToUnicode stream that cannot be read at all (unsupported filter, corrupt data) is reported as an error instead of decoding text with no cmap; only the cmap's own interpretation may fail quietly. - Inherited page attributes are found from the whole depth Page accepts (maxInheritDepth was 64 against a 1024-deep page tree, off by one too). - A negative /Length reads as no data instead of running to end of file. - A malformed pair in an object-stream index, including a negative offset, is skipped rather than ending the scan; object numbers are compared without truncation to uint32. Error reporting: - recoverMalformed replaces the per-method recover closures. Every error-returning method reports "malformed PDF: ..." exactly once, and Outline returns an empty outline on a malformed reference. The panic contract of the Value API is documented in the package comment. - Reader.resolve on a Reader that never loaded a table treats every reference as unresolvable instead of dereferencing nil. Verified against a corpus of 40530 real-world PDFs: output identical to v0.2.0 for every file.
3 participants
Problem
Text extraction can loop indefinitely when a page uses an array of content streams and a PDF object starts in one stream and finishes in the next.
This was found while processing an otherwise readable PDF whose page
/Contentsentry contains two streams. Other PDF implementations completed the document, butPage.GetPlainTextin this package consumed CPU without returning.Synthetic test case
The added regression test constructs a minimal, one-page, six-object PDF entirely in memory. The page declares:
The first content stream starts a
TJarray:The second stream completes that array and invokes the operator:
The expected extracted text is
Hello world. No real-world or user-provided PDF is included in the test.Evidence of failure
On
masterat5959a4027728, with only the regression test added, this command does not complete normally:It terminates at the test timeout with the active stack in:
This reproduces the non-progress condition without the original document.
Root cause analysis
Interpretrecognizes that a PDF stream value may be an array, but it currently creates a newbuffer(lexer) for each array element. The first buffer reaches end-of-stream whilereadArrayis still parsing the incompleteTJarray. Because the remainder of the array is only available through the next buffer, the current parser cannot make progress.A page content-stream array is one logical content sequence: its streams are interpreted in order as though concatenated. Lexical state therefore needs to be preserved across each stream boundary.
Proposed fix
For an array value, collect the ordered stream readers into one
io.MultiReader, then create onebufferover that combined reader. A single stream continues to use its existing reader. The stack and interpreter behavior are otherwise unchanged.This lets
readArraycontinue into the second stream, after which the regression test completes and extractsHello world.Validation
go test ./...passes with this change.go test -v ./..., andgo build -v ./...on Go 1.24.