From ca9c9fffb9a53ed1b32623f89fc2cae5f7eab3b5 Mon Sep 17 00:00:00 2001 From: Michael Zanetti Date: Wed, 2 Apr 2014 01:09:38 +0200 Subject: [PATCH] make json parsing a bit more verbose --- server/jsonrpc/jsonhandler.cpp | 4 +- server/jsonrpc/jsonhandler.h | 4 +- server/jsonrpc/jsonrpcserver.cpp | 5 +- server/jsonrpc/jsontypes.cpp | 152 ++++++++++++++++----------- server/jsonrpc/jsontypes.h | 16 +-- tests/scripts/addconfigureddevice.sh | 7 +- tests/scripts/addrule.sh | 4 +- 7 files changed, 116 insertions(+), 76 deletions(-) diff --git a/server/jsonrpc/jsonhandler.cpp b/server/jsonrpc/jsonhandler.cpp index 90239c19..52459c9f 100644 --- a/server/jsonrpc/jsonhandler.cpp +++ b/server/jsonrpc/jsonhandler.cpp @@ -50,13 +50,13 @@ bool JsonHandler::hasMethod(const QString &methodName) return m_descriptions.contains(methodName) && m_params.contains(methodName) && m_returns.contains(methodName); } -bool JsonHandler::validateParams(const QString &methodName, const QVariantMap ¶ms) +QPair JsonHandler::validateParams(const QString &methodName, const QVariantMap ¶ms) { QVariantMap paramTemplate = m_params.value(methodName); return JsonTypes::validateMap(paramTemplate, params); } -bool JsonHandler::validateReturns(const QString &methodName, const QVariantMap &returns) +QPair JsonHandler::validateReturns(const QString &methodName, const QVariantMap &returns) { QVariantMap returnsTemplate = m_returns.value(methodName); return JsonTypes::validateMap(returnsTemplate, returns); diff --git a/server/jsonrpc/jsonhandler.h b/server/jsonrpc/jsonhandler.h index c5f54831..310f3ae6 100644 --- a/server/jsonrpc/jsonhandler.h +++ b/server/jsonrpc/jsonhandler.h @@ -35,8 +35,8 @@ public: QVariantMap introspect(); bool hasMethod(const QString &methodName); - bool validateParams(const QString &methodName, const QVariantMap ¶ms); - bool validateReturns(const QString &methodName, const QVariantMap &returns); + QPair validateParams(const QString &methodName, const QVariantMap ¶ms); + QPair validateReturns(const QString &methodName, const QVariantMap &returns); protected: void setDescription(const QString &methodName, const QString &description); diff --git a/server/jsonrpc/jsonrpcserver.cpp b/server/jsonrpc/jsonrpcserver.cpp index d7a72a7c..573a46c3 100644 --- a/server/jsonrpc/jsonrpcserver.cpp +++ b/server/jsonrpc/jsonrpcserver.cpp @@ -162,8 +162,9 @@ void JsonRPCServer::processData(const QUuid &clientId, const QByteArray &jsonDat sendErrorResponse(clientId, commandId, "No such method"); return; } - if (!handler->validateParams(method, params)) { - sendErrorResponse(clientId, commandId, "Invalid params"); + QPair validationResult = handler->validateParams(method, params); + if (!validationResult.first) { + sendErrorResponse(clientId, commandId, "Invalid params: " + validationResult.second); return; } diff --git a/server/jsonrpc/jsontypes.cpp b/server/jsonrpc/jsontypes.cpp index 440ec044..896a719e 100644 --- a/server/jsonrpc/jsontypes.cpp +++ b/server/jsonrpc/jsontypes.cpp @@ -21,9 +21,11 @@ #include "device.h" #include +#include #include bool JsonTypes::s_initialized = false; +QString JsonTypes::s_lastError; QVariantList JsonTypes::s_basicTypes; QVariantList JsonTypes::s_ruleTypes; @@ -116,6 +118,11 @@ void JsonTypes::init() s_initialized = true; } +QPair JsonTypes::report(bool status, const QString &message) +{ + return qMakePair(status, message); +} + QVariantMap JsonTypes::allTypes() { QVariantMap allTypes; @@ -243,37 +250,44 @@ QVariantMap JsonTypes::packRule(const Rule &rule) return ruleMap; } -bool JsonTypes::validateMap(const QVariantMap &templateMap, const QVariantMap &map) +QPair JsonTypes::validateMap(const QVariantMap &templateMap, const QVariantMap &map) { + s_lastError.clear(); foreach (const QString &key, templateMap.keys()) { if (!map.contains(key)) { qDebug() << "missing key" << key << templateMap << map; - return false; + QJsonDocument jsonDoc = QJsonDocument::fromVariant(map); + return report(false, QString("Missing key \"%1\" in %2").arg(key).arg(QString(jsonDoc.toJson()))); } - if (!validateVariant(templateMap.value(key), map.value(key))) { + QPair result = validateVariant(templateMap.value(key), map.value(key)); + if (!result.first) { qDebug() << "Object not matching template"; - return false; + return result; } } - return true; + return report(true, ""); } -bool JsonTypes::validateProperty(const QVariant &templateValue, const QVariant &value) +QPair JsonTypes::validateProperty(const QVariant &templateValue, const QVariant &value) { if (templateValue == "uuid") { - return value.canConvert(QVariant::Uuid); + QString errorString = QString("Param %1 is not a uuid.").arg(value.toString()); + return report(value.canConvert(QVariant::Uuid), errorString); } if (templateValue == "string") { - return value.canConvert(QVariant::String); + QString errorString = QString("Param %1 is not a string.").arg(value.toString()); + return report(value.canConvert(QVariant::String), errorString); } if (templateValue == "bool") { - return value.canConvert(QVariant::Bool); + QString errorString = QString("Param %1 is not a bool.").arg(value.toString()); + return report(value.canConvert(QVariant::Bool), errorString); } - qWarning() << "Unhandled property type!" << templateValue; - return false; + qWarning() << QString("Unhandled property type: %1 (expected: %2)").arg(value.toString()).arg(templateValue.toString()); + QString errorString = QString("Unhandled property type: %1 (expected: %2)").arg(value.toString()).arg(templateValue.toString()); + return report(false, errorString); } -bool JsonTypes::validateList(const QVariantList &templateList, const QVariantList &list) +QPair JsonTypes::validateList(const QVariantList &templateList, const QVariantList &list) { Q_ASSERT(templateList.count() == 1); QVariant entryTemplate = templateList.first(); @@ -282,15 +296,16 @@ bool JsonTypes::validateList(const QVariantList &templateList, const QVariantLis for (int i = 0; i < list.count(); ++i) { QVariant listEntry = list.at(i); - if (!validateVariant(entryTemplate, listEntry)) { + QPair result = validateVariant(entryTemplate, listEntry); + if (!result.first) { qDebug() << "List entry not matching template"; - return false; + return result; } } - return true; + return report(true, ""); } -bool JsonTypes::validateVariant(const QVariant &templateVariant, const QVariant &variant) +QPair JsonTypes::validateVariant(const QVariant &templateVariant, const QVariant &variant) { switch(templateVariant.type()) { case QVariant::String: @@ -298,115 +313,132 @@ bool JsonTypes::validateVariant(const QVariant &templateVariant, const QVariant QString refName = templateVariant.toString(); if (refName == JsonTypes::actionRef()) { qDebug() << "validating action"; - if (!validateMap(actionDescription(), variant.toMap())) { + QPair result = validateMap(actionDescription(), variant.toMap()); + if (!result.first) { qDebug() << "Error validating action"; - return false; + return result; } } else if (refName == JsonTypes::eventRef()) { - if (!validateMap(eventDescription(), variant.toMap())) { + QPair result = validateMap(eventDescription(), variant.toMap()); + if (!result.first) { qDebug() << "event not valid"; - return false; + return result; } } else if (refName == deviceRef()) { - if (!validateMap(deviceDescription(), variant.toMap())) { + QPair result = validateMap(deviceDescription(), variant.toMap()); + if (!result.first) { qDebug() << "device not valid"; - return false; + return result; } } else if (refName == deviceClassRef()) { - if (!validateMap(deviceClassDescription(), variant.toMap())) { + QPair result = validateMap(deviceClassDescription(), variant.toMap()); + if (!result.first) { qDebug() << "device class not valid"; - return false; + return result; } } else if (refName == paramTypeRef()) { - if (!validateMap(paramTypeDescription(), variant.toMap())) { + QPair result = validateMap(paramTypeDescription(), variant.toMap()); + if (!result.first) { qDebug() << "param types not matching"; - return false; + return result; } } else if (refName == actionTypeRef()) { - if (!validateMap(actionTypeDescription(), variant.toMap())) { + QPair result = validateMap(actionTypeDescription(), variant.toMap()); + if (!result.first) { qDebug() << "action type not matching"; - return false; + return result; } } else if (refName == eventTypeRef()) { - if (!validateMap(eventTypeDescription(), variant.toMap())) { + QPair result = validateMap(eventTypeDescription(), variant.toMap()); + if (!result.first) { qDebug() << "event type not matching"; - return false; + return result; } } else if (refName == stateTypeRef()) { - if (!validateMap(stateTypeDescription(), variant.toMap())) { + QPair result = validateMap(stateTypeDescription(), variant.toMap()); + if (!result.first) { qDebug() << "state type not matching"; - return false; + return result; } } else if (refName == pluginRef()) { - if (!validateMap(pluginDescription(), variant.toMap())) { + QPair result = validateMap(pluginDescription(), variant.toMap()); + if (!result.first) { qDebug() << "plugin not matching"; - return false; + return result; } } else if (refName == ruleRef()) { - if (!validateMap(ruleDescription(), variant.toMap())) { + QPair result = validateMap(ruleDescription(), variant.toMap()); + if (!result.first) { qDebug() << "rule type not matching"; - return false; + return result; } } else if (refName == basicTypesRef()) { - if (!validateBasicType(variant)) { + QPair result = validateBasicType(variant); + if (!result.first) { qDebug() << "value not allowed in" << basicTypesRef(); - return false; + return result; } } else if (refName == ruleTypesRef()) { - if (!validateRuleType(variant)) { + QPair result = validateRuleType(variant); + if (!result.first) { qDebug() << "value not allowed in" << ruleTypesRef(); - return false; + return result; } } else { qDebug() << "unhandled ref:" << refName; - return false; + return report(false, QString("Unhandled ref %1. Server implementation incomplete.").arg(refName)); } } else { - if (!JsonTypes::validateProperty(templateVariant, variant)) { + QPair result = JsonTypes::validateProperty(templateVariant, variant); + if (!result.first) { qDebug() << "property not matching:" << templateVariant << "!=" << variant; - return false; + return result; } } break; - case QVariant::Map: - if (!validateMap(templateVariant.toMap(), variant.toMap())) { - return false; + case QVariant::Map: { + QPair result = validateMap(templateVariant.toMap(), variant.toMap()); + if (!result.first) { + return result; } break; - case QVariant::List: - if (!validateList(templateVariant.toList(), variant.toList())) { - return false; + } + case QVariant::List: { + QPair result = validateList(templateVariant.toList(), variant.toList()); + if (!result.first) { + return result; } break; + } default: qDebug() << "unhandled value" << templateVariant; - return false; + return report(false, QString("Unhandled value %1.").arg(templateVariant.toString())); } - return true; + return report(true, ""); } -bool JsonTypes::validateBasicType(const QVariant &variant) +QPair JsonTypes::validateBasicType(const QVariant &variant) { if (variant.canConvert(QVariant::Uuid)) { - return true; + return report(true, ""); } if (variant.canConvert(QVariant::String)) { - return true; + return report(true, ""); } if (variant.canConvert(QVariant::Int)) { - return true; + return report(true, ""); } if (variant.canConvert(QVariant::Double)) { - return true; + return report(true, ""); } if (variant.canConvert(QVariant::Bool)) { - return true; + return report(true, ""); } - return false; + return report(false, QString("Error validating basic type %1.").arg(variant.toString())); } -bool JsonTypes::validateRuleType(const QVariant &variant) +QPair JsonTypes::validateRuleType(const QVariant &variant) { - return s_ruleTypes.contains(variant.toString()); + return report(s_ruleTypes.contains(variant.toString()), QString("Unknown rules type %1").arg(variant.toString())); } diff --git a/server/jsonrpc/jsontypes.h b/server/jsonrpc/jsontypes.h index b4389d29..135aff98 100644 --- a/server/jsonrpc/jsontypes.h +++ b/server/jsonrpc/jsontypes.h @@ -84,16 +84,20 @@ public: static QVariantMap packDevice(Device *device); static QVariantMap packRule(const Rule &rule); - static bool validateMap(const QVariantMap &templateMap, const QVariantMap &map); - static bool validateProperty(const QVariant &templateValue, const QVariant &value); - static bool validateList(const QVariantList &templateList, const QVariantList &list); - static bool validateVariant(const QVariant &templateVariant, const QVariant &variant); - static bool validateBasicType(const QVariant &variant); - static bool validateRuleType(const QVariant &variant); + static QPair validateMap(const QVariantMap &templateMap, const QVariantMap &map); + static QPair validateProperty(const QVariant &templateValue, const QVariant &value); + static QPair validateList(const QVariantList &templateList, const QVariantList &list); + static QPair validateVariant(const QVariant &templateVariant, const QVariant &variant); + static QPair validateBasicType(const QVariant &variant); + static QPair validateRuleType(const QVariant &variant); private: static bool s_initialized; static void init(); + + static QPair report(bool status, const QString &message); + + static QString s_lastError; }; #endif // JSONTYPES_H diff --git a/tests/scripts/addconfigureddevice.sh b/tests/scripts/addconfigureddevice.sh index edde6441..992a4ca5 100755 --- a/tests/scripts/addconfigureddevice.sh +++ b/tests/scripts/addconfigureddevice.sh @@ -23,10 +23,13 @@ else elif [ $2 == "wifidetector" ]; then # Adds a WiFi detector (echo '{"id":1, "method":"Devices.AddConfiguredDevice", "params":{"deviceClassId": "{bd216356-f1ec-4324-9785-6982d2174e17}","deviceParams":{"mac":"90:cf:15:1b:ce:bb"}}}'; sleep 1) | nc $1 1234 - elif [ $2 == "mock" ]; then + elif [ $2 == "mock1" ]; then # Adds a Mock device (echo '{"id":1, "method":"Devices.AddConfiguredDevice", "params":{"deviceClassId": "{753f0d32-0468-4d08-82ed-1964aab03298}","deviceParams":{"httpport":"8081"}}}'; sleep 1) | nc $1 1234 + elif [ $2 == "mock2" ]; then + # Adds a Mock device + (echo '{"id":1, "method":"Devices.AddConfiguredDevice", "params":{"deviceClassId": "{753f0d32-0468-4d08-82ed-1964aab03298}","deviceParams":{"httpport":"8082"}}}'; sleep 1) | nc $1 1234 else - echo "unknown type $2. Possible values are: elroremote, elroswitch, intertechnoremote, meisteranker, wifidetector, mock" + echo "unknown type $2. Possible values are: elroremote, elroswitch, intertechnoremote, meisteranker, wifidetector, mock1, mock2" fi fi diff --git a/tests/scripts/addrule.sh b/tests/scripts/addrule.sh index 0c2a6c9d..7d6aab7e 100755 --- a/tests/scripts/addrule.sh +++ b/tests/scripts/addrule.sh @@ -3,6 +3,6 @@ if test -z $5; then echo "usage: $0 host sourceDevice eventTypeId targetDeviceId actionTypeId" else - (echo '{"id":1, "method":"Rules.AddRule", "params":{"events": {"eventTypeId": "'$3'", "deviceId":"'$2'", "params":{"power":"true"}}, "actions": [ { "deviceId":"'$4'", "actionTypeId":"'$5'", "params":{"power":"true"}}]}}'; sleep 1) | nc $1 1234 -# (echo '{"id":1, "method":"Rules.AddRule", "params":{"events": {"eventTypeId": "'$2'", "deviceId":"'$3'", "params":{"power":"false"}}, "actions": [ { "deviceId":"'$4'", "name":"rule 1", "params":{"power":"false"}},{ "deviceId":"'$5'", "name":"rule 1", "params":{"power":"true"}}]}}'; sleep 1) | nc $1 1234 + (echo '{"id":1, "method":"Rules.AddRule", "params":{"event": {"eventTypeId": "'$3'", "deviceId":"'$2'"}, "actions": [ { "deviceId":"'$4'", "actionTypeId":"'$5'", "params":{"power":"true"}}]}}'; sleep 1) | nc $1 1234 +# (echo '{"id":1, "method":"Rules.AddRule", "params":{"event": {"eventTypeId": "'$2'", "deviceId":"'$3'", "params":{"power":"false"}}, "actions": [ { "deviceId":"'$4'", "name":"rule 1", "params":{"power":"false"}},{ "deviceId":"'$5'", "name":"rule 1", "params":{"power":"true"}}]}}'; sleep 1) | nc $1 1234 fi