QmlDebug: Robustify packet protocol

The packet sizes need to be explicitly little endian. So far the
protocol would fail if the endpoints have different endianness.
Furthermore short reads or writes from the device should not crash the
application. We translate them to a protocol error now.

It's disturbing that no client cares about protocol errors right now,
but that is material for a different change.

Change-Id: Ibcbb170150042cc15a0c3d5d65d5df8365d7fdf9
Task-number: QTBUG-68721
Reviewed-by: Tobias Hunger <tobias.hunger@qt.io>

2 files changed, 39 insertions, 20 deletions

diff --git a/src/libs/qmldebug/qpacketprotocol.cpp b/src/libs/qmldebug/qpacketprotocol.cpp
index 76b641a6a5..dbe4088ed2 100644
--- a/src/libs/qmldebug/qpacketprotocol.cpp
+++ b/src/libs/qmldebug/qpacketprotocol.cpp
@@ -26,6 +26,7 @@
 #include "qpacketprotocol.h"
 
 #include <qelapsedtimer.h>
+#include <qendian.h>
 
 namespace QmlDebug {
 
@@ -102,8 +103,8 @@ public:
 
         QObject::connect(this, &QPacketProtocolPrivate::readyRead,
                          parent, &QPacketProtocol::readyRead);
-        QObject::connect(this, &QPacketProtocolPrivate::invalidPacket,
-                         parent, &QPacketProtocol::invalidPacket);
+        QObject::connect(this, &QPacketProtocolPrivate::protocolError,
+                         parent, &QPacketProtocol::protocolError);
         QObject::connect(dev, &QIODevice::readyRead,
                          this, &QPacketProtocolPrivate::readyToRead);
         QObject::connect(dev, &QIODevice::aboutToClose,
@@ -114,7 +115,7 @@ public:
 
 signals:
     void readyRead();
-    void invalidPacket();
+    void protocolError();
 
 public:
     void aboutToClose()
@@ -139,6 +140,17 @@ public:
         }
     }
 
+    void fail()
+    {
+        QObject::disconnect(dev, &QIODevice::readyRead,
+                            this, &QPacketProtocolPrivate::readyToRead);
+        QObject::disconnect(dev, &QIODevice::aboutToClose,
+                            this, &QPacketProtocolPrivate::aboutToClose);
+        QObject::disconnect(dev, &QIODevice::bytesWritten,
+                            this, &QPacketProtocolPrivate::bytesWritten);
+        emit protocolError();
+    }
+
     void readyToRead()
     {
         while (true) {
@@ -149,19 +161,15 @@ public:
                     return;
 
                 // Read size header
-                const qint64 read = dev->read((char *)&inProgressSize, sizeof(qint32));
+                qint32 inProgressSizeLE;
+                const qint64 read = dev->read((char *)&inProgressSizeLE, sizeof(qint32));
                 Q_ASSERT(read == sizeof(qint32));
                 Q_UNUSED(read);
+                inProgressSize = qFromLittleEndian(inProgressSizeLE);
 
                 // Check sizing constraints
                 if (inProgressSize < qint32(sizeof(qint32))) {
-                    QObject::disconnect(dev, &QIODevice::readyRead,
-                                        this, &QPacketProtocolPrivate::readyToRead);
-                    QObject::disconnect(dev, &QIODevice::aboutToClose,
-                                        this, &QPacketProtocolPrivate::aboutToClose);
-                    QObject::disconnect(dev, &QIODevice::bytesWritten,
-                                        this, &QPacketProtocolPrivate::bytesWritten);
-                    emit invalidPacket();
+                    fail();
                     return;
                 }
 
@@ -184,7 +192,7 @@ public:
     }
 
 public:
-    QList<qint64> sendingPackets;
+    QList<qint32> sendingPackets;
     QList<QByteArray> packets;
     QByteArray inProgress;
     qint32 inProgressSize;
@@ -207,18 +215,29 @@ QPacketProtocol::QPacketProtocol(QIODevice *dev, QObject *parent)
  */
 void QPacketProtocol::send(const QByteArray &p)
 {
+    static const qint32 maxSize = std::numeric_limits<qint32>::max() - sizeof(qint32);
+
     if (p.isEmpty())
         return; // We don't send empty packets
 
-    qint64 sendSize = p.size() + sizeof(qint32);
+    if (p.size() > maxSize) {
+        d->fail();
+        return;
+    }
 
+    const qint32 sendSize = p.size() + sizeof(qint32);
     d->sendingPackets.append(sendSize);
-    qint32 sendSize32 = sendSize;
-    qint64 writeBytes = d->dev->write((char *)&sendSize32, sizeof(qint32));
-    Q_ASSERT(writeBytes == sizeof(qint32));
-    writeBytes = d->dev->write(p);
-    Q_ASSERT(writeBytes == p.size());
-    Q_UNUSED(writeBytes); // For building in release mode.
+
+    const qint32 sendSizeLE = qToLittleEndian(sendSize);
+    if (d->dev->write((char *)&sendSizeLE, sizeof(qint32)) != sizeof(qint32)) {
+        d->fail();
+        return;
+    }
+
+    if (d->dev->write(p) != p.size()) {
+        d->fail();
+        return;
+    }
 }
 
 /*!
diff --git a/src/libs/qmldebug/qpacketprotocol.h b/src/libs/qmldebug/qpacketprotocol.h
index 308fd7ba58..6931c05ec1 100644
--- a/src/libs/qmldebug/qpacketprotocol.h
+++ b/src/libs/qmldebug/qpacketprotocol.h
@@ -52,7 +52,7 @@ public:
 
 signals:
     void readyRead();
-    void invalidPacket();
+    void protocolError();
 
 private:
     QPacketProtocolPrivate *d;