encoding/jsonpb: fix encoding of empty google.protobuf.Value
Description of message Value states:
`Value` represents a dynamically typed value which can be either null, a
number, a string, a boolean, a recursive struct value, or a list of values. A
producer of value is expected to set one of that variants, absence of any
variant indicates an error.
https://github.com/protocolbuffers/protobuf/blob/3.7.x/src/google/protobuf/struct.proto#L57-L60
Previous implementation was following C++ lib behavior.
Change-Id: Id51792e2fc8cc465a05a978e63410d3b6802b522
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/168901
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
diff --git a/encoding/jsonpb/encode.go b/encoding/jsonpb/encode.go
index a9ca581..9bfadda 100644
--- a/encoding/jsonpb/encode.go
+++ b/encoding/jsonpb/encode.go
@@ -111,14 +111,8 @@
continue
}
- // An empty google.protobuf.Value should NOT be marshaled out.
- // Hence need to check ahead for this.
- val := knownFields.Get(num)
- if isEmptyKnownValue(val, fd.MessageType()) {
- continue
- }
-
name := fd.JSONName()
+ val := knownFields.Get(num)
if err := e.WriteName(name); !nerr.Merge(err) {
return err
}
diff --git a/encoding/jsonpb/encode_test.go b/encoding/jsonpb/encode_test.go
index 74dd570..d6a2e62 100644
--- a/encoding/jsonpb/encode_test.go
+++ b/encoding/jsonpb/encode_test.go
@@ -1008,15 +1008,15 @@
input: &knownpb.Empty{},
want: `{}`,
}, {
- desc: "Value empty",
- input: &knownpb.Value{},
- want: ``,
+ desc: "Value empty",
+ input: &knownpb.Value{},
+ wantErr: true,
}, {
desc: "Value empty field",
input: &pb2.KnownTypes{
OptValue: &knownpb.Value{},
},
- want: `{}`,
+ wantErr: true,
}, {
desc: "Value contains NullValue",
input: &knownpb.Value{Kind: &knownpb.Value_NullValue{}},
@@ -1531,7 +1531,7 @@
},
input: func() proto.Message {
m := &knownpb.Value{}
- b, err := proto.MarshalOptions{Deterministic: true}.Marshal(m)
+ b, err := proto.Marshal(m)
if err != nil {
t.Fatalf("error in binary marshaling message for Any.value: %v", err)
}
@@ -1540,9 +1540,7 @@
Value: b,
}
}(),
- want: `{
- "@type": "type.googleapis.com/google.protobuf.Value"
-}`,
+ wantErr: true,
}, {
desc: "Any with Duration",
mo: jsonpb.MarshalOptions{
@@ -1641,7 +1639,9 @@
{Kind: &knownpb.Value_ListValue{}},
},
},
- OptValue: &knownpb.Value{},
+ OptValue: &knownpb.Value{
+ Kind: &knownpb.Value_StringValue{"world"},
+ },
OptEmpty: &knownpb.Empty{},
OptAny: &knownpb.Any{
TypeUrl: "google.protobuf.Empty",
@@ -1671,6 +1671,7 @@
{},
[]
],
+ "optValue": "world",
"optEmpty": {},
"optAny": {
"@type": "google.protobuf.Empty"
diff --git a/encoding/jsonpb/well_known_types.go b/encoding/jsonpb/well_known_types.go
index a9efdd3..101e4ba 100644
--- a/encoding/jsonpb/well_known_types.go
+++ b/encoding/jsonpb/well_known_types.go
@@ -128,10 +128,6 @@
// with corresponding custom JSON encoding of the embedded message as a
// field.
if isCustomType(emt.FullName()) {
- // An empty google.protobuf.Value should NOT be marshaled out.
- if isEmptyKnownValue(pref.ValueOf(em), emt) {
- return nil
- }
e.WriteName("value")
return e.marshalCustomType(em)
}
@@ -176,14 +172,6 @@
return e.marshalList(val.List(), fd)
}
-// isEmptyKnownValue returns true if given val is of type google.protobuf.Value
-// and does not have any of its oneof fields set.
-func isEmptyKnownValue(val pref.Value, md pref.MessageDescriptor) bool {
- return md != nil &&
- md.FullName() == "google.protobuf.Value" &&
- val.Message().KnownFields().Len() == 0
-}
-
func (e encoder) marshalKnownValue(m pref.Message) error {
msgType := m.Type()
fieldDescs := msgType.Oneofs().Get(0).Fields()
@@ -200,8 +188,8 @@
return e.marshalSingular(val, fd)
}
- // None of the fields are set.
- return nil
+ // Return error if none of the fields are set.
+ return errors.New("%s: none of the variants is set", msgType.FullName())
}
const (
