/runtime_modify: add support for query params in body

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<config_access_log_format_response_code_details>` and :ref:`gRPC access logger<envoy_api_field_data.accesslog.v2.HTTPResponseProperties.response_code_details>`.
 * admin: the administration interface now includes a :ref:`/ready endpoint <operations_admin_interface>` for easier readiness checks.
+* admin: extend :ref:`/runtime_modify endpoint <operations_admin_interface_runtime_modify>` 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 <config_network_filters_dubbo_proxy>`.

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::parseFormBody(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;

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

source/server/http/admin.cc

@@ -1075,12 +1075,21 @@ 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.
+    // TODO(rgs1): check the content-type header in the request.
+    if (admin_stream.getRequestBody() != nullptr) {
+      params = Http::Utility::parseFormBody(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 within the request's body\n");
+      response.add("use an empty value to remove a previously added override");
+      return Http::Code::BadRequest;
+    }
   }
   std::unordered_map<std::string, std::string> overrides;
   overrides.insert(params.begin(), params.end());

test/server/http/admin_test.cc

@@ -589,12 +589,19 @@ class AdminInstanceTest : public testing::TestWithParam<Network::Address::IpVers
     EXPECT_EQ(std::chrono::milliseconds(100), admin_.drainTimeout());
     admin_.tracingStats().random_sampling_.inc();
     EXPECT_TRUE(admin_.setCurrentClientCertDetails().empty());
+    admin_filter_.setDecoderFilterCallbacks(callbacks_);
   }
 
   Http::Code runCallback(absl::string_view path_and_query, Http::HeaderMap& response_headers,
-                         Buffer::Instance& response, absl::string_view method) {
+                         Buffer::Instance& response, absl::string_view method,
+                         absl::string_view body = absl::string_view()) {
     request_headers_.insertMethod().value(method.data(), method.size());
     admin_filter_.decodeHeaders(request_headers_, false);
+
+    if (!body.empty()) {
+      callbacks_.buffer_ = std::make_unique<Buffer::OwnedImpl>(body);
+    }
+
     return admin_.runCallback(path_and_query, response_headers, response, admin_filter_);
   }
 
@@ -617,6 +624,7 @@ class AdminInstanceTest : public testing::TestWithParam<Network::Address::IpVers
   AdminImpl admin_;
   Http::TestHeaderMapImpl request_headers_;
   Server::AdminFilter admin_filter_;
+  NiceMock<Http::MockStreamDecoderFilterCallbacks> callbacks_;
 };
 
 INSTANTIATE_TEST_SUITE_P(IpVersions, AdminInstanceTest,
@@ -997,6 +1005,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<std::string, std::string> 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;