Skip to content

Commit

Permalink
Fix conversion of numbers in lua args to redis args (#13115)
Browse files Browse the repository at this point in the history
Since lua_Number is not explicitly an integer or a double, we need to
make an effort
to convert it as an integer when that's possible, since the string could
later be used
in a context that doesn't support scientific notation (e.g. 1e9 instead
of 100000000).

Since fpconv_dtoa converts numbers with the equivalent of `%f` or `%e`,
which ever is shorter,
this would break if we try to pass a long integer number to a command
that takes integer.
we'll get an implicit conversion to string in Lua, and then the parsing
in getLongLongFromObjectOrReply will fail.

```
> eval "redis.call('hincrby', 'key', 'field', '1000000000')" 0
(nil)
> eval "redis.call('hincrby', 'key', 'field', tonumber('1000000000'))" 0
(error) ERR value is not an integer or out of range script: ac99c32e4daf7e300d593085b611de261954a946, on @user_script:1.
```

Switch to using ll2string if the number can be safely represented as a
long long.

The problem was introduced in #10587 (Redis 7.2).
closes #13113.

---------

Co-authored-by: Binbin <[email protected]>
Co-authored-by: debing.sun <[email protected]>
Co-authored-by: Oran Agra <[email protected]>
Signed-off-by: Anuragkillswitch <[email protected]>
  • Loading branch information
4 people authored and SoulPancake committed May 28, 2024
1 parent e5b2749 commit 868b8c0
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 2 deletions.
13 changes: 11 additions & 2 deletions src/script_lua.c
Original file line number Diff line number Diff line change
Expand Up @@ -819,8 +819,17 @@ static robj **luaArgsToRedisArgv(lua_State *lua, int *argc, int *argv_len) {
/* 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);
obj_len = fpconv_dtoa((double)num, dbuf);
dbuf[obj_len] = '\0';
/* Integer printing function is much faster, check if we can safely use it.
* Since lua_Number is not explicitly an integer or a double, we need to make an effort
* to convert it as an integer when that's possible, since the string could later be used
* in a context that doesn't support scientific notation (e.g. 1e9 instead of 100000000). */
long long lvalue;
if (double2ll((double)num, &lvalue))
obj_len = ll2string(dbuf, sizeof(dbuf), lvalue);
else {
obj_len = fpconv_dtoa((double)num, dbuf);
dbuf[obj_len] = '\0';
}
obj_s = dbuf;
} else {
obj_s = (char*)lua_tolstring(lua,j+1,&obj_len);
Expand Down
8 changes: 8 additions & 0 deletions tests/unit/scripting.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,14 @@ start_server {tags {"scripting"}} {
} 1 x
} {number 1}

test {EVAL - Lua number -> Redis integer conversion} {
r del hash
run_script {
local foo = redis.pcall('hincrby','hash','field',200000000)
return {type(foo),foo}
} 0
} {number 200000000}

test {EVAL - Redis bulk -> Lua type conversion} {
r set mykey myval
run_script {
Expand Down

0 comments on commit 868b8c0

Please sign in to comment.