Unverified Commit fd0ca747 authored by Yossi Gottlieb's avatar Yossi Gottlieb Committed by GitHub
Browse files

Fix occasional RM_Call() crashes. (#9805)

With dynamically growing argc (#9528), it is necessary to initialize
argv_len. Normally createClient() handles that, but in the case of a
module shared_client, this needs to be done explicitly.

This also addresses an issue with rewriteClientCommandArgument() which
doesn't properly handle the case where the new element extends beyond
argc but not beyond argv_len.
parent 14176484
......@@ -394,7 +394,7 @@ typedef struct RedisModuleKeyOptCtx {
void RM_FreeCallReply(RedisModuleCallReply *reply);
void RM_CloseKey(RedisModuleKey *key);
void autoMemoryCollect(RedisModuleCtx *ctx);
robj **moduleCreateArgvFromUserFormat(const char *cmdname, const char *fmt, int *argcp, int *flags, va_list ap);
robj **moduleCreateArgvFromUserFormat(const char *cmdname, const char *fmt, int *argcp, int *argvlenp, int *flags, va_list ap);
void moduleReplicateMultiIfNeeded(RedisModuleCtx *ctx);
void RM_ZsetRangeStop(RedisModuleKey *kp);
static void zsetKeyReset(RedisModuleKey *key);
......@@ -2400,7 +2400,7 @@ int RM_Replicate(RedisModuleCtx *ctx, const char *cmdname, const char *fmt, ...)
/* Create the client and dispatch the command. */
va_start(ap, fmt);
argv = moduleCreateArgvFromUserFormat(cmdname,fmt,&argc,&flags,ap);
argv = moduleCreateArgvFromUserFormat(cmdname,fmt,&argc,NULL,&flags,ap);
va_end(ap);
if (argv == NULL) return REDISMODULE_ERR;
......@@ -4765,9 +4765,9 @@ RedisModuleString *RM_CreateStringFromCallReply(RedisModuleCallReply *reply) {
}
}
/* Returns an array of robj pointers, and populates *argc with the number
* of items, by parsing the format specifier "fmt" as described for
* the RM_Call(), RM_Replicate() and other module APIs.
/* Returns an array of robj pointers, by parsing the format specifier "fmt" as described for
* the RM_Call(), RM_Replicate() and other module APIs. Populates *argcp with the number of
* items and *argvlenp with the length of the allocated argv.
*
* The integer pointed by 'flags' is populated with flags according
* to special modifiers in "fmt". For now only one exists:
......@@ -4781,7 +4781,7 @@ RedisModuleString *RM_CreateStringFromCallReply(RedisModuleCallReply *reply) {
*
* On error (format specifier error) NULL is returned and nothing is
* allocated. On success the argument vector is returned. */
robj **moduleCreateArgvFromUserFormat(const char *cmdname, const char *fmt, int *argcp, int *flags, va_list ap) {
robj **moduleCreateArgvFromUserFormat(const char *cmdname, const char *fmt, int *argcp, int *argvlenp, int *flags, va_list ap) {
int argc = 0, argv_size, j;
robj **argv = NULL;
......@@ -4847,7 +4847,8 @@ robj **moduleCreateArgvFromUserFormat(const char *cmdname, const char *fmt, int
}
p++;
}
*argcp = argc;
if (argcp) *argcp = argc;
if (argvlenp) *argvlenp = argv_size;
return argv;
fmterr:
......@@ -4909,14 +4910,14 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch
struct redisCommand *cmd;
client *c = NULL;
robj **argv = NULL;
int argc = 0, flags = 0;
int argc = 0, argv_len = 0, flags = 0;
va_list ap;
RedisModuleCallReply *reply = NULL;
int replicate = 0; /* Replicate this command? */
/* Handle arguments. */
va_start(ap, fmt);
argv = moduleCreateArgvFromUserFormat(cmdname,fmt,&argc,&flags,ap);
argv = moduleCreateArgvFromUserFormat(cmdname,fmt,&argc,&argv_len,&flags,ap);
replicate = flags & REDISMODULE_ARGV_REPLICATE;
va_end(ap);
......@@ -4941,6 +4942,7 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch
c->db = ctx->client->db;
c->argv = argv;
c->argc = argc;
c->argv_len = argv_len;
c->resp = 2;
if (flags & REDISMODULE_ARGV_RESP_3) {
c->resp = 3;
......@@ -5974,7 +5976,7 @@ void RM_EmitAOF(RedisModuleIO *io, const char *cmdname, const char *fmt, ...) {
/* Emit the arguments into the AOF in Redis protocol format. */
va_start(ap, fmt);
argv = moduleCreateArgvFromUserFormat(cmdname,fmt,&argc,&flags,ap);
argv = moduleCreateArgvFromUserFormat(cmdname,fmt,&argc,NULL,&flags,ap);
va_end(ap);
if (argv == NULL) {
serverLog(LL_WARNING,
......
......@@ -3276,9 +3276,16 @@ void replaceClientCommandVector(client *c, int argc, robj **argv) {
void rewriteClientCommandArgument(client *c, int i, robj *newval) {
robj *oldval;
retainOriginalCommandVector(c);
if (i >= c->argv_len) {
c->argv = zrealloc(c->argv,sizeof(robj*)*(i+1));
c->argc = c->argv_len = i+1;
/* We need to handle both extending beyond argc (just update it and
* initialize the new element) or beyond argv_len (realloc is needed).
*/
if (i >= c->argc) {
if (i >= c->argv_len) {
c->argv = zrealloc(c->argv,sizeof(robj*)*(i+1));
c->argv_len = i+1;
}
c->argc = i+1;
c->argv[i] = NULL;
}
oldval = c->argv[i];
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment