You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "Tyagiquamar (via GitHub)" <gi...@apache.org> on 2026/10/23 03:31:37 UTC
[PR] fix(command): reject overflowing EX and EXPIRE timestamps (#3674) [kvrocks]
Tyagiquamar opened a new pull request, #3713:
URL: https://github.com/apache/kvrocks/pull/3713
### Description
Fixes #3674.
When executing commands with extreme expiry durations (such as `SET key value EX 9223372036854775807` or `EXPIRE key 9223372036854775807`), multiplying seconds by 1000 or adding to current timestamp in milliseconds overflows `int64_t`. Because of signed integer overflow wrapping to negative values, Kvrocks treated the expiry timestamp as a timestamp in the distant past (e.g. 1970 or earlier), immediately deleting the key and returning OK.
In Redis, such overflowing expire parameters return `(error) ERR invalid expire time in 'set' ('expire')`.
This PR adds overflow checks to:
- `ParseExpireFlags` in `src/commands/ttl_util.h` for `EX`, `EXAT`, and `PX` options
- `CommandSetEX` and `CommandPSetEX` in `src/commands/cmd_string.cc`
- `CommandExpire`, `CommandPExpire`, `CommandExpireAt`, and `CommandPExpireAt` in `src/commands/cmd_key.cc`
- Adds Go integration tests in `tests/gocase/unit/expire/expire_test.go` verifying that overflowing expire values reject with `invalid expire time` error and do not mutate or delete the key.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(command): reject overflowing EX and EXPIRE timestamps (#3674) [kvrocks]
Posted by "weim0000 (via GitHub)" <gi...@apache.org>.
weim0000 commented on code in PR #3713:
URL: https://github.com/apache/kvrocks/pull/3713#discussion_r4375122676
##########
src/commands/cmd_key.cc:
##########
@@ -218,20 +234,33 @@ class CommandExpire : public Commander {
}
private:
- uint64_t ttl_ = 0;
+ int64_t ttl_ = 0;
};
class CommandPExpire : public Commander {
public:
Status Parse(const std::vector<std::string> &args) override {
- seconds_ = GET_OR_RET(ParseInt<int64_t>(args[2], 10)) + util::GetTimeStampMS();
+ auto parse_result = ParseInt<int64_t>(args[2], 10);
+ if (!parse_result) {
+ return {Status::RedisParseErr, errValueNotInteger};
+ }
+ ttl_ms_ = *parse_result;
Review Comment:
Quick question: is there a specific reason for replacing GET_OR_RET with manual error checking here? GET_OR_RET already propagates the parse error automatically, so the behavior seems identical. Just curious if I'm missing something.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(command): reject overflowing EX and EXPIRE timestamps (#3674) [kvrocks]
Posted by "weim0000 (via GitHub)" <gi...@apache.org>.
weim0000 commented on code in PR #3713:
URL: https://github.com/apache/kvrocks/pull/3713#discussion_r4375111134
##########
src/commands/cmd_key.cc:
##########
@@ -218,20 +234,33 @@ class CommandExpire : public Commander {
}
private:
- uint64_t ttl_ = 0;
+ int64_t ttl_ = 0;
Review Comment:
Suggestion: consider renaming the member variable from ttl_ to expire_at_ms_, and move all parameter validation logic into Parse. This way Parse handles all checks (including the overflow check with now_ms), and Execute only contains execution logic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(command): reject overflowing EX and EXPIRE timestamps (#3674) [kvrocks]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3713:
URL: https://github.com/apache/kvrocks/pull/3713#issuecomment-6356712102
Hi @Tyagiquamar,
Thank you for your pull request. Please review our [Contributing Guide](https://kvrocks.apache.org/community/contributing/).
Please make sure you understand your changes and explain your reasoning in this pull request. Low-quality pull requests may be closed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(command): reject overflowing EX and EXPIRE timestamps (#3674) [kvrocks]
Posted by "weim0000 (via GitHub)" <gi...@apache.org>.
weim0000 commented on code in PR #3713:
URL: https://github.com/apache/kvrocks/pull/3713#discussion_r4375131353
##########
src/commands/cmd_key.cc:
##########
@@ -241,7 +270,7 @@ class CommandPExpire : public Commander {
}
private:
- uint64_t seconds_ = 0;
+ int64_t ttl_ms_ = 0;
Review Comment:
Similar to the comment on CommandExpire above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org