From 3a1ab86a35cc2ea0cd9b477e92a5ce9116c48123 Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pcnoordhuis@gmail.com>
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. Backport of 673e1fb7 to 2.0.0.
---
 redis.c                  | 21 +++++++++------------
 tests/support/test.tcl   |  2 +-
 tests/unit/type/zset.tcl | 36 +++++++++++++++++++++---------------
 3 files changed, 31 insertions(+), 28 deletions(-)

diff --git a/redis.c b/redis.c
index 0e3dda11e9..1f7b0642f1 100644
--- a/redis.c
+++ b/redis.c
@@ -3288,7 +3288,7 @@ static 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 {
@@ -5738,11 +5738,6 @@ static void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double scor
     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();
@@ -5773,7 +5768,7 @@ static void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double scor
         }
         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
@@ -5827,15 +5822,13 @@ static void zaddGenericCommand(redisClient *c, robj *key, robj *ele, double scor
 
 static 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);
 }
 
 static 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);
 }
 
@@ -6010,8 +6003,12 @@ static void zunionInterGenericCommand(redisClient *c, robj *dstkey, int op) {
             if (remaining >= (zsetnum + 1) && !strcasecmp(c->argv[j]->ptr,"weights")) {
                 j++; remaining--;
                 for (i = 0; i < zsetnum; 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/support/test.tcl b/tests/support/test.tcl
index 4caa6ca797..d4a8abb837 100644
--- a/tests/support/test.tcl
+++ b/tests/support/test.tcl
@@ -17,7 +17,7 @@ proc assert_equal {expected value} {
 }
 
 proc assert_error {pattern code} {
-    if {[catch $code error]} {
+    if {[catch {uplevel 1 $code} error]} {
         assert_match $pattern $error
     } else {
         puts "!! ERROR\nExpected an error but nothing was catched"
diff --git a/tests/unit/type/zset.tcl b/tests/unit/type/zset.tcl
index 071dc2e555..626287259e 100644
--- a/tests/unit/type/zset.tcl
+++ b/tests/unit/type/zset.tcl
@@ -419,6 +419,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
@@ -439,6 +441,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"} {
@@ -485,22 +497,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}
+    }
 }