From 0c587999c6863b20cfc1ddbc602c4901c2a0e995 Mon Sep 17 00:00:00 2001 From: Jonathan White Date: Mon, 18 Feb 2019 08:26:56 -0500 Subject: [PATCH] Code quality updates for 2.4.0 (#2709) * Minor code quality fixes found by Codacy * Fix unused variables when WITH_XC_NETWORKING is OFF * Fix #2684, resolve entry references from the root group * Fix #2697 and Fix #2699, listen specifically for WM_QUERYENDSESSION and WM_ENDSESSION on Windows to gracefully shutdown KeePassXC * Cleanup proxy code and add explicit closure for shutdown messages --- src/browser/NativeMessagingBase.h | 2 +- src/browser/NativeMessagingHost.h | 2 +- src/core/Entry.cpp | 2 +- src/core/OSEventFilter.cpp | 38 +++++++++++++++++++++++---- src/core/OSEventFilter.h | 18 +++++++++++++ src/crypto/kdf/Kdf.cpp | 3 ++- src/crypto/ssh/BinaryStream.cpp | 24 +++-------------- src/crypto/ssh/BinaryStream.h | 10 +++---- src/gui/MainWindow.cpp | 6 ++++- src/keeshare/ShareObserver.cpp | 2 +- src/proxy/NativeMessagingHost.cpp | 6 ++--- src/proxy/NativeMessagingHost.h | 10 ++++--- src/proxy/keepassxc-proxy.cpp | 18 ++++++++++++- src/streams/SymmetricCipherStream.cpp | 1 + src/updatecheck/UpdateChecker.cpp | 1 + 15 files changed, 97 insertions(+), 46 deletions(-) diff --git a/src/browser/NativeMessagingBase.h b/src/browser/NativeMessagingBase.h index 12e551665..7a099a4ac 100644 --- a/src/browser/NativeMessagingBase.h +++ b/src/browser/NativeMessagingBase.h @@ -53,7 +53,7 @@ protected slots: protected: virtual void readLength() = 0; virtual bool readStdIn(const quint32 length) = 0; - void readNativeMessages(); + virtual void readNativeMessages(); QString jsonToString(const QJsonObject& json) const; void sendReply(const QJsonObject& json); void sendReply(const QString& reply); diff --git a/src/browser/NativeMessagingHost.h b/src/browser/NativeMessagingHost.h index 30a67378a..9ce1dab60 100644 --- a/src/browser/NativeMessagingHost.h +++ b/src/browser/NativeMessagingHost.h @@ -32,7 +32,7 @@ class NativeMessagingHost : public NativeMessagingBase public: explicit NativeMessagingHost(DatabaseTabWidget* parent = nullptr, const bool enabled = false); - ~NativeMessagingHost(); + ~NativeMessagingHost() override; int init(); void run(); void stop(); diff --git a/src/core/Entry.cpp b/src/core/Entry.cpp index 327fdc425..2ad73b055 100644 --- a/src/core/Entry.cpp +++ b/src/core/Entry.cpp @@ -926,7 +926,7 @@ QString Entry::resolveReferencePlaceholderRecursive(const QString& placeholder, Q_ASSERT(m_group); Q_ASSERT(m_group->database()); - const Entry* refEntry = m_group->findEntryBySearchTerm(searchText, searchInType); + const Entry* refEntry = m_group->database()->rootGroup()->findEntryBySearchTerm(searchText, searchInType); if (refEntry) { const QString wantedField = match.captured(EntryAttributes::WantedFieldGroupName); diff --git a/src/core/OSEventFilter.cpp b/src/core/OSEventFilter.cpp index f6ad6c76a..ea18aacc1 100644 --- a/src/core/OSEventFilter.cpp +++ b/src/core/OSEventFilter.cpp @@ -1,8 +1,30 @@ +/* + * Copyright (C) 2013 Felix Geyer + * Copyright (C) 2018 KeePassXC Team + * + * 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 + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + #include "OSEventFilter.h" #include +#include "gui/MainWindow.h" #include "autotype/AutoType.h" +#ifdef Q_OS_WIN +#include +#endif OSEventFilter::OSEventFilter() { @@ -15,12 +37,18 @@ bool OSEventFilter::nativeEventFilter(const QByteArray& eventType, void* message #if defined(Q_OS_UNIX) if (eventType == QByteArrayLiteral("xcb_generic_event_t")) { #elif defined(Q_OS_WIN) - if (eventType == QByteArrayLiteral("windows_generic_MSG") - || eventType == QByteArrayLiteral("windows_dispatcher_MSG")) { + auto winmsg = static_cast(message); + if (winmsg->message == WM_QUERYENDSESSION) { + *result = 1; + return true; + } else if (winmsg->message == WM_ENDSESSION) { + getMainWindow()->appExit(); + *result = 0; + return true; + } else if (eventType == QByteArrayLiteral("windows_generic_MSG") + || eventType == QByteArrayLiteral("windows_dispatcher_MSG")) { #endif - int retCode = autoType()->callEventFilter(message); - - return retCode == 1; + return autoType()->callEventFilter(message) == 1; } return false; diff --git a/src/core/OSEventFilter.h b/src/core/OSEventFilter.h index a27ade713..10434c0c2 100644 --- a/src/core/OSEventFilter.h +++ b/src/core/OSEventFilter.h @@ -1,3 +1,21 @@ +/* + * Copyright (C) 2013 Felix Geyer + * Copyright (C) 2018 KeePassXC Team + * + * 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 + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + #ifndef OSEVENTFILTER_H #define OSEVENTFILTER_H #include diff --git a/src/crypto/kdf/Kdf.cpp b/src/crypto/kdf/Kdf.cpp index 797723193..b4c4427c8 100644 --- a/src/crypto/kdf/Kdf.cpp +++ b/src/crypto/kdf/Kdf.cpp @@ -84,7 +84,8 @@ int Kdf::benchmark(int msec) const } Kdf::BenchmarkThread::BenchmarkThread(int msec, const Kdf* kdf) - : m_msec(msec) + : m_rounds(1) + , m_msec(msec) , m_kdf(kdf) { } diff --git a/src/crypto/ssh/BinaryStream.cpp b/src/crypto/ssh/BinaryStream.cpp index 2af04cee5..3ba4c1a81 100644 --- a/src/crypto/ssh/BinaryStream.cpp +++ b/src/crypto/ssh/BinaryStream.cpp @@ -19,12 +19,6 @@ #include "BinaryStream.h" #include -BinaryStream::BinaryStream(QObject* parent) - : QObject(parent) - , m_timeout(-1) -{ -} - BinaryStream::BinaryStream(QIODevice* device) : QObject(device) , m_timeout(-1) @@ -36,7 +30,10 @@ BinaryStream::BinaryStream(QByteArray* ba, QObject* parent) : QObject(parent) , m_timeout(-1) { - setData(ba); + m_buffer.reset(new QBuffer(ba)); + m_buffer->open(QIODevice::ReadWrite); + + m_device = m_buffer.data(); } BinaryStream::~BinaryStream() @@ -53,19 +50,6 @@ QIODevice* BinaryStream::device() const return m_device; } -void BinaryStream::setDevice(QIODevice* device) -{ - m_device = device; -} - -void BinaryStream::setData(QByteArray* ba) -{ - m_buffer.reset(new QBuffer(ba)); - m_buffer->open(QIODevice::ReadWrite); - - m_device = m_buffer.data(); -} - void BinaryStream::setTimeout(int timeout) { m_timeout = timeout; diff --git a/src/crypto/ssh/BinaryStream.h b/src/crypto/ssh/BinaryStream.h index 8f4155b65..6f95039ec 100644 --- a/src/crypto/ssh/BinaryStream.h +++ b/src/crypto/ssh/BinaryStream.h @@ -26,16 +26,14 @@ class BinaryStream : QObject { Q_OBJECT + Q_DISABLE_COPY(BinaryStream) public: - BinaryStream(QObject* parent = nullptr); - BinaryStream(QIODevice* device); - BinaryStream(QByteArray* ba, QObject* parent = nullptr); - ~BinaryStream(); + explicit BinaryStream(QIODevice* device); + explicit BinaryStream(QByteArray* ba, QObject* parent = nullptr); + ~BinaryStream() override; const QString errorString() const; QIODevice* device() const; - void setDevice(QIODevice* device); - void setData(QByteArray* ba); void setTimeout(int timeout); bool read(QByteArray& ba); diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index b3a5a9aad..8a851dd39 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -76,7 +76,7 @@ class BrowserPlugin : public ISettingsPage { public: - BrowserPlugin(DatabaseTabWidget* tabWidget) + explicit BrowserPlugin(DatabaseTabWidget* tabWidget) { m_nativeMessagingHost = QSharedPointer(new NativeMessagingHost(tabWidget, browserSettings()->isEnabled())); @@ -713,6 +713,10 @@ void MainWindow::hasUpdateAvailable(bool hasUpdate, const QString& version, bool updateCheckDialog->showUpdateCheckResponse(hasUpdate, version); updateCheckDialog->show(); } +#else + Q_UNUSED(hasUpdate) + Q_UNUSED(version) + Q_UNUSED(isManuallyRequested) #endif } diff --git a/src/keeshare/ShareObserver.cpp b/src/keeshare/ShareObserver.cpp index 89352c8ab..18ef808bb 100644 --- a/src/keeshare/ShareObserver.cpp +++ b/src/keeshare/ShareObserver.cpp @@ -787,7 +787,7 @@ ShareObserver::Result::Result(const QString& path, ShareObserver::Result::Type t bool ShareObserver::Result::isValid() const { - return !path.isEmpty() || !message.isEmpty() || !message.isEmpty() || !message.isEmpty(); + return !path.isEmpty() || !message.isEmpty(); } bool ShareObserver::Result::isError() const diff --git a/src/proxy/NativeMessagingHost.cpp b/src/proxy/NativeMessagingHost.cpp index 60f5d79ed..3c401e4c9 100644 --- a/src/proxy/NativeMessagingHost.cpp +++ b/src/proxy/NativeMessagingHost.cpp @@ -19,7 +19,7 @@ #include #ifdef Q_OS_WIN -#include +#include #endif NativeMessagingHost::NativeMessagingHost() @@ -36,14 +36,12 @@ NativeMessagingHost::NativeMessagingHost() } #ifdef Q_OS_WIN m_running.store(true); - m_future = - QtConcurrent::run(this, static_cast(&NativeMessagingHost::readNativeMessages)); + m_future = QtConcurrent::run(this, &NativeMessagingHost::readNativeMessages); #endif connect(m_localSocket, SIGNAL(readyRead()), this, SLOT(newLocalMessage())); connect(m_localSocket, SIGNAL(disconnected()), this, SLOT(deleteSocket())); connect(m_localSocket, SIGNAL(stateChanged(QLocalSocket::LocalSocketState)), - this, SLOT(socketStateChanged(QLocalSocket::LocalSocketState))); } diff --git a/src/proxy/NativeMessagingHost.h b/src/proxy/NativeMessagingHost.h index 083e12d48..5bedd9de5 100644 --- a/src/proxy/NativeMessagingHost.h +++ b/src/proxy/NativeMessagingHost.h @@ -25,7 +25,7 @@ class NativeMessagingHost : public NativeMessagingBase Q_OBJECT public: NativeMessagingHost(); - ~NativeMessagingHost(); + ~NativeMessagingHost() override; public slots: void newLocalMessage(); @@ -33,12 +33,14 @@ public slots: void socketStateChanged(QLocalSocket::LocalSocketState socketState); private: - void readNativeMessages(); - void readLength(); - bool readStdIn(const quint32 length); + void readNativeMessages() override; + void readLength() override; + bool readStdIn(const quint32 length) override; private: QLocalSocket* m_localSocket; + + Q_DISABLE_COPY(NativeMessagingHost) }; #endif // NATIVEMESSAGINGHOST_H diff --git a/src/proxy/keepassxc-proxy.cpp b/src/proxy/keepassxc-proxy.cpp index 0d0fbfb23..f9a657217 100644 --- a/src/proxy/keepassxc-proxy.cpp +++ b/src/proxy/keepassxc-proxy.cpp @@ -55,13 +55,29 @@ void catchUnixSignals(std::initializer_list quitSignals) sigaction(sig, &sa, nullptr); } } +#else +#include + +BOOL WINAPI ConsoleHandler(DWORD dwType) +{ + switch (dwType) { + case CTRL_C_EVENT: + case CTRL_SHUTDOWN_EVENT: + case CTRL_LOGOFF_EVENT: + QCoreApplication::quit(); + break; + } + return TRUE; +} #endif int main(int argc, char* argv[]) { QCoreApplication a(argc, argv); -#if defined(Q_OS_UNIX) || defined(Q_OS_LINUX) +#ifndef Q_OS_WIN catchUnixSignals({SIGQUIT, SIGINT, SIGTERM, SIGHUP}); +#else + SetConsoleCtrlHandler(static_cast(ConsoleHandler),TRUE); #endif NativeMessagingHost host; return a.exec(); diff --git a/src/streams/SymmetricCipherStream.cpp b/src/streams/SymmetricCipherStream.cpp index f6957622d..b930d8023 100644 --- a/src/streams/SymmetricCipherStream.cpp +++ b/src/streams/SymmetricCipherStream.cpp @@ -28,6 +28,7 @@ SymmetricCipherStream::SymmetricCipherStream(QIODevice* baseDevice, , m_error(false) , m_isInitialized(false) , m_dataWritten(false) + , m_streamCipher(false) { } diff --git a/src/updatecheck/UpdateChecker.cpp b/src/updatecheck/UpdateChecker.cpp index ff10821bb..2dafcea3e 100644 --- a/src/updatecheck/UpdateChecker.cpp +++ b/src/updatecheck/UpdateChecker.cpp @@ -28,6 +28,7 @@ UpdateChecker::UpdateChecker(QObject* parent) : QObject(parent) , m_netMgr(new QNetworkAccessManager(this)) , m_reply(nullptr) + , m_isManuallyRequested(false) { }