Skip to content

Commit 54e2de9

Browse files
authored
fix(server): handle INFO command with multiple sections correctly (#6093)
* fix(server): handle INFO command with multiple sections correctly Fix INFO command to support multiple sections, resolving syntax errors when more than one section is requested. Valid sections are returned, while invalid ones are ignored, matching Redis server behavior. Adds tests for positive and negative multi-section cases. Improves code clarity and ensures metrics are fetched efficiently for relevant sections. Signed-off-by: Gil Levkovich <[email protected]> * fix(server): handle INFO command with multiple sections correctly(fixes) Signed-off-by: Gil Levkovich <[email protected]> --------- Signed-off-by: Gil Levkovich <[email protected]>
1 parent 8df2bd8 commit 54e2de9

File tree

2 files changed

+50
-12
lines changed

2 files changed

+50
-12
lines changed

src/server/server_family.cc

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3374,26 +3374,46 @@ string ServerFamily::FormatInfoMetrics(const Metrics& m, std::string_view sectio
33743374
}
33753375

33763376
void ServerFamily::Info(CmdArgList args, const CommandContext& cmd_cntx) {
3377-
if (args.size() > 1) {
3378-
return cmd_cntx.rb->SendError(kSyntaxErr);
3379-
}
3380-
3381-
string section;
3377+
std::vector<std::string> sections;
3378+
bool need_metrics{false}; // Save time - do not fetch metrics if we don't need them.
3379+
Metrics metrics;
33823380

3383-
if (args.size() == 1) {
3384-
section = absl::AsciiStrToUpper(ArgS(args, 0));
3381+
sections.reserve(args.size());
3382+
for (const auto& arg : args) {
3383+
sections.emplace_back(absl::AsciiStrToUpper(arg));
3384+
const auto& section = sections.back();
3385+
if (!need_metrics && (section != "SERVER") && (section != "REPLICATION")) {
3386+
need_metrics = true;
3387+
}
33853388
}
33863389

3387-
Metrics metrics;
3388-
3389-
// Save time by not calculating metrics if we don't need them.
3390-
if (!(section == "SERVER" || section == "REPLICATION")) {
3390+
if (need_metrics || sections.empty()) {
33913391
metrics = GetMetrics(cmd_cntx.conn_cntx->ns);
33923392
} else if (!IsMaster()) {
33933393
metrics.replica_side_info = GetReplicaSummary();
33943394
}
33953395

3396-
string info = FormatInfoMetrics(metrics, section, cmd_cntx.conn_cntx->conn()->IsPrivileged());
3396+
std::string info;
3397+
// For multiple requested sections, invalid section names are ignored (not included in the
3398+
// output). The command does not abort or return an error if some sections are invalid. This
3399+
// matches Valkey behavior.
3400+
if (sections.empty()) { // No sections: default to all sections.
3401+
info = FormatInfoMetrics(metrics, "", cmd_cntx.conn_cntx->conn()->IsPrivileged());
3402+
} else if (sections.size() == 1) { // Single section
3403+
info = FormatInfoMetrics(metrics, sections[0], cmd_cntx.conn_cntx->conn()->IsPrivileged());
3404+
} else { // Multiple sections: concatenate results for each requested section.
3405+
for (const auto& section : sections) {
3406+
const std::string section_str =
3407+
FormatInfoMetrics(metrics, section, cmd_cntx.conn_cntx->conn()->IsPrivileged());
3408+
if (!section_str.empty()) {
3409+
if (!info.empty()) {
3410+
absl::StrAppend(&info, "\r\n", section_str);
3411+
} else {
3412+
info = section_str;
3413+
}
3414+
}
3415+
}
3416+
}
33973417

33983418
auto* rb = static_cast<RedisReplyBuilder*>(cmd_cntx.rb);
33993419
rb->SendVerbatimString(info);

src/server/server_family_test.cc

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -646,4 +646,22 @@ TEST_F(ServerFamilyTest, PubSubCommandErr) {
646646
"PUBSUB HELP."));
647647
}
648648

649+
TEST_F(ServerFamilyTest, InfoMultipleSections) {
650+
// Check that when querying multiple valid sections, both are returned non empty.
651+
Run({"set", "foo", "bar"}); // set some data
652+
auto resp = Run({"info", "replication", "persistence"});
653+
auto info = resp.GetString();
654+
EXPECT_NE(info.find("# Replication"), std::string::npos);
655+
EXPECT_NE(info.find("# Persistence"), std::string::npos);
656+
}
657+
658+
TEST_F(ServerFamilyTest, InfoMultipleSectionsInvalid) {
659+
// Check that when querying a valid and an invalid section, only the valid section is returned.
660+
Run({"set", "foo", "bar"}); // set some data
661+
auto resp = Run({"info", "replication", "invalidsection"});
662+
auto info = resp.GetString();
663+
EXPECT_NE(info.find("# Replication"), std::string::npos);
664+
EXPECT_EQ(info.find("# invalidsection"), std::string::npos);
665+
}
666+
649667
} // namespace dfly

0 commit comments

Comments
 (0)