diff options
author | Shaya Potter <shaya@redislabs.com> | 2022-08-28 13:10:10 +0300 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-08-28 13:10:10 +0300 |
commit | bed6d759bcb1ddcdab76da1b46a120e2e610036b (patch) | |
tree | 9003af58459db53d8be6912bbb276c7ff5200413 /src | |
parent | 8945067544706951fd60007ad8cba8e8941b970d (diff) | |
download | redis-bed6d759bcb1ddcdab76da1b46a120e2e610036b.tar.gz |
Improve cmd_flags for script/functions in RM_Call (#11159)
When RM_Call was used with `M` (reject OOM), `W` (reject writes),
as well as `S` (rejecting stale or write commands in "Script mode"),
it would have only checked the command flags, but not the declared
script flag in case it's a command that runs a script.
Refactoring: extracts out similar code in server.c's processCommand
to be usable in RM_Call as well.
Diffstat (limited to 'src')
-rw-r--r-- | src/module.c | 17 | ||||
-rw-r--r-- | src/server.c | 31 | ||||
-rw-r--r-- | src/server.h | 1 |
3 files changed, 29 insertions, 20 deletions
diff --git a/src/module.c b/src/module.c index f0f49837b..776d74b65 100644 --- a/src/module.c +++ b/src/module.c @@ -592,6 +592,7 @@ void moduleReleaseTempClient(client *c) { c->bufpos = 0; c->flags = CLIENT_MODULE; c->user = NULL; /* Root user */ + c->cmd = c->lastcmd = c->realcmd = NULL; moduleTempClients[moduleTempClientCount++] = c; } @@ -5781,7 +5782,6 @@ fmterr: * This API is documented here: https://redis.io/topics/modules-intro */ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const char *fmt, ...) { - struct redisCommand *cmd; client *c = NULL; robj **argv = NULL; int argc = 0, argv_len = 0, flags = 0; @@ -5789,6 +5789,7 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch RedisModuleCallReply *reply = NULL; int replicate = 0; /* Replicate this command? */ int error_as_call_replies = 0; /* return errors as RedisModuleCallReply object */ + uint64_t cmd_flags; /* Handle arguments. */ va_start(ap, fmt); @@ -5831,7 +5832,7 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch /* Lookup command now, after filters had a chance to make modifications * if necessary. */ - cmd = c->cmd = c->lastcmd = c->realcmd = lookupCommand(c->argv,c->argc); + c->cmd = c->lastcmd = c->realcmd = lookupCommand(c->argv,c->argc); sds err; if (!commandCheckExistence(c, error_as_call_replies? &err : NULL)) { errno = ENOENT; @@ -5846,10 +5847,12 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch goto cleanup; } + cmd_flags = getCommandFlags(c); + if (flags & REDISMODULE_ARGV_SCRIPT_MODE) { /* Basically on script mode we want to only allow commands that can * be executed on scripts (CMD_NOSCRIPT is not set on the command flags) */ - if (cmd->flags & CMD_NOSCRIPT) { + if (cmd_flags & CMD_NOSCRIPT) { errno = ESPIPE; if (error_as_call_replies) { sds msg = sdscatfmt(sdsempty(), "command '%S' is not allowed on script mode", c->cmd->fullname); @@ -5860,7 +5863,7 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch } if (flags & REDISMODULE_ARGV_RESPECT_DENY_OOM) { - if (cmd->flags & CMD_DENYOOM) { + if (cmd_flags & CMD_DENYOOM) { int oom_state; if (ctx->flags & REDISMODULE_CTX_THREAD_SAFE) { /* On background thread we can not count on server.pre_command_oom_state. @@ -5882,7 +5885,7 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch } if (flags & REDISMODULE_ARGV_NO_WRITES) { - if (cmd->flags & CMD_WRITE) { + if (cmd_flags & CMD_WRITE) { errno = ENOSPC; if (error_as_call_replies) { sds msg = sdscatfmt(sdsempty(), "Write command '%S' was " @@ -5895,7 +5898,7 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch /* Script mode tests */ if (flags & REDISMODULE_ARGV_SCRIPT_MODE) { - if (cmd->flags & CMD_WRITE) { + if (cmd_flags & CMD_WRITE) { /* on script mode, if a command is a write command, * We will not run it if we encounter disk error * or we do not have enough replicas */ @@ -5932,7 +5935,7 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch } if (server.masterhost && server.repl_state != REPL_STATE_CONNECTED && - server.repl_serve_stale_data == 0 && !(cmd->flags & CMD_STALE)) { + server.repl_serve_stale_data == 0 && !(cmd_flags & CMD_STALE)) { errno = ESPIPE; if (error_as_call_replies) { sds msg = sdsdup(shared.masterdownerr->ptr); diff --git a/src/server.c b/src/server.c index 6b1d1194f..797c79fc3 100644 --- a/src/server.c +++ b/src/server.c @@ -3604,6 +3604,23 @@ int commandCheckArity(client *c, sds *err) { return 1; } +/* If we're executing a script, try to extract a set of command flags from + * it, in case it declared them. Note this is just an attempt, we don't yet + * know the script command is well formed.*/ +uint64_t getCommandFlags(client *c) { + uint64_t cmd_flags = c->cmd->flags; + + if (c->cmd->proc == fcallCommand || c->cmd->proc == fcallroCommand) { + cmd_flags = fcallGetCommandFlags(c, cmd_flags); + } else if (c->cmd->proc == evalCommand || c->cmd->proc == evalRoCommand || + c->cmd->proc == evalShaCommand || c->cmd->proc == evalShaRoCommand) + { + cmd_flags = evalGetCommandFlags(c, cmd_flags); + } + + return cmd_flags; +} + /* If this function gets called we already read a whole * command, arguments are in the client argv/argc fields. * processCommand() execute the command or prepare the @@ -3668,19 +3685,7 @@ int processCommand(client *c) { } } - /* If we're executing a script, try to extract a set of command flags from - * it, in case it declared them. Note this is just an attempt, we don't yet - * know the script command is well formed.*/ - uint64_t cmd_flags = c->cmd->flags; - if (c->cmd->proc == evalCommand || c->cmd->proc == evalShaCommand || - c->cmd->proc == evalRoCommand || c->cmd->proc == evalShaRoCommand || - c->cmd->proc == fcallCommand || c->cmd->proc == fcallroCommand) - { - if (c->cmd->proc == fcallCommand || c->cmd->proc == fcallroCommand) - cmd_flags = fcallGetCommandFlags(c, cmd_flags); - else - cmd_flags = evalGetCommandFlags(c, cmd_flags); - } + uint64_t cmd_flags = getCommandFlags(c); int is_read_command = (cmd_flags & CMD_READONLY) || (c->cmd->proc == execCommand && (c->mstate.cmd_flags & CMD_READONLY)); diff --git a/src/server.h b/src/server.h index 2d572478f..f6e3b2009 100644 --- a/src/server.h +++ b/src/server.h @@ -2864,6 +2864,7 @@ int zslLexValueLteMax(sds value, zlexrangespec *spec); int getMaxmemoryState(size_t *total, size_t *logical, size_t *tofree, float *level); size_t freeMemoryGetNotCountedMemory(); int overMaxmemoryAfterAlloc(size_t moremem); +uint64_t getCommandFlags(client *c); int processCommand(client *c); int processPendingCommandAndInputBuffer(client *c); void setupSignalHandlers(void); |