From a84c964d37a1899bf90c920efef85a1d7202d058 Mon Sep 17 00:00:00 2001
From: Binbin <binloveplay1314@qq.com>
Date: Sun, 9 Jan 2022 19:06:51 +0800
Subject: [PATCH] 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.
---
 src/networking.c     |  2 +-
 src/server.c         | 14 ++++++++++++--
 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