[BUG] message subscription failing because of shifting topiclen position not taken into account.

[naice]

• Arduino board: NodeMCU 8266

• Arduino IDE version (found in Arduino -> About Arduino menu): VS CODE / 1.8.10

The function Adafruit_MQTT_Subscribe *Adafruit_MQTT::readSubscription(int16_t timeout) does not take into account that the position for the topic lenght can get shifted, so does the entire content of the packet.

quick fix:

`

uint16_t Adafruit_MQTT::readFullPacket(uint8_t *buffer, uint16_t maxsize,
                                    uint16_t timeout, uint8_t *offset) {
// will read a packet and Do The Right Thing with length
uint8_t *pbuff = buffer;
*offset = 0;

uint8_t rlen;

// read the packet type:
rlen = readPacket(pbuff, 1, timeout);
if (rlen != 1)
return 0;

DEBUG_PRINT(F("Packet Type:\t"));
DEBUG_PRINTBUFFER(pbuff, rlen);
pbuff++;

uint32_t value = 0;
uint32_t multiplier = 1;
uint8_t encodedByte;

do {
rlen = readPacket(pbuff, 1, timeout);
if (rlen != 1)
    return 0;
encodedByte = pbuff[0]; // save the last read val
pbuff++;                // get ready for reading the next byte
*offset = *offset + 1;
uint32_t intermediate = encodedByte & 0x7F;
intermediate *= multiplier;
value += intermediate;
multiplier *= 128;
if (multiplier > (128UL * 128UL * 128UL)) {
    DEBUG_PRINT(F("Malformed packet len\n"));
    return 0;
}
} while (encodedByte & 0x80);

DEBUG_PRINT(F("Packet Length:\t"));
DEBUG_PRINTLN(value);

if (value > (maxsize - (pbuff - buffer) - 1)) {
DEBUG_PRINTLN(F("Packet too big for buffer"));
rlen = readPacket(pbuff, (maxsize - (pbuff - buffer) - 1), timeout);
} else {
rlen = readPacket(pbuff, value, timeout);
}
// DEBUG_PRINT(F("Remaining packet:\t")); DEBUG_PRINTBUFFER(pbuff, rlen);

return ((pbuff - buffer) + rlen);
}

`

`

Adafruit_MQTT_Subscribe *Adafruit_MQTT::readSubscription(int16_t timeout) {
uint16_t i, topiclen, datalen;
uint8_t offset;

// Check if data is available to read.
uint16_t len =
    readFullPacket(buffer, MAXBUFFERSIZE, timeout, &offset); // return one full packet
if (!len)
    return NULL; // No data available, just quit.
DEBUG_PRINT("Packet len: ");
DEBUG_PRINTLN(len);
DEBUG_PRINTBUFFER(buffer, len);

if (len < 3)
    return NULL;
if ((buffer[0] & 0xF0) != (MQTT_CTRL_PUBLISH) << 4)
    return NULL;

DEBUG_PRINT(F("OFFSET "));
DEBUG_PRINTLN(offset);

uint8_t offsetTopicLen = 2 + offset;
uint8_t offsetTopic = offsetTopicLen + 1;
// Parse out length of packet.
topiclen = buffer[offsetTopicLen];
DEBUG_PRINT(F("Looking for subscription len "));
DEBUG_PRINTLN(topiclen);

// Find subscription associated with this packet.
for (i = 0; i < MAXSUBSCRIPTIONS; i++) {
    if (subscriptions[i]) {
    // Skip this subscription if its name length isn't the same as the
    // received topic name.
    if (strlen(subscriptions[i]->topic) != topiclen)
    {
        DEBUG_PRINT(F("Skipping subscription "));
        DEBUG_PRINTLN(subscriptions[i]->topic);
        continue;
    }
    // Stop if the subscription topic matches the received topic. Be careful
    // to make comparison case insensitive.
    if (strncasecmp((char *)buffer + offsetTopic, subscriptions[i]->topic, topiclen) ==
        0) {
        DEBUG_PRINT(F("Found sub #"));
        DEBUG_PRINTLN(i);
        break;
    }
    }
}
if (i == MAXSUBSCRIPTIONS)
    return NULL; // matching sub not found ???

uint8_t packet_id_len = 0;
uint16_t packetid = 0;
// Check if it is QoS 1, TODO: we dont support QoS 2
if ((buffer[0] & 0x6) == 0x2) {
    packet_id_len = 2;
    packetid = buffer[topiclen + offsetTopic];
    packetid <<= 8;
    packetid |= buffer[topiclen + offsetTopic + 1];
}

// zero out the old data
memset(subscriptions[i]->lastread, 0, SUBSCRIPTIONDATALEN);

datalen = len - topiclen - packet_id_len - offsetTopic;
if (datalen > SUBSCRIPTIONDATALEN) {
    datalen = SUBSCRIPTIONDATALEN - 1; // cut it off
}
// extract out just the data, into the subscription object itself
memmove(subscriptions[i]->lastread, buffer + offsetTopic + topiclen + packet_id_len,
        datalen);
subscriptions[i]->datalen = datalen;
DEBUG_PRINT(F("Data len: "));
DEBUG_PRINTLN(datalen);
DEBUG_PRINT(F("Data: "));
DEBUG_PRINTLN((char *)subscriptions[i]->lastread);

if ((MQTT_PROTOCOL_LEVEL > 3) && (buffer[0] & 0x6) == 0x2) {
    uint8_t ackpacket[4];

    // Construct and send puback packet.
    uint8_t len = pubackPacket(ackpacket, packetid);
    if (!sendPacket(ackpacket, len))
    DEBUG_PRINT(F("Failed"));
}

// return the valid matching subscription
return subscriptions[i];
}

`

obv. you need to change the method signature and change all occurences accordingly. but that would fix the shifting topic len problem and allow you to receive subscriptions again.

[maseth]

Same as #185

[xdylanm]

this should be resolved with PR #193