[57] Handle PUB* with unknown message id gracefully.

Allows message flow to complete where e.g. the broker didn't persist a
partially complete flow.

Thanks to jsaak jsaak and Hiram van Paassen.

Bug: https://github.com/eclipse/mosquitto/issues/57

ChangeLog.txt

@@ -14,6 +14,8 @@ Broker:
   Closes #150.
 - Fix saving of persistence messages that start with a '/'. Closes #151.
 - Fix reconnecting for bridges that use TLS on Windows. Closes #154.
+- Broker and bridges can now cope with unknown incoming PUBACK, PUBREC,
+  PUBREL, PUBCOMP without disconnecting. Closes #57.
 
 Client library:
 - Fix the case where a message received just before the keepalive timer
@@ -21,6 +23,8 @@ Client library:
 - Return value of pthread_create is now checked.
 - _mosquitto_destroy should not cancel threads that weren't created by
   libmosquitto. Closes #166.
+- Clients can now cope with unknown incoming PUBACK, PUBREC, PUBREL, PUBCOMP
+  without disconnecting. Closes #57.
 
 Clients:
 - Handle some unchecked malloc() calls. Closes #1.

lib/read_handle_shared.c

@@ -71,7 +71,12 @@ int _mosquitto_handle_pubackcomp(struct mosquitto *mosq, const char *type)
 
 	if(mid){
 		rc = mqtt3_db_message_delete(db, mosq, mid, mosq_md_out);
-		if(rc) return rc;
+		if(rc == MOSQ_ERR_NOT_FOUND){
+			_mosquitto_log_printf(mosq, MOSQ_LOG_WARNING, "Warning: Received %s from %s for an unknown packet identifier %d.", type, mosq->id, mid);
+			return MOSQ_ERR_SUCCESS;
+		}else{
+			return rc;
+		}
 	}
 #else
 	_mosquitto_log_printf(mosq, MOSQ_LOG_DEBUG, "Client %s received %s (Mid: %d)", mosq->id, type, mid);
@@ -108,7 +113,11 @@ int _mosquitto_handle_pubrec(struct mosquitto *mosq)
 
 	rc = _mosquitto_message_out_update(mosq, mid, mosq_ms_wait_for_pubcomp);
 #endif
-	if(rc) return rc;
+	if(rc == MOSQ_ERR_NOT_FOUND){
+		_mosquitto_log_printf(mosq, MOSQ_LOG_WARNING, "Warning: Received PUBREC from %s for an unknown packet identifier %d.", mosq->id, mid);
+	}else if(rc != MOSQ_ERR_SUCCESS){
+		return rc;
+	}
 	rc = _mosquitto_send_pubrel(mosq, mid);
 	if(rc) return rc;
 
@@ -137,6 +146,7 @@ int _mosquitto_handle_pubrel(struct mosquitto_db *db, struct mosquitto *mosq)
 	if(mqtt3_db_message_release(db, mosq, mid, mosq_md_in)){
 		/* Message not found. Still send a PUBCOMP anyway because this could be
 		 * due to a repeated PUBREL after a client has reconnected. */
+		_mosquitto_log_printf(mosq, MOSQ_LOG_WARNING, "Warning: Received PUBREL from %s for an unknown packet identifier %d.", mosq->id, mid);
 	}
 #else
 	_mosquitto_log_printf(mosq, MOSQ_LOG_DEBUG, "Client %s received PUBREL (Mid: %d)", mosq->id, mid);

test/broker/06-bridge-fail-persist-resend-qos1.conf

@@ -0,0 +1,11 @@
+port 1889
+
+connection bridge-u-test
+remote_clientid bridge-u-test
+address 127.0.0.1:1888
+topic bridge/# out
+
+cleansession true
+notifications false
+restart_timeout 5
+try_private false

test/broker/06-bridge-fail-persist-resend-qos1.py

@@ -0,0 +1,77 @@
+#!/usr/bin/env python
+
+# Test whether a bridge can cope with an unknown PUBACK
+
+import socket
+import subprocess
+import time
+
+import inspect, os, sys
+# From http://stackoverflow.com/questions/279237/python-import-a-module-from-a-folder
+cmd_subfolder = os.path.realpath(os.path.abspath(os.path.join(os.path.split(inspect.getfile( inspect.currentframe() ))[0],"..")))
+if cmd_subfolder not in sys.path:
+    sys.path.insert(0, cmd_subfolder)
+
+import mosq_test
+
+rc = 1
+keepalive = 60
+connect_packet = mosq_test.gen_connect("bridge-u-test", keepalive=keepalive)
+connack_packet = mosq_test.gen_connack(rc=0)
+
+mid = 180
+mid_unknown = 2000
+
+publish_packet = mosq_test.gen_publish("bridge/unknown/qos1", qos=1, payload="bridge-message", mid=mid)
+puback_packet = mosq_test.gen_puback(mid)
+puback_packet_unknown = mosq_test.gen_puback(mid_unknown)
+
+
+unsubscribe_packet = mosq_test.gen_unsubscribe(1, "bridge/#")
+unsuback_packet = mosq_test.gen_unsuback(1)
+
+
+if os.environ.get('MOSQ_USE_VALGRIND') is not None:
+    sleep_time = 5
+else:
+    sleep_time = 0.5
+
+
+sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+sock.settimeout(10)
+sock.bind(('', 1888))
+sock.listen(5)
+
+broker = mosq_test.start_broker(filename=os.path.basename(__file__), port=1889)
+time.sleep(sleep_time)
+
+try:
+    (conn, address) = sock.accept()
+    conn.settimeout(20)
+
+    if mosq_test.expect_packet(conn, "connect", connect_packet):
+        conn.send(connack_packet)
+
+        if mosq_test.expect_packet(conn, "unsubscribe", unsubscribe_packet):
+            conn.send(unsuback_packet)
+
+            # Send the unexpected puback packet
+            conn.send(puback_packet_unknown)
+
+            # Send a legitimate publish packet to verify everything is still ok
+            conn.send(publish_packet)
+
+            if mosq_test.expect_packet(conn, "puback", puback_packet):
+                rc = 0
+
+finally:
+    broker.terminate()
+    broker.wait()
+    if rc:
+        (stdo, stde) = broker.communicate()
+        print(stde)
+    sock.close()
+
+exit(rc)
+

test/broker/06-bridge-fail-persist-resend-qos2.conf

@@ -0,0 +1,11 @@
+port 1889
+
+connection bridge-u-test
+remote_clientid bridge-u-test
+address 127.0.0.1:1888
+topic bridge/# out
+
+cleansession true
+notifications false
+restart_timeout 5
+try_private false

test/broker/06-bridge-fail-persist-resend-qos2.py

@@ -0,0 +1,90 @@
+#!/usr/bin/env python
+
+# Test whether a bridge can cope with an unknown PUBACK
+
+import socket
+import subprocess
+import time
+
+import inspect, os, sys
+# From http://stackoverflow.com/questions/279237/python-import-a-module-from-a-folder
+cmd_subfolder = os.path.realpath(os.path.abspath(os.path.join(os.path.split(inspect.getfile( inspect.currentframe() ))[0],"..")))
+if cmd_subfolder not in sys.path:
+    sys.path.insert(0, cmd_subfolder)
+
+import mosq_test
+
+rc = 1
+keepalive = 60
+connect_packet = mosq_test.gen_connect("bridge-u-test", keepalive=keepalive)
+connack_packet = mosq_test.gen_connack(rc=0)
+
+mid = 180
+mid_unknown = 2000
+
+publish_packet = mosq_test.gen_publish("bridge/unknown/qos2", qos=1, payload="bridge-message", mid=mid)
+puback_packet = mosq_test.gen_puback(mid)
+
+pubrec_packet_unknown1 = mosq_test.gen_pubrec(mid_unknown+1)
+pubrel_packet_unknown1 = mosq_test.gen_pubrel(mid_unknown+1)
+
+pubrel_packet_unknown2 = mosq_test.gen_pubrel(mid_unknown+2)
+pubcomp_packet_unknown2 = mosq_test.gen_pubcomp(mid_unknown+2)
+
+pubcomp_packet_unknown3 = mosq_test.gen_pubcomp(mid_unknown+3)
+
+
+unsubscribe_packet = mosq_test.gen_unsubscribe(1, "bridge/#")
+unsuback_packet = mosq_test.gen_unsuback(1)
+
+
+if os.environ.get('MOSQ_USE_VALGRIND') is not None:
+    sleep_time = 5
+else:
+    sleep_time = 0.5
+
+
+sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+sock.settimeout(10)
+sock.bind(('', 1888))
+sock.listen(5)
+
+broker = mosq_test.start_broker(filename=os.path.basename(__file__), port=1889)
+time.sleep(sleep_time)
+
+try:
+    (conn, address) = sock.accept()
+    conn.settimeout(20)
+
+    if mosq_test.expect_packet(conn, "connect", connect_packet):
+        conn.send(connack_packet)
+
+        if mosq_test.expect_packet(conn, "unsubscribe", unsubscribe_packet):
+            conn.send(unsuback_packet)
+
+            # Send the unexpected pubrec packet
+            conn.send(pubrec_packet_unknown1)
+            if mosq_test.expect_packet(conn, "pubrel", pubrel_packet_unknown1):
+
+                conn.send(pubrel_packet_unknown2)
+                if mosq_test.expect_packet(conn, "pubcomp", pubcomp_packet_unknown2):
+
+                    conn.send(pubcomp_packet_unknown3)
+
+                    # Send a legitimate publish packet to verify everything is still ok
+                    conn.send(publish_packet)
+        
+                    if mosq_test.expect_packet(conn, "puback", puback_packet):
+                        rc = 0
+
+finally:
+    broker.terminate()
+    broker.wait()
+    if rc:
+        (stdo, stde) = broker.communicate()
+        print(stde)
+    sock.close()
+
+exit(rc)
+

test/broker/Makefile

@@ -68,6 +68,8 @@ endif
 	./06-bridge-br2b-disconnect-qos2.py
 	./06-bridge-b2br-disconnect-qos1.py
 	./06-bridge-b2br-disconnect-qos2.py
+	./06-bridge-fail-persist-resend-qos1.py
+	./06-bridge-fail-persist-resend-qos2.py
 
 07 :
 	./07-will-qos0.py