From c2d04499abc21edef9567db5920a58f7870d3d28 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Sun, 26 Nov 2017 16:58:54 -0500 Subject: [PATCH] Remove lock file and cleanup file handling [#1002] --- src/gui/DatabaseTabWidget.cpp | 182 +++++++--------------------------- src/gui/DatabaseTabWidget.h | 10 +- src/gui/DatabaseWidget.cpp | 5 +- 3 files changed, 44 insertions(+), 153 deletions(-) diff --git a/src/gui/DatabaseTabWidget.cpp b/src/gui/DatabaseTabWidget.cpp index 8cade6965..8b36c0079 100644 --- a/src/gui/DatabaseTabWidget.cpp +++ b/src/gui/DatabaseTabWidget.cpp @@ -19,7 +19,6 @@ #include "DatabaseTabWidget.h" #include -#include #include #include @@ -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 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.saveToFilename = !dbStruct.readOnly; - - dbStruct.filePath = fileInfo.absoluteFilePath(); - dbStruct.canonicalFilePath = canonicalFilePath; - dbStruct.fileName = fileInfo.fileName(); + dbStruct.fileInfo = fileInfo; 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); - } - else { - dbStruct.dbWidget->switchToOpenDatabase(dbStruct.filePath); + dbStruct.dbWidget->switchToOpenDatabase(dbStruct.fileInfo.absoluteFilePath(), pw, keyFile); + } else { + dbStruct.dbWidget->switchToOpenDatabase(dbStruct.fileInfo.absoluteFilePath()); } 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; - QString filePath = dbStruct.filePath; + bool emitDatabaseWithFileClosed = dbStruct.fileInfo.exists() && !dbStruct.readOnly; + 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) { - oldFileName = dbStruct.filePath; + if (dbStruct.fileInfo.exists()) { + 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); - QString lockFilePath; - 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 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; + if (!saveDatabase(db, fileName)) { + // Failed to save, try again 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.canonicalFilePath = fileInfo.canonicalFilePath(); - dbStruct.fileName = fileInfo.fileName(); - dbStruct.dbWidget->updateFilename(dbStruct.filePath); - delete dbStruct.lockFile; - dbStruct.lockFile = lockFile.take(); + dbStruct.fileInfo = QFileInfo(fileName); + dbStruct.dbWidget->updateFilename(dbStruct.fileInfo.absoluteFilePath()); 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; - } - else { + tabName = dbStruct.fileInfo.fileName(); + } else { tabName = db->metadata()->name(); } - setTabToolTip(index, dbStruct.filePath); - } - else { + setTabToolTip(index, dbStruct.fileInfo.absoluteFilePath()); + } 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(widget(i)); Database* db = databaseFromDatabaseWidget(dbWidget); - DatabaseWidget::Mode mode = dbWidget->currentMode(); - - if ((mode != DatabaseWidget::ViewMode && mode != DatabaseWidget::EditMode) - || !dbWidget->dbHasKey()) { + if (dbWidget->currentMode() == DatabaseWidget::LockedMode) { 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 && !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) { + 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(sender()); DatabaseManagerStruct& dbStruct = m_dbList[db]; - if (config()->get("AutoSaveAfterEveryChange").toBool() && dbStruct.saveToFilename) { + if (config()->get("AutoSaveAfterEveryChange").toBool() && !dbStruct.readOnly) { saveDatabase(db); return; } diff --git a/src/gui/DatabaseTabWidget.h b/src/gui/DatabaseTabWidget.h index db237d98c..875c3c904 100644 --- a/src/gui/DatabaseTabWidget.h +++ b/src/gui/DatabaseTabWidget.h @@ -21,6 +21,7 @@ #include #include +#include #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; - QString filePath; - QString canonicalFilePath; - QString fileName; - bool saveToFilename; + QFileInfo fileInfo; 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); diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index 387d5d2a2..f17a7ef02 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/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