Missing validation on 'connect' flags causes strange issues

[aw]

Hi,

I noticed there is missing some important validations on the CONNECT command, particularly with regards to Will and QoS.

According to the v5 spec, section 3.1.2.6:

If the Will Flag is set to 0, then the Will QoS MUST be set to 0 (0x00)
If the Will Flag is set to 1, the value of Will QoS can be 0 (0x00), 1 (0x01), or 2 (0x02). A
value of 3 (0x03) is a Malformed Packet.

However these conditions are not handled, so I am free to set:

var object = {
  cmd: 'connect',
  protocolId: 'MQTT',
  protocolVersion: 5,
  username: 'matteo',
  password: new Buffer('collina'),
  retain: false,
  clean: true,
  will: {
    topic: 'mydevice/status',
    qos: 3
  }
}

And this is accepted just fine, the Flags byte becomes 0xDE which is 11011110, notice byte[3-4] (QoS) = 3... but it gets worse:

var object = {
  cmd: 'connect',
  protocolId: 'MQTT',
  protocolVersion: 5,
  username: 'matteo',
  password: new Buffer('collina'),
  retain: false,
  clean: true,
  will: {
    topic: 'mydevice/status',
    qos: 4
  }
}

Settings qos: 4 actually overwrites the 5th bit in the CONNECT flags. When you inspect the Buffer, you'll see it sets the Flags byte to 0xE6 which is 11100110, notice byte[5] (Retain Flag) = 1, even though we specifically defined retain: false.

[mcollina]

Would you like to send a Pull Request to address this issue? Remember to add unit tests.

[aw]

The short answer is no, but thanks for the offer.