diff --git a/src/cli/Create.cpp b/src/cli/Create.cpp index c8e3b771..68289bf2 100644 --- a/src/cli/Create.cpp +++ b/src/cli/Create.cpp @@ -121,6 +121,7 @@ int Create::execute(const QStringList& arguments) QSharedPointer db(new Database); db->setKey(key); + db->setInitialized(true); if (decryptionTime != 0) { auto kdf = db->kdf(); diff --git a/src/cli/Import.cpp b/src/cli/Import.cpp index dd7b12c6..e34a4ceb 100644 --- a/src/cli/Import.cpp +++ b/src/cli/Import.cpp @@ -84,6 +84,7 @@ int Import::execute(const QStringList& arguments) Database db; db.setKdf(KeePass2::uuidToKdf(KeePass2::KDF_ARGON2)); db.setKey(key); + db.setInitialized(true); if (!db.import(xmlExportPath, &errorMessage)) { errorTextStream << QObject::tr("Unable to import XML database: %1").arg(errorMessage) << endl; diff --git a/src/core/Database.cpp b/src/core/Database.cpp index fcd48f1e..a3637bb5 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -18,6 +18,7 @@ #include "Database.h" +#include "core/AsyncTask.h" #include "core/Clock.h" #include "core/FileWatcher.h" #include "core/Group.h" @@ -159,6 +160,15 @@ bool Database::open(const QString& filePath, QSharedPointer return true; } +bool Database::isSaving() +{ + bool locked = m_saveMutex.tryLock(); + if (locked) { + m_saveMutex.unlock(); + } + return !locked; +} + /** * Save the database to the current file path. It is an error to call this function * if no file path has been defined. @@ -201,6 +211,25 @@ bool Database::save(QString* error, bool atomic, bool backup) */ bool Database::saveAs(const QString& filePath, QString* error, bool atomic, bool backup) { + // Disallow overlapping save operations + if (isSaving()) { + if (error) { + *error = tr("Database save is already in progress."); + } + return false; + } + + // Never save an uninitialized database + if (!m_initialized) { + if (error) { + *error = tr("Could not save, database has not been initialized!"); + } + return false; + } + + // Prevent destructive operations while saving + QMutexLocker locker(&m_saveMutex); + if (filePath == m_data.filePath) { // Disallow saving to the same file if read-only if (m_data.isReadOnly) { @@ -226,7 +255,7 @@ bool Database::saveAs(const QString& filePath, QString* error, bool atomic, bool m_fileWatcher->stop(); auto& canonicalFilePath = QFileInfo::exists(filePath) ? QFileInfo(filePath).canonicalFilePath() : filePath; - bool ok = performSave(canonicalFilePath, error, atomic, backup); + bool ok = AsyncTask::runAndWaitForFuture([&] { return performSave(canonicalFilePath, error, atomic, backup); }); if (ok) { markAsClean(); setFilePath(filePath); @@ -389,6 +418,9 @@ bool Database::import(const QString& xmlExportPath, QString* error) void Database::releaseData() { + // Prevent data release while saving + QMutexLocker locker(&m_saveMutex); + s_uuidMap.remove(m_uuid); m_uuid = QUuid(); @@ -397,22 +429,20 @@ void Database::releaseData() } m_data.clear(); + m_metadata->clear(); if (m_rootGroup && m_rootGroup->parent() == this) { delete m_rootGroup; } - if (m_metadata) { - delete m_metadata; - } - if (m_fileWatcher) { - delete m_fileWatcher; - } + + m_fileWatcher->stop(); m_deletedObjects.clear(); m_commonUsernames.clear(); m_initialized = false; m_modified = false; + m_modifiedTimer.stop(); } /** diff --git a/src/core/Database.h b/src/core/Database.h index d3d88e7d..74024424 100644 --- a/src/core/Database.h +++ b/src/core/Database.h @@ -21,6 +21,7 @@ #include #include +#include #include #include #include @@ -85,6 +86,7 @@ public: void setEmitModified(bool value); bool isReadOnly() const; void setReadOnly(bool readOnly); + bool isSaving(); QUuid uuid() const; QString filePath() const; @@ -208,6 +210,7 @@ private: QPointer m_rootGroup; QList m_deletedObjects; QTimer m_modifiedTimer; + QMutex m_saveMutex; QPointer m_fileWatcher; bool m_initialized = false; bool m_modified = false; diff --git a/src/core/FileWatcher.cpp b/src/core/FileWatcher.cpp index 0bc5e344..0619a3cb 100644 --- a/src/core/FileWatcher.cpp +++ b/src/core/FileWatcher.cpp @@ -43,6 +43,11 @@ FileWatcher::FileWatcher(QObject* parent) m_fileIgnoreDelayTimer.setSingleShot(true); } +FileWatcher::~FileWatcher() +{ + stop(); +} + void FileWatcher::start(const QString& filePath, int checksumIntervalSeconds, int checksumSizeKibibytes) { stop(); @@ -120,8 +125,7 @@ void FileWatcher::checkFileChanged() // Prevent reentrance m_ignoreFileChange = true; - // Only trigger the change notice if there is a checksum mismatch - auto checksum = calculateChecksum(); + auto checksum = AsyncTask::runAndWaitForFuture([this]() -> QByteArray { return calculateChecksum(); }); if (checksum != m_fileChecksum) { m_fileChecksum = checksum; m_fileChangeDelayTimer.start(0); @@ -132,21 +136,19 @@ void FileWatcher::checkFileChanged() QByteArray FileWatcher::calculateChecksum() { - return AsyncTask::runAndWaitForFuture([this]() -> QByteArray { - QFile file(m_filePath); - if (file.open(QFile::ReadOnly)) { - QCryptographicHash hash(QCryptographicHash::Sha256); - if (m_fileChecksumSizeBytes > 0) { - hash.addData(file.read(m_fileChecksumSizeBytes)); - } else { - hash.addData(&file); - } - return hash.result(); + QFile file(m_filePath); + if (file.open(QFile::ReadOnly)) { + QCryptographicHash hash(QCryptographicHash::Sha256); + if (m_fileChecksumSizeBytes > 0) { + hash.addData(file.read(m_fileChecksumSizeBytes)); + } else { + hash.addData(&file); } - // If we fail to open the file return the last known checksum, this - // prevents unnecessary merge requests on intermittent network shares - return m_fileChecksum; - }); + return hash.result(); + } + // If we fail to open the file return the last known checksum, this + // prevents unnecessary merge requests on intermittent network shares + return m_fileChecksum; } BulkFileWatcher::BulkFileWatcher(QObject* parent) diff --git a/src/core/FileWatcher.h b/src/core/FileWatcher.h index 9b55badc..7a908536 100644 --- a/src/core/FileWatcher.h +++ b/src/core/FileWatcher.h @@ -29,6 +29,7 @@ class FileWatcher : public QObject public: explicit FileWatcher(QObject* parent = nullptr); + ~FileWatcher() override; void start(const QString& path, int checksumIntervalSeconds = 0, int checksumSizeKibibytes = -1); void stop(); diff --git a/src/core/Metadata.cpp b/src/core/Metadata.cpp index fb18f711..200fef89 100644 --- a/src/core/Metadata.cpp +++ b/src/core/Metadata.cpp @@ -31,7 +31,13 @@ Metadata::Metadata(QObject* parent) , m_customData(new CustomData(this)) , m_updateDatetime(true) { - m_data.generator = "KeePassXC"; + init(); + connect(m_customData, SIGNAL(customDataModified()), SIGNAL(metadataModified())); +} + +void Metadata::init() +{ + m_data.generator = QStringLiteral("KeePassXC"); m_data.maintenanceHistoryDays = 365; m_data.masterKeyChangeRec = -1; m_data.masterKeyChangeForce = -1; @@ -52,8 +58,17 @@ Metadata::Metadata(QObject* parent) m_entryTemplatesGroupChanged = now; m_masterKeyChanged = now; m_settingsChanged = now; +} - connect(m_customData, SIGNAL(customDataModified()), this, SIGNAL(metadataModified())); +void Metadata::clear() +{ + init(); + m_customIcons.clear(); + m_customIconCacheKeys.clear(); + m_customIconScaledCacheKeys.clear(); + m_customIconsOrder.clear(); + m_customIconsHashes.clear(); + m_customData->clear(); } template bool Metadata::set(P& property, const V& value) diff --git a/src/core/Metadata.h b/src/core/Metadata.h index b39dafaf..85fb2fdd 100644 --- a/src/core/Metadata.h +++ b/src/core/Metadata.h @@ -62,6 +62,9 @@ public: bool protectNotes; }; + void init(); + void clear(); + QString generator() const; QString name() const; QDateTime nameChanged() const; diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp index 5523f7c6..b0ced46d 100644 --- a/src/gui/DatabaseTabWidget.cpp +++ b/src/gui/DatabaseTabWidget.cpp @@ -353,13 +353,17 @@ bool DatabaseTabWidget::closeDatabaseTab(DatabaseWidget* dbWidget) */ bool DatabaseTabWidget::closeAllDatabaseTabs() { - while (count() > 0) { - if (!closeDatabaseTab(0)) { - return false; + // Attempt to lock all databases first to prevent closing only a portion of tabs + if (lockDatabases()) { + while (count() > 0) { + if (!closeDatabaseTab(0)) { + return false; + } } + return true; } - return true; + return false; } bool DatabaseTabWidget::saveDatabase(int index) @@ -597,15 +601,26 @@ DatabaseWidget* DatabaseTabWidget::currentDatabaseWidget() return qobject_cast(currentWidget()); } -void DatabaseTabWidget::lockDatabases() +/** + * Attempt to lock all open databases + * + * @return return true if all databases are locked + */ +bool DatabaseTabWidget::lockDatabases() { + int numLocked = 0; for (int i = 0, c = count(); i < c; ++i) { auto dbWidget = databaseWidgetFromIndex(i); - if (dbWidget->lock() && dbWidget->database()->filePath().isEmpty()) { - // If we locked a database without a file close the tab - closeDatabaseTab(dbWidget); + if (dbWidget->lock()) { + ++numLocked; + if (dbWidget->database()->filePath().isEmpty()) { + // If we locked a database without a file close the tab + closeDatabaseTab(dbWidget); + } } } + + return numLocked == count(); } /** diff --git a/src/gui/DatabaseTabWidget.h b/src/gui/DatabaseTabWidget.h index 5b2d3f00..b8cbdbb6 100644 --- a/src/gui/DatabaseTabWidget.h +++ b/src/gui/DatabaseTabWidget.h @@ -71,7 +71,7 @@ public slots: void exportToCsv(); void exportToHtml(); - void lockDatabases(); + bool lockDatabases(); void closeDatabaseFromSender(); void unlockDatabaseInDialog(DatabaseWidget* dbWidget, DatabaseOpenDialog::Intent intent); void unlockDatabaseInDialog(DatabaseWidget* dbWidget, DatabaseOpenDialog::Intent intent, const QString& filePath); diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index 0eb713da..5bda87be 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/src/gui/DatabaseWidget.cpp @@ -255,15 +255,15 @@ QSharedPointer DatabaseWidget::database() const DatabaseWidget::Mode DatabaseWidget::currentMode() const { if (currentWidget() == nullptr) { - return DatabaseWidget::Mode::None; + return Mode::None; } else if (currentWidget() == m_mainWidget) { - return DatabaseWidget::Mode::ViewMode; + return Mode::ViewMode; } else if (currentWidget() == m_databaseOpenWidget || currentWidget() == m_keepass1OpenWidget) { - return DatabaseWidget::Mode::LockedMode; + return Mode::LockedMode; } else if (currentWidget() == m_csvImportWizard) { - return DatabaseWidget::Mode::ImportMode; + return Mode::ImportMode; } else { - return DatabaseWidget::Mode::EditMode; + return Mode::EditMode; } } @@ -272,6 +272,11 @@ bool DatabaseWidget::isLocked() const return currentMode() == Mode::LockedMode; } +bool DatabaseWidget::isSaving() const +{ + return m_db->isSaving(); +} + bool DatabaseWidget::isSearchActive() const { return m_entryView->inSearchMode(); @@ -1380,6 +1385,12 @@ bool DatabaseWidget::lock() return true; } + // Don't try to lock the database while saving, this will cause a deadlock + if (m_db->isSaving()) { + QTimer::singleShot(200, this, SLOT(lock())); + return false; + } + emit databaseLockRequested(); clipboard()->clearCopiedText(); @@ -1660,7 +1671,6 @@ bool DatabaseWidget::save() auto focusWidget = qApp->focusWidget(); - // TODO: Make this async // Lock out interactions m_entryView->setDisabled(true); m_groupView->setDisabled(true); diff --git a/src/gui/DatabaseWidget.h b/src/gui/DatabaseWidget.h index 6420a3b2..a96a34a9 100644 --- a/src/gui/DatabaseWidget.h +++ b/src/gui/DatabaseWidget.h @@ -80,6 +80,7 @@ public: DatabaseWidget::Mode currentMode() const; bool isLocked() const; + bool isSaving() const; bool isSearchActive() const; bool isEntryEditActive() const; bool isGroupEditActive() const;