Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug on command while there was nil parameter #29

Closed
coanor opened this issue Sep 4, 2013 · 2 comments
Closed

Bug on command while there was nil parameter #29

coanor opened this issue Sep 4, 2013 · 2 comments

Comments

@coanor
Copy link

coanor commented Sep 4, 2013

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
Copy link

rafis commented Dec 13, 2013

I confirm this issue, please fix.

@nrk
Copy link
Owner

nrk commented Dec 14, 2013

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"

nrk added a commit that referenced this issue Dec 14, 2013
When passing nil values in command arguments they are now converted to
empty strings explicitly, on the other hand when nil is the last value
then it will not be passed to the serializer due to how Lua works with
nil values in tables, but at least the client will not hang and Redis
will return a -ERR complaining about the wrong number of arguments for
the given command.

Fixes #29.
@nrk nrk closed this as completed in 7f2faa6 Dec 14, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants