From d309b9b23550cd64621bf62f42504bf23838ab15 Mon Sep 17 00:00:00 2001 From: Binbin Date: Fri, 14 Jun 2024 07:47:20 +0800 Subject: [PATCH] Make configs dir/dbfilename/cluster-config-file reject empty string (#636) Until now, these configuration items allowed typing empty strings, but empty strings behave strangely. Empty dir will fail in chdir with No such file or directory: ``` ./src/valkey-server --dir "" *** FATAL CONFIG FILE ERROR (Version 255.255.255) *** Reading the configuration file, at line 2 >>> 'dir ""' No such file or directory ``` Empty dbfilename will cause shutdown to fail since it will always fail in rdb save: ``` ./src/valkey-server --dbfilename "" * User requested shutdown... * Saving the final RDB snapshot before exiting. # Error moving temp DB file temp-19530.rdb on the final destination (in server root dir /xxx/xxx/valkey): No such file or directory # Error trying to save the DB, can't exit. # Errors trying to shut down the server. Check the logs for more information. ``` Empty cluster-config-file will fail in clusterLockConfig: ``` ./src/valkey-server --cluster-enabled yes --cluster-config-file "" Can't open in order to acquire a lock: No such file or directory ``` With this patch, now we will just reject it in config set like: ``` *** FATAL CONFIG FILE ERROR (Version 255.255.255) *** Reading the configuration file, at line 2 >>> 'xxx ""' xxx can't be empty ``` Signed-off-by: Binbin --- src/config.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/config.c b/src/config.c index a609a8f18d..83e2a51db1 100644 --- a/src/config.c +++ b/src/config.c @@ -2295,7 +2295,19 @@ static int isValidActiveDefrag(int val, const char **err) { return 1; } +static int isValidClusterConfigFile(char *val, const char **err) { + if (!strcmp(val, "")) { + *err = "cluster-config-file can't be empty"; + return 0; + } + return 1; +} + static int isValidDBfilename(char *val, const char **err) { + if (!strcmp(val, "")) { + *err = "dbfilename can't be empty"; + return 0; + } if (!pathIsBaseName(val)) { *err = "dbfilename can't be a path, just a filename"; return 0; @@ -2658,6 +2670,10 @@ static int setConfigDirOption(standardConfig *config, sds *argv, int argc, const *err = "wrong number of arguments"; return 0; } + if (!strcmp(argv[0], "")) { + *err = "dir can't be empty"; + return 0; + } if (chdir(argv[0]) == -1) { *err = strerror(errno); return 0; @@ -3061,7 +3077,7 @@ standardConfig static_configs[] = { createStringConfig("replica-announce-ip", "slave-announce-ip", MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.replica_announce_ip, NULL, NULL, NULL), createStringConfig("primaryuser", "masteruser", MODIFIABLE_CONFIG | SENSITIVE_CONFIG, EMPTY_STRING_IS_NULL, server.primary_user, NULL, NULL, NULL), createStringConfig("cluster-announce-ip", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.cluster_announce_ip, NULL, NULL, updateClusterIp), - createStringConfig("cluster-config-file", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.cluster_configfile, "nodes.conf", NULL, NULL), + createStringConfig("cluster-config-file", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.cluster_configfile, "nodes.conf", isValidClusterConfigFile, NULL), createStringConfig("cluster-announce-hostname", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.cluster_announce_hostname, NULL, isValidAnnouncedHostname, updateClusterHostname), createStringConfig("cluster-announce-human-nodename", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.cluster_announce_human_nodename, NULL, isValidAnnouncedNodename, updateClusterHumanNodename), createStringConfig("syslog-ident", NULL, IMMUTABLE_CONFIG, ALLOW_EMPTY_STRING, server.syslog_ident, SERVER_NAME, NULL, NULL),