Skip to content

Commit

Permalink
Scripting: Fix regression from redis#1118
Browse files Browse the repository at this point in the history
The new check-for-number behavior of Lua arguments broke
users who use large strings of just integers.

The Lua number check would convert the string to a number, but
that breaks user data because
Lua numbers have limited precision compared to an arbitrarily
precise number wrapped in a string.

Regression fixed and new test added.

Fixes redis#1118 again.
  • Loading branch information
mattsta committed Jun 10, 2014
1 parent 8ef79e7 commit 76efe12
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 1 deletion.
7 changes: 6 additions & 1 deletion src/scripting.c
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,11 @@ void luaSortArray(lua_State *lua) {
lua_pop(lua,1); /* Stack: array (sorted) */
}

int luaReallyIsString(lua_State *L, int index) {
int t = lua_type(L, index);
return t == LUA_TSTRING;
}

#define LUA_CMD_OBJCACHE_SIZE 32
#define LUA_CMD_OBJCACHE_MAX_LEN 64
int luaRedisGenericCommand(lua_State *lua, int raise_error) {
Expand Down Expand Up @@ -234,7 +239,7 @@ int luaRedisGenericCommand(lua_State *lua, int raise_error) {
size_t obj_len;
char dbuf[64];

if (lua_isnumber(lua,j+1)) {
if (!luaReallyIsString(lua, j+1) && lua_isnumber(lua, j+1)) {
/* We can't use lua_tolstring() for number -> string conversion
* since Lua uses a format specifier that loses precision. */
lua_Number num = lua_tonumber(lua,j+1);
Expand Down
7 changes: 7 additions & 0 deletions tests/unit/scripting.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,13 @@ start_server {tags {"scripting"}} {
return redis.call("get","foo")
} 0
} {9007199254740991}

test {String containing number precision test (regression of issue #1118)} {
r eval {
redis.call("set", "key", "12039611435714932082")
return redis.call("get", "key")
} 0
} {12039611435714932082}
}

# Start a new server since the last test in this stanza will kill the
Expand Down

0 comments on commit 76efe12

Please sign in to comment.