This is an automated email from the ASF dual-hosted git repository.
binbin 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 54423b6a Fix LPOS rank passing LLONG_MIN overflow issue (#1687)
54423b6a is described below
commit 54423b6a545ccdb849e2a6b038cbc559f58e3d4a
Author: Binbin <[email protected]>
AuthorDate: Mon Aug 21 17:30:55 2023 +0800
Fix LPOS rank passing LLONG_MIN overflow issue (#1687)
There is a minor overflow issue in RANK negation, passing LLONG_MIN
will overflow and is effectively be the same as passing -1.
This is the example before the fix:
```
127.0.0.1:6666> flushall
OK
127.0.0.1:6666> lpos mylist foo rank -9223372036854775808
(nil)
127.0.0.1:6666> lpush mylist foo foo foo foo foo
(integer) 5
127.0.0.1:6666> lpos mylist foo rank -1
(integer) 4
127.0.0.1:6666> lpos mylist foo rank -5
(integer) 0
127.0.0.1:6666> lpos mylist foo rank -6
(nil)
127.0.0.1:6666> lpos mylist foo rank -9223372036854775807
(nil)
-- this should return nil but it returned the last one because the overflow
rank became -1
127.0.0.1:6666> lpos mylist foo rank -9223372036854775808
(integer) 4
```
Now we limit RANK to not be LLONG_MIN and will throw an error directly
(this is the behavior of Redis 7.2, but with different error words):
```
127.0.0.1:6666> lpos mylist foo rank -9223372036854775808
(error) ERR rank would overflow
127.0.0.1:6379> lpos mylist foo rank -9223372036854775808
(error) ERR value is out of range, value must between -9223372036854775807
and 9223372036854775807
```
Unrelated change: a small cleanup, return RedisParseErr instead of
RedisExecErr in Parse.
---
src/commands/cmd_list.cc | 7 +++++--
tests/gocase/unit/type/list/list_test.go | 5 +++++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/commands/cmd_list.cc b/src/commands/cmd_list.cc
index 60621fdf..1ad581a6 100644
--- a/src/commands/cmd_list.cc
+++ b/src/commands/cmd_list.cc
@@ -697,16 +697,19 @@ class CommandLPos : public Commander {
"RANK can't be zero: use 1 to start from "
"the first match, 2 from the second ... "
"or use negative to start from the end of the list"};
+ } else if (spec_.rank == LLONG_MIN) {
+ // Negating LLONG_MIN will cause an overflow, and is effectively be
the same as passing -1.
+ return {Status::RedisParseErr, "rank would overflow"};
}
} else if (parser.EatEqICase("count")) {
spec_.count = GET_OR_RET(parser.TakeInt());
if (spec_.count < 0) {
- return {Status::RedisExecErr, "COUNT can't be negative"};
+ return {Status::RedisParseErr, "COUNT can't be negative"};
}
} else if (parser.EatEqICase("maxlen")) {
spec_.max_len = GET_OR_RET(parser.TakeInt());
if (spec_.max_len < 0) {
- return {Status::RedisExecErr, "MAXLEN can't be negative"};
+ return {Status::RedisParseErr, "MAXLEN can't be negative"};
}
} else {
return {Status::RedisParseErr, errInvalidSyntax};
diff --git a/tests/gocase/unit/type/list/list_test.go
b/tests/gocase/unit/type/list/list_test.go
index 2cf2e317..b7db17e7 100644
--- a/tests/gocase/unit/type/list/list_test.go
+++ b/tests/gocase/unit/type/list/list_test.go
@@ -942,6 +942,11 @@ func TestList(t *testing.T) {
require.Equal(t, "bar", rdb.LRange(ctx, "target", 0,
-1).Val()[0])
})
+ t.Run("LPOS rank negation overflow", func(t *testing.T) {
+ require.NoError(t, rdb.Del(ctx, "mylist").Err())
+ util.ErrorRegexp(t, rdb.Do(ctx, "LPOS", "mylist", "foo",
"RANK", "-9223372036854775808").Err(), ".*rank would overflow.*")
+ })
+
for listType, large := range largeValue {
t.Run(fmt.Sprintf("LPOS basic usage - %s", listType), func(t
*testing.T) {
createList("mylist", []string{"a", "b", "c", large,
"2", "3", "c", "c"})
