fix redis cmd_format bug in the empty string case

src/brpc/redis_command.cpp

@@ -78,12 +78,14 @@ RedisCommandFormatV(butil::IOBuf* outbuf, const char* fmt, va_list ap) {
     char quote_char = 0;
     const char* quote_pos = fmt;
     int nargs = 0;
+    bool is_empty_component = false;
     for (; *c; ++c) {
         if (*c != '%' || c[1] == '\0') {
             if (*c == ' ') {
                 if (quote_char) {
                     compbuf.push_back(*c);
-                } else if (!compbuf.empty()) {
+                } else if (!compbuf.empty() || is_empty_component) {
+                    is_empty_component = false;
                     AppendHeader(nocount_buf, '$', compbuf.size());
                     compbuf.append("\r\n", 2);
                     nocount_buf.append(compbuf);
@@ -95,6 +97,7 @@ RedisCommandFormatV(butil::IOBuf* outbuf, const char* fmt, va_list ap) {
                     quote_char = *c;
                     quote_pos = c;
                 } else if (quote_char == *c) {  // end quote
+                    is_empty_component = (c - quote_pos == 1) ? true : false; // for empty string
                     quote_char = 0;
                 } else {
                     compbuf.push_back(*c);
@@ -235,7 +238,7 @@ RedisCommandFormatV(butil::IOBuf* outbuf, const char* fmt, va_list ap) {
                             quote_pos, quote_pos - fmt);
     }
     
-    if (!compbuf.empty()) {
+    if (!compbuf.empty() || is_empty_component) {
         AppendHeader(nocount_buf, '$', compbuf.size());
         compbuf.append("\r\n", 2);
         nocount_buf.append(compbuf);
@@ -273,11 +276,13 @@ RedisCommandNoFormat(butil::IOBuf* outbuf, const butil::StringPiece& cmd) {
     int ncomponent = 0;
     char quote_char = 0;
     const char* quote_pos = cmd.data();
+    bool is_empty_component = false;
     for (const char* c = cmd.data(); c != cmd.data() + cmd.size(); ++c) {
         if (*c == ' ') {
             if (quote_char) {
                 compbuf.push_back(*c);
-            } else if (!compbuf.empty()) {
+            } else if (!compbuf.empty() || is_empty_component) {
+                is_empty_component = false;
                 AppendHeader(nocount_buf, '$', compbuf.size());
                 compbuf.append("\r\n", 2);
                 nocount_buf.append(compbuf);
@@ -289,6 +294,7 @@ RedisCommandNoFormat(butil::IOBuf* outbuf, const butil::StringPiece& cmd) {
                 quote_char = *c;
                 quote_pos = c;
             } else if (quote_char == *c) {  // end quote
+                is_empty_component = (c - quote_pos == 1) ? true : false; // for empty string
                 quote_char = 0;
             } else {
                 compbuf.push_back(*c);
@@ -303,7 +309,7 @@ RedisCommandNoFormat(butil::IOBuf* outbuf, const butil::StringPiece& cmd) {
                             quote_pos, quote_pos - cmd.data());
     }
     
-    if (!compbuf.empty()) {
+    if (!compbuf.empty() || is_empty_component) {
         AppendHeader(nocount_buf, '$', compbuf.size());
         compbuf.append("\r\n", 2);
         nocount_buf.append(compbuf);

test/brpc_redis_unittest.cpp

 // Copyright (c) 2014 Baidu, Inc.
 // Date: Thu Jun 11 14:30:07 CST 2015
 
-#if defined(BAIDU_INTERNAL)
 
 #include <iostream>
 #include "butil/time.h"
@@ -59,7 +58,9 @@ class RedisTest : public testing::Test {
 protected:
     RedisTest() {}
     void SetUp() {
+#if defined(BAIDU_INTERNAL)
         pthread_once(&download_redis_server_once, DownloadRedisServer);
+#endif
     }
     void TearDown() {}
 };
@@ -112,6 +113,7 @@ void AssertResponseEqual(const brpc::RedisResponse& r1,
     }
 }
 
+#if defined(BAIDU_INTERNAL)
 TEST_F(RedisTest, sanity) {
     brpc::ChannelOptions options;
     options.protocol = brpc::PROTOCOL_REDIS;
@@ -403,5 +405,30 @@ TEST_F(RedisTest, auth) {
     }
 }
 
-} //namespace
 #endif // BAIDU_INTERNAL
+
+TEST_F(RedisTest, cmd_format) {
+    brpc::RedisRequest request;
+    // set empty string
+    request.AddCommand("set a ''");
+    ASSERT_STREQ("*3\r\n$3\r\nset\r\n$1\r\na\r\n$0\r\n\r\n", 
+		request._buf.to_string().c_str());
+    request.Clear();
+
+    request.AddCommand("mset b '' c ''");
+    ASSERT_STREQ("*5\r\n$4\r\nmset\r\n$1\r\nb\r\n$0\r\n\r\n$1\r\nc\r\n$0\r\n\r\n",
+		request._buf.to_string().c_str());
+    request.Clear();
+    // set non-empty string
+    request.AddCommand("set a 123");
+    ASSERT_STREQ("*3\r\n$3\r\nset\r\n$1\r\na\r\n$3\r\n123\r\n", 
+		request._buf.to_string().c_str());
+    request.Clear();
+
+    request.AddCommand("mset b '' c ccc");
+    ASSERT_STREQ("*5\r\n$4\r\nmset\r\n$1\r\nb\r\n$0\r\n\r\n$1\r\nc\r\n$3\r\nccc\r\n",
+		request._buf.to_string().c_str());
+    request.Clear();
+    
+}
+} //namespace