zeekling
/
hiredis
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix redisAppendCommand error result 

Previously, redisvAppendCommand() would return OOM even with formatting
errors.  Now we use OTHER with an error string telling the user the
error was formatting related, not memory related.

This also fixes a potentially worse bug where were would pass error result
of -1 as an actual length to another function taking an unsigned length,
which would result in crash/overallocation/errors.  Now for that case,
we just return an error immediately and stop processing the command.

Fixes #177
This commit is contained in:
Matt Stancliff 2015-01-05 12:13:32 -05:00
parent 0c9ff5bb03
commit 6a00a4643b
2 changed files with 32 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
async.c
View File

@ -650,6 +650,11 @@ int redisvAsyncCommand(redisAsyncContext *ac, redisCallbackFn *fn, void *privdat
int len;
int status;
len = redisvFormatCommand(&cmd,format,ap);
/* We don't want to pass -1 or -2 to future functions as a length. */
if (len < 0)
return REDIS_ERR;
status = __redisAsyncCommand(ac,fn,privdata,cmd,len);
free(cmd);
return status;

36
hiredis.c
View File

@ -695,6 +695,7 @@ int redisvFormatCommand(char **target, const char *format, va_list ap) {
char **curargv = NULL, **newargv = NULL;
int argc = 0;
int totlen = 0;
int error_type = 0; /* 0 = no error; -1 = memory error; -2 = format error */
int j;
/* Abort if there is not target to set */
@ -711,19 +712,19 @@ int redisvFormatCommand(char **target, const char *format, va_list ap) {
if (*c == ' ') {
if (touched) {
newargv = realloc(curargv,sizeof(char*)*(argc+1));
if (newargv == NULL) goto err;
if (newargv == NULL) goto memory_err;
curargv = newargv;
curargv[argc++] = curarg;
totlen += bulklen(sdslen(curarg));
/* curarg is put in argv so it can be overwritten. */
curarg = sdsempty();
if (curarg == NULL) goto err;
if (curarg == NULL) goto memory_err;
touched = 0;
}
} else {
newarg = sdscatlen(curarg,c,1);
if (newarg == NULL) goto err;
if (newarg == NULL) goto memory_err;
curarg = newarg;
touched = 1;
}
@ -829,7 +830,7 @@ int redisvFormatCommand(char **target, const char *format, va_list ap) {
fmt_invalid:
va_end(_cpy);
goto err;
goto format_err;
fmt_valid:
_l = (_p+1)-c;
@ -848,7 +849,7 @@ int redisvFormatCommand(char **target, const char *format, va_list ap) {
}
}
if (newarg == NULL) goto err;
if (newarg == NULL) goto memory_err;
curarg = newarg;
touched = 1;
@ -860,7 +861,7 @@ int redisvFormatCommand(char **target, const char *format, va_list ap) {
/* Add the last argument if needed */
if (touched) {
newargv = realloc(curargv,sizeof(char*)*(argc+1));
if (newargv == NULL) goto err;
if (newargv == NULL) goto memory_err;
curargv = newargv;
curargv[argc++] = curarg;
totlen += bulklen(sdslen(curarg));
@ -876,7 +877,7 @@ int redisvFormatCommand(char **target, const char *format, va_list ap) {
/* Build the command at protocol level */
cmd = malloc(totlen+1);
if (cmd == NULL) goto err;
if (cmd == NULL) goto memory_err;
pos = sprintf(cmd,"*%d\r\n",argc);
for (j = 0; j < argc; j++) {
@ -894,7 +895,15 @@ int redisvFormatCommand(char **target, const char *format, va_list ap) {
*target = cmd;
return totlen;
err:
format_err:
error_type = -2;
goto cleanup;
memory_err:
error_type = -1;
goto cleanup;
cleanup:
if (curargv) {
while(argc--)
sdsfree(curargv[argc]);
@ -908,7 +917,7 @@ err:
if (cmd != NULL)
free(cmd);
return -1;
return error_type;
}
/* Format a command according to the Redis protocol. This function
@ -929,6 +938,12 @@ int redisFormatCommand(char **target, const char *format, ...) {
va_start(ap,format);
len = redisvFormatCommand(target,format,ap);
va_end(ap);
/* The API says "-1" means bad result, but we now also return "-2" in some
* cases. Force the return value to always be -1. */
if (len < 0)
len = -1;
return len;
}
@ -1354,6 +1369,9 @@ int redisvAppendCommand(redisContext *c, const char *format, va_list ap) {
if (len == -1) {
__redisSetError(c,REDIS_ERR_OOM,"Out of memory");
return REDIS_ERR;
} else if (len == -2) {
__redisSetError(c,REDIS_ERR_OTHER,"Invalid format string");
return REDIS_ERR;
}
if (__redisAppendCommand(c,cmd,len) != REDIS_OK) {