From 2804d6c3a8d7e4529e53a79c352f231bfb42cf77 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Thu, 21 Dec 2017 18:04:25 -0500 Subject: [PATCH 1/6] Add an HTML home page for the admin port, which makes it easier to surf to the other handlers with a browser. Add a private mechanism to have admin handlers supply arbitrary headers. Note that the handler callback doesn't expose a header structure, so the public v2 API does not provide this header-control. However, it was specifying text/html as content-type by default, so I changed the default to text/plain (with nosniff). Added sanitization of both the prefix string and the help text, which need to be safely injected into the HTML doc. Added some styling and a reference to the existing Envoy favicon as well. Note that no existing handlers get any response bytes changed as a result of this commit, just their content-type. Signed-off-by: Joshua Marantz --- source/common/html/BUILD | 15 ++ source/common/html/utility.cc | 25 ++++ source/common/html/utility.h | 21 +++ source/common/http/headers.h | 2 + source/server/http/BUILD | 1 + source/server/http/admin.cc | 160 ++++++++++++++++----- source/server/http/admin.h | 24 +++- test/common/html/BUILD | 17 +++ test/common/html/utility_test.cc | 15 ++ test/integration/integration_admin_test.cc | 19 ++- 10 files changed, 261 insertions(+), 38 deletions(-) create mode 100644 source/common/html/BUILD create mode 100644 source/common/html/utility.cc create mode 100644 source/common/html/utility.h create mode 100644 test/common/html/BUILD create mode 100644 test/common/html/utility_test.cc diff --git a/source/common/html/BUILD b/source/common/html/BUILD new file mode 100644 index 000000000000..e7cdb2ad7c3d --- /dev/null +++ b/source/common/html/BUILD @@ -0,0 +1,15 @@ +licenses(["notice"]) # Apache 2 + +load( + "//third_party/envoy/src/bazel:envoy_build_system.bzl", + "envoy_cc_library", + "envoy_package", +) + +envoy_package() + +envoy_cc_library( + name = "utility_lib", + srcs = ["utility.cc"], + hdrs = ["utility.h"], +) diff --git a/source/common/html/utility.cc b/source/common/html/utility.cc new file mode 100644 index 000000000000..0ece5b1bdbff --- /dev/null +++ b/source/common/html/utility.cc @@ -0,0 +1,25 @@ +#include "common/html/utility.h" + +#include + +namespace Envoy { +namespace Html { + +/* static */ +std::string Utility::sanitize(const std::string& text) { + std::string buffer; + for (char ch : text) { + switch (ch) { + case '&': buffer.append("&"); break; + case '\"': buffer.append("""); break; + case '\'': buffer.append("'"); break; + case '<': buffer.append("<"); break; + case '>': buffer.append(">"); break; + default: buffer.append(1, ch); break; + } + } + return buffer; +} + +} // namespace Html +} // namespace Envoy diff --git a/source/common/html/utility.h b/source/common/html/utility.h new file mode 100644 index 000000000000..2fb0fae443b8 --- /dev/null +++ b/source/common/html/utility.h @@ -0,0 +1,21 @@ +#pragma once + +#include + +namespace Envoy { +namespace Html { + +/** + * General HTML utilities. + */ +class Utility { + public: + /** + * Sanitizes arbitrary text so it can be included in HTML. + * @param text arbitrary text to be escaped for safe inclusion in HTML. + */ + static std::string sanitize(const std::string& text); +}; + +} // namespace Html +} // namespace Envoy diff --git a/source/common/http/headers.h b/source/common/http/headers.h index e427803a6323..745dc5631a30 100644 --- a/source/common/http/headers.h +++ b/source/common/http/headers.h @@ -24,6 +24,7 @@ class HeaderValues { const LowerCaseString AccessControlMaxAge{"access-control-max-age"}; const LowerCaseString AccessControlAllowCredentials{"access-control-allow-credentials"}; const LowerCaseString Authorization{"authorization"}; + const LowerCaseString CacheControl{"cache-control"}; const LowerCaseString ClientTraceId{"x-client-trace-id"}; const LowerCaseString Connection{"connection"}; const LowerCaseString ContentLength{"content-length"}; @@ -82,6 +83,7 @@ class HeaderValues { const LowerCaseString XB3ParentSpanId{"x-b3-parentspanid"}; const LowerCaseString XB3Sampled{"x-b3-sampled"}; const LowerCaseString XB3Flags{"x-b3-flags"}; + const LowerCaseString XContentTypeOptions{"x-content-type-options"}; struct { const std::string Close{"close"}; diff --git a/source/server/http/BUILD b/source/server/http/BUILD index b902a5c168fa..f6a188b1794e 100644 --- a/source/server/http/BUILD +++ b/source/server/http/BUILD @@ -33,6 +33,7 @@ envoy_cc_library( "//source/common/common:macros", "//source/common/common:utility_lib", "//source/common/common:version_includes", + "//source/common/html:utility_lib", "//source/common/http:codes_lib", "//source/common/http:conn_manager_lib", "//source/common/http:date_provider_lib", diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index d8adf77ee144..34ea49ba757d 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1,5 +1,4 @@ #include "server/http/admin.h" - #include #include #include @@ -20,6 +19,7 @@ #include "common/common/enum_to_int.h" #include "common/common/utility.h" #include "common/common/version.h" +#include "common/html/utility.h" #include "common/http/codes.h" #include "common/http/header_map_impl.h" #include "common/http/headers.h" @@ -417,11 +417,13 @@ void AdminFilter::onComplete() { ENVOY_STREAM_LOG(debug, "request complete: path: {}", *callbacks_, path); Buffer::OwnedImpl response; - Http::Code code = parent_.runCallback(path, response); - - Http::HeaderMapPtr headers{ - new Http::HeaderMapImpl{{Http::Headers::get().Status, std::to_string(enumToInt(code))}}}; - callbacks_->encodeHeaders(std::move(headers), response.length() == 0); + Http::HeaderMapPtr header_map{new Http::HeaderMapImpl}; + Http::Code code = parent_.runCallback(path, *header_map, response); + const auto& headers = Http::Headers::get(); + header_map->addReferenceKey(headers.Status, std::to_string(enumToInt(code))); + header_map->addReferenceKey(headers.CacheControl, "no-cache, max-age=0"); + header_map->addReferenceKey(headers.XContentTypeOptions, "nosniff"); + callbacks_->encodeHeaders(std::move(header_map), response.length() == 0); if (response.length() > 0) { callbacks_->encodeData(response, true); @@ -440,29 +442,34 @@ AdminImpl::AdminImpl(const std::string& access_log_path, const std::string& prof stats_(Http::ConnectionManagerImpl::generateStats("http.admin.", server_.stats())), tracing_stats_(Http::ConnectionManagerImpl::generateTracingStats("http.admin.tracing.", server_.stats())), - handlers_{ - {"/certs", "print certs on machine", MAKE_ADMIN_HANDLER(handlerCerts), false}, - {"/clusters", "upstream cluster status", MAKE_ADMIN_HANDLER(handlerClusters), false}, - {"/cpuprofiler", "enable/disable the CPU profiler", - MAKE_ADMIN_HANDLER(handlerCpuProfiler), false}, - {"/healthcheck/fail", "cause the server to fail health checks", - MAKE_ADMIN_HANDLER(handlerHealthcheckFail), false}, - {"/healthcheck/ok", "cause the server to pass health checks", - MAKE_ADMIN_HANDLER(handlerHealthcheckOk), false}, - {"/hot_restart_version", "print the hot restart compatability version", - MAKE_ADMIN_HANDLER(handlerHotRestartVersion), false}, - {"/logging", "query/change logging levels", MAKE_ADMIN_HANDLER(handlerLogging), false}, - {"/quitquitquit", "exit the server", MAKE_ADMIN_HANDLER(handlerQuitQuitQuit), false}, - {"/reset_counters", "reset all counters to zero", - MAKE_ADMIN_HANDLER(handlerResetCounters), false}, - {"/server_info", "print server version/status information", - MAKE_ADMIN_HANDLER(handlerServerInfo), false}, - {"/stats", "print server stats", MAKE_ADMIN_HANDLER(handlerStats), false}, - {"/listeners", "print listener addresses", MAKE_ADMIN_HANDLER(handlerListenerInfo), - false}}, listener_stats_( Http::ConnectionManagerImpl::generateListenerStats("http.admin.", listener_scope)) { + auto home_handler = [this](const std::string& path_and_query, Http::HeaderMap& header_map, + Buffer::Instance& response) -> Http::Code { + return handlerAdminHome(path_and_query, header_map, response); + }; + addTypedHandler("/", "Admin home page", home_handler, false); + addHandler("/certs", "print certs on machine", MAKE_ADMIN_HANDLER(handlerCerts), false); + addHandler("/clusters", "upstream cluster status", MAKE_ADMIN_HANDLER(handlerClusters), false); + addHandler("/cpuprofiler", "enable/disable the CPU profiler", + MAKE_ADMIN_HANDLER(handlerCpuProfiler), false); + addHandler("/healthcheck/fail", "cause the server to fail health checks", + MAKE_ADMIN_HANDLER(handlerHealthcheckFail), false); + addHandler("/healthcheck/ok", "cause the server to pass health checks", + MAKE_ADMIN_HANDLER(handlerHealthcheckOk), false); + addHandler("/hot_restart_version", "print the hot restart compatibility version", + MAKE_ADMIN_HANDLER(handlerHotRestartVersion), false); + addHandler("/logging", "query/change logging levels", MAKE_ADMIN_HANDLER(handlerLogging), false); + addHandler("/quitquitquit", "exit the server", MAKE_ADMIN_HANDLER(handlerQuitQuitQuit), false); + addHandler("/reset_counters", "reset all counters to zero", + MAKE_ADMIN_HANDLER(handlerResetCounters), false); + addHandler("/server_info", "print server version/status information", + MAKE_ADMIN_HANDLER(handlerServerInfo), false); + addHandler("/stats", "print server stats", MAKE_ADMIN_HANDLER(handlerStats), false); + addHandler("/listeners", "print listener addresses", + MAKE_ADMIN_HANDLER(handlerListenerInfo), false);; + if (!address_out_path.empty()) { std::ofstream address_out_file(address_out_path); if (!address_out_file) { @@ -496,20 +503,27 @@ void AdminImpl::createFilterChain(Http::FilterChainFactoryCallbacks& callbacks) callbacks.addStreamDecoderFilter(Http::StreamDecoderFilterSharedPtr{new AdminFilter(*this)}); } -Http::Code AdminImpl::runCallback(const std::string& path, Buffer::Instance& response) { +Http::Code AdminImpl::runCallback(const std::string& path_and_query, Http::HeaderMap& header_map, + Buffer::Instance& response) { Http::Code code = Http::Code::OK; bool found_handler = false; + + std::string::size_type query_index = path_and_query.find('?'); + if (query_index == std::string::npos) { + query_index = path_and_query.size(); + } + for (const UrlHandler& handler : handlers_) { - if (path.find(handler.prefix_) == 0) { - code = handler.handler_(path, response); + if (path_and_query.compare(0, query_index, handler.prefix_) == 0) { + code = handler.handler_(path_and_query, header_map, response); found_handler = true; break; } } if (!found_handler) { - code = Http::Code::NotFound; - response.add("envoy admin commands:\n"); + header_map.addReferenceKey(Http::Headers::get().ContentType, "text/plain; charset=UTF-8"); + response.add("invalid path. admin commands are:\n"); // Prefix order is used during searching, but for printing do them in alpha order. std::map sorted_handlers; @@ -520,17 +534,86 @@ Http::Code AdminImpl::runCallback(const std::string& path, Buffer::Instance& res for (auto handler : sorted_handlers) { response.add(fmt::format(" {}: {}\n", handler.first, handler.second->help_text_)); } + code = Http::Code::NotFound; } return code; } +Http::Code AdminImpl::handlerAdminHome(const std::string&, Http::HeaderMap& header_map, + Buffer::Instance& response) { + header_map.addReferenceKey(Http::Headers::get().ContentType, "text/html; charset=UTF-8"); + + // Prefix order is used during searching, but for printing do them in alpha order. + std::map sorted_handlers; + for (const UrlHandler& handler : handlers_) { + sorted_handlers[handler.prefix_] = &handler; + } + + response.add(R"( + + + + + + + + + + + +)"); + + for (auto handler : sorted_handlers) { + // Handlers are all specified by statically above, and are thus trusted and do + // not require escaping. + response.add(fmt::format("" + "\n", + handler.first, handler.first, + Html::Utility::sanitize(handler.second->help_text_))); + } + response.add(R"( + +
CommandDescription
{}{}
+ +)"); + return Http::Code::OK; +} + const Network::Address::Instance& AdminImpl::localAddress() { return *server_.localInfo().address(); } -bool AdminImpl::addHandler(const std::string& prefix, const std::string& help_text, - HandlerCb callback, bool removable) { +bool AdminImpl::addTypedHandler(const std::string& prefix, const std::string& help_text, + TypedHandlerCb callback, bool removable) { + // Sanitize prefix and help_text to ensure no XSS can be injected, as + // we are injecting these strings into HTML that runs in a domain that + // can mutate Envoy server state. Also rule out some characters that + // make no sense as part of a URL path: ? and :. + // + // TODO(jmarantz): replace with absl definitive absl implementation + string::size_type pos = prefix.find_first_of("&\"'<>?:"); + if (pos != std::string::npos) { + ENVOY_LOG(error, "filter \"{}\" contains invalid character '{}'" , prefix[pos]); + return false; + } + auto it = std::find_if(handlers_.cbegin(), handlers_.cend(), [&prefix](const UrlHandler& entry) { return prefix == entry.prefix_; }); if (it == handlers_.end()) { @@ -540,6 +623,17 @@ bool AdminImpl::addHandler(const std::string& prefix, const std::string& help_te return false; } +bool AdminImpl::addHandler(const std::string& prefix, const std::string& help_text, + HandlerCb callback, bool removable) { + auto typed_handler = [callback](const std::string& path_and_query, Http::HeaderMap& header_map, + Buffer::Instance& response) { + Http::Code code = callback(path_and_query, response); + header_map.addReferenceKey(Http::Headers::get().ContentType, "text/plain; charset=UTF-8"); + return code; + }; + return addTypedHandler(prefix, help_text, typed_handler, removable); +} + bool AdminImpl::removeHandler(const std::string& prefix) { const uint size_before_removal = handlers_.size(); handlers_.remove_if( diff --git a/source/server/http/admin.h b/source/server/http/admin.h index f489d404ca05..782480409390 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -35,7 +35,8 @@ class AdminImpl : public Admin, const std::string& address_out_path, Network::Address::InstanceConstSharedPtr address, Server::Instance& server, Stats::Scope& listener_scope); - Http::Code runCallback(const std::string& path, Buffer::Instance& response); + Http::Code runCallback(const std::string& path_and_query, Http::HeaderMap& headers, + Buffer::Instance& response); const Network::ListenSocket& socket() override { return *socket_; } Network::ListenSocket& mutable_socket() { return *socket_; } @@ -79,16 +80,28 @@ class AdminImpl : public Admin, Http::ConnectionManagerListenerStats& listenerStats() override { return listener_stats_; } private: + typedef std::function TypedHandlerCb; + /** * Individual admin handler including prefix, help text, and callback. */ struct UrlHandler { const std::string prefix_; const std::string help_text_; - const HandlerCb handler_; + const TypedHandlerCb handler_; const bool removable_; }; + /** + * More general purpose handler than what's declared in admin.cc, which allows for + * specifying cache-control and content-type via headers. Perhaps this should be + * promoted to src/include/envoy/server/admin.h in the future so filter-supplied admin + * handlers could produce something other than text. + */ + bool addTypedHandler(const std::string& prefix, const std::string& help_text, + TypedHandlerCb callback, bool removable); + /** * Implementation of RouteConfigProvider that returns a static null route config. */ @@ -124,18 +137,21 @@ class AdminImpl : public Admin, /** * URL handlers. */ + Http::Code handlerAdminHome(const std::string& url, Http::HeaderMap& headers, + Buffer::Instance& response); Http::Code handlerCerts(const std::string& url, Buffer::Instance& response); Http::Code handlerClusters(const std::string& url, Buffer::Instance& response); Http::Code handlerCpuProfiler(const std::string& url, Buffer::Instance& response); Http::Code handlerHealthcheckFail(const std::string& url, Buffer::Instance& response); Http::Code handlerHealthcheckOk(const std::string& url, Buffer::Instance& response); Http::Code handlerHotRestartVersion(const std::string& url, Buffer::Instance& response); + Http::Code handlerListenerInfo(const std::string& url, Buffer::Instance& response); Http::Code handlerLogging(const std::string& url, Buffer::Instance& response); + Http::Code handlerMain(const std::string& path, Buffer::Instance& response); + Http::Code handlerQuitQuitQuit(const std::string& url, Buffer::Instance& response); Http::Code handlerResetCounters(const std::string& url, Buffer::Instance& response); Http::Code handlerServerInfo(const std::string& url, Buffer::Instance& response); Http::Code handlerStats(const std::string& url, Buffer::Instance& response); - Http::Code handlerQuitQuitQuit(const std::string& url, Buffer::Instance& response); - Http::Code handlerListenerInfo(const std::string& url, Buffer::Instance& response); Server::Instance& server_; std::list access_logs_; diff --git a/test/common/html/BUILD b/test/common/html/BUILD new file mode 100644 index 000000000000..4405b0724685 --- /dev/null +++ b/test/common/html/BUILD @@ -0,0 +1,17 @@ +licenses(["notice"]) # Apache 2 + +load( + "//third_party/envoy/src/bazel:envoy_build_system.bzl", + "envoy_cc_test", + "envoy_package", +) + +envoy_package() + +envoy_cc_test( + name = "utility_test", + srcs = ["utility_test.cc"], + deps = [ + "//third_party/envoy/src/source/common/html:utility_lib", + ], +) diff --git a/test/common/html/utility_test.cc b/test/common/html/utility_test.cc new file mode 100644 index 000000000000..0e87eb6e760c --- /dev/null +++ b/test/common/html/utility_test.cc @@ -0,0 +1,15 @@ +#include "common/html/utility.h" + +#include "testing/base/public/gunit.h" + +namespace Envoy { +namespace Html { + +TEST(HttpUtility, sanitizeHtml) { + EXPECT_EQ("simple text, no cares/worries", Utility::sanitize("simple text, no cares/worries")); + EXPECT_EQ("a&b", Utility::sanitize("a&b")); + EXPECT_EQ("<script>", Utility::sanitize("", "hello", callback, true)); +} + +TEST_P(AdminInstanceTest, RejectHandlerWithEmbeddedQuery) { + auto callback = [&](const std::string&, Buffer::Instance&) -> Http::Code { + return Http::Code::Accepted; + }; + EXPECT_FALSE(admin_.addHandler("/bar?queryShouldNotBeInPrefix", "hello", callback, true)); +} + +TEST_P(AdminInstanceTest, EscapeHelpTextWithPunctuation) { + auto callback = [&](const std::string&, Buffer::Instance&) -> 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. + const std::string kPlanets = "jupiter>saturn>mars"; + EXPECT_TRUE(admin_.addHandler("/planets", kPlanets, callback, true)); + + Http::HeaderMapImpl header_map; + Buffer::OwnedImpl response; + EXPECT_EQ(Http::Code::OK, admin_.runCallback("/", header_map, response)); + Http::HeaderString& content_type = header_map.ContentType()->value(); + EXPECT_TRUE(content_type.find("text/html")) << content_type.c_str(); + EXPECT_EQ(-1, response.search(kPlanets.data(), kPlanets.size(), 0)); + const std::string kEscapedPlanets = "jupiter>saturn>mars"; + EXPECT_NE(-1, response.search(kEscapedPlanets.data(), kEscapedPlanets.size(), 0)); } } // namespace Server From 29bc7133d02b66534cefcf9eb62c71302d00412a Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Thu, 21 Dec 2017 22:47:41 -0500 Subject: [PATCH 3/6] Make the admin HandlerCb pass through a headers reference so handlers can set the content-type and/or cache-control. Signed-off-by: Joshua Marantz --- include/envoy/http/header_map.h | 1 + include/envoy/server/admin.h | 10 ++- source/common/router/rds_impl.cc | 4 +- source/common/router/rds_impl.h | 3 +- source/server/http/admin.cc | 91 ++++++++++++++-------- source/server/http/admin.h | 50 ++++++------ test/common/router/rds_impl_test.cc | 20 +++-- test/integration/integration_admin_test.cc | 36 ++++++--- test/server/http/admin_test.cc | 8 +- 9 files changed, 136 insertions(+), 87 deletions(-) diff --git a/include/envoy/http/header_map.h b/include/envoy/http/header_map.h index 80714cd8b9f1..ac7c4d3c3573 100644 --- a/include/envoy/http/header_map.h +++ b/include/envoy/http/header_map.h @@ -235,6 +235,7 @@ class HeaderEntry { HEADER_FUNC(AccessControlExposeHeaders) \ HEADER_FUNC(AccessControlMaxAge) \ HEADER_FUNC(Authorization) \ + HEADER_FUNC(CacheControl) \ HEADER_FUNC(ClientTraceId) \ HEADER_FUNC(Connection) \ HEADER_FUNC(ContentLength) \ diff --git a/include/envoy/server/admin.h b/include/envoy/server/admin.h index 4a5eb4fb8850..ceeeff38f068 100644 --- a/include/envoy/server/admin.h +++ b/include/envoy/server/admin.h @@ -6,6 +6,7 @@ #include "envoy/buffer/buffer.h" #include "envoy/common/pure.h" #include "envoy/http/codes.h" +#include "envoy/http/header_map.h" #include "envoy/network/listen_socket.h" namespace Envoy { @@ -17,8 +18,10 @@ namespace Server { * used to add static handlers as in source/server/http/admin.cc and also dynamic handlers as * done in the RouteConfigProviderManagerImpl constructor in source/common/router/rds_impl.cc. */ -#define MAKE_ADMIN_HANDLER(X) \ - [this](const std::string& url, Buffer::Instance& data) -> Http::Code { return X(url, data); } +#define MAKE_ADMIN_HANDLER(X) \ + [this](const std::string& url, Http::HeaderMap& header, Buffer::Instance& data) -> Http::Code { \ + return X(url, header, data); \ + } /** * Global admin HTTP endpoint for the server. @@ -33,7 +36,8 @@ class Admin { * @param response supplies the buffer to fill in with the response body. * @return Http::Code the response code. */ - typedef std::function HandlerCb; + typedef std::function HandlerCb; /** * Add an admin handler. diff --git a/source/common/router/rds_impl.cc b/source/common/router/rds_impl.cc index c4511542e784..6d14f5b5f998 100644 --- a/source/common/router/rds_impl.cc +++ b/source/common/router/rds_impl.cc @@ -205,8 +205,8 @@ Router::RouteConfigProviderSharedPtr RouteConfigProviderManagerImpl::getRouteCon return new_provider; }; -Http::Code RouteConfigProviderManagerImpl::handlerRoutes(const std::string& url, - Buffer::Instance& response) { +Http::Code RouteConfigProviderManagerImpl::handlerRoutes( + const std::string& url, Http::HeaderMap&, Buffer::Instance& response) { Http::Utility::QueryParams query_params = Http::Utility::parseQueryString(url); // If there are no query params, print out all the configured route tables. if (query_params.size() == 0) { diff --git a/source/common/router/rds_impl.h b/source/common/router/rds_impl.h index a02937ec46ff..12f2635dd326 100644 --- a/source/common/router/rds_impl.h +++ b/source/common/router/rds_impl.h @@ -170,7 +170,8 @@ class RouteConfigProviderManagerImpl : public ServerRouteConfigProviderManager, * @param response supplies the buffer to fill with information. * @return Http::Code OK if the endpoint can parse and operate on the url, NotFound otherwise. */ - Http::Code handlerRoutes(const std::string& url, Buffer::Instance& response); + Http::Code handlerRoutes(const std::string& url, Http::HeaderMap& headers, + Buffer::Instance& response); /** * Helper function used by handlerRoutes. The function loops through the providers diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index dfed1b85a8a3..4caf8133d015 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -142,7 +142,8 @@ void AdminImpl::addCircuitSettings(const std::string& cluster_name, const std::s resource_manager.retries().max())); } -Http::Code AdminImpl::handlerClusters(const std::string&, Buffer::Instance& response) { +Http::Code AdminImpl::handlerClusters(const std::string&, Http::HeaderMap&, + Buffer::Instance& response) { response.add(fmt::format("version_info::{}\n", server_.clusterManager().versionInfo())); for (auto& cluster : server_.clusterManager().clusters()) { @@ -198,7 +199,8 @@ Http::Code AdminImpl::handlerClusters(const std::string&, Buffer::Instance& resp return Http::Code::OK; } -Http::Code AdminImpl::handlerCpuProfiler(const std::string& url, Buffer::Instance& response) { +Http::Code AdminImpl::handlerCpuProfiler(const std::string& url, Http::HeaderMap&, + Buffer::Instance& response) { Http::Utility::QueryParams query_params = Http::Utility::parseQueryString(url); if (query_params.size() != 1 || query_params.begin()->first != "enable" || (query_params.begin()->second != "y" && query_params.begin()->second != "n")) { @@ -221,24 +223,28 @@ Http::Code AdminImpl::handlerCpuProfiler(const std::string& url, Buffer::Instanc return Http::Code::OK; } -Http::Code AdminImpl::handlerHealthcheckFail(const std::string&, Buffer::Instance& response) { +Http::Code AdminImpl::handlerHealthcheckFail(const std::string&, Http::HeaderMap&, + Buffer::Instance& response) { server_.failHealthcheck(true); response.add("OK\n"); return Http::Code::OK; } -Http::Code AdminImpl::handlerHealthcheckOk(const std::string&, Buffer::Instance& response) { +Http::Code AdminImpl::handlerHealthcheckOk(const std::string&, Http::HeaderMap&, + Buffer::Instance& response) { server_.failHealthcheck(false); response.add("OK\n"); return Http::Code::OK; } -Http::Code AdminImpl::handlerHotRestartVersion(const std::string&, Buffer::Instance& response) { +Http::Code AdminImpl::handlerHotRestartVersion(const std::string&, Http::HeaderMap&, + Buffer::Instance& response) { response.add(server_.hotRestart().version()); return Http::Code::OK; } -Http::Code AdminImpl::handlerLogging(const std::string& url, Buffer::Instance& response) { +Http::Code AdminImpl::handlerLogging(const std::string& url, Http::HeaderMap&, + Buffer::Instance& response) { Http::Utility::QueryParams query_params = Http::Utility::parseQueryString(url); Http::Code rc = Http::Code::OK; @@ -263,7 +269,8 @@ Http::Code AdminImpl::handlerLogging(const std::string& url, Buffer::Instance& r return rc; } -Http::Code AdminImpl::handlerResetCounters(const std::string&, Buffer::Instance& response) { +Http::Code AdminImpl::handlerResetCounters(const std::string&, Http::HeaderMap&, + Buffer::Instance& response) { for (const Stats::CounterSharedPtr& counter : server_.stats().counters()) { counter->reset(); } @@ -272,7 +279,8 @@ Http::Code AdminImpl::handlerResetCounters(const std::string&, Buffer::Instance& return Http::Code::OK; } -Http::Code AdminImpl::handlerServerInfo(const std::string&, Buffer::Instance& response) { +Http::Code AdminImpl::handlerServerInfo(const std::string&, Http::HeaderMap&, + Buffer::Instance& response) { time_t current_time = time(nullptr); response.add(fmt::format("envoy {} {} {} {} {}\n", VersionInfo::version(), server_.healthCheckFailed() ? "draining" : "live", @@ -282,7 +290,8 @@ Http::Code AdminImpl::handlerServerInfo(const std::string&, Buffer::Instance& re return Http::Code::OK; } -Http::Code AdminImpl::handlerStats(const std::string& url, Buffer::Instance& response) { +Http::Code AdminImpl::handlerStats(const std::string& url, Http::HeaderMap&, + Buffer::Instance& response) { // We currently don't support timers locally (only via statsd) so just group all the counters // and gauges together, alpha sort them, and spit them out. Http::Code rc = Http::Code::OK; @@ -379,13 +388,15 @@ std::string AdminImpl::statsAsJson(const std::map& all_st return strbuf.GetString(); } -Http::Code AdminImpl::handlerQuitQuitQuit(const std::string&, Buffer::Instance& response) { +Http::Code AdminImpl::handlerQuitQuitQuit(const std::string&, Http::HeaderMap&, + Buffer::Instance& response) { server_.shutdown(); response.add("OK\n"); return Http::Code::OK; } -Http::Code AdminImpl::handlerListenerInfo(const std::string&, Buffer::Instance& response) { +Http::Code AdminImpl::handlerListenerInfo(const std::string&, Http::HeaderMap&, + Buffer::Instance& response) { std::list listeners; for (auto listener : server_.listenerManager().listeners()) { listeners.push_back(listener.get().socket().localAddress()->asString()); @@ -394,7 +405,8 @@ Http::Code AdminImpl::handlerListenerInfo(const std::string&, Buffer::Instance& return Http::Code::OK; } -Http::Code AdminImpl::handlerCerts(const std::string&, Buffer::Instance& response) { +Http::Code AdminImpl::handlerCerts(const std::string&, Http::HeaderMap&, + Buffer::Instance& response) { // This set is used to track distinct certificates. We may have multiple listeners, upstreams, etc // using the same cert. std::unordered_set context_info_set; @@ -421,7 +433,15 @@ void AdminFilter::onComplete() { Http::Code code = parent_.runCallback(path, *header_map, response); const auto& headers = Http::Headers::get(); header_map->addReferenceKey(headers.Status, std::to_string(enumToInt(code))); - header_map->addReferenceKey(headers.CacheControl, "no-cache, max-age=0"); + if (header_map->ContentType() == nullptr) { + // Default to text-plain if unset. + header_map->addReferenceKey(headers.ContentType, "text/plain; charset=UTF-8"); + } + // Default to 'no-cache' if unset, but not 'no-store' which may break the 'back button. + if (header_map->CacheControl() == nullptr) { + header_map->addReferenceKey(headers.CacheControl, "no-cache, max-age=0"); + } + // Under no circumstance should browsers sniff content-type. header_map->addReferenceKey(headers.XContentTypeOptions, "nosniff"); callbacks_->encodeHeaders(std::move(header_map), response.length() == 0); @@ -442,14 +462,31 @@ AdminImpl::AdminImpl(const std::string& access_log_path, const std::string& prof stats_(Http::ConnectionManagerImpl::generateStats("http.admin.", server_.stats())), tracing_stats_(Http::ConnectionManagerImpl::generateTracingStats("http.admin.tracing.", server_.stats())), + handlers_{ + {"/", "Admin home page", MAKE_ADMIN_HANDLER(handlerAdminHome), false}, + {"/certs", "print certs on machine", MAKE_ADMIN_HANDLER(handlerCerts), false}, + {"/clusters", "upstream cluster status", MAKE_ADMIN_HANDLER(handlerClusters), false}, + {"/cpuprofiler", "enable/disable the CPU profiler", + MAKE_ADMIN_HANDLER(handlerCpuProfiler), false}, + {"/healthcheck/fail", "cause the server to fail health checks", + MAKE_ADMIN_HANDLER(handlerHealthcheckFail), false}, + {"/healthcheck/ok", "cause the server to pass health checks", + MAKE_ADMIN_HANDLER(handlerHealthcheckOk), false}, + {"/hot_restart_version", "print the hot restart compatability version", + MAKE_ADMIN_HANDLER(handlerHotRestartVersion), false}, + {"/logging", "query/change logging levels", MAKE_ADMIN_HANDLER(handlerLogging), false}, + {"/quitquitquit", "exit the server", MAKE_ADMIN_HANDLER(handlerQuitQuitQuit), false}, + {"/reset_counters", "reset all counters to zero", + MAKE_ADMIN_HANDLER(handlerResetCounters), false}, + {"/server_info", "print server version/status information", + MAKE_ADMIN_HANDLER(handlerServerInfo), false}, + {"/stats", "print server stats", MAKE_ADMIN_HANDLER(handlerStats), false}, + {"/listeners", "print listener addresses", MAKE_ADMIN_HANDLER(handlerListenerInfo), + false}}, listener_stats_( Http::ConnectionManagerImpl::generateListenerStats("http.admin.", listener_scope)) { - - auto home_handler = [this](const std::string& path_and_query, Http::HeaderMap& header_map, - Buffer::Instance& response) -> Http::Code { - return handlerAdminHome(path_and_query, header_map, response); - }; - addTypedHandler("/", "Admin home page", home_handler, false); + /* + addHandler("/", "Admin home page", MAKE_ADMIN_HANDLER(handlerAdminHome), false); addHandler("/certs", "print certs on machine", MAKE_ADMIN_HANDLER(handlerCerts), false); addHandler("/clusters", "upstream cluster status", MAKE_ADMIN_HANDLER(handlerClusters), false); addHandler("/cpuprofiler", "enable/disable the CPU profiler", @@ -469,6 +506,7 @@ AdminImpl::AdminImpl(const std::string& access_log_path, const std::string& prof addHandler("/stats", "print server stats", MAKE_ADMIN_HANDLER(handlerStats), false); addHandler("/listeners", "print listener addresses", MAKE_ADMIN_HANDLER(handlerListenerInfo), false);; + */ if (!address_out_path.empty()) { std::ofstream address_out_file(address_out_path); @@ -600,8 +638,8 @@ const Network::Address::Instance& AdminImpl::localAddress() { return *server_.localInfo().address(); } -bool AdminImpl::addTypedHandler(const std::string& prefix, const std::string& help_text, - TypedHandlerCb callback, bool removable) { +bool AdminImpl::addHandler(const std::string& prefix, const std::string& help_text, + HandlerCb callback, bool removable) { // Sanitize prefix and help_text to ensure no XSS can be injected, as // we are injecting these strings into HTML that runs in a domain that // can mutate Envoy server state. Also rule out some characters that @@ -623,17 +661,6 @@ bool AdminImpl::addTypedHandler(const std::string& prefix, const std::string& he return false; } -bool AdminImpl::addHandler(const std::string& prefix, const std::string& help_text, - HandlerCb callback, bool removable) { - auto typed_handler = [callback](const std::string& path_and_query, Http::HeaderMap& header_map, - Buffer::Instance& response) { - Http::Code code = callback(path_and_query, response); - header_map.addReferenceKey(Http::Headers::get().ContentType, "text/plain; charset=UTF-8"); - return code; - }; - return addTypedHandler(prefix, help_text, typed_handler, removable); -} - bool AdminImpl::removeHandler(const std::string& prefix) { const uint size_before_removal = handlers_.size(); handlers_.remove_if( diff --git a/source/server/http/admin.h b/source/server/http/admin.h index 782480409390..ec5df8140e67 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -80,28 +80,16 @@ class AdminImpl : public Admin, Http::ConnectionManagerListenerStats& listenerStats() override { return listener_stats_; } private: - typedef std::function TypedHandlerCb; - /** * Individual admin handler including prefix, help text, and callback. */ struct UrlHandler { const std::string prefix_; const std::string help_text_; - const TypedHandlerCb handler_; + const HandlerCb handler_; const bool removable_; }; - /** - * More general purpose handler than what's declared in admin.cc, which allows for - * specifying cache-control and content-type via headers. Perhaps this should be - * promoted to src/include/envoy/server/admin.h in the future so filter-supplied admin - * handlers could produce something other than text. - */ - bool addTypedHandler(const std::string& prefix, const std::string& help_text, - TypedHandlerCb callback, bool removable); - /** * Implementation of RouteConfigProvider that returns a static null route config. */ @@ -139,19 +127,31 @@ class AdminImpl : public Admin, */ Http::Code handlerAdminHome(const std::string& url, Http::HeaderMap& headers, Buffer::Instance& response); - Http::Code handlerCerts(const std::string& url, Buffer::Instance& response); - Http::Code handlerClusters(const std::string& url, Buffer::Instance& response); - Http::Code handlerCpuProfiler(const std::string& url, Buffer::Instance& response); - Http::Code handlerHealthcheckFail(const std::string& url, Buffer::Instance& response); - Http::Code handlerHealthcheckOk(const std::string& url, Buffer::Instance& response); - Http::Code handlerHotRestartVersion(const std::string& url, Buffer::Instance& response); - Http::Code handlerListenerInfo(const std::string& url, Buffer::Instance& response); - Http::Code handlerLogging(const std::string& url, Buffer::Instance& response); + Http::Code handlerCerts(const std::string& url, Http::HeaderMap& headers, + Buffer::Instance& response); + Http::Code handlerClusters(const std::string& url, Http::HeaderMap& headers, + Buffer::Instance& response); + Http::Code handlerCpuProfiler(const std::string& url, Http::HeaderMap& headers, + Buffer::Instance& response); + Http::Code handlerHealthcheckFail(const std::string& url, Http::HeaderMap& headers, + Buffer::Instance& response); + Http::Code handlerHealthcheckOk(const std::string& url, Http::HeaderMap& headers, + Buffer::Instance& response); + Http::Code handlerHotRestartVersion(const std::string& url, Http::HeaderMap& headers, + Buffer::Instance& response); + Http::Code handlerListenerInfo(const std::string& url, Http::HeaderMap& headers, + Buffer::Instance& response); + Http::Code handlerLogging(const std::string& url, Http::HeaderMap& headers, + Buffer::Instance& response); Http::Code handlerMain(const std::string& path, Buffer::Instance& response); - Http::Code handlerQuitQuitQuit(const std::string& url, Buffer::Instance& response); - Http::Code handlerResetCounters(const std::string& url, Buffer::Instance& response); - Http::Code handlerServerInfo(const std::string& url, Buffer::Instance& response); - Http::Code handlerStats(const std::string& url, Buffer::Instance& response); + Http::Code handlerQuitQuitQuit(const std::string& url, Http::HeaderMap& headers, + Buffer::Instance& response); + Http::Code handlerResetCounters(const std::string& url, Http::HeaderMap& headers, + Buffer::Instance& response); + Http::Code handlerServerInfo(const std::string& url, Http::HeaderMap& headers, + Buffer::Instance& response); + Http::Code handlerStats(const std::string& url, Http::HeaderMap& headers, + Buffer::Instance& response); Server::Instance& server_; std::list access_logs_; diff --git a/test/common/router/rds_impl_test.cc b/test/common/router/rds_impl_test.cc index 3d46c368bf21..55c68108bd4d 100644 --- a/test/common/router/rds_impl_test.cc +++ b/test/common/router/rds_impl_test.cc @@ -212,7 +212,8 @@ TEST_F(RdsImplTest, Basic) { ] )EOF"; EXPECT_EQ("", rds_->versionInfo()); - EXPECT_EQ(Http::Code::OK, handler_callback_("/routes", data)); + Http::HeaderMapImpl header_map; + EXPECT_EQ(Http::Code::OK, handler_callback_("/routes", header_map, data)); EXPECT_EQ(routes_expected_output_no_routes, TestUtility::bufferToString(data)); data.drain(data.length()); EXPECT_EQ(0UL, store_.gauge("foo.rds.foo_route_config.version").value()); @@ -245,7 +246,7 @@ TEST_F(RdsImplTest, Basic) { )EOF"; EXPECT_EQ("hash_15ed54077da94d8b", rds_->versionInfo()); - EXPECT_EQ(Http::Code::OK, handler_callback_("/routes", data)); + EXPECT_EQ(Http::Code::OK, handler_callback_("/routes", header_map, data)); EXPECT_EQ(routes_expected_output_only_name, TestUtility::bufferToString(data)); data.drain(data.length()); EXPECT_EQ(1580011435426663819U, store_.gauge("foo.rds.foo_route_config.version").value()); @@ -263,7 +264,7 @@ TEST_F(RdsImplTest, Basic) { EXPECT_EQ(nullptr, rds_->config()->route(Http::TestHeaderMapImpl{{":authority", "foo"}}, 0)); // Test Admin /routes handler. The route table should not change. - EXPECT_EQ(Http::Code::OK, handler_callback_("/routes", data)); + EXPECT_EQ(Http::Code::OK, handler_callback_("/routes", header_map, data)); EXPECT_EQ(routes_expected_output_only_name, TestUtility::bufferToString(data)); data.drain(data.length()); EXPECT_EQ(1580011435426663819U, store_.gauge("foo.rds.foo_route_config.version").value()); @@ -323,18 +324,20 @@ TEST_F(RdsImplTest, Basic) { ] )EOF"; - EXPECT_EQ(Http::Code::OK, handler_callback_("/routes", data)); + EXPECT_EQ(Http::Code::OK, handler_callback_("/routes", header_map, data)); EXPECT_EQ(routes_expected_output_full_table, TestUtility::bufferToString(data)); data.drain(data.length()); EXPECT_EQ(8808926191882896258U, store_.gauge("foo.rds.foo_route_config.version").value()); // Test that we get the same dump if we specify the route name. - EXPECT_EQ(Http::Code::OK, handler_callback_("/routes?route_config_name=foo_route_config", data)); + EXPECT_EQ(Http::Code::OK, handler_callback_("/routes?route_config_name=foo_route_config", + header_map, data)); EXPECT_EQ(routes_expected_output_full_table, TestUtility::bufferToString(data)); data.drain(data.length()); // Test that we get an emtpy response if the name does not match. - EXPECT_EQ(Http::Code::OK, handler_callback_("/routes?route_config_name=does_not_exist", data)); + EXPECT_EQ(Http::Code::OK, handler_callback_("/routes?route_config_name=does_not_exist", + header_map, data)); EXPECT_EQ("[\n]\n", TestUtility::bufferToString(data)); data.drain(data.length()); @@ -344,7 +347,7 @@ TEST_F(RdsImplTest, Basic) { })EOF"; // Test that we get the help text if we use the command in an invalid ways. - EXPECT_EQ(Http::Code::NotFound, handler_callback_("/routes?bad_param", data)); + EXPECT_EQ(Http::Code::NotFound, handler_callback_("/routes?bad_param", header_map, data)); EXPECT_EQ(routes_expected_output_usage, TestUtility::bufferToString(data)); data.drain(data.length()); @@ -510,7 +513,8 @@ TEST_F(RouteConfigProviderManagerImplTest, Basic) { )EOF"; // Test Admin /routes handler. - EXPECT_EQ(Http::Code::OK, handler_callback_("/routes", data)); + Http::HeaderMapImpl header_map; + EXPECT_EQ(Http::Code::OK, handler_callback_("/routes", header_map, data)); EXPECT_EQ(routes_expected_output, TestUtility::bufferToString(data)); data.drain(data.length()); diff --git a/test/integration/integration_admin_test.cc b/test/integration/integration_admin_test.cc index 1fbaec4de1d2..fd527bd74c47 100644 --- a/test/integration/integration_admin_test.cc +++ b/test/integration/integration_admin_test.cc @@ -100,6 +100,18 @@ TEST_P(IntegrationAdminTest, AdminLogging) { } } +namespace { + +const char* ContentType(const BufferingStreamDecoderPtr& response) { + const Http::HeaderEntry* entry = response->headers().ContentType(); + if (entry == nullptr) { + return "(null)"; + } + return entry->value().c_str(); +} + +} // namespace + TEST_P(IntegrationAdminTest, Admin) { initialize(); @@ -107,31 +119,31 @@ TEST_P(IntegrationAdminTest, Admin) { lookupPort("admin"), "GET", "/notfound", "", downstreamProtocol(), version_); EXPECT_TRUE(response->complete()); EXPECT_STREQ("404", response->headers().Status()->value().c_str()); - EXPECT_STREQ("text/plain; charset=UTF-8", response->headers().ContentType()->value().c_str()); + EXPECT_STREQ("text/plain; charset=UTF-8", ContentType(response)); response = IntegrationUtil::makeSingleRequest(lookupPort("admin"), "GET", "/", "", downstreamProtocol(), version_); EXPECT_TRUE(response->complete()); EXPECT_STREQ("200", response->headers().Status()->value().c_str()); - EXPECT_STREQ("text/html; charset=UTF-8", response->headers().ContentType()->value().c_str()); + EXPECT_STREQ("text/html; charset=UTF-8", ContentType(response)); response = IntegrationUtil::makeSingleRequest(lookupPort("admin"), "GET", "/server_info", "", downstreamProtocol(), version_); EXPECT_TRUE(response->complete()); EXPECT_STREQ("200", response->headers().Status()->value().c_str()); - EXPECT_STREQ("text/plain; charset=UTF-8", response->headers().ContentType()->value().c_str()); + EXPECT_STREQ("text/plain; charset=UTF-8", ContentType(response)); response = IntegrationUtil::makeSingleRequest(lookupPort("admin"), "GET", "/stats", "", downstreamProtocol(), version_); EXPECT_TRUE(response->complete()); EXPECT_STREQ("200", response->headers().Status()->value().c_str()); - EXPECT_STREQ("text/plain; charset=UTF-8", response->headers().ContentType()->value().c_str()); + EXPECT_STREQ("text/plain; charset=UTF-8", ContentType(response)); response = IntegrationUtil::makeSingleRequest(lookupPort("admin"), "GET", "/stats?format=blah", "", downstreamProtocol(), version_); EXPECT_TRUE(response->complete()); EXPECT_STREQ("404", response->headers().Status()->value().c_str()); - EXPECT_STREQ("text/plain; charset=UTF-8", response->headers().ContentType()->value().c_str()); + EXPECT_STREQ("text/plain; charset=UTF-8", ContentType(response)); response = IntegrationUtil::makeSingleRequest(lookupPort("admin"), "GET", "/stats?format=json", "", downstreamProtocol(), version_); @@ -139,7 +151,7 @@ TEST_P(IntegrationAdminTest, Admin) { EXPECT_STREQ("200", response->headers().Status()->value().c_str()); Json::ObjectSharedPtr statsjson = Json::Factory::loadFromString(response->body()); EXPECT_TRUE(statsjson->hasObject("stats")); - EXPECT_STREQ("text/plain; charset=UTF-8", response->headers().ContentType()->value().c_str()); + EXPECT_STREQ("text/plain; charset=UTF-8", ContentType(response)); response = IntegrationUtil::makeSingleRequest( lookupPort("admin"), "GET", "/stats?format=prometheus", "", downstreamProtocol(), version_); @@ -165,37 +177,37 @@ TEST_P(IntegrationAdminTest, Admin) { EXPECT_STREQ("200", response->headers().Status()->value().c_str()); EXPECT_THAT(response->body(), testing::HasSubstr("added_via_api")); EXPECT_THAT(response->body(), testing::HasSubstr("version_info::static\n")); - EXPECT_STREQ("text/plain; charset=UTF-8", response->headers().ContentType()->value().c_str()); + EXPECT_STREQ("text/plain; charset=UTF-8", ContentType(response)); response = IntegrationUtil::makeSingleRequest(lookupPort("admin"), "GET", "/cpuprofiler", "", downstreamProtocol(), version_); EXPECT_TRUE(response->complete()); EXPECT_STREQ("400", response->headers().Status()->value().c_str()); - EXPECT_STREQ("text/plain; charset=UTF-8", response->headers().ContentType()->value().c_str()); + EXPECT_STREQ("text/plain; charset=UTF-8", ContentType(response)); response = IntegrationUtil::makeSingleRequest(lookupPort("admin"), "GET", "/hot_restart_version", "", downstreamProtocol(), version_); EXPECT_TRUE(response->complete()); EXPECT_STREQ("200", response->headers().Status()->value().c_str()); - EXPECT_STREQ("text/plain; charset=UTF-8", response->headers().ContentType()->value().c_str()); + EXPECT_STREQ("text/plain; charset=UTF-8", ContentType(response)); response = IntegrationUtil::makeSingleRequest(lookupPort("admin"), "GET", "/reset_counters", "", downstreamProtocol(), version_); EXPECT_TRUE(response->complete()); EXPECT_STREQ("200", response->headers().Status()->value().c_str()); - EXPECT_STREQ("text/plain; charset=UTF-8", response->headers().ContentType()->value().c_str()); + EXPECT_STREQ("text/plain; charset=UTF-8", ContentType(response)); response = IntegrationUtil::makeSingleRequest(lookupPort("admin"), "GET", "/certs", "", downstreamProtocol(), version_); EXPECT_TRUE(response->complete()); EXPECT_STREQ("200", response->headers().Status()->value().c_str()); - EXPECT_STREQ("text/plain; charset=UTF-8", response->headers().ContentType()->value().c_str()); + EXPECT_STREQ("text/plain; charset=UTF-8", ContentType(response)); response = IntegrationUtil::makeSingleRequest(lookupPort("admin"), "GET", "/listeners", "", downstreamProtocol(), version_); EXPECT_TRUE(response->complete()); EXPECT_STREQ("200", response->headers().Status()->value().c_str()); - EXPECT_STREQ("text/plain; charset=UTF-8", response->headers().ContentType()->value().c_str()); + EXPECT_STREQ("text/plain; charset=UTF-8", ContentType(response)); Json::ObjectSharedPtr json = Json::Factory::loadFromString(response->body()); std::vector listener_info = json->asObjectArray(); diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index cff95129d821..8fb2da7ccf45 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -124,7 +124,7 @@ TEST_P(AdminInstanceTest, AdminBadAddressOutPath) { } TEST_P(AdminInstanceTest, CustomHandler) { - auto callback = [&](const std::string&, Buffer::Instance&) -> Http::Code { + auto callback = [&](const std::string&, Http::HeaderMap&, Buffer::Instance&) -> Http::Code { return Http::Code::Accepted; }; @@ -152,21 +152,21 @@ TEST_P(AdminInstanceTest, CustomHandler) { } TEST_P(AdminInstanceTest, RejectHandlerWithXss) { - auto callback = [&](const std::string&, Buffer::Instance&) -> Http::Code { + auto callback = [&](const std::string&, Http::HeaderMap&, Buffer::Instance&) -> Http::Code { return Http::Code::Accepted; }; EXPECT_FALSE(admin_.addHandler("/foo", "hello", callback, true)); } TEST_P(AdminInstanceTest, RejectHandlerWithEmbeddedQuery) { - auto callback = [&](const std::string&, Buffer::Instance&) -> Http::Code { + auto callback = [&](const std::string&, Http::HeaderMap&, Buffer::Instance&) -> Http::Code { return Http::Code::Accepted; }; EXPECT_FALSE(admin_.addHandler("/bar?queryShouldNotBeInPrefix", "hello", callback, true)); } TEST_P(AdminInstanceTest, EscapeHelpTextWithPunctuation) { - auto callback = [&](const std::string&, Buffer::Instance&) -> Http::Code { + auto callback = [&](const std::string&, Http::HeaderMap&, Buffer::Instance&) -> Http::Code { return Http::Code::Accepted; }; From 29b90650efa966be9a1a0b71d90c0511b6915827 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Thu, 11 Jan 2018 12:03:47 -0500 Subject: [PATCH 4/6] Add benchmark library from github.com/google/benchmark. This is extracted from https://github.com/envoyproxy/envoy/pull/2348 and needs to go in first -- with no code depdning on it -- so the CI works in the PR which uses the new dependency. Signed-off-by: Joshua Marantz --- bazel/envoy_build_system.bzl | 2 ++ bazel/external/BUILD | 4 ++-- bazel/target_recipes.bzl | 1 + ci/build_container/build_recipes/benchmark.sh | 21 +++++++++++++++++++ ci/prebuilt/BUILD | 7 +++++++ 5 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 ci/build_container/build_recipes/benchmark.sh diff --git a/bazel/envoy_build_system.bzl b/bazel/envoy_build_system.bzl index 482b0d7cd85b..2de6f7f11301 100644 --- a/bazel/envoy_build_system.bzl +++ b/bazel/envoy_build_system.bzl @@ -164,6 +164,7 @@ def envoy_cc_binary(name, data = [], testonly = 0, visibility = None, + external_deps = [], repository = "", stamped = False, deps = [], @@ -172,6 +173,7 @@ def envoy_cc_binary(name, if stamped: _git_stamped_genrule(repository, name) _git_stamped_genrule(repository, name + ".stripped") + deps = deps + [envoy_external_dep_path(dep) for dep in external_deps] native.cc_binary( name = name, srcs = srcs, diff --git a/bazel/external/BUILD b/bazel/external/BUILD index 82c3d20c8f25..63a81dd5e12b 100644 --- a/bazel/external/BUILD +++ b/bazel/external/BUILD @@ -4,9 +4,9 @@ cc_library( name = "all_external", srcs = [":empty.cc"], deps = [ - "@io_opentracing_cpp//:opentracing", - "@com_lightstep_tracer_cpp//:lightstep_tracer", "@com_google_googletest//:gtest", + "@com_lightstep_tracer_cpp//:lightstep_tracer", + "@io_opentracing_cpp//:opentracing", ], ) diff --git a/bazel/target_recipes.bzl b/bazel/target_recipes.bzl index 389126dc4703..0fafdd7d6b43 100644 --- a/bazel/target_recipes.bzl +++ b/bazel/target_recipes.bzl @@ -3,6 +3,7 @@ # ci/build_container/build_recipes. TARGET_RECIPES = { "ares": "cares", + "benchmark": "benchmark", "event": "libevent", "event_pthreads": "libevent", "tcmalloc_and_profiler": "gperftools", diff --git a/ci/build_container/build_recipes/benchmark.sh b/ci/build_container/build_recipes/benchmark.sh new file mode 100644 index 000000000000..78833678c365 --- /dev/null +++ b/ci/build_container/build_recipes/benchmark.sh @@ -0,0 +1,21 @@ +#!/bin/bash + +set -x + +export COMMIT="e1c3a83b8197cf02e794f61228461c27d4e78cfb" # benchmark @ Jan 11, 2018 + +git clone https://github.com/google/benchmark.git +(cd benchmark; git reset --hard "$COMMIT") +git clone https://github.com/google/googletest.git benchmark/googletest +mkdir build + +cd build +cmake -G "Unix Makefiles" ../benchmark -DCMAKE_BUILD_TYPE=RELEASE +make +cp src/libbenchmark.a "$THIRDPARTY_BUILD"/lib +cd ../benchmark + +pwd +include_dir="$THIRDPARTY_BUILD/include/testing/base/public" +mkdir -p "$include_dir" +cp include/benchmark/benchmark.h "$include_dir" diff --git a/ci/prebuilt/BUILD b/ci/prebuilt/BUILD index 1cfbb9d51985..e65c5db41683 100644 --- a/ci/prebuilt/BUILD +++ b/ci/prebuilt/BUILD @@ -9,6 +9,13 @@ cc_library( includes = ["thirdparty_build/include"], ) +cc_library( + name = "benchmark", + srcs = ["thirdparty_build/lib/libbenchmark.a"], + hdrs = ["thirdparty_build/include/testing/base/public/benchmark.h"], + includes = ["thirdparty_build/include"], +) + cc_library( name = "crypto", srcs = ["thirdparty_build/lib/libcrypto.a"], From 6097f043f59e534e90f8928c2e2053566eb276a3 Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Thu, 11 Jan 2018 13:39:07 -0500 Subject: [PATCH 5/6] Remove gtest dependency. Signed-off-by: Joshua Marantz --- ci/build_container/build_recipes/benchmark.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/ci/build_container/build_recipes/benchmark.sh b/ci/build_container/build_recipes/benchmark.sh index 78833678c365..87a3efeaa9b5 100644 --- a/ci/build_container/build_recipes/benchmark.sh +++ b/ci/build_container/build_recipes/benchmark.sh @@ -6,11 +6,12 @@ export COMMIT="e1c3a83b8197cf02e794f61228461c27d4e78cfb" # benchmark @ Jan 11, git clone https://github.com/google/benchmark.git (cd benchmark; git reset --hard "$COMMIT") -git clone https://github.com/google/googletest.git benchmark/googletest mkdir build cd build -cmake -G "Unix Makefiles" ../benchmark -DCMAKE_BUILD_TYPE=RELEASE +cmake -G "Unix Makefiles" ../benchmark \ + -DCMAKE_BUILD_TYPE=RELEASE \ + -DBENCHMARK_ENABLE_GTEST_TESTS=OFF make cp src/libbenchmark.a "$THIRDPARTY_BUILD"/lib cd ../benchmark From ca68fd50baa360c7466cd1538cc71b303d70f6bc Mon Sep 17 00:00:00 2001 From: Joshua Marantz Date: Wed, 17 Jan 2018 08:25:30 -0500 Subject: [PATCH 6/6] Address style issues. Signed-off-by: Joshua Marantz --- ci/build_container/build_recipes/benchmark.sh | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/ci/build_container/build_recipes/benchmark.sh b/ci/build_container/build_recipes/benchmark.sh index 87a3efeaa9b5..5e8f2f41ab2c 100644 --- a/ci/build_container/build_recipes/benchmark.sh +++ b/ci/build_container/build_recipes/benchmark.sh @@ -1,7 +1,6 @@ #!/bin/bash -set -x - +set -e export COMMIT="e1c3a83b8197cf02e794f61228461c27d4e78cfb" # benchmark @ Jan 11, 2018 git clone https://github.com/google/benchmark.git @@ -16,7 +15,6 @@ make cp src/libbenchmark.a "$THIRDPARTY_BUILD"/lib cd ../benchmark -pwd -include_dir="$THIRDPARTY_BUILD/include/testing/base/public" -mkdir -p "$include_dir" -cp include/benchmark/benchmark.h "$include_dir" +INCLUDE_DIR="$THIRDPARTY_BUILD/include/testing/base/public" +mkdir -p "$INCLUDE_DIR" +cp include/benchmark/benchmark.h "$INCLUDE_DIR"