From 39e68afc1a76f5e2ee19405bd32de7b335d4fb43 Mon Sep 17 00:00:00 2001 From: rsgowman Date: Tue, 1 May 2018 16:33:35 -0400 Subject: 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. --- .../firebase/firestore/remote/serializer_test.cc | 212 ++++++++++++++++++--- 1 file changed, 190 insertions(+), 22 deletions(-) (limited to 'Firestore/core/test') 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 #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 + testing::AssertionResult StatusOk(const StatusOr& 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& bytes) { + StatusOr 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 bytes; - Status status = - serializer.EncodeFieldValue(FieldValue::NullValue(), &bytes); - EXPECT_TRUE(status.ok()); + std::vector 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 EncodeFieldValue(Serializer* serializer, + const FieldValue& fv) { std::vector 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 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 bytes; - Status status = - serializer.EncodeFieldValue(FieldValue::IntegerValue(i), &bytes); - EXPECT_TRUE(status.ok()); + std::vector 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 bytes; - Status status = - serializer.EncodeFieldValue(FieldValue::StringValue(s), &bytes); - EXPECT_TRUE(status.ok()); + std::vector 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 bytes; - Status status = serializer.EncodeFieldValue(model, &bytes); - EXPECT_TRUE(status.ok()); + std::vector 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 bytes(size); bool status = proto.SerializeToArray(bytes.data(), size); EXPECT_TRUE(status); - FieldValue actual_model = serializer.DecodeFieldValue(bytes); + StatusOr 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 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 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 bytes = EncodeFieldValue( + &serializer, + FieldValue::IntegerValue(std::numeric_limits::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 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 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 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 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 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 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. -- cgit v1.2.3