Skip to content

Commit

Permalink
jsonpb: unmarshal JSON null to nil map/slice instead of empty instance (
Browse files Browse the repository at this point in the history 
#404)

* jsonpb: don't allocate memory for null-value reference types

This was missed in 748d386.

* gofmt -s -w jsonpb/jsonpb_test.go
  • Loading branch information
@tamird @cybrcodr
tamird authored and cybrcodr committed Aug 8, 2017
1 parent 748d386 commit 1909bc2
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 34 deletions.
62 changes: 33 additions & 29 deletions jsonpb/jsonpb.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -919,11 +919,13 @@ func (u *Unmarshaler) unmarshalValue(target reflect.Value, inputValue json.RawMe
if err := json.Unmarshal(inputValue, &slc); err != nil {
return err
}
len := len(slc)
target.Set(reflect.MakeSlice(targetType, len, len))
for i := 0; i < len; i++ {
if err := u.unmarshalValue(target.Index(i), slc[i], prop); err != nil {
return err
if slc != nil {
l := len(slc)
target.Set(reflect.MakeSlice(targetType, l, l))
for i := 0; i < l; i++ {
if err := u.unmarshalValue(target.Index(i), slc[i], prop); err != nil {
return err
}
}
}
return nil
Expand All @@ -935,33 +937,35 @@ func (u *Unmarshaler) unmarshalValue(target reflect.Value, inputValue json.RawMe
if err := json.Unmarshal(inputValue, &mp); err != nil {
return err
}
target.Set(reflect.MakeMap(targetType))
var keyprop, valprop *proto.Properties
if prop != nil {
// These could still be nil if the protobuf metadata is broken somehow.
// TODO: This won't work because the fields are unexported.
// We should probably just reparse them.
//keyprop, valprop = prop.mkeyprop, prop.mvalprop
}
for ks, raw := range mp {
// Unmarshal map key. The core json library already decoded the key into a
// string, so we handle that specially. Other types were quoted post-serialization.
var k reflect.Value
if targetType.Key().Kind() == reflect.String {
k = reflect.ValueOf(ks)
} else {
k = reflect.New(targetType.Key()).Elem()
if err := u.unmarshalValue(k, json.RawMessage(ks), keyprop); err != nil {
return err
}
if mp != nil {
target.Set(reflect.MakeMap(targetType))
var keyprop, valprop *proto.Properties
if prop != nil {
// These could still be nil if the protobuf metadata is broken somehow.
// TODO: This won't work because the fields are unexported.
// We should probably just reparse them.
//keyprop, valprop = prop.mkeyprop, prop.mvalprop
}
for ks, raw := range mp {
// Unmarshal map key. The core json library already decoded the key into a
// string, so we handle that specially. Other types were quoted post-serialization.
var k reflect.Value
if targetType.Key().Kind() == reflect.String {
k = reflect.ValueOf(ks)
} else {
k = reflect.New(targetType.Key()).Elem()
if err := u.unmarshalValue(k, json.RawMessage(ks), keyprop); err != nil {
return err
}
}

// Unmarshal map value.
v := reflect.New(targetType.Elem()).Elem()
if err := u.unmarshalValue(v, raw, valprop); err != nil {
return err
// Unmarshal map value.
v := reflect.New(targetType.Elem()).Elem()
if err := u.unmarshalValue(v, raw, valprop); err != nil {
return err
}
target.SetMapIndex(k, v)
}
target.SetMapIndex(k, v)
}
return nil
}
Expand Down
31 changes: 26 additions & 5 deletions jsonpb/jsonpb_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,9 +379,9 @@ var marshalingTests = []struct {
&pb.Mappy{Strry: map[string]string{`"one"`: "two", "three": "four"}},
`{"strry":{"\"one\"":"two","three":"four"}}`},
{"map<int32, Object>", marshaler,
&pb.Mappy{Objjy: map[int32]*pb.Simple3{1: &pb.Simple3{Dub: 1}}}, `{"objjy":{"1":{"dub":1}}}`},
&pb.Mappy{Objjy: map[int32]*pb.Simple3{1: {Dub: 1}}}, `{"objjy":{"1":{"dub":1}}}`},
{"map<int32, Object>", marshalerAllOptions,
&pb.Mappy{Objjy: map[int32]*pb.Simple3{1: &pb.Simple3{Dub: 1}}}, objjyPrettyJSON},
&pb.Mappy{Objjy: map[int32]*pb.Simple3{1: {Dub: 1}}}, objjyPrettyJSON},
{"map<int64, string>", marshaler, &pb.Mappy{Buggy: map[int64]string{1234: "yup"}},
`{"buggy":{"1234":"yup"}}`},
{"map<bool, bool>", marshaler, &pb.Mappy{Booly: map[bool]bool{false: true}}, `{"booly":{"false":true}}`},
Expand All @@ -395,7 +395,7 @@ var marshalingTests = []struct {
{"proto2 map<int64, string>", marshaler, &pb.Maps{MInt64Str: map[int64]string{213: "cat"}},
`{"mInt64Str":{"213":"cat"}}`},
{"proto2 map<bool, Object>", marshaler,
&pb.Maps{MBoolSimple: map[bool]*pb.Simple{true: &pb.Simple{OInt32: proto.Int32(1)}}},
&pb.Maps{MBoolSimple: map[bool]*pb.Simple{true: {OInt32: proto.Int32(1)}}},
`{"mBoolSimple":{"true":{"oInt32":1}}}`},
{"oneof, not set", marshaler, &pb.MsgWithOneof{}, `{}`},
{"oneof, set", marshaler, &pb.MsgWithOneof{Union: &pb.MsgWithOneof_Title{"Grand Poobah"}}, `{"title":"Grand Poobah"}`},
Expand Down Expand Up @@ -486,7 +486,7 @@ func TestMarshalAnyJSONPBMarshaler(t *testing.T) {
}
// after custom marshaling, it's round-tripped through JSON decoding/encoding already,
// so the keys are sorted, whitespace is compacted, and "@type" key has been added
expected := `{"@type":"type.googleapis.com/` + dynamicMessageName +`","baz":[0,1,2,3],"foo":"bar"}`
expected := `{"@type":"type.googleapis.com/` + dynamicMessageName + `","baz":[0,1,2,3],"foo":"bar"}`
if str != expected {
t.Errorf("marshalling JSON produced incorrect output: got %s, wanted %s", str, expected)
}
Expand Down Expand Up @@ -535,7 +535,7 @@ var unmarshalingTests = []struct {
{"-Inf", Unmarshaler{}, `{"oDouble":"-Infinity"}`, &pb.Simple{ODouble: proto.Float64(math.Inf(-1))}},
{"map<int64, int32>", Unmarshaler{}, `{"nummy":{"1":2,"3":4}}`, &pb.Mappy{Nummy: map[int64]int32{1: 2, 3: 4}}},
{"map<string, string>", Unmarshaler{}, `{"strry":{"\"one\"":"two","three":"four"}}`, &pb.Mappy{Strry: map[string]string{`"one"`: "two", "three": "four"}}},
{"map<int32, Object>", Unmarshaler{}, `{"objjy":{"1":{"dub":1}}}`, &pb.Mappy{Objjy: map[int32]*pb.Simple3{1: &pb.Simple3{Dub: 1}}}},
{"map<int32, Object>", Unmarshaler{}, `{"objjy":{"1":{"dub":1}}}`, &pb.Mappy{Objjy: map[int32]*pb.Simple3{1: {Dub: 1}}}},
{"proto2 extension", Unmarshaler{}, realNumberJSON, realNumber},
{"Any with message", Unmarshaler{}, anySimpleJSON, anySimple},
{"Any with message and indent", Unmarshaler{}, anySimplePrettyJSON, anySimple},
Expand Down Expand Up @@ -645,6 +645,26 @@ func TestUnmarshaling(t *testing.T) {
}
}

func TestUnmarshalNullArray(t *testing.T) {
var repeats pb.Repeats
if err := UnmarshalString(`{"rBool":null}`, &repeats); err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(repeats, pb.Repeats{}) {
t.Errorf("got non-nil fields in [%#v]", repeats)
}
}

func TestUnmarshalNullObject(t *testing.T) {
var maps pb.Maps
if err := UnmarshalString(`{"mInt64Str":null}`, &maps); err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(maps, pb.Maps{}) {
t.Errorf("got non-nil fields in [%#v]", maps)
}
}

func TestUnmarshalNext(t *testing.T) {
// We only need to check against a few, not all of them.
tests := unmarshalingTests[:5]
Expand Down Expand Up @@ -736,6 +756,7 @@ func TestUnmarshalAnyJSONPBUnmarshaler(t *testing.T) {
const (
dynamicMessageName = "google.protobuf.jsonpb.testing.dynamicMessage"
)

func init() {
// we register the custom type below so that we can use it in Any types
proto.RegisterType((*dynamicMessage)(nil), dynamicMessageName)
Expand Down

0 comments on commit 1909bc2

Please sign in to comment.