Copilot commented on code in PR #3512:
URL: https://github.com/apache/brpc/pull/3512#discussion_r3901050856
##########
test/brpc_http_message_unittest.cpp:
##########
@@ -484,6 +484,49 @@ TEST(HttpMessageTest, find_method_property_by_uri) {
ASSERT_FALSE(mp);
}
+// A path with empty segments is a different path per RFC 3986, but the
+// splitter used to resolve it skips them, so //flags used to reach the same
+// builtin service as /flags. That difference is what lets a request slip past
+// a front proxy whose ACL only matches the collapsed form, so such paths are
+// rejected rather than collapsed.
+TEST(HttpMessageTest, reject_empty_path_segments) {
+ brpc::Server server;
+ ASSERT_EQ(0, server.AddService(new test::EchoService(),
+ brpc::SERVER_OWNS_SERVICE));
+ ASSERT_EQ(0, server.Start("127.0.0.1:0", nullptr));
+ std::string unknown_method;
+
+ const char* const kRejected[] = {
+ "//",
+ "//flags",
+ "///flags",
+ "/flags//port",
+ "//EchoService/Echo",
+ "/EchoService//Echo",
+ "/EchoService/Echo//",
+ };
+ for (const char* path : kRejected) {
+ ASSERT_FALSE(FindMethodPropertyByURI(path, &server, &unknown_method))
+ << "path=" << path;
+ }
+
+ // The collapsed forms keep working.
+ ASSERT_TRUE(FindMethodPropertyByURI("/", &server, nullptr));
+ ASSERT_TRUE(FindMethodPropertyByURI("/flags/port", &server,
+ &unknown_method));
+ ASSERT_TRUE(FindMethodPropertyByURI("/EchoService/Echo", &server,
+ &unknown_method));
+
+ // -http_allow_empty_path_segments restores the old lenient behavior for
+ // deployments that depend on it.
+ brpc::policy::FLAGS_http_allow_empty_path_segments = true;
+ for (const char* path : kRejected) {
+ ASSERT_TRUE(FindMethodPropertyByURI(path, &server, &unknown_method))
+ << "path=" << path;
+ }
+ brpc::policy::FLAGS_http_allow_empty_path_segments = false;
+}
Review Comment:
This test toggles the global gflag FLAGS_http_allow_empty_path_segments
without an RAII guard. If an ASSERT inside the loop fails, the flag stays
enabled and can leak into later tests, causing order-dependent failures. Use
GFLAGS_NAMESPACE::FlagSaver (or a small scope guard) to restore the original
value on all exits.
--
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]
