[telepathy-qt4/master] Fixed bug 20082: KeyFile: double (and other types?) not correctly tested.

[Andre Moreira Magalhaes (andrunko)]

---
 TelepathyQt4/manager-file.cpp                      |   13 +++-
 tests/manager-file.cpp                             |    3 +
 .../managers/test-manager-file-invalid2.manager    |   86 ++++++++++++++++++++
 tests/telepathy/managers/test-manager-file.manager |    2 +-
 4 files changed, 101 insertions(+), 3 deletions(-)
 create mode 100644 tests/telepathy/managers/test-manager-file-invalid2.manager

diff --git a/TelepathyQt4/manager-file.cpp b/TelepathyQt4/manager-file.cpp
index 0558aea..91574e9 100644
--- a/TelepathyQt4/manager-file.cpp
+++ b/TelepathyQt4/manager-file.cpp
@@ -40,6 +40,7 @@ struct ManagerFile::Private
     QString cmName;
     KeyFile keyFile;
     QHash<QString, ParamSpecList> protocolParams;
+    bool valid;
 
     Private(const QString &cnName);
 
@@ -56,7 +57,8 @@ struct ManagerFile::Private
 };
 
 ManagerFile::Private::Private(const QString &cmName)
-    : cmName(cmName)
+    : cmName(cmName),
+      valid(false)
 {
     init();
 }
@@ -90,10 +92,12 @@ void ManagerFile::Private::init()
         QString fileName = configDir + cmName + QLatin1String(".manager");
         if (QFile::exists(fileName)) {
             debug() << "parsing manager file" << fileName;
+            protocolParams.clear();
             if (!parse(fileName)) {
                 warning() << "error parsing manager file" << fileName;
                 continue;
             }
+            valid = true;
             return;
         }
     }
@@ -167,6 +171,11 @@ bool ManagerFile::Private::parse(const QString &fileName)
                     /* map based on the param dbus signature, otherwise use
                      * QString */
                     QVariant value = valueForKey(param, spec->signature);
+                    if (value.type() == QVariant::Invalid) {
+                        warning() << "param" << paramName
+                                  << "has invalid signature";
+                        return false;
+                    }
                     spec->defaultValue = QDBusVariant(value);
                 }
             }
@@ -178,7 +187,7 @@ bool ManagerFile::Private::parse(const QString &fileName)
 
 bool ManagerFile::Private::isValid() const
 {
-    return (keyFile.status() == KeyFile::NoError);
+    return ((keyFile.status() == KeyFile::NoError) && (valid));
 }
 
 bool ManagerFile::Private::hasParameter(const QString &protocol,
diff --git a/tests/manager-file.cpp b/tests/manager-file.cpp
index 91c3392..b6e485f 100644
--- a/tests/manager-file.cpp
+++ b/tests/manager-file.cpp
@@ -24,6 +24,9 @@ void TestManagerFile::testManagerFile()
     ManagerFile invalidManagerFile("test-manager-file-invalid");
     QCOMPARE(invalidManagerFile.isValid(), false);
 
+    ManagerFile invalidManagerFile2("test-manager-file-invalid2");
+    QCOMPARE(invalidManagerFile2.isValid(), false);
+
     ManagerFile managerFile("test-manager-file");
     QCOMPARE(managerFile.isValid(), true);
 
diff --git a/tests/telepathy/managers/test-manager-file-invalid2.manager b/tests/telepathy/managers/test-manager-file-invalid2.manager
new file mode 100644
index 0000000..5c1434f
--- /dev/null
+++ b/tests/telepathy/managers/test-manager-file-invalid2.manager
@@ -0,0 +1,86 @@
+[Protocol foo]
+param-account = s required
+param-password = s required
+param-encryption-key = s secret
+param-port = q
+param-register = b
+param-server-list = as
+default-account = foo at default
+default-port = 1234
+default-server-list = foo;bar;
+
+[Protocol bar]
+param-account = s required
+param-encryption-key = s required secret
+param-password = s required
+param-port = q
+param-register = b
+param-server-list = as
+default-account = bar at default
+default-port = 4321
+default-server-list = bar;foo;
+
+[Protocol somewhat-pathological]
+# the value is "hello world"
+param-foo = s required
+default-foo =    hello world
+
+# the value is "list;of;misc;" (it's not parsed as a list)
+param-semicolons=s secret
+default-semicolons=list;of;misc;
+
+# the values is a list ["list", "of", "misc"]
+param-list = as
+default-list = list;of;misc;
+
+# the spec says this is invalid but we should probably be permissive
+param-unterminated-list = as
+default-unterminated-list = list;of;misc
+
+# the value is a list ["list", " of", " misc "] (spaces significant)
+param-spaces-in-list = as
+default-spaces-in-list = list; of; misc ;
+
+# the value is a list ["list;of", "misc"]
+param-escaped-semicolon-in-list = as
+default-escaped-semicolon-in-list = list\;of;misc;
+
+# the value is a list ["list\", "of", "misc"]
+param-doubly-escaped-semicolon-in-list = as
+default-doubly-escaped-semicolon-in-list = list\\;of;misc;
+
+# the value is a list ["list\;of", "misc"]
+param-triply-escaped-semicolon-in-list = as
+default-triply-escaped-semicolon-in-list = list\\\;of;misc;
+
+# the value is an empty list
+param-empty-list = as
+default-empty-list =
+
+# the value is a list of empty string
+param-list-of-empty-string = as
+default-list-of-empty-string = ;
+
+# this is probably technically a Desktop Entry spec violation?
+# we should be permissive, interpreting this as either "foo\;bar" or "foo;bar"
+# seems reasonable
+param-escaped-semicolon = s
+default-escaped-semicolon = foo\;bar
+
+# all the other types
+param-object = o
+default-object = /misc
+param-q = q
+default-q = 42
+param-u = u
+default-u = 42
+param-t = t
+default-t = 42
+param-n = n
+default-n = -42
+param-i = i
+default-i = -42
+param-x = x
+default-x = -42
+param-d = 42.0
+default-d = 42.0
diff --git a/tests/telepathy/managers/test-manager-file.manager b/tests/telepathy/managers/test-manager-file.manager
index 5c1434f..b738963 100644
--- a/tests/telepathy/managers/test-manager-file.manager
+++ b/tests/telepathy/managers/test-manager-file.manager
@@ -82,5 +82,5 @@ param-i = i
 default-i = -42
 param-x = x
 default-x = -42
-param-d = 42.0
+param-d = d
 default-d = 42.0
-- 
1.5.6.5