From 3eeaf81f5269ad4fc30e0a30e4f3bfe40199b128 Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Thu, 16 May 2019 19:57:08 -0400 Subject: [PATCH 01/20] /runtime_modify: add support for query params in body This adds support for setting RuntimeFractionalPercent values via `/runtime_modify`, since it needs to be YAML and YAML cannot be passed as query parameters (contains spaces & newlines). Fixes: #6935 Signed-off-by: Raul Gutierrez Segales --- source/common/http/utility.cc | 11 ++++++++++- source/common/http/utility.h | 15 +++++++++++++++ source/server/http/admin.cc | 20 +++++++++++++++----- test/server/http/admin_test.cc | 15 +++++++++++++++ 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 1f82cbc5ba976..76cad8a24aca4 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -93,13 +93,22 @@ 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) { return params; } start++; + return parseParameters(url, start); +} + +Utility::QueryParams Utility::parseFormBody(absl::string_view body) { + return parseParameters(body, 0); +} + +Utility::QueryParams Utility::parseParameters(absl::string_view data, size_t start) { + QueryParams params; + while (start < url.size()) { size_t end = url.find('&', start); if (end == std::string::npos) { diff --git a/source/common/http/utility.h b/source/common/http/utility.h index 24cb394c6fa32..020b8bc84d9d7 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 parseFormBody(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..af34e5ef18f78 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1075,12 +1075,22 @@ std::string AdminImpl::runtimeAsJson( } 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. Ideally, the content-type + // header would be `application/x-www-form-urlencoded`, but there's no way to + // check. + if (admin_stream.getRequestBody() != nullptr) { + auto body = absl::string_view(admin_stream.getRequestBody()->toString()); + params = parseFormBody(body); + } + + 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; + } } std::unordered_map overrides; overrides.insert(params.begin(), params.end()); diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index d5dc56bab2d4e..65eba051de504 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -997,6 +997,21 @@ TEST_P(AdminInstanceTest, RuntimeModify) { EXPECT_EQ("OK\n", response.toString()); } +TEST_P(AdminInstanceTest, RuntimeModifyParamsInBody) { + Http::HeaderMapImpl header_map; + Buffer::OwnedImpl response; + + Runtime::MockLoader loader; + EXPECT_CALL(server_, runtime()).WillRepeatedly(testing::ReturnPointee(&loader)); + + const std::string body = "numerator: 1\ndenominator: TEN_THOUSAND\n"; + std::unordered_map overrides; + overrides["routing.traffic_shift.foo"] = body; + EXPECT_CALL(loader, mergeValues(overrides)).Times(1); + EXPECT_EQ(Http::Code::OK, admin_.request("/runtime_modify", "POST", header_map, body)); + EXPECT_EQ("OK\n", response.toString()); +} + TEST_P(AdminInstanceTest, RuntimeModifyNoArguments) { Http::HeaderMapImpl header_map; Buffer::OwnedImpl response; From ba07c2d7d55c823724d5993c156f2dc26f031627 Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Thu, 16 May 2019 20:03:58 -0400 Subject: [PATCH 02/20] absl::string_view can be constructed implicitly from a std::string Signed-off-by: Raul Gutierrez Segales --- source/server/http/admin.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index af34e5ef18f78..3a6bde11c829b 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1082,8 +1082,7 @@ Http::Code AdminImpl::handlerRuntimeModify(absl::string_view url, Http::HeaderMa // header would be `application/x-www-form-urlencoded`, but there's no way to // check. if (admin_stream.getRequestBody() != nullptr) { - auto body = absl::string_view(admin_stream.getRequestBody()->toString()); - params = parseFormBody(body); + params = parseFormBody(admin_stream.getRequestBody()->toString()); } if (params.empty()) { From d30e256cb842a4b22bd8b136d70a611c72142193 Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Thu, 16 May 2019 21:10:30 -0400 Subject: [PATCH 03/20] Fix var rename Signed-off-by: Raul Gutierrez Segales --- source/common/http/utility.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index 76cad8a24aca4..e967f70012f08 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -109,19 +109,19 @@ Utility::QueryParams Utility::parseFormBody(absl::string_view body) { Utility::QueryParams Utility::parseParameters(absl::string_view data, size_t start) { QueryParams params; - while (start < url.size()) { - size_t end = url.find('&', start); + 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; From 2e1607fc7d9e246b88624d039cbbe34e478b401c Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Thu, 16 May 2019 21:25:52 -0400 Subject: [PATCH 04/20] Fix test Signed-off-by: Raul Gutierrez Segales --- test/server/http/admin_test.cc | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index 65eba051de504..bc1450f4811b0 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -592,9 +592,16 @@ class AdminInstanceTest : public testing::TestWithParam 0) { + Buffer::OwnedImpl data(body); + admin_filter_.decodeData(data, true); + } + return admin_.runCallback(path_and_query, response_headers, response, admin_filter_); } @@ -1008,7 +1015,7 @@ TEST_P(AdminInstanceTest, RuntimeModifyParamsInBody) { std::unordered_map overrides; overrides["routing.traffic_shift.foo"] = body; EXPECT_CALL(loader, mergeValues(overrides)).Times(1); - EXPECT_EQ(Http::Code::OK, admin_.request("/runtime_modify", "POST", header_map, body)); + EXPECT_EQ(Http::Code::OK, runCallback("/runtime_modify", header_map, response, "POST", body)); EXPECT_EQ("OK\n", response.toString()); } From 7696927112aeda2c55e9d9fc563b475463e47a4d Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Thu, 16 May 2019 21:30:45 -0400 Subject: [PATCH 05/20] Fix test a bit Signed-off-by: Raul Gutierrez Segales --- test/server/http/admin_test.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index bc1450f4811b0..3cd235988428e 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -1005,16 +1005,16 @@ TEST_P(AdminInstanceTest, RuntimeModify) { } TEST_P(AdminInstanceTest, RuntimeModifyParamsInBody) { - Http::HeaderMapImpl header_map; - Buffer::OwnedImpl response; + const std::string body = "numerator: 1\ndenominator: TEN_THOUSAND\n"; Runtime::MockLoader loader; EXPECT_CALL(server_, runtime()).WillRepeatedly(testing::ReturnPointee(&loader)); - const std::string body = "numerator: 1\ndenominator: TEN_THOUSAND\n"; - std::unordered_map overrides; - overrides["routing.traffic_shift.foo"] = body; + std::unordered_map overrides = {{"routing.traffic_shift.foo", body}}; EXPECT_CALL(loader, mergeValues(overrides)).Times(1); + + 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()); } From 45f7cbd6e489f6b2c2143d81dd51a56e37250c81 Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Thu, 16 May 2019 21:33:26 -0400 Subject: [PATCH 06/20] Fix spelling Signed-off-by: Raul Gutierrez Segales --- source/server/http/admin.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index 3a6bde11c829b..a458805607866 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1078,9 +1078,8 @@ Http::Code AdminImpl::handlerRuntimeModify(absl::string_view url, Http::HeaderMa Buffer::Instance& response, AdminStream& admin_stream) { Http::Utility::QueryParams params = Http::Utility::parseQueryString(url); if (params.empty()) { - // Check if the params are in the request's body. Ideally, the content-type - // header would be `application/x-www-form-urlencoded`, but there's no way to - // check. + // Check if the params are in the request's body. + // TODO(rgs1): check the content-type header in the request. if (admin_stream.getRequestBody() != nullptr) { params = parseFormBody(admin_stream.getRequestBody()->toString()); } From f784b380fe058b7ec3bea43822f2747160ce1989 Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Thu, 16 May 2019 21:37:27 -0400 Subject: [PATCH 07/20] Full namespace Signed-off-by: Raul Gutierrez Segales --- source/server/http/admin.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index a458805607866..237a0765c0c15 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1081,7 +1081,7 @@ Http::Code AdminImpl::handlerRuntimeModify(absl::string_view url, Http::HeaderMa // Check if the params are in the request's body. // TODO(rgs1): check the content-type header in the request. if (admin_stream.getRequestBody() != nullptr) { - params = parseFormBody(admin_stream.getRequestBody()->toString()); + params = Http::Utility::parseFormBody(admin_stream.getRequestBody()->toString()); } if (params.empty()) { From fccabfc91c9ad6712d7947ba5c339876e8e218bf Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Fri, 17 May 2019 08:47:37 +0200 Subject: [PATCH 08/20] Add missing local var Signed-off-by: Raul Gutierrez Segales --- source/common/http/utility.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index e967f70012f08..f295c3af81d87 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -95,6 +95,7 @@ std::string Utility::createSslRedirectPath(const HeaderMap& headers) { Utility::QueryParams Utility::parseQueryString(absl::string_view url) { size_t start = url.find('?'); if (start == std::string::npos) { + QueryParams params; return params; } From 4a285151b9de1367def3a7b419ea1d652dc04f3e Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Fri, 17 May 2019 14:45:02 +0200 Subject: [PATCH 09/20] Fix test * set callbacks * set decoded data Signed-off-by: Raul Gutierrez Segales --- test/server/http/admin_test.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index 3cd235988428e..b0554f56e39f8 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -589,6 +589,7 @@ class AdminInstanceTest : public testing::TestWithParam 0) { + if (!body.empty()) { Buffer::OwnedImpl data(body); - admin_filter_.decodeData(data, true); + callbacks_.buffer_ = std::move(data); } return admin_.runCallback(path_and_query, response_headers, response, admin_filter_); @@ -624,6 +625,7 @@ class AdminInstanceTest : public testing::TestWithParam callbacks_; }; INSTANTIATE_TEST_SUITE_P(IpVersions, AdminInstanceTest, From 88eabd45cddf1ec39d5a3ec268fbf7e410ebf9ab Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Fri, 17 May 2019 15:08:08 +0200 Subject: [PATCH 10/20] Make test pass I am not in a plane anymore, so I can't actually compile the tests locally :-) Signed-off-by: Raul Gutierrez Segales --- test/server/http/admin_test.cc | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index b0554f56e39f8..561daf04350c8 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -599,8 +599,7 @@ class AdminInstanceTest : public testing::TestWithParam overrides = {{"routing.traffic_shift.foo", body}}; + const std::unordered_map overrides = {{key, value}}; EXPECT_CALL(loader, mergeValues(overrides)).Times(1); Http::HeaderMapImpl header_map; From d0fa8fa81f40b33b18af58ee59b32aa2f2fe1a09 Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Fri, 17 May 2019 15:13:10 +0200 Subject: [PATCH 11/20] Update usage string Signed-off-by: Raul Gutierrez Segales --- source/server/http/admin.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index 237a0765c0c15..c49cf38822ec9 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1086,6 +1086,7 @@ Http::Code AdminImpl::handlerRuntimeModify(absl::string_view url, Http::HeaderMa if (params.empty()) { response.add("usage: /runtime_modify?key1=value1&key2=value2&keyN=valueN\n"); + response.add(" or send the parameters within the request's body\n"); response.add("use an empty value to remove a previously added override"); return Http::Code::BadRequest; } From 8d36539cd239e4c89d28b2c33d8a868e17db5fca Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Fri, 17 May 2019 18:11:59 +0200 Subject: [PATCH 12/20] Use make_unique Signed-off-by: Raul Gutierrez Segales --- test/server/http/admin_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index 561daf04350c8..afad8ebb486a7 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -599,7 +599,7 @@ class AdminInstanceTest : public testing::TestWithParam(body); } return admin_.runCallback(path_and_query, response_headers, response, admin_filter_); From 6f9f65a2b84fe419ed9602430d861b849fd28300 Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Fri, 17 May 2019 18:16:32 +0200 Subject: [PATCH 13/20] Update version history Signed-off-by: Raul Gutierrez Segales --- docs/root/intro/version_history.rst | 1 + 1 file changed, 1 insertion(+) 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 `. From b2b8232bd6ee17c0241518596866f8b1648f41b6 Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Fri, 17 May 2019 18:32:03 +0200 Subject: [PATCH 14/20] Move vars closer to usage Signed-off-by: Raul Gutierrez Segales --- test/server/http/admin_test.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index afad8ebb486a7..f188335f76a60 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -1006,16 +1006,15 @@ TEST_P(AdminInstanceTest, RuntimeModify) { } TEST_P(AdminInstanceTest, RuntimeModifyParamsInBody) { - const std::string key = "routing.traffic_shift.foo"; - const std::string value = "numerator: 1\ndenominator: TEN_THOUSAND\n"; - const std::string body = fmt::format("{}={}", key, value); - 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)); From 9209859733f245574e42bca7c4e268fef585bf68 Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Sat, 18 May 2019 01:42:50 +0200 Subject: [PATCH 15/20] parseFormBody -> parseFromBody Signed-off-by: Raul Gutierrez Segales --- source/common/http/utility.cc | 2 +- source/common/http/utility.h | 2 +- source/server/http/admin.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/source/common/http/utility.cc b/source/common/http/utility.cc index f295c3af81d87..4fb77f3b1bef1 100644 --- a/source/common/http/utility.cc +++ b/source/common/http/utility.cc @@ -103,7 +103,7 @@ Utility::QueryParams Utility::parseQueryString(absl::string_view url) { return parseParameters(url, start); } -Utility::QueryParams Utility::parseFormBody(absl::string_view body) { +Utility::QueryParams Utility::parseFromBody(absl::string_view body) { return parseParameters(body, 0); } diff --git a/source/common/http/utility.h b/source/common/http/utility.h index 020b8bc84d9d7..e2881b15181a1 100644 --- a/source/common/http/utility.h +++ b/source/common/http/utility.h @@ -72,7 +72,7 @@ QueryParams parseQueryString(absl::string_view url); * @param body supplies the body to parse. * @return QueryParams the parsed parameters, if any. */ -QueryParams parseFormBody(absl::string_view body); +QueryParams parseFromBody(absl::string_view body); /** * Parse query parameters from a URL or body. diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index c49cf38822ec9..0b97cade5ea48 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1081,7 +1081,7 @@ Http::Code AdminImpl::handlerRuntimeModify(absl::string_view url, Http::HeaderMa // Check if the params are in the request's body. // TODO(rgs1): check the content-type header in the request. if (admin_stream.getRequestBody() != nullptr) { - params = Http::Utility::parseFormBody(admin_stream.getRequestBody()->toString()); + params = Http::Utility::parseFromBody(admin_stream.getRequestBody()->toString()); } if (params.empty()) { From 29baa73c23c386f7c2c7d36aa35ea06fa21dd054 Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Tue, 21 May 2019 10:52:03 +0200 Subject: [PATCH 16/20] Check the content-type before decoding params from body Only decode params from body if content-type is `application/x-www-form-urlencoded`. Signed-off-by: Raul Gutierrez Segales --- source/server/http/admin.cc | 11 ++++++++++- source/server/http/admin.h | 1 + test/server/http/admin_test.cc | 7 ++++--- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index 0b97cade5ea48..7ef354612bb52 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1074,13 +1074,22 @@ 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() == "application/x-www-form-urlencoded"; +} + Http::Code AdminImpl::handlerRuntimeModify(absl::string_view url, Http::HeaderMap&, Buffer::Instance& response, AdminStream& admin_stream) { Http::Utility::QueryParams params = Http::Utility::parseQueryString(url); if (params.empty()) { // Check if the params are in the request's body. // TODO(rgs1): check the content-type header in the request. - if (admin_stream.getRequestBody() != nullptr) { + if (admin_stream.getRequestBody() != nullptr && + isFormUrlEncoded(admin_stream.getRequestHeaders().ContentType())) { params = Http::Utility::parseFromBody(admin_stream.getRequestBody()->toString()); } 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/server/http/admin_test.cc b/test/server/http/admin_test.cc index f188335f76a60..fe5776e03fac4 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -595,13 +595,14 @@ 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_); } From ab573b6125dd250788e7c2780b1c909dfe0e52f4 Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Tue, 21 May 2019 12:37:50 +0200 Subject: [PATCH 17/20] Hit /runtime_modify from integration test Signed-off-by: Raul Gutierrez Segales --- test/integration/integration_admin_test.cc | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/test/integration/integration_admin_test.cc b/test/integration/integration_admin_test.cc index 3c1c527913828..df0d17ece9e34 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", "application/x-www-form-urlencoded"); + 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(); From 39782546cc571dcc4b72a6a5d3739badf0399e82 Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Tue, 21 May 2019 14:48:55 +0200 Subject: [PATCH 18/20] Remove TODO comment Signed-off-by: Raul Gutierrez Segales --- source/server/http/admin.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index 7ef354612bb52..62fe211f2777c 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1087,7 +1087,6 @@ Http::Code AdminImpl::handlerRuntimeModify(absl::string_view url, Http::HeaderMa Http::Utility::QueryParams params = Http::Utility::parseQueryString(url); if (params.empty()) { // Check if the params are in the request's body. - // TODO(rgs1): check the content-type header in the request. if (admin_stream.getRequestBody() != nullptr && isFormUrlEncoded(admin_stream.getRequestHeaders().ContentType())) { params = Http::Utility::parseFromBody(admin_stream.getRequestBody()->toString()); From 7e001d8eceebc036e4af159139438b2c75219bcc Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Tue, 21 May 2019 17:38:01 +0200 Subject: [PATCH 19/20] Use a header value constant for for the content-type Signed-off-by: Raul Gutierrez Segales --- source/common/http/headers.h | 1 + source/server/http/admin.cc | 3 ++- test/integration/integration_admin_test.cc | 6 +++--- test/server/http/admin_test.cc | 3 ++- 4 files changed, 8 insertions(+), 5 deletions(-) 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/server/http/admin.cc b/source/server/http/admin.cc index 62fe211f2777c..494921fcb3a2c 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1079,7 +1079,8 @@ bool AdminImpl::isFormUrlEncoded(const Http::HeaderEntry* content_type) const { return false; } - return content_type->value().getStringView() == "application/x-www-form-urlencoded"; + return content_type->value().getStringView() == + Http::Headers::get().ContentTypeValues.FormUrlEncoded; } Http::Code AdminImpl::handlerRuntimeModify(absl::string_view url, Http::HeaderMap&, diff --git a/test/integration/integration_admin_test.cc b/test/integration/integration_admin_test.cc index df0d17ece9e34..611221973dd4f 100644 --- a/test/integration/integration_admin_test.cc +++ b/test/integration/integration_admin_test.cc @@ -319,9 +319,9 @@ 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", "application/x-www-form-urlencoded"); + 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()); diff --git a/test/server/http/admin_test.cc b/test/server/http/admin_test.cc index fe5776e03fac4..8fb669721907e 100644 --- a/test/server/http/admin_test.cc +++ b/test/server/http/admin_test.cc @@ -596,7 +596,8 @@ class AdminInstanceTest : public testing::TestWithParam(body); } From 29113c79e7541d1623f3831f428444163e81db57 Mon Sep 17 00:00:00 2001 From: Raul Gutierrez Segales Date: Tue, 21 May 2019 17:40:37 +0200 Subject: [PATCH 20/20] Update usage Signed-off-by: Raul Gutierrez Segales --- source/server/http/admin.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/server/http/admin.cc b/source/server/http/admin.cc index 494921fcb3a2c..e06de5e84898a 100644 --- a/source/server/http/admin.cc +++ b/source/server/http/admin.cc @@ -1095,7 +1095,7 @@ Http::Code AdminImpl::handlerRuntimeModify(absl::string_view url, Http::HeaderMa if (params.empty()) { response.add("usage: /runtime_modify?key1=value1&key2=value2&keyN=valueN\n"); - response.add(" or send the parameters within the request's body\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; }