diff --git a/README.md b/README.md index 320dc61..a3faad6 100644 --- a/README.md +++ b/README.md @@ -134,6 +134,52 @@ 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. + +⛔⛔ **`/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, 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 `qpdf --check` alongside: **5 agree, 0 disagree**, including a 68-page, 13 MB diff --git a/fuzz_test.go b/fuzz_test.go index cdd8a47..06fa0bb 100644 --- a/fuzz_test.go +++ b/fuzz_test.go @@ -1,6 +1,7 @@ package reader_test import ( + "fmt" "os" "path/filepath" "testing" @@ -171,3 +172,75 @@ 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. + // + // ⛔⛔ 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 %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 >>", + } { + 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() { + lin, err := reader.CheckLinearized(b) + if err != nil { + return + } + // ⛔ 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", + 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 d4cf776..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 @@ -186,6 +194,310 @@ 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 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 +// 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 err := checkFirstPage(d, lin); err != nil { + return nil, err + } + 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 + } + // 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 /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. +// +// 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 { + // ⛔ 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 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 +} + +// 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)) + } + // ⛔⛔ 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)) + } + // ⛔ 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, 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) + } + 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) + } + // ⛔ 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(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 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. +// +// ⛔⛔ 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. +// +// 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 +// 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(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) + } + 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. // @@ -239,13 +551,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 + 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 0, fmt.Errorf("%w: /Linearized is %T, want a direct number", ErrLinearizationMalformed, v) + return got, nil } // directInteger reads a required entry that must be a direct integer. @@ -258,7 +587,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". @@ -280,7 +637,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 1e365c7..90ecbaf 100644 --- a/linearized_test.go +++ b/linearized_test.go @@ -6,6 +6,7 @@ package reader import ( + "bytes" "errors" "os" "os/exec" @@ -68,6 +69,620 @@ 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 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)"}, + {"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) + + // 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)) +} + +// ⛔⛔ 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) + } +} + +// ⛔ 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. +// +// ⛔ 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, 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 + " ]"}) + // ⛔ 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("error is %v, want it to wrap ErrLinearizationStale", 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) + } +} + +// ⛔⛔ 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) { + 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) { + 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) + } +} + +// 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) + } + }) + } + } +} + func qpdfOrSkip(t *testing.T) string { t.Helper() path, err := exec.LookPath("qpdf") @@ -90,7 +705,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. diff --git a/testdata/linearized-objstm.pdf b/testdata/linearized-objstm.pdf new file mode 100644 index 0000000..b3d16fa Binary files /dev/null and b/testdata/linearized-objstm.pdf differ