From 673e1fb7e4c171f5ead560f6886e877f43730cf0 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 29 Jul 2010 22:13:31 +0200 Subject: [PATCH] Change getDoubleFromObject to fail on NaN. Return an error when the resulting value is not a number (NaN). Fix ZUNIONSTORE/ZINTERSTORE to clean up when a weight argument is not a double value. --- src/object.c | 3 ++- src/t_zset.c | 19 ++++++++----------- tests/unit/type/zset.tcl | 36 +++++++++++++++++++++--------------- 3 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/object.c b/src/object.c index 51582619..21268340 100644 --- a/src/object.c +++ b/src/object.c @@ -1,5 +1,6 @@ #include "redis.h" #include +#include robj *createObject(int type, void *ptr) { robj *o; @@ -319,7 +320,7 @@ int getDoubleFromObject(robj *o, double *target) { redisAssert(o->type == REDIS_STRING); if (o->encoding == REDIS_ENCODING_RAW) { value = strtod(o->ptr, &eptr); - if (eptr[0] != '\0') return REDIS_ERR; + if (eptr[0] != '\0' || isnan(value)) return REDIS_ERR; } else if (o->encoding == REDIS_ENCODING_INT) { value = (long)o->ptr; } else { diff --git a/src/t_zset.c b/src/t_zset.c index a85a9dc1..e93e5c40 100644 --- a/src/t_zset.c +++ b/src/t_zset.c @@ -327,11 +327,6 @@ void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double scoreval, i zset *zs; double *score; - if (isnan(scoreval)) { - addReplySds(c,sdsnew("-ERR provide score is Not A Number (nan)\r\n")); - return; - } - zsetobj = lookupKeyWrite(c->db,key); if (zsetobj == NULL) { zsetobj = createZsetObject(); @@ -361,7 +356,7 @@ void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double scoreval, i } if (isnan(*score)) { addReplySds(c, - sdsnew("-ERR resulting score is Not A Number (nan)\r\n")); + sdsnew("-ERR resulting score is not a number (NaN)\r\n")); zfree(score); /* Note that we don't need to check if the zset may be empty and * should be removed here, as we can only obtain Nan as score if @@ -417,15 +412,13 @@ void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double scoreval, i void zaddCommand(redisClient *c) { double scoreval; - - if (getDoubleFromObjectOrReply(c, c->argv[2], &scoreval, NULL) != REDIS_OK) return; + if (getDoubleFromObjectOrReply(c,c->argv[2],&scoreval,NULL) != REDIS_OK) return; zaddGenericCommand(c,c->argv[1],c->argv[3],scoreval,0); } void zincrbyCommand(redisClient *c) { double scoreval; - - if (getDoubleFromObjectOrReply(c, c->argv[2], &scoreval, NULL) != REDIS_OK) return; + if (getDoubleFromObjectOrReply(c,c->argv[2],&scoreval,NULL) != REDIS_OK) return; zaddGenericCommand(c,c->argv[1],c->argv[3],scoreval,1); } @@ -608,8 +601,12 @@ void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) { if (remaining >= (setnum + 1) && !strcasecmp(c->argv[j]->ptr,"weights")) { j++; remaining--; for (i = 0; i < setnum; i++, j++, remaining--) { - if (getDoubleFromObjectOrReply(c, c->argv[j], &src[i].weight, NULL) != REDIS_OK) + if (getDoubleFromObjectOrReply(c,c->argv[j],&src[i].weight, + "weight value is not a double") != REDIS_OK) + { + zfree(src); return; + } } } else if (remaining >= 2 && !strcasecmp(c->argv[j]->ptr,"aggregate")) { j++; remaining--; diff --git a/tests/unit/type/zset.tcl b/tests/unit/type/zset.tcl index a289d883..642922e9 100644 --- a/tests/unit/type/zset.tcl +++ b/tests/unit/type/zset.tcl @@ -435,6 +435,8 @@ start_server {tags {"zset"}} { foreach cmd {ZUNIONSTORE ZINTERSTORE} { test "$cmd with +inf/-inf scores" { + r del zsetinf1 zsetinf2 + r zadd zsetinf1 +inf key r zadd zsetinf2 +inf key r $cmd zsetinf3 2 zsetinf1 zsetinf2 @@ -455,6 +457,16 @@ start_server {tags {"zset"}} { r $cmd zsetinf3 2 zsetinf1 zsetinf2 assert_equal -inf [r zscore zsetinf3 key] } + + test "$cmd with NaN weights" { + r del zsetinf1 zsetinf2 + + r zadd zsetinf1 1.0 key + r zadd zsetinf2 1.0 key + assert_error "*weight value is not a double*" { + r $cmd zsetinf3 2 zsetinf1 zsetinf2 weights nan nan + } + } } tags {"slow"} { @@ -501,22 +513,16 @@ start_server {tags {"zset"}} { } {} } - test {ZSET element can't be set to nan with ZADD} { - set e {} - catch {r zadd myzset nan abc} e - set _ $e - } {*Not A Number*} + test {ZSET element can't be set to NaN with ZADD} { + assert_error "*not a double*" {r zadd myzset nan abc} + } - test {ZSET element can't be set to nan with ZINCRBY} { - set e {} - catch {r zincrby myzset nan abc} e - set _ $e - } {*Not A Number*} + test {ZSET element can't be set to NaN with ZINCRBY} { + assert_error "*not a double*" {r zadd myzset nan abc} + } - test {ZINCRBY calls leading to Nan are refused} { - set e {} + test {ZINCRBY calls leading to NaN result in error} { r zincrby myzset +inf abc - catch {r zincrby myzset -inf abc} e - set _ $e - } {*Not A Number*} + assert_error "*NaN*" {r zincrby myzset -inf abc} + } } -- 2.45.2