From 053a33d24df939d1c35f3b6f233a0c57ac185184 Mon Sep 17 00:00:00 2001 From: Shahar Mike Date: Mon, 6 Nov 2023 13:31:23 +0200 Subject: [PATCH] fix(rename-command): Fix subtle UB when renaming commands (#2132) In practive, commands larger than SSO would not work. Fixes #2131 --- src/server/command_registry.cc | 2 +- src/server/dragonfly_test.cc | 25 +++++++++++++++---------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/server/command_registry.cc b/src/server/command_registry.cc index 5e3483acf..fc1eae083 100644 --- a/src/server/command_registry.cc +++ b/src/server/command_registry.cc @@ -117,7 +117,7 @@ void CommandRegistry::Init(unsigned int thread_count) { } CommandRegistry& CommandRegistry::operator<<(CommandId cmd) { - auto k = cmd.name(); + string k = string(cmd.name()); absl::InlinedVector maybe_subcommand = StrSplit(cmd.name(), " "); const bool is_sub_command = maybe_subcommand.size() == 2; diff --git a/src/server/dragonfly_test.cc b/src/server/dragonfly_test.cc index cde768de9..507d89c27 100644 --- a/src/server/dragonfly_test.cc +++ b/src/server/dragonfly_test.cc @@ -99,8 +99,9 @@ class DflyRenameCommandTest : public DflyEngineTest { protected: DflyRenameCommandTest() : DflyEngineTest() { // rename flushall to myflushall, flushdb command will not be able to execute - absl::SetFlag(&FLAGS_rename_command, - std::vector({"flushall=myflushall", "flushdb="})); + absl::SetFlag( + &FLAGS_rename_command, + std::vector({"flushall=myflushall", "flushdb=", "ping=abcdefghijklmnop"})); } void TearDown() { @@ -113,16 +114,20 @@ TEST_F(DflyRenameCommandTest, RenameCommand) { Run({"set", "a", "1"}); ASSERT_EQ(1, CheckedInt({"dbsize"})); // flushall should not execute anything and should return error, as it was renamed. - RespExpr resp = Run({"flushall"}); - ASSERT_THAT(resp, ErrArg("unknown command `FLUSHALL`")); + ASSERT_THAT(Run({"flushall"}), ErrArg("unknown command `FLUSHALL`")); + ASSERT_EQ(1, CheckedInt({"dbsize"})); - resp = Run({"myflushall"}); - ASSERT_EQ(resp, "OK"); + + ASSERT_EQ(Run({"myflushall"}), "OK"); + ASSERT_EQ(0, CheckedInt({"dbsize"})); - resp = Run({"flushdb", "0"}); - ASSERT_THAT(resp, ErrArg("unknown command `FLUSHDB`")); - resp = Run({""}); - ASSERT_THAT(resp, ErrArg("unknown command ``")); + + ASSERT_THAT(Run({"flushdb", "0"}), ErrArg("unknown command `FLUSHDB`")); + + ASSERT_THAT(Run({""}), ErrArg("unknown command ``")); + + ASSERT_THAT(Run({"ping"}), ErrArg("unknown command `PING`")); + ASSERT_THAT(Run({"abcdefghijklmnop"}), "PONG"); } TEST_F(SingleThreadDflyEngineTest, GlobalSingleThread) {