Skip to content

Commit

Permalink
Correct saving files to DropBox/Drive/OneDrive
Browse files Browse the repository at this point in the history
* Replaces QSaveFile with QTemporaryFile
* Added backup before save config setting
* This method may cause data loss (see comments)
  • Loading branch information
droidmonkey committed Jan 15, 2018
1 parent 243edda commit ca9a590
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 13 deletions.
1 change: 1 addition & 0 deletions src/core/Config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ void Config::init(const QString& fileName)
m_defaults.insert("AutoSaveAfterEveryChange", false);
m_defaults.insert("AutoReloadOnChange", true);
m_defaults.insert("AutoSaveOnExit", false);
m_defaults.insert("BackupBeforeSave", false);
m_defaults.insert("SearchLimitGroup", false);
m_defaults.insert("MinimizeOnCopy", false);
m_defaults.insert("UseGroupIconOnEntryCreation", false);
Expand Down
44 changes: 33 additions & 11 deletions src/core/Database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
#include "Database.h"

#include <QFile>
#include <QSaveFile>
#include <QTemporaryFile>
#include <QTextStream>
#include <QTimer>
#include <QXmlStreamReader>
Expand Down Expand Up @@ -470,30 +470,52 @@ Database* Database::unlockFromStdin(QString databaseFilename, QString keyFilenam
return Database::openDatabaseFile(databaseFilename, compositeKey);
}

QString Database::saveToFile(QString filePath)
/**
* Save the database to a file.
*
* This function uses QTemporaryFile instead of QSaveFile due to a bug
* in Qt (https://bugreports.qt.io/browse/QTBUG-57299) that may prevent
* the QSaveFile from renaming itself when using DropBox, Drive, or OneDrive.
*
* The risk in using QTemporaryFile is that the rename function is not atomic
* and may result in loss of data if there is a crash or power loss at the
* wrong moment.
*
* @param filePath Absolute path of the file to save
* @param keepOld Rename the original database file instead of deleting
* @return error string, if any
*/
QString Database::saveToFile(QString filePath, bool keepOld)
{
KeePass2Writer writer;
QSaveFile saveFile(filePath);
if (saveFile.open(QIODevice::WriteOnly)) {

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();
}

if (saveFile.commit()) {
saveFile.close(); // flush to disk

if (keepOld) {
QFile::remove(filePath + ".old");
QFile::rename(filePath, filePath + ".old");
}

QFile::remove(filePath);
if (saveFile.rename(filePath)) {
// successfully saved database file
return QString();
} else {
return saveFile.errorString();
saveFile.setAutoRemove(false);
return {};
}
} else {
return saveFile.errorString();
}

return saveFile.errorString();
}

QSharedPointer<Kdf> Database::kdf() const
Expand Down
2 changes: 1 addition & 1 deletion src/core/Database.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class Database : public QObject
void emptyRecycleBin();
void setEmitModified(bool value);
void merge(const Database* other);
QString saveToFile(QString filePath);
QString saveToFile(QString filePath, bool keepOld = false);

/**
* Returns a unique id that is only valid as long as the Database exists.
Expand Down
4 changes: 3 additions & 1 deletion src/gui/DatabaseTabWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include "core/Database.h"
#include "core/Group.h"
#include "core/Metadata.h"
#include "core/AsyncTask.h"
#include "format/CsvExporter.h"
#include "gui/Clipboard.h"
#include "gui/DatabaseWidget.h"
Expand Down Expand Up @@ -322,7 +323,8 @@ bool DatabaseTabWidget::saveDatabase(Database* db, QString filePath)
}

dbStruct.dbWidget->blockAutoReload(true);
QString errorMessage = db->saveToFile(filePath);
// TODO: Make this async, but lock out the database widget to prevent re-entrance
QString errorMessage = db->saveToFile(filePath, config()->get("BackupBeforeSave").toBool());
dbStruct.dbWidget->blockAutoReload(false);

if (errorMessage.isEmpty()) {
Expand Down
2 changes: 2 additions & 0 deletions src/gui/SettingsWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ void SettingsWidget::loadSettings()
config()->get("OpenPreviousDatabasesOnStartup").toBool());
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->autoReloadOnChangeCheckBox->setChecked(config()->get("AutoReloadOnChange").toBool());
m_generalUi->minimizeOnCopyCheckBox->setChecked(config()->get("MinimizeOnCopy").toBool());
m_generalUi->useGroupIconOnEntryCreationCheckBox->setChecked(config()->get("UseGroupIconOnEntryCreation").toBool());
Expand Down Expand Up @@ -193,6 +194,7 @@ void SettingsWidget::saveSettings()
config()->set("AutoSaveAfterEveryChange",
m_generalUi->autoSaveAfterEveryChangeCheckBox->isChecked());
config()->set("AutoSaveOnExit", m_generalUi->autoSaveOnExitCheckBox->isChecked());
config()->set("BackupBeforeSave", m_generalUi->backupBeforeSaveCheckBox->isChecked());
config()->set("AutoReloadOnChange", m_generalUi->autoReloadOnChangeCheckBox->isChecked());
config()->set("MinimizeOnCopy", m_generalUi->minimizeOnCopyCheckBox->isChecked());
config()->set("UseGroupIconOnEntryCreation",
Expand Down
7 changes: 7 additions & 0 deletions src/gui/SettingsWidgetGeneral.ui
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,13 @@
</property>
</widget>
</item>
<item>
<widget class="QCheckBox" name="backupBeforeSaveCheckBox">
<property name="text">
<string>Backup database file before saving</string>
</property>
</widget>
</item>
<item>
<widget class="QCheckBox" name="autoReloadOnChangeCheckBox">
<property name="text">
Expand Down

0 comments on commit ca9a590

Please sign in to comment.