summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOran Agra <oran@redislabs.com>2021-01-05 08:29:20 +0200
committerOran Agra <oran@redislabs.com>2021-01-12 16:25:37 +0200
commitec56906bd6724bd2e59b9e996653ff574153bd91 (patch)
treed9953cd7dce92439067bea62aa54fc69e2ffa919
parent23d299234c8618c8d7b0e6f33d931c8f080e6fef (diff)
downloadredis-ec56906bd6724bd2e59b9e996653ff574153bd91.tar.gz
Fix wrong order of key/value in Lua map response (#8266)
When a Lua script returns a map to redis (a feature which was added in redis 6 together with RESP3), it would have returned the value first and the key second. If the client was using RESP2, it was getting them out of order, and if the client was in RESP3, it was getting a map of value => key. This was happening regardless of the Lua script using redis.setresp(3) or not. This also affects a case where the script was returning a map which it got from from redis by doing something like: redis.setresp(3); return redis.call() This fix is a breaking change for redis 6.0 users who happened to rely on the wrong order (either ones that used redis.setresp(3), or ones that returned a map explicitly). This commit also includes other two changes in the tests: 1. The test suite now handles RESP3 maps as dicts rather than nested lists 2. Remove some redundant (duplicate) tests from tracking.tcl (cherry picked from commit 2017407b4d1d19a91af1e7c0b199f2c1775dbaf9)
-rw-r--r--src/scripting.c4
-rw-r--r--tests/support/redis.tcl19
-rw-r--r--tests/unit/scripting.tcl24
3 files changed, 45 insertions, 2 deletions
diff --git a/src/scripting.c b/src/scripting.c
index 6beb6cdbf..a75871ad3 100644
--- a/src/scripting.c
+++ b/src/scripting.c
@@ -410,9 +410,9 @@ void luaReplyToRedisReply(client *c, lua_State *lua) {
lua_pushnil(lua); /* Use nil to start iteration. */
while (lua_next(lua,-2)) {
/* Stack now: table, key, value */
- luaReplyToRedisReply(c, lua); /* Return value. */
- lua_pushvalue(lua,-1); /* Dup key before consuming. */
+ lua_pushvalue(lua,-2); /* Dup key before consuming. */
luaReplyToRedisReply(c, lua); /* Return key. */
+ luaReplyToRedisReply(c, lua); /* Return value. */
/* Stack now: table, key. */
maplen++;
}
diff --git a/tests/support/redis.tcl b/tests/support/redis.tcl
index a90ac7f29..566750c0d 100644
--- a/tests/support/redis.tcl
+++ b/tests/support/redis.tcl
@@ -210,6 +210,24 @@ proc ::redis::redis_multi_bulk_read {id fd} {
return $l
}
+proc ::redis::redis_read_map {id fd} {
+ set count [redis_read_line $fd]
+ if {$count == -1} return {}
+ set d {}
+ set err {}
+ for {set i 0} {$i < $count} {incr i} {
+ if {[catch {
+ set k [redis_read_reply $id $fd] ; # key
+ set v [redis_read_reply $id $fd] ; # value
+ dict set d $k $v
+ } e] && $err eq {}} {
+ set err $e
+ }
+ }
+ if {$err ne {}} {return -code error $err}
+ return $d
+}
+
proc ::redis::redis_read_line fd {
string trim [gets $fd]
}
@@ -222,6 +240,7 @@ proc ::redis::redis_read_reply {id fd} {
- {return -code error [redis_read_line $fd]}
$ {redis_bulk_read $fd}
* {redis_multi_bulk_read $id $fd}
+ % {redis_read_map $id $fd}
default {
if {$type eq {}} {
set ::redis::fd($id) {}
diff --git a/tests/unit/scripting.tcl b/tests/unit/scripting.tcl
index 6bcba4c3f..e3cd2f87f 100644
--- a/tests/unit/scripting.tcl
+++ b/tests/unit/scripting.tcl
@@ -533,6 +533,30 @@ start_server {tags {"scripting"}} {
} e
set e
} {*wrong number*}
+
+ test {Script with RESP3 map} {
+ set expected_dict [dict create field value]
+ set expected_list [list field value]
+
+ # Sanity test for RESP3 without scripts
+ r HELLO 3
+ r hset hash field value
+ set res [r hgetall hash]
+ assert_equal $res $expected_dict
+
+ # Test RESP3 client with script in both RESP2 and RESP3 modes
+ set res [r eval {redis.setresp(3); return redis.call('hgetall', KEYS[1])} 1 hash]
+ assert_equal $res $expected_dict
+ set res [r eval {redis.setresp(2); return redis.call('hgetall', KEYS[1])} 1 hash]
+ assert_equal $res $expected_list
+
+ # Test RESP2 client with script in both RESP2 and RESP3 modes
+ r HELLO 2
+ set res [r eval {redis.setresp(3); return redis.call('hgetall', KEYS[1])} 1 hash]
+ assert_equal $res $expected_list
+ set res [r eval {redis.setresp(2); return redis.call('hgetall', KEYS[1])} 1 hash]
+ assert_equal $res $expected_list
+ }
}
# Start a new server since the last test in this stanza will kill the