Просмотр исходного кода

encoding/jsonpb,textpb: fix handling of duplicate oneof fields

Unmarshaling should fail if multiple fields in the same oneof exists in
the input.

Change-Id: I76efd88681a50c18f3eaf770c9eb48727efb412b
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/170517
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
Herbie Ong 6 лет назад
Родитель
Сommit
8a1d460e50
4 измененных файлов с 44 добавлено и 11 удалено
  1. 10 0
      encoding/jsonpb/decode.go
  2. 20 0
      encoding/jsonpb/decode_test.go
  3. 10 0
      encoding/textpb/decode.go
  4. 4 11
      encoding/textpb/decode_test.go

+ 10 - 0
encoding/jsonpb/decode.go

@@ -153,6 +153,7 @@ func (o UnmarshalOptions) unmarshalFields(m pref.Message, skipTypeURL bool) erro
 	var nerr errors.NonFatal
 	var reqNums set.Ints
 	var seenNums set.Ints
+	var seenOneofs set.Ints
 
 	msgType := m.Type()
 	knownFields := m.KnownFields()
@@ -237,6 +238,15 @@ Loop:
 				return errors.New("%v|%q: %v", fd.FullName(), name, err)
 			}
 		} else {
+			// If field is a oneof, check if it has already been set.
+			if od := fd.OneofType(); od != nil {
+				idx := uint64(od.Index())
+				if seenOneofs.Has(idx) {
+					return errors.New("%v: oneof is already set", od.FullName())
+				}
+				seenOneofs.Set(idx)
+			}
+
 			// Required or optional fields.
 			if err := o.unmarshalSingular(knownFields, fd); !nerr.Merge(err) {
 				return errors.New("%v|%q: %v", fd.FullName(), name, err)

+ 20 - 0
encoding/jsonpb/decode_test.go

@@ -677,6 +677,26 @@ func TestUnmarshal(t *testing.T) {
 				},
 			},
 		},
+	}, {
+		desc:         "oneof set to more than one field",
+		inputMessage: &pb3.Oneofs{},
+		inputText: `{
+  "oneofEnum": "ZERO",
+  "oneofString": "hello"
+}`,
+		wantErr: true,
+	}, {
+		desc:         "oneof set to null and value",
+		inputMessage: &pb3.Oneofs{},
+		inputText: `{
+  "oneofEnum": "ZERO",
+  "oneofString": null
+}`,
+		wantMessage: &pb3.Oneofs{
+			Union: &pb3.Oneofs_OneofEnum{
+				OneofEnum: pb3.Enum_ZERO,
+			},
+		},
 	}, {
 		desc:         "repeated null fields",
 		inputMessage: &pb2.Repeats{},

+ 10 - 0
encoding/textpb/decode.go

@@ -104,6 +104,7 @@ func (o UnmarshalOptions) unmarshalMessage(tmsg [][2]text.Value, m pref.Message)
 	xtTypes := knownFields.ExtensionTypes()
 	var reqNums set.Ints
 	var seenNums set.Ints
+	var seenOneofs set.Ints
 
 	for _, tfield := range tmsg {
 		tkey := tfield[0]
@@ -158,6 +159,15 @@ func (o UnmarshalOptions) unmarshalMessage(tmsg [][2]text.Value, m pref.Message)
 				return err
 			}
 		} else {
+			// If field is a oneof, check if it has already been set.
+			if od := fd.OneofType(); od != nil {
+				idx := uint64(od.Index())
+				if seenOneofs.Has(idx) {
+					return errors.New("oneof %v is already set", od.FullName())
+				}
+				seenOneofs.Set(idx)
+			}
+
 			// Required or optional fields.
 			num := uint64(fd.Number())
 			if seenNums.Has(num) {

+ 4 - 11
encoding/textpb/decode_test.go

@@ -523,20 +523,13 @@ oneof_nested: {
 			},
 		},
 	}, {
-		desc:         "oneof set to last value",
+		desc:         "oneof set to more than one field",
 		inputMessage: &pb3.Oneofs{},
 		inputText: `
-oneof_nested: {
-  s_string: "nested message"
-}
-oneof_enum: TEN
-oneof_string: "wins"
+oneof_enum: ZERO
+oneof_string: "hello"
 `,
-		wantMessage: &pb3.Oneofs{
-			Union: &pb3.Oneofs_OneofString{
-				OneofString: "wins",
-			},
-		},
+		wantErr: true,
 	}, {
 		desc:         "repeated scalar using same field name",
 		inputMessage: &pb2.Repeats{},