Skip to content

Commit

Permalink
Fix handling of RequiredNotSetError being returned by fields that imp…
Browse files Browse the repository at this point in the history
…lement Marshaler.

This has a chance of breaking some code, but only if they were already
silently broken.
  • Loading branch information
dsymonds committed Jul 28, 2015
1 parent 6683654 commit 68c687d
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 11 deletions.
41 changes: 31 additions & 10 deletions proto/all_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -423,17 +424,33 @@ func TestMarshalerEncoding(t *testing.T) {
}{
{
name: "Marshaler that fails",
m: fakeMarshaler{
m: &fakeMarshaler{
err: errors.New("some marshal err"),
b: []byte{5, 6, 7},
},
// Since there's an error, nothing should be written to buffer.
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},
Expand All @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion proto/encode.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
Expand Down

0 comments on commit 68c687d

Please sign in to comment.