Skip to content

Commit

Permalink
Fix various issues with KeeShare
Browse files Browse the repository at this point in the history
* Fix #3790, shares now use the standard FileWatcher class to detect remote file changes using checksums and file system triggers.

* Fix #3895, macOS file selection no longer hangs the app.

* Restore saving of KeeShare settings accidentally removed by 596d2cf
  • Loading branch information
droidmonkey committed May 10, 2020
1 parent ce7b34e commit dcff507
Show file tree
Hide file tree
Showing 10 changed files with 104 additions and 372 deletions.
2 changes: 1 addition & 1 deletion src/core/Database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ Database::Database()
connect(&m_modifiedTimer, SIGNAL(timeout()), SIGNAL(databaseModified()));
connect(this, SIGNAL(databaseOpened()), SLOT(updateCommonUsernames()));
connect(this, SIGNAL(databaseSaved()), SLOT(updateCommonUsernames()));
connect(m_fileWatcher, SIGNAL(fileChanged()), SIGNAL(databaseFileChanged()));
connect(m_fileWatcher, &FileWatcher::fileChanged, this, &Database::databaseFileChanged);

m_modified = false;
m_emitModified = true;
Expand Down
198 changes: 2 additions & 196 deletions src/core/FileWatcher.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/*
* Copyright (C) 2011 Felix Geyer <[email protected]>
* Copyright (C) 2017 KeePassXC Team <[email protected]>
* Copyright (C) 2020 KeePassXC Team <[email protected]>
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand All @@ -19,26 +18,19 @@
#include "FileWatcher.h"

#include "core/AsyncTask.h"
#include "core/Clock.h"

#include <QCryptographicHash>
#include <QFileInfo>

#ifdef Q_OS_LINUX
#include <sys/vfs.h>
#endif

namespace
{
const int FileChangeDelay = 200;
} // namespace

FileWatcher::FileWatcher(QObject* parent)
: QObject(parent)
{
connect(&m_fileWatcher, SIGNAL(fileChanged(QString)), SLOT(checkFileChanged()));
connect(&m_fileChecksumTimer, SIGNAL(timeout()), SLOT(checkFileChanged()));
connect(&m_fileChangeDelayTimer, SIGNAL(timeout()), SIGNAL(fileChanged()));
connect(&m_fileChangeDelayTimer, &QTimer::timeout, this, [this] { emit fileChanged(m_filePath); });
m_fileChangeDelayTimer.setSingleShot(true);
m_fileIgnoreDelayTimer.setSingleShot(true);
}
Expand Down Expand Up @@ -151,189 +143,3 @@ QByteArray FileWatcher::calculateChecksum()
// prevents unnecessary merge requests on intermittent network shares
return m_fileChecksum;
}

BulkFileWatcher::BulkFileWatcher(QObject* parent)
: QObject(parent)
{
connect(&m_fileWatcher, SIGNAL(fileChanged(QString)), SLOT(handleFileChanged(QString)));
connect(&m_fileWatcher, SIGNAL(directoryChanged(QString)), SLOT(handleDirectoryChanged(QString)));
connect(&m_watchedFilesIgnoreTimer, SIGNAL(timeout()), this, SLOT(observeFileChanges()));
connect(&m_pendingSignalsTimer, SIGNAL(timeout()), this, SLOT(emitSignals()));
m_watchedFilesIgnoreTimer.setSingleShot(true);
m_pendingSignalsTimer.setSingleShot(true);
}

void BulkFileWatcher::clear()
{
for (const QString& path : m_fileWatcher.files() + m_fileWatcher.directories()) {
const QFileInfo info(path);
m_fileWatcher.removePath(info.absoluteFilePath());
m_fileWatcher.removePath(info.absolutePath());
}
m_watchedPaths.clear();
m_watchedFilesInDirectory.clear();
m_watchedFilesIgnored.clear();
}

void BulkFileWatcher::removePath(const QString& path)
{
const QFileInfo info(path);
const QString filePath = info.absoluteFilePath();
const QString directoryPath = info.absolutePath();
m_watchedFilesInDirectory[directoryPath].remove(filePath);
m_fileWatcher.removePath(filePath);
m_watchedPaths.remove(filePath);
if (m_watchedFilesInDirectory[directoryPath].isEmpty()) {
m_fileWatcher.removePath(directoryPath);
m_watchedPaths.remove(directoryPath);
m_watchedFilesInDirectory.remove(directoryPath);
}
}

void BulkFileWatcher::addPath(const QString& path)
{
const QFileInfo info(path);
const QString filePath = info.absoluteFilePath();
const QString directoryPath = info.absolutePath();
if (!m_watchedPaths.value(filePath)) {
const bool fileSuccess = m_fileWatcher.addPath(filePath);
m_watchedPaths[filePath] = fileSuccess;
}
if (!m_watchedPaths.value(directoryPath)) {
const bool directorySuccess = m_fileWatcher.addPath(directoryPath);
m_watchedPaths[directoryPath] = directorySuccess;
}
m_watchedFilesInDirectory[directoryPath][filePath] = info.exists() ? info.lastModified().toMSecsSinceEpoch() : 0;
}

void BulkFileWatcher::handleFileChanged(const QString& path)
{
const QFileInfo info(path);
const QString filePath = info.absoluteFilePath();
const QString directoryPath = info.absolutePath();
const QMap<QString, qint64>& watchedFiles = m_watchedFilesInDirectory[directoryPath];
const qint64 lastModificationTime = info.lastModified().toMSecsSinceEpoch();
const bool created = watchedFiles[filePath] == 0 && info.exists();
const bool deleted = watchedFiles[filePath] != 0 && !info.exists();
const bool changed = !created && !deleted && lastModificationTime != watchedFiles[filePath];

addPath(path);

if (m_watchedFilesIgnored[info.canonicalFilePath()] > Clock::currentDateTimeUtc()) {
// changes are blocked
return;
}
if (created) {
qDebug("File created %s", qPrintable(path));
scheduleSignal(Created, filePath);
}
if (changed) {
qDebug("File changed %s", qPrintable(path));
scheduleSignal(Updated, filePath);
}
if (deleted) {
qDebug("File removed %s", qPrintable(path));
scheduleSignal(Removed, filePath);
}
}

void BulkFileWatcher::handleDirectoryChanged(const QString& path)
{
qDebug("Directory changed %s", qPrintable(path));
const QFileInfo directoryInfo(path);
const QString directoryPath = directoryInfo.absoluteFilePath();
QMap<QString, qint64>& watchedFiles = m_watchedFilesInDirectory[directoryPath];
for (const QString& filename : watchedFiles.keys()) {
const QFileInfo fileInfo(filename);
const QString filePath = fileInfo.absoluteFilePath();
const qint64 previousModificationTime = watchedFiles[filePath];
const qint64 lastModificationTime = fileInfo.lastModified().toMSecsSinceEpoch();
if (!fileInfo.exists() && previousModificationTime != 0) {
qDebug("Remove watch file %s", qPrintable(fileInfo.absoluteFilePath()));
m_fileWatcher.removePath(filePath);
m_watchedPaths.remove(filePath);
watchedFiles.remove(filePath);
scheduleSignal(Removed, filePath);
}
if (previousModificationTime == 0 && fileInfo.exists()) {
qDebug("Add watch file %s", qPrintable(fileInfo.absoluteFilePath()));
if (!m_watchedPaths.value(filePath)) {
const bool success = m_fileWatcher.addPath(filePath);
m_watchedPaths[filePath] = success;
watchedFiles[filePath] = lastModificationTime;
}
scheduleSignal(Created, filePath);
}
if (fileInfo.exists() && previousModificationTime != lastModificationTime) {
// this case is handled using
qDebug("Refresh watch file %s", qPrintable(fileInfo.absoluteFilePath()));
m_fileWatcher.removePath(fileInfo.absolutePath());
m_fileWatcher.addPath(fileInfo.absolutePath());
scheduleSignal(Updated, filePath);
}
m_watchedFilesInDirectory[directoryPath][filePath] = fileInfo.exists() ? lastModificationTime : 0;
}
}

void BulkFileWatcher::emitSignals()
{
QMap<QString, QList<Signal>> queued;
m_pendingSignals.swap(queued);
for (const auto& path : queued.keys()) {
const auto& signal = queued[path];
if (signal.last() == Removed) {
qDebug("Emit %s removed", qPrintable(path));
emit fileRemoved(path);
continue;
}
if (signal.first() == Created) {
qDebug("Emit %s created", qPrintable(path));
emit fileCreated(path);
continue;
}
qDebug("Emit %s changed", qPrintable(path));
emit fileChanged(path);
}
}

void BulkFileWatcher::scheduleSignal(Signal signal, const QString& path)
{
// we need to collect signals since the file watcher API may send multiple signals for a "single" change
// therefore we wait until the event loop finished before starting to import any changes
const QString filePath = QFileInfo(path).absoluteFilePath();
m_pendingSignals[filePath] << signal;

if (!m_pendingSignalsTimer.isActive()) {
m_pendingSignalsTimer.start();
}
}

void BulkFileWatcher::ignoreFileChanges(const QString& path)
{
const QFileInfo info(path);
m_watchedFilesIgnored[info.canonicalFilePath()] = Clock::currentDateTimeUtc().addMSecs(FileChangeDelay);
}

void BulkFileWatcher::observeFileChanges(bool delayed)
{
int timeout = 0;
if (delayed) {
timeout = FileChangeDelay;
} else {
const QDateTime current = Clock::currentDateTimeUtc();
for (const QString& key : m_watchedFilesIgnored.keys()) {
if (m_watchedFilesIgnored[key] < current) {
// We assume that there was no concurrent change of the database
// during our block - so no need to reimport
qDebug("Remove block from %s", qPrintable(key));
m_watchedFilesIgnored.remove(key);
continue;
}
qDebug("Keep block from %s", qPrintable(key));
timeout = qMin(timeout, static_cast<int>(current.msecsTo(m_watchedFilesIgnored[key])));
}
}
if (timeout > 0 && !m_watchedFilesIgnoreTimer.isActive()) {
m_watchedFilesIgnoreTimer.start(timeout);
}
}
55 changes: 2 additions & 53 deletions src/core/FileWatcher.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (C) 2018 KeePassXC Team <[email protected]>
* Copyright (C) 2020 KeePassXC Team <[email protected]>
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand All @@ -19,9 +19,7 @@
#define KEEPASSXC_FILEWATCHER_H

#include <QFileSystemWatcher>
#include <QSet>
#include <QTimer>
#include <QVariant>

class FileWatcher : public QObject
{
Expand All @@ -37,7 +35,7 @@ class FileWatcher : public QObject
bool hasSameFileChecksum();

signals:
void fileChanged();
void fileChanged(const QString& path);

public slots:
void pause();
Expand All @@ -60,53 +58,4 @@ private slots:
bool m_ignoreFileChange = false;
};

class BulkFileWatcher : public QObject
{
Q_OBJECT

enum Signal
{
Created,
Updated,
Removed
};

public:
explicit BulkFileWatcher(QObject* parent = nullptr);

void clear();

void removePath(const QString& path);
void addPath(const QString& path);

void ignoreFileChanges(const QString& path);

signals:
void fileCreated(QString);
void fileChanged(QString);
void fileRemoved(QString);

public slots:
void observeFileChanges(bool delayed = false);

private slots:
void handleFileChanged(const QString& path);
void handleDirectoryChanged(const QString& path);
void emitSignals();

private:
void scheduleSignal(Signal event, const QString& path);

private:
QMap<QString, bool> m_watchedPaths;
QMap<QString, QDateTime> m_watchedFilesIgnored;
QFileSystemWatcher m_fileWatcher;
QMap<QString, QMap<QString, qint64>> m_watchedFilesInDirectory;
// needed for Import/Export-References to prevent update after self-write
QTimer m_watchedFilesIgnoreTimer;
// needed to tolerate multiple signals for same event
QTimer m_pendingSignalsTimer;
QMap<QString, QList<Signal>> m_pendingSignals;
};

#endif // KEEPASSXC_FILEWATCHER_H
48 changes: 18 additions & 30 deletions src/gui/FileDialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,20 @@

#include <QDir>

namespace
{
QString modFilter(const QString& filter)
{
#ifdef Q_OS_MACOS
// Fix macOS bug that causes the file dialog to freeze when a dot is included in the filters
// See https://github.com/keepassxreboot/keepassxc/issues/3895#issuecomment-586724167
auto mod = filter;
return mod.replace("*.", "*");
#endif
return filter;
}
} // namespace

FileDialog* FileDialog::m_instance(nullptr);

QString FileDialog::getOpenFileName(QWidget* parent,
Expand All @@ -37,7 +51,7 @@ QString FileDialog::getOpenFileName(QWidget* parent,
} else {
const auto& workingDir = dir.isEmpty() ? config()->get(Config::LastDir).toString() : dir;
const auto result = QDir::toNativeSeparators(
QFileDialog::getOpenFileName(parent, caption, workingDir, filter, selectedFilter, options));
QFileDialog::getOpenFileName(parent, caption, workingDir, modFilter(filter), selectedFilter, options));

#ifdef Q_OS_MACOS
// on Mac OS X the focus is lost after closing the native dialog
Expand All @@ -63,7 +77,8 @@ QStringList FileDialog::getOpenFileNames(QWidget* parent,
return results;
} else {
const auto& workingDir = dir.isEmpty() ? config()->get(Config::LastDir).toString() : dir;
auto results = QFileDialog::getOpenFileNames(parent, caption, workingDir, filter, selectedFilter, options);
auto results =
QFileDialog::getOpenFileNames(parent, caption, workingDir, modFilter(filter), selectedFilter, options);

for (auto& path : results) {
path = QDir::toNativeSeparators(path);
Expand All @@ -82,33 +97,6 @@ QStringList FileDialog::getOpenFileNames(QWidget* parent,
}
}

QString FileDialog::getFileName(QWidget* parent,
const QString& caption,
const QString& dir,
const QString& filter,
QString* selectedFilter,
const QFileDialog::Options options)
{
if (!m_nextFileName.isEmpty()) {
const QString result = m_nextFileName;
m_nextFileName.clear();
return result;
} else {
const auto& workingDir = dir.isEmpty() ? config()->get(Config::LastDir).toString() : dir;
const auto result = QDir::toNativeSeparators(
QFileDialog::getSaveFileName(parent, caption, workingDir, filter, selectedFilter, options));

#ifdef Q_OS_MACOS
// on Mac OS X the focus is lost after closing the native dialog
if (parent) {
parent->activateWindow();
}
#endif
saveLastDir(result);
return result;
}
}

QString FileDialog::getSaveFileName(QWidget* parent,
const QString& caption,
const QString& dir,
Expand All @@ -123,7 +111,7 @@ QString FileDialog::getSaveFileName(QWidget* parent,
} else {
const auto& workingDir = dir.isEmpty() ? config()->get(Config::LastDir).toString() : dir;
const auto result = QDir::toNativeSeparators(
QFileDialog::getSaveFileName(parent, caption, workingDir, filter, selectedFilter, options));
QFileDialog::getSaveFileName(parent, caption, workingDir, modFilter(filter), selectedFilter, options));

#ifdef Q_OS_MACOS
// on Mac OS X the focus is lost after closing the native dialog
Expand Down
Loading

0 comments on commit dcff507

Please sign in to comment.