diff --git a/src/core/Metadata.cpp b/src/core/Metadata.cpp index 8bb0b52e4..a055bef6f 100644 --- a/src/core/Metadata.cpp +++ b/src/core/Metadata.cpp @@ -28,6 +28,9 @@ const int Metadata::DefaultHistoryMaxItems = 10; const int Metadata::DefaultHistoryMaxSize = 6 * 1024 * 1024; +// Fallback icon for return by reference +static const Metadata::CustomIconData NULL_ICON; + Metadata::Metadata(QObject* parent) : ModifiableObject(parent) , m_customData(new CustomData(this)) @@ -176,9 +179,14 @@ bool Metadata::protectNotes() const return m_data.protectNotes; } -QByteArray Metadata::customIcon(const QUuid& uuid) const +const Metadata::CustomIconData& Metadata::customIcon(const QUuid& uuid) const { - return m_customIcons.value(uuid); + auto icon = m_customIcons.find(uuid); + Q_ASSERT(icon != m_customIcons.end()); + if (icon == m_customIcons.end()) { + return NULL_ICON; + } + return icon.value(); } bool Metadata::hasCustomIcon(const QUuid& uuid) const @@ -339,30 +347,40 @@ void Metadata::setProtectNotes(bool value) set(m_data.protectNotes, value); } -void Metadata::addCustomIcon(const QUuid& uuid, const QByteArray& iconData) +void Metadata::addCustomIcon(const QUuid& uuid, const CustomIconData& iconData) { + Q_ASSERT(!uuid.isNull()); Q_ASSERT(!m_customIcons.contains(uuid)); - m_customIcons[uuid] = iconData; // remove all uuids to prevent duplicates in release mode + m_customIcons[uuid] = iconData; m_customIconsOrder.removeAll(uuid); m_customIconsOrder.append(uuid); + // Associate image hash to uuid - QByteArray hash = hashIcon(iconData); + QByteArray hash = hashIcon(iconData.data); m_customIconsHashes[hash] = uuid; Q_ASSERT(m_customIcons.count() == m_customIconsOrder.count()); emitModified(); } +void Metadata::addCustomIcon(const QUuid& uuid, + const QByteArray& iconBytes, + const QString& name, + const QDateTime& lastModified) +{ + addCustomIcon(uuid, {iconBytes, name, lastModified}); +} + void Metadata::removeCustomIcon(const QUuid& uuid) { Q_ASSERT(!uuid.isNull()); Q_ASSERT(m_customIcons.contains(uuid)); // Remove hash record only if this is the same uuid - QByteArray hash = hashIcon(m_customIcons[uuid]); + QByteArray hash = hashIcon(m_customIcons[uuid].data); if (m_customIconsHashes.contains(hash) && m_customIconsHashes[hash] == uuid) { m_customIconsHashes.remove(hash); } @@ -370,6 +388,7 @@ void Metadata::removeCustomIcon(const QUuid& uuid) m_customIcons.remove(uuid); m_customIconsOrder.removeAll(uuid); Q_ASSERT(m_customIcons.count() == m_customIconsOrder.count()); + dynamic_cast(parent())->addDeletedObject(uuid); emitModified(); } diff --git a/src/core/Metadata.h b/src/core/Metadata.h index 1b87ec2ad..61c9c1e6e 100644 --- a/src/core/Metadata.h +++ b/src/core/Metadata.h @@ -61,6 +61,19 @@ public: bool protectNotes; }; + struct CustomIconData + { + QByteArray data; + QString name; + QDateTime lastModified; + + bool operator==(const CustomIconData& rhs) const + { + // Compare only actual icon data + return data == rhs.data; + } + }; + void init(); void clear(); @@ -79,7 +92,7 @@ public: bool protectPassword() const; bool protectUrl() const; bool protectNotes() const; - QByteArray customIcon(const QUuid& uuid) const; + const CustomIconData& customIcon(const QUuid& uuid) const; bool hasCustomIcon(const QUuid& uuid) const; QList customIconsOrder() const; bool recycleBinEnabled() const; @@ -116,7 +129,11 @@ public: void setProtectPassword(bool value); void setProtectUrl(bool value); void setProtectNotes(bool value); - void addCustomIcon(const QUuid& uuid, const QByteArray& iconData); + void addCustomIcon(const QUuid& uuid, const CustomIconData& iconData); + void addCustomIcon(const QUuid& uuid, + const QByteArray& iconBytes, + const QString& name = {}, + const QDateTime& lastModified = {}); void removeCustomIcon(const QUuid& uuid); void copyCustomIcons(const QSet& iconList, const Metadata* otherMetadata); QUuid findCustomIcon(const QByteArray& candidate); @@ -152,7 +169,7 @@ private: MetadataData m_data; QList m_customIconsOrder; - QHash m_customIcons; + QHash m_customIcons; QHash m_customIconsHashes; QPointer m_recycleBin; diff --git a/src/format/KdbxXmlReader.cpp b/src/format/KdbxXmlReader.cpp index 5319323d2..476d33726 100644 --- a/src/format/KdbxXmlReader.cpp +++ b/src/format/KdbxXmlReader.cpp @@ -347,6 +347,8 @@ void KdbxXmlReader::parseIcon() QUuid uuid; QByteArray iconData; + QString name; + QDateTime lastModified; bool uuidSet = false; bool iconSet = false; @@ -357,6 +359,10 @@ void KdbxXmlReader::parseIcon() } else if (m_xml.name() == "Data") { iconData = readBinary(); iconSet = true; + } else if (m_xml.name() == "Name") { + name = readString(); + } else if (m_xml.name() == "LastModificationTime") { + lastModified = readDateTime(); } else { skipCurrentElement(); } @@ -367,7 +373,7 @@ void KdbxXmlReader::parseIcon() if (m_meta->hasCustomIcon(uuid)) { uuid = QUuid::createUuid(); } - m_meta->addCustomIcon(uuid, iconData); + m_meta->addCustomIcon(uuid, iconData, name, lastModified); return; } diff --git a/src/format/KdbxXmlWriter.cpp b/src/format/KdbxXmlWriter.cpp index 37f7a7b26..5b0fc3eab 100644 --- a/src/format/KdbxXmlWriter.cpp +++ b/src/format/KdbxXmlWriter.cpp @@ -21,7 +21,6 @@ #include #include "core/Endian.h" -#include "core/Metadata.h" #include "format/KeePass2RandomStream.h" #include "streams/qtiocompressor.h" @@ -162,12 +161,20 @@ void KdbxXmlWriter::writeCustomIcons() m_xml.writeEndElement(); } -void KdbxXmlWriter::writeIcon(const QUuid& uuid, const QByteArray& iconData) +void KdbxXmlWriter::writeIcon(const QUuid& uuid, const Metadata::CustomIconData& iconData) { m_xml.writeStartElement("Icon"); writeUuid("UUID", uuid); - writeBinary("Data", iconData); + if (m_kdbxVersion >= KeePass2::FILE_VERSION_4) { + if (!iconData.name.isEmpty()) { + writeString("Name", iconData.name); + } + if (iconData.lastModified.isValid()) { + writeDateTime("LastModificationTime", iconData.lastModified); + } + } + writeBinary("Data", iconData.data); m_xml.writeEndElement(); } diff --git a/src/format/KdbxXmlWriter.h b/src/format/KdbxXmlWriter.h index e8dac5bee..2f9dc9aca 100644 --- a/src/format/KdbxXmlWriter.h +++ b/src/format/KdbxXmlWriter.h @@ -22,9 +22,9 @@ #include #include "core/Group.h" +#include "core/Metadata.h" class KeePass2RandomStream; -class Metadata; class KdbxXmlWriter { @@ -47,7 +47,7 @@ private: void writeMetadata(); void writeMemoryProtection(); void writeCustomIcons(); - void writeIcon(const QUuid& uuid, const QByteArray& iconData); + void writeIcon(const QUuid& uuid, const Metadata::CustomIconData& iconData); void writeBinaries(); void writeCustomData(const CustomData* customData); void writeCustomDataItem(const QString& key, const QString& value); diff --git a/src/gui/EditWidgetIcons.cpp b/src/gui/EditWidgetIcons.cpp index c301728b6..82af65f95 100644 --- a/src/gui/EditWidgetIcons.cpp +++ b/src/gui/EditWidgetIcons.cpp @@ -19,6 +19,7 @@ #include "EditWidgetIcons.h" #include "ui_EditWidgetIcons.h" +#include "core/Clock.h" #include "core/Config.h" #include "core/Database.h" #include "core/Metadata.h" @@ -244,7 +245,7 @@ void EditWidgetIcons::addCustomIconFromFile() auto icon = QImage(filename); if (icon.isNull()) { errornames << filename; - } else if (!addCustomIcon(icon)) { + } else if (!addCustomIcon(icon, QFileInfo(filename).baseName())) { // Icon already exists in database ++numexisting; } @@ -281,7 +282,7 @@ void EditWidgetIcons::addCustomIconFromFile() } } -bool EditWidgetIcons::addCustomIcon(const QImage& icon) +bool EditWidgetIcons::addCustomIcon(const QImage& icon, const QString& name) { bool added = false; if (m_db) { @@ -295,7 +296,7 @@ bool EditWidgetIcons::addCustomIcon(const QImage& icon) QUuid uuid = m_db->metadata()->findCustomIcon(serializedIcon); if (uuid.isNull()) { uuid = QUuid::createUuid(); - m_db->metadata()->addCustomIcon(uuid, serializedIcon); + m_db->metadata()->addCustomIcon(uuid, serializedIcon, name, Clock::currentDateTimeUtc()); m_customIconModel->setIcons(Icons::customIconsPixmaps(m_db.data(), IconSize::Default), m_db->metadata()->customIconsOrder()); added = true; diff --git a/src/gui/EditWidgetIcons.h b/src/gui/EditWidgetIcons.h index e0e662848..6e134ced6 100644 --- a/src/gui/EditWidgetIcons.h +++ b/src/gui/EditWidgetIcons.h @@ -85,7 +85,7 @@ private slots: void downloadFavicon(); void iconReceived(const QString& url, const QImage& icon); void addCustomIconFromFile(); - bool addCustomIcon(const QImage& icon); + bool addCustomIcon(const QImage& icon, const QString& name = {}); void updateWidgetsDefaultIcons(bool checked); void updateWidgetsCustomIcons(bool checked); void updateRadioButtonDefaultIcons(); diff --git a/src/gui/Icons.cpp b/src/gui/Icons.cpp index 95e6aadc4..73e48154d 100644 --- a/src/gui/Icons.cpp +++ b/src/gui/Icons.cpp @@ -222,7 +222,7 @@ QPixmap Icons::customIconPixmap(const Database* db, const QUuid& uuid, IconSize return {}; } // Generate QIcon with pre-baked resolutions - auto icon = QImage::fromData(db->metadata()->customIcon(uuid)); + auto icon = QImage::fromData(db->metadata()->customIcon(uuid).data); auto basePixmap = QPixmap::fromImage(icon.scaled(64, 64, Qt::IgnoreAspectRatio, Qt::SmoothTransformation)); return QIcon(basePixmap).pixmap(databaseIcons()->iconSize(size)); } diff --git a/src/gui/group/GroupModel.cpp b/src/gui/group/GroupModel.cpp index c21d1f911..57ea56235 100644 --- a/src/gui/group/GroupModel.cpp +++ b/src/gui/group/GroupModel.cpp @@ -300,7 +300,7 @@ bool GroupModel::dropMimeData(const QMimeData* data, if (sourceDb != targetDb) { QUuid customIcon = entry->iconUuid(); if (!customIcon.isNull() && !targetDb->metadata()->hasCustomIcon(customIcon)) { - targetDb->metadata()->addCustomIcon(customIcon, sourceDb->metadata()->customIcon(customIcon)); + targetDb->metadata()->addCustomIcon(customIcon, sourceDb->metadata()->customIcon(customIcon).data); } // Reset the UUID when moving across db boundary diff --git a/tests/TestDatabase.cpp b/tests/TestDatabase.cpp index 8a05fcbfa..0bcf2c6f6 100644 --- a/tests/TestDatabase.cpp +++ b/tests/TestDatabase.cpp @@ -28,7 +28,6 @@ #include "core/Tools.h" #include "crypto/Crypto.h" #include "format/KeePass2Writer.h" -#include "keys/PasswordKey.h" #include "util/TemporaryFile.h" QTEST_GUILESS_MAIN(TestDatabase) @@ -214,3 +213,26 @@ void TestDatabase::testEmptyRecycleBinWithHierarchicalData() writer.writeDatabase(&afterCleanup, db.data()); QVERIFY(afterCleanup.size() < initialSize); } + +void TestDatabase::testCustomIcons() +{ + Database db; + + QUuid uuid1 = QUuid::createUuid(); + QByteArray icon1("icon 1"); + Q_ASSERT(!icon1.isNull()); + db.metadata()->addCustomIcon(uuid1, icon1); + Metadata::CustomIconData iconData = db.metadata()->customIcon(uuid1); + QCOMPARE(iconData.data, icon1); + QVERIFY(iconData.name.isNull()); + QVERIFY(iconData.lastModified.isNull()); + + QUuid uuid2 = QUuid::createUuid(); + QByteArray icon2("icon 2"); + QDateTime date = QDateTime::currentDateTimeUtc(); + db.metadata()->addCustomIcon(uuid2, icon2, "Test", date); + iconData = db.metadata()->customIcon(uuid2); + QCOMPARE(iconData.data, icon2); + QCOMPARE(iconData.name, QString("Test")); + QCOMPARE(iconData.lastModified, date); +} diff --git a/tests/TestDatabase.h b/tests/TestDatabase.h index dc377ef05..511703849 100644 --- a/tests/TestDatabase.h +++ b/tests/TestDatabase.h @@ -34,6 +34,7 @@ private slots: void testEmptyRecycleBinOnNotCreated(); void testEmptyRecycleBinOnEmpty(); void testEmptyRecycleBinWithHierarchicalData(); + void testCustomIcons(); }; #endif // KEEPASSX_TESTDATABASE_H diff --git a/tests/TestDeletedObjects.cpp b/tests/TestDeletedObjects.cpp index b86902efc..986ad6925 100644 --- a/tests/TestDeletedObjects.cpp +++ b/tests/TestDeletedObjects.cpp @@ -152,3 +152,15 @@ void TestDeletedObjects::testDatabaseChange() delete group; } + +void TestDeletedObjects::testCustomIconDeletion() +{ + Database db; + QCOMPARE(db.deletedObjects().size(), 0); + + QUuid uuid = QUuid::createUuid(); + db.metadata()->addCustomIcon(uuid, QByteArray()); + db.metadata()->removeCustomIcon(uuid); + QCOMPARE(db.deletedObjects().size(), 1); + QCOMPARE(db.deletedObjects().at(0).uuid, uuid); +} diff --git a/tests/TestDeletedObjects.h b/tests/TestDeletedObjects.h index 61d864a67..862b34636 100644 --- a/tests/TestDeletedObjects.h +++ b/tests/TestDeletedObjects.h @@ -34,6 +34,7 @@ private slots: void testDeletedObjectsFromFile(); void testDeletedObjectsFromNewDb(); void testDatabaseChange(); + void testCustomIconDeletion(); }; #endif // KEEPASSX_TESTDELETEDOBJECTS_H diff --git a/tests/TestGroup.cpp b/tests/TestGroup.cpp index 8296140d5..95f79de27 100644 --- a/tests/TestGroup.cpp +++ b/tests/TestGroup.cpp @@ -316,33 +316,37 @@ void TestGroup::testCopyCustomIcon() QUuid groupIconUuid = QUuid::createUuid(); QByteArray groupIcon("group icon"); - dbSource->metadata()->addCustomIcon(groupIconUuid, groupIcon); + QString groupIconName("group icon"); + dbSource->metadata()->addCustomIcon(groupIconUuid, groupIcon, groupIconName); QUuid entryIconUuid = QUuid::createUuid(); QByteArray entryIcon("entry icon"); - dbSource->metadata()->addCustomIcon(entryIconUuid, entryIcon); + QString entryIconName("entry icon"); + dbSource->metadata()->addCustomIcon(entryIconUuid, entryIcon, entryIconName); - Group* group = new Group(); + auto* group = new Group(); group->setParent(dbSource->rootGroup()); group->setIcon(groupIconUuid); - QCOMPARE(group->database()->metadata()->customIcon(groupIconUuid), groupIcon); + QCOMPARE(group->database()->metadata()->customIcon(groupIconUuid).data, groupIcon); + QCOMPARE(group->database()->metadata()->customIcon(groupIconUuid).name, groupIconName); - Entry* entry = new Entry(); + auto* entry = new Entry(); entry->setGroup(dbSource->rootGroup()); entry->setIcon(entryIconUuid); - QCOMPARE(entry->database()->metadata()->customIcon(entryIconUuid), entryIcon); + QCOMPARE(entry->database()->metadata()->customIcon(entryIconUuid).data, entryIcon); + QCOMPARE(entry->database()->metadata()->customIcon(entryIconUuid).name, entryIconName); QScopedPointer dbTarget(new Database()); group->setParent(dbTarget->rootGroup()); QVERIFY(dbTarget->metadata()->hasCustomIcon(groupIconUuid)); - QCOMPARE(dbTarget->metadata()->customIcon(groupIconUuid), groupIcon); - QCOMPARE(group->database()->metadata()->customIcon(groupIconUuid), groupIcon); + QCOMPARE(dbTarget->metadata()->customIcon(groupIconUuid).data, groupIcon); + QCOMPARE(dbTarget->metadata()->customIcon(groupIconUuid).name, groupIconName); entry->setGroup(dbTarget->rootGroup()); QVERIFY(dbTarget->metadata()->hasCustomIcon(entryIconUuid)); - QCOMPARE(dbTarget->metadata()->customIcon(entryIconUuid), entryIcon); - QCOMPARE(entry->database()->metadata()->customIcon(entryIconUuid), entryIcon); + QCOMPARE(dbTarget->metadata()->customIcon(entryIconUuid).data, entryIcon); + QCOMPARE(dbTarget->metadata()->customIcon(entryIconUuid).name, entryIconName); } void TestGroup::testClone() @@ -423,37 +427,36 @@ void TestGroup::testCopyCustomIcons() QScopedPointer dbSource(new Database()); QScopedPointer dbTarget(new Database()); - QByteArray iconImage1("icon 1"); - - QByteArray iconImage2("icon 2"); + Metadata::CustomIconData icon1 = {QByteArray("icon 1"), "icon 1", Clock::currentDateTimeUtc()}; + Metadata::CustomIconData icon2 = {QByteArray("icon 2"), "icon 2", Clock::currentDateTimeUtc()}; QScopedPointer group1(new Group()); group1->setParent(dbSource->rootGroup()); QUuid group1Icon = QUuid::createUuid(); - dbSource->metadata()->addCustomIcon(group1Icon, iconImage1); + dbSource->metadata()->addCustomIcon(group1Icon, icon1); group1->setIcon(group1Icon); QScopedPointer group2(new Group()); group2->setParent(group1.data()); QUuid group2Icon = QUuid::createUuid(); - dbSource->metadata()->addCustomIcon(group2Icon, iconImage1); + dbSource->metadata()->addCustomIcon(group2Icon, icon1); group2->setIcon(group2Icon); QScopedPointer entry1(new Entry()); entry1->setGroup(group2.data()); QUuid entry1IconOld = QUuid::createUuid(); - dbSource->metadata()->addCustomIcon(entry1IconOld, iconImage1); + dbSource->metadata()->addCustomIcon(entry1IconOld, icon1); entry1->setIcon(entry1IconOld); // add history item entry1->beginUpdate(); QUuid entry1IconNew = QUuid::createUuid(); - dbSource->metadata()->addCustomIcon(entry1IconNew, iconImage1); + dbSource->metadata()->addCustomIcon(entry1IconNew, icon1); entry1->setIcon(entry1IconNew); entry1->endUpdate(); // test that we don't overwrite icons - dbTarget->metadata()->addCustomIcon(group2Icon, iconImage2); + dbTarget->metadata()->addCustomIcon(group2Icon, icon1); dbTarget->metadata()->copyCustomIcons(group1->customIconsRecursive(), dbSource->metadata()); @@ -465,8 +468,8 @@ void TestGroup::testCopyCustomIcons() QVERIFY(metaTarget->hasCustomIcon(entry1IconOld)); QVERIFY(metaTarget->hasCustomIcon(entry1IconNew)); - QCOMPARE(metaTarget->customIcon(group1Icon), iconImage1); - QCOMPARE(metaTarget->customIcon(group2Icon), iconImage2); + QCOMPARE(metaTarget->customIcon(group1Icon), icon1); + QCOMPARE(metaTarget->customIcon(group2Icon), icon1); } void TestGroup::testFindEntry() @@ -1082,7 +1085,7 @@ void TestGroup::testApplyGroupIconRecursively() QCOMPARE(subgroupEntry->iconUuid(), subgroupIconUuid); QCOMPARE(subsubgroup->iconUuid(), subgroupIconUuid); QCOMPARE(subsubgroupEntry->iconUuid(), subgroupIconUuid); - QCOMPARE(subgroup->database()->metadata()->customIcon(subgroupIconUuid), subgroupIcon); + QCOMPARE(subgroup->database()->metadata()->customIcon(subgroupIconUuid).data, subgroupIcon); // Reset all icons to root icon database.rootGroup()->setIcon(rootIconNumber); diff --git a/tests/TestKeePass1Reader.cpp b/tests/TestKeePass1Reader.cpp index d7a04bdbb..a3676a460 100644 --- a/tests/TestKeePass1Reader.cpp +++ b/tests/TestKeePass1Reader.cpp @@ -116,7 +116,7 @@ void TestKeePass1Reader::testCustomIcons() QCOMPARE(m_db->metadata()->customIconsOrder().size(), 1); QUuid uuid = m_db->metadata()->customIconsOrder().at(0); QVERIFY(m_db->metadata()->hasCustomIcon(uuid)); - QByteArray icon = m_db->metadata()->customIcon(uuid); + QByteArray icon = m_db->metadata()->customIcon(uuid).data; QVERIFY(icon.startsWith( "\x89PNG\r\n\x1A\n\x00\x00\x00\rIHDR\x00\x00\x00\x10\x00\x00\x00\x10\b\x06\x00\x00\x00\x1F\xF3\xFF")); diff --git a/tests/TestKeePass2Format.cpp b/tests/TestKeePass2Format.cpp index 397c8bd1c..8829af0b7 100644 --- a/tests/TestKeePass2Format.cpp +++ b/tests/TestKeePass2Format.cpp @@ -113,7 +113,7 @@ void TestKeePass2Format::testXmlCustomIcons() QCOMPARE(m_xmlDb->metadata()->customIconsOrder().size(), 1); QUuid uuid = QUuid::fromRfc4122(QByteArray::fromBase64("++vyI+daLk6omox4a6kQGA==")); QVERIFY(m_xmlDb->metadata()->hasCustomIcon(uuid)); - QByteArray icon = m_xmlDb->metadata()->customIcon(uuid); + QByteArray icon = m_xmlDb->metadata()->customIcon(uuid).data; QVERIFY(icon.startsWith( "\x89PNG\r\n\x1A\n\x00\x00\x00\rIHDR\x00\x00\x00\x10\x00\x00\x00\x10\b\x06\x00\x00\x00\x1F\xF3\xFF")); diff --git a/tests/TestMerge.cpp b/tests/TestMerge.cpp index 7fc1d1dc8..014f39f41 100644 --- a/tests/TestMerge.cpp +++ b/tests/TestMerge.cpp @@ -1137,8 +1137,8 @@ void TestMerge::testMergeDuplicateCustomIcons() QUuid customIconId = QUuid::createUuid(); - QByteArray customIcon1 = QString("custom icon 1").toLocal8Bit(); - QByteArray customIcon2 = QString("custom icon 2").toLocal8Bit(); + QByteArray customIcon1("custom icon 1"); + QByteArray customIcon2("custom icon 2"); dbSource->metadata()->addCustomIcon(customIconId, customIcon1); dbDestination->metadata()->addCustomIcon(customIconId, customIcon2); @@ -1153,7 +1153,7 @@ void TestMerge::testMergeDuplicateCustomIcons() QVERIFY(dbDestination->metadata()->hasCustomIcon(customIconId)); QCOMPARE(dbDestination->metadata()->customIconsOrder().count(), 1); - QCOMPARE(dbDestination->metadata()->customIcon(customIconId), customIcon2); + QCOMPARE(dbDestination->metadata()->customIcon(customIconId).data, customIcon2); } void TestMerge::testMetadata()