PacketIdentifier should be NonZeroU16 #14
Comments
vincentdephily
added a commit
to vincentdephily/mqttrs
that referenced
this issue
Oct 16, 2019
This matches the MQTT spec which mandates non-zero pids. Mqttrs will now error out when trying to encode/decode a PacketIdentifier of 0, like for example mosquitto does. It also reduces the memory usage of `Option<PacketIdentifier>`. Fixes 00imvj00#14 Using PacketIdentifier is now more verbose, but mqttrs user won't be affected as much as mqttrs itself. While we're breaking the API, it's tempting to shorten `PacketIdentifier` to `Pid`, as we've already got `QoS` and will hopefully soone get `QoSPid`. PacketIdentifyer::new() is pretty much a TryInto trait, but using that would have raised the minimum rust version requirement for little convenience.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The spec disallows pid==0: http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718025
I wouldn't be surprised if some servers/clients out there are buggy in that respect, but as one data point, mosquitto client respects this and mosquitto server will close the connection if it receives a pid of 0.
As an optimisation bonus,
Option<NonZeroU16>takes only 2 bytes of memory compared toOption<u16>.This is another API-breaking change, looks like enough to start a 0.2 branch ;)
The text was updated successfully, but these errors were encountered: