Repository navigation
linearize: /E is the end of the first page, and the file already says where that is - #43
Merged
Merged
Conversation
… where that is ⛔ THE PARTITION WAS NEVER THE HINT TABLES' TO OWN. This checker bounded /E between 1 and the file's length and said computing it "needs the first-page object set, which is the same partition the hint tables describe". That is true of the hint tables and false of the FIRST-PAGE CROSS-REFERENCE SECTION: Annex F puts it at the front and the file's own `startxref` points at it, which is the whole trick of the format — a reader fetches the front of the file and has page one. Its entries ARE the first page's objects. Nothing had to be decoded; the answer was one section away. So /E must equal the end of the last object that section lists, and a writer one byte out is told so. That matters because /E is exactly the parameter a writer gets wrong, and #40 exists so the writer is not its own judge. ⛔⛔ THE HINT STREAM IS EXCLUDED, AND A SECOND PRODUCER IS WHY THAT IS NOT A GUESS. F.3.1 permits the primary hint stream on either side of the first page: qpdf /H 550, /E 902 — before the first page Ghostscript /H 829, /E 829 — after it, AT /E exactly On Ghostscript's file the naive maximum IS the hint stream, ending at 967 against an /E of 829, so a rule that took it would refuse every file that tool writes. ⛔ AND BOTH COMMITTED qpdf FIXTURES PLACE IT BEFORE, so neither can tell the two rules apart — the exclusion would have been a branch no test could distinguish, which is the vacuity rounds 2 and 3 spent eight tests removing. testdata/linearized-gs.pdf is committed for exactly that reason and nothing else; the gate is that removing the exclusion turns TestTheHintStreamIsNotTheEndOfTheFirstPage red and leaves the qpdf ones green. mutool was the obvious third producer and answers "Linearisation is no longer supported", so the reference set is qpdf and Ghostscript. ⛔ THE OVERFLOW HALF CANNOT BE REACHED BY ANY FILE — it needs a four-entry /H whose second pair is both a real stream and the highest-offset object, and neither producer writes one. Inline it would be uncoverable and unkillable, so it is asked of isHintObject(), where four plain cases put it. ⛔ The offsets stay int64 until they are known to fit, for the reason `addressable` exists: int(e.offset) is the same silent truncation, and these come from the file. ⛔ The old length bound is REMOVED rather than kept in front: the equality subsumes it, and a bound a later check subsumes is a bound no test is holding — the round-3 finding, applied rather than re-learned. Two existing cases now assert the new sentence, which says what contradicts the claim and not merely that something was wrong. Mutations, judged by cmd/failset on the failing test NAMES: isHintObject -> false -> TestTheHintStreamIsNotTheEndOfTheFirstPage + 2 isHintObject cases drop HasOverflowHint guard -> …/an_overflow_offset_that_is_not_declared and no other drop eolAfter in the end -> 24 tests, including all 12 of the generated qpdf corpus That last one is the reassuring one: the rule holds across the existing generated corpus — 1, 2, 5 and 25 pages in three shapes — not only the two committed files. gofmt clean, go vet clean, GOARCH=386 go vet clean, 0 uncovered blocks of 1804. Refs #40. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fourth review, three findings, all three real. One was a claim of mine that was simply false, one was a bound nothing held, and one is a limit that is now written down instead of implied. ⛔⛔ 1. I ARGUED THE INCREMENTAL-UPDATE CASE WAS IMPOSSIBLE AND IT IS NOT. The reasoning was that startxref must still name the first-page section, because a file written to after linearizing would have /L != len(b) and be refused earlier. Review rewrote /L to the new length — which costs nothing — and the file sailed past every earlier check. This then followed startxref into the UPDATE's section and compared /E with an object the update had appended, refusing with "/E says 902 and the first page ends at 1399". That is the worse kind of wrong: refused, so a suite stays green, for a reason that is not the reason. A reader believes it. Annex F puts the first-page section at part 3 and the main table at part 10, so the front section precedes /T — and checkMainXref has already established /T against the file by the time this runs. The guard says nothing about /E, so it still holds when /E is the thing under test, which ruled out the first shape I tried (comparing against /E itself, which breaks every /E case). ⛔⛔ 2. `last >= len(b)` WAS A BOUND NOTHING HELD — measured by review as a mutation to `>` that stayed GREEN. Past the end the parse fails anyway, so the only input that separates the two is an offset of EXACTLY len(b), where b[len(b):] is a legal empty slice rather than a panic. That is the test now. This is the round-3 finding about subsumed bounds, which I had written into this very commit's predecessor as applied — and then left a fresh instance of. ⛔⛔ 3. WHAT THIS PROVES IS THAT /E AGREES WITH THE FRONT SECTION, NOT THAT THE SECTION IS RIGHT, and the README said something stronger. A writer that omits objects page one genuinely needs produces an internally consistent file that passes. Review measured Ghostscript doing exactly that on a document with shared fonts, outlines and links: /H = /E = 1941, accepted here, while qpdf computes 5500..5501 and calls page one 9 objects against the hint table's 4. Deciding what page one NEEDS means walking its resources — the partition the hint tables describe — so that judgement is still outstanding and a pass here is not evidence about it. Stated in the README and the package doc. ⚠ 4. AND THIS IS STRICTER THAN qpdf, deliberately, which is now said rather than discovered. qpdf computes /E as a RANGE (901..902 on the fixture), accepting the end with or without the line ending after endobj; this requires it. A file carrying the lower value passes qpdf --check and is refused here. None has been observed — both measured producers write the higher value — and the alternative is a window that cannot tell a correct /E from one a byte short, which is the /H-length argument from round 3. Review also confirmed the sweep: 0 disagreements with qpdf across 42 files — 1, 2, 3, 5, 25 and 100 pages x plain, objstm, uncompressed, encrypt-256 and --newline-before-endstream — plus a richer 8-page document. Mutations for the two new guards, judged by cmd/failset on the failing NAMES, each red on its own test and no other: `>=` -> `>` on the length guard -> TestAnEntryStartingExactlyAtTheEndOfThe… the /T guard disabled -> TestAnIncrementallyUpdatedFileIsNotJudged… gofmt clean, go vet clean, GOARCH=386 go vet clean, 0 uncovered blocks of 1806. 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.
/Ewas the one Table F.1 parameter the checker did not gate, and it is the one a writer is most likely to get wrong — so leaving it was precisely the "writer is its own judge" hole this work exists to close.The partition was never the hint tables' to own
The checker bounded
/Ebetween 1 and the file's length, and said computing it properly "needs the first-page object set, which is the same partition the hint tables describe".That is true of the hint tables and false of the first-page cross-reference section. Annex F puts that section at the front, and the file's own
startxrefpoints at it — which is the whole trick of the format: a reader fetches the front of the file and has page one. Its entries are the first page's objects. Nothing had to be decoded; the answer was one section away.So
/Emust equal the end of the last object that section lists.⛔ The hint stream is excluded, and a second producer is why that is not a guess
F.3.1 permits the primary hint stream on either side of the first page:
/Hoffset/E-dFastWebView/EexactlyOn Ghostscript's file the naive maximum is the hint stream, ending at 967 against an
/Eof 829. A rule that took it would refuse every file that tool writes.⛔⛔ And both committed qpdf fixtures place it before, so neither can tell the two rules apart. The exclusion would have been a branch no test could distinguish — the exact vacuity rounds 2 and 3 of the previous change spent eight tests removing.
testdata/linearized-gs.pdfis committed for that reason and no other, and the gate is that removing the exclusion turnsTestTheHintStreamIsNotTheEndOfTheFirstPagered while the qpdf tests stay green.mutool was the obvious third producer; it answers
Linearisation is no longer supported, so the reference set is qpdf and Ghostscript.⚠ The overflow hint stream is excluded by the same reasoning and no file can reach it — it needs a four-entry
/Hwhose second pair is both a real stream and the highest-offset object, and neither producer writes one. Inline it would be uncoverable and unkillable, so it is asked ofisHintObject(), where four plain cases put it and a mutation kills it.Two findings from the last review, applied rather than re-learned
int64until known to fit.int(e.offset)is the same silent truncationaddressableexists to refuse, and these come from the file.Mutations, judged by
cmd/failseton the failing test NAMESThat last one is the reassuring one: the rule holds across the existing generated corpus — 1, 2, 5 and 25 pages in three shapes — not only the two committed files.
gofmt clean,
go vetclean,GOARCH=386 go vetclean, 0 uncovered blocks of 1804.Refs #40.
🤖 Generated with Claude Code