Process session-expiry-interval on CONNECT and DISCONNECT.

Add test to check for invalid values.

lib/mosquitto_internal.h

@@ -209,6 +209,7 @@ struct mosquitto {
 	pthread_t thread_id;
 #endif
 	bool clean_start;
+	uint32_t session_expiry_interval;
 #ifdef WITH_BROKER
 	char *old_id; /* for when a duplicate client connects, but we still want to
 					 know what the id was */

src/CMakeLists.txt

@@ -31,6 +31,7 @@ set (MOSQ_SRCS
 	../lib/packet_mosq.c ../lib/packet_mosq.h
 	persist.c persist.h
 	plugin.c
+	property_broker.c
 	../lib/property_mosq.c ../lib/property_mosq.h
 	read_handle.c
 	../lib/read_handle.h

src/Makefile

@@ -33,6 +33,7 @@ OBJS=	mosquitto.o \
 		net_mosq.o \
 		packet_datatypes.o \
 		packet_mosq.o \
+		property_broker.o \
 		property_mosq.o \
 		persist.o \
 		plugin.o \
@@ -141,6 +142,9 @@ packet_datatypes.o : ../lib/packet_datatypes.c ../lib/packet_mosq.h
 packet_mosq.o : ../lib/packet_mosq.c ../lib/packet_mosq.h
 	${CROSS_COMPILE}${CC} $(BROKER_CFLAGS) -c $< -o $@
 
+property_broker.o : property_broker.c mosquitto_broker_internal.h
+	${CROSS_COMPILE}${CC} $(BROKER_CFLAGS) -c $< -o $@
+
 property_mosq.o : ../lib/property_mosq.c ../lib/property_mosq.h
 	${CROSS_COMPILE}${CC} $(BROKER_CFLAGS) -c $< -o $@
 

src/handle_connect.c

@@ -254,6 +254,8 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context)
 		if(rc) return rc;
 		mosquitto_property_free_all(&properties);
 	}
+	property__process_connect(context, properties);
+
 	mosquitto_property_free_all(&properties); /* FIXME - TEMPORARY UNTIL PROPERTIES PROCESSED */
 
 	if(packet__read_string(&context->in_packet, &client_id, &slen)){
@@ -727,9 +729,11 @@ handle_connect_error:
 	return rc;
 }
 
+
 int handle__disconnect(struct mosquitto_db *db, struct mosquitto *context)
 {
 	int rc;
+	uint8_t reason_code;
 	mosquitto_property *properties = NULL;
 
 	if(!context){
@@ -737,9 +741,19 @@ int handle__disconnect(struct mosquitto_db *db, struct mosquitto *context)
 	}
 
 	if(context->protocol == mosq_p_mqtt5){
+		/* FIXME - must handle reason code */
+		rc = packet__read_byte(&context->in_packet, &reason_code);
+		if(rc) return rc;
 		rc = property__read_all(CMD_DISCONNECT, &context->in_packet, &properties);
 		if(rc) return rc;
+	}
+	rc = property__process_disconnect(context, properties);
+	if(rc){
+		if(rc == MOSQ_ERR_PROTOCOL){
+			send__disconnect(context, MQTT_RC_PROTOCOL_ERROR, NULL);
+		}
 		mosquitto_property_free_all(&properties);
+		return rc;
 	}
 	mosquitto_property_free_all(&properties); /* FIXME - TEMPORARY UNTIL PROPERTIES PROCESSED */
 

src/mosquitto_broker_internal.h

@@ -608,6 +608,12 @@ int bridge__connect_step3(struct mosquitto_db *db, struct mosquitto *context);
 void bridge__packet_cleanup(struct mosquitto *context);
 #endif
 
+/* ============================================================
+ * Property related functions
+ * ============================================================ */
+int property__process_connect(struct mosquitto *context, mosquitto_property *props);
+int property__process_disconnect(struct mosquitto *context, mosquitto_property *props);
+
 /* ============================================================
  * Security related functions
  * ============================================================ */

src/property_broker.c

@@ -0,0 +1,64 @@
+/*
+Copyright (c) 2018 Roger Light <roger@atchoo.org>
+
+All rights reserved. This program and the accompanying materials
+are made available under the terms of the Eclipse Public License v1.0
+and Eclipse Distribution License v1.0 which accompany this distribution.
+ 
+The Eclipse Public License is available at
+   http://www.eclipse.org/legal/epl-v10.html
+and the Eclipse Distribution License is available at
+  http://www.eclipse.org/org/documents/edl-v10.php.
+ 
+Contributors:
+   Roger Light - initial implementation and documentation.
+*/
+
+#include "config.h"
+
+#include <assert.h>
+#include <stdio.h>
+#include <string.h>
+
+#include "mosquitto_broker_internal.h"
+#include "mqtt_protocol.h"
+#include "property_mosq.h"
+
+/* Process the incoming properties, we should be able to assume that only valid
+ * properties for CONNECT are present here. */
+int property__process_connect(struct mosquitto *context, mosquitto_property *props)
+{
+	mosquitto_property *p;
+
+	p = props;
+
+	while(p){
+		if(p->identifier == MQTT_PROP_SESSION_EXPIRY_INTERVAL){
+			context->session_expiry_interval = p->value.i32;
+		}
+		p = p->next;
+	}
+
+	return MOSQ_ERR_SUCCESS;
+}
+
+/* Process the incoming properties, we should be able to assume that only valid
+ * properties for DISCONNECT are present here. */
+int property__process_disconnect(struct mosquitto *context, mosquitto_property *props)
+{
+	mosquitto_property *p;
+
+	p = props;
+
+	while(p){
+		if(p->identifier == MQTT_PROP_SESSION_EXPIRY_INTERVAL){
+			if(context->session_expiry_interval == 0 && p->value.i32 != 0){
+				return MOSQ_ERR_PROTOCOL;
+			}
+			context->session_expiry_interval = p->value.i32;
+		}
+		p = p->next;
+	}
+	return MOSQ_ERR_SUCCESS;
+}
+

test/broker/12-prop-session-expiry-invalid.py

@@ -0,0 +1,39 @@
+#!/usr/bin/env python
+
+# Test whether sending a non zero session expiry interval in DISCONNECT after
+# having sent a zero session expiry interval is treated correctly in MQTT v5.
+
+from mosq_test_helper import *
+
+rc = 1
+
+keepalive = 10
+props = mqtt5_props.gen_uint32_prop(mqtt5_props.PROP_SESSION_EXPIRY_INTERVAL, 0)
+props = mqtt5_props.prop_finalise(props)
+connect_packet = mosq_test.gen_connect("test", proto_ver=5, keepalive=keepalive, properties=props)
+
+connack_packet = mosq_test.gen_connack(rc=0, proto_ver=5)
+
+props = mqtt5_props.gen_uint32_prop(mqtt5_props.PROP_SESSION_EXPIRY_INTERVAL, 1)
+props = mqtt5_props.prop_finalise(props)
+disconnect_client_packet = mosq_test.gen_disconnect(proto_ver=5, properties=props)
+
+disconnect_server_packet = mosq_test.gen_disconnect(proto_ver=5, reason_code=130)
+
+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, disconnect_client_packet, disconnect_server_packet, "disconnect")
+    sock.close()
+    rc = 0
+finally:
+    broker.terminate()
+    broker.wait()
+    (stdo, stde) = broker.communicate()
+    if rc:
+        print(stde)
+
+exit(rc)
+

test/broker/Makefile

@@ -15,7 +15,7 @@ test-compile :
 ptest : test-compile
 	./ptest.py
 
-test : test-compile 01 02 03 04 05 06 07 08 09 10 11
+test : test-compile 01 02 03 04 05 06 07 08 09 10 11 12
 
 01 :
 	./01-connect-success.py
@@ -135,3 +135,6 @@ endif
 
 11 :
 	./11-persistent-subscription.py
+
+12 :
+	./12-prop-session-expiry-invalid.py

test/broker/ptest.py

@@ -105,6 +105,7 @@ tests = [
     (2, './10-listener-mount-point.py'),
 
     (1, './11-persistent-subscription.py'),
+	(1, './12-prop-session-expiry-invalid.py'),
     ]
 
 minport = 1888

test/broker/readme.txt

@@ -12,3 +12,8 @@ Numbering is as follows:
 05: Clean session tests
 06: Bridge tests
 07: Will tests
+08: TLS tests
+09: Auth plugin tests
+10: Listener tests
+11: Persistence tests
+12: Property tests

test/mosq_test.py

@@ -298,7 +298,7 @@ def to_string(packet):
         # Reserved
         return "0xF0"
 
-def gen_connect(client_id, clean_session=True, keepalive=60, username=None, password=None, will_topic=None, will_qos=0, will_retain=False, will_payload="", proto_ver=4, connect_reserved=False, properties=None):
+def gen_connect(client_id, clean_session=True, keepalive=60, username=None, password=None, will_topic=None, will_qos=0, will_retain=False, will_payload="", proto_ver=4, connect_reserved=False, properties=""):
     if (proto_ver&0x7F) == 3 or proto_ver == 0:
         remaining_length = 12
     elif (proto_ver&0x7F) == 4 or proto_ver == 5:
@@ -318,7 +318,9 @@ def gen_connect(client_id, clean_session=True, keepalive=60, username=None, pass
         connect_flags = connect_flags | 0x02
 
     if proto_ver == 5:
-        remaining_length += 1
+        if properties == "":
+            properties = struct.pack("B", 0)
+        remaining_length += len(properties)
 
     if will_topic != None:
         remaining_length = remaining_length + 2+len(will_topic) + 2+len(will_payload)
@@ -341,7 +343,7 @@ def gen_connect(client_id, clean_session=True, keepalive=60, username=None, pass
         packet = packet + struct.pack("!H4sBBH", len("MQTT"), "MQTT", proto_ver, connect_flags, keepalive)
 
     if proto_ver == 5:
-        packet += struct.pack("B", 0)
+        packet += properties
 
     if client_id != None:
         packet = packet + struct.pack("!H"+str(len(client_id))+"s", len(client_id), client_id)
@@ -467,9 +469,12 @@ def gen_pingreq():
 def gen_pingresp():
     return struct.pack('!BB', 208, 0)
 
-def gen_disconnect(reason_code=0, proto_ver=4):
+def gen_disconnect(reason_code=0, proto_ver=4, properties=""):
     if proto_ver == 5:
-        return struct.pack('!BBBB', 224, 2, reason_code, 0)
+        if properties == "":
+            properties = struct.pack("B", 0)
+
+        return struct.pack('!BBB', 224, 1+len(properties), reason_code) + properties
     else:
         return struct.pack('!BB', 224, 0)