[feat] comment

src/aof.c

@@ -701,6 +701,7 @@ struct client *createAOFClient(void) {
     c->querybuf_peak = 0;
     c->argc = 0;
     c->argv = NULL;
+    c->cmd_argv = NULL;
     c->original_argc = 0;
     c->original_argv = NULL;
     c->argv_len_sum = 0;
@@ -736,6 +737,10 @@ struct client *createAOFClient(void) {
 }
 
 void freeFakeClientArgv(struct client *c) {
+    if(c->cmd_argv) {
+        decrRefCount(c->cmd_argv);
+        c->cmd_argv = NULL;
+    }
     int j;
 
     for (j = 0; j < c->argc; j++)

src/ctrip_swap_repl.c

@@ -137,6 +137,7 @@ static void replCommandDispatch(client *wc, client *c) {
 
     wc->argc = c->argc, c->argc = 0;
     wc->argv = c->argv, c->argv = NULL;
+    wc->cmd_argv = c->cmd_argv, c->cmd_argv = NULL;
     wc->cmd = c->cmd;
     wc->lastcmd = c->lastcmd;
     wc->flags = c->flags;

src/networking.c

@@ -147,6 +147,7 @@ client *createClient(connection *conn) {
     c->reqtype = 0;
     c->argc = 0;
     c->argv = NULL;
+    c->cmd_argv = NULL;
     c->argv_len_sum = 0;
     c->original_argc = 0;
     c->original_argv = NULL;
@@ -1232,6 +1233,10 @@ void freeClientOriginalArgv(client *c) {
 }
 
 static void freeClientArgv(client *c) {
+    if(c->cmd_argv) {
+        decrRefCount(c->cmd_argv);
+        c->cmd_argv = NULL;
+    }
     int j;
     for (j = 0; j < c->argc; j++)
         decrRefCount(c->argv[j]);
@@ -1853,6 +1858,40 @@ void unprotectClient(client *c) {
     }
 }
 
+#define IS_VALID_COMMENT (1)
+#define NOT_VALID_COMMENT (1)<<1
+#define NOT_COMMENT (1)<<2
+
+static inline int commentedArgCheck(robj* val) {
+    sds comment = (sds)val->ptr;
+    sds begin, end;
+
+    begin = strstr(comment, "/*");
+    end = strstr(comment, "*/");
+    if(begin==NULL && end==NULL) return NOT_COMMENT;
+    if(begin-comment==0 && (sdslen(comment)==end-comment+strlen("*/"))) return IS_VALID_COMMENT;
+    return NOT_VALID_COMMENT;
+}
+
+static inline void processAndBuildClientArgv(client* c, robj* val, int* commentError) {
+    switch (commentedArgCheck(val)) {
+        case NOT_VALID_COMMENT:
+            decrRefCount(val);
+            *commentError = C_ERR;
+            addReplyError(c,"Protocol error: wrong format comment");
+            setProtocolError("wrong format comment",c);
+            break;
+        case IS_VALID_COMMENT:
+            serverAssert(c->cmd_argv == NULL);
+            c->cmd_argv = val;
+            break;
+        case NOT_COMMENT:
+            c->argv[c->argc] = val;
+            c->argc++;
+            break;
+    }
+}
+
 /* Like processMultibulkBuffer(), but for the inline protocol instead of RESP,
  * this function consumes the client query buffer and creates a command ready
  * to be executed inside the client structure. Returns C_OK if the command
@@ -1923,14 +1962,21 @@ int processInlineBuffer(client *c) {
         c->argv_len_sum = 0;
     }
 
+    int commentError = C_OK;
     /* Create redis objects for all arguments. */
     for (c->argc = 0, j = 0; j < argc; j++) {
-        c->argv[c->argc] = createObject(OBJ_STRING,argv[j]);
-        c->argc++;
+        robj* val = createObject(OBJ_STRING,argv[j]);
+        if (j == 0)
+            processAndBuildClientArgv(c, val, &commentError);
+        else {
+            c->argv[c->argc] = val;
+            c->argc++;
+        }
         c->argv_len_sum += sdslen(argv[j]);
     }
     zfree(argv);
-    return C_OK;
+
+    return commentError;
 }
 
 /* Helper function. Record protocol erro details in server log,
@@ -2027,6 +2073,8 @@ int processMultibulkBuffer(client *c) {
     }
 
     serverAssertWithInfo(c,NULL,c->multibulklen > 0);
+
+    int commentError = C_OK;
     while(c->multibulklen) {
         /* Read bulk length if unknown */
         if (c->bulklen == -1) {
@@ -2099,16 +2147,23 @@ int processMultibulkBuffer(client *c) {
                 c->bulklen >= PROTO_MBULK_BIG_ARG &&
                 sdslen(c->querybuf) == (size_t)(c->bulklen+2))
             {
-                c->argv[c->argc++] = createObject(OBJ_STRING,c->querybuf);
+                robj* val = createObject(OBJ_STRING,c->querybuf);
+                if (c->argc == 0)
+                    processAndBuildClientArgv(c, val, &commentError);
+                else
+                    c->argv[c->argc++] = val;
                 c->argv_len_sum += c->bulklen;
                 sdsIncrLen(c->querybuf,-2); /* remove CRLF */
                 /* Assume that if we saw a fat argument we'll see another one
                  * likely... */
                 c->querybuf = sdsnewlen(SDS_NOINIT,c->bulklen+2);
                 sdsclear(c->querybuf);
             } else {
-                c->argv[c->argc++] =
-                    createStringObject(c->querybuf+c->qb_pos,c->bulklen);
+                robj* val = createStringObject(c->querybuf+c->qb_pos,c->bulklen);
+                if (c->argc == 0)
+                    processAndBuildClientArgv(c, val, &commentError);
+                else
+                    c->argv[c->argc++] = val;
                 c->argv_len_sum += c->bulklen;
                 c->qb_pos += c->bulklen+2;
             }
@@ -2117,6 +2172,7 @@ int processMultibulkBuffer(client *c) {
         }
     }
 
+    if (commentError == C_ERR) return C_ERR;
     /* We're done when c->multibulk == 0 */
     if (c->multibulklen == 0) return C_OK;
 

src/server.h

@@ -931,6 +931,7 @@ typedef struct client {
     size_t querybuf_peak;   /* Recent (100ms or more) peak of querybuf size. */
     int argc;               /* Num of arguments of current command. */
     robj **argv;            /* Arguments of current command. */
+    robj *cmd_argv;
     int original_argc;      /* Num of arguments of original command if arguments were rewritten. */
     robj **original_argv;   /* Arguments of original command if arguments were rewritten. */
     size_t argv_len_sum;    /* Sum of lengths of objects in argv list. */

tests/test_helper.tcl

@@ -107,6 +107,7 @@ set ::disk_tests {
     unit/bitfield
     unit/tracking
     unit/pubsub
+    unit/comment
     integration/logging
     integration/redis-cli
     integration/psync2-pingoff
@@ -163,6 +164,7 @@ set ::all_tests {
     unit/bitfield
     unit/tracking
     unit/pubsub
+    unit/comment
     integration/logging
     integration/redis-cli
     integration/replication

tests/unit/comment.tcl

@@ -0,0 +1,57 @@
+start_server {tags {"comment"}} {
+    set slave [srv 0 client]
+    set slave_host [srv 0 host]
+    set slave_port [srv 0 port]
+    set slave [redis $slave_host $slave_port]
+
+    start_server {} {
+        set master [srv 0 client]
+        set master_host [srv 0 host]
+        set master_port [srv 0 port]
+        set master [redis $master_host $master_port]
+
+        set m_r [redis $master_host $master_port 0 0]
+        set s_r [redis $slave_host $slave_port 0 0]
+
+        test {Set slave} {
+            eval $slave "\"/* comment hello */\" SLAVEOF $master_host $master_port"
+            wait_for_condition 50 100 {
+                [lindex [$slave role] 0] eq {slave} &&
+                [string match {*master_link_status:up*} [$slave info replication]]
+            } else {
+                fail "Can't turn the instance into a replica"
+            }
+        }
+
+        test {no txn} {
+            assert_match OK [$master "/*comment*/" FLUSHDB]
+            assert_match OK [$master "/*comment*/" SET k v]
+            assert_match 1 [$master "/*comment*/" HSET myhash field1 HELLO]
+            assert_match HELLO [$slave "/*comment*/" HGET myhash field1]
+            assert_match v [$slave "/*comment*/" GET k]
+        }
+
+        test {txn} {
+            assert_match OK [$master "/*comment*/" FLUSHDB]
+            assert_match OK [$master "/*comment*/" MULTI]
+            assert_match QUEUED [$master "/*comment*/" SET k v]
+            assert_match QUEUED [$master "/*comment*/" HSET myhash field1 HELLO]
+            assert_match "OK 1" [$master "/*comment*/" EXEC]
+            assert_match HELLO [$slave "/*comment*/" HGET myhash field1]
+            assert_match v [$slave "/*comment*/" GET k]
+
+            assert_match OK [$master "/*comment*/" FLUSHDB]
+            assert_match OK [$master "/*comment*/" MULTI]
+            assert_match QUEUED [$master "/*comment*/" SET k v]
+            assert_match QUEUED [$master "/*comment*/" HSET myhash field1 HELLO]
+            assert_match "OK" [$master "/*comment*/" discard]
+            assert_match "" [$slave "/*comment*/" HGET myhash field1]
+            assert_match "" [$slave "/*comment*/" GET k]
+        }
+
+        test {lua notxn} {
+            catch {[$master eval "return redis.call('/*comment*/', 'set','foo',12345)" 1 foo]} {err}
+            assert_match "*Unknown Redis command called from Lua script*" $err
+        }
+    }
+}

tests/unit/protocol.tcl

@@ -190,6 +190,75 @@ start_server {tags {"protocol network"}} {
         assert_equal [r debug protocol false] 0
         set _ {}
     } {}
+
+    test "Command Comment Test - RESP" {
+        reconnect
+
+        r write "*3\r\n\$11\r\n/*comment*/\r\n\$3\r\ndel\r\n\$1\r\nk\r\n"
+        r flush
+        assert_equal "0" [r read]
+
+        r write "*3\r\n\$19\r\n/* comment hello */\r\n\$3\r\ndel\r\n\$1\r\nk\r\n"
+        r flush
+        assert_equal "0" [r read]
+
+        r write "*3\r\n\$9\r\n/*comment\r\n\$3\r\ndel\r\n\$1\r\nk\r\n"
+        r flush
+        assert_error "*wrong format comment*" {r read}
+
+        reconnect
+        r write "*3\r\n\$13\r\n /*comment*/ \r\n\$3\r\ndel\r\n\$1\r\nk\r\n"
+        r flush
+        assert_error "*wrong format comment*" {r read}
+    }
+
+    test "Command Comment Test - Inline" {
+        set redis_host [srv 0 host]
+        set redis_port [srv 0 port]
+        # set redis_host 127.1
+        # set redis_port 6379
+
+        set s [socket $redis_host $redis_port]
+        fconfigure $s -blocking 0
+
+        puts $s "/*comment*/ set k v\r\n"
+        flush $s
+        after 100
+        assert_match [read $s] +OK\n
+
+        set cmd "\"/* comment hello */\" set k v\r\n"
+        puts $s $cmd
+        flush $s
+        after 100
+        assert_match [read $s] +OK\n
+
+        puts $s "/*comment*/ del k\r\n"
+        flush $s
+        after 100
+        assert_match [read $s] ":1\n"
+
+        puts $s "del /*comment*/\r\n"
+        flush $s
+        after 100
+        assert_match [read $s] ":0\n"
+
+        puts $s "\"/* comment hello */\" del k\r\n"
+        flush $s
+        after 100
+        assert_match [read $s] :0\n
+
+        puts $s "\" /* comment */ \" del k\r\n"
+        flush $s
+        after 100
+        assert_match [read $s] "-ERR Protocol error: wrong format comment\n"
+
+        set s [socket $redis_host $redis_port]
+        fconfigure $s -blocking 0
+        puts $s "/*comment del k\r\n"
+        flush $s
+        after 100
+        assert_match [read $s] "-ERR Protocol error: wrong format comment\n"
+    }
 }
 
 start_server {tags {"regression"}} {