This is an automated email from the ASF dual-hosted git repository.
wwbmmm 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 6f0a54a9 Allow enabled_protocols to name http, h2 and rdma_handshake
(#3518)
6f0a54a9 is described below
commit 6f0a54a97e3979ff35fa65ebd7cbf07c236f08b3
Author: Bright Chen <[email protected]>
AuthorDate: Sat Sep 5 15:21:08 2026 +0800
Allow enabled_protocols to name http, h2 and rdma_handshake (#3518)
---
src/brpc/server.cpp | 17 +++++++-----
src/brpc/server.h | 4 +++
test/brpc_server_unittest.cpp | 61 ++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 74 insertions(+), 8 deletions(-)
diff --git a/src/brpc/server.cpp b/src/brpc/server.cpp
index 0852cfc1..baf8b8fa 100644
--- a/src/brpc/server.cpp
+++ b/src/brpc/server.cpp
@@ -17,6 +17,7 @@
#include <iomanip>
+#include <unordered_set>
#include <arpa/inet.h> // inet_aton
#include <fcntl.h> // O_CREAT
#include <sys/stat.h> // mkdir
@@ -619,7 +620,7 @@ BUTIL_FORCE_INLINE bool is_rdma_handshake_protocol(const
char* name) {
}
Acceptor* Server::BuildAcceptor() {
- std::set<std::string> whitelist;
+ std::unordered_set<std::string> whitelist;
for (butil::StringSplitter sp(_options.enabled_protocols.c_str(), ' ');
sp; ++sp) {
std::string protocol(sp.field(), sp.length());
@@ -635,10 +636,13 @@ Acceptor* Server::BuildAcceptor() {
// The protocol does not support server-side.
continue;
}
- if (has_whitelist &&
+ // Erase whatever the exemptions below say. http, h2 and
+ // rdma_handshake are always served, but they are still
+ // valid names for the whitelist.
+ bool in_whitelist = (whitelist.erase(protocols[i].name) != 0);
+ if (has_whitelist && !in_whitelist &&
!is_http_protocol(protocols[i].name) &&
- !is_rdma_handshake_protocol(protocols[i].name) &&
- !whitelist.erase(protocols[i].name)) {
+ !is_rdma_handshake_protocol(protocols[i].name)) {
// the protocol is not allowed to serve.
RPC_VLOG << "Skip protocol=" << protocols[i].name;
continue;
@@ -659,9 +663,8 @@ Acceptor* Server::BuildAcceptor() {
if (!whitelist.empty()) {
std::ostringstream err;
err << "ServerOptions.enabled_protocols has unknown protocols=`";
- for (std::set<std::string>::const_iterator it = whitelist.begin();
- it != whitelist.end(); ++it) {
- err << *it << ' ';
+ for (const auto& protocol : whitelist) {
+ err << protocol << ' ';
}
err << '\'';
delete acceptor;
diff --git a/src/brpc/server.h b/src/brpc/server.h
index 30846f75..276e8795 100644
--- a/src/brpc/server.h
+++ b/src/brpc/server.h
@@ -260,6 +260,10 @@ struct ServerOptions {
// Only enable these protocols, separated by spaces.
// All names inside must be valid, check protocols name in global.cpp
+ // http/h2 and rdma_handshake are served whatever this field says:
+ // the builtin services are only reachable over http/h2, and
+ // rdma_handshake is a transport level handshake dispatching no request.
+ // Naming them here is allowed and changes nothing.
// Default: empty (all protocols)
std::string enabled_protocols;
diff --git a/test/brpc_server_unittest.cpp b/test/brpc_server_unittest.cpp
index 4e64a851..19e2f9a7 100644
--- a/test/brpc_server_unittest.cpp
+++ b/test/brpc_server_unittest.cpp
@@ -460,11 +460,70 @@ TEST_F(ServerTest,
only_allow_protocols_in_enabled_protocols) {
stub.Echo(&cntl, &req, &res, nullptr);
ASSERT_TRUE(cntl.Failed());
ASSERT_TRUE(cntl.ErrorText().find("Got EOF of ") != std::string::npos);
-
+
ASSERT_EQ(0, server.Stop(0));
ASSERT_EQ(0, server.Join());
}
+// http, h2 and rdma_handshake are served whatever enabled_protocols says, but
+// naming them in it used to make Start() fail: the exemption was tested
+// before whitelist.erase() and `&&' short-circuited it, so the names survived
+// into the leftover check and came back as "unknown protocols=`http '".
+TEST_F(ServerTest, enabled_protocols_can_name_always_enabled_protocols) {
+ std::string cases[] = {
+ "baidu_std http",
+ "baidu_std h2",
+ "baidu_std rdma_handshake",
+ "baidu_std http h2 rdma_handshake",
+ };
+ for (size_t i = 0; i < arraysize(cases); ++i) {
+ brpc::Server server;
+ EchoServiceImpl echo_svc;
+ ASSERT_EQ(0, server.AddService(
+ &echo_svc, brpc::SERVER_DOESNT_OWN_SERVICE));
+ brpc::ServerOptions opt;
+ opt.enabled_protocols = cases[i];
+ ASSERT_EQ(0, server.Start("127.0.0.1:0", &opt)) << cases[i];
+ butil::EndPoint ep = server.listen_address();
+
+ brpc::ChannelOptions copt;
+ brpc::Controller cntl;
+ test::EchoRequest req;
+ test::EchoResponse res;
+ req.set_message(EXP_REQUEST);
+
+ // The protocol that actually needed whitelisting still serves.
+ copt.protocol = "baidu_std";
+ brpc::Channel chan;
+ ASSERT_EQ(0, chan.Init(ep, &copt));
+ test::EchoService_Stub stub(&chan);
+ stub.Echo(&cntl, &req, &res, nullptr);
+ ASSERT_FALSE(cntl.Failed()) << cases[i] << ": " << cntl.ErrorText();
+
+ // And so does http, as it does for any whitelist.
+ copt.protocol = "http";
+ brpc::Channel http_channel;
+ ASSERT_EQ(0, http_channel.Init(ep, &copt));
+ cntl.Reset();
+ cntl.http_request().uri() = "/version";
+ http_channel.CallMethod(nullptr, &cntl, nullptr, nullptr, nullptr);
+ ASSERT_FALSE(cntl.Failed()) << cases[i] << ": " << cntl.ErrorText();
+
+ // A protocol left out of the whitelist is still refused.
+ copt.protocol = "hulu_pbrpc";
+ brpc::Channel hulu_channel;
+ ASSERT_EQ(0, hulu_channel.Init(ep, &copt));
+ cntl.Reset();
+ test::EchoService_Stub hulu_stub(&hulu_channel);
+ hulu_stub.Echo(&cntl, &req, &res, nullptr);
+ ASSERT_TRUE(cntl.Failed()) << cases[i];
+ LOG(INFO) << "Expected error: " << cntl.ErrorText();
+
+ ASSERT_EQ(0, server.Stop(0));
+ ASSERT_EQ(0, server.Join());
+ }
+}
+
TEST_F(ServerTest, services_in_different_ns) {
const int port = 9200;
brpc::Server server1;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
