Bug fix: Some unfin conditions are not properly handled

src/protocol/data/redis/parse.c

@@ -99,17 +99,26 @@ parse_req(struct request *req, struct buf *buf)
     }
     status = token_array_nelem(&nelem, buf);
     if (status != PARSE_OK) {
+        log_verb("getting array size returns status %d", status);
         buf->rpos = old_rpos;
         return status;
+    } else {
+        log_verb("array size is %"PRId64, nelem);
     }
+
     if (nelem < 1 || nelem > req->token->nalloc) {
         log_debug("parse req: invalid array size, %d not in [1, %"PRIu32"]",
                 nelem, req->token->nalloc);
         return PARSE_EINVALID;
     }
 
+
     /* parse elements */
     while (nelem > 0) {
+        if (buf_rsize(buf) == 0) {
+            buf->rpos = old_rpos;
+            return PARSE_EUNFIN;
+        }
         el = array_push(req->token);
         status = parse_element(el, buf);
         if (status != PARSE_OK) {

src/protocol/data/redis/response.h

@@ -38,7 +38,7 @@ typedef struct {
  * - a dummy entry RSP_UNKNOWN so we can use it as the initial type value;
  * - a RSP_NUMERIC type that doesn't have a corresponding message body.
  */
-#define RSP_STR_OK "+OK\r\n"
+#define RSP_STR_OK "OK"
 
 /*
  * NOTE(yao): we store fields as location in rbuf, this assumes the data will

src/protocol/data/redis/token.c

@@ -190,11 +190,19 @@ token_is_array(struct buf *buf)
 parse_rstatus_t
 token_array_nelem(int64_t *nelem, struct buf *buf)
 {
+    parse_rstatus_t status;
+    char *pos;
+
     ASSERT(nelem != NULL && buf != NULL);
     ASSERT(token_is_array(buf));
 
-    buf->rpos++;
-    return _read_int(nelem, buf, -1, ARRAY_MAXLEN);
+    pos = buf->rpos++;
+    status = _read_int(nelem, buf, -1, ARRAY_MAXLEN);
+    if (status == PARSE_EUNFIN) {
+        buf->rpos = pos;
+    }
+
+    return status;
 }
 
 

test/protocol/data/redis/check_redis.c

@@ -263,9 +263,9 @@ START_TEST(test_nil_bulk)
 }
 END_TEST
 
-START_TEST(test_unfin)
+START_TEST(test_unfin_token)
 {
-    char *token[10] = {
+    char *token[13] = {
         "+hello ",
         "-err",
         "-err\r",
@@ -276,19 +276,33 @@ START_TEST(test_unfin)
         "$5\r\n",
         "$5\r\nabc",
         "$5\r\nabcde\r",
+        "*5",
+        "*5\r",
     };
+    char *pos;
+    size_t len;
 
     for (int i = 0; i < 10; i++) {
-        size_t len = strlen(token[i]);
         struct element el;
-        char *pos;
 
+        len = strlen(token[i]);
         buf_reset(buf);
         buf_write(buf, token[i], len);
         pos = buf->rpos;
         ck_assert_int_eq(parse_element(&el, buf), PARSE_EUNFIN);
         ck_assert(buf->rpos == pos);
     }
+
+    for (int i = 10; i < 12; i++) {
+        int64_t nelem;
+
+        len = strlen(token[i]);
+        buf_reset(buf);
+        buf_write(buf, token[i], len);
+        pos = buf->rpos;
+        ck_assert_int_eq(token_array_nelem(&nelem, buf), PARSE_EUNFIN);
+        ck_assert(buf->rpos == pos);
+    }
 }
 END_TEST
 
@@ -391,6 +405,29 @@ START_TEST(test_ping)
 }
 END_TEST
 
+START_TEST(test_unfin_req)
+{
+    char *token[4] = {
+        "*2\r\n",
+        "*2\r\n$3\r\n",
+        "*2\r\n$3\r\nfoo\r\n",
+        "*2\r\n$3\r\nfoo\r\n$3\r\n",
+    };
+
+    for (int i = 0; i < 4; i++) {
+        char *pos;
+        size_t len;
+
+        len = strlen(token[i]);
+        buf_reset(buf);
+        buf_write(buf, token[i], len);
+        pos = buf->rpos;
+        ck_assert_int_eq(parse_req(req, buf), PARSE_EUNFIN);
+        ck_assert(buf->rpos == pos);
+    }
+}
+END_TEST
+
 /*
  * response
  */
@@ -531,14 +568,15 @@ redis_suite(void)
     tcase_add_test(tc_token, test_bulk_string);
     tcase_add_test(tc_token, test_array);
     tcase_add_test(tc_token, test_nil_bulk);
-    tcase_add_test(tc_token, test_unfin);
+    tcase_add_test(tc_token, test_unfin_token);
 
     /* basic requests */
     TCase *tc_request = tcase_create("request");
     suite_add_tcase(s, tc_request);
 
     tcase_add_test(tc_request, test_quit);
     tcase_add_test(tc_request, test_ping);
+    tcase_add_test(tc_request, test_unfin_req);
 
     /* basic response */
     TCase *tc_response = tcase_create("response");