Chiebot-Mirror
/
protobuf
mirror of https://github.com/protocolbuffers/protobuf.git
8
0
Fork 0
Code Issues Projects Releases Wiki Activity

Fix reserializing of nodes for `map<uint32, Enum>` when the enum value is

Browse Source 
unknown and the key has the high bit on.
It was being serialized as signed, which caused it to be sign extended. The
value is equivalent after parsing as 32-bit, but it is inconsistent with other forms of
serialization.

PiperOrigin-RevId: 523428645
pull/12359/head
Protobuf Team Bot 2 years ago committed by Copybara-Service
parent f11079b01f
commit c23f226585
3 changed files with 73 additions and 33 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Unified View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 44
      src/google/protobuf/generated_message_tctable_decl.h
  2. 12
      src/google/protobuf/generated_message_tctable_lite.cc
  3. 50
      src/google/protobuf/map_test.inc

44
src/google/protobuf/generated_message_tctable_decl.h
Unescape View File

@ -173,12 +173,11 @@ class MapTypeCard {
enum CppType { kBool, k32, k64, kString, kMessage }; enum CppType { kBool, k32, k64, kString, kMessage };
MapTypeCard() = default; MapTypeCard() = default;
constexpr MapTypeCard(WireFormatLite::WireType wiretype, CppType cpp_type, constexpr MapTypeCard(WireFormatLite::WireType wiretype, CppType cpp_type,
bool is_zigzag_utf8) bool is_zigzag_utf8, bool is_signed)
: data_( : data_(static_cast<uint8_t>((static_cast<uint8_t>(wiretype) << 0) |
static_cast<uint8_t>((static_cast<uint8_t>(wiretype) << 0) | (static_cast<uint8_t>(cpp_type) << 3) |
(static_cast<uint8_t>(cpp_type) << 3) | (static_cast<uint8_t>(is_zigzag_utf8) << 6) |
(static_cast<uint8_t>(is_zigzag_utf8) << 6))) { (static_cast<uint8_t>(is_signed) << 7))) {}
}
WireFormatLite::WireType wiretype() const { WireFormatLite::WireType wiretype() const {
return static_cast<WireFormatLite::WireType>((data_ >> 0) & 0x7); return static_cast<WireFormatLite::WireType>((data_ >> 0) & 0x7);
@ -186,6 +185,11 @@ class MapTypeCard {
CppType cpp_type() const { return static_cast<CppType>((data_ >> 3) & 0x7); } CppType cpp_type() const { return static_cast<CppType>((data_ >> 3) & 0x7); }
bool is_signed() const {
ABSL_DCHECK(cpp_type() == CppType::k32 || cpp_type() == CppType::k64);
return static_cast<bool>(data_ >> 7);
}
bool is_zigzag() const { bool is_zigzag() const {
ABSL_DCHECK(wiretype() == WireFormatLite::WIRETYPE_VARINT); ABSL_DCHECK(wiretype() == WireFormatLite::WIRETYPE_VARINT);
ABSL_DCHECK(cpp_type() == CppType::k32 || cpp_type() == CppType::k64); ABSL_DCHECK(cpp_type() == CppType::k32 || cpp_type() == CppType::k64);
@ -207,43 +211,51 @@ static_assert(sizeof(MapTypeCard) == sizeof(uint8_t), "");
constexpr MapTypeCard MakeMapTypeCard(WireFormatLite::FieldType type) { constexpr MapTypeCard MakeMapTypeCard(WireFormatLite::FieldType type) {
switch (type) { switch (type) {
case WireFormatLite::TYPE_FLOAT: case WireFormatLite::TYPE_FLOAT:
return {WireFormatLite::WIRETYPE_FIXED32, MapTypeCard::k32, false, true};
case WireFormatLite::TYPE_FIXED32: case WireFormatLite::TYPE_FIXED32:
return {WireFormatLite::WIRETYPE_FIXED32, MapTypeCard::k32, false, false};
case WireFormatLite::TYPE_SFIXED32: case WireFormatLite::TYPE_SFIXED32:
return {WireFormatLite::WIRETYPE_FIXED32, MapTypeCard::k32, false}; return {WireFormatLite::WIRETYPE_FIXED32, MapTypeCard::k32, false, true};
case WireFormatLite::TYPE_DOUBLE: case WireFormatLite::TYPE_DOUBLE:
return {WireFormatLite::WIRETYPE_FIXED64, MapTypeCard::k64, false, true};
case WireFormatLite::TYPE_FIXED64: case WireFormatLite::TYPE_FIXED64:
return {WireFormatLite::WIRETYPE_FIXED64, MapTypeCard::k64, false, false};
case WireFormatLite::TYPE_SFIXED64: case WireFormatLite::TYPE_SFIXED64:
return {WireFormatLite::WIRETYPE_FIXED64, MapTypeCard::k64, false}; return {WireFormatLite::WIRETYPE_FIXED64, MapTypeCard::k64, false, true};
case WireFormatLite::TYPE_BOOL: case WireFormatLite::TYPE_BOOL:
return {WireFormatLite::WIRETYPE_VARINT, MapTypeCard::kBool, false}; return {WireFormatLite::WIRETYPE_VARINT, MapTypeCard::kBool, false,
false};
case WireFormatLite::TYPE_ENUM: case WireFormatLite::TYPE_ENUM:
// Enum validation is handled via `value_is_validated_enum` below. // Enum validation is handled via `value_is_validated_enum` below.
return {WireFormatLite::WIRETYPE_VARINT, MapTypeCard::k32, false, true};
case WireFormatLite::TYPE_INT32: case WireFormatLite::TYPE_INT32:
return {WireFormatLite::WIRETYPE_VARINT, MapTypeCard::k32, false, true};
case WireFormatLite::TYPE_UINT32: case WireFormatLite::TYPE_UINT32:
return {WireFormatLite::WIRETYPE_VARINT, MapTypeCard::k32, false}; return {WireFormatLite::WIRETYPE_VARINT, MapTypeCard::k32, false, false};
case WireFormatLite::TYPE_INT64: case WireFormatLite::TYPE_INT64:
return {WireFormatLite::WIRETYPE_VARINT, MapTypeCard::k64, false, true};
case WireFormatLite::TYPE_UINT64: case WireFormatLite::TYPE_UINT64:
return {WireFormatLite::WIRETYPE_VARINT, MapTypeCard::k64, false}; return {WireFormatLite::WIRETYPE_VARINT, MapTypeCard::k64, false, false};
case WireFormatLite::TYPE_SINT32: case WireFormatLite::TYPE_SINT32:
return {WireFormatLite::WIRETYPE_VARINT, MapTypeCard::k32, true}; return {WireFormatLite::WIRETYPE_VARINT, MapTypeCard::k32, true, true};
case WireFormatLite::TYPE_SINT64: case WireFormatLite::TYPE_SINT64:
return {WireFormatLite::WIRETYPE_VARINT, MapTypeCard::k64, true}; return {WireFormatLite::WIRETYPE_VARINT, MapTypeCard::k64, true, true};
case WireFormatLite::TYPE_STRING: case WireFormatLite::TYPE_STRING:
return {WireFormatLite::WIRETYPE_LENGTH_DELIMITED, MapTypeCard::kString, return {WireFormatLite::WIRETYPE_LENGTH_DELIMITED, MapTypeCard::kString,
true}; true, false};
case WireFormatLite::TYPE_BYTES: case WireFormatLite::TYPE_BYTES:
return {WireFormatLite::WIRETYPE_LENGTH_DELIMITED, MapTypeCard::kString, return {WireFormatLite::WIRETYPE_LENGTH_DELIMITED, MapTypeCard::kString,
false}; false, false};
case WireFormatLite::TYPE_MESSAGE: case WireFormatLite::TYPE_MESSAGE:
return {WireFormatLite::WIRETYPE_LENGTH_DELIMITED, MapTypeCard::kMessage, return {WireFormatLite::WIRETYPE_LENGTH_DELIMITED, MapTypeCard::kMessage,
false}; false, false};
default: default:
PROTOBUF_ASSUME(false); PROTOBUF_ASSUME(false);

12
src/google/protobuf/generated_message_tctable_lite.cc
Unescape View File

@ -2377,10 +2377,14 @@ static void SerializeMapKey(const NodeBase* node, MapTypeCard type_card,
WireFormatLite::WriteSInt32( WireFormatLite::WriteSInt32(
1, static_cast<const KeyNode<uint32_t>*>(node)->key(), 1, static_cast<const KeyNode<uint32_t>*>(node)->key(),
&coded_output); &coded_output);
} else { } else if (type_card.is_signed()) {
WireFormatLite::WriteInt32( WireFormatLite::WriteInt32(
1, static_cast<const KeyNode<uint32_t>*>(node)->key(), 1, static_cast<const KeyNode<uint32_t>*>(node)->key(),
&coded_output); &coded_output);
} else {
WireFormatLite::WriteUInt32(
1, static_cast<const KeyNode<uint32_t>*>(node)->key(),
&coded_output);
} }
break; break;
case MapTypeCard::k64: case MapTypeCard::k64:
@ -2388,10 +2392,14 @@ static void SerializeMapKey(const NodeBase* node, MapTypeCard type_card,
WireFormatLite::WriteSInt64( WireFormatLite::WriteSInt64(
1, static_cast<const KeyNode<uint64_t>*>(node)->key(), 1, static_cast<const KeyNode<uint64_t>*>(node)->key(),
&coded_output); &coded_output);
} else { } else if (type_card.is_signed()) {
WireFormatLite::WriteInt64( WireFormatLite::WriteInt64(
1, static_cast<const KeyNode<uint64_t>*>(node)->key(), 1, static_cast<const KeyNode<uint64_t>*>(node)->key(),
&coded_output); &coded_output);
} else {
WireFormatLite::WriteUInt64(
1, static_cast<const KeyNode<uint64_t>*>(node)->key(),
&coded_output);
} }
break; break;
default: default:

50
src/google/protobuf/map_test.inc
Unescape View File

@ -2764,12 +2764,11 @@ TEST(GeneratedMapFieldTest, Proto2UnknownEnum) {
} }
TEST(GeneratedMapFieldTest, Proto2UnknownEnumAllKeyTypesWork) { TEST(GeneratedMapFieldTest, Proto2UnknownEnumAllKeyTypesWork) {
#define PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(Type, Value) \ #define PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(Type, ...) \
{ \ ABSL_LOG(INFO) << "Testing " << #Type; \
for (auto value : {__VA_ARGS__}) { \
UNITTEST::TestEnumMapPlusExtra from; \ UNITTEST::TestEnumMapPlusExtra from; \
auto value = Value; \ (*from.mutable_unknown_map_field_##Type())[value] = \
ABSL_LOG(INFO) << "Testing " << #Type; \
(*from.mutable_unknown_map_field_##Type())[Value] = \
UNITTEST::E_PROTO2_MAP_ENUM_EXTRA; \ UNITTEST::E_PROTO2_MAP_ENUM_EXTRA; \
UNITTEST::TestEnumMap to; \ UNITTEST::TestEnumMap to; \
ASSERT_TRUE(to.ParseFromString(from.SerializeAsString())); \ ASSERT_TRUE(to.ParseFromString(from.SerializeAsString())); \
@ -2783,20 +2782,41 @@ TEST(GeneratedMapFieldTest, Proto2UnknownEnumAllKeyTypesWork) {
EXPECT_TRUE(from.ParseFromString(to.SerializeAsString())); \ EXPECT_TRUE(from.ParseFromString(to.SerializeAsString())); \
EXPECT_THAT(from.unknown_map_field_##Type(), \ EXPECT_THAT(from.unknown_map_field_##Type(), \
ElementsAre(Pair(value, UNITTEST::E_PROTO2_MAP_ENUM_EXTRA))); \ ElementsAre(Pair(value, UNITTEST::E_PROTO2_MAP_ENUM_EXTRA))); \
EXPECT_EQ(from.SerializeAsString(), to.SerializeAsString()); \
} }
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(int64, 17); PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(int64, int64_t{17},
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(uint64, 17); std::numeric_limits<int64_t>::min(),
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(int32, 17); std::numeric_limits<int64_t>::max());
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(uint32, 17); PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(uint64, uint64_t{17},
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(fixed32, 17); std::numeric_limits<uint64_t>::min(),
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(fixed64, 17); std::numeric_limits<uint64_t>::max());
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(int32, int32_t{17},
std::numeric_limits<int32_t>::min(),
std::numeric_limits<int32_t>::max());
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(uint32, uint32_t{17},
std::numeric_limits<uint32_t>::min(),
std::numeric_limits<uint32_t>::max());
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(fixed32, uint32_t{17},
std::numeric_limits<uint32_t>::min(),
std::numeric_limits<uint32_t>::max());
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(fixed64, uint64_t{17},
std::numeric_limits<uint64_t>::min(),
std::numeric_limits<uint64_t>::max());
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(bool, true); PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(bool, true);
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(string, "17"); PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(string, "17");
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(sint32, 17); PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(sint32, int32_t{17},
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(sint64, 17); std::numeric_limits<int32_t>::min(),
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(sfixed32, 17); std::numeric_limits<int32_t>::max());
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(sfixed64, 17); PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(sint64, int64_t{17},
std::numeric_limits<int64_t>::min(),
std::numeric_limits<int64_t>::max());
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(sfixed32, int32_t{17},
std::numeric_limits<int32_t>::min(),
std::numeric_limits<int32_t>::max());
PROTOBUF_INTERNAL_TEST_MAP_KEY_TYPE(sfixed64, int64_t{17},
std::numeric_limits<int64_t>::min(),
std::numeric_limits<int64_t>::max());
} }
TEST(GeneratedMapFieldTest, StandardWireFormat) { TEST(GeneratedMapFieldTest, StandardWireFormat) {

Write Preview
Loading…
Cancel
Save