Fix RESTORE with TTL (#14071)

restoreCommand() creates a key-value object (kv) with a TTL in two steps.
During the second step, setExpire() may reallocate the kv object. To ensure
correct behavior, kv must be updated after this call, as it might be used later
in the function.
This commit is contained in:
Moti Cohen 2025-05-28 08:02:10 +03:00 committed by GitHub
parent 0ac822e154
commit 79b37ff535
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 24 additions and 7 deletions

View file

@ -250,7 +250,7 @@ void restoreCommand(client *c) {
}
if (ttl) {
setExpire(c,c->db,key,ttl);
kv = setExpire(c,c->db,key,ttl); /* might realloc kvobj */
if (!absttl) {
/* Propagate TTL as absolute timestamp */
robj *ttl_obj = createStringObjectFromLongLong(ttl);

View file

@ -1818,7 +1818,7 @@ void renameGenericCommand(client *c, int nx) {
dbDelete(c->db,c->argv[1]);
dbAdd(c->db, c->argv[2], &o);
if (expire != -1) setExpire(c, c->db, c->argv[2], expire);
if (expire != -1) o = setExpire(c, c->db, c->argv[2], expire);
/* If hash with HFEs, register in db->hexpires */
if (minHashExpireTime != EB_EXPIRE_TIME_INVALID)
@ -2021,7 +2021,7 @@ void copyCommand(client *c) {
/* if key with expiration then set it */
if (expire != -1)
setExpire(c, dst, newkey, expire);
newobj = setExpire(c, dst, newkey, expire);
/* If minExpiredField was set, then the object is hash with expiration
* on fields and need to register it in global HFE DS */
@ -2240,7 +2240,9 @@ int removeExpire(redisDb *db, robj *key) {
/* Set an expire to the specified key. If the expire is set in the context
* of an user calling a command 'c' is the client, otherwise 'c' is set
* to NULL. The 'when' parameter is the absolute unix time in milliseconds
* after which the key will no longer be considered valid. */
* after which the key will no longer be considered valid.
*
* Note: It may reallocate kvobj. The returned ref may point to a new object. */
kvobj *setExpire(client *c, redisDb *db, robj *key, long long when) {
return setExpireByLink(c,db,key->ptr,when,NULL);
}

View file

@ -719,7 +719,7 @@ void expireGenericCommand(client *c, long long basetime, int unit) {
addReply(c, shared.cone);
return;
} else {
setExpire(c,c->db,key,when);
kv = setExpire(c,c->db,key,when); /* might realloc kv */
addReply(c,shared.cone);
/* Propagate as PEXPIREAT millisecond-timestamp
* Only rewrite the command arg if not already PEXPIREAT */

View file

@ -4269,7 +4269,8 @@ int RM_SetExpire(RedisModuleKey *key, mstime_t expire) {
return REDISMODULE_ERR;
if (expire != REDISMODULE_NO_EXPIRE) {
expire += commandTimeSnapshot();
setExpire(key->ctx->client,key->db,key->key,expire);
/* setExpire() might realloc kvobj */
key->kv = setExpire(key->ctx->client,key->db,key->key,expire);
} else {
removeExpire(key->db,key->key);
}

View file

@ -397,7 +397,7 @@ void getexCommand(client *c) {
notifyKeyspaceEvent(NOTIFY_GENERIC, "del", c->argv[1], c->db->id);
server.dirty++;
} else if (expire) {
setExpire(c,c->db,c->argv[1],milliseconds);
o = setExpire(c,c->db,c->argv[1],milliseconds);
/* Propagate as PXEXPIREAT millisecond-timestamp if there is
* EX/PX/EXAT/PXAT flag and the key has not expired. */
robj *milliseconds_obj = createStringObjectFromLongLong(milliseconds);

View file

@ -59,6 +59,20 @@ start_server {tags {"dump"}} {
assert_equal [r get foo] {bar}
r config set maxmemory-policy noeviction
} {OK} {needs:config-maxmemory}
test {RESTORE with TTL maintain valid object} {
# RESTORE Creates a string with TTL in two steps. The second step potentially
# reallocates the object. Access the object and verify it is not corrupted
r del foo
r set foo bar
set encoded [r dump foo]
# Iterate several times and verify it is consistent
for {set i 0} {$i < 100} {incr i} {
r del foo
r restore foo 1000 $encoded IDLETIME 500
assert_equal [r get foo] {bar}
}
}
test {RESTORE can set LFU} {
r set foo bar