diff --git a/src/config.c b/src/config.c index 04f0dbcd8f..adc708d247 100644 --- a/src/config.c +++ b/src/config.c @@ -487,9 +487,23 @@ void loadServerConfigFromString(char *config) { err = "wrong number of arguments"; goto loaderr; } - /* Set config using all arguments that follows */ - if (!config->interface.set(config, &argv[1], argc-1, &err)) { - goto loaderr; + + if ((config->flags & MULTI_ARG_CONFIG) && argc == 2 && sdslen(argv[1])) { + /* For MULTI_ARG_CONFIGs, if we only have one argument, try to split it by spaces. + * Only if the argument is not empty, otherwise something like --save "" will fail. + * So that we can support something like --config "arg1 arg2 arg3". */ + sds *new_argv; + int new_argc; + new_argv = sdssplitargs(argv[1], &new_argc); + if (!config->interface.set(config, new_argv, new_argc, &err)) { + goto loaderr; + } + sdsfreesplitres(new_argv, new_argc); + } else { + /* Set config using all arguments that follows */ + if (!config->interface.set(config, &argv[1], argc-1, &err)) { + goto loaderr; + } } sdsfreesplitres(argv,argc); diff --git a/src/server.c b/src/server.c index 12aec11fca..01914dc507 100644 --- a/src/server.c +++ b/src/server.c @@ -6959,6 +6959,7 @@ int main(int argc, char **argv) { server.exec_argv[1] = zstrdup(server.configfile); j = 2; // Skip this arg when parsing options } + int handled_last_config_arg = 1; while(j < argc) { /* Either first or last argument - Should we read config from stdin? */ if (argv[j][0] == '-' && argv[j][1] == '\0' && (j == 1 || j == argc-1)) { @@ -6967,16 +6968,20 @@ int main(int argc, char **argv) { /* All the other options are parsed and conceptually appended to the * configuration file. For instance --port 6380 will generate the * string "port 6380\n" to be parsed after the actual config file - * and stdin input are parsed (if they exist). */ - else if (argv[j][0] == '-' && argv[j][1] == '-') { + * and stdin input are parsed (if they exist). + * Only consider that if the last config has at least one argument. */ + else if (handled_last_config_arg && argv[j][0] == '-' && argv[j][1] == '-') { /* Option name */ if (sdslen(options)) options = sdscat(options,"\n"); + /* argv[j]+2 for removing the preceding `--` */ options = sdscat(options,argv[j]+2); options = sdscat(options," "); + handled_last_config_arg = 0; } else { /* Option argument */ options = sdscatrepr(options,argv[j],strlen(argv[j])); options = sdscat(options," "); + handled_last_config_arg = 1; } j++; } diff --git a/tests/unit/introspection.tcl b/tests/unit/introspection.tcl index 41d20e20a6..3a11a7027e 100644 --- a/tests/unit/introspection.tcl +++ b/tests/unit/introspection.tcl @@ -464,6 +464,70 @@ start_server {tags {"introspection"}} { assert {[dict exists $res bind]} } + test {redis-server command line arguments - error cases} { + catch {exec src/redis-server --port} err + assert_match {*'port'*wrong number of arguments*} $err + + catch {exec src/redis-server --port 6380 --loglevel} err + assert_match {*'loglevel'*wrong number of arguments*} $err + + # Take `6379` and `6380` as the port option value. + catch {exec src/redis-server --port 6379 6380} err + assert_match {*'port "6379" "6380"'*wrong number of arguments*} $err + + # Take `--loglevel` and `verbose` as the port option value. + catch {exec src/redis-server --port --loglevel verbose} err + assert_match {*'port "--loglevel" "verbose"'*wrong number of arguments*} $err + + # Take `--bla` as the port option value. + catch {exec src/redis-server --port --bla --loglevel verbose} err + assert_match {*'port "--bla"'*argument couldn't be parsed into an integer*} $err + + # Take `--bla` as the loglevel option value. + catch {exec src/redis-server --logfile --my--log--file --loglevel --bla} err + assert_match {*'loglevel "--bla"'*argument(s) must be one of the following*} $err + + # Using MULTI_ARG's own check, empty option value + catch {exec src/redis-server --shutdown-on-sigint} err + assert_match {*'shutdown-on-sigint'*argument(s) must be one of the following*} $err + catch {exec src/redis-server --shutdown-on-sigint "now force" --shutdown-on-sigterm} err + assert_match {*'shutdown-on-sigterm'*argument(s) must be one of the following*} $err + + # Something like `redis-server --some-config --config-value1 --config-value2 --loglevel debug` would break, + # because if you want to pass a value to a config starting with `--`, it can only be a single value. + catch {exec src/redis-server --replicaof 127.0.0.1 abc} err + assert_match {*'replicaof "127.0.0.1" "abc"'*Invalid master port*} $err + catch {exec src/redis-server --replicaof --127.0.0.1 abc} err + assert_match {*'replicaof "--127.0.0.1" "abc"'*Invalid master port*} $err + catch {exec src/redis-server --replicaof --127.0.0.1 --abc} err + assert_match {*'replicaof "--127.0.0.1"'*wrong number of arguments*} $err + } {} {external:skip} + + test {redis-server command line arguments - allow option value to use the `--` prefix} { + start_server {config "default.conf" args {--proc-title-template --my--title--template --loglevel verbose}} { + assert_match [r config get proc-title-template] {proc-title-template --my--title--template} + assert_match [r config get loglevel] {loglevel verbose} + } + } {} {external:skip} + + test {redis-server command line arguments - save with empty input} { + # Take `--loglevel` as the save option value. + catch {exec src/redis-server --save --loglevel verbose} err + assert_match {*'save "--loglevel" "verbose"'*Invalid save parameters*} $err + + start_server {config "default.conf" args {--save {} --loglevel verbose}} { + assert_match [r config get save] {save {}} + assert_match [r config get loglevel] {loglevel verbose} + } + } {} {external:skip} + + test {redis-server command line arguments - take one bulk string with spaces for MULTI_ARG configs parsing} { + start_server {config "default.conf" args {--shutdown-on-sigint nosave force now --shutdown-on-sigterm "nosave force"}} { + assert_match [r config get shutdown-on-sigint] {shutdown-on-sigint {nosave now force}} + assert_match [r config get shutdown-on-sigterm] {shutdown-on-sigterm {nosave force}} + } + } {} {external:skip} + # Config file at this point is at a weird state, and includes all # known keywords. Might be a good idea to avoid adding tests here. }