From ba91e69c040f5ec95626c826485bd41447702a88 Mon Sep 17 00:00:00 2001 From: Kishore Nallan Date: Wed, 27 Oct 2021 21:20:31 +0530 Subject: [PATCH] Proper error in API response when GET query > 4K chars. --- include/string_utils.h | 2 +- src/http_server.cpp | 9 +++++- src/string_utils.cpp | 10 +----- test/string_utils_test.cpp | 66 ++++++++++++++++++++++++-------------- 4 files changed, 52 insertions(+), 35 deletions(-) diff --git a/include/string_utils.h b/include/string_utils.h index 3d386a68..728bcdab 100644 --- a/include/string_utils.h +++ b/include/string_utils.h @@ -309,7 +309,7 @@ struct StringUtils { return str.rfind(prefix, 0) == 0; } - static std::map parse_query_string(const std::string& query); + static void parse_query_string(const std::string& query, std::map& query_map); static std::string float_to_str(float value); diff --git a/src/http_server.cpp b/src/http_server.cpp index 31cead75..084696e0 100644 --- a/src/http_server.cpp +++ b/src/http_server.cpp @@ -326,8 +326,15 @@ int HttpServer::catch_all_handler(h2o_handler_t *_h2o_handler, h2o_req_t *req) { h2o_iovec_init(req->path.base + req->query_at, req->path.len - req->query_at) : h2o_iovec_init(H2O_STRLIT("")); + if(query.len > 4000) { + nlohmann::json resp; + resp["message"] = "Query string exceeds max allowed length of 4000. Use the /multi_search end-point for larger payloads."; + return send_response(req, 400, resp.dump()); + } + std::string query_str(query.base, query.len); - std::map query_map = StringUtils::parse_query_string(query_str); + std::map query_map; + StringUtils::parse_query_string(query_str, query_map); // Extract auth key from header. If that does not exist, look for a GET parameter. std::string api_auth_key_sent = ""; diff --git a/src/string_utils.cpp b/src/string_utils.cpp index 0811b3fc..0e7cad64 100644 --- a/src/string_utils.cpp +++ b/src/string_utils.cpp @@ -80,13 +80,7 @@ std::string StringUtils::hash_sha256(const std::string& str) { return StringUtils::str2hex(std::string(reinterpret_cast(hash_buf), SHA256_SIZE)); } -std::map StringUtils::parse_query_string(const std::string &query) { - if(query.size() > 4000) { - LOG(ERROR) << "Query string exceeds max allowed length of 4000. Actual length: " << query.size(); - return {}; - } - - std::map query_map; +void StringUtils::parse_query_string(const std::string& query, std::map& query_map) { std::string key_value; int query_len = int(query.size()); @@ -145,8 +139,6 @@ std::map StringUtils::parse_query_string(const std::st i++; } - - return query_map; } void StringUtils::split_to_values(const std::string& vals_str, std::vector& filter_values) { diff --git a/test/string_utils_test.cpp b/test/string_utils_test.cpp index 44422929..f6869df0 100644 --- a/test/string_utils_test.cpp +++ b/test/string_utils_test.cpp @@ -102,120 +102,138 @@ TEST(StringUtilsTest, ShouldComputeSHA256) { } TEST(StringUtilsTest, ShouldParseQueryString) { + std::map qmap; + std::string qs = "?q=bar&filter_by=points: >100 && points: <200"; - auto qmap = StringUtils::parse_query_string(qs); + + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(2, qmap.size()); ASSERT_EQ("bar", qmap["q"]); ASSERT_EQ("points: >100 && points: <200", qmap["filter_by"]); qs = "?q=bar&filter_by=points%3A%20%3E100%20%26%26%20points%3A%20%3C200"; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(2, qmap.size()); ASSERT_EQ("bar", qmap["q"]); ASSERT_EQ("points: >100 && points: <200", qmap["filter_by"]); qs = "?q=bar&filter_by=points%3A%20%3E100%20%26%26%20points%3A%20%3C200&"; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(2, qmap.size()); ASSERT_EQ("bar", qmap["q"]); ASSERT_EQ("points: >100 && points: <200", qmap["filter_by"]); qs = "q=bar&filter_by=baz&&"; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(2, qmap.size()); ASSERT_EQ("bar", qmap["q"]); ASSERT_EQ("baz&", qmap["filter_by"]); qs = "q=bar&filter_by="; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(2, qmap.size()); ASSERT_EQ("bar", qmap["q"]); ASSERT_EQ("", qmap["filter_by"]); qs = "q=bread && breakfast&filter_by="; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(2, qmap.size()); ASSERT_EQ("bread && breakfast", qmap["q"]); ASSERT_EQ("", qmap["filter_by"]); qs = "q=bread & breakfast&filter_by="; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(3, qmap.size()); ASSERT_EQ("bread ", qmap["q"]); ASSERT_EQ("", qmap[" breakfast"]); ASSERT_EQ("", qmap["filter_by"]); qs = "q=bar&filter_by=&"; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(2, qmap.size()); ASSERT_EQ("bar", qmap["q"]); ASSERT_EQ("", qmap["filter_by"]); qs = "q=bar&filter_by=points :> 100&enable_typos"; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(3, qmap.size()); ASSERT_EQ("bar", qmap["q"]); ASSERT_EQ("points :> 100", qmap["filter_by"]); ASSERT_EQ("", qmap["enable_typos"]); - qs = "foo=" + StringUtils::randstring(4000); - qmap = StringUtils::parse_query_string(qs); - ASSERT_EQ(0, qmap.size()); - qs = "foo=bar&baz=&bazinga=true"; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(3, qmap.size()); ASSERT_EQ("bar", qmap["foo"]); ASSERT_EQ("", qmap["baz"]); ASSERT_EQ("true", qmap["bazinga"]); qs = "foo=bar&bazinga=true&foo=buzz"; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(2, qmap.size()); ASSERT_EQ("buzz", qmap["foo"]); ASSERT_EQ("true", qmap["bazinga"]); qs = "filter_by=points:>100&bazinga=true&filter_by=points:<=200"; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(2, qmap.size()); ASSERT_EQ("points:>100&&points:<=200", qmap["filter_by"]); ASSERT_EQ("true", qmap["bazinga"]); qs = "filter_by=points:>100 && brand:= nike&bazinga=true&filter_by=points:<=200"; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(2, qmap.size()); ASSERT_EQ("points:>100 && brand:= nike&&points:<=200", qmap["filter_by"]); ASSERT_EQ("true", qmap["bazinga"]); qs = "foo"; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(1, qmap.size()); ASSERT_EQ("", qmap["foo"]); qs = "?foo="; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(1, qmap.size()); ASSERT_EQ("", qmap["foo"]); qs = "?foo"; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(1, qmap.size()); ASSERT_EQ("", qmap["foo"]); qs = "?"; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(0, qmap.size()); qs = ""; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(0, qmap.size()); qs = "&"; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(0, qmap.size()); qs = "&&"; - qmap = StringUtils::parse_query_string(qs); + qmap.clear(); + StringUtils::parse_query_string(qs, qmap); ASSERT_EQ(0, qmap.size()); }