Carry the colour a print file declares, and audit the rest of the catalogue

README.md

@@ -400,6 +400,44 @@ Helvetica-Bold, Courier and Courier-Bold — so nothing is embedded and a
 watermark costs about a kilobyte. Stamp text may say `{page}`, `{pages}`
 and `{n}`, the last being a counter for serial numbering.
 
+
+## ⛔ What a rewrite carries, audited rather than assumed
+
+Twice in one afternoon this package was found to be **dropping** a catalogue
+entry whose absence is not "no opinion" but a **default** — so the loss changed
+the document silently. So the whole list was measured: a document holding
+twenty-three catalogue entries, a watermark over it, and of each entry the
+question *what would its absence MEAN?*
+
+| | |
+| --- | --- |
+| **`/PageLabels`** | the viewer numbers pages by position — a book's front matter came back 1, 2, 3 instead of i, ii, iii |
+| **`/OCProperties`** | everything optional becomes **visible** — a hidden layer was revealed |
+| **`/OutputIntents`** | **no colour management**: a prepress file lost the ICC profile it declares, and would print differently with nothing to say why |
+| **`/URI`** | relative links resolve against wherever the viewer thinks the document is — the link is not removed, it is silently pointed elsewhere |
+
+All four are carried now, the first two **rebuilt** (they cannot be copied: one
+is keyed by page index, the other names objects the content reaches another
+way) and the last two copied.
+
+### What is deliberately not carried, and why
+
+- **`/OpenAction`, `/AA`** — what the document does when it is opened. Dropping
+  them is *safer*, and is what `sanitize` exists for: carrying them would carry
+  **code** across a rebuild;
+- **`/Legal`, `/DSS`** — a signature's attestation and its validation data. A
+  rewrite has already invalidated the signature the moment it changed a byte,
+  and carrying the validation data of a signature that no longer verifies would
+  say the document is still attested when it is not;
+- **`/Threads`, `/Collection`** — these point *into* the document, at pages and
+  at embedded files, which is why `/Names /Dests` is not carried either.
+
+⛔ And **three the audit reported as lost that are not**: `/AcroForm`,
+`/Outlines` and `/StructTreeRoot` are all carried. The audit's fixture held an
+**empty** one of each, and an empty form, outline or structure tree is
+deliberately not written. *A measured absence has to be read before it is
+believed* — which is the other half of why an audit is worth running.
+
 ## Verified against real files
 
 Every operation is checked on the same corpus of **118 863 real PDFs** the

catalogue.go

@@ -28,8 +28,41 @@ var documentKeys = []reader.Name{
 	"PageMode",          // whether to open with the bookmarks showing
 	"Metadata",          // the XMP packet
 	"Extensions",        // which extensions to the format the file uses
+
+	// ⛔ OutputIntents is the colour a print file DECLARES — the ICC profile a
+	// PDF/A or PDF/X names so that a press knows what its numbers mean. It was
+	// dropped until an audit of this list measured what the absence of each
+	// entry MEANS: no output intent is not "no opinion", it is "no colour
+	// management", and a prepress file that came back from a watermark with
+	// its profile gone would print differently with nothing to say why. It
+	// names a profile stream and no page, so it copies safely.
+	"OutputIntents",
+	// ⛔ URI is the BASE for the document's relative links. Without it,
+	// "../figures/one.png" resolves against wherever the viewer happens to
+	// think the document is, so dropping it does not remove a link — it
+	// silently points it somewhere else.
+	"URI",
 }
 
+// What is deliberately NOT carried, measured rather than assumed. An audit
+// built a document holding twenty-three catalogue entries, ran a watermark
+// over it, and asked of each one what its absence would MEAN:
+//
+//   - OpenAction and AA are what a document does when it is opened and when
+//     things happen to it. Dropping them is SAFER, and is what sanitize
+//     exists for; carrying them would carry code across a rebuild.
+//   - Legal and DSS belong to a signature, and a rewrite has already
+//     invalidated that signature the moment it changed a byte. Carrying the
+//     validation data of a signature that no longer verifies would say the
+//     document is still attested when it is not.
+//   - Threads and Collection point INTO the document — at pages, and at
+//     embedded files — which is why /Names /Dests is not carried either.
+//
+// ⛔ And three the audit reported as lost that are NOT: AcroForm, Outlines and
+// StructTreeRoot are all carried, and the audit's fixture held an EMPTY one of
+// each — which is deliberately not written. A measured absence has to be read
+// before it is believed.
+
 // sensitiveKeys are entries a sanitised file does not keep: the XMP packet
 // says who wrote the document, on what machine, and when.
 var sensitiveKeys = map[reader.Name]bool{"Metadata": true}

catalogue_test.go

@@ -431,3 +431,98 @@ func withAnnots(t *testing.T, src []byte) []byte {
 	}
 	return out
 }
+
+// ⛔ The colour a print file DECLARES, carried across a rewrite. Measured
+// before this: a document naming an ICC profile came back from a watermark
+// with no /OutputIntents at all — and no output intent is not "no opinion",
+// it is "no colour management", so the file would print differently with
+// nothing to say why.
+//
+// The assertion reaches the PROFILE and not just the array, because an
+// /OutputIntents whose /DestOutputProfile went with the rebuild declares a
+// colour space it cannot supply.
+func TestTheOutputIntentSurvivesARewrite(t *testing.T) {
+	w := reader.NewWriter("1.7")
+	pagesRef := w.Reserve()
+	profile := w.Add(&reader.Stream{
+		Dict: reader.Dict{"N": reader.Integer(3)},
+		Raw:  []byte("not really an ICC profile, but a stream"),
+	})
+	pageRef := w.Add(reader.Dict{"Type": reader.Name("Page"), "Parent": pagesRef,
+		"MediaBox": reader.Array{reader.Integer(0), reader.Integer(0),
+			reader.Integer(100), reader.Integer(100)},
+		"Contents": w.Add(&reader.Stream{Dict: reader.Dict{}, Raw: []byte{}})})
+	w.Put(pagesRef, reader.Dict{"Type": reader.Name("Pages"),
+		"Kids": reader.Array{pageRef}, "Count": reader.Integer(1)})
+	src, err := w.Finish(reader.Dict{"Root": w.Add(reader.Dict{
+		"Type": reader.Name("Catalog"), "Pages": pagesRef,
+		"OutputIntents": reader.Array{reader.Dict{
+			"Type":                      reader.Name("OutputIntent"),
+			"S":                         reader.Name("GTS_PDFA1"),
+			"OutputConditionIdentifier": reader.String("sRGB IEC61966-2.1"),
+			"DestOutputProfile":         profile,
+		}},
+		"URI": reader.Dict{"Base": reader.String("https://example.com/paper/")},
+	})})
+	if err != nil {
+		t.Fatal(err)
+	}
+	d, err := Open(src)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := d.Watermark("all", "DRAFT"); err != nil {
+		t.Fatal(err)
+	}
+	out, err := d.Bytes()
+	if err != nil {
+		t.Fatal(err)
+	}
+	back, err := reader.Open(out)
+	if err != nil {
+		t.Fatal(err)
+	}
+	catalog, err := back.Catalog()
+	if err != nil {
+		t.Fatal(err)
+	}
+	intents, err := back.Resolve(catalog["OutputIntents"])
+	if err != nil {
+		t.Fatal(err)
+	}
+	arr, ok := intents.(reader.Array)
+	if !ok || len(arr) != 1 {
+		t.Fatalf("OutputIntents = %#v, want an array of one", intents)
+	}
+	intent, err := back.Resolve(arr[0])
+	if err != nil {
+		t.Fatal(err)
+	}
+	dict, ok := intent.(reader.Dict)
+	if !ok {
+		t.Fatalf("the intent is %#v", intent)
+	}
+	if got, _ := back.Resolve(dict["OutputConditionIdentifier"]); string(got.(reader.String)) != "sRGB IEC61966-2.1" {
+		t.Errorf("the condition is %#v", got)
+	}
+	// ⛔ The profile itself, which is the whole point: an intent that names a
+	// profile the file no longer holds declares a colour space it cannot
+	// supply.
+	prof, err := back.Resolve(dict["DestOutputProfile"])
+	if err != nil {
+		t.Fatalf("the profile: %v", err)
+	}
+	if _, ok := prof.(*reader.Stream); !ok {
+		t.Errorf("the profile is %#v, want a stream", prof)
+	}
+	// And the base URI, whose absence points relative links elsewhere rather
+	// than removing them.
+	uri, _ := back.Resolve(catalog["URI"])
+	udict, ok := uri.(reader.Dict)
+	if !ok {
+		t.Fatalf("URI = %#v", uri)
+	}
+	if got, _ := back.Resolve(udict["Base"]); string(got.(reader.String)) != "https://example.com/paper/" {
+		t.Errorf("the base URI is %#v", got)
+	}
+}