summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--CHANGELOG.md51
-rw-r--r--Makefile11
-rw-r--r--async.c38
-rw-r--r--async.h1
-rw-r--r--fmacros.h30
-rw-r--r--hiredis.c38
-rw-r--r--hiredis.h30
-rw-r--r--net.c22
-rw-r--r--net.h4
-rw-r--r--read.c157
-rw-r--r--test.c100
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:
diff --git a/Makefile b/Makefile
index 28a7849..06ca994 100644
--- a/Makefile
+++ b/Makefile
@@ -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..."
diff --git a/async.c b/async.c
index 0670aa7..cb5b841 100644
--- a/async.c
+++ b/async.c
@@ -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);
}
diff --git a/async.h b/async.h
index 59cbf46..e69d840 100644
--- a/async.h
+++ b/async.h
@@ -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;
diff --git a/fmacros.h b/fmacros.h
index 4cdbc13..3227faa 100644
--- a/fmacros.h
+++ b/fmacros.h
@@ -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
diff --git a/hiredis.c b/hiredis.c
index b344962..98f43c9 100644
--- a/hiredis.c
+++ b/hiredis.c
@@ -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);
}
diff --git a/hiredis.h b/hiredis.h
index 77d5797..bd53e7e 100644
--- a/hiredis.h
+++ b/hiredis.h
@@ -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
diff --git a/net.c b/net.c
index 33d9a82..5d50f7c 100644
--- a/net.c
+++ b/net.c
@@ -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) {
diff --git a/net.h b/net.h
index fc8ddd3..d9dc362 100644
--- a/net.h
+++ b/net.h
@@ -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);
diff --git a/read.c b/read.c
index 061bbda..cc21267 100644
--- a/read.c
+++ b/read.c
@@ -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);
}
diff --git a/test.c b/test.c
index a23d606..0f5bfe5 100644
--- a/test.c
+++ b/test.c
@@ -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);
}