C++ bindings: A struct's Deserialize() now does validation before deserializing.

mojo/public/cpp/bindings/tests/serialization_api_unittest.cc

Index: mojo/public/cpp/bindings/tests/serialization_api_unittest.cc
diff --git a/mojo/public/cpp/bindings/tests/serialization_api_unittest.cc b/mojo/public/cpp/bindings/tests/serialization_api_unittest.cc
index 28ada48a0609dc46c2422c27d222d6870e346c70..1f65b8cf91b0928cb91918f951bc53ef58be0ed6 100644
--- a/mojo/public/cpp/bindings/tests/serialization_api_unittest.cc
+++ b/mojo/public/cpp/bindings/tests/serialization_api_unittest.cc
@@ -23,25 +23,29 @@ class StructSerializationAPITest : public testing::Test {
   void SerializeAndDeserialize(
       Type* val,
       mojo::internal::ValidationError expected_validation_error) {
+    size_t bytes_written = 0;
     size_t num_bytes = val->GetSerializedSize();
     std::vector<uint8_t> bytes(num_bytes + 1);
 
     // Last byte is a magic value, helps catch a buffer overflow for
     // serialization.
     bytes[num_bytes] = 170;
-    val->Serialize(bytes.data(), num_bytes);
+    val->Serialize(bytes.data(), num_bytes, &bytes_written);
     EXPECT_EQ(170u, bytes[num_bytes]);
+    EXPECT_EQ(num_bytes, bytes_written);
 
     mojo::internal::BoundsChecker bounds_checker(bytes.data(), num_bytes, 0);
     auto actual_validation_error =
         Type::Data_::Validate(bytes.data(), &bounds_checker, nullptr);
     EXPECT_EQ(expected_validation_error, actual_validation_error);
 
+    Type out_val;
+    bool deserialize_ret = out_val.Deserialize(bytes.data(), bytes.size());
     if (actual_validation_error == mojo::internal::ValidationError::NONE) {
-      Type out_val;
-      out_val.Deserialize(bytes.data());
       EXPECT_TRUE(val->Equals(out_val));
     }
+    EXPECT_EQ(actual_validation_error == mojo::internal::ValidationError::NONE,
+              deserialize_ret);
   }
 
  private:
@@ -90,6 +94,23 @@ TEST_F(StructSerializationAPITest, BasicStructSerialization) {
     SerializeAndDeserialize(&default_values,
                             mojo::internal::ValidationError::NONE);
   }
+
+  {
+    SCOPED_TRACE("NoDefaultFieldValues.Serialize() should fail");
+    NoDefaultFieldValues nd;
+    nd.f0 = true;
+    nd.f23 = mojo::Array<mojo::String>::New(10);
+
+    char buf[1000];

[viettrungluu, 2016/03/30 17:49:01]

Please initialize this (|= {}| is sufficient).

[vardhan, 2016/03/30 22:49:07]

Done.

+    EXPECT_FALSE(nd.Serialize(buf, sizeof(buf)));
+
+    size_t bytes_written;

[viettrungluu, 2016/03/30 17:49:01]

Please initialize this, preferably to something that isn't 160.

[vardhan, 2016/03/30 22:49:07]

Done.

+    EXPECT_FALSE(nd.Serialize(buf, sizeof(buf), &bytes_written));
+    EXPECT_EQ(160UL, bytes_written);
+    // The Serialize() shouldn't get around to reserving space for the |f23|
+    // array field.
+    EXPECT_LT(bytes_written, nd.GetSerializedSize());
+  }
 }
 
 // This tests serialization of handles -- These should be deaths or
@@ -142,6 +163,47 @@ TEST_F(StructSerializationAPITest, NullableHandleSerialization) {
                           mojo::internal::ValidationError::NONE);
 }
 
+// Test that |Deserialize()| appropriately fails on validation.
+TEST_F(StructSerializationAPITest, DeserializationFailure) {
+  char* buf[100];

[viettrungluu, 2016/03/30 17:49:01]

a) Why is this an array of pointers?
b) Please initialize this.

[vardhan, 2016/03/30 22:49:07]

Oops!  thanks for catching this, bad typo.
Done.

+  EmptyStruct es;
+
+  // Bounds checker should fail this, since buf_size is too small.
+  EXPECT_FALSE(es.Deserialize(buf, 1));
+
+  es.Serialize(buf, sizeof(buf));
+  EXPECT_TRUE(es.Deserialize(buf, sizeof(buf)));
+
+  // Invalid struct header: this should happen inside
+  // EmptyStruct::Data_::Validate()).
+  es.Serialize(buf, sizeof(buf));
+  EmptyStruct::Data_* es_data = reinterpret_cast<EmptyStruct::Data_*>(buf);

[viettrungluu, 2016/03/30 17:49:01]

buf being an array of char*'s makes this UB.

[vardhan, 2016/03/30 22:49:07]

Acknowledged.

+  es_data->header_.num_bytes = 0;
+  EXPECT_FALSE(es.Deserialize(buf, sizeof(buf)));
+}
+
+TEST_F(StructSerializationAPITest, DeserializationWithoutValidation) {
+  const int32_t kMagic = 456;
+  char* buf[100];

[viettrungluu, 2016/03/30 17:49:01]

"

[vardhan, 2016/03/30 22:49:07]

Done.

+  ContainsOther::Data_* co_data = reinterpret_cast<ContainsOther::Data_*>(buf);
+  ContainsOther co;
+
+  // Success case.
+  co.other = kMagic;
+  memset(buf, 0, sizeof(buf));

[viettrungluu, 2016/03/30 17:49:01]

And by initializing, you can skip this.

[vardhan, 2016/03/30 22:49:07]

Done.

+  EXPECT_TRUE(co.Serialize(buf, sizeof(buf)));
+  co.other = 0;
+  co.DeserializeWithoutValidation(buf);
+  EXPECT_EQ(kMagic, co.other);
+
+  // Invalid struct header, but will pass (i.e., won't crash) anyway because we
+  // don't Validate.
+  co_data->header_.num_bytes = 0u;
+  co.DeserializeWithoutValidation(buf);
+  EXPECT_EQ(kMagic, co_data->other);
+  EXPECT_EQ(0u, co_data->header_.num_bytes);
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace mojo