From af955682f92baf4fce134dc72a0d2b1564562973 Mon Sep 17 00:00:00 2001 From: rsgowman Date: Wed, 9 May 2018 12:23:34 -0400 Subject: [De]serialize Timestamps via the remote Serializer (#1233) --- .../core/src/firebase/firestore/CMakeLists.txt | 1 + .../src/firebase/firestore/remote/serializer.cc | 88 ++++++++++++++++++++++ .../src/firebase/firestore/timestamp_internal.h | 49 ++++++++++++ .../firebase/firestore/remote/serializer_test.cc | 52 +++++++++++++ 4 files changed, 190 insertions(+) create mode 100644 Firestore/core/src/firebase/firestore/timestamp_internal.h (limited to 'Firestore/core') diff --git a/Firestore/core/src/firebase/firestore/CMakeLists.txt b/Firestore/core/src/firebase/firestore/CMakeLists.txt index aad2ebb..9836b05 100644 --- a/Firestore/core/src/firebase/firestore/CMakeLists.txt +++ b/Firestore/core/src/firebase/firestore/CMakeLists.txt @@ -18,6 +18,7 @@ cc_library( SOURCES geo_point.cc timestamp.cc + timestamp_internal.h DEPENDS firebase_firestore_util ) diff --git a/Firestore/core/src/firebase/firestore/remote/serializer.cc b/Firestore/core/src/firebase/firestore/remote/serializer.cc index e81ea2d..209f2b1 100644 --- a/Firestore/core/src/firebase/firestore/remote/serializer.cc +++ b/Firestore/core/src/firebase/firestore/remote/serializer.cc @@ -25,13 +25,17 @@ #include #include "Firestore/Protos/nanopb/google/firestore/v1beta1/document.pb.h" +#include "Firestore/core/include/firebase/firestore/timestamp.h" #include "Firestore/core/src/firebase/firestore/model/resource_path.h" +#include "Firestore/core/src/firebase/firestore/timestamp_internal.h" #include "Firestore/core/src/firebase/firestore/util/firebase_assert.h" namespace firebase { namespace firestore { namespace remote { +using firebase::Timestamp; +using firebase::TimestampInternal; using firebase::firestore::model::DatabaseId; using firebase::firestore::model::DocumentKey; using firebase::firestore::model::FieldValue; @@ -95,6 +99,15 @@ class Writer { */ void WriteTag(Tag tag); + /** + * Writes a nanopb message to the output stream. + * + * This essentially wraps calls to nanopb's `pb_encode()` method. If we didn't + * use `oneof`s in our protos, this would be the primary way of encoding + * messages. + */ + void WriteNanopbMessage(const pb_field_t fields[], const void* src_struct); + void WriteSize(size_t size); void WriteNull(); void WriteBool(bool bool_value); @@ -179,6 +192,15 @@ class Reader { */ Tag ReadTag(); + /** + * Reads a nanopb message from the input stream. + * + * This essentially wraps calls to nanopb's pb_decode() method. If we didn't + * use `oneof`s in our protos, this would be the primary way of decoding + * messages. + */ + void ReadNanopbMessage(const pb_field_t fields[], void* dest_struct); + void ReadNull(); bool ReadBool(); int64_t ReadInteger(); @@ -299,6 +321,23 @@ Tag Reader::ReadTag() { return tag; } +void Writer::WriteNanopbMessage(const pb_field_t fields[], + const void* src_struct) { + if (!status_.ok()) return; + + if (!pb_encode(&stream_, fields, src_struct)) { + FIREBASE_ASSERT_MESSAGE(false, PB_GET_ERROR(&stream_)); + } +} + +void Reader::ReadNanopbMessage(const pb_field_t fields[], void* dest_struct) { + if (!status_.ok()) return; + + if (!pb_decode(&stream_, fields, dest_struct)) { + status_ = Status(FirestoreErrorCode::DataLoss, PB_GET_ERROR(&stream_)); + } +} + void Writer::WriteSize(size_t size) { return WriteVarint(size); } @@ -415,6 +454,43 @@ std::string Reader::ReadString() { return result; } +void EncodeTimestamp(Writer* writer, const Timestamp& timestamp_value) { + google_protobuf_Timestamp timestamp_proto = + google_protobuf_Timestamp_init_zero; + timestamp_proto.seconds = timestamp_value.seconds(); + timestamp_proto.nanos = timestamp_value.nanoseconds(); + writer->WriteNanopbMessage(google_protobuf_Timestamp_fields, + ×tamp_proto); +} + +Timestamp DecodeTimestamp(Reader* reader) { + google_protobuf_Timestamp timestamp_proto = + google_protobuf_Timestamp_init_zero; + reader->ReadNanopbMessage(google_protobuf_Timestamp_fields, ×tamp_proto); + + // The Timestamp ctor will assert if we provide values outside the valid + // range. However, since we're decoding, a single corrupt byte could cause + // this to occur, so we'll verify the ranges before passing them in since we'd + // rather not abort in these situations. + if (timestamp_proto.seconds < TimestampInternal::Min().seconds()) { + reader->set_status(Status( + FirestoreErrorCode::DataLoss, + "Invalid message: timestamp beyond the earliest supported date")); + return {}; + } else if (TimestampInternal::Max().seconds() < timestamp_proto.seconds) { + reader->set_status( + Status(FirestoreErrorCode::DataLoss, + "Invalid message: timestamp behond the latest supported date")); + return {}; + } else if (timestamp_proto.nanos < 0 || timestamp_proto.nanos > 999999999) { + reader->set_status(Status( + FirestoreErrorCode::DataLoss, + "Invalid message: timestamp nanos must be between 0 and 999999999")); + return {}; + } + return Timestamp{timestamp_proto.seconds, timestamp_proto.nanos}; +} + // Named '..Impl' so as to not conflict with Serializer::EncodeFieldValue. // TODO(rsgowman): Refactor to use a helper class that wraps the stream struct. // This will help with error handling, and should eliminate the issue of two @@ -447,6 +523,14 @@ void EncodeFieldValueImpl(Writer* writer, const FieldValue& field_value) { writer->WriteString(field_value.string_value()); break; + case FieldValue::Type::Timestamp: + writer->WriteTag( + {PB_WT_STRING, google_firestore_v1beta1_Value_timestamp_value_tag}); + writer->WriteNestedMessage([&field_value](Writer* writer) { + EncodeTimestamp(writer, field_value.timestamp_value()); + }); + break; + case FieldValue::Type::Object: writer->WriteTag( {PB_WT_STRING, google_firestore_v1beta1_Value_map_value_tag}); @@ -477,6 +561,7 @@ FieldValue DecodeFieldValueImpl(Reader* reader) { break; case google_firestore_v1beta1_Value_string_value_tag: + case google_firestore_v1beta1_Value_timestamp_value_tag: case google_firestore_v1beta1_Value_map_value_tag: if (tag.wire_type != PB_WT_STRING) { reader->set_status( @@ -517,6 +602,9 @@ FieldValue DecodeFieldValueImpl(Reader* reader) { return FieldValue::IntegerValue(reader->ReadInteger()); case google_firestore_v1beta1_Value_string_value_tag: return FieldValue::StringValue(reader->ReadString()); + case google_firestore_v1beta1_Value_timestamp_value_tag: + return FieldValue::TimestampValue( + reader->ReadNestedMessage(DecodeTimestamp)); case google_firestore_v1beta1_Value_map_value_tag: return FieldValue::ObjectValueFromMap(DecodeObject(reader)); diff --git a/Firestore/core/src/firebase/firestore/timestamp_internal.h b/Firestore/core/src/firebase/firestore/timestamp_internal.h new file mode 100644 index 0000000..3516176 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/timestamp_internal.h @@ -0,0 +1,49 @@ +/* + * Copyright 2018 Google + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_TIMESTAMP_INTERNAL_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_TIMESTAMP_INTERNAL_H_ + +#include "Firestore/core/include/firebase/firestore/timestamp.h" + +namespace firebase { + +/** + * Details about the Timestamp class which are useful internally, but we don't + * want to expose publicly. + */ +class TimestampInternal { + public: + /** + * Represents the maximum allowable time that the Timestamp class handles, + * specifically 9999-12-31T23:59:59.999999999Z. + */ + static firebase::Timestamp Max() { + return {253402300800L - 1, 999999999}; + } + + /** + * Represents the minimum allowable time that the Timestamp class handles, + * specifically 0001-01-01T00:00:00Z. + */ + static firebase::Timestamp Min() { + return {-62135596800L, 0}; + } +}; + +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_TIMESTAMP_INTERNAL_H_ diff --git a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc index 1dded05..38277e2 100644 --- a/Firestore/core/test/firebase/firestore/remote/serializer_test.cc +++ b/Firestore/core/test/firebase/firestore/remote/serializer_test.cc @@ -34,7 +34,9 @@ #include "Firestore/Protos/cpp/google/firestore/v1beta1/document.pb.h" #include "Firestore/core/include/firebase/firestore/firestore_errors.h" +#include "Firestore/core/include/firebase/firestore/timestamp.h" #include "Firestore/core/src/firebase/firestore/model/field_value.h" +#include "Firestore/core/src/firebase/firestore/timestamp_internal.h" #include "Firestore/core/src/firebase/firestore/util/status.h" #include "Firestore/core/src/firebase/firestore/util/statusor.h" #include "Firestore/core/test/firebase/firestore/testutil/testutil.h" @@ -42,6 +44,8 @@ #include "google/protobuf/util/message_differencer.h" #include "gtest/gtest.h" +using firebase::Timestamp; +using firebase::TimestampInternal; using firebase::firestore::FirestoreErrorCode; using firebase::firestore::model::DatabaseId; using firebase::firestore::model::FieldValue; @@ -179,6 +183,15 @@ class SerializerTest : public ::testing::Test { return proto; } + google::firestore::v1beta1::Value ValueProto(const Timestamp& ts) { + std::vector bytes = + EncodeFieldValue(&serializer, FieldValue::TimestampValue(ts)); + google::firestore::v1beta1::Value proto; + bool ok = proto.ParseFromArray(bytes.data(), bytes.size()); + EXPECT_TRUE(ok); + return proto; + } + private: void ExpectSerializationRoundTrip( const FieldValue& model, @@ -259,6 +272,23 @@ TEST_F(SerializerTest, EncodesString) { } } +TEST_F(SerializerTest, EncodesTimestamps) { + std::vector cases{ + {}, // epoch + {1234, 0}, + {1234, 999999999}, + {-1234, 0}, + {-1234, 999999999}, + TimestampInternal::Max(), + TimestampInternal::Min(), + }; + + for (const Timestamp& ts_value : cases) { + FieldValue model = FieldValue::TimestampValue(ts_value); + ExpectRoundTrip(model, ValueProto(ts_value), FieldValue::Type::Timestamp); + } +} + TEST_F(SerializerTest, EncodesEmptyMap) { FieldValue model = FieldValue::ObjectValueFromMap({}); @@ -366,6 +396,28 @@ TEST_F(SerializerTest, BadStringValue) { Status(FirestoreErrorCode::DataLoss, "ignored"), bytes); } +TEST_F(SerializerTest, BadTimestampValue_TooLarge) { + std::vector bytes = EncodeFieldValue( + &serializer, FieldValue::TimestampValue(TimestampInternal::Max())); + + // Add some time, which should push us above the maximum allowed timestamp. + Mutate(&bytes[4], 0x82, 0x83); + + ExpectFailedStatusDuringDecode( + Status(FirestoreErrorCode::DataLoss, "ignored"), bytes); +} + +TEST_F(SerializerTest, BadTimestampValue_TooSmall) { + std::vector bytes = EncodeFieldValue( + &serializer, FieldValue::TimestampValue(TimestampInternal::Min())); + + // Remove some time, which should push us below the minimum allowed timestamp. + Mutate(&bytes[4], 0x92, 0x91); + + ExpectFailedStatusDuringDecode( + Status(FirestoreErrorCode::DataLoss, "ignored"), bytes); +} + TEST_F(SerializerTest, BadTag) { std::vector bytes = EncodeFieldValue(&serializer, FieldValue::NullValue()); -- cgit v1.2.3