Skip to content

Commit

Permalink
Fix memleak in ObjectIOStream usage
Browse files Browse the repository at this point in the history
  • Loading branch information
@bhennion
bhennion committed Sep 24, 2023
1 parent d07309b commit 214b4cf
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 112 deletions.
6 changes: 1 addition & 5 deletions src/core/model/Stroke.cpp
View file
Expand Up @@ -196,11 +196,7 @@ void Stroke::readSerialized(ObjectInputStream& in) {

this->capStyle = static_cast<StrokeCapStyle>(in.readInt());

Point* p{};
int count{};
in.readData(reinterpret_cast<void**>(&p), &count);
this->points = std::vector<Point>{p, p + count};
g_free(p);
in.readData(this->points);
this->lineStyle.readSerialized(in);

in.endObject();
Expand Down
9 changes: 3 additions & 6 deletions src/core/model/TexImage.cpp
View file
Expand Up @@ -135,7 +135,7 @@ void TexImage::serialize(ObjectOutputStream& out) const {
out.writeDouble(this->height);
out.writeString(this->text);

out.writeData(this->binaryData.c_str(), this->binaryData.length(), 1);
out.writeString(this->binaryData);

out.endObject();
}
Expand All @@ -151,11 +151,8 @@ void TexImage::readSerialized(ObjectInputStream& in) {

freeImageAndPdf();

char* data = nullptr;
int len = 0;
in.readData(reinterpret_cast<void**>(&data), &len);

this->loadData(std::string(data, len), nullptr);
std::string data = in.readString();
this->loadData(std::move(data), nullptr);

in.endObject();
this->calcSize();
Expand Down
37 changes: 35 additions & 2 deletions src/util/include/util/serializing/ObjectInputStream.h
View file
Expand Up @@ -16,6 +16,8 @@
#include <string> // for string
#include <vector> // for vector

#include "InputStreamException.h"

class ObjectInputStream {
public:
ObjectInputStream() = default;
Expand All @@ -34,7 +36,6 @@ class ObjectInputStream {
size_t readSizeT();
std::string readString();

void readData(void** data, int* len);
template <typename T>
void readData(std::vector<T>& data);

Expand All @@ -46,10 +47,42 @@ class ObjectInputStream {

static std::string getType(char type);

template <class T>
T readType();

private:
std::istringstream istream;
size_t pos();
size_t len = 0;
};

extern template void ObjectInputStream::readData(std::vector<double>& data);
extern template int ObjectInputStream::readType<int>();

template <typename T>
void ObjectInputStream::readData(std::vector<T>& data) {
checkType('b');

if (istream.str().size() < 2 * sizeof(int)) {
throw InputStreamException("End reached, but try to read data len and width", __FILE__, __LINE__);
}

int len = readType<int>();
int width = readType<int>();

if (width != sizeof(T)) {
throw InputStreamException("Data width mismatch requested type width", __FILE__, __LINE__);
}

if (len < 0) {
throw InputStreamException("Negative length of data array", __FILE__, __LINE__);
}

if (istream.str().size() < static_cast<size_t>(len * width)) {
throw InputStreamException("End reached, but try to read data", __FILE__, __LINE__);
}

if (len) {
data.resize(static_cast<size_t>(len));
istream.read((char*)data.data(), len * width);
}
}
5 changes: 4 additions & 1 deletion src/util/include/util/serializing/ObjectOutputStream.h
View file
Expand Up @@ -49,4 +49,7 @@ class ObjectOutputStream {
ObjectEncoding* encoder = nullptr;
};

extern template void ObjectOutputStream::writeData(const std::vector<double>& data);
template <typename T>
void ObjectOutputStream::writeData(const std::vector<T>& data) {
writeData(data.data(), static_cast<int>(data.size()), sizeof(T));
}
64 changes: 7 additions & 57 deletions src/util/serializing/ObjectInputStream.cpp
View file
Expand Up @@ -9,11 +9,11 @@
#include "util/serializing/InputStreamException.h" // for InputStreamException
#include "util/serializing/Serializable.h" // for XML_VERSION_STR

template void ObjectInputStream::readData(std::vector<double>& data);
template int ObjectInputStream::readType<int>();

// This function requires that T is read from its binary representation to work (e.g. integer type)
template <typename T>
T readTypeFromSStream(std::istringstream& istream) {
T ObjectInputStream::readType() {
if (istream.str().size() < sizeof(T)) {
std::ostringstream oss;
oss << "End reached: trying to read " << sizeof(T) << " bytes while only " << istream.str().size()
Expand Down Expand Up @@ -76,23 +76,23 @@ void ObjectInputStream::endObject() { checkType('}'); }

auto ObjectInputStream::readInt() -> int {
checkType('i');
return readTypeFromSStream<int>(istream);
return readType<int>();
}

auto ObjectInputStream::readDouble() -> double {
checkType('d');
return readTypeFromSStream<double>(istream);
return readType<double>();
}

auto ObjectInputStream::readSizeT() -> size_t {
checkType('l');
return readTypeFromSStream<size_t>(istream);
return readType<size_t>();
}

auto ObjectInputStream::readString() -> std::string {
checkType('s');

size_t lenString = (size_t)readTypeFromSStream<int>(istream);
size_t lenString = (size_t)readType<int>();

if (istream.str().size() < len) {
throw InputStreamException("End reached, but try to read an string", __FILE__, __LINE__);
Expand All @@ -106,64 +106,14 @@ auto ObjectInputStream::readString() -> std::string {
return output;
}

void ObjectInputStream::readData(void** data, int* length) {
checkType('b');

if (istream.str().size() < 2 * sizeof(int)) {
throw InputStreamException("End reached, but try to read data len and width", __FILE__, __LINE__);
}

int len = readTypeFromSStream<int>(istream);
int width = readTypeFromSStream<int>(istream);

if (istream.str().size() < static_cast<size_t>(len * width)) {
throw InputStreamException("End reached, but try to read data", __FILE__, __LINE__);
}

if (len == 0) {
*length = 0;
*data = nullptr;
} else {
*data = (void*)new char[(size_t)(len * width)];
*length = len;

istream.read((char*)*data, len * width);
}
}

template <typename T>
void ObjectInputStream::readData(std::vector<T>& data) {
checkType('b');

if (istream.str().size() < 2 * sizeof(int)) {
throw InputStreamException("End reached, but try to read data len and width", __FILE__, __LINE__);
}

int len = readTypeFromSStream<int>(istream);
int width = readTypeFromSStream<int>(istream);

if (width != sizeof(T)) {
throw InputStreamException("Data width mismatch requested type width", __FILE__, __LINE__);
}

if (istream.str().size() < static_cast<size_t>(len * width)) {
throw InputStreamException("End reached, but try to read data", __FILE__, __LINE__);
}

if (len) {
data.resize(len);
istream.read((char*)data.data(), len * width);
}
}

auto ObjectInputStream::readImage() -> std::string {
checkType('m');

if (istream.str().size() < sizeof(size_t)) {
throw InputStreamException("End reached, but try to read an image's data's length", __FILE__, __LINE__);
}

const size_t len = readTypeFromSStream<size_t>(istream);
const size_t len = readType<size_t>();
if (istream.str().size() < len) {
throw InputStreamException("End reached, but try to read an image", __FILE__, __LINE__);
}
Expand Down
12 changes: 0 additions & 12 deletions src/util/serializing/ObjectOutputStream.cpp
View file
Expand Up @@ -5,8 +5,6 @@
#include "util/serializing/ObjectEncoding.h" // for ObjectEncoding
#include "util/serializing/Serializable.h" // for XML_VERSION_STR

template void ObjectOutputStream::writeData(const std::vector<double>& data);

ObjectOutputStream::ObjectOutputStream(ObjectEncoding* encoder) {
g_assert(encoder != nullptr);
this->encoder = encoder;
Expand Down Expand Up @@ -62,16 +60,6 @@ void ObjectOutputStream::writeData(const void* data, int len, int width) {
}
}

template <typename T>
void ObjectOutputStream::writeData(const std::vector<T>& data) {
this->encoder->addStr("_b");
const int len = static_cast<int>(data.size());
const int width = sizeof(T);
this->encoder->addData(&len, sizeof(int));
this->encoder->addData(&width, sizeof(int));
this->encoder->addData(data.data(), static_cast<int>(data.size() * sizeof(T)));
}

void ObjectOutputStream::writeImage(const std::string_view& imgData) {
this->encoder->addStr("_m");
size_t len = imgData.length();
Expand Down
42 changes: 13 additions & 29 deletions test/unit_tests/util/ObjectIOStreamTest.cpp
View file
Expand Up @@ -14,14 +14,6 @@
#include "util/serializing/ObjectOutputStream.h"
#include "util/serializing/Serializable.h"

template <typename T, unsigned N>
std::string serializeData(const std::array<T, N>& data) {
ObjectOutputStream outStream(new BinObjectEncoding);
outStream.writeData(&data[0], N, sizeof(T));
auto outStr = outStream.getStr();
return {outStr->str, outStr->len};
}

template <typename T>
std::string serializeDataVector(const std::vector<T>& data) {
ObjectOutputStream outStream(new BinObjectEncoding);
Expand Down Expand Up @@ -73,21 +65,6 @@ std::string serializeStroke(Stroke& stroke) {
return {outStr->str, outStr->len};
}

template <typename T, unsigned int N>
void testReadDataType(const std::array<T, N>& data) {
std::string str = serializeData<T, N>(data);

ObjectInputStream stream;
EXPECT_TRUE(stream.read(&str[0], (int)str.size() + 1));

int length = 0;
T* outputData = nullptr;
stream.readData((void**)&outputData, &length);
EXPECT_EQ(length, (int)N);

for (size_t i = 0; i < (size_t)length / sizeof(T); ++i) { EXPECT_EQ(outputData[i], data.at(i)); }
}

template <typename T>
void testReadDataType(const std::vector<T>& data) {
std::string str = serializeDataVector<T>(data);
Expand All @@ -102,12 +79,19 @@ void testReadDataType(const std::vector<T>& data) {


TEST(UtilObjectIOStream, testReadData) {
testReadDataType<char, 3>(std::array<char, 3>{0, 42, -42});
testReadDataType<long, 3>(std::array<long, 3>{0, 42, -42});
testReadDataType<long long, 3>(std::array<long long, 3>{0, 420000000000, -42000000000});
testReadDataType<double, 3>(std::array<double, 3>{0, 42., -42.});
testReadDataType<float, 3>(std::array<float, 3>{0, 42., -42.});
testReadDataType<double>(std::vector<double>{0, 42., -42.});
testReadDataType(std::vector<char>{0, 42, -42});
testReadDataType(std::vector<long>{0, 42, -42});
testReadDataType(std::vector<long long>{0, 420000000000, -42000000000});
testReadDataType(std::vector<float>{0, 42., -42.});
testReadDataType(std::vector<double>{0, 42., -42.});

struct Data {
bool operator==(const Data& o) const { return s == o.s && f == o.f && b == o.b; }
size_t s;
float f;
bool b;
};
testReadDataType(std::vector<Data>{{243254, 0.4534314213f, true}, {2, -4243213.32f, false}});
}

TEST(UtilObjectIOStream, testReadImage) {
Expand Down

0 comments on commit 214b4cf

Please sign in to comment.