Fixed some subtle bug in the command processing code almost impossible to spot in the real world, thanks to gcov

redis.c

@@ -1880,6 +1880,7 @@ again:
             if (sdslen(query) == 0) {
                 /* Ignore empty query */
                 sdsfree(query);
+                if (sdslen(c->querybuf)) goto again;
                 return;
             }
             argv = sdssplitlen(query,sdslen(query)," ",1,&argc);
@@ -1897,10 +1898,16 @@ again:
                 }
             }
             zfree(argv);
-            /* Execute the command. If the client is still valid
+            if (c->argc) {
-             * after processCommand() return and there is something
+                /* Execute the command. If the client is still valid
-             * on the query buffer try to process the next command. */
+                 * after processCommand() return and there is something
-            if (c->argc && processCommand(c) && sdslen(c->querybuf)) goto again;
+                 * on the query buffer try to process the next command. */
+                if (processCommand(c) && sdslen(c->querybuf)) goto again;
+            } else {
+                /* Nothing to process, argc == 0. Just process the query
+                 * buffer if it's not empty or return to the caller */
+                if (sdslen(c->querybuf)) goto again;
+            }
             return;
         } else if (sdslen(c->querybuf) >= REDIS_REQUEST_MAX_SIZE) {
             redisLog(REDIS_DEBUG, "Client protocol error");

test-redis.tcl

@@ -89,6 +89,11 @@ proc main {server port} {
         $r get x
     } {foobar}
 
+    test {SET and GET an empty item} {
+        $r set x {}
+        $r get x
+    } {}
+
     test {DEL against a single item} {
         $r del x
         $r get x
@@ -1229,6 +1234,51 @@ proc main {server port} {
         $r get x
     } {10}
 
+    test {Handle an empty query well} {
+        set fd [$r channel]
+        puts -nonewline $fd "\r\n"
+        flush $fd
+        $r ping
+    } {PONG}
+
+    test {Negative multi bulk command does not create problems} {
+        set fd [$r channel]
+        puts -nonewline $fd "*-10\r\n"
+        flush $fd
+        $r ping
+    } {PONG}
+
+    test {Negative multi bulk payload} {
+        set fd [$r channel]
+        puts -nonewline $fd "SET x -10\r\n"
+        flush $fd
+        gets $fd
+    } {*invalid bulk*}
+
+    test {Too big bulk payload} {
+        set fd [$r channel]
+        puts -nonewline $fd "SET x 2000000000\r\n"
+        flush $fd
+        gets $fd
+    } {*invalid bulk*count*}
+
+    test {Multi bulk request not followed by bulk args} {
+        set fd [$r channel]
+        puts -nonewline $fd "*1\r\nfoo\r\n"
+        flush $fd
+        gets $fd
+    } {*protocol error*}
+
+    test {Generic wrong number of args} {
+        catch {$r ping x y z} err
+        set _ $err
+    } {*wrong*arguments*ping*}
+
+    test {SELECT an out of range DB} {
+        catch {$r select 1000000} err
+        set _ $err
+    } {*invalid*}
+
     # Leave the user with a clean DB before to exit
     test {FLUSHDB} {
         set aux {}
@@ -1322,7 +1372,7 @@ for {set j 0} {$j < [llength $argv]} {incr j} {
         set ::last $arg
         incr j
     } else {
-        echo "Wrong argument: $opt"
+        puts "Wrong argument: $opt"
         exit 1
     }
 }