Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix LPOS rank passing LLONG_MIN overflow issue #1687

Merged
merged 1 commit into from
Aug 21, 2023

Conversation

enjoy-binbin
Copy link
Member

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.

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
```
@JoverZhang
Copy link
Contributor

Thank you.

@enjoy-binbin enjoy-binbin merged commit 54423b6 into apache:unstable Aug 21, 2023
26 checks passed
@enjoy-binbin enjoy-binbin deleted the lpos_rank_overflow branch August 21, 2023 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants