diff --git a/src/brpc/builtin/common.cpp b/src/brpc/builtin/common.cpp index bfb8e54628..1e1ff05bb6 100644 --- a/src/brpc/builtin/common.cpp +++ b/src/brpc/builtin/common.cpp @@ -25,6 +25,8 @@ #include "butil/file_util.h" // butil::FilePath #include "butil/third_party/murmurhash3/murmurhash3.h" #include "butil/process_util.h" // ReadCommandLine +#include "brpc/controller.h" +#include "brpc/grpc.h" #include "brpc/server.h" #include "brpc/reloadable_flags.h" #include "brpc/builtin/common.h" @@ -122,27 +124,36 @@ void PrintRealDateTime(std::ostream& os, int64_t tm, } } +static std::string EncodeUriPath(const char* path) { + std::string encoded; + PercentEncode(path, &encoded); + size_t pos = 0; + while ((pos = encoded.find("%2f", pos)) != std::string::npos) { + encoded.replace(pos, 3, "/"); + ++pos; + } + return encoded; +} + std::ostream& operator<<(std::ostream& os, const PrintedAsDateTime& d) { PrintRealDateTime(os, d.realtime); return os; } std::ostream& operator<<(std::ostream& os, const Path& link) { - if (link.html_addr) { - if (link.html_addr != Path::LOCAL) { - os << ""; - } else { - os << ""; - } + const char* const text = (link.text ? link.text : link.uri); + if (!link.html_addr) { + os << text; + return os; } - if (link.text) { - os << link.text; + + const std::string escaped_uri = WebEscape(EncodeUriPath(link.uri)); + if (link.html_addr != Path::LOCAL) { + os << ""; } else { - os << link.uri; - } - if (link.html_addr) { - os << ""; + os << ""; } + os << WebEscape(text) << ""; return os; } diff --git a/src/brpc/builtin/hotspots_service.cpp b/src/brpc/builtin/hotspots_service.cpp index 7ed4295312..71c72201d5 100644 --- a/src/brpc/builtin/hotspots_service.cpp +++ b/src/brpc/builtin/hotspots_service.cpp @@ -24,8 +24,10 @@ #include "butil/popen.h" // butil::read_command_output #include "butil/fd_guard.h" // butil::fd_guard #include "butil/iobuf_profiler.h" +#include "butil/strings/string_util.h" #include "brpc/log.h" #include "brpc/controller.h" +#include "brpc/grpc.h" #include "brpc/server.h" #include "brpc/reloadable_flags.h" #include "brpc/builtin/pprof_perl.h" @@ -1063,6 +1065,9 @@ static void StartProfiling(ProfilingType type, for (butil::FilePath name = prof_enum.Next(); !name.empty(); name = prof_enum.Next()) { // NOTE: name already includes dir. + if (!ValidProfilePath(name.value())) { + continue; + } if (past_profs.empty()) { past_profs.reserve(16); } @@ -1095,11 +1100,11 @@ static void StartProfiling(ProfilingType type, ""; os << "
Display: 
" @@ -1120,11 +1125,11 @@ static void StartProfiling(ProfilingType type, "
"; } diff --git a/test/brpc_builtin_service_unittest.cpp b/test/brpc_builtin_service_unittest.cpp index 79444d6892..f3f666b313 100644 --- a/test/brpc_builtin_service_unittest.cpp +++ b/test/brpc_builtin_service_unittest.cpp @@ -22,16 +22,20 @@ #include #include #include +#include #include #include #include #include "gperftools_helper.h" +#include "butil/file_util.h" +#include "butil/files/scoped_temp_dir.h" #include "butil/time.h" #include "butil/macros.h" #include "brpc/socket.h" #include "brpc/server.h" #include "brpc/channel.h" #include "brpc/controller.h" +#include "brpc/grpc.h" #include "brpc/span.h" #include "brpc/reloadable_flags.h" #include "brpc/builtin/version_service.h" @@ -815,6 +819,76 @@ TEST_F(BuiltinServiceTest, hotspots_query_escaping) { } } +TEST_F(BuiltinServiceTest, hotspots_profile_filename_escaping) { + GFLAGS_NAMESPACE::FlagSaver flag_saver; + butil::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + brpc::FLAGS_rpc_profiling_dir = temp_dir.path().value(); + const butil::FilePath profile_dir = temp_dir.path().Append( + brpc::GetProgramChecksum()); + ASSERT_TRUE(butil::CreateDirectory(profile_dir)); + + const std::string filename = "profile'>.cpu"; + const butil::FilePath profile_path = profile_dir.Append(filename); + ASSERT_EQ(0, butil::WriteFile(profile_path, "", 0)); + + ClosureChecker done; + brpc::Controller cntl; + brpc::HotspotsRequest req; + brpc::HotspotsResponse res; + brpc::HotspotsService service; + SetUpController(&cntl, true); + service.cpu(&cntl, &req, &res, &done); + + const std::string& body = cntl.response_attachment().to_string(); + EXPECT_EQ(std::string::npos, body.find(filename)) + << "unescaped profile filename in html: " << body; + EXPECT_EQ(std::string::npos, body.find(brpc::WebEscape(filename))); + +} + +TEST_F(BuiltinServiceTest, hotspots_non_responsive_raw_profile_paths) { + GFLAGS_NAMESPACE::FlagSaver flag_saver; + butil::ScopedTempDir temp_dir; + ASSERT_TRUE(temp_dir.CreateUniqueTempDir()); + brpc::FLAGS_rpc_profiling_dir = temp_dir.path().value(); + const butil::FilePath profile_dir = temp_dir.path().Append( + brpc::GetProgramChecksum()); + ASSERT_TRUE(butil::CreateDirectory(profile_dir)); + const butil::FilePath view_path = profile_dir.Append("selected.cpu"); + const butil::FilePath base_path = profile_dir.Append("base.cpu"); + ASSERT_EQ(0, butil::WriteFile(view_path, "", 0)); + ASSERT_EQ(0, butil::WriteFile(base_path, "", 0)); + + ClosureChecker done; + brpc::Controller cntl; + brpc::HotspotsRequest req; + brpc::HotspotsResponse res; + brpc::HotspotsService service; + SetUpController(&cntl, true); + cntl.http_request().uri().SetQuery("view", view_path.value()); + cntl.http_request().uri().SetQuery("base", base_path.value()); + service.cpu_non_responsive(&cntl, &req, &res, &done); + EXPECT_FALSE(cntl.Failed()) << cntl.ErrorText(); +} + +TEST_F(BuiltinServiceTest, path_html_escaping) { + const std::string payload = "\"'>&"; + std::ostringstream html; + html << brpc::Path(payload.c_str(), brpc::Path::LOCAL, payload.c_str()); + EXPECT_EQ(std::string::npos, html.str().find(payload)); + EXPECT_NE(std::string::npos, html.str().find(brpc::WebEscape(payload))); + + std::ostringstream reserved_html; + reserved_html << brpc::Path("/dir/a?x#fragment", brpc::Path::LOCAL); + EXPECT_NE(std::string::npos, + reserved_html.str().find("href=\"/dir/a%3fx%23fragment\"")); + + std::ostringstream text; + text << brpc::Path(payload.c_str(), nullptr, payload.c_str()); + EXPECT_EQ(payload, text.str()); +} + TEST_F(BuiltinServiceTest, rpcz_query_escaping) { // Restore -enable_rpcz on any exit of this test. GFLAGS_NAMESPACE::FlagSaver flag_saver;