From 920128a260b0056f7b14b479232d96405d9a6e62 Mon Sep 17 00:00:00 2001 From: Bjorn Svensson Date: Mon, 25 Jan 2021 15:43:40 +0100 Subject: Stack allocate dict iterators Replacing the get & release functions with an initiation function. Simplifies the code and will make sure we run subscription callbacks in OOM scenarios. --- async.c | 20 +++++++------------- dict.c | 11 +---------- dict.h | 3 +-- 3 files changed, 9 insertions(+), 25 deletions(-) diff --git a/async.c b/async.c index 145d949..938838a 100644 --- a/async.c +++ b/async.c @@ -306,7 +306,7 @@ static void __redisRunPushCallback(redisAsyncContext *ac, redisReply *reply) { static void __redisAsyncFree(redisAsyncContext *ac) { redisContext *c = &(ac->c); redisCallback cb; - dictIterator *it; + dictIterator it; dictEntry *de; /* Execute pending callbacks with NULL reply. */ @@ -319,23 +319,17 @@ static void __redisAsyncFree(redisAsyncContext *ac) { /* Run subscription callbacks with NULL reply */ if (ac->sub.channels) { - it = dictGetIterator(ac->sub.channels); - if (it != NULL) { - while ((de = dictNext(it)) != NULL) - __redisRunCallback(ac,dictGetEntryVal(de),NULL); - dictReleaseIterator(it); - } + dictInitIterator(&it,ac->sub.channels); + while ((de = dictNext(&it)) != NULL) + __redisRunCallback(ac,dictGetEntryVal(de),NULL); dictRelease(ac->sub.channels); } if (ac->sub.patterns) { - it = dictGetIterator(ac->sub.patterns); - if (it != NULL) { - while ((de = dictNext(it)) != NULL) - __redisRunCallback(ac,dictGetEntryVal(de),NULL); - dictReleaseIterator(it); - } + dictInitIterator(&it,ac->sub.patterns); + while ((de = dictNext(&it)) != NULL) + __redisRunCallback(ac,dictGetEntryVal(de),NULL); dictRelease(ac->sub.patterns); } diff --git a/dict.c b/dict.c index 34a33ea..ad57181 100644 --- a/dict.c +++ b/dict.c @@ -267,16 +267,11 @@ static dictEntry *dictFind(dict *ht, const void *key) { return NULL; } -static dictIterator *dictGetIterator(dict *ht) { - dictIterator *iter = hi_malloc(sizeof(*iter)); - if (iter == NULL) - return NULL; - +static void dictInitIterator(dictIterator *iter, dict *ht) { iter->ht = ht; iter->index = -1; iter->entry = NULL; iter->nextEntry = NULL; - return iter; } static dictEntry *dictNext(dictIterator *iter) { @@ -299,10 +294,6 @@ static dictEntry *dictNext(dictIterator *iter) { return NULL; } -static void dictReleaseIterator(dictIterator *iter) { - hi_free(iter); -} - /* ------------------------- private functions ------------------------------ */ /* Expand the hash table if needed */ diff --git a/dict.h b/dict.h index 95fcd28..6ad0acd 100644 --- a/dict.h +++ b/dict.h @@ -119,8 +119,7 @@ static int dictReplace(dict *ht, void *key, void *val); static int dictDelete(dict *ht, const void *key); static void dictRelease(dict *ht); static dictEntry * dictFind(dict *ht, const void *key); -static dictIterator *dictGetIterator(dict *ht); +static void dictInitIterator(dictIterator *iter, dict *ht); static dictEntry *dictNext(dictIterator *iter); -static void dictReleaseIterator(dictIterator *iter); #endif /* __DICT_H */ -- cgit v1.2.3 From 4bba72103c93eaaa8a6e07176e60d46ab277cf8a Mon Sep 17 00:00:00 2001 From: Bjorn Svensson Date: Tue, 26 Jan 2021 09:10:14 +0100 Subject: Handle OOM during async command callback registration Unless the callback is pushed to the list it will trigger an assert in redisProcessCallbacks() when the response arrives. This change let the user get an early error instead, available in the async context directly. --- async.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/async.c b/async.c index 145d949..074b46e 100644 --- a/async.c +++ b/async.c @@ -802,17 +802,21 @@ static int __redisAsyncCommand(redisAsyncContext *ac, redisCallbackFn *fn, void /* (P)UNSUBSCRIBE does not have its own response: every channel or * pattern that is unsubscribed will receive a message. This means we * should not append a callback function for this command. */ - } else if(strncasecmp(cstr,"monitor\r\n",9) == 0) { - /* Set monitor flag and push callback */ - c->flags |= REDIS_MONITORING; - __redisPushCallback(&ac->replies,&cb); + } else if (strncasecmp(cstr,"monitor\r\n",9) == 0) { + /* Set monitor flag and push callback */ + c->flags |= REDIS_MONITORING; + if (__redisPushCallback(&ac->replies,&cb) != REDIS_OK) + goto oom; } else { - if (c->flags & REDIS_SUBSCRIBED) + if (c->flags & REDIS_SUBSCRIBED) { /* This will likely result in an error reply, but it needs to be * received and passed to the callback. */ - __redisPushCallback(&ac->sub.invalid,&cb); - else - __redisPushCallback(&ac->replies,&cb); + if (__redisPushCallback(&ac->sub.invalid,&cb) != REDIS_OK) + goto oom; + } else { + if (__redisPushCallback(&ac->replies,&cb) != REDIS_OK) + goto oom; + } } __redisAppendCommand(c,cmd,len); @@ -823,6 +827,7 @@ static int __redisAsyncCommand(redisAsyncContext *ac, redisCallbackFn *fn, void return REDIS_OK; oom: __redisSetError(&(ac->c), REDIS_ERR_OOM, "Out of memory"); + __redisAsyncCopyError(ac); return REDIS_ERR; } -- cgit v1.2.3 From 9390de006d3a122b365407c01650ef242a592053 Mon Sep 17 00:00:00 2001 From: cheese1 Date: Wed, 17 Feb 2021 16:15:45 +0100 Subject: http -> https --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d71e33c..8330275 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ # HIREDIS -Hiredis is a minimalistic C client library for the [Redis](http://redis.io/) database. +Hiredis is a minimalistic C client library for the [Redis](https://redis.io/) database. It is minimalistic because it just adds minimal support for the protocol, but at the same time it uses a high level printf-alike API in order to make it -- cgit v1.2.3 From 49539fd1a74bef7f0646565adad9deaead2b1664 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 15 Oct 2020 17:40:07 -0400 Subject: redisReply: Fix - set len in double objects --- hiredis.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hiredis.c b/hiredis.c index 3035f79..5b7a1b8 100644 --- a/hiredis.c +++ b/hiredis.c @@ -235,6 +235,7 @@ static void *createDoubleObject(const redisReadTask *task, double value, char *s * decimal string conversion artifacts. */ memcpy(r->str, str, len); r->str[len] = '\0'; + r->len = len; if (task->parent) { parent = task->parent->obj; -- cgit v1.2.3 From 8039c7d26c553509afe8b2dce0e9deca28957e9f Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 15 Oct 2020 17:41:55 -0400 Subject: test: Add test case for doubles --- test.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test.c b/test.c index 7e7e1cb..4ec7bfa 100644 --- a/test.c +++ b/test.c @@ -11,6 +11,7 @@ #include #include #include +#include #include "hiredis.h" #include "async.h" @@ -583,6 +584,18 @@ static void test_reply_reader(void) { ((redisReply*)reply)->element[1]->integer == 42); freeReplyObject(reply); redisReaderFree(reader); + + test("Can parse RESP3 doubles: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, ",3.14159265358979323846\r\n",25); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_OK && + ((redisReply*)reply)->type == REDIS_REPLY_DOUBLE && + fabs(((redisReply*)reply)->dval - 3.14159265358979323846) < 0.00000001 && + ((redisReply*)reply)->len == 22 && + strcmp(((redisReply*)reply)->str, "3.14159265358979323846") == 0); + freeReplyObject(reply); + redisReaderFree(reader); } static void test_free_null(void) { -- cgit v1.2.3 From f913e9b997e2cc4b847923e1b3b0649d77c85923 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 15 Oct 2020 17:46:15 -0400 Subject: read: Fix double validation and infinity parsing The ',' protocol byte gets removed in processItem(), so it should not be compared against in processLineItem(). strtod() allows multiple representations of infinity and NaN that are not RESP3 compliant. Since we explicitly check for the two compliant infinity cases, strtod() should only return finite values. --- read.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/read.c b/read.c index 0952469..34fce2e 100644 --- a/read.c +++ b/read.c @@ -299,13 +299,17 @@ static int processLineItem(redisReader *r) { memcpy(buf,p,len); buf[len] = '\0'; - if (strcasecmp(buf,",inf") == 0) { + if (strcasecmp(buf,"inf") == 0) { d = INFINITY; /* Positive infinite. */ - } else if (strcasecmp(buf,",-inf") == 0) { + } else if (strcasecmp(buf,"-inf") == 0) { d = -INFINITY; /* Negative infinite. */ } else { d = strtod((char*)buf,&eptr); - if (buf[0] == '\0' || eptr[0] != '\0' || isnan(d)) { + /* RESP3 only allows "inf", "-inf", and finite values, while + * strtod() allows other variations on infinity, NaN, + * etc. We explicity handle our two allowed infinite cases + * above, so strtod() should only result in finite values. */ + if (buf[0] == '\0' || eptr[0] != '\0' || !isfinite(d)) { __redisReaderSetError(r,REDIS_ERR_PROTOCOL, "Bad double value"); return REDIS_ERR; -- cgit v1.2.3 From 96e8ea611022e8c360f1883b81f6ec2a386b9ed3 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 15 Oct 2020 17:50:49 -0400 Subject: test: Add test cases for infinite and NaN doubles --- test.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test.c b/test.c index 4ec7bfa..19cbd89 100644 --- a/test.c +++ b/test.c @@ -596,6 +596,26 @@ static void test_reply_reader(void) { strcmp(((redisReply*)reply)->str, "3.14159265358979323846") == 0); freeReplyObject(reply); redisReaderFree(reader); + + test("Correctly parses RESP3 double INFINITY: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, ",inf\r\n",6); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_OK && + ((redisReply*)reply)->type == REDIS_REPLY_DOUBLE && + isinf(((redisReply*)reply)->dval) && + ((redisReply*)reply)->dval > 0); + freeReplyObject(reply); + redisReaderFree(reader); + + test("Set error when RESP3 double is NaN: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, ",nan\r\n",6); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_ERR && + strcasecmp(reader->errstr,"Bad double value") == 0); + freeReplyObject(reply); + redisReaderFree(reader); } static void test_free_null(void) { -- cgit v1.2.3 From d8899fbc190d49bb09c2576035d4db4ead185846 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 15 Oct 2020 17:54:58 -0400 Subject: read: Add additional RESP3 nil validation RESP3 nil should consist of "_\r\n" and nothing else. --- read.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/read.c b/read.c index 34fce2e..57da853 100644 --- a/read.c +++ b/read.c @@ -320,6 +320,12 @@ static int processLineItem(redisReader *r) { obj = (void*)REDIS_REPLY_DOUBLE; } } else if (cur->type == REDIS_REPLY_NIL) { + if (len != 0) { + __redisReaderSetError(r,REDIS_ERR_PROTOCOL, + "Bad nil value"); + return REDIS_ERR; + } + if (r->fn && r->fn->createNil) obj = r->fn->createNil(cur); else -- cgit v1.2.3 From 790b4d3b4da0687a475e8a42724fbf6739bbd947 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 15 Oct 2020 18:17:35 -0400 Subject: test: Add test cases for RESP3 nil --- test.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test.c b/test.c index 19cbd89..5deff7a 100644 --- a/test.c +++ b/test.c @@ -616,6 +616,24 @@ static void test_reply_reader(void) { strcasecmp(reader->errstr,"Bad double value") == 0); freeReplyObject(reply); redisReaderFree(reader); + + test("Can parse RESP3 nil: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, "_\r\n",3); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_OK && + ((redisReply*)reply)->type == REDIS_REPLY_NIL); + freeReplyObject(reply); + redisReaderFree(reader); + + test("Set error on invalid RESP3 nil: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, "_nil\r\n",6); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_ERR && + strcasecmp(reader->errstr,"Bad nil value") == 0); + freeReplyObject(reply); + redisReaderFree(reader); } static void test_free_null(void) { -- cgit v1.2.3 From 51e693f4fd54d6a9dfbdafa1b361f6618e4f7ba3 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 15 Oct 2020 17:55:30 -0400 Subject: read: Add additional RESP3 bool validation RESP3 bools should be only one of "#t\r\n" or "#f\r\n". We also allow capital 'T' and 'F' to be lenient. --- read.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/read.c b/read.c index 57da853..1378671 100644 --- a/read.c +++ b/read.c @@ -331,7 +331,15 @@ static int processLineItem(redisReader *r) { else obj = (void*)REDIS_REPLY_NIL; } else if (cur->type == REDIS_REPLY_BOOL) { - int bval = p[0] == 't' || p[0] == 'T'; + int bval; + + if (len != 1 || !strchr("tTfF", p[0])) { + __redisReaderSetError(r,REDIS_ERR_PROTOCOL, + "Bad bool value"); + return REDIS_ERR; + } + + bval = p[0] == 't' || p[0] == 'T'; if (r->fn && r->fn->createBool) obj = r->fn->createBool(cur,bval); else -- cgit v1.2.3 From 81c48a9821e3350191ed6a1e0a3ed575d75df35d Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 15 Oct 2020 18:18:20 -0400 Subject: test: Add test cases for RESP3 bool --- test.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test.c b/test.c index 5deff7a..50055e2 100644 --- a/test.c +++ b/test.c @@ -634,6 +634,35 @@ static void test_reply_reader(void) { strcasecmp(reader->errstr,"Bad nil value") == 0); freeReplyObject(reply); redisReaderFree(reader); + + test("Can parse RESP3 bool (true): "); + reader = redisReaderCreate(); + redisReaderFeed(reader, "#t\r\n",4); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_OK && + ((redisReply*)reply)->type == REDIS_REPLY_BOOL && + ((redisReply*)reply)->integer); + freeReplyObject(reply); + redisReaderFree(reader); + + test("Can parse RESP3 bool (false): "); + reader = redisReaderCreate(); + redisReaderFeed(reader, "#f\r\n",4); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_OK && + ((redisReply*)reply)->type == REDIS_REPLY_BOOL && + !((redisReply*)reply)->integer); + freeReplyObject(reply); + redisReaderFree(reader); + + test("Set error on invalid RESP3 bool: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, "#foobar\r\n",9); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_ERR && + strcasecmp(reader->errstr,"Bad bool value") == 0); + freeReplyObject(reply); + redisReaderFree(reader); } static void test_free_null(void) { -- cgit v1.2.3 From 397fe2630170e33eee06533749b1802f9371cb3e Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 15 Oct 2020 19:17:58 -0400 Subject: read: Use memchr() in seekNewline() instead of looping over entire string --- read.c | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/read.c b/read.c index 1378671..22d6e47 100644 --- a/read.c +++ b/read.c @@ -123,29 +123,22 @@ static char *readBytes(redisReader *r, unsigned int bytes) { /* Find pointer to \r\n. */ static char *seekNewline(char *s, size_t len) { - int pos = 0; + char *_s = s, *ret; int _len = len-1; - /* Position should be < len-1 because the character at "pos" should be - * followed by a \n. Note that strchr cannot be used because it doesn't - * allow to search a limited length and the buffer that is being searched - * might not have a trailing NULL character. */ - while (pos < _len) { - while(pos < _len && s[pos] != '\r') pos++; - if (pos==_len) { - /* Not found. */ - return NULL; - } else { - if (s[pos+1] == '\n') { - /* Found. */ - return s+pos; - } else { - /* Continue searching. */ - pos++; - } + /* Exclude the last character from the searched length because the found + * '\r' should be followed by a '\n' */ + while ((ret = memchr(_s, '\r', _len)) != NULL) { + if (ret[1] == '\n') { + /* Found. */ + break; } + /* Continue searching. */ + ret++; + _len -= ret - _s; + _s = ret; } - return NULL; + return ret; } /* Convert a string into a long long. Returns REDIS_OK if the string could be -- cgit v1.2.3 From 33c06dd5036014b2ded7f2ef6fb994f2f3eadbe5 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 15 Oct 2020 19:44:51 -0400 Subject: test: Add test case for RESP3 map --- test.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test.c b/test.c index 50055e2..574f4bb 100644 --- a/test.c +++ b/test.c @@ -663,6 +663,26 @@ static void test_reply_reader(void) { strcasecmp(reader->errstr,"Bad bool value") == 0); freeReplyObject(reply); redisReaderFree(reader); + + test("Can parse RESP3 map: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, "%2\r\n+first\r\n:123\r\n$6\r\nsecond\r\n#t\r\n",34); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_OK && + ((redisReply*)reply)->type == REDIS_REPLY_MAP && + ((redisReply*)reply)->elements == 4 && + ((redisReply*)reply)->element[0]->type == REDIS_REPLY_STATUS && + ((redisReply*)reply)->element[0]->len == 5 && + !strcmp(((redisReply*)reply)->element[0]->str,"first") && + ((redisReply*)reply)->element[1]->type == REDIS_REPLY_INTEGER && + ((redisReply*)reply)->element[1]->integer == 123 && + ((redisReply*)reply)->element[2]->type == REDIS_REPLY_STRING && + ((redisReply*)reply)->element[2]->len == 6 && + !strcmp(((redisReply*)reply)->element[2]->str,"second") && + ((redisReply*)reply)->element[3]->type == REDIS_REPLY_BOOL && + ((redisReply*)reply)->element[3]->integer); + freeReplyObject(reply); + redisReaderFree(reader); } static void test_free_null(void) { -- cgit v1.2.3 From 0f92518847cbc7c83357a48059a273db8fe078a3 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 15 Oct 2020 20:13:25 -0400 Subject: test: Add test case for RESP3 set --- test.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test.c b/test.c index 574f4bb..baa5a87 100644 --- a/test.c +++ b/test.c @@ -683,6 +683,28 @@ static void test_reply_reader(void) { ((redisReply*)reply)->element[3]->integer); freeReplyObject(reply); redisReaderFree(reader); + + test("Can parse RESP3 set: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, "~5\r\n+orange\r\n$5\r\napple\r\n#f\r\n:100\r\n:999\r\n",40); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_OK && + ((redisReply*)reply)->type == REDIS_REPLY_SET && + ((redisReply*)reply)->elements == 5 && + ((redisReply*)reply)->element[0]->type == REDIS_REPLY_STATUS && + ((redisReply*)reply)->element[0]->len == 6 && + !strcmp(((redisReply*)reply)->element[0]->str,"orange") && + ((redisReply*)reply)->element[1]->type == REDIS_REPLY_STRING && + ((redisReply*)reply)->element[1]->len == 5 && + !strcmp(((redisReply*)reply)->element[1]->str,"apple") && + ((redisReply*)reply)->element[2]->type == REDIS_REPLY_BOOL && + !((redisReply*)reply)->element[2]->integer && + ((redisReply*)reply)->element[3]->type == REDIS_REPLY_INTEGER && + ((redisReply*)reply)->element[3]->integer == 100 && + ((redisReply*)reply)->element[4]->type == REDIS_REPLY_INTEGER && + ((redisReply*)reply)->element[4]->integer == 999); + freeReplyObject(reply); + redisReaderFree(reader); } static void test_free_null(void) { -- cgit v1.2.3 From ff73f1f9e79625843c08ab4e4846f21dde36ee93 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Thu, 15 Oct 2020 21:08:20 -0400 Subject: redisReply: Explicitly list nil and bool cases in freeReplyObject() switch. --- hiredis.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hiredis.c b/hiredis.c index 5b7a1b8..2dadf15 100644 --- a/hiredis.c +++ b/hiredis.c @@ -96,6 +96,8 @@ void freeReplyObject(void *reply) { switch(r->type) { case REDIS_REPLY_INTEGER: + case REDIS_REPLY_NIL: + case REDIS_REPLY_BOOL: break; /* Nothing to free */ case REDIS_REPLY_ARRAY: case REDIS_REPLY_MAP: -- cgit v1.2.3 From c8adea4024bbc405d96736c26bd2357c902a1f6b Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Fri, 16 Oct 2020 12:49:33 -0400 Subject: redisReply: Fix parent type assertions during double, nil, bool creation Per RESP3, push messages are able to contain exactly what array messages can contain (that is, any other type). --- hiredis.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hiredis.c b/hiredis.c index 2dadf15..5591cb9 100644 --- a/hiredis.c +++ b/hiredis.c @@ -243,7 +243,8 @@ static void *createDoubleObject(const redisReadTask *task, double value, char *s parent = task->parent->obj; assert(parent->type == REDIS_REPLY_ARRAY || parent->type == REDIS_REPLY_MAP || - parent->type == REDIS_REPLY_SET); + parent->type == REDIS_REPLY_SET || + parent->type == REDIS_REPLY_PUSH); parent->element[task->idx] = r; } return r; @@ -280,7 +281,8 @@ static void *createBoolObject(const redisReadTask *task, int bval) { parent = task->parent->obj; assert(parent->type == REDIS_REPLY_ARRAY || parent->type == REDIS_REPLY_MAP || - parent->type == REDIS_REPLY_SET); + parent->type == REDIS_REPLY_SET || + parent->type == REDIS_REPLY_PUSH); parent->element[task->idx] = r; } return r; -- cgit v1.2.3 From e43061156cffea910ab46dab1135e16e70774dce Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Fri, 16 Oct 2020 17:32:38 -0400 Subject: read: Additional validation and test case for RESP3 double This ensures that malformed RESP3 double messages that include an invalid null byte are not parsed as valid. --- read.c | 6 +++--- test.c | 9 +++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/read.c b/read.c index 22d6e47..01fa9dc 100644 --- a/read.c +++ b/read.c @@ -292,9 +292,9 @@ static int processLineItem(redisReader *r) { memcpy(buf,p,len); buf[len] = '\0'; - if (strcasecmp(buf,"inf") == 0) { + if (len == 3 && strcasecmp(buf,"inf") == 0) { d = INFINITY; /* Positive infinite. */ - } else if (strcasecmp(buf,"-inf") == 0) { + } else if (len == 4 && strcasecmp(buf,"-inf") == 0) { d = -INFINITY; /* Negative infinite. */ } else { d = strtod((char*)buf,&eptr); @@ -302,7 +302,7 @@ static int processLineItem(redisReader *r) { * strtod() allows other variations on infinity, NaN, * etc. We explicity handle our two allowed infinite cases * above, so strtod() should only result in finite values. */ - if (buf[0] == '\0' || eptr[0] != '\0' || !isfinite(d)) { + if (buf[0] == '\0' || eptr != &buf[len] || !isfinite(d)) { __redisReaderSetError(r,REDIS_ERR_PROTOCOL, "Bad double value"); return REDIS_ERR; diff --git a/test.c b/test.c index baa5a87..fa861b3 100644 --- a/test.c +++ b/test.c @@ -597,6 +597,15 @@ static void test_reply_reader(void) { freeReplyObject(reply); redisReaderFree(reader); + test("Set error on invalid RESP3 double: "); + reader = redisReaderCreate(); + redisReaderFeed(reader, ",3.14159\000265358979323846\r\n",26); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_ERR && + strcasecmp(reader->errstr,"Bad double value") == 0); + freeReplyObject(reply); + redisReaderFree(reader); + test("Correctly parses RESP3 double INFINITY: "); reader = redisReaderCreate(); redisReaderFeed(reader, ",inf\r\n",6); -- cgit v1.2.3 From c6646cb19242b0e0966760b38e7df74742b3a7b2 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Fri, 16 Oct 2020 17:38:38 -0400 Subject: read: Ensure no invalid '\r' or '\n' in simple status/error strings --- read.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/read.c b/read.c index 01fa9dc..89b5b5a 100644 --- a/read.c +++ b/read.c @@ -339,6 +339,13 @@ static int processLineItem(redisReader *r) { obj = (void*)REDIS_REPLY_BOOL; } else { /* Type will be error or status. */ + for (int i = 0; i < len; i++) { + if (p[i] == '\r' || p[i] == '\n') { + __redisReaderSetError(r,REDIS_ERR_PROTOCOL, + "Bad simple string value"); + return REDIS_ERR; + } + } if (r->fn && r->fn->createString) obj = r->fn->createString(cur,p,len); else -- cgit v1.2.3 From 83c14504257de168c45ae7730a00c930ab17cfa3 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Fri, 16 Oct 2020 18:35:58 -0400 Subject: read: Add support for the RESP3 bignum type --- hiredis.c | 4 +++- hiredis.h | 3 ++- read.c | 20 ++++++++++++++++++++ test.c | 11 +++++++++++ 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/hiredis.c b/hiredis.c index 5591cb9..2c1fb82 100644 --- a/hiredis.c +++ b/hiredis.c @@ -114,6 +114,7 @@ void freeReplyObject(void *reply) { case REDIS_REPLY_STRING: case REDIS_REPLY_DOUBLE: case REDIS_REPLY_VERB: + case REDIS_REPLY_BIGNUM: hi_free(r->str); break; } @@ -131,7 +132,8 @@ static void *createStringObject(const redisReadTask *task, char *str, size_t len assert(task->type == REDIS_REPLY_ERROR || task->type == REDIS_REPLY_STATUS || task->type == REDIS_REPLY_STRING || - task->type == REDIS_REPLY_VERB); + task->type == REDIS_REPLY_VERB || + task->type == REDIS_REPLY_BIGNUM); /* Copy string value */ if (task->type == REDIS_REPLY_VERB) { diff --git a/hiredis.h b/hiredis.h index a629930..e77a88a 100644 --- a/hiredis.h +++ b/hiredis.h @@ -112,7 +112,8 @@ typedef struct redisReply { double dval; /* The double when type is REDIS_REPLY_DOUBLE */ size_t len; /* Length of string */ char *str; /* Used for REDIS_REPLY_ERROR, REDIS_REPLY_STRING - REDIS_REPLY_VERB, and REDIS_REPLY_DOUBLE (in additional to dval). */ + REDIS_REPLY_VERB, REDIS_REPLY_DOUBLE (in additional to dval), + and REDIS_REPLY_BIGNUM. */ char vtype[4]; /* Used for REDIS_REPLY_VERB, contains the null terminated 3 character content type, such as "txt". */ size_t elements; /* number of elements, for REDIS_REPLY_ARRAY */ diff --git a/read.c b/read.c index 89b5b5a..5e0e0b4 100644 --- a/read.c +++ b/read.c @@ -337,6 +337,22 @@ static int processLineItem(redisReader *r) { obj = r->fn->createBool(cur,bval); else obj = (void*)REDIS_REPLY_BOOL; + } else if (cur->type == REDIS_REPLY_BIGNUM) { + /* Ensure all characters are decimal digits (with possible leading + * minus sign). */ + for (int i = 0; i < len; i++) { + /* XXX Consider: Allow leading '+'? Error on leading '0's? */ + if (i == 0 && p[0] == '-') continue; + if (p[i] < '0' || p[i] > '9') { + __redisReaderSetError(r,REDIS_ERR_PROTOCOL, + "Bad bignum value"); + return REDIS_ERR; + } + } + if (r->fn && r->fn->createString) + obj = r->fn->createString(cur,p,len); + else + obj = (void*)REDIS_REPLY_BIGNUM; } else { /* Type will be error or status. */ for (int i = 0; i < len; i++) { @@ -587,6 +603,9 @@ static int processItem(redisReader *r) { case '>': cur->type = REDIS_REPLY_PUSH; break; + case '(': + cur->type = REDIS_REPLY_BIGNUM; + break; default: __redisReaderSetErrorProtocolByte(r,*p); return REDIS_ERR; @@ -605,6 +624,7 @@ static int processItem(redisReader *r) { case REDIS_REPLY_DOUBLE: case REDIS_REPLY_NIL: case REDIS_REPLY_BOOL: + case REDIS_REPLY_BIGNUM: return processLineItem(r); case REDIS_REPLY_STRING: case REDIS_REPLY_VERB: diff --git a/test.c b/test.c index fa861b3..f830695 100644 --- a/test.c +++ b/test.c @@ -714,6 +714,17 @@ static void test_reply_reader(void) { ((redisReply*)reply)->element[4]->integer == 999); freeReplyObject(reply); redisReaderFree(reader); + + test("Can parse RESP3 bignum: "); + reader = redisReaderCreate(); + redisReaderFeed(reader,"(3492890328409238509324850943850943825024385\r\n",46); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_OK && + ((redisReply*)reply)->type == REDIS_REPLY_BIGNUM && + ((redisReply*)reply)->len == 43 && + !strcmp(((redisReply*)reply)->str,"3492890328409238509324850943850943825024385")); + freeReplyObject(reply); + redisReaderFree(reader); } static void test_free_null(void) { -- cgit v1.2.3 From 5f9242a1f8d99bf9b7864e8ecdce443fe821ab40 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Fri, 16 Oct 2020 18:36:52 -0400 Subject: read: Remove obsolete comment on nested multi bulk depth limitation --- read.c | 1 - 1 file changed, 1 deletion(-) diff --git a/read.c b/read.c index 5e0e0b4..9759dbc 100644 --- a/read.c +++ b/read.c @@ -487,7 +487,6 @@ static int processAggregateItem(redisReader *r) { long long elements; int root = 0, len; - /* Set error for nested multi bulks with depth > 7 */ if (r->ridx == r->tasks - 1) { if (redisReaderGrow(r) == REDIS_ERR) return REDIS_ERR; -- cgit v1.2.3 From bd7488d27d235a35a16519446173bef99fd4f200 Mon Sep 17 00:00:00 2001 From: Alex Smith Date: Fri, 16 Oct 2020 18:57:56 -0400 Subject: read: Validate line items prior to checking for object creation callbacks --- read.c | 63 +++++++++++++++++++++++++++++++++------------------------------ 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/read.c b/read.c index 9759dbc..24c2b58 100644 --- a/read.c +++ b/read.c @@ -267,47 +267,50 @@ static int processLineItem(redisReader *r) { if ((p = readLine(r,&len)) != NULL) { if (cur->type == REDIS_REPLY_INTEGER) { + long long v; + + if (string2ll(p, len, &v) == REDIS_ERR) { + __redisReaderSetError(r,REDIS_ERR_PROTOCOL, + "Bad integer value"); + return REDIS_ERR; + } + 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 if (cur->type == REDIS_REPLY_DOUBLE) { - if (r->fn && r->fn->createDouble) { - char buf[326], *eptr; - double d; + char buf[326], *eptr; + double d; - if ((size_t)len >= sizeof(buf)) { + if ((size_t)len >= sizeof(buf)) { + __redisReaderSetError(r,REDIS_ERR_PROTOCOL, + "Double value is too large"); + return REDIS_ERR; + } + + memcpy(buf,p,len); + buf[len] = '\0'; + + if (len == 3 && strcasecmp(buf,"inf") == 0) { + d = INFINITY; /* Positive infinite. */ + } else if (len == 4 && strcasecmp(buf,"-inf") == 0) { + d = -INFINITY; /* Negative infinite. */ + } else { + d = strtod((char*)buf,&eptr); + /* RESP3 only allows "inf", "-inf", and finite values, while + * strtod() allows other variations on infinity, NaN, + * etc. We explicity handle our two allowed infinite cases + * above, so strtod() should only result in finite values. */ + if (buf[0] == '\0' || eptr != &buf[len] || !isfinite(d)) { __redisReaderSetError(r,REDIS_ERR_PROTOCOL, - "Double value is too large"); + "Bad double value"); return REDIS_ERR; } + } - memcpy(buf,p,len); - buf[len] = '\0'; - - if (len == 3 && strcasecmp(buf,"inf") == 0) { - d = INFINITY; /* Positive infinite. */ - } else if (len == 4 && strcasecmp(buf,"-inf") == 0) { - d = -INFINITY; /* Negative infinite. */ - } else { - d = strtod((char*)buf,&eptr); - /* RESP3 only allows "inf", "-inf", and finite values, while - * strtod() allows other variations on infinity, NaN, - * etc. We explicity handle our two allowed infinite cases - * above, so strtod() should only result in finite values. */ - if (buf[0] == '\0' || eptr != &buf[len] || !isfinite(d)) { - __redisReaderSetError(r,REDIS_ERR_PROTOCOL, - "Bad double value"); - return REDIS_ERR; - } - } + if (r->fn && r->fn->createDouble) { obj = r->fn->createDouble(cur,d,buf,len); } else { obj = (void*)REDIS_REPLY_DOUBLE; -- cgit v1.2.3 From 410c24d2a977288ec97ffac23a303597b9f98d67 Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Mon, 19 Oct 2020 10:11:51 -0700 Subject: Fix off-by-one error in seekNewline --- read.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/read.c b/read.c index 24c2b58..de62b9a 100644 --- a/read.c +++ b/read.c @@ -123,21 +123,27 @@ static char *readBytes(redisReader *r, unsigned int bytes) { /* Find pointer to \r\n. */ static char *seekNewline(char *s, size_t len) { - char *_s = s, *ret; - int _len = len-1; + char *ret; - /* Exclude the last character from the searched length because the found - * '\r' should be followed by a '\n' */ - while ((ret = memchr(_s, '\r', _len)) != NULL) { + /* We cannot match with fewer than 2 bytes */ + if (len < 2) + return NULL; + + /* Search up to len - 1 characters */ + len--; + + /* Look for the \r */ + while ((ret = memchr(s, '\r', len)) != NULL) { if (ret[1] == '\n') { /* Found. */ break; } /* Continue searching. */ ret++; - _len -= ret - _s; - _s = ret; + len -= ret - s; + s = ret; } + return ret; } -- cgit v1.2.3 From 6204182aae71ca321418c069eac907936eeaa33a Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Tue, 15 Dec 2020 17:46:11 -0800 Subject: Handle the case where an invalidation is sent second. RESP3 invalidation messages always seemed to be sent before the response to a given command, but it appears this is not always the case: In Redis 6.2.0RC1 Redis sends the invalidation after the HSET in the following sequence: ``` hget hash field $5 value hset hash field value :0 >2 $10 invalidate *1 $4 hash ``` To account for this possibility just wrap redisGetReplyFromReader in a loop as it is called twice in redisGetReply. --- hiredis.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/hiredis.c b/hiredis.c index 2c1fb82..f23ff45 100644 --- a/hiredis.c +++ b/hiredis.c @@ -994,17 +994,6 @@ oom: return REDIS_ERR; } -/* Internal helper function to try and get a reply from the reader, - * or set an error in the context otherwise. */ -int redisGetReplyFromReader(redisContext *c, void **reply) { - if (redisReaderGetReply(c->reader,reply) == REDIS_ERR) { - __redisSetError(c,c->reader->err,c->reader->errstr); - return REDIS_ERR; - } - - return REDIS_OK; -} - /* Internal helper that returns 1 if the reply was a RESP3 PUSH * message and we handled it with a user-provided callback. */ static int redisHandledPushReply(redisContext *c, void *reply) { @@ -1016,6 +1005,19 @@ static int redisHandledPushReply(redisContext *c, void *reply) { return 0; } +/* Internal helper function to try and get a reply from the reader, + * or set an error in the context otherwise. */ +int redisGetReplyFromReader(redisContext *c, void **reply) { + do { + if (redisReaderGetReply(c->reader,reply) == REDIS_ERR) { + __redisSetError(c,c->reader->err,c->reader->errstr); + return REDIS_ERR; + } + } while (redisHandledPushReply(c, *reply)); + + return REDIS_OK; +} + int redisGetReply(redisContext *c, void **reply) { int wdone = 0; void *aux = NULL; @@ -1037,12 +1039,8 @@ int redisGetReply(redisContext *c, void **reply) { if (redisBufferRead(c) == REDIS_ERR) return REDIS_ERR; - /* We loop here in case the user has specified a RESP3 - * PUSH handler (e.g. for client tracking). */ - do { - if (redisGetReplyFromReader(c,&aux) == REDIS_ERR) - return REDIS_ERR; - } while (redisHandledPushReply(c, aux)); + if (redisGetReplyFromReader(c,&aux) == REDIS_ERR) + return REDIS_ERR; } while (aux == NULL); } -- cgit v1.2.3 From dfa33e60b07c13328133a16065d88d171a2a61d4 Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Fri, 18 Dec 2020 09:26:36 -0800 Subject: Change order independant push logic to not change behavior. Since redisGetReplyFromReader is exposed in a header file, we probably shouldn't modify how it behaves in any way. For this reason, handle the changed logic in an internal static helper method. --- hiredis.c | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/hiredis.c b/hiredis.c index f23ff45..5825174 100644 --- a/hiredis.c +++ b/hiredis.c @@ -1005,14 +1005,23 @@ static int redisHandledPushReply(redisContext *c, void *reply) { return 0; } -/* Internal helper function to try and get a reply from the reader, - * or set an error in the context otherwise. */ +/* Get a reply from our reader or set an error in the context. */ int redisGetReplyFromReader(redisContext *c, void **reply) { + if (redisReaderGetReply(c->reader, reply) == REDIS_ERR) { + __redisSetError(c,c->reader->err,c->reader->errstr); + return REDIS_ERR; + } + + return REDIS_OK; +} + +/* Internal helper to get the next reply from our reader while handling + * any PUSH messages we encounter along the way. This is separate from + * redisGetReplyFromReader so as to not change its behavior. */ +static int redisNextInBandReplyFromReader(redisContext *c, void **reply) { do { - if (redisReaderGetReply(c->reader,reply) == REDIS_ERR) { - __redisSetError(c,c->reader->err,c->reader->errstr); + if (redisGetReplyFromReader(c, reply) == REDIS_ERR) return REDIS_ERR; - } } while (redisHandledPushReply(c, *reply)); return REDIS_OK; @@ -1023,7 +1032,7 @@ int redisGetReply(redisContext *c, void **reply) { void *aux = NULL; /* Try to read pending replies */ - if (redisGetReplyFromReader(c,&aux) == REDIS_ERR) + if (redisNextInBandReplyFromReader(c,&aux) == REDIS_ERR) return REDIS_ERR; /* For the blocking context, flush output buffer and read reply */ @@ -1039,7 +1048,7 @@ int redisGetReply(redisContext *c, void **reply) { if (redisBufferRead(c) == REDIS_ERR) return REDIS_ERR; - if (redisGetReplyFromReader(c,&aux) == REDIS_ERR) + if (redisNextInBandReplyFromReader(c,&aux) == REDIS_ERR) return REDIS_ERR; } while (aux == NULL); } -- cgit v1.2.3 From e06ecf7e45c6a976a2089240fe0b1eae3098e18a Mon Sep 17 00:00:00 2001 From: Kristján Valur Jónsson Date: Thu, 8 Apr 2021 09:38:33 +0000 Subject: Ignore timeout callback from a successful connect --- async.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/async.c b/async.c index dd78dda..3f31286 100644 --- a/async.c +++ b/async.c @@ -690,9 +690,17 @@ void redisAsyncHandleTimeout(redisAsyncContext *ac) { redisContext *c = &(ac->c); redisCallback cb; - if ((c->flags & REDIS_CONNECTED) && ac->replies.head == NULL) { - /* Nothing to do - just an idle timeout */ - return; + if ((c->flags & REDIS_CONNECTED)) { + if ( ac->replies.head == NULL) { + /* Nothing to do - just an idle timeout */ + return; + } + + if (!ac->c.command_timeout || + (!ac->c.command_timeout->tv_sec && !ac->c.command_timeout->tv_usec)) { + /* A belated connect timeout arriving, ignore */ + return; + } } if (!c->err) { -- cgit v1.2.3 From 5f4382247a0074d2080921956cd9784bbd1824c2 Mon Sep 17 00:00:00 2001 From: Hans Zandbelt Date: Sun, 11 Apr 2021 18:49:38 +0200 Subject: improve SSL leak fix redis/hiredis#896 Free SSL object when redisSSLConnect fails but avoid doing that for callers of redisInitiateSSL who are supposed to manager their own SSL object. Signed-off-by: Hans Zandbelt --- ssl.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ssl.c b/ssl.c index c856bbc..c581f63 100644 --- a/ssl.c +++ b/ssl.c @@ -351,7 +351,6 @@ static int redisSSLConnect(redisContext *c, SSL *ssl) { } hi_free(rssl); - SSL_free(ssl); return REDIS_ERR; } @@ -393,7 +392,11 @@ int redisInitiateSSLWithContext(redisContext *c, redisSSLContext *redis_ssl_ctx) } } - return redisSSLConnect(c, ssl); + if (redisSSLConnect(c, ssl) != REDIS_OK) { + goto error; + } + + return REDIS_OK; error: if (ssl) -- cgit v1.2.3 From 0743f57bbae3999206d13cdfd15e35740026698c Mon Sep 17 00:00:00 2001 From: plan-do-break-fix Date: Sat, 24 Apr 2021 02:23:36 -0500 Subject: fix(docs): corrects typos in project README --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 8330275..78f6d89 100644 --- a/README.md +++ b/README.md @@ -169,7 +169,7 @@ Hiredis also supports every new `RESP3` data type which are as follows. For mor * **`REDIS_REPLY_MAP`**: * An array with the added invariant that there will always be an even number of elements. - The MAP is functionally equivelant to `REDIS_REPLY_ARRAY` except for the previously mentioned invariant. + The MAP is functionally equivalent to `REDIS_REPLY_ARRAY` except for the previously mentioned invariant. * **`REDIS_REPLY_SET`**: * An array response where each entry is unique. @@ -189,7 +189,7 @@ Hiredis also supports every new `RESP3` data type which are as follows. For mor * **`REDIS_REPLY_VERB`**: * A verbatim string, intended to be presented to the user without modification. - The string payload is stored in the `str` memeber, and type data is stored in the `vtype` member (e.g. `txt` for raw text or `md` for markdown). + The string payload is stored in the `str` member, and type data is stored in the `vtype` member (e.g. `txt` for raw text or `md` for markdown). Replies should be freed using the `freeReplyObject()` function. Note that this function will take care of freeing sub-reply objects -- cgit v1.2.3 From b6f86f38c2bbf0caa63d489174ac3a9777b97807 Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Sun, 23 May 2021 10:44:34 -0700 Subject: Fix README.md Closes #929 --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 78f6d89..d309a63 100644 --- a/README.md +++ b/README.md @@ -261,9 +261,9 @@ a single call to `read(2)`): redisReply *reply; redisAppendCommand(context,"SET foo bar"); redisAppendCommand(context,"GET foo"); -redisGetReply(context,(void *)&reply); // reply for SET +redisGetReply(context,(void**)&reply); // reply for SET freeReplyObject(reply); -redisGetReply(context,(void *)&reply); // reply for GET +redisGetReply(context,(void**)&reply); // reply for GET freeReplyObject(reply); ``` This API can also be used to implement a blocking subscriber: -- cgit v1.2.3 From 5850a8ecd2fb4ab39d80773e3017f02aff097ec4 Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Thu, 17 Jun 2021 13:01:15 -0700 Subject: Ensure we curry any connect error to an async context. --- async.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/async.c b/async.c index 3f31286..29f6924 100644 --- a/async.c +++ b/async.c @@ -604,7 +604,8 @@ static int __redisAsyncHandleConnect(redisAsyncContext *ac) { if (redisCheckConnectDone(c, &completed) == REDIS_ERR) { /* Error! */ - redisCheckSocketError(c); + if (redisCheckSocketError(c) == REDIS_ERR) + __redisAsyncCopyError(ac); __redisAsyncHandleConnectFailure(ac); return REDIS_ERR; } else if (completed == 1) { @@ -696,7 +697,7 @@ void redisAsyncHandleTimeout(redisAsyncContext *ac) { return; } - if (!ac->c.command_timeout || + if (!ac->c.command_timeout || (!ac->c.command_timeout->tv_sec && !ac->c.command_timeout->tv_usec)) { /* A belated connect timeout arriving, ignore */ return; -- cgit v1.2.3 From f5f31ff9b92b6bdf628716449d0d0782ceb7704a Mon Sep 17 00:00:00 2001 From: "Meir Shpilraien (Spielrein)" Date: Sun, 11 Jul 2021 21:26:20 +0300 Subject: Added REDIS_NO_AUTO_FREE_REPLIES flag (#962) When set hiredis will not automatically free replies in an async context, and the replies must be freed instead by the user. Co-authored-by: Michael Grunder --- async.c | 4 +++- hiredis.c | 3 +++ hiredis.h | 8 ++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/async.c b/async.c index 29f6924..e37afbd 100644 --- a/async.c +++ b/async.c @@ -569,7 +569,9 @@ void redisProcessCallbacks(redisAsyncContext *ac) { if (cb.fn != NULL) { __redisRunCallback(ac,&cb,reply); - c->reader->fn->freeObject(reply); + if (!(c->flags & REDIS_NO_AUTO_FREE_REPLIES)){ + c->reader->fn->freeObject(reply); + } /* Proceed with free'ing when redisAsyncFree() was called. */ if (c->flags & REDIS_FREEING) { diff --git a/hiredis.c b/hiredis.c index 5825174..9947b1e 100644 --- a/hiredis.c +++ b/hiredis.c @@ -804,6 +804,9 @@ redisContext *redisConnectWithOptions(const redisOptions *options) { if (options->options & REDIS_OPT_NOAUTOFREE) { c->flags |= REDIS_NO_AUTO_FREE; } + if (options->options & REDIS_OPT_NOAUTOFREEREPLIES) { + c->flags |= REDIS_NO_AUTO_FREE_REPLIES; + } /* Set any user supplied RESP3 PUSH handler or use freeReplyObject * as a default unless specifically flagged that we don't want one. */ diff --git a/hiredis.h b/hiredis.h index e77a88a..be8525f 100644 --- a/hiredis.h +++ b/hiredis.h @@ -86,6 +86,9 @@ typedef long long ssize_t; */ #define REDIS_NO_AUTO_FREE 0x200 +/* Flag that indicates the user does not want replies to be automatically freed */ +#define REDIS_NO_AUTO_FREE_REPLIES 0x400 + #define REDIS_KEEPALIVE_INTERVAL 15 /* seconds */ /* number of times we retry to connect in the case of EADDRNOTAVAIL and @@ -153,6 +156,11 @@ struct redisSsl; /* Don't automatically intercept and free RESP3 PUSH replies. */ #define REDIS_OPT_NO_PUSH_AUTOFREE 0x08 +/** + * Don't automatically free replies + */ +#define REDIS_OPT_NOAUTOFREEREPLIES 0x10 + /* In Unix systems a file descriptor is a regular signed int, with -1 * representing an invalid descriptor. In Windows it is a SOCKET * (32- or 64-bit unsigned integer depending on the architecture), where -- cgit v1.2.3 From 2d9d77518d012a54ae34f9822e4b4d19823c4b75 Mon Sep 17 00:00:00 2001 From: rouzier Date: Wed, 18 Aug 2021 22:13:34 -0400 Subject: Don't leak memory if an invalid type is set (#906) Co-authored-by: James Rouzier --- hiredis.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hiredis.c b/hiredis.c index 9947b1e..7e7af82 100644 --- a/hiredis.c +++ b/hiredis.c @@ -835,7 +835,7 @@ redisContext *redisConnectWithOptions(const redisOptions *options) { c->fd = options->endpoint.fd; c->flags |= REDIS_CONNECTED; } else { - // Unknown type - FIXME - FREE + redisFree(c); return NULL; } -- cgit v1.2.3 From 9eca1f36f4884f5fae8553aef3a0033c13700096 Mon Sep 17 00:00:00 2001 From: Yunier Perez Date: Thu, 30 Sep 2021 11:18:32 -0500 Subject: Allow to override OPENSSL_PREFIX in Linux --- Makefile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index a8d37a2..7e41c97 100644 --- a/Makefile +++ b/Makefile @@ -77,7 +77,12 @@ ifeq ($(USE_SSL),1) endif ifeq ($(uname_S),Linux) - SSL_LDFLAGS=-lssl -lcrypto + ifdef OPENSSL_PREFIX + CFLAGS+=-I$(OPENSSL_PREFIX)/include + SSL_LDFLAGS+=-L$(OPENSSL_PREFIX)/lib -lssl -lcrypto + else + SSL_LDFLAGS=-lssl -lcrypto + endif else OPENSSL_PREFIX?=/usr/local/opt/openssl CFLAGS+=-I$(OPENSSL_PREFIX)/include -- cgit v1.2.3 From 76a7b10005c70babee357a7d0f2becf28ec7ed1e Mon Sep 17 00:00:00 2001 From: Yossi Gottlieb Date: Tue, 13 Jul 2021 15:16:14 -0700 Subject: Fix for integer/buffer overflow CVE-2021-32765 This fix prevents hiredis from trying to allocate more than `SIZE_MAX` bytes, which would result in a buffer overrun. [Full Details](https://github.com/redis/hiredis/security/advisories/GHSA-hfm9-39pp-55p2) --- hiredis.c | 1 + test.c | 14 ++++++++++++++ 2 files changed, 15 insertions(+) diff --git a/hiredis.c b/hiredis.c index a7fbf48..ab0e398 100644 --- a/hiredis.c +++ b/hiredis.c @@ -174,6 +174,7 @@ static void *createArrayObject(const redisReadTask *task, size_t elements) { return NULL; if (elements > 0) { + if (SIZE_MAX / sizeof(redisReply*) < elements) return NULL; /* Don't overflow */ r->element = hi_calloc(elements,sizeof(redisReply*)); if (r->element == NULL) { freeReplyObject(r); diff --git a/test.c b/test.c index c0eeca7..397f564 100644 --- a/test.c +++ b/test.c @@ -493,6 +493,20 @@ static void test_reply_reader(void) { freeReplyObject(reply); redisReaderFree(reader); + test("Multi-bulk never overflows regardless of maxelements: "); + size_t bad_mbulk_len = (SIZE_MAX / sizeof(void *)) + 3; + char bad_mbulk_reply[100]; + snprintf(bad_mbulk_reply, sizeof(bad_mbulk_reply), "*%llu\r\n+asdf\r\n", + (unsigned long long) bad_mbulk_len); + + reader = redisReaderCreate(); + reader->maxelements = 0; /* Don't rely on default limit */ + redisReaderFeed(reader, bad_mbulk_reply, strlen(bad_mbulk_reply)); + ret = redisReaderGetReply(reader,&reply); + test_cond(ret == REDIS_ERR && strcasecmp(reader->errstr, "Out of memory") == 0); + freeReplyObject(reply); + redisReaderFree(reader); + #if LLONG_MAX > SIZE_MAX test("Set error when array > SIZE_MAX: "); reader = redisReaderCreate(); -- cgit v1.2.3 From 8d1bfac4640fe90cd6f800d09b7f53e886569b98 Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Mon, 4 Oct 2021 12:25:58 -0700 Subject: Prepare for v1.0.1 GA --- CHANGELOG.md | 10 ++++++++++ README.md | 4 ++++ hiredis.h | 4 ++-- 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 271f1fc..18000ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +## [1.0.1](https://github.com/redis/hiredis/tree/v1.0.1) - (2021-10-04) + +Announcing Hiredis v1.0.1, a security release fixing CVE-2021-32765 + +- Fix for [CVE-2021-32765](https://github.com/redis/hiredis/security/advisories/GHSA-hfm9-39pp-55p2) + [commit](https://github.com/redis/hiredis/commit/76a7b10005c70babee357a7d0f2becf28ec7ed1e) + ([Yossi Gottlieb](https://github.com/yossigo)) + +_Thanks to [Yossi Gottlieb](https://github.com/yossigo) for the security fix and to [Microsoft Security Vulnerability Research](https://www.microsoft.com/en-us/msrc/msvr) for finding the bug._ :sparkling_heart: + ## [1.0.0](https://github.com/redis/hiredis/tree/v1.0.0) - (2020-08-03) Announcing Hiredis v1.0.0, which adds support for RESP3, SSL connections, allocator injection, and better Windows support! :tada: diff --git a/README.md b/README.md index 3a22553..f770539 100644 --- a/README.md +++ b/README.md @@ -22,6 +22,10 @@ Redis version >= 1.2.0. The library comes with multiple APIs. There is the *synchronous API*, the *asynchronous API* and the *reply parsing API*. +## Upgrading to `1.0.1` + +Version 1.0.1 is simply 1.0.0 with a fix for [CVE-2021-32765](https://github.com/redis/hiredis/security/advisories/GHSA-hfm9-39pp-55p2). They are otherwise identical. + ## Upgrading to `1.0.0` Version 1.0.0 marks the first stable release of Hiredis. diff --git a/hiredis.h b/hiredis.h index 0622aaa..3d38114 100644 --- a/hiredis.h +++ b/hiredis.h @@ -47,8 +47,8 @@ typedef long long ssize_t; #define HIREDIS_MAJOR 1 #define HIREDIS_MINOR 0 -#define HIREDIS_PATCH 0 -#define HIREDIS_SONAME 1.0.0 +#define HIREDIS_PATCH 1 +#define HIREDIS_SONAME 1.0.1 /* Connection type can be blocking or non-blocking and is set in the * least significant bit of the flags field in redisContext. */ -- cgit v1.2.3 From d4e6f109a064690cde64765c654e679fea1d3548 Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Thu, 7 Oct 2021 09:31:15 -0700 Subject: Revert erroneous SONAME bump --- hiredis.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hiredis.h b/hiredis.h index 3d38114..3bc46d9 100644 --- a/hiredis.h +++ b/hiredis.h @@ -47,8 +47,8 @@ typedef long long ssize_t; #define HIREDIS_MAJOR 1 #define HIREDIS_MINOR 0 -#define HIREDIS_PATCH 1 -#define HIREDIS_SONAME 1.0.1 +#define HIREDIS_PATCH 2 +#define HIREDIS_SONAME 1.0.0 /* Connection type can be blocking or non-blocking and is set in the * least significant bit of the flags field in redisContext. */ -- cgit v1.2.3 From b731283245f3183af527237166261ad0768ba7d4 Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Thu, 7 Oct 2021 09:52:38 -0700 Subject: Prepare for v1.0.2 GA --- CHANGELOG.md | 9 +++++++++ README.md | 6 ++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18000ba..2a2bc31 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ +## [1.0.2](https://github.com/redis/hiredis/tree/v1.0.2) - (2021-10-07) + +Announcing Hiredis v1.0.2, which fixes CVE-2021-32765 but returns the SONAME to the correct value of `1.0.0`. + +- [Revert SONAME bump](https://github.com/redis/hiredis/commit/d4e6f109a064690cde64765c654e679fea1d3548) + ([Michael Grunder](https://github.com/michael-grunder)) + ## [1.0.1](https://github.com/redis/hiredis/tree/v1.0.1) - (2021-10-04) +This release erroneously bumped the SONAME, please use [1.0.2](https://github.com/redis/hiredis/tree/v1.0.2) + Announcing Hiredis v1.0.1, a security release fixing CVE-2021-32765 - Fix for [CVE-2021-32765](https://github.com/redis/hiredis/security/advisories/GHSA-hfm9-39pp-55p2) diff --git a/README.md b/README.md index f770539..c544d57 100644 --- a/README.md +++ b/README.md @@ -22,9 +22,11 @@ Redis version >= 1.2.0. The library comes with multiple APIs. There is the *synchronous API*, the *asynchronous API* and the *reply parsing API*. -## Upgrading to `1.0.1` +## Upgrading to `1.0.2` -Version 1.0.1 is simply 1.0.0 with a fix for [CVE-2021-32765](https://github.com/redis/hiredis/security/advisories/GHSA-hfm9-39pp-55p2). They are otherwise identical. +NOTE: v1.0.1 erroneously bumped SONAME, which is why it is skipped here. + +Version 1.0.2 is simply 1.0.0 with a fix for [CVE-2021-32765](https://github.com/redis/hiredis/security/advisories/GHSA-hfm9-39pp-55p2). They are otherwise identical. ## Upgrading to `1.0.0` -- cgit v1.2.3 From 51c740824be0a604d931bdc6738a74f1ee0abb36 Mon Sep 17 00:00:00 2001 From: Tongliang Liao Date: Wed, 6 Oct 2021 00:28:36 +0800 Subject: Remove extra comma from cmake var. Or it'll be treated as part of the var name. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a8dfaa9..eb43f01 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -4,7 +4,7 @@ PROJECT(hiredis) OPTION(ENABLE_SSL "Build hiredis_ssl for SSL support" OFF) OPTION(DISABLE_TESTS "If tests should be compiled or not" OFF) -OPTION(ENABLE_SSL_TESTS, "Should we test SSL connections" OFF) +OPTION(ENABLE_SSL_TESTS "Should we test SSL connections" OFF) MACRO(getVersionBit name) SET(VERSION_REGEX "^#define ${name} (.+)$") -- cgit v1.2.3 From e489846b7226958718ae91fa0c4999b420c706e2 Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Thu, 7 Oct 2021 14:47:11 -0700 Subject: Minor refactor of CVE-2021-32765 fix. Since `hi_calloc` always passes through one of our wrapper functions, we can perform this overflow in the wrapper, and get protection everywhere. Previous commit: 76a7b10005c70babee357a7d0f2becf28ec7ed1e Related vuln ID: CVE-2021-32765 [Full Details](https://github.com/redis/hiredis/security/advisories/GHSA-hfm9-39pp-55p2) --- alloc.c | 4 ++++ alloc.h | 5 +++++ hiredis.c | 1 - test.c | 16 ++++++++++++++++ 4 files changed, 25 insertions(+), 1 deletion(-) diff --git a/alloc.c b/alloc.c index 7fb6b35..0902286 100644 --- a/alloc.c +++ b/alloc.c @@ -68,6 +68,10 @@ void *hi_malloc(size_t size) { } void *hi_calloc(size_t nmemb, size_t size) { + /* Overflow check as the user can specify any arbitrary allocator */ + if (SIZE_MAX / size < nmemb) + return NULL; + return hiredisAllocFns.callocFn(nmemb, size); } diff --git a/alloc.h b/alloc.h index 34a05f4..771f9fe 100644 --- a/alloc.h +++ b/alloc.h @@ -32,6 +32,7 @@ #define HIREDIS_ALLOC_H #include /* for size_t */ +#include #ifdef __cplusplus extern "C" { @@ -59,6 +60,10 @@ static inline void *hi_malloc(size_t size) { } static inline void *hi_calloc(size_t nmemb, size_t size) { + /* Overflow check as the user can specify any arbitrary allocator */ + if (SIZE_MAX / size < nmemb) + return NULL; + return hiredisAllocFns.callocFn(nmemb, size); } diff --git a/hiredis.c b/hiredis.c index 15de4ad..7e7af82 100644 --- a/hiredis.c +++ b/hiredis.c @@ -178,7 +178,6 @@ static void *createArrayObject(const redisReadTask *task, size_t elements) { return NULL; if (elements > 0) { - if (SIZE_MAX / sizeof(redisReply*) < elements) return NULL; /* Don't overflow */ r->element = hi_calloc(elements,sizeof(redisReply*)); if (r->element == NULL) { freeReplyObject(r); diff --git a/test.c b/test.c index 9c91107..91feaed 100644 --- a/test.c +++ b/test.c @@ -59,6 +59,8 @@ struct pushCounters { int str; }; +static int insecure_calloc_calls; + #ifdef HIREDIS_TEST_SSL redisSSLContext *_ssl_ctx = NULL; #endif @@ -765,6 +767,11 @@ static void *hi_calloc_fail(size_t nmemb, size_t size) { return NULL; } +static void *hi_calloc_insecure(size_t nmemb, size_t size) { + insecure_calloc_calls++; + return (void*)0xdeadc0de; +} + static void *hi_realloc_fail(void *ptr, size_t size) { (void)ptr; (void)size; @@ -772,6 +779,8 @@ static void *hi_realloc_fail(void *ptr, size_t size) { } static void test_allocator_injection(void) { + void *ptr; + hiredisAllocFuncs ha = { .mallocFn = hi_malloc_fail, .callocFn = hi_calloc_fail, @@ -791,6 +800,13 @@ static void test_allocator_injection(void) { redisReader *reader = redisReaderCreate(); test_cond(reader == NULL); + /* Make sure hiredis itself protects against a non-overflow checking calloc */ + test("hiredis calloc wrapper protects against overflow: "); + ha.callocFn = hi_calloc_insecure; + hiredisSetAllocators(&ha); + ptr = hi_calloc((SIZE_MAX / sizeof(void*)) + 3, sizeof(void*)); + test_cond(ptr == NULL && insecure_calloc_calls == 0); + // Return allocators to default hiredisResetAllocators(); } -- cgit v1.2.3 From fa900ef76fbdc9f2bcec0d2cd507000dce9d8d03 Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Sun, 10 Oct 2021 11:58:19 -0700 Subject: Fix unused variable warning. --- test.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test.c b/test.c index 91feaed..78290a2 100644 --- a/test.c +++ b/test.c @@ -768,6 +768,8 @@ static void *hi_calloc_fail(size_t nmemb, size_t size) { } static void *hi_calloc_insecure(size_t nmemb, size_t size) { + (void)nmemb; + (void)size; insecure_calloc_calls++; return (void*)0xdeadc0de; } -- cgit v1.2.3 From 0cac8dae1b161f400d81bf789fb3713d6dc2a18e Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Sun, 10 Oct 2021 11:58:19 -0700 Subject: Switch to GitHub actions Since TravisCI.org was deprecated we've been without any tests. This commit adds back basic tests in Ubuntu, CentOS, and MacOS. More sophisticated tests/platforms to come in the future (e.g. 32bit tests). See: #992 --- .github/workflows/build.yml | 65 +++++++++++++++++++++++++++++++++++++++++++++ README.md | 3 ++- 2 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/build.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 0000000..e2d94c5 --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,65 @@ +name: Build and test +on: push + +jobs: + ubuntu: + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v2 + with: + repository: ${{ env.GITHUB_REPOSITORY }} + ref: ${{ env.GITHUB_HEAD_REF }} + + - name: Install redis-server + run: | + sudo add-apt-repository -y ppa:chris-lea/redis-server + sudo apt-get update + sudo apt-get install -y redis-server + + - name: Build hiredis + run: USE_SSL=1 make + + - name: Run tests + run: $GITHUB_WORKSPACE/test.sh + + centos7: + runs-on: ubuntu-latest + container: centos:7 + steps: + - name: Checkout code + uses: actions/checkout@v2 + with: + repository: ${{ env.GITHUB_REPOSITORY }} + ref: ${{ env.GITHUB_HEAD_REF }} + + - name: Install redis-server + run: | + yum -y install http://rpms.remirepo.net/enterprise/remi-release-7.rpm + yum -y --enablerepo=remi install redis + yum -y install gcc make openssl-devel + + - name: Build hiredis + run: USE_SSL=1 make + + - name: Run tests + run: $GITHUB_WORKSPACE/test.sh + + macos: + runs-on: macos-latest + steps: + - name: Checkout code + uses: actions/checkout@v2 + with: + repository: ${{ env.GITHUB_REPOSITORY }} + ref: ${{ env.GITHUB_HEAD_REF }} + + - name: Install dependencies + run: | + brew install openssl redis + + - name: Build hiredis + run: USE_SSL=1 make + + - name: Run tests + run: $GITHUB_WORKSPACE/test.sh diff --git a/README.md b/README.md index b073f8e..ed66220 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,5 @@ -[![Build Status](https://travis-ci.org/redis/hiredis.png)](https://travis-ci.org/redis/hiredis) + +[![Build Status](https://github.com/redis/hiredis/actions/workflows/build.yml/badge.svg)](https://github.com/redis/hiredis/actions/workflows/build.yml) **This Readme reflects the latest changed in the master branch. See [v1.0.0](https://github.com/redis/hiredis/tree/v1.0.0) for the Readme and documentation for the latest release ([API/ABI history](https://abi-laboratory.pro/?view=timeline&l=hiredis)).** -- cgit v1.2.3 From 783a3789c2c2a7fb1cc28c33d532a4366db9100a Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Sun, 10 Oct 2021 13:38:39 -0700 Subject: Add Windows tests in GitHub actions See: #992 TODO: MinGW/cygwin tests --- .github/workflows/build.yml | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e2d94c5..3f146db 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -63,3 +63,27 @@ jobs: - name: Run tests run: $GITHUB_WORKSPACE/test.sh + + windows: + runs-on: windows-latest + steps: + - name: Checkout code + uses: actions/checkout@v2 + with: + repository: ${{ env.GITHUB_REPOSITORY }} + ref: ${{ env.GITHUB_HEAD_REF }} + + - name: Install dependencies + run: | + choco install -y ninja memurai-developer + + - uses: ilammy/msvc-dev-cmd@v1 + - name: Build hiredis + run: | + mkdir build && cd build + cmake .. -G Ninja -DCMAKE_BUILD_TYPE=Release -DENABLE_EXAMPLES=ON + ninja -v + + - name: Run tests + run: | + ./build/hiredis-test.exe -- cgit v1.2.3 From 6ad4ccf3c7c6bed282f55d4658462b8c40c6ad39 Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Sun, 10 Oct 2021 15:47:15 -0700 Subject: Add Cygwin build test --- .github/workflows/build.yml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 3f146db..9053d27 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -87,3 +87,21 @@ jobs: - name: Run tests run: | ./build/hiredis-test.exe + + - name: Setup cygwin + uses: egor-tensin/setup-cygwin@v3 + with: + platform: x64 + packages: cmake gcc-core gcc-g++ + + - name: Build in cygwin + env: + HIREDIS_PATH: ${{ github.workspace }} + run: | + build_hiredis() { + cd $(cygpath -u $HIREDIS_PATH) + rm -rf build && mkdir build && cd build + cmake .. -G "Unix Makefiles" && make VERBOSE=1 + } + build_hiredis + shell: C:\tools\cygwin\bin\bash.exe --login --norc -eo pipefail -o igncr '{0}' -- cgit v1.2.3 From e9f64738450374e7f324c99988308d7d1f16451d Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Tue, 12 Oct 2021 13:17:45 -0700 Subject: We should run actions on PRs --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 9053d27..fbf1596 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,5 +1,5 @@ name: Build and test -on: push +on: [push, pull_request] jobs: ubuntu: -- cgit v1.2.3 From b73c2d410f4c7c2510cf63e98660bab2ca82fbc9 Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Tue, 12 Oct 2021 13:49:17 -0700 Subject: Add Centos8 I'm sure this can be done with a container matrix but figuring that out is left for another day. --- .github/workflows/build.yml | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index fbf1596..25d4fa3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -33,7 +33,7 @@ jobs: repository: ${{ env.GITHUB_REPOSITORY }} ref: ${{ env.GITHUB_HEAD_REF }} - - name: Install redis-server + - name: Install dependencies run: | yum -y install http://rpms.remirepo.net/enterprise/remi-release-7.rpm yum -y --enablerepo=remi install redis @@ -45,6 +45,29 @@ jobs: - name: Run tests run: $GITHUB_WORKSPACE/test.sh + centos8: + runs-on: ubuntu-latest + container: centos:8 + steps: + - name: Checkout code + uses: actions/checkout@v2 + with: + repository: ${{ env.GITHUB_REPOSITORY }} + ref: ${{ env.GITHUB_HEAD_REF }} + + - name: Install dependencies + run: | + dnf -y install https://rpms.remirepo.net/enterprise/remi-release-8.rpm + dnf -y module install redis:remi-6.0 + dnf -y group install "Development Tools" + dnf -y install openssl-devel + + - name: Build hiredis + run: USE_SSL=1 make + + - name: Run tests + run: $GITHUB_WORKSPACE/test.sh + macos: runs-on: macos-latest steps: -- cgit v1.2.3 From 4a126e8a9c52ac2fd0bb46cae57afa3f1cb44fd6 Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Wed, 13 Oct 2021 12:15:54 -0700 Subject: Add valgrind and CMake to tests --- .github/workflows/build.yml | 61 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 53 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 25d4fa3..a787700 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -11,18 +11,33 @@ jobs: repository: ${{ env.GITHUB_REPOSITORY }} ref: ${{ env.GITHUB_HEAD_REF }} - - name: Install redis-server + - name: Install dependencies run: | sudo add-apt-repository -y ppa:chris-lea/redis-server sudo apt-get update - sudo apt-get install -y redis-server - - - name: Build hiredis + sudo apt-get install -y redis-server valgrind + + - name: Build using cmake + env: + EXTRA_CMAKE_OPTS: -DENABLE_EXAMPLES:BOOL=ON -DENABLE_SSL:BOOL=ON -DENABLE_SSL_TESTS:BOOL=ON + CFLAGS: -Werror + CXXFLAGS: -Werror + run: mkdir build-ubuntu && cd build-ubuntu && cmake .. + + - name: Build using makefile run: USE_SSL=1 make - name: Run tests + env: + SKIPS_AS_FAILS: 1 run: $GITHUB_WORKSPACE/test.sh + # - name: Run tests under valgrind + # env: + # SKIPS_AS_FAILS: 1 + # TEST_PREFIX: valgrind --track-origins=yes --leak-check=full + # run: $GITHUB_WORKSPACE/test.sh + centos7: runs-on: ubuntu-latest container: centos:7 @@ -37,12 +52,27 @@ jobs: run: | yum -y install http://rpms.remirepo.net/enterprise/remi-release-7.rpm yum -y --enablerepo=remi install redis - yum -y install gcc make openssl-devel + yum -y install gcc gcc-c++ make openssl-devel cmake3 valgrind - - name: Build hiredis + - name: Build using cmake + env: + EXTRA_CMAKE_OPTS: -DENABLE_EXAMPLES:BOOL=ON -DENABLE_SSL:BOOL=ON -DENABLE_SSL_TESTS:BOOL=ON + CFLAGS: -Werror + CXXFLAGS: -Werror + run: mkdir build-centos7 && cd build-centos7 && cmake3 .. + + - name: Build using Makefile run: USE_SSL=1 make - name: Run tests + env: + SKIPS_AS_FAILS: 1 + run: $GITHUB_WORKSPACE/test.sh + + - name: Run tests under valgrind + env: + SKIPS_AS_FAILS: 1 + TEST_PREFIX: valgrind --track-origins=yes --leak-check=full run: $GITHUB_WORKSPACE/test.sh centos8: @@ -60,12 +90,27 @@ jobs: dnf -y install https://rpms.remirepo.net/enterprise/remi-release-8.rpm dnf -y module install redis:remi-6.0 dnf -y group install "Development Tools" - dnf -y install openssl-devel + dnf -y install openssl-devel cmake valgrind - - name: Build hiredis + - name: Build using cmake + env: + EXTRA_CMAKE_OPTS: -DENABLE_EXAMPLES:BOOL=ON -DENABLE_SSL:BOOL=ON -DENABLE_SSL_TESTS:BOOL=ON + CFLAGS: -Werror + CXXFLAGS: -Werror + run: mkdir build-centos8 && cd build-centos8 && cmake .. + + - name: Build using Makefile run: USE_SSL=1 make - name: Run tests + env: + SKIPS_AS_FAILS: 1 + run: $GITHUB_WORKSPACE/test.sh + + - name: Run tests under valgrind + env: + SKIPS_AS_FAILS: 1 + TEST_PREFIX: valgrind --track-origins=yes --leak-check=full run: $GITHUB_WORKSPACE/test.sh macos: -- cgit v1.2.3 From 30ff8d850e3cbc26064c5bc6ad4edbc34addd15d Mon Sep 17 00:00:00 2001 From: Björn Svensson Date: Wed, 20 Oct 2021 10:44:18 +0200 Subject: Run SSL tests in CI --- .github/workflows/build.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a787700..2ce06a1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -30,6 +30,7 @@ jobs: - name: Run tests env: SKIPS_AS_FAILS: 1 + TEST_SSL: 1 run: $GITHUB_WORKSPACE/test.sh # - name: Run tests under valgrind @@ -52,7 +53,7 @@ jobs: run: | yum -y install http://rpms.remirepo.net/enterprise/remi-release-7.rpm yum -y --enablerepo=remi install redis - yum -y install gcc gcc-c++ make openssl-devel cmake3 valgrind + yum -y install gcc gcc-c++ make openssl openssl-devel cmake3 valgrind - name: Build using cmake env: @@ -67,11 +68,13 @@ jobs: - name: Run tests env: SKIPS_AS_FAILS: 1 + TEST_SSL: 1 run: $GITHUB_WORKSPACE/test.sh - name: Run tests under valgrind env: SKIPS_AS_FAILS: 1 + TEST_SSL: 1 TEST_PREFIX: valgrind --track-origins=yes --leak-check=full run: $GITHUB_WORKSPACE/test.sh @@ -105,11 +108,13 @@ jobs: - name: Run tests env: SKIPS_AS_FAILS: 1 + TEST_SSL: 1 run: $GITHUB_WORKSPACE/test.sh - name: Run tests under valgrind env: SKIPS_AS_FAILS: 1 + TEST_SSL: 1 TEST_PREFIX: valgrind --track-origins=yes --leak-check=full run: $GITHUB_WORKSPACE/test.sh @@ -130,6 +135,8 @@ jobs: run: USE_SSL=1 make - name: Run tests + env: + TEST_SSL: 1 run: $GITHUB_WORKSPACE/test.sh windows: -- cgit v1.2.3 From c98c6994ded45e774c3459baa2936f06f8a2c331 Mon Sep 17 00:00:00 2001 From: Bjorn Svensson Date: Thu, 21 Oct 2021 22:17:21 +0200 Subject: Correcting the build target `coverage` for enabled SSL (#1009) * Exclude includes from /usr in coverage reporting * Correct build target `coverage` for enabled ssl `USE_SSL=1 make coverage` will now build the test binary with the forwarded define HIREDIS_TEST_SSL. This avoids inconsistency between built test binary and the testrunner `test.sh`. This enables test coverage measurements for SSL too. --- Makefile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 7e41c97..34c7909 100644 --- a/Makefile +++ b/Makefile @@ -73,7 +73,7 @@ USE_SSL?=0 # This is required for test.c only ifeq ($(USE_SSL),1) - CFLAGS+=-DHIREDIS_TEST_SSL + export CFLAGS+=-DHIREDIS_TEST_SSL endif ifeq ($(uname_S),Linux) @@ -299,12 +299,12 @@ gprof: $(MAKE) CFLAGS="-pg" LDFLAGS="-pg" gcov: - $(MAKE) CFLAGS="-fprofile-arcs -ftest-coverage" LDFLAGS="-fprofile-arcs" + $(MAKE) CFLAGS+="-fprofile-arcs -ftest-coverage" LDFLAGS="-fprofile-arcs" coverage: gcov make check mkdir -p tmp/lcov - lcov -d . -c -o tmp/lcov/hiredis.info + lcov -d . -c --exclude '/usr*' -o tmp/lcov/hiredis.info genhtml --legend -o tmp/lcov/report tmp/lcov/hiredis.info noopt: -- cgit v1.2.3 From 648763c36e9f6493b13a77da35eb33ef0652b4e2 Mon Sep 17 00:00:00 2001 From: Björn Svensson Date: Mon, 25 Oct 2021 10:13:48 +0200 Subject: Add build options for enabling async tests Asynchronous testcases that requires the event library `libevent` can be built and enabled by using the added build flags: - ENABLE_ASYNC_TESTS when using CMake - TEST_ASYNC when using Make The async tests are disabled by default to avoid adding new requirements, but the testcases are built and run in CI. --- .github/workflows/build.yml | 18 +++++++++--------- CMakeLists.txt | 10 +++++++--- Makefile | 6 ++++++ 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2ce06a1..af834ab 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -15,17 +15,17 @@ jobs: run: | sudo add-apt-repository -y ppa:chris-lea/redis-server sudo apt-get update - sudo apt-get install -y redis-server valgrind + sudo apt-get install -y redis-server valgrind libevent-dev - name: Build using cmake env: - EXTRA_CMAKE_OPTS: -DENABLE_EXAMPLES:BOOL=ON -DENABLE_SSL:BOOL=ON -DENABLE_SSL_TESTS:BOOL=ON + EXTRA_CMAKE_OPTS: -DENABLE_EXAMPLES:BOOL=ON -DENABLE_SSL:BOOL=ON -DENABLE_SSL_TESTS:BOOL=ON -DENABLE_ASYNC_TESTS:BOOL=ON CFLAGS: -Werror CXXFLAGS: -Werror run: mkdir build-ubuntu && cd build-ubuntu && cmake .. - name: Build using makefile - run: USE_SSL=1 make + run: USE_SSL=1 TEST_ASYNC=1 make - name: Run tests env: @@ -53,17 +53,17 @@ jobs: run: | yum -y install http://rpms.remirepo.net/enterprise/remi-release-7.rpm yum -y --enablerepo=remi install redis - yum -y install gcc gcc-c++ make openssl openssl-devel cmake3 valgrind + yum -y install gcc gcc-c++ make openssl openssl-devel cmake3 valgrind libevent-devel - name: Build using cmake env: - EXTRA_CMAKE_OPTS: -DENABLE_EXAMPLES:BOOL=ON -DENABLE_SSL:BOOL=ON -DENABLE_SSL_TESTS:BOOL=ON + EXTRA_CMAKE_OPTS: -DENABLE_EXAMPLES:BOOL=ON -DENABLE_SSL:BOOL=ON -DENABLE_SSL_TESTS:BOOL=ON -DENABLE_ASYNC_TESTS:BOOL=ON CFLAGS: -Werror CXXFLAGS: -Werror run: mkdir build-centos7 && cd build-centos7 && cmake3 .. - name: Build using Makefile - run: USE_SSL=1 make + run: USE_SSL=1 TEST_ASYNC=1 make - name: Run tests env: @@ -93,17 +93,17 @@ jobs: dnf -y install https://rpms.remirepo.net/enterprise/remi-release-8.rpm dnf -y module install redis:remi-6.0 dnf -y group install "Development Tools" - dnf -y install openssl-devel cmake valgrind + dnf -y install openssl-devel cmake valgrind libevent-devel - name: Build using cmake env: - EXTRA_CMAKE_OPTS: -DENABLE_EXAMPLES:BOOL=ON -DENABLE_SSL:BOOL=ON -DENABLE_SSL_TESTS:BOOL=ON + EXTRA_CMAKE_OPTS: -DENABLE_EXAMPLES:BOOL=ON -DENABLE_SSL:BOOL=ON -DENABLE_SSL_TESTS:BOOL=ON -DENABLE_ASYNC_TESTS:BOOL=ON CFLAGS: -Werror CXXFLAGS: -Werror run: mkdir build-centos8 && cd build-centos8 && cmake .. - name: Build using Makefile - run: USE_SSL=1 make + run: USE_SSL=1 TEST_ASYNC=1 make - name: Run tests env: diff --git a/CMakeLists.txt b/CMakeLists.txt index eb43f01..6d290a7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -5,6 +5,7 @@ PROJECT(hiredis) OPTION(ENABLE_SSL "Build hiredis_ssl for SSL support" OFF) OPTION(DISABLE_TESTS "If tests should be compiled or not" OFF) OPTION(ENABLE_SSL_TESTS "Should we test SSL connections" OFF) +OPTION(ENABLE_ASYNC_TESTS "Should we run all asynchronous API tests" OFF) MACRO(getVersionBit name) SET(VERSION_REGEX "^#define ${name} (.+)$") @@ -218,11 +219,14 @@ ENDIF() IF(NOT DISABLE_TESTS) ENABLE_TESTING() ADD_EXECUTABLE(hiredis-test test.c) + TARGET_LINK_LIBRARIES(hiredis-test hiredis) IF(ENABLE_SSL_TESTS) ADD_DEFINITIONS(-DHIREDIS_TEST_SSL=1) - TARGET_LINK_LIBRARIES(hiredis-test hiredis hiredis_ssl) - ELSE() - TARGET_LINK_LIBRARIES(hiredis-test hiredis) + TARGET_LINK_LIBRARIES(hiredis-test hiredis_ssl) + ENDIF() + IF(ENABLE_ASYNC_TESTS) + ADD_DEFINITIONS(-DHIREDIS_TEST_ASYNC=1) + TARGET_LINK_LIBRARIES(hiredis-test event) ENDIF() ADD_TEST(NAME hiredis-test COMMAND ${CMAKE_CURRENT_SOURCE_DIR}/test.sh) diff --git a/Makefile b/Makefile index 34c7909..f30c2ac 100644 --- a/Makefile +++ b/Makefile @@ -75,6 +75,9 @@ USE_SSL?=0 ifeq ($(USE_SSL),1) export CFLAGS+=-DHIREDIS_TEST_SSL endif +ifeq ($(TEST_ASYNC),1) + export CFLAGS+=-DHIREDIS_TEST_ASYNC +endif ifeq ($(uname_S),Linux) ifdef OPENSSL_PREFIX @@ -211,6 +214,9 @@ ifeq ($(USE_SSL),1) TEST_LIBS += $(SSL_STLIBNAME) TEST_LDFLAGS = $(SSL_LDFLAGS) -lssl -lcrypto -lpthread endif +ifeq ($(TEST_ASYNC),1) + TEST_LDFLAGS += -levent +endif hiredis-test: test.o $(TEST_LIBS) $(CC) -o $@ $(REAL_CFLAGS) -I. $^ $(REAL_LDFLAGS) $(TEST_LDFLAGS) -- cgit v1.2.3 From 4021726a69abfa31005c8653e1856b59c21bb032 Mon Sep 17 00:00:00 2001 From: Björn Svensson Date: Mon, 25 Oct 2021 11:18:08 +0200 Subject: Add asynchronous test for pubsub using RESP2 The testcase will subscribe to a channel, and via a second client a message is published to the channel. After receiving the message the testcase will unsubscribe and disconnect. --- test.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/test.c b/test.c index 78290a2..870c4c3 100644 --- a/test.c +++ b/test.c @@ -18,6 +18,10 @@ #ifdef HIREDIS_TEST_SSL #include "hiredis_ssl.h" #endif +#ifdef HIREDIS_TEST_ASYNC +#include "adapters/libevent.h" +#include +#endif #include "net.h" #include "win32.h" @@ -1443,6 +1447,104 @@ static void test_throughput(struct config config) { // redisFree(c); // } +#ifdef HIREDIS_TEST_ASYNC +struct event_base *base; + +typedef struct TestState { + redisOptions *options; + int checkpoint; +} TestState; + +/* Testcase timeout, will trigger a failure */ +void timeout_cb(int fd, short event, void *arg) { + (void) fd; (void) event; (void) arg; + printf("Timeout in async testing!\n"); + exit(1); +} + +/* Unexpected call, will trigger a failure */ +void unexpected_cb(redisAsyncContext *ac, void *r, void *privdata) { + (void) ac; (void) r; + printf("Unexpected call: %s\n",(char*)privdata); + exit(1); +} + +/* Helper function to publish a message via own client. */ +void publish_msg(redisOptions *options, const char* channel, const char* msg) { + redisContext *c = redisConnectWithOptions(options); + assert(c != NULL); + redisReply *reply = redisCommand(c,"PUBLISH %s %s",channel,msg); + assert(reply->type == REDIS_REPLY_INTEGER && reply->integer == 1); + freeReplyObject(reply); + disconnect(c, 0); +} + +/* Subscribe callback for test_pubsub_handling: + * - a published message triggers an unsubscribe + * - an unsubscribe response triggers a disconnect */ +void subscribe_cb(redisAsyncContext *ac, void *r, void *privdata) { + redisReply *reply = r; + TestState *state = privdata; + + assert(reply != NULL && + reply->type == REDIS_REPLY_ARRAY && + reply->elements == 3); + + if (strcmp(reply->element[0]->str,"subscribe") == 0) { + assert(strcmp(reply->element[1]->str,"mychannel") == 0 && + reply->element[2]->str == NULL); + publish_msg(state->options,"mychannel","Hello!"); + } else if (strcmp(reply->element[0]->str,"message") == 0) { + assert(strcmp(reply->element[1]->str,"mychannel") == 0 && + strcmp(reply->element[2]->str,"Hello!") == 0); + state->checkpoint++; + + /* Unsubscribe after receiving the published message. Send unsubscribe + * which should call the callback registered during subscribe */ + redisAsyncCommand(ac,unexpected_cb, + (void*)"unsubscribe should call subscribe_cb()", + "unsubscribe"); + } else if (strcmp(reply->element[0]->str,"unsubscribe") == 0) { + assert(strcmp(reply->element[1]->str,"mychannel") == 0 && + reply->element[2]->str == NULL); + + /* Disconnect after unsubscribe */ + redisAsyncDisconnect(ac); + event_base_loopbreak(base); + } else { + printf("Unexpected pubsub command: %s\n", reply->element[0]->str); + exit(1); + } +} + +static void test_pubsub_handling(struct config config) { + test("Subscribe, handle published message and unsubscribe: "); + /* Setup event dispatcher with a testcase timeout */ + base = event_base_new(); + struct event timeout; + evtimer_assign(&timeout,base,timeout_cb,NULL); + struct timeval timeout_tv = {.tv_sec = 10}; + evtimer_add(&timeout, &timeout_tv); + + /* Connect */ + redisOptions options = get_redis_tcp_options(config); + redisAsyncContext *ac = redisAsyncConnectWithOptions(&options); + assert(ac != NULL && ac->err == 0); + redisLibeventAttach(ac,base); + + /* Start subscribe */ + TestState state = {.options = &options}; + redisAsyncCommand(ac,subscribe_cb,&state,"subscribe mychannel"); + + /* Start event dispatching loop */ + test_cond(event_base_dispatch(base) == 0); + event_base_free(base); + + /* Verify test checkpoints */ + assert(state.checkpoint == 1); +} +#endif + int main(int argc, char **argv) { struct config cfg = { .tcp = { @@ -1561,6 +1663,11 @@ int main(int argc, char **argv) { } #endif +#ifdef HIREDIS_TEST_ASYNC + printf("\nTesting asynchronous API against TCP connection (%s:%d):\n", cfg.tcp.host, cfg.tcp.port); + test_pubsub_handling(cfg); +#endif + if (test_inherit_fd) { printf("\nTesting against inherited fd (%s): ", cfg.unix_sock.path); if (test_unix_socket) { -- cgit v1.2.3 From 7ad38dc4a87393efeca405df8b14a83eb43158b9 Mon Sep 17 00:00:00 2001 From: michael-grunder Date: Wed, 17 Nov 2021 14:37:27 -0800 Subject: Small tweaks of the async tests --- test.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/test.c b/test.c index 870c4c3..04d5197 100644 --- a/test.c +++ b/test.c @@ -20,7 +20,7 @@ #endif #ifdef HIREDIS_TEST_ASYNC #include "adapters/libevent.h" -#include +#include #endif #include "net.h" #include "win32.h" @@ -1521,10 +1521,12 @@ static void test_pubsub_handling(struct config config) { test("Subscribe, handle published message and unsubscribe: "); /* Setup event dispatcher with a testcase timeout */ base = event_base_new(); - struct event timeout; - evtimer_assign(&timeout,base,timeout_cb,NULL); + struct event *timeout = evtimer_new(base, timeout_cb, NULL); + assert(timeout != NULL); + + evtimer_assign(timeout,base,timeout_cb,NULL); struct timeval timeout_tv = {.tv_sec = 10}; - evtimer_add(&timeout, &timeout_tv); + evtimer_add(timeout, &timeout_tv); /* Connect */ redisOptions options = get_redis_tcp_options(config); @@ -1538,6 +1540,7 @@ static void test_pubsub_handling(struct config config) { /* Start event dispatching loop */ test_cond(event_base_dispatch(base) == 0); + event_free(timeout); event_base_free(base); /* Verify test checkpoints */ -- cgit v1.2.3 From c4333203e3895538664be2eed3091025130edf6a Mon Sep 17 00:00:00 2001 From: Björn Svensson Date: Mon, 16 Aug 2021 13:45:33 +0200 Subject: Remove unused parameter warning in libev adapter A warning in `redisLibevTimeout(..)` is triggered when building the libev adapter with Clang using -Wextra/-Wunused-parameter. Works fine with gcc.. --- adapters/libev.h | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/adapters/libev.h b/adapters/libev.h index 6191543..c59d3da 100644 --- a/adapters/libev.h +++ b/adapters/libev.h @@ -66,8 +66,9 @@ static void redisLibevWriteEvent(EV_P_ ev_io *watcher, int revents) { static void redisLibevAddRead(void *privdata) { redisLibevEvents *e = (redisLibevEvents*)privdata; +#if EV_MULTIPLICITY struct ev_loop *loop = e->loop; - ((void)loop); +#endif if (!e->reading) { e->reading = 1; ev_io_start(EV_A_ &e->rev); @@ -76,8 +77,9 @@ static void redisLibevAddRead(void *privdata) { static void redisLibevDelRead(void *privdata) { redisLibevEvents *e = (redisLibevEvents*)privdata; +#if EV_MULTIPLICITY struct ev_loop *loop = e->loop; - ((void)loop); +#endif if (e->reading) { e->reading = 0; ev_io_stop(EV_A_ &e->rev); @@ -86,8 +88,9 @@ static void redisLibevDelRead(void *privdata) { static void redisLibevAddWrite(void *privdata) { redisLibevEvents *e = (redisLibevEvents*)privdata; +#if EV_MULTIPLICITY struct ev_loop *loop = e->loop; - ((void)loop); +#endif if (!e->writing) { e->writing = 1; ev_io_start(EV_A_ &e->wev); @@ -96,8 +99,9 @@ static void redisLibevAddWrite(void *privdata) { static void redisLibevDelWrite(void *privdata) { redisLibevEvents *e = (redisLibevEvents*)privdata; +#if EV_MULTIPLICITY struct ev_loop *loop = e->loop; - ((void)loop); +#endif if (e->writing) { e->writing = 0; ev_io_stop(EV_A_ &e->wev); @@ -106,8 +110,9 @@ static void redisLibevDelWrite(void *privdata) { static void redisLibevStopTimer(void *privdata) { redisLibevEvents *e = (redisLibevEvents*)privdata; +#if EV_MULTIPLICITY struct ev_loop *loop = e->loop; - ((void)loop); +#endif ev_timer_stop(EV_A_ &e->timer); } @@ -120,6 +125,9 @@ static void redisLibevCleanup(void *privdata) { } static void redisLibevTimeout(EV_P_ ev_timer *timer, int revents) { +#if EV_MULTIPLICITY + ((void)EV_A); +#endif ((void)revents); redisLibevEvents *e = (redisLibevEvents*)timer->data; redisAsyncHandleTimeout(e->context); @@ -127,8 +135,9 @@ static void redisLibevTimeout(EV_P_ ev_timer *timer, int revents) { static void redisLibevSetTimeout(void *privdata, struct timeval tv) { redisLibevEvents *e = (redisLibevEvents*)privdata; +#if EV_MULTIPLICITY struct ev_loop *loop = e->loop; - ((void)loop); +#endif if (!ev_is_active(&e->timer)) { ev_init(&e->timer, redisLibevTimeout); -- cgit v1.2.3 From a83f4b8905484fb2b81c7fd71430ccf3aabe24cd Mon Sep 17 00:00:00 2001 From: Björn Svensson Date: Mon, 16 Aug 2021 14:06:53 +0200 Subject: Correct CMake warning for libevent adapter example --- examples/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt index 1d5bc56..49cd8d4 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -21,7 +21,7 @@ ENDIF() FIND_PATH(LIBEVENT event.h) if (LIBEVENT) - ADD_EXECUTABLE(example-libevent example-libevent) + ADD_EXECUTABLE(example-libevent example-libevent.c) TARGET_LINK_LIBRARIES(example-libevent hiredis event) ENDIF() -- cgit v1.2.3 From 1aed21a8c50fe075e8abce9db30c3860673f0fc7 Mon Sep 17 00:00:00 2001 From: Michael Grunder Date: Thu, 18 Nov 2021 13:50:09 -0800 Subject: Move to using make directly in Cygwin (#1020) CMake started hanging when trying to detect the C compiler ABI in cygin, so for now just build with make directly. --- .github/workflows/build.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index af834ab..c43beb6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -167,7 +167,7 @@ jobs: uses: egor-tensin/setup-cygwin@v3 with: platform: x64 - packages: cmake gcc-core gcc-g++ + packages: make git gcc-core - name: Build in cygwin env: @@ -175,8 +175,8 @@ jobs: run: | build_hiredis() { cd $(cygpath -u $HIREDIS_PATH) - rm -rf build && mkdir build && cd build - cmake .. -G "Unix Makefiles" && make VERBOSE=1 + git clean -xfd + make } build_hiredis shell: C:\tools\cygwin\bin\bash.exe --login --norc -eo pipefail -o igncr '{0}' -- cgit v1.2.3 From b5716ee82926316f7764b834eec636f5652d5600 Mon Sep 17 00:00:00 2001 From: Bjorn Svensson Date: Thu, 25 Nov 2021 08:09:23 +0100 Subject: Valgrind returns error exit code when errors found (#1011) By default Valgrind will return the exit code from the tested process. Since our test can return 0 (ALL TESTS PASS) even when a leak was found we need to tell Valgrind to return an error code. This will fail the CI job when issues are found. --- .github/workflows/build.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index c43beb6..e4dde05 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -36,7 +36,7 @@ jobs: # - name: Run tests under valgrind # env: # SKIPS_AS_FAILS: 1 - # TEST_PREFIX: valgrind --track-origins=yes --leak-check=full + # TEST_PREFIX: valgrind --error-exitcode=99 --track-origins=yes --leak-check=full # run: $GITHUB_WORKSPACE/test.sh centos7: @@ -75,7 +75,7 @@ jobs: env: SKIPS_AS_FAILS: 1 TEST_SSL: 1 - TEST_PREFIX: valgrind --track-origins=yes --leak-check=full + TEST_PREFIX: valgrind --error-exitcode=99 --track-origins=yes --leak-check=full run: $GITHUB_WORKSPACE/test.sh centos8: @@ -115,7 +115,7 @@ jobs: env: SKIPS_AS_FAILS: 1 TEST_SSL: 1 - TEST_PREFIX: valgrind --track-origins=yes --leak-check=full + TEST_PREFIX: valgrind --error-exitcode=99 --track-origins=yes --leak-check=full run: $GITHUB_WORKSPACE/test.sh macos: -- cgit v1.2.3 From da5a4ff3622e8744b772a76f6ce580dc9134fb38 Mon Sep 17 00:00:00 2001 From: Bjorn Svensson Date: Wed, 1 Dec 2021 20:43:23 +0100 Subject: Add asynchronous test for pubsub using RESP3 (#1012) * Include `unsubscribe` as a subscribe reply in RESP3 By providing the (p)unsubscribe message via the subscribe callback, instead of via the push callback, we get the same behavior in RESP3 as in RESP2. * Add asynchronous test for pubsub using RESP3 The testcase will subscribe to a channel, and via a second client a message is published to the channel. After receiving the message the testcase will unsubscribe and disconnect. This RESP3 testcase reuses the subscribe callback from the RESP2 testcase to make sure we have a common behavior. --- async.c | 4 ++-- test.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/async.c b/async.c index e37afbd..4459c19 100644 --- a/async.c +++ b/async.c @@ -496,8 +496,8 @@ static int redisIsSubscribeReply(redisReply *reply) { len = reply->element[0]->len - off; return !strncasecmp(str, "subscribe", len) || - !strncasecmp(str, "message", len); - + !strncasecmp(str, "message", len) || + !strncasecmp(str, "unsubscribe", len); } void redisProcessCallbacks(redisAsyncContext *ac) { diff --git a/test.c b/test.c index 04d5197..7af9bee 100644 --- a/test.c +++ b/test.c @@ -1453,6 +1453,7 @@ struct event_base *base; typedef struct TestState { redisOptions *options; int checkpoint; + int resp3; } TestState; /* Testcase timeout, will trigger a failure */ @@ -1479,7 +1480,7 @@ void publish_msg(redisOptions *options, const char* channel, const char* msg) { disconnect(c, 0); } -/* Subscribe callback for test_pubsub_handling: +/* Subscribe callback for test_pubsub_handling and test_pubsub_handling_resp3: * - a published message triggers an unsubscribe * - an unsubscribe response triggers a disconnect */ void subscribe_cb(redisAsyncContext *ac, void *r, void *privdata) { @@ -1487,7 +1488,7 @@ void subscribe_cb(redisAsyncContext *ac, void *r, void *privdata) { TestState *state = privdata; assert(reply != NULL && - reply->type == REDIS_REPLY_ARRAY && + reply->type == (state->resp3 ? REDIS_REPLY_PUSH : REDIS_REPLY_ARRAY) && reply->elements == 3); if (strcmp(reply->element[0]->str,"subscribe") == 0) { @@ -1546,6 +1547,49 @@ static void test_pubsub_handling(struct config config) { /* Verify test checkpoints */ assert(state.checkpoint == 1); } + +/* Unexpected push message, will trigger a failure */ +void unexpected_push_cb(redisAsyncContext *ac, void *r) { + (void) ac; (void) r; + printf("Unexpected call to the PUSH callback!\n"); + exit(1); +} + +static void test_pubsub_handling_resp3(struct config config) { + test("Subscribe, handle published message and unsubscribe using RESP3: "); + /* Setup event dispatcher with a testcase timeout */ + base = event_base_new(); + struct event *timeout = evtimer_new(base, timeout_cb, NULL); + assert(timeout != NULL); + + evtimer_assign(timeout,base,timeout_cb,NULL); + struct timeval timeout_tv = {.tv_sec = 10}; + evtimer_add(timeout, &timeout_tv); + + /* Connect */ + redisOptions options = get_redis_tcp_options(config); + redisAsyncContext *ac = redisAsyncConnectWithOptions(&options); + assert(ac != NULL && ac->err == 0); + redisLibeventAttach(ac,base); + + /* Not expecting any push messages in this test */ + redisAsyncSetPushCallback(ac, unexpected_push_cb); + + /* Switch protocol */ + redisAsyncCommand(ac,NULL,NULL,"HELLO 3"); + + /* Start subscribe */ + TestState state = {.options = &options, .resp3 = 1}; + redisAsyncCommand(ac,subscribe_cb,&state,"subscribe mychannel"); + + /* Start event dispatching loop */ + test_cond(event_base_dispatch(base) == 0); + event_free(timeout); + event_base_free(base); + + /* Verify test checkpoints */ + assert(state.checkpoint == 1); +} #endif int main(int argc, char **argv) { @@ -1668,7 +1712,15 @@ int main(int argc, char **argv) { #ifdef HIREDIS_TEST_ASYNC printf("\nTesting asynchronous API against TCP connection (%s:%d):\n", cfg.tcp.host, cfg.tcp.port); + cfg.type = CONN_TCP; + + int major; + redisContext *c = do_connect(cfg); + get_redis_version(c, &major, NULL); + disconnect(c, 0); + test_pubsub_handling(cfg); + if (major >= 6) test_pubsub_handling_resp3(cfg); #endif if (test_inherit_fd) { -- cgit v1.2.3