From 68c687dc49948540b356a6b47931c9be4fcd0245 Mon Sep 17 00:00:00 2001
From: David Symonds <dsymonds@golang.org>
Date: Mon, 27 Jul 2015 19:06:00 +1000
Subject: [PATCH] Fix handling of RequiredNotSetError being returned by fields
 that implement Marshaler.

This has a chance of breaking some code, but only if they were already
silently broken.
---
 proto/all_test.go | 41 +++++++++++++++++++++++++++++++----------
 proto/encode.go   |  2 +-
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/proto/all_test.go b/proto/all_test.go
index 5a9b6a47f6..b787d58aad 100644
--- a/proto/all_test.go
+++ b/proto/all_test.go
@@ -401,17 +401,18 @@ type fakeMarshaler struct {
 	err error
 }
 
-func (f fakeMarshaler) Marshal() ([]byte, error) {
-	return f.b, f.err
-}
+func (f *fakeMarshaler) Marshal() ([]byte, error) { return f.b, f.err }
+func (f *fakeMarshaler) String() string           { return fmt.Sprintf("Bytes: %v Error: %v", f.b, f.err) }
+func (f *fakeMarshaler) ProtoMessage()            {}
+func (f *fakeMarshaler) Reset()                   {}
 
-func (f fakeMarshaler) String() string {
-	return fmt.Sprintf("Bytes: %v Error: %v", f.b, f.err)
+type msgWithFakeMarshaler struct {
+	M *fakeMarshaler `protobuf:"bytes,1,opt,name=fake"`
 }
 
-func (f fakeMarshaler) ProtoMessage() {}
-
-func (f fakeMarshaler) Reset() {}
+func (m *msgWithFakeMarshaler) String() string { return CompactTextString(m) }
+func (m *msgWithFakeMarshaler) ProtoMessage()  {}
+func (m *msgWithFakeMarshaler) Reset()         {}
 
 // Simple tests for proto messages that implement the Marshaler interface.
 func TestMarshalerEncoding(t *testing.T) {
@@ -423,7 +424,7 @@ func TestMarshalerEncoding(t *testing.T) {
 	}{
 		{
 			name: "Marshaler that fails",
-			m: fakeMarshaler{
+			m: &fakeMarshaler{
 				err: errors.New("some marshal err"),
 				b:   []byte{5, 6, 7},
 			},
@@ -431,9 +432,25 @@ func TestMarshalerEncoding(t *testing.T) {
 			want:    nil,
 			wantErr: errors.New("some marshal err"),
 		},
+		{
+			name: "Marshaler that fails with RequiredNotSetError",
+			m: &msgWithFakeMarshaler{
+				M: &fakeMarshaler{
+					err: &RequiredNotSetError{},
+					b:   []byte{5, 6, 7},
+				},
+			},
+			// Since there's an error that can be continued after,
+			// the buffer should be written.
+			want: []byte{
+				10, 3, // for &msgWithFakeMarshaler
+				5, 6, 7, // for &fakeMarshaler
+			},
+			wantErr: &RequiredNotSetError{},
+		},
 		{
 			name: "Marshaler that succeeds",
-			m: fakeMarshaler{
+			m: &fakeMarshaler{
 				b: []byte{0, 1, 2, 3, 4, 127, 255},
 			},
 			want:    []byte{0, 1, 2, 3, 4, 127, 255},
@@ -443,6 +460,10 @@ func TestMarshalerEncoding(t *testing.T) {
 	for _, test := range tests {
 		b := NewBuffer(nil)
 		err := b.Marshal(test.m)
+		if _, ok := err.(*RequiredNotSetError); ok {
+			// We're not in package proto, so we can only assert the type in this case.
+			err = &RequiredNotSetError{}
+		}
 		if !reflect.DeepEqual(test.wantErr, err) {
 			t.Errorf("%s: got err %v wanted %v", test.name, err, test.wantErr)
 		}
diff --git a/proto/encode.go b/proto/encode.go
index 72c780b915..91f3f0784d 100644
--- a/proto/encode.go
+++ b/proto/encode.go
@@ -529,7 +529,7 @@ func (o *Buffer) enc_struct_message(p *Properties, base structPointer) error {
 		}
 		o.buf = append(o.buf, p.tagcode...)
 		o.EncodeRawBytes(data)
-		return nil
+		return state.err
 	}
 
 	o.buf = append(o.buf, p.tagcode...)