diff options
author | Binbin <binloveplay1314@qq.com> | 2022-01-09 19:06:51 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-01-09 13:06:51 +0200 |
commit | a84c964d37a1899bf90c920efef85a1d7202d058 (patch) | |
tree | 47c816699b2764987bc92677adf3be90ce3422ff | |
parent | 75c50a15633881bb2bf0455bdabcbbabc0e47044 (diff) | |
download | redis-a84c964d37a1899bf90c920efef85a1d7202d058.tar.gz |
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.
-rw-r--r-- | src/networking.c | 2 | ||||
-rw-r--r-- | src/server.c | 14 | ||||
-rw-r--r-- | tests/unit/other.tcl | 10 |
3 files changed, 22 insertions, 4 deletions
diff --git a/src/networking.c b/src/networking.c index 50c7d99ca..48f248020 100644 --- a/src/networking.c +++ b/src/networking.c @@ -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); } diff --git a/src/server.c b/src/server.c index a1d0f1cc5..46b454ff3 100644 --- a/src/server.c +++ b/src/server.c @@ -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(); diff --git a/tests/unit/other.tcl b/tests/unit/other.tcl index 6bf451279..f4d540fcf 100644 --- a/tests/unit/other.tcl +++ b/tests/unit/other.tcl @@ -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 |