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. --- .../src/firebase/firestore/remote/serializer.cc | 155 +++++++++++---- .../src/firebase/firestore/remote/serializer.h | 18 +- .../firebase/firestore/remote/serializer_test.cc | 212 ++++++++++++++++++--- 3 files changed, 316 insertions(+), 69 deletions(-) (limited to 'Firestore/core') diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.cc b/Firestore/core/src/firebase/firestore/remote/serializer.cc index b5a0720..2a89515 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.cc +++ b/Firestore/core/src/firebase/firestore/remote/serializer.cc @@ -34,6 +34,7 @@ namespace remote { using firebase::firestore::model::FieldValue; using firebase::firestore::model::ObjectValue; using firebase::firestore::util::Status; +using firebase::firestore::util::StatusOr; namespace { @@ -43,7 +44,7 @@ class Reader; void EncodeObject(Writer* writer, const ObjectValue& object_value); -ObjectValue DecodeObject(Reader* reader); +ObjectValue::Map DecodeObject(Reader* reader); /** * Represents a nanopb tag. @@ -196,6 +197,14 @@ class Reader { return stream_.bytes_left; } + Status status() const { + return status_; + } + + void set_status(Status status) { + status_ = status; + } + private: /** * Creates a new Reader, based on the given nanopb pb_istream_t. Note that @@ -220,6 +229,8 @@ class Reader { */ uint64_t ReadVarint(); + Status status_ = Status::OK(); + pb_istream_t stream_; }; @@ -270,12 +281,17 @@ void Writer::WriteTag(Tag tag) { Tag Reader::ReadTag() { Tag tag; + if (!status_.ok()) return tag; + bool eof; - bool ok = pb_decode_tag(&stream_, &tag.wire_type, &tag.field_number, &eof); - if (!ok || eof) { - // TODO(rsgowman): figure out error handling - abort(); + if (!pb_decode_tag(&stream_, &tag.wire_type, &tag.field_number, &eof)) { + status_ = Status(FirestoreErrorCode::DataLoss, PB_GET_ERROR(&stream_)); + return tag; } + + // nanopb code always returns a false status when setting eof. + FIREBASE_ASSERT_MESSAGE(!eof, "nanopb set both ok status and eof to true"); + return tag; } @@ -301,10 +317,11 @@ void Writer::WriteVarint(uint64_t value) { * @return The decoded varint as a uint64_t. */ uint64_t Reader::ReadVarint() { - uint64_t varint_value; + if (!status_.ok()) return 0; + + uint64_t varint_value = 0; if (!pb_decode_varint(&stream_, &varint_value)) { - // TODO(rsgowman): figure out error handling - abort(); + status_ = Status(FirestoreErrorCode::DataLoss, PB_GET_ERROR(&stream_)); } return varint_value; } @@ -315,9 +332,11 @@ void Writer::WriteNull() { void Reader::ReadNull() { uint64_t varint = ReadVarint(); + if (!status_.ok()) return; + if (varint != google_protobuf_NullValue_NULL_VALUE) { - // TODO(rsgowman): figure out error handling - abort(); + status_ = Status(FirestoreErrorCode::DataLoss, + "Input proto bytes cannot be parsed (invalid null value)"); } } @@ -327,14 +346,18 @@ void Writer::WriteBool(bool bool_value) { bool Reader::ReadBool() { uint64_t varint = ReadVarint(); + if (!status_.ok()) return false; + switch (varint) { case 0: return false; case 1: return true; default: - // TODO(rsgowman): figure out error handling - abort(); + status_ = + Status(FirestoreErrorCode::DataLoss, + "Input proto bytes cannot be parsed (invalid bool value)"); + return false; } } @@ -357,17 +380,21 @@ void Writer::WriteString(const std::string& string_value) { } std::string Reader::ReadString() { + if (!status_.ok()) return ""; + pb_istream_t substream; if (!pb_make_string_substream(&stream_, &substream)) { - // TODO(rsgowman): figure out error handling - abort(); + status_ = Status(FirestoreErrorCode::DataLoss, PB_GET_ERROR(&stream_)); + pb_close_string_substream(&stream_, &substream); + return ""; } std::string result(substream.bytes_left, '\0'); if (!pb_read(&substream, reinterpret_cast(&result[0]), substream.bytes_left)) { - // TODO(rsgowman): figure out error handling - abort(); + status_ = Status(FirestoreErrorCode::DataLoss, PB_GET_ERROR(&stream_)); + pb_close_string_substream(&stream_, &substream); + return ""; } // NB: future versions of nanopb read the remaining characters out of the @@ -375,10 +402,9 @@ std::string Reader::ReadString() { // check within pb_close_string_substream. Unfortunately, that's not present // in the current version (0.38). We'll make a stronger assertion and check // to make sure there *are* no remaining characters in the substream. - if (substream.bytes_left != 0) { - // TODO(rsgowman): figure out error handling - abort(); - } + FIREBASE_ASSERT_MESSAGE( + substream.bytes_left == 0, + "Bytes remaining in substream after supposedly reading all of them."); pb_close_string_substream(&stream_, &substream); @@ -431,29 +457,52 @@ void EncodeFieldValueImpl(Writer* writer, const FieldValue& field_value) { FieldValue DecodeFieldValueImpl(Reader* reader) { Tag tag = reader->ReadTag(); + if (!reader->status().ok()) return FieldValue::NullValue(); // Ensure the tag matches the wire type - // TODO(rsgowman): figure out error handling switch (tag.field_number) { case google_firestore_v1beta1_Value_null_value_tag: case google_firestore_v1beta1_Value_boolean_value_tag: case google_firestore_v1beta1_Value_integer_value_tag: if (tag.wire_type != PB_WT_VARINT) { - abort(); + reader->set_status( + Status(FirestoreErrorCode::DataLoss, + "Input proto bytes cannot be parsed (mismatch between " + "the wiretype and the field number (tag))")); } break; case google_firestore_v1beta1_Value_string_value_tag: case google_firestore_v1beta1_Value_map_value_tag: if (tag.wire_type != PB_WT_STRING) { - abort(); + reader->set_status( + Status(FirestoreErrorCode::DataLoss, + "Input proto bytes cannot be parsed (mismatch between " + "the wiretype and the field number (tag))")); } break; default: - abort(); + // We could get here for one of two reasons; either because the input + // bytes are corrupt, or because we're attempting to parse a tag that we + // haven't implemented yet. Long term, the latter reason should become + // less likely (especially in production), so we'll assume former. + + // TODO(rsgowman): While still in development, we'll contradict the above + // and assume the latter. Remove the following assertion when we're + // confident that we're handling all the tags in the protos. + FIREBASE_ASSERT_MESSAGE( + false, + "Unhandled message field number (tag): %i. (Or possibly " + "corrupt input bytes)", + tag.field_number); + reader->set_status(Status( + FirestoreErrorCode::DataLoss, + "Input proto bytes cannot be parsed (invalid field number (tag))")); } + if (!reader->status().ok()) return FieldValue::NullValue(); + switch (tag.field_number) { case google_firestore_v1beta1_Value_null_value_tag: reader->ReadNull(); @@ -465,12 +514,15 @@ FieldValue DecodeFieldValueImpl(Reader* reader) { case google_firestore_v1beta1_Value_string_value_tag: return FieldValue::StringValue(reader->ReadString()); case google_firestore_v1beta1_Value_map_value_tag: - return FieldValue::ObjectValueFromMap( - DecodeObject(reader).internal_value); + return FieldValue::ObjectValueFromMap(DecodeObject(reader)); default: - // TODO(rsgowman): figure out error handling - abort(); + // This indicates an internal error as we've already ensured that this is + // a valid field_number. + FIREBASE_ASSERT_MESSAGE( + false, + "Somehow got an unexpected field number (tag) after verifying that " + "the field number was expected."); } } @@ -533,24 +585,34 @@ template T Reader::ReadNestedMessage(const std::function& read_message_fn) { // Implementation note: This is roughly modeled on pb_decode_delimited, // adjusted to account for the oneof in FieldValue. + + if (!status_.ok()) return T(); + pb_istream_t raw_substream; if (!pb_make_string_substream(&stream_, &raw_substream)) { - // TODO(rsgowman): figure out error handling - abort(); + status_ = Status(FirestoreErrorCode::DataLoss, PB_GET_ERROR(&stream_)); + pb_close_string_substream(&stream_, &raw_substream); + return T(); } Reader substream(raw_substream); + // If this fails, we *won't* return right away so that we can cleanup the + // substream (although technically, that turns out not to matter; no resource + // leaks occur if we don't do this.) + // TODO(rsgowman): Consider RAII here. (Watch out for Reader class which also + // wraps streams.) T message = read_message_fn(&substream); + status_ = substream.status(); // NB: future versions of nanopb read the remaining characters out of the // substream (and return false if that fails) as an additional safety // check within pb_close_string_substream. Unfortunately, that's not present // in the current version (0.38). We'll make a stronger assertion and check // to make sure there *are* no remaining characters in the substream. - if (substream.bytes_left() != 0) { - // TODO(rsgowman): figure out error handling - abort(); - } + FIREBASE_ASSERT_MESSAGE( + substream.bytes_left() == 0, + "Bytes remaining in substream after supposedly reading all of them."); + pb_close_string_substream(&stream_, &substream.stream_); return message; @@ -591,6 +653,7 @@ void EncodeFieldsEntry(Writer* writer, const ObjectValue::Map::value_type& kv) { ObjectValue::Map::value_type DecodeFieldsEntry(Reader* reader) { Tag tag = reader->ReadTag(); + if (!reader->status().ok()) return {}; // TODO(rsgowman): figure out error handling: We can do better than a failed // assertion. @@ -600,6 +663,7 @@ ObjectValue::Map::value_type DecodeFieldsEntry(Reader* reader) { std::string key = reader->ReadString(); tag = reader->ReadTag(); + if (!reader->status().ok()) return {}; FIREBASE_ASSERT(tag.field_number == google_firestore_v1beta1_MapValue_FieldsEntry_value_tag); FIREBASE_ASSERT(tag.wire_type == PB_WT_STRING); @@ -607,7 +671,7 @@ ObjectValue::Map::value_type DecodeFieldsEntry(Reader* reader) { FieldValue value = reader->ReadNestedMessage(DecodeFieldValueImpl); - return {key, value}; + return ObjectValue::Map::value_type{key, value}; } void EncodeObject(Writer* writer, const ObjectValue& object_value) { @@ -622,12 +686,17 @@ void EncodeObject(Writer* writer, const ObjectValue& object_value) { }); } -ObjectValue DecodeObject(Reader* reader) { - ObjectValue::Map internal_value = reader->ReadNestedMessage( +ObjectValue::Map DecodeObject(Reader* reader) { + if (!reader->status().ok()) return ObjectValue::Map(); + + return reader->ReadNestedMessage( [](Reader* reader) -> ObjectValue::Map { ObjectValue::Map result; + if (!reader->status().ok()) return result; + while (reader->bytes_left()) { Tag tag = reader->ReadTag(); + if (!reader->status().ok()) return result; FIREBASE_ASSERT(tag.field_number == google_firestore_v1beta1_MapValue_fields_tag); FIREBASE_ASSERT(tag.wire_type == PB_WT_STRING); @@ -640,6 +709,7 @@ ObjectValue DecodeObject(Reader* reader) { // map. // TODO(rsgowman): figure out error handling: We can do better than a // failed assertion. + if (!reader->status().ok()) return result; FIREBASE_ASSERT(result.find(fv.first) == result.end()); // Add this key,fieldvalue to the results map. @@ -647,7 +717,6 @@ ObjectValue DecodeObject(Reader* reader) { } return result; }); - return ObjectValue{internal_value}; } } // namespace @@ -659,9 +728,15 @@ Status Serializer::EncodeFieldValue(const FieldValue& field_value, return writer.status(); } -FieldValue Serializer::DecodeFieldValue(const uint8_t* bytes, size_t length) { +StatusOr Serializer::DecodeFieldValue(const uint8_t* bytes, + size_t length) { Reader reader = Reader::Wrap(bytes, length); - return DecodeFieldValueImpl(&reader); + FieldValue fv = DecodeFieldValueImpl(&reader); + if (reader.status().ok()) { + return fv; + } else { + return reader.status(); + } } } // namespace remote diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.h b/Firestore/core/src/firebase/firestore/remote/serializer.h index 7f08f7d..7d13583 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.h +++ b/Firestore/core/src/firebase/firestore/remote/serializer.h @@ -24,6 +24,7 @@ #include "Firestore/core/src/firebase/firestore/model/field_value.h" #include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" #include "Firestore/core/src/firebase/firestore/util/status.h" +#include "Firestore/core/src/firebase/firestore/util/statusor.h" #include "absl/base/attributes.h" namespace firebase { @@ -66,8 +67,9 @@ class Serializer { * @param field_value the model to convert. * @param[out] out_bytes A buffer to place the output. The bytes will be * appended to this vector. + * @return A Status, which if not ok(), indicates what went wrong. Note that + * errors during encoding generally indicate a serious/fatal error. */ - // TODO(rsgowman): error handling, incl return code. // TODO(rsgowman): If we never support any output except to a vector, it may // make sense to have Serializer own the vector and provide an accessor rather // than asking the user to create it first. @@ -80,20 +82,22 @@ class Serializer { * * @param bytes The bytes to convert. It's assumed that exactly all of the * bytes will be used by this conversion. - * @return The model equivalent of the bytes. + * @return The model equivalent of the bytes or a Status indicating + * what went wrong. */ - // TODO(rsgowman): error handling. - model::FieldValue DecodeFieldValue(const uint8_t* bytes, size_t length); + util::StatusOr DecodeFieldValue(const uint8_t* bytes, + size_t length); /** * @brief Converts from bytes to the model FieldValue format. * * @param bytes The bytes to convert. It's assumed that exactly all of the * bytes will be used by this conversion. - * @return The model equivalent of the bytes. + * @return The model equivalent of the bytes or a Status indicating + * what went wrong. */ - // TODO(rsgowman): error handling. - model::FieldValue DecodeFieldValue(const std::vector& bytes) { + util::StatusOr DecodeFieldValue( + const std::vector& bytes) { return DecodeFieldValue(bytes.data(), bytes.size()); } 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