encoding/protojson, encoding/prototext: always allow partial Any
The wire representation of the contents of an Any is a `bytes` field
containing the the wire encoding of the contained message. The in-memory
representation is just these bytes.
The wire (un)marshaler has no special handling for Any values, and
happily accepts whatever bytes are present.
The text and JSON marshalers will unmarshal the bytes in an Any,
and the unmarshalers will marshal the Any to produce the in-memory
representation. This makes them stricter than the wire (un)marshaler:
Marshaling an Any which contains invalid data to text/JSON fails, while
marshaling it to wire format does not. This does make some sense, since
the Any already contains wire-format data; validation is performed at an
earlier time when producing that data.
This change brings the text and JSON (un)marshal functions a bit more
into alignment with the wire format by never performing checks for
required fields on the contents of an Any.
This has the advantage of consistently performing checks for required
fields exactly once per marshal/unmarshal operation, and over the same
data no matter which encoding is used. It also eliminates the one case
where a required field check is considered "non-fatal", generating an
error but not terminating the (un)marshal operation.
Change-Id: I70c62419d37ea0a07cb73c3ee2d26c0b0bec724b
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/182982
Reviewed-by: Herbie Ong <herbie@google.com>
diff --git a/encoding/protojson/decode_test.go b/encoding/protojson/decode_test.go
index e3427b8..c680c8a 100644
--- a/encoding/protojson/decode_test.go
+++ b/encoding/protojson/decode_test.go
@@ -2011,7 +2011,7 @@
inputText: `{"@type": "foo/pb2.Nested"}`,
wantErr: true,
}, {
- desc: "Any with missing required error",
+ desc: "Any with missing required",
umo: protojson.UnmarshalOptions{
Resolver: preg.NewTypes(pimpl.Export{}.MessageTypeOf(&pb2.PartialRequired{})),
},
@@ -2036,7 +2036,6 @@
Value: b,
}
}(),
- wantErr: true,
}, {
desc: "Any with partial required and AllowPartial",
umo: protojson.UnmarshalOptions{
diff --git a/encoding/protojson/encode_test.go b/encoding/protojson/encode_test.go
index 691fcf7..174a6ba 100644
--- a/encoding/protojson/encode_test.go
+++ b/encoding/protojson/encode_test.go
@@ -1563,7 +1563,7 @@
input: &anypb.Any{TypeUrl: "foo/pb2.Nested"},
wantErr: true,
}, {
- desc: "Any with missing required error",
+ desc: "Any with missing required",
mo: protojson.MarshalOptions{
Resolver: preg.NewTypes(pimpl.Export{}.MessageTypeOf(&pb2.PartialRequired{})),
},
@@ -1587,7 +1587,6 @@
"@type": "pb2.PartialRequired",
"optString": "embedded inside Any"
}`,
- wantErr: true,
}, {
desc: "Any with partial required and AllowPartial",
mo: protojson.MarshalOptions{
diff --git a/encoding/protojson/well_known_types.go b/encoding/protojson/well_known_types.go
index 8895e5b..c42cd61 100644
--- a/encoding/protojson/well_known_types.go
+++ b/encoding/protojson/well_known_types.go
@@ -178,11 +178,8 @@
}
em := emt.New()
- // TODO: If binary unmarshaling returns required not set error, need to
- // return another required not set error that contains both the path to this
- // field and the path inside the embedded message.
err = proto.UnmarshalOptions{
- AllowPartial: o.AllowPartial,
+ AllowPartial: true, // never check required fields inside an Any
Resolver: o.Resolver,
}.Unmarshal(valueVal.Bytes(), em.Interface())
if !nerr.Merge(err) {
@@ -253,11 +250,8 @@
}
// Serialize the embedded message and assign the resulting bytes to the
// proto value field.
- // TODO: If binary marshaling returns required not set error, need to return
- // another required not set error that contains both the path to this field
- // and the path inside the embedded message.
b, err := proto.MarshalOptions{
- AllowPartial: o.AllowPartial,
+ AllowPartial: true, // never check required fields inside an Any
Deterministic: true,
}.Marshal(em.Interface())
if !nerr.Merge(err) {
diff --git a/encoding/prototext/decode.go b/encoding/prototext/decode.go
index 4444de6..2d8f9e1 100644
--- a/encoding/prototext/decode.go
+++ b/encoding/prototext/decode.go
@@ -463,11 +463,8 @@
return err
}
// Serialize the embedded message and assign the resulting bytes to the value field.
- // TODO: If binary marshaling returns required not set error, need to
- // return another required not set error that contains both the path to this
- // field and the path inside the embedded message.
b, err := proto.MarshalOptions{
- AllowPartial: o.AllowPartial,
+ AllowPartial: true, // never check required fields inside an Any
Deterministic: true,
}.Marshal(m2.Interface())
if !nerr.Merge(err) {
diff --git a/encoding/prototext/decode_test.go b/encoding/prototext/decode_test.go
index 39d470a..c681591 100644
--- a/encoding/prototext/decode_test.go
+++ b/encoding/prototext/decode_test.go
@@ -1429,7 +1429,7 @@
TypeUrl: "foo.com/pb2.Nested",
},
}, {
- desc: "Any expanded with missing required error",
+ desc: "Any expanded with missing required",
umo: prototext.UnmarshalOptions{
Resolver: preg.NewTypes(pimpl.Export{}.MessageTypeOf(&pb2.PartialRequired{})),
},
@@ -1455,7 +1455,6 @@
Value: b,
}
}(),
- wantErr: true,
}, {
desc: "Any with invalid UTF-8",
umo: prototext.UnmarshalOptions{
diff --git a/encoding/prototext/encode.go b/encoding/prototext/encode.go
index bf2d6a5..f3aba56 100644
--- a/encoding/prototext/encode.go
+++ b/encoding/prototext/encode.go
@@ -354,11 +354,8 @@
return text.Value{}, err
}
em := emt.New().Interface()
- // TODO: If binary unmarshaling returns required not set error, need to
- // return another required not set error that contains both the path to this
- // field and the path inside the embedded message.
err = proto.UnmarshalOptions{
- AllowPartial: o.AllowPartial,
+ AllowPartial: true,
Resolver: o.Resolver,
}.Unmarshal(value.Bytes(), em)
if !nerr.Merge(err) {
diff --git a/encoding/prototext/encode_test.go b/encoding/prototext/encode_test.go
index cece731..ea97c89 100644
--- a/encoding/prototext/encode_test.go
+++ b/encoding/prototext/encode_test.go
@@ -1196,7 +1196,7 @@
}
`,
}, {
- desc: "Any expanded with missing required error",
+ desc: "Any expanded with missing required",
mo: prototext.MarshalOptions{
Resolver: preg.NewTypes(pimpl.Export{}.MessageTypeOf(&pb2.PartialRequired{})),
},
@@ -1220,7 +1220,6 @@
opt_string: "embedded inside Any"
}
`,
- wantErr: true,
}, {
desc: "Any with invalid UTF-8",
mo: prototext.MarshalOptions{
