[Bug] MQTT: states (publish records) in MQTTContext are not well maintained?

[cpp-59487]

HI, I found this issue occurs in the following scenario.

The following is pseudo code.

/// skipped initialization and establishing connections
uint16_t packet_id = 2u;
MQTT_Publish(context, publish_info, packet_id); /// publishes a message with QoS 1
packet_id = 3u;
MQTT_Publish(context, publish_info, packet_id); /// publishes another message with QoS 1

/**
 * After sending out these messages, the outgoing publish records in MQTT context should look like the following.
 * 
 * {{packetId = 2, qos = MQTTQoS1, publishState = MQTTPubAckPending},
 *  {packetId = 3, qos = MQTTQoS1, publishState = MQTTPubAckPending},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull}}
 */

MQTT_ProcessLoop(context, timeout);
/**
 * Now it receives an ACK for packet id 2. Then the states become the following.
 *
 * {{packetId = 0, qos = MQTTQoS1, publishState = MQTTPubAckPending},
 *  {packetId = 3, qos = MQTTQoS1, publishState = MQTTPubAckPending},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull},
 *  {packetId = 0, qos = MQTTQoS0, publishState = MQTTStateNull}}
 */

MQTTStateCursor_t cursor = MQTT_STATE_CURSOR_INITIALIZER;
packet_id = MQTT_PublishToResend(context, &cursor); /// returns 0

In the above case, I would expect packet_id returned from MQTT_PublishToResend() is 3. However, it turns out to be 0 which is MQTT_PACKET_ID_INVALID. I am not sure this is expected or a bug.

I find the cause of this comes from the following line https://github.com/FreeRTOS/coreMQTT/blob/main/source/core_mqtt_state.c#L572 while updating states. This lines deletes the state entry by just marking the packet id invalid without resetting all other fields, which makes MQTT_PublishToResend() confused and return unexpected values.

If this is a bug, I would suggest replace the line https://github.com/FreeRTOS/coreMQTT/blob/main/source/core_mqtt_state.c#L572 with the following.

memset(&(records[ recordIndex ]), 0, sizeof(MQTTPubAckInfo_t));

Thank you!

[archigup]

Thanks for the bug report! I've made a PR for a fix to coreMQTT.

[archigup]

Fixed by https://github.com/FreeRTOS/coreMQTT/pull/187 and https://github.com/FreeRTOS/coreMQTT/pull/188