From f110de4b239d23b8436fdc742a557103c5852aad Mon Sep 17 00:00:00 2001
From: Lu JJ <41555481+ncghost1@users.noreply.github.com>
Date: Wed, 6 Apr 2022 02:45:45 +0800
Subject: [PATCH] Fix the bug that caused hash encoding errors when using
 hincrbyfloat or hincrby commands (#10479)

Fixed a bug that used the `hincrbyfloat` or `hincrby` commands to make the field or value exceed the
`hash_max_listpack_value` but did not change the object encoding of the hash structure.

Add a length check for field and value, check the length of value first, if the length of value does not
exceed `hash_max_listpack_value` then check the length of field.

If the length of field or value is too long, it will reduce the efficiency of listpack, and the object encoding
will become hashtable after AOF restart, so this is also to keep the same before and after AOF restart.
---
 src/t_hash.c             |  8 ++++++++
 tests/unit/type/hash.tcl | 25 +++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/src/t_hash.c b/src/t_hash.c
index 92f5cb2b0..3f6d9528a 100644
--- a/src/t_hash.c
+++ b/src/t_hash.c
@@ -205,6 +205,14 @@ int hashTypeExists(robj *o, sds field) {
 int hashTypeSet(robj *o, sds field, sds value, int flags) {
     int update = 0;
 
+    /* Check if the field is too long for listpack, and convert before adding the item.
+     * This is needed for HINCRBY* case since in other commands this is handled early by
+     * hashTypeTryConversion, so this check will be a NOP. */
+    if (o->encoding == OBJ_ENCODING_LISTPACK) {
+        if (sdslen(field) > server.hash_max_listpack_value || sdslen(value) > server.hash_max_listpack_value)
+            hashTypeConvert(o, OBJ_ENCODING_HT);
+    }
+    
     if (o->encoding == OBJ_ENCODING_LISTPACK) {
         unsigned char *zl, *fptr, *vptr;
 
diff --git a/tests/unit/type/hash.tcl b/tests/unit/type/hash.tcl
index 75ba29f77..7e3b3e2ca 100644
--- a/tests/unit/type/hash.tcl
+++ b/tests/unit/type/hash.tcl
@@ -648,6 +648,31 @@ start_server {tags {"hash"}} {
         }
     }
 
+    test {HINCRBYFLOAT over hash-max-listpack-value encoded with a listpack} {
+        set original_max_value [lindex [r config get hash-max-ziplist-value] 1]
+        r config set hash-max-listpack-value 8
+        
+        # hash's value exceeds hash-max-listpack-value
+        r del smallhash
+        r del bighash
+        r hset smallhash tmp 0
+        r hset bighash tmp 0
+        r hincrbyfloat smallhash tmp 0.000005
+        r hincrbyfloat bighash tmp 0.0000005
+        assert_encoding listpack smallhash
+        assert_encoding hashtable bighash
+
+        # hash's field exceeds hash-max-listpack-value
+        r del smallhash
+        r del bighash
+        r hincrbyfloat smallhash abcdefgh 1
+        r hincrbyfloat bighash abcdefghi 1
+        assert_encoding listpack smallhash
+        assert_encoding hashtable bighash
+
+        r config set hash-max-listpack-value $original_max_value
+    }
+
     test {Hash ziplist regression test for large keys} {
         r hset hash kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk a
         r hset hash kkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkk b