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