Rafactor Packet fragmentation

- Move Json packet fragmentation into the JsonRpcServer.
  This way we only have to do it once.

- fixes a bug in TcpServer and BluetoothServer
  where multiple clients would corrupt each others buffer

- fixes a bug in CloudTransport where it would leak client
  sockets after a remote connection disconnect

libnymea-core/bluetoothserver.cpp

@@ -82,6 +82,14 @@ void BluetoothServer::sendData(const QList<QUuid> &clients, const QByteArray &da
         sendData(client, data);
 }
 
+void BluetoothServer::terminateClientConnection(const QUuid &clientId)
+{
+    QBluetoothSocket *client = m_clientList.value(clientId);
+    if (client) {
+        client->abort();
+    }
+}
+
 void BluetoothServer::onHostModeChanged(const QBluetoothLocalDevice::HostMode &mode)
 {
     if (!m_server || !m_localDevice)
@@ -140,19 +148,7 @@ void BluetoothServer::readData()
     if (!client)
         return;
 
-    m_receiveBuffer.append(client->readAll());
+    emit dataAvailable(m_clientList.key(client), client->readAll());
-    qCDebug(dcBluetoothServerTraffic()) << "Current data buffer:" << qUtf8Printable(m_receiveBuffer);
-    int splitIndex = m_receiveBuffer.indexOf("}\n{");
-    while (splitIndex > -1) {
-        emit dataAvailable(m_clientList.key(client), m_receiveBuffer.left(splitIndex + 1));
-        m_receiveBuffer = m_receiveBuffer.right(m_receiveBuffer.length() - splitIndex - 2);
-        splitIndex = m_receiveBuffer.indexOf("}\n{");
-    }
-
-    if (m_receiveBuffer.endsWith("}\n")) {
-        emit dataAvailable(m_clientList.key(client), m_receiveBuffer.trimmed());
-        m_receiveBuffer.clear();
-    }
 }
 
 bool BluetoothServer::startServer()
@@ -250,7 +246,6 @@ bool BluetoothServer::stopServer()
         m_localDevice = nullptr;
     }
 
-    m_receiveBuffer.clear();
     return true;
 }
 

libnymea-core/bluetoothserver.h

@@ -34,7 +34,7 @@ class BluetoothServer : public TransportInterface
 {
     Q_OBJECT
 public:
-    explicit BluetoothServer(QObject *parent = 0);
+    explicit BluetoothServer(QObject *parent = nullptr);
     ~BluetoothServer();
 
     static bool hardwareAvailable();
@@ -42,6 +42,8 @@ public:
     void sendData(const QUuid &clientId, const QByteArray &data) override;
     void sendData(const QList<QUuid> &clients, const QByteArray &data) override;
 
+    void terminateClientConnection(const QUuid &clientId) override;
+
 private:
     QBluetoothServer *m_server = nullptr;
     QBluetoothLocalDevice *m_localDevice = nullptr;
@@ -49,7 +51,6 @@ private:
 
     // Client storage
     QHash<QUuid, QBluetoothSocket *> m_clientList;
-    QByteArray m_receiveBuffer;
 
 private slots:
     void onHostModeChanged(const QBluetoothLocalDevice::HostMode &mode);

libnymea-core/cloud/cloudtransport.cpp

@@ -53,6 +53,16 @@ void CloudTransport::sendData(const QList<QUuid> &clientIds, const QByteArray &d
     }
 }
 
+void CloudTransport::terminateClientConnection(const QUuid &clientId)
+{
+    foreach (const ConnectionContext &ctx, m_connections) {
+        if (ctx.clientId == clientId) {
+            ctx.proxyConnection->disconnectServer();
+            return;
+        }
+    }
+}
+
 bool CloudTransport::startServer()
 {
     qCDebug(dcCloud()) << "Started cloud transport";
@@ -80,28 +90,34 @@ void CloudTransport::connectToCloud(const QString &token, const QString &nonce)
     connect(context.proxyConnection, &RemoteProxyConnection::ready, this, &CloudTransport::transportReady);
     connect(context.proxyConnection, &RemoteProxyConnection::stateChanged, this, &CloudTransport::remoteConnectionStateChanged);
     connect(context.proxyConnection, &RemoteProxyConnection::dataReady, this, &CloudTransport::transportDataReady);
+    connect(context.proxyConnection, &RemoteProxyConnection::remoteConnectionEstablished, this, &CloudTransport::transportConnected);
+    connect(context.proxyConnection, &RemoteProxyConnection::disconnected, this, &CloudTransport::transportDisconnected);
 
     context.proxyConnection->connectServer(m_proxyUrl);
 }
 
 void CloudTransport::remoteConnectionStateChanged(RemoteProxyConnection::State state)
+{
+    qCDebug(dcCloudTraffic()) << "Remote connection state changed" << state;
+}
+
+void CloudTransport::transportConnected()
 {
     RemoteProxyConnection *proxyConnection = qobject_cast<RemoteProxyConnection*>(sender());
     ConnectionContext context = m_connections.value(proxyConnection);
 
-    switch (state) {
+    qCDebug(dcCloud()) << "The remote client connected successfully" << proxyConnection->tunnelPartnerName() << proxyConnection->tunnelPartnerUuid();
-    case RemoteProxyConnection::StateRemoteConnected:
+    emit clientConnected(context.clientId);
-        qCDebug(dcCloud()) << "The remote client connected successfully" << proxyConnection->tunnelPartnerName() << proxyConnection->tunnelPartnerUuid();
+}
-        emit clientConnected(context.clientId);
+
-        break;
+void CloudTransport::transportDisconnected()
-    case RemoteProxyConnection::StateDisconnected:
+{
-        qCDebug(dcCloud()) << "The remote connection disconnected.";
+    RemoteProxyConnection *proxyConnection = qobject_cast<RemoteProxyConnection*>(sender());
-        emit clientDisconnected(context.clientId);
+    ConnectionContext context = m_connections.take(proxyConnection);
-        break;
+    proxyConnection->deleteLater();
-    default:
+
-        qCDebug(dcCloud()) << "Remote connection state changed" << state;
+    qCDebug(dcCloud()) << "The remote connection disconnected." << context.clientId;
-        break;
+    emit clientDisconnected(context.clientId);
-    }
 }
 
 void CloudTransport::transportReady()
@@ -118,7 +134,7 @@ void CloudTransport::transportDataReady(const QByteArray &data)
 {
     RemoteProxyConnection *proxyConnection = qobject_cast<RemoteProxyConnection*>(sender());
     ConnectionContext context = m_connections.value(proxyConnection);
-    qCDebug(dcCloudTraffic()) << "Date received:" << context.clientId.toString() << data;
+    qCDebug(dcCloudTraffic()) << "Data received:" << context.clientId.toString() << data;
     emit dataAvailable(context.clientId, data);
 }
 

libnymea-core/cloud/cloudtransport.h

@@ -36,6 +36,8 @@ public:
     void sendData(const QUuid &clientId, const QByteArray &data) override;
     void sendData(const QList<QUuid> &clientIds, const QByteArray &data) override;
 
+    void terminateClientConnection(const QUuid &clientId) override;
+
     bool startServer() override;
     bool stopServer() override;
 
@@ -43,11 +45,13 @@ signals:
 
 public slots:
     void connectToCloud(const QString &token, const QString &nonce);
-    void remoteConnectionStateChanged(remoteproxyclient::RemoteProxyConnection::State state);
 
 private slots:
+    void remoteConnectionStateChanged(remoteproxyclient::RemoteProxyConnection::State state);
+    void transportConnected();
     void transportReady();
     void transportDataReady(const QByteArray &data);
+    void transportDisconnected();
 
 private:
     QUrl m_proxyUrl;

libnymea-core/jsonrpc/jsonrpcserver.cpp

@@ -500,6 +500,30 @@ void JsonRPCServer::processData(const QUuid &clientId, const QByteArray &data)
     qCDebug(dcJsonRpcTraffic()) << "Incoming data:" << data;
 
     TransportInterface *interface = qobject_cast<TransportInterface *>(sender());
+
+    // Handle packet fragmentation
+    QByteArray buffer = m_clientBuffers[clientId];
+    buffer.append(data);
+    int splitIndex = buffer.indexOf("}\n{");
+    while (splitIndex > -1) {
+        processJsonPacket(interface, clientId, buffer.left(splitIndex + 1));
+        buffer = buffer.right(buffer.length() - splitIndex - 2);
+        splitIndex = buffer.indexOf("}\n{");
+    }
+    if (buffer.trimmed().endsWith("}")) {
+        processJsonPacket(interface, clientId, buffer);
+        buffer.clear();
+    }
+    m_clientBuffers[clientId] = buffer;
+
+    if (buffer.size() > 1024 * 10) {
+        qCWarning(dcJsonRpc()) << "Client buffer larger than 10KB and no valid data. Dropping client connection.";
+        interface->terminateClientConnection(clientId);
+    }
+}
+
+void JsonRPCServer::processJsonPacket(TransportInterface *interface, const QUuid &clientId, const QByteArray &data)
+{
     QJsonParseError error;
     QJsonDocument jsonDoc = QJsonDocument::fromJson(data, &error);
 
@@ -716,6 +740,7 @@ void JsonRPCServer::clientDisconnected(const QUuid &clientId)
     qCDebug(dcJsonRpc()) << "Client disconnected:" << clientId;
     m_clientTransports.remove(clientId);
     m_clientNotifications.remove(clientId);
+    m_clientBuffers.remove(clientId);
     if (m_pushButtonTransactions.values().contains(clientId)) {
         NymeaCore::instance()->userManager()->cancelPushButtonAuth(m_pushButtonTransactions.key(clientId));
     }

libnymea-core/jsonrpc/jsonrpcserver.h

@@ -79,6 +79,8 @@ private:
     void sendUnauthorizedResponse(TransportInterface *interface, const QUuid &clientId, int commandId, const QString &error);
     QVariantMap createWelcomeMessage(TransportInterface *interface) const;
 
+    void processJsonPacket(TransportInterface *interface, const QUuid &clientId, const QByteArray &data);
+
 private slots:
     void setup();
 
@@ -101,6 +103,7 @@ private:
     QHash<JsonReply *, TransportInterface *> m_asyncReplies;
 
     QHash<QUuid, TransportInterface*> m_clientTransports;
+    QHash<QUuid, QByteArray> m_clientBuffers;
     QHash<QUuid, bool> m_clientNotifications;
     QHash<int, QUuid> m_pushButtonTransactions;
 

libnymea-core/mocktcpserver.cpp

@@ -52,6 +52,12 @@ void MockTcpServer::sendData(const QList<QUuid> &clients, const QByteArray &data
     }
 }
 
+void MockTcpServer::terminateClientConnection(const QUuid &clientId)
+{
+    emit connectionTerminated(clientId);
+    emit clientDisconnected(clientId);
+}
+
 QList<MockTcpServer *> MockTcpServer::servers()
 {
     return s_allServers;

libnymea-core/mocktcpserver.h

@@ -35,17 +35,19 @@ class MockTcpServer : public TransportInterface
 {
     Q_OBJECT
 public:
-    explicit MockTcpServer(QObject *parent = 0);
+    explicit MockTcpServer(QObject *parent = nullptr);
-    ~MockTcpServer();
+    ~MockTcpServer() override;
 
     void sendData(const QUuid &clientId, const QByteArray &data) override;
     void sendData(const QList<QUuid> &clients, const QByteArray &data) override;
+    void terminateClientConnection(const QUuid &clientId) override;
 
 /************** Used for testing **************************/
     static QList<MockTcpServer*> servers();
     void injectData(const QUuid &clientId, const QByteArray &data);
 signals:
     void outgoingData(const QUuid &clientId, const QByteArray &data);
+    void connectionTerminated(const QUuid &clientId);
 /************** Used for testing **************************/
 
 public slots:

libnymea-core/tcpserver.cpp

@@ -105,6 +105,14 @@ void TcpServer::sendData(const QList<QUuid> &clients, const QByteArray &data)
     }
 }
 
+void TcpServer::terminateClientConnection(const QUuid &clientId)
+{
+    QTcpSocket *client = m_clientList.value(clientId);
+    if (client) {
+        client->abort();
+    }
+}
+
 /*! Sending \a data to the client with the given \a clientId.*/
 void TcpServer::sendData(const QUuid &clientId, const QByteArray &data)
 {
@@ -245,6 +253,8 @@ void SslServer::incomingConnection(qintptr socketDescriptor)
 {
     QSslSocket *sslSocket = new QSslSocket(this);
 
+    qCDebug(dcTcpServer()) << "New client socket connection:" << sslSocket;
+
     connect(sslSocket, &QSslSocket::encrypted, [this, sslSocket](){ emit clientConnected(sslSocket); });
     connect(sslSocket, &QSslSocket::readyRead, this, &SslServer::onSocketReadyRead);
     connect(sslSocket, &QSslSocket::disconnected, this, &SslServer::onClientDisconnected);
@@ -265,6 +275,7 @@ void SslServer::incomingConnection(qintptr socketDescriptor)
 void SslServer::onClientDisconnected()
 {
     QSslSocket *socket = static_cast<QSslSocket*>(sender());
+    qCDebug(dcTcpServer()) << "Client socket disconnected:" << socket;
     emit clientDisconnected(socket);
     socket->deleteLater();
 }
@@ -273,18 +284,8 @@ void SslServer::onSocketReadyRead()
 {
     QSslSocket *socket = static_cast<QSslSocket*>(sender());
     QByteArray data = socket->readAll();
-    qCDebug(dcTcpServerTraffic()) << "SocketReadyRead:" << data;
+    qCDebug(dcTcpServerTraffic()) << "Reading socket data:" << data;
-    m_receiveBuffer.append(data);
+    emit dataAvailable(socket, data);
-    int splitIndex = m_receiveBuffer.indexOf("}\n{");
-    while (splitIndex > -1) {
-        emit dataAvailable(socket, m_receiveBuffer.left(splitIndex + 1));
-        m_receiveBuffer = m_receiveBuffer.right(m_receiveBuffer.length() - splitIndex - 2);
-        splitIndex = m_receiveBuffer.indexOf("}\n{");
-    }
-    if (m_receiveBuffer.endsWith("}\n")) {
-        emit dataAvailable(socket, m_receiveBuffer);
-        m_receiveBuffer.clear();
-    }
 }
 
 }

libnymea-core/tcpserver.h

@@ -65,7 +65,6 @@ private slots:
 private:
     bool m_sslEnabled = false;
     QSslConfiguration m_config;
-    QByteArray m_receiveBuffer;
 };
 
 class TcpServer : public TransportInterface
@@ -80,6 +79,8 @@ public:
     void sendData(const QUuid &clientId, const QByteArray &data) override;
     void sendData(const QList<QUuid> &clients, const QByteArray &data) override;
 
+    void terminateClientConnection(const QUuid &clientId) override;
+
 private:
     QTimer *m_timer;
 

libnymea-core/transportinterface.cpp

@@ -62,6 +62,11 @@
     Pure virtual method for sending \a data to \a clients over the corresponding \l{TransportInterface}.
 */
 
+/*! \fn void nymeaserver::TransportInterface::terminateClientConnection(const QUuid &clientId);
+    Pure virtual method for terminating \a clients connection. The JSON RPC server might call this when a
+    client violates the protocol. Transports should immediately abort the connection to the client.
+*/
+
 /*! \fn void nymeaserver::TransportInterface::dataAvailable(const QUuid &clientId, const QByteArray &data);
     This signal is emitted when valid \a data from the client with the given \a clientId are available.
 

libnymea-core/transportinterface.h

@@ -40,6 +40,8 @@ public:
     virtual void sendData(const QUuid &clientId, const QByteArray &data) = 0;
     virtual void sendData(const QList<QUuid> &clients, const QByteArray &data) = 0;
 
+    virtual void terminateClientConnection(const QUuid &clientId) = 0;
+
     void setConfiguration(const ServerConfiguration &config);
     ServerConfiguration configuration() const;
 

libnymea-core/websocketserver.cpp

@@ -109,6 +109,14 @@ void WebSocketServer::sendData(const QList<QUuid> &clients, const QByteArray &da
     }
 }
 
+void WebSocketServer::terminateClientConnection(const QUuid &clientId)
+{
+    QWebSocket *client = m_clientList.value(clientId);
+    if (client) {
+        client->abort();
+    }
+}
+
 QHash<QString, QString> WebSocketServer::createTxtRecord()
 {
     // Note: reversed order

libnymea-core/websocketserver.h

@@ -50,6 +50,8 @@ public:
     void sendData(const QUuid &clientId, const QByteArray &data) override;
     void sendData(const QList<QUuid> &clients, const QByteArray &data) override;
 
+    void terminateClientConnection(const QUuid &clientId) override;
+
 private:
     QWebSocketServer *m_server = nullptr;
     QHash<QUuid, QWebSocket *> m_clientList;

tests/auto/jsonrpc/testjsonrpc.cpp

@@ -41,6 +41,8 @@ class TestJSONRPC: public NymeaTestBase
     Q_OBJECT
 
 private slots:
+    void initTestCase();
+
     void testHandshake();
 
     void testInitialSetup();
@@ -100,6 +102,12 @@ private slots:
     void testPushButtonAuthConnectionDrop();
 
     void testInitialSetupWithPushButtonAuth();
+
+    void testDataFragmentation_data();
+    void testDataFragmentation();
+
+    void testGarbageData();
+
 private:
     QStringList extractRefs(const QVariant &variant);
 
@@ -129,6 +137,13 @@ QStringList TestJSONRPC::extractRefs(const QVariant &variant)
     return QStringList();
 }
 
+void TestJSONRPC::initTestCase()
+{
+    NymeaTestBase::initTestCase();
+    QLoggingCategory::setFilterRules("*.debug=false\n"
+                                     "JsonRpc*.debug=true");
+}
+
 void TestJSONRPC::testHandshake()
 {
     // first test if the handshake message is auto-sent upon connecting
@@ -179,7 +194,7 @@ void TestJSONRPC::testInitialSetup()
     QVERIFY(spy.isValid());
 
     // Introspect call should work in any case
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Introspect\"}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Introspect\"}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -192,7 +207,7 @@ void TestJSONRPC::testInitialSetup()
 
     // Hello call should work in any case too
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Hello\"}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Hello\"}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -205,7 +220,7 @@ void TestJSONRPC::testInitialSetup()
 
     // Any other call should fail with "unauthorized" even if we use a previously valid token
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"token\": \"" + m_apiToken + "\", \"method\": \"JSONRPC.Version\"}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"token\": \"" + m_apiToken + "\", \"method\": \"JSONRPC.Version\"}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -219,7 +234,7 @@ void TestJSONRPC::testInitialSetup()
 
     // But it should still fail when giving a an invalid username
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.CreateUser\", \"params\": {\"username\": \"dummy\", \"password\": \"DummyPW1!\"}}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.CreateUser\", \"params\": {\"username\": \"dummy\", \"password\": \"DummyPW1!\"}}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -232,7 +247,7 @@ void TestJSONRPC::testInitialSetup()
 
     // or when giving a bad password
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.CreateUser\", \"params\": {\"username\": \"dummy@guh.io\", \"password\": \"weak\"}}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.CreateUser\", \"params\": {\"username\": \"dummy@guh.io\", \"password\": \"weak\"}}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -245,7 +260,7 @@ void TestJSONRPC::testInitialSetup()
 
     // Now lets play by the rules (with an uppercase email)
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.CreateUser\", \"params\": {\"username\": \"Dummy@guh.io\", \"password\": \"DummyPW1!\"}}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.CreateUser\", \"params\": {\"username\": \"Dummy@guh.io\", \"password\": \"DummyPW1!\"}}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -258,7 +273,7 @@ void TestJSONRPC::testInitialSetup()
 
     // Now that we have a user, initialSetup should be false in the Hello call
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Hello\"}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Hello\"}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -271,7 +286,7 @@ void TestJSONRPC::testInitialSetup()
 
     // Calls should still fail, given we didn't get a new token yet
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"token\": \"" + m_apiToken + "\", \"method\": \"JSONRPC.Version\"}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"token\": \"" + m_apiToken + "\", \"method\": \"JSONRPC.Version\"}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -283,7 +298,7 @@ void TestJSONRPC::testInitialSetup()
 
     // Now lets authenticate with a wrong user
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Authenticate\", \"params\": {\"username\": \"Dummy@wrong.domain\", \"password\": \"DummyPW1!\", \"deviceName\": \"testcase\"}}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Authenticate\", \"params\": {\"username\": \"Dummy@wrong.domain\", \"password\": \"DummyPW1!\", \"deviceName\": \"testcase\"}}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -298,7 +313,7 @@ void TestJSONRPC::testInitialSetup()
 
     // Now lets authenticate with a wrong password
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Authenticate\", \"params\": {\"username\": \"Dummy@guh.io\", \"password\": \"wrongpw\", \"deviceName\": \"testcase\"}}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Authenticate\", \"params\": {\"username\": \"Dummy@guh.io\", \"password\": \"wrongpw\", \"deviceName\": \"testcase\"}}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -313,7 +328,7 @@ void TestJSONRPC::testInitialSetup()
 
     // Now lets authenticate for real (but intentionally use a lowercase email here, should still work)
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Authenticate\", \"params\": {\"username\": \"dummy@guh.io\", \"password\": \"DummyPW1!\", \"deviceName\": \"testcase\"}}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Authenticate\", \"params\": {\"username\": \"dummy@guh.io\", \"password\": \"DummyPW1!\", \"deviceName\": \"testcase\"}}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -328,7 +343,7 @@ void TestJSONRPC::testInitialSetup()
 
     // Now do a Version call with the valid token and it should work
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"token\": \"" + m_apiToken + "\", \"method\": \"JSONRPC.Version\"}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"token\": \"" + m_apiToken + "\", \"method\": \"JSONRPC.Version\"}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -347,7 +362,7 @@ void TestJSONRPC::testRevokeToken()
 
     // Now get all the tokens
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 123, \"token\": \"" + m_apiToken + "\", \"method\": \"JSONRPC.Tokens\"}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 123, \"token\": \"" + m_apiToken + "\", \"method\": \"JSONRPC.Tokens\"}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -362,7 +377,7 @@ void TestJSONRPC::testRevokeToken()
 
     // Authenticate and create a new token
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Authenticate\", \"params\": {\"username\": \"dummy@guh.io\", \"password\": \"DummyPW1!\", \"deviceName\": \"testcase\"}}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Authenticate\", \"params\": {\"username\": \"dummy@guh.io\", \"password\": \"DummyPW1!\", \"deviceName\": \"testcase\"}}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -377,7 +392,7 @@ void TestJSONRPC::testRevokeToken()
 
     // Now do a Version call with the new token and it should work
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"token\": \"" + newToken + "\", \"method\": \"JSONRPC.Version\"}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"token\": \"" + newToken + "\", \"method\": \"JSONRPC.Version\"}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -389,7 +404,7 @@ void TestJSONRPC::testRevokeToken()
 
     // Now get all the tokens using the old token
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 123, \"token\": \"" + m_apiToken + "\", \"method\": \"JSONRPC.Tokens\"}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 123, \"token\": \"" + m_apiToken + "\", \"method\": \"JSONRPC.Tokens\"}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -412,7 +427,7 @@ void TestJSONRPC::testRevokeToken()
 
     // Revoke the new token
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 123, \"token\": \"" + m_apiToken + "\", \"method\": \"JSONRPC.RemoveToken\", \"params\": {\"tokenId\": \"" + newTokenId.toByteArray() + "\"}}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 123, \"token\": \"" + m_apiToken + "\", \"method\": \"JSONRPC.RemoveToken\", \"params\": {\"tokenId\": \"" + newTokenId.toByteArray() + "\"}}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -424,7 +439,7 @@ void TestJSONRPC::testRevokeToken()
 
     // Do a call with the now removed token, it should be forbidden
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"token\": \"" + newToken + "\", \"method\": \"JSONRPC.Version\"}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"token\": \"" + newToken + "\", \"method\": \"JSONRPC.Version\"}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -441,14 +456,16 @@ void TestJSONRPC::testBasicCall_data()
     QTest::addColumn<bool>("idValid");
     QTest::addColumn<bool>("valid");
 
-    QTest::newRow("valid call") << QByteArray("{\"id\":42, \"method\":\"JSONRPC.Introspect\"}") << true << true;
+    QTest::newRow("valid call 1") << QByteArray("{\"id\":42, \"method\":\"JSONRPC.Introspect\"}") << true << true;
-    QTest::newRow("missing id") << QByteArray("{\"method\":\"JSONRPC.Introspect\"}") << false << false;
+    QTest::newRow("valid call 2") << QByteArray("{\"id\":42, \"method\":\"JSONRPC.Introspect\"}\n") << true << true;
-    QTest::newRow("missing method") << QByteArray("{\"id\":42}") << true << false;
+    QTest::newRow("valid call 3") << QByteArray("{\"id\":42, \"method\":\"JSONRPC.Introspect\"}\n\n\n\n") << true << true;
-    QTest::newRow("borked") << QByteArray("{\"id\":42, \"method\":\"JSO") << false << false;
+    QTest::newRow("missing id") << QByteArray("{\"method\":\"JSONRPC.Introspect\"}\n") << false << false;
-    QTest::newRow("invalid function") << QByteArray("{\"id\":42, \"method\":\"JSONRPC.Foobar\"}") << true << false;
+    QTest::newRow("missing method") << QByteArray("{\"id\":42}\n") << true << false;
-    QTest::newRow("invalid namespace") << QByteArray("{\"id\":42, \"method\":\"FOO.Introspect\"}") << true << false;
+    QTest::newRow("borked") << QByteArray("{\"id\":42, \"method\":\"JSO}\n") << false << false;
-    QTest::newRow("missing dot") << QByteArray("{\"id\":42, \"method\":\"JSONRPCIntrospect\"}") << true << false;
+    QTest::newRow("invalid function") << QByteArray("{\"id\":42, \"method\":\"JSONRPC.Foobar\"}\n") << true << false;
-    QTest::newRow("invalid params") << QByteArray("{\"id\":42, \"method\":\"JSONRPC.Introspect\", \"params\":{\"törööö\":\"chooo-chooo\"}}") << true << false;
+    QTest::newRow("invalid namespace") << QByteArray("{\"id\":42, \"method\":\"FOO.Introspect\"}\n") << true << false;
+    QTest::newRow("missing dot") << QByteArray("{\"id\":42, \"method\":\"JSONRPCIntrospect\"}\n") << true << false;
+    QTest::newRow("invalid params") << QByteArray("{\"id\":42, \"method\":\"JSONRPC.Introspect\", \"params\":{\"törööö\":\"chooo-chooo\"}}\n") << true << false;
 }
 
 void TestJSONRPC::testBasicCall()
@@ -1136,7 +1153,7 @@ void TestJSONRPC::testInitialSetupWithPushButtonAuth()
 
     // Hello call should work in any case, telling us initial setup is required
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Hello\"}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Hello\"}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -1176,7 +1193,7 @@ void TestJSONRPC::testInitialSetupWithPushButtonAuth()
 
     // initialSetupRequired should be false in Hello call now
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Hello\"}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.Hello\"}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -1190,7 +1207,7 @@ void TestJSONRPC::testInitialSetupWithPushButtonAuth()
 
     // CreateUser without a token should fail now even though there are 0 users in the DB
     spy.clear();
-    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.CreateUser\", \"params\": {\"username\": \"Dummy@guh.io\", \"password\": \"DummyPW1!\"}}");
+    m_mockTcpServer->injectData(m_clientId, "{\"id\": 555, \"method\": \"JSONRPC.CreateUser\", \"params\": {\"username\": \"Dummy@guh.io\", \"password\": \"DummyPW1!\"}}\n");
     if (spy.count() == 0) {
         spy.wait();
     }
@@ -1203,6 +1220,66 @@ void TestJSONRPC::testInitialSetupWithPushButtonAuth()
 
 }
 
+void TestJSONRPC::testDataFragmentation_data()
+{
+    QTest::addColumn<QList<QByteArray> >("packets");
+
+    QList<QByteArray> packets;
+
+    packets.append("{\"id\": 555, \"method\": \"JSONRPC.Hello\"}\n");
+    QTest::newRow("1 packet") << packets;
+
+    packets.clear();
+    packets.append("{\"id\": 555, \"m");
+    packets.append("ethod\": \"JSONRPC.Hello\"}\n");
+    QTest::newRow("2 packets") << packets;
+
+    packets.clear();
+    packets.append("{\"id\": 555, \"m");
+    packets.append("ethod\": \"JSONRP");
+    packets.append("C.Hello\"}\n");
+    QTest::newRow("3 packets") << packets;
+
+    packets.clear();
+    packets.append("{\"id\": 555, \"method\": \"JSONRPC.Hello\"}\n{\"id\": 5556, \"metho");
+    QTest::newRow("next packet start appended") << packets;
+}
+
+void TestJSONRPC::testDataFragmentation()
+{
+    QJsonDocument jsonDoc;
+    QSignalSpy spy(m_mockTcpServer, SIGNAL(outgoingData(QUuid,QByteArray)));
+
+    QFETCH(QList<QByteArray>, packets);
+
+    foreach (const QByteArray &packet, packets) {
+        spy.clear();
+        m_mockTcpServer->injectData(m_clientId, packet);
+    }
+    if (spy.count() == 0) {
+        spy.wait();
+    }
+    QCOMPARE(spy.count(), 1);
+    jsonDoc = QJsonDocument::fromJson(spy.first().at(1).toByteArray());
+    QCOMPARE(jsonDoc.toVariant().toMap().value("status").toString(), QStringLiteral("success"));
+}
+
+void TestJSONRPC::testGarbageData()
+{
+    QSignalSpy spy(m_mockTcpServer, &MockTcpServer::connectionTerminated);
+
+    QByteArray data;
+    for (int i = 0; i < 1024; i++) {
+        data.append("a");
+    }
+    for (int i = 0; i < 11; i ++) {
+        m_mockTcpServer->injectData(m_clientId, data);
+    }
+
+    QCOMPARE(spy.count(), 1);
+
+}
+
 #include "testjsonrpc.moc"
 
 QTEST_MAIN(TestJSONRPC)

tests/auto/nymeatestbase.cpp

@@ -98,20 +98,6 @@ EventTypeId mockParentChildEventId = EventTypeId("d24ede5f-4064-4898-bb84-cfb533
 ActionTypeId mockParentChildActionId = ActionTypeId("d24ede5f-4064-4898-bb84-cfb533b1fbc0");
 StateTypeId mockParentChildStateId = StateTypeId("d24ede5f-4064-4898-bb84-cfb533b1fbc0");
 
-static QHash<QString, bool> s_loggingFilters;
-
-static void loggingCategoryFilter(QLoggingCategory *category)
-{
-    if (s_loggingFilters.contains(category->categoryName())) {
-        bool debugEnabled = s_loggingFilters.value(category->categoryName());
-        category->setEnabled(QtDebugMsg, debugEnabled);
-        category->setEnabled(QtWarningMsg, debugEnabled || s_loggingFilters.value("Warnings"));
-    } else {
-        category->setEnabled(QtDebugMsg, true);
-        category->setEnabled(QtWarningMsg, true);
-    }
-}
-
 NymeaTestBase::NymeaTestBase(QObject *parent) :
     QObject(parent),
     m_commandId(0)
@@ -144,39 +130,7 @@ void NymeaTestBase::initTestCase()
     NymeaSettings nymeadSettings(NymeaSettings::SettingsRoleGlobal);
     nymeadSettings.clear();
 
-    // debug categories
+    QLoggingCategory::setFilterRules("*.debug=false");
-    // logging filers for core and libnymea
-    s_loggingFilters.insert("Application", true);
-    s_loggingFilters.insert("Warnings", true);
-    s_loggingFilters.insert("DeviceManager", true);
-    s_loggingFilters.insert("RuleEngine", true);
-    s_loggingFilters.insert("Hardware", true);
-    s_loggingFilters.insert("Connection", true);
-    s_loggingFilters.insert("LogEngine", true);
-    s_loggingFilters.insert("TcpServer", true);
-    s_loggingFilters.insert("WebServer", true);
-    s_loggingFilters.insert("WebSocketServer", true);
-    s_loggingFilters.insert("JsonRpc", true);
-    s_loggingFilters.insert("Rest", true);
-    s_loggingFilters.insert("OAuth2", true);
-    s_loggingFilters.insert("TimeManager", true);
-
-
-    QHash<QString, bool> loggingFiltersPlugins;
-    foreach (const QJsonObject &pluginMetadata, DeviceManager::pluginsMetadata()) {
-        loggingFiltersPlugins.insert(pluginMetadata.value("idName").toString(), false);
-    }
-
-    // add plugin metadata to the static hash
-    foreach (const QString &category, loggingFiltersPlugins.keys()) {
-        if (category == "MockDevice") {
-            s_loggingFilters.insert(category, true);
-        } else {
-            s_loggingFilters.insert(category, false);
-        }
-    }
-
-    QLoggingCategory::installFilter(loggingCategoryFilter);
 
     // Start the server
     NymeaCore::instance();
@@ -239,7 +193,7 @@ QVariant NymeaTestBase::injectAndWait(const QString &method, const QVariantMap &
     QJsonDocument jsonDoc = QJsonDocument::fromVariant(call);
     QSignalSpy spy(m_mockTcpServer, SIGNAL(outgoingData(QUuid,QByteArray)));
 
-    m_mockTcpServer->injectData(clientId.isNull() ? m_clientId : clientId, jsonDoc.toJson(QJsonDocument::Compact));
+    m_mockTcpServer->injectData(clientId.isNull() ? m_clientId : clientId, jsonDoc.toJson(QJsonDocument::Compact) + "\n");
 
     if (spy.count() == 0) {
         spy.wait();

tests/auto/nymeatestbase.h

@@ -101,7 +101,7 @@ class NymeaTestBase : public QObject
 {
     Q_OBJECT
 public:
-    explicit NymeaTestBase(QObject *parent = 0);
+    explicit NymeaTestBase(QObject *parent = nullptr);
 
 protected slots:
     void initTestCase();