diff --git a/lib/malloy/core/http/utils.hpp b/lib/malloy/core/http/utils.hpp index 79a5472..d92a332 100644 --- a/lib/malloy/core/http/utils.hpp +++ b/lib/malloy/core/http/utils.hpp @@ -73,26 +73,34 @@ namespace malloy::http * @param cookie_name The cookie name. * @return The cookie value (if any). */ + // ToDo: This implementation could use some love. It's comparably inefficient as we're splitting first on each + // cookie value separation ("; ") and then on each value-pair separator ("="). A more elegant implementation + // would just continuously advance through the string. template [[nodiscard]] std::optional cookie_value(const boost::beast::http::header& header, const std::string_view cookie_name) { - const auto& [begin, end] = header.equal_range(field::cookie); - for (auto it = begin; it != end; it++) { - const auto& str = it->value(); + using namespace std::string_view_literals; - const auto& sep_pos = it->value().find('='); - if (sep_pos == std::string::npos) - continue; + // Get cookie field + const auto& it = header.find(field::cookie); + if (it == header.cend()) + return std::nullopt; - const std::string_view key{ str.substr(0, sep_pos) }; - const std::string_view value{ str.substr(sep_pos+1) }; + // Split pairs + const auto& pairs = split_header_value(it->value()); - if (key != cookie_name) + // Check each pair + for (const std::string_view& pair : pairs) { + // Split + const auto& parts = malloy::split(pair, "="sv); + if (parts.size() != 2) continue; - return value; + // Check cookie name + if (parts[0] == cookie_name) + return parts[1]; } return std::nullopt; diff --git a/test/test_suites/components/utils_core_http.cpp b/test/test_suites/components/utils_core_http.cpp index 8db5529..b99c9bb 100644 --- a/test/test_suites/components/utils_core_http.cpp +++ b/test/test_suites/components/utils_core_http.cpp @@ -39,3 +39,70 @@ TEST_SUITE("components - utils - core - http") } } } + +TEST_SUITE("components - core - utils - http - cookies") +{ + + TEST_CASE("cookie_value()") + { + using namespace std::string_view_literals; + + // ToDo: Don't use req.insert() - Use malloy provided infrastructure to set cookies instead. + + malloy::http::request req; + + SUBCASE("none") + { + CHECK_EQ(cookie_value(req, "foo"), std::nullopt); + } + + SUBCASE("one") + { + std::string_view cookie_str = "foo=bar"sv; + + SUBCASE("exists") + { + req.insert(malloy::http::field::cookie, cookie_str); + auto o = cookie_value(req, "foo"); + REQUIRE(o.has_value()); + CHECK_EQ(*o, "bar"); + } + + SUBCASE("non-exist") + { + auto o = cookie_value(req, "nope"); + CHECK_FALSE(o.has_value()); + + req.insert(malloy::http::field::cookie, cookie_str); + + o = cookie_value(req, "nope"); + CHECK_FALSE(o.has_value()); + } + } + + SUBCASE("multiple") + { + std::string_view cookie_str = "foo=bar; one=two; something=else"; + + SUBCASE("exists") + { + req.insert(malloy::http::field::cookie, cookie_str); + auto o = cookie_value(req, "foo"); + REQUIRE(o.has_value()); + CHECK_EQ(*o, "bar"); + } + + SUBCASE("non-exist") + { + auto o = cookie_value(req, "nope"); + CHECK_FALSE(o.has_value()); + + req.insert(malloy::http::field::cookie, cookie_str); + + o = cookie_value(req, "nope"); + CHECK_FALSE(o.has_value()); + } + } + } + +}