Git-Mirrors/keepassxc
1
0
Fork 0
mirror of https://github.com/keepassxreboot/keepassxc.git synced 2025-02-18 13:24:10 -05:00
Code Issues Packages Projects Releases Wiki Activity

FdoSecrets: simplify collection internal states

Browse Source 
This gets rid of the m_registered state, so whenever there is a valid m_backend, it is guaranteed to be registered already.

While at it, this commit also improves DBusObject::registerWithPath a little bit by allowing properly registering multiple paths using the same adaptor, mostly for supporting Collection aliases.

Now when DBus registration fails, the code does not go into an inconsistent state or crash.
This commit is contained in:
Aetf 2020-11-03 13:06:04 -05:00
parent f5caf3968f
commit 804a3b6706
13 changed files with 94 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
src/fdosecrets/FdoSecretsPlugin.cpp
View File

@ -104,6 +104,7 @@ void FdoSecretsPlugin::emitRequestShowNotification(const QString& msg, const QSt
void FdoSecretsPlugin::emitError(const QString& msg)
{
emit error(tr("<b>Fdo Secret Service:</b> %1").arg(msg));
qDebug() << msg;
}
QString FdoSecretsPlugin::reportExistingService() const

59
src/fdosecrets/objects/Collection.cpp
View File

@ -48,14 +48,13 @@ namespace FdoSecrets
}
Collection::Collection(Service* parent, DatabaseWidget* backend)
: DBusObject(parent)
: DBusObjectHelper(parent)
, m_backend(backend)
, m_exposedGroup(nullptr)
, m_registered(false)
{
// whenever the file path or the database object itself change, we do a full reload.
connect(backend, &DatabaseWidget::databaseFilePathChanged, this, &Collection::reloadBackend);
connect(backend, &DatabaseWidget::databaseReplaced, this, &Collection::reloadBackend);
connect(backend, &DatabaseWidget::databaseFilePathChanged, this, &Collection::reloadBackendOrDelete);
connect(backend, &DatabaseWidget::databaseReplaced, this, &Collection::reloadBackendOrDelete);
// also remember to clear/populate the database when lock state changes.
connect(backend, &DatabaseWidget::databaseUnlocked, this, &Collection::onDatabaseLockChanged);
@ -72,33 +71,24 @@ namespace FdoSecrets
bool Collection::reloadBackend()
{
if (m_registered) {
// delete all items
// this has to be done because the backend is actually still there, just we don't expose them
// NOTE: Do NOT use a for loop, because Item::doDelete will remove itself from m_items.
while (!m_items.isEmpty()) {
m_items.first()->doDelete();
}
cleanupConnections();
unregisterCurrentPath();
m_registered = false;
}
Q_ASSERT(m_backend);
// delete all items
// this has to be done because the backend is actually still there, just we don't expose them
// NOTE: Do NOT use a for loop, because Item::doDelete will remove itself from m_items.
while (!m_items.isEmpty()) {
m_items.first()->doDelete();
}
cleanupConnections();
unregisterPrimaryPath();
// make sure we have updated copy of the filepath, which is used to identify the database.
m_backendPath = m_backend->database()->filePath();
// the database may not have a name (derived from filePath) yet, which may happen if it's newly created.
// defer the registration to next time a file path change happens.
if (!name().isEmpty()) {
auto path = QStringLiteral(DBUS_PATH_TEMPLATE_COLLECTION).arg(p()->objectPath().path(), encodePath(name()));
if (!registerWithPath(path, new CollectionAdaptor(this))) {
service()->plugin()->emitError(tr("Failed to register database on DBus under the name '%1'").arg(name()));
return false;
}
m_registered = true;
auto path = QStringLiteral(DBUS_PATH_TEMPLATE_COLLECTION).arg(p()->objectPath().path(), encodePath(name()));
if (!registerWithPath(path)) {
service()->plugin()->emitError(tr("Failed to register database on DBus under the name '%1'").arg(name()));
return false;
}
// populate contents after expose on dbus, because items rely on parent's dbus object path
@ -111,6 +101,13 @@ namespace FdoSecrets
return true;
}
void Collection::reloadBackendOrDelete()
{
if (!reloadBackend()) {
doDelete();
}
}
DBusReturn<void> Collection::ensureBackend() const
{
if (!m_backend) {
@ -418,8 +415,7 @@ namespace FdoSecrets
emit aliasAboutToAdd(alias);
bool ok = QDBusConnection::sessionBus().registerObject(
QStringLiteral(DBUS_PATH_TEMPLATE_ALIAS).arg(p()->objectPath().path(), alias), this);
bool ok = registerWithPath(QStringLiteral(DBUS_PATH_TEMPLATE_ALIAS).arg(p()->objectPath().path(), alias), false);
if (ok) {
m_aliases.insert(alias);
emit aliasAdded(alias);
@ -454,6 +450,7 @@ namespace FdoSecrets
QString Collection::name() const
{
// todo: make sure the name is never empty
if (m_backendPath.isEmpty()) {
// This is a newly created db without saving to file.
// This name is also used to register dbus path.
@ -486,7 +483,7 @@ namespace FdoSecrets
void Collection::populateContents()
{
if (!m_registered) {
if (!m_backend) {
return;
}
@ -645,7 +642,7 @@ namespace FdoSecrets
emit collectionAboutToDelete();
unregisterCurrentPath();
unregisterPrimaryPath();
// remove alias manually to trigger signal
for (const auto& a : aliases()) {
@ -663,6 +660,8 @@ namespace FdoSecrets
// reset backend and delete self
m_backend = nullptr;
deleteLater();
// items will be removed automatically as they are children objects
}
void Collection::cleanupConnections()

8
src/fdosecrets/objects/Collection.h
View File

@ -36,7 +36,7 @@ namespace FdoSecrets
class Item;
class PromptBase;
class Service;
class Collection : public DBusObject
class Collection : public DBusObjectHelper<Collection, CollectionAdaptor>
{
Q_OBJECT
@ -124,9 +124,13 @@ namespace FdoSecrets
private slots:
void onDatabaseLockChanged();
void onDatabaseExposedGroupChanged();
// force reload info from backend, potentially delete self
bool reloadBackend();
// calls reloadBackend, delete self when error
void reloadBackendOrDelete();
private:
friend class DeleteCollectionPrompt;
friend class CreateCollectionPrompt;
@ -165,8 +169,6 @@ namespace FdoSecrets
QSet<QString> m_aliases;
QList<Item*> m_items;
QMap<const Entry*, Item*> m_entryToItem;
bool m_registered;
};
} // namespace FdoSecrets

19
src/fdosecrets/objects/DBusObject.cpp
View File

@ -17,15 +17,12 @@
#include "DBusObject.h"
#include <QDBusAbstractAdaptor>
#include <QFile>
#include <QRegularExpression>
#include <QTextStream>
#include <QUrl>
#include <QUuid>
#include <utility>
namespace FdoSecrets
{
@ -35,14 +32,13 @@ namespace FdoSecrets
{
}
bool DBusObject::registerWithPath(const QString& path, QDBusAbstractAdaptor* adaptor)
bool DBusObject::registerWithPath(const QString& path, bool primary)
{
Q_ASSERT(!m_dbusAdaptor);
if (primary) {
m_objectPath.setPath(path);
}
m_objectPath.setPath(path);
m_dbusAdaptor = adaptor;
adaptor->setParent(this);
return QDBusConnection::sessionBus().registerObject(m_objectPath.path(), this);
return QDBusConnection::sessionBus().registerObject(path, this);
}
QString DBusObject::callingPeerName() const
@ -58,7 +54,10 @@ namespace FdoSecrets
QString encodePath(const QString& value)
{
// force "-.~_" to be encoded
// toPercentEncoding encodes everything except those in the unreserved group:
// ALPHA / DIGIT / "-" / "." / "_" / "~"
// we want only ALPHA / DIGIT / "_", with "_" as the escape character
// so force "-.~_" to be encoded
return QUrl::toPercentEncoding(value, "", "-.~_").replace('%', '_');
}

42
src/fdosecrets/objects/DBusObject.h
View File

@ -21,6 +21,7 @@
#include "fdosecrets/objects/DBusReturn.h"
#include "fdosecrets/objects/DBusTypes.h"
#include <QDBusAbstractAdaptor>
#include <QDBusConnection>
#include <QDBusConnectionInterface>
#include <QDBusContext>
@ -31,21 +32,19 @@
#include <QObject>
#include <QScopedPointer>
class QDBusAbstractAdaptor;
namespace FdoSecrets
{
class Service;
/**
* @brief A common base class for all dbus-exposed objects
* @brief A common base class for all dbus-exposed objects.
* However, derived class should inherit from `DBusObjectHelper`, which is
* the only way to set DBus adaptor and enforces correct adaptor creation.
*/
class DBusObject : public QObject, public QDBusContext
{
Q_OBJECT
public:
explicit DBusObject(DBusObject* parent = nullptr);
const QDBusObjectPath& objectPath() const
{
return m_objectPath;
@ -57,12 +56,21 @@ namespace FdoSecrets
}
protected:
bool registerWithPath(const QString& path, QDBusAbstractAdaptor* adaptor);
/**
* @brief Register this object at given DBus path
* @param path DBus path to register at
* @param primary whether this path to be considered primary. The primary path is the one to be returned by
* `DBusObject::objectPath`.
* @return true on success
*/
bool registerWithPath(const QString& path, bool primary = true);
void unregisterCurrentPath()
void unregisterPrimaryPath()
{
if (m_objectPath.path() == QStringLiteral("/")) {
return;
}
QDBusConnection::sessionBus().unregisterObject(m_objectPath.path());
m_dbusAdaptor = nullptr;
m_objectPath.setPath(QStringLiteral("/"));
}
@ -85,6 +93,8 @@ namespace FdoSecrets
}
private:
explicit DBusObject(DBusObject* parent);
/**
* Derived class should not directly use sendErrorReply.
* Instead, use raiseError
@ -92,12 +102,26 @@ namespace FdoSecrets
using QDBusContext::sendErrorReply;
template <typename U> friend class DBusReturn;
template <typename Object, typename Adaptor> friend class DBusObjectHelper;
private:
QDBusAbstractAdaptor* m_dbusAdaptor;
QDBusObjectPath m_objectPath;
};
template <typename Object, typename Adaptor> class DBusObjectHelper : public DBusObject
{
protected:
explicit DBusObjectHelper(DBusObject* parent)
: DBusObject(parent)
{
// creating new Adaptor has to be delayed into constructor's body,
// and can't be simply moved to initializer list, because at that
// point the base QObject class hasn't been initialized and will sigfault.
m_dbusAdaptor = new Adaptor(static_cast<Object*>(this));
m_dbusAdaptor->setParent(this);
}
};
/**
* Return the object path of the pointed DBusObject, or "/" if the pointer is null
* @tparam T

6
src/fdosecrets/objects/Item.cpp
View File

@ -60,7 +60,7 @@ namespace FdoSecrets
}
Item::Item(Collection* parent, Entry* backend)
: DBusObject(parent)
: DBusObjectHelper(parent)
, m_backend(backend)
{
Q_ASSERT(!p()->objectPath().path().isEmpty());
@ -71,7 +71,7 @@ namespace FdoSecrets
bool Item::registerSelf()
{
auto path = QStringLiteral(DBUS_PATH_TEMPLATE_ITEM).arg(p()->objectPath().path(), m_backend->uuidToHex());
bool ok = registerWithPath(path, new ItemAdaptor(this));
bool ok = registerWithPath(path);
if (!ok) {
service()->plugin()->emitError(tr("Failed to register item on DBus at path '%1'").arg(path));
}
@ -340,7 +340,7 @@ namespace FdoSecrets
// Unregister current path early, do not rely on deleteLater's call to destructor
// as in case of Entry moving between groups, new Item will be created at the same DBus path
// before the current Item is deleted in the event loop.
unregisterCurrentPath();
unregisterPrimaryPath();
m_backend = nullptr;
deleteLater();

3
src/fdosecrets/objects/Item.h
View File

@ -38,7 +38,7 @@ namespace FdoSecrets
class Collection;
class PromptBase;
class Item : public DBusObject
class Item : public DBusObjectHelper<Item, ItemAdaptor>
{
Q_OBJECT
@ -100,7 +100,6 @@ namespace FdoSecrets
public slots:
void doDelete();
private:
/**
* @brief Register self on DBus
* @return

4
src/fdosecrets/objects/Prompt.cpp
View File

@ -34,7 +34,7 @@ namespace FdoSecrets
{
PromptBase::PromptBase(Service* parent)
: DBusObject(parent)
: DBusObjectHelper(parent)
{
connect(this, &PromptBase::completed, this, &PromptBase::deleteLater);
}
@ -42,7 +42,7 @@ namespace FdoSecrets
bool PromptBase::registerSelf()
{
auto path = QStringLiteral(DBUS_PATH_TEMPLATE_PROMPT).arg(p()->objectPath().path(), Tools::uuidToHex(QUuid::createUuid()));
bool ok = registerWithPath(path, new PromptAdaptor(this));
bool ok = registerWithPath(path);
if (!ok) {
service()->plugin()->emitError(tr("Failed to register item on DBus at path '%1'").arg(path));
}

2
src/fdosecrets/objects/Prompt.h
View File

@ -32,7 +32,7 @@ namespace FdoSecrets
class Service;
class PromptBase : public DBusObject
class PromptBase : public DBusObjectHelper<PromptBase, PromptAdaptor>
{
Q_OBJECT
public:

12
src/fdosecrets/objects/Service.cpp
View File

@ -51,7 +51,7 @@ namespace FdoSecrets
Service::Service(FdoSecretsPlugin* plugin,
QPointer<DatabaseTabWidget> dbTabs) // clazy: exclude=ctor-missing-parent-argument
: DBusObject(nullptr)
: DBusObjectHelper(nullptr)
, m_plugin(plugin)
, m_databases(std::move(dbTabs))
, m_insideEnsureDefaultAlias(false)
@ -74,7 +74,7 @@ namespace FdoSecrets
return false;
}
if (!registerWithPath(QStringLiteral(DBUS_PATH_SECRETS), new ServiceAdaptor(this))) {
if (!registerWithPath(QStringLiteral(DBUS_PATH_SECRETS))) {
plugin()->emitError(tr("Failed to register DBus path %1.<br/>").arg(QStringLiteral(DBUS_PATH_SECRETS)));
return false;
}
@ -112,12 +112,12 @@ namespace FdoSecrets
// When the Collection finds that no exposed group, it will delete itself.
// Thus the service also needs to monitor it and recreate the collection if the user changes
// from no exposed to exposed something.
connect(dbWidget, &DatabaseWidget::databaseReplaced, this, [this, dbWidget]() {
monitorDatabaseExposedGroup(dbWidget);
});
if (!dbWidget->isLocked()) {
monitorDatabaseExposedGroup(dbWidget);
}
connect(dbWidget, &DatabaseWidget::databaseUnlocked, this, [this, dbWidget]() {
monitorDatabaseExposedGroup(dbWidget);
});
auto coll = Collection::Create(this, dbWidget);
if (!coll) {
@ -129,7 +129,7 @@ namespace FdoSecrets
m_collections << coll;
m_dbToCollection[dbWidget] = coll;
// handle alias
// keep record of alias
connect(coll, &Collection::aliasAboutToAdd, this, &Service::onCollectionAliasAboutToAdd);
connect(coll, &Collection::aliasAdded, this, &Service::onCollectionAliasAdded);
connect(coll, &Collection::aliasRemoved, this, &Service::onCollectionAliasRemoved);

2
src/fdosecrets/objects/Service.h
View File

@ -45,7 +45,7 @@ namespace FdoSecrets
class ServiceAdaptor;
class Session;
class Service : public DBusObject // clazy: exclude=ctor-missing-parent-argument
class Service : public DBusObjectHelper<Service, ServiceAdaptor> // clazy: exclude=ctor-missing-parent-argument
{
Q_OBJECT

4
src/fdosecrets/objects/Session.cpp
View File

@ -38,7 +38,7 @@ namespace FdoSecrets
}
Session::Session(std::unique_ptr<CipherPair>&& cipher, const QString& peer, Service* parent)
: DBusObject(parent)
: DBusObjectHelper(parent)
, m_cipher(std::move(cipher))
, m_peer(peer)
, m_id(QUuid::createUuid())
@ -48,7 +48,7 @@ namespace FdoSecrets
bool Session::registerSelf()
{
auto path = QStringLiteral(DBUS_PATH_TEMPLATE_SESSION).arg(p()->objectPath().path(), id());
bool ok = registerWithPath(path, new SessionAdaptor(this));
bool ok = registerWithPath(path);
if (!ok) {
service()->plugin()->emitError(tr("Failed to register session on DBus at path '%1'").arg(path));
}

2
src/fdosecrets/objects/Session.h
View File

@ -34,7 +34,7 @@ namespace FdoSecrets
{
class CipherPair;
class Session : public DBusObject
class Session : public DBusObjectHelper<Session, SessionAdaptor>
{
Q_OBJECT