MQTTPacket_readnb fails to decode pingresp

[jpwsutton]

migrated from Bugzilla #464367
status UNCONFIRMED severity normal in component MQTT-Embedded-C for 1.1
Reported in version 1.0 on platform PC
Assigned to: Ian Craggs

Original attachment names and IDs:

• fix.patch (ID 252282)

On 2015-04-10 04:13:47 -0400, Srikanth Kyatham wrote:

Created attachment 252282
If only header is available goto header bits

MQTTPacket_readnb is unable to decode the pingresp.
Since the pingresp is exactly 2 bytes. Its only the header bits and nothing else.

Its failing in here if I am right

line 390

case 2:
/* read the rest of the buffer using a callback to supply the rest of the data */

I have attached a patch hope it helps

Fix is
case 2:
if (trp->rem_len == 0)
goto header_exit;

[scaprile]

Hi, I wrote this function.
This issue is also present in its blocking parent function, since the code is mostly taken from there and split; although it manages to get past this since as there are no more bytes to read, the socket returns 0 and the code advances gracefully. I wrote two samples to test ping, blocking and non-blocking, and the suggested patch, also fixing issue #57 , is the following: fixpong.zip

diff --git a/MQTTPacket/src/MQTTPacket.c b/MQTTPacket/src/MQTTPacket.c
index bd5f90a..4f1f95a 100644
--- a/MQTTPacket/src/MQTTPacket.c
+++ b/MQTTPacket/src/MQTTPacket.c
@@ -306,7 +306,7 @@ int MQTTPacket_read(unsigned char* buf, int buflen, int (*ge
        /* 3. read the rest of the buffer using a callback to supply the rest of
        if((rem_len + len) > buflen)
                goto exit;
-       if ((*getfn)(buf + len, rem_len) != rem_len)
+       if (rem_len && ((*getfn)(buf + len, rem_len) != rem_len))
                goto exit;

        header.byte = buf[0];
@@ -333,7 +333,7 @@ static int MQTTPacket_decodenb(MQTTTransport *trp)
        }
        do {
                int frc;
-               if (++(trp->len) > MAX_NO_OF_REMAINING_LENGTH_BYTES)
+               if (trp->len >= MAX_NO_OF_REMAINING_LENGTH_BYTES)
                        goto exit;
                if ((frc=(*trp->getfn)(trp->sck, &c, 1)) == -1)
                        goto exit;
@@ -341,6 +341,7 @@ static int MQTTPacket_decodenb(MQTTTransport *trp)
                        rc = 0;
                        goto exit;
                }
+               ++(trp->len);
                trp->rem_len += (c & 127) * trp->multiplier;
                trp->multiplier *= 128;
        } while ((c & 128) != 0);
@@ -388,16 +389,17 @@ int MQTTPacket_readnb(unsigned char* buf, int buflen, MQTT
                ++trp->state;
                /*FALLTHROUGH*/
        case 2:
-               /* read the rest of the buffer using a callback to supply the re
-               if ((frc=(*trp->getfn)(trp->sck, buf + trp->len, trp->rem_len))
-                       goto exit;
-               if (frc == 0)
-                       return 0;
-               trp->rem_len -= frc;
-               trp->len += frc;
-               if(trp->rem_len)
-                       return 0;
-
+               if(trp->rem_len){
+                       /* read the rest of the buffer using a callback to suppl
+                       if ((frc=(*trp->getfn)(trp->sck, buf + trp->len, trp->re
+                               goto exit;
+                       if (frc == 0)
+                               return 0;
+                       trp->rem_len -= frc;
+                       trp->len += frc;
+                       if(trp->rem_len)
+                               return 0;
+               }
                header.byte = buf[0];
                rc = header.bits.type;
                break;

Regards

PS: The samples: pingsamples.zip

[icraggs]

I've merged the PR. I assume that this issue is now fixed.