Repository navigation
linearize: the shared object hint table is required, and nothing looked inside /H - #46
Merged
Merged
Conversation
…d so
⛔⛔ EVERY TEST THAT AGREES WITH qpdf HAS BEEN SKIPPING ON EVERY LANE. Measured
on run 43595391186: the string "qpdf" appears ZERO times in the whole log.
⛔ AND THE TIME IS THE WITNESS, which is the only thing that could have been
noticed: this job reports
ok github.com/go-pdfkit/reader 0.197s (CI)
ok github.com/go-pdfkit/reader 0.758s (a workstation with qpdf)
The sweep generates 42 files by invoking qpdf; it cannot do that in 0.197s. So
the agreement across 1, 2, 3, 5, 25 and 100 pages in five shapes, AND the
negative control that an ordinary file is REFUSED by the foreign judge — the
one #44 added yesterday — have never been checked by anything.
⛔ `go test` WITHOUT -v PRINTS NOTHING ABOUT A SKIP, so the log was not
hiding this badly, it could not report it at all. And coverage stayed at
100.0% throughout: the same statements are reached by the tests that need no
qpdf, so the coverage gate cannot see this and never could. Two gates were
green about a third thing neither measures.
⛔ THIS REPOSITORY ALREADY HOLDS THE OPPOSITE STANDARD, one job down, about a
missing qemu emulator: "A missing emulator FAILS this lane. A lane that
reports success for an architecture it never ran is the one outcome this
matrix exists to rule out." A missing foreign judge is that outcome for a
different reason. go-pdfkit/ops already does it right for pdftoppm.
⛔ REQUIRE_QPDF, NOT CI, because not every lane CAN run it and a rule that
demands the impossible gets deleted. Under qemu-user the test binary is
emulated and an execve of the host's x86-64 qpdf is not something the
emulator can carry out; the Windows lane has no apt. So ONE lane installs
qpdf and sets REQUIRE_QPDF, and there a missing judge FAILS. The others skip,
which is now a stated design instead of an accident nobody could see.
Measured, both directions, rather than predicted:
REQUIRE_QPDF=1, qpdf off PATH -> FAIL, naming the reason
unset, qpdf off PATH -> skip, exit 0
⛔ aptneed FOR THE INSTALL, because a mirror outage must not arrive as a
missing judge: go-pdfkit/ops#90 was four red runs in half an hour on a third
party's DNS. `--need qpdf` asks for the BINARY, since apt saying yes is not
qpdf being on PATH. And `qpdf --version` is printed, so which judge agreed is
answerable afterwards rather than assumed.
⚠ NOT FIXED, and deliberately: actionlint reports SC2086 on `go test $ex
./...` in the qemu job. It is pre-existing — main carries it at line 141 — and
shellcheck is WRONG here. `ex` is empty on the 386 lane, which runs natively
on purpose, so quoting it would hand `go test` an empty package pattern and
break the one lane that must not be emulated. Reader's CI never ran
actionlint, which is why it went unnoticed; silencing it properly is its own
change.
gofmt clean, go vet clean, 0 uncovered blocks of 1806.
Refs #40.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
⛔⛔ SECOND OCCURRENCE IN TWO DAYS, AND I PREDICTED THIS ONE IN WRITING. The
first push of the qpdf lane failed on
go: …/cmd/aptneed@latest: fleettools@v0.112.0 requires go >= 1.27.4
(running go 1.27.3; GOTOOLCHAIN=local)
go-pdfkit/ops hit the identical line on 2026-11-13, and after fixing it I
wrote on that pull request: "Worth knowing for other repositories in the fleet
that pin a toolchain and install fleettools commands in CI: the same pin
blocks them the moment they try." Then I walked into it myself, one repository
later, the next day. A note is applied when somebody remembers to apply it,
and at the moment of adding an install step one is thinking about the install.
So it is recorded as a recurrence rather than a third note. The gate that
catches it is a check over `.github/workflows` for a `<module>/cmd/<x>@latest`
install beside a `go-version` pin below that module's `go` directive —
already filed as pending on go-fleettools/fleettools#103, where the `@latest`
half of the same problem lives. `tagscan` cannot see either: it reads
repositories, not workflows.
All three jobs move to 1.27.4, not only the one that installs aptneed. The
other two do not need it, but a matrix measuring on two different toolchains
is a difference nobody chose, and this fleet has already been bitten by
`stable` quietly selecting a minor.
⛔ GOTOOLCHAIN IS STILL LEFT ALONE. That is what made this a clean failure
naming both versions instead of a runner silently fetching 1.27.4 and
measuring a chain nobody picked.
go.mod stays at `go 1.27.1`: that is the language version this module needs,
not the toolchain a runner must have, and nothing here requires it to move.
actionlint reports only the pre-existing SC2086 discussed in the previous
commit.
Refs #40, #44.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ed inside /H First result of reading Annex F itself, which #40 listed under "what has NOT been read yet". ⛔⛔ /S IS REQUIRED AND A WRITER THAT FORGOT IT PASSED EVERYTHING. Table F.2 lists the shared object hint table as "(Required)", and F.3.6 says the primary hint stream's dictionary "shall contain entries giving the position of the beginning of each hint table in the stream". Every check this file had about /H was about the stream's EXTENT — it starts at a real indirect object, that object is a stream, its length matches what the parse measured. Its dictionary was never read. So a primary hint stream naming no hint table at all was sound by this checker, and a reader that fetched the front of the file would have nothing to look up. This is the first thing checked INSIDE a hint stream rather than about it. Everything else is an offset or a length; /S is content. ⛔ ONLY THE PRIMARY, AND THE SPEC IS EXPLICIT ABOUT WHY. F.3.6: "the dictionary of the overflow hint stream shall not contain these entries". The same requirement in checkOverflowHint would refuse exactly the conforming file. The two halves of /H are not symmetric and this is where they differ — which is also why no test asserts the absence of the check there: with the fixture's overflow pair pointing at the primary stream, adding it would break nothing, so it would be a claim no mutation could kill. ⛔ PRESENCE, NOT POSITION. The page offset table "shall be the first table in the stream and shall start at offset 0" and has no key at all, so the positions inside the stream are F.4's question and this does not open it. Measured on both judges: - all three committed fixtures carry /S — the two qpdf shapes AND the Ghostscript one, so the positive control spans two producers; - a same-width edit of `/S 43` to `/X 43` is refused here by name; - `qpdf --check` refuses that same file too, with "ERROR: Integer operation attempted on invalid object" — it reads the missing /S as an integer. So this is NOT stricter than the reference, and the message here is the more useful of the two. ⛔ AND ONE PROPERTY OF THE FOREIGN JUDGE WORTH RECORDING: qpdf --check prints "File is linearized" from the parameter dictionary BEFORE validating the hint tables, and then errors. That line is not its approval. Anything reading qpdf as a verdict rather than a report has to read past it. ⛔ A COMMENT OF MINE WAS WRONG AND THE SPEC CORRECTED IT. checkFirstPageEnd said "Annex F puts the first-page section at part 3 and the main one at part 10". The main cross-reference table is part 11; part 10 is the OVERFLOW HINT STREAM. The code was right — it compares against /T — and the sentence explaining it named the wrong part. Mutation, judged by cmd/failset on the failing NAMES: dropping the /S check is red on TestAPrimaryHintStreamWithNoSharedObjectTableIsRefused and no other test. gofmt clean, go vet clean, 0 uncovered blocks of 1808. Refs #40. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
⛔ PRESENCE IS WEAKER THAN WHAT THE SPEC SAYS, and I shipped the weaker thing in the previous commit. F.3.6: "the values of all entries in the hint streams' dictionaries shall be direct objects and may contain no indirect object references", the stream Length excepted. /S is a position counted in bytes from the start of the stream data. A name, or an indirect reference, is unusable by precisely the reader this format exists for — one that has fetched the front of the file and can resolve nothing. `/S /x` passed the presence check and names no position at all. Same-width edit again, `/S 43` -> `/S /x`, so only the value changes. gofmt clean, go vet clean, 0 uncovered blocks of 1810. Refs #40. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ition Review found the gap by mutating what I had not mutated, which is exactly why it was asked. ⛔⛔ THE TEST COULD ONLY EXPRESS ONE WRONG KIND. Narrowing the check from "refuse anything that is not an Integer" to "refuse only a Name" left the suite GREEN: `/S /x` is the single case a same-width edit of `/S 43` can write, and it is a Name, so the narrowed check still refused it. The code was right and the evidence for it covered one seventh of what it claims. ⛔ SAME-WIDTH CANNOT REACH THE REST, and that is why they were missing rather than forgotten. `/S 4.3` is six characters against five; an indirect reference or an array is longer still. And the hint stream sits AFTER the fixture's padding run, so growing it there moves the object /H points at and the file is refused for its hint length instead — the wrong reason, which reads as a pass for this test. ⭐ SO THE FUNCTION IS ASKED DIRECTLY. checkHintStream takes bytes and parameters; these cases need nothing else. /H's extent is computed FROM THE PARSE so that nothing earlier can be what refuses the file, and each case differs from the accepted one in exactly one thing: the kind of /S. Seven kinds — name, indirect reference, array, real, string, boolean, null — and a CONTROL asserting an integer /S is accepted, without which the seven would hold against a version that refused this hand-built stream outright. Re-measured: narrowing to Name is now red on six of the seven and the parent, and no other test. The one it does not redden is the Name case, which that mutation still refuses — as it should be. ⛔ AND -1 WAS A POSITION. Review measured `/S -1` accepted, qpdf answering "overflow reading bit stream" — a symptom of seeking outside the data, not a statement about /S. F.3.6: the positions "shall be counted in bytes relative to the beginning of the stream data", so negative names nothing. Same-width edit, so only the value changes. ⚠ THE UPPER BOUND STAYS OPEN, DELIBERATELY. /S is an offset into the DECODED stream data, so bounding it means decoding the hint stream — the first step of reading the tables themselves (F.4), which belongs with that work and not smuggled in here. qpdf reports "/S (shared object) offset is out of bounds" for a /S past the data and this does not. Stated in the README and recorded on #40 rather than left for somebody to discover. ⭐ And the review settled the question this check stands or falls on. /S is required UNCONDITIONALLY, from F.4.2: "All objects associated with the first page (part 6) shall have entries in the shared object hint table, regardless of whether they are actually shared." The first page always has at least its page object, so the table is never empty and no conforming writer may omit it. Requiring a Real is also wrong per F.3.6's "counted in bytes". gofmt clean, go vet clean, 0 uncovered blocks of 1812. Refs #40. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
First result of reading Annex F itself, which
#40listed under "what has NOT been read yet".⛔
/Sis required, and a writer that forgot it passed everythingTable F.2 lists the shared object hint table as (Required), and §F.3.6 says the primary hint stream's dictionary
Every check this package had about
/Hwas about the stream's extent — it starts at a real indirect object, that object is a stream, its length matches what the parse measured. Its dictionary was never read. So a primary hint stream naming no hint table at all was sound here, and a reader that fetched the front of the file would have had nothing to look up.This is the first thing checked inside a hint stream rather than about it. Everything else is an offset or a length;
/Sis content.Measured on both judges
/S— the two qpdf shapes and the Ghostscript one, so the positive control spans two producers/S 43→/X 43qpdf --checkon that same fileERROR: Integer operation attempted on invalid object, exit 2So this is not stricter than the reference — and the message here is the more useful of the two, since qpdf's is the symptom of reading a missing
/Sas an integer rather than a statement about what is missing.⛔ And one property of the foreign judge worth recording:
qpdf --checkprintsFile is linearizedfrom the parameter dictionary before it validates the hint tables, then reports errors after it. That line is not its approval. Anything reading qpdf as a verdict rather than a report has to read past it.⚠ Only the primary stream, and the spec says why
§F.3.6: "the dictionary of the overflow hint stream shall not contain these entries." The same requirement in
checkOverflowHintwould refuse exactly the conforming file.That is also why no test asserts the absence of the check there: with the fixture's overflow pair pointing at the primary stream, adding it would break nothing, so it would be a claim no mutation could kill. I would rather say that than ship an assertion that cannot fail.
Presence, not position: the page offset table "shall be the first table in the stream and shall start at offset 0" and has no key at all, so positions inside the stream are §F.4's question and this does not open it.
⛔ A comment of mine was wrong and the spec corrected it
checkFirstPageEndsaid "Annex F puts the first-page section at part 3 and the main one at part 10". The main cross-reference table is part 11; part 10 is the overflow hint stream. The code was right — it compares against/T— and the sentence explaining it named the wrong part.Mutation
Judged by
cmd/failseton the failing test names: dropping the/Scheck is red onTestAPrimaryHintStreamWithNoSharedObjectTableIsRefusedand no other test.gofmt clean,
go vetclean, 0 uncovered blocks of 1808.Refs #40.
🤖 Generated with Claude Code