Merge pull request protocolbuffers#8505 from haberman/sync-stage

Integrate from Piper for C++, Java, and Python

CHANGES.txt

@@ -5,6 +5,7 @@ Unreleased Changes (C++/Java/Python/PHP/Objective-C/C#/Ruby/JavaScript)
   * Fix bug where `Descriptor::DebugString()` printed proto3 synthetic oneofs.
   * Provide stable versions of `SortAndUnique()`.
   * Make sure to cache proto3 optional message fields when they are cleared.
+  * Expose UnsafeArena methods to Reflection.
 
   Kotlin
   * Restrict extension setter and getter operators to non-nullable T.

conformance/failure_list_java_lite.txt

@@ -0,0 +1,10 @@
+# This is the list of conformance tests that are known to fail for the Java
+# implementation right now. These should be fixed.
+#
+# By listing them here we can keep tabs on which ones are failing and be sure
+# that we don't introduce regressions in other tests.
+
+Required.Proto3.ProtobufInput.PrematureEofInDelimitedDataForKnownNonRepeatedValue.MESSAGE
+Required.Proto3.ProtobufInput.PrematureEofInDelimitedDataForKnownRepeatedValue.MESSAGE
+Required.Proto2.ProtobufInput.PrematureEofInDelimitedDataForKnownNonRepeatedValue.MESSAGE
+Required.Proto2.ProtobufInput.PrematureEofInDelimitedDataForKnownRepeatedValue.MESSAGE

conformance/text_format_failure_list_java_lite.txt

@@ -0,0 +1,5 @@
+# This is the list of conformance tests that are known to fail for the Java
+# Lite TextFormat implementation right now. These should be fixed.
+#
+# By listing them here we can keep tabs on which ones are failing and be sure
+# that we don't introduce regressions in other tests.

src/google/protobuf/map_test.cc

@@ -2415,6 +2415,21 @@ TEST(GeneratedMapFieldTest, SerializationToStream) {
   MapTestUtil::ExpectMapFieldsSet(message2);
 }
 
+TEST(GeneratedMapFieldTest, ParseFailsIfMalformed) {
+  unittest::TestMapSubmessage o, p;
+  auto m = o.mutable_test_map()->mutable_map_int32_foreign_message();
+  (*m)[0].set_c(-1);
+  std::string serialized;
+  EXPECT_TRUE(o.SerializeToString(&serialized));
+
+  // Should parse correctly.
+  EXPECT_TRUE(p.ParseFromString(serialized));
+
+  // Overwriting the last byte to 0xFF results in malformed wire.
+  serialized[serialized.size() - 1] = 0xFF;
+  EXPECT_FALSE(p.ParseFromString(serialized));
+}
+
 
 TEST(GeneratedMapFieldTest, SameTypeMaps) {
   const Descriptor* map1 = unittest::TestSameTypeMap::descriptor()

src/google/protobuf/message.h

@@ -592,12 +592,20 @@ class PROTOBUF_EXPORT Reflection final {
   // the compiled-in class for this type, NOT DynamicMessage.
   Message* MutableMessage(Message* message, const FieldDescriptor* field,
                           MessageFactory* factory = nullptr) const;
+
   // Replaces the message specified by 'field' with the already-allocated object
   // sub_message, passing ownership to the message.  If the field contained a
   // message, that message is deleted.  If sub_message is nullptr, the field is
   // cleared.
   void SetAllocatedMessage(Message* message, Message* sub_message,
                            const FieldDescriptor* field) const;
+
+  // Similar to `SetAllocatedMessage`, but omits all internal safety and
+  // ownership checks.  This method should only be used when the objects are on
+  // the same arena or paired with a call to `UnsafeArenaReleaseMessage`.
+  void UnsafeArenaSetAllocatedMessage(Message* message, Message* sub_message,
+                                      const FieldDescriptor* field) const;
+
   // Releases the message specified by 'field' and returns the pointer,
   // ReleaseMessage() will return the message the message object if it exists.
   // Otherwise, it may or may not return nullptr.  In any case, if the return
@@ -609,6 +617,13 @@ class PROTOBUF_EXPORT Reflection final {
       Message* message, const FieldDescriptor* field,
       MessageFactory* factory = nullptr) const;
 
+  // Similar to `ReleaseMessage`, but omits all internal safety and ownership
+  // checks.  This method should only be used when the objects are on the same
+  // arena or paired with a call to `UnsafeArenaSetAllocatedMessage`.
+  Message* UnsafeArenaReleaseMessage(Message* message,
+                                     const FieldDescriptor* field,
+                                     MessageFactory* factory = nullptr) const;
+
 
   // Repeated field getters ------------------------------------------
   // These get the value of one element of a repeated field.
@@ -1123,13 +1138,6 @@ class PROTOBUF_EXPORT Reflection final {
   void AddEnumValueInternal(Message* message, const FieldDescriptor* field,
                             int value) const;
 
-  Message* UnsafeArenaReleaseMessage(Message* message,
-                                     const FieldDescriptor* field,
-                                     MessageFactory* factory = nullptr) const;
-
-  void UnsafeArenaSetAllocatedMessage(Message* message, Message* sub_message,
-                                      const FieldDescriptor* field) const;
-
   friend inline  // inline so nobody can call this function.
       void
       RegisterAllTypesInternal(const Metadata* file_level_metadata, int size);

src/google/protobuf/message_unittest.inc

@@ -56,9 +56,11 @@
 #include <google/protobuf/arena.h>
 #include <google/protobuf/descriptor.h>
 #include <google/protobuf/generated_message_reflection.h>
+#include <gmock/gmock.h>
 #include <google/protobuf/testing/googletest.h>
 #include <gtest/gtest.h>
 #include <google/protobuf/stubs/logging.h>
+#include <google/protobuf/stubs/substitute.h>
 #include <google/protobuf/io/io_win32.h>
 
 
@@ -208,6 +210,171 @@ TEST(MESSAGE_TEST_NAME, ParseFailsIfNotInitialized) {
       errors[0]);
 }
 
+TEST(MESSAGE_TEST_NAME, ParseFailsIfSubmessageNotInitialized) {
+  UNITTEST::TestRequiredForeign source, message;
+  source.mutable_optional_message()->set_dummy2(100);
+  std::string serialized = source.SerializePartialAsString();
+
+  EXPECT_TRUE(message.ParsePartialFromString(serialized));
+  EXPECT_FALSE(message.IsInitialized());
+
+  std::vector<std::string> errors;
+  {
+    ScopedMemoryLog log;
+    EXPECT_FALSE(message.ParseFromString(source.SerializePartialAsString()));
+    errors = log.GetMessages(ERROR);
+  }
+
+  EXPECT_THAT(
+      errors,
+      testing::ElementsAre(
+          "Can't parse message of type \"" +
+          std::string(UNITTEST_PACKAGE_NAME) +
+          ".TestRequiredForeign\" because it is missing required fields: "
+          "optional_message.a, optional_message.b, optional_message.c"));
+}
+
+TEST(MESSAGE_TEST_NAME, ParseFailsIfExtensionNotInitialized) {
+  UNITTEST::TestChildExtension source, message;
+  auto* r = source.mutable_optional_extension()->MutableExtension(
+      UNITTEST::TestRequired::single);
+  r->set_dummy2(100);
+  std::string serialized = source.SerializePartialAsString();
+
+  EXPECT_TRUE(message.ParsePartialFromString(serialized));
+  EXPECT_FALSE(message.IsInitialized());
+
+  std::vector<std::string> errors;
+  {
+    ScopedMemoryLog log;
+    EXPECT_FALSE(message.ParseFromString(source.SerializePartialAsString()));
+    errors = log.GetMessages(ERROR);
+  }
+
+  EXPECT_THAT(errors,
+              testing::ElementsAre(strings::Substitute(
+                  "Can't parse message of type \"$0.TestChildExtension\" "
+                  "because it is missing required fields: "
+                  "optional_extension.($0.TestRequired.single).a, "
+                  "optional_extension.($0.TestRequired.single).b, "
+                  "optional_extension.($0.TestRequired.single).c",
+                  UNITTEST_PACKAGE_NAME)));
+}
+
+TEST(MESSAGE_TEST_NAME, ParseFailsIfSubmessageTruncated) {
+  UNITTEST::NestedTestAllTypes o, p;
+  constexpr int kDepth = 5;
+  auto* child = o.mutable_child();
+  for (int i = 0; i < kDepth; i++) {
+    child = child->mutable_child();
+  }
+  TestUtil::SetAllFields(child->mutable_payload());
+
+  std::string serialized;
+  EXPECT_TRUE(o.SerializeToString(&serialized));
+
+  // Should parse correctly.
+  EXPECT_TRUE(p.ParseFromString(serialized));
+
+  constexpr int kMaxTruncate = 50;
+  ASSERT_GT(serialized.size(), kMaxTruncate);
+
+  for (int i = 1; i < kMaxTruncate; i += 3) {
+    EXPECT_FALSE(
+        p.ParseFromString(serialized.substr(0, serialized.size() - i)));
+  }
+}
+
+TEST(MESSAGE_TEST_NAME, ParseFailsIfWireMalformed) {
+  UNITTEST::NestedTestAllTypes o, p;
+  constexpr int kDepth = 5;
+  auto* child = o.mutable_child();
+  for (int i = 0; i < kDepth; i++) {
+    child = child->mutable_child();
+  }
+  // -1 becomes \xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x1
+  child->mutable_payload()->set_optional_int32(-1);
+
+  std::string serialized;
+  EXPECT_TRUE(o.SerializeToString(&serialized));
+
+  // Should parse correctly.
+  EXPECT_TRUE(p.ParseFromString(serialized));
+
+  // Overwriting the last byte to 0xFF results in malformed wire.
+  serialized[serialized.size() - 1] = 0xFF;
+  EXPECT_FALSE(p.ParseFromString(serialized));
+}
+
+TEST(MESSAGE_TEST_NAME, ParseFailsIfOneofWireMalformed) {
+  UNITTEST::NestedTestAllTypes o, p;
+  constexpr int kDepth = 5;
+  auto* child = o.mutable_child();
+  for (int i = 0; i < kDepth; i++) {
+    child = child->mutable_child();
+  }
+  // -1 becomes \xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x1
+  child->mutable_payload()->mutable_oneof_nested_message()->set_bb(-1);
+
+  std::string serialized;
+  EXPECT_TRUE(o.SerializeToString(&serialized));
+
+  // Should parse correctly.
+  EXPECT_TRUE(p.ParseFromString(serialized));
+
+  // Overwriting the last byte to 0xFF results in malformed wire.
+  serialized[serialized.size() - 1] = 0xFF;
+  EXPECT_FALSE(p.ParseFromString(serialized));
+}
+
+TEST(MESSAGE_TEST_NAME, ParseFailsIfExtensionWireMalformed) {
+  UNITTEST::TestChildExtension o, p;
+  auto* m = o.mutable_optional_extension()->MutableExtension(
+      UNITTEST::optional_nested_message_extension);
+
+  // -1 becomes \xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x1
+  m->set_bb(-1);
+
+  std::string serialized;
+  EXPECT_TRUE(o.SerializeToString(&serialized));
+
+  // Should parse correctly.
+  EXPECT_TRUE(p.ParseFromString(serialized));
+
+  // Overwriting the last byte to 0xFF results in malformed wire.
+  serialized[serialized.size() - 1] = 0xFF;
+  EXPECT_FALSE(p.ParseFromString(serialized));
+}
+
+TEST(MESSAGE_TEST_NAME, Swap) {
+  UNITTEST::NestedTestAllTypes o;
+  constexpr int kDepth = 5;
+  auto* child = o.mutable_child();
+  for (int i = 0; i < kDepth; i++) {
+    child = child->mutable_child();
+  }
+  TestUtil::SetAllFields(child->mutable_payload());
+
+  std::string serialized;
+  EXPECT_TRUE(o.SerializeToString(&serialized));
+
+  {
+    Arena arena;
+    UNITTEST::NestedTestAllTypes* p1 =
+        Arena::CreateMessage<UNITTEST::NestedTestAllTypes>(&arena);
+
+    // Should parse correctly.
+    EXPECT_TRUE(p1->ParseFromString(serialized));
+
+    UNITTEST::NestedTestAllTypes* p2 =
+        Arena::CreateMessage<UNITTEST::NestedTestAllTypes>(&arena);
+
+    p1->Swap(p2);
+
+    EXPECT_EQ(o.SerializeAsString(), p2->SerializeAsString());
+  }
+}
+
 TEST(MESSAGE_TEST_NAME, BypassInitializationCheckOnParse) {
   UNITTEST::TestRequired message;
   io::ArrayInputStream raw_input(nullptr, 0);

src/google/protobuf/unittest.proto

@@ -364,6 +364,12 @@ message TestNestedExtension {
   }
 }
 
+message TestChildExtension {
+  optional string a = 1;
+  optional string b = 2;
+  optional TestAllExtensions optional_extension = 3;
+}
+
 // We have separate messages for testing required fields because it's
 // annoying to have to fill in required fields in TestProto in order to
 // do anything with it.  Note that we don't need to test every type of

src/google/protobuf/wire_format_unittest.cc

@@ -641,7 +641,7 @@ void SerializeReverseOrder(const unittest::TestMessageSetExtension1& message,
                            io::CodedOutputStream* coded_output) {
   WireFormatLite::WriteTag(15,  // i
                            WireFormatLite::WIRETYPE_VARINT, coded_output);
-  coded_output->WriteVarint32(message.i());
+  coded_output->WriteVarint64(message.i());
   WireFormatLite::WriteTag(16,  // recursive
                            WireFormatLite::WIRETYPE_LENGTH_DELIMITED,
                            coded_output);
@@ -692,6 +692,66 @@ TEST(WireFormatTest, ParseMessageSetWithDeepRecReverseOrder) {
   EXPECT_FALSE(message_set.ParseFromString(data));
 }
 
+TEST(WireFormatTest, ParseFailMalformedMessageSet) {
+  constexpr int kDepth = 5;
+  std::string data;
+  {
+    proto2_wireformat_unittest::TestMessageSet message_set;
+    proto2_wireformat_unittest::TestMessageSet* mset = &message_set;
+    for (int i = 0; i < kDepth; i++) {
+      auto m = mset->MutableExtension(
+          unittest::TestMessageSetExtension1::message_set_extension);
+      m->set_i(i);
+      mset = m->mutable_recursive();
+    }
+    auto m = mset->MutableExtension(
+        unittest::TestMessageSetExtension1::message_set_extension);
+    // -1 becomes \xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x1
+    m->set_i(-1);
+
+    EXPECT_TRUE(message_set.SerializeToString(&data));
+    // Make the proto mal-formed.
+    data[data.size() - 2 - kDepth] = 0xFF;
+  }
+
+  proto2_wireformat_unittest::TestMessageSet message_set;
+  EXPECT_FALSE(message_set.ParseFromString(data));
+}
+
+TEST(WireFormatTest, ParseFailMalformedMessageSetReverseOrder) {
+  constexpr int kDepth = 5;
+  std::string data;
+  {
+    proto2_wireformat_unittest::TestMessageSet message_set;
+    proto2_wireformat_unittest::TestMessageSet* mset = &message_set;
+    for (int i = 0; i < kDepth; i++) {
+      auto m = mset->MutableExtension(
+          unittest::TestMessageSetExtension1::message_set_extension);
+      m->set_i(i);
+      mset = m->mutable_recursive();
+    }
+    auto m = mset->MutableExtension(
+        unittest::TestMessageSetExtension1::message_set_extension);
+    // -1 becomes \xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\x1
+    m->set_i(-1);
+    // SerializeReverseOrder() assumes "recursive" is always present.
+    m->mutable_recursive();
+
+    message_set.ByteSizeLong();
+
+    // Serialize with reverse payload tag order
+    io::StringOutputStream output_stream(&data);
+    io::CodedOutputStream coded_output(&output_stream);
+    SerializeReverseOrder(message_set, &coded_output);
+  }
+
+  // Make varint for -1 malformed.
+  data[data.size() - 5 * (kDepth + 1) - 4] = 0xFF;
+
+  proto2_wireformat_unittest::TestMessageSet message_set;
+  EXPECT_FALSE(message_set.ParseFromString(data));
+}
+
 TEST(WireFormatTest, ParseBrokenMessageSet) {
   proto2_wireformat_unittest::TestMessageSet message_set;
   std::string input("goodbye");  // Invalid wire format data.