summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBinbin <binloveplay1314@qq.com>2022-01-09 19:06:51 +0800
committerGitHub <noreply@github.com>2022-01-09 13:06:51 +0200
commita84c964d37a1899bf90c920efef85a1d7202d058 (patch)
tree47c816699b2764987bc92677adf3be90ce3422ff
parent75c50a15633881bb2bf0455bdabcbbabc0e47044 (diff)
downloadredis-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.c2
-rw-r--r--src/server.c14
-rw-r--r--tests/unit/other.tcl10
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