Fix ContainerBase::DeepCopy to not modify the source object.

In the worst case, the source object is a constant (eg a global default instance) and modifying them has undefined behavior.

PiperOrigin-RevId: 673402981
pull/18157/head
Protobuf Team Bot 6 months ago committed by Copybara-Service
parent 5a68dddcf9
commit 9fa1f4f9bb
  1. 35
      python/google/protobuf/internal/reflection_cpp_test.py
  2. 16
      python/google/protobuf/pyext/message.cc

@ -0,0 +1,35 @@
# -*- coding: utf-8 -*-
# Protocol Buffers - Google's data interchange format
# Copyright 2008 Google Inc. All rights reserved.
#
# Use of this source code is governed by a BSD-style
# license that can be found in the LICENSE file or at
# https://developers.google.com/open-source/licenses/bsd
"""Unittest for reflection.py, with C++ protos linked in."""
import copy
import unittest
from google.protobuf.internal import testing_refleaks
from google.protobuf.internal import _parameterized
from google.protobuf import unittest_pb2
from google.protobuf import unittest_proto3_arena_pb2
@_parameterized.named_parameters(
('_proto2', unittest_pb2), ('_proto3', unittest_proto3_arena_pb2)
)
@testing_refleaks.TestCase
class ReflectionTest(unittest.TestCase):
def testEmptyCompositeContainerDeepCopy(self, message_module):
proto1 = message_module.NestedTestAllTypes(
payload=message_module.TestAllTypes(optional_string='foo')
)
nested2 = copy.deepcopy(proto1.child.repeated_child)
self.assertEqual(0, len(nested2))
if __name__ == '__main__':
unittest.main()

@ -2651,11 +2651,17 @@ PyObject* ContainerBase::DeepCopy() {
cmessage::NewEmptyMessage(this->parent->GetMessageClass());
new_parent->message = this->parent->message->New(nullptr);
// Copy the map field into the new message.
this->parent->message->GetReflection()->SwapFields(
this->parent->message, new_parent->message,
{this->parent_field_descriptor});
this->parent->message->MergeFrom(*new_parent->message);
// There is no API to copy a single field. The closest operation we have is
// SwapFields.
// So, we copy the source into a disposable message and then swap the one
// field we care about from it.
// If the performance of this operation matters we can do a copy of the single
// field, but that would require huge switches for each type+cardinality to
// call the right read/write field functions.
std::unique_ptr<Message> tmp(this->parent->message->New(nullptr));
tmp->MergeFrom(*this->parent->message);
tmp->GetReflection()->SwapFields(tmp.get(), new_parent->message,
{this->parent_field_descriptor});
PyObject* result =
cmessage::GetFieldValue(new_parent, this->parent_field_descriptor);

Loading…
Cancel
Save