Remove lock file and cleanup file handling [#1002]

src/gui/DatabaseTabWidget.cpp

@@ -19,7 +19,6 @@
 #include "DatabaseTabWidget.h"
 
 #include <QFileInfo>
-#include <QLockFile>
 #include <QTabWidget>
 #include <QPushButton>
 
@@ -42,8 +41,6 @@
 
 DatabaseManagerStruct::DatabaseManagerStruct()
     : dbWidget(nullptr)
-    , lockFile(nullptr)
-    , saveToFilename(false)
     , modified(false)
     , readOnly(false)
 {
@@ -107,7 +104,7 @@ void DatabaseTabWidget::newDatabase()
 void DatabaseTabWidget::openDatabase()
 {
     QString filter = QString("%1 (*.kdbx);;%2 (*)").arg(tr("KeePass 2 Database"), tr("All files"));
-    QString fileName = fileDialog()->getOpenFileName(this, tr("Open database"), QString(),
+    QString fileName = fileDialog()->getOpenFileName(this, tr("Open database"), QDir::homePath(),
                                                      filter);
     if (!fileName.isEmpty()) {
         openDatabase(fileName);
@@ -128,10 +125,10 @@ void DatabaseTabWidget::openDatabase(const QString& fileName, const QString& pw,
     QHashIterator<Database*, DatabaseManagerStruct> i(m_dbList);
     while (i.hasNext()) {
         i.next();
-        if (i.value().canonicalFilePath == canonicalFilePath) {
+        if (i.value().fileInfo.canonicalFilePath() == canonicalFilePath) {
             if (!i.value().dbWidget->dbHasKey() && !(pw.isNull() && keyFile.isEmpty())) {
                 // If the database is locked and a pw or keyfile is provided, unlock it
-                i.value().dbWidget->switchToOpenDatabase(i.value().filePath, pw, keyFile);
+                i.value().dbWidget->switchToOpenDatabase(i.value().fileInfo.absoluteFilePath(), pw, keyFile);
             } else {
                 setCurrentIndex(databaseIndex(i.key()));
             }
@@ -157,49 +154,9 @@ void DatabaseTabWidget::openDatabase(const QString& fileName, const QString& pw,
     }
     file.close();
 
-    QLockFile* lockFile = new QLockFile(QString("%1/.%2.lock").arg(fileInfo.canonicalPath(), fileInfo.fileName()));
-    lockFile->setStaleLockTime(0);
-
-    if (!dbStruct.readOnly && !lockFile->tryLock()) {
-        // for now silently ignore if we can't create a lock file
-        // due to lack of permissions
-        if (lockFile->error() != QLockFile::PermissionError) {
-            QMessageBox msgBox;
-            msgBox.setWindowTitle(tr("Database already opened"));
-            msgBox.setText(tr("The database you are trying to open is locked by another instance of KeePassXC.\n\n"
-                              "Do you want to open it anyway?"));
-            msgBox.setIcon(QMessageBox::Question);
-            msgBox.addButton(QMessageBox::Yes);
-            msgBox.addButton(QMessageBox::No);
-            auto readOnlyButton = msgBox.addButton(tr("Open read-only"), QMessageBox::NoRole);
-            msgBox.setDefaultButton(readOnlyButton);
-            msgBox.setEscapeButton(QMessageBox::No);
-            auto result = msgBox.exec();
-
-            if (msgBox.clickedButton() == readOnlyButton) {
-                dbStruct.readOnly = true;
-                delete lockFile;
-                lockFile = nullptr;
-            } else if (result == QMessageBox::Yes) {
-                // take over the lock file if possible
-                if (lockFile->removeStaleLockFile()) {
-                    lockFile->tryLock();
-                }
-            } else {
-                delete lockFile;
-                return;
-            }
-        }
-    }
-
     Database* db = new Database();
     dbStruct.dbWidget = new DatabaseWidget(db, this);
-    dbStruct.lockFile = lockFile;
+    dbStruct.fileInfo = fileInfo;
-    dbStruct.saveToFilename = !dbStruct.readOnly;
-
-    dbStruct.filePath = fileInfo.absoluteFilePath();
-    dbStruct.canonicalFilePath = canonicalFilePath;
-    dbStruct.fileName = fileInfo.fileName();
 
     insertDatabase(db, dbStruct);
 
@@ -207,13 +164,12 @@ void DatabaseTabWidget::openDatabase(const QString& fileName, const QString& pw,
         emit messageTab(tr("File opened in read only mode."), MessageWidget::Warning);
     }
 
-    updateLastDatabases(dbStruct.filePath);
+    updateLastDatabases(dbStruct.fileInfo.absoluteFilePath());
 
     if (!(pw.isNull() && keyFile.isEmpty())) {
-        dbStruct.dbWidget->switchToOpenDatabase(dbStruct.filePath, pw, keyFile);
+        dbStruct.dbWidget->switchToOpenDatabase(dbStruct.fileInfo.absoluteFilePath(), pw, keyFile);
-    }
+    } else {
-    else {
+        dbStruct.dbWidget->switchToOpenDatabase(dbStruct.fileInfo.absoluteFilePath());
-        dbStruct.dbWidget->switchToOpenDatabase(dbStruct.filePath);
     }
     emit messageDismissGlobal();
 }
@@ -322,15 +278,14 @@ bool DatabaseTabWidget::closeDatabase(Database* db)
 void DatabaseTabWidget::deleteDatabase(Database* db)
 {
     const DatabaseManagerStruct dbStruct = m_dbList.value(db);
-    bool emitDatabaseWithFileClosed = dbStruct.saveToFilename;
+    bool emitDatabaseWithFileClosed = dbStruct.fileInfo.exists() && !dbStruct.readOnly;
-    QString filePath = dbStruct.filePath;
+    QString filePath = dbStruct.fileInfo.absoluteFilePath();
 
     int index = databaseIndex(db);
 
     removeTab(index);
     toggleTabbar();
     m_dbList.remove(db);
-    delete dbStruct.lockFile;
     delete dbStruct.dbWidget;
     delete db;
 
@@ -349,7 +304,7 @@ bool DatabaseTabWidget::closeAllDatabases()
     return true;
 }
 
-bool DatabaseTabWidget::saveDatabase(Database* db)
+bool DatabaseTabWidget::saveDatabase(Database* db, QString filePath)
 {
     DatabaseManagerStruct& dbStruct = m_dbList[db];
 
@@ -359,9 +314,13 @@ bool DatabaseTabWidget::saveDatabase(Database* db)
         return true;
     }
 
-    if (dbStruct.saveToFilename) {
+    if (!dbStruct.readOnly) {
+        if (filePath.isEmpty()) {
+            filePath = dbStruct.fileInfo.canonicalFilePath();
+        }
+
         dbStruct.dbWidget->blockAutoReload(true);
-        QString errorMessage = db->saveToFile(dbStruct.canonicalFilePath);
+        QString errorMessage = db->saveToFile(filePath);
         dbStruct.dbWidget->blockAutoReload(false);
 
         if (errorMessage.isEmpty()) {
@@ -388,71 +347,26 @@ bool DatabaseTabWidget::saveDatabaseAs(Database* db)
     while (true) {
         DatabaseManagerStruct& dbStruct = m_dbList[db];
         QString oldFileName;
-        if (dbStruct.saveToFilename) {
+        if (dbStruct.fileInfo.exists()) {
-            oldFileName = dbStruct.filePath;
+            oldFileName = dbStruct.fileInfo.absoluteFilePath();
         } else {
-            oldFileName = tr("Passwords").append(".kdbx");
+            oldFileName = QDir::toNativeSeparators(QDir::homePath() + "/" + tr("Passwords").append(".kdbx"));
         }
         QString fileName = fileDialog()->getSaveFileName(this, tr("Save database as"),
                                                         oldFileName, tr("KeePass 2 Database").append(" (*.kdbx)"),
                                                         nullptr, 0, "kdbx");
         if (!fileName.isEmpty()) {
-            QFileInfo fileInfo(fileName);
+            if (!saveDatabase(db, fileName)) {
-            QString lockFilePath;
+                // Failed to save, try again
-            if (fileInfo.exists()) {
-                // returns empty string when file doesn't exist
-                lockFilePath = fileInfo.canonicalPath();
-            } else {
-                lockFilePath = fileInfo.absolutePath();
-            }
-            QString lockFileName = QString("%1/.%2.lock").arg(lockFilePath, fileInfo.fileName());
-            QScopedPointer<QLockFile> lockFile(new QLockFile(lockFileName));
-            lockFile->setStaleLockTime(0);
-            if (!lockFile->tryLock()) {
-                // for now silently ignore if we can't create a lock file
-                // due to lack of permissions
-                if (lockFile->error() != QLockFile::PermissionError) {
-                    QMessageBox::StandardButton result = MessageBox::question(this, tr("Save database as"),
-                        tr("The database you are trying to save as is locked by another instance of KeePassXC.\n"
-                        "Do you want to save it anyway?"),
-                        QMessageBox::Yes | QMessageBox::No);
-
-                    if (result == QMessageBox::No) {
-                        return false;
-                    } else {
-                        // take over the lock file if possible
-                        if (lockFile->removeStaleLockFile()) {
-                            lockFile->tryLock();
-                        }
-                    }
-                }
-            }
-
-            // setup variables so saveDatabase succeeds
-            dbStruct.saveToFilename = true;
-            dbStruct.canonicalFilePath = fileName;
-
-            if (!saveDatabase(db)) {
-                // failed to save, revert back
-                dbStruct.saveToFilename = false;
-                dbStruct.canonicalFilePath = oldFileName;
                 continue;
             }
 
-            // refresh fileinfo since the file didn't exist before
-            fileInfo.refresh();
-
             dbStruct.modified = false;
-            dbStruct.saveToFilename = true;
             dbStruct.readOnly = false;
-            dbStruct.filePath = fileInfo.absoluteFilePath();
+            dbStruct.fileInfo = QFileInfo(fileName);
-            dbStruct.canonicalFilePath = fileInfo.canonicalFilePath();
+            dbStruct.dbWidget->updateFilename(dbStruct.fileInfo.absoluteFilePath());
-            dbStruct.fileName = fileInfo.fileName();
-            dbStruct.dbWidget->updateFilename(dbStruct.filePath);
-            delete dbStruct.lockFile;
-            dbStruct.lockFile = lockFile.take();
             updateTabName(db);
-            updateLastDatabases(dbStruct.filePath);
+            updateLastDatabases(dbStruct.fileInfo.absoluteFilePath());
             return true;
         } else {
             return false;
@@ -548,7 +462,7 @@ bool DatabaseTabWidget::canSave(int index)
     }
 
     const DatabaseManagerStruct& dbStruct = indexDatabaseManagerStruct(index);
-    return !dbStruct.saveToFilename || (dbStruct.modified && !dbStruct.readOnly);
+    return dbStruct.modified && !dbStruct.readOnly;
 }
 
 bool DatabaseTabWidget::isModified(int index)
@@ -566,7 +480,7 @@ QString DatabaseTabWidget::databasePath(int index)
         index = currentIndex();
     }
 
-    return indexDatabaseManagerStruct(index).filePath;
+    return indexDatabaseManagerStruct(index).fileInfo.absoluteFilePath();
 }
 
 
@@ -579,21 +493,18 @@ void DatabaseTabWidget::updateTabName(Database* db)
 
     QString tabName;
 
-    if (dbStruct.saveToFilename || dbStruct.readOnly) {
+    if (dbStruct.fileInfo.exists()) {
         if (db->metadata()->name().isEmpty()) {
-            tabName = dbStruct.fileName;
+            tabName = dbStruct.fileInfo.fileName();
-        }
+        } else {
-        else {
             tabName = db->metadata()->name();
         }
 
-        setTabToolTip(index, dbStruct.filePath);
+        setTabToolTip(index, dbStruct.fileInfo.absoluteFilePath());
-    }
+    } else {
-    else {
         if (db->metadata()->name().isEmpty()) {
             tabName = tr("New database");
-        }
+        } else {
-        else {
             tabName = QString("%1 [%2]").arg(db->metadata()->name(), tr("New database"));
         }
     }
@@ -756,17 +667,14 @@ void DatabaseTabWidget::lockDatabases()
         DatabaseWidget* dbWidget = static_cast<DatabaseWidget*>(widget(i));
         Database* db = databaseFromDatabaseWidget(dbWidget);
 
-        DatabaseWidget::Mode mode = dbWidget->currentMode();
+        if (dbWidget->currentMode() == DatabaseWidget::LockedMode) {
-
-        if ((mode != DatabaseWidget::ViewMode && mode != DatabaseWidget::EditMode)
-                || !dbWidget->dbHasKey()) {
             continue;
         }
 
         // show the correct tab widget before we are asking questions about it
         setCurrentWidget(dbWidget);
 
-        if (mode == DatabaseWidget::EditMode && dbWidget->isEditWidgetModified()) {
+        if (dbWidget->currentMode() == DatabaseWidget::EditMode && dbWidget->isEditWidgetModified()) {
             QMessageBox::StandardButton result =
                 MessageBox::question(
                     this, tr("Lock database"),
@@ -777,23 +685,7 @@ void DatabaseTabWidget::lockDatabases()
             }
         }
 
-
+        if (m_dbList[db].modified) {
-        if (m_dbList[db].modified && !m_dbList[db].saveToFilename) {
-            QMessageBox::StandardButton result =
-                MessageBox::question(
-                    this, tr("Lock database"),
-                    tr("This database has never been saved.\nYou can save the database or stop locking it."),
-                    QMessageBox::Save | QMessageBox::Cancel, QMessageBox::Cancel);
-            if (result == QMessageBox::Save) {
-                if (!saveDatabase(db)) {
-                    continue;
-                }
-            }
-            else if (result == QMessageBox::Cancel) {
-                continue;
-            }
-        }
-        else if (m_dbList[db].modified) {
             QMessageBox::StandardButton result =
                 MessageBox::question(
                     this, tr("Lock database"),
@@ -828,7 +720,7 @@ void DatabaseTabWidget::modified()
     Database* db = static_cast<Database*>(sender());
     DatabaseManagerStruct& dbStruct = m_dbList[db];
 
-    if (config()->get("AutoSaveAfterEveryChange").toBool() && dbStruct.saveToFilename) {
+    if (config()->get("AutoSaveAfterEveryChange").toBool() && !dbStruct.readOnly) {
         saveDatabase(db);
         return;
     }

src/gui/DatabaseTabWidget.h

@@ -21,6 +21,7 @@
 
 #include <QHash>
 #include <QTabWidget>
+#include <QFileInfo>
 
 #include "gui/DatabaseWidget.h"
 #include "gui/MessageWidget.h"
@@ -29,7 +30,6 @@ class DatabaseWidget;
 class DatabaseWidgetStateSync;
 class DatabaseOpenWidget;
 class QFile;
-class QLockFile;
 class MessageWidget;
 
 struct DatabaseManagerStruct
@@ -37,11 +37,7 @@ struct DatabaseManagerStruct
     DatabaseManagerStruct();
 
     DatabaseWidget* dbWidget;
-    QLockFile* lockFile;
+    QFileInfo fileInfo;
-    QString filePath;
-    QString canonicalFilePath;
-    QString fileName;
-    bool saveToFilename;
     bool modified;
     bool readOnly;
 };
@@ -106,7 +102,7 @@ private slots:
     void emitDatabaseUnlockedFromDbWidgetSender();
 
 private:
-    bool saveDatabase(Database* db);
+    bool saveDatabase(Database* db, QString filePath = "");
     bool saveDatabaseAs(Database* db);
     bool closeDatabase(Database* db);
     void deleteDatabase(Database* db);

src/gui/DatabaseWidget.cpp

@@ -1284,8 +1284,11 @@ void DatabaseWidget::reloadDatabaseFile()
         }
     } else {
         m_messageWidget->showMessage(
-            tr("Could not open the new database file while attempting to autoreload this database."),
+            tr("Could not open the new database file while attempting to autoreload this database.")
+                    .append("\n").append(file.errorString()),
             MessageWidget::Error);
+        // Mark db as modified since existing data may differ from file or file was deleted
+        m_db->modified();
     }
 
     // Rewatch the database file