fix(hset_family): Fix crash on scan after expiry set (#4802)

When expiry is set on an hset its members are migrated to a string map.

If a scan is performed on the hset, the code branch for string map
accessed the value pointer directly by loading the 64 bit value from the
object pointer.

When creating a new entry in string map with TTL, we also set a bit on
the stored pointer as metadata, this is set as 1 << 63.

As a result when loading back the pointer we need to unset the same bit
to get the correct address, this is done already in string map internals
but the scan code did not do this, resulting in segfault.

This change adds an AND operation to unset the TTL bit before
dereferencing the pointer.

Signed-off-by: Abhijat Malviya <abhijat@dragonflydb.io>

Author: abhijat

src/core/string_map.cc

@@ -22,12 +22,6 @@ namespace {
 constexpr uint64_t kValTtlBit = 1ULL << 63;
 constexpr uint64_t kValMask = ~kValTtlBit;
 
-inline sds GetValue(sds key) {
-  char* valptr = key + sdslen(key) + 1;
-  uint64_t val = absl::little_endian::Load64(valptr);
-  return (sds)(kValMask & val);
-}
-
 // Returns key, tagged value pair
 pair<sds, uint64_t> CreateEntry(string_view field, string_view value, uint32_t time_now,
                                 uint32_t ttl_sec) {
@@ -189,6 +183,12 @@ void StringMap::RandomPairs(unsigned int count, std::vector<sds>& keys, std::vec
   }
 }
 
+sds StringMap::GetValue(sds key) {
+  char* valptr = key + sdslen(key) + 1;
+  const uint64_t val = absl::little_endian::Load64(valptr);
+  return (sds)(kValMask & val);
+}
+
 pair<sds, bool> StringMap::ReallocIfNeeded(void* obj, float ratio) {
   sds key = (sds)obj;
   size_t key_len = sdslen(key);

src/core/string_map.h

@@ -147,6 +147,8 @@ class StringMap : public DenseSet {
   void RandomPairs(unsigned int count, std::vector<sds>& keys, std::vector<sds>& vals,
                    bool with_value);
 
+  static sds GetValue(sds key);
+
  private:
   // Reallocate key and/or value if their pages are underutilized.
   // Returns new pointer (stays same if key utilization is enough) and if reallocation happened.

src/server/hset_family.cc

@@ -320,8 +320,7 @@ OpResult<StringVec> OpScan(const OpArgs& op_args, std::string_view key, uint64_t
       size_t len = sdslen(val);
       if (scan_op.Matches(string_view(val, len))) {
         res.emplace_back(val, len);
-        val += (len + 1);
-        val = (sds)absl::little_endian::Load64(val);
+        val = StringMap::GetValue(val);
         res.emplace_back(val, sdslen(val));
       }
     };

src/server/hset_family_test.cc

@@ -507,4 +507,18 @@ TEST_F(HSetFamilyTest, EmptyHashBug) {
   EXPECT_THAT(Run({"EXISTS", "foo"}), IntArg(0));
 }
 
+TEST_F(HSetFamilyTest, ScanAfterExpireSet) {
+  EXPECT_THAT(Run({"HSET", "aset", "afield", "avalue"}), IntArg(1));
+  EXPECT_THAT(Run({"HEXPIRE", "aset", "1", "FIELDS", "1", "afield"}), IntArg(1));
+
+  const auto resp = Run({"HSCAN", "aset", "0", "count", "100"});
+  EXPECT_THAT(resp, ArrLen(2));
+
+  const auto vec = StrArray(resp.GetVec()[1]);
+  EXPECT_EQ(vec.size(), 2);
+
+  EXPECT_THAT(vec, Contains("afield").Times(1));
+  EXPECT_THAT(vec, Contains("avalue").Times(1));
+}
+
 }  // namespace dfly