Tests and fixes for shortened DISCONNECT packets.

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);

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);

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){

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)

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

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'),

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)