search

mqttjs / MQTT.js

The MQTT client for Node.js and the browser
Other
8.62k stars 1.42k forks source link

Missing implementation for SP (Session present) bit, from the MQTT 3.1.1 spec section 3.2.2.2 ? #235

Closed jpolaroid1978 closed 9 years ago

jpolaroid1978 commented 9 years ago

Hi ,

It seems like the SP (Session present) bit, from the MQTT 3.1.1 spec section 3.2.2.2 (ref link) below is not implemented in the client. This seems to cause CONNACK packets returned from the server return code to be mistakenly processed as rc 256 for a return code of zero.

http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html

mcollina commented 9 years ago

Nice catch! Would you like to help with the implementation of the fix? I think the parsing needs to be fixed into mqtt-packet, plus some handling of it should be done here.

Which broker and version of it are you using?

jpolaroid1978 commented 9 years ago

Hi,

Sure I would be happy to help any way that I can. What would be the optimal way to get a proposed change out to you for review?

We saw the problem while trying the client out yesterday with the IBM IoT Foundation service https://internetofthings.ibmcloud.com/ .

mcollina commented 9 years ago

Send me a pull request on both projects, remember to update unit tests.

jpolaroid1978 commented 9 years ago

Hi,

I sent a pull request for mqtt-packet.

After looking at some other clients, it is not clear to me yet what to do as far as handling other than not failing when the bit is set (which the mqtt-packet fix achieves I think).

Please let me know what you think, thanks!

mcollina commented 9 years ago

That was released as mqtt-packet 3.2.0. You should get it if you reinstall MQTT.js.