C#: Ignore invalid enum string values in JSON, when invalid fields are being ignored.

PiperOrigin-RevId: 605310357

conformance/failure_list_csharp.txt

@@ -13,9 +13,3 @@ Required.Proto3.JsonInput.OneofFieldNullSecond.JsonOutput
 Required.Proto3.JsonInput.OneofFieldNullSecond.ProtobufOutput
 Recommended.Proto3.ValueRejectInfNumberValue.JsonOutput
 Recommended.Proto3.ValueRejectNanNumberValue.JsonOutput
-Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
-Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput
-Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
-Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
-Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput
-Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput

csharp/src/Google.Protobuf.Test/JsonParserTest.cs

@@ -13,6 +13,7 @@ using Google.Protobuf.WellKnownTypes;
 using NUnit.Framework;
 using ProtobufTestMessages.Proto2;
 using ProtobufTestMessages.Proto3;
+using ProtobufUnittest;
 using System;
 using UnitTest.Issues.TestProtos;
 
@@ -924,6 +925,52 @@ namespace Google.Protobuf
             Assert.Throws<InvalidProtocolBufferException>(() => TestAllTypes.Parser.ParseJson(json));
         }
 
+        [Test]
+        public void Enum_InvalidString_IgnoreUnknownFields()
+        {
+            // When ignoring unknown fields, invalid enum value strings are ignored too.
+            // This test uses TestProto3Optional so we can check we're not just setting the field to the 0 value.
+            var parser = new JsonParser(JsonParser.Settings.Default.WithIgnoreUnknownFields(true));
+            string json = "{ \"optionalNestedEnum\": \"NOT_A_VALID_VALUE\" }";
+            var parsed = parser.Parse<TestProto3Optional>(json);
+            Assert.IsFalse(parsed.HasOptionalNestedEnum);
+        }
+
+        [Test]
+        public void RepeatedEnum_InvalidString_IgnoreUnknownFields()
+        {
+            // When ignoring unknown fields, invalid enum value strings are ignored too.
+            // For a repeated field, the value is removed entirely.
+            var parser = new JsonParser(JsonParser.Settings.Default.WithIgnoreUnknownFields(true));
+            string json = "{ \"repeatedForeignEnum\": [ \"FOREIGN_FOO\", \"NOT_A_VALID_VALUE\", \"FOREIGN_BAR\" ] }";
+            var parsed = parser.Parse<TestAllTypes>(json);
+            var expected = new[] { TestProtos.ForeignEnum.ForeignFoo, TestProtos.ForeignEnum.ForeignBar };
+            Assert.AreEqual(expected, parsed.RepeatedForeignEnum);
+        }
+
+        [Test]
+        public void EnumValuedMap_InvalidString_IgnoreUnknownFields()
+        {
+            // When ignoring unknown fields, invalid enum value strings are ignored too.
+            // For a map field, the entry is removed entirely.
+            var parser = new JsonParser(JsonParser.Settings.Default.WithIgnoreUnknownFields(true));
+            string json = "{ \"mapInt32Enum\": { \"1\": \"MAP_ENUM_BAR\", \"2\": \"NOT_A_VALID_VALUE\" } }";
+            var parsed = parser.Parse<TestMap>(json);
+            Assert.AreEqual(1, parsed.MapInt32Enum.Count);
+            Assert.AreEqual(MapEnum.Bar, parsed.MapInt32Enum[1]);
+            Assert.False(parsed.MapInt32Enum.ContainsKey(2));
+        }
+
+        [Test]
+        public void Enum_InvalidNumber_IgnoreUnknownFields()
+        {
+            // Even when ignoring unknown fields, fail for non-integer numeric values, because
+            // they could *never* be valid.
+            var parser = new JsonParser(JsonParser.Settings.Default.WithIgnoreUnknownFields(true));
+            string json = "{ \"singleForeignEnum\": 5.5 }";
+            Assert.Throws<InvalidProtocolBufferException>(() => parser.Parse<TestAllTypes>(json));
+        }
+
         [Test]
         public void OneofDuplicate_Invalid()
         {

csharp/src/Google.Protobuf/JsonParser.cs

@@ -1,4 +1,4 @@
-#region Copyright notice and license
+#region Copyright notice and license
 // Protocol Buffers - Google's data interchange format
 // Copyright 2015 Google Inc.  All rights reserved.
 //
@@ -219,8 +219,10 @@ namespace Google.Protobuf
             }
             else
             {
-                var value = ParseSingleValue(field, tokenizer);
-                field.Accessor.SetValue(message, value);
+                if (TryParseSingleValue(field, tokenizer, out var value))
+                {
+                    field.Accessor.SetValue(message, value);
+                }
             }
         }
 
@@ -241,12 +243,14 @@ namespace Google.Protobuf
                     return;
                 }
                 tokenizer.PushBack(token);
-                object value = ParseSingleValue(field, tokenizer);
-                if (value == null)
+                if (TryParseSingleValue(field, tokenizer, out object value))
                 {
-                    throw new InvalidProtocolBufferException("Repeated field elements cannot be null");
+                    if (value == null)
+                    {
+                        throw new InvalidProtocolBufferException("Repeated field elements cannot be null");
+                    }
+                    list.Add(value);
                 }
-                list.Add(value);
             }
         }
 
@@ -276,8 +280,10 @@ namespace Google.Protobuf
                     return;
                 }
                 object key = ParseMapKey(keyField, token.StringValue);
-                object value = ParseSingleValue(valueField, tokenizer);
-                dictionary[key] = value ?? throw new InvalidProtocolBufferException("Map values must not be null");
+                if (TryParseSingleValue(valueField, tokenizer, out object value))
+                {
+                    dictionary[key] = value ?? throw new InvalidProtocolBufferException("Map values must not be null");
+                }
             }
         }
 
@@ -293,7 +299,15 @@ namespace Google.Protobuf
                 field.EnumType.FullName == NullValueDescriptor.FullName;
         }
 
-        private object ParseSingleValue(FieldDescriptor field, JsonTokenizer tokenizer)
+        /// <summary>
+        /// Attempts to parse a single value from the JSON. When the value is completely invalid,
+        /// this will still throw an exception; when it's "conditionally invalid" (currently meaning
+        /// "when there's an unknown enum string value") the method returns false instead.
+        /// </summary>
+        /// <returns>
+        /// true if the value was parsed successfully; false for an ignorable parse failure.
+        /// </returns>
+        private bool TryParseSingleValue(FieldDescriptor field, JsonTokenizer tokenizer, out object value)
         {
             var token = tokenizer.Next();
             if (token.Type == JsonToken.TokenType.Null)
@@ -302,13 +316,17 @@ namespace Google.Protobuf
                 // dynamically.
                 if (IsGoogleProtobufValueField(field))
                 {
-                    return Value.ForNull();
+                    value = Value.ForNull();
                 }
-                if (IsGoogleProtobufNullValueField(field))
+                else if (IsGoogleProtobufNullValueField(field))
                 {
-                    return NullValue.NullValue;
+                    value = NullValue.NullValue;
                 }
-                return null;
+                else
+                {
+                    value = null;
+                }
+                return true;
             }
 
             var fieldType = field.FieldType;
@@ -327,7 +345,8 @@ namespace Google.Protobuf
                     tokenizer.PushBack(token);
                     IMessage subMessage = NewMessageForField(field);
                     Merge(subMessage, tokenizer);
-                    return subMessage;
+                    value = subMessage;
+                    return true;
                 }
             }
 
@@ -337,18 +356,26 @@ namespace Google.Protobuf
                 case JsonToken.TokenType.False:
                     if (fieldType == FieldType.Bool)
                     {
-                        return token.Type == JsonToken.TokenType.True;
+                        value = token.Type == JsonToken.TokenType.True;
+                        return true;
                     }
                     // Fall through to "we don't support this type for this case"; could duplicate the behaviour of the default
                     // case instead, but this way we'd only need to change one place.
                     goto default;
                 case JsonToken.TokenType.StringValue:
-                    return ParseSingleStringValue(field, token.StringValue);
+                    if (field.FieldType != FieldType.Enum)
+                    {
+                        value = ParseSingleStringValue(field, token.StringValue);
+                        return true;
+                    }
+                    else
+                    {
+                        return TryParseEnumStringValue(field, token.StringValue, out value);
+                    }
                 // Note: not passing the number value itself here, as we may end up storing the string value in the token too.
                 case JsonToken.TokenType.Number:
-                    return ParseSingleNumberValue(field, token);
-                case JsonToken.TokenType.Null:
-                    throw new NotImplementedException("Haven't worked out what to do for null yet");
+                    value = ParseSingleNumberValue(field, token);
+                    return true;
                 default:
                     throw new InvalidProtocolBufferException("Unsupported JSON token type " + token.Type + " for field type " + fieldType);
             }
@@ -694,18 +721,32 @@ namespace Google.Protobuf
                     ValidateInfinityAndNan(text, float.IsPositiveInfinity(f), float.IsNegativeInfinity(f), float.IsNaN(f));
                     return f;
                 case FieldType.Enum:
-                    var enumValue = field.EnumType.FindValueByName(text);
-                    if (enumValue == null)
-                    {
-                        throw new InvalidProtocolBufferException($"Invalid enum value: {text} for enum type: {field.EnumType.FullName}");
-                    }
-                    // Just return it as an int, and let the CLR convert it.
-                    return enumValue.Number;
+                    throw new InvalidOperationException($"Use TryParseEnumStringValue for enums");
                 default:
                     throw new InvalidProtocolBufferException($"Unsupported conversion from JSON string for field type {field.FieldType}");
             }
         }
 
+        private bool TryParseEnumStringValue(FieldDescriptor field, string text, out object value)
+        {
+            var enumValue = field.EnumType.FindValueByName(text);
+            if (enumValue == null)
+            {
+                if (settings.IgnoreUnknownFields)
+                {
+                    value = null;
+                    return false;
+                }
+                else
+                {
+                    throw new InvalidProtocolBufferException($"Invalid enum value: {text} for enum type: {field.EnumType.FullName}");
+                }
+            }
+            // Just return it as an int, and let the CLR convert it.
+            value = enumValue.Number;
+            return true;
+        }
+
         /// <summary>
         /// Creates a new instance of the message type for the given field.
         /// </summary>