Will does not work when using temporary String values when calling setWill()

[garin1000]

Hi hieakitai,
thanks for writing this wrapper to lwmqtt! It really got me to speed up my hobby project!

My LWT did not work as expected, the content of the will that was sent out being random non-printable characters. After some code reading, I noticed that setWill() copies const char * pointers from topic and payload into the respective lwmqtt structures. If topic and payload are temporary values (as was tha case with my payload), the String::c_str() results are no longer valid at the time the connection is set up, leading to the pointers pointing to invalid data.

I solved the issue by adding String fields to the MQTTPubSubClient class, holding the respective strings, as per this diff:

diff --git a/src/MQTTPubSubClient.h b/src/MQTTPubSubClient.h
index 9632ccd..50dce2e 100644
--- a/src/MQTTPubSubClient.h
+++ b/src/MQTTPubSubClient.h
@@ -78,6 +78,8 @@ namespace mqtt {
 
         // required variables
         ClientType* client {nullptr};
+        String willTopic;
+        String willPayload;
         lwmqtt_will_t* will {nullptr};
         global_callback_t global_callback;
         TopicCallbacks callbacks;
@@ -227,12 +229,15 @@ namespace mqtt {
 
             clearWill();
 
+            willTopic=topic;
+            willPayload=payload;
+
             will = (lwmqtt_will_t*)malloc(sizeof(lwmqtt_will_t));
             memset(will, 0, sizeof(lwmqtt_will_t));
-            will->topic = lwmqtt_string(topic.c_str());
+            will->topic = lwmqtt_string(willTopic.c_str());
 
             if (payload.length() > 0)
-                will->payload = lwmqtt_string(payload.c_str());
+                will->payload = lwmqtt_string(willPayload.c_str());
             will->retained = retained;
             will->qos = (lwmqtt_qos_t)qos;
         }

[garin1000]

clearWill() needs to be changed as well. As c_str() returns pointers into the internal structure of the String implementation, these must not be freed when the "will" structure is freed.

 src/MQTTPubSubClient.h | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/MQTTPubSubClient.h b/src/MQTTPubSubClient.h
index 9632ccd..2e42695 100644
--- a/src/MQTTPubSubClient.h
+++ b/src/MQTTPubSubClient.h
@@ -78,6 +78,8 @@ namespace mqtt {
 
         // required variables
         ClientType* client {nullptr};
+        String willTopic;
+        String willPayload;
         lwmqtt_will_t* will {nullptr};
         global_callback_t global_callback;
         TopicCallbacks callbacks;
@@ -227,12 +229,15 @@ namespace mqtt {
 
             clearWill();
 
+            willTopic=topic;
+            willPayload=payload;
+
             will = (lwmqtt_will_t*)malloc(sizeof(lwmqtt_will_t));
             memset(will, 0, sizeof(lwmqtt_will_t));
-            will->topic = lwmqtt_string(topic.c_str());
+            will->topic = lwmqtt_string(willTopic.c_str());
 
             if (payload.length() > 0)
-                will->payload = lwmqtt_string(payload.c_str());
+                will->payload = lwmqtt_string(willPayload.c_str());
             will->retained = retained;
             will->qos = (lwmqtt_qos_t)qos;
         }
@@ -240,10 +245,6 @@ namespace mqtt {
         void clearWill() {
             if (will == nullptr)
                 return;
-            if (will->payload.len > 0)
-                free(will->payload.data);
-            if (will->topic.len > 0)
-                free(will->topic.data);
             free(will);
             will = nullptr;
         }

[hideakitai]

@garin1000 Thank you for the report! I will try to fix it in a few days.

[hideakitai]

@garin1000 Sorry for the late reply. I've fixed this in v0.1.3. Please check it, and if you still have the problem, feel free to reopen it.

[garin1000]

Hi @hideakitai
I finished the project, and setting it all up again would take a lot of time.
I reviewed your changes, though, and besides ordering and naming conventions, this should work the same as my patch above :)
Best,
garin1000