• Registered: 2021-07-01
  • Posts: 11

Topic: Publishing: PUBREC fails outright instead of reporting reason code

This is a rather special case, but I believe it is allowed in the MQTT standards.

During our testing of the Wolf Library, we published to a topic which did not have any subscribers (that is, the subscribers had not yet been set up).
The Broker responded with a shorten PUBREC packet, with "reason code" (16) of "No matching Subscribers".

However, the Library did not support the shortened packet, discarded reason code, and returned "result code" (-2) of  MQTT_CODE_ERROR_OUT_OF_BUFFER.
The library actually tried to process "props" that were not in the shortened packet.

The MQTT-v5.0 standard mentions (in section 3.5.2.2.1) that "If the Remaining Length is less than 4 there is no Property Length and the value of 0 is used."

The reason code is discarded as the Library uses a NULL packet object for this part of the processing during MQTT_MSG_WAIT.
For reference, see line 820 of mqtt_client.c, where
   rc = MqttClient_DecodePacket(client, client->rx_buf,
          client->packet.buf_len, NULL, &packet_type, NULL, &packet_id);

Consequently, the broker's valid response is rejected, without telling the application the "reason code".
Can you please advise how to handle this situation.

Details for recreating issue:
  Broker was "broker.hivemq.com"
  Publish to any non-subscribed topic.
  PUBREC data received was 50 03 00 01 10,
        where 50=PUBREC, 03=Remaining Length, 0001=packet ID, 10=reason code

Many thanks
Kelvin

Share

  • Registered: 2015-10-07
  • Posts: 430

Re: Publishing: PUBREC fails outright instead of reporting reason code

Hi Kelvin,

Thanks for this report. We are trying to reproduce and should get back shortly with a fix.

David Garske, wolfSSL

Share

  • From: Mobile, AL
  • Registered: 2013-12-11
  • Posts: 351

Re: Publishing: PUBREC fails outright instead of reporting reason code

Hello Kelvin,

I was able to reproduce this issue with a local instance of the mosquitto broker, also. I'll post a fix as soon as it is available.

Thanks,
Eric @ wolfSSL Support

embhorn's Website

Share

  • Registered: 2021-07-01
  • Posts: 11

Re: Publishing: PUBREC fails outright instead of reporting reason code

Many thanks for looking into this issue.

I believe that there is a simple fix,
but I must refrain from providing suggestions as I am not familiar with the history and implications of the code.

With MQTT v5.0 specification trying to provide backward compatibility with earlier versions,
you might check for other instances, whilst you are on the case, of similar loopholes that need plugging (or hopefully not).
Just a suggestion, but I am always trying to get more for the price of one !!

Once again, many thanks.

Kind regards,
Kelvin

Share

  • From: Mobile, AL
  • Registered: 2013-12-11
  • Posts: 351

Re: Publishing: PUBREC fails outright instead of reporting reason code

Thanks for your suggestions. I've created a fix for the puback issue, as well as other packet types that make use of reason codes. https://github.com/wolfSSL/wolfMQTT/pull/220

Looking forward to your feedback!

Thanks,
Eric @ wolfSSL Support

embhorn's Website

Share

  • Registered: 2021-07-01
  • Posts: 11

Re: Publishing: PUBREC fails outright instead of reporting reason code

I have tried your code (wolfMQTT-1541ea071de48d6cf3b4664d814cb0883d041850).
The issue of reading non-existing props is now working, but there are still some other issues that need attention.

Summary:
1. The PUBREC reason code is not reported to the application
2. The PUBREL contained an invalid reason code.
    (cause: the PUBREC reason code was sent back in the PUBREL packet, instead of one of the allowed codes).
3. A compiler warning on line 511 of mqtt_packet.c

For information:
1. The PUBREC reason code is lost as the code uses a temporary PublishResp,
   instead of referencing the session's MqttPublish.resp or the MqttClient.msg.publish_resp, both of which could have reported the reason code back to the application.
   The final stage PUBCOMP would report a reason code of 0 (success), if the PUBREL did not fail, so this is probably acceptable if it had completed successfully.
   
2. The MQTT-v5.0 standard mentions, in Table 3.6, that only values of 0 and 146 can be used as reasons codes for PUBREL,
   although there might be some confusion in the specification as section 4.3.3 which allows values under 0x80 (128).
   However the HiveMq broker rejected the PUBREC reason code of 16, so I guess that it should not have been used in the PUBREL.

3. The compiler warning is illustrated in the attached picture.
   I am not overly concerned, as long as error conditions are reliably captured, eg for values like MQTT_PROP_TYPE_MAX.
   Also, it is always nice to have a absolutely clean build.

Details for recreating issue:
  Broker was "broker.hivemq.com"
  Publish to any non-subscribed topic.
  PUBREC data received was 50 03 00 01 10,
        where 50=PUBREC, 03=Remaining Length, 0001=packet ID, 10=reason code
  PUBREL data sent was 62 03 00 01 10,
        where 62=PUBREL, 03=Remaining Length, 0001=packet ID, 10=reason code
  DISCONNECT data received was decoded as
       "property (31) reason: PUBREL with invalid reason code was sent. This is a protocol violation."

Many thanks
Kelvin

Post's attachments

mqtt_packet warning for wolfMQTT-1541ea071de48d6cf3b4664d814cb0883d041850 version.png
mqtt_packet warning for wolfMQTT-1541ea071de48d6cf3b4664d814cb0883d041850 version.png 80.11 kb, file has never been downloaded. 

You don't have the permssions to download the attachments of this post.

Share

  • From: Mobile, AL
  • Registered: 2013-12-11
  • Posts: 351

Re: Publishing: PUBREC fails outright instead of reporting reason code

Hi Kelvin,

Thanks for this additional report. I had to switch over to the HiveMQ broker to reproduce this issue because the Mosquitto broker apparently does not send the reason code with QoS 2 PUBREC response (but it does send it with PUBACK).

I've posted a patch here:
https://github.com/wolfSSL/wolfMQTT/pull/224

It does not fully address your concern about reporting the PUBREC reason code, but I have added that as a feature request.

Thanks,
Eric

embhorn's Website

Share

  • Registered: 2021-07-01
  • Posts: 11

Re: Publishing: PUBREC fails outright instead of reporting reason code

I am not sure about my earlier reply, it seems to have gone missing.
If it re-appears, then this is a briefer replication of that post.

I have tested the code (wolfMQTT-83d3725d365c33981bbc7fcb794b9bd3bacad4d7),
and it is working.
The new code:
1. continues to process the packet when no props are included in the PUBREC packet
2. responds with a valid reason code (actually with no reason code) in the PUBREL packet
3. has removed the condition that raised a compiler warning, so clean build is achieved

Discussion:
It is proposed that the reporting of the "reason code" to the application layer would be handled as a "feature request".
This is an acceptable suggestion.
The "reason code" just provides additional information for the higher layers to make informed decisions, such as Re-authenticate etc. It also provides extra information when errors occur outside of the MQTT protocol, such as No subscribers to your published topic. It could be useful, but it is not a show stopper.

So, many thanks for resolving the fundamental issues.
Kelvin

Share