search

njh / ruby-mqtt

Pure Ruby gem that implements the MQTT protocol, a lightweight protocol for publish/subscribe messaging.
http://www.rubydoc.info/gems/mqtt
MIT License
541 stars 135 forks source link

Packet#validate_flags change to better fit spec. #91

Closed Aquaj closed 7 years ago

Aquaj commented 7 years ago

As per v3.1 of the MQTT protocol (ref used), most packets don't use the flags (last 4 bytes of the header's first byte).

In the current version of the gem, we check to make sure those all are set to false - however they might not be, since after all the spec doesn't require them to be. (In my case the suback/_puback_s from Orange IoT platform LiveObjects keeps returning [false, true, false, false] as flags). This would raise an exception without any real reason.

This PR aims to solve that issue.

njh commented 7 years ago

Hello,

I would rather refer to version 3.1.1 of MQTT as the reference standard, which clarifies a lot of the edge cases when implementing MQTT: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html

Version 3.1 is being deprecated by some systems and v5.0 of MQTT will be along soon.

Version 3.1.1 clarifies the use of the flags in the Fixed header, particularly for the packet types that don't really use the flags (everything apart from the Publish type) - section 2.2.2: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718022

Section 2.2.2 is very clear clear about what these bits should be set to and what should happen is they are not set correctly:

Where a flag bit is marked as “Reserved” in Table 2.2 - Flag Bits, it is reserved for future use and MUST be set to the value listed in that table [MQTT-2.2.2-1]. If invalid flags are received, the receiver MUST close the Network Connection

I was wondering if there should be a 'relaxed' mode that can be enabled, but MUST in the specification is very strongly worded.

So it sounds like the Orange IoT platform needs to correct it's flags - admittedly this doesn't really help you and it doesn't follow the Jon Postel's robustness principle "Be conservative in what you send, be liberal in what you accept". But it does give something clear for you to point Orange to.

I guess it might be possible to adjust how v3.1 and v3.1.1 behave but at the moment, the MQTT v3.1 and v3.1.1 implementations in the ruby-mqtt codebase are very closely interlinked. I am not sure what changes will need to happen for v5 - which includes a lot of different features and behaviours. It may be that there need to be different classes for different versions of the protocol.

nick.

Aquaj commented 7 years ago

I wasn't aware of the 3.1.1 reference, thanks a lot for the link and for your explanations. It indeed makes a lot of sense to follow the spec to the letter. I forwarded all of that to the Orange developers I am in contact with and they're definitely going to take a look at it.

I might look into how a "relaxed" mode would fit in the codebase if I need to work again with MQTT later on. As you said though: you might have worked it all out by then due to v5 by then.

Everything seems to be in order then, so I'm closing this PR. Thanks again !