http, utility: Allow to optionally decode the parsed param value when parsing query string

source/common/config/udpa_resource.cc

@@ -131,7 +131,7 @@ void decodePath(absl::string_view path, std::string* resource_type,
 void decodeQueryParams(absl::string_view query_params,
                        udpa::core::v1::ContextParams& context_params) {
   Http::Utility::QueryParams query_params_components =
-      Http::Utility::parseQueryString(query_params);
+      Http::Utility::parseQueryString(query_params, /*decode_param_value=*/false);
   for (const auto& it : query_params_components) {
     (*context_params.mutable_params())[PercentEncoding::decode(it.first)] =
         PercentEncoding::decode(it.second);

source/common/http/utility.cc

@@ -264,22 +264,23 @@ std::string Utility::createSslRedirectPath(const RequestHeaderMap& headers) {
   return fmt::format("https://{}{}", headers.getHostValue(), headers.getPathValue());
 }
 
-Utility::QueryParams Utility::parseQueryString(absl::string_view url) {
+Utility::QueryParams Utility::parseQueryString(absl::string_view url, bool decode_param_value) {
   size_t start = url.find('?');
   if (start == std::string::npos) {
     QueryParams params;
     return params;
   }
 
   start++;
-  return parseParameters(url, start);
+  return parseParameters(url, start, decode_param_value);
 }
 
 Utility::QueryParams Utility::parseFromBody(absl::string_view body) {
-  return parseParameters(body, 0);
+  return parseParameters(body, 0, /*decode_param_value=*/true);
 }
 
-Utility::QueryParams Utility::parseParameters(absl::string_view data, size_t start) {
+Utility::QueryParams Utility::parseParameters(absl::string_view data, size_t start,
+                                              bool decode_param_value) {
   QueryParams params;
 
   while (start < data.size()) {
@@ -291,8 +292,9 @@ Utility::QueryParams Utility::parseParameters(absl::string_view data, size_t sta
 
     const size_t equal = param.find('=');
     if (equal != std::string::npos) {
+      const auto param_value = StringUtil::subspan(data, start + equal + 1, end);
       params.emplace(StringUtil::subspan(data, start, start + equal),
-                     StringUtil::subspan(data, start + equal + 1, end));
+                     decode_param_value ? PercentEncoding::decode(param_value) : param_value);
     } else {
       params.emplace(StringUtil::subspan(data, start, end), "");
     }

source/common/http/utility.h

@@ -189,9 +189,11 @@ std::string createSslRedirectPath(const RequestHeaderMap& headers);
 /**
  * Parse a URL into query parameters.
  * @param url supplies the url to parse.
+ * @param decode_param_value supplies the flag whether to percent-decode the parsed param values.
+ *        Set to false to keep the param values encoded. Defaults to true.
  * @return QueryParams the parsed parameters, if any.
  */
-QueryParams parseQueryString(absl::string_view url);
+QueryParams parseQueryString(absl::string_view url, bool decode_param_value = true);
 
 /**
  * Parse a a request body into query parameters.
@@ -204,9 +206,11 @@ 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.
+ * @param decode_param_value supplies the flag whether to percent-decode the parsed param value.
+ *        Set to false to keep the param values encoded.
  * @return QueryParams the parsed parameters, if any.
  */
-QueryParams parseParameters(absl::string_view data, size_t start);
+QueryParams parseParameters(absl::string_view data, size_t start, bool decode_param_value);
 
 /**
  * Finds the start of the query string in a path

source/server/admin/stats_handler.cc

@@ -140,7 +140,8 @@ Http::Code StatsHandler::handlerStats(absl::string_view url,
 Http::Code StatsHandler::handlerPrometheusStats(absl::string_view path_and_query,
                                                 Http::ResponseHeaderMap&,
                                                 Buffer::Instance& response, AdminStream&) {
-  const Http::Utility::QueryParams params = Http::Utility::parseQueryString(path_and_query);
+  const Http::Utility::QueryParams params =
+      Http::Utility::parseQueryString(path_and_query, /*decode_param_value=*/true);
   const bool used_only = params.find("usedonly") != params.end();
   absl::optional<std::regex> regex;
   if (!Utility::filterParam(params, response, regex)) {

test/common/http/utility_fuzz_test.cc

@@ -57,7 +57,8 @@ DEFINE_PROTO_FUZZER(const test::common::http::UtilityTestCase& input) {
   }
   case test::common::http::UtilityTestCase::kParseParameters: {
     const auto& parse_parameters = input.parse_parameters();
-    Http::Utility::parseParameters(parse_parameters.data(), parse_parameters.start());
+    Http::Utility::parseParameters(parse_parameters.data(), parse_parameters.start(),
+                                   /*decode_param_value*/ true);
     break;
   }
   case test::common::http::UtilityTestCase::kFindQueryString: {

test/common/http/utility_test.cc

@@ -27,17 +27,66 @@ namespace Envoy {
 namespace Http {
 
 TEST(HttpUtility, parseQueryString) {
+  bool decode_param_value = true;
+
   EXPECT_EQ(Utility::QueryParams(), Utility::parseQueryString("/hello"));
+  EXPECT_EQ(Utility::QueryParams(), Utility::parseQueryString("/hello", !decode_param_value));
+
   EXPECT_EQ(Utility::QueryParams(), Utility::parseQueryString("/hello?"));
+  EXPECT_EQ(Utility::QueryParams(), Utility::parseQueryString("/hello?", !decode_param_value));
+
   EXPECT_EQ(Utility::QueryParams({{"hello", ""}}), Utility::parseQueryString("/hello?hello"));
+  EXPECT_EQ(Utility::QueryParams({{"hello", ""}}),
+            Utility::parseQueryString("/hello?hello", !decode_param_value));
+
   EXPECT_EQ(Utility::QueryParams({{"hello", "world"}}),
             Utility::parseQueryString("/hello?hello=world"));
+  EXPECT_EQ(Utility::QueryParams({{"hello", "world"}}),
+            Utility::parseQueryString("/hello?hello=world", !decode_param_value));
+
   EXPECT_EQ(Utility::QueryParams({{"hello", ""}}), Utility::parseQueryString("/hello?hello="));
+  EXPECT_EQ(Utility::QueryParams({{"hello", ""}}),
+            Utility::parseQueryString("/hello?hello=", !decode_param_value));
+
   EXPECT_EQ(Utility::QueryParams({{"hello", ""}}), Utility::parseQueryString("/hello?hello=&"));
+  EXPECT_EQ(Utility::QueryParams({{"hello", ""}}),
+            Utility::parseQueryString("/hello?hello=&", !decode_param_value));
+
   EXPECT_EQ(Utility::QueryParams({{"hello", ""}, {"hello2", "world2"}}),
             Utility::parseQueryString("/hello?hello=&hello2=world2"));
+  EXPECT_EQ(Utility::QueryParams({{"hello", ""}, {"hello2", "world2"}}),
+            Utility::parseQueryString("/hello?hello=&hello2=world2", !decode_param_value));
+
   EXPECT_EQ(Utility::QueryParams({{"name", "admin"}, {"level", "trace"}}),
             Utility::parseQueryString("/logging?name=admin&level=trace"));
+  EXPECT_EQ(Utility::QueryParams({{"name", "admin"}, {"level", "trace"}}),
+            Utility::parseQueryString("/logging?name=admin&level=trace", !decode_param_value));
+
+  EXPECT_EQ(Utility::QueryParams({{"param_value_has_encoded_ampersand", "a&b"}}),
+            Utility::parseQueryString("/hello?param_value_has_encoded_ampersand=a%26b"));
+  EXPECT_EQ(Utility::QueryParams({{"param_value_has_encoded_ampersand", "a%26b"}}),
+            Utility::parseQueryString("/hello?param_value_has_encoded_ampersand=a%26b",
+                                      !decode_param_value));
+
+  // A sample of an encoded query string of a request by prometheus:
+  // https://github.com/envoyproxy/envoy/issues/10926#issuecomment-651085261.
+  EXPECT_EQ(
+      Utility::QueryParams(
+          {{"filter", "(cluster.upstream_(rq_total|rq_time_sum|rq_time_count|rq_time_bucket|rq_xx|"
+                      "rq_complete|rq_active|cx_active))|(server.version)"}}),
+      Utility::parseQueryString(
+          "/stats?filter=%28cluster.upstream_%28rq_total%7Crq_time_sum%7Crq_time_count%7Crq_time_"
+          "bucket%7Crq_xx%7Crq_complete%7Crq_active%7Ccx_active%29%29%7C%28server.version%29",
+          decode_param_value));
+  EXPECT_EQ(
+      Utility::QueryParams(
+          {{"filter",
+            "%28cluster.upstream_%28rq_total%7Crq_time_sum%7Crq_time_count%7Crq_time_"
+            "bucket%7Crq_xx%7Crq_complete%7Crq_active%7Ccx_active%29%29%7C%28server.version%29"}}),
+      Utility::parseQueryString(
+          "/stats?filter=%28cluster.upstream_%28rq_total%7Crq_time_sum%7Crq_time_count%7Crq_time_"
+          "bucket%7Crq_xx%7Crq_complete%7Crq_active%7Ccx_active%29%29%7C%28server.version%29",
+          !decode_param_value));
 }
 
 TEST(HttpUtility, getResponseStatus) {
@@ -1188,7 +1237,19 @@ TEST(PercentEncoding, EncodeDecode) {
   validatePercentEncodingEncodeDecode("_-ok-_", "_-ok-_");
 }
 
-TEST(PercentEncoding, Trailing) {
+TEST(PercentEncoding, Decoding) {
+  EXPECT_EQ(Utility::PercentEncoding::decode("a%26b"), "a&b");
+  EXPECT_EQ(Utility::PercentEncoding::decode("hello%20world"), "hello world");
+  EXPECT_EQ(Utility::PercentEncoding::decode("upstream%7Cdownstream"), "upstream|downstream");
+  EXPECT_EQ(
+      Utility::PercentEncoding::decode(
+          "filter=%28cluster.upstream_%28rq_total%7Crq_time_sum%7Crq_time_count%7Crq_time_bucket%"
+          "7Crq_xx%7Crq_complete%7Crq_active%7Ccx_active%29%29%7C%28server.version%29"),
+      "filter=(cluster.upstream_(rq_total|rq_time_sum|rq_time_count|rq_time_bucket|rq_xx|rq_"
+      "complete|rq_active|cx_active))|(server.version)");
+}
+
+TEST(PercentEncoding, DecodingWithTrailingInput) {
   EXPECT_EQ(Utility::PercentEncoding::decode("too%20lar%20"), "too lar ");
   EXPECT_EQ(Utility::PercentEncoding::decode("too%20larg%e"), "too larg%e");
   EXPECT_EQ(Utility::PercentEncoding::decode("too%20large%"), "too large%");