Skip to content
Permalink
Browse files
Merge pull request #290 from 2012-wangjiaqi/brpc-dev
fix redis cmd_format bug in the empty string case
  • Loading branch information
jamesge committed Apr 3, 2018
2 parents 88481be + 362a18e commit bd1ae0ac7b2efa8efbc48decae20b1e1269eff9f
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 6 deletions.
@@ -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);
@@ -1,7 +1,6 @@
// 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,45 @@ 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();

request.AddCommand("get ''key value"); // == get key value
ASSERT_STREQ("*3\r\n$3\r\nget\r\n$3\r\nkey\r\n$5\r\nvalue\r\n", request._buf.to_string().c_str());
request.Clear();

request.AddCommand("get key'' value"); // == get key value
ASSERT_STREQ("*3\r\n$3\r\nget\r\n$3\r\nkey\r\n$5\r\nvalue\r\n", request._buf.to_string().c_str());
request.Clear();

request.AddCommand("get 'ext'key value "); // == get extkey value
ASSERT_STREQ("*3\r\n$3\r\nget\r\n$6\r\nextkey\r\n$5\r\nvalue\r\n", request._buf.to_string().c_str());
request.Clear();

request.AddCommand(" get key'ext' value "); // == get keyext value
ASSERT_STREQ("*3\r\n$3\r\nget\r\n$6\r\nkeyext\r\n$5\r\nvalue\r\n", request._buf.to_string().c_str());
request.Clear();
}
} //namespace

0 comments on commit bd1ae0a

Please sign in to comment.