Skip to content

Commit

Permalink
fix(rename-command): Fix subtle UB when renaming commands (#2132)
Browse files Browse the repository at this point in the history
In practive, commands larger than SSO would not work.

Fixes #2131
  • Loading branch information
chakaz authored and romange committed Nov 6, 2023
1 parent ac457a6 commit 053a33d
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/server/command_registry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string_view, 2> maybe_subcommand = StrSplit(cmd.name(), " ");
const bool is_sub_command = maybe_subcommand.size() == 2;
Expand Down
25 changes: 15 additions & 10 deletions src/server/dragonfly_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string>({"flushall=myflushall", "flushdb="}));
absl::SetFlag(
&FLAGS_rename_command,
std::vector<std::string>({"flushall=myflushall", "flushdb=", "ping=abcdefghijklmnop"}));
}

void TearDown() {
Expand All @@ -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) {
Expand Down

0 comments on commit 053a33d

Please sign in to comment.