Salvage what a damaged stream can give, keep it out of the field callers paint from, and say what went wrong

README.md

@@ -29,16 +29,39 @@ the **document structure**:
   resolved the way the specification requires.
 - **Filters** — `FlateDecode`, `LZWDecode` (with PDF's `EarlyChange`, which the
   standard library's LZW does not implement), `ASCIIHexDecode`,
-  `ASCII85Decode`, `RunLengthDecode`, plus the PNG and TIFF predictors.
-  Image filters (`DCTDecode`, `JPXDecode`, `CCITTFaxDecode`, `JBIG2Decode`) are
-  reported rather than applied, so an image consumer gets the encoded bytes.
+  `ASCII85Decode`, `RunLengthDecode`, `Crypt`, plus the PNG and TIFF
+  predictors. Image filters (`DCTDecode`, `JPXDecode`, `CCITTFaxDecode`,
+  `JBIG2Decode`) are reported rather than applied, so an image consumer gets
+  the encoded bytes.
+- **Filters that fail** — a chain that cannot be run to the end is not the end
+  of the stream. `DecodeRecovering` returns what the filters before the failure
+  produced — the prefix a damaged Flate stream did inflate, not its compressed
+  bytes — and says so through `Decoded.Recovered` and `Decoded.Cause`. Bytes no
+  filter decoded arrive in `Decoded.Undecoded`, never in `Decoded.Data`, so a
+  compressed stream cannot be painted as samples or tokenised as content by a
+  caller that forgot to check a flag.
+  `Decode` is the strict reading and refuses such a stream outright, so a
+  caller that must not act on damaged data says which it wants by which one it
+  calls. Page content takes the lenient reading, because a page whose content
+  will not decode is still a page; `PageContentDecoded` reports whether any
+  salvaging happened. Cross-reference streams take the strict one: a table that
+  has to be guessed at is worse than no table, and the repair below is the
+  answer to one.
 - **Cross-references** — classic tables, cross-reference **streams**, **object
   streams**, `/Prev` chains, and the `/XRefStm` of a hybrid file, newest
   definition winning.
 - **Repair** — a file whose tables are missing, truncated or simply wrong is
   rebuilt by scanning it for object headers, trailers and, failing those, for
   a catalogue; a file that kept its pages but lost its catalogue gets one.
-  This is not an exceptional path: it is what makes a reader usable.
+  This is not an exceptional path: it is what makes a reader usable. The
+  rebuild establishes the file key before it reads object streams, so a file
+  whose catalogue lives in an encrypted one can be rebuilt at all; it takes
+  `/Encrypt` from a cross-reference stream's dictionary when the file has no
+  trailer keyword to name it in. Where two object streams claim the same
+  object, the later one in the file wins, the same rule the header scan
+  applies to objects written directly — so the same damaged file reads the
+  same way twice. When a rebuild fails, the error says what the rebuild found
+  and not merely that the tables could not be read.
 - **Documents** — `Open`, object resolution with cycle and recursion guards,
   the trailer, the catalogue, and the page tree with the four attributes a
   page inherits from its ancestors.

content.go

@@ -253,59 +253,79 @@ func Operations(data []byte) ([]Operation, error) {
 // PageContent returns the decoded content stream of the i'th page, counting
 // from one. A page whose /Contents is an array has its streams joined, as the
 // specification requires, with a newline between them.
+//
+// A content stream whose filters cannot be applied contributes what could be
+// salvaged from it rather than failing the page, which is what a renderer
+// needs; the error is for a page that is not there at all.
+// [Document.PageContentDecoded] says whether any salvaging happened.
 func (d *Document) PageContent(i int) ([]byte, error) {
+	dec, err := d.PageContentDecoded(i)
+	return dec.Data, err
+}
+
+// PageContentDecoded is [Document.PageContent] with the outcome of the decode
+// attached: [Decoded.Recovered] reports that at least one of the page's content
+// streams could not be decoded cleanly, and [Decoded.Cause] says why.
+func (d *Document) PageContentDecoded(i int) (Decoded, error) {
 	page, err := d.Page(i)
 	if err != nil {
-		return nil, err
+		return Decoded{}, err
 	}
 	return d.contentOf(page)
 }
 
 // contentOf decodes a page's /Contents, whether one stream or several.
-func (d *Document) contentOf(page Dict) ([]byte, error) {
+func (d *Document) contentOf(page Dict) (Decoded, error) {
 	o, err := d.Resolve(page.Get("Contents"))
 	if err != nil {
-		return nil, err
+		return Decoded{}, err
 	}
 	switch v := o.(type) {
 	case *Stream:
-		return d.decodedContent(v)
+		return d.decodedContent(v), nil
 	case Array:
-		var out []byte
+		var out Decoded
 		for _, e := range v {
 			eo, err := d.Resolve(e)
 			if err != nil {
-				return nil, err
+				return Decoded{}, err
 			}
 			s, ok := ToStream(eo)
 			if !ok {
 				continue
 			}
-			part, err := d.decodedContent(s)
-			if err != nil {
-				return nil, err
+			part := d.decodedContent(s)
+			if part.Recovered && !out.Recovered {
+				out.Recovered, out.Cause, out.Filter = true, part.Cause, part.Filter
+				out.Undecoded = part.Undecoded
 			}
-			if len(out) > 0 {
-				out = append(out, '\n')
+			if len(part.Data) == 0 {
+				continue
+			}
+			if len(out.Data) > 0 {
+				out.Data = append(out.Data, '\n')
 			}
-			out = append(out, part...)
+			out.Data = append(out.Data, part.Data...)
 		}
 		return out, nil
 	}
-	return nil, nil
+	return Decoded{}, nil
 }
 
-// decodedContent decodes one content stream, refusing an image filter, which
-// has no business being there.
-func (d *Document) decodedContent(s *Stream) ([]byte, error) {
-	data, img, err := d.DecodeStream(s)
-	if err != nil {
-		return nil, err
-	}
-	if img != "" {
-		return nil, fmt.Errorf("reader: a content stream is filtered as an image (/%s)", img)
+// decodedContent decodes one content stream. An image filter has no business
+// being there, so the bytes it holds are not content: they are reported as
+// salvage, not handed over as if they could be tokenised.
+func (d *Document) decodedContent(s *Stream) Decoded {
+	dec := d.DecodeStreamRecovering(s)
+	if dec.Image != "" {
+		return Decoded{
+			Undecoded: dec.Data,
+			Recovered: true,
+			Filter:    dec.Image,
+			Cause:     fmt.Errorf("reader: a content stream is filtered as an image (/%s)", dec.Image),
+		}
 	}
-	return data, nil
+	return dec
 }
 
 // PageOperations tokenises the i'th page's content stream, counting from one.

content_test.go

@@ -242,8 +242,16 @@ func TestPageContentImageFilterIsRefused(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if _, err := d.PageContent(1); err == nil {
-		t.Error("want an error")
+	dec, err := d.PageContentDecoded(1)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !dec.Recovered || dec.Cause == nil || dec.Filter != "DCTDecode" {
+		t.Errorf("got %+v", dec)
+	}
+	// Bytes an image filter holds are not content, so none are handed over.
+	if len(dec.Data) != 0 {
+		t.Errorf("got %q, want no content", dec.Data)
 	}
 }
 
@@ -258,8 +266,22 @@ func TestPageContentUndecodable(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		if _, err := d.PageContent(1); err == nil {
-			t.Errorf("%s: want an error", contents)
+		// A page whose only content stream will not decode is still a page:
+		// the raw bytes come back flagged, not as a failure.
+		dec, err := d.PageContentDecoded(1)
+		if err != nil {
+			t.Fatalf("%s: %v", contents, err)
+		}
+		if !dec.Recovered || dec.Cause == nil || dec.Filter != "FlateDecode" {
+			t.Errorf("%s: got %+v", contents, dec)
+		}
+		// The bytes come back, but not as content: a scanner must not be
+		// handed a compressed stream to tokenise.
+		if len(dec.Data) != 0 {
+			t.Errorf("%s: undecoded bytes arrived as content: %q", contents, dec.Data)
+		}
+		if string(dec.Undecoded) != "not deflate data" {
+			t.Errorf("%s: got %q, want the raw bytes", contents, dec.Undecoded)
 		}
 	}
 }

crypt.go

@@ -169,6 +169,18 @@ func newDecryptor(enc Dict, id []byte, password string, r Resolver) (*decryptor,
 	}
 
 	dec.perm = Permissions(uint32(perm)) & AllPermissions
+	if dec.streams == cryptNone && dec.strings == cryptNone {
+		// /StmF and /StrF both name the identity crypt filter, so nothing in
+		// the document body is encrypted and no key is needed to read it. A
+		// file like that is opened without validating the password: refusing
+		// it would refuse a document whose every byte is already plain, which
+		// is what every other reader shows. Only /EFF — the embedded files —
+		// would need the key, and this reader does not hand those out.
+		//
+		// Nothing was authenticated, so dec.owner stays false and Protection
+		// reports the method as "none": a caller can see exactly what it got.
+		return dec, nil
+	}
 	if rev >= 5 {
 		key, asOwner, err := deriveKeyR5(enc, password, r)
 		if err != nil {
@@ -520,12 +532,28 @@ func (dec *decryptor) walk(num, gen int, o Object) Object {
 			return v
 		}
 		v.Dict, _ = ToDict(dec.walk(num, gen, v.Dict))
+		if streamIsPlain(v.Dict) {
+			return v
+		}
 		v.Raw = dec.decryptBytes(num, gen, dec.streams, v.Raw)
 		return v
 	}
 	return o
 }
 
+// streamIsPlain reports whether a stream's own filter chain says its bytes were
+// left unencrypted: a leading /Crypt filter naming /Identity, which is what
+// /Crypt with no /Name means too. Producers use it for the metadata stream of a
+// file whose /EncryptMetadata is false, and decrypting such a stream turns
+// readable XML into noise.
+func streamIsPlain(d Dict) bool {
+	if first, ok := firstFilter(d); !ok || first != "Crypt" {
+		return false
+	}
+	name, ok := ToName(firstDecodeParms(d).Get("Name"))
+	return !ok || name == "Identity"
+}
+
 // resolved is a helper for reading an /Encrypt entry that may be indirect.
 func resolved(d Dict, key Name, r Resolver) Object {
 	o, err := Resolve(d.Get(key), r)

crypt_filter_test.go

@@ -0,0 +1,121 @@
+package reader
+
+import (
+	"fmt"
+	"testing"
+)
+
+// /Crypt is not a transformation: /Identity, and /Crypt with no /Name at all,
+// leave the bytes exactly as they are.
+func TestCryptFilterIsIdentity(t *testing.T) {
+	for _, d := range []Dict{
+		{"Filter": Name("Crypt")},
+		{"Filter": Name("Crypt"), "DecodeParms": Dict{"Name": Name("Identity")}},
+		{"Filter": Array{Name("Crypt")}, "DecodeParms": Array{Dict{"Name": Name("Identity")}}},
+	} {
+		dec := DecodeRecovering(d, []byte("<?xpacket begin=?>"), nil)
+		if dec.Recovered || string(dec.Data) != "<?xpacket begin=?>" {
+			t.Errorf("%v: got %+v", d, dec)
+		}
+	}
+}
+
+// A /Crypt filter chained ahead of a real one is stepped over.
+func TestCryptFilterAheadOfFlate(t *testing.T) {
+	raw := deflateBytes(t, []byte("metadata"))
+	d := Dict{
+		"Filter":      Array{Name("Crypt"), Name("FlateDecode")},
+		"DecodeParms": Array{Dict{"Name": Name("Identity")}, Null{}},
+	}
+	dec := DecodeRecovering(d, raw, nil)
+	if dec.Recovered || string(dec.Data) != "metadata" {
+		t.Fatalf("got %+v", dec)
+	}
+}
+
+// A crypt filter this reader cannot account for is reported, because the bytes
+// would otherwise be ciphertext passed off as data.
+func TestCryptFilterRefusesANamedFilter(t *testing.T) {
+	d := Dict{"Filter": Name("Crypt"), "DecodeParms": Dict{"Name": Name("StdCF")}}
+	dec := DecodeRecovering(d, []byte("cipher"), nil)
+	if !dec.Recovered || dec.Cause == nil || dec.Filter != "Crypt" {
+		t.Fatalf("got %+v", dec)
+	}
+}
+
+// A /Name that cannot be resolved is reported too.
+func TestCryptFilterUnresolvableName(t *testing.T) {
+	fail := func(Ref) (Object, error) { return nil, fmt.Errorf("no") }
+	d := Dict{"Filter": Name("Crypt"), "DecodeParms": Dict{"Name": Ref{Num: 9}}}
+	dec := DecodeRecovering(d, []byte("cipher"), fail)
+	if !dec.Recovered || dec.Cause == nil {
+		t.Fatalf("got %+v", dec)
+	}
+}
+
+func TestFirstFilterAndDecodeParms(t *testing.T) {
+	for _, c := range []struct {
+		d    Dict
+		name Name
+		ok   bool
+	}{
+		{Dict{"Filter": Name("Fl")}, "Fl", true},
+		{Dict{"Filter": Array{Name("Crypt"), Name("Fl")}}, "Crypt", true},
+		{Dict{"Filter": Array{Integer(1)}}, "", false},
+		{Dict{"Filter": Array{}}, "", false},
+		{Dict{"Filter": Integer(3)}, "", false},
+		{Dict{}, "", false},
+	} {
+		if got, ok := firstFilter(c.d); got != c.name || ok != c.ok {
+			t.Errorf("firstFilter(%v) = %q, %v", c.d, got, ok)
+		}
+	}
+	for _, c := range []struct {
+		d    Dict
+		want Name
+	}{
+		{Dict{"DecodeParms": Dict{"Name": Name("A")}}, "A"},
+		{Dict{"DecodeParms": Array{Dict{"Name": Name("B")}}}, "B"},
+		{Dict{"DecodeParms": Array{Integer(0)}}, ""},
+		{Dict{"DecodeParms": Array{}}, ""},
+		{Dict{"DecodeParms": Integer(0)}, ""},
+		{Dict{}, ""},
+	} {
+		got, _ := ToName(firstDecodeParms(c.d).Get("Name"))
+		if got != c.want {
+			t.Errorf("firstDecodeParms(%v)/Name = %q, want %q", c.d, got, c.want)
+		}
+	}
+}
+
+// A stream whose chain begins with /Crypt /Identity was never encrypted, so
+// decrypting it would turn readable bytes into noise.
+func TestStreamIsPlain(t *testing.T) {
+	for _, c := range []struct {
+		d    Dict
+		want bool
+	}{
+		{Dict{"Filter": Name("Crypt")}, true},
+		{Dict{"Filter": Name("Crypt"), "DecodeParms": Dict{"Name": Name("Identity")}}, true},
+		{Dict{"Filter": Name("Crypt"), "DecodeParms": Dict{"Name": Name("StdCF")}}, false},
+		{Dict{"Filter": Name("FlateDecode")}, false},
+		{Dict{}, false},
+	} {
+		if got := streamIsPlain(c.d); got != c.want {
+			t.Errorf("streamIsPlain(%v) = %v", c.d, got)
+		}
+	}
+}
+
+// The decryptor leaves such a stream alone, and still decrypts its neighbours.
+func TestDecryptorSkipsAPlainStream(t *testing.T) {
+	dec := &decryptor{revision: 4, streams: cryptRC4, strings: cryptRC4, key: []byte("0123456789abcdef")}
+	plain := &Stream{Dict: Dict{"Filter": Name("Crypt")}, Raw: []byte("<?xpacket?>")}
+	if got := dec.decryptObject(7, 0, plain).(*Stream); string(got.Raw) != "<?xpacket?>" {
+		t.Errorf("a plain stream was decrypted: %q", got.Raw)
+	}
+	other := &Stream{Dict: Dict{"Filter": Name("FlateDecode")}, Raw: []byte("<?xpacket?>")}
+	if got := dec.decryptObject(7, 0, other).(*Stream); string(got.Raw) == "<?xpacket?>" {
+		t.Error("an encrypted stream was left alone")
+	}
+}