diff --git a/README.md b/README.md index ca58d9d..ffb5f2c 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,7 @@ judgement a writer needs on bytes it has just produced. It establishes that every parameter **points at what it claims to** — `/N` is the real page count, `/O` is the real first page *and* is a `/Page`, `/H` begins at a real stream object, `/T` reaches the main cross-reference (either shape: Table F.1 gives -it two meanings and both are real), `/E` is inside the file. +it two meanings and both are real), `/E` is where the first page really ends. What gates it are files `Linearized` **accepts** and `CheckLinearized` refuses: the committed qpdf fixture with one integer @@ -160,13 +160,56 @@ different document. The replacement's width is asserted by the helper, because an edit one byte longer would be refused for the length instead and the deep check would never run. -⛔⛔ **`/E` is NOT gated, and a writer must not read a pass here as evidence -about it.** Table F.1 calls `/E` the offset of the end of the first page, and -qpdf *computes* it — on the committed fixture it reports `computed = 901..902` -and refuses a wrong value. This only bounds `/E` between 1 and the file -length, so a writer that miscomputes it passes here and fails qpdf. Computing -it needs the first-page object set, which is the same partition the hint -tables describe, so it waits for that work rather than being approximated now. +⛔ **`/E` is checked against the objects the file says are the first page's, +not merely against the file's length.** Table F.1 calls `/E` the offset of the +end of the first page. This used to bound it only between 1 and the file +length, on the reasoning that the first-page partition "is the hint tables' +job" — which was true of the hint tables and false of the **first-page +cross-reference section**, which Annex F puts at the front and the file's own +`startxref` points at. Its entries *are* the first page's objects; that is the +whole trick of the format. So `/E` must equal the end of the last of them, and +a writer that is one byte out is now told so instead of passing. + +⛔⛔ **The hint stream is excluded, and a second producer is why that is not a +guess.** F.3.1 permits the primary hint stream on either side of the first +page, so the highest-offset entry in that section is not always a first-page +object: + +| producer | `/H` offset | `/E` | hint stream sits | +|---|---|---|---| +| qpdf | 550 | 902 | **before** the first page | +| Ghostscript `-dFastWebView` | 829 | 829 | **after** it, at `/E` exactly | + +On Ghostscript's file the naive maximum *is* the hint stream, ending at 967 +against an `/E` of 829 — so a rule that took it would refuse every file that +tool writes. Both qpdf fixtures place it before and **cannot tell the two +rules apart**, which is why `testdata/linearized-gs.pdf` is committed beside +them: removing the exclusion turns exactly that test red and leaves the qpdf +ones green. A second producer measures what the first cannot. + +⛔⛔ **What this proves is that `/E` agrees with the first-page section — not +that the section is right.** A writer that leaves objects page one genuinely +needs out of that section produces a file that is internally consistent and +passes here. Ghostscript does exactly this on a document with shared fonts, +outlines and links: it writes `/H` = `/E` = 1941, this checker accepts it, and +`qpdf --check` computes `5500..5501` and reports the first page as having 9 +objects where the hint table claims 4. Deciding which objects page one *needs* +means walking its resources, which is the partition the hint tables describe — +so that judgement is still outstanding, and a pass here is not evidence about +it. + +⚠ **Stricter than qpdf, deliberately.** qpdf computes `/E` as a *range* +(`901..902` on the committed fixture), accepting the end with or without the +line ending after `endobj`. This requires the end including it, which is what +both measured producers actually write. A file carrying the lower value would +pass `qpdf --check` and be refused here; none has been observed, and the +alternative is a window that cannot tell a correct `/E` from one a byte short. + +⚠ The **overflow** hint stream is excluded by the same reasoning and no file +can reach it — it needs a four-entry `/H` whose second pair is both a real +stream and the highest-offset object, and neither producer writes one. So the +question is asked of a function rather than inline, where four plain cases can +put it and a mutation can kill it. ⛔⛔ **A value this build cannot address is refused, not truncated**, and the same bytes therefore get the same answer on a 32-bit build as on a 64-bit one. diff --git a/linearized.go b/linearized.go index c048f0a..a30a941 100644 --- a/linearized.go +++ b/linearized.go @@ -255,17 +255,142 @@ func CheckLinearized(b []byte) (*Linearization, error) { if err := checkMainXref(b, lin); err != nil { return nil, err } - // Table F.1: /E is "the offset of the end of the first page". Nothing here - // knows which objects the first page owns — that is the hint tables' job — - // so what is checked is the one thing that is checkable without them: an - // offset past the end of the file sends a reader nowhere. - if lin.FirstPageEnd <= 0 || lin.FirstPageEnd > len(b) { - return nil, fmt.Errorf("%w: /E is %d and the file is %d bytes", - ErrLinearizationStale, lin.FirstPageEnd, len(b)) + if err := checkFirstPageEnd(b, lin); err != nil { + return nil, err } return lin, nil } +// isHintObject says whether an object at this offset is one of the hint +// streams /H names, and so is not a first-page object whose end could be /E. +// +// ⛔⛔ IT IS A FUNCTION SO THAT THE SECOND HALF CAN BE TESTED AT ALL. The +// primary hint stream is reachable from a committed file — Ghostscript puts +// it at /E — but the OVERFLOW stream is not: it needs a four-entry /H whose +// second pair is both a real stream and the highest-offset object in the +// first-page section, and neither qpdf nor Ghostscript writes one. Left +// inline, that condition would be a branch no fixture could reach and no +// mutation could kill, which is the vacuity this file has already spent +// three review rounds removing. Asked directly it is four plain cases. +func isHintObject(lin *Linearization, off int64) bool { + if off == int64(lin.HintOffset) { + return true + } + return lin.HasOverflowHint && off == int64(lin.OverflowHintOffset) +} + +// checkFirstPageEnd checks /E, Table F.1's "offset of the end of the first +// page", against the objects the file itself says belong to the first page. +// +// ⛔ THE FIRST-PAGE CROSS-REFERENCE SECTION IS THE ANSWER, and it costs +// nothing to read. Annex F puts it at the front (part 3) and the file's own +// `startxref` points at it, which is the whole trick of the format: a reader +// fetches the front of the file and has page one. So its entries ARE the +// first page's objects, and no hint table has to be decoded to learn that. +// This checker used to say the partition "is the hint tables' job" and bound +// /E only to the file's length; that was true of the hint tables and not of +// this section. +// +// ⛔⛔ THE HINT STREAM IS EXCLUDED, AND A SECOND PRODUCER IS WHY THAT IS NOT +// A GUESS. F.3.1 permits the primary hint stream on either side of the first +// page, so the highest-offset entry here is not always a first-page object: +// +// qpdf /H at 550, /E 902 — the hint stream comes BEFORE +// Ghostscript /H at 829, /E 829 — it comes AFTER, AT /E exactly +// +// On Ghostscript's file the naive maximum is the hint stream itself, ending +// at 967 against an /E of 829, so a rule that took it would refuse every +// file that tool writes. Both committed qpdf fixtures place the hint stream +// before the first page and CANNOT tell the two rules apart — which is why +// testdata/linearized-gs.pdf is committed beside them. +// +// ⛔⛔ AND WHAT THIS PROVES IS THAT /E AGREES WITH THAT SECTION, NOT THAT THE +// SECTION IS RIGHT. A writer that leaves out objects page one genuinely needs +// produces a file that is internally consistent and passes here. Review +// measured Ghostscript doing it on a document with shared fonts, outlines and +// links: /H = /E = 1941, accepted here, while qpdf computes 5500..5501 and +// reports page one as 9 objects where the hint table claims 4. Deciding what +// page one NEEDS means walking its resources — the partition the hint tables +// describe — and that judgement is still outstanding (#40). +func checkFirstPageEnd(b []byte, lin *Linearization) error { + // ⛔ A SEPARATE Document, not the one CheckLinearized opened. Open walks + // /Prev and merges every section into one table, and setEntry keeps the + // first definition it saw, so by then the front section's entries are no + // longer distinguishable from the rest of the file's. Reading that one + // section alone is the only way to ask which objects it listed. + front := &Document{ + buf: b, + xref: map[int]xrefEntry{}, + cache: map[int]Object{}, + loading: map[int]bool{}, + objStms: map[int]map[int]Object{}, + } + off, err := front.findStartxref() + if err != nil { + return fmt.Errorf("%w: the first-page cross-reference section cannot be found: %v", + ErrLinearizationStale, err) + } + // ⛔⛔ THE SECTION startxref NAMES IS NOT ALWAYS THE FIRST-PAGE ONE, and + // an earlier draft of this claimed /L had already made it so. It has not: + // an incremental update appends its own section and points startxref at + // THAT, and a file whose /L was rewritten to match its new length passes + // every earlier check. Following it then lands on the update's entries and + // compares /E with an object that has nothing to do with the first page — + // a refusal with a reason that is not the reason, which is worse than a + // miss because a reader believes it. + // + // Annex F puts the first-page section at part 3 and the main one at part + // 10, so the front section precedes /T. checkMainXref has already + // established /T against the file by the time this runs, and this says + // nothing about /E, so it still holds when /E is the thing under test. + if off >= int64(lin.MainXrefOffset) { + return fmt.Errorf("%w: the cross-reference section at byte %d is not the first-page one — "+ + "it is at or past the main table /T names (%d), so the file has been written to since it was linearized", + ErrLinearizationStale, off, lin.MainXrefOffset) + } + if _, err := front.readXrefSection(off); err != nil { + return fmt.Errorf("%w: the first-page cross-reference section at byte %d does not read: %v", + ErrLinearizationStale, off, err) + } + + // ⛔ THE COMPARISON STAYS IN int64 UNTIL THE VALUE IS KNOWN TO FIT. These + // offsets come from the file, and int(e.offset) is the same silent + // truncation `addressable` exists to refuse everywhere else. + last := int64(-1) + for _, e := range front.xref { + // An object inside an object stream has no offset of its own; its + // container has one and is listed here too, so skipping it loses + // nothing. + if e.kind != 'n' || isHintObject(lin, e.offset) { + continue + } + if e.offset > last { + last = e.offset + } + } + if last < 0 { + return fmt.Errorf("%w: the first-page cross-reference section lists no object in the file", + ErrLinearizationStale) + } + if last >= int64(len(b)) { + return fmt.Errorf("%w: the first-page cross-reference section puts an object at byte %d and the file is %d bytes", + ErrLinearizationStale, last, len(b)) + } + at := int(last) + _, _, n, err := ParseIndirectObject(b[at:], nil) + if err != nil { + return fmt.Errorf("%w: the last first-page object, at byte %d, does not parse: %v", + ErrLinearizationStale, at, err) + } + // The same end-of-object rule the /H length uses: Table F.1 counts the + // line ending after `endobj` and the parse stops at it. + if end := at + n + eolAfter(b, at+n); lin.FirstPageEnd != end { + return fmt.Errorf("%w: /E says %d and the first page ends at %d", + ErrLinearizationStale, lin.FirstPageEnd, end) + } + return nil +} + // checkFirstPage checks /N and /O against the document's own page tree. // // ⛔⛔ THERE IS NO "AND IS IT REALLY A PAGE?" CHECK, AND ITS ABSENCE IS THE diff --git a/linearized_test.go b/linearized_test.go index 90ecbaf..60ec460 100644 --- a/linearized_test.go +++ b/linearized_test.go @@ -8,6 +8,7 @@ package reader import ( "bytes" "errors" + "fmt" "os" "os/exec" "path/filepath" @@ -102,7 +103,7 @@ func TestAParameterTheDocumentContradictsIsCaughtOnlyByTheDeepCheck(t *testing.T {"a hint stream running off the end", "/H [ 550 127 ]", "/H [ 550 999 ]", "/H is 999 bytes from 550"}, {"a hint offset pointing at something that is not a stream", "/H [ 550 127 ]", "/H [ 501 127 ]", "not a stream"}, - {"a first page that ends nowhere", "/E 902", "/E 000", "/E is 0"}, + {"a first page that ends nowhere", "/E 902", "/E 000", "/E says 0 and the first page ends at 902"}, {"a main cross-reference that is not there", "/T 1169", "/T 1160", "/T is 1160"}, {"a main cross-reference at no offset at all", "/T 1169", "/T 0000", "/T is 0"}, // ⛔ One byte from the end: the free-entry test has to notice that the @@ -608,8 +609,210 @@ func TestAFirstPageEndingPastTheFileIsRefused(t *testing.T) { if !errors.Is(err, ErrLinearizationStale) { t.Fatalf("error is %v, want ErrLinearizationStale", err) } - if !strings.Contains(err.Error(), "/E is 99999") { - t.Errorf("it does not name the claim: %v", err) + // ⛔ THE MESSAGE CHANGED WITH THE CHECK, AND SO DID WHAT IT PROVES. This + // used to be refused for being past the end of the file, which is the + // weakest thing wrong with it. It is now refused for disagreeing with the + // objects the file's own first-page section lists, and names both numbers. + if !strings.Contains(err.Error(), "/E says 99999 and the first page ends at 902") { + t.Errorf("it does not name the claim and what contradicts it: %v", err) + } +} + +// ⛔⛔ /E OFF BY ONE IN EITHER DIRECTION, which is what a writer that +// miscomputes it actually produces — not 99999. Neither is past the end of +// the file, so the bound this check replaced accepted both. +func TestAFirstPageEndingOffByOneIsRefused(t *testing.T) { + for _, to := range []string{"/E 901", "/E 903"} { + t.Run(to, func(t *testing.T) { + b := editedFixturePadded(t, [2]string{"/E 902", to}) + if _, err := Linearized(b); err != nil { + t.Fatalf("the cheap check refused it: %v", err) + } + _, err := CheckLinearized(b) + if !errors.Is(err, ErrLinearizationStale) { + t.Fatalf("error is %v, want ErrLinearizationStale", err) + } + if !strings.Contains(err.Error(), "the first page ends at 902") { + t.Errorf("it does not say where the first page really ends: %v", err) + } + }) + } +} + +// ⛔⛔ THE FIXTURE THAT MAKES THE /H EXCLUSION A TEST RATHER THAN A HOPE. +// Ghostscript's -dFastWebView writes the primary hint stream AFTER the first +// page, at exactly /E — where qpdf writes it before — so it is the +// highest-offset entry in the first-page section. Take it and this file is +// refused although it is sound: the naive maximum ends at 967 against an /E +// of 829. +// +// Both qpdf fixtures place it before and cannot distinguish the two rules, +// which is the whole reason a second producer is committed here. Measured +// with Ghostscript 10.08.0. +func TestTheHintStreamIsNotTheEndOfTheFirstPage(t *testing.T) { + b, err := os.ReadFile(filepath.Join("testdata", "linearized-gs.pdf")) + if err != nil { + t.Fatal(err) + } + lin, err := CheckLinearized(b) + if err != nil { + t.Fatalf("a Ghostscript-linearized file was refused: %v", err) + } + // The precondition the test rests on: if a future fixture moved the hint + // stream before the first page, this test would pass while proving + // nothing, which is the failure mode it exists to prevent. + if lin.HintOffset != lin.FirstPageEnd { + t.Fatalf("this fixture no longer places /H at /E (/H=%d, /E=%d), so it no longer "+ + "distinguishes the naive rule from the one with the exclusion", + lin.HintOffset, lin.FirstPageEnd) + } +} + +// ⛔⛔ THE OVERFLOW HALF OF THE HINT EXCLUSION, WHICH NO FILE CAN REACH. It +// needs a four-entry /H whose second pair is both a real stream and the +// highest-offset object of the first-page section, and neither producer +// writes one. Asked directly it is four cases, and each kills a mutation: +// dropping the HasOverflowHint guard turns "an overflow offset that is not +// declared" red, and replacing the second return with false turns "the +// overflow hint stream" red. +func TestOnlyTheStreamsSlashHNamesAreExcludedFromTheFirstPage(t *testing.T) { + for _, c := range []struct { + name string + lin Linearization + off int64 + want bool + }{ + {"the primary hint stream", Linearization{HintOffset: 550}, 550, true}, + {"an ordinary first-page object", Linearization{HintOffset: 550}, 871, false}, + {"the overflow hint stream", Linearization{ + HintOffset: 550, HasOverflowHint: true, OverflowHintOffset: 900}, 900, true}, + {"an overflow offset that is not declared", Linearization{ + HintOffset: 550, OverflowHintOffset: 900}, 900, false}, + } { + t.Run(c.name, func(t *testing.T) { + lin := c.lin + if got := isHintObject(&lin, c.off); got != c.want { + t.Errorf("isHintObject(%d) = %v, want %v", c.off, got, c.want) + } + }) + } +} + +// ⛔ THE WAYS THE FIRST-PAGE SECTION CAN FAIL TO ANSWER, each reached by a +// same-width edit so the file still opens and /L still matches. A reader that +// cannot find or read that section cannot judge /E, and saying so beats +// returning nil and calling the file sound. +func TestAFirstPageSectionThatCannotAnswerIsRefused(t *testing.T) { + for _, c := range []struct{ name, from, to, want string }{ + {"a startxref that names no offset", "startxref\n216", "startxref\n/Na", + "cannot be found"}, + {"a startxref pointing at no section", "startxref\n216", "startxref\n217", + "does not read"}, + {"an entry past the end of the file", "0000000871 00000 n", "0009999871 00000 n", + "and the file is 1367 bytes"}, + {"an entry pointing at no object", "0000000871 00000 n", "0000000860 00000 n", + "does not parse"}, + } { + t.Run(c.name, func(t *testing.T) { + b := editedFixturePadded(t, [2]string{c.from, c.to}) + _, err := CheckLinearized(b) + if err == nil { + t.Fatal("it was accepted") + } + if !errors.Is(err, ErrLinearizationStale) { + t.Fatalf("error is %v, want ErrLinearizationStale", err) + } + if !strings.Contains(err.Error(), c.want) { + t.Errorf("it was refused, but not for the reason this case is about: %v", err) + } + }) + } +} + +// ⛔⛔ AN ENTRY AT EXACTLY len(b), which is the boundary the length guard is +// about and the only input that tells `>=` from `>`. Past the end the parse +// would fail anyway and the guard would be a bound nothing is holding — +// review measured that mutation staying green — so the case that pins it is +// the equal one: b[len(b):] is a legal empty slice, not a panic, and without +// the guard this is refused for the wrong reason. +func TestAnEntryStartingExactlyAtTheEndOfTheFileIsRefused(t *testing.T) { + b := editedFixturePadded(t, [2]string{"0000000871 00000 n", "0000001367 00000 n"}) + if len(b) != 1367 { + t.Fatalf("this test hard-codes the fixture's length; it is %d", len(b)) + } + _, err := CheckLinearized(b) + if !errors.Is(err, ErrLinearizationStale) { + t.Fatalf("error is %v, want ErrLinearizationStale", err) + } + if !strings.Contains(err.Error(), "puts an object at byte 1367 and the file is 1367 bytes") { + t.Errorf("it was refused, but not by the length guard this test is about: %v", err) + } +} + +// ⛔⛔ AN INCREMENTAL UPDATE POINTS startxref AT ITS OWN SECTION, and a +// previous version of this reasoned that /L had already made that impossible. +// It had not: rewriting /L to the new length passes every earlier check, and +// the first-page end was then compared against an object from the update — +// refused, with a reason that was not the reason. +// +// The file below is exactly that: a real appended revision whose /L is +// corrected, which qpdf still calls linearized. +func TestAnIncrementallyUpdatedFileIsNotJudgedByTheUpdatesSection(t *testing.T) { + orig, err := os.ReadFile(filepath.Join("testdata", "linearized.pdf")) + if err != nil { + t.Fatal(err) + } + body := "9 0 obj\n<< /Type /Nothing >>\nendobj\n" + xrefAt := len(orig) + len(body) + tail := fmt.Sprintf("xref\n0 1\n0000000000 65535 f \ntrailer << /Root 5 0 R /Size 10 /Prev 216 >>\nstartxref\n%d\n%%%%EOF\n", xrefAt) + s := string(orig) + body + tail + + // /L must say the new length, or the cheap check refuses it for that and + // this test would never reach what it is about. Same width keeps every + // other offset valid, so the appended revision is the only change. + old, want := "/L 1367", fmt.Sprintf("/L %d", len(s)) + if len(want) != len(old) { + t.Fatalf("the updated file is %d bytes, which is not the same width as the fixture's /L", len(s)) + } + if strings.Count(s, old) != 1 { + t.Fatalf("%q appears %d times in the fixture, so this edit is not the one it is named for", + old, strings.Count(s, old)) + } + b := []byte(strings.Replace(s, old, want, 1)) + + if _, err := Linearized(b); err != nil { + t.Fatalf("the cheap check refused it, so /L is not what this test is about: %v", err) + } + _, err = CheckLinearized(b) + if !errors.Is(err, ErrLinearizationStale) { + t.Fatalf("error is %v, want ErrLinearizationStale", err) + } + // ⛔ THE REASON IS THE ASSERTION. Without the guard this file is still + // refused — by the /E comparison, naming an object the update appended — + // so a test that only checked for an error would pass either way. + if !strings.Contains(err.Error(), "written to since it was linearized") { + t.Errorf("it was refused, but not as an updated file: %v", err) + } +} + +// ⛔ A FIRST-PAGE SECTION LISTING ONLY FREE OBJECTS names nothing whose end +// could be /E. Every entry is marked `f`, which is the same width as `n`, so +// the file is otherwise untouched. +func TestAFirstPageSectionWithNoObjectInTheFileIsRefused(t *testing.T) { + b := editedFixturePadded(t, + [2]string{"0000000015 00000 n", "0000000015 00000 f"}, + [2]string{"0000000501 00000 n", "0000000501 00000 f"}, + [2]string{"0000000550 00000 n", "0000000550 00000 f"}, + [2]string{"0000000677 00000 n", "0000000677 00000 f"}, + [2]string{"0000000773 00000 n", "0000000773 00000 f"}, + [2]string{"0000000871 00000 n", "0000000871 00000 f"}, + ) + _, err := CheckLinearized(b) + if err == nil { + t.Fatal("it was accepted") + } + if !strings.Contains(err.Error(), "lists no object in the file") { + t.Errorf("it was refused, but not for the reason this test is about: %v", err) } } diff --git a/testdata/linearized-gs.pdf b/testdata/linearized-gs.pdf new file mode 100644 index 0000000..13d6bab Binary files /dev/null and b/testdata/linearized-gs.pdf differ