diff --git a/README.md b/README.md index 0430ac2..011e29c 100644 --- a/README.md +++ b/README.md @@ -170,6 +170,35 @@ range, because the reversal absorbs the difference. It passed at the ceiling **and at the floor**, so it proved nothing; the negative control is what said so, and the assertion now uses `-keep-backs`, where the split is load-bearing. + +`repair` writes a file back out with its structure rebuilt, and says whether +rebuilding was needed. `-check` answers that question and writes nothing. + +```sh +pdfops repair -check suspect.pdf +pdfops repair broken.pdf fixed.pdf +``` + +⛔ **The rebuilding is not this verb's doing.** +[go-pdfkit/reader](https://github.com/go-pdfkit/reader) repairs a damaged file +on its own and quietly, which is the right behaviour for a reader — a caller +asking for page three wants page three, not a lecture. What was missing was a +way to **say** it happened and to keep the result, and that is the whole of +this verb. `Doc.Repaired()` is the same answer in Go. + +It reports on **any** source, not all: a merged document's pages come from +several files, and one damaged source is the answer to *"did this need +repairing"*. A true that depended on every source being broken would be false +for every real merge. + +⛔ And the scope is **reader's**, not QPDF's. What it recovers is a +cross-reference table that is wrong, missing, or points into the middle of +something else — the commonest damage, and the kind where the objects are all +still there. It is not a general salvage engine, and a file whose objects are +themselves truncated may still be unreadable. A file that did not need +repairing is **said out loud and is not an error**, so that "repair everything +in this directory" can be scripted. + `attachments` lists the files a document carries, and writes them out with `-to`. A name comes out of the document rather than from whoever runs the command, so it is stripped to its last element: `../escaped.txt` is written diff --git a/cmd/pdfops/expose_test.go b/cmd/pdfops/expose_test.go index b3c6459..a392286 100644 --- a/cmd/pdfops/expose_test.go +++ b/cmd/pdfops/expose_test.go @@ -1,9 +1,11 @@ package main import ( + "bytes" "fmt" "os" "path/filepath" + "regexp" "strconv" "strings" "testing" @@ -783,3 +785,104 @@ func writeDoc(t *testing.T, build func(*ops.Doc)) string { } func itoa(n int) string { return strconv.Itoa(n) } + +// repair writes a file back out with its structure rebuilt, and says whether +// rebuilding was needed. +// +// ⛔ Both answers are asserted, because a verb that said "rebuilt" whatever it +// was given would pass a test that only ever handed it a damaged file — and +// that is the direction this one would fail in, since the honest answer for a +// healthy file is the less interesting one. +func TestRepair(t *testing.T) { + dir := t.TempDir() + healthy, damaged := fixture(t, 2), damagedFixture(t) + + out := filepath.Join(dir, "healthy.pdf") + code, said, msg := exec("repair", healthy, out) + if code != 0 { + t.Fatalf("code %d: %s", code, msg) + } + if !strings.Contains(said, "nothing needed rebuilding") { + t.Errorf("a healthy file: said %q", said) + } + if got := len(contentsOf(t, out)); got != 2 { + t.Errorf("%d pages came out, want 2", got) + } + + out = filepath.Join(dir, "fixed.pdf") + code, said, msg = exec("repair", damaged, out) + if code != 0 { + t.Fatalf("code %d: %s", code, msg) + } + if !strings.Contains(said, "rebuilt") || strings.Contains(said, "nothing") { + t.Errorf("a damaged file: said %q", said) + } + if got := len(contentsOf(t, out)); got != 2 { + t.Errorf("%d pages came out of the repair, want 2", got) + } + // ⛔ And what came out is itself clean: `repair` on the result says there + // was nothing to do. A repair whose output still needs repairing is not + // one. + code, said, _ = exec("repair", "-check", out) + if code != 0 || !strings.Contains(said, "nothing needed rebuilding") { + t.Errorf("the repaired file still needs repairing: %q", said) + } +} + +func TestRepairCheck(t *testing.T) { + code, said, msg := exec("repair", "-check", damagedFixture(t)) + if code != 0 { + t.Fatalf("code %d: %s", code, msg) + } + if !strings.Contains(said, "rebuilt") { + t.Errorf("-check said %q", said) + } + // -check takes no output file, because it writes nothing. + if code, _, _ := exec("repair", "-check", damagedFixture(t), filepath.Join(t.TempDir(), "x.pdf")); code != 1 { + t.Error("-check accepted an output file") + } +} + +func TestRepairRefusals(t *testing.T) { + in := fixture(t, 1) + out := filepath.Join(t.TempDir(), "out.pdf") + if code, _, _ := exec("repair", in); code != 1 { + t.Error("no output file was accepted") + } + if code, _, _ := exec("repair", "/no/such/file.pdf", out); code != 1 { + t.Error("a missing file was accepted") + } + if code, _, _ := exec("repair", "-nonsense", in, out); code != 1 { + t.Error("an unknown flag was accepted") + } + // Both branches have their own save, so both need their own unwritable + // output: a healthy file and a damaged one take different paths through + // the verb and only one of them was covered by the first draft. + if code, _, _ := exec("repair", in, "/no/such/directory/out.pdf"); code != 1 { + t.Error("an unwritable output was accepted for a healthy file") + } + if code, _, _ := exec("repair", damagedFixture(t), "/no/such/directory/out.pdf"); code != 1 { + t.Error("an unwritable output was accepted for a damaged file") + } +} + +// damagedFixture points a file's startxref at the wrong place, which is the +// commonest way a PDF arrives damaged: the objects are all there and the table +// that says where they are is not. +func damagedFixture(t *testing.T) string { + t.Helper() + b, err := os.ReadFile(fixture(t, 2)) + if err != nil { + t.Fatal(err) + } + re := regexp.MustCompile(`startxref\s*\n\s*\d+`) + broken := re.ReplaceAll(b, []byte("startxref\n999999")) + if bytes.Equal(broken, b) { + t.Fatal("the fixture has no startxref to break, so this proves nothing") + } + path := filepath.Join(t.TempDir(), "damaged.pdf") + if err := os.WriteFile(path, broken, 0o644); err != nil { + t.Fatal(err) + } + return path +} diff --git a/cmd/pdfops/run.go b/cmd/pdfops/run.go index c545a59..67b34ae 100644 --- a/cmd/pdfops/run.go +++ b/cmd/pdfops/run.go @@ -61,6 +61,7 @@ var commands = []command{ {"info", "", "print what the file says about itself", runInfo}, {"metadata", "(-set … | -clear) ", "write what the file says about itself", runMetadata}, {"version", "-set ", "set the PDF version the file declares", runVersion}, + {"repair", "[-check] [out.pdf]", "rebuild a file whose structure is damaged, and say whether it was", runRepair}, {"strip", "[-annotations] [-bookmarks] ", "write the file without its metadata", runStrip}, {"sanitize", " ", "remove what runs rather than shows: scripts, launching, embedded files", runSanitize}, {"outline", "(-drop | -from ) ", "replace or remove the bookmarks", runOutline}, @@ -1722,3 +1723,46 @@ func runDeblank(c *context, args []string) error { fmt.Fprintln(c.out, "dropped", len(went), "page(s):", went) return nil } + +// runRepair writes a file back out with its structure rebuilt, and says +// whether rebuilding was needed. +// +// ⛔ The rebuilding is not this verb's doing: go-pdfkit/reader repairs a +// damaged file on its own, quietly, which is the right behaviour for a reader +// — a caller asking for page three wants page three. What was missing was a +// way to SAY it happened and to keep the result, which is the whole of this +// verb. It is also why -check exists: the question "is this file damaged" has +// an answer that is worth having without writing anything. +func runRepair(c *context, args []string) error { + fs := flags("repair") + check := fs.Bool("check", false, "say whether the file needed rebuilding and write nothing") + if err := fs.Parse(args); err != nil { + return err + } + want, usage := 2, " " + if *check { + want, usage = 1, "" + } + if err := wantArgs(fs, want, usage); err != nil { + return err + } + d, err := c.open(fs.Arg(0)) + if err != nil { + return err + } + if !d.Repaired() { + // ⛔ Said out loud, and not an error. A file that did not need + // repairing is the good case, and reporting it as a failure would + // make "repair everything in this directory" impossible to script. + fmt.Fprintln(c.out, "nothing needed rebuilding") + if *check { + return nil + } + return save(d, fs.Arg(1)) + } + fmt.Fprintln(c.out, "the structure was rebuilt to read it") + if *check { + return nil + } + return save(d, fs.Arg(1)) +} diff --git a/repaired.go b/repaired.go new file mode 100644 index 0000000..4976cf2 --- /dev/null +++ b/repaired.go @@ -0,0 +1,30 @@ +// Copyright (c) 2026, the go-pdfkit/ops authors +// All rights reserved. +// +// SPDX-License-Identifier: BSD-3-Clause + +package ops + +// Repaired says whether any file this document's pages came from had to be +// rebuilt before it could be read: a cross-reference table that did not match +// the objects, a trailer that was not there, an offset that pointed into the +// middle of something else. +// +// [reader] does the rebuilding on its own, and quietly, which is the right +// behaviour for a reader — a caller asking for page three wants page three, +// not a lecture. But it leaves nobody able to SAY it happened, and that is +// worth saying for two different reasons: a file that needed rebuilding should +// be written back out and kept in its rebuilt form, and a file that did not +// need it should not be presented as rescued. +// +// ⛔ ANY, not all. A merged document's pages come from several files, and one +// damaged source is the answer to "did this need repairing": a true that +// depended on every source being broken would be false for every real merge. +func (d *Doc) Repaired() bool { + for _, p := range d.pages { + if p.src != nil && p.src.Repaired() { + return true + } + } + return false +} diff --git a/repaired_test.go b/repaired_test.go new file mode 100644 index 0000000..ed5360a --- /dev/null +++ b/repaired_test.go @@ -0,0 +1,108 @@ +// Copyright (c) 2026, the go-pdfkit/ops authors +// All rights reserved. +// +// SPDX-License-Identifier: BSD-3-Clause + +package ops + +import ( + "bytes" + "regexp" + "testing" +) + +// brokenXref points a file's startxref at the wrong place, which is the +// commonest way a PDF arrives damaged: the objects are all there and the table +// that says where they are is not. +func brokenXref(t *testing.T, b []byte) []byte { + t.Helper() + re := regexp.MustCompile(`startxref\s*\n\s*\d+`) + out := re.ReplaceAll(b, []byte("startxref\n999999")) + if bytes.Equal(out, b) { + t.Fatal("the fixture has no startxref to break, so this proves nothing") + } + return out +} + +// ⛔ A POSITIVE control first. Without it, "Repaired() is false" would pass on +// a fixture that was never broken, and the whole test would be measuring +// nothing — which is what the brokenXref helper's own fatal guards against on +// the other side. +func TestRepairedIsFalseForAFileThatIsNotDamaged(t *testing.T) { + d, err := Open(simple(t, 2)) + if err != nil { + t.Fatal(err) + } + if d.Repaired() { + t.Error("a file that opens normally was reported as rebuilt") + } +} + +func TestRepairedIsTrueForAFileWhoseTableIsWrong(t *testing.T) { + d, err := Open(brokenXref(t, simple(t, 2))) + if err != nil { + t.Fatalf("a file with a wrong startxref could not be opened at all: %v", err) + } + if !d.Repaired() { + t.Error("a file whose cross-reference table was wrong was not reported as rebuilt") + } + // And it is really readable: a repair that loses the pages is not a repair. + if got := d.PageCount(); got != 2 { + t.Errorf("PageCount() = %d, want 2", got) + } + out, err := d.Bytes() + if err != nil { + t.Fatal(err) + } + // ⛔ What comes out must NOT itself need repairing. That is the whole point + // of writing it back: a rebuilt file kept in its rebuilt form. + back, err := Open(out) + if err != nil { + t.Fatal(err) + } + if back.Repaired() { + t.Error("what was written had to be rebuilt again to be read") + } +} + +// ⛔ ANY source, not all. A merged document's pages come from several files, +// and one damaged source is the answer to "did this need repairing" — a true +// that depended on every source being broken would be false for every real +// merge. +func TestOneDamagedSourceIsEnough(t *testing.T) { + good, err := Open(simple(t, 1)) + if err != nil { + t.Fatal(err) + } + bad, err := Open(brokenXref(t, simple(t, 1))) + if err != nil { + t.Fatal(err) + } + good.Append(bad) + if !good.Repaired() { + t.Error("a merge of a clean file and a damaged one was not reported as rebuilt") + } + // The other order too, because "any" is not "the first". + good2, err := Open(simple(t, 1)) + if err != nil { + t.Fatal(err) + } + bad2, err := Open(brokenXref(t, simple(t, 1))) + if err != nil { + t.Fatal(err) + } + bad2.Append(good2) + if !bad2.Repaired() { + t.Error("the damaged file first was not reported as rebuilt") + } +} + +// A document built here rather than borrowed has no source to have been +// repaired, and must not claim one. +func TestADocumentBuiltFromNothingWasNotRepaired(t *testing.T) { + d := New() + d.Blank(595, 842) + if d.Repaired() { + t.Error("a document with no source file was reported as rebuilt") + } +}