nrk / redis-lua

A Lua client library for the redis key value storage system.
MIT License
733 stars 239 forks source link

Bug on command while there was `nil` parameter #29

Closed coanor closed 10 years ago

coanor commented 11 years ago

I have wrapped the redis command as:

-- Lua 5.1.5
local function redis_cmd(cli, cmd, ...)
    local arg = {...}

    -- XXX: add here to avoid the first arg is `nil`,
    -- but do nothing with multi-cmd mode
    if #arg > 1 then    
        print(unpack(arg))
        assert(arg[1])
    end

    cmd = string.lower(cmd)
    local ok, res = nil, nil
    if #arg > 0 then
        ok, res = pcall(cli[cmd], cli, unpack(arg))
    else
        ok, res = pcall(cli[cmd], cli)
    end

    return ok, res
end

and I call redis_cmd like that:

local redis_cli = redis.connect('192.168.1.103', 6379)
local ok, res = redis_cmd(redis_cli, 'set', nil, 1)

while the command need 2 or more parameters(i.e., lrange, set, and so on), and the key is nil, the redis_cmd will hang there without any error message. And I added a if judgement(XXX:) on this situation, but while I call multi-set like this:

local ok, res = redis_cmd(redis_cli, 'mset', 'key', 2, nil, 1)

it still hang, I think my if statement do nothing with this situation. Is it a bug in redis.lua?


Add just for more information: After testing in LuaJIT 2.0.1, the form:

local ok, res = redis_cmd(redis_cli, 'mset', 'key', 2, nil, 1)

will not hang and return true, but the form still hang.

local ok, res = redis_cmd(redis_cli, 'set', nil, 1)

Git update hist:

    commit 61e77607a39321017a7399be644ab7d33ce5c707
    Author: Pierre Chapuis <catwell-github@catwell.info>
    Date:   Mon Aug 19 16:25:51 2013 +0200

        Return score as a number when parsing response from ZINCRBY.
rafis commented 10 years ago

I confirm this issue, please fix.

nrk commented 10 years ago

This problem could be fixed by changing these two lines of the command serialization in a way that the iteration of command arguments wouldn't break anymore when nil is the first value and and all nils would be converted to empty strings. Basically, something like this:

for i = 1, argsn do
    local s_argument = tostring(args[i] or '')

Please note that Redis actually accepts empty strings as keys, so redis.set(nil, "foobar") won't return any error and the server will store the string "foobar" into the key "" (empty string). This can be easily tested by doing:

client:set("", "foobar")
local value = client:get("")

print(value)    -- prints "foobar"