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>
This commit is contained in:
Ulf Hermann
2018-06-06 15:15:27 +02:00
parent 36d5d3acc1
commit 0cda3a8e5a
2 changed files with 39 additions and 20 deletions

View File

@@ -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;
}
}
/*!

View File

@@ -52,7 +52,7 @@ public:
signals:
void readyRead();
void invalidPacket();
void protocolError();
private:
QPacketProtocolPrivate *d;