Fix packed reflection handling bug in edition 2023.

This is a very narrow edge case where touching a packed extension via generated APIs first, and then doing so reflectively will trigger a DCHECK.  Otherwise, reflective APIs will work but not use packed encoding for the extension.  This was likely a pre-existing bug dating back to proto3, where it would only be visible on custom options (the only extensions allowed in proto3).

To help qualify this and uncover similar issues, unittest.proto was migrated to editions.  This turned up some other minor issues in DebugString and python.

PiperOrigin-RevId: 675785611
pull/18317/head
Mike Kruskal 6 months ago committed by Copybara-Service
parent 7dfbf7912e
commit 4c923285a3
  1. 6
      compatibility/BUILD.bazel
  2. 125
      java/core/src/test/java/com/google/protobuf/DescriptorsTest.java
  3. 6
      python/descriptor.c
  4. 7
      python/google/protobuf/internal/descriptor_test.py
  5. 22
      python/google/protobuf/internal/message_test.py
  6. 4
      python/google/protobuf/internal/reflection_test.py
  7. 5
      python/google/protobuf/pyext/descriptor.cc
  8. 2
      src/google/protobuf/compiler/cpp/file_unittest.cc
  9. 12
      src/google/protobuf/descriptor.cc
  10. 53
      src/google/protobuf/descriptor_unittest.cc
  11. 23
      src/google/protobuf/dynamic_message_unittest.cc
  12. 3
      src/google/protobuf/extension_set_unittest.cc
  13. 3
      src/google/protobuf/generated_message_reflection.cc
  14. 43
      src/google/protobuf/generated_message_reflection_unittest.cc
  15. 38
      src/google/protobuf/test_util.inc
  16. 2316
      src/google/protobuf/unittest.proto

@ -33,12 +33,6 @@ java_runtime_conformance(
gencode_version = "main", gencode_version = "main",
) )
# Generates a build_test named "conformance_v3.25.0"
java_runtime_conformance(
name = "java_conformance_v3.25.0",
gencode_version = "3.25.0",
)
# Breaking change detection for well-known types and descriptor.proto. # Breaking change detection for well-known types and descriptor.proto.
buf_breaking_test( buf_breaking_test(
name = "any_proto_breaking", name = "any_proto_breaking",

@ -19,6 +19,7 @@ import com.google.protobuf.DescriptorProtos.EnumValueDescriptorProto;
import com.google.protobuf.DescriptorProtos.FeatureSetDefaults; import com.google.protobuf.DescriptorProtos.FeatureSetDefaults;
import com.google.protobuf.DescriptorProtos.FeatureSetDefaults.FeatureSetEditionDefault; import com.google.protobuf.DescriptorProtos.FeatureSetDefaults.FeatureSetEditionDefault;
import com.google.protobuf.DescriptorProtos.FieldDescriptorProto; import com.google.protobuf.DescriptorProtos.FieldDescriptorProto;
import com.google.protobuf.DescriptorProtos.FieldOptions;
import com.google.protobuf.DescriptorProtos.FileDescriptorProto; import com.google.protobuf.DescriptorProtos.FileDescriptorProto;
import com.google.protobuf.DescriptorProtos.FileOptions; import com.google.protobuf.DescriptorProtos.FileOptions;
import com.google.protobuf.DescriptorProtos.MethodDescriptorProto; import com.google.protobuf.DescriptorProtos.MethodDescriptorProto;
@ -53,7 +54,6 @@ import protobuf_unittest.UnittestProto.TestReservedEnumFields;
import protobuf_unittest.UnittestProto.TestReservedFields; import protobuf_unittest.UnittestProto.TestReservedFields;
import protobuf_unittest.UnittestProto.TestService; import protobuf_unittest.UnittestProto.TestService;
import protobuf_unittest.UnittestRetention; import protobuf_unittest.UnittestRetention;
import proto3_unittest.UnittestProto3;
import protobuf_unittest.UnittestProto3Extensions.Proto3FileExtensions; import protobuf_unittest.UnittestProto3Extensions.Proto3FileExtensions;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
@ -1256,32 +1256,106 @@ public class DescriptorsTest {
} }
@Test @Test
public void testLegacyInferRequired() { public void testLegacyInferRequired() throws Exception {
FieldDescriptor field = UnittestProto.TestRequired.getDescriptor().findFieldByName("a"); FileDescriptor file =
FileDescriptor.buildFrom(
FileDescriptorProto.newBuilder()
.setName("some/filename/some.proto")
.setSyntax("proto2")
.addMessageType(
DescriptorProto.newBuilder()
.setName("Foo")
.addField(
FieldDescriptorProto.newBuilder()
.setName("a")
.setNumber(1)
.setType(FieldDescriptorProto.Type.TYPE_INT32)
.setLabel(FieldDescriptorProto.Label.LABEL_REQUIRED)
.build())
.build())
.build(),
new FileDescriptor[0]);
FieldDescriptor field = file.findMessageTypeByName("Foo").findFieldByName("a");
assertThat(field.features.getFieldPresence()) assertThat(field.features.getFieldPresence())
.isEqualTo(DescriptorProtos.FeatureSet.FieldPresence.LEGACY_REQUIRED); .isEqualTo(DescriptorProtos.FeatureSet.FieldPresence.LEGACY_REQUIRED);
} }
@Test @Test
public void testLegacyInferGroup() { public void testLegacyInferGroup() throws Exception {
FieldDescriptor field = FileDescriptor file =
UnittestProto.TestAllTypes.getDescriptor().findFieldByName("optionalgroup"); FileDescriptor.buildFrom(
FileDescriptorProto.newBuilder()
.setName("some/filename/some.proto")
.setSyntax("proto2")
.addMessageType(
DescriptorProto.newBuilder()
.setName("Foo")
.addNestedType(
DescriptorProto.newBuilder().setName("OptionalGroup").build())
.addField(
FieldDescriptorProto.newBuilder()
.setName("optionalgroup")
.setNumber(1)
.setType(FieldDescriptorProto.Type.TYPE_GROUP)
.setLabel(FieldDescriptorProto.Label.LABEL_OPTIONAL)
.setTypeName("Foo.OptionalGroup")
.build())
.build())
.build(),
new FileDescriptor[0]);
FieldDescriptor field = file.findMessageTypeByName("Foo").findFieldByName("optionalgroup");
assertThat(field.features.getMessageEncoding()) assertThat(field.features.getMessageEncoding())
.isEqualTo(DescriptorProtos.FeatureSet.MessageEncoding.DELIMITED); .isEqualTo(DescriptorProtos.FeatureSet.MessageEncoding.DELIMITED);
} }
@Test @Test
public void testLegacyInferProto2Packed() { public void testLegacyInferProto2Packed() throws Exception {
FieldDescriptor field = FileDescriptor file =
UnittestProto.TestPackedTypes.getDescriptor().findFieldByName("packed_int32"); FileDescriptor.buildFrom(
FileDescriptorProto.newBuilder()
.setName("some/filename/some.proto")
.setSyntax("proto2")
.addMessageType(
DescriptorProto.newBuilder()
.setName("Foo")
.addField(
FieldDescriptorProto.newBuilder()
.setName("a")
.setNumber(1)
.setLabel(FieldDescriptorProto.Label.LABEL_REPEATED)
.setType(FieldDescriptorProto.Type.TYPE_INT32)
.setOptions(FieldOptions.newBuilder().setPacked(true).build())
.build())
.build())
.build(),
new FileDescriptor[0]);
FieldDescriptor field = file.findMessageTypeByName("Foo").findFieldByName("a");
assertThat(field.features.getRepeatedFieldEncoding()) assertThat(field.features.getRepeatedFieldEncoding())
.isEqualTo(DescriptorProtos.FeatureSet.RepeatedFieldEncoding.PACKED); .isEqualTo(DescriptorProtos.FeatureSet.RepeatedFieldEncoding.PACKED);
} }
@Test @Test
public void testLegacyInferProto3Expanded() { public void testLegacyInferProto3Expanded() throws Exception {
FieldDescriptor field = FileDescriptor file =
UnittestProto3.TestUnpackedTypes.getDescriptor().findFieldByName("repeated_int32"); FileDescriptor.buildFrom(
FileDescriptorProto.newBuilder()
.setName("some/filename/some.proto")
.setSyntax("proto3")
.addMessageType(
DescriptorProto.newBuilder()
.setName("Foo")
.addField(
FieldDescriptorProto.newBuilder()
.setName("a")
.setNumber(1)
.setType(FieldDescriptorProto.Type.TYPE_INT32)
.setLabel(FieldDescriptorProto.Label.LABEL_REPEATED)
.setOptions(FieldOptions.newBuilder().setPacked(false).build())
.build())
.build())
.build(),
new FileDescriptor[0]);
FieldDescriptor field = file.findMessageTypeByName("Foo").findFieldByName("a");
assertThat(field.features.getRepeatedFieldEncoding()) assertThat(field.features.getRepeatedFieldEncoding())
.isEqualTo(DescriptorProtos.FeatureSet.RepeatedFieldEncoding.EXPANDED); .isEqualTo(DescriptorProtos.FeatureSet.RepeatedFieldEncoding.EXPANDED);
} }
@ -1302,9 +1376,16 @@ public class DescriptorsTest {
} }
@Test @Test
public void testProto2Defaults() { public void testProto2Defaults() throws Exception {
FieldDescriptor proto2Field = TestAllTypes.getDescriptor().findFieldByName("optional_int32"); FileDescriptor proto2File =
DescriptorProtos.FeatureSet features = proto2Field.features; FileDescriptor.buildFrom(
FileDescriptorProto.newBuilder()
.setName("some/filename/some.proto")
.setPackage("protobuf_unittest")
.setSyntax("proto2")
.build(),
new FileDescriptor[0]);
DescriptorProtos.FeatureSet features = proto2File.features;
assertThat(features.getFieldPresence()) assertThat(features.getFieldPresence())
.isEqualTo(DescriptorProtos.FeatureSet.FieldPresence.EXPLICIT); .isEqualTo(DescriptorProtos.FeatureSet.FieldPresence.EXPLICIT);
assertThat(features.getEnumType()).isEqualTo(DescriptorProtos.FeatureSet.EnumType.CLOSED); assertThat(features.getEnumType()).isEqualTo(DescriptorProtos.FeatureSet.EnumType.CLOSED);
@ -1323,10 +1404,16 @@ public class DescriptorsTest {
} }
@Test @Test
public void testProto3Defaults() { public void testProto3Defaults() throws Exception {
FieldDescriptor proto3Field = FileDescriptor proto3File =
UnittestProto3.TestAllTypes.getDescriptor().findFieldByName("optional_int32"); FileDescriptor.buildFrom(
DescriptorProtos.FeatureSet features = proto3Field.features; FileDescriptorProto.newBuilder()
.setName("some/filename/some.proto")
.setPackage("proto3_unittest")
.setSyntax("proto3")
.build(),
new FileDescriptor[0]);
DescriptorProtos.FeatureSet features = proto3File.features;
assertThat(features.getFieldPresence()) assertThat(features.getFieldPresence())
.isEqualTo(DescriptorProtos.FeatureSet.FieldPresence.IMPLICIT); .isEqualTo(DescriptorProtos.FeatureSet.FieldPresence.IMPLICIT);
assertThat(features.getEnumType()).isEqualTo(DescriptorProtos.FeatureSet.EnumType.OPEN); assertThat(features.getEnumType()).isEqualTo(DescriptorProtos.FeatureSet.EnumType.OPEN);

@ -1062,6 +1062,11 @@ static PyObject* PyUpb_FieldDescriptor_GetIsExtension(
return PyBool_FromLong(upb_FieldDef_IsExtension(self->def)); return PyBool_FromLong(upb_FieldDef_IsExtension(self->def));
} }
static PyObject* PyUpb_FieldDescriptor_GetIsPacked(PyUpb_DescriptorBase* self,
void* closure) {
return PyBool_FromLong(upb_FieldDef_IsPacked(self->def));
}
static PyObject* PyUpb_FieldDescriptor_GetNumber(PyUpb_DescriptorBase* self, static PyObject* PyUpb_FieldDescriptor_GetNumber(PyUpb_DescriptorBase* self,
void* closure) { void* closure) {
return PyLong_FromLong(upb_FieldDef_Number(self->def)); return PyLong_FromLong(upb_FieldDef_Number(self->def));
@ -1164,6 +1169,7 @@ static PyGetSetDef PyUpb_FieldDescriptor_Getters[] = {
"Default Value"}, "Default Value"},
{"has_default_value", (getter)PyUpb_FieldDescriptor_HasDefaultValue}, {"has_default_value", (getter)PyUpb_FieldDescriptor_HasDefaultValue},
{"is_extension", (getter)PyUpb_FieldDescriptor_GetIsExtension, NULL, "ID"}, {"is_extension", (getter)PyUpb_FieldDescriptor_GetIsExtension, NULL, "ID"},
{"is_packed", (getter)PyUpb_FieldDescriptor_GetIsPacked, NULL, "Is Packed"},
// TODO // TODO
//{ "id", (getter)GetID, NULL, "ID"}, //{ "id", (getter)GetID, NULL, "ID"},
{"message_type", (getter)PyUpb_FieldDescriptor_GetMessageType, NULL, {"message_type", (getter)PyUpb_FieldDescriptor_GetMessageType, NULL,

@ -18,17 +18,18 @@ from google.protobuf import descriptor_pool
from google.protobuf import symbol_database from google.protobuf import symbol_database
from google.protobuf import text_format from google.protobuf import text_format
from google.protobuf.internal import api_implementation from google.protobuf.internal import api_implementation
from google.protobuf.internal import test_proto2_pb2
from google.protobuf.internal import test_util from google.protobuf.internal import test_util
from google.protobuf.internal import testing_refleaks from google.protobuf.internal import testing_refleaks
from google.protobuf.internal import _parameterized from google.protobuf.internal import _parameterized
from google.protobuf import unittest_legacy_features_pb2
from google.protobuf import unittest_custom_options_pb2 from google.protobuf import unittest_custom_options_pb2
from google.protobuf import unittest_features_pb2 from google.protobuf import unittest_features_pb2
from google.protobuf import unittest_import_pb2 from google.protobuf import unittest_import_pb2
from google.protobuf import unittest_legacy_features_pb2
from google.protobuf import unittest_pb2 from google.protobuf import unittest_pb2
from google.protobuf import unittest_proto3_pb2
from google.protobuf import unittest_proto3_extensions_pb2 from google.protobuf import unittest_proto3_extensions_pb2
from google.protobuf import unittest_proto3_pb2
TEST_EMPTY_MESSAGE_DESCRIPTOR_ASCII = """ TEST_EMPTY_MESSAGE_DESCRIPTOR_ASCII = """
@ -1325,7 +1326,7 @@ class FeaturesTest(_parameterized.TestCase):
) )
def testProto2Defaults(self): def testProto2Defaults(self):
features = unittest_pb2.TestAllTypes.DESCRIPTOR.fields_by_name[ features = test_proto2_pb2.TestProto2.DESCRIPTOR.fields_by_name[
'optional_int32' 'optional_int32'
]._GetFeatures() ]._GetFeatures()
fs = descriptor_pb2.FeatureSet fs = descriptor_pb2.FeatureSet

@ -1408,18 +1408,20 @@ class Proto2Test(unittest.TestCase):
del msg.repeated_nested_message del msg.repeated_nested_message
def testAssignInvalidEnum(self): def testAssignInvalidEnum(self):
"""Assigning an invalid enum number is not allowed in proto2.""" """Assigning an invalid enum number is not allowed for closed enums."""
m = unittest_pb2.TestAllTypes() m = unittest_pb2.TestAllTypes()
# Proto2 can not assign unknown enum. # TODO Enable these once upb's behavior is made conformant.
with self.assertRaises(ValueError) as _: if api_implementation.Type() != 'upb':
m.optional_nested_enum = 1234567 # Can not assign unknown enum to closed enums.
self.assertRaises(ValueError, m.repeated_nested_enum.append, 1234567) with self.assertRaises(ValueError) as _:
# Assignment is a different code path than append for the C++ impl. m.optional_nested_enum = 1234567
m.repeated_nested_enum.append(2) self.assertRaises(ValueError, m.repeated_nested_enum.append, 1234567)
m.repeated_nested_enum[0] = 2 # Assignment is a different code path than append for the C++ impl.
with self.assertRaises(ValueError): m.repeated_nested_enum.append(2)
m.repeated_nested_enum[0] = 123456 m.repeated_nested_enum[0] = 2
with self.assertRaises(ValueError):
m.repeated_nested_enum[0] = 123456
# Unknown enum value can be parsed but is ignored. # Unknown enum value can be parsed but is ignored.
m2 = unittest_proto3_arena_pb2.TestAllTypes() m2 = unittest_proto3_arena_pb2.TestAllTypes()

@ -3261,13 +3261,13 @@ class OptionsTest(unittest.TestCase):
proto.optional_int32 = 1 proto.optional_int32 = 1
proto.optional_double = 3.0 proto.optional_double = 3.0
for field_descriptor, _ in proto.ListFields(): for field_descriptor, _ in proto.ListFields():
self.assertEqual(False, field_descriptor.GetOptions().packed) self.assertEqual(False, field_descriptor.is_packed)
proto = unittest_pb2.TestPackedTypes() proto = unittest_pb2.TestPackedTypes()
proto.packed_int32.append(1) proto.packed_int32.append(1)
proto.packed_double.append(3.0) proto.packed_double.append(3.0)
for field_descriptor, _ in proto.ListFields(): for field_descriptor, _ in proto.ListFields():
self.assertEqual(True, field_descriptor.GetOptions().packed) self.assertEqual(True, field_descriptor.is_packed)
self.assertEqual(descriptor.FieldDescriptor.LABEL_REPEATED, self.assertEqual(descriptor.FieldDescriptor.LABEL_REPEATED,
field_descriptor.label) field_descriptor.label)

@ -853,6 +853,10 @@ static PyObject* IsExtension(PyBaseDescriptor *self, void *closure) {
return PyBool_FromLong(_GetDescriptor(self)->is_extension()); return PyBool_FromLong(_GetDescriptor(self)->is_extension());
} }
static PyObject* IsPacked(PyBaseDescriptor* self, void* closure) {
return PyBool_FromLong(_GetDescriptor(self)->is_packed());
}
static PyObject* HasDefaultValue(PyBaseDescriptor *self, void *closure) { static PyObject* HasDefaultValue(PyBaseDescriptor *self, void *closure) {
return PyBool_FromLong(_GetDescriptor(self)->has_default_value()); return PyBool_FromLong(_GetDescriptor(self)->has_default_value());
} }
@ -1050,6 +1054,7 @@ static PyGetSetDef Getters[] = {
{"default_value", (getter)GetDefaultValue, nullptr, "Default Value"}, {"default_value", (getter)GetDefaultValue, nullptr, "Default Value"},
{"has_default_value", (getter)HasDefaultValue}, {"has_default_value", (getter)HasDefaultValue},
{"is_extension", (getter)IsExtension, nullptr, "ID"}, {"is_extension", (getter)IsExtension, nullptr, "ID"},
{"is_packed", (getter)IsPacked, nullptr, "Is Packed"},
{"id", (getter)GetID, nullptr, "ID"}, {"id", (getter)GetID, nullptr, "ID"},
{"_cdescriptor", (getter)GetCDescriptor, nullptr, "HAACK REMOVE ME"}, {"_cdescriptor", (getter)GetCDescriptor, nullptr, "HAACK REMOVE ME"},

@ -48,6 +48,7 @@ TEST(FileTest, TopologicallyOrderedDescriptors) {
"TestUnpackedTypes", "TestUnpackedTypes",
"TestUnpackedExtensions", "TestUnpackedExtensions",
"TestReservedFields", "TestReservedFields",
"TestRequiredOpenEnum",
"TestRequiredOneof.NestedMessage", "TestRequiredOneof.NestedMessage",
"TestRequiredNoMaskMulti", "TestRequiredNoMaskMulti",
"TestRequiredEnumNoMask", "TestRequiredEnumNoMask",
@ -111,6 +112,7 @@ TEST(FileTest, TopologicallyOrderedDescriptors) {
"RedactedFields.MapUnredactedStringEntry", "RedactedFields.MapUnredactedStringEntry",
"RedactedFields.MapRedactedStringEntry", "RedactedFields.MapRedactedStringEntry",
"OptionalGroup_extension", "OptionalGroup_extension",
"OpenEnumMessage",
"OneString", "OneString",
"OneBytes", "OneBytes",
"MoreString", "MoreString",

@ -3599,8 +3599,10 @@ void Descriptor::DebugString(int depth, std::string* contents,
if (reserved_name_count() > 0) { if (reserved_name_count() > 0) {
absl::SubstituteAndAppend(contents, "$0 reserved ", prefix); absl::SubstituteAndAppend(contents, "$0 reserved ", prefix);
for (int i = 0; i < reserved_name_count(); i++) { for (int i = 0; i < reserved_name_count(); i++) {
absl::SubstituteAndAppend(contents, "\"$0\", ", absl::SubstituteAndAppend(
absl::CEscape(reserved_name(i))); contents,
file()->edition() < Edition::EDITION_2023 ? "\"$0\", " : "$0, ",
absl::CEscape(reserved_name(i)));
} }
contents->replace(contents->size() - 2, 2, ";\n"); contents->replace(contents->size() - 2, 2, ";\n");
} }
@ -3819,8 +3821,10 @@ void EnumDescriptor::DebugString(
if (reserved_name_count() > 0) { if (reserved_name_count() > 0) {
absl::SubstituteAndAppend(contents, "$0 reserved ", prefix); absl::SubstituteAndAppend(contents, "$0 reserved ", prefix);
for (int i = 0; i < reserved_name_count(); i++) { for (int i = 0; i < reserved_name_count(); i++) {
absl::SubstituteAndAppend(contents, "\"$0\", ", absl::SubstituteAndAppend(
absl::CEscape(reserved_name(i))); contents,
file()->edition() < Edition::EDITION_2023 ? "\"$0\", " : "$0, ",
absl::CEscape(reserved_name(i)));
} }
contents->replace(contents->size() - 2, 2, ";\n"); contents->replace(contents->size() - 2, 2, ";\n");
} }

@ -4500,6 +4500,28 @@ TEST_F(ValidationErrorTest, ReservedFieldsDebugString) {
file->DebugString()); file->DebugString());
} }
TEST_F(ValidationErrorTest, ReservedFieldsDebugString2023) {
const FileDescriptor* file = BuildFile(R"pb(
syntax: "editions"
edition: EDITION_2023
name: "foo.proto"
message_type {
name: "Foo"
reserved_name: "foo"
reserved_name: "bar"
reserved_range { start: 5 end: 6 }
reserved_range { start: 10 end: 20 }
})pb");
ASSERT_EQ(
"edition = \"2023\";\n\n"
"message Foo {\n"
" reserved 5, 10 to 19;\n"
" reserved foo, bar;\n"
"}\n\n",
file->DebugString());
}
TEST_F(ValidationErrorTest, DebugStringReservedRangeMax) { TEST_F(ValidationErrorTest, DebugStringReservedRangeMax) {
const FileDescriptor* file = BuildFile(absl::Substitute( const FileDescriptor* file = BuildFile(absl::Substitute(
"name: \"foo.proto\" " "name: \"foo.proto\" "
@ -4686,6 +4708,37 @@ TEST_F(ValidationErrorTest, EnumReservedFieldsDebugString) {
file->DebugString()); file->DebugString());
} }
TEST_F(ValidationErrorTest, EnumReservedFieldsDebugString2023) {
const FileDescriptor* file = BuildFile(R"pb(
syntax: "editions"
edition: EDITION_2023
name: "foo.proto"
enum_type {
name: "Foo"
value { name: "FOO" number: 3 }
options { features { enum_type: CLOSED } }
reserved_name: "foo"
reserved_name: "bar"
reserved_range { start: -6 end: -6 }
reserved_range { start: -5 end: -4 }
reserved_range { start: -1 end: 1 }
reserved_range { start: 5 end: 5 }
reserved_range { start: 10 end: 19 }
})pb");
ASSERT_EQ(
"edition = \"2023\";\n\n"
"enum Foo {\n"
" option features = {\n"
" enum_type: CLOSED\n"
" };\n"
" FOO = 3;\n"
" reserved -6, -5 to -4, -1 to 1, 5, 10 to 19;\n"
" reserved foo, bar;\n"
"}\n\n",
file->DebugString());
}
TEST_F(ValidationErrorTest, InvalidDefaults) { TEST_F(ValidationErrorTest, InvalidDefaults) {
BuildFileWithErrors( BuildFileWithErrors(
"name: \"foo.proto\" " "name: \"foo.proto\" "

@ -77,6 +77,8 @@ class DynamicMessageTest : public ::testing::TestWithParam<bool> {
const Message* prototype_; const Message* prototype_;
const Descriptor* extensions_descriptor_; const Descriptor* extensions_descriptor_;
const Message* extensions_prototype_; const Message* extensions_prototype_;
const Descriptor* packed_extensions_descriptor_;
const Message* packed_extensions_prototype_;
const Descriptor* packed_descriptor_; const Descriptor* packed_descriptor_;
const Message* packed_prototype_; const Message* packed_prototype_;
const Descriptor* oneof_descriptor_; const Descriptor* oneof_descriptor_;
@ -98,6 +100,12 @@ class DynamicMessageTest : public ::testing::TestWithParam<bool> {
ASSERT_TRUE(extensions_descriptor_ != nullptr); ASSERT_TRUE(extensions_descriptor_ != nullptr);
extensions_prototype_ = factory_.GetPrototype(extensions_descriptor_); extensions_prototype_ = factory_.GetPrototype(extensions_descriptor_);
packed_extensions_descriptor_ =
pool_.FindMessageTypeByName("protobuf_unittest.TestPackedExtensions");
ASSERT_TRUE(packed_extensions_descriptor_ != nullptr);
packed_extensions_prototype_ =
factory_.GetPrototype(packed_extensions_descriptor_);
packed_descriptor_ = packed_descriptor_ =
pool_.FindMessageTypeByName("protobuf_unittest.TestPackedTypes"); pool_.FindMessageTypeByName("protobuf_unittest.TestPackedTypes");
ASSERT_TRUE(packed_descriptor_ != nullptr); ASSERT_TRUE(packed_descriptor_ != nullptr);
@ -162,6 +170,21 @@ TEST_P(DynamicMessageTest, Extensions) {
} }
} }
TEST_P(DynamicMessageTest, PackedExtensions) {
// Check that extensions work.
Arena arena;
Message* message =
packed_extensions_prototype_->New(GetParam() ? &arena : nullptr);
TestUtil::ReflectionTester reflection_tester(packed_extensions_descriptor_);
reflection_tester.SetPackedFieldsViaReflection(message);
reflection_tester.ExpectPackedFieldsSetViaReflection(*message);
if (!GetParam()) {
delete message;
}
}
TEST_P(DynamicMessageTest, PackedFields) { TEST_P(DynamicMessageTest, PackedFields) {
// Check that packed fields work properly. // Check that packed fields work properly.
Arena arena; Arena arena;

@ -1193,6 +1193,9 @@ TEST(ExtensionSetTest, DynamicExtensions) {
// Test adding a dynamic extension to a compiled-in message object. // Test adding a dynamic extension to a compiled-in message object.
FileDescriptorProto dynamic_proto; FileDescriptorProto dynamic_proto;
unittest::TestDynamicExtensions::descriptor()->file()->CopyHeadingTo(
&dynamic_proto);
dynamic_proto.clear_dependency();
dynamic_proto.set_name("dynamic_extensions_test.proto"); dynamic_proto.set_name("dynamic_extensions_test.proto");
dynamic_proto.add_dependency( dynamic_proto.add_dependency(
unittest::TestAllExtensions::descriptor()->file()->name()); unittest::TestAllExtensions::descriptor()->file()->name());

@ -1768,8 +1768,7 @@ void Reflection::ListFields(const Message& message,
USAGE_MUTABLE_CHECK_ALL(Add##TYPENAME, REPEATED, CPPTYPE); \ USAGE_MUTABLE_CHECK_ALL(Add##TYPENAME, REPEATED, CPPTYPE); \
if (field->is_extension()) { \ if (field->is_extension()) { \
MutableExtensionSet(message)->Add##TYPENAME( \ MutableExtensionSet(message)->Add##TYPENAME( \
field->number(), field->type(), field->options().packed(), value, \ field->number(), field->type(), field->is_packed(), value, field); \
field); \
} else { \ } else { \
AddField<TYPE>(message, field, value); \ AddField<TYPE>(message, field, value); \
} \ } \

@ -360,6 +360,19 @@ TEST_P(GeneratedMessageReflectionSwapTest, Extensions) {
TestUtil::ExpectAllExtensionsSet(rhs); TestUtil::ExpectAllExtensionsSet(rhs);
} }
TEST_P(GeneratedMessageReflectionSwapTest, PackedExtensions) {
unittest::TestPackedExtensions lhs;
unittest::TestPackedExtensions rhs;
TestUtil::SetPackedExtensions(&lhs);
Swap(lhs.GetReflection(), &lhs, &rhs);
EXPECT_EQ(lhs.SerializeAsString(), "");
TestUtil::ExpectPackedExtensionsSet(rhs);
}
TEST_P(GeneratedMessageReflectionSwapTest, Unknown) { TEST_P(GeneratedMessageReflectionSwapTest, Unknown) {
unittest::TestEmptyMessage lhs, rhs; unittest::TestEmptyMessage lhs, rhs;
@ -695,6 +708,18 @@ TEST(GeneratedMessageReflectionTest, RemoveLastExtensions) {
TestUtil::ExpectLastRepeatedExtensionsRemoved(message); TestUtil::ExpectLastRepeatedExtensionsRemoved(message);
} }
TEST(GeneratedMessageReflectionTest, RemoveLastPackedExtensions) {
unittest::TestPackedExtensions message;
TestUtil::ReflectionTester reflection_tester(
unittest::TestPackedExtensions::descriptor());
TestUtil::SetPackedExtensions(&message);
reflection_tester.RemoveLastRepeatedsViaReflection(&message);
TestUtil::ExpectLastRepeatedExtensionsRemoved(message);
}
TEST(GeneratedMessageReflectionTest, ReleaseLast) { TEST(GeneratedMessageReflectionTest, ReleaseLast) {
unittest::TestAllTypes message; unittest::TestAllTypes message;
const Descriptor* descriptor = message.GetDescriptor(); const Descriptor* descriptor = message.GetDescriptor();
@ -795,6 +820,24 @@ TEST(GeneratedMessageReflectionTest, Extensions) {
TestUtil::ExpectRepeatedExtensionsModified(message); TestUtil::ExpectRepeatedExtensionsModified(message);
} }
TEST(GeneratedMessageReflectionTest, PackedExtensions) {
// Set every extension to a unique value then go back and check all those
// values.
unittest::TestPackedExtensions message;
// First set the extensions via the generated API (see b/366468123).
TestUtil::SetPackedExtensions(&message);
TestUtil::ExpectPackedExtensionsSet(message);
message.Clear();
TestUtil::ReflectionTester reflection_tester(
unittest::TestPackedExtensions::descriptor());
reflection_tester.SetPackedFieldsViaReflection(&message);
TestUtil::ExpectPackedExtensionsSet(message);
reflection_tester.ExpectPackedFieldsSetViaReflection(message);
}
TEST(GeneratedMessageReflectionTest, FindExtensionTypeByNumber) { TEST(GeneratedMessageReflectionTest, FindExtensionTypeByNumber) {
const Reflection* reflection = const Reflection* reflection =
unittest::TestAllExtensions::default_instance().GetReflection(); unittest::TestAllExtensions::default_instance().GetReflection();

@ -76,6 +76,8 @@ inline void ExpectOneofClear(const UNITTEST::TestOneof2& message);
inline void ExpectLastRepeatedsRemoved(const UNITTEST::TestAllTypes& message); inline void ExpectLastRepeatedsRemoved(const UNITTEST::TestAllTypes& message);
inline void ExpectLastRepeatedExtensionsRemoved( inline void ExpectLastRepeatedExtensionsRemoved(
const UNITTEST::TestAllExtensions& message); const UNITTEST::TestAllExtensions& message);
inline void ExpectLastRepeatedExtensionsRemoved(
const UNITTEST::TestPackedExtensions& message);
inline void ExpectLastRepeatedsReleased(const UNITTEST::TestAllTypes& message); inline void ExpectLastRepeatedsReleased(const UNITTEST::TestAllTypes& message);
inline void ExpectLastRepeatedExtensionsReleased( inline void ExpectLastRepeatedExtensionsReleased(
const UNITTEST::TestAllExtensions& message); const UNITTEST::TestAllExtensions& message);
@ -2073,6 +2075,42 @@ inline void TestUtil::ExpectLastRepeatedExtensionsRemoved(
EXPECT_EQ("225", message.GetExtension(UNITTEST::repeated_cord_extension, 0)); EXPECT_EQ("225", message.GetExtension(UNITTEST::repeated_cord_extension, 0));
} }
inline void TestUtil::ExpectLastRepeatedExtensionsRemoved(
const UNITTEST::TestPackedExtensions& message) {
// Test that one element was removed.
ASSERT_EQ(1, message.ExtensionSize(UNITTEST::packed_int32_extension));
ASSERT_EQ(1, message.ExtensionSize(UNITTEST::packed_int64_extension));
ASSERT_EQ(1, message.ExtensionSize(UNITTEST::packed_uint32_extension));
ASSERT_EQ(1, message.ExtensionSize(UNITTEST::packed_uint64_extension));
ASSERT_EQ(1, message.ExtensionSize(UNITTEST::packed_sint32_extension));
ASSERT_EQ(1, message.ExtensionSize(UNITTEST::packed_sint64_extension));
ASSERT_EQ(1, message.ExtensionSize(UNITTEST::packed_fixed32_extension));
ASSERT_EQ(1, message.ExtensionSize(UNITTEST::packed_fixed64_extension));
ASSERT_EQ(1, message.ExtensionSize(UNITTEST::packed_sfixed32_extension));
ASSERT_EQ(1, message.ExtensionSize(UNITTEST::packed_sfixed64_extension));
ASSERT_EQ(1, message.ExtensionSize(UNITTEST::packed_float_extension));
ASSERT_EQ(1, message.ExtensionSize(UNITTEST::packed_double_extension));
ASSERT_EQ(1, message.ExtensionSize(UNITTEST::packed_bool_extension));
ASSERT_EQ(1, message.ExtensionSize(UNITTEST::packed_enum_extension));
// Test that the remaining element is the correct one.
EXPECT_EQ(601, message.GetExtension(UNITTEST::packed_int32_extension, 0));
EXPECT_EQ(602, message.GetExtension(UNITTEST::packed_int64_extension, 0));
EXPECT_EQ(603, message.GetExtension(UNITTEST::packed_uint32_extension, 0));
EXPECT_EQ(604, message.GetExtension(UNITTEST::packed_uint64_extension, 0));
EXPECT_EQ(605, message.GetExtension(UNITTEST::packed_sint32_extension, 0));
EXPECT_EQ(606, message.GetExtension(UNITTEST::packed_sint64_extension, 0));
EXPECT_EQ(607, message.GetExtension(UNITTEST::packed_fixed32_extension, 0));
EXPECT_EQ(608, message.GetExtension(UNITTEST::packed_fixed64_extension, 0));
EXPECT_EQ(609, message.GetExtension(UNITTEST::packed_sfixed32_extension, 0));
EXPECT_EQ(610, message.GetExtension(UNITTEST::packed_sfixed64_extension, 0));
EXPECT_EQ(611, message.GetExtension(UNITTEST::packed_float_extension, 0));
EXPECT_EQ(612, message.GetExtension(UNITTEST::packed_double_extension, 0));
EXPECT_TRUE(message.GetExtension(UNITTEST::packed_bool_extension, 0));
EXPECT_EQ(UNITTEST::FOREIGN_BAR,
message.GetExtension(UNITTEST::packed_enum_extension, 0));
}
inline void TestUtil::ExpectLastRepeatedsReleased( inline void TestUtil::ExpectLastRepeatedsReleased(
const UNITTEST::TestAllTypes& message) { const UNITTEST::TestAllTypes& message) {
ASSERT_EQ(1, message.repeatedgroup_size()); ASSERT_EQ(1, message.repeatedgroup_size());

File diff suppressed because it is too large Load Diff
Loading…
Cancel
Save