diff --git a/envoy/server/admin.h b/envoy/server/admin.h index 4ca2bbefbe5ee..658f6f631f9b1 100644 --- a/envoy/server/admin.h +++ b/envoy/server/admin.h @@ -72,9 +72,9 @@ class AdminStream { * done in the RouteConfigProviderManagerImpl constructor in source/common/router/rds_impl.cc. */ #define MAKE_ADMIN_HANDLER(X) \ - [this](absl::string_view path_and_query, Http::ResponseHeaderMap& response_headers, \ - Buffer::Instance& data, Server::AdminStream& admin_stream) -> Http::Code { \ - return X(path_and_query, response_headers, data, admin_stream); \ + [this](Http::ResponseHeaderMap& response_headers, Buffer::Instance& data, \ + Server::AdminStream& admin_stream) -> Http::Code { \ + return X(response_headers, data, admin_stream); \ } /** @@ -152,7 +152,7 @@ class Admin { /** * Lambda to generate a Request. */ - using GenRequestFn = std::function; + using GenRequestFn = std::function; /** * Individual admin handler including prefix, help text, and callback. @@ -176,9 +176,9 @@ class Admin { * its data. * @return Http::Code the response code. */ - using HandlerCb = std::function; + using HandlerCb = + std::function; /** * Add a legacy admin handler where the entire response is written in diff --git a/source/extensions/common/tap/admin.cc b/source/extensions/common/tap/admin.cc index 89d8fbb6e3543..8fed89e7660de 100644 --- a/source/extensions/common/tap/admin.cc +++ b/source/extensions/common/tap/admin.cc @@ -42,7 +42,7 @@ AdminHandler::~AdminHandler() { ASSERT(rc); } -Http::Code AdminHandler::handler(absl::string_view, Http::HeaderMap&, Buffer::Instance& response, +Http::Code AdminHandler::handler(Http::HeaderMap&, Buffer::Instance& response, Server::AdminStream& admin_stream) { if (attached_request_ != nullptr) { // TODO(mattlklein123): Consider supporting concurrent admin /tap streams. Right now we support diff --git a/source/extensions/common/tap/admin.h b/source/extensions/common/tap/admin.h index 2765f9dc70aff..56ffffcde436d 100644 --- a/source/extensions/common/tap/admin.h +++ b/source/extensions/common/tap/admin.h @@ -204,8 +204,8 @@ class AdminHandler : public Singleton::Instance, AdminHandler& parent_; }; - Http::Code handler(absl::string_view path_and_query, Http::HeaderMap& response_headers, - Buffer::Instance& response, Server::AdminStream& admin_stream); + Http::Code handler(Http::HeaderMap& response_headers, Buffer::Instance& response, + Server::AdminStream& admin_stream); Http::Code badRequest(Buffer::Instance& response, absl::string_view error); Server::Admin& admin_; diff --git a/source/extensions/stat_sinks/hystrix/hystrix.cc b/source/extensions/stat_sinks/hystrix/hystrix.cc index 1df26f9e9448f..2ae45959cdf37 100644 --- a/source/extensions/stat_sinks/hystrix/hystrix.cc +++ b/source/extensions/stat_sinks/hystrix/hystrix.cc @@ -290,8 +290,7 @@ HystrixSink::HystrixSink(Server::Configuration::ServerFactoryContext& server, MAKE_ADMIN_HANDLER(handlerHystrixEventStream), false, false); } -Http::Code HystrixSink::handlerHystrixEventStream(absl::string_view, - Http::ResponseHeaderMap& response_headers, +Http::Code HystrixSink::handlerHystrixEventStream(Http::ResponseHeaderMap& response_headers, Buffer::Instance&, Server::AdminStream& admin_stream) { diff --git a/source/extensions/stat_sinks/hystrix/hystrix.h b/source/extensions/stat_sinks/hystrix/hystrix.h index ab2502d762eea..caad4be42ebba 100644 --- a/source/extensions/stat_sinks/hystrix/hystrix.h +++ b/source/extensions/stat_sinks/hystrix/hystrix.h @@ -48,8 +48,8 @@ using ClusterStatsCachePtr = std::unique_ptr; class HystrixSink : public Stats::Sink, public Logger::Loggable { public: HystrixSink(Server::Configuration::ServerFactoryContext& server, uint64_t num_buckets); - Http::Code handlerHystrixEventStream(absl::string_view, Http::ResponseHeaderMap& response_headers, - Buffer::Instance&, Server::AdminStream& admin_stream); + Http::Code handlerHystrixEventStream(Http::ResponseHeaderMap& response_headers, Buffer::Instance&, + Server::AdminStream& admin_stream); void flush(Stats::MetricSnapshot& snapshot) override; void onHistogramComplete(const Stats::Histogram&, uint64_t) override{}; diff --git a/source/server/admin/admin.cc b/source/server/admin/admin.cc index 61aab94fde56d..e77b73ff3710c 100644 --- a/source/server/admin/admin.cc +++ b/source/server/admin/admin.cc @@ -295,20 +295,17 @@ class StaticTextRequest : public Admin::Request { // generates the entire admin output in one shot. class RequestGasket : public Admin::Request { public: - RequestGasket(Admin::HandlerCb handler_cb, absl::string_view path_and_query, - AdminStream& admin_stream) - : path_and_query_(std::string(path_and_query)), handler_cb_(handler_cb), - admin_stream_(admin_stream) {} + RequestGasket(Admin::HandlerCb handler_cb, AdminStream& admin_stream) + : handler_cb_(handler_cb), admin_stream_(admin_stream) {} static Admin::GenRequestFn makeGen(Admin::HandlerCb callback) { - return [callback](absl::string_view path_and_query, - AdminStream& admin_stream) -> Server::Admin::RequestPtr { - return std::make_unique(callback, path_and_query, admin_stream); + return [callback](AdminStream& admin_stream) -> Server::Admin::RequestPtr { + return std::make_unique(callback, admin_stream); }; } Http::Code start(Http::ResponseHeaderMap& response_headers) override { - return handler_cb_(path_and_query_, response_headers, response_, admin_stream_); + return handler_cb_(response_headers, response_, admin_stream_); } bool nextChunk(Buffer::Instance& response) override { @@ -317,7 +314,6 @@ class RequestGasket : public Admin::Request { } private: - std::string path_and_query_; Admin::HandlerCb handler_cb_; AdminStream& admin_stream_; Buffer::OwnedImpl response_; @@ -333,10 +329,9 @@ Admin::RequestPtr Admin::makeStaticTextRequest(Buffer::Instance& response, Http: return std::make_unique(response, code); } -Http::Code AdminImpl::runCallback(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, +Http::Code AdminImpl::runCallback(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream& admin_stream) { - RequestPtr request = makeRequest(path_and_query, admin_stream); + RequestPtr request = makeRequest(admin_stream); Http::Code code = request->start(response_headers); bool more_data; do { @@ -346,8 +341,8 @@ Http::Code AdminImpl::runCallback(absl::string_view path_and_query, return code; } -Admin::RequestPtr AdminImpl::makeRequest(absl::string_view path_and_query, - AdminStream& admin_stream) const { +Admin::RequestPtr AdminImpl::makeRequest(AdminStream& admin_stream) const { + absl::string_view path_and_query = admin_stream.getRequestHeaders().getPathValue(); std::string::size_type query_index = path_and_query.find('?'); if (query_index == std::string::npos) { query_index = path_and_query.size(); @@ -367,7 +362,7 @@ Admin::RequestPtr AdminImpl::makeRequest(absl::string_view path_and_query, } ASSERT(admin_stream.getRequestHeaders().getPathValue() == path_and_query); - return handler.handler_(path_and_query, admin_stream); + return handler.handler_(admin_stream); } } @@ -390,8 +385,8 @@ std::vector AdminImpl::sortedHandlers() const { return sorted_handlers; } -Http::Code AdminImpl::handlerHelp(absl::string_view, Http::ResponseHeaderMap&, - Buffer::Instance& response, AdminStream&) { +Http::Code AdminImpl::handlerHelp(Http::ResponseHeaderMap&, Buffer::Instance& response, + AdminStream&) { getHelp(response); return Http::Code::OK; } @@ -477,7 +472,7 @@ Http::Code AdminImpl::request(absl::string_view path_and_query, absl::string_vie filter.decodeHeaders(*request_headers, false); Buffer::OwnedImpl response; - Http::Code code = runCallback(path_and_query, response_headers, response, filter); + Http::Code code = runCallback(response_headers, response, filter); Utility::populateFallbackResponseHeaders(code, response_headers); body = response.toString(); return code; diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h index 829ef3c94bdc2..ce561f1aae677 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -71,8 +71,7 @@ class AdminImpl : public Admin, AdminImpl(const std::string& profile_path, Server::Instance& server, bool ignore_global_conn_limit); - Http::Code runCallback(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, + Http::Code runCallback(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream& admin_stream); const Network::Socket& socket() override { return *socket_; } Network::Socket& mutableSocket() { return *socket_; } @@ -204,9 +203,7 @@ class AdminImpl : public Admin, void addListenerToHandler(Network::ConnectionHandler* handler) override; GenRequestFn createRequestFunction() const { - return [this](absl::string_view path_and_query, AdminStream& admin_stream) -> RequestPtr { - return makeRequest(path_and_query, admin_stream); - }; + return [this](AdminStream& admin_stream) -> RequestPtr { return makeRequest(admin_stream); }; } uint64_t maxRequestsPerConnection() const override { return 0; } const HttpConnectionManagerProto::ProxyStatusConfig* proxyStatusConfig() const override { @@ -221,9 +218,9 @@ class AdminImpl : public Admin, friend class AdminTestingPeer; /** - * Creates a Request from a url. + * Creates a Request from the request in the admin stream. */ - RequestPtr makeRequest(absl::string_view path_and_query, AdminStream& admin_stream) const; + RequestPtr makeRequest(AdminStream& admin_stream) const; /** * Creates a UrlHandler structure from a non-chunked callback. @@ -234,7 +231,7 @@ class AdminImpl : public Admin, /** * Creates a URL prefix bound to chunked handler. Handler is expected to - * supply a method makeRequest(absl::string_view, AdminStream&). + * supply a method makeRequest(AdminStream&). * * @param prefix the prefix to register * @param help_text a help text ot display in a table in the admin home page @@ -248,8 +245,8 @@ class AdminImpl : public Admin, UrlHandler makeStreamingHandler(const std::string& prefix, const std::string& help_text, Handler& handler, bool removable, bool mutates_state) { return {prefix, help_text, - [&handler](absl::string_view path, AdminStream& admin_stream) -> Admin::RequestPtr { - return handler.makeRequest(path, admin_stream); + [&handler](AdminStream& admin_stream) -> Admin::RequestPtr { + return handler.makeRequest(admin_stream); }, removable, mutates_state}; } @@ -341,12 +338,10 @@ class AdminImpl : public Admin, /** * URL handlers. */ - Http::Code handlerAdminHome(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, + Http::Code handlerAdminHome(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&); - Http::Code handlerHelp(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, + Http::Code handlerHelp(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&); void getHelp(Buffer::Instance& response) const; diff --git a/source/server/admin/admin_filter.cc b/source/server/admin/admin_filter.cc index e1341366b0df1..054c886e06bbd 100644 --- a/source/server/admin/admin_filter.cc +++ b/source/server/admin/admin_filter.cc @@ -86,7 +86,7 @@ void AdminFilter::onComplete() { auto header_map = Http::ResponseHeaderMapImpl::create(); RELEASE_ASSERT(request_headers_, ""); - Admin::RequestPtr handler = admin_handler_fn_(path, *this); + Admin::RequestPtr handler = admin_handler_fn_(*this); Http::Code code = handler->start(*header_map); Utility::populateFallbackResponseHeaders(code, *header_map); decoder_callbacks_->encodeHeaders(std::move(header_map), false, diff --git a/source/server/admin/admin_html.cc b/source/server/admin/admin_html.cc index 05eb41e6cb52d..6860fafd24703 100644 --- a/source/server/admin/admin_html.cc +++ b/source/server/admin/admin_html.cc @@ -5,7 +5,7 @@ namespace Envoy { namespace Server { -Http::Code AdminImpl::handlerAdminHome(absl::string_view, Http::ResponseHeaderMap& response_headers, +Http::Code AdminImpl::handlerAdminHome(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&) { StatsHtmlRender html(response_headers, response, StatsParams()); html.tableBegin(response); diff --git a/source/server/admin/admin_no_html.cc b/source/server/admin/admin_no_html.cc index 9335cdfce4701..111ec556b2f82 100644 --- a/source/server/admin/admin_no_html.cc +++ b/source/server/admin/admin_no_html.cc @@ -3,7 +3,7 @@ namespace Envoy { namespace Server { -Http::Code AdminImpl::handlerAdminHome(absl::string_view, Http::ResponseHeaderMap& response_headers, +Http::Code AdminImpl::handlerAdminHome(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&) { response_headers.setReferenceContentType(Http::Headers::get().ContentTypeValues.Text); response.add("HTML output was disabled by building with --define=admin_html=disabled"); diff --git a/source/server/admin/clusters_handler.cc b/source/server/admin/clusters_handler.cc index 12dc2f6209bd2..9641038d178aa 100644 --- a/source/server/admin/clusters_handler.cc +++ b/source/server/admin/clusters_handler.cc @@ -42,8 +42,7 @@ void addCircuitBreakerSettingsAsJson(const envoy::config::core::v3::RoutingPrior ClustersHandler::ClustersHandler(Server::Instance& server) : HandlerContextBase(server) {} -Http::Code ClustersHandler::handlerClusters(absl::string_view, - Http::ResponseHeaderMap& response_headers, +Http::Code ClustersHandler::handlerClusters(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream& admin_stream) { const auto format_value = Utility::formatParam(admin_stream.queryParams()); diff --git a/source/server/admin/clusters_handler.h b/source/server/admin/clusters_handler.h index 7786755cbeb6e..c03623edb36b6 100644 --- a/source/server/admin/clusters_handler.h +++ b/source/server/admin/clusters_handler.h @@ -18,8 +18,7 @@ class ClustersHandler : public HandlerContextBase { public: ClustersHandler(Server::Instance& server); - Http::Code handlerClusters(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, + Http::Code handlerClusters(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&); private: diff --git a/source/server/admin/config_dump_handler.cc b/source/server/admin/config_dump_handler.cc index f456057e49b6b..e88099ff49b52 100644 --- a/source/server/admin/config_dump_handler.cc +++ b/source/server/admin/config_dump_handler.cc @@ -157,8 +157,7 @@ buildNameMatcher(const Http::Utility::QueryParams& params) { ConfigDumpHandler::ConfigDumpHandler(ConfigTracker& config_tracker, Server::Instance& server) : HandlerContextBase(server), config_tracker_(config_tracker) {} -Http::Code ConfigDumpHandler::handlerConfigDump(absl::string_view, - Http::ResponseHeaderMap& response_headers, +Http::Code ConfigDumpHandler::handlerConfigDump(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream& admin_stream) const { Http::Utility::QueryParams query_params = admin_stream.queryParams(); diff --git a/source/server/admin/config_dump_handler.h b/source/server/admin/config_dump_handler.h index d64801eaa8211..6d0bbdc46315b 100644 --- a/source/server/admin/config_dump_handler.h +++ b/source/server/admin/config_dump_handler.h @@ -22,8 +22,7 @@ class ConfigDumpHandler : public HandlerContextBase { public: ConfigDumpHandler(ConfigTracker& config_tracker, Server::Instance& server); - Http::Code handlerConfigDump(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, + Http::Code handlerConfigDump(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&) const; private: diff --git a/source/server/admin/init_dump_handler.cc b/source/server/admin/init_dump_handler.cc index 55c91115d3dcc..f8d57b7b155d3 100644 --- a/source/server/admin/init_dump_handler.cc +++ b/source/server/admin/init_dump_handler.cc @@ -18,8 +18,7 @@ absl::optional maskParam(const Http::Utility::QueryParams& params) InitDumpHandler::InitDumpHandler(Server::Instance& server) : HandlerContextBase(server) {} -Http::Code InitDumpHandler::handlerInitDump(absl::string_view, - Http::ResponseHeaderMap& response_headers, +Http::Code InitDumpHandler::handlerInitDump(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream& admin_stream) const { const auto mask = maskParam(admin_stream.queryParams()); diff --git a/source/server/admin/init_dump_handler.h b/source/server/admin/init_dump_handler.h index 26156c4e91222..f3651e934b0ba 100644 --- a/source/server/admin/init_dump_handler.h +++ b/source/server/admin/init_dump_handler.h @@ -19,8 +19,7 @@ class InitDumpHandler : public HandlerContextBase { public: InitDumpHandler(Server::Instance& server); - Http::Code handlerInitDump(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, + Http::Code handlerInitDump(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&) const; private: diff --git a/source/server/admin/listeners_handler.cc b/source/server/admin/listeners_handler.cc index c3adafcafcab5..490cefacc0cd1 100644 --- a/source/server/admin/listeners_handler.cc +++ b/source/server/admin/listeners_handler.cc @@ -12,7 +12,7 @@ namespace Server { ListenersHandler::ListenersHandler(Server::Instance& server) : HandlerContextBase(server) {} -Http::Code ListenersHandler::handlerDrainListeners(absl::string_view, Http::ResponseHeaderMap&, +Http::Code ListenersHandler::handlerDrainListeners(Http::ResponseHeaderMap&, Buffer::Instance& response, AdminStream& admin_query) { const Http::Utility::QueryParams params = admin_query.queryParams(); @@ -38,8 +38,7 @@ Http::Code ListenersHandler::handlerDrainListeners(absl::string_view, Http::Resp return Http::Code::OK; } -Http::Code ListenersHandler::handlerListenerInfo(absl::string_view, - Http::ResponseHeaderMap& response_headers, +Http::Code ListenersHandler::handlerListenerInfo(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream& admin_query) { const Http::Utility::QueryParams query_params = admin_query.queryParams(); diff --git a/source/server/admin/listeners_handler.h b/source/server/admin/listeners_handler.h index 52d52ea0298b9..4b072cc8996bb 100644 --- a/source/server/admin/listeners_handler.h +++ b/source/server/admin/listeners_handler.h @@ -18,12 +18,10 @@ class ListenersHandler : public HandlerContextBase { public: ListenersHandler(Server::Instance& server); - Http::Code handlerDrainListeners(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, + Http::Code handlerDrainListeners(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&); - Http::Code handlerListenerInfo(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, + Http::Code handlerListenerInfo(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&); private: diff --git a/source/server/admin/logs_handler.cc b/source/server/admin/logs_handler.cc index d8afe2c6acf6f..41eadd5d10506 100644 --- a/source/server/admin/logs_handler.cc +++ b/source/server/admin/logs_handler.cc @@ -37,8 +37,8 @@ std::vector LogsHandler::levelStrings() { return strings; } -Http::Code LogsHandler::handlerLogging(absl::string_view, Http::ResponseHeaderMap&, - Buffer::Instance& response, AdminStream& admin_stream) { +Http::Code LogsHandler::handlerLogging(Http::ResponseHeaderMap&, Buffer::Instance& response, + AdminStream& admin_stream) { Http::Utility::QueryParams query_params = admin_stream.queryParams(); Http::Code rc = Http::Code::OK; @@ -75,8 +75,8 @@ Http::Code LogsHandler::handlerLogging(absl::string_view, Http::ResponseHeaderMa return rc; } -Http::Code LogsHandler::handlerReopenLogs(absl::string_view, Http::ResponseHeaderMap&, - Buffer::Instance& response, AdminStream&) { +Http::Code LogsHandler::handlerReopenLogs(Http::ResponseHeaderMap&, Buffer::Instance& response, + AdminStream&) { server_.accessLogManager().reopen(); response.add("OK\n"); return Http::Code::OK; diff --git a/source/server/admin/logs_handler.h b/source/server/admin/logs_handler.h index c25bef7ca058c..94a384d2cf0b3 100644 --- a/source/server/admin/logs_handler.h +++ b/source/server/admin/logs_handler.h @@ -22,12 +22,10 @@ class LogsHandler : public HandlerContextBase, Logger::Loggablereset(); } @@ -32,7 +32,7 @@ Http::Code StatsHandler::handlerResetCounters(absl::string_view, Http::ResponseH return Http::Code::OK; } -Http::Code StatsHandler::handlerStatsRecentLookups(absl::string_view, Http::ResponseHeaderMap&, +Http::Code StatsHandler::handlerStatsRecentLookups(Http::ResponseHeaderMap&, Buffer::Instance& response, AdminStream&) { Stats::SymbolTable& symbol_table = server_.stats().symbolTable(); std::string table; @@ -49,15 +49,14 @@ Http::Code StatsHandler::handlerStatsRecentLookups(absl::string_view, Http::Resp return Http::Code::OK; } -Http::Code StatsHandler::handlerStatsRecentLookupsClear(absl::string_view, Http::ResponseHeaderMap&, +Http::Code StatsHandler::handlerStatsRecentLookupsClear(Http::ResponseHeaderMap&, Buffer::Instance& response, AdminStream&) { server_.stats().symbolTable().clearRecentLookups(); response.add("OK\n"); return Http::Code::OK; } -Http::Code StatsHandler::handlerStatsRecentLookupsDisable(absl::string_view, - Http::ResponseHeaderMap&, +Http::Code StatsHandler::handlerStatsRecentLookupsDisable(Http::ResponseHeaderMap&, Buffer::Instance& response, AdminStream&) { server_.stats().symbolTable().setRecentLookupCapacity(0); @@ -65,18 +64,17 @@ Http::Code StatsHandler::handlerStatsRecentLookupsDisable(absl::string_view, return Http::Code::OK; } -Http::Code StatsHandler::handlerStatsRecentLookupsEnable(absl::string_view, - Http::ResponseHeaderMap&, +Http::Code StatsHandler::handlerStatsRecentLookupsEnable(Http::ResponseHeaderMap&, Buffer::Instance& response, AdminStream&) { server_.stats().symbolTable().setRecentLookupCapacity(RecentLookupsCapacity); response.add("OK\n"); return Http::Code::OK; } -Admin::RequestPtr StatsHandler::makeRequest(absl::string_view path, AdminStream& /*admin_stream*/) { +Admin::RequestPtr StatsHandler::makeRequest(AdminStream& admin_stream) { StatsParams params; Buffer::OwnedImpl response; - Http::Code code = params.parse(path, response); + Http::Code code = params.parse(admin_stream.getRequestHeaders().getPathValue(), response); if (code != Http::Code::OK) { return Admin::makeStaticTextRequest(response, code); } @@ -107,10 +105,10 @@ Admin::RequestPtr StatsHandler::makeRequest(Stats::Store& stats, const StatsPara return std::make_unique(stats, params, url_handler_fn); } -Http::Code StatsHandler::handlerPrometheusStats(absl::string_view path_and_query, - Http::ResponseHeaderMap&, - Buffer::Instance& response, AdminStream&) { - return prometheusStats(path_and_query, response); +Http::Code StatsHandler::handlerPrometheusStats(Http::ResponseHeaderMap&, + Buffer::Instance& response, + AdminStream& admin_stream) { + return prometheusStats(admin_stream.getRequestHeaders().getPathValue(), response); } Http::Code StatsHandler::prometheusStats(absl::string_view path_and_query, @@ -147,8 +145,7 @@ void StatsHandler::prometheusRender(Stats::Store& stats, custom_namespaces); } -Http::Code StatsHandler::handlerContention(absl::string_view, - Http::ResponseHeaderMap& response_headers, +Http::Code StatsHandler::handlerContention(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&) { if (server_.options().mutexTracingEnabled() && server_.mutexTracer() != nullptr) { @@ -170,9 +167,7 @@ Admin::UrlHandler StatsHandler::statsHandler() { return { "/stats", "print server stats", - [this](absl::string_view path, AdminStream& admin_stream) -> Admin::RequestPtr { - return makeRequest(path, admin_stream); - }, + [this](AdminStream& admin_stream) -> Admin::RequestPtr { return makeRequest(admin_stream); }, false, false, {{Admin::ParamDescriptor::Type::Boolean, "usedonly", diff --git a/source/server/admin/stats_handler.h b/source/server/admin/stats_handler.h index 29b7a08fd7a02..a938fd3fe8e62 100644 --- a/source/server/admin/stats_handler.h +++ b/source/server/admin/stats_handler.h @@ -23,23 +23,17 @@ class StatsHandler : public HandlerContextBase { public: StatsHandler(Server::Instance& server); - Http::Code handlerResetCounters(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, + Http::Code handlerResetCounters(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&); - Http::Code handlerStatsRecentLookups(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, + Http::Code handlerStatsRecentLookups(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&); - Http::Code handlerStatsRecentLookupsClear(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, + Http::Code handlerStatsRecentLookupsClear(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&); - Http::Code handlerStatsRecentLookupsDisable(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, + Http::Code handlerStatsRecentLookupsDisable(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&); - Http::Code handlerStatsRecentLookupsEnable(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, + Http::Code handlerStatsRecentLookupsEnable(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&); - Http::Code handlerPrometheusStats(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, + Http::Code handlerPrometheusStats(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&); /** @@ -75,8 +69,7 @@ class StatsHandler : public HandlerContextBase { const Stats::CustomStatNamespaces& custom_namespaces, const StatsParams& params, Buffer::Instance& response); - Http::Code handlerContention(absl::string_view path_and_query, - Http::ResponseHeaderMap& response_headers, + Http::Code handlerContention(Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, AdminStream&); /** @@ -92,7 +85,7 @@ class StatsHandler : public HandlerContextBase { static Admin::RequestPtr makeRequest(Stats::Store& stats, const StatsParams& params, StatsRequest::UrlHandlerFn url_handler_fn = nullptr); - Admin::RequestPtr makeRequest(absl::string_view path, AdminStream&); + Admin::RequestPtr makeRequest(AdminStream&); // static Admin::RequestPtr makeRequest(Stats::Store& stats, const StatsParams& params, // StatsRequest::UrlHandlerFn url_handler_fn); diff --git a/test/extensions/common/tap/admin_test.cc b/test/extensions/common/tap/admin_test.cc index 12610102208e3..2a4ca3fe56fc6 100644 --- a/test/extensions/common/tap/admin_test.cc +++ b/test/extensions/common/tap/admin_test.cc @@ -86,19 +86,25 @@ class BaseAdminHandlerTest : public testing::Test { return handler_->attached_request_; } + Http::Code makeRequest(absl::string_view path) { + Http::TestResponseHeaderMapImpl response_headers; + request_headers_.setPath(path); + EXPECT_CALL(admin_stream_, getRequestHeaders()).WillRepeatedly(ReturnRef(request_headers_)); + return cb_(response_headers, response_, admin_stream_); + } + protected: Server::MockAdmin admin_; + Http::TestRequestHeaderMapImpl request_headers_; + Server::StrictMockAdminStream admin_stream_; std::unique_ptr handler_; Server::Admin::HandlerCb cb_; MockDispatcherQueued main_thread_dispatcher_{"test_main_thread"}; + Buffer::OwnedImpl response_; }; class AdminHandlerTest : public BaseAdminHandlerTest { public: - Http::TestResponseHeaderMapImpl response_headers_; - Buffer::OwnedImpl response_; - Server::MockAdminStream admin_stream_; - const std::string streaming_admin_request_yaml_ = R"EOF( config_id: test_config_id @@ -135,9 +141,6 @@ config_id: test_config_id return fmt::format(buffered_admin_request_yaml_, max_traces, timeout_s); } - Http::TestResponseHeaderMapImpl response_headers_; - Buffer::OwnedImpl response_; - Server::StrictMockAdminStream admin_stream_; // Cannot be moved into individual test cases as expected calls are validated on object // destruction, and the code that satisfies the expected calls on sink_ is in the TearDown method. StrictMock sink_; @@ -156,7 +159,7 @@ TEST_F(AdminHandlerTest, AdminWithPipeSocket) { TEST_F(AdminHandlerTest, NoBody) { setup(); EXPECT_CALL(admin_stream_, getRequestBody()); - EXPECT_EQ(Http::Code::BadRequest, cb_("/tap", response_headers_, response_, admin_stream_)); + EXPECT_EQ(Http::Code::BadRequest, makeRequest("/tap")); EXPECT_EQ("/tap requires a JSON/YAML body", response_.toString()); } @@ -165,7 +168,7 @@ TEST_F(AdminHandlerTest, BadBody) { setup(); Buffer::OwnedImpl bad_body("hello"); EXPECT_CALL(admin_stream_, getRequestBody()).WillRepeatedly(Return(&bad_body)); - EXPECT_EQ(Http::Code::BadRequest, cb_("/tap", response_headers_, response_, admin_stream_)); + EXPECT_EQ(Http::Code::BadRequest, makeRequest("/tap")); EXPECT_EQ("Unable to convert YAML as JSON: hello", response_.toString()); } @@ -174,7 +177,7 @@ TEST_F(AdminHandlerTest, UnknownConfigId) { setup(); Buffer::OwnedImpl body(streaming_admin_request_yaml_); EXPECT_CALL(admin_stream_, getRequestBody()).WillRepeatedly(Return(&body)); - EXPECT_EQ(Http::Code::BadRequest, cb_("/tap", response_headers_, response_, admin_stream_)); + EXPECT_EQ(Http::Code::BadRequest, makeRequest("/tap")); EXPECT_EQ("Unknown config id 'test_config_id'. No extension has registered with this id.", response_.toString()); } @@ -190,9 +193,9 @@ TEST_F(AdminHandlerTest, RequestTapWhileAttached) { EXPECT_CALL(extension_config, newTapConfig(_, handler_.get())); EXPECT_CALL(admin_stream_, setEndStreamOnComplete(false)); EXPECT_CALL(admin_stream_, addOnDestroyCallback(_)); - EXPECT_EQ(Http::Code::OK, cb_("/tap", response_headers_, response_, admin_stream_)); + EXPECT_EQ(Http::Code::OK, makeRequest("/tap")); - EXPECT_EQ(Http::Code::BadRequest, cb_("/tap", response_headers_, response_, admin_stream_)); + EXPECT_EQ(Http::Code::BadRequest, makeRequest("/tap")); EXPECT_EQ("An attached /tap admin stream already exists. Detach it.", response_.toString()); } @@ -209,7 +212,7 @@ TEST_F(AdminHandlerTest, CloseMidStream) { EXPECT_CALL(extension_config, newTapConfig(_, handler_.get())); EXPECT_CALL(admin_stream_, setEndStreamOnComplete(false)); EXPECT_CALL(admin_stream_, addOnDestroyCallback(_)); - EXPECT_EQ(Http::Code::OK, cb_("/tap", response_headers_, response_, admin_stream_)); + EXPECT_EQ(Http::Code::OK, makeRequest("/tap")); // Direct access to the handle is required so we can submit traces directly PerTapSinkHandlePtr sinkHandle = @@ -239,7 +242,7 @@ TEST_F(BufferedAdminHandlerTest, BufferedTapWrites) { EXPECT_CALL(admin_stream_, addOnDestroyCallback(_)); EXPECT_CALL(admin_stream_, getDecoderFilterCallbacks()).Times(AtLeast(traces)); EXPECT_CALL(sink_, encodeData(_, _)).Times(Between(traces, traces + 1)); - EXPECT_EQ(Http::Code::OK, cb_("/tap", response_headers_, response_, admin_stream_)); + EXPECT_EQ(Http::Code::OK, makeRequest("/tap")); // Direct access to the handle is required so we can submit traces directly PerTapSinkHandlePtr sinkHandle = @@ -278,8 +281,7 @@ TEST_F(BufferedAdminHandlerTest, BufferedTapRace) { EXPECT_CALL(admin_stream_, setEndStreamOnComplete(false)); EXPECT_CALL(admin_stream_, addOnDestroyCallback(_)); EXPECT_CALL(main_thread_dispatcher_, post(_)).Times(2); - EXPECT_EQ(Http::Code::OK, - cb_("/tap", response_headers_, response_, admin_stream_)); // AdminHandler::handler + EXPECT_EQ(Http::Code::OK, makeRequest("/tap")); // Direct access to the handle is required so we can submit traces directly PerTapSinkHandlePtr sink_handle = @@ -323,8 +325,7 @@ TEST_F(BufferedAdminHandlerTest, BufferedTapTimeoutRace) { EXPECT_CALL(main_thread_dispatcher_, post(_)).Times(3); EXPECT_CALL(admin_stream_, getDecoderFilterCallbacks()).Times(AtLeast(1)); EXPECT_CALL(sink_, encodeData(_, _)).Times(Between(1, 2)); - EXPECT_EQ(Http::Code::OK, - cb_("/tap", response_headers_, response_, admin_stream_)); // AdminHandler::handler + EXPECT_EQ(Http::Code::OK, makeRequest("/tap")); // Direct access to the handle is required so we can submit traces directly PerTapSinkHandlePtr sink_handle = @@ -366,8 +367,7 @@ TEST_F(BufferedAdminHandlerTest, BufferedTapDoubleFlush) { EXPECT_CALL(admin_stream_, getDecoderFilterCallbacks()).Times(AtLeast(1)); EXPECT_CALL(main_thread_dispatcher_, post(_)).Times(2); EXPECT_CALL(sink_, encodeData(_, _)).Times(Between(1, 2)); - EXPECT_EQ(Http::Code::OK, - cb_("/tap", response_headers_, response_, admin_stream_)); // AdminHandler::handler + EXPECT_EQ(Http::Code::OK, makeRequest("/tap")); // Direct access to the handle is required so we can submit traces directly PerTapSinkHandlePtr sink_handle = diff --git a/test/extensions/stats_sinks/hystrix/hystrix_test.cc b/test/extensions/stats_sinks/hystrix/hystrix_test.cc index 98299a662dc01..50683f5d26171 100644 --- a/test/extensions/stats_sinks/hystrix/hystrix_test.cc +++ b/test/extensions/stats_sinks/hystrix/hystrix_test.cc @@ -507,7 +507,6 @@ TEST_F(HystrixSinkTest, HystrixEventStreamHandler) { sink_->registerConnection(&callbacks_); // This value doesn't matter in handlerHystrixEventStream - absl::string_view path_and_query; Http::TestResponseHeaderMapImpl response_headers; @@ -525,9 +524,9 @@ TEST_F(HystrixSinkTest, HystrixEventStreamHandler) { addr_instance_); EXPECT_CALL(stream_encoder_options, disableChunkEncoding()); - ASSERT_EQ(sink_->handlerHystrixEventStream(path_and_query, response_headers, - cluster_stats_buffer_, admin_stream_mock), - Http::Code::OK); + ASSERT_EQ( + sink_->handlerHystrixEventStream(response_headers, cluster_stats_buffer_, admin_stream_mock), + Http::Code::OK); // Check that response_headers has been set correctly EXPECT_EQ(response_headers.ContentType()->value(), "text/event-stream"); diff --git a/test/integration/integration_admin_test.cc b/test/integration/integration_admin_test.cc index 6bf73b5dbce66..fda542ebbb137 100644 --- a/test/integration/integration_admin_test.cc +++ b/test/integration/integration_admin_test.cc @@ -487,7 +487,7 @@ TEST_P(IntegrationAdminTest, AdminOnDestroyCallbacks) { bool test = true; // add an handler which adds a callback to the list of callback called when connection is dropped. - auto callback = [&test](absl::string_view, Http::HeaderMap&, Buffer::Instance&, + auto callback = [&test](Http::HeaderMap&, Buffer::Instance&, Server::AdminStream& admin_stream) -> Http::Code { auto on_destroy_callback = [&test]() { test = false; }; diff --git a/test/server/admin/admin_filter_test.cc b/test/server/admin/admin_filter_test.cc index a780c5b92a415..9627cfd1ca31e 100644 --- a/test/server/admin/admin_filter_test.cc +++ b/test/server/admin/admin_filter_test.cc @@ -25,10 +25,8 @@ class AdminFilterTest : public testing::TestWithParam callbacks_; Http::TestRequestHeaderMapImpl request_headers_; - static Admin::RequestPtr adminHandlerCallback(absl::string_view path_and_query, - AdminStream& admin_stream) { + static Admin::RequestPtr adminHandlerCallback(AdminStream& admin_stream) { // silence compiler warnings for unused params - UNREFERENCED_PARAMETER(path_and_query); UNREFERENCED_PARAMETER(admin_stream); return AdminImpl::makeStaticTextRequest("OK\n", Http::Code::OK); } diff --git a/test/server/admin/admin_html_test.cc b/test/server/admin/admin_html_test.cc index feae8252e6afd..b29e060db9acb 100644 --- a/test/server/admin/admin_html_test.cc +++ b/test/server/admin/admin_html_test.cc @@ -13,8 +13,9 @@ INSTANTIATE_TEST_SUITE_P(IpVersions, AdminInstanceTest, TestUtility::ipTestParamsToString); TEST_P(AdminInstanceTest, EscapeHelpTextWithPunctuation) { - auto callback = [](absl::string_view, Http::HeaderMap&, Buffer::Instance&, - AdminStream&) -> Http::Code { return Http::Code::Accepted; }; + auto callback = [](Http::HeaderMap&, Buffer::Instance&, AdminStream&) -> Http::Code { + return Http::Code::Accepted; + }; // It's OK to have help text with HTML characters in it, but when we render the home // page they need to be escaped. diff --git a/test/server/admin/admin_instance.cc b/test/server/admin/admin_instance.cc index 478430758bee5..ad4c9862a2b58 100644 --- a/test/server/admin/admin_instance.cc +++ b/test/server/admin/admin_instance.cc @@ -38,7 +38,7 @@ Http::Code AdminInstanceTest::runCallback(absl::string_view path_and_query, request_headers_.setPath(path_and_query); admin_filter_.decodeHeaders(request_headers_, false); - return admin_.runCallback(path_and_query, response_headers, response, admin_filter_); + return admin_.runCallback(response_headers, response, admin_filter_); } Http::Code AdminInstanceTest::getCallback(absl::string_view path_and_query, diff --git a/test/server/admin/admin_test.cc b/test/server/admin/admin_test.cc index 19da5f6257e4a..fb78d0a6272d1 100644 --- a/test/server/admin/admin_test.cc +++ b/test/server/admin/admin_test.cc @@ -100,8 +100,9 @@ TEST_P(AdminInstanceTest, AdminBadAddressOutPath) { } TEST_P(AdminInstanceTest, CustomHandler) { - auto callback = [](absl::string_view, Http::HeaderMap&, Buffer::Instance&, - AdminStream&) -> Http::Code { return Http::Code::Accepted; }; + auto callback = [](Http::HeaderMap&, Buffer::Instance&, AdminStream&) -> Http::Code { + return Http::Code::Accepted; + }; // Test removable handler. EXPECT_NO_LOGS(EXPECT_TRUE(admin_.addHandler("/foo/bar", "hello", callback, true, false))); @@ -199,7 +200,7 @@ class ChunkedHandler : public Admin::Request { }; TEST_P(AdminInstanceTest, CustomChunkedHandler) { - auto callback = [](absl::string_view, AdminStream&) -> Admin::RequestPtr { + auto callback = [](AdminStream&) -> Admin::RequestPtr { Admin::RequestPtr handler = Admin::RequestPtr(new ChunkedHandler); return handler; }; @@ -267,8 +268,9 @@ TEST_P(AdminInstanceTest, StatsWithMultipleChunks) { } TEST_P(AdminInstanceTest, RejectHandlerWithXss) { - auto callback = [](absl::string_view, Http::HeaderMap&, Buffer::Instance&, - AdminStream&) -> Http::Code { return Http::Code::Accepted; }; + auto callback = [](Http::HeaderMap&, Buffer::Instance&, AdminStream&) -> Http::Code { + return Http::Code::Accepted; + }; EXPECT_LOG_CONTAINS("error", "filter \"/foo\" contains invalid character '<'", EXPECT_FALSE(admin_.addHandler("/foo", "hello", @@ -276,8 +278,9 @@ TEST_P(AdminInstanceTest, RejectHandlerWithXss) { } TEST_P(AdminInstanceTest, RejectHandlerWithEmbeddedQuery) { - auto callback = [](absl::string_view, Http::HeaderMap&, Buffer::Instance&, - AdminStream&) -> Http::Code { return Http::Code::Accepted; }; + auto callback = [](Http::HeaderMap&, Buffer::Instance&, AdminStream&) -> Http::Code { + return Http::Code::Accepted; + }; EXPECT_LOG_CONTAINS("error", "filter \"/bar?queryShouldNotBeInPrefix\" contains invalid character '?'", EXPECT_FALSE(admin_.addHandler("/bar?queryShouldNotBeInPrefix", "hello", diff --git a/test/server/admin/profiling_handler_test.cc b/test/server/admin/profiling_handler_test.cc index e4417c54748d6..315764a207ffa 100644 --- a/test/server/admin/profiling_handler_test.cc +++ b/test/server/admin/profiling_handler_test.cc @@ -69,13 +69,11 @@ TEST_P(AdminInstanceTest, AdminBadProfiler) { AdminImpl admin_bad_profile_path(TestEnvironment::temporaryPath("some/unlikely/bad/path.prof"), server_, false); Http::TestResponseHeaderMapImpl header_map; - constexpr absl::string_view url = "/cpuprofiler?enable=y"; request_headers_.setMethod(Http::Headers::get().MethodValues.Post); - request_headers_.setPath(url); + request_headers_.setPath("/cpuprofiler?enable=y"); admin_filter_.decodeHeaders(request_headers_, false); - EXPECT_NO_LOGS( - EXPECT_EQ(Http::Code::InternalServerError, - admin_bad_profile_path.runCallback(url, header_map, data, admin_filter_))); + EXPECT_NO_LOGS(EXPECT_EQ(Http::Code::InternalServerError, + admin_bad_profile_path.runCallback(header_map, data, admin_filter_))); EXPECT_FALSE(Profiler::Cpu::profilerEnabled()); } diff --git a/test/server/admin/stats_handler_test.cc b/test/server/admin/stats_handler_test.cc index 274844724ffa4..b5bbb66e69ed5 100644 --- a/test/server/admin/stats_handler_test.cc +++ b/test/server/admin/stats_handler_test.cc @@ -82,13 +82,15 @@ class StatsHandlerTest { */ CodeResponse handlerStats(absl::string_view url) { MockInstance instance; + EXPECT_CALL(admin_stream_, getRequestHeaders()).WillRepeatedly(ReturnRef(request_headers_)); EXPECT_CALL(instance, statsConfig()).WillRepeatedly(ReturnRef(stats_config_)); EXPECT_CALL(stats_config_, flushOnAdmin()).WillRepeatedly(Return(false)); EXPECT_CALL(instance, stats()).WillRepeatedly(ReturnRef(*store_)); EXPECT_CALL(instance, api()).WillRepeatedly(ReturnRef(api_)); EXPECT_CALL(api_, customStatNamespaces()).WillRepeatedly(ReturnRef(custom_namespaces_)); StatsHandler handler(instance); - Admin::RequestPtr request = handler.makeRequest(url, admin_stream_); + request_headers_.setPath(url); + Admin::RequestPtr request = handler.makeRequest(admin_stream_); Http::TestResponseHeaderMapImpl response_headers; Http::Code code = request->start(response_headers); Buffer::OwnedImpl data; @@ -131,6 +133,7 @@ class StatsHandlerTest { Stats::MockSink sink_; Stats::ThreadLocalStoreImplPtr store_; Stats::CustomStatNamespacesImpl custom_namespaces_; + Http::TestRequestHeaderMapImpl request_headers_; MockAdminStream admin_stream_; Configuration::MockStatsConfig stats_config_; TestScopedRuntime scoped_runtime_; diff --git a/test/server/admin/stats_html_render_test.cc b/test/server/admin/stats_html_render_test.cc index 50e0b85c5f563..4718ad0b0206c 100644 --- a/test/server/admin/stats_html_render_test.cc +++ b/test/server/admin/stats_html_render_test.cc @@ -14,7 +14,7 @@ namespace Server { class StatsHtmlRenderTest : public StatsRenderTestBase { protected: - static Admin::RequestPtr handlerCallback(absl::string_view, AdminStream&) { + static Admin::RequestPtr handlerCallback(AdminStream&) { return Admin::makeStaticTextRequest("", Http::Code::OK); }