The QMqtt namespace was missing from the TOC as the .qhp subproject
selected only classes and headers.
Pick-to: 6.12
Change-Id: I5692ae2e77b80d9b3c4361945891e84c1ef4ccf7
Reviewed-by: Alexei Cazacov <alexei.cazacov@qt.io>
- The readBufferTyped<T>() method returns T{0} when it fails. But
in QtMqtt the value 0 is also used as "Success" return code.
In particular, src/mqtt/qmqttconnection.cpp
=> readBufferTyped<T>() returns 0 on failure, and finalize_connack()
reads the login result with it. src/mqtt/qmqttglobal.h =>
QMqtt::ReasonCode defines Success = 0.
So our failure value and the protocol's success value are the same
number.
- To fix this problem I make readBufferTyped<>() return std::nullopt
when the read fails, while using the returned value to represent
uccessfully read data. This allows callers to distinguish read errors
from valid zero values.
[ChangeLog][QMqttConnection][Important Behavior Changes] The Qt MQTT
client now stops parsing and closes the connection with the
ProtocolViolation error when a read error occurs while
parsing a packet.
Task-number: QTBUG-149867
Change-Id: I829a34cecfcb49ba330d5eb65aba755eee9326b0
Reviewed-by: Maurice Kalinowski <maurice.kalinowski@qt.io>
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
- Enforce the configured packet size limit locally for MQTT 3.1/3.1.1.
- Close the connection if the broker sends a packet exceeding the limit.
MQTT 3.1 and 3.1.1 have no way to tell the broker the maximum packet
size the client accepts, so the limit can only be applied on the
client side. This is helpful for clients on embedded devices, where
memory is limited and a very large packet from the broker could
exhaust it.
[ChangeLog][QMqttClient] The maximumPacketSize connection property is
now also enforced for MQTT 3.1 and 3.1.1 connections. Previously it was
only honored for MQTT 5.0. The client now closes the connection if the
broker sends a packet larger than this limit. This helps clients on
embedded devices to protect their limited memory. The default behavior
is unchanged: if no limit is set, only the protocol maximum of 256 MB
applies.
Change-Id: I26625325f490e2396fa493005c1f322253007a33
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
- this function should be used to protect your socket against receiving
too much data, which may eventually cause your application to run out
of memory.
- setSocketReadBufferSize(size) sets the size of internal read buffer
to be 'size' bytes.
[ChangeLog][QMqttClient] Added a readBufferSize property to limit
the transport buffer size.
Task-number: QTBUG-149866
Change-Id: I6ff843b66b50d019ecf620c25c47e0d779518113
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
- Document that disconnectFromHost() may block while waiting for pending
data to be written, with a timeout of up to 30 seconds.
- Recommend keeping QMqttClient and its transport outside the UI thread
because blocking I/O may occur and affect UI responsiveness.
Pick-to: 6.12
Task-number: QTBUG-150345
Change-Id: I27d300210750710c4a5138f8c3f97c7c929466af
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
Disconnect with reason code 0x95 (Packet too large) if a packet
exceeds the Maximum Packet Size limit, as required by section 3.1.2.11.4
of the MQTT 5 spec.
Note: This fix doesn't protect against memory exhaustion in user apps.
It just implements the spec requirement about message limit.
Pick-to: 6.12
Task-number: QTBUG-149866
Change-Id: I2baa8ceb95e4a045437e2caa51e5508b2fa8b2eb
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
- When QMqttConnection reads a message, it keeps a count of the bytes
left to read -> by the dataSize argument. If a read fails, the count
is never reduced.
- The fix cleaning up the count of the bytes left to read, since
the reading is not working anymore and parsing should be stopped.
It allows to get read of potential freezes.
The patch also updates the QByteArray specialization to return a
default-constructed QBA in case of an error. This is more in line
with the default implementation.
Pick-to: 6.11 6.12 6.8
Task-number: QTBUG-149563
Change-Id: I8c8c5ef354103a996ac6e360fbc5c8f61d965f3d
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
- 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>
- finalize_pingresp() lowers m_pingTimeout every time, with no lower
limit. A broker can send a pile of unasked-for replies to push the
counter deep below zero, and then the client can spend some unexpected
amount of time to reach back the number that means:
"the broker has stopped answering, drop the link.”
- The fix prohibits pushing the counter below zero.
Task-number: QTBUG-149662
Pick-to: 6.11 6.12 6.8
Change-Id: I7b25e8ac3f83fa262a19010df9a91b2ee1c006fd
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
- finalize_suback() and finalize_unsuback() read the 2-byte request ID.
If no matching request was found, they returned immediately.
However, m_missingData remained non-zero.
The leftover bytes are then incorrectly interpreted as the header of
the next message!
- This change fixes the issue universally for all handlers in the
processDataHelper().
Pick-to: 6.11 6.12 6.8
Task-number: QTBUG-149570
Change-Id: I06c86414c3297caa386059a02fdec5c3765a0315
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
- In the f8ecf4d56a we found that the
m_receiveAliases bounds check used a wrong property:
m_clientPrivate->m_serverConnectionProperties.maximumTopicAlias(),
but it must use instead:
m_clientPrivate->m_connectionProperties.maximumTopicAlias().
So, the idea is m_receiveAliases is sized according to the maximum
topic alias value declared by the client in CONNECT, while
m_publishAliases is sized according to the maximum value declared
by the server in CONNACK.
The previous fix did not update all related code.
1) The resize() calls for m_receiveAliases and m_publishAliases
still used the wrong properties.
2) The bounds check for m_publishAliases still used the client's
maximumTopicAlias() value instead of the server's.
- Update the resize() calls to use the appropriate connection
properties and fix the bounds check for m_publishAliases.
Amends f8ecf4d56a
Pick-to: 6.11 6.12 6.8
Task-number: QTBUG-149564
Change-Id: I252c67966bf28c6994f34b19024d1b7894748473
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
- Parsing continued after closeConnection(), leaving the client in the
"connected" state with a dead socket. This led to a permanent,
unrecoverable wedge in Release builds and a crash in Debug builds.
- Check whether the client is already disconnected before changing its
state to BrokerConnected.
Pick-to: 6.11 6.12 6.8
Task-number: QTBUG-149565
Change-Id: I13d45c24d24fe862d9aec6a7e421f5ef4d494db0
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
- On error, readVariableByteInteger() closes the connection and
returns-1. In this case, message handling should stop.
- Add checks for the return value to stop processing when -1 is returned.
Pick-to: 6.11 6.8 6.12
Task-number: QTBUG-149565
Change-Id: I8f6cc8d6f3011a89d3bd4033a3432f383e1e9164
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
- The length reader checked its 4-byte limit after doing the maths.
So, a 5th byte was multiplied in and overflows before the value
was thrown away. But it was not noticed, because we thrown it away
and returned -1.
- The fix moves iteration++ and if (iteration > 4) before the
calculation, preventing the overflow.
Pick-to: 6.11 6.8 6.12
Task-number: QTBUG-149571
Change-Id: I78c2063c7e01af3e8f196f824216d31de92fab48
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
- The main differences between v2 and v3 are:
* `coin_module_test_template_v2.yaml` ignores autotest return
codes because the `COIN_CTEST_IGNORE_EXIT_CODE` flag is set.
* The Axivion build was moved from coin_module_build_template_v2.yaml
to coin_module_test_template_v3.yaml in commit
c3ddec2f15a1cf806d8df444952cb2c2231ae4ad
- Switch the qtmqtt submodule to the newer v3 version to improve
the submodule CI experience for autotests and restore Axivion
checks.
Pick-to: 6.12 6.11
Change-Id: I1695c8ca1af616512dff437a0a4cdbd0491412d4
Reviewed-by: Toni Saario <toni.saario@qt.io>
- use std::as_const() where possible, and introduce an intermediate
local const variable where it is not.
Axivion-Id: qt_Add-ons_qtmqtt_dev_linux:SV34
Axivion-Id: qt_Add-ons_qtmqtt_dev_linux:SV35
Axivion-Id: qt_Add-ons_qtmqtt_dev_linux:SV36
Axivion-Id: qt_Add-ons_qtmqtt_dev_linux:SV37
Pick-to: 6.12
Task-number: QTBUG-144762
Change-Id: I7b6d53116269007aca7d6f050b2abeb2515e9390
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
