From fc8f6e8fd9599de011d8cc85da78aa4c06bc7ff0 Mon Sep 17 00:00:00 2001 From: David Girault Date: Thu, 4 Apr 2019 12:03:12 +0200 Subject: [PATCH] mqtt: fix first packet checking which fail if MQTT_VAR_HEADER_BUFFER_LEN > 1516 If client reception buffer is bigger than the first frame we receive, the first packet test will always fail for the second one if it is shorter the the diffence between reception buffer size and first frame length. For example, if we receive a PUBLISH message with length = 1517 (payload len = 1514 + header len = 3), this result in total message length of 1517. altcp_tls will send MQTT client frame up to 1516 bytes max. This result to PUBLISH message splitted in two frame: first is 1516 bytes, the second of 1 bytes. If MQTT_VAR_HEADER_BUFFER_LEN is 1520 (1516 + 4 bytes for stored fixed header), the second frame of 1 bytes is considered as first publish frame because client->msg_idx (1517) < MQTT_VAR_HEADER_BUFFER_LEN (1520). This result in disconnection AND application callback never called for the end of the payload. The fix will check `(client->msg_idx - (fixed_hdr_len + length)) == 0` which can be only true for the first frame of a message. Below logs showing the bug: ``` April 3rd 2019, 23:14:05.459 lwip_dbg mqtt_parse_incoming: Remaining length after fixed header: 1514 April 3rd 2019, 23:14:05.460 lwip_dbg mqtt_parse_incoming: msg_idx: 1516, cpy_len: 1513, remaining 1 April 3rd 2019, 23:14:05.460 lwip_dbg mqtt_incomming_publish: Received message with QoS 1 at topic: v2/inte... April 3rd 2019, 23:14:05.461 lwip_dbg mqtt_parse_incoming: Remaining length after fixed header: 1514 April 3rd 2019, 23:14:05.461 lwip_dbg mqtt_parse_incoming: msg_idx: 1517, cpy_len: 1, remaining 0 April 3rd 2019, 23:14:05.461 lwip_dbg mqtt_message_received: Received short PUBLISH packet ``` --- src/apps/mqtt/mqtt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/apps/mqtt/mqtt.c b/src/apps/mqtt/mqtt.c index ae3a7577..c09642be 100644 --- a/src/apps/mqtt/mqtt.c +++ b/src/apps/mqtt/mqtt.c @@ -711,8 +711,8 @@ mqtt_message_received(mqtt_client_t *client, u8_t fixed_hdr_len, u16_t length, u u16_t payload_length = length; u8_t qos = MQTT_CTL_PACKET_QOS(client->rx_buffer[0]); - if (client->msg_idx <= MQTT_VAR_HEADER_BUFFER_LEN) { - /* Should have topic and pkt id*/ + if (client->msg_idx == (fixed_hdr_len + length)) { + /* First publish message frame. Should have topic and pkt id*/ size_t var_hdr_payload_bufsize = sizeof(client->rx_buffer) - fixed_hdr_len; u8_t *topic; u16_t after_topic;