Mqtt 5 puback is different from spec

[tekjar]

3.4.2.2.1 Property Length
The length of the Properties in the PUBACK packet Variable Header encoded as a Variable Byte Integer. If the Remaining Length is less than 4 there is no Property Length and the value of 0 is used.

Spec says that if remaining len is < 4, there's no property length. I think 'there's no property length' implies that it's not part of the byte stream (beacause it checks out with mosquitto)

const mqtt = require('mqtt-packet');

let puback = {
  cmd: 'puback',
  messageId: 42,
}

const opts = { protocolVersion: 5 }; // default is 4. Usually, opts is a connect packet
let buf = mqtt.generate(puback, opts)
console.log(buf.toString('hex').match(/../g).map((x) => `0x${x}`).join(', '))

Output of the above code is

0x40, 0x03, 0x00, 0x2a, 0x00

Output of ack from mosquitto with no reason code and properties is this

[
    0x40,
    0x2,
    0x0,
    0x9,
]

Is the last 0x00 necessary?

[mcollina]

I actually don't know. Would you like to send a PR? Can you check if this works with Mosquitto?

[tekjar]

@mcollina I can try but my java script skills are close to 0

[bkp7]

Looking at the output @tekjar produced from mqtt-packet the last Byte is not the Property Length:

[
  0x40, 0x03, // Fixed Header (PUBACK, Remaining Length - 3 Bytes)
  0x00, 0x2a, // Packet Identifier MSB, LSB: 42
  0x00 // PUBACK Reason Code: Success
]

The question remains as to whether the last Byte is necessary as both Mosquitto and HiveMQ ommit it (tested against their respective online test servers today). You need to infer that the Reason Code is Success per the comment in 3.4.2.1 "The Reason Code and Property Length can be omitted......":

[
  0x40, 0x02, // Fixed Header (PUBACK, Remaining Length - 2 Bytes)
  0x00, 0x2a, // Packet Identifier MSB, LSB: 42
]

Reading the specification I conclude that either is acceptable, as also this would be:

[
  0x40, 0x04, // Fixed Header (PUBACK, Remaining Length - 4 Bytes)
  0x00, 0x2a, // Packet Identifier MSB, LSB: 42
  0x00, // PUBACK Reason Code: Success
  0x00 // Property Length
]

I believe the format currently produced by mqtt-packet is valid, but not optimised.

[bkp7]

@mcollina should this be closed?

[alaendle]

@mcollina @bkp7 - Please reopen this issue - the format is not valid as is (and e.g. not accepted by RabbitMQ).

What mqtt-paket generates is a message length of 3:

And this is not valid to the spec:

The Client or Server sending the PUBACK packet MUST use one of the PUBACK Reason Codes [MQTT-3.4.2-1]. The Reason Code and Property Length can be omitted if the Reason Code is 0x00 (Success) and there are no Properties. In this case the PUBACK has a Remaining Length of 2.

https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901124

And generally talking about the properties length:

The Property Length is encoded as a Variable Byte Integer. The Property Length does not include the bytes used to encode itself, but includes the length of the Properties. If there are no properties, this MUST be indicated by including a Property Length of zero [MQTT-2.2.2-1].

https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901028

[robertsLando]

@alaendle Please open a PR to fix the issue