Fix crash when error [sub]command name contains | (#10082)

The following error commands will crash redis-server:
```
> get|
Error: Server closed the connection
> get|set
Error: Server closed the connection
> get|other
```

The reason is in #9504, we use `lookupCommandBySds` for find the
container command. And it split the command (argv[0]) with `|`.
If we input something like `get|other`, after the split, `get`
will become a valid command name, pass the `ERR unknown command`
check, and finally crash in `addReplySubcommandSyntaxError`

In this case we do not need to split the command name with `|`
and just look in the commands dict to find if `argv[0]` is a
container command.

So this commit introduce a new function call `isContainerCommandBySds`
that it will return true if a command name is a container command.

Also with the old code, there is a incorrect error message:
```
> config|get set
(error) ERR Unknown subcommand or wrong number of arguments for 'set'. Try CONFIG|GET HELP.
```

The crash was reported in #10070.
This commit is contained in:
Binbin 2022-01-09 19:06:51 +08:00 committed by GitHub
parent 75c50a1563
commit a84c964d37
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 22 additions and 4 deletions

View File

@ -962,7 +962,7 @@ void addReplySubcommandSyntaxError(client *c) {
sds cmd = sdsnew((char*) c->argv[0]->ptr);
sdstoupper(cmd);
addReplyErrorFormat(c,
"Unknown subcommand or wrong number of arguments for '%s'. Try %s HELP.",
"Unknown subcommand or wrong number of arguments for '%.128s'. Try %s HELP.",
(char*)c->argv[1]->ptr,cmd);
sdsfree(cmd);
}

View File

@ -2766,6 +2766,12 @@ void redisOpArrayFree(redisOpArray *oa) {
/* ====================== Commands lookup and execution ===================== */
int isContainerCommandBySds(sds s) {
struct redisCommand *base_cmd = dictFetchValue(server.commands, s);
int has_subcommands = base_cmd && base_cmd->subcommands_dict;
return has_subcommands;
}
struct redisCommand *lookupCommandLogic(dict *commands, robj **argv, int argc) {
struct redisCommand *base_cmd = dictFetchValue(commands, argv[0]->ptr);
int has_subcommands = base_cmd && base_cmd->subcommands_dict;
@ -3330,10 +3336,14 @@ int processCommand(client *c) {
* such as wrong arity, bad command name and so forth. */
c->cmd = c->lastcmd = lookupCommand(c->argv,c->argc);
if (!c->cmd) {
if (lookupCommandBySds(c->argv[0]->ptr)) {
if (isContainerCommandBySds(c->argv[0]->ptr)) {
/* If we can't find the command but argv[0] by itself is a command
* it means we're dealing with an invalid subcommand. Print Help. */
addReplySubcommandSyntaxError(c);
sds cmd = sdsnew((char *)c->argv[0]->ptr);
sdstoupper(cmd);
rejectCommandFormat(c, "Unknown subcommand '%.128s'. Try %s HELP.",
(char *)c->argv[1]->ptr, cmd);
sdsfree(cmd);
return C_OK;
}
sds args = sdsempty();

View File

@ -305,10 +305,18 @@ start_server {tags {"other"}} {
assert_equal [r acl whoami] default
} {} {needs:reset}
test "Subcommand syntax error crash (issue #10070)" {
assert_error {*unknown command*} {r GET|}
assert_error {*unknown command*} {r GET|SET}
assert_error {*unknown command*} {r GET|SET|OTHER}
assert_error {*unknown command*} {r CONFIG|GET GET_XX}
assert_error {*Unknown subcommand*} {r CONFIG GET_XX}
}
}
start_server {tags {"other external:skip"}} {
test {Don't rehash if redis has child proecess} {
test {Don't rehash if redis has child process} {
r config set save ""
r config set rdb-key-save-delay 1000000