Repository navigation
Run the tests where poppler does not go, and on 32-bit - #10
Merged
Merged
Conversation
Cross-compiling proves the code is well-formed for a target. It says nothing about whether it computes the right answer there, and this package reassembles multi-byte numbers out of a byte stream and multiplies widths by heights -- the two shapes in which an endianness assumption and an integer overflow hide. Six lanes under qemu-user-static: riscv64, loong64, ppc64le, s390x (big-endian) and 386/arm (32-bit int). The same lanes on go-pdfkit/render found a real defect within the hour: a ceiling written as `cw*ch > max` that a 32-bit int wrapped past, letting a 1 KB file ask the decoder for four gigabytes.
The lanes added in this branch did not even build:
./tounicodebound_test.go:137: cannot use 0xffff0000 (untyped int constant
4294901760) as int value in map index (overflows)
A ToUnicode code is assembled into an int a byte at a time, so a four-byte code
lands on a different number depending on how wide an int is: 4 294 901 760 where
int is 64 bits, minus 65 536 where it is 32. Reading and writing both go through
that same conversion, so the package is self-consistent on either width -- but a
constant written out in the test is not, and on 32-bit it is not even a legal
int.
The test now assembles the key the way the package does.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
One job, six lanes —
riscv64,loong64,ppc64le,s390x,386,arm—each running the full test suite under
qemu-user-static.fail-fast: false,so one architecture failing does not hide the next one's result.
Why a cross-compile was not enough
A build proves the code is well-formed for a target. It proves nothing about
whether it computes the right answer there, and this is a byte-stream reader:
every multi-byte number it reassembles is a place an endianness assumption can
sit undisturbed for as long as nobody looks, and every width × height is a
product that fits a 64-bit
intand may not fit a 32-bit one.That is not hypothetical. The same two lanes added to
go-pdfkit/renderfound aceiling that a 32-bit
intwalked straight through —65535 × 65535wraps to−131 071, so a 1 KB file made the decoder ask for four gigabytes — and, from
the same thread, a
MediaBoxof1e300that panicked the renderer on 64-bittoo.
Where this leaves the references
poppler and pdfium test on none of the four 64-bit architectures here. 32-bit
ARM they do hold, and we did not.
🤖 Generated with Claude Code