From ffd8d3e6b90e3729fe7fa1b00f97779242aaaa70 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Sat, 19 Oct 2024 09:37:59 -0700 Subject: [PATCH] QtDBus: disallow sending method calls without destinations on a bus QtDBus has allowed this because the bus would stop the message and return it to us as an UnknownMethod. But it makes no sense to send it, so let's block it early. For practical purposes, this allows the tst_qdbusmarshall test to continue working regardless of which daemon is running the bus. The message it was checking against only came from dbus-daemon; for users of systems now running dbus-broker (like my openSUSE Tumbleweed) the message was different and the test was failing. Change-Id: Ia134ca414080cf243974fffd913fdad09d80cc60 Reviewed-by: Ahmad Samir (cherry picked from commit d19db44d3f8a0ca0c69c6861e2a871de5ae7d106) Reviewed-by: Qt Cherry-pick Bot --- src/dbus/qdbusconnection.cpp | 4 +++- src/dbus/qdbusconnection_p.h | 9 ++++++++- src/dbus/qdbusmessage.cpp | 7 +++++-- tests/auto/dbus/qdbusmarshall/tst_qdbusmarshall.cpp | 5 ++--- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/dbus/qdbusconnection.cpp b/src/dbus/qdbusconnection.cpp index f6918b70b02..14c9ebe6205 100644 --- a/src/dbus/qdbusconnection.cpp +++ b/src/dbus/qdbusconnection.cpp @@ -873,7 +873,9 @@ QString QDBusConnection::name() const */ QDBusConnection::ConnectionCapabilities QDBusConnection::connectionCapabilities() const { - return d ? d->connectionCapabilities() : ConnectionCapabilities(); + if (!d) + return {}; + return d->connectionCapabilities() & ~QDBusConnectionPrivate::InternalCapabilitiesMask; } /*! diff --git a/src/dbus/qdbusconnection_p.h b/src/dbus/qdbusconnection_p.h index 53e68748186..5cb175c1cdc 100644 --- a/src/dbus/qdbusconnection_p.h +++ b/src/dbus/qdbusconnection_p.h @@ -276,11 +276,18 @@ signals: void callWithCallbackFailed(const QDBusError &error, const QDBusMessage &message); public: + static constexpr QDBusConnection::ConnectionCapabilities ConnectionIsBus = + QDBusConnection::ConnectionCapabilities::fromInt(0x8000'0000); + static constexpr QDBusConnection::ConnectionCapabilities InternalCapabilitiesMask = ConnectionIsBus; + QAtomicInt ref; QAtomicInt capabilities; QDBusConnection::ConnectionCapabilities connectionCapabilities() const { - return (QDBusConnection::ConnectionCapabilities)capabilities.loadRelaxed(); + uint capa = capabilities.loadRelaxed(); + if (mode == ClientMode) + capa |= ConnectionIsBus; + return QDBusConnection::ConnectionCapabilities(capa); } QString name; // this connection's name QString baseService; // this connection's base service diff --git a/src/dbus/qdbusmessage.cpp b/src/dbus/qdbusmessage.cpp index 6ef762f2257..b89ab6a3988 100644 --- a/src/dbus/qdbusmessage.cpp +++ b/src/dbus/qdbusmessage.cpp @@ -109,7 +109,10 @@ DBusMessage *QDBusMessagePrivate::toDBusMessage(const QDBusMessage &message, QDB case QDBusMessage::MethodCallMessage: // only service and interface can be empty -> path and name must not be empty if (!d_ptr->parametersValidated) { - if (!QDBusUtil::checkBusName(d_ptr->service, QDBusUtil::EmptyAllowed, error)) + using namespace QDBusUtil; + AllowEmptyFlag serviceCheckMode = capabilities & QDBusConnectionPrivate::ConnectionIsBus + ? EmptyNotAllowed : EmptyAllowed; + if (!checkBusName(d_ptr->service, serviceCheckMode, error)) return nullptr; if (!QDBusUtil::checkObjectPath(d_ptr->path, QDBusUtil::EmptyNotAllowed, error)) return nullptr; @@ -146,7 +149,7 @@ DBusMessage *QDBusMessagePrivate::toDBusMessage(const QDBusMessage &message, QDB } break; case QDBusMessage::SignalMessage: - // only the service name can be empty here + // only the service name can be empty here (even for bus connections) if (!d_ptr->parametersValidated) { if (!QDBusUtil::checkBusName(d_ptr->service, QDBusUtil::EmptyAllowed, error)) return nullptr; diff --git a/tests/auto/dbus/qdbusmarshall/tst_qdbusmarshall.cpp b/tests/auto/dbus/qdbusmarshall/tst_qdbusmarshall.cpp index 82dc9059e02..ea515536e6e 100644 --- a/tests/auto/dbus/qdbusmarshall/tst_qdbusmarshall.cpp +++ b/tests/auto/dbus/qdbusmarshall/tst_qdbusmarshall.cpp @@ -933,10 +933,9 @@ void tst_QDBusMarshall::sendCallErrors_data() QTest::addColumn("errorMsg"); QTest::addColumn("ignoreMsg"); - // this error comes from the bus server QTest::newRow("empty-service") << "" << objectPath << interfaceName << "ping" << QVariantList() - << "org.freedesktop.DBus.Error.UnknownMethod" - << "Method \"ping\" with signature \"\" on interface \"org.qtproject.autotests.qpong\" doesn't exist\n" << (const char*)0; + << "org.qtproject.QtDBus.Error.InvalidService" + << "Service name cannot be empty" << ""; QTest::newRow("invalid-service-single-label") << "service" << objectPath << interfaceName << "ping" << QVariantList() << "org.qtproject.QtDBus.Error.InvalidService"