diff --git a/src/multi.c b/src/multi.c index 35ddf92af..a99c308be 100644 --- a/src/multi.c +++ b/src/multi.c @@ -127,7 +127,8 @@ void execCommandPropagateExec(client *c) { /* Aborts a transaction, with a specific error message. * The transaction is always aboarted with -EXECABORT so that the client knows * the server exited the multi state, but the actual reason for the abort is - * included too. */ + * included too. + * Note: 'error' may or may not end with \r\n. see addReplyErrorFormat. */ void execCommandAbort(client *c, sds error) { discardTransaction(c); diff --git a/src/networking.c b/src/networking.c index 295f75283..4a58cec40 100644 --- a/src/networking.c +++ b/src/networking.c @@ -357,14 +357,18 @@ void addReplyProto(client *c, const char *s, size_t len) { * * If the error code is already passed in the string 's', the error * code provided is used, otherwise the string "-ERR " for the generic - * error code is automatically added. */ + * error code is automatically added. + * Note that 's' must NOT end with \r\n. */ void addReplyErrorLength(client *c, const char *s, size_t len) { /* If the string already starts with "-..." then the error code * is provided by the caller. Otherwise we use "-ERR". */ if (!len || s[0] != '-') addReplyProto(c,"-ERR ",5); addReplyProto(c,s,len); addReplyProto(c,"\r\n",2); +} +/* Do some actions after an error reply was sent (Log if needed, updates stats, etc.) */ +void afterErrorReply(client *c, const char *s, size_t len) { /* Sometimes it could be normal that a slave replies to a master with * an error and this function gets called. Actually the error will never * be sent because addReply*() against master clients has no effect... @@ -390,10 +394,11 @@ void addReplyErrorLength(client *c, const char *s, size_t len) { from = "master"; } + if (len > 4096) len = 4096; char *cmdname = c->lastcmd ? c->lastcmd->name : ""; serverLog(LL_WARNING,"== CRITICAL == This %s is sending an error " - "to its %s: '%s' after processing the command " - "'%s'", from, to, s, cmdname); + "to its %s: '%.*s' after processing the command " + "'%s'", from, to, (int)len, s, cmdname); if (ctype == CLIENT_TYPE_MASTER && server.repl_backlog && server.repl_backlog_histlen > 0) { @@ -403,31 +408,39 @@ void addReplyErrorLength(client *c, const char *s, size_t len) { } } +/* The 'err' object is expected to start with -ERRORCODE and end with \r\n. + * Unlike addReplyErrorSds and others alike which rely on addReplyErrorLength. */ +void addReplyErrorObject(client *c, robj *err) { + addReply(c, err); + afterErrorReply(c, err->ptr, sdslen(err->ptr)-2); /* Ignore trailing \r\n */ +} + +/* See addReplyErrorLength for expectations from the input string. */ void addReplyError(client *c, const char *err) { addReplyErrorLength(c,err,strlen(err)); + afterErrorReply(c,err,strlen(err)); } -/* See addReplyErrorLength. - * Makes sure there are no newlines in the string, otherwise invalid protocol - * is emitted. */ -void addReplyErrorSafe(client *c, char *s, size_t len) { - size_t j; - /* Trim any newlines at the end (ones will be added by addReplyErrorLength) */ - while (s[len-1] == '\r' || s[len-1] == '\n') - len--; - /* Replace any newlines in the rest of the string with spaces. */ - for (j = 0; j < len; j++) { - if (s[j] == '\r' || s[j] == '\n') s[j] = ' '; - } - addReplyErrorLength(c,s,len); +/* See addReplyErrorLength for expectations from the input string. */ +void addReplyErrorSds(client *c, sds err) { + addReplyErrorLength(c,err,sdslen(err)); + afterErrorReply(c,err,sdslen(err)); } +/* See addReplyErrorLength for expectations from the formatted string. + * The formatted string is safe to contain \r and \n anywhere. */ void addReplyErrorFormat(client *c, const char *fmt, ...) { va_list ap; va_start(ap,fmt); sds s = sdscatvprintf(sdsempty(),fmt,ap); va_end(ap); - addReplyErrorSafe(c, s, sdslen(s)); + /* Trim any newlines at the end (ones will be added by addReplyErrorLength) */ + s = sdstrim(s, "\r\n"); + /* Make sure there are no newlines in the middle of the string, otherwise + * invalid protocol is emitted. */ + s = sdsmapchars(s, "\r\n", " ", 2); + addReplyErrorLength(c,s,sdslen(s)); + afterErrorReply(c,s,sdslen(s)); sdsfree(s); } diff --git a/src/server.c b/src/server.c index 6363a9b3b..4da4aeeec 100644 --- a/src/server.c +++ b/src/server.c @@ -3498,14 +3498,15 @@ void call(client *c, int flags) { /* Used when a command that is ready for execution needs to be rejected, due to * varios pre-execution checks. it returns the appropriate error to the client. * If there's a transaction is flags it as dirty, and if the command is EXEC, - * it aborts the transaction. */ + * it aborts the transaction. + * Note: 'reply' is expected to end with \r\n */ void rejectCommand(client *c, robj *reply) { flagTransaction(c); if (c->cmd && c->cmd->proc == execCommand) { execCommandAbort(c, reply->ptr); } else { /* using addReplyError* rather than addReply so that the error can be logged. */ - addReplyErrorSafe(c, reply->ptr, sdslen(reply->ptr)); + addReplyErrorObject(c, reply); } } @@ -3515,10 +3516,13 @@ void rejectCommandFormat(client *c, const char *fmt, ...) { va_start(ap,fmt); sds s = sdscatvprintf(sdsempty(),fmt,ap); va_end(ap); + /* 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); } else { - addReplyErrorSafe(c, s, sdslen(s)); + addReplyErrorSds(c, s); } sdsfree(s); } @@ -3688,7 +3692,7 @@ int processCommand(client *c) { rejectCommand(c, shared.bgsaveerr); else rejectCommandFormat(c, - "-MISCONF Errors writing to the AOF file: %s\r\n", + "-MISCONF Errors writing to the AOF file: %s", strerror(server.aof_last_write_errno)); return C_OK; } diff --git a/src/server.h b/src/server.h index 97f663076..53b4fcce3 100644 --- a/src/server.h +++ b/src/server.h @@ -1657,7 +1657,8 @@ void addReplyBulkLongLong(client *c, long long ll); void addReply(client *c, robj *obj); void addReplySds(client *c, sds s); void addReplyBulkSds(client *c, sds s); -void addReplyErrorSafe(client *c, char *s, size_t len); +void addReplyErrorObject(client *c, robj *err); +void addReplyErrorSds(client *c, sds err); void addReplyError(client *c, const char *err); void addReplyStatus(client *c, const char *status); void addReplyDouble(client *c, double d);