This is an automated email from the ASF dual-hosted git repository.
hulk pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new fdf044dc fix: better checking for prefix matches (#2599)
fdf044dc is described below
commit fdf044dc09206d0bc2f800a2f36eee01fcc9a3f7
Author: Nathan <[email protected]>
AuthorDate: Tue Oct 15 00:07:01 2024 -0400
fix: better checking for prefix matches (#2599)
---
src/commands/cmd_server.cc | 15 +++++----------
src/commands/scan_base.h | 6 ++++--
tests/gocase/unit/keyspace/keyspace_test.go | 4 ++++
tests/gocase/unit/scan/scan_test.go | 7 +++++++
4 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc
index 2f30c55e..66637f4f 100644
--- a/src/commands/cmd_server.cc
+++ b/src/commands/cmd_server.cc
@@ -114,20 +114,15 @@ class CommandNamespace : public Commander {
class CommandKeys : public Commander {
public:
Status Execute(engine::Context &ctx, Server *srv, Connection *conn,
std::string *output) override {
- std::string prefix = args_[1];
+ const std::string &prefix = args_[1];
std::vector<std::string> keys;
redis::Database redis(srv->storage, conn->GetNamespace());
- rocksdb::Status s;
- if (prefix == "*") {
- s = redis.Keys(ctx, std::string(), &keys);
- } else {
- if (prefix[prefix.size() - 1] != '*') {
- return {Status::RedisExecErr, "only keys prefix match was supported"};
- }
-
- s = redis.Keys(ctx, prefix.substr(0, prefix.size() - 1), &keys);
+ if (prefix.empty() || prefix.find('*') != prefix.size() - 1) {
+ return {Status::RedisExecErr, "only keys prefix match was supported"};
}
+
+ const rocksdb::Status s = redis.Keys(ctx, prefix.substr(0, prefix.size() -
1), &keys);
if (!s.ok()) {
return {Status::RedisExecErr, s.ToString()};
}
diff --git a/src/commands/scan_base.h b/src/commands/scan_base.h
index 007e49ff..b3773b94 100644
--- a/src/commands/scan_base.h
+++ b/src/commands/scan_base.h
@@ -45,8 +45,10 @@ class CommandScanBase : public Commander {
while (parser.Good()) {
if (parser.EatEqICase("match")) {
prefix_ = GET_OR_RET(parser.TakeStr());
- if (!prefix_.empty() && prefix_.back() == '*') {
- prefix_ = prefix_.substr(0, prefix_.size() - 1);
+ // The match pattern should contain exactly one '*' at the end; remove
the * to
+ // get the prefix to match.
+ if (!prefix_.empty() && prefix_.find('*') == prefix_.size() - 1) {
+ prefix_.pop_back();
} else {
return {Status::RedisParseErr, "currently only key prefix matching
is supported"};
}
diff --git a/tests/gocase/unit/keyspace/keyspace_test.go
b/tests/gocase/unit/keyspace/keyspace_test.go
index 704d4475..6fbb84a7 100644
--- a/tests/gocase/unit/keyspace/keyspace_test.go
+++ b/tests/gocase/unit/keyspace/keyspace_test.go
@@ -65,6 +65,10 @@ func TestKeyspace(t *testing.T) {
require.Equal(t, []string{"foo_a", "foo_b", "foo_c"}, keys)
})
+ t.Run("KEYS with invalid pattern", func(t *testing.T) {
+ require.Error(t, rdb.Keys(ctx, "*ab*").Err())
+ })
+
t.Run("KEYS to get all keys", func(t *testing.T) {
keys := rdb.Keys(ctx, "*").Val()
sort.Slice(keys, func(i, j int) bool {
diff --git a/tests/gocase/unit/scan/scan_test.go
b/tests/gocase/unit/scan/scan_test.go
index 62b91aff..cade5dcc 100644
--- a/tests/gocase/unit/scan/scan_test.go
+++ b/tests/gocase/unit/scan/scan_test.go
@@ -97,6 +97,13 @@ func ScanTest(t *testing.T, rdb *redis.Client, ctx
context.Context) {
require.Len(t, keys, 1000)
})
+ t.Run("SCAN MATCH invalid pattern", func(t *testing.T) {
+ require.NoError(t, rdb.FlushDB(ctx).Err())
+ util.Populate(t, rdb, "*ab", 1000, 10)
+ // SCAN MATCH with invalid pattern should return an error
+ require.Error(t, rdb.Do(context.Background(), "SCAN", "match",
"*ab*").Err())
+ })
+
t.Run("SCAN guarantees check under write load", func(t *testing.T) {
require.NoError(t, rdb.FlushDB(ctx).Err())
util.Populate(t, rdb, "", 100, 10)
