This is an automated email from the ASF dual-hosted git repository.
wwbmmm pushed a commit to branch revert-3035-limit-redis-alloc-size
in repository https://gitbox.apache.org/repos/asf/brpc.git
commit 39b96e8ccc2b215db2a76a9b3dc8c554d3d51667
Author: Weibing Wang <wwb...@gmail.com>
AuthorDate: Thu Jul 31 10:14:41 2025 +0800
Revert "Add redis allocation size limit (#3035)"
This reverts commit 75341f896d75f3a6724c8df21a211be331c2b6ad.
---
src/brpc/redis_command.cpp | 15 ++-----
src/brpc/redis_reply.cpp | 19 ++++-----
test/brpc_redis_unittest.cpp | 95 --------------------------------------------
3 files changed, 10 insertions(+), 119 deletions(-)
diff --git a/src/brpc/redis_command.cpp b/src/brpc/redis_command.cpp
index 5803aaa8..d2620323 100644
--- a/src/brpc/redis_command.cpp
+++ b/src/brpc/redis_command.cpp
@@ -21,7 +21,6 @@
#include "butil/logging.h"
#include "brpc/log.h"
#include "brpc/redis_command.h"
-#include "gflags/gflags.h"
namespace {
@@ -31,8 +30,6 @@ const size_t CTX_WIDTH = 5;
namespace brpc {
-DECLARE_int32(redis_max_allocation_size);
-
// Much faster than snprintf(..., "%lu", d);
inline size_t AppendDecimal(char* outbuf, unsigned long d) {
char buf[24]; // enough for decimal 64-bit integers
@@ -459,12 +456,6 @@ ParseError RedisCommandParser::Consume(butil::IOBuf& buf,
return PARSE_ERROR_ABSOLUTELY_WRONG;
}
if (!_parsing_array) {
- if (value > (int64_t)(FLAGS_redis_max_allocation_size /
sizeof(butil::StringPiece))) {
- LOG(ERROR) << "command array size exceeds limit! max="
- << (FLAGS_redis_max_allocation_size /
sizeof(butil::StringPiece))
- << ", actually=" << value;
- return PARSE_ERROR_ABSOLUTELY_WRONG;
- }
buf.pop_front(crlf_pos + 2/*CRLF*/);
_parsing_array = true;
_length = value;
@@ -479,9 +470,9 @@ ParseError RedisCommandParser::Consume(butil::IOBuf& buf,
LOG(ERROR) << "string in command is nil!";
return PARSE_ERROR_ABSOLUTELY_WRONG;
}
- if (len > FLAGS_redis_max_allocation_size) {
- LOG(ERROR) << "command string exceeds max allocation size! max="
- << FLAGS_redis_max_allocation_size << ", actually=" << len;
+ if (len > (int64_t)std::numeric_limits<uint32_t>::max()) {
+ LOG(ERROR) << "string in command is too long! max length=2^32-1,"
+ " actually=" << len;
return PARSE_ERROR_ABSOLUTELY_WRONG;
}
if (buf.size() < crlf_pos + 2 + (size_t)len + 2/*CRLF*/) {
diff --git a/src/brpc/redis_reply.cpp b/src/brpc/redis_reply.cpp
index 26fa2449..8710df16 100644
--- a/src/brpc/redis_reply.cpp
+++ b/src/brpc/redis_reply.cpp
@@ -20,13 +20,9 @@
#include "butil/logging.h"
#include "butil/string_printf.h"
#include "brpc/redis_reply.h"
-#include "gflags/gflags.h"
namespace brpc {
-DEFINE_int32(redis_max_allocation_size, 64 * 1024 * 1024,
- "Maximum memory allocation size in bytes for a single redis
request or reply (64MB by default)");
-
//BAIDU_CASSERT(sizeof(RedisReply) == 24, size_match);
const int RedisReply::npos = -1;
@@ -179,9 +175,9 @@ ParseError RedisReply::ConsumePartialIOBuf(butil::IOBuf&
buf) {
_data.integer = 0;
return PARSE_OK;
}
- if (len > FLAGS_redis_max_allocation_size) {
- LOG(ERROR) << "bulk string exceeds max allocation size! max="
- << FLAGS_redis_max_allocation_size << ", actually="
<< len;
+ if (len > (int64_t)std::numeric_limits<uint32_t>::max()) {
+ LOG(ERROR) << "bulk string is too long! max length=2^32-1,"
+ " actually=" << len;
return PARSE_ERROR_ABSOLUTELY_WRONG;
}
// We provide c_str(), thus even if bulk string is started with
@@ -233,14 +229,13 @@ ParseError RedisReply::ConsumePartialIOBuf(butil::IOBuf&
buf) {
_data.array.replies = NULL;
return PARSE_OK;
}
- int64_t array_size = sizeof(RedisReply) * count;
- if (array_size > FLAGS_redis_max_allocation_size) {
- LOG(ERROR) << "array allocation exceeds max allocation size!
max="
- << FLAGS_redis_max_allocation_size << ", actually="
<< array_size;
+ if (count > (int64_t)std::numeric_limits<uint32_t>::max()) {
+ LOG(ERROR) << "Too many sub replies! max count=2^32-1,"
+ " actually=" << count;
return PARSE_ERROR_ABSOLUTELY_WRONG;
}
// FIXME(gejun): Call allocate_aligned instead.
- RedisReply* subs = (RedisReply*)_arena->allocate(array_size);
+ RedisReply* subs =
(RedisReply*)_arena->allocate(sizeof(RedisReply) * count);
if (subs == NULL) {
LOG(FATAL) << "Fail to allocate RedisReply[" << count << "]";
return PARSE_ERROR_ABSOLUTELY_WRONG;
diff --git a/test/brpc_redis_unittest.cpp b/test/brpc_redis_unittest.cpp
index 5e38c374..ac6872e0 100644
--- a/test/brpc_redis_unittest.cpp
+++ b/test/brpc_redis_unittest.cpp
@@ -26,11 +26,9 @@
#include <brpc/server.h>
#include <brpc/redis_command.h>
#include <gtest/gtest.h>
-#include <gflags/gflags.h>
namespace brpc {
DECLARE_int32(idle_timeout_second);
-DECLARE_int32(redis_max_allocation_size);
}
int main(int argc, char* argv[]) {
@@ -1352,97 +1350,4 @@ TEST_F(RedisTest, server_handle_pipeline) {
ASSERT_STREQ(response.reply(7).c_str(), "world");
}
-TEST_F(RedisTest, memory_allocation_limits) {
- int32_t original_limit = brpc::FLAGS_redis_max_allocation_size;
- brpc::FLAGS_redis_max_allocation_size = 1024;
-
- butil::Arena arena;
-
- // Test redis_reply.cpp limits
- {
- // Test bulk string exceeding limit
- butil::IOBuf buf;
- std::string large_string = "*1\r\n$2000\r\n";
- large_string.append(2000, 'a');
- large_string.append("\r\n");
- buf.append(large_string);
-
- brpc::RedisReply reply(&arena);
- brpc::ParseError err = reply.ConsumePartialIOBuf(buf);
- ASSERT_EQ(brpc::PARSE_ERROR_ABSOLUTELY_WRONG, err);
- }
-
- {
- // Test array allocation exceeding limit
- butil::IOBuf buf;
- int32_t large_count = brpc::FLAGS_redis_max_allocation_size /
sizeof(brpc::RedisReply) + 1;
- std::string large_array = "*" + std::to_string(large_count) + "\r\n";
- buf.append(large_array);
-
- brpc::RedisReply reply(&arena);
- brpc::ParseError err = reply.ConsumePartialIOBuf(buf);
- ASSERT_EQ(brpc::PARSE_ERROR_ABSOLUTELY_WRONG, err);
- }
-
- // Test redis_command.cpp limits
- {
- // Test command string exceeding limit
- brpc::RedisCommandParser parser;
- butil::IOBuf buf;
- std::string large_cmd = "*2\r\n$3\r\nget\r\n$2000\r\n";
- large_cmd.append(2000, 'b');
- large_cmd.append("\r\n");
- buf.append(large_cmd);
-
- std::vector<butil::StringPiece> args;
- brpc::ParseError err = parser.Consume(buf, &args, &arena);
- ASSERT_EQ(brpc::PARSE_ERROR_ABSOLUTELY_WRONG, err);
- }
-
- {
- // Test command array size exceeding limit
- brpc::RedisCommandParser parser;
- butil::IOBuf buf;
- int32_t large_array_size = brpc::FLAGS_redis_max_allocation_size /
sizeof(butil::StringPiece) + 1;
- std::string large_array_cmd = "*" + std::to_string(large_array_size) +
"\r\n";
- buf.append(large_array_cmd);
-
- std::vector<butil::StringPiece> args;
- brpc::ParseError err = parser.Consume(buf, &args, &arena);
- ASSERT_EQ(brpc::PARSE_ERROR_ABSOLUTELY_WRONG, err);
- }
-
- // Test valid cases within limits
- {
- // Test small bulk string should work
- butil::IOBuf buf;
- std::string small_string = "*1\r\n$10\r\nhelloworld\r\n";
- buf.append(small_string);
-
- brpc::RedisReply reply(&arena);
- brpc::ParseError err = reply.ConsumePartialIOBuf(buf);
- ASSERT_EQ(brpc::PARSE_OK, err);
- ASSERT_TRUE(reply.is_array());
- ASSERT_EQ(1, (int)reply.size());
- ASSERT_STREQ("helloworld", reply[0].c_str());
- }
-
- {
- // Test small command should work
- brpc::RedisCommandParser parser;
- butil::IOBuf buf;
- std::string small_cmd = "*2\r\n$3\r\nget\r\n$5\r\nmykey\r\n";
- buf.append(small_cmd);
-
- std::vector<butil::StringPiece> args;
- brpc::ParseError err = parser.Consume(buf, &args, &arena);
- ASSERT_EQ(brpc::PARSE_OK, err);
- ASSERT_EQ(2, (int)args.size());
- ASSERT_EQ("get", args[0].as_string());
- ASSERT_EQ("mykey", args[1].as_string());
- }
-
- brpc::FLAGS_redis_max_allocation_size = original_limit;
-}
-
} //namespace
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org
For additional commands, e-mail: dev-h...@brpc.apache.org
