diff options
-rw-r--r-- | CHANGELOG.md | 51 | ||||
-rw-r--r-- | Makefile | 11 | ||||
-rw-r--r-- | async.c | 38 | ||||
-rw-r--r-- | async.h | 1 | ||||
-rw-r--r-- | fmacros.h | 30 | ||||
-rw-r--r-- | hiredis.c | 38 | ||||
-rw-r--r-- | hiredis.h | 30 | ||||
-rw-r--r-- | net.c | 22 | ||||
-rw-r--r-- | net.h | 4 | ||||
-rw-r--r-- | read.c | 157 | ||||
-rw-r--r-- | test.c | 100 |
11 files changed, 322 insertions, 160 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index f40ec53..a7fe3ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,51 @@ ### 1.0.0 (unreleased) -**Fixes**: +**BREAKING CHANGES**: + +* Bulk and multi-bulk lengths less than -1 or greater than `LLONG_MAX` are now + protocol errors. This is consistent with the RESP specification. On 32-bit + platforms, the upper bound is lowered to `SIZE_MAX`. + +* Change `redisReply.len` to `size_t`, as it denotes the the size of a string + User code should compare this to `size_t` values as well. If it was used to + compare to other values, casting might be necessary or can be removed, if + casting was applied before. + +### 0.14.0 (2018-09-25) + +* Make string2ll static to fix conflict with Redis (Tom Lee [c3188b]) +* Use -dynamiclib instead of -shared for OSX (Ryan Schmidt [a65537]) +* Use string2ll from Redis w/added tests (Michael Grunder [7bef04, 60f622]) +* Makefile - OSX compilation fixes (Ryan Schmidt [881fcb, 0e9af8]) +* Remove redundant NULL checks (Justin Brewer [54acc8, 58e6b8]) +* Fix bulk and multi-bulk length truncation (Justin Brewer [109197]) +* Fix SIGSEGV in OpenBSD by checking for NULL before calling freeaddrinfo (Justin Brewer [546d94]) +* Several POSIX compatibility fixes (Justin Brewer [bbeab8, 49bbaa, d1c1b6]) +* Makefile - Compatibility fixes (Dimitri Vorobiev [3238cf, 12a9d1]) +* Makefile - Fix make install on FreeBSD (Zach Shipko [a2ef2b]) +* Makefile - don't assume $(INSTALL) is cp (Igor Gnatenko [725a96]) +* Separate side-effect causing function from assert and small cleanup (amallia [b46413, 3c3234]) +* Don't send negative values to `__redisAsyncCommand` (Frederik Deweerdt [706129]) +* Fix leak if setsockopt fails (Frederik Deweerdt [e21c9c]) +* Fix libevent leak (zfz [515228]) +* Clean up GCC warning (Ichito Nagata [2ec774]) +* Keep track of errno in `__redisSetErrorFromErrno()` as snprintf may use it (Jin Qing [25cd88]) +* Solaris compilation fix (Donald Whyte [41b07d]) +* Reorder linker arguments when building examples (Tustfarm-heart [06eedd]) +* Keep track of subscriptions in case of rapid subscribe/unsubscribe (Hyungjin Kim [073dc8, be76c5, d46999]) +* libuv use after free fix (Paul Scott [cbb956]) +* Properly close socket fd on reconnect attempt (WSL [64d1ec]) +* Skip valgrind in OSX tests (Jan-Erik Rediger [9deb78]) +* Various updates for Travis testing OSX (Ted Nyman [fa3774, 16a459, bc0ea5]) +* Update libevent (Chris Xin [386802]) +* Change sds.h for building in C++ projects (Ali Volkan ATLI [f5b32e]) +* Use proper format specifier in redisFormatSdsCommandArgv (Paulino Huerta, Jan-Erik Rediger [360a06, 8655a6]) +* Better handling of NULL reply in example code (Jan-Erik Rediger [1b8ed3]) +* Prevent overflow when formatting an error (Jan-Erik Rediger [0335cb]) +* Compatibility fix for strerror_r (Tom Lee [bb1747]) +* Properly detect integer parse/overflow errors (Justin Brewer [93421f]) +* Adds CI for Windows and cygwin fixes (owent, [6c53d6, 6c3e40]) * Catch a buffer overflow when formatting the error message * Import latest upstream sds. This breaks applications that are linked against the old hiredis v0.13 * Fix warnings, when compiled with -Wshadow @@ -9,11 +53,6 @@ **BREAKING CHANGES**: -* Change `redisReply.len` to `size_t`, as it denotes the the size of a string - -User code should compare this to `size_t` values as well. -If it was used to compare to other values, casting might be necessary or can be removed, if casting was applied before. - * Remove backwards compatibility macro's This removes the following old function aliases, use the new name now: @@ -41,7 +41,7 @@ CXX:=$(shell sh -c 'type $${CXX%% *} >/dev/null 2>/dev/null && echo $(CXX) || ec OPTIMIZATION?=-O3 WARNINGS=-Wall -W -Wstrict-prototypes -Wwrite-strings DEBUG_FLAGS?= -g -ggdb -REAL_CFLAGS=$(OPTIMIZATION) -fPIC $(CFLAGS) $(WARNINGS) $(DEBUG_FLAGS) +REAL_CFLAGS=$(OPTIMIZATION) -fPIC $(CPPFLAGS) $(CFLAGS) $(WARNINGS) $(DEBUG_FLAGS) REAL_LDFLAGS=$(LDFLAGS) DYLIBSUFFIX=so @@ -58,12 +58,11 @@ uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo not') ifeq ($(uname_S),SunOS) REAL_LDFLAGS+= -ldl -lnsl -lsocket DYLIB_MAKE_CMD=$(CC) -G -o $(DYLIBNAME) -h $(DYLIB_MINOR_NAME) $(LDFLAGS) - INSTALL= cp -r endif ifeq ($(uname_S),Darwin) DYLIBSUFFIX=dylib DYLIB_MINOR_NAME=$(LIBNAME).$(HIREDIS_SONAME).$(DYLIBSUFFIX) - DYLIB_MAKE_CMD=$(CC) -shared -Wl,-install_name,$(DYLIB_MINOR_NAME) -o $(DYLIBNAME) $(LDFLAGS) + DYLIB_MAKE_CMD=$(CC) -dynamiclib -Wl,-install_name,$(PREFIX)/$(LIBRARY_PATH)/$(DYLIB_MINOR_NAME) -o $(DYLIBNAME) $(LDFLAGS) endif all: $(DYLIBNAME) $(STLIBNAME) hiredis-test $(PKGCONFNAME) @@ -161,11 +160,7 @@ clean: dep: $(CC) -MM *.c -ifeq ($(uname_S),$(filter $(uname_S),SunOS OpenBSD)) - INSTALL?= cp -r -endif - -INSTALL?= cp -a +INSTALL?= cp -pPR $(PKGCONFNAME): hiredis.h @echo "Generating $@ for pkgconfig..." @@ -365,6 +365,7 @@ void redisAsyncDisconnect(redisAsyncContext *ac) { static int __redisGetSubscribeCallback(redisAsyncContext *ac, redisReply *reply, redisCallback *dstcb) { redisContext *c = &(ac->c); dict *callbacks; + redisCallback *cb; dictEntry *de; int pvariant; char *stype; @@ -388,16 +389,28 @@ static int __redisGetSubscribeCallback(redisAsyncContext *ac, redisReply *reply, sname = sdsnewlen(reply->element[1]->str,reply->element[1]->len); de = dictFind(callbacks,sname); if (de != NULL) { - memcpy(dstcb,dictGetEntryVal(de),sizeof(*dstcb)); + cb = dictGetEntryVal(de); + + /* If this is an subscribe reply decrease pending counter. */ + if (strcasecmp(stype+pvariant,"subscribe") == 0) { + cb->pending_subs -= 1; + } + + memcpy(dstcb,cb,sizeof(*dstcb)); /* If this is an unsubscribe message, remove it. */ if (strcasecmp(stype+pvariant,"unsubscribe") == 0) { - dictDelete(callbacks,sname); + if (cb->pending_subs == 0) + dictDelete(callbacks,sname); /* If this was the last unsubscribe message, revert to * non-subscribe mode. */ assert(reply->element[2]->type == REDIS_REPLY_INTEGER); - if (reply->element[2]->integer == 0) + + /* Unset subscribed flag only when no pipelined pending subscribe. */ + if (reply->element[2]->integer == 0 + && dictSize(ac->sub.channels) == 0 + && dictSize(ac->sub.patterns) == 0) c->flags &= ~REDIS_SUBSCRIBED; } } @@ -411,7 +424,7 @@ static int __redisGetSubscribeCallback(redisAsyncContext *ac, redisReply *reply, void redisProcessCallbacks(redisAsyncContext *ac) { redisContext *c = &(ac->c); - redisCallback cb = {NULL, NULL, NULL}; + redisCallback cb = {NULL, NULL, 0, NULL}; void *reply = NULL; int status; @@ -584,6 +597,9 @@ static const char *nextArgument(const char *start, const char **str, size_t *len static int __redisAsyncCommand(redisAsyncContext *ac, redisCallbackFn *fn, void *privdata, const char *cmd, size_t len) { redisContext *c = &(ac->c); redisCallback cb; + struct dict *cbdict; + dictEntry *de; + redisCallback *existcb; int pvariant, hasnext; const char *cstr, *astr; size_t clen, alen; @@ -597,6 +613,7 @@ static int __redisAsyncCommand(redisAsyncContext *ac, redisCallbackFn *fn, void /* Setup callback */ cb.fn = fn; cb.privdata = privdata; + cb.pending_subs = 1; /* Find out which command will be appended. */ p = nextArgument(cmd,&cstr,&clen); @@ -613,9 +630,18 @@ static int __redisAsyncCommand(redisAsyncContext *ac, redisCallbackFn *fn, void while ((p = nextArgument(p,&astr,&alen)) != NULL) { sname = sdsnewlen(astr,alen); if (pvariant) - ret = dictReplace(ac->sub.patterns,sname,&cb); + cbdict = ac->sub.patterns; else - ret = dictReplace(ac->sub.channels,sname,&cb); + cbdict = ac->sub.channels; + + de = dictFind(cbdict,sname); + + if (de != NULL) { + existcb = dictGetEntryVal(de); + cb.pending_subs = existcb->pending_subs + 1; + } + + ret = dictReplace(cbdict,sname,&cb); if (ret == 0) sdsfree(sname); } @@ -45,6 +45,7 @@ typedef void (redisCallbackFn)(struct redisAsyncContext*, void*, void*); typedef struct redisCallback { struct redisCallback *next; /* simple singly linked list */ redisCallbackFn *fn; + int pending_subs; void *privdata; } redisCallback; @@ -1,38 +1,12 @@ #ifndef __HIREDIS_FMACRO_H #define __HIREDIS_FMACRO_H -#if defined(__linux__) -#define _BSD_SOURCE -#define _DEFAULT_SOURCE -#endif - -#if defined(__CYGWIN__) -#include <sys/cdefs.h> -#endif - -#if defined(__linux__) || defined(__OpenBSD__) || defined(__NetBSD__) #define _XOPEN_SOURCE 600 -#elif defined(__APPLE__) && defined(__MACH__) -#define _XOPEN_SOURCE -#elif defined(__FreeBSD__) -// intentionally left blank, don't define _XOPEN_SOURCE -#elif defined(AIX) -// intentionally left blank, don't define _XOPEN_SOURCE -#else -#define _XOPEN_SOURCE -#endif - -#if defined(__sun__) #define _POSIX_C_SOURCE 200112L -#endif #if defined(__APPLE__) && defined(__MACH__) -#define _OSX -#endif - -#ifndef AIX -# define _XOPEN_SOURCE_EXTENDED 1 -# define _ALL_SOURCE +/* Enable TCP_KEEPALIVE */ +#define _DARWIN_C_SOURCE #endif #endif @@ -84,16 +84,14 @@ void freeReplyObject(void *reply) { case REDIS_REPLY_ARRAY: if (r->element != NULL) { for (j = 0; j < r->elements; j++) - if (r->element[j] != NULL) - freeReplyObject(r->element[j]); + freeReplyObject(r->element[j]); free(r->element); } break; case REDIS_REPLY_ERROR: case REDIS_REPLY_STATUS: case REDIS_REPLY_STRING: - if (r->str != NULL) - free(r->str); + free(r->str); break; } free(r); @@ -432,11 +430,7 @@ cleanup: } sdsfree(curarg); - - /* No need to check cmd since it is the last statement that can fail, - * but do it anyway to be as defensive as possible. */ - if (cmd != NULL) - free(cmd); + free(cmd); return error_type; } @@ -581,7 +575,7 @@ void __redisSetError(redisContext *c, int type, const char *str) { } else { /* Only REDIS_ERR_IO may lack a description! */ assert(type == REDIS_ERR_IO); - __redis_strerror_r(errno, c->errstr, sizeof(c->errstr)); + strerror_r(errno, c->errstr, sizeof(c->errstr)); } } @@ -596,14 +590,8 @@ static redisContext *redisContextInit(void) { if (c == NULL) return NULL; - c->err = 0; - c->errstr[0] = '\0'; c->obuf = sdsempty(); c->reader = redisReaderCreate(); - c->tcp.host = NULL; - c->tcp.source_addr = NULL; - c->unix_sock.path = NULL; - c->timeout = NULL; if (c->obuf == NULL || c->reader == NULL) { redisFree(c); @@ -618,18 +606,12 @@ void redisFree(redisContext *c) { return; if (c->fd > 0) close(c->fd); - if (c->obuf != NULL) - sdsfree(c->obuf); - if (c->reader != NULL) - redisReaderFree(c->reader); - if (c->tcp.host) - free(c->tcp.host); - if (c->tcp.source_addr) - free(c->tcp.source_addr); - if (c->unix_sock.path) - free(c->unix_sock.path); - if (c->timeout) - free(c->timeout); + sdsfree(c->obuf); + redisReaderFree(c->reader); + free(c->tcp.host); + free(c->tcp.source_addr); + free(c->unix_sock.path); + free(c->timeout); free(c); } @@ -40,9 +40,9 @@ #include "sds.h" /* for sds */ #define HIREDIS_MAJOR 0 -#define HIREDIS_MINOR 13 -#define HIREDIS_PATCH 3 -#define HIREDIS_SONAME 0.13 +#define HIREDIS_MINOR 14 +#define HIREDIS_PATCH 0 +#define HIREDIS_SONAME 0.14 /* Connection type can be blocking or non-blocking and is set in the * least significant bit of the flags field in redisContext. */ @@ -80,30 +80,6 @@ * SO_REUSEADDR is being used. */ #define REDIS_CONNECT_RETRIES 10 -/* strerror_r has two completely different prototypes and behaviors - * depending on system issues, so we need to operate on the error buffer - * differently depending on which strerror_r we're using. */ -#ifndef _GNU_SOURCE -/* "regular" POSIX strerror_r that does the right thing. */ -#define __redis_strerror_r(errno, buf, len) \ - do { \ - strerror_r((errno), (buf), (len)); \ - } while (0) -#else -/* "bad" GNU strerror_r we need to clean up after. */ -#define __redis_strerror_r(errno, buf, len) \ - do { \ - char *err_str = strerror_r((errno), (buf), (len)); \ - /* If return value _isn't_ the start of the buffer we passed in, \ - * then GNU strerror_r returned an internal static buffer and we \ - * need to copy the result into our private buffer. */ \ - if (err_str != (buf)) { \ - strncpy((buf), err_str, ((len) - 1)); \ - (buf)[(len)-1] = '\0'; \ - } \ - } while (0) -#endif - #ifdef __cplusplus extern "C" { #endif @@ -71,7 +71,7 @@ static void __redisSetErrorFromErrno(redisContext *c, int type, const char *pref if (prefix != NULL) len = snprintf(buf,sizeof(buf),"%s: ",prefix); - __redis_strerror_r(errorno, (char *)(buf + len), sizeof(buf) - len); + strerror_r(errorno, (char *)(buf + len), sizeof(buf) - len); __redisSetError(c,type,buf); } @@ -136,7 +136,7 @@ int redisKeepAlive(redisContext *c, int interval) { val = interval; -#ifdef _OSX +#if defined(__APPLE__) && defined(__MACH__) if (setsockopt(fd, IPPROTO_TCP, TCP_KEEPALIVE, &val, sizeof(val)) < 0) { __redisSetError(c,REDIS_ERR_OTHER,strerror(errno)); return REDIS_ERR; @@ -285,8 +285,7 @@ static int _redisContextConnectTcp(redisContext *c, const char *addr, int port, * This is a bit ugly, but atleast it works and doesn't leak memory. **/ if (c->tcp.host != addr) { - if (c->tcp.host) - free(c->tcp.host); + free(c->tcp.host); c->tcp.host = strdup(addr); } @@ -299,8 +298,7 @@ static int _redisContextConnectTcp(redisContext *c, const char *addr, int port, memcpy(c->timeout, timeout, sizeof(struct timeval)); } } else { - if (c->timeout) - free(c->timeout); + free(c->timeout); c->timeout = NULL; } @@ -412,7 +410,10 @@ addrretry: error: rv = REDIS_ERR; end: - freeaddrinfo(servinfo); + if(servinfo) { + freeaddrinfo(servinfo); + } + return rv; // Need to return REDIS_OK if alright } @@ -432,7 +433,7 @@ int redisContextConnectUnix(redisContext *c, const char *path, const struct time struct sockaddr_un sa; long timeout_msec = -1; - if (redisCreateSocket(c,AF_LOCAL) < 0) + if (redisCreateSocket(c,AF_UNIX) < 0) return REDIS_ERR; if (redisSetBlocking(c,0) != REDIS_OK) return REDIS_ERR; @@ -449,15 +450,14 @@ int redisContextConnectUnix(redisContext *c, const char *path, const struct time memcpy(c->timeout, timeout, sizeof(struct timeval)); } } else { - if (c->timeout) - free(c->timeout); + free(c->timeout); c->timeout = NULL; } if (redisContextTimeoutMsec(c,&timeout_msec) != REDIS_OK) return REDIS_ERR; - sa.sun_family = AF_LOCAL; + sa.sun_family = AF_UNIX; strncpy(sa.sun_path,path,sizeof(sa.sun_path)-1); if (connect(c->fd, (struct sockaddr*)&sa, sizeof(sa)) == -1) { if (errno == EINPROGRESS && !blocking) { @@ -37,10 +37,6 @@ #include "hiredis.h" -#if defined(__sun) || defined(AIX) -#define AF_LOCAL AF_UNIX -#endif - int redisCheckSocketError(redisContext *c); int redisContextSetTimeout(redisContext *c, const struct timeval tv); int redisContextConnectTcp(redisContext *c, const char *addr, int port, const struct timeval *timeout); @@ -39,6 +39,7 @@ #include <assert.h> #include <errno.h> #include <ctype.h> +#include <limits.h> #include "read.h" #include "sds.h" @@ -52,11 +53,9 @@ static void __redisReaderSetError(redisReader *r, int type, const char *str) { } /* Clear input buffer on errors. */ - if (r->buf != NULL) { - sdsfree(r->buf); - r->buf = NULL; - r->pos = r->len = 0; - } + sdsfree(r->buf); + r->buf = NULL; + r->pos = r->len = 0; /* Reset task stack. */ r->ridx = -1; @@ -143,33 +142,79 @@ static char *seekNewline(char *s, size_t len) { return NULL; } -/* Read a long long value starting at *s, under the assumption that it will be - * terminated by \r\n. Ambiguously returns -1 for unexpected input. */ -static long long readLongLong(char *s) { - long long v = 0; - int dec, mult = 1; - char c; - - if (*s == '-') { - mult = -1; - s++; - } else if (*s == '+') { - mult = 1; - s++; +/* Convert a string into a long long. Returns REDIS_OK if the string could be + * parsed into a (non-overflowing) long long, REDIS_ERR otherwise. The value + * will be set to the parsed value when appropriate. + * + * Note that this function demands that the string strictly represents + * a long long: no spaces or other characters before or after the string + * representing the number are accepted, nor zeroes at the start if not + * for the string "0" representing the zero number. + * + * Because of its strictness, it is safe to use this function to check if + * you can convert a string into a long long, and obtain back the string + * from the number without any loss in the string representation. */ +static int string2ll(const char *s, size_t slen, long long *value) { + const char *p = s; + size_t plen = 0; + int negative = 0; + unsigned long long v; + + if (plen == slen) + return REDIS_ERR; + + /* Special case: first and only digit is 0. */ + if (slen == 1 && p[0] == '0') { + if (value != NULL) *value = 0; + return REDIS_OK; } - while ((c = *(s++)) != '\r') { - dec = c - '0'; - if (dec >= 0 && dec < 10) { - v *= 10; - v += dec; - } else { - /* Should not happen... */ - return -1; - } + if (p[0] == '-') { + negative = 1; + p++; plen++; + + /* Abort on only a negative sign. */ + if (plen == slen) + return REDIS_ERR; + } + + /* First digit should be 1-9, otherwise the string should just be 0. */ + if (p[0] >= '1' && p[0] <= '9') { + v = p[0]-'0'; + p++; plen++; + } else if (p[0] == '0' && slen == 1) { + *value = 0; + return REDIS_OK; + } else { + return REDIS_ERR; } - return mult*v; + while (plen < slen && p[0] >= '0' && p[0] <= '9') { + if (v > (ULLONG_MAX / 10)) /* Overflow. */ + return REDIS_ERR; + v *= 10; + + if (v > (ULLONG_MAX - (p[0]-'0'))) /* Overflow. */ + return REDIS_ERR; + v += p[0]-'0'; + + p++; plen++; + } + + /* Return if not all bytes were used. */ + if (plen < slen) + return REDIS_ERR; + + if (negative) { + if (v > ((unsigned long long)(-(LLONG_MIN+1))+1)) /* Overflow. */ + return REDIS_ERR; + if (value != NULL) *value = -v; + } else { + if (v > LLONG_MAX) /* Overflow. */ + return REDIS_ERR; + if (value != NULL) *value = v; + } + return REDIS_OK; } static char *readLine(redisReader *r, int *_len) { @@ -220,10 +265,17 @@ static int processLineItem(redisReader *r) { if ((p = readLine(r,&len)) != NULL) { if (cur->type == REDIS_REPLY_INTEGER) { - if (r->fn && r->fn->createInteger) - obj = r->fn->createInteger(cur,readLongLong(p)); - else + if (r->fn && r->fn->createInteger) { + long long v; + if (string2ll(p, len, &v) == REDIS_ERR) { + __redisReaderSetError(r,REDIS_ERR_PROTOCOL, + "Bad integer value"); + return REDIS_ERR; + } + obj = r->fn->createInteger(cur,v); + } else { obj = (void*)REDIS_REPLY_INTEGER; + } } else { /* Type will be error or status. */ if (r->fn && r->fn->createString) @@ -250,7 +302,7 @@ static int processBulkItem(redisReader *r) { redisReadTask *cur = &(r->rstack[r->ridx]); void *obj = NULL; char *p, *s; - long len; + long long len; unsigned long bytelen; int success = 0; @@ -259,9 +311,20 @@ static int processBulkItem(redisReader *r) { if (s != NULL) { p = r->buf+r->pos; bytelen = s-(r->buf+r->pos)+2; /* include \r\n */ - len = readLongLong(p); - if (len < 0) { + if (string2ll(p, bytelen - 2, &len) == REDIS_ERR) { + __redisReaderSetError(r,REDIS_ERR_PROTOCOL, + "Bad bulk string length"); + return REDIS_ERR; + } + + if (len < -1 || (LLONG_MAX > SIZE_MAX && len > (long long)SIZE_MAX)) { + __redisReaderSetError(r,REDIS_ERR_PROTOCOL, + "Bulk string length out of range"); + return REDIS_ERR; + } + + if (len == -1) { /* The nil object can always be created. */ if (r->fn && r->fn->createNil) obj = r->fn->createNil(cur); @@ -303,8 +366,8 @@ static int processMultiBulkItem(redisReader *r) { redisReadTask *cur = &(r->rstack[r->ridx]); void *obj; char *p; - long elements; - int root = 0; + long long elements; + int root = 0, len; /* Set error for nested multi bulks with depth > 7 */ if (r->ridx == 8) { @@ -313,10 +376,21 @@ static int processMultiBulkItem(redisReader *r) { return REDIS_ERR; } - if ((p = readLine(r,NULL)) != NULL) { - elements = readLongLong(p); + if ((p = readLine(r,&len)) != NULL) { + if (string2ll(p, len, &elements) == REDIS_ERR) { + __redisReaderSetError(r,REDIS_ERR_PROTOCOL, + "Bad multi-bulk length"); + return REDIS_ERR; + } + root = (r->ridx == 0); + if (elements < -1 || elements > INT_MAX) { + __redisReaderSetError(r,REDIS_ERR_PROTOCOL, + "Multi-bulk length out of range"); + return REDIS_ERR; + } + if (elements == -1) { if (r->fn && r->fn->createNil) obj = r->fn->createNil(cur); @@ -420,8 +494,6 @@ redisReader *redisReaderCreateWithFunctions(redisReplyObjectFunctions *fn) { if (r == NULL) return NULL; - r->err = 0; - r->errstr[0] = '\0'; r->fn = fn; r->buf = sdsempty(); r->maxbuf = REDIS_READER_MAX_BUF; @@ -435,10 +507,11 @@ redisReader *redisReaderCreateWithFunctions(redisReplyObjectFunctions *fn) { } void redisReaderFree(redisReader *r) { + if (r == NULL) + return; if (r->reply != NULL && r->fn && r->fn->freeObject) r->fn->freeObject(r->reply); - if (r->buf != NULL) - sdsfree(r->buf); + sdsfree(r->buf); free(r); } @@ -302,6 +302,82 @@ static void test_reply_reader(void) { strncasecmp(reader->errstr,"No support for",14) == 0); redisReaderFree(reader); + test("Correctly parses LLONG_MAX: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, ":9223372036854775807\r\n",22); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_OK && + ((redisReply*)reply)->type == REDIS_REPLY_INTEGER && + ((redisReply*)reply)->integer == LLONG_MAX); + freeReplyObject(reply); + redisReaderFree(reader); + + test("Set error when > LLONG_MAX: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, ":9223372036854775808\r\n",22); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_ERR && + strcasecmp(reader->errstr,"Bad integer value") == 0); + freeReplyObject(reply); + redisReaderFree(reader); + + test("Correctly parses LLONG_MIN: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, ":-9223372036854775808\r\n",23); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_OK && + ((redisReply*)reply)->type == REDIS_REPLY_INTEGER && + ((redisReply*)reply)->integer == LLONG_MIN); + freeReplyObject(reply); + redisReaderFree(reader); + + test("Set error when < LLONG_MIN: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, ":-9223372036854775809\r\n",23); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_ERR && + strcasecmp(reader->errstr,"Bad integer value") == 0); + freeReplyObject(reply); + redisReaderFree(reader); + + test("Set error when array < -1: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, "*-2\r\n+asdf\r\n",12); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_ERR && + strcasecmp(reader->errstr,"Multi-bulk length out of range") == 0); + freeReplyObject(reply); + redisReaderFree(reader); + + test("Set error when bulk < -1: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, "$-2\r\nasdf\r\n",11); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_ERR && + strcasecmp(reader->errstr,"Bulk string length out of range") == 0); + freeReplyObject(reply); + redisReaderFree(reader); + + test("Set error when array > INT_MAX: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, "*9223372036854775807\r\n+asdf\r\n",29); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_ERR && + strcasecmp(reader->errstr,"Multi-bulk length out of range") == 0); + freeReplyObject(reply); + redisReaderFree(reader); + +#if LLONG_MAX > SIZE_MAX + test("Set error when bulk > SIZE_MAX: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, "$9223372036854775807\r\nasdf\r\n",28); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_ERR && + strcasecmp(reader->errstr,"Bulk string length out of range") == 0); + freeReplyObject(reply); + redisReaderFree(reader); +#endif + test("Works with NULL functions for reply: "); reader = redisReaderCreate(); reader->fn = NULL; @@ -616,6 +692,17 @@ static void test_throughput(struct config config) { free(replies); printf("\t(%dx LRANGE with 500 elements: %.3fs)\n", num, (t2-t1)/1000000.0); + replies = malloc(sizeof(redisReply*)*num); + t1 = usec(); + for (i = 0; i < num; i++) { + replies[i] = redisCommand(c, "INCRBY incrkey %d", 1000000); + assert(replies[i] != NULL && replies[i]->type == REDIS_REPLY_INTEGER); + } + t2 = usec(); + for (i = 0; i < num; i++) freeReplyObject(replies[i]); + free(replies); + printf("\t(%dx INCRBY: %.3fs)\n", num, (t2-t1)/1000000.0); + num = 10000; replies = malloc(sizeof(redisReply*)*num); for (i = 0; i < num; i++) @@ -644,6 +731,19 @@ static void test_throughput(struct config config) { free(replies); printf("\t(%dx LRANGE with 500 elements (pipelined): %.3fs)\n", num, (t2-t1)/1000000.0); + replies = malloc(sizeof(redisReply*)*num); + for (i = 0; i < num; i++) + redisAppendCommand(c,"INCRBY incrkey %d", 1000000); + t1 = usec(); + for (i = 0; i < num; i++) { + assert(redisGetReply(c, (void*)&replies[i]) == REDIS_OK); + assert(replies[i] != NULL && replies[i]->type == REDIS_REPLY_INTEGER); + } + t2 = usec(); + for (i = 0; i < num; i++) freeReplyObject(replies[i]); + free(replies); + printf("\t(%dx INCRBY (pipelined): %.3fs)\n", num, (t2-t1)/1000000.0); + disconnect(c, 0); } |