Skip to content

Commit

Permalink
fix(cluster): Reply with correct \n / \r\n from CLUSTER sub cmd (
Browse files Browse the repository at this point in the history
…#2731)

* fix(cluster): Reply with correct `\n` / `\r\n` from `CLUSTER` sub commands

Fixes #2726

* fix tests
  • Loading branch information
chakaz authored Mar 17, 2024
1 parent 8e38d24 commit e9548a2
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 9 deletions.
4 changes: 3 additions & 1 deletion src/server/cluster/cluster_family.cc
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,8 @@ void ClusterNodesImpl(const ClusterShards& config, string_view my_id, Connection
}
}

absl::StrAppend(&result, "\r\n");
// Separate lines with only \n, not \r\n, see #2726
absl::StrAppend(&result, "\n");
};

for (const auto& shard : config) {
Expand Down Expand Up @@ -280,6 +281,7 @@ namespace {
void ClusterInfoImpl(const ClusterShards& config, ConnectionContext* cntx) {
std::string msg;
auto append = [&msg](absl::AlphaNum a1, absl::AlphaNum a2) {
// Separate lines with \r\n, not \n, see #2726
absl::StrAppend(&msg, a1, ":", a2, "\r\n");
};

Expand Down
16 changes: 8 additions & 8 deletions src/server/cluster/cluster_family_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ TEST_F(ClusterFamilyTest, ClusterConfigNoReplicas) {
"abcd1234")))));

EXPECT_EQ(Run({"cluster", "nodes"}),
"abcd1234 10.0.0.1:7000@7000 master - 0 0 0 connected 0-16383\r\n");
"abcd1234 10.0.0.1:7000@7000 master - 0 0 0 connected 0-16383\n");
}

TEST_F(ClusterFamilyTest, ClusterConfigFull) {
Expand Down Expand Up @@ -259,8 +259,8 @@ TEST_F(ClusterFamilyTest, ClusterConfigFull) {
"wxyz")))));

EXPECT_EQ(Run({"cluster", "nodes"}),
"abcd1234 10.0.0.1:7000@7000 master - 0 0 0 connected 0-16383\r\n"
"wxyz 10.0.0.10:8000@8000 slave abcd1234 0 0 0 connected\r\n");
"abcd1234 10.0.0.1:7000@7000 master - 0 0 0 connected 0-16383\n"
"wxyz 10.0.0.10:8000@8000 slave abcd1234 0 0 0 connected\n");
}

TEST_F(ClusterFamilyTest, ClusterConfigFullMultipleInstances) {
Expand Down Expand Up @@ -383,10 +383,10 @@ TEST_F(ClusterFamilyTest, ClusterConfigFullMultipleInstances) {
"qwerty")))))));

EXPECT_THAT(Run({"cluster", "nodes"}),
"abcd1234 10.0.0.1:7000@7000 master - 0 0 0 connected 0-10000\r\n"
"wxyz 10.0.0.10:8000@8000 slave abcd1234 0 0 0 connected\r\n"
"efgh7890 10.0.0.2:7001@7001 master - 0 0 0 connected 10001-16383\r\n"
"qwerty 10.0.0.11:8001@8001 slave efgh7890 0 0 0 connected\r\n");
"abcd1234 10.0.0.1:7000@7000 master - 0 0 0 connected 0-10000\n"
"wxyz 10.0.0.10:8000@8000 slave abcd1234 0 0 0 connected\n"
"efgh7890 10.0.0.2:7001@7001 master - 0 0 0 connected 10001-16383\n"
"qwerty 10.0.0.11:8001@8001 slave efgh7890 0 0 0 connected\n");

absl::InsecureBitGen eng;
while (true) {
Expand Down Expand Up @@ -818,7 +818,7 @@ TEST_F(ClusterFamilyEmulatedTest, ClusterSlots) {

TEST_F(ClusterFamilyEmulatedTest, ClusterNodes) {
EXPECT_THAT(Run({"cluster", "nodes"}),
GetMyId() + " fake-host:6379@6379 myself,master - 0 0 0 connected 0-16383\r\n");
GetMyId() + " fake-host:6379@6379 myself,master - 0 0 0 connected 0-16383\n");
}

} // namespace
Expand Down

0 comments on commit e9548a2

Please sign in to comment.