LPUSH seems to have a buffer problem

[GoogleCodeExporter]

What steps will reproduce the problem?

jcarouth@jcarouth-laptop:~/redis-0.096> telnet localhost 6379
Trying 127.0.0.1...
Connected to localhost.
Escape character is '^]'.
DBSIZE
:9
FLUSHALL  
+OK
DBSIZE
:0
LPUSH mylist value1
DBSIZE
+OK
DBSIZE
-ERR unknown command
:1
DBSIZE
:1
LLEN mylist
:1
LPUSH mylist value2
LLEN mylist
+OK
LLEN mylist
-ERR unknown command
:2
LLEN mylist
:2
LPOP mylist
$0

LPOP mylist
$0

LPOP mylist
$-1

What version of the product are you using? On what operating system?

Redis v0.096
Linux jcarouth-laptop 2.6.27.21-0.1-pae #1 SMP 2009-03-31 14:50:44 +0200
i686 i686 i386 GNU/Linux

Original issue reported on code.google.com by jcaro...@gmail.com on 12 May 2009 at 6:52

[GoogleCodeExporter]

I see similar behavior with 0.091 on Mac OS X.
It did the same thing with RPUSH as well. 

Original comment by dbosw...@gmail.com on 13 May 2009 at 12:59

[GoogleCodeExporter]

A more succinct reproduction:

$ echo -e "LPUSH mylist item\nLLEN mylist" | nc localhost 6379
+OK
-ERR unknown command
$ echo -e "LPUSH mylist item\n  LLEN mylist" | nc localhost 6379
+OK
:2

Not sure why those 2 extra spaces before LLEN would make the difference.
I'll poke around in the code and see if there's anything obvious.

Original comment by dbosw...@gmail.com on 13 May 2009 at 1:23

[GoogleCodeExporter]

I think I understand what is happening...

LPUSH is a "bulk" command expecting an input of the form
"LPUSH <listname> <num_bytes>\r\n
<num_bytes worth of data>\r\n"

So the commands we are giving it don't match this, because we're doing "LPUSH
<listname> <data>" which only works for "inline" type commands.

That was a misunderstanding on my part (although the Retwit tutorial does have 
some
examples that imply "LPUSH mylist item" might work).

But there is still a bug - this error should be handled gracefully.  The root 
problem
is in processCommand():

int bulklen = atoi(c->argv[c->argc-1]->ptr);
if (bulklen < 0 || bulklen > 1024*1024*1024) {

If the last argument isn't a number, atoi returns 0.
So I think that line should really be "<=" :

if (bulklen <= 0 || bulklen > 1024*1024*1024) {

That is, unless bulk commands of the form "LPUSH mylist 0\r\n" are expected to 
work.

Original comment by dbosw...@gmail.com on 13 May 2009 at 3:07

[GoogleCodeExporter]

That is my experience as well. Since this is the case, the command reference 
needs
updating.

http://code.google.com/p/redis/wiki/RpushCommand

Which clearly indicates that "LPUSH <key> <value>" is the correct command 
syntax.

Original comment by jcaro...@gmail.com on 13 May 2009 at 1:54

[GoogleCodeExporter]

Hello,

the command reference is about the semantic of the command, not the low level
protocol implementing this semantic. In order to fix I think all is needed is 
to trap
that the argument is not a number and report an error that makes the user aware 
of
what is happening. 

Original comment by anti...@gmail.com on 15 May 2009 at 9:16

[GoogleCodeExporter]

Original comment by anti...@gmail.com on 22 May 2009 at 4:37

• Changed state: Invalid