From f4f9aec52f5b4c46ff32fb9527229da081a14e11 Mon Sep 17 00:00:00 2001
From: Petr Prokhorenkov <prokhorenkov@gmail.com>
Date: Thu, 14 Apr 2016 11:17:54 +0300
Subject: 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.
---
 src/google/protobuf/compiler/cpp/cpp_enum_field.cc |  6 +++--
 src/google/protobuf/lite_unittest.cc               | 27 ++++++++++++++++++++++
 src/google/protobuf/unittest_lite.proto            | 19 +++++++++++++++
 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 ];
+}
-- 
cgit v1.2.3