Reject topic/payloads that exceed MQTT_MAX_PACKET_SIZE

PubSubClient/src/PubSubClient.cpp

@@ -335,6 +335,10 @@ boolean PubSubClient::publish(const char* topic, const uint8_t* payload, unsigne
 
 boolean PubSubClient::publish(const char* topic, const uint8_t* payload, unsigned int plength, boolean retained) {
     if (connected()) {
+        if (MQTT_MAX_PACKET_SIZE < 5 + 2+strlen(topic) + plength) {
+            // Too long
+            return false;
+        }
         // Leave room in the buffer for header and variable length field
         uint16_t length = 5;
         length = writeString(topic,buffer,length);
@@ -428,9 +432,13 @@ boolean PubSubClient::subscribe(const char* topic) {
 }
 
 boolean PubSubClient::subscribe(const char* topic, uint8_t qos) {
-    if (qos < 0 || qos > 1)
+    if (qos < 0 || qos > 1) {
         return false;
-
+    }
+    if (MQTT_MAX_PACKET_SIZE < 9 + strlen(topic)) {
+        // Too long
+        return false;
+    }
     if (connected()) {
         // Leave room in the buffer for header and variable length field
         uint16_t length = 5;
@@ -448,6 +456,10 @@ boolean PubSubClient::subscribe(const char* topic, uint8_t qos) {
 }
 
 boolean PubSubClient::unsubscribe(const char* topic) {
+    if (MQTT_MAX_PACKET_SIZE < 9 + strlen(topic)) {
+        // Too long
+        return false;
+    }
     if (connected()) {
         uint16_t length = 5;
         nextMsgId++;

tests/src/publish_spec.cpp

@@ -102,6 +102,26 @@ int test_publish_not_connected() {
     END_IT
 }
 
+int test_publish_too_long() {
+    IT("publish fails when topic/payload are too long");
+    ShimClient shimClient;
+    shimClient.setAllowConnect(true);
+
+    byte connack[] = { 0x20, 0x02, 0x00, 0x00 };
+    shimClient.respond(connack,4);
+
+    PubSubClient client(server, 1883, callback, shimClient);
+    int rc = client.connect((char*)"client_test1");
+    IS_TRUE(rc);
+
+    //                                         0        1         2         3         4         5         6         7         8         9         0         1         2
+    rc = client.publish((char*)"topic",(char*)"123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890");
+    IS_FALSE(rc);
+
+    IS_FALSE(shimClient.error());
+
+    END_IT
+}
 
 int test_publish_P() {
     IT("publishes using PROGMEM");
@@ -130,6 +150,8 @@ int test_publish_P() {
 }
 
 
+
+
 int main()
 {
     SUITE("Publish");
@@ -137,6 +159,7 @@ int main()
     test_publish_bytes();
     test_publish_retained();
     test_publish_not_connected();
+    test_publish_too_long();
     test_publish_P();
 
     FINISH

tests/src/receive_spec.cpp

@@ -105,7 +105,7 @@ int test_receive_max_sized_message() {
     int rc = client.connect((char*)"client_test1");
     IS_TRUE(rc);
 
-    byte length = MQTT_MAX_PACKET_SIZE;
+    int length = MQTT_MAX_PACKET_SIZE;
     byte publish[] = {0x30,length-2,0x0,0x5,0x74,0x6f,0x70,0x69,0x63,0x70,0x61,0x79,0x6c,0x6f,0x61,0x64};
     byte bigPublish[length];
     memset(bigPublish,'A',length);
@@ -141,7 +141,7 @@ int test_receive_oversized_message() {
     int rc = client.connect((char*)"client_test1");
     IS_TRUE(rc);
 
-    byte length = MQTT_MAX_PACKET_SIZE+1;
+    int length = MQTT_MAX_PACKET_SIZE+1;
     byte publish[] = {0x30,length-2,0x0,0x5,0x74,0x6f,0x70,0x69,0x63,0x70,0x61,0x79,0x6c,0x6f,0x61,0x64};
     byte bigPublish[length];
     memset(bigPublish,'A',length);
@@ -176,7 +176,7 @@ int test_receive_oversized_stream_message() {
     int rc = client.connect((char*)"client_test1");
     IS_TRUE(rc);
 
-    byte length = MQTT_MAX_PACKET_SIZE+1;
+    int length = MQTT_MAX_PACKET_SIZE+1;
     byte publish[] = {0x30,length-2,0x0,0x5,0x74,0x6f,0x70,0x69,0x63,0x70,0x61,0x79,0x6c,0x6f,0x61,0x64};
 
     byte bigPublish[length];

tests/src/subscribe_spec.cpp

@@ -97,6 +97,33 @@ int test_subscribe_invalid_qos() {
     END_IT
 }
 
+int test_subscribe_too_long() {
+    IT("subscribe fails with too long topic");
+    ShimClient shimClient;
+    shimClient.setAllowConnect(true);
+
+    byte connack[] = { 0x20, 0x02, 0x00, 0x00 };
+    shimClient.respond(connack,4);
+
+    PubSubClient client(server, 1883, callback, shimClient);
+    int rc = client.connect((char*)"client_test1");
+    IS_TRUE(rc);
+
+    // max length should be allowed
+    //                            0        1         2         3         4         5         6         7         8         9         0         1         2
+    rc = client.subscribe((char*)"12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789");
+    IS_TRUE(rc);
+
+    //                            0        1         2         3         4         5         6         7         8         9         0         1         2
+    rc = client.subscribe((char*)"123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890");
+    IS_FALSE(rc);
+
+    IS_FALSE(shimClient.error());
+
+    END_IT
+}
+
+
 int test_unsubscribe() {
     IT("unsubscribes");
     ShimClient shimClient;
@@ -143,6 +170,7 @@ int main()
     test_subscribe_qos_1();
     test_subscribe_not_connected();
     test_subscribe_invalid_qos();
+    test_subscribe_too_long();
     test_unsubscribe();
     test_unsubscribe_not_connected();
     FINISH