encoding/protojson: ignore unknown enum name if DiscardUnknown=true

This makes encoding/protojson pass all protobuf conformance tests. Other Similar CL: - https://go-review.googlesource.com/c/protobuf/+/350469 - https://go-review.googlesource.com/c/protobuf/+/256677 Fixes golang/protobuf#1208 Change-Id: I9f74162b80a3c7ee4750160fc59f0313345046de Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/525535 Reviewed-by: Cassondra Foesch <[email protected]> Reviewed-by: Michael Stapelberg <[email protected]> Reviewed-by: Matthew Dempsky <[email protected]>
protocolbuffers · Sep 8, 2023 · 70db1e1 · 70db1e1
1 parent f9212a8
commit 70db1e1
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 10 deletions.
diff --git a/encoding/protojson/decode.go b/encoding/protojson/decode.go
@@ -37,7 +37,7 @@ type UnmarshalOptions struct {
 	// required fields will not return an error.
 	AllowPartial bool
 
-	// If DiscardUnknown is set, unknown fields are ignored.
+	// If DiscardUnknown is set, unknown fields and enum name values are ignored.
 	DiscardUnknown bool
 
 	// Resolver is used for looking up types when unmarshaling
@@ -266,7 +266,9 @@ func (d decoder) unmarshalSingular(m protoreflect.Message, fd protoreflect.Field
 	if err != nil {
 		return err
 	}
-	m.Set(fd, val)
+	if val.IsValid() {
+		m.Set(fd, val)
+	}
 	return nil
 }
 
@@ -329,7 +331,7 @@ func (d decoder) unmarshalScalar(fd protoreflect.FieldDescriptor) (protoreflect.
 		}
 
 	case protoreflect.EnumKind:
-		if v, ok := unmarshalEnum(tok, fd); ok {
+		if v, ok := unmarshalEnum(tok, fd, d.opts.DiscardUnknown); ok {
 			return v, nil
 		}
 
@@ -474,14 +476,17 @@ func unmarshalBytes(tok json.Token) (protoreflect.Value, bool) {
 	return protoreflect.ValueOfBytes(b), true
 }
 
-func unmarshalEnum(tok json.Token, fd protoreflect.FieldDescriptor) (protoreflect.Value, bool) {
+func unmarshalEnum(tok json.Token, fd protoreflect.FieldDescriptor, discardUnknown bool) (protoreflect.Value, bool) {
 	switch tok.Kind() {
 	case json.String:
 		// Lookup EnumNumber based on name.
 		s := tok.ParsedString()
 		if enumVal := fd.Enum().Values().ByName(protoreflect.Name(s)); enumVal != nil {
 			return protoreflect.ValueOfEnum(enumVal.Number()), true
 		}
+		if discardUnknown {
+			return protoreflect.Value{}, true
+		}
 
 	case json.Number:
 		if n, ok := tok.Int(32); ok {
@@ -542,7 +547,9 @@ func (d decoder) unmarshalList(list protoreflect.List, fd protoreflect.FieldDesc
 			if err != nil {
 				return err
 			}
-			list.Append(val)
+			if val.IsValid() {
+				list.Append(val)
+			}
 		}
 	}
 
@@ -609,8 +616,9 @@ Loop:
 		if err != nil {
 			return err
 		}
-
-		mmap.Set(pkey, pval)
+		if pval.IsValid() {
+			mmap.Set(pkey, pval)
+		}
 	}
 
 	return nil

diff --git a/encoding/protojson/decode_test.go b/encoding/protojson/decode_test.go
@@ -2436,6 +2436,43 @@ func TestUnmarshal(t *testing.T) {
 		wantMessage: &anypb.Any{
 			TypeUrl: "type.googleapis.com/google.protobuf.Empty",
 		},
+	}, {
+		desc:         "DiscardUnknown: unknown enum name",
+		inputMessage: &pb3.Enums{},
+		inputText: `{
+  "sEnum": "UNNAMED"
+}`,
+		umo:         protojson.UnmarshalOptions{DiscardUnknown: true},
+		wantMessage: &pb3.Enums{},
+	}, {
+		desc:         "DiscardUnknown: repeated enum unknown name",
+		inputMessage: &pb2.Enums{},
+		inputText: `{
+  "rptEnum"      : ["TEN", 1, 42, "UNNAMED"]
+}`,
+		umo: protojson.UnmarshalOptions{DiscardUnknown: true},
+		wantMessage: &pb2.Enums{
+			RptEnum: []pb2.Enum{pb2.Enum_TEN, pb2.Enum_ONE, 42},
+		},
+	}, {
+		desc:         "DiscardUnknown: enum map value unknown name",
+		inputMessage: &pb3.Maps{},
+		inputText: `{
+  "uint64ToEnum": {
+    "1" : "ONE",
+	"2" : 2,
+	"10": 101,
+	"3": "UNNAMED"
+  }
+}`,
+		umo: protojson.UnmarshalOptions{DiscardUnknown: true},
+		wantMessage: &pb3.Maps{
+			Uint64ToEnum: map[uint64]pb3.Enum{
+				1:  pb3.Enum_ONE,
+				2:  pb3.Enum_TWO,
+				10: 101,
+			},
+		},
 	}, {
 		desc:         "weak fields",
 		inputMessage: &testpb.TestWeak{},

diff --git a/internal/conformance/failing_tests.txt b/internal/conformance/failing_tests.txt
@@ -1,3 +0,0 @@
-Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
-Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput
-Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput