From 253535f556a62052fcbb2aeecc624761105fb406 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 13 Nov 2026 20:37:30 +0100 Subject: [PATCH 1/2] linearize: /E is the end of the first page, and the file already says where that is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⛔ 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 --- README.md | 41 ++++++++--- linearized.go | 112 +++++++++++++++++++++++++++-- linearized_test.go | 142 ++++++++++++++++++++++++++++++++++++- testdata/linearized-gs.pdf | Bin 0 -> 4107 bytes 4 files changed, 277 insertions(+), 18 deletions(-) create mode 100644 testdata/linearized-gs.pdf diff --git a/README.md b/README.md index ca58d9d..859d1cb 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,7 @@ judgement a writer needs on bytes it has just produced. It establishes that every parameter **points at what it claims to** — `/N` is the real page count, `/O` is the real first page *and* is a `/Page`, `/H` begins at a real stream object, `/T` reaches the main cross-reference (either shape: Table F.1 gives -it two meanings and both are real), `/E` is inside the file. +it two meanings and both are real), `/E` is where the first page really ends. What gates it are files `Linearized` **accepts** and `CheckLinearized` refuses: the committed qpdf fixture with one integer @@ -160,13 +160,38 @@ different document. The replacement's width is asserted by the helper, because an edit one byte longer would be refused for the length instead and the deep check would never run. -⛔⛔ **`/E` is NOT gated, and a writer must not read a pass here as evidence -about it.** Table F.1 calls `/E` the offset of the end of the first page, and -qpdf *computes* it — on the committed fixture it reports `computed = 901..902` -and refuses a wrong value. This only bounds `/E` between 1 and the file -length, so a writer that miscomputes it passes here and fails qpdf. Computing -it needs the first-page object set, which is the same partition the hint -tables describe, so it waits for that work rather than being approximated now. +⛔ **`/E` is checked against the objects the file says are the first page's, +not merely against the file's length.** Table F.1 calls `/E` the offset of the +end of the first page. This used to bound it only between 1 and the file +length, on the reasoning that the first-page partition "is the hint tables' +job" — which was true of the hint tables and false of the **first-page +cross-reference section**, which Annex F puts at the front and the file's own +`startxref` points at. Its entries *are* the first page's objects; that is the +whole trick of the format. So `/E` must equal the end of the last of them, and +a writer that is one byte out is now told so instead of passing. + +⛔⛔ **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, so the highest-offset entry in that section is not always a first-page +object: + +| producer | `/H` offset | `/E` | hint stream sits | +|---|---|---|---| +| qpdf | 550 | 902 | **before** the first page | +| Ghostscript `-dFastWebView` | 829 | 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. Both qpdf fixtures place it before and **cannot tell the two +rules apart**, which is why `testdata/linearized-gs.pdf` is committed beside +them: removing the exclusion turns exactly that test red and leaves the qpdf +ones green. A second producer measures what the first cannot. + +⚠ The **overflow** hint stream is excluded by the same reasoning and no file +can reach it — it needs a four-entry `/H` whose second pair is both a real +stream and the highest-offset object, and neither producer writes one. So the +question is asked of a function rather than inline, where four plain cases can +put it and a mutation can kill it. ⛔⛔ **A value this build cannot address is refused, not truncated**, and the same bytes therefore get the same answer on a 32-bit build as on a 64-bit one. diff --git a/linearized.go b/linearized.go index c048f0a..0762219 100644 --- a/linearized.go +++ b/linearized.go @@ -255,17 +255,115 @@ func CheckLinearized(b []byte) (*Linearization, error) { if err := checkMainXref(b, lin); err != nil { return nil, err } - // Table F.1: /E is "the offset of the end of the first page". Nothing here - // knows which objects the first page owns — that is the hint tables' job — - // so what is checked is the one thing that is checkable without them: an - // offset past the end of the file sends a reader nowhere. - if lin.FirstPageEnd <= 0 || lin.FirstPageEnd > len(b) { - return nil, fmt.Errorf("%w: /E is %d and the file is %d bytes", - ErrLinearizationStale, lin.FirstPageEnd, len(b)) + if err := checkFirstPageEnd(b, lin); err != nil { + return nil, err } return lin, nil } +// isHintObject says whether an object at this offset is one of the hint +// streams /H names, and so is not a first-page object whose end could be /E. +// +// ⛔⛔ IT IS A FUNCTION SO THAT THE SECOND HALF CAN BE TESTED AT ALL. The +// primary hint stream is reachable from a committed file — Ghostscript puts +// it at /E — but the OVERFLOW stream is not: it needs a four-entry /H whose +// second pair is both a real stream and the highest-offset object in the +// first-page section, and neither qpdf nor Ghostscript writes one. Left +// inline, that condition would be a branch no fixture could reach and no +// mutation could kill, which is the vacuity this file has already spent +// three review rounds removing. Asked directly it is four plain cases. +func isHintObject(lin *Linearization, off int64) bool { + if off == int64(lin.HintOffset) { + return true + } + return lin.HasOverflowHint && off == int64(lin.OverflowHintOffset) +} + +// checkFirstPageEnd checks /E, Table F.1's "offset of the end of the first +// page", against the objects the file itself says belong to the first page. +// +// ⛔ THE FIRST-PAGE CROSS-REFERENCE SECTION IS THE ANSWER, and it costs +// nothing to read. Annex F puts it at the front (part 3) 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. So its entries ARE the +// first page's objects, and no hint table has to be decoded to learn that. +// This checker used to say the partition "is the hint tables' job" and bound +// /E only to the file's length; that was true of the hint tables and not of +// this section. +// +// ⛔⛔ 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, so the highest-offset entry here is not always a first-page object: +// +// qpdf /H at 550, /E 902 — the hint stream comes BEFORE +// Ghostscript /H at 829, /E 829 — it comes AFTER, AT /E exactly +// +// On Ghostscript's file the naive maximum is the hint stream itself, ending +// at 967 against an /E of 829, so a rule that took it would refuse every +// file that tool writes. Both committed qpdf fixtures place the hint stream +// before the first page and CANNOT tell the two rules apart — which is why +// testdata/linearized-gs.pdf is committed beside them. +func checkFirstPageEnd(b []byte, lin *Linearization) error { + // ⛔ A SEPARATE Document, not the one CheckLinearized opened. Open walks + // /Prev and merges every section into one table, and setEntry keeps the + // first definition it saw, so by then the front section's entries are no + // longer distinguishable from the rest of the file's. Reading that one + // section alone is the only way to ask which objects it listed. + front := &Document{ + buf: b, + xref: map[int]xrefEntry{}, + cache: map[int]Object{}, + loading: map[int]bool{}, + objStms: map[int]map[int]Object{}, + } + off, err := front.findStartxref() + if err != nil { + return fmt.Errorf("%w: the first-page cross-reference section cannot be found: %v", + ErrLinearizationStale, err) + } + if _, err := front.readXrefSection(off); err != nil { + return fmt.Errorf("%w: the first-page cross-reference section at byte %d does not read: %v", + ErrLinearizationStale, off, err) + } + + // ⛔ THE COMPARISON STAYS IN int64 UNTIL THE VALUE IS KNOWN TO FIT. These + // offsets come from the file, and int(e.offset) is the same silent + // truncation `addressable` exists to refuse everywhere else. + last := int64(-1) + for _, e := range front.xref { + // An object inside an object stream has no offset of its own; its + // container has one and is listed here too, so skipping it loses + // nothing. + if e.kind != 'n' || isHintObject(lin, e.offset) { + continue + } + if e.offset > last { + last = e.offset + } + } + if last < 0 { + return fmt.Errorf("%w: the first-page cross-reference section lists no object in the file", + ErrLinearizationStale) + } + if last >= int64(len(b)) { + return fmt.Errorf("%w: the first-page cross-reference section puts an object at byte %d and the file is %d bytes", + ErrLinearizationStale, last, len(b)) + } + at := int(last) + _, _, n, err := ParseIndirectObject(b[at:], nil) + if err != nil { + return fmt.Errorf("%w: the last first-page object, at byte %d, does not parse: %v", + ErrLinearizationStale, at, err) + } + // The same end-of-object rule the /H length uses: Table F.1 counts the + // line ending after `endobj` and the parse stops at it. + if end := at + n + eolAfter(b, at+n); lin.FirstPageEnd != end { + return fmt.Errorf("%w: /E says %d and the first page ends at %d", + ErrLinearizationStale, lin.FirstPageEnd, end) + } + return nil +} + // checkFirstPage checks /N and /O against the document's own page tree. // // ⛔⛔ THERE IS NO "AND IS IT REALLY A PAGE?" CHECK, AND ITS ABSENCE IS THE diff --git a/linearized_test.go b/linearized_test.go index 90ecbaf..c42a50d 100644 --- a/linearized_test.go +++ b/linearized_test.go @@ -102,7 +102,7 @@ func TestAParameterTheDocumentContradictsIsCaughtOnlyByTheDeepCheck(t *testing.T {"a hint stream running off the end", "/H [ 550 127 ]", "/H [ 550 999 ]", "/H is 999 bytes from 550"}, {"a hint offset pointing at something that is not a stream", "/H [ 550 127 ]", "/H [ 501 127 ]", "not a stream"}, - {"a first page that ends nowhere", "/E 902", "/E 000", "/E is 0"}, + {"a first page that ends nowhere", "/E 902", "/E 000", "/E says 0 and the first page ends at 902"}, {"a main cross-reference that is not there", "/T 1169", "/T 1160", "/T is 1160"}, {"a main cross-reference at no offset at all", "/T 1169", "/T 0000", "/T is 0"}, // ⛔ One byte from the end: the free-entry test has to notice that the @@ -608,8 +608,144 @@ func TestAFirstPageEndingPastTheFileIsRefused(t *testing.T) { if !errors.Is(err, ErrLinearizationStale) { t.Fatalf("error is %v, want ErrLinearizationStale", err) } - if !strings.Contains(err.Error(), "/E is 99999") { - t.Errorf("it does not name the claim: %v", err) + // ⛔ THE MESSAGE CHANGED WITH THE CHECK, AND SO DID WHAT IT PROVES. This + // used to be refused for being past the end of the file, which is the + // weakest thing wrong with it. It is now refused for disagreeing with the + // objects the file's own first-page section lists, and names both numbers. + if !strings.Contains(err.Error(), "/E says 99999 and the first page ends at 902") { + t.Errorf("it does not name the claim and what contradicts it: %v", err) + } +} + +// ⛔⛔ /E OFF BY ONE IN EITHER DIRECTION, which is what a writer that +// miscomputes it actually produces — not 99999. Neither is past the end of +// the file, so the bound this check replaced accepted both. +func TestAFirstPageEndingOffByOneIsRefused(t *testing.T) { + for _, to := range []string{"/E 901", "/E 903"} { + t.Run(to, func(t *testing.T) { + b := editedFixturePadded(t, [2]string{"/E 902", to}) + if _, err := Linearized(b); err != nil { + t.Fatalf("the cheap check refused it: %v", err) + } + _, err := CheckLinearized(b) + if !errors.Is(err, ErrLinearizationStale) { + t.Fatalf("error is %v, want ErrLinearizationStale", err) + } + if !strings.Contains(err.Error(), "the first page ends at 902") { + t.Errorf("it does not say where the first page really ends: %v", err) + } + }) + } +} + +// ⛔⛔ THE FIXTURE THAT MAKES THE /H EXCLUSION A TEST RATHER THAN A HOPE. +// Ghostscript's -dFastWebView writes the primary hint stream AFTER the first +// page, at exactly /E — where qpdf writes it before — so it is the +// highest-offset entry in the first-page section. Take it and this file is +// refused although it is sound: the naive maximum ends at 967 against an /E +// of 829. +// +// Both qpdf fixtures place it before and cannot distinguish the two rules, +// which is the whole reason a second producer is committed here. Measured +// with Ghostscript 10.08.0. +func TestTheHintStreamIsNotTheEndOfTheFirstPage(t *testing.T) { + b, err := os.ReadFile(filepath.Join("testdata", "linearized-gs.pdf")) + if err != nil { + t.Fatal(err) + } + lin, err := CheckLinearized(b) + if err != nil { + t.Fatalf("a Ghostscript-linearized file was refused: %v", err) + } + // The precondition the test rests on: if a future fixture moved the hint + // stream before the first page, this test would pass while proving + // nothing, which is the failure mode it exists to prevent. + if lin.HintOffset != lin.FirstPageEnd { + t.Fatalf("this fixture no longer places /H at /E (/H=%d, /E=%d), so it no longer "+ + "distinguishes the naive rule from the one with the exclusion", + lin.HintOffset, lin.FirstPageEnd) + } +} + +// ⛔⛔ THE OVERFLOW HALF OF THE HINT EXCLUSION, WHICH NO FILE CAN REACH. It +// needs a four-entry /H whose second pair is both a real stream and the +// highest-offset object of the first-page section, and neither producer +// writes one. Asked directly it is four cases, and each kills a mutation: +// dropping the HasOverflowHint guard turns "an overflow offset that is not +// declared" red, and replacing the second return with false turns "the +// overflow hint stream" red. +func TestOnlyTheStreamsSlashHNamesAreExcludedFromTheFirstPage(t *testing.T) { + for _, c := range []struct { + name string + lin Linearization + off int64 + want bool + }{ + {"the primary hint stream", Linearization{HintOffset: 550}, 550, true}, + {"an ordinary first-page object", Linearization{HintOffset: 550}, 871, false}, + {"the overflow hint stream", Linearization{ + HintOffset: 550, HasOverflowHint: true, OverflowHintOffset: 900}, 900, true}, + {"an overflow offset that is not declared", Linearization{ + HintOffset: 550, OverflowHintOffset: 900}, 900, false}, + } { + t.Run(c.name, func(t *testing.T) { + lin := c.lin + if got := isHintObject(&lin, c.off); got != c.want { + t.Errorf("isHintObject(%d) = %v, want %v", c.off, got, c.want) + } + }) + } +} + +// ⛔ THE WAYS THE FIRST-PAGE SECTION CAN FAIL TO ANSWER, each reached by a +// same-width edit so the file still opens and /L still matches. A reader that +// cannot find or read that section cannot judge /E, and saying so beats +// returning nil and calling the file sound. +func TestAFirstPageSectionThatCannotAnswerIsRefused(t *testing.T) { + for _, c := range []struct{ name, from, to, want string }{ + {"a startxref that names no offset", "startxref\n216", "startxref\n/Na", + "cannot be found"}, + {"a startxref pointing at no section", "startxref\n216", "startxref\n217", + "does not read"}, + {"an entry past the end of the file", "0000000871 00000 n", "0009999871 00000 n", + "and the file is 1367 bytes"}, + {"an entry pointing at no object", "0000000871 00000 n", "0000000860 00000 n", + "does not parse"}, + } { + t.Run(c.name, func(t *testing.T) { + b := editedFixturePadded(t, [2]string{c.from, c.to}) + _, err := CheckLinearized(b) + if err == nil { + t.Fatal("it was accepted") + } + if !errors.Is(err, ErrLinearizationStale) { + t.Fatalf("error is %v, want ErrLinearizationStale", err) + } + if !strings.Contains(err.Error(), c.want) { + t.Errorf("it was refused, but not for the reason this case is about: %v", err) + } + }) + } +} + +// ⛔ A FIRST-PAGE SECTION LISTING ONLY FREE OBJECTS names nothing whose end +// could be /E. Every entry is marked `f`, which is the same width as `n`, so +// the file is otherwise untouched. +func TestAFirstPageSectionWithNoObjectInTheFileIsRefused(t *testing.T) { + b := editedFixturePadded(t, + [2]string{"0000000015 00000 n", "0000000015 00000 f"}, + [2]string{"0000000501 00000 n", "0000000501 00000 f"}, + [2]string{"0000000550 00000 n", "0000000550 00000 f"}, + [2]string{"0000000677 00000 n", "0000000677 00000 f"}, + [2]string{"0000000773 00000 n", "0000000773 00000 f"}, + [2]string{"0000000871 00000 n", "0000000871 00000 f"}, + ) + _, err := CheckLinearized(b) + if err == nil { + t.Fatal("it was accepted") + } + if !strings.Contains(err.Error(), "lists no object in the file") { + t.Errorf("it was refused, but not for the reason this test is about: %v", err) } } diff --git a/testdata/linearized-gs.pdf b/testdata/linearized-gs.pdf new file mode 100644 index 0000000000000000000000000000000000000000..13d6bab9e63427f702563dd94336e77fcc36e311 GIT binary patch literal 4107 zcmeHKPmkL~6wiW?iV@#ow6OBR;>?VlII$IbmBvZCV$+6f+OD)(X&rlx^6 zv{;e0e);q3@1(7*z4&3`xjal_6Adz?WKQ$`UZZJ^{9uxXoFT<;xfy@J`u9UNv3NQb zQ#b3q#%=h&bGK8!chCgsvwM5Z9@-Tjse%Yf`kza+8r~0M=BD8}^AW}Ss6nz)f+9?;sWTkY=d5ZA&cn#RBMchcr!D0CDINnGH%P$|V$|#ax+c>vI(d zUW^NRx}mPkRVi7YgQS+W5>MSQVyOV(0P=@)@m?Gxs0842aW_dgs^;RJb5zq+VmO9T zZrp6vOO;Bq;W!OPuWO{zAdXRL+Ar5Q#+{Ts1ORCj0L-|Xask5o5`ii=@3v-OO2X&7 zHoZq9hH%5>Zj=lV?zjV%p|Xfc!fnP~AMPMMzh0FDQoLxNTYpDNng+r@>(XpKMQ0*P`x7IgT)2XW+lj% zD-;TE7Yc9H3mYK)QFsUB9||`>=K0J9{BC><8p|PM@fG@FY!rn5OTn!I5mmf~Du`4s z7GXVgXr5~>f$;Eb1w`}AnE(v(#~w?!Z*}(3tdl!5|?oy`Gu zY&)h(RGm_)!GE<>-62#aLkavF&-?YrE5t>zy%<`fL!!mi*UG zKg%feE%`yYP1>UdI}LB2r|jVTPR~0(^9*0!wWZpw3;QTjrtU}y@?Oah%RFnd(cN!t z3WcdT9gc?3>}WcS;>?^{a&BURET*xHaut4N$%pL@;z<&ng^*RzC{l%LpP(RhhisCh zXNVSc8J0}_!0bA$`GRm|$)}u;OpGU!NpYeTlXQToVHlXGSXC9!P_joccc)66eE|OQ zK*GbNS)3K~n2KI9#Ni>PMS>TO97so09jc6kW4E7-xwW}DUj?3G&tC>^z&aYF2a>tq zk|qBOnPfaq>?QsH%;)=q3e4lKfsP533xh|3b+%xoLIsvRm6&SP%&BP-Qp1-tp;};? zlbT({^n&KhbV0YP7(b(X!E=)KfC9FBFFuQtNn9U|qHy;9AkPBMz0S32Sk&YKW>-eB z-F`8S!=Yg2aMW&_PU4M+@Z{KYEO|T*eKQF9O!q6kLM#0eKpXgqcq}WbN~x=QWg2MY z6$<(ap>$!R2n81#k(*_f?6bffNBnx6-DhbAu0-1OsvdlA3F~N_MtLjx9%c~}2xdT0 z3b^{786@e@<**s7>WiUMWV;48dB`JX=Z#*E_)LgG(Q;9DvQhoQF3)3=po1u zh4xIyE%9o#=TIe|f~^EtgO?kX{rYc(%jfe<`0o~4?an{|OP+|mckP*d$@`njHwe5z z;Qx(4e##`MvN%zSbE94^Yh@HHPnm|kcA})Dx^^0+)L7S3DZFxuu{w=Xt+Z}iBJgIp U(7Sx_gqO(GaZpk Date: Fri, 13 Nov 2026 20:49:15 +0100 Subject: [PATCH 2/2] linearize: the section startxref names is not always the first-page one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- README.md | 18 +++++++++++++ linearized.go | 27 +++++++++++++++++++ linearized_test.go | 67 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 112 insertions(+) diff --git a/README.md b/README.md index 859d1cb..ffb5f2c 100644 --- a/README.md +++ b/README.md @@ -187,6 +187,24 @@ rules apart**, which is why `testdata/linearized-gs.pdf` is committed beside them: removing the exclusion turns exactly that test red and leaves the qpdf ones green. A second producer measures what the first cannot. +⛔⛔ **What this proves is that `/E` agrees with the first-page section — not +that the section is right.** A writer that leaves objects page one genuinely +needs out of that section produces a file that is internally consistent and +passes here. Ghostscript does exactly this on a document with shared fonts, +outlines and links: it writes `/H` = `/E` = 1941, this checker accepts it, and +`qpdf --check` computes `5500..5501` and reports the first page as having 9 +objects where the hint table claims 4. Deciding which objects page one *needs* +means walking its resources, which is the partition the hint tables describe — +so that judgement is still outstanding, and a pass here is not evidence about +it. + +⚠ **Stricter than qpdf, deliberately.** qpdf computes `/E` as a *range* +(`901..902` on the committed fixture), accepting the end with or without the +line ending after `endobj`. This requires the end including it, which is what +both measured producers actually write. A file carrying the lower value would +pass `qpdf --check` and be refused here; none has been observed, and the +alternative is a window that cannot tell a correct `/E` from one a byte short. + ⚠ The **overflow** hint stream is excluded by the same reasoning and no file can reach it — it needs a four-entry `/H` whose second pair is both a real stream and the highest-offset object, and neither producer writes one. So the diff --git a/linearized.go b/linearized.go index 0762219..a30a941 100644 --- a/linearized.go +++ b/linearized.go @@ -303,6 +303,15 @@ func isHintObject(lin *Linearization, off int64) bool { // file that tool writes. Both committed qpdf fixtures place the hint stream // before the first page and CANNOT tell the two rules apart — which is why // testdata/linearized-gs.pdf is committed beside them. +// +// ⛔⛔ AND WHAT THIS PROVES IS THAT /E AGREES WITH THAT SECTION, NOT THAT THE +// SECTION IS RIGHT. A writer that leaves out objects page one genuinely needs +// produces a file that is internally consistent and passes here. Review +// measured Ghostscript doing it on a document with shared fonts, outlines and +// links: /H = /E = 1941, accepted here, while qpdf computes 5500..5501 and +// reports page one as 9 objects where the hint table claims 4. Deciding what +// page one NEEDS means walking its resources — the partition the hint tables +// describe — and that judgement is still outstanding (#40). func checkFirstPageEnd(b []byte, lin *Linearization) error { // ⛔ A SEPARATE Document, not the one CheckLinearized opened. Open walks // /Prev and merges every section into one table, and setEntry keeps the @@ -321,6 +330,24 @@ func checkFirstPageEnd(b []byte, lin *Linearization) error { return fmt.Errorf("%w: the first-page cross-reference section cannot be found: %v", ErrLinearizationStale, err) } + // ⛔⛔ THE SECTION startxref NAMES IS NOT ALWAYS THE FIRST-PAGE ONE, and + // an earlier draft of this claimed /L had already made it so. It has not: + // an incremental update appends its own section and points startxref at + // THAT, and a file whose /L was rewritten to match its new length passes + // every earlier check. Following it then lands on the update's entries and + // compares /E with an object that has nothing to do with the first page — + // a refusal with a reason that is not the reason, which is worse than a + // miss because a reader believes it. + // + // Annex F puts the first-page section at part 3 and the main one at part + // 10, so the front section precedes /T. checkMainXref has already + // established /T against the file by the time this runs, and this says + // nothing about /E, so it still holds when /E is the thing under test. + if off >= int64(lin.MainXrefOffset) { + return fmt.Errorf("%w: the cross-reference section at byte %d is not the first-page one — "+ + "it is at or past the main table /T names (%d), so the file has been written to since it was linearized", + ErrLinearizationStale, off, lin.MainXrefOffset) + } if _, err := front.readXrefSection(off); err != nil { return fmt.Errorf("%w: the first-page cross-reference section at byte %d does not read: %v", ErrLinearizationStale, off, err) diff --git a/linearized_test.go b/linearized_test.go index c42a50d..60ec460 100644 --- a/linearized_test.go +++ b/linearized_test.go @@ -8,6 +8,7 @@ package reader import ( "bytes" "errors" + "fmt" "os" "os/exec" "path/filepath" @@ -728,6 +729,72 @@ func TestAFirstPageSectionThatCannotAnswerIsRefused(t *testing.T) { } } +// ⛔⛔ AN ENTRY AT EXACTLY len(b), which is the boundary the length guard is +// about and the only input that tells `>=` from `>`. Past the end the parse +// would fail anyway and the guard would be a bound nothing is holding — +// review measured that mutation staying green — so the case that pins it is +// the equal one: b[len(b):] is a legal empty slice, not a panic, and without +// the guard this is refused for the wrong reason. +func TestAnEntryStartingExactlyAtTheEndOfTheFileIsRefused(t *testing.T) { + b := editedFixturePadded(t, [2]string{"0000000871 00000 n", "0000001367 00000 n"}) + if len(b) != 1367 { + t.Fatalf("this test hard-codes the fixture's length; it is %d", len(b)) + } + _, err := CheckLinearized(b) + if !errors.Is(err, ErrLinearizationStale) { + t.Fatalf("error is %v, want ErrLinearizationStale", err) + } + if !strings.Contains(err.Error(), "puts an object at byte 1367 and the file is 1367 bytes") { + t.Errorf("it was refused, but not by the length guard this test is about: %v", err) + } +} + +// ⛔⛔ AN INCREMENTAL UPDATE POINTS startxref AT ITS OWN SECTION, and a +// previous version of this reasoned that /L had already made that impossible. +// It had not: rewriting /L to the new length passes every earlier check, and +// the first-page end was then compared against an object from the update — +// refused, with a reason that was not the reason. +// +// The file below is exactly that: a real appended revision whose /L is +// corrected, which qpdf still calls linearized. +func TestAnIncrementallyUpdatedFileIsNotJudgedByTheUpdatesSection(t *testing.T) { + orig, err := os.ReadFile(filepath.Join("testdata", "linearized.pdf")) + if err != nil { + t.Fatal(err) + } + body := "9 0 obj\n<< /Type /Nothing >>\nendobj\n" + xrefAt := len(orig) + len(body) + tail := fmt.Sprintf("xref\n0 1\n0000000000 65535 f \ntrailer << /Root 5 0 R /Size 10 /Prev 216 >>\nstartxref\n%d\n%%%%EOF\n", xrefAt) + s := string(orig) + body + tail + + // /L must say the new length, or the cheap check refuses it for that and + // this test would never reach what it is about. Same width keeps every + // other offset valid, so the appended revision is the only change. + old, want := "/L 1367", fmt.Sprintf("/L %d", len(s)) + if len(want) != len(old) { + t.Fatalf("the updated file is %d bytes, which is not the same width as the fixture's /L", len(s)) + } + if strings.Count(s, old) != 1 { + t.Fatalf("%q appears %d times in the fixture, so this edit is not the one it is named for", + old, strings.Count(s, old)) + } + b := []byte(strings.Replace(s, old, want, 1)) + + if _, err := Linearized(b); err != nil { + t.Fatalf("the cheap check refused it, so /L is not what this test is about: %v", err) + } + _, err = CheckLinearized(b) + if !errors.Is(err, ErrLinearizationStale) { + t.Fatalf("error is %v, want ErrLinearizationStale", err) + } + // ⛔ THE REASON IS THE ASSERTION. Without the guard this file is still + // refused — by the /E comparison, naming an object the update appended — + // so a test that only checked for an error would pass either way. + if !strings.Contains(err.Error(), "written to since it was linearized") { + t.Errorf("it was refused, but not as an updated file: %v", err) + } +} + // ⛔ A FIRST-PAGE SECTION LISTING ONLY FREE OBJECTS names nothing whose end // could be /E. Every entry is marked `f`, which is the same width as `n`, so // the file is otherwise untouched.