Skip to content

Commit

Permalink
Fix JSONPb marshaler panics when input is nil interface (grpc-ecosyst…
Browse files Browse the repository at this point in the history
  • Loading branch information
jhump authored and achew22 committed May 3, 2018
1 parent 18a8e43 commit 7dc3ff1
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 15 deletions.
19 changes: 17 additions & 2 deletions runtime/marshal_json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func TestJSONBuiltinsnmarshal(t *testing.T) {
func TestJSONBuiltinUnmarshalField(t *testing.T) {
var m runtime.JSONBuiltin
for _, fixt := range builtinFieldFixtures {
dest := reflect.New(reflect.TypeOf(fixt.data))
dest := alloc(reflect.TypeOf(fixt.data))
if err := m.Unmarshal([]byte(fixt.json), dest.Interface()); err != nil {
t.Errorf("m.Unmarshal(%q, dest) failed with %v; want success", fixt.json, err)
}
Expand All @@ -95,6 +95,14 @@ func TestJSONBuiltinUnmarshalField(t *testing.T) {
}
}

func alloc(t reflect.Type) reflect.Value {
if t == nil {
return reflect.ValueOf(new(interface{}))
} else {
return reflect.New(t)
}
}

func TestJSONBuiltinUnmarshalFieldKnownErrors(t *testing.T) {
var m runtime.JSONBuiltin
for _, fixt := range builtinKnownErrors {
Expand Down Expand Up @@ -167,7 +175,7 @@ func TestJSONBuiltinDecoderFields(t *testing.T) {
for _, fixt := range builtinFieldFixtures {
r := strings.NewReader(fixt.json)
dec := m.NewDecoder(r)
dest := reflect.New(reflect.TypeOf(fixt.data))
dest := alloc(reflect.TypeOf(fixt.data))
if err := dec.Decode(dest.Interface()); err != nil {
t.Errorf("dec.Decode(dest) failed with %v; want success; data = %q", err, fixt.json)
}
Expand Down Expand Up @@ -204,6 +212,13 @@ var (
{data: (*string)(nil), json: "null"},
{data: new(empty.Empty), json: "{}"},
{data: examplepb.NumericEnum_ONE, json: "1"},
{data: nil, json: "null"},
{data: (*string)(nil), json: "null"},
{data: []interface{}{nil, "foo", -1.0, 1.234, true}, json: `[null,"foo",-1,1.234,true]`},
{
data: map[string]interface{}{"bar": nil, "baz": -1.0, "fiz": 1.234, "foo": true},
json: `{"bar":null,"baz":-1,"fiz":1.234,"foo":true}`,
},
{
data: (*examplepb.NumericEnum)(proto.Int32(int32(examplepb.NumericEnum_ONE))),
json: "1",
Expand Down
7 changes: 4 additions & 3 deletions runtime/marshal_jsonpb.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,7 @@ func (*JSONPb) ContentType() string {
return "application/json"
}

// Marshal marshals "v" into JSON
// Currently it can marshal only proto.Message.
// TODO(yugui) Support fields of primitive types in a message.
// Marshal marshals "v" into JSON.
func (j *JSONPb) Marshal(v interface{}) ([]byte, error) {
if _, ok := v.(proto.Message); !ok {
return j.marshalNonProtoField(v)
Expand Down Expand Up @@ -58,6 +56,9 @@ func (j *JSONPb) marshalTo(w io.Writer, v interface{}) error {
// i.e. primitive types, enums; pointers to primitives or enums; maps from
// integer/string types to primitives/enums/pointers to messages.
func (j *JSONPb) marshalNonProtoField(v interface{}) ([]byte, error) {
if v == nil {
return []byte("null"), nil
}
rv := reflect.ValueOf(v)
for rv.Kind() == reflect.Ptr {
if rv.IsNil() {
Expand Down
18 changes: 8 additions & 10 deletions runtime/marshal_jsonpb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,25 +121,23 @@ func TestJSONPbMarshal(t *testing.T) {

func TestJSONPbMarshalFields(t *testing.T) {
var m runtime.JSONPb
for _, spec := range []struct {
val interface{}
want string
}{} {
buf, err := m.Marshal(spec.val)
m.EnumsAsInts = true // builtin fixtures include an enum, expected to be marshaled as int
for _, spec := range builtinFieldFixtures {
buf, err := m.Marshal(spec.data)
if err != nil {
t.Errorf("m.Marshal(%#v) failed with %v; want success", spec.val, err)
t.Errorf("m.Marshal(%#v) failed with %v; want success", spec.data, err)
}
if got, want := string(buf), spec.want; got != want {
t.Errorf("m.Marshal(%#v) = %q; want %q", spec.val, got, want)
if got, want := string(buf), spec.json; got != want {
t.Errorf("m.Marshal(%#v) = %q; want %q", spec.data, got, want)
}
}

m.EnumsAsInts = true
m.EnumsAsInts = false
buf, err := m.Marshal(examplepb.NumericEnum_ONE)
if err != nil {
t.Errorf("m.Marshal(%#v) failed with %v; want success", examplepb.NumericEnum_ONE, err)
}
if got, want := string(buf), "1"; got != want {
if got, want := string(buf), `"ONE"`; got != want {
t.Errorf("m.Marshal(%#v) = %q; want %q", examplepb.NumericEnum_ONE, got, want)
}
}
Expand Down

0 comments on commit 7dc3ff1

Please sign in to comment.