eclipse/paho.mqtt.c

Client not honoring server keep alive in CONNACK

fpagliughi opened this issue · 5 comments

There’s an issue reported to the Paho C++ library that the client is not honoring the server override of the requested keepalive time.

eclipse/paho.mqtt.cpp#423

In the reported case, the server demands a longer keep alive interval than the client requested. (Client asked for 20sec, server demanded 120sec; client sent at 20 sec anyway).

Asking for a longer time is not assumed to be the normal case by the v5 spec. I have not verified this in practice yet, but am investigating.

In the MQTT 5.0 standard, the concept of Server Keep Alive was also introduced, allowing the broker to choose to accept the Keep Alive value carried in the client request, or to override it, depending on its implementation and other factors. If the broker chooses to override this value, it needs to set the new value in the Server Keep Alive field of the Connection Acknowledgement Packet (CONNACK), and the client needs to use this value to override its own previous Keep Alive value when it reads it in the CONNACK.

https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901094

I don't think we do update the keepalive with the server keep alive at all. There is non-normative text which says:

The primary use of the Server Keep Alive is for the Server to inform the Client that it will disconnect the Client for inactivity sooner than the Keep Alive specified by the Client.

but nothing in normative text, so it seems we should update the keepalive value regardless of whether it is smaller or greater than the value sent in the connect packet.

@icraggs Agreed. It should be updated in either case.

I just pointed out the non-normative text of the spec in case an assumption had been made in the library.

Not sure where I was looking before, but:

If the Server sends a Server Keep Alive on the CONNACK packet, the Client MUST use this value instead of the Keep Alive value the Client sent on CONNECT [MQTT-3.2.2-21]

So, given the above conformance clause I've set the client to use the server keep alive regardless of whether it's higher or lower than the original requested.