Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [paho-dev] Embedded C++ client: problematic handling of buffer overflows.

Joe,

there are two reasons why this situation isn't handled cleanly, or at all in the current embedded code.

1) MQTT does not have a good way of handling this situation at the moment. I'm hoping we might get to address this in the MQTT standard, but that's not going to be very quick. The only option strictly open to the application is to break the connection. But that would result in a QoS 1 or 2 message being resent.

2) In trying to keep the client code as small as possible, the situation is not handled at the moment, except to "never send a message bigger than expected to the embedded application". If it were added, I would probably want any code to handle this situation to be conditionally compiled, so you can omit it if desired.

Consider that the maximum message size that could be sent by MQTT is 256MB. It would take quite a long time to receive that in 50 byte chunks!

My view is, that at the moment, the situation is best handled administratively, at the server, or in the application design.

Ian

On 04/20/2015 04:44 PM, Joe Planisky wrote:
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

_______________________________________________
paho-dev mailing list
paho-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/paho-dev

--
Ian Craggs
icraggs@xxxxxxxxxx                 IBM United Kingdom
Paho Project Lead; Committer on Mosquitto



Back to the top