From fa4a5d59220b1c9e7833b563cb7e9e314747bdf7 Mon Sep 17 00:00:00 2001
From: antirez <antirez@gmail.com>
Date: Tue, 22 May 2012 17:40:20 +0200
Subject: [PATCH] BITOP: handle integer encoded objects correctly.

A bug in the implementation caused BITOP to crash the server if at least
one one of the source objects was integer encoded.

The new implementation takes an additional array of Redis objects
pointers and calls getDecodedObject() to get a reference to a string
encoded object, and then uses decrRefCount() to release the object.

Tests modified to cover the regression and improve coverage.
---
 src/bitops.c          | 18 ++++++++++++++++--
 tests/unit/bitops.tcl | 19 ++++++++++++++++++-
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/src/bitops.c b/src/bitops.c
index 0b9c7b02..2f62ccc4 100644
--- a/src/bitops.c
+++ b/src/bitops.c
@@ -159,6 +159,7 @@ void bitopCommand(redisClient *c) {
     char *opname = c->argv[1]->ptr;
     robj *o, *targetkey = c->argv[2];
     long op, j, numkeys;
+    robj **objects;      /* Array of soruce objects. */
     unsigned char **src; /* Array of source strings pointers. */
     long *len, maxlen = 0; /* Array of length of src strings, and max len. */
     unsigned char *res = NULL; /* Resulting string. */
@@ -187,22 +188,30 @@ void bitopCommand(redisClient *c) {
     numkeys = c->argc - 3;
     src = zmalloc(sizeof(unsigned char*) * numkeys);
     len = zmalloc(sizeof(long) * numkeys);
+    objects = zmalloc(sizeof(robj*) * numkeys);
     for (j = 0; j < numkeys; j++) {
         o = lookupKeyRead(c->db,c->argv[j+3]);
         /* Handle non-existing keys as empty strings. */
         if (o == NULL) {
+            objects[j] = NULL;
             src[j] = NULL;
             len[j] = 0;
             continue;
         }
         /* Return an error if one of the keys is not a string. */
         if (checkType(c,o,REDIS_STRING)) {
+            for (j = j-1; j >= 0; j--) {
+                if (objects[j])
+                    decrRefCount(objects[j]);
+            }
             zfree(src);
             zfree(len);
+            zfree(objects);
             return;
         }
-        src[j] = o->ptr;
-        len[j] = sdslen(o->ptr);
+        objects[j] = getDecodedObject(o);
+        src[j] = objects[j]->ptr;
+        len[j] = sdslen(objects[j]->ptr);
         if (len[j] > maxlen) maxlen = len[j];
     }
 
@@ -226,8 +235,13 @@ void bitopCommand(redisClient *c) {
             res[j] = output;
         }
     }
+    for (j = 0; j < numkeys; j++) {
+        if (objects[j])
+            decrRefCount(objects[j]);
+    }
     zfree(src);
     zfree(len);
+    zfree(objects);
 
     /* Store the computed value into the target key */
     if (maxlen) {
diff --git a/tests/unit/bitops.tcl b/tests/unit/bitops.tcl
index bc479943..b7a76abb 100644
--- a/tests/unit/bitops.tcl
+++ b/tests/unit/bitops.tcl
@@ -44,7 +44,7 @@ start_server {tags {"bitops"}} {
     } 0
 
     catch {unset num}
-    foreach vec [list "" "\xaa" "\x00\x00\xff" "foobar"] {
+    foreach vec [list "" "\xaa" "\x00\x00\xff" "foobar" "123"] {
         incr num
         test "BITCOUNT against test vector #$num" {
             r set str $vec
@@ -119,6 +119,7 @@ start_server {tags {"bitops"}} {
     foreach op {and or xor} {
         test "BITOP $op fuzzing" {
             for {set i 0} {$i < 10} {incr i} {
+                r flushall
                 set vec {}
                 set veckeys {}
                 set numvec [expr {[randomInt 10]+1}]
@@ -133,4 +134,20 @@ start_server {tags {"bitops"}} {
             }
         }
     }
+
+    test {BITOP with integer encoded source objects} {
+        r set a 1
+        r set b 2
+        r bitop xor dest a b a
+        r get dest
+    } {2}
+
+    test {BITOP with non string source key} {
+        r del c
+        r set a 1
+        r set b 2
+        r lpush c foo
+        catch {r bitop xor dest a b c d} e
+        set e
+    } {*ERR*}
 }
-- 
2.45.2