From 6d98fe2d8801228cad8bd85c4e8c36e1306849e9 Mon Sep 17 00:00:00 2001
From: Wang Xiaofeng <wasphin@gmail.com>
Date: Tue, 6 Oct 2026 16:48:41 +0800
Subject: [PATCH] Validate Consul response object types

Check that each node and its Service value are JSON objects before using
object accessors. Skip invalid entries while retaining valid services and
reject responses containing only invalid entries.

Add HTTP client/server regression coverage for non-object values at both
boundaries, mixed valid and invalid entries, and stale result clearing.
---
 src/brpc/policy/consul_naming_service.cpp | 10 ++++
 test/brpc_naming_service_unittest.cpp     | 66 +++++++++++++++++++++++
 2 files changed, 76 insertions(+)

diff --git a/src/brpc/policy/consul_naming_service.cpp b/src/brpc/policy/consul_naming_service.cpp
index 5bee4093aa..bf35f5bbb3 100644
--- a/src/brpc/policy/consul_naming_service.cpp
+++ b/src/brpc/policy/consul_naming_service.cpp
@@ -137,6 +137,11 @@ int ConsulNamingService::GetServers(const char* service_name,
     }
 
     for (BUTIL_RAPIDJSON_NAMESPACE::SizeType i = 0; i < services.Size(); ++i) {
+        if (!services[i].IsObject()) {
+            LOG(ERROR) << "Service node is not a json object: "
+                       << RapidjsonValueToString(services[i]);
+            continue;
+        }
         auto itr_service = services[i].FindMember("Service");
         if (itr_service == services[i].MemberEnd()) {
             LOG(ERROR) << "No service info in node: "
@@ -145,6 +150,11 @@ int ConsulNamingService::GetServers(const char* service_name,
         }
 
         const BUTIL_RAPIDJSON_NAMESPACE::Value& service = itr_service->value;
+        if (!service.IsObject()) {
+            LOG(ERROR) << "Service info is not a json object: "
+                       << RapidjsonValueToString(service);
+            continue;
+        }
         auto itr_address = service.FindMember("Address");
         auto itr_port = service.FindMember("Port");
         if (itr_address == service.MemberEnd() ||
diff --git a/test/brpc_naming_service_unittest.cpp b/test/brpc_naming_service_unittest.cpp
index 8ef3315055..d54f39d455 100644
--- a/test/brpc_naming_service_unittest.cpp
+++ b/test/brpc_naming_service_unittest.cpp
@@ -427,6 +427,72 @@ class ConsulNamingServiceImpl : public test::UserNamingService {
     butil::atomic<int64_t> touch_count;
 };
 
+class ConsulResponseService : public test::UserNamingService {
+public:
+    explicit ConsulResponseService(const std::string& response)
+        : _response(response) {}
+
+    void ListNames(google::protobuf::RpcController* cntl_base,
+                   const test::HttpRequest*,
+                   test::HttpResponse*,
+                   google::protobuf::Closure* done) override {
+        brpc::ClosureGuard done_guard(done);
+        brpc::Controller* cntl = static_cast<brpc::Controller*>(cntl_base);
+        cntl->http_response().SetHeader("X-Consul-Index", "1");
+        cntl->response_attachment().append(_response);
+    }
+
+private:
+    const std::string _response;
+};
+
+TEST(NamingServiceTest, consul_response_object_types) {
+    GFLAGS_NAMESPACE::FlagSaver flags_saver;
+    brpc::policy::FLAGS_consul_enable_degrade_to_file_naming_service = false;
+    brpc::policy::FLAGS_consul_service_discovery_url = "/v1/health/service/";
+    const char* invalid_entries[] = {
+        R"(null, true, false, 1, 1.5, "node", [])",
+        R"({"Service":null}, {"Service":true}, {"Service":false},
+            {"Service":1}, {"Service":1.5}, {"Service":"service"},
+            {"Service":[]})"
+    };
+    butil::EndPoint endpoint;
+    ASSERT_EQ(0, butil::str2endpoint("127.0.0.1:8003", &endpoint));
+    const brpc::ServerNode expected_node(endpoint, "tag");
+
+    for (const char* entries : invalid_entries) {
+        for (bool with_valid_service : {false, true}) {
+            SCOPED_TRACE(entries);
+            SCOPED_TRACE(with_valid_service);
+            std::string response = std::string("[") + entries;
+            if (with_valid_service) {
+                response += R"(,{"Service":{"Address":"127.0.0.1",
+                              "Port":8003,"Tags":["tag"]}})";
+            }
+            response += "]";
+            ConsulResponseService svc(response);
+            brpc::Server server;
+            ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE,
+                                          "/v1/health/service/test => ListNames"));
+            ASSERT_EQ(0, server.Start(0, nullptr));
+            brpc::policy::FLAGS_consul_agent_addr = butil::string_printf(
+                "http://%s", butil::endpoint2str(server.listen_address()).c_str());
+
+            brpc::policy::ConsulNamingService cns;
+            // GetServers must clear stale results, including on invalid input.
+            std::vector<brpc::ServerNode> servers(1, expected_node);
+            ASSERT_EQ(with_valid_service ? 0 : -1,
+                      cns.GetServers("test", &servers));
+            if (with_valid_service) {
+                ASSERT_EQ(1u, servers.size());
+                EXPECT_EQ(expected_node, servers[0]);
+            } else {
+                EXPECT_TRUE(servers.empty());
+            }
+        }
+    }
+}
+
 TEST(NamingServiceTest, consul_with_backup_file) {
     GFLAGS_NAMESPACE::FlagSaver flags_saver;
     brpc::policy::FLAGS_consul_enable_degrade_to_file_naming_service = true;