diff options
author | Shaya Potter <shaya@redislabs.com> | 2023-03-20 08:04:13 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-03-20 08:04:13 +0200 |
commit | 6cf8fc08f59cd34cfcf364ab6a233f100611078c (patch) | |
tree | b496e35be3d0fb18e50557f8244274d367c99f56 /tests | |
parent | bbf364a442b463f3cf0d310ad052727de68a493a (diff) | |
download | redis-6cf8fc08f59cd34cfcf364ab6a233f100611078c.tar.gz |
Don't run command filter on blocked command reprocessing (#11895)
Previously we would run the module command filters even upon blocked
command reprocessing. This could modify the command, and it's args.
This is irrelevant in the context of a command being reprocessed (it already
went through the filters), as well as breaks the crashed command lookup
that exists in the case of a reprocessed command.
fixes #11894.
Co-authored-by: Oran Agra <oran@redislabs.com>
Diffstat (limited to 'tests')
-rw-r--r-- | tests/modules/commandfilter.c | 31 | ||||
-rw-r--r-- | tests/unit/moduleapi/commandfilter.tcl | 25 |
2 files changed, 55 insertions, 1 deletions
diff --git a/tests/modules/commandfilter.c b/tests/modules/commandfilter.c index 8ed779714..043b6805f 100644 --- a/tests/modules/commandfilter.c +++ b/tests/modules/commandfilter.c @@ -10,7 +10,7 @@ static const char retained_command_name[] = "commandfilter.retained"; static const char unregister_command_name[] = "commandfilter.unregister"; static int in_log_command = 0; -static RedisModuleCommandFilter *filter; +static RedisModuleCommandFilter *filter, *filter1; static RedisModuleString *retained; int CommandFilter_UnregisterCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) @@ -89,6 +89,32 @@ int CommandFilter_LogCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int return REDISMODULE_OK; } +/* Filter to protect against Bug #11894 reappearing + * + * ensures that the filter is only run the first time through, and not on reprocessing + */ +void CommandFilter_BlmoveSwap(RedisModuleCommandFilterCtx *filter) +{ + if (RedisModule_CommandFilterArgsCount(filter) != 6) + return; + + RedisModuleString *arg = RedisModule_CommandFilterArgGet(filter, 0); + size_t arg_len; + const char *arg_str = RedisModule_StringPtrLen(arg, &arg_len); + + if (arg_len != 6 || strncmp(arg_str, "blmove", 6)) + return; + + /* + * Swapping directional args (right/left) from source and destination. + * need to hold here, can't push into the ArgReplace func, as it will cause other to freed -> use after free + */ + RedisModuleString *dir1 = RedisModule_HoldString(NULL, RedisModule_CommandFilterArgGet(filter, 3)); + RedisModuleString *dir2 = RedisModule_HoldString(NULL, RedisModule_CommandFilterArgGet(filter, 4)); + RedisModule_CommandFilterArgReplace(filter, 3, dir2); + RedisModule_CommandFilterArgReplace(filter, 4, dir1); +} + void CommandFilter_CommandFilter(RedisModuleCommandFilterCtx *filter) { if (in_log_command) return; /* don't process our own RM_Call() from CommandFilter_LogCommand() */ @@ -170,6 +196,9 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) noself ? REDISMODULE_CMDFILTER_NOSELF : 0)) == NULL) return REDISMODULE_ERR; + if ((filter1 = RedisModule_RegisterCommandFilter(ctx, CommandFilter_BlmoveSwap, 0)) == NULL) + return REDISMODULE_ERR; + return REDISMODULE_OK; } diff --git a/tests/unit/moduleapi/commandfilter.tcl b/tests/unit/moduleapi/commandfilter.tcl index 427609d3e..d6193b1ca 100644 --- a/tests/unit/moduleapi/commandfilter.tcl +++ b/tests/unit/moduleapi/commandfilter.tcl @@ -116,3 +116,28 @@ test {RM_CommandFilterArgInsert and script argv caching} { } } +# previously, there was a bug that command filters would be rerun (which would cause args to swap back) +# this test is meant to protect against that bug +test {Blocking Commands don't run through command filter when reprocessed} { + start_server {tags {"modules"}} { + r module load $testmodule log-key 0 + + r del list1{t} + r del list2{t} + + r lpush list2{t} a b c d e + + set rd [redis_deferring_client] + # we're asking to pop from the left, but the command filter swaps the two arguments, + # if it didn't swap it, we would end up with e d c b a 5 (5 being the left most of the following lpush) + # but since we swap the arguments, we end up with 1 e d c b a (1 being the right most of it). + # if the command filter would run again on unblock, they would be swapped back. + $rd blmove list1{t} list2{t} left right 0 + wait_for_blocked_client + r lpush list1{t} 1 2 3 4 5 + # validate that we moved the correct element with the swapped args + assert_equal [$rd read] 1 + # validate that we moved the correct elements to the correct side of the list + assert_equal [r lpop list2{t}] 1 + } +} |