Broker: Fix parsing of ints in config. Add extra config test cases.

Author: ralight

src/conf.c

@@ -2005,7 +2005,7 @@ static int config__read_file_core(struct mosquitto__config *config, bool reload,
 				}else if(!strcmp(token, "max_packet_size")){
 					if(conf__parse_int(&token, "max_packet_size", &tmp_int, &saveptr)) return MOSQ_ERR_INVAL;
 					if(tmp_int < 20){
-						log__printf(NULL, MOSQ_LOG_ERR, "Error: Invalid 'max_packet_size' value (%d).", tmp_int);
+						log__printf(NULL, MOSQ_LOG_ERR, "Error: 'max_packet_size' must be >= 20.");
 						return MOSQ_ERR_INVAL;
 					}
 					config->max_packet_size = (uint32_t)tmp_int;
@@ -2696,7 +2696,17 @@ static int conf__parse_int(char **token, const char *name, int *value, char **sa
 {
 	*token = strtok_r(NULL, " ", saveptr);
 	if(*token){
-		*value = atoi(*token);
+		char *endptr = NULL;
+		long l = strtol(*token, &endptr, 0);
+		if(endptr == *token || endptr[0] != '\0'){
+			log__printf(NULL, MOSQ_LOG_ERR, "Error: '%s' value not a number.", name);
+			return MOSQ_ERR_INVAL;
+		}
+		if(l > INT_MAX || l < INT_MIN){
+			log__printf(NULL, MOSQ_LOG_ERR, "Error: '%s' value out of range.", name);
+			return MOSQ_ERR_INVAL;
+		}
+		*value = (int)l;
 	}else{
 		log__printf(NULL, MOSQ_LOG_ERR, "Error: Empty '%s' value in configuration.", name);
 		return MOSQ_ERR_INVAL;

test/broker/15-persist-migrate-db.py

@@ -66,6 +66,7 @@ def do_failure_test(create_db_of_version: list[int]):
             port=port,
             rc_expected=3,
             error_log_entry=f"Unknown database_schema version {'.'.join([str(i) for i in create_db_of_version])}",
+            with_test_config=False,
         )
     except Exception as exc:
         print(f"Exception: {str(exc)}")

test/broker/16-config-parse-errors-tls.py

@@ -19,14 +19,14 @@
 do_test_broker_failure(conf_file, [f"listener {port}","certfile"], port, 3) # empty certfile
 do_test_broker_failure(conf_file, [f"listener {port}","keyfile"], port, 3) # empty keyfile
 
-do_test_broker_failure(conf_file, [f"listener {port}","certfile ./16-config-parse-errors.py","keyfile ../ssl/server.key"], port, 1) # invalid certfile
-do_test_broker_failure(conf_file, [f"listener {port}","certfile ../ssl/server.crt","keyfile ./16-config-parse-errors.py"], port, 1) # invalid keyfile
-do_test_broker_failure(conf_file, [f"listener {port}","certfile ../ssl/server.crt","keyfile ../ssl/client.key"], port, 1) # mismatched certfile / keyfile
+do_test_broker_failure(conf_file, [f"listener {port}","certfile ./16-config-parse-errors.py","keyfile ../ssl/server.key"], port, 1, with_test_config=False) # invalid certfile
+do_test_broker_failure(conf_file, [f"listener {port}","certfile ../ssl/server.crt","keyfile ./16-config-parse-errors.py"], port, 1, with_test_config=False) # invalid keyfile
+do_test_broker_failure(conf_file, [f"listener {port}","certfile ../ssl/server.crt","keyfile ../ssl/client.key"], port, 1, with_test_config=False) # mismatched certfile / keyfile
 
-do_test_broker_failure(conf_file, [f"listener {port}","certfile ../ssl/server.crt","keyfile ../ssl/server.key","tls_version invalid"], port, 1) # invalid tls_version
+do_test_broker_failure(conf_file, [f"listener {port}","certfile ../ssl/server.crt","keyfile ../ssl/server.key","tls_version invalid"], port, 1, with_test_config=False) # invalid tls_version
 
-do_test_broker_failure(conf_file, [f"listener {port}","certfile ../ssl/server.crt","keyfile ../ssl/server.key","crlfile invalid"], port, 1) # missing crl file
-do_test_broker_failure(conf_file, [f"listener {port}","certfile ../ssl/server.crt","keyfile ../ssl/server.key","ciphers invalid"], port, 1) # invalid ciphers
-do_test_broker_failure(conf_file, [f"listener {port}","certfile ../ssl/server.crt","keyfile ../ssl/server.key","ciphers_tls1.3 invalid"], port, 1) # invalid ciphers_tls1.3
+do_test_broker_failure(conf_file, [f"listener {port}","certfile ../ssl/server.crt","keyfile ../ssl/server.key","crlfile invalid"], port, 1, with_test_config=False) # missing crl file
+do_test_broker_failure(conf_file, [f"listener {port}","certfile ../ssl/server.crt","keyfile ../ssl/server.key","ciphers invalid"], port, 1, with_test_config=False) # invalid ciphers
+do_test_broker_failure(conf_file, [f"listener {port}","certfile ../ssl/server.crt","keyfile ../ssl/server.key","ciphers_tls1.3 invalid"], port, 1, with_test_config=False) # invalid ciphers_tls1.3
 
 exit(0)

test/broker/16-config-parse-errors-without-tls.py

@@ -19,10 +19,12 @@
 do_test_broker_failure(conf_file, ["allow_anonymous falst"], port, 3, "Error: Invalid 'allow_anonymous' value (falst).") # Invalid bool
 
 do_test_broker_failure(conf_file, ["autosave_interval"], port, 3, "Error: Empty 'autosave_interval' value in configuration.") # Empty int
-#do_test_broker_failure(conf_file, ["autosave_interval string"], port, 3, "bla") # Invalid int
+do_test_broker_failure(conf_file, ["autosave_interval string"], port, 3, "Error: 'autosave_interval' value not a number.") # Invalid int
 do_test_broker_failure(conf_file, ["listener"], port, 3, "Error: Empty 'listener port' value in configuration.") # Empty listener
 do_test_broker_failure(conf_file, ["mount_point test/"], port, 3, "Error: The 'mount_point' option requires a listener to be defined first.") # Missing listener config
 do_test_broker_failure(conf_file, [f"listener {port}","mount_point test/+/"], port, 3, "Error: Invalid 'mount_point' value (test/+/). Does it contain a wildcard character?") # Wildcard in mount point.
+do_test_broker_failure(conf_file, [f"listener 100000"], port, 3, "Error: Invalid 'port' value (100000).") # Out of range
+do_test_broker_failure(conf_file, [f"listener 0"], port, 3, "Error: A listener with port 0 must provide a Unix socket path.") # Missing unix socket
 do_test_broker_failure(conf_file, [f"listener {port}","protocol"], port, 3, "Error: Empty 'protocol' value in configuration.") # Empty proto
 do_test_broker_failure(conf_file, [f"listener {port}","protocol test"], port, 3, "Error: Invalid 'protocol' value (test).") # Invalid proto
 do_test_broker_failure(conf_file, [f"listener {port}","accept_protocol_versions"], port, 3, "Error: Empty 'accept_protocol_versions' value in configuration.")
@@ -70,6 +72,9 @@
 
 do_test_broker_failure(conf_file, ["max_inflight_messages 65536"], port, 3, "Error: 'max_inflight_messages' must be <= 65535.") # Invalid value
 
+do_test_broker_failure(conf_file, ["max_packet_size 19"], port, 3, "Error: 'max_packet_size' must be >= 20.") # Invalid value
+do_test_broker_failure(conf_file, ["message_size_limit 556000000"], port, 3, "Error: Invalid 'message_size_limit' value (556000000).") # Invalid value
+
 do_test_broker_failure(conf_file, ["max_keepalive 65536"], port, 3, "Error: Invalid 'max_keepalive' value (65536).") # Invalid value
 do_test_broker_failure(conf_file, ["max_keepalive -1"], port, 3, "Error: Invalid 'max_keepalive' value (-1).") # Invalid value
 

test/broker/mosq_test_helper.py

@@ -43,10 +43,15 @@ def do_test_broker_failure(
     rc_expected: int,
     error_log_entry: str = None,
     stdout_entry: str = None,
-    cmd_args: list = None,
+    cmd_args: list = [],
+    with_test_config: bool = True,
 ):
     rc = 1
 
+    cmd_args = cmd_args.copy()
+    if with_test_config:
+        cmd_args.insert(0, "--test-config")
+
     use_conf_file = len(conf_file)
     create_conf_file = use_conf_file and len(config)
     if create_conf_file:
@@ -100,7 +105,7 @@ def do_test_broker_failure(
                 pass
         if rc:
             print(
-                f"While testing 'config {chr(10).join(config) if len(config) else ''}'{', args'+ ' '.join(cmd_args) if cmd_args is not None else ''}"
+                f"While testing 'config {chr(10).join(config) if len(config) else ''}'{', args '+ ' '.join(cmd_args) if cmd_args is not None else ''}"
             )
             exit(rc)