- Every MQTT packet begins with a fixed header, whose second field is the
Remaining Length: the number of bytes that follow. It is not a byte. It
is spread over one to four bytes, seven value bits each, with the top
bit meaning "another byte follows". So values up to 127 take one byte,
and larger values take more.
Six packet types read it as a single quint8: SUBACK, UNSUBACK, PUBACK,
PUBREC, PUBREL and PUBCOMP. For most of them that used to be correct.
In MQTT 3.1.1 those packets contain nothing but a two byte identifier,
so the length was always exactly 2, which always fits in one byte. The
code did not even use the byte it read: it checked that the value was 2
and then hardcoded the length.
MQTT 5.0 support (ac66658) gave those packets an optional reason code
and properties, so the length can now be far above 127. That commit
switched off the "must be 2" check for MQTT 5.0 and began using the
byte as the real length, but kept reading only one byte. The check had
been the only thing making a one byte read safe, because a multi-byte
length always has its top bit set and so could never equal 2.
SUBACK never had a fixed length at all, since its payload carries one
reason code per subscribed topic filter. It has been misparsed for any
SUBACK above 127 bytes since the parser was written (d96011b).
Reading one byte of a two byte length consumes one byte too few, so the
remainder of the packet is then read as the next packet's header. The
client rejects a valid acknowledgement and stays out of step for the
rest of the connection. Brokers trigger this on ordinary traffic as
soon as they attach a reason string or user properties.
All six now use readVariableByteInteger(), and each rejects a length too
short to hold the fields the specification requires.
PUBREL also accepted a length of exactly 2 even for MQTT 5.0, which
rejected valid PUBREL packets carrying a reason code. It now accepts 2
or more there, and still requires exactly 2 for MQTT 3.1.1.
- Adds three regression tests driving the parser through the existing
FakeServer, so they need no broker.
Pick-to: 6.11 6.12 6.8
Task-number: QTBUG-149569
Change-Id: Ib239d836556e0cc14e13c116dee757fdd1a7ae81
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
06d301aa50