Copilot commented on code in PR #3382:
URL: https://github.com/apache/brpc/pull/3382#discussion_r3566347619
##########
test/brpc_couchbase_unittest.cpp:
##########
@@ -34,6 +39,28 @@ namespace {
// Unit Tests - No Server Required
class CouchbaseUnitTest : public testing::Test {};
+TEST_F(CouchbaseUnitTest, RejectOversizedResponseBeforeBufferingBody) {
+ const uint64_t saved_max_body_size = brpc::FLAGS_max_body_size;
+ brpc::FLAGS_max_body_size = 1024;
+
+ brpc::SocketId id;
+ brpc::SocketOptions options;
+ ASSERT_EQ(0, brpc::Socket::Create(options, &id));
+ brpc::SocketUniquePtr socket;
+ ASSERT_EQ(0, brpc::Socket::Address(id, &socket));
+
+ brpc::policy::CouchbaseResponseHeader couchbase_header = {};
+ couchbase_header.magic = brpc::policy::CB_MAGIC_RESPONSE;
+ couchbase_header.total_body_length = butil::HostToNet32(1025);
+ butil::IOBuf couchbase_buf;
+ couchbase_buf.append(&couchbase_header, sizeof(couchbase_header));
+ EXPECT_EQ(brpc::PARSE_ERROR_TOO_BIG_DATA,
+ brpc::policy::ParseCouchbaseMessage(
+ &couchbase_buf, socket.get(), false, NULL).error());
+
+ brpc::FLAGS_max_body_size = saved_max_body_size;
Review Comment:
The test mutates the global FLAGS_max_body_size but restores it manually at
the end. If any ASSERT_* before the restore line fails, the flag won't be
restored and can leak into later tests in the same binary (causing flaky
failures). Prefer an RAII restore guard immediately after changing the flag.
##########
test/brpc_memcache_unittest.cpp:
##########
@@ -33,6 +39,29 @@ int main(int argc, char* argv[]) {
}
namespace {
+
+TEST(MemcacheParserTest, RejectOversizedResponseBeforeBufferingBody) {
+ const uint64_t saved_max_body_size = brpc::FLAGS_max_body_size;
+ brpc::FLAGS_max_body_size = 1024;
+
+ brpc::SocketId id;
+ brpc::SocketOptions options;
+ ASSERT_EQ(0, brpc::Socket::Create(options, &id));
+ brpc::SocketUniquePtr socket;
+ ASSERT_EQ(0, brpc::Socket::Address(id, &socket));
+
+ brpc::policy::MemcacheResponseHeader header = {};
+ header.magic = brpc::policy::MC_MAGIC_RESPONSE;
+ header.total_body_length = butil::HostToNet32(1025);
+ butil::IOBuf buf;
+ buf.append(&header, sizeof(header));
+ EXPECT_EQ(brpc::PARSE_ERROR_TOO_BIG_DATA,
+ brpc::policy::ParseMemcacheMessage(
+ &buf, socket.get(), false, NULL).error());
+
+ brpc::FLAGS_max_body_size = saved_max_body_size;
Review Comment:
The test mutates the global FLAGS_max_body_size but restores it manually at
the end. If any ASSERT_* before the restore line fails, the flag won't be
restored and can leak into later tests in the same binary (causing flaky
failures). Prefer an RAII restore guard immediately after changing the flag.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
