Conversation
7d8218b to
bee99c5
Compare
…ient_id` We now provide an additional function, `try_set_clean_session` that returns error if `client_id` is a 0 byte string and `clean_session` is not `true`.
bee99c5 to
7237d55
Compare
swanandx
left a comment
There was a problem hiding this comment.
Thanks for the PR! I have added some review comments, plz have a look once :)
Other than that:
- please do same changes in v5's MqttOptions!
- can we add test to verify we will only panic if client id is empty && clean session is false
Rest LGTM! 🚀 Nice work!
Co-authored-by: Swanand Mulay <73115739+swanandx@users.noreply.github.com>
swanandx
left a comment
There was a problem hiding this comment.
can we please add some tests to verify set_clean_session is working correctly?
thanks :)
Co-authored-by: Swanand Mulay <73115739+swanandx@users.noreply.github.com>
|
Hey @arunanshub , any updates!? |
|
Working on it. Upon referencing Mosquitto lib, it has been difficult to
find where they are validating the "clean_start", "client_id" and save
session options. It seems that a single codebase provides support for v3
and v5. v5 is enabled by flag `CLIENT_RR` (MQTT v5 request response), and
they are using a lot of conditional compilation. Will keep you updated.
…On Thu, 8 Feb, 2024, 10:07 Swanand Mulay, ***@***.***> wrote:
Hey @arunanshub <https://github.com/arunanshub> , any updates!?
—
Reply to this email directly, view it on GitHub
<#791 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALRQYQ24CMX7VUAIJTFH32DYSRJBJAVCNFSM6AAAAABCWEUCCSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZTGM2TCNRRG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
as discussed already, they ( or other clients ) don't validate it! This PR is blocked by adding tests to check if are using assert correctly and other changes suggested in review. Can you please prioritise em so we can close the PR? Then you can continue your research and open follow up PR ( if required ) 😁 wdyt? your call, otherwise if you prefer, lmk when we are good to go for this PR 👍 |
Fixes #667
Type of change
Checklist:
cargo fmtCHANGELOG.mdif it's relevant to the users of the library. If it's not relevant mention why.