Fix missing reason_code on v5 UNSUBACK.

Closes #1167. Thanks to Christoph Krey.

src/handle_unsubscribe.c

@@ -31,6 +31,9 @@ int handle__unsubscribe(struct mosquitto_db *db, struct mosquitto *context)
 	char *sub;
 	int slen;
 	int rc;
+	int reason_code_count = 0;
+	int reason_code_max;
+	uint8_t *reason_codes = NULL, *reason_tmp;
 	mosquitto_property *properties = NULL;
 
 	if(!context) return MOSQ_ERR_INVAL;
@@ -57,32 +60,49 @@ int handle__unsubscribe(struct mosquitto_db *db, struct mosquitto *context)
 			return MOSQ_ERR_PROTOCOL;
 		}
 	}
+
+	reason_code_max = 10;
+	reason_codes = mosquitto__malloc(reason_code_max);
+	if(!reason_codes){
+		return MOSQ_ERR_NOMEM;
+	}
+
 	while(context->in_packet.pos < context->in_packet.remaining_length){
 		sub = NULL;
 		if(packet__read_string(&context->in_packet, &sub, &slen)){
 			return 1;
 		}
 
-		if(sub){
-			if(!slen){
-				log__printf(NULL, MOSQ_LOG_INFO,
-						"Empty unsubscription string from %s, disconnecting.",
-						context->id);
-				mosquitto__free(sub);
-				return 1;
-			}
-			if(mosquitto_sub_topic_check(sub)){
-				log__printf(NULL, MOSQ_LOG_INFO,
-						"Invalid unsubscription string from %s, disconnecting.",
-						context->id);
-				mosquitto__free(sub);
-				return 1;
-			}
-
-			log__printf(NULL, MOSQ_LOG_DEBUG, "\t%s", sub);
-			sub__remove(db, context, sub, db->subs);
-			log__printf(NULL, MOSQ_LOG_UNSUBSCRIBE, "%s %s", context->id, sub);
+		if(!slen){
+			log__printf(NULL, MOSQ_LOG_INFO,
+					"Empty unsubscription string from %s, disconnecting.",
+					context->id);
 			mosquitto__free(sub);
+			return 1;
+		}
+		if(mosquitto_sub_topic_check(sub)){
+			log__printf(NULL, MOSQ_LOG_INFO,
+					"Invalid unsubscription string from %s, disconnecting.",
+					context->id);
+			mosquitto__free(sub);
+			return 1;
+		}
+
+		log__printf(NULL, MOSQ_LOG_DEBUG, "\t%s", sub);
+		sub__remove(db, context, sub, db->subs);
+		log__printf(NULL, MOSQ_LOG_UNSUBSCRIBE, "%s %s", context->id, sub);
+		mosquitto__free(sub);
+
+		reason_codes[reason_code_count] = 0;
+		reason_code_count++;
+		if(reason_code_count == reason_code_max){
+			reason_tmp = mosquitto__realloc(reason_codes, reason_code_max*2);
+			if(!reason_tmp){
+				mosquitto__free(reason_codes);
+				return MOSQ_ERR_NOMEM;
+			}
+			reason_codes = reason_tmp;
+			reason_code_max *= 2;
 		}
 	}
 #ifdef WITH_PERSISTENCE
@@ -92,6 +112,8 @@ int handle__unsubscribe(struct mosquitto_db *db, struct mosquitto *context)
 	log__printf(NULL, MOSQ_LOG_DEBUG, "Sending UNSUBACK to %s", context->id);
 
 	/* We don't use Reason String or User Property yet. */
-	return send__unsuback(context, mid, NULL);
+	rc = send__unsuback(context, mid, reason_code_count, reason_codes, NULL);
+	mosquitto__free(reason_codes);
+	return rc;
 }
 

src/mosquitto_broker_internal.h

@@ -525,7 +525,7 @@ int restore_privileges(void);
  * ============================================================ */
 int send__connack(struct mosquitto_db *db, struct mosquitto *context, int ack, int reason_code, const mosquitto_property *properties);
 int send__suback(struct mosquitto *context, uint16_t mid, uint32_t payloadlen, const void *payload);
-int send__unsuback(struct mosquitto *context, uint16_t mid, const mosquitto_property *properties);
+int send__unsuback(struct mosquitto *context, uint16_t mid, int reason_code_count, uint8_t *reason_codes, const mosquitto_property *properties);
 
 /* ============================================================
  * Network functions

src/send_unsuback.c

@@ -25,7 +25,7 @@ Contributors:
 #include "property_mosq.h"
 
 
-int send__unsuback(struct mosquitto *mosq, uint16_t mid, const mosquitto_property *properties)
+int send__unsuback(struct mosquitto *mosq, uint16_t mid, int reason_code_count, uint8_t *reason_codes, const mosquitto_property *properties)
 {
 	struct mosquitto__packet *packet = NULL;
 	int rc;
@@ -41,7 +41,7 @@ int send__unsuback(struct mosquitto *mosq, uint16_t mid, const mosquitto_propert
 	if(mosq->protocol == mosq_p_mqtt5){
 		proplen = property__get_length_all(properties);
 		varbytes = packet__varint_bytes(proplen);
-		packet->remaining_length += varbytes + proplen;
+		packet->remaining_length += varbytes + proplen + reason_code_count;
 	}
 
 	rc = packet__alloc(packet);
@@ -54,6 +54,7 @@ int send__unsuback(struct mosquitto *mosq, uint16_t mid, const mosquitto_propert
 
 	if(mosq->protocol == mosq_p_mqtt5){
 		property__write_all(packet, properties, true);
+        packet__write_bytes(packet, reason_codes, reason_code_count);
 	}
 
 	return packet__queue(mosq, packet);

test/broker/02-unsubscribe-qos2-multiple-v5.py

@@ -0,0 +1,34 @@
+#!/usr/bin/env python
+
+# Test whether a v5 UNSUBSCRIBE to multiple topics with QoS 2 results in the correct UNSUBACK packet.
+
+from mosq_test_helper import *
+
+rc = 1
+mid = 3
+keepalive = 60
+connect_packet = mosq_test.gen_connect("unsubscribe-qos2-test", keepalive=keepalive, proto_ver=5)
+connack_packet = mosq_test.gen_connack(rc=0, proto_ver=5)
+
+unsubscribe_packet = mosq_test.gen_unsubscribe_multiple(mid, ["qos2/one", "qos2/two"], proto_ver=5)
+unsuback_packet = mosq_test.gen_unsuback(mid, proto_ver=5, reason_code=[0, 0])
+
+port = mosq_test.get_port()
+broker = mosq_test.start_broker(filename=os.path.basename(__file__), port=port)
+
+try:
+    sock = mosq_test.do_client_connect(connect_packet, connack_packet, port=port)
+    mosq_test.do_send_receive(sock, unsubscribe_packet, unsuback_packet, "unsuback")
+
+    rc = 0
+
+    sock.close()
+finally:
+    broker.terminate()
+    broker.wait()
+    (stdo, stde) = broker.communicate()
+    if rc:
+        print(stde)
+
+exit(rc)
+

test/broker/Makefile

@@ -76,6 +76,7 @@ endif
 	./02-unsubscribe-qos2-v5.py
 	./02-unsubscribe-invalid-no-topic.py
 	./02-unsubscribe-qos2-multiple.py
+	./02-unsubscribe-qos2-multiple-v5.py
 	./02-subscribe-invalid-utf8.py
 	./02-subscribe-persistence-flipflop.py
 	./02-subhier-crash.py

test/broker/test.py

@@ -57,6 +57,7 @@ tests = [
     (1, './02-unsubscribe-qos2-v5.py'),
     (1, './02-unsubscribe-invalid-no-topic.py'),
     (1, './02-unsubscribe-qos2-multiple.py'),
+    (1, './02-unsubscribe-qos2-multiple-v5.py'),
     (1, './02-subscribe-invalid-utf8.py'),
     (1, './02-subscribe-persistence-flipflop.py'),
     (1, './02-subhier-crash.py'),

test/mosq_test.py

@@ -498,9 +498,16 @@ def gen_unsubscribe_multiple(mid, topics, proto_ver=4):
 
         return struct.pack("!BBH", 162, remaining_length, mid) + packet
 
-def gen_unsuback(mid, proto_ver=4):
+def gen_unsuback(mid, proto_ver=4, reason_code=0):
     if proto_ver == 5:
-        return struct.pack('!BBHB', 176, 3, mid, 0)
+        if isinstance(reason_code, list):
+            reason_code_count = len(reason_code)
+            p = struct.pack('!BBHB', 176, 3+reason_code_count, mid, 0)
+            for r in reason_code:
+                p += struct.pack('B', r)
+            return p
+        else:
+            return struct.pack('!BBHBB', 176, 4, mid, 0, reason_code)
     else:
         return struct.pack('!BBH', 176, 2, mid)