Fix DBus signature generation for complex types.

When generating the DBus signature of a registered custom type the
marshaller appends the signatures of the map entries and array items
after the map/array causing an invalid DBus signature to be generated.
This happens because beginArray() and beginMap() output the full
signature of the data.

Fixed by suppressing changes to the signature within
beginArray()/endArray() and beginMap()/endMap() blocks.

Change-Id: Icaf23b2fe58a3e1f575b81e4a100f02684f68452
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>

3 files changed, 109 insertions, 34 deletions

diff --git a/src/dbus/qdbusargument_p.h b/src/dbus/qdbusargument_p.h
index 12039307e1..5f8c13c2c5 100644
--- a/src/dbus/qdbusargument_p.h
+++ b/src/dbus/qdbusargument_p.h
@@ -97,7 +97,7 @@ public:
 class QDBusMarshaller: public QDBusArgumentPrivate
 {
 public:
-    QDBusMarshaller(int flags) : QDBusArgumentPrivate(flags), parent(0), ba(0), closeCode(0), ok(true)
+    QDBusMarshaller(int flags) : QDBusArgumentPrivate(flags), parent(0), ba(0), closeCode(0), ok(true), skipSignature(false)
     { direction = Marshalling; }
     ~QDBusMarshaller();
 
@@ -145,6 +145,7 @@ public:
     QString errorString;
     char closeCode;
     bool ok;
+    bool skipSignature;
 
 private:
     Q_DISABLE_COPY(QDBusMarshaller)
diff --git a/src/dbus/qdbusmarshaller.cpp b/src/dbus/qdbusmarshaller.cpp
index 0e21e2f282..60b6db6aa6 100644
--- a/src/dbus/qdbusmarshaller.cpp
+++ b/src/dbus/qdbusmarshaller.cpp
@@ -62,55 +62,65 @@ inline QString QDBusMarshaller::currentSignature()
 
 inline void QDBusMarshaller::append(uchar arg)
 {
-    qIterAppend(&iterator, ba, DBUS_TYPE_BYTE, &arg);
+    if (!skipSignature)
+        qIterAppend(&iterator, ba, DBUS_TYPE_BYTE, &arg);
 }
 
 inline void QDBusMarshaller::append(bool arg)
 {
     dbus_bool_t cast = arg;
-    qIterAppend(&iterator, ba, DBUS_TYPE_BOOLEAN, &cast);
+    if (!skipSignature)
+        qIterAppend(&iterator, ba, DBUS_TYPE_BOOLEAN, &cast);
 }
 
 inline void QDBusMarshaller::append(short arg)
 {
-    qIterAppend(&iterator, ba, DBUS_TYPE_INT16, &arg);
+    if (!skipSignature)
+        qIterAppend(&iterator, ba, DBUS_TYPE_INT16, &arg);
 }
 
 inline void QDBusMarshaller::append(ushort arg)
 {
-    qIterAppend(&iterator, ba, DBUS_TYPE_UINT16, &arg);
+    if (!skipSignature)
+        qIterAppend(&iterator, ba, DBUS_TYPE_UINT16, &arg);
 }
 
 inline void QDBusMarshaller::append(int arg)
 {
-    qIterAppend(&iterator, ba, DBUS_TYPE_INT32, &arg);
+    if (!skipSignature)
+        qIterAppend(&iterator, ba, DBUS_TYPE_INT32, &arg);
 }
 
 inline void QDBusMarshaller::append(uint arg)
 {
-    qIterAppend(&iterator, ba, DBUS_TYPE_UINT32, &arg);
+    if (!skipSignature)
+        qIterAppend(&iterator, ba, DBUS_TYPE_UINT32, &arg);
 }
 
 inline void QDBusMarshaller::append(qlonglong arg)
 {
-    qIterAppend(&iterator, ba, DBUS_TYPE_INT64, &arg);
+    if (!skipSignature)
+        qIterAppend(&iterator, ba, DBUS_TYPE_INT64, &arg);
 }
 
 inline void QDBusMarshaller::append(qulonglong arg)
 {
-    qIterAppend(&iterator, ba, DBUS_TYPE_UINT64, &arg);
+    if (!skipSignature)
+        qIterAppend(&iterator, ba, DBUS_TYPE_UINT64, &arg);
 }
 
 inline void QDBusMarshaller::append(double arg)
 {
-    qIterAppend(&iterator, ba, DBUS_TYPE_DOUBLE, &arg);
+    if (!skipSignature)
+        qIterAppend(&iterator, ba, DBUS_TYPE_DOUBLE, &arg);
 }
 
 void QDBusMarshaller::append(const QString &arg)
 {
     QByteArray data = arg.toUtf8();
     const char *cdata = data.constData();
-    qIterAppend(&iterator, ba, DBUS_TYPE_STRING, &cdata);
+    if (!skipSignature)
+        qIterAppend(&iterator, ba, DBUS_TYPE_STRING, &cdata);
 }
 
 inline void QDBusMarshaller::append(const QDBusObjectPath &arg)
@@ -120,7 +130,8 @@ inline void QDBusMarshaller::append(const QDBusObjectPath &arg)
         error(QLatin1String("Invalid object path passed in arguments"));
     } else {
         const char *cdata = data.constData();
-        qIterAppend(&iterator, ba, DBUS_TYPE_OBJECT_PATH, &cdata);
+        if (!skipSignature)
+            qIterAppend(&iterator, ba, DBUS_TYPE_OBJECT_PATH, &cdata);
     }
 }
 
@@ -131,7 +142,8 @@ inline void QDBusMarshaller::append(const QDBusSignature &arg)
         error(QLatin1String("Invalid signature passed in arguments"));
     } else {
         const char *cdata = data.constData();
-        qIterAppend(&iterator, ba, DBUS_TYPE_SIGNATURE, &cdata);
+        if (!skipSignature)
+            qIterAppend(&iterator, ba, DBUS_TYPE_SIGNATURE, &cdata);
     }
 }
 
@@ -141,14 +153,16 @@ inline void QDBusMarshaller::append(const QDBusUnixFileDescriptor &arg)
     if (!ba && fd == -1) {
         error(QLatin1String("Invalid file descriptor passed in arguments"));
     } else {
-        qIterAppend(&iterator, ba, DBUS_TYPE_UNIX_FD, &fd);
+        if (!skipSignature)
+            qIterAppend(&iterator, ba, DBUS_TYPE_UNIX_FD, &fd);
     }
 }
 
 inline void QDBusMarshaller::append(const QByteArray &arg)
 {
     if (ba) {
-        *ba += DBUS_TYPE_ARRAY_AS_STRING DBUS_TYPE_BYTE_AS_STRING;
+        if (!skipSignature)
+            *ba += DBUS_TYPE_ARRAY_AS_STRING DBUS_TYPE_BYTE_AS_STRING;
         return;
     }
 
@@ -163,7 +177,8 @@ inline void QDBusMarshaller::append(const QByteArray &arg)
 inline bool QDBusMarshaller::append(const QDBusVariant &arg)
 {
     if (ba) {
-        *ba += DBUS_TYPE_VARIANT_AS_STRING;
+        if (!skipSignature)
+            *ba += DBUS_TYPE_VARIANT_AS_STRING;
         return true;
     }
 
@@ -206,7 +221,8 @@ inline bool QDBusMarshaller::append(const QDBusVariant &arg)
 inline void QDBusMarshaller::append(const QStringList &arg)
 {
     if (ba) {
-        *ba += DBUS_TYPE_ARRAY_AS_STRING DBUS_TYPE_STRING_AS_STRING;
+        if (!skipSignature)
+            *ba += DBUS_TYPE_ARRAY_AS_STRING DBUS_TYPE_STRING_AS_STRING;
         return;
     }
 
@@ -288,25 +304,30 @@ void QDBusMarshaller::open(QDBusMarshaller &sub, int code, const char *signature
     sub.ba = ba;
     sub.ok = true;
     sub.capabilities = capabilities;
+    sub.skipSignature = skipSignature;
 
-    if (ba)
-        switch (code) {
-        case DBUS_TYPE_ARRAY:
-            *ba += char(code);
-            *ba += signature;
-            // fall through
-
-        case DBUS_TYPE_DICT_ENTRY:
-            sub.closeCode = 0;
-            break;
-
-        case DBUS_TYPE_STRUCT:
-            *ba += DBUS_STRUCT_BEGIN_CHAR;
-            sub.closeCode = DBUS_STRUCT_END_CHAR;
-            break;
+    if (ba) {
+        if (!skipSignature) {
+            switch (code) {
+            case DBUS_TYPE_ARRAY:
+                *ba += char(code);
+                *ba += signature;
+                // fall through
+
+            case DBUS_TYPE_DICT_ENTRY:
+                sub.closeCode = 0;
+                sub.skipSignature = true;
+                break;
+
+            case DBUS_TYPE_STRUCT:
+                *ba += DBUS_STRUCT_BEGIN_CHAR;
+                sub.closeCode = DBUS_STRUCT_END_CHAR;
+                break;
+            }
         }
-    else
+    } else {
         q_dbus_message_iter_open_container(&iterator, code, signature, &sub.iterator);
+    }
 }
 
 QDBusMarshaller *QDBusMarshaller::beginCommon(int code, const char *signature)
@@ -338,7 +359,7 @@ QDBusMarshaller *QDBusMarshaller::endCommon()
 void QDBusMarshaller::close()
 {
     if (ba) {
-        if (closeCode)
+        if (!skipSignature && closeCode)
             *ba += closeCode;
     } else if (parent) {
         q_dbus_message_iter_close_container(&parent->iterator, &iterator);
diff --git a/tests/auto/dbus/qdbusmetatype/tst_qdbusmetatype.cpp b/tests/auto/dbus/qdbusmetatype/tst_qdbusmetatype.cpp
index fc20b47768..f80a969384 100644
--- a/tests/auto/dbus/qdbusmetatype/tst_qdbusmetatype.cpp
+++ b/tests/auto/dbus/qdbusmetatype/tst_qdbusmetatype.cpp
@@ -69,6 +69,19 @@ struct Struct4                  // (ssa(ss)sayasx)
     QStringList m6;
     qlonglong m7;
 };
+struct Struct5                  // a{sa{sv}} - non-standard outer struct is used as a local
+{                               // container, see marshalling operator below.
+    QVariantMap m1;
+    QVariantMap m2;
+    QVariantMap m3;
+};
+struct Struct6                  // av - non-standard outer struct is used as a local container,
+{                               // see marshalling operator below.
+    QVariant v1;
+    QVariant v2;
+    QVariant v3;
+};
+
 
 struct Invalid0 { };            // empty
 struct Invalid1 { };            // s
@@ -84,6 +97,8 @@ Q_DECLARE_METATYPE(Struct2)
 Q_DECLARE_METATYPE(Struct3)
 Q_DECLARE_METATYPE(Struct4)
 Q_DECLARE_METATYPE(StringPair)
+Q_DECLARE_METATYPE(Struct5)
+Q_DECLARE_METATYPE(Struct6)
 
 Q_DECLARE_METATYPE(Invalid0)
 Q_DECLARE_METATYPE(Invalid1)
@@ -141,6 +156,34 @@ QDBusArgument &operator<<(QDBusArgument &arg, const Struct4 &s)
     return arg;
 }
 
+QDBusArgument &operator<<(QDBusArgument &arg, const Struct5 &s)
+{
+    arg.beginMap(qMetaTypeId<QString>(), qMetaTypeId<QVariantMap>());
+
+    arg.beginMapEntry();
+    arg << QStringLiteral("map1") << s.m1;
+    arg.endMapEntry();
+
+    arg.beginMapEntry();
+    arg << QStringLiteral("map2") << s.m2;
+    arg.endMapEntry();
+
+    arg.beginMapEntry();
+    arg << QStringLiteral("map3") << s.m3;
+    arg.endMapEntry();
+
+    arg.endMap();
+    return arg;
+}
+
+QDBusArgument &operator<<(QDBusArgument &arg, const Struct6 &s)
+{
+    arg.beginArray(qMetaTypeId<QDBusVariant>());
+    arg << QDBusVariant(s.v1) << QDBusVariant(s.v2) << QDBusVariant(s.v3);
+    arg.endArray();
+    return arg;
+}
+
 QDBusArgument &operator<<(QDBusArgument &arg, const Invalid0 &)
 {
     return arg;
@@ -194,6 +237,10 @@ const QDBusArgument &operator>>(const QDBusArgument &arg, Struct3 &)
 { return arg; }
 const QDBusArgument &operator>>(const QDBusArgument &arg, Struct4 &)
 { return arg; }
+const QDBusArgument &operator>>(const QDBusArgument &arg, Struct5 &)
+{ return arg; }
+const QDBusArgument &operator>>(const QDBusArgument &arg, Struct6 &)
+{ return arg; }
 const QDBusArgument &operator>>(const QDBusArgument &arg, StringPair &)
 { return arg; }
 const QDBusArgument &operator>>(const QDBusArgument &arg, Invalid0 &)
@@ -220,6 +267,8 @@ void tst_QDBusMetaType::initTestCase()
     qDBusRegisterMetaType<Struct2>();
     qDBusRegisterMetaType<Struct3>();
     qDBusRegisterMetaType<Struct4>();
+    qDBusRegisterMetaType<Struct5>();
+    qDBusRegisterMetaType<Struct6>();
     qDBusRegisterMetaType<StringPair>();
 
     qDBusRegisterMetaType<QList<Struct1> >();
@@ -297,6 +346,10 @@ void tst_QDBusMetaType::dynamicTypes_data()
     QTest::newRow("Struct4") << QVariant::Type(qMetaTypeId<Struct4>()) << "(ssa(ss)sayasx)";
     QTest::newRow("QList<Struct4>") << QVariant::Type(qMetaTypeId<QList<Struct4> >()) << "a(ssa(ss)sayasx)";
 
+    QTest::newRow("Struct5") << QVariant::Type(qMetaTypeId<Struct5>()) << "a{sa{sv}}";
+
+    QTest::newRow("Struct6") << QVariant::Type(qMetaTypeId<Struct6>()) << "av";
+
     QTest::newRow("QMap<int,QString>") << QVariant::Type(intStringMap) << "a{is}";
     QTest::newRow("QMap<QString,QString>") << QVariant::Type(stringStringMap) << "a{ss}";
     QTest::newRow("QMap<QString,Struct1>") << QVariant::Type(stringStruct1Map) << "a{s(s)}";