From 50ff460fe18dd7f30883cadb9926ed1da056e107 Mon Sep 17 00:00:00 2001
From: Casey Marshall <casey.marshall@gmail.com>
Date: Tue, 18 Dec 2012 17:57:21 -0500
Subject: [PATCH] openpgp: remove TeeReader from packet.Read

This is a followup to issue 6927044. I *really* don't want to
break streaming for large encrypted data packets.

Removing the automatic re-parsing. OpaqueReader can be
used for recovering useful information from mangled/unsupported packets.

R=agl
CC=golang-dev
https://golang.org/cl/6944056
---
 openpgp/packet/opaque.go      |  8 +++-
 openpgp/packet/opaque_test.go | 77 +++++++++++++++++++++++++++++++++++
 openpgp/packet/packet.go      | 18 +-------
 3 files changed, 86 insertions(+), 17 deletions(-)
 create mode 100644 openpgp/packet/opaque_test.go

diff --git a/openpgp/packet/opaque.go b/openpgp/packet/opaque.go
index 14b9759..6104d42 100644
--- a/openpgp/packet/opaque.go
+++ b/openpgp/packet/opaque.go
@@ -46,9 +46,15 @@ func (op *OpaquePacket) Parse() (p Packet, err error) {
 	hdr := bytes.NewBuffer(nil)
 	err = serializeHeader(hdr, packetType(op.Tag), len(op.Contents))
 	if err != nil {
+		op.Reason = err
 		return op, err
 	}
-	return Read(io.MultiReader(hdr, bytes.NewBuffer(op.Contents)))
+	p, err = Read(io.MultiReader(hdr, bytes.NewBuffer(op.Contents)))
+	if err != nil {
+		op.Reason = err
+		p = op
+	}
+	return
 }
 
 // OpaqueReader reads OpaquePackets from an io.Reader.
diff --git a/openpgp/packet/opaque_test.go b/openpgp/packet/opaque_test.go
new file mode 100644
index 0000000..dc275ef
--- /dev/null
+++ b/openpgp/packet/opaque_test.go
@@ -0,0 +1,77 @@
+// Copyright 2011 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package packet
+
+import (
+	"bytes"
+	"code.google.com/p/go.crypto/openpgp/armor"
+	"io"
+	"testing"
+)
+
+// This key contains version 2 public key and signature packets.
+// If these are ever supported, this test will need to be updated
+// with bad packets that won't parse.
+const UnsupportedKeyArmor = `-----BEGIN PGP PUBLIC KEY BLOCK-----
+Version: SKS 1.0.10
+
+mI0CLnoYogAAAQQA1qwA2SuJwfQ5bCQ6u5t20ulnOtY0gykf7YjiK4LiVeRBwHjGq7v30tGV
+5Qti7qqRW4Ww7CDCJc4sZMFnystucR2vLkXaSoNWoFm4Fg47NiisDdhDezHwbVPW6OpCFNSi
+ZAamtj4QAUBu8j4LswafrJqZqR9336/V3g8Yil2l48kABRG0J0FybWluIE0uIFdhcmRhIDx3
+YXJkYUBuZXBoaWxpbS5ydWhyLmRlPoiVAgUQLok2xwXR6zmeWEiZAQE/DgP/WgxPQh40/Po4
+gSkWZCDAjNdph7zexvAb0CcUWahcwiBIgg3U5ErCx9I5CNVA9U+s8bNrDZwgSIeBzp3KhWUx
+524uhGgm6ZUTOAIKA6CbV6pfqoLpJnRYvXYQU5mIWsNa99wcu2qu18OeEDnztb7aLA6Ra9OF
+YFCbq4EjXRoOrYM=
+=LPjs
+-----END PGP PUBLIC KEY BLOCK-----`
+
+// Test packet.Read error handling in OpaquePacket.Parse,
+// which attempts to re-read an OpaquePacket as a supported
+// Packet type.
+func TestOpaqueParseReason(t *testing.T) {
+	armorBlock, err := armor.Decode(bytes.NewBufferString(UnsupportedKeyArmor))
+	if err != nil {
+		t.Fatalf("armor Decode failed: %v", err)
+	}
+	or := NewOpaqueReader(armorBlock.Body)
+	count := 0
+	badPackets := 0
+	var uid *UserId
+	for {
+		op, err := or.Next()
+		if err == io.EOF {
+			break
+		} else if err != nil {
+			t.Errorf("#%d: opaque read error: %v", count, err)
+			break
+		}
+		// try to parse opaque packet
+		p, err := op.Parse()
+		switch pkt := p.(type) {
+		case *UserId:
+			uid = pkt
+		case *OpaquePacket:
+			// If an OpaquePacket can't re-parse, packet.Read
+			// certainly had its reasons.
+			if pkt.Reason == nil {
+				t.Errorf("#%d: opaque packet, no reason", count)
+			} else {
+				badPackets++
+			}
+		}
+		count++
+	}
+
+	const expectedBad = 2
+	// Test post-conditions, make sure we actually parsed packets as expected.
+	if badPackets != expectedBad {
+		t.Errorf("unexpected # unparseable packets: %d (want %d)", badPackets, expectedBad)
+	}
+	if uid == nil {
+		t.Errorf("failed to find expected UID in unsupported keyring")
+	} else if uid.Id != "Armin M. Warda <warda@nephilim.ruhr.de>" {
+		t.Errorf("unexpected UID: %v", uid.Id)
+	}
+}
diff --git a/openpgp/packet/packet.go b/openpgp/packet/packet.go
index 9c4a1b1..9b5cd88 100644
--- a/openpgp/packet/packet.go
+++ b/openpgp/packet/packet.go
@@ -7,7 +7,6 @@
 package packet
 
 import (
-	"bytes"
 	"code.google.com/p/go.crypto/cast5"
 	"code.google.com/p/go.crypto/openpgp/errors"
 	"crypto/aes"
@@ -339,23 +338,10 @@ func Read(r io.Reader) (p Packet, err error) {
 		se.MDC = true
 		p = se
 	default:
-		p = new(OpaquePacket)
+		err = errors.UnknownPacketTypeError(tag)
 	}
 	if p != nil {
-		backup := bytes.NewBuffer(nil)
-		// FIXME: this causes large packets to be buffered in memory.
-		// Ideally we would like a better solution here so that huge,
-		// encrypted files could still be handled in a streaming
-		// fashion. Perhaps the encrypted data packet could call a
-		// method on backup that causes it to stop recording.
-		tee := io.TeeReader(contents, backup)
-		err = p.parse(tee)
-		switch err.(type) {
-		case errors.UnsupportedError, errors.UnknownPacketTypeError:
-			p = &OpaquePacket{Tag: uint8(tag), Reason: err}
-			err = nil
-			err = p.parse(io.MultiReader(backup, contents))
-		}
+		err = p.parse(contents)
 	}
 	if err != nil {
 		consumeAll(contents)