Skip to content

Add tests to cover EXPIRE overflow detection fix#9839

Merged
oranagra merged 1 commit intoredis:unstablefrom
enjoy-binbin:add_expire_test
Nov 24, 2021
Merged

Add tests to cover EXPIRE overflow detection fix#9839
oranagra merged 1 commit intoredis:unstablefrom
enjoy-binbin:add_expire_test

Conversation

@enjoy-binbin
Copy link
Contributor

In #8287, some overflow checks have been added. But when
when *= 1000 overflows, it will become a positive number.
And the check not able to catch it. The key will be added with
a short expiration time and will deleted a few seconds later.

In #9601, will check the overflow after *= and return an
error first, and avoiding this situation.

In this commit, added some tests to cover those code paths.
Found it in #9825, and close it.

In redis#8287, some overflow checks have been added. But when
`when *= 1000` overflows, it will become a positive number.
And the check not able to catch it. The key will be added with
a short expiration time and will deleted a few seconds later.

In redis#9601, will check the overflow after `*=` and return an
error first, and avoiding this situation.

In this commit, added some tests to cover those code paths.
Found it in redis#9825, and close it.
@oranagra oranagra linked an issue Nov 24, 2021 that may be closed by this pull request
@oranagra oranagra changed the title Add tests to cover EXPIRE overflow Add tests to cover EXPIRE overflow detection fix Nov 24, 2021
@oranagra oranagra added the release-notes indication that this issue needs to be mentioned in the release notes label Nov 24, 2021
@oranagra
Copy link
Member

@enjoy-binbin did you verify that this test fails on the code before the fix (v6.2)?

@enjoy-binbin
Copy link
Contributor Author

Actually only this r EXPIRE foo 18446744073709561 will fail, in 6.2.6

My words is a bit unclear, when *= overflow can be negative or integer, for example:

expire foor 18446744063600560 -> `when *=` will became -10108991616
expire foor 18446744073709561 -> `when *=` will became 9384

In old code, if when *= 1000 overflow and became a small negative number, the when += basetime will correct it to be positive

    if (unit == UNIT_SECONDS) when *= 1000;
    when += basetime;

@oranagra
Copy link
Member

ok, great, i assumed only one of them is the one who fails on 6.2, but it's good you added all of them.

@oranagra oranagra merged commit 9273d09 into redis:unstable Nov 24, 2021
@enjoy-binbin enjoy-binbin deleted the add_expire_test branch November 24, 2021 07:48
hwware pushed a commit to hwware/redis that referenced this pull request Dec 20, 2021
In redis#8287, some overflow checks have been added. But when
`when *= 1000` overflows, it will become a positive number.
And the check not able to catch it. The key will be added with
a short expiration time and will deleted a few seconds later.

In redis#9601, will check the overflow after `*=` and return an
error first, and avoiding this situation.

In this commit, added some tests to cover those code paths.
Found it in redis#9825, and close it.
@oranagra oranagra added the breaking-change This change can potentially break existing application label Mar 22, 2022
This was referenced Apr 27, 2022
oranagra pushed a commit that referenced this pull request Apr 27, 2022
In #8287, some overflow checks have been added. But when
`when *= 1000` overflows, it will become a positive number.
And the check not able to catch it. The key will be added with
a short expiration time and will deleted a few seconds later.

In #9601, will check the overflow after `*=` and return an
error first, and avoiding this situation.

In this commit, added some tests to cover those code paths.
Found it in #9825, and close it.

(cherry picked from commit 9273d09)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change This change can potentially break existing application release-notes indication that this issue needs to be mentioned in the release notes

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[BUG] EXPIRE and SETEX have inconsistent overflow handling

2 participants