From 7ebcd88e666494339cb9c24494604e1d64c0fce6 Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Wed, 12 Nov 2025 18:55:38 -0800 Subject: [PATCH] Automated rollback of commit 6d93331d946f4fa79cbdcffb208375dfbeb08d23. PiperOrigin-RevId: 831632187 --- .../protobuf/internal/descriptor_pool_test.py | 51 +------------------ .../protobuf/internal/factory_test2.proto | 9 ---- .../google/protobuf/pyext/extension_dict.cc | 22 ++++---- python/google/protobuf/pyext/message.cc | 10 ++-- 4 files changed, 17 insertions(+), 75 deletions(-) diff --git a/python/google/protobuf/internal/descriptor_pool_test.py b/python/google/protobuf/internal/descriptor_pool_test.py index 7ad8b39b5f..cd42189b6c 100644 --- a/python/google/protobuf/internal/descriptor_pool_test.py +++ b/python/google/protobuf/internal/descriptor_pool_test.py @@ -409,13 +409,9 @@ class DescriptorPoolTestBase(object): factory_test2 = self.pool.FindFileByName( 'google/protobuf/internal/factory_test2.proto') another_field = factory_test2.extensions_by_name['another_field'] - message_field1 = factory_test2.extensions_by_name['message_field1'] - message_field2 = factory_test2.extensions_by_name['message_field2'] extensions = self.pool.FindAllExtensions(factory1_message) - expected_extension_numbers = set( - [one_more_field, another_field, message_field1, message_field2] - ) + expected_extension_numbers = set([one_more_field, another_field]) self.assertEqual(expected_extension_numbers, set(extensions)) # Verify that mutating the returned list does not affect the pool. extensions.append('unexpected_element') @@ -440,49 +436,6 @@ class DescriptorPoolTestBase(object): with self.assertRaises(KeyError): extension = self.pool.FindExtensionByNumber(factory1_message, 1234567) - def testExtensionsLenFromParsed(self): - factory1_message = self.pool.FindMessageTypeByName( - 'google.protobuf.python.internal.Factory1Message' - ) - # Build factory_test2.proto which will put extensions to the pool - self.pool.FindFileByName( - 'google/protobuf/internal/factory_test2.proto' - ) - - message_class = message_factory.GetMessageClass(factory1_message) - message = message_class() - self.assertEqual(len(message.Extensions), 0) - message.ParseFromString(b'\xda\x3e\000\xe2\x3e\000') - - self.assertEqual(len(message.Extensions), 2) - - # Verify consistency with related methods. - self.assertEqual(len(list(message.Extensions)), 2) - self.assertEqual(len(message.ListFields()), 2) - - def testExtensionsLenFromSet(self): - factory1_message = self.pool.FindMessageTypeByName( - 'google.protobuf.python.internal.Factory1Message' - ) - # Build factory_test2.proto which will put extensions to the pool - self.pool.FindFileByName( - 'google/protobuf/internal/factory_test2.proto' - ) - - message_class = message_factory.GetMessageClass(factory1_message) - message = message_class() - self.assertEqual(len(message.Extensions), 0) - extension1 = self.pool.FindExtensionByNumber(factory1_message, 1003) - extension2 = self.pool.FindExtensionByNumber(factory1_message, 1004) - message.Extensions[extension1].a = 1 - message.Extensions[extension2].a = 2 - - self.assertEqual(len(message.Extensions), 2) - - # Verify consistency with related methods. - self.assertEqual(len(list(message.Extensions)), 2) - self.assertEqual(len(message.ListFields()), 2) - def testExtensionsAreNotFields(self): with self.assertRaises(KeyError): self.pool.FindFieldByName('google.protobuf.python.internal.another_field') @@ -1677,7 +1630,7 @@ class FallBackDBTest(unittest.TestCase): def testFindAllExtensions(self): extensions = self.pool.FindAllExtensions(self.message_desc) - self.assertEqual(len(extensions), 4) + self.assertEqual(len(extensions), 2) def testIgnoreBadFindExtensionByNumber(self): file_desc = self.bad_pool.FindFileByName( diff --git a/python/google/protobuf/internal/factory_test2.proto b/python/google/protobuf/internal/factory_test2.proto index 51376ae53c..12551e03a7 100644 --- a/python/google/protobuf/internal/factory_test2.proto +++ b/python/google/protobuf/internal/factory_test2.proto @@ -78,12 +78,3 @@ message MessageWithOption { optional int32 field1 = 1; } - -message FactoryMessageExtension { - optional int32 a = 1; -} - -extend Factory1Message { - optional FactoryMessageExtension message_field1 = 1003; - optional FactoryMessageExtension message_field2 = 1004; -} diff --git a/python/google/protobuf/pyext/extension_dict.cc b/python/google/protobuf/pyext/extension_dict.cc index f19f8297a1..63422e7b6d 100644 --- a/python/google/protobuf/pyext/extension_dict.cc +++ b/python/google/protobuf/pyext/extension_dict.cc @@ -20,7 +20,6 @@ #include "google/protobuf/dynamic_message.h" #include "google/protobuf/message.h" #include "google/protobuf/pyext/descriptor.h" -#include "google/protobuf/pyext/descriptor_pool.h" #include "google/protobuf/pyext/message.h" #include "google/protobuf/pyext/message_factory.h" #include "google/protobuf/pyext/repeated_composite_container.h" @@ -49,11 +48,11 @@ static Py_ssize_t len(ExtensionDict* self) { for (size_t i = 0; i < fields.size(); ++i) { if (fields[i]->is_extension()) { - // When using the default descriptor pool, avoid exposing extensions that - // happened to be linked in from C++ but not imported via Python. This is - // for consistency with the pure Python implementation. - if (fields[i]->file()->pool() == GetDefaultDescriptorPool()->pool && - fields[i]->message_type() != nullptr && + // With C++ descriptors, the field can always be retrieved, but for + // unknown extensions which have not been imported in Python code, there + // is no message class and we cannot retrieve the value. + // ListFields() has the same behavior. + if (fields[i]->message_type() != nullptr && message_factory::GetMessageClass( cmessage::GetFactoryForMessage(self->parent), fields[i]->message_type()) == nullptr) { @@ -409,12 +408,11 @@ PyObject* IterNext(PyObject* _self) { index = self->index; ++self->index; if (self->fields[index]->is_extension()) { - // When using the default descriptor pool, avoid exposing extensions that - // happened to be linked in from C++ but not imported via Python. This is - // for consistency with the pure Python implementation. - if (self->fields[index]->file()->pool() == - GetDefaultDescriptorPool()->pool && - self->fields[index]->message_type() != nullptr && + // With C++ descriptors, the field can always be retrieved, but for + // unknown extensions which have not been imported in Python code, there + // is no message class and we cannot retrieve the value. + // ListFields() has the same behavior. + if (self->fields[index]->message_type() != nullptr && message_factory::GetMessageClass( cmessage::GetFactoryForMessage(self->extension_dict->parent), self->fields[index]->message_type()) == nullptr) { diff --git a/python/google/protobuf/pyext/message.cc b/python/google/protobuf/pyext/message.cc index 11dbd1b622..0137b1ea3b 100644 --- a/python/google/protobuf/pyext/message.cc +++ b/python/google/protobuf/pyext/message.cc @@ -2091,11 +2091,11 @@ static PyObject* ListFields(CMessage* self) { if (extension_field == nullptr) { return nullptr; } - // When using the default descriptor pool, avoid exposing extensions that - // happened to be linked in from C++ but not imported via Python. This is - // for consistency with the pure Python implementation. - if (fields[i]->file()->pool() == GetDefaultDescriptorPool()->pool && - fields[i]->message_type() != nullptr && + // With C++ descriptors, the field can always be retrieved, but for + // unknown extensions which have not been imported in Python code, there + // is no message class and we cannot retrieve the value. + // TODO: consider building the class on the fly! + if (fields[i]->message_type() != nullptr && message_factory::GetMessageClass(GetFactoryForMessage(self), fields[i]->message_type()) == nullptr) {