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.
This commit is contained in:
antirez 2012-05-22 17:40:20 +02:00
parent 7c34643f15
commit fa4a5d5922
2 changed files with 34 additions and 3 deletions

View File

@ -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) {
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) {
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) {

View File

@ -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"}} {
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"}} {
} }
} }
} }
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*}
} }