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]>

Author: glevkovich

src/server/server_family.cc

@@ -3270,7 +3270,7 @@ string ServerFamily::FormatInfoMetrics(const Metrics& m, std::string_view sectio
       if (show) {
         size_t total = stats.events.hits + stats.events.misses;
         double hit_ratio =
-            (total > 0) ? static_cast<double>(stats.events.hits) / (total)*100.0 : 0.0;
+            (total > 0) ? static_cast<double>(stats.events.hits) / (total) * 100.0 : 0.0;
         string val = StrCat("keys=", stats.key_count, ",expires=", stats.expire_count,
                             ",hits=", stats.events.hits, ",misses=", stats.events.misses,
                             ",hit_ratio=", absl::StrFormat("%.2f", hit_ratio),
@@ -3330,26 +3330,44 @@ string ServerFamily::FormatInfoMetrics(const Metrics& m, std::string_view sectio
 }
 
 void ServerFamily::Info(CmdArgList args, const CommandContext& cmd_cntx) {
-  if (args.size() > 1) {
-    return cmd_cntx.rb->SendError(kSyntaxErr);
-  }
-
-  string section;
+  std::vector<std::string> sections;
+  auto need_metrics{false};  // Save time - do not fetch metrics if we don't need them.
+  Metrics metrics;
 
-  if (args.size() == 1) {
-    section = absl::AsciiStrToUpper(ArgS(args, 0));
+  sections.reserve(args.size());
+  for (const auto& arg : args) {
+    sections.emplace_back(absl::AsciiStrToUpper(arg));
+    const auto& section = sections.back();
+    if (!need_metrics && (section != "SERVER") && (section != "REPLICATION")) {
+      need_metrics = true;
+      metrics = GetMetrics(cmd_cntx.conn_cntx->ns);
+    }
   }
 
-  Metrics metrics;
-
-  // Save time by not calculating metrics if we don't need them.
-  if (!(section == "SERVER" || section == "REPLICATION")) {
-    metrics = GetMetrics(cmd_cntx.conn_cntx->ns);
-  } else if (!IsMaster()) {
+  if (!need_metrics && !IsMaster()) {
     metrics.replica_side_info = GetReplicaSummary();
   }
 
-  string info = FormatInfoMetrics(metrics, section, cmd_cntx.conn_cntx->conn()->IsPrivileged());
+  std::string info;
+  // For multiple requested sections, invalid section names are ignored (not included in the output).
+  // The command does not abort or return an error if some sections are invalid. This matches Redis behavior.
+  if (sections.empty()) {  // No sections: default to all sections.
+    info = FormatInfoMetrics(metrics, "", cmd_cntx.conn_cntx->conn()->IsPrivileged());
+  } else if (sections.size() == 1) {  // Single section
+    info = FormatInfoMetrics(metrics, sections[0], cmd_cntx.conn_cntx->conn()->IsPrivileged());
+  } else {  // Multiple sections: concatenate results for each requested section.
+    for (const auto& section : sections) {
+      const std::string section_str =
+          FormatInfoMetrics(metrics, section, cmd_cntx.conn_cntx->conn()->IsPrivileged());
+      if (!section_str.empty()) {
+        if (!info.empty()) {
+          absl::StrAppend(&info, "\r\n", section_str);
+        } else {
+          info = section_str;
+        }
+      }
+    }
+  }
 
   auto* rb = static_cast<RedisReplyBuilder*>(cmd_cntx.rb);
   rb->SendVerbatimString(info);

src/server/server_family_test.cc

@@ -646,4 +646,22 @@ TEST_F(ServerFamilyTest, PubSubCommandErr) {
                      "PUBSUB HELP."));
 }
 
+TEST_F(ServerFamilyTest, InfoMultipleSections) {
+  // Check that when querying multiple valid sections, both are returned non empty.
+  Run({"set", "foo", "bar"}); // set some data
+  auto resp = Run({"info", "replication", "persistence"});
+  auto info = resp.GetString();
+  EXPECT_NE(info.find("# Replication"), std::string::npos);
+  EXPECT_NE(info.find("# Persistence"), std::string::npos);
+}
+
+TEST_F(ServerFamilyTest, InfoMultipleSectionsInvalid) {
+  // Check that when querying a valid and an invalid section, only the valid section is returned.
+  Run({"set", "foo", "bar"}); // set some data
+  auto resp = Run({"info", "replication", "invalidsection"});
+  auto info = resp.GetString();
+  EXPECT_NE(info.find("# Replication"), std::string::npos);
+  EXPECT_EQ(info.find("# invalidsection"), std::string::npos);
+}
+
 }  // namespace dfly