From 60c2d89cb02f5821e87906e5cbf1ac1582f5e613 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Sun, 13 Dec 2020 11:23:39 -0500 Subject: [PATCH] Prevent crash when KeeShare merges an entry that is in edit mode * Hack for #5722 until a refactor of KeeShare, Merger, and EditEntryWidget can be performed. This hack should only ever be triggered on the rare occurrence of two people editing the same entry at the same time. The end result is potential data loss, but the current result is a hard crash. Unfortunately the way everything is interfaced currently doesn't afford any solution without a major refactor. * Additionally add a short delay before actually reloading a share to prevent read/write locks from preventing proper import. This delay also prevents conflicting saves between the main database and the KeeShare database. This should eventually be moved into the FileObserver itself to smooth out all merge operations once the above refactor occurs. Side note: KeeShare operates independently of DatabaseWidget causing unexpected behavior when files are updated/merged/etc. This needs to be corrected in a refactor. --- src/gui/entry/EditEntryWidget.cpp | 9 ++++++++ src/keeshare/ShareObserver.cpp | 38 ++++++++++++++++++------------- src/keeshare/ShareObserver.h | 1 + 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp index 718a08a84..123bb9d48 100644 --- a/src/gui/entry/EditEntryWidget.cpp +++ b/src/gui/entry/EditEntryWidget.cpp @@ -994,6 +994,15 @@ bool EditEntryWidget::commitEntry() return true; } + // HACK: Check that entry pointer is still valid, see https://github.com/keepassxreboot/keepassxc/issues/5722 + if (!m_entry) { + QMessageBox::information(this, + tr("Invalid Entry"), + tr("An external merge operation has invalidated this entry.\n" + "Unfortunately, any changes made have been lost.")); + return true; + } + // Check Auto-Type validity early if (!AutoType::verifyAutoTypeSyntax(m_autoTypeUi->sequenceEdit->text())) { return false; diff --git a/src/keeshare/ShareObserver.cpp b/src/keeshare/ShareObserver.cpp index 6fd629b4c..f74e800a0 100644 --- a/src/keeshare/ShareObserver.cpp +++ b/src/keeshare/ShareObserver.cpp @@ -186,23 +186,29 @@ void ShareObserver::handleDatabaseChanged() void ShareObserver::handleFileUpdated(const QString& path) { - const Result result = importShare(path); - if (!result.isValid()) { - return; + if (!m_inFileUpdate) { + QTimer::singleShot(100, this, [this, path] { + const Result result = importShare(path); + m_inFileUpdate = false; + if (!result.isValid()) { + return; + } + QStringList success; + QStringList warning; + QStringList error; + if (result.isError()) { + error << tr("Import from %1 failed (%2)").arg(result.path, result.message); + } else if (result.isWarning()) { + warning << tr("Import from %1 failed (%2)").arg(result.path, result.message); + } else if (result.isInfo()) { + success << tr("Import from %1 successful (%2)").arg(result.path, result.message); + } else { + success << tr("Imported from %1").arg(result.path); + } + notifyAbout(success, warning, error); + }); + m_inFileUpdate = true; } - QStringList success; - QStringList warning; - QStringList error; - if (result.isError()) { - error << tr("Import from %1 failed (%2)").arg(result.path, result.message); - } else if (result.isWarning()) { - warning << tr("Import from %1 failed (%2)").arg(result.path, result.message); - } else if (result.isInfo()) { - success << tr("Import from %1 successful (%2)").arg(result.path, result.message); - } else { - success << tr("Imported from %1").arg(result.path); - } - notifyAbout(success, warning, error); } ShareObserver::Result ShareObserver::importShare(const QString& path) diff --git a/src/keeshare/ShareObserver.h b/src/keeshare/ShareObserver.h index b98d58981..8b881142d 100644 --- a/src/keeshare/ShareObserver.h +++ b/src/keeshare/ShareObserver.h @@ -83,6 +83,7 @@ private: QMap, KeeShareSettings::Reference> m_groupToReference; QMap> m_shareToGroup; QMap> m_fileWatchers; + bool m_inFileUpdate = false; }; #endif // KEEPASSXC_SHAREOBSERVER_H