Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QoS::AtMostOnce not handled properly when encoding/decoding a publish packet #6

Closed
vincentdephily opened this issue Jul 23, 2019 · 5 comments

Comments

@vincentdephily
Copy link
Collaborator

Mqttrs always expects a PacketIdentifier to be present, but the spec explains that the field is only present in PUBLISH Packets where the QoS level is 1 or 2.

The symptoms are missing two initial bytes of payload when decoding an AtMostOnce publish packet, a panic when trying to encode a publish packet with pid=None, corrupted payload when read by conforming clients, etc.

I'll try to send a PR.

@00imvj00
Copy link
Owner

awesome, thank you.

@vincentdephily
Copy link
Collaborator Author

Thanks for the fix, sorry for never following up with a PR: priorities shifted and holidays happened. I'll get back to QoS soon and hopefully stress-test this.

@00imvj00
Copy link
Owner

It is ok. :) and thank you for finding issues.

@vincentdephily
Copy link
Collaborator Author

Sorry, not sure how I missed that, but only from_buffer() was fixed, leaving to_buffer() with the same bug. It should be possible to encode a Publish packet without a PacketIdentifier, if the QoS is 0.

@vincentdephily
Copy link
Collaborator Author

I've got the fix, just working on a more comprehensive test.

vincentdephily added a commit to vincentdephily/mqttrs that referenced this issue Oct 15, 2019
This is the second half of the fix for bug 00imvj00#6. The previous commit introduces the corresponding
unittest.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants