From 113603168bb644715395f86b78e0803f1e6b67a0 Mon Sep 17 00:00:00 2001 From: Roger Light Date: Sun, 13 Dec 2020 23:11:02 +0000 Subject: [PATCH] Fix LWT not being sent on client takeover. This was not happening for the case when the existing session wasn't being continued. Closes #1946. Thanks to Rory Piper. --- ChangeLog.txt | 2 ++ src/handle_connect.c | 8 +++++ test/broker/07-will-takeover.py | 53 ++++++++++++++++++++++++--------- 3 files changed, 49 insertions(+), 14 deletions(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index 3255c430..bfdf3d9e 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,6 +1,8 @@ Broker: - Fix excessive CPU use on non-Linux systems when the open file limit is set high. Closes #1947. +- Fix LWT not being sent on client takeover when the existing session wasn't + being continued. Closes #1946. Apps: - Fix `mosquitto_passwd -b` using username as password (not if `-c` is also diff --git a/src/handle_connect.c b/src/handle_connect.c index 7be9833a..5a33fdde 100644 --- a/src/handle_connect.c +++ b/src/handle_connect.c @@ -165,6 +165,14 @@ int connect__on_authorised(struct mosquitto *context, void *auth_data_out, uint1 if(context->clean_start == true){ sub__clean_session(found_context); } + if((found_context->protocol == mosq_p_mqtt5 && found_context->session_expiry_interval == 0) + || (found_context->protocol != mosq_p_mqtt5 && found_context->clean_start == true) + || (context->clean_start == true) + ){ + + context__send_will(found_context); + } + session_expiry__remove(found_context); will_delay__remove(found_context); will__clear(found_context); diff --git a/test/broker/07-will-takeover.py b/test/broker/07-will-takeover.py index 8e04b423..1024a46a 100755 --- a/test/broker/07-will-takeover.py +++ b/test/broker/07-will-takeover.py @@ -5,7 +5,7 @@ from mosq_test_helper import * -def do_test(proto_ver, clean_session): +def do_test(proto_ver, clean_session1, clean_session2): rc = 1 keepalive = 60 @@ -13,17 +13,34 @@ def do_test(proto_ver, clean_session): connect1_packet = mosq_test.gen_connect("will-helper", keepalive=keepalive, proto_ver=proto_ver) connack1_packet = mosq_test.gen_connack(rc=0, proto_ver=proto_ver) - connect2_packet = mosq_test.gen_connect("will-test", keepalive=keepalive, proto_ver=proto_ver, will_topic="will/test", will_payload=b"LWT", clean_session=clean_session) - connack2a_packet = mosq_test.gen_connack(rc=0, proto_ver=proto_ver) - if clean_session == False and proto_ver == 4: - connack2b_packet = mosq_test.gen_connack(rc=0, flags=1, proto_ver=proto_ver) + if proto_ver == 5: + if clean_session1 == False: + connect_props1 = mqtt5_props.gen_uint32_prop(mqtt5_props.PROP_SESSION_EXPIRY_INTERVAL, 60) + else: + connect_props1 = mqtt5_props.gen_uint32_prop(mqtt5_props.PROP_SESSION_EXPIRY_INTERVAL, 0) + + if clean_session2 == False: + connect_props2 = mqtt5_props.gen_uint32_prop(mqtt5_props.PROP_SESSION_EXPIRY_INTERVAL, 60) + else: + connect_props2 = mqtt5_props.gen_uint32_prop(mqtt5_props.PROP_SESSION_EXPIRY_INTERVAL, 0) else: - connack2b_packet = mosq_test.gen_connack(rc=0, proto_ver=proto_ver) + connect_props1 = b"" + connect_props2 = b"" + + connect2_packet = mosq_test.gen_connect("will-test", keepalive=keepalive, proto_ver=proto_ver, will_topic="will/test", will_payload=b"LWT", clean_session=clean_session1, properties=connect_props1) + connack2_packet = mosq_test.gen_connack(rc=0, proto_ver=proto_ver) + + connect3_packet = mosq_test.gen_connect("will-test", keepalive=keepalive, proto_ver=proto_ver, clean_session=clean_session2, properties=connect_props2) + if clean_session1 == False and clean_session2 == False: + connack3_packet = mosq_test.gen_connack(rc=0, flags=1, proto_ver=proto_ver) + else: + connack3_packet = mosq_test.gen_connack(rc=0, proto_ver=proto_ver) subscribe_packet = mosq_test.gen_subscribe(mid, "will/test", 0, proto_ver=proto_ver) suback_packet = mosq_test.gen_suback(mid, 0, proto_ver=proto_ver) publish_packet = mosq_test.gen_publish(topic="will/test", qos=0, payload="Client ready", proto_ver=proto_ver) + publish_lwt_packet = mosq_test.gen_publish(topic="will/test", qos=0, payload="LWT", proto_ver=proto_ver) port = mosq_test.get_port() broker = mosq_test.start_broker(filename=os.path.basename(__file__), port=port) @@ -34,17 +51,21 @@ def do_test(proto_ver, clean_session): mosq_test.do_send_receive(sock1, subscribe_packet, suback_packet, "suback") # Connect client with will - sock2 = mosq_test.do_client_connect(connect2_packet, connack2a_packet, timeout=5, port=port) + sock2 = mosq_test.do_client_connect(connect2_packet, connack2_packet, timeout=5, port=port) # Send a "ready" message sock2.send(publish_packet) mosq_test.expect_packet(sock1, "publish 1", publish_packet) # Connect client with will again as a separate connection, this should - # take over from the previous one but not trigger a Will. - sock3 = mosq_test.do_client_connect(connect2_packet, connack2b_packet, timeout=5, port=port) + # take over from the previous one but only trigger a Will if we are taking + # over a clean session/session-expiry-interval==0 client + sock3 = mosq_test.do_client_connect(connect3_packet, connack3_packet, timeout=5, port=port) sock2.close() + if clean_session1 == True or clean_session2 == True: + mosq_test.expect_packet(sock1, "publish LWT", publish_lwt_packet) + # Send the "ready" message again sock3.send(publish_packet) mosq_test.expect_packet(sock1, "publish 2", publish_packet) @@ -63,11 +84,15 @@ def do_test(proto_ver, clean_session): (stdo, stde) = broker.communicate() if rc: print(stde.decode('utf-8')) - print("proto_ver=%d clean_session=%d" % (proto_ver, clean_session)) + print("proto_ver=%d clean_session1=%d clean_session2=%d" % (proto_ver, clean_session1, clean_session2)) exit(rc) -do_test(proto_ver=4, clean_session=True) -do_test(proto_ver=4, clean_session=False) -do_test(proto_ver=5, clean_session=True) -do_test(proto_ver=5, clean_session=False) +do_test(proto_ver=4, clean_session1=True, clean_session2=True) +do_test(proto_ver=4, clean_session1=False, clean_session2=True) +do_test(proto_ver=4, clean_session1=True, clean_session2=False) +do_test(proto_ver=4, clean_session1=False, clean_session2=False) +do_test(proto_ver=5, clean_session1=True, clean_session2=True) +do_test(proto_ver=5, clean_session1=False, clean_session2=True) +do_test(proto_ver=5, clean_session1=True, clean_session2=False) +do_test(proto_ver=5, clean_session1=False, clean_session2=False)