From 9b5b4d876eb77e65b3246614855088be101eebf3 Mon Sep 17 00:00:00 2001 From: rsgowman Date: Wed, 28 Feb 2018 15:32:22 -0500 Subject: Serialize (and deserialize) string values (#864) --- .../src/firebase/firestore/model/field_value.h | 5 ++ .../src/firebase/firestore/remote/serializer.cc | 78 +++++++++++++++++++++- .../firebase/firestore/remote/serializer_test.cc | 40 +++++++++++ 3 files changed, 122 insertions(+), 1 deletion(-) diff --git a/Firestore/core/src/firebase/firestore/model/field_value.h b/Firestore/core/src/firebase/firestore/model/field_value.h index 72b9481..cb219f5 100644 --- a/Firestore/core/src/firebase/firestore/model/field_value.h +++ b/Firestore/core/src/firebase/firestore/model/field_value.h @@ -106,6 +106,11 @@ class FieldValue { return integer_value_; } + const std::string& string_value() const { + FIREBASE_ASSERT(tag_ == Type::String); + return string_value_; + } + /** factory methods. */ static const FieldValue& NullValue(); static const FieldValue& TrueValue(); diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.cc b/Firestore/core/src/firebase/firestore/remote/serializer.cc index 79ed98d..b7ab891 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.cc +++ b/Firestore/core/src/firebase/firestore/remote/serializer.cc @@ -19,6 +19,8 @@ #include #include +#include + namespace firebase { namespace firestore { namespace remote { @@ -98,6 +100,47 @@ int64_t DecodeInteger(pb_istream_t* stream) { return DecodeVarint(stream); } +void EncodeString(pb_ostream_t* stream, const std::string& string_value) { + bool status = pb_encode_string( + stream, reinterpret_cast(string_value.c_str()), + string_value.length()); + if (!status) { + // TODO(rsgowman): figure out error handling + abort(); + } +} + +std::string DecodeString(pb_istream_t* stream) { + pb_istream_t substream; + bool status = pb_make_string_substream(stream, &substream); + if (!status) { + // TODO(rsgowman): figure out error handling + abort(); + } + + std::string result(substream.bytes_left, '\0'); + status = pb_read(&substream, reinterpret_cast(&result[0]), + substream.bytes_left); + if (!status) { + // TODO(rsgowman): figure out error handling + abort(); + } + + // 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(); + } + + pb_close_string_substream(stream, &substream); + + return result; +} + } // namespace using firebase::firestore::model::FieldValue; @@ -144,6 +187,16 @@ void Serializer::EncodeFieldValue(const FieldValue& field_value, EncodeInteger(&stream, field_value.integer_value()); break; + case FieldValue::Type::String: + status = pb_encode_tag(&stream, PB_WT_STRING, + google_firestore_v1beta1_Value_string_value_tag); + if (!status) { + // TODO(rsgowman): figure out error handling + abort(); + } + EncodeString(&stream, field_value.string_value()); + break; + default: // TODO(rsgowman): implement the other types abort(); @@ -158,11 +211,32 @@ FieldValue Serializer::DecodeFieldValue(const uint8_t* bytes, size_t length) { uint32_t tag; bool eof; bool status = pb_decode_tag(&stream, &wire_type, &tag, &eof); - if (!status || wire_type != PB_WT_VARINT) { + if (!status) { // TODO(rsgowman): figure out error handling abort(); } + // Ensure the tag matches the wire type + // TODO(rsgowman): figure out error handling + switch (tag) { + 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 (wire_type != PB_WT_VARINT) { + abort(); + } + break; + + case google_firestore_v1beta1_Value_string_value_tag: + if (wire_type != PB_WT_STRING) { + abort(); + } + break; + + default: + abort(); + } + switch (tag) { case google_firestore_v1beta1_Value_null_value_tag: DecodeNull(&stream); @@ -171,6 +245,8 @@ FieldValue Serializer::DecodeFieldValue(const uint8_t* bytes, size_t length) { return FieldValue::BooleanValue(DecodeBool(&stream)); case google_firestore_v1beta1_Value_integer_value_tag: return FieldValue::IntegerValue(DecodeInteger(&stream)); + case google_firestore_v1beta1_Value_string_value_tag: + return FieldValue::StringValue(DecodeString(&stream)); default: // TODO(rsgowman): figure out error handling diff --git a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc index 9ce60f5..8f89064 100644 --- a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc +++ b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc @@ -151,6 +151,46 @@ TEST_F(SerializerTest, EncodesIntegersModelToBytes) { } } +TEST_F(SerializerTest, EncodesStringModelToBytes) { + struct TestCase { + std::string value; + std::vector bytes; + }; + + std::vector cases{ + // TEXT_FORMAT_PROTO: 'string_value: ""' + {"", {0x8a, 0x01, 0x00}}, + // TEXT_FORMAT_PROTO: 'string_value: "a"' + {"a", {0x8a, 0x01, 0x01, 0x61}}, + // TEXT_FORMAT_PROTO: 'string_value: "abc def"' + {"abc def", {0x8a, 0x01, 0x07, 0x61, 0x62, 0x63, 0x20, 0x64, 0x65, 0x66}}, + // TEXT_FORMAT_PROTO: 'string_value: "æ"' + {"æ", {0x8a, 0x01, 0x02, 0xc3, 0xa6}}, + // TEXT_FORMAT_PROTO: 'string_value: "\0\ud7ff\ue000\uffff"' + // Note: Each one of the three embedded universal character names + // (\u-escaped) maps to three chars, so the total length of the string + // literal is 10 (ignoring the terminating null), and the resulting string + // literal is the same as '\0\xed\x9f\xbf\xee\x80\x80\xef\xbf\xbf'". The + // size of 10 must be added, or else std::string will see the \0 at the + // start and assume that's the end of the string. + {{"\0\ud7ff\ue000\uffff", 10}, + {0x8a, 0x01, 0x0a, 0x00, 0xed, 0x9f, 0xbf, 0xee, 0x80, 0x80, 0xef, 0xbf, + 0xbf}}, + {{"\0\xed\x9f\xbf\xee\x80\x80\xef\xbf\xbf", 10}, + {0x8a, 0x01, 0x0a, 0x00, 0xed, 0x9f, 0xbf, 0xee, 0x80, 0x80, 0xef, 0xbf, + 0xbf}}, + // TEXT_FORMAT_PROTO: 'string_value: "(╯°□°）╯︵ ┻━┻"' + {"(╯°□°）╯︵ ┻━┻", + {0x8a, 0x01, 0x1e, 0x28, 0xe2, 0x95, 0xaf, 0xc2, 0xb0, 0xe2, 0x96, + 0xa1, 0xc2, 0xb0, 0xef, 0xbc, 0x89, 0xe2, 0x95, 0xaf, 0xef, 0xb8, + 0xb5, 0x20, 0xe2, 0x94, 0xbb, 0xe2, 0x94, 0x81, 0xe2, 0x94, 0xbb}}}; + + for (const TestCase& test : cases) { + FieldValue model = FieldValue::StringValue(test.value); + ExpectRoundTrip(model, test.bytes, FieldValue::Type::String); + } +} + // TODO(rsgowman): Test [en|de]coding multiple protos into the same output // vector. -- cgit v1.2.3