Fix binary serializiation of shared_ptr too big for variable length encoding

* Retain backward compatibility by using either variable length int
  or regular 64-bit int extending the flags
* Add test for both cases
This commit is contained in:
Martchus 2019-03-08 17:40:08 +01:00
parent b058e9e9b9
commit 04852627b2
2 changed files with 52 additions and 6 deletions

View File

@ -107,15 +107,15 @@ template <typename Type, Traits::EnableIf<Traits::IsSpecializationOf<Type, std::
template <typename Type, Traits::EnableIf<Traits::IsSpecializationOf<Type, std::shared_ptr>> *> void BinaryDeserializer::read(Type &pointer) template <typename Type, Traits::EnableIf<Traits::IsSpecializationOf<Type, std::shared_ptr>> *> void BinaryDeserializer::read(Type &pointer)
{ {
const auto occurence = readByte(); auto mode = readByte();
if (!occurence) { if (!mode) {
// pointer not set // pointer not set
pointer.reset(); pointer.reset();
return; return;
} }
const auto id = readVariableLengthUIntBE(); const auto id = (mode & 0x4) ? readUInt64BE() : readVariableLengthUIntBE(); // the 3rd bit being flagged indicates a big ID
if (occurence == 1) { if ((mode & 0x3) == 1) {
// first occurence: make a new pointer // first occurence: make a new pointer
m_pointer[id] = pointer = std::make_shared<typename Type::element_type>(); m_pointer[id] = pointer = std::make_shared<typename Type::element_type>();
read(*pointer); read(*pointer);
@ -200,9 +200,18 @@ template <typename Type, Traits::EnableIf<Traits::IsSpecializingAnyOf<Type, std:
return; return;
} }
const auto id = reinterpret_cast<uint64>(pointer.get()); const auto id = reinterpret_cast<uint64>(pointer.get());
const auto bigId = id >= 0x80000000000000;
auto &alreadyWritten = m_pointer[id]; auto &alreadyWritten = m_pointer[id];
writeByte(alreadyWritten ? 2 : 1); byte mode = alreadyWritten ? 2 : 1;
writeVariableLengthUIntBE(id); if (bigId) {
mode = mode | 0x4; // "flag" 3rd bit to indicate big ID
}
writeByte(mode);
if (bigId) {
writeUInt64BE(id);
} else {
writeVariableLengthUIntBE(id);
}
if (!alreadyWritten) { if (!alreadyWritten) {
alreadyWritten = true; alreadyWritten = true;
write(*pointer); write(*pointer);

View File

@ -12,6 +12,7 @@ using TestUtilities::operator<<; // must be visible prior to the call site
#include <cppunit/extensions/HelperMacros.h> #include <cppunit/extensions/HelperMacros.h>
#include <iostream> #include <iostream>
#include <limits>
#include <map> #include <map>
#include <sstream> #include <sstream>
#include <string> #include <string>
@ -137,6 +138,8 @@ class BinaryReflectorTests : public TestFixture {
CPPUNIT_TEST(testDeserializeSimpleStruct); CPPUNIT_TEST(testDeserializeSimpleStruct);
CPPUNIT_TEST(testSerializeNestedStruct); CPPUNIT_TEST(testSerializeNestedStruct);
CPPUNIT_TEST(testDeserializeNestedStruct); CPPUNIT_TEST(testDeserializeNestedStruct);
CPPUNIT_TEST(testSmallSharedPointer);
CPPUNIT_TEST(testBigSharedPointer);
CPPUNIT_TEST_SUITE_END(); CPPUNIT_TEST_SUITE_END();
public: public:
@ -150,6 +153,9 @@ public:
void testSerializeNestedStruct(); void testSerializeNestedStruct();
void testDeserializeNestedStruct(); void testDeserializeNestedStruct();
void assertTestObject(const TestObjectBinary &deserialized); void assertTestObject(const TestObjectBinary &deserialized);
void testSharedPointer(std::uintptr_t fakePointer);
void testSmallSharedPointer();
void testBigSharedPointer();
private: private:
vector<unsigned char> m_buffer; vector<unsigned char> m_buffer;
@ -292,3 +298,34 @@ void BinaryReflectorTests::assertTestObject(const TestObjectBinary &deserialized
CPPUNIT_ASSERT_EQUAL(m_testObj.someUnorderedSet, deserialized.someUnorderedSet); CPPUNIT_ASSERT_EQUAL(m_testObj.someUnorderedSet, deserialized.someUnorderedSet);
CPPUNIT_ASSERT_EQUAL(m_testObj.someUnorderedMultiset, deserialized.someUnorderedMultiset); CPPUNIT_ASSERT_EQUAL(m_testObj.someUnorderedMultiset, deserialized.someUnorderedMultiset);
} }
void BinaryReflectorTests::testSharedPointer(uintptr_t fakePointer)
{
// create a shared pointer for the fake pointer ensuring that it is not actually deleted
shared_ptr<int> sharedPointer(reinterpret_cast<int *>(fakePointer), [](int *) {});
// setup stream
stringstream stream(ios_base::in | ios_base::out | ios_base::binary);
stream.exceptions(ios_base::failbit | ios_base::badbit);
// serialize the shared pointer assuming its contents have been written before (to prevent actually dereferencing it)
BinaryReflector::BinarySerializer serializer(&stream);
serializer.m_pointer[fakePointer] = true;
serializer.write(sharedPointer);
// deserialize the shared pointer assuming it has already been read and the type matches
deserializer.m_pointer[fakePointer] = make_shared<int>(42);
deserializer.read(readPtr);
CPPUNIT_ASSERT(readPtr != nullptr);
CPPUNIT_ASSERT_EQUAL(42, *readPtr);
}
void BinaryReflectorTests::testSmallSharedPointer()
{
testSharedPointer(std::numeric_limits<std::uintptr_t>::min() + 1);
}
void BinaryReflectorTests::testBigSharedPointer()
{
testSharedPointer(std::numeric_limits<std::uintptr_t>::max());
}