Null terminate zero length payload

[tjlusco]

This may sound like an odd request, but it solves an issue I guarantee anyone who has used this library has encountered.

I'd like to propose that if you receive a packet with no payload, instead of setting the payload to NULL, instead it allocates a single byte. This follows the logic that the memory allocated for a payload is payload length + 1. All that needs to be done is remove the special handling of the payload if the length is zero.

https://github.com/eclipse/mosquitto/blob/15292b20b0894ec7c5c3d47e4b22ee9d89f91132/lib/handle_publish.c#L102
https://github.com/eclipse/mosquitto/blob/15292b20b0894ec7c5c3d47e4b22ee9d89f91132/lib/messages_mosq.c#L73

Now, if you have code that blindly uses a payload as a string without checking the length, you won't get a segmentation fault.
This makes code safer by preventing denial of service of clients that do not handle zero length payloads. For example:

https://github.com/eclipse/mosquitto/blob/15292b20b0894ec7c5c3d47e4b22ee9d89f91132/examples/subscribe/basic-1.c#L69

One possible argument against the change would be if existing code is checking for the existence of payload by checking if msg->payload == NULL, but returning a payload of zero length would be safe with any combination of memcpy / strcpy, or any of the functions from string.h. It would only cause issues if the client blindly assumes a payload length without actually checking it first, which is already unsafe code.

I would also suggest documenting that payloads are guaranteed to be null terminated so that you can safely use the payload as a string. This behaviour is useful but not documented anywhere. By null terminating zero length payloads, the previous statement would also become true.

Thanks for reading. 🙂