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..8f19c348d6183ba5fe11a1335881965b781823ee 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;
+    auto deserialize_ret = out_val.Deserialize(bytes.data(), bytes.size());

[viettrungluu, 2016/03/23 17:43:53]

You may as well say "bool" instead of "auto", especially since you rely on this
below.

[vardhan, 2016/03/23 23:28:42]

Done.

     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];
+    EXPECT_FALSE(nd.Serialize(buf, sizeof(buf)));
+
+    size_t bytes_written;
+    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,43 @@ TEST_F(StructSerializationAPITest, NullableHandleSerialization) {
                           mojo::internal::ValidationError::NONE);
 }
 
+// Test that |Deserialize()| appropriately fails on validation.
+TEST_F(StructSerializationAPITest, DeserializationFailure) {
+  void* buf[100];
+  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);
+  es_data->header_.num_bytes = 0;
+  EXPECT_FALSE(es.Deserialize(buf, sizeof(buf)));
+}
+
+// Test DeserializeWithoutValidation

[viettrungluu, 2016/03/23 17:43:53]

nit: This comment would need some punctuation, but it's basically a useless
comment so you may as well just delete it.

[vardhan, 2016/03/23 23:28:43]

Done.

+TEST_F(StructSerializationAPITest, DeserializationWithoutValidation) {
+  void* buf[100];

[viettrungluu, 2016/03/23 17:43:53]

void*?

[vardhan, 2016/03/23 23:28:42]

Done.

+  EmptyStruct es;
+
+  // Since there is no bounds checker, this will pass even though it's bad.

[viettrungluu, 2016/03/23 17:43:53]

Since you didn't initialize the contents of buf, you don't even know that it's
bad.

[vardhan, 2016/03/23 23:28:43]

oops!

+  EXPECT_TRUE(es.DeserializeWithoutValidation(buf, 1));
+
+  es.Serialize(buf, sizeof(buf));
+  EXPECT_TRUE(es.DeserializeWithoutValidation(buf, sizeof(buf)));
+
+  // Invalid struct header, but will pass anyway because we don't Validate.
+  es.Serialize(buf, sizeof(buf));
+  EmptyStruct::Data_* es_data = reinterpret_cast<EmptyStruct::Data_*>(buf);
+  es_data->header_.num_bytes = 0;
+  EXPECT_TRUE(es.DeserializeWithoutValidation(buf, sizeof(buf)));
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace mojo