Skip to content

Commit

Permalink
Make configs dir/dbfilename/cluster-config-file reject empty string (#…
Browse files Browse the repository at this point in the history
…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 <[email protected]>
  • Loading branch information
enjoy-binbin authored Jun 13, 2024
1 parent 76fc041 commit d309b9b
Showing 1 changed file with 17 additions and 1 deletion.
18 changes: 17 additions & 1 deletion src/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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),
Expand Down

0 comments on commit d309b9b

Please sign in to comment.