diff --git a/CMakeLists.txt b/CMakeLists.txt index 3708667a7c..8137c40d8e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -172,14 +172,13 @@ if(CMAKE_SYSTEM_NAME STREQUAL "Linux") set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} -Wl,-z,relro,-z,now") endif() +add_gcc_compiler_cflags("-std=c99") add_gcc_compiler_cxxflags("-std=c++11") if(APPLE) add_gcc_compiler_cxxflags("-stdlib=libc++") endif() -add_gcc_compiler_cflags("-ansi") - if(WITH_DEV_BUILD) add_definitions(-DQT_DEPRECATED_WARNINGS -DGCRYPT_NO_DEPRECATED) endif() diff --git a/src/core/Config.cpp b/src/core/Config.cpp index 3f49bafefc..328dfaed8a 100644 --- a/src/core/Config.cpp +++ b/src/core/Config.cpp @@ -115,6 +115,7 @@ void Config::init(const QString& fileName) m_defaults.insert("AutoReloadOnChange", true); m_defaults.insert("AutoSaveOnExit", false); m_defaults.insert("BackupBeforeSave", false); + m_defaults.insert("UseAtomicSaves", true); m_defaults.insert("SearchLimitGroup", false); m_defaults.insert("MinimizeOnCopy", false); m_defaults.insert("UseGroupIconOnEntryCreation", false); diff --git a/src/core/Database.cpp b/src/core/Database.cpp index c65c566f96..9a1fe0b3db 100644 --- a/src/core/Database.cpp +++ b/src/core/Database.cpp @@ -19,6 +19,7 @@ #include "Database.h" #include +#include #include #include #include @@ -482,40 +483,92 @@ Database* Database::unlockFromStdin(QString databaseFilename, QString keyFilenam * wrong moment. * * @param filePath Absolute path of the file to save - * @param keepOld Rename the original database file instead of deleting + * @param atomic Use atomic file transactions + * @param backup Backup the existing database file, if exists * @return error string, if any */ -QString Database::saveToFile(QString filePath, bool keepOld) -{ - KeePass2Writer writer; - QTemporaryFile saveFile; - if (saveFile.open()) { - // write the database to the file - setEmitModified(false); - writer.writeDatabase(&saveFile, this); - setEmitModified(true); - - if (writer.hasError()) { - // the writer failed - return writer.errorString(); +QString Database::saveToFile(QString filePath, bool atomic, bool backup) +{ + QString error; + if (atomic) { + QSaveFile saveFile(filePath); + if (saveFile.open(QIODevice::WriteOnly)) { + // write the database to the file + error = writeDatabase(&saveFile); + if (!error.isEmpty()) { + return error; + } + + if (backup) { + backupDatabase(filePath); + } + + if (saveFile.commit()) { + // successfully saved database file + return {}; + } } - - saveFile.close(); // flush to disk - - if (keepOld) { - QFile::remove(filePath + ".old"); - QFile::rename(filePath, filePath + ".old"); + error = saveFile.errorString(); + } else { + QTemporaryFile tempFile; + if (tempFile.open()) { + // write the database to the file + error = writeDatabase(&tempFile); + if (!error.isEmpty()) { + return error; + } + + tempFile.close(); // flush to disk + + if (backup) { + backupDatabase(filePath); + } + + // Delete the original db and move the temp file in place + QFile::remove(filePath); + if (tempFile.rename(filePath)) { + // successfully saved database file + tempFile.setAutoRemove(false); + return {}; + } } + error = tempFile.errorString(); + } + // Saving failed + return error; +} - QFile::remove(filePath); - if (saveFile.rename(filePath)) { - // successfully saved database file - saveFile.setAutoRemove(false); - return {}; - } +QString Database::writeDatabase(QIODevice* device) +{ + KeePass2Writer writer; + setEmitModified(false); + writer.writeDatabase(device, this); + setEmitModified(true); + + if (writer.hasError()) { + // the writer failed + return writer.errorString(); } + return {}; +} - return saveFile.errorString(); +/** + * Remove the old backup and replace it with a new one + * backups are named .old.kdbx4 + * + * @param filePath Path to the file to backup + * @return + */ +bool Database::backupDatabase(QString filePath) +{ + QString backupFilePath = filePath; + backupFilePath.replace(".kdbx", ".old.kdbx", Qt::CaseInsensitive); + if (!backupFilePath.endsWith(".old.kdbx")) { + // Fallback in case of poorly named file + backupFilePath = filePath + ".old"; + } + QFile::remove(backupFilePath); + return QFile::copy(filePath, backupFilePath); } QSharedPointer Database::kdf() const diff --git a/src/core/Database.h b/src/core/Database.h index 31d190877e..384ca814ea 100644 --- a/src/core/Database.h +++ b/src/core/Database.h @@ -32,6 +32,7 @@ enum class EntryReferenceType; class Group; class Metadata; class QTimer; +class QIODevice; struct DeletedObject { @@ -111,7 +112,7 @@ class Database : public QObject void emptyRecycleBin(); void setEmitModified(bool value); void merge(const Database* other); - QString saveToFile(QString filePath, bool keepOld = false); + QString saveToFile(QString filePath, bool atomic = true, bool backup = false); /** * Returns a unique id that is only valid as long as the Database exists. @@ -144,6 +145,8 @@ private slots: Group* findGroupRecursive(const Uuid& uuid, Group* group); void createRecycleBin(); + QString writeDatabase(QIODevice* device); + bool backupDatabase(QString filePath); Metadata* const m_metadata; Group* m_rootGroup; diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp index 06ff84ed39..1765a9cdf1 100644 --- a/src/gui/DatabaseTabWidget.cpp +++ b/src/gui/DatabaseTabWidget.cpp @@ -44,6 +44,7 @@ DatabaseManagerStruct::DatabaseManagerStruct() : dbWidget(nullptr) , modified(false) , readOnly(false) + , saveAttempts(0) { } @@ -324,12 +325,14 @@ bool DatabaseTabWidget::saveDatabase(Database* db, QString filePath) dbStruct.dbWidget->blockAutoReload(true); // TODO: Make this async, but lock out the database widget to prevent re-entrance - QString errorMessage = db->saveToFile(filePath, config()->get("BackupBeforeSave").toBool()); + bool useAtomicSaves = config()->get("UseAtomicSaves").toBool(); + QString errorMessage = db->saveToFile(filePath, useAtomicSaves, config()->get("BackupBeforeSave").toBool()); dbStruct.dbWidget->blockAutoReload(false); if (errorMessage.isEmpty()) { // successfully saved database file dbStruct.modified = false; + dbStruct.saveAttempts = 0; dbStruct.fileInfo = QFileInfo(filePath); dbStruct.dbWidget->databaseSaved(); updateTabName(db); @@ -338,6 +341,22 @@ bool DatabaseTabWidget::saveDatabase(Database* db, QString filePath) } else { dbStruct.modified = true; updateTabName(db); + + if (++dbStruct.saveAttempts > 2 && useAtomicSaves) { + // Saving failed 3 times, issue a warning and attempt to resolve + auto choice = MessageBox::question(this, tr("Disable safe saves?"), + tr("KeePassXC has failed to save the database multiple times. " + "This is likely caused by file sync services holding a lock on " + "the save file.\nDisable safe saves and try again?"), + QMessageBox::Yes | QMessageBox::No, QMessageBox::Yes); + if (choice == QMessageBox::Yes) { + config()->set("UseAtomicSaves", false); + return saveDatabase(db, filePath); + } + // Reset save attempts without changing anything + dbStruct.saveAttempts = 0; + } + emit messageTab(tr("Writing the database failed.").append("\n").append(errorMessage), MessageWidget::Error); return false; diff --git a/src/gui/DatabaseTabWidget.h b/src/gui/DatabaseTabWidget.h index 875c3c904e..b216750ea0 100644 --- a/src/gui/DatabaseTabWidget.h +++ b/src/gui/DatabaseTabWidget.h @@ -40,6 +40,7 @@ struct DatabaseManagerStruct QFileInfo fileInfo; bool modified; bool readOnly; + int saveAttempts; }; Q_DECLARE_TYPEINFO(DatabaseManagerStruct, Q_MOVABLE_TYPE); diff --git a/src/gui/SettingsWidget.cpp b/src/gui/SettingsWidget.cpp index 12c9708825..919edf9fd4 100644 --- a/src/gui/SettingsWidget.cpp +++ b/src/gui/SettingsWidget.cpp @@ -118,6 +118,7 @@ void SettingsWidget::loadSettings() m_generalUi->autoSaveAfterEveryChangeCheckBox->setChecked(config()->get("AutoSaveAfterEveryChange").toBool()); m_generalUi->autoSaveOnExitCheckBox->setChecked(config()->get("AutoSaveOnExit").toBool()); m_generalUi->backupBeforeSaveCheckBox->setChecked(config()->get("BackupBeforeSave").toBool()); + m_generalUi->useAtomicSavesCheckBox->setChecked(config()->get("UseAtomicSaves").toBool()); m_generalUi->autoReloadOnChangeCheckBox->setChecked(config()->get("AutoReloadOnChange").toBool()); m_generalUi->minimizeOnCopyCheckBox->setChecked(config()->get("MinimizeOnCopy").toBool()); m_generalUi->useGroupIconOnEntryCreationCheckBox->setChecked(config()->get("UseGroupIconOnEntryCreation").toBool()); @@ -195,6 +196,7 @@ void SettingsWidget::saveSettings() m_generalUi->autoSaveAfterEveryChangeCheckBox->isChecked()); config()->set("AutoSaveOnExit", m_generalUi->autoSaveOnExitCheckBox->isChecked()); config()->set("BackupBeforeSave", m_generalUi->backupBeforeSaveCheckBox->isChecked()); + config()->set("UseAtomicSaves", m_generalUi->useAtomicSavesCheckBox->isChecked()); config()->set("AutoReloadOnChange", m_generalUi->autoReloadOnChangeCheckBox->isChecked()); config()->set("MinimizeOnCopy", m_generalUi->minimizeOnCopyCheckBox->isChecked()); config()->set("UseGroupIconOnEntryCreation", diff --git a/src/gui/SettingsWidgetGeneral.ui b/src/gui/SettingsWidgetGeneral.ui index a80c8c2f9c..f8414f07ce 100644 --- a/src/gui/SettingsWidgetGeneral.ui +++ b/src/gui/SettingsWidgetGeneral.ui @@ -34,131 +34,134 @@ - - - Start only a single instance of KeePassXC - - - true - - - - - - - Remember last databases - - - true - - - - - - - Remember last key files and security dongles - - - true - - - - - - - Load previous databases on startup - - - - - - - Automatically save on exit - - - - - - - Automatically save after every change - - - - - - - Backup database file before saving - - - - - - - Automatically reload the database when modified externally - - - - - - - Minimize when copying to clipboard - - - - - - - Minimize window at application startup + + + Startup + + + + + Start only a single instance of KeePassXC + + + true + + + + + + + Remember last databases + + + true + + + + + + + Remember last key files and security dongles + + + true + + + + + + + Load previous databases on startup + + + + + + + Minimize window at application startup + + + + - - - Use group icon on entry creation - - - true + + + File Management + + + + + Safely save database files (may be incompatible with DropBox, etc) + + + true + + + + + + + Backup database file before saving + + + + + + + Automatically save after every change + + + + + + + Automatically save on exit + + + + + + + Don't mark database as modified for non-data changes (e.g., expanding groups) + + + + + + + Automatically reload the database when modified externally + + + + - - - Don't mark database as modified for non-data changes (e.g., expanding groups) + + + Entry Management - - - - - - - 0 - - - 0 - - - 0 - - - 0 - + - - - Qt::Vertical + + + Use group icon on entry creation - - QSizePolicy::Fixed + + true - - - 20 - 30 - + + + + + + Minimize when copying to clipboard - + @@ -167,6 +170,15 @@ + + + + + + + General + + @@ -175,114 +187,162 @@ - + + + + 0 + + + 0 + + + 0 + + + 0 + + + + + 0 + + + QLayout::SetMaximumSize + + + + + Qt::Horizontal + + + QSizePolicy::Fixed + + + + 40 + 20 + + + + + + + + false + + + + 0 + 0 + + + + Hide window to system tray when minimized + + + + + + + + + 0 + + + QLayout::SetMaximumSize + + + + + Qt::Horizontal + + + QSizePolicy::Fixed + + + + 40 + 20 + + + + + + + + false + + + Hide window to system tray instead of app exit + + + + + + + + + 0 + + + QLayout::SetMaximumSize + + + + + Qt::Horizontal + + + QSizePolicy::Fixed + + + + 40 + 20 + + + + + + + + false + + + Dark system tray icon + + + + + + + + + + - 0 + 15 - - QLayout::SetMaximumSize - - - - - Qt::Horizontal - - - QSizePolicy::Fixed - - - - 40 - 20 - - - - - - - - false - + + - + 0 0 - Hide window to system tray when minimized + Language - - - - - - 0 - - - QLayout::SetMaximumSize - - - - Qt::Horizontal - - - QSizePolicy::Fixed - - - - 40 - 20 - - - - - - - - false - - - Hide window to system tray instead of app exit - - - - - - - - - 0 - - - QLayout::SetMaximumSize - - - - - Qt::Horizontal - - - QSizePolicy::Fixed - - - - 40 - 20 - - - - - - - - false - - - Dark system tray icon + + + + 0 + 0 + @@ -291,52 +351,6 @@ - - - - Qt::Vertical - - - QSizePolicy::Fixed - - - - 20 - 30 - - - - - - - - 15 - - - - - - 0 - 0 - - - - Language - - - - - - - - 0 - 0 - - - - - -