sahvx655-wq commented on code in PR #3622:
URL: https://github.com/apache/brpc/pull/3622#discussion_r4308539564
##########
test/brpc_rtmp_unittest.cpp:
##########
@@ -1286,6 +1286,71 @@ TEST(RtmpTest, reject_unauthenticated_connection) {
server.Join();
}
+class RejectAllAuthenticator : public brpc::Authenticator {
+public:
+ int GenerateCredential(std::string*) const override { return 0; }
+ int VerifyCredential(const std::string&, const butil::EndPoint&,
+ brpc::AuthContext*) const override { return -1; }
+};
+
+// Returns a port nothing is listening on, or -1.
+static int PickUnusedPort() {
+ butil::fd_guard sockfd(butil::tcp_listen(butil::EndPoint(butil::IP_ANY,
0)));
+ if (sockfd < 0) {
+ return -1;
+ }
+ butil::EndPoint point;
+ if (butil::get_local_side(sockfd, &point) != 0) {
+ return -1;
+ }
+ return point.port;
+}
+
+TEST(RtmpTest, not_served_on_internal_port) {
+ PlayingDummyService rtmp_service;
+ RejectAllAuthenticator auth;
+ brpc::Server server;
+ brpc::ServerOptions server_opt;
+ server_opt.rtmp_service = &rtmp_service;
+ server_opt.auth = &auth;
+ bool started = false;
+ for (int i = 0; i < 10 && !started; ++i) {
+ server_opt.internal_port = PickUnusedPort();
+ started = (server_opt.internal_port >= 0 &&
+ 0 == server.Start("127.0.0.1:0", &server_opt));
+ }
+ ASSERT_TRUE(started);
+
+ butil::EndPoint internal_point;
+ ASSERT_EQ(0, butil::str2endpoint(
+ "127.0.0.1", server_opt.internal_port, &internal_point));
+ butil::fd_guard fd(butil::tcp_connect(internal_point, nullptr));
+ ASSERT_GE(fd, 0);
+ timeval tv = { 2, 0 };
+ ASSERT_EQ(0, setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)));
+
+ // Builtin services on internal_port pass authentication without any
+ // credential, which latches the connection as authenticated.
+ const std::string http_req = "GET /version HTTP/1.1\r\n\r\n";
+ ASSERT_EQ((ssize_t)http_req.size(),
+ write(fd, http_req.data(), http_req.size()));
+ char resp[1024];
+ ASSERT_GT(read(fd, resp, sizeof(resp)), 0);
+ ASSERT_EQ(0, memcmp(resp, "HTTP/1.1 200", 12));
Review Comment:
Fair point, a short read there would have had memcmp looking past the bytes
that actually arrived. It never showed up locally because the whole /version
reply comes back in one segment on loopback, but nothing guarantees that. The
test now loops on read() until the 12-byte status prefix is buffered and only
then compares it, so a split reply just takes another iteration instead of a
stale comparison.
The RTMP part of the test is unchanged, it still reads a single byte after
sending C0C1 and expects the connection to be closed rather than an S0. Rebuilt
brpc_rtmp_unittest with the change and all 32 tests pass, including
not_served_on_internal_port.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
