opensourcemirror
/
redis
mirror of https://github.com/antirez/redis.git
1
0
Fork 0
Code Issues Releases Wiki Activity

Add new RM_Call flags for script mode, no writes, and error replies. (#10372) 

The PR extends RM_Call with 3 new capabilities using new flags that
are given to RM_Call as part of the `fmt` argument.
It aims to assist modules that are getting a list of commands to be
executed from the user (not hard coded as part of the module logic),
think of a module that implements a new scripting language...

* `S` - Run the command in a script mode, this means that it will raise an
  error if a command which are not allowed inside a script (flaged with the
  `deny-script` flag) is invoked (like SHUTDOWN). In addition, on script mode,
  write commands are not allowed if there is not enough good replicas (as
  configured with `min-replicas-to-write`) and/or a disk error happened.

* `W` - no writes mode, Redis will reject any command that is marked with `write`
  flag. Again can be useful to modules that implement a new scripting language
  and wants to prevent any write commands.

* `E` - Return errors as RedisModuleCallReply. Today the errors that happened
  before the command was invoked (like unknown commands or acl error) return
  a NULL reply and set errno. This might be missing important information about
  the failure and it is also impossible to just pass the error to the user using
  RM_ReplyWithCallReply. This new flag allows you to get a RedisModuleCallReply
  object with the relevant error message and treat it as if it was an error that was
  raised by the command invocation.

Tests were added to verify the new code paths.

In addition small refactoring was done to share some code between modules,
scripts, and `processCommand` function:
1. `getAclErrorMessage` was added to `acl.c` to unified to log message extraction
  from the acl result
2. `checkGoodReplicasStatus` was added to `replication.c` to check the status of
  good replicas. It is used on `scriptVerifyWriteCommandAllow`, `RM_Call`, and
  `processCommand`.
3. `writeCommandsGetDiskErrorMessage` was added to `server.c` to get the error
  message on persistence failure. Again it is used on `scriptVerifyWriteCommandAllow`,
  `RM_Call`, and `processCommand`.
This commit is contained in:
Meir Shpilraien (Spielrein) 2022-03-22 14:13:28 +02:00 committed by GitHub
parent 08aed7e7dd
commit f3855a0930
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 256 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
src/acl.c
View File

@ -2472,6 +2472,22 @@ void addACLLogEntry(client *c, int reason, int context, int argpos, sds username
}
}
const char* getAclErrorMessage(int acl_res) {
/* Notice that a variant of this code also exists on aclCommand so
* it also need to be updated on changed. */
switch (acl_res) {
case ACL_DENIED_CMD:
return "can't run this command or subcommand";
case ACL_DENIED_KEY:
return "can't access at least one of the keys mentioned in the command arguments";
case ACL_DENIED_CHANNEL:
return "can't publish to the channel mentioned in the command";
default:
return "lacking the permissions for the command";
}
serverPanic("Reached deadcode on getAclErrorMessage");
}
/* =============================================================================
* ACL related commands
* ==========================================================================*/
@ -2863,6 +2879,8 @@ setuser_cleanup:
int idx;
int result = ACLCheckAllUserCommandPerm(u, cmd, c->argv + 3, c->argc - 3, &idx);
/* Notice that a variant of this code also exists on getAclErrorMessage so
* it also need to be updated on changed. */
if (result != ACL_OK) {
sds err = sdsempty();
if (result == ACL_DENIED_CMD) {

15
src/call_reply.c
View File

@ -525,3 +525,18 @@ CallReply *callReplyCreate(sds reply, list *deferred_error_list, void *private_d
res->deferred_error_list = deferred_error_list;
return res;
}
/* Create a new CallReply struct from the reply blob representing an error message.
* Automatically creating deferred_error_list and set a copy of the reply in it.
* Refer to callReplyCreate for detailed explanation. */
CallReply *callReplyCreateError(sds reply, void *private_data) {
sds err_buff = reply;
if (err_buff[0] != '-') {
err_buff = sdscatfmt(sdsempty(), "-ERR %S\r\n", reply);
sdsfree(reply);
}
list *deferred_error_list = listCreate();
listSetFreeMethod(deferred_error_list, (void (*)(void*))sdsfree);
listAddNodeTail(deferred_error_list, sdsnew(err_buff));
return callReplyCreate(err_buff, deferred_error_list, private_data);
}

1
src/call_reply.h
View File

@ -35,6 +35,7 @@
typedef struct CallReply CallReply;
CallReply *callReplyCreate(sds reply, list *deferred_error_list, void *private_data);
CallReply *callReplyCreateError(sds reply, void *private_data);
int callReplyType(CallReply *rep);
const char *callReplyGetString(CallReply *rep, size_t *len);
long long callReplyGetLongLong(CallReply *rep);

113
src/module.c
View File

@ -352,6 +352,9 @@ typedef struct RedisModuleServerInfoData {
#define REDISMODULE_ARGV_RESP_3 (1<<3)
#define REDISMODULE_ARGV_RESP_AUTO (1<<4)
#define REDISMODULE_ARGV_CHECK_ACL (1<<5)
#define REDISMODULE_ARGV_SCRIPT_MODE (1<<6)
#define REDISMODULE_ARGV_NO_WRITES (1<<7)
#define REDISMODULE_ARGV_CALL_REPLIES_AS_ERRORS (1<<8)
/* Determine whether Redis should signalModifiedKey implicitly.
* In case 'ctx' has no 'module' member (and therefore no module->options),
@ -5548,6 +5551,12 @@ robj **moduleCreateArgvFromUserFormat(const char *cmdname, const char *fmt, int
if (flags) (*flags) |= REDISMODULE_ARGV_RESP_AUTO;
} else if (*p == 'C') {
if (flags) (*flags) |= REDISMODULE_ARGV_CHECK_ACL;
} else if (*p == 'S') {
if (flags) (*flags) |= REDISMODULE_ARGV_SCRIPT_MODE;
} else if (*p == 'W') {
if (flags) (*flags) |= REDISMODULE_ARGV_NO_WRITES;
} else if (*p == 'E') {
if (flags) (*flags) |= REDISMODULE_ARGV_CALL_REPLIES_AS_ERRORS;
} else {
goto fmterr;
}
@ -5587,6 +5596,17 @@ fmterr:
* same as the client attached to the given RedisModuleCtx. This will
* probably used when you want to pass the reply directly to the client.
* * `C` -- Check if command can be executed according to ACL rules.
* * 'S' -- Run the command in a script mode, this means that it will raise
* an error if a command which are not allowed inside a script
* (flagged with the `deny-script` flag) is invoked (like SHUTDOWN).
* In addition, on script mode, write commands are not allowed if there are
* not enough good replicas (as configured with `min-replicas-to-write`)
* or when the server is unable to persist to the disk.
* * 'W' -- Do not allow to run any write command (flagged with the `write` flag).
* * 'E' -- Return error as RedisModuleCallReply. If there is an error before
* invoking the command, the error is returned using errno mechanism.
* This flag allows to get the error also as an error CallReply with
* relevant error message.
* * **...**: The actual arguments to the Redis command.
*
* On success a RedisModuleCallReply object is returned, otherwise
@ -5601,6 +5621,8 @@ fmterr:
* * ENETDOWN: operation in Cluster instance when cluster is down.
* * ENOTSUP: No ACL user for the specified module context
* * EACCES: Command cannot be executed, according to ACL rules
* * ENOSPC: Write command is not allowed
* * ESPIPE: Command not allowed on script mode
*
* Example code fragment:
*
@ -5620,11 +5642,13 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch
va_list ap;
RedisModuleCallReply *reply = NULL;
int replicate = 0; /* Replicate this command? */
int error_as_call_replies = 0; /* return errors as RedisModuleCallReply object */
/* Handle arguments. */
va_start(ap, fmt);
argv = moduleCreateArgvFromUserFormat(cmdname,fmt,&argc,&argv_len,&flags,ap);
replicate = flags & REDISMODULE_ARGV_REPLICATE;
error_as_call_replies = flags & REDISMODULE_ARGV_CALL_REPLIES_AS_ERRORS;
va_end(ap);
c = moduleAllocTempClient();
@ -5647,6 +5671,10 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch
/* We handle the above format error only when the client is setup so that
* we can free it normally. */
if (argv == NULL) {
/* We do not return a call reply here this is an error that should only
* be catch by the module indicating wrong fmt was given, the module should
* handle this error and decide how to continue. It is not an error that
* should be propagated to the user. */
errno = EBADF;
goto cleanup;
}
@ -5660,6 +5688,11 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch
cmd = lookupCommand(c->argv,c->argc);
if (!cmd) {
errno = ENOENT;
if (error_as_call_replies) {
sds msg = sdscatfmt(sdsempty(),"Unknown Redis "
"command '%S'.",c->argv[0]->ptr);
reply = callReplyCreateError(msg, ctx);
}
goto cleanup;
}
c->cmd = c->lastcmd = c->realcmd = cmd;
@ -5667,9 +5700,66 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch
/* Basic arity checks. */
if ((cmd->arity > 0 && cmd->arity != argc) || (argc < -cmd->arity)) {
errno = EINVAL;
if (error_as_call_replies) {
sds msg = sdscatfmt(sdsempty(), "Wrong number of "
"args calling Redis command '%S'.", c->cmd->fullname);
reply = callReplyCreateError(msg, ctx);
}
goto cleanup;
}
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) {
errno = ESPIPE;
if (error_as_call_replies) {
sds msg = sdscatfmt(sdsempty(), "command '%S' is not allowed on script mode", c->cmd->fullname);
reply = callReplyCreateError(msg, ctx);
}
goto cleanup;
}
}
if (cmd->flags & CMD_WRITE) {
if (flags & REDISMODULE_ARGV_NO_WRITES) {
errno = ENOSPC;
if (error_as_call_replies) {
sds msg = sdscatfmt(sdsempty(), "Write command '%S' was "
"called while write is not allowed.", c->cmd->fullname);
reply = callReplyCreateError(msg, ctx);
}
goto cleanup;
}
if (flags & REDISMODULE_ARGV_SCRIPT_MODE) {
/* 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 */
if (!checkGoodReplicasStatus()) {
errno = ENOSPC;
if (error_as_call_replies) {
sds msg = sdsdup(shared.noreplicaserr->ptr);
reply = callReplyCreateError(msg, ctx);
}
goto cleanup;
}
int deny_write_type = writeCommandsDeniedByDiskError();
if (deny_write_type != DISK_ERROR_TYPE_NONE) {
errno = ENOSPC;
if (error_as_call_replies) {
sds msg = writeCommandsGetDiskErrorMessage(deny_write_type);
reply = callReplyCreateError(msg, ctx);
}
goto cleanup;
}
}
}
/* Check if the user can run this command according to the current
* ACLs. */
if (flags & REDISMODULE_ARGV_CHECK_ACL) {
@ -5678,12 +5768,20 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch
if (ctx->client->user == NULL) {
errno = ENOTSUP;
if (error_as_call_replies) {
sds msg = sdsnew("acl verification failed, context is not attached to a client.");
reply = callReplyCreateError(msg, ctx);
}
goto cleanup;
}
acl_retval = ACLCheckAllUserCommandPerm(ctx->client->user,c->cmd,c->argv,c->argc,&acl_errpos);
if (acl_retval != ACL_OK) {
sds object = (acl_retval == ACL_DENIED_CMD) ? sdsdup(c->cmd->fullname) : sdsdup(c->argv[acl_errpos]->ptr);
addACLLogEntry(ctx->client, acl_retval, ACL_LOG_CTX_MODULE, -1, ctx->client->user->name, object);
if (error_as_call_replies) {
sds msg = sdscatfmt(sdsempty(), "acl verification failed, %s.", getAclErrorMessage(acl_retval));
reply = callReplyCreateError(msg, ctx);
}
errno = EACCES;
goto cleanup;
}
@ -5700,13 +5798,26 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch
if (getNodeByQuery(c,c->cmd,c->argv,c->argc,NULL,&error_code) !=
server.cluster->myself)
{
sds msg = NULL;
if (error_code == CLUSTER_REDIR_DOWN_RO_STATE) {
if (error_as_call_replies) {
msg = sdscatfmt(sdsempty(), "Can not execute a write command '%S' while the cluster is down and readonly", c->cmd->fullname);
}
errno = EROFS;
} else if (error_code == CLUSTER_REDIR_DOWN_STATE) {
if (error_as_call_replies) {
msg = sdscatfmt(sdsempty(), "Can not execute a command '%S' while the cluster is down", c->cmd->fullname);
}
errno = ENETDOWN;
} else {
if (error_as_call_replies) {
msg = sdsnew("Attempted to access a non local key in a cluster node");
}
errno = EPERM;
}
if (msg) {
reply = callReplyCreateError(msg, ctx);
}
goto cleanup;
}
}
@ -5748,9 +5859,9 @@ RedisModuleCallReply *RM_Call(RedisModuleCtx *ctx, const char *cmdname, const ch
}
reply = callReplyCreate(proto, c->deferred_reply_errors, ctx);
c->deferred_reply_errors = NULL; /* now the responsibility of the reply object. */
autoMemoryAdd(ctx,REDISMODULE_AM_REPLY,reply);
cleanup:
if (reply) autoMemoryAdd(ctx,REDISMODULE_AM_REPLY,reply);
if (ctx->module) ctx->module->in_call--;
moduleReleaseTempClient(c);
return reply;

8
src/replication.c
View File

@ -3335,6 +3335,14 @@ void refreshGoodSlavesCount(void) {
server.repl_good_slaves_count = good;
}
/* return true if status of good replicas is OK. otherwise false */
int checkGoodReplicasStatus(void) {
return server.masterhost || /* not a primary status should be OK */
!server.repl_min_slaves_max_lag || /* Min slave max lag not configured */
!server.repl_min_slaves_to_write || /* Min slave to write not configured */
server.repl_good_slaves_count >= server.repl_min_slaves_to_write; /* check if we have enough slaves */
}
/* ----------------------- SYNCHRONOUS REPLICATION --------------------------
* Redis synchronous replication design can be summarized in points:
*

35
src/script.c
View File

@ -312,25 +312,7 @@ static int scriptVerifyACL(client *c, sds *err) {
int acl_retval = ACLCheckAllPerm(c, &acl_errpos);
if (acl_retval != ACL_OK) {
addACLLogEntry(c,acl_retval,ACL_LOG_CTX_LUA,acl_errpos,NULL,NULL);
switch (acl_retval) {
case ACL_DENIED_CMD:
*err = sdsnew("The user executing the script can't run this "
"command or subcommand");
break;
case ACL_DENIED_KEY:
*err = sdsnew("The user executing the script can't access "
"at least one of the keys mentioned in the "
"command arguments");
break;
case ACL_DENIED_CHANNEL:
*err = sdsnew("The user executing the script can't publish "
"to the channel mentioned in the command");
break;
default:
*err = sdsnew("The user executing the script is lacking the "
"permissions for the command");
break;
}
*err = sdscatfmt(sdsempty(), "The user executing the script %s", getAclErrorMessage(acl_retval));
return C_ERR;
}
return C_OK;
@ -360,14 +342,7 @@ static int scriptVerifyWriteCommandAllow(scriptRunCtx *run_ctx, char **err) {
}
if (deny_write_type != DISK_ERROR_TYPE_NONE) {
if (deny_write_type == DISK_ERROR_TYPE_RDB) {
*err = sdsdup(shared.bgsaveerr->ptr);
} else {
*err = sdsempty();
*err = sdscatfmt(*err,
"-MISCONF Errors writing to the AOF file: %s\r\n",
strerror(server.aof_last_write_errno));
}
*err = writeCommandsGetDiskErrorMessage(deny_write_type);
return C_ERR;
}
@ -375,11 +350,7 @@ static int scriptVerifyWriteCommandAllow(scriptRunCtx *run_ctx, char **err) {
* user configured the min-slaves-to-write option. Note this only reachable
* for Eval scripts that didn't declare flags, see the other check in
* scriptPrepareForRun */
if (server.masterhost == NULL &&
server.repl_min_slaves_max_lag &&
server.repl_min_slaves_to_write &&
server.repl_good_slaves_count < server.repl_min_slaves_to_write)
{
if (!checkGoodReplicasStatus()) {
*err = sdsdup(shared.noreplicaserr->ptr);
return C_ERR;
}

45
src/server.c
View File

@ -3426,6 +3426,16 @@ void rejectCommand(client *c, robj *reply) {
}
}
void rejectCommandSds(client *c, sds s) {
if (c->cmd && c->cmd->proc == execCommand) {
execCommandAbort(c, s);
sdsfree(s);
} else {
/* The following frees 's'. */
addReplyErrorSds(c, s);
}
}
void rejectCommandFormat(client *c, const char *fmt, ...) {
if (c->cmd) c->cmd->rejected_calls++;
flagTransaction(c);
@ -3436,13 +3446,7 @@ void rejectCommandFormat(client *c, const char *fmt, ...) {
/* Make sure there are no newlines in the string, otherwise invalid protocol
* is emitted (The args come from the user, they may contain any character). */
sdsmapchars(s, "\r\n", " ", 2);
if (c->cmd && c->cmd->proc == execCommand) {
execCommandAbort(c, s);
sdsfree(s);
} else {
/* The following frees 's'. */
addReplyErrorSds(c, s);
}
rejectCommandSds(c, s);
}
/* This is called after a command in call, we can do some maintenance job in it. */
@ -3725,23 +3729,14 @@ int processCommand(client *c) {
server.masterhost == NULL &&
(is_write_command ||c->cmd->proc == pingCommand))
{
if (deny_write_type == DISK_ERROR_TYPE_RDB)
rejectCommand(c, shared.bgsaveerr);
else
rejectCommandFormat(c,
"-MISCONF Errors writing to the AOF file: %s",
strerror(server.aof_last_write_errno));
sds err = writeCommandsGetDiskErrorMessage(deny_write_type);
rejectCommandSds(c, err);
return C_OK;
}
/* Don't accept write commands if there are not enough good slaves and
* user configured the min-slaves-to-write option. */
if (server.masterhost == NULL &&
server.repl_min_slaves_to_write &&
server.repl_min_slaves_max_lag &&
is_write_command &&
server.repl_good_slaves_count < server.repl_min_slaves_to_write)
{
if (is_write_command && !checkGoodReplicasStatus()) {
rejectCommand(c, shared.noreplicaserr);
return C_OK;
}
@ -4166,6 +4161,18 @@ int writeCommandsDeniedByDiskError(void) {
return DISK_ERROR_TYPE_NONE;
}
sds writeCommandsGetDiskErrorMessage(int error_code) {
sds ret = NULL;
if (error_code == DISK_ERROR_TYPE_RDB) {
ret = sdsdup(shared.bgsaveerr->ptr);
} else {
ret = sdscatfmt(sdsempty(),
"-MISCONF Errors writing to the AOF file: %s",
strerror(server.aof_last_write_errno));
}
return ret;
}
/* The PING command. It works in a different way if the client is in
* in Pub/Sub mode. */
void pingCommand(client *c) {

3
src/server.h
View File

@ -2649,6 +2649,7 @@ void resizeReplicationBacklog();
void replicationSetMaster(char *ip, int port);
void replicationUnsetMaster(void);
void refreshGoodSlavesCount(void);
int checkGoodReplicasStatus(void);
void processClientsWaitingReplicas(void);
void unblockClientWaitingReplicas(client *c);
int replicationCountAcksByOffset(long long offset);
@ -2689,6 +2690,7 @@ int allPersistenceDisabled(void);
#define DISK_ERROR_TYPE_RDB 2 /* Don't accept writes: RDB errors. */
#define DISK_ERROR_TYPE_NONE 0 /* No problems, we can accept writes. */
int writeCommandsDeniedByDiskError(void);
sds writeCommandsGetDiskErrorMessage(int);
/* RDB persistence */
#include "rdb.h"
@ -2770,6 +2772,7 @@ void addReplyCommandCategories(client *c, struct redisCommand *cmd);
user *ACLCreateUnlinkedUser();
void ACLFreeUserAndKillClients(user *u);
void addACLLogEntry(client *c, int reason, int context, int argpos, sds username, sds object);
const char* getAclErrorMessage(int acl_res);
void ACLUpdateDefaultUserPassword(sds password);
/* Sorted sets data type */

20
tests/modules/aclcheck.c
View File

@ -136,6 +136,22 @@ int rm_call_aclcheck_cmd_module_user(RedisModuleCtx *ctx, RedisModuleString **ar
return res;
}
int rm_call_aclcheck_with_errors(RedisModuleCtx *ctx, RedisModuleString **argv, int argc){
REDISMODULE_NOT_USED(argv);
REDISMODULE_NOT_USED(argc);
if(argc < 2){
return RedisModule_WrongArity(ctx);
}
const char* cmd = RedisModule_StringPtrLen(argv[1], NULL);
RedisModuleCallReply* rep = RedisModule_Call(ctx, cmd, "vEC", argv + 2, argc - 2);
RedisModule_ReplyWithCallReply(ctx, rep);
RedisModule_FreeCallReply(rep);
return REDISMODULE_OK;
}
/* A wrap for RM_Call that pass the 'C' flag to do ACL check on the command. */
int rm_call_aclcheck(RedisModuleCtx *ctx, RedisModuleString **argv, int argc){
REDISMODULE_NOT_USED(argv);
@ -190,5 +206,9 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
"write",0,0,0) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_CreateCommand(ctx,"aclcheck.rm_call_with_errors", rm_call_aclcheck_with_errors,
"write",0,0,0) == REDISMODULE_ERR)
return REDISMODULE_ERR;
return REDISMODULE_OK;
}

31
tests/modules/basics.c
View File

@ -718,6 +718,25 @@ end:
/* Return 1 if the reply matches the specified string, otherwise log errors
* in the server log and return 0. */
int TestAssertErrorReply(RedisModuleCtx *ctx, RedisModuleCallReply *reply, char *str, size_t len) {
RedisModuleString *mystr, *expected;
if (RedisModule_CallReplyType(reply) != REDISMODULE_REPLY_ERROR) {
return 0;
}
mystr = RedisModule_CreateStringFromCallReply(reply);
expected = RedisModule_CreateString(ctx,str,len);
if (RedisModule_StringCompare(mystr,expected) != 0) {
const char *mystr_ptr = RedisModule_StringPtrLen(mystr,NULL);
const char *expected_ptr = RedisModule_StringPtrLen(expected,NULL);
RedisModule_Log(ctx,"warning",
"Unexpected Error reply reply '%s' (instead of '%s')",
mystr_ptr, expected_ptr);
return 0;
}
return 1;
}
int TestAssertStringReply(RedisModuleCtx *ctx, RedisModuleCallReply *reply, char *str, size_t len) {
RedisModuleString *mystr, *expected;
@ -846,6 +865,18 @@ int TestBasics(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
if (!TestAssertStringReply(ctx,RedisModule_CallReplyArrayElement(reply, 0),"test",4)) goto fail;
if (!TestAssertStringReply(ctx,RedisModule_CallReplyArrayElement(reply, 1),"1234",4)) goto fail;
T("foo", "E");
if (!TestAssertErrorReply(ctx,reply,"ERR Unknown Redis command 'foo'.",32)) goto fail;
T("set", "Ec", "x");
if (!TestAssertErrorReply(ctx,reply,"ERR Wrong number of args calling Redis command 'set'.",53)) goto fail;
T("shutdown", "SE");
if (!TestAssertErrorReply(ctx,reply,"ERR command 'shutdown' is not allowed on script mode",52)) goto fail;
T("set", "WEcc", "x", "1");
if (!TestAssertErrorReply(ctx,reply,"ERR Write command 'set' was called while write is not allowed.",62)) goto fail;
RedisModule_ReplyWithSimpleString(ctx,"ALL TESTS PASSED");
return REDISMODULE_OK;

2
tests/modules/blockedclient.c
View File

@ -195,7 +195,7 @@ int do_rm_call(RedisModuleCtx *ctx, RedisModuleString **argv, int argc){
const char* cmd = RedisModule_StringPtrLen(argv[1], NULL);
RedisModuleCallReply* rep = RedisModule_Call(ctx, cmd, "v", argv + 2, argc - 2);
RedisModuleCallReply* rep = RedisModule_Call(ctx, cmd, "Ev", argv + 2, argc - 2);
if(!rep){
RedisModule_ReplyWithError(ctx, "NULL reply returned");
}else{

5
tests/unit/moduleapi/aclcheck.tcl
View File

@ -62,10 +62,13 @@ start_server {tags {"modules acl"}} {
# rm call check for key permission (y: only WRITE)
assert_equal [r aclcheck.rm_call set y 5] OK
assert_error {*NOPERM*} {r aclcheck.rm_call set y 5 get}
assert_error {ERR acl verification failed, can't access at least one of the keys mentioned in the command arguments.} {r aclcheck.rm_call_with_errors set y 5 get}
# rm call check for key permission (z: only READ)
assert_error {*NOPERM*} {r aclcheck.rm_call set z 5}
assert_error {ERR acl verification failed, can't access at least one of the keys mentioned in the command arguments.} {r aclcheck.rm_call_with_errors set z 5}
assert_error {*NOPERM*} {r aclcheck.rm_call set z 6 get}
assert_error {ERR acl verification failed, can't access at least one of the keys mentioned in the command arguments.} {r aclcheck.rm_call_with_errors set z 6 get}
# verify that new log entry added
set entry [lindex [r ACL LOG] 0]
@ -77,6 +80,8 @@ start_server {tags {"modules acl"}} {
r acl setuser default -set
catch {r aclcheck.rm_call set x 5} e
assert_match {*NOPERM*} $e
catch {r aclcheck.rm_call_with_errors set x 5} e
assert_match {ERR acl verification failed, can't run this command or subcommand.} $e
# verify that new log entry added
set entry [lindex [r ACL LOG] 0]

8
tests/unit/moduleapi/blockedclient.tcl
View File

@ -184,17 +184,17 @@ start_server {tags {"modules"}} {
r config resetstat
# simple module command that replies with string error
assert_error "NULL reply returned" {r do_rm_call hgetalllll}
assert_equal [errorrstat NULL r] {count=1}
assert_error "ERR Unknown Redis command 'hgetalllll'." {r do_rm_call hgetalllll}
assert_equal [errorrstat ERR r] {count=1}
# module command that replies with string error from bg thread
assert_error "NULL reply returned" {r do_bg_rm_call hgetalllll}
assert_equal [errorrstat NULL r] {count=2}
assert_equal [errorrstat NULL r] {count=1}
# module command that returns an arity error
r do_rm_call set x x
assert_error "ERR wrong number of arguments for 'do_rm_call' command" {r do_rm_call}
assert_equal [errorrstat ERR r] {count=1}
assert_equal [errorrstat ERR r] {count=2}
# RM_Call that propagates an error
assert_error "WRONGTYPE*" {r do_rm_call hgetall x}

9
tests/unit/moduleapi/cluster.tcl
View File

@ -20,6 +20,7 @@ proc csi {args} {
set testmodule [file normalize tests/modules/blockonkeys.so]
set testmodule_nokey [file normalize tests/modules/blockonbackground.so]
set testmodule_blockedclient [file normalize tests/modules/blockedclient.so]
# make sure the test infra won't use SELECT
set old_singledb $::singledb
@ -44,6 +45,10 @@ start_server [list overrides $base_conf] {
$node2 module load $testmodule_nokey
$node3 module load $testmodule_nokey
$node1 module load $testmodule_blockedclient
$node2 module load $testmodule_blockedclient
$node3 module load $testmodule_blockedclient
test {Create 3 node cluster} {
exec src/redis-cli --cluster-yes --cluster create \
127.0.0.1:[srv 0 port] \
@ -194,6 +199,10 @@ start_server [list overrides $base_conf] {
assert_equal [s -1 blocked_clients] {0}
}
test "Verify command RM_Call is rejected when cluster is down" {
assert_error "ERR Can not execute a command 'set' while the cluster is down" {$node1 do_rm_call set x 1}
}
exec kill -SIGCONT $node3_pid
$node1_rd close
$node2_rd close