This is an automated email from the ASF dual-hosted git repository.
chenBright pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new b13bbcef Validate Consul response object types (#3584)
b13bbcef is described below
commit b13bbcefecdc758a987973f1901b279e43ec521e
Author: Xiaofeng Wang <[email protected]>
AuthorDate: Tue Oct 6 18:21:57 2026 +0800
Validate Consul response object types (#3584)
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 5bee4093..bf35f5bb 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 8ef33150..d54f39d4 100644
--- a/test/brpc_naming_service_unittest.cpp
+++ b/test/brpc_naming_service_unittest.cpp
@@ -427,6 +427,72 @@ public:
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;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
