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.

diff --git a/src/bitops.c b/src/bitops.c
index 0b9c7b02fb6c095e1b92b8b97f28fcb7a785f7e3..2f62ccc460494bd0f12b6482cc3fceea545f950c 100644 (file)
--- 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;
     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. */
     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);
     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) {
     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)) {
             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(src);
             zfree(len);

+            zfree(objects);

             return;
         }
             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];
     }
 
         if (len[j] > maxlen) maxlen = len[j];
     }
 


@@ -226,8 +235,13 @@ void bitopCommand(redisClient *c) {
             res[j] = output;
         }
     }
             res[j] = output;
         }
     }

+    for (j = 0; j < numkeys; j++) {
+        if (objects[j])
+            decrRefCount(objects[j]);
+    }

     zfree(src);
     zfree(len);
     zfree(src);
     zfree(len);

+    zfree(objects);

 
     /* Store the computed value into the target key */
     if (maxlen) {
 
     /* Store the computed value into the target key */
     if (maxlen) {

diff --git a/tests/unit/bitops.tcl b/tests/unit/bitops.tcl
index bc4799432c7b44beec082709945ef364e18b8994..b7a76abb12921fcca06376700b9b80b369cce55e 100644 (file)
--- a/tests/unit/bitops.tcl
+++ b/tests/unit/bitops.tcl
@@ -44,7 +44,7 @@ start_server {tags {"bitops"}} {
     } 0
 
     catch {unset num}
     } 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
         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} {
     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}]
                 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*}

 }
 }