Fix memory leaks (mostly) in tests (#3922)

This makes most tests run successfully with asan.

The GUI tests still have a bunch of leaks, some from library code, and
some that look real but which I didn't immediately manage to figure out.

* TestOpVaultReader: use QSharedPointer
This commit is contained in:
Carlo Teubner 2019-12-21 14:25:17 +00:00 committed by Jonathan White
parent c0f29cc790
commit c70ebe6dce
3 changed files with 55 additions and 56 deletions

View File

@ -60,7 +60,7 @@ SearchWidget::SearchWidget(QWidget* parent)
.arg(QKeySequence(QKeySequence::Find).toString(QKeySequence::NativeText))); .arg(QKeySequence(QKeySequence::Find).toString(QKeySequence::NativeText)));
m_ui->searchEdit->installEventFilter(this); m_ui->searchEdit->installEventFilter(this);
m_searchMenu = new QMenu(); m_searchMenu = new QMenu(this);
m_actionCaseSensitive = m_searchMenu->addAction(tr("Case sensitive"), this, SLOT(updateCaseSensitive())); m_actionCaseSensitive = m_searchMenu->addAction(tr("Case sensitive"), this, SLOT(updateCaseSensitive()));
m_actionCaseSensitive->setObjectName("actionSearchCaseSensitive"); m_actionCaseSensitive->setObjectName("actionSearchCaseSensitive");
m_actionCaseSensitive->setCheckable(true); m_actionCaseSensitive->setCheckable(true);

View File

@ -20,6 +20,7 @@
#include "TestGlobal.h" #include "TestGlobal.h"
#include "mock/MockClock.h" #include "mock/MockClock.h"
#include <QScopedPointer>
#include <QSignalSpy> #include <QSignalSpy>
#include "core/Metadata.h" #include "core/Metadata.h"
@ -798,16 +799,16 @@ void TestGroup::testAddEntryWithPath()
void TestGroup::testIsRecycled() void TestGroup::testIsRecycled()
{ {
Database* db = new Database(); Database db;
db->metadata()->setRecycleBinEnabled(true); db.metadata()->setRecycleBinEnabled(true);
Group* group1 = new Group(); Group* group1 = new Group();
group1->setName("group1"); group1->setName("group1");
group1->setParent(db->rootGroup()); group1->setParent(db.rootGroup());
Group* group2 = new Group(); Group* group2 = new Group();
group2->setName("group2"); group2->setName("group2");
group2->setParent(db->rootGroup()); group2->setParent(db.rootGroup());
Group* group3 = new Group(); Group* group3 = new Group();
group3->setName("group3"); group3->setName("group3");
@ -815,16 +816,16 @@ void TestGroup::testIsRecycled()
Group* group4 = new Group(); Group* group4 = new Group();
group4->setName("group4"); group4->setName("group4");
group4->setParent(db->rootGroup()); group4->setParent(db.rootGroup());
db->recycleGroup(group2); db.recycleGroup(group2);
QVERIFY(!group1->isRecycled()); QVERIFY(!group1->isRecycled());
QVERIFY(group2->isRecycled()); QVERIFY(group2->isRecycled());
QVERIFY(group3->isRecycled()); QVERIFY(group3->isRecycled());
QVERIFY(!group4->isRecycled()); QVERIFY(!group4->isRecycled());
db->recycleGroup(group4); db.recycleGroup(group4);
QVERIFY(group4->isRecycled()); QVERIFY(group4->isRecycled());
} }
@ -1052,12 +1053,12 @@ void TestGroup::testChildrenSort()
void TestGroup::testHierarchy() void TestGroup::testHierarchy()
{ {
Group* group1 = new Group(); Group group1;
group1->setName("group1"); group1.setName("group1");
Group* group2 = new Group(); Group* group2 = new Group();
group2->setName("group2"); group2->setName("group2");
group2->setParent(group1); group2->setParent(&group1);
Group* group3 = new Group(); Group* group3 = new Group();
group3->setName("group3"); group3->setName("group3");
@ -1085,11 +1086,11 @@ void TestGroup::testHierarchy()
void TestGroup::testApplyGroupIconRecursively() void TestGroup::testApplyGroupIconRecursively()
{ {
// Create a database with two nested groups with one entry each // Create a database with two nested groups with one entry each
Database* database = new Database(); Database database;
Group* subgroup = new Group(); Group* subgroup = new Group();
subgroup->setName("Subgroup"); subgroup->setName("Subgroup");
subgroup->setParent(database->rootGroup()); subgroup->setParent(database.rootGroup());
QVERIFY(subgroup); QVERIFY(subgroup);
Group* subsubgroup = new Group(); Group* subsubgroup = new Group();
@ -1108,10 +1109,10 @@ void TestGroup::testApplyGroupIconRecursively()
// Set an icon per number to the root group and apply recursively // Set an icon per number to the root group and apply recursively
// -> all groups and entries have the same icon // -> all groups and entries have the same icon
const int rootIconNumber = 42; const int rootIconNumber = 42;
database->rootGroup()->setIcon(rootIconNumber); database.rootGroup()->setIcon(rootIconNumber);
QVERIFY(database->rootGroup()->iconNumber() == rootIconNumber); QVERIFY(database.rootGroup()->iconNumber() == rootIconNumber);
database->rootGroup()->applyGroupIconToChildGroups(); database.rootGroup()->applyGroupIconToChildGroups();
database->rootGroup()->applyGroupIconToChildEntries(); database.rootGroup()->applyGroupIconToChildEntries();
QVERIFY(subgroup->iconNumber() == rootIconNumber); QVERIFY(subgroup->iconNumber() == rootIconNumber);
QVERIFY(subgroupEntry->iconNumber() == rootIconNumber); QVERIFY(subgroupEntry->iconNumber() == rootIconNumber);
QVERIFY(subsubgroup->iconNumber() == rootIconNumber); QVERIFY(subsubgroup->iconNumber() == rootIconNumber);
@ -1124,7 +1125,7 @@ void TestGroup::testApplyGroupIconRecursively()
QVERIFY(subsubgroup->iconNumber() == subsubgroupIconNumber); QVERIFY(subsubgroup->iconNumber() == subsubgroupIconNumber);
subsubgroup->applyGroupIconToChildGroups(); subsubgroup->applyGroupIconToChildGroups();
subsubgroup->applyGroupIconToChildEntries(); subsubgroup->applyGroupIconToChildEntries();
QVERIFY(database->rootGroup()->iconNumber() == rootIconNumber); QVERIFY(database.rootGroup()->iconNumber() == rootIconNumber);
QVERIFY(subgroup->iconNumber() == rootIconNumber); QVERIFY(subgroup->iconNumber() == rootIconNumber);
QVERIFY(subgroupEntry->iconNumber() == rootIconNumber); QVERIFY(subgroupEntry->iconNumber() == rootIconNumber);
QVERIFY(subsubgroup->iconNumber() == subsubgroupIconNumber); QVERIFY(subsubgroup->iconNumber() == subsubgroupIconNumber);
@ -1135,11 +1136,11 @@ void TestGroup::testApplyGroupIconRecursively()
const QUuid subgroupIconUuid = QUuid::createUuid(); const QUuid subgroupIconUuid = QUuid::createUuid();
QImage subgroupIcon(16, 16, QImage::Format_RGB32); QImage subgroupIcon(16, 16, QImage::Format_RGB32);
subgroupIcon.setPixel(0, 0, qRgb(255, 0, 0)); subgroupIcon.setPixel(0, 0, qRgb(255, 0, 0));
database->metadata()->addCustomIcon(subgroupIconUuid, subgroupIcon); database.metadata()->addCustomIcon(subgroupIconUuid, subgroupIcon);
subgroup->setIcon(subgroupIconUuid); subgroup->setIcon(subgroupIconUuid);
subgroup->applyGroupIconToChildGroups(); subgroup->applyGroupIconToChildGroups();
subgroup->applyGroupIconToChildEntries(); subgroup->applyGroupIconToChildEntries();
QVERIFY(database->rootGroup()->iconNumber() == rootIconNumber); QVERIFY(database.rootGroup()->iconNumber() == rootIconNumber);
QCOMPARE(subgroup->iconUuid(), subgroupIconUuid); QCOMPARE(subgroup->iconUuid(), subgroupIconUuid);
QCOMPARE(subgroup->icon(), subgroupIcon); QCOMPARE(subgroup->icon(), subgroupIcon);
QCOMPARE(subgroupEntry->iconUuid(), subgroupIconUuid); QCOMPARE(subgroupEntry->iconUuid(), subgroupIconUuid);
@ -1150,10 +1151,10 @@ void TestGroup::testApplyGroupIconRecursively()
QCOMPARE(subsubgroupEntry->icon(), subgroupIcon); QCOMPARE(subsubgroupEntry->icon(), subgroupIcon);
// Reset all icons to root icon // Reset all icons to root icon
database->rootGroup()->setIcon(rootIconNumber); database.rootGroup()->setIcon(rootIconNumber);
QVERIFY(database->rootGroup()->iconNumber() == rootIconNumber); QVERIFY(database.rootGroup()->iconNumber() == rootIconNumber);
database->rootGroup()->applyGroupIconToChildGroups(); database.rootGroup()->applyGroupIconToChildGroups();
database->rootGroup()->applyGroupIconToChildEntries(); database.rootGroup()->applyGroupIconToChildEntries();
QVERIFY(subgroup->iconNumber() == rootIconNumber); QVERIFY(subgroup->iconNumber() == rootIconNumber);
QVERIFY(subgroupEntry->iconNumber() == rootIconNumber); QVERIFY(subgroupEntry->iconNumber() == rootIconNumber);
QVERIFY(subsubgroup->iconNumber() == rootIconNumber); QVERIFY(subsubgroup->iconNumber() == rootIconNumber);
@ -1161,10 +1162,10 @@ void TestGroup::testApplyGroupIconRecursively()
// Apply only for child groups // Apply only for child groups
const int iconForGroups = 10; const int iconForGroups = 10;
database->rootGroup()->setIcon(iconForGroups); database.rootGroup()->setIcon(iconForGroups);
QVERIFY(database->rootGroup()->iconNumber() == iconForGroups); QVERIFY(database.rootGroup()->iconNumber() == iconForGroups);
database->rootGroup()->applyGroupIconToChildGroups(); database.rootGroup()->applyGroupIconToChildGroups();
QVERIFY(database->rootGroup()->iconNumber() == iconForGroups); QVERIFY(database.rootGroup()->iconNumber() == iconForGroups);
QVERIFY(subgroup->iconNumber() == iconForGroups); QVERIFY(subgroup->iconNumber() == iconForGroups);
QVERIFY(subgroupEntry->iconNumber() == rootIconNumber); QVERIFY(subgroupEntry->iconNumber() == rootIconNumber);
QVERIFY(subsubgroup->iconNumber() == iconForGroups); QVERIFY(subsubgroup->iconNumber() == iconForGroups);
@ -1172,10 +1173,10 @@ void TestGroup::testApplyGroupIconRecursively()
// Apply only for child entries // Apply only for child entries
const int iconForEntries = 20; const int iconForEntries = 20;
database->rootGroup()->setIcon(iconForEntries); database.rootGroup()->setIcon(iconForEntries);
QVERIFY(database->rootGroup()->iconNumber() == iconForEntries); QVERIFY(database.rootGroup()->iconNumber() == iconForEntries);
database->rootGroup()->applyGroupIconToChildEntries(); database.rootGroup()->applyGroupIconToChildEntries();
QVERIFY(database->rootGroup()->iconNumber() == iconForEntries); QVERIFY(database.rootGroup()->iconNumber() == iconForEntries);
QVERIFY(subgroup->iconNumber() == iconForGroups); QVERIFY(subgroup->iconNumber() == iconForGroups);
QVERIFY(subgroupEntry->iconNumber() == iconForEntries); QVERIFY(subgroupEntry->iconNumber() == iconForEntries);
QVERIFY(subsubgroup->iconNumber() == iconForGroups); QVERIFY(subsubgroup->iconNumber() == iconForGroups);
@ -1184,15 +1185,15 @@ void TestGroup::testApplyGroupIconRecursively()
void TestGroup::testUsernamesRecursive() void TestGroup::testUsernamesRecursive()
{ {
Database* database = new Database(); Database database;
// Create a subgroup // Create a subgroup
Group* subgroup = new Group(); Group* subgroup = new Group();
subgroup->setName("Subgroup"); subgroup->setName("Subgroup");
subgroup->setParent(database->rootGroup()); subgroup->setParent(database.rootGroup());
// Generate entries in the root group and the subgroup // Generate entries in the root group and the subgroup
Entry* rootGroupEntry = database->rootGroup()->addEntryWithPath("Root group entry"); Entry* rootGroupEntry = database.rootGroup()->addEntryWithPath("Root group entry");
rootGroupEntry->setUsername("Name1"); rootGroupEntry->setUsername("Name1");
Entry* subgroupEntry = subgroup->addEntryWithPath("Subgroup entry"); Entry* subgroupEntry = subgroup->addEntryWithPath("Subgroup entry");
@ -1201,7 +1202,7 @@ void TestGroup::testUsernamesRecursive()
Entry* subgroupEntryReusingUsername = subgroup->addEntryWithPath("Another subgroup entry"); Entry* subgroupEntryReusingUsername = subgroup->addEntryWithPath("Another subgroup entry");
subgroupEntryReusingUsername->setUsername("Name2"); subgroupEntryReusingUsername->setUsername("Name2");
QList<QString> usernames = database->rootGroup()->usernamesRecursive(); QList<QString> usernames = database.rootGroup()->usernamesRecursive();
QCOMPARE(usernames.size(), 2); QCOMPARE(usernames.size(), 2);
QVERIFY(usernames.contains("Name1")); QVERIFY(usernames.contains("Name1"));
QVERIFY(usernames.contains("Name2")); QVERIFY(usernames.contains("Name2"));

View File

@ -49,24 +49,24 @@ QPair<QString, QString>* split1PTextExportKV(QByteArray& line)
return new QPair<QString, QString>(k, v); return new QPair<QString, QString>(k, v);
} }
QJsonArray* read1PasswordTextExport(QFile& f) QSharedPointer<QJsonArray> read1PasswordTextExport(QFile& f)
{ {
auto result = new QJsonArray;
auto current = new QJsonObject;
if (!f.open(QIODevice::ReadOnly)) { if (!f.open(QIODevice::ReadOnly)) {
qCritical("Unable to open your text export file for reading"); qCritical("Unable to open your text export file for reading");
return nullptr; return {};
} }
auto result = QSharedPointer<QJsonArray>::create();
QJsonObject current;
while (!f.atEnd()) { while (!f.atEnd()) {
auto line = f.readLine(1024); auto line = f.readLine(1024);
if (line.size() == 1 and line[0] == '\n') { if (line.size() == 1 and line[0] == '\n') {
if (!current->isEmpty()) { if (!current.isEmpty()) {
result->append(*current); result->append(current);
} }
current = new QJsonObject; current = QJsonObject();
continue; continue;
} }
const auto kv = split1PTextExportKV(line); const auto kv = split1PTextExportKV(line);
@ -95,14 +95,14 @@ QJsonArray* read1PasswordTextExport(QFile& f)
} }
} }
auto v = lines.join(""); auto v = lines.join("");
(*current)[k] = v; current[k] = v;
} else { } else {
(*current)[k] = kv->second; current[k] = kv->second;
} }
delete kv; delete kv;
} }
if (!current->isEmpty()) { if (!current.isEmpty()) {
result->append(*current); result->append(current);
} }
f.close(); f.close();
@ -120,10 +120,9 @@ void TestOpVaultReader::initTestCase()
m_password = "freddy"; m_password = "freddy";
QFile testData(m_opVaultTextExportPath); QFile testData(m_opVaultTextExportPath);
QJsonArray* data = read1PasswordTextExport(testData); auto data = read1PasswordTextExport(testData);
QVERIFY(data); QVERIFY(data);
QCOMPARE(data->size(), 27); QCOMPARE(data->size(), 27);
delete data;
m_categoryMap.insert("001", "Login"); m_categoryMap.insert("001", "Login");
m_categoryMap.insert("002", "Credit Card"); m_categoryMap.insert("002", "Credit Card");
@ -149,9 +148,9 @@ void TestOpVaultReader::testReadIntoDatabase()
{ {
QDir opVaultDir(m_opVaultPath); QDir opVaultDir(m_opVaultPath);
auto reader = new OpVaultReader(); OpVaultReader reader;
auto db = reader->readDatabase(opVaultDir, m_password); QScopedPointer<Database> db(reader.readDatabase(opVaultDir, m_password));
QVERIFY2(!reader->hasError(), qPrintable(reader->errorString())); QVERIFY2(!reader.hasError(), qPrintable(reader.errorString()));
QVERIFY(db); QVERIFY(db);
QVERIFY(!db->children().isEmpty()); QVERIFY(!db->children().isEmpty());
@ -179,7 +178,6 @@ void TestOpVaultReader::testReadIntoDatabase()
QUuid u = Tools::hexToUuid(value["uuid"].toString()); QUuid u = Tools::hexToUuid(value["uuid"].toString());
objectsByUuid[u] = value; objectsByUuid[u] = value;
} }
delete testData;
QCOMPARE(objectsByUuid.size(), 27); QCOMPARE(objectsByUuid.size(), 27);
for (QUuid u : objectsByUuid.keys()) { for (QUuid u : objectsByUuid.keys()) {
@ -240,11 +238,11 @@ void TestOpVaultReader::testKeyDerivation()
void TestOpVaultReader::testBandEntry1() void TestOpVaultReader::testBandEntry1()
{ {
auto reader = new OpVaultReader(); OpVaultReader reader;
QByteArray json(R"({"hello": "world"})"); QByteArray json(R"({"hello": "world"})");
QJsonDocument doc = QJsonDocument::fromJson(json); QJsonDocument doc = QJsonDocument::fromJson(json);
QJsonObject data; QJsonObject data;
QByteArray entryKey; QByteArray entryKey;
QByteArray entryHmacKey; QByteArray entryHmacKey;
QVERIFY(!reader->decryptBandEntry(doc.object(), data, entryKey, entryHmacKey)); QVERIFY(!reader.decryptBandEntry(doc.object(), data, entryKey, entryHmacKey));
} }