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

Will publishing is not working as expected when Will Delay Interval is longer than Session Expiry Interval #1401

Closed
wlisac opened this issue Sep 5, 2019 · 8 comments

Comments

@wlisac
Copy link

commented Sep 5, 2019

Overview

The Will Message is not published when the Will Delay Interval is longer than the Session Expiry Interval.

Steps to Reproduce

  • Start the mosquitto broker

    $ docker run -it -p 1883:1883 eclipse-mosquitto
    
  • Add a subscriber to a will-topic topic

    $ mosquitto_sub -h localhost -p 1883 -t 'will-topic' -d
    
  • Add a subscriber with a LWT and Will Delay Interval that is longer than the Session Expiry Interval

    $ mosquitto_sub -h localhost -p 1883 -t 'test' -d --will-payload 'goodbye' --will-topic 'will-topic' -V 5 -D WILL will-delay-interval 30 -D CONNECT session-expiry-interval 5
    
  • Terminate the connection of the subscriber with the LWT using ⌃c to trigger a disconnect

Expected Results

The Will Message should be published when the session expires (after 5 seconds).

Relevant sections from the MQTT 5.0 spec:

The Will Message MUST be published after the Network Connection is subsequently closed and either the Will Delay Interval has elapsed or the Session ends, unless the Will Message has been deleted by the Server on receipt of a DISCONNECT packet with Reason Code 0x00 (Normal disconnection) or a new Network Connection for the ClientID is opened before the Will Delay Interval has elapsed [MQTT-3.1.2-8].

The Server delays publishing the Client’s Will Message until the Will Delay Interval has passed or the Session ends, whichever happens first.

Non-normative comment
The Client can arrange for the Will Message to notify that Session Expiry has occurred by setting the Will Delay Interval to be longer than the Session Expiry Interval and sending DISCONNECT with Reason Code 0x04 (Disconnect with Will Message).

Observed Results

The Will Message is not published when either the Will Delay Interval has elapsed or the session ends when the Will Delay Interval is longer than the Session Expiry Interval.

I've also sometimes seen the broker exit with a status of 139 after the Will Delay Interval (30 seconds in this example).

Additional Notes

The Will Publish works as expected when the Will Delay Interval is shorter than the Session Expiry Interval.

@ralight

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

Thanks for the good description, I'm looking at this now.

ralight added a commit that referenced this issue Sep 5, 2019
Closes #1401. Thanks to Will Lisac.
@ralight ralight added this to the 1.6.5 milestone Sep 5, 2019
@ralight

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

I believe this is fixed in the fixes branch, are you able to confirm it works for you?

@wlisac

This comment has been minimized.

Copy link
Author

commented Sep 6, 2019

@ralight thanks for the quick response. I just tested the fix and it works as expected 👍

@wlisac

This comment has been minimized.

Copy link
Author

commented Sep 6, 2019

@ralight I was running an automated test suite for an MQTT client that I'm writing for SwiftNIO and I noticed a race / crash still in play with LWT, Will Delay, and Session Expiry after the fix.

It would be a little difficult for me to share that test suite since the project is still WIP (although I'm open to it if needed, since it's reproducing the bug consistently), but I was able to quickly put together a simple node.js script that often reproduces the same issue.

I've attached the an archive of the node.js app that can reproduce the race / crash.

$ node test.js

Let me know what you think / if it makes sense and I'm happy to provide more info to help close this out.

Thanks again,
Will

mqtt-js-test.zip

ralight added a commit that referenced this issue Sep 6, 2019
@ralight

This comment has been minimized.

Copy link
Contributor

commented Sep 6, 2019

I think the new commit sorts that out.

@wlisac

This comment has been minimized.

Copy link
Author

commented Sep 7, 2019

@ralight tests pass – thanks 🙌

@ralight

This comment has been minimized.

Copy link
Contributor

commented Sep 8, 2019

Thanks for confirming.

@ralight ralight closed this Sep 8, 2019
ralight added a commit that referenced this issue Sep 18, 2019
ralight added a commit that referenced this issue Sep 18, 2019
@carnil

This comment has been minimized.

Copy link

commented Oct 19, 2019

@ralight: Can you confirm if this is CVE-2019-11778?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.