Merge remote branch 'pietern/zfixes-2.0.0' into 2.0.0

redis.c

@@ -4903,9 +4903,9 @@ static void lrangeCommand(redisClient *c) {
     if (start < 0) start = llen+start;
     if (end < 0) end = llen+end;
     if (start < 0) start = 0;
-    if (end < 0) end = 0;
 
-    /* indexes sanity checks */
+    /* Invariant: start >= 0, so this test will be true when end < 0.
+     * The range is empty when start > end or start >= length. */
     if (start > end || start >= llen) {
         /* Out of range start or start > end result in empty list */
         addReply(c,shared.emptymultibulk);
@@ -4942,9 +4942,9 @@ static void ltrimCommand(redisClient *c) {
     if (start < 0) start = llen+start;
     if (end < 0) end = llen+end;
     if (start < 0) start = 0;
-    if (end < 0) end = 0;
 
-    /* indexes sanity checks */
+    /* Invariant: start >= 0, so this test will be true when end < 0.
+     * The range is empty when start > end or start >= length. */
     if (start > end || start >= llen) {
         /* Out of range start or start > end result in empty list */
         ltrim = llen;
@@ -5909,9 +5909,9 @@ static void zremrangebyrankCommand(redisClient *c) {
     if (start < 0) start = llen+start;
     if (end < 0) end = llen+end;
     if (start < 0) start = 0;
-    if (end < 0) end = 0;
 
-    /* indexes sanity checks */
+    /* Invariant: start >= 0, so this test will be true when end < 0.
+     * The range is empty when start > end or start >= length. */
     if (start > end || start >= llen) {
         addReply(c,shared.czero);
         return;
@@ -6159,11 +6159,10 @@ static void zrangeGenericCommand(redisClient *c, int reverse) {
     if (start < 0) start = llen+start;
     if (end < 0) end = llen+end;
     if (start < 0) start = 0;
-    if (end < 0) end = 0;
 
-    /* indexes sanity checks */
+    /* Invariant: start >= 0, so this test will be true when end < 0.
+     * The range is empty when start > end or start >= length. */
     if (start > end || start >= llen) {
-        /* Out of range start or start > end result in empty list */
         addReply(c,shared.emptymultibulk);
         return;
     }

tests/support/test.tcl

@@ -2,7 +2,38 @@ set ::passed 0
 set ::failed 0
 set ::testnum 0
 
-proc test {name code okpattern} {
+proc assert_match {pattern value} {
+    if {![string match $pattern $value]} {
+        puts "!! ERROR\nExpected '$value' to match '$pattern'"
+        error "assertion"
+    }
+}
+
+proc assert_equal {expected value} {
+    if {$expected ne $value} {
+        puts "!! ERROR\nExpected '$value' to be equal to '$expected'"
+        error "assertion"
+    }
+}
+
+proc assert_error {pattern code} {
+    if {[catch $code error]} {
+        assert_match $pattern $error
+    } else {
+        puts "!! ERROR\nExpected an error but nothing was catched"
+        error "assertion"
+    }
+}
+
+proc assert_encoding {enc key} {
+    assert_match "* encoding:$enc *" [r debug object $key]
+}
+
+proc assert_type {type key} {
+    assert_equal $type [r type $key]
+}
+
+proc test {name code {okpattern notspecified}} {
     # abort if tagged with a tag to deny
     foreach tag $::denytags {
         if {[lsearch $::tags $tag] >= 0} {
@@ -28,17 +59,22 @@ proc test {name code okpattern} {
     puts -nonewline [format "#%03d %-68s " $::testnum $name]
     flush stdout
     if {[catch {set retval [uplevel 1 $code]} error]} {
+        if {$error eq "assertion"} {
+            incr ::failed
+        } else {
             puts "EXCEPTION"
             puts "\nCaught error: $error"
             error "exception"
         }
-    if {$okpattern eq $retval || [string match $okpattern $retval]} {
+    } else {
+        if {$okpattern eq "notspecified" || $okpattern eq $retval || [string match $okpattern $retval]} {
             puts "PASSED"
             incr ::passed
         } else {
             puts "!! ERROR expected\n'$okpattern'\nbut got\n'$retval'"
             incr ::failed
         }
+    }
     if {$::traceleaks} {
         if {![string match {*0 leaks*} [exec leaks redis-server]]} {
             puts "--------- Test $::testnum LEAKED! --------"

tests/unit/type/list.tcl

@@ -210,6 +210,10 @@ start_server {tags {"list"}} {
         r lrange mylist -1000 1000
     } {0 1 2 3 4 5 6 7 8 9}
 
+    test {LRANGE out of range negative end index} {
+        list [r lrange mylist 0 -10] [r lrange mylist 0 -11]
+    } {0 {}}
+
     test {LRANGE against non existing key} {
         r lrange nosuchkey 0 1
     } {}
@@ -223,6 +227,11 @@ start_server {tags {"list"}} {
         r lrange mylist 0 -1
     } {99 98 97 96 95}
 
+    test {LTRIM with out of range negative end index} {
+        r ltrim mylist 0 -6
+        r llen mylist
+    } {0}
+
     test {LTRIM stress testing} {
         set mylist {}
         set err {}

tests/unit/type/zset.tcl

@@ -1,4 +1,11 @@
 start_server {tags {"zset"}} {
+    proc create_zset {key items} {
+        r del $key
+        foreach {score entry} $items {
+            r zadd $key $score $entry
+        }
+    }
+
     test {ZSET basic ZADD and score update} {
         r zadd ztmp 10 x
         r zadd ztmp 20 y
@@ -17,6 +24,66 @@ start_server {tags {"zset"}} {
         r zcard ztmp-blabla
     } {0}
 
+    test "ZRANGE basics" {
+        r del ztmp
+        r zadd ztmp 1 a
+        r zadd ztmp 2 b
+        r zadd ztmp 3 c
+        r zadd ztmp 4 d
+
+        assert_equal {a b c d} [r zrange ztmp 0 -1]
+        assert_equal {a b c} [r zrange ztmp 0 -2]
+        assert_equal {b c d} [r zrange ztmp 1 -1]
+        assert_equal {b c} [r zrange ztmp 1 -2]
+        assert_equal {c d} [r zrange ztmp -2 -1]
+        assert_equal {c} [r zrange ztmp -2 -2]
+
+        # out of range start index
+        assert_equal {a b c} [r zrange ztmp -5 2]
+        assert_equal {a b} [r zrange ztmp -5 1]
+        assert_equal {} [r zrange ztmp 5 -1]
+        assert_equal {} [r zrange ztmp 5 -2]
+
+        # out of range end index
+        assert_equal {a b c d} [r zrange ztmp 0 5]
+        assert_equal {b c d} [r zrange ztmp 1 5]
+        assert_equal {} [r zrange ztmp 0 -5]
+        assert_equal {} [r zrange ztmp 1 -5]
+
+        # withscores
+        assert_equal {a 1 b 2 c 3 d 4} [r zrange ztmp 0 -1 withscores]
+    }
+
+    test "ZREVRANGE basics" {
+        r del ztmp
+        r zadd ztmp 1 a
+        r zadd ztmp 2 b
+        r zadd ztmp 3 c
+        r zadd ztmp 4 d
+
+        assert_equal {d c b a} [r zrevrange ztmp 0 -1]
+        assert_equal {d c b} [r zrevrange ztmp 0 -2]
+        assert_equal {c b a} [r zrevrange ztmp 1 -1]
+        assert_equal {c b} [r zrevrange ztmp 1 -2]
+        assert_equal {b a} [r zrevrange ztmp -2 -1]
+        assert_equal {b} [r zrevrange ztmp -2 -2]
+
+        # out of range start index
+        assert_equal {d c b} [r zrevrange ztmp -5 2]
+        assert_equal {d c} [r zrevrange ztmp -5 1]
+        assert_equal {} [r zrevrange ztmp 5 -1]
+        assert_equal {} [r zrevrange ztmp 5 -2]
+
+        # out of range end index
+        assert_equal {d c b a} [r zrevrange ztmp 0 5]
+        assert_equal {c b a} [r zrevrange ztmp 1 5]
+        assert_equal {} [r zrevrange ztmp 0 -5]
+        assert_equal {} [r zrevrange ztmp 1 -5]
+
+        # withscores
+        assert_equal {d 4 c 3 b 2 a 1} [r zrevrange ztmp 0 -1 withscores]
+    }
+
     test {ZRANK basics} {
         r zadd zranktmp 10 x
         r zadd zranktmp 20 y
@@ -69,15 +136,6 @@ start_server {tags {"zset"}} {
         set _ $err
     } {}
 
-    test {ZRANGE and ZREVRANGE basics} {
-        list [r zrange ztmp 0 -1] [r zrevrange ztmp 0 -1] \
-            [r zrange ztmp 1 -1] [r zrevrange ztmp 1 -1]
-    } {{y x z} {z x y} {x z} {x y}}
-
-    test {ZRANGE WITHSCORES} {
-        r zrange ztmp 0 -1 withscores
-    } {y 1 x 10 z 30}
-
     test {ZSETs stress tester - sorting is working well?} {
         set delta 0
         for {set test 0} {$test < 2} {incr test} {
@@ -288,15 +346,32 @@ start_server {tags {"zset"}} {
         list [r zremrangebyscore zset -inf +inf] [r zrange zset 0 -1]
     } {5 {}}
 
-    test {ZREMRANGEBYRANK basics} {
+    test "ZREMRANGEBYRANK basics" {
-        r del zset
+        proc remrangebyrank {min max} {
-        r zadd zset 1 a
+            create_zset zset {1 a 2 b 3 c 4 d 5 e}
-        r zadd zset 2 b
+            r zremrangebyrank zset $min $max
-        r zadd zset 3 c
+        }
-        r zadd zset 4 d
+
-        r zadd zset 5 e
+        # inner range
-        list [r zremrangebyrank zset 1 3] [r zrange zset 0 -1]
+        assert_equal 3 [remrangebyrank 1 3]
-    } {3 {a e}}
+        assert_equal {a e} [r zrange zset 0 -1]
+
+        # start underflow
+        assert_equal 1 [remrangebyrank -10 0]
+        assert_equal {b c d e} [r zrange zset 0 -1]
+
+        # start overflow
+        assert_equal 0 [remrangebyrank 10 -1]
+        assert_equal {a b c d e} [r zrange zset 0 -1]
+
+        # end underflow
+        assert_equal 0 [remrangebyrank 0 -10]
+        assert_equal {a b c d e} [r zrange zset 0 -1]
+
+        # end overflow
+        assert_equal 5 [remrangebyrank 0 10]
+        assert_equal {} [r zrange zset 0 -1]
+    }
 
     test {ZUNIONSTORE against non-existing key doesn't set destination} {
       r del zseta