From 8cfc14f022f558728049bb7799b69309db55ce84 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Fri, 17 Apr 2020 10:55:19 -0700 Subject: [PATCH] all: consistently treat nil message interface as an empty read-only message To assist users in migrating from github.com/golang/protobuf to google.golang.org/protobuf, make it such that functiionality like proto.Marshal doesn't panic on nil interfaces. Similar to how the new implementation treats a typed nil message as an empty message, we treat a nil interface as being equivalent to an "untyped" empty message. Change-Id: Ic037f386f855b122f732b34d370e524b7c0d76f1 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/228837 Reviewed-by: Damien Neil --- encoding/protojson/encode.go | 6 ++++++ encoding/prototext/encode.go | 6 ++++++ proto/checkinit.go | 6 ++++++ proto/encode.go | 15 +++++++++++++++ proto/merge.go | 3 +++ proto/nil_test.go | 2 -- proto/size.go | 5 +++++ 7 files changed, 41 insertions(+), 2 deletions(-) diff --git a/encoding/protojson/encode.go b/encoding/protojson/encode.go index 243d1588..e545feb5 100644 --- a/encoding/protojson/encode.go +++ b/encoding/protojson/encode.go @@ -116,6 +116,12 @@ func (o MarshalOptions) Marshal(m proto.Message) ([]byte, error) { return nil, err } + // Treat nil message interface as an empty message, + // in which case the output in an empty JSON object. + if m == nil { + return []byte("{}"), nil + } + enc := encoder{internalEnc, o} if err := enc.marshalMessage(m.ProtoReflect()); err != nil { return nil, err diff --git a/encoding/prototext/encode.go b/encoding/prototext/encode.go index 83b65a66..e207c692 100644 --- a/encoding/prototext/encode.go +++ b/encoding/prototext/encode.go @@ -106,6 +106,12 @@ func (o MarshalOptions) Marshal(m proto.Message) ([]byte, error) { return nil, err } + // Treat nil message interface as an empty message, + // in which case there is nothing to output. + if m == nil { + return []byte{}, nil + } + enc := encoder{internalEnc, o} err = enc.marshalMessage(m.ProtoReflect(), false) if err != nil { diff --git a/proto/checkinit.go b/proto/checkinit.go index d7c99235..3e9a6a2f 100644 --- a/proto/checkinit.go +++ b/proto/checkinit.go @@ -12,6 +12,12 @@ import ( // CheckInitialized returns an error if any required fields in m are not set. func CheckInitialized(m Message) error { + // Treat a nil message interface as an "untyped" empty message, + // which we assume to have no required fields. + if m == nil { + return nil + } + return checkInitialized(m.ProtoReflect()) } diff --git a/proto/encode.go b/proto/encode.go index fa738a15..862c38b9 100644 --- a/proto/encode.go +++ b/proto/encode.go @@ -74,12 +74,22 @@ type MarshalOptions struct { // Marshal returns the wire-format encoding of m. func Marshal(m Message) ([]byte, error) { + // Treat nil message interface as an empty message; nothing to output. + if m == nil { + return nil, nil + } + out, err := MarshalOptions{}.marshal(nil, m.ProtoReflect()) return out.Buf, err } // Marshal returns the wire-format encoding of m. func (o MarshalOptions) Marshal(m Message) ([]byte, error) { + // Treat nil message interface as an empty message; nothing to output. + if m == nil { + return nil, nil + } + out, err := o.marshal(nil, m.ProtoReflect()) return out.Buf, err } @@ -87,6 +97,11 @@ func (o MarshalOptions) Marshal(m Message) ([]byte, error) { // MarshalAppend appends the wire-format encoding of m to b, // returning the result. func (o MarshalOptions) MarshalAppend(b []byte, m Message) ([]byte, error) { + // Treat nil message interface as an empty message; nothing to append. + if m == nil { + return b, nil + } + out, err := o.marshal(b, m.ProtoReflect()) return out.Buf, err } diff --git a/proto/merge.go b/proto/merge.go index df72f989..27ab1a68 100644 --- a/proto/merge.go +++ b/proto/merge.go @@ -21,6 +21,9 @@ import ( // It is semantically equivalent to unmarshaling the encoded form of src // into dst with the UnmarshalOptions.Merge option specified. func Merge(dst, src Message) { + // TODO: Should nil src be treated as semantically equivalent to a + // untyped, read-only, empty message? What about a nil dst? + dstMsg, srcMsg := dst.ProtoReflect(), src.ProtoReflect() if dstMsg.Descriptor() != srcMsg.Descriptor() { panic("descriptor mismatch") diff --git a/proto/nil_test.go b/proto/nil_test.go index 06e76aec..d4563a84 100644 --- a/proto/nil_test.go +++ b/proto/nil_test.go @@ -27,14 +27,12 @@ func TestNil(t *testing.T) { }{{ label: "Size", test: func() { proto.Size(nil) }, - panic: true, }, { label: "Size", test: func() { proto.Size(nilMsg) }, }, { label: "Marshal", test: func() { proto.Marshal(nil) }, - panic: true, }, { label: "Marshal", test: func() { proto.Marshal(nilMsg) }, diff --git a/proto/size.go b/proto/size.go index a4e72bd9..11ba8414 100644 --- a/proto/size.go +++ b/proto/size.go @@ -18,6 +18,11 @@ func Size(m Message) int { // Size returns the size in bytes of the wire-format encoding of m. func (o MarshalOptions) Size(m Message) int { + // Treat a nil message interface as an empty message; nothing to output. + if m == nil { + return 0 + } + return sizeMessage(m.ProtoReflect()) }