SET overwriting a non-string key stores an empty value on log replay / remote-cluster / MULTI / Lua paths

[liunyl]

SetCommand::CommitOn commits the value twice in the branch that overwrites a non-string key with a string. Because CommitSet moves the value when the source EloqString is owned (which is exactly the replay / remote-cluster / cloned-command case), the first call empties value_ and the second stores "".

Evidence

src/redis_command.cpp:1417-1435 (SetCommand::CommitOn, non-string-object branch):

else {  // existing object is not string type
    std::unique_ptr<...> new_obj_uptr;
    if (obj_expire_ts_ != UINT64_MAX) {
        new_obj_uptr = std::make_unique<RedisStringTTLObject>();
        auto &str_obj = static_cast<RedisStringTTLObject &>(*new_obj_uptr);
        str_obj.CommitSet(value_);           // commit #1
        new_obj_uptr->SetTTL(obj_expire_ts_);
    } else {
        new_obj_uptr = StringCommand::CreateObject(nullptr);
        auto &str_obj = static_cast<RedisStringObject &>(*new_obj_uptr);
        str_obj.CommitSet(value_);           // commit #1
    }
    auto &str_obj = static_cast<RedisStringObject &>(*new_obj_uptr);
    str_obj.CommitSet(value_);               // commit #2 — redundant + harmful
    return ...;
}

src/redis_string_object.cpp:442-454 — CommitSet moves when the value is not a View:

void RedisStringObject::CommitSet(EloqString &val) {
    if (val.Type() == StorageType::View) { str_obj_ = val.Clone(); }   // normal client cmd: copy
    else { str_obj_ = std::move(val); }                                // replay/cloned cmd: MOVE
}

Normal client SET stores a View into the request buffer, so both calls clone and the bug is masked. Replayed-from-WAL / remote-cluster-forwarded / MULTI-and-Lua-cloned commands carry an owned value, so commit #1 moves it out and commit #2 stores empty.

Repro

HSET k f v (k is a hash), then SET k hello where k is owned by a remote node group (cluster) or after a crash + WAL replay → GET k returns "" instead of hello.

Fix: delete the trailing unconditional str_obj.CommitSet(value_); (lines 1433-1434) — each sub-branch already commits.

---

Found during a code audit (docs PR #492). Verified against source at the cited lines.

🤖 Found with Claude Code