Skip to content

Commit

Permalink
Prevent new entry loss on database file reload
Browse files Browse the repository at this point in the history
* Fix keepassxreboot#3651

* Correct data loss when the database reloads due to a file change while creating a new entry. The issue occurred due to the "new parent group" pointer being invalid after the database is reloaded following merge.

* Also fix re-selecting entries following database file reload. If the entry was moved out of the current group it would result in an assert hit. This fix prevents recursively looking for the entry.
  • Loading branch information
droidmonkey committed Oct 24, 2019
1 parent b8830df commit af263fd
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 4 deletions.
9 changes: 7 additions & 2 deletions src/core/Group.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -585,13 +585,18 @@ QList<Entry*> Group::referencesRecursive(const Entry* entry) const
[entry](const Entry* e) { return e->hasReferencesTo(entry->uuid()); });
}

Entry* Group::findEntryByUuid(const QUuid& uuid) const
Entry* Group::findEntryByUuid(const QUuid& uuid, bool recursive) const
{
if (uuid.isNull()) {
return nullptr;
}

for (Entry* entry : entriesRecursive(false)) {
auto entries = m_entries;
if (recursive) {
entries = entriesRecursive(false);
}

for (auto entry : entries) {
if (entry->uuid() == uuid) {
return entry;
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/Group.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class Group : public QObject
static const QString RootAutoTypeSequence;

Group* findChildByName(const QString& name);
Entry* findEntryByUuid(const QUuid& uuid) const;
Entry* findEntryByUuid(const QUuid& uuid, bool recursive = true) const;
Entry* findEntryByPath(const QString& entryPath);
Entry* findEntryBySearchTerm(const QString& term, EntryReferenceType referenceType);
Group* findGroupByUuid(const QUuid& uuid);
Expand Down
17 changes: 16 additions & 1 deletion src/gui/DatabaseWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,12 @@ void DatabaseWidget::createEntry()

void DatabaseWidget::replaceDatabase(QSharedPointer<Database> db)
{
// Save off new parent UUID which will be valid when creating a new entry
QUuid newParentUuid;
if (m_newParent) {
newParentUuid = m_newParent->uuid();
}

// TODO: instead of increasing the ref count temporarily, there should be a clean
// break from the old database. Without this crashes occur due to the change
// signals triggering dangling pointers.
Expand All @@ -390,6 +396,15 @@ void DatabaseWidget::replaceDatabase(QSharedPointer<Database> db)
m_groupView->changeDatabase(m_db);
processAutoOpen();

// Restore the new parent group pointer, if not found default to the root group
// this prevents data loss when merging a database while creating a new entry
if (!newParentUuid.isNull()) {
m_newParent = m_db->rootGroup()->findGroupByUuid(newParentUuid);
if (!m_newParent) {
m_newParent = m_db->rootGroup();
}
}

emit databaseReplaced(oldDb, m_db);

#if defined(WITH_XC_KEESHARE)
Expand Down Expand Up @@ -1480,7 +1495,7 @@ void DatabaseWidget::restoreGroupEntryFocus(const QUuid& groupUuid, const QUuid&
auto group = m_db->rootGroup()->findGroupByUuid(groupUuid);
if (group) {
m_groupView->setCurrentGroup(group);
auto entry = group->findEntryByUuid(entryUuid);
auto entry = group->findEntryByUuid(entryUuid, false);
if (entry) {
m_entryView->setCurrentEntry(entry);
}
Expand Down

0 comments on commit af263fd

Please sign in to comment.