Skip to content

Commit 86abf35

Browse files
antongrbincopybara-github
antongrbin
authored and
copybara-github
committed
Python JSON parser: Ignore invalid enum string values if ignore_unknown_fields is set (#15887)
# Motivation This PR fixes failing conformance tests for python with name `IgnoreUnknownEnumStringValue`. The JSON parsing spec was discussed in #7392. Recent equivalent changes for other languages: * Swift: apple/swift-protobuf#1345 * C#: #15758 # Changes - 1st commit is a noop refactoring to make relevant _ConvertScalarFieldValue invocations localized - 2nd commit introduces the child exception of `ParseError` named `EnumStringValueParseError` which is suppressed if `ignore_unknown_fields` is set - 3rd commit updates the conformance test failure lists Closes #15887 COPYBARA_INTEGRATE_REVIEW=#15887 from noom:anton/7392/fix-python-test fbcc93a PiperOrigin-RevId: 619288323
1 parent 7e033c0 commit 86abf35

File tree

7 files changed

+150
-85
lines changed

7 files changed

+150
-85
lines changed

‎conformance/failure_list_python.txt

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +0,0 @@
1-
Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
2-
Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput
3-
Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
4-
Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
5-
Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput
6-
Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
7-
Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
8-
Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput
9-
Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
10-
Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
11-
Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput
12-
Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
13-
Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
14-
Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
15-
Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
16-
Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
17-
Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput
18-
Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput
19-
Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput
20-
Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput

‎conformance/failure_list_python_cpp.txt

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,3 @@
66
#
77
# TODO: insert links to corresponding bugs tracking the issue.
88
# Should we use GitHub issues or the Google-internal bug tracker?
9-
Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
10-
Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput
11-
Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
12-
Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
13-
Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput
14-
Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
15-
Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
16-
Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput
17-
Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
18-
Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
19-
Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput
20-
Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
21-
Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
22-
Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
23-
Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
24-
Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
25-
Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput
26-
Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput
27-
Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput
28-
Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput

‎conformance/failure_list_python_upb.txt

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +0,0 @@
1-
Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
2-
Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput
3-
Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
4-
Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
5-
Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput
6-
Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
7-
Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
8-
Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput
9-
Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
10-
Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInMapValue.ProtobufOutput
11-
Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInOptionalField.ProtobufOutput
12-
Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedField.ProtobufOutput
13-
Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
14-
Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
15-
Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
16-
Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInRepeatedPart.ProtobufOutput
17-
Recommended.Editions_Proto2.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput
18-
Recommended.Editions_Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput
19-
Recommended.Proto2.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput
20-
Recommended.Proto3.JsonInput.IgnoreUnknownEnumStringValueInMapPart.ProtobufOutput

‎python/google/protobuf/internal/json_format_test.py

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1008,7 +1008,7 @@ def testParseEnumValue(self):
10081008
# Proto3 accepts numeric unknown enums.
10091009
text = '{"enumValue": 12345}'
10101010
json_format.Parse(text, message)
1011-
# Proto2 does not accept unknown enums.
1011+
# Proto2 does not accept numeric unknown enums.
10121012
message = unittest_pb2.TestAllTypes()
10131013
self.assertRaisesRegex(
10141014
json_format.ParseError,
@@ -1020,6 +1020,80 @@ def testParseEnumValue(self):
10201020
message,
10211021
)
10221022

1023+
def testParseUnknownEnumStringValue_Scalar_Proto2(self):
1024+
message = json_format_pb2.TestNumbers()
1025+
text = '{"a": "UNKNOWN_STRING_VALUE"}'
1026+
json_format.Parse(text, message, ignore_unknown_fields=True)
1027+
1028+
self.assertFalse(message.HasField('a'))
1029+
1030+
def testParseErrorForUnknownEnumValue_ScalarWithoutIgnore_Proto2(self):
1031+
message = json_format_pb2.TestNumbers()
1032+
self.assertRaisesRegex(
1033+
json_format.ParseError,
1034+
'Invalid enum value',
1035+
json_format.Parse, '{"a": "UNKNOWN_STRING_VALUE"}', message)
1036+
1037+
def testParseUnknownEnumStringValue_Repeated_Proto2(self):
1038+
message = json_format_pb2.TestRepeatedEnum()
1039+
text = '{"repeatedEnum": ["UNKNOWN_STRING_VALUE", "BUFFER"]}'
1040+
json_format.Parse(text, message, ignore_unknown_fields=True)
1041+
1042+
self.assertEqual(len(message.repeated_enum), 1)
1043+
self.assertTrue(message.repeated_enum[0] == json_format_pb2.BUFFER)
1044+
1045+
def testParseUnknownEnumStringValue_Map_Proto2(self):
1046+
message = json_format_pb2.TestMapOfEnums()
1047+
text = '{"enumMap": {"key1": "BUFFER", "key2": "UNKNOWN_STRING_VALUE"}}'
1048+
json_format.Parse(text, message, ignore_unknown_fields=True)
1049+
1050+
self.assertTrue(message.enum_map['key1'] == json_format_pb2.BUFFER)
1051+
self.assertFalse('key2' in message.enum_map)
1052+
1053+
def testParseUnknownEnumStringValue_ExtensionField_Proto2(self):
1054+
message = json_format_pb2.TestMessageWithExtension()
1055+
text = """
1056+
{"[protobuf_unittest.TestExtension.enum_ext]": "UNKNOWN_STRING_VALUE"}
1057+
"""
1058+
json_format.Parse(text, message, ignore_unknown_fields=True)
1059+
1060+
self.assertFalse(json_format_pb2.TestExtension.enum_ext in
1061+
message.Extensions)
1062+
1063+
def testParseUnknownEnumStringValue_ExtensionFieldWithoutIgnore_Proto2(self):
1064+
message = json_format_pb2.TestMessageWithExtension()
1065+
text = """
1066+
{"[protobuf_unittest.TestExtension.enum_ext]": "UNKNOWN_STRING_VALUE"}
1067+
"""
1068+
self.assertRaisesRegex(
1069+
json_format.ParseError,
1070+
'Invalid enum value',
1071+
json_format.Parse, text, message)
1072+
1073+
def testParseUnknownEnumStringValue_Scalar_Proto3(self):
1074+
message = json_format_proto3_pb2.TestMessage()
1075+
text = '{"enumValue": "UNKNOWN_STRING_VALUE"}'
1076+
1077+
json_format.Parse(text, message, ignore_unknown_fields=True)
1078+
self.assertEqual(message.enum_value, 0)
1079+
1080+
def testParseUnknownEnumStringValue_Repeated_Proto3(self):
1081+
message = json_format_proto3_pb2.TestMessage()
1082+
text = '{"repeatedEnumValue": ["UNKNOWN_STRING_VALUE", "FOO"]}'
1083+
json_format.Parse(text, message, ignore_unknown_fields=True)
1084+
1085+
self.assertEqual(len(message.repeated_enum_value), 1)
1086+
self.assertTrue(message.repeated_enum_value[0] ==
1087+
json_format_proto3_pb2.FOO)
1088+
1089+
def testParseUnknownEnumStringValue_Map_Proto3(self):
1090+
message = json_format_proto3_pb2.MapOfEnums()
1091+
text = '{"map": {"key1": "FOO", "key2": "UNKNOWN_STRING_VALUE"}}'
1092+
json_format.Parse(text, message, ignore_unknown_fields=True)
1093+
1094+
self.assertTrue(message.map['key1'] == json_format_proto3_pb2.FOO)
1095+
self.assertFalse('key2' in message.map)
1096+
10231097
def testBytes(self):
10241098
message = json_format_proto3_pb2.TestMessage()
10251099
# Test url base64

‎python/google/protobuf/json_format.py

Lines changed: 62 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,12 @@ class ParseError(Error):
7070
"""Thrown in case of parsing error."""
7171

7272

73+
class EnumStringValueParseError(ParseError):
74+
"""Thrown if unknown string enum value is encountered.
75+
This exception is suppressed if ignore_unknown_fields is set.
76+
"""
77+
78+
7379
def MessageToJson(
7480
message,
7581
preserving_proto_field_name=False,
@@ -658,11 +664,8 @@ def _ConvertFieldValuePair(self, js, message, path):
658664
path, name, index
659665
)
660666
)
661-
getattr(message, field.name).append(
662-
_ConvertScalarFieldValue(
663-
item, field, '{0}.{1}[{2}]'.format(path, name, index)
664-
)
665-
)
667+
self._ConvertAndAppendScalar(
668+
message, field, item, '{0}.{1}[{2}]'.format(path, name, index))
666669
elif field.cpp_type == descriptor.FieldDescriptor.CPPTYPE_MESSAGE:
667670
if field.is_extension:
668671
sub_message = message.Extensions[field]
@@ -672,17 +675,9 @@ def _ConvertFieldValuePair(self, js, message, path):
672675
self.ConvertMessage(value, sub_message, '{0}.{1}'.format(path, name))
673676
else:
674677
if field.is_extension:
675-
message.Extensions[field] = _ConvertScalarFieldValue(
676-
value, field, '{0}.{1}'.format(path, name)
677-
)
678+
self._ConvertAndSetScalarExtension(message, field, value, '{0}.{1}'.format(path, name))
678679
else:
679-
setattr(
680-
message,
681-
field.name,
682-
_ConvertScalarFieldValue(
683-
value, field, '{0}.{1}'.format(path, name)
684-
),
685-
)
680+
self._ConvertAndSetScalar(message, field, value, '{0}.{1}'.format(path, name))
686681
except ParseError as e:
687682
if field and field.containing_oneof is None:
688683
raise ParseError(
@@ -795,11 +790,7 @@ def _ConvertStructMessage(self, value, message, path):
795790
def _ConvertWrapperMessage(self, value, message, path):
796791
"""Convert a JSON representation into Wrapper message."""
797792
field = message.DESCRIPTOR.fields_by_name['value']
798-
setattr(
799-
message,
800-
'value',
801-
_ConvertScalarFieldValue(value, field, path='{0}.value'.format(path)),
802-
)
793+
self._ConvertAndSetScalar(message, field, value, path='{0}.value'.format(path))
803794

804795
def _ConvertMapFieldValue(self, value, message, field, path):
805796
"""Convert map field value for a message map field.
@@ -832,9 +823,51 @@ def _ConvertMapFieldValue(self, value, message, field, path):
832823
'{0}[{1}]'.format(path, key_value),
833824
)
834825
else:
835-
getattr(message, field.name)[key_value] = _ConvertScalarFieldValue(
836-
value[key], value_field, path='{0}[{1}]'.format(path, key_value)
837-
)
826+
self._ConvertAndSetScalarToMapKey(
827+
message,
828+
field,
829+
key_value,
830+
value[key],
831+
path='{0}[{1}]'.format(path, key_value))
832+
833+
def _ConvertAndSetScalarExtension(self, message, extension_field, js_value, path):
834+
"""Convert scalar from js_value and assign it to message.Extensions[extension_field]."""
835+
try:
836+
message.Extensions[extension_field] = _ConvertScalarFieldValue(
837+
js_value, extension_field, path)
838+
except EnumStringValueParseError:
839+
if not self.ignore_unknown_fields:
840+
raise
841+
842+
def _ConvertAndSetScalar(self, message, field, js_value, path):
843+
"""Convert scalar from js_value and assign it to message.field."""
844+
try:
845+
setattr(
846+
message,
847+
field.name,
848+
_ConvertScalarFieldValue(js_value, field, path))
849+
except EnumStringValueParseError:
850+
if not self.ignore_unknown_fields:
851+
raise
852+
853+
def _ConvertAndAppendScalar(self, message, repeated_field, js_value, path):
854+
"""Convert scalar from js_value and append it to message.repeated_field."""
855+
try:
856+
getattr(message, repeated_field.name).append(
857+
_ConvertScalarFieldValue(js_value, repeated_field, path))
858+
except EnumStringValueParseError:
859+
if not self.ignore_unknown_fields:
860+
raise
861+
862+
def _ConvertAndSetScalarToMapKey(self, message, map_field, converted_key, js_value, path):
863+
"""Convert scalar from 'js_value' and add it to message.map_field[converted_key]."""
864+
try:
865+
getattr(message, map_field.name)[converted_key] = _ConvertScalarFieldValue(
866+
js_value, map_field.message_type.fields_by_name['value'], path,
867+
)
868+
except EnumStringValueParseError:
869+
if not self.ignore_unknown_fields:
870+
raise
838871

839872

840873
def _ConvertScalarFieldValue(value, field, path, require_str=False):
@@ -851,6 +884,7 @@ def _ConvertScalarFieldValue(value, field, path, require_str=False):
851884
852885
Raises:
853886
ParseError: In case of convert problems.
887+
EnumStringValueParseError: In case of unknown enum string value.
854888
"""
855889
try:
856890
if field.cpp_type in _INT_TYPES:
@@ -882,7 +916,9 @@ def _ConvertScalarFieldValue(value, field, path, require_str=False):
882916
number = int(value)
883917
enum_value = field.enum_type.values_by_number.get(number, None)
884918
except ValueError as e:
885-
raise ParseError(
919+
# Since parsing to integer failed and lookup in values_by_name didn't
920+
# find this name, we have an enum string value which is unknown.
921+
raise EnumStringValueParseError(
886922
'Invalid enum value {0} for enum type {1}'.format(
887923
value, field.enum_type.full_name
888924
)
@@ -897,6 +933,8 @@ def _ConvertScalarFieldValue(value, field, path, require_str=False):
897933
else:
898934
return number
899935
return enum_value.number
936+
except EnumStringValueParseError as e:
937+
raise EnumStringValueParseError('{0} at {1}'.format(e, path)) from e
900938
except ParseError as e:
901939
raise ParseError('{0} at {1}'.format(e, path)) from e
902940

‎src/google/protobuf/util/json_format.proto

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ message TestMessageWithExtension {
101101
message TestExtension {
102102
extend TestMessageWithExtension {
103103
optional TestExtension ext = 100;
104+
optional EnumValue enum_ext = 101;
104105
}
105106
optional string value = 1;
106107
}
@@ -114,3 +115,11 @@ enum EnumValue {
114115
message TestDefaultEnumValue {
115116
optional EnumValue enum_value = 1 [default = DEFAULT];
116117
}
118+
119+
message TestMapOfEnums {
120+
map<string, EnumValue> enum_map = 1;
121+
}
122+
123+
message TestRepeatedEnum {
124+
repeated EnumValue repeated_enum = 1;
125+
}

‎src/google/protobuf/util/json_format_proto3.proto

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,10 @@ message MapOfObjects {
253253
map<string, M> map = 1;
254254
}
255255

256+
message MapOfEnums {
257+
map<string, EnumType> map = 1;
258+
}
259+
256260
message MapIn {
257261
string other = 1;
258262
repeated string things = 2;

0 commit comments

Comments
 (0)