aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorGravatar Petr Prokhorenkov <prokhorenkov@gmail.com>2016-04-14 11:17:54 +0300
committerGravatar Petr Prokhorenkov <prokhorenkov@gmail.com>2016-04-21 09:27:33 +0300
commitf4f9aec52f5b4c46ff32fb9527229da081a14e11 (patch)
tree02d8ff141ad7e0228bfcf6b2f5ee1322c5df5fbd
parent814685ca2cd9280ca401e1842fd6311440921a0a (diff)
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.
-rw-r--r--src/google/protobuf/compiler/cpp/cpp_enum_field.cc6
-rw-r--r--src/google/protobuf/lite_unittest.cc27
-rw-r--r--src/google/protobuf/unittest_lite.proto19
3 files changed, 50 insertions, 2 deletions
diff --git a/src/google/protobuf/compiler/cpp/cpp_enum_field.cc b/src/google/protobuf/compiler/cpp/cpp_enum_field.cc
index 824e2205..06cb18d1 100644
--- a/src/google/protobuf/compiler/cpp/cpp_enum_field.cc
+++ b/src/google/protobuf/compiler/cpp/cpp_enum_field.cc
@@ -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");
diff --git a/src/google/protobuf/lite_unittest.cc b/src/google/protobuf/lite_unittest.cc
index d1948ab5..3ca3fbaf 100644
--- a/src/google/protobuf/lite_unittest.cc
+++ b/src/google/protobuf/lite_unittest.cc
@@ -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;
}
diff --git a/src/google/protobuf/unittest_lite.proto b/src/google/protobuf/unittest_lite.proto
index 41ed845f..878ec7c1 100644
--- a/src/google/protobuf/unittest_lite.proto
+++ b/src/google/protobuf/unittest_lite.proto
@@ -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 ];
+}