diff --git a/ChangeLog.txt b/ChangeLog.txt index 698bbe12..8981c600 100644 --- a/ChangeLog.txt +++ b/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. diff --git a/lib/read_handle_shared.c b/lib/read_handle_shared.c index 8a559092..6a135dfc 100644 --- a/lib/read_handle_shared.c +++ b/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); diff --git a/test/broker/06-bridge-fail-persist-resend-qos1.conf b/test/broker/06-bridge-fail-persist-resend-qos1.conf new file mode 100644 index 00000000..daaf1571 --- /dev/null +++ b/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 diff --git a/test/broker/06-bridge-fail-persist-resend-qos1.py b/test/broker/06-bridge-fail-persist-resend-qos1.py new file mode 100755 index 00000000..0b4e93c4 --- /dev/null +++ b/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) + diff --git a/test/broker/06-bridge-fail-persist-resend-qos2.conf b/test/broker/06-bridge-fail-persist-resend-qos2.conf new file mode 100644 index 00000000..daaf1571 --- /dev/null +++ b/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 diff --git a/test/broker/06-bridge-fail-persist-resend-qos2.py b/test/broker/06-bridge-fail-persist-resend-qos2.py new file mode 100755 index 00000000..e6b0b5e7 --- /dev/null +++ b/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) + diff --git a/test/broker/Makefile b/test/broker/Makefile index 510d90f9..ea6ab074 100644 --- a/test/broker/Makefile +++ b/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