Skip to content

Commit

Permalink
Make dir/dbfilename/cluster-config-file reject empty string
Browse files Browse the repository at this point in the history
Currently these configuration items allow 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 committed Jun 12, 2024
1 parent e65b2d2 commit b2c9390
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 b2c9390

Please sign in to comment.