Skip to content

Commit

Permalink
Fix v5 DISCONNECT packets with remaining length == 2.
Browse files Browse the repository at this point in the history
These were being treated as a protocol error.

Closes #1367. Thanks to Frank Pagliughi.
  • Loading branch information
ralight committed Sep 17, 2019
1 parent a8492b7 commit a415d41
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 8 deletions.
4 changes: 4 additions & 0 deletions ChangeLog.txt
@@ -1,3 +1,7 @@
Broker:
- Fix v5 DISCONNECT packets with remaining length == 2 being treated as a
protocol error. Closes #1367.

Build:
- Fix missing function warnings on NetBSD.

Expand Down
6 changes: 3 additions & 3 deletions src/handle_disconnect.c
Expand Up @@ -34,12 +34,12 @@ int handle__disconnect(struct mosquitto_db *db, struct mosquitto *context)
return MOSQ_ERR_INVAL;
}

if(context->protocol == mosq_p_mqtt5 && context->in_packet.remaining_length > 1){
if(context->protocol == mosq_p_mqtt5 && context->in_packet.remaining_length > 0){
/* FIXME - must handle reason code */
rc = packet__read_byte(&context->in_packet, &reason_code);
if(rc) return rc;

if(context->in_packet.remaining_length > 2){
if(context->in_packet.remaining_length > 1){
rc = property__read_all(CMD_DISCONNECT, &context->in_packet, &properties);
if(rc) return rc;
}
Expand All @@ -54,7 +54,7 @@ int handle__disconnect(struct mosquitto_db *db, struct mosquitto *context)
}
mosquitto_property_free_all(&properties); /* FIXME - TEMPORARY UNTIL PROPERTIES PROCESSED */

if(context->in_packet.remaining_length != 0){
if(context->in_packet.pos != context->in_packet.remaining_length){
return MOSQ_ERR_PROTOCOL;
}
log__printf(NULL, MOSQ_LOG_DEBUG, "Received DISCONNECT from %s", context->id);
Expand Down
26 changes: 21 additions & 5 deletions test/broker/01-connect-disconnect-v5.py
Expand Up @@ -7,16 +7,32 @@
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()
sock1 = mosq_test.do_client_connect(connect1_packet, connack1_packet, port=port)
mosq_test.do_send_receive(sock1, subscribe1_packet, suback1_packet, "suback1")


sock2 = mosq_test.do_client_connect(connect2_packet, connack2_packet, port=port)
sock2.send(disconnect_packet)
sock2.close()

# If this fails then we probably received the will
mosq_test.do_ping(sock1)

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)

connect1_packet = mosq_test.gen_connect("sub", proto_ver=5, keepalive=keepalive)
connack1_packet = mosq_test.gen_connack(rc=0, proto_ver=5)

mid = 1
subscribe1_packet = mosq_test.gen_subscribe(mid, "#", 0, proto_ver=5)
suback1_packet = mosq_test.gen_suback(mid, 0, proto_ver=5)

connect2_packet = mosq_test.gen_connect("connect-disconnect-test", proto_ver=5, keepalive=keepalive, will_topic="failure", will_payload=b"failure")
connack2_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)
Expand Down

0 comments on commit a415d41

Please sign in to comment.