encoding/jsonpb: add unmarshal option to ignore unknown fields

This feature seems to be used quite a bit, and the conformance tests
treat this as required, perhaps as a "required option" since the
developer guide states:

"Proto3 JSON parser should reject unknown fields by default but may
provide an option to ignore unknown fields in parsing."

Also, all invalid UTF-8 errors in skipped values are also returned as it
is similar to a parse error, except it is a non-fatal one.

Change-Id: Ia26e9a355daecdbf99af23f3061353fffa32d47d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/174017
Reviewed-by: Damien Neil <dneil@google.com>

encoding/jsonpb/decode.go

@@ -29,11 +29,13 @@ func Unmarshal(m proto.Message, b []byte) error {
 type UnmarshalOptions struct {
 	pragma.NoUnkeyedLiterals
 
-	// AllowPartial accepts input for messages that will result in missing
-	// required fields. If AllowPartial is false (the default), Unmarshal will
-	// return error if there are any missing required fields.
+	// If AllowPartial is set, input for messages that will result in missing
+	// required fields will not return an error.
 	AllowPartial bool
 
+	// If DiscardUnknown is set, unknown fields are ignored.
+	DiscardUnknown bool
+
 	// Resolver is the registry used for type lookups when unmarshaling extensions
 	// and processing Any. If Resolver is not set, unmarshaling will default to
 	// using protoregistry.GlobalTypes.
@@ -217,7 +219,12 @@ Loop:
 
 		if fd == nil {
 			// Field is unknown.
-			// TODO: Provide option to ignore unknown message fields.
+			if o.DiscardUnknown {
+				if err := skipJSONValue(o.decoder); !nerr.Merge(err) {
+					return err
+				}
+				continue
+			}
 			return newError("%v contains unknown field %s", msgType.FullName(), jval)
 		}
 

encoding/jsonpb/decode_test.go

@@ -2008,18 +2008,14 @@ func TestUnmarshal(t *testing.T) {
 			Resolver: preg.NewTypes((&pb2.Nested{}).ProtoReflect().Type()),
 		},
 		inputMessage: &knownpb.Any{},
-		inputText: `{
-  "@type": "foo/pb2.Nested"
-}`,
-		wantMessage: &knownpb.Any{TypeUrl: "foo/pb2.Nested"},
+		inputText:    `{"@type": "foo/pb2.Nested"}`,
+		wantMessage:  &knownpb.Any{TypeUrl: "foo/pb2.Nested"},
 	}, {
 		desc:         "Any without registered type",
 		umo:          jsonpb.UnmarshalOptions{Resolver: preg.NewTypes()},
 		inputMessage: &knownpb.Any{},
-		inputText: `{
-  "@type": "foo/pb2.Nested"
-}`,
-		wantErr: true,
+		inputText:    `{"@type": "foo/pb2.Nested"}`,
+		wantErr:      true,
 	}, {
 		desc: "Any with missing required error",
 		umo: jsonpb.UnmarshalOptions{
@@ -2129,17 +2125,9 @@ func TestUnmarshal(t *testing.T) {
   "value": {},
   "@type": "type.googleapis.com/google.protobuf.Empty"
 }`,
-		wantMessage: func() proto.Message {
-			m := &knownpb.Empty{}
-			b, err := proto.MarshalOptions{Deterministic: true}.Marshal(m)
-			if err != nil {
-				t.Fatalf("error in binary marshaling message for Any.value: %v", err)
-			}
-			return &knownpb.Any{
-				TypeUrl: "type.googleapis.com/google.protobuf.Empty",
-				Value:   b,
-			}
-		}(),
+		wantMessage: &knownpb.Any{
+			TypeUrl: "type.googleapis.com/google.protobuf.Empty",
+		},
 	}, {
 		desc: "Any with missing Empty",
 		umo: jsonpb.UnmarshalOptions{
@@ -2499,6 +2487,109 @@ func TestUnmarshal(t *testing.T) {
 				Paths: []string{"foo_bar", "bar_foo"},
 			},
 		},
+	}, {
+		desc:         "DiscardUnknown: regular messages",
+		umo:          jsonpb.UnmarshalOptions{DiscardUnknown: true},
+		inputMessage: &pb3.Nests{},
+		inputText: `{
+  "sNested": {
+    "unknown": {
+      "foo": 1,
+	  "bar": [1, 2, 3]
+    }
+  },
+  "unknown": "not known"
+}`,
+		wantMessage: &pb3.Nests{SNested: &pb3.Nested{}},
+	}, {
+		desc:         "DiscardUnknown: repeated",
+		umo:          jsonpb.UnmarshalOptions{DiscardUnknown: true},
+		inputMessage: &pb2.Nests{},
+		inputText: `{
+  "rptNested": [
+    {"unknown": "blah"},
+	{"optString": "hello"}
+  ]
+}`,
+		wantMessage: &pb2.Nests{
+			RptNested: []*pb2.Nested{
+				{},
+				{OptString: scalar.String("hello")},
+			},
+		},
+	}, {
+		desc:         "DiscardUnknown: map",
+		umo:          jsonpb.UnmarshalOptions{DiscardUnknown: true},
+		inputMessage: &pb3.Maps{},
+		inputText: `{
+  "strToNested": {
+    "nested_one": {
+	  "unknown": "what you see is not"
+    }
+  }
+}`,
+		wantMessage: &pb3.Maps{
+			StrToNested: map[string]*pb3.Nested{
+				"nested_one": {},
+			},
+		},
+	}, {
+		desc:         "DiscardUnknown: extension",
+		umo:          jsonpb.UnmarshalOptions{DiscardUnknown: true},
+		inputMessage: &pb2.Extensions{},
+		inputText: `{
+  "[pb2.opt_ext_nested]": {
+	"unknown": []
+  }
+}`,
+		wantMessage: func() proto.Message {
+			m := &pb2.Extensions{}
+			setExtension(m, pb2.E_OptExtNested, &pb2.Nested{})
+			return m
+		}(),
+	}, {
+		desc:         "DiscardUnknown: Empty",
+		umo:          jsonpb.UnmarshalOptions{DiscardUnknown: true},
+		inputMessage: &knownpb.Empty{},
+		inputText:    `{"unknown": "something"}`,
+		wantMessage:  &knownpb.Empty{},
+	}, {
+		desc:         "DiscardUnknown: Any without type",
+		umo:          jsonpb.UnmarshalOptions{DiscardUnknown: true},
+		inputMessage: &knownpb.Any{},
+		inputText: `{
+  "value": {"foo": "bar"},
+  "unknown": true
+}`,
+		wantMessage: &knownpb.Any{},
+	}, {
+		desc: "DiscardUnknown: Any",
+		umo: jsonpb.UnmarshalOptions{
+			DiscardUnknown: true,
+			Resolver:       preg.NewTypes((&pb2.Nested{}).ProtoReflect().Type()),
+		},
+		inputMessage: &knownpb.Any{},
+		inputText: `{
+  "@type": "foo/pb2.Nested",
+  "unknown": "none"
+}`,
+		wantMessage: &knownpb.Any{
+			TypeUrl: "foo/pb2.Nested",
+		},
+	}, {
+		desc: "DiscardUnknown: Any with Empty",
+		umo: jsonpb.UnmarshalOptions{
+			DiscardUnknown: true,
+			Resolver:       preg.NewTypes((&knownpb.Empty{}).ProtoReflect().Type()),
+		},
+		inputMessage: &knownpb.Any{},
+		inputText: `{
+  "@type": "type.googleapis.com/google.protobuf.Empty",
+  "value": {"unknown": 47}
+}`,
+		wantMessage: &knownpb.Any{
+			TypeUrl: "type.googleapis.com/google.protobuf.Empty",
+		},
 	}}
 
 	for _, tt := range tests {

encoding/jsonpb/well_known_types.go

@@ -219,6 +219,10 @@ func (o UnmarshalOptions) unmarshalAny(m pref.Message) error {
 		o.decoder.Read() // Read json.EndObject.
 		return nil
 	}
+	if o.DiscardUnknown && err == errMissingType {
+		// Treat all fields as unknowns, similar to an empty object.
+		return skipJSONValue(o.decoder)
+	}
 	var nerr errors.NonFatal
 	if !nerr.Merge(err) {
 		return errors.New("google.protobuf.Any: %v", err)
@@ -260,6 +264,7 @@ func (o UnmarshalOptions) unmarshalAny(m pref.Message) error {
 }
 
 var errEmptyObject = errors.New(`empty object`)
+var errMissingType = errors.New(`missing "@type" field`)
 
 // findTypeURL returns the "@type" field value from the given JSON bytes. It is
 // expected that the given bytes start with json.StartObject. It returns
@@ -284,7 +289,7 @@ Loop:
 			if typeURL == "" {
 				// Did not find @type field.
 				if numFields > 0 {
-					return "", errors.New(`missing "@type" field`)
+					return "", errMissingType
 				}
 				return "", errEmptyObject
 			}
@@ -298,7 +303,7 @@ Loop:
 			}
 			if name != "@type" {
 				// Skip value.
-				if err := skipJSONValue(dec); err != nil {
+				if err := skipJSONValue(dec); !nerr.Merge(err) {
 					return "", err
 				}
 				continue
@@ -331,7 +336,6 @@ Loop:
 // JSON value. It relies on Decoder.Read returning an error if the types are
 // not in valid sequence.
 func skipJSONValue(dec *json.Decoder) error {
-	// Ignore non-fatal errors, do not return nerr.E.
 	var nerr errors.NonFatal
 	jval, err := dec.Read()
 	if !nerr.Merge(err) {
@@ -350,7 +354,7 @@ func skipJSONValue(dec *json.Decoder) error {
 				return nil
 			case json.Name:
 				// Skip object field value.
-				if err := skipJSONValue(dec); err != nil {
+				if err := skipJSONValue(dec); !nerr.Merge(err) {
 					return err
 				}
 			}
@@ -367,13 +371,13 @@ func skipJSONValue(dec *json.Decoder) error {
 				return err
 			default:
 				// Skip array item.
-				if err := skipJSONValue(dec); err != nil {
+				if err := skipJSONValue(dec); !nerr.Merge(err) {
 					return err
 				}
 			}
 		}
 	}
-	return nil
+	return nerr.E
 }
 
 // unmarshalAnyValue unmarshals the given custom-type message from the JSON
@@ -403,6 +407,12 @@ func (o UnmarshalOptions) unmarshalAnyValue(m pref.Message) error {
 			}
 			switch name {
 			default:
+				if o.DiscardUnknown {
+					if err := skipJSONValue(o.decoder); !nerr.Merge(err) {
+						return err
+					}
+					continue
+				}
 				return errors.New("unknown field %q", name)
 
 			case "@type":
@@ -463,6 +473,7 @@ func (o MarshalOptions) marshalEmpty(pref.Message) error {
 }
 
 func (o UnmarshalOptions) unmarshalEmpty(pref.Message) error {
+	var nerr errors.NonFatal
 	jval, err := o.decoder.Read()
 	if err != nil {
 		return err
@@ -470,14 +481,30 @@ func (o UnmarshalOptions) unmarshalEmpty(pref.Message) error {
 	if jval.Type() != json.StartObject {
 		return unexpectedJSONError{jval}
 	}
-	jval, err = o.decoder.Read()
-	if err != nil {
-		return err
-	}
-	if jval.Type() != json.EndObject {
-		return unexpectedJSONError{jval}
+
+	for {
+		jval, err := o.decoder.Read()
+		if !nerr.Merge(err) {
+			return err
+		}
+		switch jval.Type() {
+		case json.EndObject:
+			return nerr.E
+
+		case json.Name:
+			if o.DiscardUnknown {
+				if err := skipJSONValue(o.decoder); !nerr.Merge(err) {
+					return err
+				}
+				continue
+			}
+			name, _ := jval.Name()
+			return errors.New("unknown field %q", name)
+
+		default:
+			return unexpectedJSONError{jval}
+		}
 	}
-	return nil
 }
 
 // The JSON representation for Struct is a JSON object that contains the encoded

internal/cmd/conformance/failure_list_go.txt

@@ -1,7 +1 @@
-Required.Proto3.JsonInput.IgnoreUnknownJsonFalse.ProtobufOutput
-Required.Proto3.JsonInput.IgnoreUnknownJsonNull.ProtobufOutput
-Required.Proto3.JsonInput.IgnoreUnknownJsonNumber.ProtobufOutput
-Required.Proto3.JsonInput.IgnoreUnknownJsonObject.ProtobufOutput
-Required.Proto3.JsonInput.IgnoreUnknownJsonString.ProtobufOutput
-Required.Proto3.JsonInput.IgnoreUnknownJsonTrue.ProtobufOutput
 Recommended.Proto3.JsonInput.FieldMaskInvalidCharacter

internal/cmd/conformance/main.go

@@ -69,7 +69,9 @@ func handle(req *pb.ConformanceRequest) *pb.ConformanceResponse {
 	case *pb.ConformanceRequest_ProtobufPayload:
 		err = proto.Unmarshal(p.ProtobufPayload, msg)
 	case *pb.ConformanceRequest_JsonPayload:
-		err = jsonpb.Unmarshal(msg, []byte(p.JsonPayload))
+		err = jsonpb.UnmarshalOptions{
+			DiscardUnknown: req.TestCategory == pb.TestCategory_JSON_IGNORE_UNKNOWN_PARSING_TEST,
+		}.Unmarshal(msg, []byte(p.JsonPayload))
 	default:
 		return &pb.ConformanceResponse{
 			Result: &pb.ConformanceResponse_RuntimeError{