Permalink
Browse files

[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: #57
  • Loading branch information...
1 parent e8185dd commit a187b3f5fa800387ab7507aae5a738b43464630e @ralight ralight committed May 19, 2016
View
@@ -14,13 +14,17 @@ 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
expired would cause the client to miss the keepalive timer.
- 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.
@@ -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);
@@ -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
@@ -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)
+
@@ -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
@@ -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)
+
@@ -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

0 comments on commit a187b3f

Please sign in to comment.