diff options
author | Thiago Macieira <thiago.macieira@intel.com> | 2024-04-30 17:59:51 -0700 |
---|---|---|
committer | Thiago Macieira <thiago.macieira@intel.com> | 2024-05-10 13:17:20 -0700 |
commit | ed6d1fa71a79a70b7e6a20fbbc737ed9f6c287b1 (patch) | |
tree | b63cf0ef32bdbfd4193c9ad66051ec88033822c8 | |
parent | e3f520e621a9ea631d8a1d2f9bfc7a5f20dc698a (diff) |
QDBusSignature: accept empty strings as valid
QDBusSignature holds a D-Bus value of type SIGNATURE, which is zero or
more signatures, not one or more. This changes the default constructor
to create a valid signature, which we denote by not being a null
QString. That means we need to use something other than the default
constructor in our tests for attempting to pass invalid signatures.
[ChangeLog][QtDBus][QDBusSignature] Fixed a bug that caused the class
not to accept an empty string as a valid D-Bus SIGNATURE value.
Fixes: QTBUG-124919
Pick-to: 6.5 6.7
Change-Id: I262c3499666e4f4fbcfbfffd17cb3793dcf2eae3
Reviewed-by: MÃ¥rten Nordheim <marten.nordheim@qt.io>
-rw-r--r-- | src/dbus/qdbusextratypes.cpp | 2 | ||||
-rw-r--r-- | src/dbus/qdbusextratypes.h | 5 | ||||
-rw-r--r-- | src/dbus/qdbusmarshaller.cpp | 2 | ||||
-rw-r--r-- | src/dbus/qdbusutil.cpp | 8 | ||||
-rw-r--r-- | tests/auto/dbus/qdbusmarshall/tst_qdbusmarshall.cpp | 6 | ||||
-rw-r--r-- | tests/auto/dbus/qdbustype/tst_qdbustype.cpp | 13 |
6 files changed, 25 insertions, 11 deletions
diff --git a/src/dbus/qdbusextratypes.cpp b/src/dbus/qdbusextratypes.cpp index 61f2075443..3354e76577 100644 --- a/src/dbus/qdbusextratypes.cpp +++ b/src/dbus/qdbusextratypes.cpp @@ -34,6 +34,8 @@ void QDBusSignature::doCheck() if (!QDBusUtil::isValidSignature(m_signature)) { qWarning("QDBusSignature: invalid signature \"%s\"", qPrintable(m_signature)); m_signature.clear(); + } else if (m_signature.isEmpty()) { + m_signature.detach(); // we need it to not be null } } diff --git a/src/dbus/qdbusextratypes.h b/src/dbus/qdbusextratypes.h index 1bc0f3086d..1c0826b992 100644 --- a/src/dbus/qdbusextratypes.h +++ b/src/dbus/qdbusextratypes.h @@ -77,7 +77,10 @@ class Q_DBUS_EXPORT QDBusSignature { QString m_signature; public: - QDBusSignature() noexcept : m_signature() {} + QDBusSignature() noexcept + { + m_signature.detach(); // mark non-null (empty signatures are valid) + } // compiler-generated copy/move constructor/assignment operators are ok! // compiler-generated destructor is ok! diff --git a/src/dbus/qdbusmarshaller.cpp b/src/dbus/qdbusmarshaller.cpp index b2ed2586fb..d1f1a41ab0 100644 --- a/src/dbus/qdbusmarshaller.cpp +++ b/src/dbus/qdbusmarshaller.cpp @@ -120,7 +120,7 @@ inline void QDBusMarshaller::append(const QDBusObjectPath &arg) inline void QDBusMarshaller::append(const QDBusSignature &arg) { QByteArray data = arg.signature().toUtf8(); - if (!ba && data.isEmpty()) { + if (!ba && data.isNull()) { error("Invalid signature passed in arguments"_L1); } else { const char *cdata = data.constData(); diff --git a/src/dbus/qdbusutil.cpp b/src/dbus/qdbusutil.cpp index 827418c487..78338aa054 100644 --- a/src/dbus/qdbusutil.cpp +++ b/src/dbus/qdbusutil.cpp @@ -512,14 +512,14 @@ namespace QDBusUtil bool isValidSignature(const QString &signature) { QByteArray ba = signature.toLatin1(); - const char *data = ba.constData(); - while (true) { + const char *data = ba.constBegin(); + const char *end = ba.constEnd(); + while (data != end) { data = validateSingleType(data); if (!data) return false; - if (*data == '\0') - return true; } + return true; } /*! diff --git a/tests/auto/dbus/qdbusmarshall/tst_qdbusmarshall.cpp b/tests/auto/dbus/qdbusmarshall/tst_qdbusmarshall.cpp index e7a8273115..355a65bc59 100644 --- a/tests/auto/dbus/qdbusmarshall/tst_qdbusmarshall.cpp +++ b/tests/auto/dbus/qdbusmarshall/tst_qdbusmarshall.cpp @@ -161,7 +161,9 @@ void basicStringTypes_data() { QTest::newRow("string") << QVariant("ping") << "s" << "\"ping\""; QTest::newRow("objectpath") << QVariant::fromValue(QDBusObjectPath("/org/kde")) << "o" << "[ObjectPath: /org/kde]"; + QTest::newRow("emptysignature") << QVariant::fromValue(QDBusSignature(QString())) << "g" << "[Signature: ]"; QTest::newRow("signature") << QVariant::fromValue(QDBusSignature("g")) << "g" << "[Signature: g]"; + QTest::newRow("multisignature") << QVariant::fromValue(QDBusSignature("bit")) << "g" << "[Signature: bit]"; QTest::newRow("emptystring") << QVariant("") << "s" << "\"\""; QTest::newRow("nullstring") << QVariant(QString()) << "s" << "\"\""; } @@ -907,7 +909,7 @@ void tst_QDBusMarshall::sendSignalErrors() QTest::ignoreMessage(QtWarningMsg, "QDBusConnection: error: could not send signal to service \"\" path \"/foo\" interface \"local.interfaceName\" member \"signalName\": Marshalling failed: Invalid object path passed in arguments"); QVERIFY(!con.send(msg)); - QDBusSignature sig; + QDBusSignature sig(QChar(0)); msg.setArguments(QVariantList() << QVariant::fromValue(sig)); QTest::ignoreMessage(QtWarningMsg, "QDBusConnection: error: could not send signal to service \"\" path \"/foo\" interface \"local.interfaceName\" member \"signalName\": Marshalling failed: Invalid signature passed in arguments"); QVERIFY(!con.send(msg)); @@ -992,7 +994,7 @@ void tst_QDBusMarshall::sendCallErrors_data() << ""; QTest::newRow("invalid-signature-arg") << serviceName << objectPath << interfaceName << "ping" - << (QVariantList() << QVariant::fromValue(QDBusSignature())) + << (QVariantList() << QVariant::fromValue(QDBusSignature(QChar(0)))) << "org.freedesktop.DBus.Error.Failed" << "Marshalling failed: Invalid signature passed in arguments" << ""; diff --git a/tests/auto/dbus/qdbustype/tst_qdbustype.cpp b/tests/auto/dbus/qdbustype/tst_qdbustype.cpp index f4ad4cb77a..63cb7d4a65 100644 --- a/tests/auto/dbus/qdbustype/tst_qdbustype.cpp +++ b/tests/auto/dbus/qdbustype/tst_qdbustype.cpp @@ -206,6 +206,7 @@ void tst_QDBusType::isValidBasicType() void tst_QDBusType::isValidSingleSignature_data() { addColumns(); + QTest::newRow("empty") << "" << false; addSingleSignatures(); addNakedDictEntry(); } @@ -222,6 +223,7 @@ void tst_QDBusType::isValidSingleSignature() void tst_QDBusType::isValidArray_data() { addColumns(); + QTest::newRow("empty") << "" << false; addSingleSignatures(); } @@ -241,7 +243,10 @@ void tst_QDBusType::isValidArray() void tst_QDBusType::isValidSignature_data() { - isValidSingleSignature_data(); + addColumns(); + QTest::newRow("empty") << "" << true; + addSingleSignatures(); + addNakedDictEntry(); } void tst_QDBusType::isValidSignature() @@ -250,8 +255,10 @@ void tst_QDBusType::isValidSignature() QFETCH(bool, result); data.append(data); - if (data.at(0).unicode()) - QCOMPARE(bool(q_dbus_signature_validate(data.toLatin1(), 0)), result); + if (!data.isEmpty() && data.at(0).unicode()) { + // libdbus-1 API can't deal with string containing NULs + QCOMPARE(bool(q_dbus_signature_validate(data.toLatin1(), nullptr)), result); + } QCOMPARE(QDBusUtil::isValidSignature(data), result); } |