Przeglądaj źródła

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>
Damien Neil 6 lat temu
rodzic
commit
0c9f0a9c5d

+ 1 - 2
encoding/protojson/decode_test.go

@@ -2011,7 +2011,7 @@ func TestUnmarshal(t *testing.T) {
 		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 @@ func TestUnmarshal(t *testing.T) {
 				Value:   b,
 			}
 		}(),
-		wantErr: true,
 	}, {
 		desc: "Any with partial required and AllowPartial",
 		umo: protojson.UnmarshalOptions{

+ 1 - 2
encoding/protojson/encode_test.go

@@ -1563,7 +1563,7 @@ func TestMarshal(t *testing.T) {
 		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 @@ func TestMarshal(t *testing.T) {
   "@type": "pb2.PartialRequired",
   "optString": "embedded inside Any"
 }`,
-		wantErr: true,
 	}, {
 		desc: "Any with partial required and AllowPartial",
 		mo: protojson.MarshalOptions{

+ 2 - 8
encoding/protojson/well_known_types.go

@@ -178,11 +178,8 @@ func (o MarshalOptions) marshalAny(m pref.Message) error {
 	}
 
 	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 @@ func (o UnmarshalOptions) unmarshalAny(m pref.Message) error {
 	}
 	// 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) {

+ 1 - 4
encoding/prototext/decode.go

@@ -463,11 +463,8 @@ func (o UnmarshalOptions) unmarshalAny(tfield [2]text.Value, m pref.Message) err
 		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) {

+ 1 - 2
encoding/prototext/decode_test.go

@@ -1429,7 +1429,7 @@ value: "some bytes"
 			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: "some bytes"
 				Value:   b,
 			}
 		}(),
-		wantErr: true,
 	}, {
 		desc: "Any with invalid UTF-8",
 		umo: prototext.UnmarshalOptions{

+ 1 - 4
encoding/prototext/encode.go

@@ -354,11 +354,8 @@ func (o MarshalOptions) marshalAny(m pref.Message) (text.Value, error) {
 		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) {

+ 1 - 2
encoding/prototext/encode_test.go

@@ -1196,7 +1196,7 @@ value: "\n\x13embedded inside Any\x12\x0b\n\tinception"
 }
 `,
 	}, {
-		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 @@ value: "\n\x13embedded inside Any\x12\x0b\n\tinception"
   opt_string: "embedded inside Any"
 }
 `,
-		wantErr: true,
 	}, {
 		desc: "Any with invalid UTF-8",
 		mo: prototext.MarshalOptions{