From e82c1aedeaa7aee8c6290d6ceaafb347996eb671 Mon Sep 17 00:00:00 2001
From: guybe7 <guy.benoish@redislabs.com>
Date: Mon, 21 Mar 2022 11:33:27 +0200
Subject: [PATCH] BITSET and BITFIELD SET should propagate even if just length
 changed (#10459)

Bug introduced in #9403, caused inconsistency between master and
replica in case just the length (i.e. set a high-index bit to 0)
changed.
---
 src/bitops.c          | 24 +++++++++++++-----------
 tests/unit/bitops.tcl |  6 ++++++
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/src/bitops.c b/src/bitops.c
index 14bcc2371..8a6dee44d 100644
--- a/src/bitops.c
+++ b/src/bitops.c
@@ -478,19 +478,21 @@ int getBitfieldTypeFromArgument(client *c, robj *o, int *sign, int *bits) {
  * so that the 'maxbit' bit can be addressed. The object is finally
  * returned. Otherwise if the key holds a wrong type NULL is returned and
  * an error is sent to the client. */
-robj *lookupStringForBitCommand(client *c, uint64_t maxbit, int *created) {
+robj *lookupStringForBitCommand(client *c, uint64_t maxbit, int *dirty) {
     size_t byte = maxbit >> 3;
     robj *o = lookupKeyWrite(c->db,c->argv[1]);
     if (checkType(c,o,OBJ_STRING)) return NULL;
+    if (dirty) *dirty = 0;
 
     if (o == NULL) {
-        if (created) *created = 1;
         o = createObject(OBJ_STRING,sdsnewlen(NULL, byte+1));
         dbAdd(c->db,c->argv[1],o);
+        if (dirty) *dirty = 1;
     } else {
-        if (created) *created = 0;
         o = dbUnshareStringValue(c->db,c->argv[1],o);
+        size_t oldlen = sdslen(o->ptr);
         o->ptr = sdsgrowzero(o->ptr,byte+1);
+        if (dirty && oldlen != sdslen(o->ptr)) *dirty = 1;
     }
     return o;
 }
@@ -547,8 +549,8 @@ void setbitCommand(client *c) {
         return;
     }
 
-    int created;
-    if ((o = lookupStringForBitCommand(c,bitoffset,&created)) == NULL) return;
+    int dirty;
+    if ((o = lookupStringForBitCommand(c,bitoffset,&dirty)) == NULL) return;
 
     /* Get current values */
     byte = bitoffset >> 3;
@@ -556,10 +558,10 @@ void setbitCommand(client *c) {
     bit = 7 - (bitoffset & 0x7);
     bitval = byteval & (1 << bit);
 
-    /* Either it is newly created, or the bit changes before and after.
+    /* Either it is newly created, changed length, or the bit changes before and after.
      * Note that the bitval here is actually a decimal number.
      * So we need to use `!!` to convert it to 0 or 1 for comparison. */
-    if (created || (!!bitval != on)) {
+    if (dirty || (!!bitval != on)) {
         /* Update byte with new bit value. */
         byteval &= ~(1 << bit);
         byteval |= ((on & 0x1) << bit);
@@ -1028,7 +1030,7 @@ struct bitfieldOp {
 void bitfieldGeneric(client *c, int flags) {
     robj *o;
     uint64_t bitoffset;
-    int j, numops = 0, changes = 0, created = 0;
+    int j, numops = 0, changes = 0, dirty = 0;
     struct bitfieldOp *ops = NULL; /* Array of ops to execute at end. */
     int owtype = BFOVERFLOW_WRAP; /* Overflow type. */
     int readonly = 1;
@@ -1122,7 +1124,7 @@ void bitfieldGeneric(client *c, int flags) {
         /* Lookup by making room up to the farthest bit reached by
          * this operation. */
         if ((o = lookupStringForBitCommand(c,
-            highest_write_offset,&created)) == NULL) {
+            highest_write_offset,&dirty)) == NULL) {
             zfree(ops);
             return;
         }
@@ -1172,7 +1174,7 @@ void bitfieldGeneric(client *c, int flags) {
                     setSignedBitfield(o->ptr,thisop->offset,
                                       thisop->bits,newval);
 
-                    if (created || (oldval != newval))
+                    if (dirty || (oldval != newval))
                         changes++;
                 } else {
                     addReplyNull(c);
@@ -1204,7 +1206,7 @@ void bitfieldGeneric(client *c, int flags) {
                     setUnsignedBitfield(o->ptr,thisop->offset,
                                         thisop->bits,newval);
 
-                    if (created || (oldval != newval))
+                    if (dirty || (oldval != newval))
                         changes++;
                 } else {
                     addReplyNull(c);
diff --git a/tests/unit/bitops.tcl b/tests/unit/bitops.tcl
index 89431c81f..ec08a060d 100644
--- a/tests/unit/bitops.tcl
+++ b/tests/unit/bitops.tcl
@@ -433,6 +433,12 @@ start_server {tags {"bitops"}} {
         r bitfield foo3{t} incrby i5 0 1
         set dirty5 [s rdb_changes_since_last_save]
         assert {$dirty5 == $dirty4 + 2}
+
+        # Change length only
+        r setbit foo{t} 90 0
+        r bitfield foo2{t} set i5 90 0
+        set dirty6 [s rdb_changes_since_last_save]
+        assert {$dirty6 == $dirty5 + 2}
     }
 
     test {BITPOS bit=1 fuzzy testing using SETBIT} {