Add error handler for serializer (for deserializing cases) (#1181)

This is more interesting than the serializing case, as we should expect
to see occasional corruption of our input byte vector.

1 files changed, 190 insertions, 22 deletions

diff --git a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc
index 96125f7..a2036a8 100644
--- a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc
+++ b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc
@@ -33,18 +33,27 @@
 #include <vector>
 
 #include "Firestore/Protos/cpp/google/firestore/v1beta1/document.pb.h"
+#include "Firestore/core/include/firebase/firestore/firestore_errors.h"
 #include "Firestore/core/src/firebase/firestore/model/field_value.h"
 #include "Firestore/core/src/firebase/firestore/util/status.h"
+#include "Firestore/core/src/firebase/firestore/util/statusor.h"
 #include "google/protobuf/stubs/common.h"
 #include "google/protobuf/util/message_differencer.h"
 #include "gtest/gtest.h"
 
+using firebase::firestore::FirestoreErrorCode;
 using firebase::firestore::model::FieldValue;
 using firebase::firestore::model::ObjectValue;
 using firebase::firestore::remote::Serializer;
 using firebase::firestore::util::Status;
+using firebase::firestore::util::StatusOr;
 using google::protobuf::util::MessageDifferencer;
 
+#define ASSERT_OK(status) ASSERT_TRUE(StatusOk(status))
+#define ASSERT_NOT_OK(status) ASSERT_FALSE(StatusOk(status))
+#define EXPECT_OK(status) EXPECT_TRUE(StatusOk(status))
+#define EXPECT_NOT_OK(status) EXPECT_FALSE(StatusOk(status))
+
 TEST(Serializer, CanLinkToNanopb) {
   // This test doesn't actually do anything interesting as far as actually using
   // nanopb is concerned but that it can run at all is proof that all the
@@ -74,22 +83,69 @@ class SerializerTest : public ::testing::Test {
     ExpectDeserializationRoundTrip(model, proto, type);
   }
 
+  /**
+   * Checks the status. Don't use directly; use one of the relevant macros
+   * instead. eg:
+   *
+   *   Status good_status = ...;
+   *   ASSERT_OK(good_status);
+   *
+   *   Status bad_status = ...;
+   *   EXPECT_NOT_OK(bad_status);
+   */
+  testing::AssertionResult StatusOk(const Status& status) {
+    if (!status.ok()) {
+      return testing::AssertionFailure()
+             << "Status should have been ok, but instead contained "
+             << status.ToString();
+    }
+    return testing::AssertionSuccess();
+  }
+
+  template <typename T>
+  testing::AssertionResult StatusOk(const StatusOr<T>& status) {
+    return StatusOk(status.status());
+  }
+
+  /**
+   * Ensures that decoding fails with the given status.
+   *
+   * @param status the expected (failed) status. Only the code() is verified.
+   */
+  void ExpectFailedStatusDuringDecode(Status status,
+                                      const std::vector<uint8_t>& bytes) {
+    StatusOr<FieldValue> bad_status = serializer.DecodeFieldValue(bytes);
+    ASSERT_NOT_OK(bad_status);
+    EXPECT_EQ(status.code(), bad_status.status().code());
+  }
+
   google::firestore::v1beta1::Value ValueProto(nullptr_t) {
-    std::vector<uint8_t> bytes;
-    Status status =
-        serializer.EncodeFieldValue(FieldValue::NullValue(), &bytes);
-    EXPECT_TRUE(status.ok());
+    std::vector<uint8_t> bytes =
+        EncodeFieldValue(&serializer, FieldValue::NullValue());
     google::firestore::v1beta1::Value proto;
     bool ok = proto.ParseFromArray(bytes.data(), bytes.size());
     EXPECT_TRUE(ok);
     return proto;
   }
 
-  google::firestore::v1beta1::Value ValueProto(bool b) {
+  std::vector<uint8_t> EncodeFieldValue(Serializer* serializer,
+                                        const FieldValue& fv) {
     std::vector<uint8_t> bytes;
-    Status status =
-        serializer.EncodeFieldValue(FieldValue::BooleanValue(b), &bytes);
-    EXPECT_TRUE(status.ok());
+    Status status = serializer->EncodeFieldValue(fv, &bytes);
+    EXPECT_OK(status);
+    return bytes;
+  }
+
+  void Mutate(uint8_t* byte,
+              uint8_t expected_initial_value,
+              uint8_t new_value) {
+    ASSERT_EQ(*byte, expected_initial_value);
+    *byte = new_value;
+  }
+
+  google::firestore::v1beta1::Value ValueProto(bool b) {
+    std::vector<uint8_t> bytes =
+        EncodeFieldValue(&serializer, FieldValue::BooleanValue(b));
     google::firestore::v1beta1::Value proto;
     bool ok = proto.ParseFromArray(bytes.data(), bytes.size());
     EXPECT_TRUE(ok);
@@ -97,10 +153,8 @@ class SerializerTest : public ::testing::Test {
   }
 
   google::firestore::v1beta1::Value ValueProto(int64_t i) {
-    std::vector<uint8_t> bytes;
-    Status status =
-        serializer.EncodeFieldValue(FieldValue::IntegerValue(i), &bytes);
-    EXPECT_TRUE(status.ok());
+    std::vector<uint8_t> bytes =
+        EncodeFieldValue(&serializer, FieldValue::IntegerValue(i));
     google::firestore::v1beta1::Value proto;
     bool ok = proto.ParseFromArray(bytes.data(), bytes.size());
     EXPECT_TRUE(ok);
@@ -112,10 +166,8 @@ class SerializerTest : public ::testing::Test {
   }
 
   google::firestore::v1beta1::Value ValueProto(const std::string& s) {
-    std::vector<uint8_t> bytes;
-    Status status =
-        serializer.EncodeFieldValue(FieldValue::StringValue(s), &bytes);
-    EXPECT_TRUE(status.ok());
+    std::vector<uint8_t> bytes =
+        EncodeFieldValue(&serializer, FieldValue::StringValue(s));
     google::firestore::v1beta1::Value proto;
     bool ok = proto.ParseFromArray(bytes.data(), bytes.size());
     EXPECT_TRUE(ok);
@@ -128,9 +180,7 @@ class SerializerTest : public ::testing::Test {
       const google::firestore::v1beta1::Value& proto,
       FieldValue::Type type) {
     EXPECT_EQ(type, model.type());
-    std::vector<uint8_t> bytes;
-    Status status = serializer.EncodeFieldValue(model, &bytes);
-    EXPECT_TRUE(status.ok());
+    std::vector<uint8_t> bytes = EncodeFieldValue(&serializer, model);
     google::firestore::v1beta1::Value actual_proto;
     bool ok = actual_proto.ParseFromArray(bytes.data(), bytes.size());
     EXPECT_TRUE(ok);
@@ -145,7 +195,10 @@ class SerializerTest : public ::testing::Test {
     std::vector<uint8_t> bytes(size);
     bool status = proto.SerializeToArray(bytes.data(), size);
     EXPECT_TRUE(status);
-    FieldValue actual_model = serializer.DecodeFieldValue(bytes);
+    StatusOr<FieldValue> actual_model_status =
+        serializer.DecodeFieldValue(bytes);
+    EXPECT_OK(actual_model_status);
+    FieldValue actual_model = actual_model_status.ValueOrDie();
     EXPECT_EQ(type, actual_model.type());
     EXPECT_EQ(model, actual_model);
   }
@@ -258,7 +311,122 @@ TEST_F(SerializerTest, WritesNestedObjects) {
   ExpectRoundTrip(model, proto, FieldValue::Type::Object);
 }
 
+TEST_F(SerializerTest, BadNullValue) {
+  std::vector<uint8_t> bytes =
+      EncodeFieldValue(&serializer, FieldValue::NullValue());
+
+  // Alter the null value from 0 to 1.
+  Mutate(&bytes[1], /*expected_initial_value=*/0, /*new_value=*/1);
+
+  ExpectFailedStatusDuringDecode(
+      Status(FirestoreErrorCode::DataLoss, "ignored"), bytes);
+}
+
+TEST_F(SerializerTest, BadBoolValue) {
+  std::vector<uint8_t> bytes =
+      EncodeFieldValue(&serializer, FieldValue::BooleanValue(true));
+
+  // Alter the bool value from 1 to 2. (Value values are 0,1)
+  Mutate(&bytes[1], /*expected_initial_value=*/1, /*new_value=*/2);
+
+  ExpectFailedStatusDuringDecode(
+      Status(FirestoreErrorCode::DataLoss, "ignored"), bytes);
+}
+
+TEST_F(SerializerTest, BadIntegerValue) {
+  // Encode 'maxint'. This should result in 9 0xff bytes, followed by a 1.
+  std::vector<uint8_t> bytes = EncodeFieldValue(
+      &serializer,
+      FieldValue::IntegerValue(std::numeric_limits<uint64_t>::max()));
+  ASSERT_EQ(11u, bytes.size());
+  for (size_t i = 1; i < bytes.size() - 1; i++) {
+    ASSERT_EQ(0xff, bytes[i]);
+  }
+
+  // make the number a bit bigger
+  Mutate(&bytes[10], /*expected_initial_value=*/1, /*new_value=*/0xff);
+  bytes.resize(12);
+  bytes[11] = 0x7f;
+
+  ExpectFailedStatusDuringDecode(
+      Status(FirestoreErrorCode::DataLoss, "ignored"), bytes);
+}
+
+TEST_F(SerializerTest, BadStringValue) {
+  std::vector<uint8_t> bytes =
+      EncodeFieldValue(&serializer, FieldValue::StringValue("a"));
+
+  // Claim that the string length is 5 instead of 1. (The first two bytes are
+  // used by the encoded tag.)
+  Mutate(&bytes[2], /*expected_initial_value=*/1, /*new_value=*/5);
+
+  ExpectFailedStatusDuringDecode(
+      Status(FirestoreErrorCode::DataLoss, "ignored"), bytes);
+}
+
+TEST_F(SerializerTest, BadTag) {
+  std::vector<uint8_t> bytes =
+      EncodeFieldValue(&serializer, FieldValue::NullValue());
+
+  // The google::firestore::v1beta1::Value value_type oneof currently has tags
+  // up to 18. For this test, we'll pick a tag that's unlikely to be added in
+  // the near term but still fits within a uint8_t even when encoded.
+  // Specifically 31. 0xf8 represents field number 31 encoded as a varint.
+  Mutate(&bytes[0], /*expected_initial_value=*/0x58, /*new_value=*/0xf8);
+
+  // TODO(rsgowman): The behaviour is *temporarily* slightly different during
+  // development; this will cause a failed assertion rather than a failed
+  // status. Remove this EXPECT_ANY_THROW statement (and reenable the
+  // following commented out statement) once the corresponding assert has been
+  // removed from serializer.cc.
+  EXPECT_ANY_THROW(ExpectFailedStatusDuringDecode(
+      Status(FirestoreErrorCode::DataLoss, "ignored"), bytes));
+  // ExpectFailedStatusDuringDecode(
+  //    Status(FirestoreErrorCode::DataLoss, "ignored"), bytes);
+}
+
+TEST_F(SerializerTest, TagVarintWiretypeStringMismatch) {
+  std::vector<uint8_t> bytes =
+      EncodeFieldValue(&serializer, FieldValue::BooleanValue(true));
+
+  // 0x0a represents a bool value encoded as a string. (We're using a
+  // boolean_value tag here, but any tag that would be represented by a varint
+  // would do.)
+  Mutate(&bytes[0], /*expected_initial_value=*/0x08, /*new_value=*/0x0a);
+
+  ExpectFailedStatusDuringDecode(
+      Status(FirestoreErrorCode::DataLoss, "ignored"), bytes);
+}
+
+TEST_F(SerializerTest, TagStringWiretypeVarintMismatch) {
+  std::vector<uint8_t> bytes =
+      EncodeFieldValue(&serializer, FieldValue::StringValue("foo"));
+
+  // 0x88 represents a string value encoded as a varint.
+  Mutate(&bytes[0], /*expected_initial_value=*/0x8a, /*new_value=*/0x88);
+
+  ExpectFailedStatusDuringDecode(
+      Status(FirestoreErrorCode::DataLoss, "ignored"), bytes);
+}
+
+TEST_F(SerializerTest, IncompleteFieldValue) {
+  std::vector<uint8_t> bytes =
+      EncodeFieldValue(&serializer, FieldValue::NullValue());
+  ASSERT_EQ(2u, bytes.size());
+
+  // Remove the (null) payload
+  ASSERT_EQ(0x00, bytes[1]);
+  bytes.pop_back();
+
+  ExpectFailedStatusDuringDecode(
+      Status(FirestoreErrorCode::DataLoss, "ignored"), bytes);
+}
+
+TEST_F(SerializerTest, IncompleteTag) {
+  std::vector<uint8_t> bytes;
+  ExpectFailedStatusDuringDecode(
+      Status(FirestoreErrorCode::DataLoss, "ignored"), bytes);
+}
+
 // TODO(rsgowman): Test [en|de]coding multiple protos into the same output
 // vector.
-
-// TODO(rsgowman): Death test for decoding invalid bytes.