From 9e8b30c2135944d9d3a6e53b70e534581e2dbe2b Mon Sep 17 00:00:00 2001 From: Mike Kruskal Date: Wed, 18 Sep 2024 12:35:43 -0700 Subject: [PATCH] Fix cord handling in DynamicMessage and oneofs. This fixes a memory corruption vulnerability for anyone using cord with dynamically built descriptor pools. PiperOrigin-RevId: 676091224 --- .bazelrc | 2 + ci/common.bazelrc | 2 + .../protobuf/internal/descriptor_test.py | 2 +- .../protobuf/internal/field_mask_test.py | 2 +- python/google/protobuf/internal/test_util.py | 3 + .../protobuf/internal/unknown_fields_test.py | 6 +- src/google/protobuf/compiler/cpp/field.cc | 43 +--- src/google/protobuf/compiler/cpp/field.h | 5 - .../cpp/field_generators/string_view_field.cc | 2 - src/google/protobuf/compiler/cpp/helpers.cc | 3 +- src/google/protobuf/compiler/cpp/tracker.cc | 10 +- src/google/protobuf/compiler/cpp/unittest.inc | 4 + src/google/protobuf/descriptor.h | 3 +- src/google/protobuf/descriptor_unittest.cc | 4 +- src/google/protobuf/dynamic_message.cc | 72 ++++-- src/google/protobuf/edition_unittest.proto | 2 + .../protobuf/generated_message_reflection.cc | 216 +++++++++++------- .../protobuf/generated_message_tctable_gen.cc | 26 ++- .../generated_message_tctable_lite.cc | 6 + src/google/protobuf/lite_unittest.cc | 24 ++ src/google/protobuf/message.cc | 8 +- src/google/protobuf/test_util.h | 17 ++ src/google/protobuf/test_util.inc | 12 + .../testdata/text_format_unittest_data.txt | 1 + ...format_unittest_data_oneof_implemented.txt | 1 + .../text_format_unittest_data_pointy.txt | 1 + ...text_format_unittest_data_pointy_oneof.txt | 1 + .../text_format_unittest_extensions_data.txt | 1 + ...format_unittest_extensions_data_pointy.txt | 1 + src/google/protobuf/unittest.proto | 3 + src/google/protobuf/unittest_lite.proto | 2 + .../protobuf/unittest_proto3_arena.proto | 1 + .../protobuf/util/field_mask_util_test.cc | 2 +- src/google/protobuf/wire_format.cc | 8 +- 34 files changed, 318 insertions(+), 178 deletions(-) diff --git a/.bazelrc b/.bazelrc index 43c30abb687d..7c6d4089a9d0 100644 --- a/.bazelrc +++ b/.bazelrc @@ -26,6 +26,8 @@ build:ubsan --action_env=UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 # Workaround for the fact that Bazel links with $CC, not $CXX # https://github.com/bazelbuild/bazel/issues/11122#issuecomment-613746748 build:ubsan --copt=-fno-sanitize=function --copt=-fno-sanitize=vptr +# Abseil passes nullptr to memcmp with 0 size +build:ubsan --copt=-fno-sanitize=nonnull-attribute # TODO: migrate all dependencies from WORKSPACE to MODULE.bazel # https://github.com/protocolbuffers/protobuf/issues/14313 diff --git a/ci/common.bazelrc b/ci/common.bazelrc index 50e737a35d4d..9b58ac21c416 100644 --- a/ci/common.bazelrc +++ b/ci/common.bazelrc @@ -31,6 +31,8 @@ build:ubsan --action_env=UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1 # Workaround for the fact that Bazel links with $CC, not $CXX # https://github.com/bazelbuild/bazel/issues/11122#issuecomment-613746748 build:ubsan --copt=-fno-sanitize=function --copt=-fno-sanitize=vptr +# Abseil passes nullptr to memcmp with 0 size +build:ubsan --copt=-fno-sanitize=nonnull-attribute # Workaround Bazel 7 remote cache issues. # See https://github.com/bazelbuild/bazel/issues/20161 diff --git a/python/google/protobuf/internal/descriptor_test.py b/python/google/protobuf/internal/descriptor_test.py index 9ca3c8c2fe86..7c6ad14714bd 100755 --- a/python/google/protobuf/internal/descriptor_test.py +++ b/python/google/protobuf/internal/descriptor_test.py @@ -727,7 +727,7 @@ def CheckDescriptorMapping(self, mapping): excepted_dict['new_key'] = 'new' self.assertNotEqual(mapping, excepted_dict) self.assertRaises(KeyError, mapping.__getitem__, 'key_error') - self.assertRaises(KeyError, mapping.__getitem__, len(mapping) + 1) + self.assertRaises(KeyError, mapping.__getitem__, len(mapping) * 2) # TODO: Add __repr__ support for DescriptorMapping. if api_implementation.Type() == 'cpp': self.assertEqual(str(mapping)[0], '<') diff --git a/python/google/protobuf/internal/field_mask_test.py b/python/google/protobuf/internal/field_mask_test.py index ed7c9ef60e26..ef286f8ba5d0 100644 --- a/python/google/protobuf/internal/field_mask_test.py +++ b/python/google/protobuf/internal/field_mask_test.py @@ -53,7 +53,7 @@ def testDescriptorToFieldMask(self): mask = field_mask_pb2.FieldMask() msg_descriptor = unittest_pb2.TestAllTypes.DESCRIPTOR mask.AllFieldsFromDescriptor(msg_descriptor) - self.assertEqual(79, len(mask.paths)) + self.assertEqual(80, len(mask.paths)) self.assertTrue(mask.IsValidForDescriptor(msg_descriptor)) for field in msg_descriptor.fields: self.assertTrue(field.name in mask.paths) diff --git a/python/google/protobuf/internal/test_util.py b/python/google/protobuf/internal/test_util.py index 53492d8a1587..46059fe1eba2 100755 --- a/python/google/protobuf/internal/test_util.py +++ b/python/google/protobuf/internal/test_util.py @@ -77,6 +77,7 @@ def SetAllNonLazyFields(message): message.optional_string_piece = u'124' message.optional_cord = u'125' + message.optional_bytes_cord = b'optional bytes cord' # # Repeated fields. @@ -247,6 +248,7 @@ def SetAllExtensions(message): extensions[pb2.optional_string_piece_extension] = u'124' extensions[pb2.optional_cord_extension] = u'125' + extensions[pb2.optional_bytes_cord_extension] = b'optional bytes cord' # # Repeated fields. @@ -423,6 +425,7 @@ def ExpectAllFieldsSet(test_case, message): test_case.assertTrue(message.HasField('optional_string_piece')) test_case.assertTrue(message.HasField('optional_cord')) + test_case.assertTrue(message.HasField('optional_bytes_cord')) test_case.assertEqual(101, message.optional_int32) test_case.assertEqual(102, message.optional_int64) diff --git a/python/google/protobuf/internal/unknown_fields_test.py b/python/google/protobuf/internal/unknown_fields_test.py index 57224ec74c74..33ad76653799 100755 --- a/python/google/protobuf/internal/unknown_fields_test.py +++ b/python/google/protobuf/internal/unknown_fields_test.py @@ -212,7 +212,7 @@ def testCheckUnknownFieldValue(self): unknown_field_set, (17, 0, 117)) - self.assertEqual(98, len(unknown_field_set)) + self.assertEqual(99, len(unknown_field_set)) def testCopyFrom(self): message = unittest_pb2.TestEmptyMessage() @@ -250,7 +250,7 @@ def testClear(self): self.empty_message.Clear() # All cleared, even unknown fields. self.assertEqual(self.empty_message.SerializeToString(), b'') - self.assertEqual(len(unknown_field_set), 98) + self.assertEqual(len(unknown_field_set), 99) @unittest.skipIf((sys.version_info.major, sys.version_info.minor) < (3, 4), 'tracemalloc requires python 3.4+') @@ -309,7 +309,7 @@ def testUnknownField(self): def testUnknownExtensions(self): message = unittest_pb2.TestEmptyMessageWithExtensions() message.ParseFromString(self.all_fields_data) - self.assertEqual(len(unknown_fields.UnknownFieldSet(message)), 98) + self.assertEqual(len(unknown_fields.UnknownFieldSet(message)), 99) self.assertEqual(message.SerializeToString(), self.all_fields_data) diff --git a/src/google/protobuf/compiler/cpp/field.cc b/src/google/protobuf/compiler/cpp/field.cc index cee1b665b8a4..b7a8d1f450fb 100644 --- a/src/google/protobuf/compiler/cpp/field.cc +++ b/src/google/protobuf/compiler/cpp/field.cc @@ -130,7 +130,6 @@ FieldGeneratorBase::FieldGeneratorBase(const FieldDescriptor* field, break; case FieldDescriptor::CPPTYPE_STRING: is_string_ = true; - string_type_ = field->options().ctype(); is_inlined_ = IsStringInlined(field, options); is_bytes_ = field->type() == FieldDescriptor::TYPE_BYTES; has_default_constexpr_constructor_ = is_repeated_or_map; @@ -229,40 +228,6 @@ void FieldGeneratorBase::GenerateCopyConstructorCode(io::Printer* p) const { } namespace { -// Use internal types instead of ctype or string_type. -enum class StringType { - kView, - kString, - kCord, - kStringPiece, -}; - -StringType GetStringType(const FieldDescriptor& field) { - ABSL_CHECK_EQ(field.cpp_type(), FieldDescriptor::CPPTYPE_STRING); - - if (field.options().has_ctype()) { - switch (field.options().ctype()) { - case FieldOptions::CORD: - return StringType::kCord; - case FieldOptions::STRING_PIECE: - return StringType::kStringPiece; - default: - return StringType::kString; - } - } - - const pb::CppFeatures& cpp_features = - CppGenerator::GetResolvedSourceFeatures(field).GetExtension(::pb::cpp); - switch (cpp_features.string_type()) { - case pb::CppFeatures::CORD: - return StringType::kCord; - case pb::CppFeatures::VIEW: - return StringType::kView; - default: - return StringType::kString; - } -} - std::unique_ptr MakeGenerator(const FieldDescriptor* field, const Options& options, MessageSCCAnalyzer* scc) { @@ -279,7 +244,7 @@ std::unique_ptr MakeGenerator(const FieldDescriptor* field, case FieldDescriptor::CPPTYPE_MESSAGE: return MakeRepeatedMessageGenerator(field, options, scc); case FieldDescriptor::CPPTYPE_STRING: { - if (GetStringType(*field) == StringType::kView) { + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kView) { return MakeRepeatedStringViewGenerator(field, options, scc); } else { return MakeRepeatedStringGenerator(field, options, scc); @@ -303,10 +268,10 @@ std::unique_ptr MakeGenerator(const FieldDescriptor* field, case FieldDescriptor::CPPTYPE_ENUM: return MakeSinguarEnumGenerator(field, options, scc); case FieldDescriptor::CPPTYPE_STRING: { - switch (GetStringType(*field)) { - case StringType::kView: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kView: return MakeSingularStringViewGenerator(field, options, scc); - case StringType::kCord: + case FieldDescriptor::CppStringType::kCord: if (field->type() == FieldDescriptor::TYPE_BYTES) { if (field->real_containing_oneof()) { return MakeOneofCordGenerator(field, options, scc); diff --git a/src/google/protobuf/compiler/cpp/field.h b/src/google/protobuf/compiler/cpp/field.h index df1abb3a04b7..4722fc523625 100644 --- a/src/google/protobuf/compiler/cpp/field.h +++ b/src/google/protobuf/compiler/cpp/field.h @@ -95,9 +95,6 @@ class FieldGeneratorBase { // Returns true if the field API uses bytes (void) instead of chars. bool is_bytes() const { return is_bytes_; } - // Returns the public API string type for string fields. - FieldOptions::CType string_type() const { return string_type_; } - // Returns true if this field is part of a oneof field. bool is_oneof() const { return is_oneof_; } @@ -217,7 +214,6 @@ class FieldGeneratorBase { bool is_lazy_ = false; bool is_weak_ = false; bool is_oneof_ = false; - FieldOptions::CType string_type_ = FieldOptions::STRING; bool has_default_constexpr_constructor_ = false; }; @@ -269,7 +265,6 @@ class FieldGenerator { bool is_foreign() const { return impl_->is_foreign(); } bool is_string() const { return impl_->is_string(); } bool is_bytes() const { return impl_->is_bytes(); } - FieldOptions::CType string_type() const { return impl_->string_type(); } bool is_oneof() const { return impl_->is_oneof(); } bool is_inlined() const { return impl_->is_inlined(); } bool has_default_constexpr_constructor() const { diff --git a/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc b/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc index 2002cd59c1c5..43a4beb8814b 100644 --- a/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc +++ b/src/google/protobuf/compiler/cpp/field_generators/string_view_field.cc @@ -216,8 +216,6 @@ void SingularStringView::GenerateStaticMembers(io::Printer* p) const { } void SingularStringView::GenerateAccessorDeclarations(io::Printer* p) const { - ABSL_CHECK(!field_->options().has_ctype()); - auto v1 = p->WithVars(AnnotatedAccessors(field_, {""})); auto v2 = p->WithVars( AnnotatedAccessors(field_, {"set_"}, AnnotationCollector::kSet)); diff --git a/src/google/protobuf/compiler/cpp/helpers.cc b/src/google/protobuf/compiler/cpp/helpers.cc index 105f5f602248..e6a89222c432 100644 --- a/src/google/protobuf/compiler/cpp/helpers.cc +++ b/src/google/protobuf/compiler/cpp/helpers.cc @@ -1597,7 +1597,8 @@ MessageAnalysis MessageSCCAnalyzer::GetSCCAnalysis(const SCC* scc) { switch (field->type()) { case FieldDescriptor::TYPE_STRING: case FieldDescriptor::TYPE_BYTES: { - if (field->options().ctype() == FieldOptions::CORD) { + if (field->cpp_string_type() == + FieldDescriptor::CppStringType::kCord) { result.contains_cord = true; } break; diff --git a/src/google/protobuf/compiler/cpp/tracker.cc b/src/google/protobuf/compiler/cpp/tracker.cc index 6b679cc91db4..477672d41fb8 100644 --- a/src/google/protobuf/compiler/cpp/tracker.cc +++ b/src/google/protobuf/compiler/cpp/tracker.cc @@ -221,7 +221,8 @@ Getters RepeatedFieldGetters(const FieldDescriptor* field, Getters StringFieldGetters(const FieldDescriptor* field, const Options& opts) { std::string member = FieldMemberName(field, ShouldSplit(field, opts)); - bool is_std_string = field->options().ctype() == FieldOptions::STRING; + bool is_std_string = + field->cpp_string_type() == FieldDescriptor::CppStringType::kString; Getters getters; if (is_std_string && !field->default_value_string().empty()) { @@ -241,7 +242,8 @@ Getters StringOneofGetters(const FieldDescriptor* field, ABSL_CHECK(oneof != nullptr); std::string member = FieldMemberName(field, ShouldSplit(field, opts)); - bool is_std_string = field->options().ctype() == FieldOptions::STRING; + bool is_std_string = + field->cpp_string_type() == FieldDescriptor::CppStringType::kString; std::string field_ptr = member; if (is_std_string) { @@ -258,8 +260,8 @@ Getters StringOneofGetters(const FieldDescriptor* field, } Getters getters; - if (field->default_value_string().empty() || - field->options().ctype() == FieldOptions::STRING_PIECE) { + if (field->default_value_string().empty() + ) { getters.base = absl::Substitute("$0 ? $1 : nullptr", has, field_ptr); } else { getters.base = diff --git a/src/google/protobuf/compiler/cpp/unittest.inc b/src/google/protobuf/compiler/cpp/unittest.inc index a417bbfb09d9..1b66273f0918 100644 --- a/src/google/protobuf/compiler/cpp/unittest.inc +++ b/src/google/protobuf/compiler/cpp/unittest.inc @@ -428,6 +428,7 @@ TEST(GENERATED_MESSAGE_TEST_NAME, SwapWithOther) { message1.set_optional_string("abc"); message1.mutable_optional_nested_message()->set_bb(1); message1.set_optional_nested_enum(UNITTEST::TestAllTypes::FOO); + message1.set_optional_bytes_cord("bytes cord"); message1.add_repeated_int32(1); message1.add_repeated_int32(2); message1.add_repeated_string("a"); @@ -441,6 +442,7 @@ TEST(GENERATED_MESSAGE_TEST_NAME, SwapWithOther) { message2.set_optional_string("def"); message2.mutable_optional_nested_message()->set_bb(2); message2.set_optional_nested_enum(UNITTEST::TestAllTypes::BAR); + message2.set_optional_bytes_cord("bytes cord"); message2.add_repeated_int32(3); message2.add_repeated_string("c"); message2.add_repeated_nested_message()->set_bb(9); @@ -452,6 +454,7 @@ TEST(GENERATED_MESSAGE_TEST_NAME, SwapWithOther) { EXPECT_EQ("def", message1.optional_string()); EXPECT_EQ(2, message1.optional_nested_message().bb()); EXPECT_EQ(UNITTEST::TestAllTypes::BAR, message1.optional_nested_enum()); + EXPECT_EQ(absl::Cord("bytes cord"), message1.optional_bytes_cord()); ASSERT_EQ(1, message1.repeated_int32_size()); EXPECT_EQ(3, message1.repeated_int32(0)); ASSERT_EQ(1, message1.repeated_string_size()); @@ -465,6 +468,7 @@ TEST(GENERATED_MESSAGE_TEST_NAME, SwapWithOther) { EXPECT_EQ("abc", message2.optional_string()); EXPECT_EQ(1, message2.optional_nested_message().bb()); EXPECT_EQ(UNITTEST::TestAllTypes::FOO, message2.optional_nested_enum()); + EXPECT_EQ(absl::Cord("bytes cord"), message2.optional_bytes_cord()); ASSERT_EQ(2, message2.repeated_int32_size()); EXPECT_EQ(1, message2.repeated_int32(0)); EXPECT_EQ(2, message2.repeated_int32(1)); diff --git a/src/google/protobuf/descriptor.h b/src/google/protobuf/descriptor.h index d65028a96298..ccbc409746b8 100644 --- a/src/google/protobuf/descriptor.h +++ b/src/google/protobuf/descriptor.h @@ -2954,7 +2954,8 @@ PROTOBUF_EXPORT bool HasHasbit(const FieldDescriptor* field); template typename FieldOpts::CType EffectiveStringCType(const FieldDesc* field) { - // TODO Replace this function with FieldDescriptor::string_type; + // TODO Replace this function with + // FieldDescriptor::cpp_string_type; switch (field->cpp_string_type()) { case FieldDescriptor::CppStringType::kCord: return FieldOpts::CORD; diff --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index f2e9aee5347a..32eb59551678 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -7571,7 +7571,7 @@ TEST_F(FeaturesTest, Proto2Features) { name: "cord" number: 8 label: LABEL_OPTIONAL - type: TYPE_STRING + type: TYPE_BYTES options { ctype: CORD } } field { @@ -7656,6 +7656,8 @@ TEST_F(FeaturesTest, Proto2Features) { EXPECT_EQ(message->FindFieldByName("str")->cpp_string_type(), FieldDescriptor::CppStringType::kString); + EXPECT_EQ(message->FindFieldByName("cord")->cpp_string_type(), + FieldDescriptor::CppStringType::kCord); // Check round-trip consistency. FileDescriptorProto proto; diff --git a/src/google/protobuf/dynamic_message.cc b/src/google/protobuf/dynamic_message.cc index 1af6bc5d61f3..12434fc606b1 100644 --- a/src/google/protobuf/dynamic_message.cc +++ b/src/google/protobuf/dynamic_message.cc @@ -117,9 +117,11 @@ int FieldSpaceUsed(const FieldDescriptor* field) { } case FD::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + return sizeof(RepeatedField); + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return sizeof(RepeatedPtrField); } break; @@ -147,9 +149,11 @@ int FieldSpaceUsed(const FieldDescriptor* field) { return sizeof(Message*); case FD::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + return sizeof(absl::Cord); + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return sizeof(ArenaStringPtr); } break; @@ -399,9 +403,31 @@ void DynamicMessage::SharedCtor(bool lock_factory) { break; case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + if (!field->is_repeated()) { + if (field->has_default_value()) { + new (field_ptr) absl::Cord(field->default_value_string()); + } else { + new (field_ptr) absl::Cord; + } + if (arena != nullptr) { + // Cord does not support arena so here we need to notify arena + // to remove the data it allocated on the heap by calling its + // destructor. + arena->OwnDestructor(static_cast(field_ptr)); + } + } else { + new (field_ptr) RepeatedField(arena); + if (arena != nullptr) { + // Needs to destroy Cord elements. + arena->OwnDestructor( + static_cast*>(field_ptr)); + } + } + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (!field->is_repeated()) { ArenaStringPtr* asp = new (field_ptr) ArenaStringPtr(); asp->InitDefault(); @@ -491,9 +517,12 @@ DynamicMessage::~DynamicMessage() { if (*(reinterpret_cast(field_ptr)) == field->number()) { field_ptr = MutableOneofFieldRaw(field); if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING) { - switch (field->options().ctype()) { - default: - case FieldOptions::STRING: { + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + delete *reinterpret_cast(field_ptr); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { reinterpret_cast(field_ptr)->Destroy(); break; } @@ -525,9 +554,13 @@ DynamicMessage::~DynamicMessage() { #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + reinterpret_cast*>(field_ptr) + ->~RepeatedField(); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: reinterpret_cast*>(field_ptr) ->~RepeatedPtrField(); break; @@ -545,9 +578,12 @@ DynamicMessage::~DynamicMessage() { } } else if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING) { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: { + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + reinterpret_cast(field_ptr)->~Cord(); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { reinterpret_cast(field_ptr)->Destroy(); break; } diff --git a/src/google/protobuf/edition_unittest.proto b/src/google/protobuf/edition_unittest.proto index 81848b87e4fe..c0d0ebbbf768 100644 --- a/src/google/protobuf/edition_unittest.proto +++ b/src/google/protobuf/edition_unittest.proto @@ -91,6 +91,7 @@ message TestAllTypes { string optional_string_piece = 24 [ctype=STRING_PIECE]; string optional_cord = 25 [ctype=CORD]; + bytes optional_bytes_cord = 86 [ctype=CORD]; // Defined in unittest_import_public.proto protobuf_unittest_import.PublicImportMessage @@ -266,6 +267,7 @@ extend TestAllExtensions { // TODO: ctype=CORD is not supported for extension. Add // ctype=CORD option back after it is supported. string optional_cord_extension = 25; + bytes optional_bytes_cord_extension = 86; protobuf_unittest_import.PublicImportMessage optional_public_import_message_extension = 26; diff --git a/src/google/protobuf/generated_message_reflection.cc b/src/google/protobuf/generated_message_reflection.cc index 6a75583bfa1f..391318ed21e2 100644 --- a/src/google/protobuf/generated_message_reflection.cc +++ b/src/google/protobuf/generated_message_reflection.cc @@ -403,9 +403,13 @@ size_t Reflection::SpaceUsedLong(const Message& message) const { #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + total_size += GetRaw>(message, field) + .SpaceUsedExcludingSelfLong(); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: total_size += GetRaw >(message, field) .SpaceUsedExcludingSelfLong(); @@ -444,8 +448,8 @@ size_t Reflection::SpaceUsedLong(const Message& message) const { break; case FieldDescriptor::CPPTYPE_STRING: { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { total_size += GetField(message, field) ->EstimatedMemoryUsage(); @@ -457,8 +461,8 @@ size_t Reflection::SpaceUsedLong(const Message& message) const { sizeof(absl::Cord); } break; - default: - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (IsInlined(field)) { const std::string* ptr = &GetField(message, field).GetNoArena(); @@ -554,15 +558,14 @@ struct OneofFieldMover { to->SetString(from->GetString()); break; } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: to->SetCord(from->GetCord()); break; - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: to->SetArenaStringPtr(from->GetArenaStringPtr()); break; - } } break; default: @@ -625,9 +628,19 @@ template void SwapFieldHelper::SwapRepeatedStringField(const Reflection* r, Message* lhs, Message* rhs, const FieldDescriptor* field) { - switch (field->options().ctype()) { - default: - case FieldOptions::STRING: { + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: { + auto* lhs_cord = r->MutableRaw>(lhs, field); + auto* rhs_cord = r->MutableRaw>(rhs, field); + if (unsafe_shallow_swap) { + lhs_cord->InternalSwap(rhs_cord); + } else { + lhs_cord->Swap(rhs_cord); + } + break; + } + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { auto* lhs_string = r->MutableRaw(lhs, field); auto* rhs_string = r->MutableRaw(rhs, field); if (unsafe_shallow_swap) { @@ -691,14 +704,14 @@ template void SwapFieldHelper::SwapStringField(const Reflection* r, Message* lhs, Message* rhs, const FieldDescriptor* field) { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: // Always shallow swap for Cord. std::swap(*r->MutableRaw(lhs, field), *r->MutableRaw(rhs, field)); break; - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { if (r->IsInlined(field)) { SwapFieldHelper::SwapInlinedStrings(r, lhs, rhs, field); @@ -1162,7 +1175,9 @@ void Reflection::SwapFieldsImpl( // may depend on the information in has bits. if (!field->is_repeated()) { SwapHasBit(message1, message2, field); - if (field->options().ctype() == FieldOptions::STRING && + if (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && + field->cpp_string_type() == + FieldDescriptor::CppStringType::kString && IsInlined(field)) { ABSL_DCHECK(!unsafe_shallow_swap || message1->GetArena() == message2->GetArena()); @@ -1276,9 +1291,10 @@ void Reflection::InternalSwap(Message* lhs, Message* rhs) const { int inlined_string_count = 0; for (int i = 0; i < descriptor_->field_count(); i++) { const FieldDescriptor* field = descriptor_->field(i); + if (field->cpp_type() != FieldDescriptor::CPPTYPE_STRING) continue; if (field->is_extension() || field->is_repeated() || schema_.InRealOneof(field) || - field->options().ctype() != FieldOptions::STRING || + field->cpp_string_type() != FieldDescriptor::CppStringType::kString || !IsInlined(field)) { continue; } @@ -1330,6 +1346,10 @@ int Reflection::FieldSize(const Message& message, #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { + return GetRaw >(message, field).size(); + } + ABSL_FALLTHROUGH_INTENDED; case FieldDescriptor::CPPTYPE_MESSAGE: if (IsMapFieldInApi(field)) { const internal::MapFieldBase& map = @@ -1388,8 +1408,8 @@ void Reflection::ClearField(Message* message, break; case FieldDescriptor::CPPTYPE_STRING: { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (field->has_default_value()) { *MutableRaw(message, field) = field->default_value_string(); @@ -1397,8 +1417,8 @@ void Reflection::ClearField(Message* message, MutableRaw(message, field)->Clear(); } break; - default: - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (IsInlined(field)) { // Currently, string with default value can't be inlined. So we // don't have to handle default value here. @@ -1445,9 +1465,12 @@ void Reflection::ClearField(Message* message, #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + MutableRaw>(message, field)->Clear(); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: MutableRaw >(message, field)->Clear(); break; } @@ -1495,9 +1518,12 @@ void Reflection::RemoveLast(Message* message, #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + MutableRaw>(message, field)->RemoveLast(); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: MutableRaw >(message, field) ->RemoveLast(); break; @@ -1589,6 +1615,12 @@ void Reflection::SwapElements(Message* message, const FieldDescriptor* field, #undef HANDLE_TYPE case FieldDescriptor::CPPTYPE_STRING: + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { + MutableRaw >(message, field) + ->SwapElements(index1, index2); + break; + } + ABSL_FALLTHROUGH_INTENDED; case FieldDescriptor::CPPTYPE_MESSAGE: if (IsMapFieldInApi(field)) { MutableRaw(message, field) @@ -1795,15 +1827,15 @@ std::string Reflection::GetString(const Message& message, if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { return std::string(field->default_value_string()); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { return std::string(*GetField(message, field)); } else { return std::string(GetField(message, field)); } - default: - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (IsInlined(field)) { return GetField(message, field).GetNoArena(); } else { @@ -1812,6 +1844,7 @@ std::string Reflection::GetString(const Message& message, : str.Get(); } } + internal::Unreachable(); } } @@ -1827,8 +1860,8 @@ const std::string& Reflection::GetStringReference(const Message& message, if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { return internal::DefaultValueStringAsString(field); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { absl::CopyCordToString(*GetField(message, field), scratch); @@ -1836,8 +1869,8 @@ const std::string& Reflection::GetStringReference(const Message& message, absl::CopyCordToString(GetField(message, field), scratch); } return *scratch; - default: - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (IsInlined(field)) { return GetField(message, field).GetNoArena(); } else { @@ -1846,6 +1879,7 @@ const std::string& Reflection::GetStringReference(const Message& message, : str.Get(); } } + internal::Unreachable(); } } @@ -1859,15 +1893,15 @@ absl::Cord Reflection::GetCord(const Message& message, if (schema_.InRealOneof(field) && !HasOneofField(message, field)) { return absl::Cord(field->default_value_string()); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { return *GetField(message, field); } else { return GetField(message, field); } - default: - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (IsInlined(field)) { return absl::Cord( GetField(message, field).GetNoArena()); @@ -1877,9 +1911,7 @@ absl::Cord Reflection::GetCord(const Message& message, : str.Get()); } } - - ABSL_LOG(FATAL) << "Can't get here."; - return absl::Cord(); // Make compiler happy. + internal::Unreachable(); } } @@ -1896,8 +1928,8 @@ absl::string_view Reflection::GetStringView(const Message& message, return field->default_value_string(); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: { + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: { const auto& cord = schema_.InRealOneof(field) ? *GetField(message, field) : GetField(message, field); @@ -1917,8 +1949,8 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, return MutableExtensionSet(message)->SetString( field->number(), field->type(), std::move(value), field); } else { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { if (!HasOneofField(*message, field)) { ClearOneof(message, field->containing_oneof()); @@ -1930,8 +1962,8 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, } *MutableField(message, field) = value; break; - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { if (IsInlined(field)) { const uint32_t index = schema_.InlinedStringIndex(field); ABSL_DCHECK_GT(index, 0u); @@ -1969,8 +2001,8 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, MutableExtensionSet(message)->MutableString( field->number(), field->type(), field)); } else { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: if (schema_.InRealOneof(field)) { if (!HasOneofField(*message, field)) { ClearOneof(message, field->containing_oneof()); @@ -1982,8 +2014,8 @@ void Reflection::SetString(Message* message, const FieldDescriptor* field, *MutableField(message, field) = value; } break; - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { // Oneof string fields are never set as a default instance. // We just need to pass some arbitrary default string to make it work. // This allows us to not have the real default accessible from @@ -2019,11 +2051,14 @@ std::string Reflection::GetRepeatedString(const Message& message, if (field->is_extension()) { return GetExtensionSet(message).GetRepeatedString(field->number(), index); } else { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + return std::string(GetRepeatedField(message, field, index)); + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return GetRepeatedPtrField(message, field, index); } + internal::Unreachable(); } } @@ -2035,11 +2070,16 @@ const std::string& Reflection::GetRepeatedStringReference( if (field->is_extension()) { return GetExtensionSet(message).GetRepeatedString(field->number(), index); } else { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + absl::CopyCordToString( + GetRepeatedField(message, field, index), scratch); + return *scratch; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return GetRepeatedPtrField(message, field, index); } + internal::Unreachable(); } } @@ -2054,11 +2094,16 @@ absl::string_view Reflection::GetRepeatedStringView( return GetExtensionSet(message).GetRepeatedString(field->number(), index); } - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::STRING: - default: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: { + auto& cord = GetRepeatedField(message, field, index); + return scratch.CopyFromCord(cord); + } + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return GetRepeatedPtrField(message, field, index); } + internal::Unreachable(); } @@ -2070,9 +2115,12 @@ void Reflection::SetRepeatedString(Message* message, MutableExtensionSet(message)->SetRepeatedString(field->number(), index, std::move(value)); } else { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + SetRepeatedField(message, field, index, absl::Cord(value)); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: MutableRepeatedField(message, field, index) ->assign(std::move(value)); break; @@ -2088,9 +2136,12 @@ void Reflection::AddString(Message* message, const FieldDescriptor* field, MutableExtensionSet(message)->AddString(field->number(), field->type(), std::move(value), field); } else { - switch (field->options().ctype()) { - default: // TODO: Support other string reps. - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + AddField(message, field, absl::Cord(value)); + break; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: AddField(message, field)->assign(std::move(value)); break; } @@ -2618,7 +2669,8 @@ const void* Reflection::GetRawRepeatedField(const Message& message, ReportReflectionUsageTypeError(descriptor_, field, "GetRawRepeatedField", cpptype); if (ctype >= 0) - ABSL_CHECK_EQ(field->options().ctype(), ctype) << "subtype mismatch"; + ABSL_CHECK_EQ(internal::cpp::EffectiveStringCType(field), ctype) + << "subtype mismatch"; if (desc != nullptr) ABSL_CHECK_EQ(field->message_type(), desc) << "wrong submessage type"; if (field->is_extension()) { @@ -2744,7 +2796,7 @@ static Type* AllocIfDefault(const FieldDescriptor* field, Type*& ptr, // be e.g. char). if (field->cpp_type() < FieldDescriptor::CPPTYPE_STRING || (field->cpp_type() == FieldDescriptor::CPPTYPE_STRING && - internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD)) { + field->cpp_string_type() == FieldDescriptor::CppStringType::kCord)) { ptr = reinterpret_cast(Arena::Create>(arena)); } else { @@ -2923,11 +2975,11 @@ bool Reflection::HasFieldSingular(const Message& message, // (which uses HasField()) needs to be consistent with this. switch (field->cpp_type()) { case FieldDescriptor::CPPTYPE_STRING: - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: return !GetField(message, field).empty(); - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { if (IsInlined(field)) { return !GetField(message, field) .GetNoArena() @@ -2937,7 +2989,7 @@ bool Reflection::HasFieldSingular(const Message& message, return GetField(message, field).Get().size() > 0; } } - return false; + internal::Unreachable(); case FieldDescriptor::CPPTYPE_BOOL: return GetRaw(message, field) != false; case FieldDescriptor::CPPTYPE_INT32: @@ -3039,12 +3091,12 @@ void Reflection::ClearOneof(Message* message, if (message->GetArena() == nullptr) { switch (field->cpp_type()) { case FieldDescriptor::CPPTYPE_STRING: { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: delete *MutableRaw(message, field); break; - default: - case FieldOptions::STRING: { + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: { // Oneof string fields are never set as a default instance. // We just need to pass some arbitrary default string to make it // work. This allows us to not have the real default accessible diff --git a/src/google/protobuf/generated_message_tctable_gen.cc b/src/google/protobuf/generated_message_tctable_gen.cc index c0be3dad1f65..f46425031421 100644 --- a/src/google/protobuf/generated_message_tctable_gen.cc +++ b/src/google/protobuf/generated_message_tctable_gen.cc @@ -167,10 +167,10 @@ TailCallTableInfo::FastFieldInfo::Field MakeFastFieldEntry( : field->is_repeated() ? PROTOBUF_PICK_FUNCTION(fn##R) \ : PROTOBUF_PICK_FUNCTION(fn##S)) -#define PROTOBUF_PICK_STRING_FUNCTION(fn) \ - (field->options().ctype() == FieldOptions::CORD \ - ? PROTOBUF_PICK_FUNCTION(fn##cS) \ - : options.is_string_inlined ? PROTOBUF_PICK_FUNCTION(fn##iS) \ +#define PROTOBUF_PICK_STRING_FUNCTION(fn) \ + (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord \ + ? PROTOBUF_PICK_FUNCTION(fn##cS) \ + : options.is_string_inlined ? PROTOBUF_PICK_FUNCTION(fn##iS) \ : PROTOBUF_PICK_REPEATABLE_FUNCTION(fn)) const FieldDescriptor* field = entry.field; @@ -303,10 +303,12 @@ bool IsFieldEligibleForFastParsing( switch (field->type()) { // Some bytes fields can be handled on fast path. case FieldDescriptor::TYPE_STRING: - case FieldDescriptor::TYPE_BYTES: - if (field->options().ctype() == FieldOptions::STRING) { + case FieldDescriptor::TYPE_BYTES: { + auto ctype = field->cpp_string_type(); + if (ctype == FieldDescriptor::CppStringType::kString || + ctype == FieldDescriptor::CppStringType::kView) { // strings are fine... - } else if (field->options().ctype() == FieldOptions::CORD) { + } else if (ctype == FieldDescriptor::CppStringType::kCord) { // Cords are worth putting into the fast table, if they're not repeated if (field->is_repeated()) return false; } else { @@ -320,6 +322,7 @@ bool IsFieldEligibleForFastParsing( aux_idx = entry.inlined_string_idx; } break; + } case FieldDescriptor::TYPE_ENUM: { uint8_t rmax_value; @@ -758,12 +761,13 @@ uint16_t MakeTypeCardForField( // Fill in extra information about string and bytes field representations. if (field->type() == FieldDescriptor::TYPE_BYTES || field->type() == FieldDescriptor::TYPE_STRING) { - switch (internal::cpp::EffectiveStringCType(field)) { - case FieldOptions::CORD: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: // `Cord` is always used, even for repeated fields. type_card |= fl::kRepCord; break; - case FieldOptions::STRING: + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: if (field->is_repeated()) { // A repeated string field uses RepeatedPtrField // (unless it has a ctype option; see above). @@ -773,8 +777,6 @@ uint16_t MakeTypeCardForField( type_card |= fl::kRepAString; } break; - default: - Unreachable(); } } diff --git a/src/google/protobuf/generated_message_tctable_lite.cc b/src/google/protobuf/generated_message_tctable_lite.cc index 47e11eacb707..6ed1157ce2ce 100644 --- a/src/google/protobuf/generated_message_tctable_lite.cc +++ b/src/google/protobuf/generated_message_tctable_lite.cc @@ -1647,6 +1647,12 @@ bool TcParser::ChangeOneof(const TcParseTableBase* table, field.Destroy(); break; } + case field_layout::kRepCord: { + if (msg->GetArena() == nullptr) { + delete RefAt(msg, current_entry->offset); + } + break; + } case field_layout::kRepSString: case field_layout::kRepIString: default: diff --git a/src/google/protobuf/lite_unittest.cc b/src/google/protobuf/lite_unittest.cc index 2999f7e5dce4..cd5db17b0b00 100644 --- a/src/google/protobuf/lite_unittest.cc +++ b/src/google/protobuf/lite_unittest.cc @@ -960,6 +960,18 @@ TYPED_TEST(LiteTest, AllLite43) { EXPECT_TRUE(message2.MergeFromCodedStream(&input_stream)); EXPECT_EQ(17, message2.oneof_int32()); } + + // Bytes [ctype = CORD] + { + protobuf_unittest::TestOneofParsingLite message2; + message2.set_oneof_bytes_cord("bytes cord"); + io::CodedInputStream input_stream( + reinterpret_cast(serialized.data()), + serialized.size()); + EXPECT_TRUE(message2.MergeFromCodedStream(&input_stream)); + EXPECT_EQ(17, message2.oneof_int32()); + } + } // Verify that we can successfully parse fields of various types within oneof @@ -1046,6 +1058,18 @@ TYPED_TEST(LiteTest, AllLite44) { } } + // Bytes [ctype = CORD] + { + protobuf_unittest::TestOneofParsingLite original; + original.set_oneof_bytes_cord("bytes cord"); + std::string serialized; + EXPECT_TRUE(original.SerializeToString(&serialized)); + protobuf_unittest::TestOneofParsingLite parsed; + EXPECT_TRUE(parsed.MergeFromString(serialized)); + EXPECT_EQ("bytes cord", std::string(parsed.oneof_bytes_cord())); + EXPECT_TRUE(parsed.MergeFromString(serialized)); + } + std::cout << "PASS" << std::endl; } diff --git a/src/google/protobuf/message.cc b/src/google/protobuf/message.cc index d265e284e10f..9eae5b305547 100644 --- a/src/google/protobuf/message.cc +++ b/src/google/protobuf/message.cc @@ -477,9 +477,11 @@ const internal::RepeatedFieldAccessor* Reflection::RepeatedFieldAccessor( HANDLE_PRIMITIVE_TYPE(ENUM, int32_t) #undef HANDLE_PRIMITIVE_TYPE case FieldDescriptor::CPPTYPE_STRING: - switch (field->options().ctype()) { - default: - case FieldOptions::STRING: + switch (field->cpp_string_type()) { + case FieldDescriptor::CppStringType::kCord: + ABSL_LOG(FATAL) << "Repeated cords are not supported."; + case FieldDescriptor::CppStringType::kView: + case FieldDescriptor::CppStringType::kString: return GetSingleton(); } break; diff --git a/src/google/protobuf/test_util.h b/src/google/protobuf/test_util.h index 338ba5a92a14..9e43eda0e91a 100644 --- a/src/google/protobuf/test_util.h +++ b/src/google/protobuf/test_util.h @@ -224,6 +224,8 @@ inline void TestUtil::ReflectionTester::SetAllFieldsViaReflection( reflection->SetString(message, F("optional_string_piece"), "124"); reflection->SetString(message, F("optional_cord"), "125"); + reflection->SetString(message, F("optional_bytes_cord"), + "optional bytes cord"); sub_message = reflection->MutableMessage(message, F("optional_public_import_message")); @@ -492,6 +494,7 @@ inline void TestUtil::ReflectionTester::ExpectAllFieldsSetViaReflection1( EXPECT_TRUE(reflection->HasField(message, F("optional_string_piece"))); EXPECT_TRUE(reflection->HasField(message, F("optional_cord"))); + EXPECT_TRUE(reflection->HasField(message, F("optional_bytes_cord"))); EXPECT_EQ(101, reflection->GetInt32(message, F("optional_int32"))); EXPECT_EQ(102, reflection->GetInt64(message, F("optional_int64"))); @@ -553,6 +556,14 @@ inline void TestUtil::ReflectionTester::ExpectAllFieldsSetViaReflection1( EXPECT_EQ("125", reflection->GetStringReference(message, F("optional_cord"), &scratch)); + EXPECT_EQ("optional bytes cord", + reflection->GetString(message, F("optional_bytes_cord"))); + EXPECT_EQ("optional bytes cord", + reflection->GetStringReference(message, F("optional_bytes_cord"), + &scratch)); + EXPECT_EQ("optional bytes cord", + reflection->GetCord(message, F("optional_bytes_cord"))); + EXPECT_TRUE(reflection->HasField(message, F("oneof_bytes"))); EXPECT_EQ("604", reflection->GetString(message, F("oneof_bytes"))); @@ -913,6 +924,7 @@ inline void TestUtil::ReflectionTester::ExpectClearViaReflection( EXPECT_FALSE(reflection->HasField(message, F("optional_string_piece"))); EXPECT_FALSE(reflection->HasField(message, F("optional_cord"))); + EXPECT_FALSE(reflection->HasField(message, F("optional_bytes_cord"))); // Optional fields without defaults are set to zero or something like it. EXPECT_EQ(0, reflection->GetInt32(message, F("optional_int32"))); @@ -979,6 +991,11 @@ inline void TestUtil::ReflectionTester::ExpectClearViaReflection( EXPECT_EQ("", reflection->GetStringReference(message, F("optional_cord"), &scratch)); + EXPECT_EQ("", reflection->GetString(message, F("optional_bytes_cord"))); + EXPECT_EQ("", reflection->GetStringReference( + message, F("optional_bytes_cord"), &scratch)); + EXPECT_EQ("", reflection->GetCord(message, F("optional_bytes_cord"))); + // Repeated fields are empty. EXPECT_EQ(0, reflection->FieldSize(message, F("repeated_int32"))); EXPECT_EQ(0, reflection->FieldSize(message, F("repeated_int64"))); diff --git a/src/google/protobuf/test_util.inc b/src/google/protobuf/test_util.inc index a98674bbebbf..6b2be0fa7051 100644 --- a/src/google/protobuf/test_util.inc +++ b/src/google/protobuf/test_util.inc @@ -140,6 +140,7 @@ inline void TestUtil::SetOptionalFields(UNITTEST::TestAllTypes* message) { message, message->GetDescriptor()->FindFieldByName("optional_cord"), "125"); #endif // !PROTOBUF_TEST_NO_DESCRIPTORS + message->set_optional_bytes_cord("optional bytes cord"); } // ------------------------------------------------------------------- @@ -355,6 +356,7 @@ inline void TestUtil::ExpectAllFieldsSet( EXPECT_TRUE(message.has_optional_string_piece()); EXPECT_TRUE(message.has_optional_cord()); #endif + EXPECT_TRUE(message.has_optional_bytes_cord()); EXPECT_EQ(101, message.optional_int32()); EXPECT_EQ(102, message.optional_int64()); @@ -384,6 +386,7 @@ inline void TestUtil::ExpectAllFieldsSet( EXPECT_EQ(UNITTEST::FOREIGN_BAZ, message.optional_foreign_enum()); EXPECT_EQ(UNITTEST_IMPORT::IMPORT_BAZ, message.optional_import_enum()); + EXPECT_EQ("optional bytes cord", message.optional_bytes_cord()); // ----------------------------------------------------------------- @@ -557,6 +560,7 @@ inline void TestUtil::ExpectClear(const UNITTEST::TestAllTypes& message) { EXPECT_FALSE(message.has_optional_string_piece()); EXPECT_FALSE(message.has_optional_cord()); + EXPECT_FALSE(message.has_optional_bytes_cord()); // Optional fields without defaults are set to zero or something like it. EXPECT_EQ(0, message.optional_int32()); @@ -597,6 +601,7 @@ inline void TestUtil::ExpectClear(const UNITTEST::TestAllTypes& message) { EXPECT_EQ(UNITTEST::FOREIGN_FOO, message.optional_foreign_enum()); EXPECT_EQ(UNITTEST_IMPORT::IMPORT_FOO, message.optional_import_enum()); + EXPECT_EQ("", message.optional_bytes_cord()); // Repeated fields are empty. EXPECT_EQ(0, message.repeated_int32_size()); @@ -978,6 +983,8 @@ inline void TestUtil::SetAllExtensions(UNITTEST::TestAllExtensions* message) { message->SetExtension(UNITTEST::optional_string_piece_extension, "124"); message->SetExtension(UNITTEST::optional_cord_extension, "125"); + message->SetExtension(UNITTEST::optional_bytes_cord_extension, + "optional bytes cord"); message->MutableExtension(UNITTEST::optional_public_import_message_extension) ->set_e(126); @@ -1207,6 +1214,7 @@ inline void TestUtil::ExpectAllExtensionsSet( EXPECT_TRUE(message.HasExtension(UNITTEST::optional_string_piece_extension)); EXPECT_TRUE(message.HasExtension(UNITTEST::optional_cord_extension)); + EXPECT_TRUE(message.HasExtension(UNITTEST::optional_bytes_cord_extension)); EXPECT_EQ(101, message.GetExtension(UNITTEST::optional_int32_extension)); EXPECT_EQ(102, message.GetExtension(UNITTEST::optional_int64_extension)); @@ -1245,6 +1253,8 @@ inline void TestUtil::ExpectAllExtensionsSet( EXPECT_EQ("124", message.GetExtension(UNITTEST::optional_string_piece_extension)); EXPECT_EQ("125", message.GetExtension(UNITTEST::optional_cord_extension)); + EXPECT_EQ("optional bytes cord", + message.GetExtension(UNITTEST::optional_bytes_cord_extension)); EXPECT_EQ( 126, message.GetExtension(UNITTEST::optional_public_import_message_extension) @@ -1493,6 +1503,7 @@ inline void TestUtil::ExpectExtensionsClear( EXPECT_FALSE(message.HasExtension(UNITTEST::optional_string_piece_extension)); EXPECT_FALSE(message.HasExtension(UNITTEST::optional_cord_extension)); + EXPECT_FALSE(message.HasExtension(UNITTEST::optional_bytes_cord_extension)); // Optional fields without defaults are set to zero or something like it. EXPECT_EQ(0, message.GetExtension(UNITTEST::optional_int32_extension)); @@ -1560,6 +1571,7 @@ inline void TestUtil::ExpectExtensionsClear( EXPECT_EQ("", message.GetExtension(UNITTEST::optional_string_piece_extension)); EXPECT_EQ("", message.GetExtension(UNITTEST::optional_cord_extension)); + EXPECT_EQ("", message.GetExtension(UNITTEST::optional_bytes_cord_extension)); // Repeated fields are empty. EXPECT_EQ(0, message.ExtensionSize(UNITTEST::repeated_int32_extension)); diff --git a/src/google/protobuf/testdata/text_format_unittest_data.txt b/src/google/protobuf/testdata/text_format_unittest_data.txt index 7a874b54cdba..b218f291b067 100644 --- a/src/google/protobuf/testdata/text_format_unittest_data.txt +++ b/src/google/protobuf/testdata/text_format_unittest_data.txt @@ -126,6 +126,7 @@ default_foreign_enum: FOREIGN_FOO default_import_enum: IMPORT_FOO default_string_piece: "424" default_cord: "425" +optional_bytes_cord: "optional bytes cord" oneof_uint32: 601 oneof_nested_message { bb: 602 diff --git a/src/google/protobuf/testdata/text_format_unittest_data_oneof_implemented.txt b/src/google/protobuf/testdata/text_format_unittest_data_oneof_implemented.txt index 86389c93cbce..c477bbef1b91 100644 --- a/src/google/protobuf/testdata/text_format_unittest_data_oneof_implemented.txt +++ b/src/google/protobuf/testdata/text_format_unittest_data_oneof_implemented.txt @@ -129,4 +129,5 @@ default_foreign_enum: FOREIGN_FOO default_import_enum: IMPORT_FOO default_string_piece: "424" default_cord: "425" +optional_bytes_cord: "optional bytes cord" oneof_bytes: "604" diff --git a/src/google/protobuf/testdata/text_format_unittest_data_pointy.txt b/src/google/protobuf/testdata/text_format_unittest_data_pointy.txt index 788025c51530..d46ec7bb3a4f 100644 --- a/src/google/protobuf/testdata/text_format_unittest_data_pointy.txt +++ b/src/google/protobuf/testdata/text_format_unittest_data_pointy.txt @@ -129,6 +129,7 @@ default_foreign_enum: FOREIGN_FOO default_import_enum: IMPORT_FOO default_string_piece: "424" default_cord: "425" +optional_bytes_cord: "optional bytes cord" oneof_uint32: 601 oneof_nested_message < bb: 602 diff --git a/src/google/protobuf/testdata/text_format_unittest_data_pointy_oneof.txt b/src/google/protobuf/testdata/text_format_unittest_data_pointy_oneof.txt index b2d3367098ce..139e3df348e7 100644 --- a/src/google/protobuf/testdata/text_format_unittest_data_pointy_oneof.txt +++ b/src/google/protobuf/testdata/text_format_unittest_data_pointy_oneof.txt @@ -129,4 +129,5 @@ default_foreign_enum: FOREIGN_FOO default_import_enum: IMPORT_FOO default_string_piece: "424" default_cord: "425" +optional_bytes_cord: "optional bytes cord" oneof_bytes: "604" diff --git a/src/google/protobuf/testdata/text_format_unittest_extensions_data.txt b/src/google/protobuf/testdata/text_format_unittest_extensions_data.txt index 5c3a03ac3708..8ab4d2e85b0c 100644 --- a/src/google/protobuf/testdata/text_format_unittest_extensions_data.txt +++ b/src/google/protobuf/testdata/text_format_unittest_extensions_data.txt @@ -129,6 +129,7 @@ [protobuf_unittest.default_import_enum_extension]: IMPORT_FOO [protobuf_unittest.default_string_piece_extension]: "424" [protobuf_unittest.default_cord_extension]: "425" +[protobuf_unittest.optional_bytes_cord_extension]: "optional bytes cord" [protobuf_unittest.oneof_uint32_extension]: 601 [protobuf_unittest.oneof_nested_message_extension] { bb: 602 diff --git a/src/google/protobuf/testdata/text_format_unittest_extensions_data_pointy.txt b/src/google/protobuf/testdata/text_format_unittest_extensions_data_pointy.txt index 4233ca78f301..20b5ea116292 100644 --- a/src/google/protobuf/testdata/text_format_unittest_extensions_data_pointy.txt +++ b/src/google/protobuf/testdata/text_format_unittest_extensions_data_pointy.txt @@ -129,6 +129,7 @@ [protobuf_unittest.default_import_enum_extension]: IMPORT_FOO [protobuf_unittest.default_string_piece_extension]: "424" [protobuf_unittest.default_cord_extension]: "425" +[protobuf_unittest.optional_bytes_cord_extension]: "optional bytes cord" [protobuf_unittest.oneof_uint32_extension]: 601 [protobuf_unittest.oneof_nested_message_extension] < bb: 602 diff --git a/src/google/protobuf/unittest.proto b/src/google/protobuf/unittest.proto index 3d23298e4265..14cf08e0961c 100644 --- a/src/google/protobuf/unittest.proto +++ b/src/google/protobuf/unittest.proto @@ -96,6 +96,7 @@ message TestAllTypes { string optional_cord = 25 [ ctype = CORD ]; + bytes optional_bytes_cord = 86 [ctype=CORD]; // Defined in unittest_import_public.proto protobuf_unittest_import.PublicImportMessage optional_public_import_message = 26; @@ -368,6 +369,8 @@ extend TestAllExtensions { // TODO: ctype=CORD is not supported for extension. Add // ctype=CORD option back after it is supported. string optional_cord_extension = 25; + bytes optional_bytes_cord_extension = 86; + protobuf_unittest_import.PublicImportMessage optional_public_import_message_extension = 26; TestAllTypes.NestedMessage optional_lazy_message_extension = 27 [ diff --git a/src/google/protobuf/unittest_lite.proto b/src/google/protobuf/unittest_lite.proto index 4bc78c4de6a8..0a6c0456c332 100644 --- a/src/google/protobuf/unittest_lite.proto +++ b/src/google/protobuf/unittest_lite.proto @@ -72,6 +72,7 @@ message TestAllTypesLite { string optional_string_piece = 24 [ctype = STRING_PIECE]; string optional_cord = 25 [ctype = CORD]; + bytes optional_bytes_cord = 86 [ctype = CORD]; // Defined in unittest_import_public.proto protobuf_unittest_import.PublicImportMessageLite @@ -261,6 +262,7 @@ extend TestAllExtensionsLite { // TODO: ctype=CORD is not supported for extension. Add // ctype=CORD option back after it is supported. string optional_cord_extension_lite = 25; + bytes optional_bytes_cord_extension_lite = 86; protobuf_unittest_import.PublicImportMessageLite optional_public_import_message_extension_lite = 26; TestAllTypesLite.NestedMessage optional_lazy_message_extension_lite = 27 diff --git a/src/google/protobuf/unittest_proto3_arena.proto b/src/google/protobuf/unittest_proto3_arena.proto index 96a9ea85e295..ea801cef647e 100644 --- a/src/google/protobuf/unittest_proto3_arena.proto +++ b/src/google/protobuf/unittest_proto3_arena.proto @@ -67,6 +67,7 @@ message TestAllTypes { string optional_string_piece = 24 [ctype=STRING_PIECE]; string optional_cord = 25 [ctype=CORD]; + bytes optional_bytes_cord = 86 [ctype=CORD]; // Defined in unittest_import_public.proto protobuf_unittest_import.PublicImportMessage diff --git a/src/google/protobuf/util/field_mask_util_test.cc b/src/google/protobuf/util/field_mask_util_test.cc index 79fe32c9dfda..624d6ac613ce 100644 --- a/src/google/protobuf/util/field_mask_util_test.cc +++ b/src/google/protobuf/util/field_mask_util_test.cc @@ -202,7 +202,7 @@ TEST(FieldMaskUtilTest, TestGetFieldMaskForAllFields) { EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("bb", mask)); mask = FieldMaskUtil::GetFieldMaskForAllFields(); - EXPECT_EQ(79, mask.paths_size()); + EXPECT_EQ(80, mask.paths_size()); EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("optional_int32", mask)); EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("optional_int64", mask)); EXPECT_TRUE(FieldMaskUtil::IsPathInFieldMask("optional_uint32", mask)); diff --git a/src/google/protobuf/wire_format.cc b/src/google/protobuf/wire_format.cc index 9969cda4edb9..c215ac6ab057 100644 --- a/src/google/protobuf/wire_format.cc +++ b/src/google/protobuf/wire_format.cc @@ -562,7 +562,7 @@ bool WireFormat::ParseAndMergeField( } case FieldDescriptor::TYPE_BYTES: { - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { absl::Cord value; if (!WireFormatLite::ReadBytes(input, &value)) return false; message_reflection->SetString(message, field, value); @@ -1005,7 +1005,7 @@ const char* WireFormat::_InternalParseAndMergeField( case FieldDescriptor::TYPE_BYTES: { int size = ReadSize(&ptr); if (ptr == nullptr) return nullptr; - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { absl::Cord value; ptr = ctx->ReadCord(ptr, size, &value); if (ptr == nullptr) return nullptr; @@ -1428,7 +1428,7 @@ uint8_t* WireFormat::InternalSerializeField(const FieldDescriptor* field, } case FieldDescriptor::TYPE_BYTES: { - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { absl::Cord value = message_reflection->GetCord(message, field); target = stream->WriteString(field->number(), value, target); break; @@ -1748,7 +1748,7 @@ size_t WireFormat::FieldDataOnlyByteSize(const FieldDescriptor* field, // instead of copying. case FieldDescriptor::TYPE_STRING: case FieldDescriptor::TYPE_BYTES: { - if (internal::cpp::EffectiveStringCType(field) == FieldOptions::CORD) { + if (field->cpp_string_type() == FieldDescriptor::CppStringType::kCord) { for (size_t j = 0; j < count; j++) { absl::Cord value = message_reflection->GetCord(message, field); data_size += WireFormatLite::StringSize(value);