Fix inconsistent response format with Redis (#1269)

- For the type command, should return the simple string instead of the bulk string - For the scan command, should return the empty string key instead of the nil string
apache · Feb 20, 2023 · 6a9a88c · 6a9a88c
1 parent a08a0ff
commit 6a9a88c
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 3 deletions.
diff --git a/src/commands/cmd_key.cc b/src/commands/cmd_key.cc
@@ -34,7 +34,7 @@ class CommandType : public Commander {
     RedisType type = kRedisNone;
     auto s = redis.Type(args_[1], &type);
     if (s.ok()) {
-      *output = Redis::BulkString(RedisTypeNames[type]);
+      *output = Redis::SimpleString(RedisTypeNames[type]);
       return Status::OK();
     }
 

diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc
@@ -737,7 +737,7 @@ class CommandScan : public CommandScanBase {
       list.emplace_back(Redis::BulkString("0"));
     }
 
-    list.emplace_back(Redis::MultiBulkString(keys));
+    list.emplace_back(Redis::MultiBulkString(keys, false));
 
     return Redis::Array(list);
   }

diff --git a/src/commands/scan_base.h b/src/commands/scan_base.h
@@ -71,7 +71,7 @@ class CommandScanBase : public Commander {
       list.emplace_back(Redis::BulkString("0"));
     }
 
-    list.emplace_back(Redis::MultiBulkString(keys));
+    list.emplace_back(Redis::MultiBulkString(keys, false));
 
     return Redis::Array(list);
   }

diff --git a/tests/gocase/unit/protocol/protocol_test.go b/tests/gocase/unit/protocol/protocol_test.go
@@ -117,4 +117,13 @@ func TestProtocolNetwork(t *testing.T) {
 		require.NoError(t, c.Write("*3\n$3\r\nset\r\n$3\r\nkey\r\n$3\r\nval\r\n"))
 		c.MustMatch(t, "invalid multibulk length")
 	})
+
+	t.Run("command type should return the simple string", func(t *testing.T) {
+		c := srv.NewTCPClient()
+		defer func() { require.NoError(t, c.Close()) }()
+		require.NoError(t, c.Write("set foo bar\n"))
+		c.MustRead(t, "+OK")
+		require.NoError(t, c.Write("type foo\n"))
+		c.MustRead(t, "+string")
+	})
 }
diff --git a/tests/gocase/unit/scan/scan_test.go b/tests/gocase/unit/scan/scan_test.go
@@ -32,6 +32,26 @@ import (
 	"golang.org/x/exp/slices"
 )
 
+func TestScanEmptyKey(t *testing.T) {
+	srv := util.StartServer(t, map[string]string{})
+	defer srv.Close()
+
+	ctx := context.Background()
+	rdb := srv.NewClient()
+	defer func() { require.NoError(t, rdb.Close()) }()
+
+	require.NoError(t, rdb.Set(ctx, "", "empty", 0).Err())
+	require.NoError(t, rdb.Set(ctx, "foo", "bar", 0).Err())
+	require.Equal(t, []string{"", "foo"}, scanAll(t, rdb))
+
+	require.NoError(t, rdb.SAdd(ctx, "sadd_key", "", "fab", "fiz", "foobar").Err())
+	keys, _, err := rdb.SScan(ctx, "sadd_key", 0, "*", 10000).Result()
+	require.NoError(t, err)
+	slices.Sort(keys)
+	slices.Compact(keys)
+	require.Equal(t, []string{"", "fab", "fiz", "foobar"}, keys)
+}
+
 func TestScan(t *testing.T) {
 	srv := util.StartServer(t, map[string]string{})
 	defer srv.Close()