From 8513af4da5c8d98c8fa3d6c6cef44f8133ad025d Mon Sep 17 00:00:00 2001 From: Roger Light Date: Thu, 17 Jan 2019 18:51:23 +0000 Subject: [PATCH] Tests and fixes for shortened DISCONNECT packets. --- lib/handle_disconnect.c | 6 ++- lib/send_disconnect.c | 18 +++++---- src/handle_connect.c | 9 +++-- test/broker/01-connect-disconnect-v5.py | 51 +++++++++++++++++++++++++ test/broker/Makefile | 1 + test/broker/ptest.py | 1 + test/mosq_test.py | 15 ++++++-- 7 files changed, 86 insertions(+), 15 deletions(-) create mode 100755 test/broker/01-connect-disconnect-v5.py diff --git a/lib/handle_disconnect.c b/lib/handle_disconnect.c index 2f3bb58a..0ec1c8b3 100644 --- a/lib/handle_disconnect.c +++ b/lib/handle_disconnect.c @@ -38,10 +38,14 @@ int handle__disconnect(struct mosquitto *mosq) return MOSQ_ERR_INVAL; } + if(mosq->protocol != mosq_p_mqtt5){ + return MOSQ_ERR_PROTOCOL; + } + rc = packet__read_byte(&mosq->in_packet, &reason_code); if(rc) return rc; - if(mosq->protocol == mosq_p_mqtt5){ + if(mosq->in_packet.remaining_length > 2){ rc = property__read_all(CMD_DISCONNECT, &mosq->in_packet, &properties); if(rc) return rc; mosquitto_property_free_all(&properties); diff --git a/lib/send_disconnect.c b/lib/send_disconnect.c index c0959fbb..3615a001 100644 --- a/lib/send_disconnect.c +++ b/lib/send_disconnect.c @@ -51,11 +51,13 @@ int send__disconnect(struct mosquitto *mosq, uint8_t reason_code, const mosquitt if(!packet) return MOSQ_ERR_NOMEM; packet->command = CMD_DISCONNECT; - if(mosq->protocol == mosq_p_mqtt5){ - proplen = property__get_length_all(properties); - varbytes = packet__varint_bytes(proplen); - /* 1 here is the reason code */ - packet->remaining_length = 1 + proplen + varbytes; + if(mosq->protocol == mosq_p_mqtt5 && (reason_code != 0 || properties)){ + packet->remaining_length = 1; + if(properties){ + proplen = property__get_length_all(properties); + varbytes = packet__varint_bytes(proplen); + packet->remaining_length += proplen + varbytes; + } }else{ packet->remaining_length = 0; } @@ -65,9 +67,11 @@ int send__disconnect(struct mosquitto *mosq, uint8_t reason_code, const mosquitt mosquitto__free(packet); return rc; } - if(mosq->protocol == mosq_p_mqtt5){ + if(mosq->protocol == mosq_p_mqtt5 && (reason_code != 0 || properties)){ packet__write_byte(packet, reason_code); - property__write_all(packet, properties, true); + if(properties){ + property__write_all(packet, properties, true); + } } return packet__queue(mosq, packet); diff --git a/src/handle_connect.c b/src/handle_connect.c index cefd20d8..739cd4d6 100644 --- a/src/handle_connect.c +++ b/src/handle_connect.c @@ -818,12 +818,15 @@ int handle__disconnect(struct mosquitto_db *db, struct mosquitto *context) return MOSQ_ERR_INVAL; } - if(context->protocol == mosq_p_mqtt5){ + if(context->protocol == mosq_p_mqtt5 && context->in_packet.remaining_length > 1){ /* FIXME - must handle reason code */ rc = packet__read_byte(&context->in_packet, &reason_code); if(rc) return rc; - rc = property__read_all(CMD_DISCONNECT, &context->in_packet, &properties); - if(rc) return rc; + + if(context->in_packet.remaining_length > 2){ + rc = property__read_all(CMD_DISCONNECT, &context->in_packet, &properties); + if(rc) return rc; + } } rc = property__process_disconnect(context, properties); if(rc){ diff --git a/test/broker/01-connect-disconnect-v5.py b/test/broker/01-connect-disconnect-v5.py new file mode 100755 index 00000000..96dd4d89 --- /dev/null +++ b/test/broker/01-connect-disconnect-v5.py @@ -0,0 +1,51 @@ +#!/usr/bin/env python + +# loop through the different v5 DISCONNECT reason_code/properties options. + +from mosq_test_helper import * + +def disco_test(test, disconnect_packet): + global rc + + sock = mosq_test.do_client_connect(connect_packet, connack_packet, port=port) + sock.send(disconnect_packet) + sock.close() + rc -= 1 + + +rc = 4 +keepalive = 10 +connect_packet = mosq_test.gen_connect("connect-disconnect-test", proto_ver=5, keepalive=keepalive) +connack_packet = mosq_test.gen_connack(rc=0, proto_ver=5) + +port = mosq_test.get_port() +broker = mosq_test.start_broker(filename=os.path.basename(__file__), port=port) + + +try: + # No reason code, no properties, len=0 + disconnect_packet = mosq_test.gen_disconnect(proto_ver=5) + disco_test("disco len=0", disconnect_packet) + + # Reason code, no properties, len=1 + disconnect_packet = mosq_test.gen_disconnect(proto_ver=5, reason_code=0) + disco_test("disco len=1", disconnect_packet) + + # Reason code, empty properties, len=2 + disconnect_packet = mosq_test.gen_disconnect(proto_ver=5, reason_code=0, properties="") + disco_test("disco len=2", disconnect_packet) + + # Reason code, one property, len>2 + props = mqtt5_props.gen_string_pair_prop(mqtt5_props.PROP_USER_PROPERTY, "key", "value") + disconnect_packet = mosq_test.gen_disconnect(proto_ver=5, reason_code=0, properties=props) + disco_test("disco len>2", disconnect_packet) +finally: + broker.terminate() + broker.wait() + (stdo, stde) = broker.communicate() + if rc: + print(stde) + +if rc != 0: + print(test) + exit(rc) diff --git a/test/broker/Makefile b/test/broker/Makefile index 6295d729..74b7f434 100644 --- a/test/broker/Makefile +++ b/test/broker/Makefile @@ -23,6 +23,7 @@ test : test-compile 01 02 03 04 05 06 07 08 09 10 11 12 ./01-connect-duplicate-v5.py ./01-connect-success.py ./01-connect-success-v5.py + ./01-connect-disconnect-v5.py ./01-connect-invalid-protonum.py ./01-connect-invalid-id-0.py ./01-connect-invalid-id-0-311.py diff --git a/test/broker/ptest.py b/test/broker/ptest.py index 3f96794c..cffe6cb0 100755 --- a/test/broker/ptest.py +++ b/test/broker/ptest.py @@ -12,6 +12,7 @@ tests = [ (1, './01-connect-duplicate-v5.py'), (1, './01-connect-success.py'), (1, './01-connect-success-v5.py'), + (1, './01-connect-disconnect-v5.py'), (1, './01-connect-invalid-protonum.py'), (1, './01-connect-invalid-id-0.py'), (1, './01-connect-invalid-id-0-311.py'), diff --git a/test/mosq_test.py b/test/mosq_test.py index a09367cc..b4a4b125 100644 --- a/test/mosq_test.py +++ b/test/mosq_test.py @@ -486,11 +486,18 @@ def gen_pingreq(): def gen_pingresp(): return struct.pack('!BB', 208, 0) -def gen_disconnect(reason_code=0, proto_ver=4, properties=""): - if proto_ver == 5: - properties = mqtt5_props.prop_finalise(properties) +def gen_disconnect(reason_code=-1, proto_ver=4, properties=None): + if proto_ver == 5 and (reason_code != -1 or properties is not None): + if reason_code == -1: + reason_code = 0 - return struct.pack('!BBB', 224, 1+len(properties), reason_code) + properties + if properties is None: + return struct.pack('!BBB', 224, 1, reason_code) + elif properties == "": + return struct.pack('!BBBB', 224, 2, reason_code, 0) + else: + properties = mqtt5_props.prop_finalise(properties) + return struct.pack("!BBB", 224, 1+len(properties), reason_code) + properties else: return struct.pack('!BB', 224, 0)