From ab76c57ec81049282242af4abcb3f585cbee5a8f Mon Sep 17 00:00:00 2001 From: krupnov Date: Fri, 16 Dec 2016 20:37:04 +0400 Subject: [PATCH 1/2] random access iterator for vector added (#4119) * random access iterator for vector added * Style changes --- include/flatbuffers/flatbuffers.h | 41 ++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/include/flatbuffers/flatbuffers.h b/include/flatbuffers/flatbuffers.h index e830c4404..a69f8a337 100644 --- a/include/flatbuffers/flatbuffers.h +++ b/include/flatbuffers/flatbuffers.h @@ -252,9 +252,9 @@ template struct IndirectHelper { // calling Get() for every element. template struct VectorIterator - : public std::iterator { + : public std::iterator { - typedef std::iterator super_type; + typedef std::iterator super_type; public: VectorIterator(const uint8_t *data, uoffset_t i) : @@ -274,15 +274,15 @@ public: return *this; } - bool operator==(const VectorIterator& other) const { + bool operator==(const VectorIterator &other) const { return data_ == other.data_; } - bool operator!=(const VectorIterator& other) const { + bool operator!=(const VectorIterator &other) const { return data_ != other.data_; } - ptrdiff_t operator-(const VectorIterator& other) const { + ptrdiff_t operator-(const VectorIterator &other) const { return (data_ - other.data_) / IndirectHelper::element_stride; } @@ -300,11 +300,40 @@ public: } VectorIterator operator++(int) { - VectorIterator temp(data_,0); + VectorIterator temp(data_, 0); data_ += IndirectHelper::element_stride; return temp; } + VectorIterator operator+(const uoffset_t &offset) { + return VectorIterator(data_ + offset * IndirectHelper::element_stride, 0); + } + + VectorIterator& operator+=(const uoffset_t &offset) { + data_ += offset * IndirectHelper::element_stride; + return *this; + } + + VectorIterator &operator--() { + data_ -= IndirectHelper::element_stride; + return *this; + } + + VectorIterator operator--(int) { + VectorIterator temp(data_, 0); + data_ -= IndirectHelper::element_stride; + return temp; + } + + VectorIterator operator-(const uoffset_t &offset) { + return VectorIterator(data_ - offset * IndirectHelper::element_stride, 0); + } + + VectorIterator& operator-=(const uoffset_t &offset) { + data_ -= offset * IndirectHelper::element_stride; + return *this; + } + private: const uint8_t *data_; }; From 6d6271db2f8c4a5b6f0ff82506da0b5252f0e3ca Mon Sep 17 00:00:00 2001 From: Zarian Waheed Date: Fri, 16 Dec 2016 08:46:30 -0800 Subject: [PATCH 2/2] Changes for verifying a buffer dynamically using reflection. (#4102) * Changes for verifying a buffer dynamically using reflection. * Fixing build issues on linux and applied code reformatting. * Fixing the file order in cmake file that was messing up the macro based code inclusion. Added tests for reflection based verification. * Changes for verifying a buffer dynamically using reflection. Fixing build issues on linux and applied code reformatting. Fixing the file order in cmake file that was messing up the macro based code inclusion. Added tests for reflection based verification. * Incorporated the code review changes that were requested: 1. Changed the Verify function signature. 2. Changed the variable names to use snake_case. 3. Added better comments. 4. Refactored duplicate code. 5. Changed the verifier class so that it has the same size when compiled with or without FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE macro. * Setting FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE and FLATBUFFERS_DEBUG_VERIFICATION_FAILURE through cmake for flattests so that it gets propagted to all translation units of flattests. * Making the Verifier struct fields the same in all cases. Also reverting the target_compile_definitions change in cmake file because build machine on travis does not have cmake version 3.0 or higher which was the version when target_compile_definitions was added in cmake. * Defining macros through cmake in a portable way using functions that are available in cmake 2.8. --- CMakeLists.txt | 3 + include/flatbuffers/flatbuffers.h | 6 +- include/flatbuffers/reflection.h | 9 ++ src/reflection.cpp | 226 ++++++++++++++++++++++++++++++ tests/test.cpp | 23 ++- 5 files changed, 261 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 7a6b37093..7834f7a1c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -203,6 +203,9 @@ if(FLATBUFFERS_BUILD_TESTS) compile_flatbuffers_schema_to_cpp(tests/monster_test.fbs) include_directories(${CMAKE_CURRENT_BINARY_DIR}/tests) add_executable(flattests ${FlatBuffers_Tests_SRCS}) + set_property(TARGET flattests + PROPERTY COMPILE_DEFINITIONS FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE + FLATBUFFERS_DEBUG_VERIFICATION_FAILURE=1) compile_flatbuffers_schema_to_cpp(samples/monster.fbs) include_directories(${CMAKE_CURRENT_BINARY_DIR}/samples) diff --git a/include/flatbuffers/flatbuffers.h b/include/flatbuffers/flatbuffers.h index a69f8a337..bae169803 100644 --- a/include/flatbuffers/flatbuffers.h +++ b/include/flatbuffers/flatbuffers.h @@ -1324,7 +1324,7 @@ class Verifier FLATBUFFERS_FINAL_CLASS { : buf_(buf), end_(buf + buf_len), depth_(0), max_depth_(_max_depth), num_tables_(0), max_tables_(_max_tables) #ifdef FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE - , upper_bound_(buf) + , upper_bound_(buf) #endif {} @@ -1482,9 +1482,9 @@ class Verifier FLATBUFFERS_FINAL_CLASS { size_t max_depth_; size_t num_tables_; size_t max_tables_; - #ifdef FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE +#ifdef FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE mutable const uint8_t *upper_bound_; - #endif +#endif }; // Convenient way to bundle a buffer and its length, to pass it around diff --git a/include/flatbuffers/reflection.h b/include/flatbuffers/reflection.h index ae331ce23..632acdafb 100644 --- a/include/flatbuffers/reflection.h +++ b/include/flatbuffers/reflection.h @@ -428,6 +428,15 @@ Offset CopyTable(FlatBufferBuilder &fbb, const Table &table, bool use_string_pooling = false); +// Verifies the provided flatbuffer using reflection. +// root should point to the root type for this flatbuffer. +// buf should point to the start of flatbuffer data. +// length specifies the size of the flatbuffer data. +bool Verify(const reflection::Schema &schema, + const reflection::Object &root, + const uint8_t *buf, + size_t length); + } // namespace flatbuffers #endif // FLATBUFFERS_REFLECTION_H_ diff --git a/src/reflection.cpp b/src/reflection.cpp index e5f45dcb8..e2548a49d 100644 --- a/src/reflection.cpp +++ b/src/reflection.cpp @@ -481,4 +481,230 @@ Offset CopyTable(FlatBufferBuilder &fbb, } } +bool VerifyStruct(flatbuffers::Verifier &v, + const flatbuffers::Table &parent_table, + voffset_t field_offset, + const reflection::Object &obj, + bool required) { + auto offset = parent_table.GetOptionalFieldOffset(field_offset); + if (required && !offset) { + return false; + } + + return !offset || v.Verify(reinterpret_cast(&parent_table) + + offset, obj.bytesize()); +} + +bool VerifyVectorOfStructs(flatbuffers::Verifier &v, + const flatbuffers::Table &parent_table, + voffset_t field_offset, + const reflection::Object &obj, + bool required) { + auto p = parent_table.GetPointer(field_offset); + const uint8_t* end; + if (required && !p) { + return false; + } + + return !p || v.VerifyVector(p, obj.bytesize(), &end); +} + +// forward declare to resolve cyclic deps between VerifyObject and VerifyVector +bool VerifyObject(flatbuffers::Verifier &v, + const reflection::Schema &schema, + const reflection::Object &obj, + const flatbuffers::Table *table, + bool isRequired); + +bool VerifyVector(flatbuffers::Verifier &v, + const reflection::Schema &schema, + const flatbuffers::Table &table, + const reflection::Field &vec_field) { + assert(vec_field.type()->base_type() == reflection::BaseType::Vector); + if (!table.VerifyField(v, vec_field.offset())) + return false; + + switch (vec_field.type()->element()) { + case reflection::BaseType::None: + assert(false); + break; + case reflection::BaseType::UType: + return v.Verify(flatbuffers::GetFieldV(table, vec_field)); + case reflection::BaseType::Bool: + case reflection::BaseType::Byte: + case reflection::BaseType::UByte: + return v.Verify(flatbuffers::GetFieldV(table, vec_field)); + case reflection::BaseType::Short: + case reflection::BaseType::UShort: + return v.Verify(flatbuffers::GetFieldV(table, vec_field)); + case reflection::BaseType::Int: + case reflection::BaseType::UInt: + return v.Verify(flatbuffers::GetFieldV(table, vec_field)); + case reflection::BaseType::Long: + case reflection::BaseType::ULong: + return v.Verify(flatbuffers::GetFieldV(table, vec_field)); + case reflection::BaseType::Float: + return v.Verify(flatbuffers::GetFieldV(table, vec_field)); + case reflection::BaseType::Double: + return v.Verify(flatbuffers::GetFieldV(table, vec_field)); + case reflection::BaseType::String: { + auto vecString = + flatbuffers::GetFieldV>(table, vec_field); + if (v.Verify(vecString) && v.VerifyVectorOfStrings(vecString)) { + return true; + } else { + return false; + } + } + case reflection::BaseType::Vector: + assert(false); + break; + case reflection::BaseType::Obj: { + auto obj = schema.objects()->Get(vec_field.type()->index()); + if (obj->is_struct()) { + if (!VerifyVectorOfStructs(v, table, vec_field.offset(), *obj, + vec_field.required())) { + return false; + } + } else { + auto vec = + flatbuffers::GetFieldV>(table, vec_field); + if (!v.Verify(vec)) + return false; + if (vec) { + for (uoffset_t j = 0; j < vec->size(); j++) { + if (!VerifyObject(v, schema, *obj, vec->Get(j), true)) { + return false; + } + } + } + } + return true; + } + case reflection::BaseType::Union: + assert(false); + break; + default: + assert(false); + break; + } + + return false; +} + +bool VerifyObject(flatbuffers::Verifier &v, + const reflection::Schema &schema, + const reflection::Object &obj, + const flatbuffers::Table *table, + bool required) { + if (!table) { + if (!required) + return true; + else + return false; + } + + if (!table->VerifyTableStart(v)) + return false; + + for (size_t i = 0; i < obj.fields()->size(); i++) { + auto field_def = obj.fields()->Get(i); + switch (field_def->type()->base_type()) { + case reflection::BaseType::None: + assert(false); + break; + case reflection::BaseType::UType: + if (!table->VerifyField(v, field_def->offset())) + return false; + break; + case reflection::BaseType::Bool: + case reflection::BaseType::Byte: + case reflection::BaseType::UByte: + if (!table->VerifyField(v, field_def->offset())) + return false; + break; + case reflection::BaseType::Short: + case reflection::BaseType::UShort: + if (!table->VerifyField(v, field_def->offset())) + return false; + break; + case reflection::BaseType::Int: + case reflection::BaseType::UInt: + if (!table->VerifyField(v, field_def->offset())) + return false; + break; + case reflection::BaseType::Long: + case reflection::BaseType::ULong: + if (!table->VerifyField(v, field_def->offset())) + return false; + break; + case reflection::BaseType::Float: + if (!table->VerifyField(v, field_def->offset())) + return false; + break; + case reflection::BaseType::Double: + if (!table->VerifyField(v, field_def->offset())) + return false; + break; + case reflection::BaseType::String: + if (!table->VerifyField(v, field_def->offset()) || + !v.Verify(flatbuffers::GetFieldS(*table, *field_def))) { + return false; + } + break; + case reflection::BaseType::Vector: + if (!VerifyVector(v, schema, *table, *field_def)) + return false; + break; + case reflection::BaseType::Obj: { + auto child_obj = schema.objects()->Get(field_def->type()->index()); + if (child_obj->is_struct()) { + if (!VerifyStruct(v, *table, field_def->offset(), *child_obj, + field_def->required())) { + return false; + } + } else { + if (!VerifyObject(v, schema, *child_obj, + flatbuffers::GetFieldT(*table, *field_def), + field_def->required())) { + return false; + } + } + break; + } + case reflection::BaseType::Union: { + // get union type from the prev field + voffset_t utype_offset = field_def->offset() - sizeof(voffset_t); + auto utype = table->GetField(utype_offset, 0); + if (utype != 0) { + // Means we have this union field present + auto fb_enum = schema.enums()->Get(field_def->type()->index()); + auto child_obj = fb_enum->values()->Get(utype)->object(); + if (!VerifyObject(v, schema, *child_obj, + flatbuffers::GetFieldT(*table, *field_def), + field_def->required())) { + return false; + } + } + break; + } + default: + assert(false); + break; + } + } + + return true; +} + +bool Verify(const reflection::Schema &schema, + const reflection::Object &root, + const uint8_t *buf, + size_t length) { + Verifier v(buf, length); + return VerifyObject(v, schema, root, flatbuffers::GetAnyRoot(buf), true); +} + } // namespace flatbuffers diff --git a/tests/test.cpp b/tests/test.cpp index ab18354c2..4f4495289 100644 --- a/tests/test.cpp +++ b/tests/test.cpp @@ -14,9 +14,6 @@ * limitations under the License. */ -#define FLATBUFFERS_DEBUG_VERIFICATION_FAILURE 1 -#define FLATBUFFERS_TRACK_VERIFIER_BUFFER_SIZE - #include "flatbuffers/flatbuffers.h" #include "flatbuffers/idl.h" #include "flatbuffers/util.h" @@ -493,8 +490,15 @@ void ReflectionTest(uint8_t *flatbuf, size_t length) { TEST_NOTNULL(pos_table_ptr); TEST_EQ_STR(pos_table_ptr->name()->c_str(), "MyGame.Example.Vec3"); + + // Now use it to dynamically access a buffer. auto &root = *flatbuffers::GetAnyRoot(flatbuf); + + // Verify the buffer first using reflection based verification + TEST_EQ(flatbuffers::Verify(schema, *schema.root_table(), flatbuf, length), + true); + auto hp = flatbuffers::GetFieldI(root, hp_field); TEST_EQ(hp, 80); @@ -523,6 +527,10 @@ void ReflectionTest(uint8_t *flatbuf, size_t length) { hp_int64 = flatbuffers::GetAnyFieldI(root, hp_field); TEST_EQ(hp_int64, 300); + // Test buffer is valid after the modifications + TEST_EQ(flatbuffers::Verify(schema, *schema.root_table(), flatbuf, length), + true); + // Reset it, for further tests. flatbuffers::SetField(&root, hp_field, 80); @@ -584,6 +592,11 @@ void ReflectionTest(uint8_t *flatbuf, size_t length) { reinterpret_cast(resizingbuf.data()), resizingbuf.size()); TEST_EQ(VerifyMonsterBuffer(resize_verifier), true); + + // Test buffer is valid using reflection as well + TEST_EQ(flatbuffers::Verify(schema, *schema.root_table(), resizingbuf.data(), + resizingbuf.size()), true); + // As an additional test, also set it on the name field. // Note: unlike the name change above, this just overwrites the offset, // rather than changing the string in-place. @@ -600,6 +613,10 @@ void ReflectionTest(uint8_t *flatbuf, size_t length) { fbb.Finish(root_offset, MonsterIdentifier()); // Test that it was copied correctly: AccessFlatBufferTest(fbb.GetBufferPointer(), fbb.GetSize()); + + // Test buffer is valid using reflection as well + TEST_EQ(flatbuffers::Verify(schema, *schema.root_table(), + fbb.GetBufferPointer(), fbb.GetSize()), true); } // Parse a .proto schema, output as .fbs