diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst index fa875e0ebe772..26f0843d2f8c6 100644 --- a/docs/root/intro/version_history.rst +++ b/docs/root/intro/version_history.rst @@ -6,6 +6,7 @@ Version history * access log: added a new field for downstream TLS session ID to file and gRPC access logger. * access log: added a new field for response code details in :ref:`file access logger` and :ref:`gRPC access logger`. * admin: the administration interface now includes a :ref:`/ready endpoint ` for easier readiness checks. +* admin: extend :ref:`/runtime_modify endpoint ` to support parameters within the request body. * api: track and report requests issued since last load report. * build: releases are built with Clang and linked with LLD. * dubbo_proxy: support the :ref:`Dubbo proxy filter `. diff --git a/source/common/http/headers.h b/source/common/http/headers.h index 38403792c545e..5aa15e718bf83 100644 --- a/source/common/http/headers.h +++ b/source/common/http/headers.h @@ -130,6 +130,7 @@ class HeaderValues { const std::string GrpcWebText{"application/grpc-web-text"}; const std::string GrpcWebTextProto{"application/grpc-web-text+proto"}; const std::string Json{"application/json"}; + const std::string FormUrlEncoded{"application/x-www-form-urlencoded"}; } ContentTypeValues; struct { diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 1f82cbc5ba976..4fb77f3b1bef1 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -93,26 +93,36 @@ std::string Utility::createSslRedirectPath(const HeaderMap& headers) { } Utility::QueryParams Utility::parseQueryString(absl::string_view url) { - QueryParams params; size_t start = url.find('?'); if (start == std::string::npos) { + QueryParams params; return params; } start++; - while (start < url.size()) { - size_t end = url.find('&', start); + return parseParameters(url, start); +} + +Utility::QueryParams Utility::parseFromBody(absl::string_view body) { + return parseParameters(body, 0); +} + +Utility::QueryParams Utility::parseParameters(absl::string_view data, size_t start) { + QueryParams params; + + while (start < data.size()) { + size_t end = data.find('&', start); if (end == std::string::npos) { - end = url.size(); + end = data.size(); } - absl::string_view param(url.data() + start, end - start); + absl::string_view param(data.data() + start, end - start); const size_t equal = param.find('='); if (equal != std::string::npos) { - params.emplace(StringUtil::subspan(url, start, start + equal), - StringUtil::subspan(url, start + equal + 1, end)); + params.emplace(StringUtil::subspan(data, start, start + equal), + StringUtil::subspan(data, start + equal + 1, end)); } else { - params.emplace(StringUtil::subspan(url, start, end), ""); + params.emplace(StringUtil::subspan(data, start, end), ""); } start = end + 1; diff --git a/source/common/http/utility.h b/source/common/http/utility.h index 24cb394c6fa32..e2881b15181a1 100644 --- a/source/common/http/utility.h +++ b/source/common/http/utility.h @@ -67,6 +67,21 @@ std::string createSslRedirectPath(const HeaderMap& headers); */ QueryParams parseQueryString(absl::string_view url); +/** + * Parse a a request body into query parameters. + * @param body supplies the body to parse. + * @return QueryParams the parsed parameters, if any. + */ +QueryParams parseFromBody(absl::string_view body); + +/** + * Parse query parameters from a URL or body. + * @param data supplies the data to parse. + * @param start supplies the offset within the data. + * @return QueryParams the parsed parameters, if any. + */ +QueryParams parseParameters(absl::string_view data, size_t start); + /** * Finds the start of the query string in a path * @param path supplies a HeaderString& to search for the query string diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index e6554b2251491..e06de5e84898a 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1074,13 +1074,31 @@ std::string AdminImpl::runtimeAsJson( return strbuf.GetString(); } +bool AdminImpl::isFormUrlEncoded(const Http::HeaderEntry* content_type) const { + if (content_type == nullptr) { + return false; + } + + return content_type->value().getStringView() == + Http::Headers::get().ContentTypeValues.FormUrlEncoded; +} + Http::Code AdminImpl::handlerRuntimeModify(absl::string_view url, Http::HeaderMap&, - Buffer::Instance& response, AdminStream&) { - const Http::Utility::QueryParams params = Http::Utility::parseQueryString(url); + Buffer::Instance& response, AdminStream& admin_stream) { + Http::Utility::QueryParams params = Http::Utility::parseQueryString(url); if (params.empty()) { - response.add("usage: /runtime_modify?key1=value1&key2=value2&keyN=valueN\n"); - response.add("use an empty value to remove a previously added override"); - return Http::Code::BadRequest; + // Check if the params are in the request's body. + if (admin_stream.getRequestBody() != nullptr && + isFormUrlEncoded(admin_stream.getRequestHeaders().ContentType())) { + params = Http::Utility::parseFromBody(admin_stream.getRequestBody()->toString()); + } + + if (params.empty()) { + response.add("usage: /runtime_modify?key1=value1&key2=value2&keyN=valueN\n"); + response.add(" or send the parameters as form values\n"); + response.add("use an empty value to remove a previously added override"); + return Http::Code::BadRequest; + } } std::unordered_map overrides; overrides.insert(params.begin(), params.end()); diff --git a/source/server/http/admin.h b/source/server/http/admin.h index f09e5bf61bf9d..7b2ab3944c652 100644 --- a/source/server/http/admin.h +++ b/source/server/http/admin.h @@ -253,6 +253,7 @@ class AdminImpl : public Admin, Http::Code handlerRuntimeModify(absl::string_view path_and_query, Http::HeaderMap& response_headers, Buffer::Instance& response, AdminStream&); + bool isFormUrlEncoded(const Http::HeaderEntry* content_type) const; class AdminListener : public Network::ListenerConfig { public: diff --git a/test/integration/integration_admin_test.cc b/test/integration/integration_admin_test.cc index 3c1c527913828..611221973dd4f 100644 --- a/test/integration/integration_admin_test.cc +++ b/test/integration/integration_admin_test.cc @@ -319,19 +319,32 @@ TEST_P(IntegrationAdminTest, Admin) { EXPECT_EQ("200", response->headers().Status()->value().getStringView()); EXPECT_EQ("application/json", ContentType(response)); + response = IntegrationUtil::makeSingleRequest( + lookupPort("admin"), "POST", "/runtime_modify", "foo=bar&foo1=bar1", downstreamProtocol(), + version_, "host", Http::Headers::get().ContentTypeValues.FormUrlEncoded); + EXPECT_TRUE(response->complete()); + EXPECT_EQ("200", response->headers().Status()->value().getStringView()); + response = IntegrationUtil::makeSingleRequest(lookupPort("admin"), "GET", "/runtime?format=json", "", downstreamProtocol(), version_); EXPECT_TRUE(response->complete()); EXPECT_EQ("200", response->headers().Status()->value().getStringView()); EXPECT_EQ("application/json", ContentType(response)); + Json::ObjectSharedPtr json = Json::Factory::loadFromString(response->body()); + auto entries = json->getObject("entries"); + auto foo_obj = entries->getObject("foo"); + EXPECT_EQ("bar", foo_obj->getString("final_value")); + auto foo1_obj = entries->getObject("foo1"); + EXPECT_EQ("bar1", foo1_obj->getString("final_value")); + response = IntegrationUtil::makeSingleRequest(lookupPort("admin"), "GET", "/listeners", "", downstreamProtocol(), version_); EXPECT_TRUE(response->complete()); EXPECT_EQ("200", response->headers().Status()->value().getStringView()); EXPECT_EQ("application/json", ContentType(response)); - Json::ObjectSharedPtr json = Json::Factory::loadFromString(response->body()); + json = Json::Factory::loadFromString(response->body()); std::vector listener_info = json->asObjectArray(); auto listener_info_it = listener_info.cbegin(); auto listeners = test_server_->server().listenerManager().listeners(); diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index d5dc56bab2d4e..8fb669721907e 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -589,12 +589,21 @@ class AdminInstanceTest : public testing::TestWithParam(body); + } + request_headers_.insertMethod().value(method.data(), method.size()); admin_filter_.decodeHeaders(request_headers_, false); + return admin_.runCallback(path_and_query, response_headers, response, admin_filter_); } @@ -617,6 +626,7 @@ class AdminInstanceTest : public testing::TestWithParam callbacks_; }; INSTANTIATE_TEST_SUITE_P(IpVersions, AdminInstanceTest, @@ -997,6 +1007,22 @@ TEST_P(AdminInstanceTest, RuntimeModify) { EXPECT_EQ("OK\n", response.toString()); } +TEST_P(AdminInstanceTest, RuntimeModifyParamsInBody) { + Runtime::MockLoader loader; + EXPECT_CALL(server_, runtime()).WillRepeatedly(testing::ReturnPointee(&loader)); + + const std::string key = "routing.traffic_shift.foo"; + const std::string value = "numerator: 1\ndenominator: TEN_THOUSAND\n"; + const std::unordered_map overrides = {{key, value}}; + EXPECT_CALL(loader, mergeValues(overrides)).Times(1); + + const std::string body = fmt::format("{}={}", key, value); + Http::HeaderMapImpl header_map; + Buffer::OwnedImpl response; + EXPECT_EQ(Http::Code::OK, runCallback("/runtime_modify", header_map, response, "POST", body)); + EXPECT_EQ("OK\n", response.toString()); +} + TEST_P(AdminInstanceTest, RuntimeModifyNoArguments) { Http::HeaderMapImpl header_map; Buffer::OwnedImpl response;