Commit f4f9aec5 authored by Petr Prokhorenkov's avatar Petr Prokhorenkov

Fix bug with silent message corruption in LITE_RUNTIME.

A protobuf message will be corrupted in the following scenario:
  1. Use LITE_RUNTIME.
  2. Have an optional enum field following some other field.
  3. Update protocol by adding new values to the enum.
  4. Have an old client parse and serialize a message having enum field
      set to a value the client does not understand.
  5. Field preceeding the enum is now corrupted.

The bug is due to the fact that optimized fallthrough in parser code
does not update variablle 'tag' when jumping to the parser code for the
next field.
parent 814685ca
......@@ -36,6 +36,7 @@
#include <google/protobuf/compiler/cpp/cpp_helpers.h>
#include <google/protobuf/io/printer.h>
#include <google/protobuf/stubs/strutil.h>
#include <google/protobuf/wire_format.h>
namespace google {
namespace protobuf {
......@@ -142,8 +143,9 @@ GenerateMergeFromCodedStream(io::Printer* printer) const {
} else {
printer->Print(
"} else {\n"
" unknown_fields_stream.WriteVarint32(tag);\n"
" unknown_fields_stream.WriteVarint32(value);\n");
" unknown_fields_stream.WriteVarint32($tag$);\n"
" unknown_fields_stream.WriteVarint32(value);\n",
"tag", SimpleItoa(internal::WireFormat::MakeTag(descriptor_)));
}
printer->Print(variables_,
"}\n");
......
......@@ -686,6 +686,33 @@ int main(int argc, char* argv[]) {
EXPECT_TRUE(map_message.IsInitialized());
}
{
// Check that adding more values to enum does not corrupt message
// when passed through an old client.
protobuf_unittest::V2MessageLite v2_message;
v2_message.set_int_field(800);
// Set enum field to the value not understood by the old client.
v2_message.set_enum_field(protobuf_unittest::V2_SECOND);
string v2_bytes = v2_message.SerializeAsString();
protobuf_unittest::V1MessageLite v1_message;
v1_message.ParseFromString(v2_bytes);
EXPECT_TRUE(v1_message.IsInitialized());
EXPECT_EQ(v1_message.int_field(), v2_message.int_field());
// V1 client does not understand V2_SECOND value, so it discards it and
// uses default value instead.
EXPECT_EQ(v1_message.enum_field(), protobuf_unittest::V1_FIRST);
// However, when re-serialized, it should preserve enum value.
string v1_bytes = v1_message.SerializeAsString();
protobuf_unittest::V2MessageLite same_v2_message;
same_v2_message.ParseFromString(v1_bytes);
EXPECT_EQ(v2_message.int_field(), same_v2_message.int_field());
EXPECT_EQ(v2_message.enum_field(), same_v2_message.enum_field());
}
std::cout << "PASS" << std::endl;
return 0;
}
......@@ -386,3 +386,22 @@ message TestEmptyMessageLite{
message TestEmptyMessageWithExtensionsLite {
extensions 1 to max;
}
enum V1EnumLite {
V1_FIRST = 1;
}
enum V2EnumLite {
V2_FIRST = 1;
V2_SECOND = 2;
}
message V1MessageLite {
required int32 int_field = 1;
optional V1EnumLite enum_field = 2 [ default = V1_FIRST ];
}
message V2MessageLite {
required int32 int_field = 1;
optional V2EnumLite enum_field = 2 [ default = V2_FIRST ];
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment