Allow specifing database backup paths. (#7035)

- Default backupFilePath is '{DB_FILENAME}.old.kdbx' to conform to existing standards
- Implement backupPathPattern tests.
- Show tooltip on how to format database backup location text field.

share/translations/keepassxc_en.ts

@@ -215,6 +215,10 @@
         <source>Monochrome</source>
         <translation type="unfinished"></translation>
     </message>
+    <message>
+        <source>Select backup storage directory</source>
+        <translation type="unfinished"></translation>
+    </message>
 </context>
 <context>
     <name>ApplicationSettingsWidgetGeneral</name>
@@ -440,6 +444,22 @@
         <source>Directly write to database file (dangerous)</source>
         <translation type="unfinished"></translation>
     </message>
+    <message>
+        <source>Choose...</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>Backup destination</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>Specifies the database backup file location. Occurences of &quot;{DB_FILENAME}&quot; are replaced with the filename of the saved database without extension. {TIME:&lt;format&gt;} is replaced with the backup time, see https://doc.qt.io/qt-5/qdatetime.html#toString. &lt;format&gt; defaults to format string &quot;dd_MM_yyyy_hh-mm-ss&quot;.</source>
+        <translation type="unfinished"></translation>
+    </message>
+    <message>
+        <source>{DB_FILENAME}.old.kdbx</source>
+        <translation type="unfinished"></translation>
+    </message>
 </context>
 <context>
     <name>ApplicationSettingsWidgetSecurity</name>

src/cli/Add.cpp

@@ -121,7 +121,7 @@ int Add::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<Q
     }
 
     QString errorMessage;
-    if (!database->save(Database::Atomic, false, &errorMessage)) {
+    if (!database->save(Database::Atomic, QString(), &errorMessage)) {
         err << QObject::tr("Writing the database failed %1.").arg(errorMessage) << endl;
         return EXIT_FAILURE;
     }

src/cli/AddGroup.cpp

@@ -63,7 +63,7 @@ int AddGroup::executeWithDatabase(QSharedPointer<Database> database, QSharedPoin
     newGroup->setParent(parentGroup);
 
     QString errorMessage;
-    if (!database->save(Database::Atomic, false, &errorMessage)) {
+    if (!database->save(Database::Atomic, QString(), &errorMessage)) {
         err << QObject::tr("Writing the database failed %1.").arg(errorMessage) << endl;
         return EXIT_FAILURE;
     }

src/cli/Create.cpp

@@ -165,7 +165,7 @@ int Create::execute(const QStringList& arguments)
     }
 
     QString errorMessage;
-    if (!db->saveAs(databaseFilename, Database::Atomic, false, &errorMessage)) {
+    if (!db->saveAs(databaseFilename, Database::Atomic, QString(), &errorMessage)) {
         err << QObject::tr("Failed to save the database: %1.").arg(errorMessage) << endl;
         return EXIT_FAILURE;
     }

src/cli/Edit.cpp

@@ -126,7 +126,7 @@ int Edit::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<
     entry->endUpdate();
 
     QString errorMessage;
-    if (!database->save(Database::Atomic, false, &errorMessage)) {
+    if (!database->save(Database::Atomic, QString(), &errorMessage)) {
         err << QObject::tr("Writing the database failed: %1").arg(errorMessage) << endl;
         return EXIT_FAILURE;
     }

src/cli/Import.cpp

@@ -75,7 +75,7 @@ int Import::execute(const QStringList& arguments)
         return EXIT_FAILURE;
     }
 
-    if (!db->saveAs(dbPath, Database::Atomic, false, &errorMessage)) {
+    if (!db->saveAs(dbPath, Database::Atomic, QString(), &errorMessage)) {
         err << QObject::tr("Failed to save the database: %1.").arg(errorMessage) << endl;
         return EXIT_FAILURE;
     }

src/cli/Merge.cpp

@@ -95,7 +95,7 @@ int Merge::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer
 
     if (!changeList.isEmpty() && !parser->isSet(Merge::DryRunOption)) {
         QString errorMessage;
-        if (!database->save(Database::Atomic, false, &errorMessage)) {
+        if (!database->save(Database::Atomic, QString(), &errorMessage)) {
             err << QObject::tr("Unable to save database to file : %1").arg(errorMessage) << endl;
             return EXIT_FAILURE;
         }

src/cli/Move.cpp

@@ -65,7 +65,7 @@ int Move::executeWithDatabase(QSharedPointer<Database> database, QSharedPointer<
     entry->endUpdate();
 
     QString errorMessage;
-    if (!database->save(Database::Atomic, false, &errorMessage)) {
+    if (!database->save(Database::Atomic, QString(), &errorMessage)) {
         err << QObject::tr("Writing the database failed %1.").arg(errorMessage) << endl;
         return EXIT_FAILURE;
     }

src/cli/Remove.cpp

@@ -53,7 +53,7 @@ int Remove::executeWithDatabase(QSharedPointer<Database> database, QSharedPointe
     };
 
     QString errorMessage;
-    if (!database->save(Database::Atomic, false, &errorMessage)) {
+    if (!database->save(Database::Atomic, QString(), &errorMessage)) {
         err << QObject::tr("Unable to save database to file: %1").arg(errorMessage) << endl;
         return EXIT_FAILURE;
     }

src/cli/RemoveGroup.cpp

@@ -63,7 +63,7 @@ int RemoveGroup::executeWithDatabase(QSharedPointer<Database> database, QSharedP
     };
 
     QString errorMessage;
-    if (!database->save(Database::Atomic, false, &errorMessage)) {
+    if (!database->save(Database::Atomic, QString(), &errorMessage)) {
         err << QObject::tr("Unable to save database to file: %1").arg(errorMessage) << endl;
         return EXIT_FAILURE;
     }

src/core/Config.cpp

@@ -61,6 +61,7 @@ static const QHash<Config::ConfigKey, ConfigDirective> configStrings = {
     {Config::AutoSaveOnExit,{QS("AutoSaveOnExit"), Roaming, true}},
     {Config::AutoSaveNonDataChanges,{QS("AutoSaveNonDataChanges"), Roaming, true}},
     {Config::BackupBeforeSave,{QS("BackupBeforeSave"), Roaming, false}},
+    {Config::BackupFilePathPattern,{QS("BackupFilePathPattern"), Roaming, QString("{DB_FILENAME}.old.kdbx")}},
     {Config::UseAtomicSaves,{QS("UseAtomicSaves"), Roaming, true}},
     {Config::UseDirectWriteSaves,{QS("UseDirectWriteSaves"), Local, false}},
     {Config::SearchLimitGroup,{QS("SearchLimitGroup"), Roaming, false}},
@@ -229,6 +230,11 @@ QVariant Config::get(ConfigKey key)
     return m_settings->value(cfg.name, defaultValue);
 }
 
+QVariant Config::getDefault(Config::ConfigKey key)
+{
+    return configStrings[key].defaultValue;
+}
+
 bool Config::hasAccessError()
 {
     return m_settings->status() & QSettings::AccessError;

src/core/Config.h

@@ -43,6 +43,7 @@ class Config : public QObject
         AutoSaveOnExit,
         AutoSaveNonDataChanges,
         BackupBeforeSave,
+        BackupFilePathPattern,
         UseAtomicSaves,
         UseDirectWriteSaves,
         SearchLimitGroup,
@@ -195,6 +196,7 @@ class Config : public QObject
 
     ~Config() override;
     QVariant get(ConfigKey key);
+    QVariant getDefault(ConfigKey key);
     QString getFileName();
     void set(ConfigKey key, const QVariant& value);
     void remove(ConfigKey key);

src/core/Database.cpp

@@ -178,10 +178,10 @@ bool Database::isSaving()
  *
  * @param error error message in case of failure
  * @param atomic Use atomic file transactions
- * @param backup Backup the existing database file, if exists
+ * @param backupFilePath Absolute file path to write the backup file to. Pass an empty QString to disable backup.
  * @return true on success
  */
-bool Database::save(SaveAction action, bool backup, QString* error)
+bool Database::save(SaveAction action, const QString& backupFilePath, QString* error)
 {
     Q_ASSERT(!m_data.filePath.isEmpty());
     if (m_data.filePath.isEmpty()) {
@@ -191,7 +191,7 @@ bool Database::save(SaveAction action, bool backup, QString* error)
         return false;
     }
 
-    return saveAs(m_data.filePath, action, backup, error);
+    return saveAs(m_data.filePath, action, backupFilePath, error);
 }
 
 /**
@@ -209,10 +209,11 @@ bool Database::save(SaveAction action, bool backup, QString* error)
  * @param filePath Absolute path of the file to save
  * @param error error message in case of failure
  * @param atomic Use atomic file transactions
- * @param backup Backup the existing database file, if exists
+ * @param backupFilePath Absolute path to the location where the backup should be stored. Passing an empty string
+ * disables backup.
  * @return true on success
  */
-bool Database::saveAs(const QString& filePath, SaveAction action, bool backup, QString* error)
+bool Database::saveAs(const QString& filePath, SaveAction action, const QString& backupFilePath, QString* error)
 {
     // Disallow overlapping save operations
     if (isSaving()) {
@@ -260,7 +261,7 @@ bool Database::saveAs(const QString& filePath, SaveAction action, bool backup, Q
     QFileInfo fileInfo(filePath);
     auto realFilePath = fileInfo.exists() ? fileInfo.canonicalFilePath() : fileInfo.absoluteFilePath();
     bool isNewFile = !QFile::exists(realFilePath);
-    bool ok = AsyncTask::runAndWaitForFuture([&] { return performSave(realFilePath, action, backup, error); });
+    bool ok = AsyncTask::runAndWaitForFuture([&] { return performSave(realFilePath, action, backupFilePath, error); });
     if (ok) {
         markAsClean();
         setFilePath(filePath);
@@ -276,10 +277,10 @@ bool Database::saveAs(const QString& filePath, SaveAction action, bool backup, Q
     return ok;
 }
 
-bool Database::performSave(const QString& filePath, SaveAction action, bool backup, QString* error)
+bool Database::performSave(const QString& filePath, SaveAction action, const QString& backupFilePath, QString* error)
 {
-    if (backup) {
-        backupDatabase(filePath);
+    if (!backupFilePath.isNull()) {
+        backupDatabase(filePath, backupFilePath);
     }
 
 #if QT_VERSION >= QT_VERSION_CHECK(5, 10, 0)
@@ -337,7 +338,7 @@ bool Database::performSave(const QString& filePath, SaveAction action, bool back
                 tempFile.setFileTime(createTime, QFile::FileBirthTime);
 #endif
                 return true;
-            } else if (!backup || !restoreDatabase(filePath)) {
+            } else if (backupFilePath.isEmpty() || !restoreDatabase(filePath, backupFilePath)) {
                 // Failed to copy new database in place, and
                 // failed to restore from backup or backups disabled
                 tempFile.setAutoRemove(false);
@@ -485,23 +486,26 @@ void Database::releaseData()
 }
 
 /**
- * Remove the old backup and replace it with a new one
- * backups are named <filename>.old.<extension>
+ * Remove the old backup and replace it with a new one. Backup name is taken from destinationFilePath.
+ * Non-existing parent directories will be created automatically.
  *
  * @param filePath Path to the file to backup
+ * @param destinationFilePath Path to the backup destination file
  * @return true on success
  */
-bool Database::backupDatabase(const QString& filePath)
+bool Database::backupDatabase(const QString& filePath, const QString& destinationFilePath)
 {
-    static auto re = QRegularExpression("(\\.[^.]+)$");
-
-    auto match = re.match(filePath);
-    auto backupFilePath = filePath;
+    // Ensure that the path to write to actually exists
+    auto parentDirectory = QFileInfo(destinationFilePath).absoluteDir();
+    if (!parentDirectory.exists()) {
+        if (!QDir().mkpath(parentDirectory.absolutePath())) {
+            return false;
+        }
+    }
     auto perms = QFile::permissions(filePath);
-    backupFilePath = backupFilePath.replace(re, "") + ".old" + match.captured(1);
-    QFile::remove(backupFilePath);
-    bool res = QFile::copy(filePath, backupFilePath);
-    QFile::setPermissions(backupFilePath, perms);
+    QFile::remove(destinationFilePath);
+    bool res = QFile::copy(filePath, destinationFilePath);
+    QFile::setPermissions(destinationFilePath, perms);
     return res;
 }
 
@@ -513,17 +517,13 @@ bool Database::backupDatabase(const QString& filePath)
  * @param filePath Path to the file to restore
  * @return true on success
  */
-bool Database::restoreDatabase(const QString& filePath)
+bool Database::restoreDatabase(const QString& filePath, const QString& fromBackupFilePath)
 {
-    static auto re = QRegularExpression("^(.*?)(\\.[^.]+)?$");
-
-    auto match = re.match(filePath);
     auto perms = QFile::permissions(filePath);
-    auto backupFilePath = match.captured(1) + ".old" + match.captured(2);
     // Only try to restore if the backup file actually exists
-    if (QFile::exists(backupFilePath)) {
+    if (QFile::exists(fromBackupFilePath)) {
         QFile::remove(filePath);
-        if (QFile::copy(backupFilePath, filePath)) {
+        if (QFile::copy(fromBackupFilePath, filePath)) {
             return QFile::setPermissions(filePath, perms);
         }
     }

src/core/Database.h

@@ -79,8 +79,11 @@ class Database : public ModifiableObject
               QSharedPointer<const CompositeKey> key,
               QString* error = nullptr,
               bool readOnly = false);
-    bool save(SaveAction action = Atomic, bool backup = false, QString* error = nullptr);
-    bool saveAs(const QString& filePath, SaveAction action = Atomic, bool backup = false, QString* error = nullptr);
+    bool save(SaveAction action = Atomic, const QString& backupFilePath = QString(), QString* error = nullptr);
+    bool saveAs(const QString& filePath,
+                SaveAction action = Atomic,
+                const QString& backupFilePath = QString(),
+                QString* error = nullptr);
     bool extract(QByteArray&, QString* error = nullptr);
     bool import(const QString& xmlExportPath, QString* error = nullptr);
 
@@ -203,9 +206,9 @@ public slots:
     void createRecycleBin();
 
     bool writeDatabase(QIODevice* device, QString* error = nullptr);
-    bool backupDatabase(const QString& filePath);
-    bool restoreDatabase(const QString& filePath);
-    bool performSave(const QString& filePath, SaveAction flags, bool backup, QString* error);
+    bool backupDatabase(const QString& filePath, const QString& destinationFilePath);
+    bool restoreDatabase(const QString& filePath, const QString& fromBackupFilePath);
+    bool performSave(const QString& filePath, SaveAction flags, const QString& backupFilePath, QString* error);
     void startModifiedTimer();
     void stopModifiedTimer();
 

src/core/Tools.cpp

@@ -22,7 +22,10 @@
 #include "config-keepassx.h"
 #include "git-info.h"
 
+#include "core/Clock.h"
+
 #include <QElapsedTimer>
+#include <QFileInfo>
 #include <QImageReader>
 #include <QLocale>
 #include <QMetaProperty>
@@ -376,4 +379,37 @@ namespace Tools
         }
         return result;
     }
+
+    QString substituteBackupFilePath(QString pattern, const QString& databasePath)
+    {
+        // Fail if substitution fails
+        if (databasePath.isEmpty()) {
+            return {};
+        }
+
+        // Replace backup pattern
+        QFileInfo dbFileInfo(databasePath);
+        QString baseName = dbFileInfo.completeBaseName();
+
+        pattern.replace(QString("{DB_FILENAME}"), baseName);
+
+        auto re = QRegularExpression(R"(\{TIME(?::([^\\]*))?\})");
+        auto match = re.match(pattern);
+        while (match.hasMatch()) {
+            // Extract time format specifier
+            auto formatSpecifier = QString("dd_MM_yyyy_hh-mm-ss");
+            if (!match.captured(1).isEmpty()) {
+                formatSpecifier = match.captured(1);
+            }
+            auto replacement = Clock::currentDateTime().toString(formatSpecifier);
+            pattern.replace(match.capturedStart(), match.capturedLength(), replacement);
+            match = re.match(pattern);
+        }
+
+        // Replace escaped braces
+        pattern.replace("\\{", "{");
+        pattern.replace("\\}", "}");
+
+        return pattern;
+    }
 } // namespace Tools

src/core/Tools.h

@@ -75,6 +75,8 @@ namespace Tools
     }
 
     QVariantMap qo2qvm(const QObject* object, const QStringList& ignoredProperties = {"objectName"});
+
+    QString substituteBackupFilePath(QString pattern, const QString& databasePath);
 } // namespace Tools
 
 #endif // KEEPASSX_TOOLS_H