Commit f0d962c0 authored by antirez's avatar antirez
Browse files

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.
parent c19bfe9f
...@@ -159,6 +159,7 @@ void bitopCommand(redisClient *c) { ...@@ -159,6 +159,7 @@ void bitopCommand(redisClient *c) {
char *opname = c->argv[1]->ptr; char *opname = c->argv[1]->ptr;
robj *o, *targetkey = c->argv[2]; robj *o, *targetkey = c->argv[2];
long op, j, numkeys; long op, j, numkeys;
robj **objects; /* Array of soruce objects. */
unsigned char **src; /* Array of source strings pointers. */ unsigned char **src; /* Array of source strings pointers. */
long *len, maxlen = 0; /* Array of length of src strings, and max len. */ long *len, maxlen = 0; /* Array of length of src strings, and max len. */
unsigned char *res = NULL; /* Resulting string. */ unsigned char *res = NULL; /* Resulting string. */
...@@ -187,22 +188,30 @@ void bitopCommand(redisClient *c) { ...@@ -187,22 +188,30 @@ void bitopCommand(redisClient *c) {
numkeys = c->argc - 3; numkeys = c->argc - 3;
src = zmalloc(sizeof(unsigned char*) * numkeys); src = zmalloc(sizeof(unsigned char*) * numkeys);
len = zmalloc(sizeof(long) * numkeys); len = zmalloc(sizeof(long) * numkeys);
objects = zmalloc(sizeof(robj*) * numkeys);
for (j = 0; j < numkeys; j++) { for (j = 0; j < numkeys; j++) {
o = lookupKeyRead(c->db,c->argv[j+3]); o = lookupKeyRead(c->db,c->argv[j+3]);
/* Handle non-existing keys as empty strings. */ /* Handle non-existing keys as empty strings. */
if (o == NULL) { if (o == NULL) {
objects[j] = NULL;
src[j] = NULL; src[j] = NULL;
len[j] = 0; len[j] = 0;
continue; continue;
} }
/* Return an error if one of the keys is not a string. */ /* Return an error if one of the keys is not a string. */
if (checkType(c,o,REDIS_STRING)) { if (checkType(c,o,REDIS_STRING)) {
for (j = j-1; j >= 0; j--) {
if (objects[j])
decrRefCount(objects[j]);
}
zfree(src); zfree(src);
zfree(len); zfree(len);
zfree(objects);
return; return;
} }
src[j] = o->ptr; objects[j] = getDecodedObject(o);
len[j] = sdslen(o->ptr); 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) { ...@@ -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(src);
zfree(len); zfree(len);
zfree(objects);
/* Store the computed value into the target key */ /* Store the computed value into the target key */
if (maxlen) { if (maxlen) {
......
...@@ -44,7 +44,7 @@ start_server {tags {"bitops"}} { ...@@ -44,7 +44,7 @@ start_server {tags {"bitops"}} {
} 0 } 0
catch {unset num} catch {unset num}
foreach vec [list "" "\xaa" "\x00\x00\xff" "foobar"] { foreach vec [list "" "\xaa" "\x00\x00\xff" "foobar" "123"] {
incr num incr num
test "BITCOUNT against test vector #$num" { test "BITCOUNT against test vector #$num" {
r set str $vec r set str $vec
...@@ -119,6 +119,7 @@ start_server {tags {"bitops"}} { ...@@ -119,6 +119,7 @@ start_server {tags {"bitops"}} {
foreach op {and or xor} { foreach op {and or xor} {
test "BITOP $op fuzzing" { test "BITOP $op fuzzing" {
for {set i 0} {$i < 10} {incr i} { for {set i 0} {$i < 10} {incr i} {
r flushall
set vec {} set vec {}
set veckeys {} set veckeys {}
set numvec [expr {[randomInt 10]+1}] set numvec [expr {[randomInt 10]+1}]
...@@ -133,4 +134,20 @@ start_server {tags {"bitops"}} { ...@@ -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*}
} }
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment