[paho-dev] Embedded C++ client: problematic handling of buffer overflows

[Joe Planisky <joe.planisky@xxxxxxxxxx>]

Unless I'm missing something, it seems to me that the way the current Embedded C++ client handles buffer overflows (i.e. receiving a message larger than the configured MAX_MQTT_PACKET_SIZE) can lead to some infinite loops. (By "current", I mean the code in the Embedded-C "develop" branch, as of commit b51dd44 on Apr 16, 2015.)

Client::readPacket(...) reads enough of the header to check the incoming message length.  If it's greater than the configured maximum packet size, it doesn't read any more bytes, but just returns a BUFFER_OVERFLOW return code.  Client::cycle(...) which called readPacket(...) simply passes that return code on to its caller, which is Client::yield(...), which converts the BUFFER_OVERFLOW code into FAILURE.  The message is not ACK'd, so if it had QoS > 0, the broker will send it again.

Since the application doesn't have any insight into why a call to Client::yield(...) failed, the only practical response to such a return code is to completely reset the connection to the broker.  That is, call Client::disconnect(), IPStack::disconnect(), IPStack::connect, Client::connect() with clean session set, and Client::subscribe().  The clean session is needed because without it, the broker will try resending the message if it had QoS > 0, leading to an infinite (or until the broker gives up) loop. 

You also get an infinite loop regardless of QoS if the oversize message was published with the retain bit set, as the broker will try to send the retained message as soon as the client resubscribes.

It seems to me that in the case of an oversize message, Client::readPacket should read as much of the message as will fit into the message buffer (for debugging purposes, i.e. so the developer can tell which particular message is too big.)  It should continue to read and discard the remaining bytes to remove them from the IPStack's receive buffer.  The message should be ACK'ed as required, and delivered to the registered handler with a flag indicating the message is incomplete. The application's message handler function can deal with it however it sees fit, most likely by logging an error and ignoring it.  

Am I missing some simpler way to handle these cases?  

—
Joe