From 55e50ba40679eb82269c62872116b76c5923d74d Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Thu, 8 Feb 2024 07:27:31 -0800 Subject: [PATCH] C#: Ignore invalid enum string values in JSON, when invalid fields are being ignored. PiperOrigin-RevId: 605310357 --- conformance/failure_list_csharp.txt | 6 -- .../Google.Protobuf.Test/JsonParserTest.cs | 47 +++++++++ csharp/src/Google.Protobuf/JsonParser.cs | 95 +++++++++++++------ 3 files changed, 115 insertions(+), 33 deletions(-) diff --git a/conformance/failure_list_csharp.txt b/conformance/failure_list_csharp.txt index 4d4a355b597e..7cd3728078d0 100644 --- a/conformance/failure_list_csharp.txt +++ b/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 diff --git a/csharp/src/Google.Protobuf.Test/JsonParserTest.cs b/csharp/src/Google.Protobuf.Test/JsonParserTest.cs index 4d1cbef1d2eb..ef6d63dbdc08 100644 --- a/csharp/src/Google.Protobuf.Test/JsonParserTest.cs +++ b/csharp/src/Google.Protobuf.Test/JsonParserTest.cs @@ -13,6 +13,7 @@ using NUnit.Framework; using ProtobufTestMessages.Proto2; using ProtobufTestMessages.Proto3; +using ProtobufUnittest; using System; using UnitTest.Issues.TestProtos; @@ -924,6 +925,52 @@ public void Enum_Invalid(string value) Assert.Throws(() => 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(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(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(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(() => parser.Parse(json)); + } + [Test] public void OneofDuplicate_Invalid() { diff --git a/csharp/src/Google.Protobuf/JsonParser.cs b/csharp/src/Google.Protobuf/JsonParser.cs index d73d0832ba5e..1a0da4a39236 100644 --- a/csharp/src/Google.Protobuf/JsonParser.cs +++ b/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 @@ private void MergeField(IMessage message, FieldDescriptor field, JsonTokenizer t } 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 @@ private void MergeRepeatedField(IMessage message, FieldDescriptor field, JsonTok 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 @@ private void MergeMapField(IMessage message, FieldDescriptor field, JsonTokenize 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 @@ private static bool IsGoogleProtobufNullValueField(FieldDescriptor field) field.EnumType.FullName == NullValueDescriptor.FullName; } - private object ParseSingleValue(FieldDescriptor field, JsonTokenizer tokenizer) + /// + /// 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. + /// + /// + /// true if the value was parsed successfully; false for an ignorable parse failure. + /// + private bool TryParseSingleValue(FieldDescriptor field, JsonTokenizer tokenizer, out object value) { var token = tokenizer.Next(); if (token.Type == JsonToken.TokenType.Null) @@ -302,13 +316,17 @@ private object ParseSingleValue(FieldDescriptor field, JsonTokenizer tokenizer) // 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 @@ private object ParseSingleValue(FieldDescriptor field, JsonTokenizer tokenizer) tokenizer.PushBack(token); IMessage subMessage = NewMessageForField(field); Merge(subMessage, tokenizer); - return subMessage; + value = subMessage; + return true; } } @@ -337,18 +356,26 @@ private object ParseSingleValue(FieldDescriptor field, JsonTokenizer tokenizer) 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 @@ private static object ParseSingleStringValue(FieldDescriptor field, string text) 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; + } + /// /// Creates a new instance of the message type for the given field. ///