summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--src/acl.c18
-rw-r--r--src/call_reply.c15
-rw-r--r--src/call_reply.h1
-rw-r--r--src/module.c113
-rw-r--r--src/replication.c8
-rw-r--r--src/script.c35
-rw-r--r--src/server.c45
-rw-r--r--src/server.h3
-rw-r--r--tests/modules/aclcheck.c20
-rw-r--r--tests/modules/basics.c31
-rw-r--r--tests/modules/blockedclient.c2
-rw-r--r--tests/unit/moduleapi/aclcheck.tcl5
-rw-r--r--tests/unit/moduleapi/blockedclient.tcl8
-rw-r--r--tests/unit/moduleapi/cluster.tcl9
14 files changed, 256 insertions, 57 deletions
diff --git a/src/acl.c b/src/acl.c
index e4d7744a7..0054402b3 100644
--- a/src/acl.c
+++ b/src/acl.c
@@ -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) {
diff --git a/src/call_reply.c b/src/call_reply.c
index 3694db55e..759cd792a 100644
--- a/src/call_reply.c
+++ b/src/call_reply.c
@@ -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);
+}
diff --git a/src/call_reply.h b/src/call_reply.h
index ff98f7f5a..ff1c4ba3f 100644
--- a/src/call_reply.h
+++ b/src/call_reply.h
@@ -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);
diff --git a/src/module.c b/src/module.c
index ff928b950..727da4783 100644
--- a/src/module.c
+++ b/src/module.c
@@ -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;
diff --git a/src/replication.c b/src/replication.c
index c1671dab2..b93c512fc 100644
--- a/src/replication.c
+++ b/src/replication.c
@@ -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:
*
diff --git a/src/script.c b/src/script.c
index a34f53e66..990248c45 100644
--- a/src/script.c
+++ b/src/script.c
@@ -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;
}
diff --git a/src/server.c b/src/server.c
index 3bf04a770..531709d8a 100644
--- a/src/server.c
+++ b/src/server.c
@@ -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) {
diff --git a/src/server.h b/src/server.h
index 4f261e91a..5f5417de1 100644
--- a/src/server.h
+++ b/src/server.h
@@ -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 */
diff --git a/tests/modules/aclcheck.c b/tests/modules/aclcheck.c
index 0e9c9af29..8a9d468a6 100644
--- a/tests/modules/aclcheck.c
+++ b/tests/modules/aclcheck.c
@@ -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;
}
diff --git a/tests/modules/basics.c b/tests/modules/basics.c
index 4d639d682..ecd1b8852 100644
--- a/tests/modules/basics.c
+++ b/tests/modules/basics.c
@@ -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;
diff --git a/tests/modules/blockedclient.c b/tests/modules/blockedclient.c
index a2d7c6d00..9c2598a34 100644
--- a/tests/modules/blockedclient.c
+++ b/tests/modules/blockedclient.c
@@ -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{
diff --git a/tests/unit/moduleapi/aclcheck.tcl b/tests/unit/moduleapi/aclcheck.tcl
index 953f4bf05..5adf65371 100644
--- a/tests/unit/moduleapi/aclcheck.tcl
+++ b/tests/unit/moduleapi/aclcheck.tcl
@@ -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]
diff --git a/tests/unit/moduleapi/blockedclient.tcl b/tests/unit/moduleapi/blockedclient.tcl
index ea2d6f5a4..de3cf5946 100644
--- a/tests/unit/moduleapi/blockedclient.tcl
+++ b/tests/unit/moduleapi/blockedclient.tcl
@@ -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}
diff --git a/tests/unit/moduleapi/cluster.tcl b/tests/unit/moduleapi/cluster.tcl
index 3bd4977fd..f1238992d 100644
--- a/tests/unit/moduleapi/cluster.tcl
+++ b/tests/unit/moduleapi/cluster.tcl
@@ -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
View Lorry Controller Status