From 946342c19019c5cf81b965cac3dbea5007d74049 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 6 May 2010 20:35:00 +0200 Subject: [PATCH 1/2] hincrby should report an error when called against a hash key that doesn't contain an integer --- redis.c | 15 +++++++-------- test-redis.tcl | 14 +++++++++----- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/redis.c b/redis.c index 05f1c64a4..465398ada 100644 --- a/redis.c +++ b/redis.c @@ -3221,7 +3221,7 @@ static int getDoubleFromObject(robj *o, double *target) { } else if (o->encoding == REDIS_ENCODING_INT) { value = (long)o->ptr; } else { - redisAssert(1 != 1); + redisPanic("Unknown string encoding"); } } @@ -3258,7 +3258,7 @@ static int getLongLongFromObject(robj *o, long long *target) { } else if (o->encoding == REDIS_ENCODING_INT) { value = (long)o->ptr; } else { - redisAssert(1 != 1); + redisPanic("Unknown string encoding"); } } @@ -6462,12 +6462,11 @@ static void hincrbyCommand(redisClient *c) { if (getLongLongFromObjectOrReply(c,c->argv[3],&incr,NULL) != REDIS_OK) return; if ((o = hashLookupWriteOrCreate(c,c->argv[1])) == NULL) return; if ((current = hashGet(o,c->argv[2])) != NULL) { - if (current->encoding == REDIS_ENCODING_RAW) - value = strtoll(current->ptr,NULL,10); - else if (current->encoding == REDIS_ENCODING_INT) - value = (long)current->ptr; - else - redisAssert(1 != 1); + if (getLongLongFromObjectOrReply(c,current,&value, + "hash value is not an integer") != REDIS_OK) { + decrRefCount(current); + return; + } decrRefCount(current); } else { value = 0; diff --git a/test-redis.tcl b/test-redis.tcl index 8f71c6241..d4a9ecdb0 100644 --- a/test-redis.tcl +++ b/test-redis.tcl @@ -1898,11 +1898,15 @@ proc main {} { list [$r hincrby smallhash tmp 17179869184] [$r hincrby bighash tmp 17179869184] } {34359738368 34359738368} - test {HINCRBY against key with spaces (no integer encoded)} { - $r hset smallhash tmp " 11 " - $r hset bighash tmp " 11 " - list [$r hincrby smallhash tmp 1] [$r hincrby bighash tmp 1] - } {12 12} + test {HINCRBY fails against hash value with spaces} { + $r hset smallhash str " 11 " + $r hset bighash str " 11 " + catch {$r hincrby smallhash str 1} smallerr + catch {$r hincrby smallhash str 1} bigerr + set rv {} + lappend rv [string match "ERR*not an integer*" $smallerr] + lappend rv [string match "ERR*not an integer*" $bigerr] + } {1 1} # TODO: # Randomized test, small and big From 4132ad8d49e9a26425497048404f32b151362fe1 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 6 May 2010 22:00:04 +0200 Subject: [PATCH 2/2] log error and quit when the AOF contains an unfinished MULTI --- redis.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/redis.c b/redis.c index 465398ada..d8d024e77 100644 --- a/redis.c +++ b/redis.c @@ -8111,12 +8111,14 @@ static struct redisClient *createFakeClient(void) { c->reply = listCreate(); listSetFreeMethod(c->reply,decrRefCount); listSetDupMethod(c->reply,dupClientReplyValue); + initClientMultiState(c); return c; } static void freeFakeClient(struct redisClient *c) { sdsfree(c->querybuf); listRelease(c->reply); + freeClientMultiState(c); zfree(c); } @@ -8128,6 +8130,7 @@ int loadAppendOnlyFile(char *filename) { FILE *fp = fopen(filename,"r"); struct redis_stat sb; unsigned long long loadedkeys = 0; + int appendonly = server.appendonly; if (redis_fstat(fileno(fp),&sb) != -1 && sb.st_size == 0) return REDIS_ERR; @@ -8137,6 +8140,10 @@ int loadAppendOnlyFile(char *filename) { exit(1); } + /* Temporarily disable AOF, to prevent EXEC from feeding a MULTI + * to the same file we're about to read. */ + server.appendonly = 0; + fakeClient = createFakeClient(); while(1) { int argc, j; @@ -8192,8 +8199,14 @@ int loadAppendOnlyFile(char *filename) { } } } + + /* This point can only be reached when EOF is reached without errors. + * If the client is in the middle of a MULTI/EXEC, log error and quit. */ + if (fakeClient->flags & REDIS_MULTI) goto readerr; + fclose(fp); freeFakeClient(fakeClient); + server.appendonly = appendonly; return REDIS_OK; readerr: