redis-server command line arguments allow passing config name and value in the same arg (#10866)

This commit has two topics.

## Passing config name and value in the same arg
In #10660 (Redis 7.0.1), when we supported the config values that can start with `--` prefix (one of the two topics of that PR),
we broke another pattern: `redis-server redis.config "name value"`, passing both config name
and it's value in the same arg, see #10865

This wasn't a intended change (i.e we didn't realize this pattern used to work).
Although this is a wrong usage, we still like to fix it.

Now we support something like:
```
src/redis-server redis.conf "--maxmemory '700mb'" "--maxmemory-policy volatile-lru" --proc-title-template --my--title--template --loglevel verbose
```

## Changes around --save
Also in this PR, we undo the breaking change we made in #10660 on purpose.
1. `redis-server redis.conf --save --loglevel verbose` (missing `save` argument before anotehr argument).
    In 7.0.1, it was throwing an wrong arg error.
    Now it will work and reset the save, similar to how it used to be in 7.0.0 and 6.2.x.
3. `redis-server redis.conf --loglevel verbose --save` (missing `save` argument as last argument).
    In 6.2, it did not reset the save, which was a bug (inconsistent with the previous bullet).
    Now we will make it work and reset the save as well (a bug fix).
This commit is contained in:
Binbin 2022-06-26 19:36:39 +08:00 committed by GitHub
parent 6272ca609e
commit d443e312ad
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 76 additions and 4 deletions

View File

@ -6881,6 +6881,8 @@ int main(int argc, char **argv) {
server.exec_argv[1] = zstrdup(server.configfile);
j = 2; // Skip this arg when parsing options
}
sds *argv_tmp;
int argc_tmp;
int handled_last_config_arg = 1;
while(j < argc) {
/* Either first or last argument - Should we read config from stdin? */
@ -6898,7 +6900,37 @@ int main(int argc, char **argv) {
/* argv[j]+2 for removing the preceding `--` */
options = sdscat(options,argv[j]+2);
options = sdscat(options," ");
handled_last_config_arg = 0;
argv_tmp = sdssplitargs(argv[j], &argc_tmp);
if (argc_tmp == 1) {
/* Means that we only have one option name, like --port or "--port " */
handled_last_config_arg = 0;
if ((j != argc-1) && argv[j+1][0] == '-' && argv[j+1][1] == '-' &&
!strcasecmp(argv[j], "--save"))
{
/* Special case: handle some things like `--save --config value`.
* In this case, if next argument starts with `--`, we will reset
* handled_last_config_arg flag and append an empty "" config value
* to the options, so it will become `--save "" --config value`.
* We are doing it to be compatible with pre 7.0 behavior (which we
* break it in #10660, 7.0.1), since there might be users who generate
* a command line from an array and when it's empty that's what they produce. */
options = sdscat(options, "\"\"");
handled_last_config_arg = 1;
}
else if ((j == argc-1) && !strcasecmp(argv[j], "--save")) {
/* Special case: when empty save is the last argument.
* In this case, we append an empty "" config value to the options,
* so it will become `--save ""` and will follow the same reset thing. */
options = sdscat(options, "\"\"");
}
} else {
/* Means that we are passing both config name and it's value in the same arg,
* like "--port 6380", so we need to reset handled_last_config_arg flag. */
handled_last_config_arg = 1;
}
sdsfreesplitres(argv_tmp, argc_tmp);
} else {
/* Option argument */
options = sdscatrepr(options,argv[j],strlen(argv[j]));

View File

@ -503,6 +503,21 @@ start_server {tags {"introspection"}} {
assert_match {*'replicaof "--127.0.0.1"'*wrong number of arguments*} $err
} {} {external:skip}
test {redis-server command line arguments - allow passing option name and option value in the same arg} {
start_server {config "default.conf" args {"--maxmemory 700mb" "--maxmemory-policy volatile-lru"}} {
assert_match [r config get maxmemory] {maxmemory 734003200}
assert_match [r config get maxmemory-policy] {maxmemory-policy volatile-lru}
}
} {} {external:skip}
test {redis-server command line arguments - wrong usage that we support anyway} {
start_server {config "default.conf" args {loglevel verbose "--maxmemory '700mb'" "--maxmemory-policy 'volatile-lru'"}} {
assert_match [r config get loglevel] {loglevel verbose}
assert_match [r config get maxmemory] {maxmemory 734003200}
assert_match [r config get maxmemory-policy] {maxmemory-policy volatile-lru}
}
} {} {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}
@ -510,15 +525,40 @@ start_server {tags {"introspection"}} {
}
} {} {external:skip}
test {redis-server command line arguments - option name and option value in the same arg and `--` 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}
}
start_server {config "default.conf" args {--loglevel verbose --save}} {
assert_match [r config get save] {save {}}
assert_match [r config get loglevel] {loglevel verbose}
}
start_server {config "default.conf" args {--save {} --loglevel verbose}} {
assert_match [r config get save] {save {}}
assert_match [r config get loglevel] {loglevel verbose}
}
start_server {config "default.conf" args {--loglevel verbose --save {}}} {
assert_match [r config get save] {save {}}
assert_match [r config get loglevel] {loglevel verbose}
}
start_server {config "default.conf" args {--proc-title-template --save --save {} --loglevel verbose}} {
assert_match [r config get proc-title-template] {proc-title-template --save}
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} {