Fix crash and wrong return values in the hash stringRef module API (#4504)

Three issues in the `ValkeyModule_HashSetStringRef` /
`ValkeyModule_HashHasStringRef` path:

1. NULL dereference. `hashTypeHasStringRef()` used `hashtableFindRef()`
and dereferenced the returned pointer unconditionally.
`hashtableFindRef()` returns NULL when the field is not present, so
`ValkeyModule_HashHasStringRef()` on a non-existing (or expired) field
crashed the server.

2. HashHasStringRef reported false positives. For a missing key or a
non-hash key the function returned VALKEYMODULE_ERR, which is 1 — the
exact same value it returns when the field does hold a string reference.
A module could not tell "error" from "yes". Since this is a boolean
predicate, there is no room for a third status value, so it now returns
plain 0/1 and never an error code.

3. HashSetStringRef leaked internal status codes. It returned
hashTypeUpdateAsStringRef()'s C_OK/C_ERR directly, so failures surfaced
to modules as -1 instead of VALKEYMODULE_ERR (1). The status is now
mapped explicitly.

## Compatibility
ValkeyModule_HashHasStringRef changes its error return from
VALKEYMODULE_ERR to 0. This is
a behavior change for the API.

---------

Signed-off-by: Shubham S Taple <shubham.s.taple@gmail.com>

src/module.c

@@ -5493,18 +5493,29 @@ int VM_ZsetRangePrev(ValkeyModuleKey *key) {
  * For example, valkey-search uses this interface to avoid maintaining two copies of the
  * indexed vectors.
  *
- * The function receives the hash key, field name, buffer to share along with its size. */
+ * The function receives the hash key, field name, buffer to share along with its size.
+ *
+ * Returns VALKEYMODULE_OK on success. Returns VALKEYMODULE_ERR if the key or field
+ * is missing, the key is not a hash, or the update fails. */
 int VM_HashSetStringRef(ValkeyModuleKey *key, ValkeyModuleString *field, const char *buf, size_t len) {
     if (!key || !key->value || objectGetType(key->value) != OBJ_HASH || !field || !buf) return VALKEYMODULE_ERR;
-    return hashTypeUpdateAsStringRef(key->value, objectGetVal(field), buf, len);
+    if (hashTypeUpdateAsStringRef(key->value, objectGetVal(field), buf, len) != C_OK) return VALKEYMODULE_ERR;
+    return VALKEYMODULE_OK;
 }
 
 /* Checks if the value of a hash entry is a shared string reference (stringRef).
- * The function receives the hash key and field name to perform the check against. */
+ * The function receives the hash key and field name to perform the check against.
+ *
+ * Returns 1 if the field exists and holds a string reference. Returns 0 in every
+ * other case, including a missing or non-hash key, a missing field name and a
+ * field which doesn't exist or is already expired. Note that the return value is
+ * a plain boolean and not an OK/ERR status, since VALKEYMODULE_ERR can't be told
+ * apart from a positive answer. */
 int VM_HashHasStringRef(ValkeyModuleKey *key, ValkeyModuleString *field) {
-    if (!key || !key->value || objectGetType(key->value) != OBJ_HASH) return VALKEYMODULE_ERR;
+    if (!key || !key->value || objectGetType(key->value) != OBJ_HASH || !field) return 0;
     return hashTypeHasStringRef(key->value, objectGetVal(field));
 }
+
 /* Set the field of the specified hash field to the specified value.
  * If the key is an empty key open for writing, it is created with an empty
  * hash value, in order to set the specified field.

src/t_hash.c

@@ -434,11 +434,14 @@ int hashTypeExists(robj *o, sds field) {
     return hashTypeGetValue(o, field, &vstr, &vlen, &vll, NULL) == C_OK;
 }
 
+/* Test if the value of the specified field is a string reference (stringRef).
+ * Returns false if the field doesn't exist, is already expired or its value is
+ * not a string reference. Listpack encoded hashes can't hold string references. */
 bool hashTypeHasStringRef(robj *o, sds field) {
     if (objectGetEncoding(o) == OBJ_ENCODING_LISTPACK) return false;
-    hashtable *ht = objectGetVal(o);
-    void **entry_ref = hashtableFindRef(ht, field);
-    return (entryHasStringRef(*entry_ref));
+    void *entry = NULL;
+    if (!hashtableFind(objectGetVal(o), field, &entry)) return false;
+    return entryHasStringRef(entry);
 }
 
 /* Update a hash field value with a string reference value.

tests/unit/moduleapi/hash_stringref.tcl

@@ -14,6 +14,53 @@ start_server {tags {"modules"}} {
         assert_equal "1" [r hash.has_stringref k f]
     }
 
+    test {Module hash set_stringref on a field which does not exist} {
+        r del k
+        r hset k f hello1
+        set status [catch {r hash.set_stringref k nofield hello2} errmsg]
+        assert {$status == 1}
+        assert_equal "hello1" [r hget k f]
+        assert_equal "0" [r hash.has_stringref k f]
+        r hash.set_stringref k f hello1
+        assert_equal "hello1" [r hget k f]
+        assert_equal "1" [r hash.has_stringref k f]
+        assert_equal "0" [r hash.has_stringref k nofield]
+    }
+
+    test {Module hash has_stringref on a field which does not exist} {
+        r del k
+        r hset k f hello1
+        assert_encoding listpack k
+        assert_equal "0" [r hash.has_stringref k nofield]
+
+        r hash.set_stringref k f hello1
+        assert_encoding hashtable k
+        assert_equal "1" [r hash.has_stringref k f]
+        assert_equal "0" [r hash.has_stringref k nofield]
+    }
+
+    test {Module hash has_stringref on an expired field} {
+        r del k
+        r hset k f1 hello1 f2 hello2
+        r hash.set_stringref k f1 hello1
+        r hash.set_stringref k f2 hello2
+        r hpexpire k 1 FIELDS 1 f1
+        wait_for_condition 50 100 {
+            [r hash.has_stringref k f1] eq 0
+        } else {
+            fail "Hash field did not expire"
+        }
+        assert_equal "1" [r hash.has_stringref k f2]
+    }
+
+    test {Module hash has_stringref on a missing key or a non hash key} {
+        r del k
+        assert_equal "0" [r hash.has_stringref k f]
+        r lpush k v
+        assert_equal "0" [r hash.has_stringref k f]
+        r del k
+    }
+
     test "Unload the module - hash" {
         assert_equal {OK} [r module unload hash.stringref]
     }