MQTT subscribe Remaining Length encoding is incorrect
See this discussion for background, also MQTT spec V3.1.1 section 2.2.3.
This may be academic as its main impact is if the topic field is long. This is to record the existence of the issue.
The current encoding stores Remaining Length in a byte. It should be stored as a Variable Byte Integer as per .publish. This will cause a failure if Remaining Length is greater than 127, therefore imposing a limit on the length of the topic field.
I can submit a PR if the maintainers think this is worth fixing.
umqtt.simple: refactor packet de/encoding and fix remaining length encoding (fixes #284)
As stated in #284, the MQTTClient.subscribe method doesn't handle topic lengths >= 122 correctly, since it only allocates one byte for the remaining length portion of the SUBSCRIPTION packet. I've refactored the variable length encoding of the remaining length bytes to use the new _varlen_encode method consistently and also got rid of the usage of the ustruct module.
- Add
_varlen_encodemethod to write variable length encoded remaining length bytes to packet buffer and use it consistently. - Use
int.from_bytes/to_bytesto de/encode short integers and replaceustructusage with it. - Correctly separate bytes of fixed and variable header in
connectmethod.
In a separate commit I've also removed all (commented ou) debug printing calls in simple.py and added a debug wrapper for MQTTClient (umqtt_debug.py) to the examples, which can optionally be used with examples, and I've also expanded example_pub.py and example_sub.py so that they can be parametrized form the command line. If desired, I can separate this commit into an extra PR.
This PR is based on the branch for PR #302. If the latter is merged, I will rebase this on master.
@pfalcon this looks like an important bugfix - any chance you could review this?
@Kriechi It seems that only the fork (or original, depending on how you look at it) of
micropython-libat https://github.com/pfalcon/micropython-lib is maintained and developed any further, ATM.I could re-submit my PR there, but I have basically given up on MicroPython, and I probably won't put any more effort into it. Feel free to take this PR (and any others I have made in this repo) and re-submit it (them) to pfalcon's repo.
Thank you @SpotlightKid! Without your refactor bug fix I wasn't able to subscribe to my AWS topic, although it was only 63 bytes in length. Now it is working!
Closing this since I'm cleaning my unmerged PRs and this hasn't been acted upon in years.