From c4433d2a6aa9deac835c1032d72622ca9d2aadc6 Mon Sep 17 00:00:00 2001
From: oranagra <oran@redislabs.com>
Date: Tue, 24 May 2016 14:52:43 +0300
Subject: [PATCH 1/2] fix crash in BITFIELD GET on non existing key or wrong
 type see #3259

this was a bug in the recent refactoring: bee963c4459223d874e3294a0d8638a588d33c8e
---
 src/bitops.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/bitops.c b/src/bitops.c
index a7fad899..a7e193f8 100644
--- a/src/bitops.c
+++ b/src/bitops.c
@@ -1049,12 +1049,14 @@ void bitfieldCommand(client *c) {
         } else {
             /* GET */
             unsigned char buf[9];
-            long strlen;
+            long strlen = 0;
             unsigned char *src = NULL;
             char llbuf[LONG_STR_SIZE];
 
-            o = lookupKeyRead(c->db,c->argv[1]);
-            src = getObjectReadOnlyString(o,&strlen,llbuf);
+            if ((o = lookupKeyRead(c->db,c->argv[1])) != NULL) {
+                if (checkType(c,o,OBJ_STRING)) continue;
+                src = getObjectReadOnlyString(o,&strlen,llbuf);
+            }
 
             /* For GET we use a trick: before executing the operation
              * copy up to 9 bytes to a local buffer, so that we can easily

From 5d96b7ed4ffc1b90195fd4074ead331236e8e28f Mon Sep 17 00:00:00 2001
From: oranagra <oran@redislabs.com>
Date: Tue, 24 May 2016 23:31:36 +0300
Subject: [PATCH 2/2] check WRONGTYPE in BITFIELD before looping on the
 operations.

optimization: lookup key only once, and grow at once to the max need
fixes #3259 and #3221, and also an early return if wrongtype is discovered by SET
---
 src/bitops.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/src/bitops.c b/src/bitops.c
index a7e193f8..bd51ddbb 100644
--- a/src/bitops.c
+++ b/src/bitops.c
@@ -895,6 +895,8 @@ void bitfieldCommand(client *c) {
     int j, numops = 0, changes = 0;
     struct bitfieldOp *ops = NULL; /* Array of ops to execute at end. */
     int owtype = BFOVERFLOW_WRAP; /* Overflow type. */
+    int readonly = 1;
+    long highestWriteOffset = 0;
 
     for (j = 2; j < c->argc; j++) {
         int remargs = c->argc-j-1; /* Remaining args other than current. */
@@ -942,8 +944,10 @@ void bitfieldCommand(client *c) {
             return;
         }
 
-        /* INCRBY and SET require another argument. */
         if (opcode != BITFIELDOP_GET) {
+            readonly = 0;
+            highestWriteOffset = bitoffset + bits - 1;
+            /* INCRBY and SET require another argument. */
             if (getLongLongFromObjectOrReply(c,c->argv[j+3],&i64,NULL) != C_OK){
                 zfree(ops);
                 return;
@@ -963,6 +967,18 @@ void bitfieldCommand(client *c) {
         j += 3 - (opcode == BITFIELDOP_GET);
     }
 
+    if (readonly) {
+        /* Lookup for read is ok if key doesn't exit, but errors 
+         * if it's not a string*/
+        o = lookupKeyRead(c->db,c->argv[1]);
+        if (o != NULL && checkType(c,o,OBJ_STRING)) return;
+    } else {
+        /* Lookup by making room up to the farest bit reached by
+         * this operation. */
+        if ((o = lookupStringForBitCommand(c,
+            highestWriteOffset)) == NULL) return;
+    }
+
     addReplyMultiBulkLen(c,numops);
 
     /* Actually process the operations. */
@@ -977,11 +993,6 @@ void bitfieldCommand(client *c) {
              * for simplicity. SET return value is the previous value so
              * we need fetch & store as well. */
 
-            /* Lookup by making room up to the farest bit reached by
-             * this operation. */
-            if ((o = lookupStringForBitCommand(c,
-                thisop->offset + (thisop->bits-1))) == NULL) return;
-
             /* We need two different but very similar code paths for signed
              * and unsigned operations, since the set of functions to get/set
              * the integers and the used variables types are different. */
@@ -1053,10 +1064,8 @@ void bitfieldCommand(client *c) {
             unsigned char *src = NULL;
             char llbuf[LONG_STR_SIZE];
 
-            if ((o = lookupKeyRead(c->db,c->argv[1])) != NULL) {
-                if (checkType(c,o,OBJ_STRING)) continue;
+            if (o != NULL)
                 src = getObjectReadOnlyString(o,&strlen,llbuf);
-            }
 
             /* For GET we use a trick: before executing the operation
              * copy up to 9 bytes to a local buffer, so that we can easily