Fix handling of repeated extension fields in PyProto JSON

protobuf-github-bot · copybara-github · commit 07ef6769c670 · 2025-09-05T11:46:02.000-07:00
Both serialize and parse path would not work on repeated extensions before these fixes. #22989 PiperOrigin-RevId: 803542611
diff --git a/python/google/protobuf/internal/json_format_test.py b/python/google/protobuf/internal/json_format_test.py
@@ -183,6 +183,46 @@ def testExtensionToDictAndBackWithScalar(self):
     json_format.ParseDict(message_dict, parsed_message)
     self.assertEqual(message, parsed_message)
 
+  def testScalarExtensionToDictAndBack(self):
+    message = unittest_pb2.TestAllExtensions()
+    message.Extensions[unittest_pb2.optional_int32_extension] = 7
+    message.Extensions[unittest_pb2.optional_string_extension] = 'hello'
+    message_dict = json_format.MessageToDict(message)
+    self.assertEqual(
+        message_dict,
+        {
+            '[proto2_unittest.optional_int32_extension]': 7,
+            '[proto2_unittest.optional_string_extension]': 'hello',
+        },
+    )
+    parsed_message = unittest_pb2.TestAllExtensions()
+    json_format.ParseDict(message_dict, parsed_message)
+    self.assertEqual(message, parsed_message)
+
+  def testRepeatedScalarExtensionToDictAndBack(self):
+    message = unittest_pb2.TestAllExtensions()
+    ext1 = unittest_pb2.repeated_int32_extension
+    message.Extensions[ext1].extend([1, 2, 3])
+    message_dict = json_format.MessageToDict(message)
+    self.assertIn('[proto2_unittest.repeated_int32_extension]', message_dict)
+    parsed_message = unittest_pb2.TestAllExtensions()
+    json_format.ParseDict(message_dict, parsed_message)
+    self.assertEqual(message, parsed_message)
+
+  def testRepeatedMessageExtensionToDictAndBack(self):
+    message = unittest_pb2.TestAllExtensions()
+    ext1 = unittest_pb2.repeated_nested_message_extension
+    sub = unittest_pb2.TestAllTypes.NestedMessage()
+    sub.bb = 1
+    message.Extensions[ext1].append(sub)
+    message_dict = json_format.MessageToDict(message)
+    self.assertIn(
+        '[proto2_unittest.repeated_nested_message_extension]', message_dict
+    )
+    parsed_message = unittest_pb2.TestAllExtensions()
+    json_format.ParseDict(message_dict, parsed_message)
+    self.assertEqual(message, parsed_message)
+
   def testJsonParseDictToAnyDoesNotAlterInput(self):
     orig_dict = {
         'int32Value': 20,
@@ -1041,7 +1081,10 @@ def testParseErrorForUnknownEnumValue_ScalarWithoutIgnore_Proto2(self):
     self.assertRaisesRegex(
         json_format.ParseError,
         'Invalid enum value',
-        json_format.Parse, '{"a": "UNKNOWN_STRING_VALUE"}', message)
+        json_format.Parse,
+        '{"a": "UNKNOWN_STRING_VALUE"}',
+        message,
+    )
 
   def testParseUnknownEnumStringValue_Repeated_Proto2(self):
     message = json_format_pb2.TestRepeatedEnum()
@@ -1066,8 +1109,9 @@ def testParseUnknownEnumStringValue_ExtensionField_Proto2(self):
     """
     json_format.Parse(text, message, ignore_unknown_fields=True)
 
-    self.assertFalse(json_format_pb2.TestExtension.enum_ext in
-                     message.Extensions)
+    self.assertFalse(
+        json_format_pb2.TestExtension.enum_ext in message.Extensions
+    )
 
   def testParseUnknownEnumStringValue_ExtensionFieldWithoutIgnore_Proto2(self):
     message = json_format_pb2.TestMessageWithExtension()
@@ -1077,7 +1121,10 @@ def testParseUnknownEnumStringValue_ExtensionFieldWithoutIgnore_Proto2(self):
     self.assertRaisesRegex(
         json_format.ParseError,
         'Invalid enum value',
-        json_format.Parse, text, message)
+        json_format.Parse,
+        text,
+        message,
+    )
 
   def testParseUnknownEnumStringValue_Scalar_Proto3(self):
     message = json_format_proto3_pb2.TestMessage()
@@ -1092,8 +1139,9 @@ def testParseUnknownEnumStringValue_Repeated_Proto3(self):
     json_format.Parse(text, message, ignore_unknown_fields=True)
 
     self.assertEqual(len(message.repeated_enum_value), 1)
-    self.assertTrue(message.repeated_enum_value[0] ==
-                    json_format_proto3_pb2.FOO)
+    self.assertTrue(
+        message.repeated_enum_value[0] == json_format_proto3_pb2.FOO
+    )
 
   def testParseUnknownEnumStringValue_Map_Proto3(self):
     message = json_format_proto3_pb2.MapOfEnums()
@@ -1155,6 +1203,28 @@ def testDuplicateField(self):
         'Failed to load JSON: duplicate key int32Value.',
     )
 
+  def testDuplicateFieldAlternateNames(self):
+    # Note: this behavior is non-spec and an oversight bug in the
+    # implementation, but would be a breaking change to fix. The duplicate field
+    # checker intends reject inputs with duplicate key names, but it only
+    # catches keys that are exact matches and not alternate spellings that
+    # correspond to the same field.
+    parsed_message = json_format_proto3_pb2.TestMessage()
+    json_format.Parse('{"int32Value": 1,"int32_value":2}', parsed_message)
+    self.assertEqual(parsed_message.int32_value, 2)
+
+  def testDuplicateFieldAlternateNamesMap(self):
+    # Note: this behavior is non-spec and an oversight bug in the
+    # implementation, but would be a breaking change to fix. The duplicate field
+    # checker intends reject inputs with duplicate key names, but it only
+    # catches keys that are exact matches and not alternate spellings that
+    # correspond to the same field.
+    parsed_message = json_format_proto3_pb2.TestMap()
+    json_format.Parse(
+        '{"int32Map": {"1": 2}, "int32_map": {"3": 4}}', parsed_message
+    )
+    self.assertEqual(parsed_message.int32_map, {3: 4})
+
   def testInvalidBoolValue(self):
     self.CheckError(
         '{"boolValue": 1}',
@@ -1458,7 +1528,13 @@ def testFieldMaskInvalidStringValue(self):
   def testInvalidAny(self):
     message = any_pb2.Any()
     text = '{"@type": "type.googleapis.com/google.protobuf.Int32Value"}'
-    self.assertRaisesRegex(json_format.ParseError, 'KeyError: \'value\'', json_format.Parse, text, message)
+    self.assertRaisesRegex(
+        json_format.ParseError,
+        "KeyError: 'value'",
+        json_format.Parse,
+        text,
+        message,
+    )
     text = '{"value": 1234}'
     self.assertRaisesRegex(
         json_format.ParseError,
@@ -1739,5 +1815,6 @@ def testManyRecursionsRaisesParseError(self):
     with self.assertRaises(json_format.ParseError):
       json_format.Parse(text, json_format_proto3_pb2.TestMessage())
 
+
 if __name__ == '__main__':
   unittest.main()
diff --git a/python/google/protobuf/json_format.py b/python/google/protobuf/json_format.py
@@ -224,10 +224,13 @@ def _RegularMessageToJsonObject(self, message, js):
 
     try:
       for field, value in fields:
-        if self.preserving_proto_field_name:
+        if field.is_extension:
+          name = '[%s]' % field.full_name
+        elif self.preserving_proto_field_name:
           name = field.name
         else:
           name = field.json_name
+
         if _IsMapEntry(field):
           # Convert a map field.
           v_field = field.message_type.fields_by_name['value']
@@ -245,9 +248,6 @@ def _RegularMessageToJsonObject(self, message, js):
         elif field.is_repeated:
           # Convert a repeated field.
           js[name] = [self._FieldToJsonObject(field, k) for k in value]
-        elif field.is_extension:
-          name = '[%s]' % field.full_name
-          js[name] = self._FieldToJsonObject(field, value)
         else:
           js[name] = self._FieldToJsonObject(field, value)
 
@@ -562,6 +562,25 @@ def _ConvertFieldValuePair(self, js, message, path):
     fields_by_json_name = dict(
         (f.json_name, f) for f in message_descriptor.fields
     )
+
+    def _ClearFieldOrExtension(message, field):
+      if field.is_extension:
+        message.ClearExtension(field)
+      else:
+        message.ClearField(field.name)
+
+    def _GetFieldOrExtension(message, field):
+      if field.is_extension:
+        return message.Extensions[field]
+      else:
+        return getattr(message, field.name)
+
+    def _SetFieldOrExtension(message, field, value):
+      if field.is_extension:
+        message.Extensions[field] = value
+      else:
+        setattr(message, field.name, value)
+
     for name in js:
       try:
         field = fields_by_json_name.get(name, None)
@@ -625,25 +644,25 @@ def _ConvertFieldValuePair(self, js, message, path):
               field.cpp_type == descriptor.FieldDescriptor.CPPTYPE_MESSAGE
               and field.message_type.full_name == 'google.protobuf.Value'
           ):
-            sub_message = getattr(message, field.name)
+            sub_message = _GetFieldOrExtension(message, field)
             sub_message.null_value = 0
           elif (
               field.cpp_type == descriptor.FieldDescriptor.CPPTYPE_ENUM
               and field.enum_type.full_name == 'google.protobuf.NullValue'
           ):
-            setattr(message, field.name, 0)
+            _SetFieldOrExtension(message, field, 0)
           else:
-            message.ClearField(field.name)
+            _ClearFieldOrExtension(message, field)
           continue
 
         # Parse field value.
         if _IsMapEntry(field):
-          message.ClearField(field.name)
+          _ClearFieldOrExtension(message, field)
           self._ConvertMapFieldValue(
               value, message, field, '{0}.{1}'.format(path, name)
           )
         elif field.is_repeated:
-          message.ClearField(field.name)
+          _ClearFieldOrExtension(message, field)
           if not isinstance(value, _LIST_LIKE):
             raise ParseError(
                 'repeated field {0} must be in [] which is {1} at {2}'.format(
@@ -653,7 +672,7 @@ def _ConvertFieldValuePair(self, js, message, path):
           if field.cpp_type == descriptor.FieldDescriptor.CPPTYPE_MESSAGE:
             # Repeated message field.
             for index, item in enumerate(value):
-              sub_message = getattr(message, field.name).add()
+              sub_message = _GetFieldOrExtension(message, field).add()
               # None is a null_value in Value.
               if (
                   item is None
@@ -683,21 +702,13 @@ def _ConvertFieldValuePair(self, js, message, path):
                   message, field, item, '{0}.{1}[{2}]'.format(path, name, index)
               )
         elif field.cpp_type == descriptor.FieldDescriptor.CPPTYPE_MESSAGE:
-          if field.is_extension:
-            sub_message = message.Extensions[field]
-          else:
-            sub_message = getattr(message, field.name)
+          sub_message = _GetFieldOrExtension(message, field)
           sub_message.SetInParent()
           self.ConvertMessage(value, sub_message, '{0}.{1}'.format(path, name))
         else:
-          if field.is_extension:
-            self._ConvertAndSetScalarExtension(
-                message, field, value, '{0}.{1}'.format(path, name)
-            )
-          else:
-            self._ConvertAndSetScalar(
-                message, field, value, '{0}.{1}'.format(path, name)
-            )
+          self._ConvertAndSetScalar(
+              message, field, value, '{0}.{1}'.format(path, name)
+          )
       except ParseError as e:
         if field and field.containing_oneof is None:
           raise ParseError(
@@ -855,34 +866,27 @@ def _ConvertMapFieldValue(self, value, message, field, path):
             path='{0}[{1}]'.format(path, key_value),
         )
 
-  def _ConvertAndSetScalarExtension(
-      self, message, extension_field, js_value, path
-  ):
-    """Convert scalar from js_value and assign it to message.Extensions[extension_field]."""
-    try:
-      message.Extensions[extension_field] = _ConvertScalarFieldValue(
-          js_value, extension_field, path
-      )
-    except EnumStringValueParseError:
-      if not self.ignore_unknown_fields:
-        raise
-
   def _ConvertAndSetScalar(self, message, field, js_value, path):
     """Convert scalar from js_value and assign it to message.field."""
     try:
-      setattr(
-          message, field.name, _ConvertScalarFieldValue(js_value, field, path)
-      )
+      value = _ConvertScalarFieldValue(js_value, field, path)
+      if field.is_extension:
+        message.Extensions[field] = value
+      else:
+        setattr(message, field.name, value)
     except EnumStringValueParseError:
       if not self.ignore_unknown_fields:
         raise
 
   def _ConvertAndAppendScalar(self, message, repeated_field, js_value, path):
     """Convert scalar from js_value and append it to message.repeated_field."""
     try:
-      getattr(message, repeated_field.name).append(
-          _ConvertScalarFieldValue(js_value, repeated_field, path)
-      )
+      if repeated_field.is_extension:
+        repeated = message.Extensions[repeated_field]
+      else:
+        repeated = getattr(message, repeated_field.name)
+      value = _ConvertScalarFieldValue(js_value, repeated_field, path)
+      repeated.append(value)
     except EnumStringValueParseError:
       if not self.ignore_unknown_fields:
         raise
