Fix handling of incomplete messages in client/server communication

Do not assume that the socket always contains enough data. Instead,
prefix every 'packet' with its size, and back off until the full
packet is available.

The actual encoding/decoding is done in Protocol::sendPacket,
Protocol::receivePacket. To be able to use the methods everywhere,
replies are now prefixed by a Protocol::Reply command.

Change-Id: I75a89605b2cc3fe2f2f841d8e3159fc8aea65d77
Reviewed-by: Karsten Heimrich <karsten.heimrich@theqtcompany.com>

1 files changed, 176 insertions, 38 deletions

diff --git a/tests/auto/installer/clientserver/tst_clientserver.cpp b/tests/auto/installer/clientserver/tst_clientserver.cpp
index 316bc3051..e5be5a8a0 100644
--- a/tests/auto/installer/clientserver/tst_clientserver.cpp
+++ b/tests/auto/installer/clientserver/tst_clientserver.cpp
@@ -39,12 +39,14 @@
 #include <remotefileengine.h>
 #include <remoteserver.h>
 
+#include <QBuffer>
 #include <QSettings>
 #include <QLocalSocket>
 #include <QTest>
 #include <QSignalSpy>
 #include <QTemporaryFile>
 #include <QUuid>
+#include <QLocalServer>
 
 using namespace QInstaller;
 
@@ -52,12 +54,154 @@ class tst_ClientServer : public QObject
 {
     Q_OBJECT
 
+private:
+    template<typename T>
+    void sendCommand(QIODevice *device, const QByteArray &cmd, T t)
+    {
+        QByteArray data;
+        QDataStream stream(&data, QIODevice::WriteOnly);
+        stream << t;
+        sendPacket(device, cmd, data);
+    }
+
+    template<typename T>
+    void receiveCommand(QIODevice *device, QByteArray *cmd, T *t)
+    {
+        QByteArray data;
+        while (!receivePacket(device, cmd, &data))
+            device->waitForReadyRead(-1);
+        QDataStream stream(&data, QIODevice::ReadOnly);
+        stream >> *t;
+        QCOMPARE(stream.status(), QDataStream::Ok);
+        QVERIFY(stream.atEnd());
+    }
+
 private slots:
     void initTestCase()
     {
         RemoteClient::instance().setActive(true);
     }
 
+    void sendReceivePacket()
+    {
+        QByteArray validPackage;
+        typedef qint32 PackageSize;
+
+        // first try sendPacket ...
+        {
+            QBuffer device(&validPackage);
+            device.open(QBuffer::WriteOnly);
+
+            const QByteArray cmd = "say";
+            const QByteArray data = "hello" ;
+            QInstaller::sendPacket(&device, cmd, data);
+
+            // 1 is delimiter (\0)
+            QCOMPARE(device.buffer().size(), (int)sizeof(PackageSize) + cmd.size() + 1 + data.size());
+            QCOMPARE(device.buffer().right(data.size()), data);
+            QCOMPARE(device.buffer().mid(sizeof(PackageSize), cmd.size()), cmd);
+        }
+
+        // now try successful receivePacket ...
+        {
+            QBuffer device(&validPackage);
+            device.open(QBuffer::ReadOnly);
+
+            QByteArray cmd;
+            QByteArray data;
+            QCOMPARE(QInstaller::receivePacket(&device, &cmd, &data), true);
+
+            QCOMPARE(device.pos(), device.size());
+            QCOMPARE(cmd, QByteArray("say"));
+            QCOMPARE(data, QByteArray("hello"));
+        }
+
+
+        // now try read of incomplete packet ...
+        {
+            QByteArray incompletePackage = validPackage;
+            char toStrip = validPackage.at(validPackage.size() - 1);
+            incompletePackage.resize(incompletePackage.size() - 1);
+            QBuffer device(&incompletePackage);
+            device.open(QBuffer::ReadOnly);
+
+            QByteArray cmd;
+            QByteArray data;
+            QCOMPARE(QInstaller::receivePacket(&device, &cmd, &data), false);
+
+            QCOMPARE(device.pos(), 0);
+            QCOMPARE(cmd, QByteArray());
+            QCOMPARE(data, QByteArray());
+
+            // make packet complete again, retry
+            device.buffer().append(toStrip);
+            QCOMPARE(device.buffer(), validPackage);
+
+            QCOMPARE(QInstaller::receivePacket(&device, &cmd, &data), true);
+
+            QCOMPARE(device.pos(), device.size());
+            QCOMPARE(cmd, QByteArray("say"));
+            QCOMPARE(data, QByteArray("hello"));
+        }
+    }
+
+    void localSocket()
+    {
+        //
+        // test roundtrip of a (big) packet via QLocalSocket
+        //
+        const QString socketName(__FUNCTION__);
+        QLocalServer::removeServer(socketName);
+
+        QEventLoop loop;
+
+        const QByteArray command = "HELLO";
+        const QByteArray message(10905, '0');
+
+        QLocalServer server;
+        { // server
+            QLocalSocket *rcv = 0;
+            auto srvDataArrived = [&]() {
+                QByteArray command, message;
+                if (!receivePacket(rcv, &command, &message))
+                    return;
+                sendPacket(rcv, command, message);
+            };
+
+            connect(&server, &QLocalServer::newConnection, [&,srvDataArrived]() {
+                rcv = server.nextPendingConnection();
+                connect(rcv, &QLocalSocket::readyRead, srvDataArrived);
+            });
+
+            server.listen(socketName);
+        }
+
+
+        QLocalSocket snd;
+        { // client
+            auto clientDataArrived = [&]() {
+                QByteArray cmd, msg;
+                if (!receivePacket(&snd, &cmd, &msg))
+                    return;
+                QCOMPARE(cmd, command);
+                QCOMPARE(msg, message);
+                loop.exit();
+            };
+
+            connect(&snd, &QLocalSocket::readyRead, clientDataArrived);
+
+            QTimer::singleShot(0, [&]() {
+                snd.connect(&snd, &QLocalSocket::connected, [&](){
+                    sendPacket(&snd,  command, message);
+                });
+                snd.connectToServer(socketName);
+            });
+        }
+
+        loop.exec();
+    }
+
+
     void testServerConnectDebug()
     {
         RemoteServer server;
@@ -72,28 +216,25 @@ private slots:
         QVERIFY2(socket.waitForConnected(), "Could not connect to server.");
         QCOMPARE(socket.state() == QLocalSocket::ConnectedState, true);
 
-        QDataStream stream;
-        stream.setDevice(&socket);
-        stream << QString::fromLatin1(Protocol::Authorize) << QString(Protocol::DefaultAuthorizationKey);
-
-        socket.waitForBytesWritten(-1);
-        if (!socket.bytesAvailable())
-            socket.waitForReadyRead(-1);
+        sendCommand(&socket, Protocol::Authorize, QString(Protocol::DefaultAuthorizationKey));
 
-        quint32 size; stream >> size;
-        bool authorized;
-        stream >> authorized;
-        QCOMPARE(authorized, true);
+        {
+            QByteArray command;
+            bool authorized;
+            receiveCommand(&socket, &command, &authorized);
+            QCOMPARE(command, QByteArray(Protocol::Reply));
+            QCOMPARE(authorized, true);
+        }
 
-        socket.flush();
-        stream << QString::fromLatin1(Protocol::Authorize) << QString("SomeKey");
-        socket.waitForBytesWritten(-1);
-        if (!socket.bytesAvailable())
-            socket.waitForReadyRead(-1);
+        sendCommand(&socket, Protocol::Authorize, QString::fromLatin1("Some Key"));
 
-        stream >> size;
-        stream >> authorized;
-        QCOMPARE(authorized, false);
+        {
+            QByteArray command;
+            bool authorized;
+            receiveCommand(&socket, &command, &authorized);
+            QCOMPARE(command, QByteArray(Protocol::Reply));
+            QCOMPARE(authorized, false);
+        }
     }
 
     void testServerConnectRelease()
@@ -108,28 +249,25 @@ private slots:
         QVERIFY2(socket.waitForConnected(), "Could not connect to server.");
         QCOMPARE(socket.state() == QLocalSocket::ConnectedState, true);
 
-        QDataStream stream;
-        stream.setDevice(&socket);
-        stream << QString::fromLatin1(Protocol::Authorize) << QString("SomeKey");
+        sendCommand(&socket, Protocol::Authorize, QString::fromLatin1("SomeKey"));
 
-        socket.waitForBytesWritten(-1);
-        if (!socket.bytesAvailable())
-            socket.waitForReadyRead(-1);
-
-        quint32 size; stream >> size;
-        bool authorized;
-        stream >> authorized;
-        QCOMPARE(authorized, true);
+        {
+            QByteArray command;
+            bool authorized;
+            receiveCommand(&socket, &command, &authorized);
+            QCOMPARE(command, QByteArray(Protocol::Reply));
+            QCOMPARE(authorized, true);
+        }
 
-        socket.flush();
-        stream << QString::fromLatin1(Protocol::Authorize) << QString(Protocol::DefaultAuthorizationKey);
-        socket.waitForBytesWritten(-1);
-        if (!socket.bytesAvailable())
-            socket.waitForReadyRead(-1);
+        sendCommand(&socket, Protocol::Authorize, QString::fromLatin1(Protocol::DefaultAuthorizationKey));
 
-        stream >> size;
-        stream >> authorized;
-        QCOMPARE(authorized, false);
+        {
+            QByteArray command;
+            bool authorized;
+            receiveCommand(&socket, &command, &authorized);
+            QCOMPARE(command, QByteArray(Protocol::Reply));
+            QCOMPARE(authorized, false);
+        }
     }
 
     void testQSettingsWrapper()