diff --git a/common/binaryserializer.h b/common/binaryserializer.h index 2d97a70f0..18fc34723 100644 --- a/common/binaryserializer.h +++ b/common/binaryserializer.h @@ -3,6 +3,8 @@ #include "common/armhelper.h" +using namespace std; + namespace swss { class BinarySerializer { @@ -64,7 +66,7 @@ class BinarySerializer { size); } - auto pkey = tmp_buffer; + auto pkey = string(tmp_buffer, *pkeylen); tmp_buffer += *pkeylen; WARNINGS_NO_CAST_ALIGN; @@ -79,7 +81,7 @@ class BinarySerializer { size); } - auto pval = tmp_buffer; + auto pval = string(tmp_buffer, *pvallen); tmp_buffer += *pvallen; values.push_back(std::make_pair(pkey, pval)); @@ -107,9 +109,8 @@ class BinarySerializer { void setKeyAndValue(const char* key, size_t klen, const char* value, size_t vlen) { - // to improve deserialize performance, copy null-terminated string. - setData(key, klen + 1); - setData(value, vlen + 1); + setData(key, klen); + setData(value, vlen); m_kvp_count++; } diff --git a/common/rediscommand.cpp b/common/rediscommand.cpp index 3b8ed7041..5cc7422b9 100644 --- a/common/rediscommand.cpp +++ b/common/rediscommand.cpp @@ -25,16 +25,18 @@ void RedisCommand::format(const char *fmt, ...) redisFreeCommand(temp); temp = nullptr; } + len = 0; va_list ap; va_start(ap, fmt); - len = redisvFormatCommand(&temp, fmt, ap); + int ret = redisvFormatCommand(&temp, fmt, ap); va_end(ap); - if (len == -1) { + if (ret == -1) { throw std::bad_alloc(); - } else if (len == -2) { + } else if (ret == -2) { throw std::invalid_argument("fmt"); } + len = ret; } void RedisCommand::formatArgv(int argc, const char **argv, const size_t *argvlen) @@ -44,11 +46,13 @@ void RedisCommand::formatArgv(int argc, const char **argv, const size_t *argvlen redisFreeCommand(temp); temp = nullptr; } + len = 0; - len = redisFormatCommandArgv(&temp, argc, argv, argvlen); - if (len == -1) { + int ret = redisFormatCommandArgv(&temp, argc, argv, argvlen); + if (ret == -1) { throw std::bad_alloc(); } + len = ret; } void RedisCommand::format(const vector &commands) @@ -135,6 +139,8 @@ std::string RedisCommand::toPrintableString() const const char *RedisCommand::c_str() const { + if (len == 0) + return nullptr; return temp; } diff --git a/tests/Makefile.am b/tests/Makefile.am index 2be3114ea..ac69ddce6 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -5,6 +5,7 @@ LDADD_GTEST = -L/usr/src/gtest -lgtest -lgtest_main -lgmock -lgmock_main tests_tests_SOURCES = tests/redis_ut.cpp \ tests/redis_piped_ut.cpp \ + tests/redis_command_ut.cpp \ tests/redis_state_ut.cpp \ tests/redis_piped_state_ut.cpp \ tests/tokenize_ut.cpp \ @@ -44,5 +45,5 @@ tests_tests_SOURCES = tests/redis_ut.cpp \ tests/main.cpp tests_tests_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS) -tests_tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS) +tests_tests_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(LIBNL_CFLAGS) -fno-access-control tests_tests_LDADD = $(LDADD_GTEST) -lpthread common/libswsscommon.la $(LIBNL_LIBS) $(CODE_COVERAGE_LIBS) sonic-db-cli/libsonicdbcli.la -lzmq -luuid -lboost_serialization diff --git a/tests/binary_serializer_ut.cpp b/tests/binary_serializer_ut.cpp index beae65d4b..19219cb82 100644 --- a/tests/binary_serializer_ut.cpp +++ b/tests/binary_serializer_ut.cpp @@ -29,7 +29,7 @@ TEST(BinarySerializer, serialize_deserialize) test_table); string serialized_str(buffer); - EXPECT_EQ(serialized_len, 107); + EXPECT_EQ(serialized_len, 101); auto ptr = std::make_shared(); KeyOpFieldsValuesTuple &kco = *ptr; @@ -84,3 +84,48 @@ TEST(BinarySerializer, deserialize_overflow) auto& deserialized_values = kfvFieldsValues(kco); EXPECT_THROW(BinarySerializer::deserializeBuffer(buffer, serialized_len - 10, deserialized_values), runtime_error); } + +TEST(BinarySerializer, protocol_buffer) +{ + string test_entry_key = "test_key"; + string test_command = "test_command"; + string test_db = "test_db"; + string test_table = "test_table"; + string test_key = "key"; + unsigned char binary_proto_buf[] = {0x0a, 0x05, 0x0d, 0x0a, 0x01, 0x00, 0x20, 0x10, 0xe1, 0x21}; + string proto_buf_val = string((const char *)binary_proto_buf, sizeof(binary_proto_buf)); + EXPECT_TRUE(proto_buf_val.length() == sizeof(binary_proto_buf)); + + char buffer[200]; + std::vector values; + values.push_back(std::make_pair(test_key, proto_buf_val)); + int serialized_len = (int)BinarySerializer::serializeBuffer( + buffer, + sizeof(buffer), + test_entry_key, + values, + test_command, + test_db, + test_table); + string serialized_str(buffer); + + EXPECT_EQ(serialized_len, 106); + + auto ptr = std::make_shared(); + KeyOpFieldsValuesTuple &kco = *ptr; + auto& deserialized_values = kfvFieldsValues(kco); + BinarySerializer::deserializeBuffer(buffer, serialized_len, deserialized_values); + + swss::FieldValueTuple fvt = deserialized_values.at(0); + EXPECT_TRUE(fvField(fvt) == test_db); + EXPECT_TRUE(fvValue(fvt) == test_table); + + fvt = deserialized_values.at(1); + EXPECT_TRUE(fvField(fvt) == test_entry_key); + EXPECT_TRUE(fvValue(fvt) == test_command); + + fvt = deserialized_values.at(2); + EXPECT_TRUE(fvField(fvt) == test_key); + EXPECT_TRUE(fvValue(fvt) == proto_buf_val); + EXPECT_TRUE(fvValue(fvt).length() == sizeof(binary_proto_buf)); +} diff --git a/tests/redis_command_ut.cpp b/tests/redis_command_ut.cpp new file mode 100644 index 000000000..4e7e40922 --- /dev/null +++ b/tests/redis_command_ut.cpp @@ -0,0 +1,12 @@ +#include "gtest/gtest.h" +#include "common/rediscommand.h" + +TEST(RedisCommand, invalid_redis_command) +{ + swss::RedisCommand cmd; + EXPECT_THROW(cmd.format("Invalid redis command %l^", 1), std::invalid_argument); + EXPECT_EQ(cmd.c_str(), nullptr); + EXPECT_EQ(cmd.length(), 0); + EXPECT_EQ(cmd.len, 0); + EXPECT_EQ(cmd.temp, nullptr); +}