From f3bbb0c0adf4b2d42043d6529ece67c97756aa04 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Thu, 12 Nov 2026 21:32:23 +0100 Subject: [PATCH 1/7] linearize: check the parameters against the document, not only against themselves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⛔⛔ Linearized() IS A WEAKER TEST THAN IT LOOKS, AND A WRITER DEVELOPED AGAINST IT WOULD PASS WITHOUT LINEARIZING ANYTHING. Of Table F.1's claims it checks exactly one against the file — /L against the byte count. /O, /E, /N and /T are read for type and never consulted. So a file carrying a correct /L and four invented integers is ACCEPTED, which is precisely what a first linearizer emits. That is not a defect in Linearized(): it is deliberately cheap, because the reader it serves has fetched 1024 bytes and has no cross-reference table to resolve anything with. It is the wrong judge for a writer, and reader#40 opens the writer, so the judge comes first and separately — developed against a check that was not written to let it through. CheckLinearized(b) answers the other question: not "may a reader trust the front of this file" but "is this file actually what the front says". It opens the document and establishes that every parameter POINTS AT WHAT IT CLAIMS TO: /N the real page count /O the real first page, and an object whose /Type is /Page — a number that happens to be right while the object is something else is a file whose page tree was rebuilt under it /H an indirect object that is a STREAM, at the offset Table F.1 says is "the beginning of the stream object, not the beginning of the stream data" /T the main cross-reference — ⛔ IN EITHER SHAPE. Table F.1 gives it two meanings and both are real: the white space before the object-zero entry of a traditional table, or a cross-reference STREAM object. Accepting only one would call every file of the other kind stale. /E inside the file. Nothing here knows which objects the first page owns — that is the hint tables' job — so this is the one claim that can be checked without them. ⛔ THE FOUR GATES ARE FILES Linearized() ACCEPTS AND THIS REFUSES. Each is the committed qpdf fixture with ONE integer replaced by one of THE SAME WIDTH, so /L still equals the byte count and nothing at the front of the file is wrong — what is wrong is that it describes a different document. The helper ASSERTS the width: an edit one byte longer would be refused for the length instead, the deep check would never run, and the test would pass measuring nothing. ⛔ And the negative control: an ordinary file written by this package is refused by BOTH checks. A suite in which every input passes measures nothing. Each of the four checks was mutated to always agree, and the verdict was taken from cmd/failset rather than from a pass/fail count — every mutation red on its own subtest and no other: checkFirstPage -> …/a_first_page_that_is_not_the_first_page the /N comparison -> …/a_page_count_the_document_contradicts checkHintStream -> …/a_hint_stream_that_starts_one_byte_late checkMainXref -> …/a_main_cross-reference_that_is_not_there Still not checked, by either: the hint tables (F.4). qpdf --check remains the foreign judge, and CheckLinearized agrees with qpdf's output at 1, 2 and 5 pages. Refs #40. gofmt clean, go vet clean, 99.4% of statements. Co-Authored-By: Claude Opus 5 --- README.md | 29 ++++++++ linearized.go | 175 +++++++++++++++++++++++++++++++++++++++++++++ linearized_test.go | 115 +++++++++++++++++++++++++++++ 3 files changed, 319 insertions(+) diff --git a/README.md b/README.md index 320dc61..14a3c38 100644 --- a/README.md +++ b/README.md @@ -134,6 +134,35 @@ types; and `/L` against the real file length. Not checked: the hint tables themselves (F.4) — a file can pass this and still send a reader to the wrong offset. +⛔⛔ **`Linearized` on its own is a weaker test than it looks, and a writer +developed against it would pass without linearizing anything.** Of Table F.1's +claims it checks exactly one against the file — `/L` against the byte count — +because it is deliberately cheap: a reader deciding whether to stream a file +has fetched 1024 bytes and has no cross-reference table to resolve anything +with. `/O`, `/E`, `/N` and `/T` are read for type and never consulted, so a +file carrying a correct `/L` and four **invented integers** is accepted. That +is precisely what a first linearizer emits. + +`CheckLinearized(b)` is the other question, for the other caller: not "may a +reader trust the front of this file" but "is this file actually what the front +says". It opens the document, so it costs what opening costs, and it is the +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. + +The four cases that gate it are files `Linearized` **accepts** and +`CheckLinearized` refuses: the committed qpdf fixture with one integer +replaced by one of **the same width**, so `/L` still matches and nothing at +the front of the file is wrong — what is wrong is that it describes a +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. + +Still not checked, by either: the hint tables (F.4). `qpdf --check` remains the +foreign judge. + The positives are **qpdf's** output, not fixtures written to agree with this reading of Annex F. On five real library scans linearized by qpdf and judged by `qpdf --check` alongside: **5 agree, 0 disagree**, including a 68-page, 13 MB diff --git a/linearized.go b/linearized.go index d4cf776..22c4de9 100644 --- a/linearized.go +++ b/linearized.go @@ -186,6 +186,181 @@ func Linearized(b []byte) (*Linearization, error) { return lin, nil } +// CheckLinearized reads the parameter dictionary and then checks what it says +// against the document it describes. +// +// ⛔⛔ [Linearized] ON ITS OWN IS A WEAKER TEST THAN IT LOOKS, AND A WRITER +// DEVELOPED AGAINST IT WOULD PASS WITHOUT LINEARIZING ANYTHING. Of Table F.1's +// claims it checks exactly one against the file — /L against the byte count — +// because it is deliberately cheap: a reader deciding whether to stream a file +// has fetched 1024 bytes and has no cross-reference table to resolve anything +// with. /O, /E, /N and /T are read for type and never consulted. So a file +// carrying a correct /L and four invented integers is accepted, which is +// precisely what a first linearizer would emit. +// +// This is the other question, for the other caller: not "may a reader trust +// the front of this file" but "is this file actually what the front says". +// It opens the document, so it costs what opening costs, and it is the +// judgement a writer needs on bytes it has just produced. +// +// ⛔ It still does NOT validate the hint tables (F.4), which is a larger job +// again. What it establishes is that every parameter POINTS AT WHAT IT CLAIMS +// TO: the page count is the real one, /O is the real first page, /H begins at +// a real stream object, /T reaches the main cross-reference. A file can pass +// this and still send a reader to the wrong offset inside a hint table — so +// the error says which claim failed, and a caller that needs more must read +// F.4. qpdf's `--check` remains the foreign judge the tests print by. +// +// The error wraps the same sentinels [Linearized] uses: a dictionary that +// disagrees with the document around it is [ErrLinearizationStale], which is +// what Table F.1 says such a file is — ordinary PDF. +func CheckLinearized(b []byte) (*Linearization, error) { + lin, err := Linearized(b) + if err != nil { + return nil, err + } + d, err := Open(b) + if err != nil { + return nil, fmt.Errorf("%w: the file carries linearization parameters and does not open: %v", + ErrLinearizationStale, err) + } + if n := d.PageCount(); n != lin.PageCount { + return nil, fmt.Errorf("%w: /N says %d page(s) and the document has %d", + ErrLinearizationStale, lin.PageCount, n) + } + if err := checkFirstPage(d, lin); err != nil { + return nil, err + } + if err := checkHintStream(b, lin); err != nil { + return nil, err + } + 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)) + } + return lin, nil +} + +// checkFirstPage checks /O against the document's real first page. +// +// ⛔ PageRef IS 1-BASED HERE. Asking for page 0 returns nothing and would make +// every file look stale, which is the kind of off-by-one a checker reports as +// the subject's defect rather than its own. +func checkFirstPage(d *Document, lin *Linearization) error { + ref, ok := d.PageRef(1) + if !ok { + return fmt.Errorf("%w: /O names object %d and the document has no first page", + ErrLinearizationStale, lin.FirstPageObject) + } + if ref.Num != lin.FirstPageObject { + return fmt.Errorf("%w: /O says object %d and the first page is object %d", + ErrLinearizationStale, lin.FirstPageObject, ref.Num) + } + // And it is a page. A /O that happens to equal the right number while the + // object is something else is a file whose page tree was rebuilt under it. + obj, err := d.Get(Ref{Num: lin.FirstPageObject}) + if err != nil { + return fmt.Errorf("%w: /O names object %d, which does not read: %v", + ErrLinearizationStale, lin.FirstPageObject, err) + } + dict, ok := obj.(Dict) + if !ok { + return fmt.Errorf("%w: /O names object %d, which is %T and not a page", + ErrLinearizationStale, lin.FirstPageObject, obj) + } + if t, _ := dict["Type"].(Name); t != "Page" { + return fmt.Errorf("%w: /O names object %d, whose /Type is %q and not /Page", + ErrLinearizationStale, lin.FirstPageObject, string(t)) + } + return nil +} + +// checkHintStream checks the first pair of /H. +// +// Table F.1 is explicit that the offset is "the beginning of the stream +// object, not the beginning of the stream data", so what must be at it is an +// indirect object, and that object must be a stream — the hint tables live in +// its data. +func checkHintStream(b []byte, lin *Linearization) error { + if lin.HintOffset <= 0 || lin.HintOffset >= len(b) { + return fmt.Errorf("%w: /H starts at %d and the file is %d bytes", + ErrLinearizationStale, lin.HintOffset, len(b)) + } + if lin.HintLength <= 0 || lin.HintOffset+lin.HintLength > len(b) { + return fmt.Errorf("%w: /H is %d bytes from %d and the file is %d", + ErrLinearizationStale, lin.HintLength, lin.HintOffset, len(b)) + } + // ⛔ No resolver, deliberately: a hint stream whose /Length is an indirect + // reference cannot be read by a reader that has only fetched the front of + // the file, which is the reader this format exists for. + _, obj, _, err := ParseIndirectObject(b[lin.HintOffset:], nil) + if err != nil { + return fmt.Errorf("%w: /H points at byte %d, where no indirect object parses: %v", + ErrLinearizationStale, lin.HintOffset, err) + } + if _, ok := obj.(*Stream); !ok { + return fmt.Errorf("%w: /H points at byte %d, where the object is %T and not a stream", + ErrLinearizationStale, lin.HintOffset, obj) + } + return nil +} + +// checkMainXref checks /T. +// +// ⛔ TABLE F.1 GIVES IT TWO MEANINGS AND BOTH ARE REAL. For a file with a +// traditional cross-reference table it is "the offset of the white-space +// character preceding the first entry of the main cross-reference table (the +// entry for object number 0)"; for a file using cross-reference streams it is +// "the offset of the main cross-reference stream object". Accepting only one +// would call every file of the other kind stale, so both are tried and the +// failure names both. +func checkMainXref(b []byte, lin *Linearization) error { + if lin.MainXrefOffset <= 0 || lin.MainXrefOffset >= len(b) { + return fmt.Errorf("%w: /T is %d and the file is %d bytes", + ErrLinearizationStale, lin.MainXrefOffset, len(b)) + } + rest := b[lin.MainXrefOffset:] + if freeEntryFollows(rest) { + return nil + } + if _, obj, _, err := ParseIndirectObject(rest, nil); err == nil { + if s, ok := obj.(*Stream); ok { + if t, _ := s.Dict["Type"].(Name); t == "XRef" { + return nil + } + } + } + return fmt.Errorf("%w: /T is %d, where there is neither the free entry of a "+ + "cross-reference table nor a cross-reference stream", ErrLinearizationStale, + lin.MainXrefOffset) +} + +// freeEntryFollows says whether the bytes begin with the whitespace and then +// the object-zero entry of a traditional cross-reference table: ten digits, a +// space, 65535, a space and `f`. +func freeEntryFollows(b []byte) bool { + i := 0 + for i < len(b) && isSpace(b[i]) { + i++ + } + if i+18 > len(b) { + return false + } + for j := i; j < i+10; j++ { + if b[j] < '0' || b[j] > '9' { + return false + } + } + return string(b[i+10:i+18]) == " 65535 f" +} + // firstBodyDict parses the first indirect object of the body and returns its // dictionary with the offset one past its end. // diff --git a/linearized_test.go b/linearized_test.go index 1e365c7..0d1fd02 100644 --- a/linearized_test.go +++ b/linearized_test.go @@ -68,6 +68,121 @@ func TestLinearizedReadsTheCommittedFixture(t *testing.T) { } } +// ⛔⛔ THE PAIR THAT MAKES CheckLinearized WORTH HAVING. Each case below is a +// file that [Linearized] ACCEPTS and [CheckLinearized] refuses: the dictionary +// is well formed, within the window, and /L still equals the byte count, +// because every edit replaces an integer with one of THE SAME WIDTH. Nothing +// in the front of the file is wrong — what is wrong is that it describes a +// different document. +// +// That is the shape a first linearizer emits, and it is why a writer must not +// be developed against the cheap check alone: it would pass while linearizing +// nothing. +func TestAParameterTheDocumentContradictsIsCaughtOnlyByTheDeepCheck(t *testing.T) { + for _, c := range []struct { + name, from, to, says string + }{ + {"a page count the document contradicts", "/N 2", "/N 3", "/N says 3 page(s)"}, + {"a first page that is not the first page", "/O 7", "/O 6", "/O says object 6"}, + {"a hint stream that starts one byte late", "/H [ 550 127 ]", "/H [ 551 127 ]", "/H points at byte 551"}, + {"a main cross-reference that is not there", "/T 1169", "/T 1160", "/T is 1160"}, + } { + t.Run(c.name, func(t *testing.T) { + b := editedFixture(t, c.from, c.to) + + // The cheap check still passes, which is the whole point. + if _, err := Linearized(b); err != nil { + t.Fatalf("Linearized refused it, so this case proves nothing "+ + "about the deep check: %v", err) + } + _, err := CheckLinearized(b) + if err == nil { + t.Fatal("CheckLinearized accepted a file that describes another document") + } + if !errors.Is(err, ErrLinearizationStale) { + t.Errorf("error is %v, want it to wrap ErrLinearizationStale", err) + } + if !strings.Contains(err.Error(), c.says) { + t.Errorf("it does not name the claim that failed: %v", err) + } + }) + } +} + +// editedFixture rewrites one entry of the committed fixture's parameter +// dictionary, keeping the byte count identical. +// +// ⛔ THE WIDTH IS ASSERTED, NOT ASSUMED. A replacement one byte longer changes +// the file's length, so /L would no longer match and [Linearized] would refuse +// the file for THAT reason — the deep check would never run, and the test +// would pass while measuring nothing. +func editedFixture(t *testing.T, from, to string) []byte { + t.Helper() + if len(from) != len(to) { + t.Fatalf("the edit %q -> %q changes the file's length, which breaks /L "+ + "and makes this test measure the wrong refusal", from, to) + } + b, err := os.ReadFile(filepath.Join("testdata", "linearized.pdf")) + if err != nil { + t.Fatal(err) + } + s := string(b) + if strings.Count(s, from) != 1 { + t.Fatalf("%q appears %d times in the fixture, so this edit is not the one "+ + "it is named for", from, strings.Count(s, from)) + } + return []byte(strings.Replace(s, from, to, 1)) +} + +// And the fixture itself passes the deep check, so the refusals above are +// about the edits and not about the checker disliking qpdf's output. +func TestCheckLinearizedAcceptsTheCommittedFixture(t *testing.T) { + b, err := os.ReadFile(filepath.Join("testdata", "linearized.pdf")) + if err != nil { + t.Fatal(err) + } + lin, err := CheckLinearized(b) + if err != nil { + t.Fatalf("qpdf's own linearized file was refused: %v", err) + } + if lin.PageCount != 2 { + t.Errorf("/N %d, want 2", lin.PageCount) + } +} + +// ⛔ THE NEGATIVE CONTROL, and it is the first thing this file needed. An +// ordinary document written by this package is NOT linearized, and both checks +// must say so — a suite in which every input passes measures nothing. +func TestAnOrdinaryFileIsRefusedByBothChecks(t *testing.T) { + b := buildPages(t, 2) + if _, err := Linearized(b); !errors.Is(err, ErrNotLinearized) { + t.Errorf("Linearized said %v, want ErrNotLinearized", err) + } + if _, err := CheckLinearized(b); !errors.Is(err, ErrNotLinearized) { + t.Errorf("CheckLinearized said %v, want ErrNotLinearized", err) + } +} + +// qpdf's output passes the deep check at several page counts, not only at the +// one the committed fixture happens to have. +func TestCheckLinearizedAgreesWithQpdfAcrossPageCounts(t *testing.T) { + for _, pages := range []int{1, 2, 5} { + t.Run(strconv.Itoa(pages)+" pages", func(t *testing.T) { + plain, linear := linearizedFixture(t, pages) + if _, err := CheckLinearized(plain); !errors.Is(err, ErrNotLinearized) { + t.Errorf("the plain file was not refused: %v", err) + } + lin, err := CheckLinearized(linear) + if err != nil { + t.Fatalf("qpdf linearized it and this refused it: %v", err) + } + if lin.PageCount != pages { + t.Errorf("/N %d, want %d", lin.PageCount, pages) + } + }) + } +} + func qpdfOrSkip(t *testing.T) string { t.Helper() path, err := exec.LookPath("qpdf") From 6af56b35f61abc12f30ea1533aaf1da7190f81fa Mon Sep 17 00:00:00 2001 From: tannevaled Date: Thu, 12 Nov 2026 21:46:20 +0100 Subject: [PATCH 2/7] linearize: reach every branch, and delete the two that no file could MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI was red on the exact-100% coverage gate: 14 uncovered blocks, every one of them an error path I had added. The gate was right to refuse, and two of the 14 were not missing tests — they were branches that could not be reached from any file at all. ⛔⛔ THE /O CHECK WAS ASKING A STRICTER QUESTION THAN THE READER DOES, AND THAT WAS THE DEFECT THE GATE FOUND. It resolved the object /O names and refused the file unless its /Type was /Page. But Document.PageRef is what the document layer CALLS the first page, and it does not require /Type: walkPages appends any resolvable dictionary with no /Kids. So the checker had a second, stricter definition of "first page" than the reader that will actually open the file — a judge and a subject that do not count alike — and a file the reader is perfectly happy with would have been reported as stale. What /O must equal is what a reader will treat as page one, and nothing else. The Get-error and not-a-Dict branches went with it; no file could reach them, which is what dead code dressed as thoroughness looks like. ⛔ /N < 1 IS NOW ITS OWN ANSWER, AND IT HAS TO COME FIRST. Table F.1 calls /N "the number of pages in the document"; a linearized document with none is not a document this format describes. Checked before the comparison, because a /N of 0 reported as "the document has 2" sends the reader to a page tree that is not the problem. It also makes the "no first page" branch unreachable — the count is lin.PageCount and that is now at least 1 — so that branch is gone rather than tested. ⛔⛔ AND TABLE F.1 GIVES /T TWO MEANINGS WHILE THE FIXTURE ONLY HAD ONE. Every test ran against testdata/linearized.pdf, a traditional cross-reference table, so the cross-reference STREAM arm had NEVER RUN. Until the gate said so, "both shapes are handled" was a comment and nothing more. testdata/linearized-objstm.pdf is the same document through `qpdf --linearize --object-streams=generate` — still an independent implementation's output, and its /T is an /XRef stream. Seven more same-width edits reach the bounds checks: /E at 0, /H at offset 0, /H running past the end, /H pointing at a dictionary rather than a stream, /T at 0, and /T one byte from the end — that last one so the free-entry test has to notice the buffer is shorter than an entry rather than read past it. And one case that is not an integer edit: a front that still says "linearized" over a body that is gone. The first 1024 bytes are untouched and /L still equals the length, so the cheap check passes; everything after the parameter dictionary is blanked, so there is no cross-reference table to open with. That is a truncated download padded back to length. ⛔ MEASURED WITHOUT QPDF, WHICH IS CI'S CONDITION. `go test -c -cover` and the binary run under a PATH with no qpdf: 0 uncovered blocks, so the committed fixtures carry the gate and the skipping tests are not propping it up. Said out loud because reader's CI installs no qpdf at all — see #40. ⛔ covblocks, not `go tool cover -func`: the latter rounded this to "100.0%" while one block was still uncovered. checkFirstPage disabled as a mutation turns exactly its three subtests red — /N too small, /N contradicted, /O wrong — judged by cmd/failset on the failing NAMES rather than on a count. gofmt clean, go vet clean, 0 uncovered blocks of 1743. Refs #40. Co-Authored-By: Claude Opus 5 --- linearized.go | 57 +++++++++++++------------- linearized_test.go | 71 +++++++++++++++++++++++++++++++++ testdata/linearized-objstm.pdf | Bin 0 -> 1612 bytes 3 files changed, 100 insertions(+), 28 deletions(-) create mode 100644 testdata/linearized-objstm.pdf diff --git a/linearized.go b/linearized.go index 22c4de9..c953945 100644 --- a/linearized.go +++ b/linearized.go @@ -224,10 +224,6 @@ func CheckLinearized(b []byte) (*Linearization, error) { return nil, fmt.Errorf("%w: the file carries linearization parameters and does not open: %v", ErrLinearizationStale, err) } - if n := d.PageCount(); n != lin.PageCount { - return nil, fmt.Errorf("%w: /N says %d page(s) and the document has %d", - ErrLinearizationStale, lin.PageCount, n) - } if err := checkFirstPage(d, lin); err != nil { return nil, err } @@ -248,37 +244,42 @@ func CheckLinearized(b []byte) (*Linearization, error) { return lin, nil } -// checkFirstPage checks /O against the document's real first page. +// 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 +// POINT. A first version asked d.Get for the object /O names and refused it +// unless its /Type was /Page. But [Document.PageRef] is what the document +// layer calls the first page, and it does not require /Type: walkPages treats +// any resolvable dictionary with no /Kids as a leaf. A stricter definition +// here would make this checker disagree with the reader that will actually +// open the file — a judge and a subject that do not count alike — and would +// report the reader's notion of the page tree as the file's defect. What /O +// has to equal is what a reader will treat as page one. // -// ⛔ PageRef IS 1-BASED HERE. Asking for page 0 returns nothing and would make -// every file look stale, which is the kind of off-by-one a checker reports as -// the subject's defect rather than its own. +// The coverage gate said so first: those branches could not be reached from +// any file, which is what dead code dressed as thoroughness looks like. func checkFirstPage(d *Document, lin *Linearization) error { - ref, ok := d.PageRef(1) - if !ok { - return fmt.Errorf("%w: /O names object %d and the document has no first page", - ErrLinearizationStale, lin.FirstPageObject) + // ⛔ BEFORE THE COMPARISON, or a /N of 0 is reported as a disagreement + // with whatever the document really holds, which sends the reader looking + // for a page tree that is not the problem. Table F.1 calls /N "the number + // of pages in the document"; a linearized document with none is not a + // document this format describes. + if lin.PageCount < 1 { + return fmt.Errorf("%w: /N says %d page(s), and a linearized document has at least one", + ErrLinearizationStale, lin.PageCount) } + if n := d.PageCount(); n != lin.PageCount { + return fmt.Errorf("%w: /N says %d page(s) and the document has %d", + ErrLinearizationStale, lin.PageCount, n) + } + // PageRef counts from one, and the page count has just been established + // as lin.PageCount, which is at least one — so this cannot fail, and a + // branch for it would be one no file could reach. + ref, _ := d.PageRef(1) if ref.Num != lin.FirstPageObject { return fmt.Errorf("%w: /O says object %d and the first page is object %d", ErrLinearizationStale, lin.FirstPageObject, ref.Num) } - // And it is a page. A /O that happens to equal the right number while the - // object is something else is a file whose page tree was rebuilt under it. - obj, err := d.Get(Ref{Num: lin.FirstPageObject}) - if err != nil { - return fmt.Errorf("%w: /O names object %d, which does not read: %v", - ErrLinearizationStale, lin.FirstPageObject, err) - } - dict, ok := obj.(Dict) - if !ok { - return fmt.Errorf("%w: /O names object %d, which is %T and not a page", - ErrLinearizationStale, lin.FirstPageObject, obj) - } - if t, _ := dict["Type"].(Name); t != "Page" { - return fmt.Errorf("%w: /O names object %d, whose /Type is %q and not /Page", - ErrLinearizationStale, lin.FirstPageObject, string(t)) - } return nil } diff --git a/linearized_test.go b/linearized_test.go index 0d1fd02..b5856a2 100644 --- a/linearized_test.go +++ b/linearized_test.go @@ -6,6 +6,7 @@ package reader import ( + "bytes" "errors" "os" "os/exec" @@ -83,9 +84,19 @@ func TestAParameterTheDocumentContradictsIsCaughtOnlyByTheDeepCheck(t *testing.T name, from, to, says string }{ {"a page count the document contradicts", "/N 2", "/N 3", "/N says 3 page(s)"}, + {"a document with no pages at all", "/N 2", "/N 0", "a linearized document has at least one"}, {"a first page that is not the first page", "/O 7", "/O 6", "/O says object 6"}, {"a hint stream that starts one byte late", "/H [ 550 127 ]", "/H [ 551 127 ]", "/H points at byte 551"}, + {"a hint stream at no offset at all", "/H [ 550 127 ]", "/H [ 000 127 ]", "/H starts at 0"}, + {"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 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 + // buffer is shorter than an entry rather than read past it. + {"a main cross-reference one byte from the end", "/T 1169", "/T 1366", "/T is 1366"}, } { t.Run(c.name, func(t *testing.T) { b := editedFixture(t, c.from, c.to) @@ -134,6 +145,66 @@ func editedFixture(t *testing.T, from, to string) []byte { return []byte(strings.Replace(s, from, to, 1)) } +// ⛔⛔ TABLE F.1 GIVES /T TWO MEANINGS AND THE FIRST FIXTURE ONLY HAS ONE OF +// THEM. testdata/linearized.pdf uses a traditional cross-reference table, so +// every test above exercises the free-entry arm and the cross-reference STREAM +// arm had never run — the coverage gate is what said so, and until it did, the +// claim that both shapes are handled was a comment and nothing more. +// +// testdata/linearized-objstm.pdf is the same document through +// `qpdf --linearize --object-streams=generate`: still an independent +// implementation's output, and its /T is the offset of an /XRef stream object. +func TestCheckLinearizedAcceptsACrossReferenceStreamFile(t *testing.T) { + b, err := os.ReadFile(filepath.Join("testdata", "linearized-objstm.pdf")) + if err != nil { + t.Fatal(err) + } + lin, err := CheckLinearized(b) + if err != nil { + t.Fatalf("a file whose /T names a cross-reference stream was refused: %v", err) + } + if lin.PageCount != 2 { + t.Errorf("/N %d, want 2", lin.PageCount) + } +} + +// ⛔ A FRONT THAT STILL SAYS "LINEARIZED" OVER A BODY THAT IS GONE. The first +// 1024 bytes are untouched and /L still equals the file length, so the cheap +// check passes; everything after the parameter dictionary is blanked, so there +// is no cross-reference table to open the document with. +// +// This is what a truncated download padded back to length looks like, and it +// is the one case where the deep check's answer is about the DOCUMENT rather +// than about a parameter. +func TestAFrontThatSaysLinearizedOverABodyThatIsGone(t *testing.T) { + b, err := os.ReadFile(filepath.Join("testdata", "linearized.pdf")) + if err != nil { + t.Fatal(err) + } + end := bytes.Index(b, []byte("endobj")) + if end < 0 { + t.Fatal("the fixture has no first object to keep") + } + end += len("endobj") + gutted := make([]byte, 0, len(b)) + gutted = append(gutted, b[:end]...) + gutted = append(gutted, bytes.Repeat([]byte(" "), len(b)-end)...) + if len(gutted) != len(b) { + t.Fatalf("the blanking changed the length, so /L would be the refusal: %d vs %d", + len(gutted), len(b)) + } + if _, err := Linearized(gutted); err != nil { + t.Fatalf("the cheap check refused it, so this proves nothing about the deep one: %v", err) + } + _, err = CheckLinearized(gutted) + if !errors.Is(err, ErrLinearizationStale) { + t.Fatalf("error is %v, want it to wrap ErrLinearizationStale", err) + } + if !strings.Contains(err.Error(), "does not open") { + t.Errorf("it does not say the document would not open: %v", err) + } +} + // And the fixture itself passes the deep check, so the refusals above are // about the edits and not about the checker disliking qpdf's output. func TestCheckLinearizedAcceptsTheCommittedFixture(t *testing.T) { diff --git a/testdata/linearized-objstm.pdf b/testdata/linearized-objstm.pdf new file mode 100644 index 0000000000000000000000000000000000000000..b3d16fa9349cb91f018f32466a8f0c159c60468e GIT binary patch literal 1612 zcmc(f&ubGw6vtOh7P=IyUaaEbL7PIcx;wL*-3<+)^hao++B6Mqp%nJlY?_i}WwWi( zlLlJElOjkD9<-o%k}4i79;DEVMN1F<0X^t{Ak<66*(5Dn5AX1zI(1mKgoyhmI*=5>z@ z64SBhuG*N=-9ia`5eA(6LZMg!3HX#tteojNE@h{9z)w*r5Jg755FoUrE|sY>*;74Kik>< zxUUB`ekyI7h8AKn-$UR#RX=hJ&p03RplDKhmXv5Y)W~bf59pqrcd~(!Py#2K+AskU z)vIn+R5;3{Y1-*k%qCfOxuf-RtH2my~bf&$i8G*=JSK_S|gnr zNI&2dJyP^a)V8Ww*Qv>Rzf<;2t}f78p#Kk%(xl`pxF&_9<>6OBjnU^00Q$iWBxwp* zN5kq7G%+T zLGoj_m1^&x&F7@r19Q47`6SPF(I*2NE1L|{BA%*t?!Wf~+F^9pE4#$zFp{}QWN0kO F{RZxX!AAf9 literal 0 HcmV?d00001 From 45aeb2e711d77e86e25da171134b4aa33661f022 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Thu, 12 Nov 2026 21:51:43 +0100 Subject: [PATCH 3/7] linearize: the /H bound overflowed on 32-bit, and the check is fuzzed now MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⛔⛔ A SECURITY AUDIT OF MY OWN NEW CODE, AND IT FOUND ONE. checkHintStream bounded the hint stream with lin.HintOffset+lin.HintLength > len(b) Both operands come straight out of the first object of the file, so an attacker chooses them. On a 32-bit build, int is 32 bits: a /H length near MaxInt32 makes that sum OVERFLOW to a negative number, the comparison is false, and the check admits exactly the file it exists to refuse. It is now a subtraction — the offset is already known to be inside the file, so len(b)-lin.HintOffset is positive and cannot wrap. Nothing could read past the end as a result (the slice is taken at the offset, which is bounds-checked separately), so this was a wrong VERDICT rather than a wrong ACCESS. It is still the defect class CLAUDE.md names, in code I wrote an hour ago, and it was invisible on this 64-bit workstation. ⛔ AND THE CHECK IS FUZZED NOW, because it is a parser of numbers an attacker chose: /H, /O, /E and /T are used as offsets and lengths into the file, which is where a bad bound stops being a wrong answer and becomes a read past the end. The cheap Linearized() only type-checks them; this one slices at them. FuzzCheckLinearized seeds with the two committed fixtures — byte substitutions in those keep the LENGTH, so /L still matches and the mutator reaches the deep checks instead of stopping at the front — plus three fronts whose integers are absurd on purpose, including MaxInt32 in both halves of /H, which is the regression seed for the overflow above. ⛔ It asserts an INVARIANT rather than merely "it did not panic": the deep check is strictly stronger than the cheap one, so anything it accepts the cheap one must accept too. A disagreement would mean the two read the front of a file differently — which is how a reader and a writer come to disagree about the same bytes. It also asserts that every offset accepted is inside the file accepted, which is the property the overflow broke. Measured: 2 579 861 executions, 217 interesting inputs, no crasher and no invariant violation. gofmt clean, go vet clean, 0 uncovered blocks of 1743. Refs #40. Co-Authored-By: Claude Opus 5 --- fuzz_test.go | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++ linearized.go | 9 +++++++- 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/fuzz_test.go b/fuzz_test.go index cdd8a47..51ff850 100644 --- a/fuzz_test.go +++ b/fuzz_test.go @@ -171,3 +171,63 @@ func FuzzDecode(f *testing.F) { }) }) } + +// FuzzCheckLinearized drives the deep linearization check. +// +// ⛔ IT IS A PARSER OF NUMBERS AN ATTACKER CHOSE. /H, /O, /E, /T and /N come +// out of the first object of the file and are used as OFFSETS and LENGTHS into +// it. The cheap [reader.Linearized] only type-checks them; this one slices the +// file at them, which is where a bad bound stops being a wrong answer and +// starts being a read past the end. +// +// The seeds are the two committed fixtures — byte substitutions in them keep +// the length, so /L still matches and the mutator reaches the deep checks +// rather than stopping at the front — plus fronts whose integers are absurd on +// purpose. +func FuzzCheckLinearized(f *testing.F) { + for _, name := range []string{"linearized.pdf", "linearized-objstm.pdf"} { + if b, err := os.ReadFile(filepath.Join("testdata", name)); err == nil { + f.Add(b) + } + } + // ⛔ MaxInt32 in both halves of /H. Written as an addition, the bound check + // overflows to a negative number on a 32-bit build and admits the file; + // this seed is the regression for that, and it is the shape a hostile file + // takes rather than one a generator would invent. + for _, s := range []string{ + "<< /Linearized 1 /L 1 /H [ 2147483647 2147483647 ] /O 1 /E 1 /N 1 /T 1 >>", + "<< /Linearized 1 /L 1 /H [ -1 -1 ] /O -1 /E -1 /N -1 /T -1 >>", + "<< /Linearized 1 /L 1 /H [ 0 0 ] /O 0 /E 0 /N 0 /T 0 >>", + } { + f.Add([]byte("%PDF-1.7\n1 0 obj\n" + s + "\nendobj\n")) + } + f.Fuzz(func(t *testing.T, b []byte) { + budget(t, 2*time.Second, len(b), "CheckLinearized", func() { + lin, err := reader.CheckLinearized(b) + if err != nil { + return + } + // ⛔ THE INVARIANT, not merely "it did not panic". The deep check + // is strictly stronger than the cheap one, so anything it accepts + // the cheap one must accept too. A disagreement would mean one of + // them reads the front of a file differently from the other, which + // is how a reader and a writer come to disagree about the same + // bytes. + if _, err := reader.Linearized(b); err != nil { + t.Fatalf("CheckLinearized accepted a file Linearized refuses: %v", err) + } + // And every offset it accepted is inside the file it accepted. + if lin.HintOffset < 0 || lin.HintOffset > len(b) || + lin.HintLength < 0 || lin.HintLength > len(b)-lin.HintOffset { + t.Fatalf("/H [%d %d] accepted for a file of %d bytes", + lin.HintOffset, lin.HintLength, len(b)) + } + if lin.MainXrefOffset < 0 || lin.MainXrefOffset > len(b) { + t.Fatalf("/T %d accepted for a file of %d bytes", lin.MainXrefOffset, len(b)) + } + if lin.FirstPageEnd < 0 || lin.FirstPageEnd > len(b) { + t.Fatalf("/E %d accepted for a file of %d bytes", lin.FirstPageEnd, len(b)) + } + }) + }) +} diff --git a/linearized.go b/linearized.go index c953945..84e5fda 100644 --- a/linearized.go +++ b/linearized.go @@ -294,7 +294,14 @@ func checkHintStream(b []byte, lin *Linearization) error { return fmt.Errorf("%w: /H starts at %d and the file is %d bytes", ErrLinearizationStale, lin.HintOffset, len(b)) } - if lin.HintLength <= 0 || lin.HintOffset+lin.HintLength > len(b) { + // ⛔⛔ SUBTRACTION, NOT ADDITION, AND THAT IS A 32-BIT DEFECT RATHER THAN A + // STYLE. Both values come straight out of the file, so an attacker chooses + // them. Written `lin.HintOffset+lin.HintLength > len(b)`, a /H length near + // MaxInt32 OVERFLOWS to a negative number on a 32-bit build, the comparison + // is false, and the check lets through exactly the file it exists to + // refuse. The offset is already known to be inside the file, so + // len(b)-lin.HintOffset is positive and the subtraction cannot wrap. + if lin.HintLength <= 0 || lin.HintLength > len(b)-lin.HintOffset { return fmt.Errorf("%w: /H is %d bytes from %d and the file is %d", ErrLinearizationStale, lin.HintLength, lin.HintOffset, len(b)) } From 46b8bdd4f76d394b2eb8c8e22c7e15b97e0c7d2d Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 13 Nov 2026 08:27:23 +0100 Subject: [PATCH 4/7] linearize: the checks review found missing, and the ones qpdf disagreed about MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⛔⛔ AN INDEPENDENT REVIEW BUILT FILES AND ASKED BOTH JUDGES. Five of them were accepted here and refused by qpdf 12.4.2 — the judge this whole issue says the writer must be built against. A checker that is weaker than its own foreign judge is a checker a writer can satisfy while producing something qpdf calls not linearized. - /Linearized 9 WAS ACCEPTED. The version was read for its TYPE and its value never looked at. It is the one entry that says which format the rest of the dictionary is written in, so a version this has never seen makes every other entry a guess. Refused in the CHEAP check, as MALFORMED: a reader deciding whether to stream a file consults exactly this. 1 and 1.0 are both accepted, because the specification's Example 2 writes one and qpdf writes the other. - /H [ 550 100 ] FOR A 127-BYTE OBJECT WAS ACCEPTED. The pair was bounded against the FILE and never compared with the stream it describes. It is compared with the parse's own extent now. ⛔ NOT WITH EQUALITY, AND MEASUREMENT IS WHY: requiring n == declared refused every file qpdf produces, because the parse ends at `endobj` while Table F.1's length counts the terminating end-of-line. Measured on qpdf output at 1, 2 and 5 pages and on both committed fixtures: declared was n+1 in every case. Two bytes are allowed for CRLF. The rule is asymmetric on purpose — a declared length SHORTER than the object is always wrong. - THE OVERFLOW PAIR OF /H WAS READ AND NEVER EXAMINED. A file carrying `/H [ 550 127 9 9 ]` was accepted while qpdf said the overflow hint table is not a stream. An entry present and unchecked is worse than one absent. - /O NAMING THE HINT STREAM WAS ACCEPTED, because walkPages treats any resolvable object with no /Kids as a page leaf. ⛔ This is NOT the /Type check deleted in the previous commit returning: requiring /Type /Page would impose a stricter notion of a page than the reader that opens the file; requiring a DICTIONARY imposes nothing, since 7.7.3.3 says a page object is one. The page walker's leniency is its own question. ⛔ THE 32-BIT BOUND WAS CORRECT AND UNTESTED, which review proved by restoring the addition form and watching the whole suite stay green. No test carried a length anywhere near the limit. Two do now, at MaxInt64 and MaxUint32, and they fail on every word size with the addition form. ⛔ AND THE FUZZ SEEDS NEVER REACHED THE CODE THEY WERE WRITTEN FOR: every one said `/L 1` for a file hundreds of bytes long, so Linearized refused them on the length and checkHintStream never ran. The placeholder is ten digits now, so filling it in cannot change the length it measures. 2 579 861 executions before, 12 733 408 after, 345 interesting inputs, no crasher. ⛔ A TAUTOLOGY WAS REMOVED FROM THE FUZZ INVARIANTS: "anything CheckLinearized accepts, Linearized accepts" cannot fail, because the first calls the second. The bound assertions stay, with the reason they are NOT restatements — they re-derive the property from len(b) after the function has committed, which is what catches an arithmetic fault rather than a missing branch. ⛔ THE DIRECTION THAT MATTERS MOST — not refusing what qpdf accepts — is now judged across SHAPES as well as page counts: as qpdf writes it, with --object-streams=generate, and with --compress-streams=n, at 1, 2, 5 and 25 pages. Twelve comparisons, all agreeing. One branch went the other way: `d.Get` after PageRef cannot fail, since walkPages resolved that reference to produce it, and an object that did not read is nil — which the dictionary check already reports. Dead code dressed as thoroughness, and the coverage gate is what said so. gofmt clean, go vet clean, 0 uncovered blocks of 1766. Refs #40. Co-Authored-By: Claude Opus 5 --- fuzz_test.go | 41 +++++--- linearized.go | 113 +++++++++++++++++++- linearized_test.go | 252 +++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 369 insertions(+), 37 deletions(-) diff --git a/fuzz_test.go b/fuzz_test.go index 51ff850..06fa0bb 100644 --- a/fuzz_test.go +++ b/fuzz_test.go @@ -1,6 +1,7 @@ package reader_test import ( + "fmt" "os" "path/filepath" "testing" @@ -194,12 +195,21 @@ func FuzzCheckLinearized(f *testing.F) { // overflows to a negative number on a 32-bit build and admits the file; // this seed is the regression for that, and it is the shape a hostile file // takes rather than one a generator would invent. + // + // ⛔⛔ THE SEED'S /L MUST BE THE SEED'S OWN LENGTH, which review found it + // was not: every one of them said `/L 1` for a file hundreds of bytes + // long, so [reader.Linearized] refused them on the length and the deep + // checks — the ones these seeds were written for — never ran at all. The + // placeholder is TEN DIGITS so that filling it in cannot change the + // length it is measuring. for _, s := range []string{ - "<< /Linearized 1 /L 1 /H [ 2147483647 2147483647 ] /O 1 /E 1 /N 1 /T 1 >>", - "<< /Linearized 1 /L 1 /H [ -1 -1 ] /O -1 /E -1 /N -1 /T -1 >>", - "<< /Linearized 1 /L 1 /H [ 0 0 ] /O 0 /E 0 /N 0 /T 0 >>", + "<< /Linearized 1 /L %010d /H [ 2147483647 2147483647 ] /O 1 /E 1 /N 1 /T 1 >>", + "<< /Linearized 1 /L %010d /H [ -1 -1 ] /O -1 /E -1 /N -1 /T -1 >>", + "<< /Linearized 1 /L %010d /H [ 0 0 ] /O 0 /E 0 /N 0 /T 0 >>", + "<< /Linearized 1 /L %010d /H [ 40 20 ] /O 1 /E 60 /N 1 /T 50 >>", } { - f.Add([]byte("%PDF-1.7\n1 0 obj\n" + s + "\nendobj\n")) + body := "%PDF-1.7\n1 0 obj\n" + s + "\nendobj\n" + f.Add([]byte(fmt.Sprintf(body, len(fmt.Sprintf(body, 0))))) } f.Fuzz(func(t *testing.T, b []byte) { budget(t, 2*time.Second, len(b), "CheckLinearized", func() { @@ -207,16 +217,19 @@ func FuzzCheckLinearized(f *testing.F) { if err != nil { return } - // ⛔ THE INVARIANT, not merely "it did not panic". The deep check - // is strictly stronger than the cheap one, so anything it accepts - // the cheap one must accept too. A disagreement would mean one of - // them reads the front of a file differently from the other, which - // is how a reader and a writer come to disagree about the same - // bytes. - if _, err := reader.Linearized(b); err != nil { - t.Fatalf("CheckLinearized accepted a file Linearized refuses: %v", err) - } - // And every offset it accepted is inside the file it accepted. + // ⛔ A TAUTOLOGY WAS REMOVED FROM HERE. It asserted that anything + // CheckLinearized accepts, Linearized accepts too — which cannot + // fail, because CheckLinearized calls Linearized first and returns + // its error. Review named it; an invariant that cannot fail is a + // line that looks like a gate and is not one. + // + // ⛔ WHAT FOLLOWS IS NOT A RESTATEMENT OF THE CHECKS. The checks + // bound each value as they read it; this re-derives the same + // property at the API boundary, from len(b) alone, AFTER the + // function has committed to an answer. That difference is what + // catches an arithmetic fault rather than a missing branch: with + // the old `HintOffset+HintLength > len(b)` form, a length near + // MaxInt passes the check and fails here. if lin.HintOffset < 0 || lin.HintOffset > len(b) || lin.HintLength < 0 || lin.HintLength > len(b)-lin.HintOffset { t.Fatalf("/H [%d %d] accepted for a file of %d bytes", diff --git a/linearized.go b/linearized.go index 84e5fda..5fb6e78 100644 --- a/linearized.go +++ b/linearized.go @@ -230,6 +230,9 @@ func CheckLinearized(b []byte) (*Linearization, error) { if err := checkHintStream(b, lin); err != nil { return nil, err } + if err := checkOverflowHint(b, lin); err != nil { + return nil, err + } if err := checkMainXref(b, lin); err != nil { return nil, err } @@ -280,6 +283,29 @@ func checkFirstPage(d *Document, lin *Linearization) error { return fmt.Errorf("%w: /O says object %d and the first page is object %d", ErrLinearizationStale, lin.FirstPageObject, ref.Num) } + // ⛔⛔ AND IT MUST BE A DICTIONARY. This is NOT the /Type check deleted + // above, and the difference is the whole argument. Requiring /Type /Page + // would impose a stricter notion of "page" than the reader that opens the + // file, which is a judge disagreeing with its subject. Requiring a + // DICTIONARY imposes nothing extra: 7.7.3.3 says a page object is a + // dictionary, and no stream is one. + // + // Review found this reachable and harmful: a file whose page tree names + // the HINT STREAM as its first kid was accepted here, because walkPages + // treats any resolvable object with no /Kids as a leaf. qpdf reported + // that the page tree includes a non-dictionary object. That leniency in + // the page walker is its own question; what this must not do is repeat it + // while claiming to have checked the file. + // ⛔ The error is not branched on: PageRef returned this reference because + // walkPages had already resolved it, so a failure here is unreachable — + // and an object that did not read is nil, which is not a Dict and is + // reported by the one check below. A separate branch would be dead code + // dressed as thoroughness, which the coverage gate names. + obj, _ := d.Get(ref) + if _, ok := obj.(Dict); !ok { + return fmt.Errorf("%w: /O names object %d, which is %T — a page object is a dictionary", + ErrLinearizationStale, lin.FirstPageObject, obj) + } return nil } @@ -308,7 +334,7 @@ func checkHintStream(b []byte, lin *Linearization) error { // ⛔ No resolver, deliberately: a hint stream whose /Length is an indirect // reference cannot be read by a reader that has only fetched the front of // the file, which is the reader this format exists for. - _, obj, _, err := ParseIndirectObject(b[lin.HintOffset:], nil) + _, obj, n, err := ParseIndirectObject(b[lin.HintOffset:], nil) if err != nil { return fmt.Errorf("%w: /H points at byte %d, where no indirect object parses: %v", ErrLinearizationStale, lin.HintOffset, err) @@ -317,6 +343,66 @@ func checkHintStream(b []byte, lin *Linearization) error { return fmt.Errorf("%w: /H points at byte %d, where the object is %T and not a stream", ErrLinearizationStale, lin.HintOffset, obj) } + // ⛔ AND THE LENGTH IS COMPARED WITH THE OBJECT THAT IS ACTUALLY THERE, + // which review found it was not: the pair was bounds-checked against the + // file and never against the stream it describes, so `/H [ 550 100 ]` for + // a 127-byte object passed while qpdf reported a hint table length + // mismatch. Table F.1 counts the whole object, so this is the parse's own + // extent. + if !spans(n, lin.HintLength) { + return fmt.Errorf("%w: /H says the hint stream is %d bytes and the object at %d is %d", + ErrLinearizationStale, lin.HintLength, lin.HintOffset, n) + } + return nil +} + +// spans says whether a declared length describes an object the parse measured +// as n bytes. +// +// ⛔ IT IS NOT EQUALITY, AND MEASUREMENT IS WHY. Requiring n == declared +// refused every file qpdf produces: the parse ends at `endobj`, while Table +// F.1's length counts the object AND the end-of-line that terminates it. +// Measured on qpdf 12.4.2 output at 1, 2 and 5 pages and on both committed +// fixtures — declared was n+1 in every case. Two bytes are allowed because a +// writer may use CRLF; more than that is a length describing something else. +// +// The asymmetry is deliberate: a declared length SHORTER than the object is +// always wrong, because the object does not fit in the span the file claims. +func spans(n, declared int) bool { + return declared >= n && declared-n <= 2 +} + +// checkOverflowHint checks the second pair of /H, which Table F.1 makes +// optional and which review found was read and never examined: a file +// carrying `/H [ 550 127 9 9 ]` was accepted while qpdf reported that the +// overflow hint table is not a stream. An entry present and unchecked is +// worse than one absent, because the file claims something nothing verified. +func checkOverflowHint(b []byte, lin *Linearization) error { + if !lin.HasOverflowHint { + return nil + } + if lin.OverflowHintOffset <= 0 || lin.OverflowHintOffset >= len(b) { + return fmt.Errorf("%w: the overflow /H starts at %d and the file is %d bytes", + ErrLinearizationStale, lin.OverflowHintOffset, len(b)) + } + if lin.OverflowHintLength <= 0 || + lin.OverflowHintLength > len(b)-lin.OverflowHintOffset { + return fmt.Errorf("%w: the overflow /H is %d bytes from %d and the file is %d", + ErrLinearizationStale, lin.OverflowHintLength, lin.OverflowHintOffset, len(b)) + } + _, obj, n, err := ParseIndirectObject(b[lin.OverflowHintOffset:], nil) + if err != nil { + return fmt.Errorf("%w: the overflow /H points at byte %d, where no indirect object parses: %v", + ErrLinearizationStale, lin.OverflowHintOffset, err) + } + if _, ok := obj.(*Stream); !ok { + return fmt.Errorf("%w: the overflow /H points at byte %d, where the object is %T and not a stream", + ErrLinearizationStale, lin.OverflowHintOffset, obj) + } + if !spans(n, lin.OverflowHintLength) { + return fmt.Errorf("%w: the overflow /H says %d bytes and the object at %d is %d", + ErrLinearizationStale, lin.OverflowHintLength, lin.OverflowHintOffset, n) + } return nil } @@ -422,13 +508,30 @@ func firstBodyDict(b []byte) (Dict, int, error) { // real branch uncovered, which is how I noticed that the example and the // implementation everybody uses disagree about it. func linearizedVersion(v Object) (float64, error) { + var got float64 switch n := v.(type) { case Integer: - return float64(n), nil + got = float64(n) case Real: - return float64(n), nil - } - return 0, fmt.Errorf("%w: /Linearized is %T, want a direct number", ErrLinearizationMalformed, v) + got = float64(n) + default: + return 0, fmt.Errorf("%w: /Linearized is %T, want a direct number", ErrLinearizationMalformed, v) + } + // ⛔⛔ THE VALUE IS CHECKED, AND IT WAS NOT. Review measured the gap: a + // file whose first object says `/Linearized 9` was ACCEPTED here while + // qpdf reported "File is not linearized". /Linearized is the one entry + // that says which format the rest of the dictionary is written in, so a + // version this code has never seen makes every other entry a guess — + // reading /H and /O out of it and reporting the file as sound is the + // checker agreeing with a document it cannot have understood. + // + // 1 and 1.0 are both accepted because the specification's own Example 2 + // writes `/Linearized 1.0` and qpdf writes `/Linearized 1`. + if got != 1 { + return 0, fmt.Errorf("%w: /Linearized is %v, and the only version this reads is 1", + ErrLinearizationMalformed, got) + } + return got, nil } // directInteger reads a required entry that must be a direct integer. diff --git a/linearized_test.go b/linearized_test.go index b5856a2..107acb0 100644 --- a/linearized_test.go +++ b/linearized_test.go @@ -83,6 +83,10 @@ func TestAParameterTheDocumentContradictsIsCaughtOnlyByTheDeepCheck(t *testing.T for _, c := range []struct { name, from, to, says string }{ + {"a hint length shorter than the object it describes", "/H [ 550 127 ]", "/H [ 550 125 ]", + "/H says the hint stream is 125 bytes"}, + {"a hint length of nothing at all", "/H [ 550 127 ]", "/H [ 550 000 ]", "/H is 0 bytes"}, + {"a main cross-reference at the very end", "/T 1169", "/T 1367", "/T is 1367"}, {"a page count the document contradicts", "/N 2", "/N 3", "/N says 3 page(s)"}, {"a document with no pages at all", "/N 2", "/N 0", "a linearized document has at least one"}, {"a first page that is not the first page", "/O 7", "/O 6", "/O says object 6"}, @@ -205,6 +209,197 @@ func TestAFrontThatSaysLinearizedOverABodyThatIsGone(t *testing.T) { } } +// editedFixturePadded applies several edits whose lengths need NOT match, and +// pays the difference out of the run of spaces qpdf leaves between the +// linearization dictionary and the first-page cross-reference section. +// +// ⛔ THE PADDING IS WHERE IT IS FOR A REASON. Everything from `xref` onwards +// keeps its byte offset, so the file still opens and `/L` still equals the +// length — which is what lets these cases reach the deep check at all. A +// naive edit that grew the file would move every object and be refused for +// its cross-reference table instead, measuring the wrong thing. +func editedFixturePadded(t *testing.T, edits ...[2]string) []byte { + t.Helper() + b, err := os.ReadFile(filepath.Join("testdata", "linearized.pdf")) + if err != nil { + t.Fatal(err) + } + s := string(b) + for _, e := range edits { + if strings.Count(s, e[0]) != 1 { + t.Fatalf("%q appears %d times, so this edit is not the one it is named for", + e[0], strings.Count(s, e[0])) + } + s = strings.Replace(s, e[0], e[1], 1) + } + grew := len(s) - len(b) + i := strings.Index(s, "endobj") + if i < 0 { + t.Fatal("the fixture has no first object") + } + j := i + len("endobj") + for j < len(s) && (s[j] == '\n' || s[j] == '\r') { + j++ + } + k := j + for k < len(s) && s[k] == ' ' { + k++ + } + switch { + case grew > 0: + if k-j < grew { + t.Fatalf("the edits need %d bytes of padding and the fixture has %d", grew, k-j) + } + s = s[:j] + s[j+grew:] + case grew < 0: + s = s[:j] + strings.Repeat(" ", -grew) + s[j:] + } + if len(s) != len(b) { + t.Fatalf("compensation failed: %d vs %d", len(s), len(b)) + } + return []byte(s) +} + +// ⛔⛔ THE VERSION IS CHECKED, AND IT WAS NOT — a disagreement with qpdf that +// review measured and that would have shipped. A file whose first object says +// `/Linearized 9` was accepted here while `qpdf --check` reported "File is +// not linearized". +// +// ⛔ The refusal belongs in the CHEAP check, not the deep one: /Linearized is +// the entry that says which format the rest of the dictionary is written in, +// so a version this code has never seen makes every other entry a guess — +// and a reader deciding whether to stream the file consults exactly this. +// That makes it MALFORMED rather than stale. +func TestAVersionThisDoesNotReadIsRefusedByBothChecks(t *testing.T) { + b := editedFixture(t, "/Linearized 1", "/Linearized 9") + for _, c := range []struct { + name string + fn func([]byte) (*Linearization, error) + }{{"Linearized", Linearized}, {"CheckLinearized", CheckLinearized}} { + t.Run(c.name, func(t *testing.T) { + _, err := c.fn(b) + if !errors.Is(err, ErrLinearizationMalformed) { + t.Fatalf("error is %v, want ErrLinearizationMalformed", err) + } + if !strings.Contains(err.Error(), "the only version this reads is 1") { + t.Errorf("it does not say what is wrong: %v", err) + } + }) + } +} + +// And the spelling the specification's own example uses is still accepted. +func TestTheRealSpellingOfTheVersionIsAccepted(t *testing.T) { + b := editedFixturePadded(t, [2]string{"/Linearized 1 ", "/Linearized 1.0 "}) + if _, err := CheckLinearized(b); err != nil { + t.Fatalf("/Linearized 1.0 was refused: %v", err) + } +} + +// ⛔⛔ /O NAMING SOMETHING THAT IS NOT A DICTIONARY, which review found +// accepted. The page tree's first kid is pointed at object 6 — the HINT +// STREAM — and /O follows it. `walkPages` treats any resolvable object with +// no /Kids as a leaf, so the document layer calls that stream "page 1"; qpdf +// reports a page tree containing a non-dictionary object. +// +// This is not the deleted /Type check returning. Requiring /Type /Page would +// impose a stricter notion of a page than the reader; requiring a DICTIONARY +// imposes nothing, because 7.7.3.3 says a page object is one. +func TestAFirstPageThatIsNotADictionaryIsRefused(t *testing.T) { + b := editedFixturePadded(t, + [2]string{"/Kids [ 7 0 R", "/Kids [ 6 0 R"}, + [2]string{"/O 7", "/O 6"}) + if _, err := Linearized(b); err != nil { + t.Fatalf("the cheap check refused it, so this proves nothing: %v", err) + } + _, err := CheckLinearized(b) + if !errors.Is(err, ErrLinearizationStale) { + t.Fatalf("error is %v, want ErrLinearizationStale", err) + } + if !strings.Contains(err.Error(), "a page object is a dictionary") { + t.Errorf("it does not say what is wrong: %v", err) + } +} + +// ⛔ THE OVERFLOW PAIR OF /H WAS READ AND NEVER EXAMINED. Table F.1 makes the +// second pair optional; a file carrying one that points at nothing was +// accepted, while qpdf reported that the overflow hint table is not a stream. +func TestAnOverflowHintThatPointsAtNothingIsRefused(t *testing.T) { + for _, c := range []struct{ name, to, says string }{ + {"at an offset that is not a stream", "/H [ 550 127 9 9 ]", "and not a stream"}, + {"past the end of the file", "/H [ 550 127 1360 99 ]", "and the file is 1367"}, + {"at no offset at all", "/H [ 550 127 0 9 ]", "the overflow /H starts at 0"}, + {"in the middle of another object", "/H [ 550 127 690 50 ]", "no indirect object parses"}, + {"with a length the object contradicts", "/H [ 550 127 550 099 ]", + "the overflow /H says 99 bytes"}, + } { + t.Run(c.name, func(t *testing.T) { + b := editedFixturePadded(t, [2]string{"/H [ 550 127 ]", c.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(), c.says) { + t.Errorf("it does not name the claim that failed: %v", err) + } + }) + } +} + +// And a well-formed overflow pair is ACCEPTED, so the refusals above are +// about the edits rather than about this refusing the entry outright. It +// names the primary hint stream twice, which is not a sensible document but +// is a valid pair: a real stream, at a real offset, of the length it claims. +func TestAWellFormedOverflowHintIsAccepted(t *testing.T) { + b := editedFixturePadded(t, [2]string{"/H [ 550 127 ]", "/H [ 550 127 550 127 ]"}) + lin, err := CheckLinearized(b) + if err != nil { + t.Fatalf("a valid overflow pair was refused: %v", err) + } + if !lin.HasOverflowHint || lin.OverflowHintOffset != 550 { + t.Errorf("the overflow pair was not read back: %+v", lin) + } +} + +// ⛔⛔ THE 32-BIT OVERFLOW, WITH A VALUE THAT OVERFLOWS 64 BITS TOO. The bound +// was written `HintOffset+HintLength > len(b)` and is now a subtraction; +// review measured that the whole suite stayed green with the addition form +// restored, because no test carried a length anywhere near the limit. This is +// that test, and it fails on every word size with the addition form. +func TestAHintLengthNearTheIntegerLimitIsRefused(t *testing.T) { + for _, huge := range []string{"9223372036854775807", "4294967295"} { + t.Run(huge, func(t *testing.T) { + b := editedFixturePadded(t, [2]string{"/H [ 550 127 ]", "/H [ 550 " + huge + " ]"}) + 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("a hint length of %s was accepted: %v", huge, err) + } + }) + } +} + +// /E past the end of the file, which needed more digits than the fixture's +// own value has. +func TestAFirstPageEndingPastTheFileIsRefused(t *testing.T) { + b := editedFixturePadded(t, [2]string{"/E 902", "/E 99999"}) + 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(), "/E is 99999") { + t.Errorf("it does not name the claim: %v", err) + } +} + // And the fixture itself passes the deep check, so the refusals above are // about the edits and not about the checker disliking qpdf's output. func TestCheckLinearizedAcceptsTheCommittedFixture(t *testing.T) { @@ -234,23 +429,44 @@ func TestAnOrdinaryFileIsRefusedByBothChecks(t *testing.T) { } } -// qpdf's output passes the deep check at several page counts, not only at the -// one the committed fixture happens to have. -func TestCheckLinearizedAgreesWithQpdfAcrossPageCounts(t *testing.T) { - for _, pages := range []int{1, 2, 5} { - t.Run(strconv.Itoa(pages)+" pages", func(t *testing.T) { - plain, linear := linearizedFixture(t, pages) - if _, err := CheckLinearized(plain); !errors.Is(err, ErrNotLinearized) { - t.Errorf("the plain file was not refused: %v", err) - } - lin, err := CheckLinearized(linear) - if err != nil { - t.Fatalf("qpdf linearized it and this refused it: %v", err) - } - if lin.PageCount != pages { - t.Errorf("/N %d, want %d", lin.PageCount, pages) - } - }) +// qpdfExtra lets a test ask qpdf for a different kind of linearized file; it +// is reset by each subtest that sets it. +var qpdfExtra []string + +// ⛔⛔ THE DIRECTION THAT MATTERS MOST: this must not REFUSE what qpdf +// accepts, and the checks added after review — a hint length compared with +// the real object, a version, a first page that must be a dictionary — each +// risked exactly that. So qpdf's output is judged across page counts AND +// across the ways it writes a file. +// +// `--compress-streams=n` is here because the hint length is now compared with +// the parsed extent of the stream, which an uncompressed stream changes. +func TestCheckLinearizedAgreesWithQpdfAcrossPageCountsAndShapes(t *testing.T) { + for _, shape := range []struct { + name string + extra []string + }{ + {"as qpdf writes it", nil}, + {"with object streams", []string{"--object-streams=generate"}}, + {"with streams uncompressed", []string{"--compress-streams=n"}}, + } { + for _, pages := range []int{1, 2, 5, 25} { + t.Run(shape.name+", "+strconv.Itoa(pages)+" pages", func(t *testing.T) { + qpdfExtra = shape.extra + defer func() { qpdfExtra = nil }() + plain, linear := linearizedFixture(t, pages) + if _, err := CheckLinearized(plain); !errors.Is(err, ErrNotLinearized) { + t.Errorf("the plain file was not refused: %v", err) + } + lin, err := CheckLinearized(linear) + if err != nil { + t.Fatalf("qpdf linearized it and this refused it: %v", err) + } + if lin.PageCount != pages { + t.Errorf("/N %d, want %d", lin.PageCount, pages) + } + }) + } } } @@ -276,7 +492,7 @@ func linearizedFixture(t *testing.T, pages int) (plain, linear []byte) { if err := os.WriteFile(in, plain, 0o644); err != nil { t.Fatal(err) } - cmd := exec.Command(qpdf, "--linearize", in, out) + cmd := exec.Command(qpdf, append([]string{"--linearize"}, append(qpdfExtra, in, out)...)...) cmd.Stdin = nil if b, err := cmd.CombinedOutput(); err != nil { // qpdf exits 3 for warnings and still writes the file. From 2f6bd025d75d17a8f5b83208e4ea6b65d858a463 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 13 Nov 2026 09:54:45 +0100 Subject: [PATCH 5/7] linearize: read the line ending instead of guessing it, and correct a claim I published MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⛔⛔ THE PREVIOUS COMMIT MESSAGE SAID SOMETHING FALSE, and review measured it. It claimed the two huge-/H tests "fail on every word size with the addition form". With the addition form restored the suite stayed GREEN: `spans` refused those files a few lines later for a different reason, so the tests never reached the bound they are named for. I asserted that from reading the code instead of running the mutation — with cmd/failset, the tool built this session FOR exactly that, already used twice that hour. Third time in one session that a published claim about a mutation was wrong, and the first two were caught by reviewers too. The test names the BOUND'S OWN SENTENCE now, so only the length check can satisfy it. Re-measured: with the addition form, MaxInt64 turns it red and MaxUint32 does NOT — the latter overflows only where int is 32 bits, which is the build the bound exists for and is not the one the tests run on. That is in the test, because a reader would otherwise assume both cases catch it. ⛔⛔ AND `spans` WAS A GUESS WHERE IT COULD BE A READING. It allowed the declared length to exceed the parsed extent by 0..2 bytes, on the reasoning that a writer might use CRLF. Review measured what that costs: /H [ 550 126 ] and /H [ 550 128 ] for a 126-byte object are both ACCEPTED here and both refused by qpdf — "first page object offset mismatch" and "hint table length mismatch". A tolerance wide enough to cover every line ending is wide enough to hide a wrong length, and this file exists to be no weaker than the judge it will be measured against. The end-of-line is READ now (eolAfter), and the declared length must be exactly the object plus it. Both neighbours of the right answer are tests. ⛔ Every spelling of a line ending has its own case — LF, lone CR, CRLF, none at all, past the end of the buffer — because qpdf writes LF and the fixtures would otherwise exercise one branch behind a suite that looks thorough, which is how the window got there to begin with. ⛔ /E IS NOT GATED, AND THAT IS NOW SAID OUT LOUD in the doc comment and the README rather than left to be assumed. qpdf COMPUTES it (on the fixture: "computed = 901..902") and refuses a wrong value; this only bounds it between 1 and the file length, so a writer that miscomputes /E passes here and fails qpdf. Computing it needs the first-page object set — the same partition the hint tables describe — so it waits for that work rather than being approximated. A pass here is not evidence about /E. The twelve qpdf comparisons (1/2/5/25 pages x plain, object-streams, uncompressed-streams) still agree under the exact rule, which is the stronger statement: declared == object + its own end-of-line holds for every file qpdf writes, where the window was only approximating it. gofmt clean, go vet clean, 0 uncovered blocks of 1775. Refs #40. Co-Authored-By: Claude Opus 5 --- README.md | 19 ++++++++++++- linearized.go | 71 ++++++++++++++++++++++++++++++++++------------ linearized_test.go | 71 ++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 136 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 14a3c38..a3faad6 100644 --- a/README.md +++ b/README.md @@ -160,8 +160,25 @@ 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. + +⛔ **The `/H` length is compared with the object exactly**, not within a +window. A first version allowed the declared length to exceed the parsed +extent by 0–2 bytes, reasoning that a writer might use CRLF; that accepts +`/H [ 550 126 ]` and `/H [ 550 128 ]` for a 126-byte object, both of which +qpdf refuses. The end-of-line is now **read** rather than guessed, and every +spelling of one (LF, CR, CRLF, none) has its own test — a tolerance wide +enough to cover every line ending is wide enough to hide a wrong length. + Still not checked, by either: the hint tables (F.4). `qpdf --check` remains the -foreign judge. +foreign judge, and it is run across 1/2/5/25 pages × plain, object-streams and +uncompressed-streams. The positives are **qpdf's** output, not fixtures written to agree with this reading of Annex F. On five real library scans linearized by qpdf and judged by diff --git a/linearized.go b/linearized.go index 5fb6e78..9fa869e 100644 --- a/linearized.go +++ b/linearized.go @@ -206,10 +206,21 @@ func Linearized(b []byte) (*Linearization, error) { // ⛔ It still does NOT validate the hint tables (F.4), which is a larger job // again. What it establishes is that every parameter POINTS AT WHAT IT CLAIMS // TO: the page count is the real one, /O is the real first page, /H begins at -// a real stream object, /T reaches the main cross-reference. A file can pass -// this and still send a reader to the wrong offset inside a hint table — so -// the error says which claim failed, and a caller that needs more must read -// F.4. qpdf's `--check` remains the foreign judge the tests print by. +// a real stream object of the length it declares, /T reaches the main +// cross-reference. A file can pass this and still send a reader to the wrong +// offset inside a hint table — so the error says which claim failed, and a +// caller that needs more must read F.4. qpdf's `--check` remains the foreign +// judge the tests print by. +// +// ⛔⛔ AND /E IS NOT GATED, SAID OUT LOUD BECAUSE A WRITER WILL BE TEMPTED TO +// TRUST 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 /E passes here and fails qpdf. Review +// measured the gap. Computing it needs the first-page object set — the same +// partition the hint tables describe — which is why it waits for that work +// rather than being approximated now. Until then: do not read a pass from +// this as evidence about /E. // // The error wraps the same sentinels [Linearized] uses: a dictionary that // disagrees with the document around it is [ErrLinearizationStale], which is @@ -349,27 +360,51 @@ func checkHintStream(b []byte, lin *Linearization) error { // a 127-byte object passed while qpdf reported a hint table length // mismatch. Table F.1 counts the whole object, so this is the parse's own // extent. - if !spans(n, lin.HintLength) { + if !spans(b, lin.HintOffset, n, lin.HintLength) { return fmt.Errorf("%w: /H says the hint stream is %d bytes and the object at %d is %d", ErrLinearizationStale, lin.HintLength, lin.HintOffset, n) } return nil } -// spans says whether a declared length describes an object the parse measured -// as n bytes. +// spans says whether a declared length describes the object at off, which the +// parse measured as n bytes. +// +// ⛔ IT IS NOT EQUALITY WITH n, AND MEASUREMENT IS WHY. Requiring n == +// declared refused every file qpdf produces: the parse ends at `endobj`, +// while Table F.1's length counts the object AND the end-of-line that +// terminates it. // -// ⛔ IT IS NOT EQUALITY, AND MEASUREMENT IS WHY. Requiring n == declared -// refused every file qpdf produces: the parse ends at `endobj`, while Table -// F.1's length counts the object AND the end-of-line that terminates it. -// Measured on qpdf 12.4.2 output at 1, 2 and 5 pages and on both committed -// fixtures — declared was n+1 in every case. Two bytes are allowed because a -// writer may use CRLF; more than that is a length describing something else. +// ⛔⛔ BUT A WINDOW IS NOT GOOD ENOUGH EITHER, which review measured. A first +// version allowed declared-n in 0..2, reasoning that a writer might use CRLF. +// That accepts /H [ 550 126 ] and /H [ 550 128 ] for a 126-byte object, both +// of which qpdf refuses — "first page object offset mismatch" and "hint table +// length mismatch". A tolerance wide enough to cover every line ending is +// wide enough to hide a wrong length, and this file's whole purpose is to be +// no weaker than the judge it will be measured against. // -// The asymmetry is deliberate: a declared length SHORTER than the object is -// always wrong, because the object does not fit in the span the file claims. -func spans(n, declared int) bool { - return declared >= n && declared-n <= 2 +// So the end-of-line is READ rather than guessed, and the length must be +// exactly the object plus it. +func spans(b []byte, off, n, declared int) bool { + return declared == n+eolAfter(b, off+n) +} + +// eolAfter is the length of the line ending at i: 2 for CRLF, 1 for a lone CR +// or LF, 0 for anything else. +func eolAfter(b []byte, i int) int { + if i >= len(b) { + return 0 + } + if b[i] == '\r' { + if i+1 < len(b) && b[i+1] == '\n' { + return 2 + } + return 1 + } + if b[i] == '\n' { + return 1 + } + return 0 } // checkOverflowHint checks the second pair of /H, which Table F.1 makes @@ -399,7 +434,7 @@ func checkOverflowHint(b []byte, lin *Linearization) error { return fmt.Errorf("%w: the overflow /H points at byte %d, where the object is %T and not a stream", ErrLinearizationStale, lin.OverflowHintOffset, obj) } - if !spans(n, lin.OverflowHintLength) { + if !spans(b, lin.OverflowHintOffset, n, lin.OverflowHintLength) { return fmt.Errorf("%w: the overflow /H says %d bytes and the object at %d is %d", ErrLinearizationStale, lin.OverflowHintLength, lin.OverflowHintOffset, n) } diff --git a/linearized_test.go b/linearized_test.go index 107acb0..025c8e6 100644 --- a/linearized_test.go +++ b/linearized_test.go @@ -85,6 +85,13 @@ func TestAParameterTheDocumentContradictsIsCaughtOnlyByTheDeepCheck(t *testing.T }{ {"a hint length shorter than the object it describes", "/H [ 550 127 ]", "/H [ 550 125 ]", "/H says the hint stream is 125 bytes"}, + // ⛔ THE TWO NEIGHBOURS OF THE RIGHT ANSWER, which a 0..2 window let + // through and qpdf refuses: 126 omits the end-of-line Table F.1 + // counts, 128 claims a byte that belongs to the next object. + {"a hint length one byte short of the terminator", "/H [ 550 127 ]", "/H [ 550 126 ]", + "/H says the hint stream is 126 bytes"}, + {"a hint length one byte past the terminator", "/H [ 550 127 ]", "/H [ 550 128 ]", + "/H says the hint stream is 128 bytes"}, {"a hint length of nothing at all", "/H [ 550 127 ]", "/H [ 550 000 ]", "/H is 0 bytes"}, {"a main cross-reference at the very end", "/T 1169", "/T 1367", "/T is 1367"}, {"a page count the document contradicts", "/N 2", "/N 3", "/N says 3 page(s)"}, @@ -209,6 +216,41 @@ func TestAFrontThatSaysLinearizedOverABodyThatIsGone(t *testing.T) { } } +// ⛔ THE LINE ENDING IS READ, SO EVERY SPELLING OF ONE HAS TO BE. qpdf writes +// LF, so the committed fixtures exercise exactly one branch and the others +// would go untested behind a suite that looks thorough — which is how the +// 0..2 window got there in the first place, guessing at line endings instead +// of reading them. +// +// A PDF may use CR alone (7.2.3 lists CR, LF and CRLF as end-of-line), and an +// object that ends at EOF has no terminator at all. +func TestEverySpellingOfALineEnding(t *testing.T) { + for _, c := range []struct { + name string + tail string + want int + }{ + {"a line feed, which is what qpdf writes", "\n", 1}, + {"a carriage return alone", "\r", 1}, + {"a carriage return and a line feed", "\r\n", 2}, + {"a carriage return at the very end of the file", "\r", 1}, + {"no terminator at all", "", 0}, + {"something that is not a line ending", "x", 0}, + } { + t.Run(c.name, func(t *testing.T) { + b := []byte("endobj" + c.tail) + if got := eolAfter(b, len("endobj")); got != c.want { + t.Errorf("eolAfter(%q) = %d, want %d", c.tail, got, c.want) + } + }) + } + // And past the end of the buffer entirely, which is what an object + // finishing at EOF gives. + if got := eolAfter([]byte("endobj"), 99); got != 0 { + t.Errorf("past the end = %d, want 0", got) + } +} + // editedFixturePadded applies several edits whose lengths need NOT match, and // pays the difference out of the run of spaces qpdf leaves between the // linearization dictionary and the first-page cross-reference section. @@ -364,12 +406,26 @@ func TestAWellFormedOverflowHintIsAccepted(t *testing.T) { } } -// ⛔⛔ THE 32-BIT OVERFLOW, WITH A VALUE THAT OVERFLOWS 64 BITS TOO. The bound -// was written `HintOffset+HintLength > len(b)` and is now a subtraction; -// review measured that the whole suite stayed green with the addition form -// restored, because no test carried a length anywhere near the limit. This is -// that test, and it fails on every word size with the addition form. -func TestAHintLengthNearTheIntegerLimitIsRefused(t *testing.T) { +// ⛔⛔ THE 32-BIT OVERFLOW, AND A CORRECTION I PUBLISHED WRONG. The bound was +// written `HintOffset+HintLength > len(b)` and is now a subtraction, which +// cannot wrap. The previous commit's message claimed these two cases "fail on +// every word size with the addition form". ⛔ THAT WAS FALSE, and review +// measured it: with the addition form restored the suite stayed GREEN, +// because `spans` refuses the file a few lines later for a different reason +// and the test never reached the bound it was named for. I asserted it from +// reading the code instead of running the mutation — with the tool for +// running mutations already built and already used twice that hour. +// +// The fix is for the test to name the BOUND'S OWN SENTENCE. Only the length +// check produces it; under the addition form the overflow slips past that +// check and the refusal comes from `spans` with different words, so the +// assertion goes red exactly as the name promises. +func TestAHintLengthNearTheIntegerLimitIsRefusedByTheBound(t *testing.T) { + // ⛔ AND WHICH CASE CATCHES IT DEPENDS ON THE WORD SIZE, so say so rather + // than let a reader assume both do. Re-measured with the addition form + // restored: MaxInt64 turns this red on a 64-bit build, MaxUint32 does NOT + // — it overflows only where int is 32 bits, which is the build the bound + // exists for and is not the one these run on. for _, huge := range []string{"9223372036854775807", "4294967295"} { t.Run(huge, func(t *testing.T) { b := editedFixturePadded(t, [2]string{"/H [ 550 127 ]", "/H [ 550 " + huge + " ]"}) @@ -380,6 +436,9 @@ func TestAHintLengthNearTheIntegerLimitIsRefused(t *testing.T) { if !errors.Is(err, ErrLinearizationStale) { t.Fatalf("a hint length of %s was accepted: %v", huge, err) } + if !strings.Contains(err.Error(), "/H is "+huge+" bytes from 550") { + t.Errorf("it was refused, but not by the length bound this test is about: %v", err) + } }) } } From 20c31e2b5641d8351dc68fbba24a81d795f2e985 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 13 Nov 2026 10:01:56 +0100 Subject: [PATCH 6/7] linearize: refuse a value this build cannot address, instead of truncating it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ⛔⛔ CI'S 386 AND arm LANES WENT RED, AND THE TEST WAS THE SMALL HALF OF IT. Both 9223372036854775807 and 4294967295 arrive as -1 on a 32-bit build, because `int(Integer)` discards the high half in silence. The assertion written for a 64-bit machine failed on exactly the two lanes that exist to catch this. ⛔ THE REAL HAZARD IS NOT THE VALUE THAT BECOMES -1. A /H length of 2^32+127 truncates to 127 — the fixture's own hint length, a perfectly plausible value that every later bound then accepts. The same bytes would be judged SOUND on a 32-bit build and refused on a 64-bit one, and nothing would say so. The wild values were caught only because they happened to land where a bound objected. Review named this hazard; CI reached it first by another route. Every integer the dictionary offers now goes through `addressable`, which refuses what this build cannot represent rather than letting it wrap — /H included, since both halves are used as a byte range. ⛔ intMax IS A VARIABLE SO THE BRANCH HAS A TEST HERE. On a 64-bit build the refusal is unreachable from any file, so it would be a block no test could enter — reported by the coverage gate and otherwise exercised only in CI, i.e. after pushing. Shrinking it to a 32-bit machine's largest int reproduces that machine's arithmetic on this one, and the 2^32+127 case is the test. ⛔ AND THE WORD SIZE DECIDES WHICH REFUSAL IS CORRECT, which the tests now say instead of assuming. On 32 bits the conversion refuses before any bound is consulted, so asserting that the cheap check ACCEPTS the file — the right precondition on 64 bits — is what turned those lanes red. Both paths are asserted, each where it applies. `GOARCH=386 go vet` is clean; the 32-bit behaviour itself is CI's to confirm, and this workstation cannot run it. gofmt clean, go vet clean, 0 uncovered blocks of 1780. Refs #40. Co-Authored-By: Claude Opus 5 --- linearized.go | 38 +++++++++++++++++++++++-- linearized_test.go | 69 ++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 100 insertions(+), 7 deletions(-) diff --git a/linearized.go b/linearized.go index 9fa869e..c3c8a57 100644 --- a/linearized.go +++ b/linearized.go @@ -579,7 +579,35 @@ func directInteger(d Dict, key Name) (int, error) { if !ok { return 0, fmt.Errorf("%w: /%s is %T, want a direct integer", ErrLinearizationMalformed, key, v) } - return int(n), nil + return addressable(int64(n), key) +} + +// addressable converts a value the file chose into an int, REFUSING one this +// build cannot represent rather than letting it wrap. +// +// ⛔⛔ SILENT TRUNCATION IS THE 32-BIT HAZARD, NOT THE ADDITION. A plain +// `int(n)` discards the high half where int is 32 bits, so a /H length of +// 2^32+127 reads as 127 — a perfectly plausible value that every later bound +// then accepts. The file would be judged sound on a 32-bit build and refused +// on a 64-bit one, from the same bytes. +// +// Review named this hazard; CI proved it a different way first, with the 386 +// and arm lanes failing because 9223372036854775807 and 4294967295 BOTH +// arrive as -1. That is the same truncation, caught only because the value +// happened to land somewhere a bound rejected. +// ⛔ intMax is a VARIABLE so that a test can shrink it. The refusal below is +// unreachable on a 64-bit build — every int64 fits — so on this workstation +// it would be a branch no test could enter, which the coverage gate reports +// and which is otherwise only exercised by the 386 and arm lanes in CI. A +// seam is how the 32-bit case gets a test where 32-bit machines are not. +var intMax = int64(^uint(0) >> 1) + +func addressable(v int64, key Name) (int, error) { + if v > intMax || v < -intMax-1 { + return 0, fmt.Errorf("%w: /%s is %d, which this build cannot address "+ + "(its largest int is %d)", ErrLinearizationMalformed, key, v, intMax) + } + return int(v), nil } // readHintArray reads /H, "an array of two or four integers". @@ -601,7 +629,13 @@ func readHintArray(d Dict, lin *Linearization) error { if !ok { return fmt.Errorf("%w: /H[%d] is %T, want a direct integer", ErrLinearizationMalformed, i, e) } - n[i] = int(v) + // ⛔ The same refusal as every other entry: /H is where the hostile + // value actually arrives, since both halves are used as a byte range. + got, err := addressable(int64(v), Name(fmt.Sprintf("H[%d]", i))) + if err != nil { + return err + } + n[i] = got } lin.HintOffset, lin.HintLength = n[0], n[1] if len(n) == 4 { diff --git a/linearized_test.go b/linearized_test.go index 025c8e6..d0e1e90 100644 --- a/linearized_test.go +++ b/linearized_test.go @@ -429,20 +429,79 @@ func TestAHintLengthNearTheIntegerLimitIsRefusedByTheBound(t *testing.T) { for _, huge := range []string{"9223372036854775807", "4294967295"} { t.Run(huge, func(t *testing.T) { b := editedFixturePadded(t, [2]string{"/H [ 550 127 ]", "/H [ 550 " + huge + " ]"}) - if _, err := Linearized(b); err != nil { - t.Fatalf("the cheap check refused it: %v", err) + // ⛔ ON A 32-BIT BUILD THE CHEAP CHECK REFUSES IT FIRST, AND THAT + // IS CORRECT: the value cannot be converted at all, so no bound is + // ever consulted. Asserting that the cheap check accepts — which + // is the right precondition on a 64-bit build — is what made this + // test fail on the 386 and arm lanes. + cheap := func() error { _, e := Linearized(b); return e }() + if strconv.IntSize == 32 { + if !errors.Is(cheap, ErrLinearizationMalformed) { + t.Fatalf("a value this build cannot address was read anyway: %v", cheap) + } + if !strings.Contains(cheap.Error(), "cannot address") { + t.Errorf("it does not say why: %v", cheap) + } + return + } + if cheap != nil { + t.Fatalf("the cheap check refused it: %v", cheap) } _, err := CheckLinearized(b) + if err == nil { + t.Fatalf("a hint length of %s was accepted", huge) + } + // ⛔⛔ WHICH REFUSAL IS CORRECT DEPENDS ON THE WORD SIZE, and + // saying "either" would be a test that cannot fail. CI found this + // the hard way: both values arrive as -1 on 386 and arm, because + // the conversion truncated, so the assertion written for a 64-bit + // build failed on the two lanes that exist to catch exactly this. + want := "/H is " + huge + " bytes from 550" if !errors.Is(err, ErrLinearizationStale) { - t.Fatalf("a hint length of %s was accepted: %v", huge, err) + t.Fatalf("error is %v, want it to wrap ErrLinearizationStale", err) } - if !strings.Contains(err.Error(), "/H is "+huge+" bytes from 550") { - t.Errorf("it was refused, but not by the length bound this test is about: %v", err) + if !strings.Contains(err.Error(), want) { + t.Errorf("it was refused, but not for the reason this test is about: %v", err) } }) } } +// ⛔⛔ THE TRUNCATION ITSELF, WHICH IS THE REAL 32-BIT HAZARD AND IS INVISIBLE +// ON THIS WORKSTATION. 2^32+127 read through a plain `int(n)` on a 32-bit +// build is 127 — not a wild value a bound would catch, but a perfectly +// plausible hint length that every later check then accepts. The same bytes +// would be judged sound on one machine and refused on another. +// +// The conversion refuses what it cannot represent now, so both builds answer; +// they answer differently, and both answers are right. +func TestAHintLengthThatWrapsOnA32BitBuildIsRefused(t *testing.T) { + // ⛔ THE SEAM IS WHAT MAKES THIS A TEST AND NOT A HOPE. On a 64-bit build + // every int64 fits an int, so the refusal cannot be reached from any + // file; shrinking intMax to a 32-bit machine's largest int reproduces + // that machine's arithmetic here. Without it the branch is exercised only + // by CI's 386 and arm lanes, i.e. after pushing. + old := intMax + t.Cleanup(func() { intMax = old }) + intMax = 1<<31 - 1 + + // 4294967423 == 2^32 + 127, and 127 is the fixture's own hint length — so + // a plain int() conversion would read it as a PLAUSIBLE value that every + // later bound accepts, not as a wild one something would catch. + b := editedFixturePadded(t, [2]string{"/H [ 550 127 ]", "/H [ 550 4294967423 ]"}) + + _, err := Linearized(b) + if !errors.Is(err, ErrLinearizationMalformed) { + t.Fatalf("2^32+127 was read as a hint length on a 32-bit build: %v", err) + } + if !strings.Contains(err.Error(), "cannot address") { + t.Errorf("it does not say why: %v", err) + } + if _, err := CheckLinearized(b); !errors.Is(err, ErrLinearizationMalformed) { + t.Errorf("the deep check accepted it: %v", err) + } +} + // /E past the end of the file, which needed more digits than the fixture's // own value has. func TestAFirstPageEndingPastTheFileIsRefused(t *testing.T) { From 5d730a01564561aa42bfb8486db40d023222613b Mon Sep 17 00:00:00 2001 From: tannevaled Date: Fri, 13 Nov 2026 17:41:24 +0100 Subject: [PATCH 7/7] linearize: the refusal was written once and tested in one place only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third review. CI is green at 20c31e2 on all ten lanes, so nothing here is a fix for a red build — all four findings are tests that could not have failed, which is the harder kind to notice and the reason the review was asked for. ⛔⛔ addressable WAS REACHED ONLY THROUGH /H. Review measured `int(int32(n))` in directInteger — the conversion for /L, /O, /E, /N and /T — and the suite stayed GREEN. The seam test shrinks intMax and then edits /H, so every other entry's refusal was written and never exercised. /H is where a hostile value is most likely to arrive, which is why it was written first; that is not a reason for it to be the only one a test can see. ⛔ THE LOWER HALF OF THAT REFUSAL HAD NEVER BEEN REACHED BY ANY FILE. `v < -intMax-1` was written from symmetry with the upper bound, not from a case, and deleting it left the suite green. ⛔⛔ THE OVERFLOW PAIR'S LENGTH BOUND WAS HELD BY NOTHING — the round-2 vacuity, surviving in the half the fix was never copied into. Rewriting it as `off+len > len(b)` left the suite green, because the file is still refused one step later by spans, for a different reason. A bound subsumed by a later check is a bound no test is holding, and a test that asserts only "an error" cannot tell the two refusals apart. The new test asserts the SENTENCE. ⛔ AND /P WAS THE ONE ENTRY STILL CONVERTING DIRECTLY. It is never used as an offset, which is why it was missed and is not a reason to leave it: it is a public field, so a 32-bit build would hand a caller 0 for 2^32 from the same bytes a 64-bit build reads correctly, with nothing to say which it got. Mutations re-run for this commit, judged by cmd/failset on the failing test NAMES rather than on a count, each red on its own test and no other: ovf bound -> addition form -> TestAnOverflowHintLengthNearTheIntegerLimit… directInteger int(int32(n)) -> TestAnEntryThatWrapsOnA32BitBuildIsRefused…, TestAValueBelowTheSmallestIntThisBuildHas… drop `v < -intMax-1` -> TestAValueBelowTheSmallestIntThisBuildHas… /P back to int(n) -> TestAFirstPageNumberThatWrapsOnA32Bit… Review also confirmed the seam is safe: t.Cleanup is registered before the shrink, so the restore is unconditional, and nothing in the package calls t.Parallel. /E stays documented as NOT gated. Review found it is now cheap to close for table files — obj 9 at 871 gives 871+n+eolAfter == 902, which equals /E — but the xref-stream shape needs more, and that is its own change rather than a rider on this one. gofmt clean, go vet clean, GOARCH=386 go vet clean, 0 uncovered blocks of 1782. Refs #40. Co-Authored-By: Claude Opus 5 --- linearized.go | 10 ++++- linearized_test.go | 95 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/linearized.go b/linearized.go index c3c8a57..c048f0a 100644 --- a/linearized.go +++ b/linearized.go @@ -163,7 +163,15 @@ func Linearized(b []byte) (*Linearization, error) { if !ok { return nil, fmt.Errorf("%w: /P is %T, want an integer", ErrLinearizationMalformed, v) } - lin.FirstPageNumber = int(n) + // ⛔ /P is never used as an offset, which is exactly why it was the one + // entry left converting directly — and not a reason to leave it. It is + // a public field, so a 32-bit build would hand a caller 0 for 2^32 from + // the same bytes a 64-bit build reads correctly. + got, err := addressable(int64(n), "P") + if err != nil { + return nil, err + } + lin.FirstPageNumber = got } if err := readHintArray(dict, lin); err != nil { return nil, err diff --git a/linearized_test.go b/linearized_test.go index d0e1e90..90ecbaf 100644 --- a/linearized_test.go +++ b/linearized_test.go @@ -502,6 +502,101 @@ func TestAHintLengthThatWrapsOnA32BitBuildIsRefused(t *testing.T) { } } +// ⛔⛔ THE OVERFLOW PAIR'S LENGTH BOUND WAS HELD BY NOTHING, which review +// measured rather than read: rewriting it in the addition form `off+len > +// len(b)` left the whole suite GREEN, because `spans` refuses the file one +// step later for a different reason. A bound subsumed by a later check is a +// bound no test is holding — the same vacuity the primary pair had in round 2, +// surviving in the half nobody had copied the fix into. +func TestAnOverflowHintLengthNearTheIntegerLimitIsRefusedByItsOwnBound(t *testing.T) { + // The smallest length that makes `550 + len` overflow int64, so the + // addition form walks straight past the bound instead of refusing. + const huge = "9223372036854775807" + b := editedFixturePadded(t, [2]string{"/H [ 550 127 ]", "/H [ 550 127 550 " + huge + " ]"}) + + cheap := func() error { _, e := Linearized(b); return e }() + if strconv.IntSize == 32 { + // As for the primary pair: the conversion refuses before any bound is + // consulted, and that is the correct answer on this build. + if !errors.Is(cheap, ErrLinearizationMalformed) { + t.Fatalf("a value this build cannot address was read anyway: %v", cheap) + } + return + } + if cheap != nil { + t.Fatalf("the cheap check refused it: %v", cheap) + } + _, err := CheckLinearized(b) + if !errors.Is(err, ErrLinearizationStale) { + t.Fatalf("error is %v, want it to wrap ErrLinearizationStale", err) + } + // ⛔ THE MESSAGE IS THE ASSERTION, not merely that something was refused. + // Under the addition form this file is still refused — by spans, naming a + // different reason — so a test asserting only "an error" passes either way, + // which is exactly how that mutation survived. + want := "the overflow /H is " + huge + " bytes from 550" + if !strings.Contains(err.Error(), want) { + t.Errorf("it was refused, but not by the length bound this test is about: %v", err) + } +} + +// ⛔⛔ addressable WAS REACHED ONLY THROUGH /H, so every other entry's refusal +// was untested: review measured `int(int32(n))` in directInteger — /L, /O, /E, +// /N and /T — leaving the suite green. /H is where a hostile value is most +// likely to arrive, which is why it was written first; it is not where a +// truncation is least harmful. +func TestAnEntryThatWrapsOnA32BitBuildIsRefusedForEveryKeyNotOnlyH(t *testing.T) { + old := intMax + t.Cleanup(func() { intMax = old }) + intMax = 1<<31 - 1 + + b := editedFixturePadded(t, [2]string{"/E 902", "/E 4294967296"}) + _, err := Linearized(b) + if !errors.Is(err, ErrLinearizationMalformed) { + t.Fatalf("2^32 was read as /E on a 32-bit build: %v", err) + } + if !strings.Contains(err.Error(), "/E is 4294967296") { + t.Errorf("it does not name the entry it refused: %v", err) + } +} + +// ⛔ THE LOWER HALF OF THE SAME REFUSAL, which no file had ever reached. A +// negative past -intMax-1 truncates exactly as a positive past intMax does, +// and `v < -intMax-1` was written from symmetry rather than from a test. +func TestAValueBelowTheSmallestIntThisBuildHasIsRefused(t *testing.T) { + old := intMax + t.Cleanup(func() { intMax = old }) + intMax = 1<<31 - 1 + + b := editedFixturePadded(t, [2]string{"/E 902", "/E -4294967296"}) + _, err := Linearized(b) + if !errors.Is(err, ErrLinearizationMalformed) { + t.Fatalf("-2^32 was read as /E on a 32-bit build: %v", err) + } + if !strings.Contains(err.Error(), "cannot address") { + t.Errorf("it does not say why: %v", err) + } +} + +// ⛔ /P IS NEVER USED AS AN OFFSET, WHICH IS WHY IT WAS MISSED AND NOT A +// REASON TO LEAVE IT. It is a public field: a 32-bit build would hand a caller +// 0 for 2^32 from the same bytes a 64-bit build reads correctly, and the +// caller has no way to know which it got. +func TestAFirstPageNumberThatWrapsOnA32BitBuildIsRefused(t *testing.T) { + old := intMax + t.Cleanup(func() { intMax = old }) + intMax = 1<<31 - 1 + + b := editedFixturePadded(t, [2]string{"/E 902", "/E 902 /P 4294967296"}) + _, err := Linearized(b) + if !errors.Is(err, ErrLinearizationMalformed) { + t.Fatalf("2^32 was read as /P on a 32-bit build: %v", err) + } + if !strings.Contains(err.Error(), "/P is 4294967296") { + t.Errorf("it does not name the entry it refused: %v", err) + } +} + // /E past the end of the file, which needed more digits than the fixture's // own value has. func TestAFirstPageEndingPastTheFileIsRefused(t *testing.T) {