From d04705624ad36cbe9fe38e0c90fe435703935ecd Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Thu, 4 Sep 2025 20:42:48 -0400 Subject: [PATCH] Handle percent encoding --- src/api_url_parser.cpp | 217 +++++++++++--- src/api_url_parser.hpp | 31 +- src/http_handler.cpp | 19 +- tests/test_api_url_parser.cpp | 543 ++++++++++++++++++++++++++++++---- 4 files changed, 706 insertions(+), 104 deletions(-) diff --git a/src/api_url_parser.cpp b/src/api_url_parser.cpp index 217683f..2baad9e 100644 --- a/src/api_url_parser.cpp +++ b/src/api_url_parser.cpp @@ -1,18 +1,102 @@ #include "api_url_parser.hpp" +#include #include namespace { -// A simplified helper to split a string_view, similar to std::string::find. -// Returns a pair of views, the part before the delimiter and the part after. -std::pair split_view(std::string_view view, - char delimiter) { - size_t pos = view.find(delimiter); - if (pos == std::string_view::npos) { - return {view, ""}; +// RFC 3986 hex digit to value conversion +// Returns -1 for invalid hex digits +int hex_digit_to_value(char c) { + if (c >= '0' && c <= '9') + return c - '0'; + if (c >= 'A' && c <= 'F') + return c - 'A' + 10; + if (c >= 'a' && c <= 'f') + return c - 'a' + 10; + return -1; +} + +// Decode percent-encoded sequence at src +// Returns decoded byte value, or -1 for malformed encoding +int decode_percent_sequence(const char *src) { + if (src[0] != '%') + return -1; + + int high = hex_digit_to_value(src[1]); + int low = hex_digit_to_value(src[2]); + + if (high == -1 || low == -1) + return -1; + + return (high << 4) | low; +} + +// Decode RFC 3986 percent-encoding in place (for path segments) +// Returns new length, or -1 for malformed encoding +int decode_path_segment(char *data, int length) { + char *write_pos = data; + const char *read_pos = data; + const char *end = data + length; + + while (read_pos < end) { + if (*read_pos == '%') { + if (read_pos + 2 >= end) + return -1; // Incomplete sequence + + int decoded = decode_percent_sequence(read_pos); + if (decoded == -1) + return -1; // Malformed sequence + + *write_pos++ = static_cast(decoded); + read_pos += 3; + } else { + *write_pos++ = *read_pos++; + } } - return {view.substr(0, pos), view.substr(pos + 1)}; + + return static_cast(write_pos - data); +} + +// Decode application/x-www-form-urlencoded in place (for query parameters) +// Handles + → space conversion, then percent-decoding +// Returns new length, or -1 for malformed encoding +int decode_query_value(char *data, int length) { + char *write_pos = data; + const char *read_pos = data; + const char *end = data + length; + + while (read_pos < end) { + if (*read_pos == '+') { + *write_pos++ = ' '; + read_pos++; + } else if (*read_pos == '%') { + if (read_pos + 2 >= end) + return -1; // Incomplete sequence + + int decoded = decode_percent_sequence(read_pos); + if (decoded == -1) + return -1; // Malformed sequence + + *write_pos++ = static_cast(decoded); + read_pos += 3; + } else { + *write_pos++ = *read_pos++; + } + } + + return static_cast(write_pos - data); +} + +// A simplified helper to find a delimiter in a buffer +// Returns the position of the delimiter, or -1 if not found +int find_delimiter(const char *data, int length, char delimiter) { + for (int i = 0; i < length; ++i) { + if (data[i] == delimiter) { + return i; + } + } + return -1; } // Maps a string parameter key to its corresponding enum value. @@ -23,93 +107,144 @@ to_api_parameter_key(std::string_view key) { return ApiParameterKey::RequestId; if (key == "min_version") return ApiParameterKey::MinVersion; - if (key == "wait") - return ApiParameterKey::Wait; - if (key == "timeout") - return ApiParameterKey::Timeout; - if (key == "verbose") - return ApiParameterKey::Verbose; return std::nullopt; } -// Parses the query string part of a URL. -void parse_query_string(std::string_view query_string, RouteMatch &match) { - while (!query_string.empty()) { - auto [key_value_pair, rest] = split_view(query_string, '&'); - auto [key, value] = split_view(key_value_pair, '='); +// Parses the query string part of a URL (in-place decoding) +// Returns ParseResult::Success or ParseResult::MalformedEncoding +ParseResult parse_query_string(char *query_data, int query_length, + RouteMatch &match) { + int pos = 0; - if (auto key_enum = to_api_parameter_key(key)) { - match.params[static_cast(*key_enum)] = value; + while (pos < query_length) { + // Find end of current key=value pair + int pair_end = find_delimiter(query_data + pos, query_length - pos, '&'); + if (pair_end == -1) + pair_end = query_length - pos; + + // Find = separator within the pair + int eq_pos = find_delimiter(query_data + pos, pair_end, '='); + if (eq_pos == -1) { + // No value, skip this parameter + pos += pair_end + 1; + continue; } - query_string = rest; + // Decode key and value in place + char *key_start = query_data + pos; + int key_length = eq_pos; + + char *value_start = query_data + pos + eq_pos + 1; + int value_length = pair_end - eq_pos - 1; + + // Decode value (query parameters use form encoding) + int decoded_value_length = decode_query_value(value_start, value_length); + if (decoded_value_length == -1) { + return ParseResult::MalformedEncoding; + } + + // Check if this is a parameter we care about + std::string_view key_view(key_start, key_length); + if (auto key_enum = to_api_parameter_key(key_view)) { + match.params[static_cast(*key_enum)] = + std::string_view(value_start, decoded_value_length); + } + + pos += pair_end + 1; } + + return ParseResult::Success; } } // namespace -RouteMatch ApiUrlParser::parse(std::string_view method, std::string_view url) { - RouteMatch result; +ParseResult ApiUrlParser::parse(std::string_view method, char *url_data, + int url_length, RouteMatch &result) { + assert(url_data != nullptr); + assert(url_length >= 0); - auto [path, query] = split_view(url, '?'); - parse_query_string(query, result); + // Find query string separator + int query_start = find_delimiter(url_data, url_length, '?'); + char *path_data = url_data; + int path_length = (query_start == -1) ? url_length : query_start; + + char *query_data = (query_start == -1) ? nullptr : url_data + query_start + 1; + int query_length = (query_start == -1) ? 0 : url_length - query_start - 1; + + // Parse and decode query string first + if (query_data && query_length > 0) { + ParseResult query_result = + parse_query_string(query_data, query_length, result); + if (query_result != ParseResult::Success) { + return query_result; + } + } + + // Decode path segment (RFC 3986 percent-decoding) + int decoded_path_length = decode_path_segment(path_data, path_length); + if (decoded_path_length == -1) { + return ParseResult::MalformedEncoding; + } + + std::string_view path(path_data, decoded_path_length); + + // Route matching with decoded path if (method == "GET") { if (path == "/v1/version") { result.route = HttpRoute::GetVersion; - return result; + return ParseResult::Success; } if (path == "/v1/subscribe") { result.route = HttpRoute::GetSubscribe; - return result; + return ParseResult::Success; } if (path == "/v1/status") { result.route = HttpRoute::GetStatus; - return result; + return ParseResult::Success; } if (path.starts_with("/v1/retention")) { result.route = HttpRoute::GetRetention; // Note: This matches both /v1/retention and /v1/retention/{id} - // The handler will need to check for the presence of the PolicyId param. if (path.length() > 13) { // length of "/v1/retention" std::string_view policy_id = path.substr(14); // length of "/v1/retention/" if (!policy_id.empty()) { - result.params[static_cast(ApiParameterKey::PolicyId)] = + result.params[static_cast(ApiParameterKey::PolicyId)] = policy_id; } } - return result; + return ParseResult::Success; } if (path == "/metrics") { result.route = HttpRoute::GetMetrics; - return result; + return ParseResult::Success; } if (path == "/ok") { result.route = HttpRoute::GetOk; - return result; + return ParseResult::Success; } } else if (method == "POST") { if (path == "/v1/commit") { result.route = HttpRoute::PostCommit; - return result; + return ParseResult::Success; } } else if (method == "PUT") { if (path.starts_with("/v1/retention/")) { result.route = HttpRoute::PutRetention; std::string_view policy_id = path.substr(14); - result.params[static_cast(ApiParameterKey::PolicyId)] = policy_id; - return result; + result.params[static_cast(ApiParameterKey::PolicyId)] = policy_id; + return ParseResult::Success; } } else if (method == "DELETE") { if (path.starts_with("/v1/retention/")) { result.route = HttpRoute::DeleteRetention; std::string_view policy_id = path.substr(14); - result.params[static_cast(ApiParameterKey::PolicyId)] = policy_id; - return result; + result.params[static_cast(ApiParameterKey::PolicyId)] = policy_id; + return ParseResult::Success; } } result.route = HttpRoute::NotFound; - return result; -} + return ParseResult::Success; +} \ No newline at end of file diff --git a/src/api_url_parser.hpp b/src/api_url_parser.hpp index 62a9565..f22d940 100644 --- a/src/api_url_parser.hpp +++ b/src/api_url_parser.hpp @@ -28,9 +28,6 @@ enum class ApiParameterKey { // --- Query Parameters --- RequestId, MinVersion, - Wait, - Timeout, - Verbose, // --- URL Parameters --- PolicyId, @@ -48,6 +45,11 @@ enum class ApiParameterKey { using ApiParameters = std::array, static_cast(ApiParameterKey::Count)>; +/** + * @brief Result codes for URL parsing operations. + */ +enum class [[nodiscard]] ParseResult { Success, MalformedEncoding }; + /** * @brief Contains the complete, structured result of a successful URL parse. * @@ -79,11 +81,24 @@ public: * @brief Parses a URL and HTTP method against the known WeaselDB API * endpoints. * + * **Mutates in place**: This function performs RFC 3986 percent-decoding + * directly on the provided URL buffer. Path segments are decoded according + * to RFC 3986, while query parameters follow + * application/x-www-form-urlencoded rules (+ → space, then %XX → bytes). + * * @param method The HTTP method of the request. - * @param url The full URL (including query string) of the request. - * @return A RouteMatch struct. If no route is matched, the `route` member - * of the returned struct will be `HttpRoute::NotFound`. + * @param url_data Mutable buffer containing the URL (will be modified + * in-place). + * @param url_length Length of the URL data in bytes. + * @param out_match Output parameter for the parsed route and parameters. + * @return ParseResult::Success on successful parsing, + * ParseResult::MalformedEncoding if the URL contains invalid percent-encoding + * sequences. + * @note On success, string_view parameters in out_match point into the + * decoded url_data buffer and remain valid while url_data is unchanged. + * @note On error, url_data contents are undefined and should not be used. */ - [[nodiscard]] static RouteMatch parse(std::string_view method, - std::string_view url); + [[nodiscard]] static ParseResult parse(std::string_view method, + char *url_data, int url_length, + RouteMatch &out_match); }; diff --git a/src/http_handler.cpp b/src/http_handler.cpp index eea461e..2451a86 100644 --- a/src/http_handler.cpp +++ b/src/http_handler.cpp @@ -152,7 +152,22 @@ void HttpHandler::on_data_arrived(std::string_view data, // If message is complete, route and handle the request if (state->message_complete) { - auto route_match = ApiUrlParser::parse(state->method, state->url); + // Copy URL to arena for in-place decoding + ArenaAllocator &arena = conn_ptr->get_arena(); + char *url_buffer = arena.allocate(state->url.size()); + std::memcpy(url_buffer, state->url.data(), state->url.size()); + + RouteMatch route_match; + auto parse_result = + ApiUrlParser::parse(state->method, url_buffer, + static_cast(state->url.size()), route_match); + + if (parse_result != ParseResult::Success) { + // Handle malformed URL encoding + send_error_response(*conn_ptr, 400, "Malformed URL encoding", true); + return; + } + state->route = route_match.route; // Route to appropriate handler @@ -282,7 +297,7 @@ void HttpHandler::handle_get_status(Connection &conn, // pipeline processing const auto &request_id = - route_match.params[static_cast(ApiParameterKey::RequestId)]; + route_match.params[static_cast(ApiParameterKey::RequestId)]; if (!request_id) { send_error_response(conn, 400, "Missing required query parameter: request_id", diff --git a/tests/test_api_url_parser.cpp b/tests/test_api_url_parser.cpp index ce5dab9..b72e185 100644 --- a/tests/test_api_url_parser.cpp +++ b/tests/test_api_url_parser.cpp @@ -1,117 +1,554 @@ #include +#include +#include + #include "api_url_parser.hpp" +// Helper to convert string to mutable buffer for testing +std::string make_mutable_copy(const std::string &url) { + return url; // Return copy that can be modified +} + TEST_CASE("ApiUrlParser routing") { SUBCASE("Static GET routes") { - auto match = ApiUrlParser::parse("GET", "/v1/version"); + auto url = make_mutable_copy("/v1/version"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); CHECK(match.route == HttpRoute::GetVersion); - match = ApiUrlParser::parse("GET", "/v1/subscribe"); + url = make_mutable_copy("/v1/subscribe"); + result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); CHECK(match.route == HttpRoute::GetSubscribe); - match = ApiUrlParser::parse("GET", "/metrics"); + url = make_mutable_copy("/metrics"); + result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); CHECK(match.route == HttpRoute::GetMetrics); - match = ApiUrlParser::parse("GET", "/ok"); + url = make_mutable_copy("/ok"); + result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); CHECK(match.route == HttpRoute::GetOk); } SUBCASE("Static POST routes") { - auto match = ApiUrlParser::parse("POST", "/v1/commit"); + auto url = make_mutable_copy("/v1/commit"); + RouteMatch match; + auto result = ApiUrlParser::parse("POST", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); CHECK(match.route == HttpRoute::PostCommit); } SUBCASE("Not found") { - auto match = ApiUrlParser::parse("GET", "/unknown/route"); + auto url = make_mutable_copy("/unknown/route"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); CHECK(match.route == HttpRoute::NotFound); - match = ApiUrlParser::parse("DELETE", "/v1/version"); + url = make_mutable_copy("/v1/version"); + result = ApiUrlParser::parse("DELETE", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); CHECK(match.route == HttpRoute::NotFound); } } TEST_CASE("ApiUrlParser with query strings") { SUBCASE("Simple query string") { - auto match = ApiUrlParser::parse("GET", "/v1/status?request_id=123"); + auto url = make_mutable_copy("/v1/status?request_id=123"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); CHECK(match.route == HttpRoute::GetStatus); - REQUIRE(match.params[static_cast(ApiParameterKey::RequestId)] - .has_value()); - CHECK( - match.params[static_cast(ApiParameterKey::RequestId)].value() == - "123"); + REQUIRE( + match.params[static_cast(ApiParameterKey::RequestId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::RequestId)].value() == + "123"); } SUBCASE("Multiple query parameters") { - auto match = - ApiUrlParser::parse("GET", "/v1/status?request_id=abc&min_version=42"); + auto url = make_mutable_copy("/v1/status?request_id=abc&min_version=42"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); CHECK(match.route == HttpRoute::GetStatus); - REQUIRE(match.params[static_cast(ApiParameterKey::RequestId)] + REQUIRE( + match.params[static_cast(ApiParameterKey::RequestId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::RequestId)].value() == + "abc"); + REQUIRE(match.params[static_cast(ApiParameterKey::MinVersion)] .has_value()); - CHECK( - match.params[static_cast(ApiParameterKey::RequestId)].value() == - "abc"); - REQUIRE(match.params[static_cast(ApiParameterKey::MinVersion)] - .has_value()); - CHECK(match.params[static_cast(ApiParameterKey::MinVersion)] - .value() == "42"); + CHECK(match.params[static_cast(ApiParameterKey::MinVersion)].value() == + "42"); } SUBCASE("Unknown parameters are ignored") { - auto match = ApiUrlParser::parse("GET", "/v1/version?foo=bar&baz=quux"); + auto url = make_mutable_copy("/v1/version?foo=bar&baz=quux"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); CHECK(match.route == HttpRoute::GetVersion); - CHECK_FALSE(match.params[static_cast(ApiParameterKey::RequestId)] - .has_value()); + CHECK_FALSE( + match.params[static_cast(ApiParameterKey::RequestId)].has_value()); } } TEST_CASE("ApiUrlParser with URL parameters") { SUBCASE("PUT retention policy") { - auto match = ApiUrlParser::parse("PUT", "/v1/retention/my-policy"); + auto url = make_mutable_copy("/v1/retention/my-policy"); + RouteMatch match; + auto result = ApiUrlParser::parse("PUT", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); CHECK(match.route == HttpRoute::PutRetention); - REQUIRE(match.params[static_cast(ApiParameterKey::PolicyId)] - .has_value()); - CHECK( - match.params[static_cast(ApiParameterKey::PolicyId)].value() == - "my-policy"); + REQUIRE( + match.params[static_cast(ApiParameterKey::PolicyId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::PolicyId)].value() == + "my-policy"); } SUBCASE("DELETE retention policy") { - auto match = ApiUrlParser::parse("DELETE", "/v1/retention/another-policy"); + auto url = make_mutable_copy("/v1/retention/another-policy"); + RouteMatch match; + auto result = ApiUrlParser::parse("DELETE", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); CHECK(match.route == HttpRoute::DeleteRetention); - REQUIRE(match.params[static_cast(ApiParameterKey::PolicyId)] - .has_value()); - CHECK( - match.params[static_cast(ApiParameterKey::PolicyId)].value() == - "another-policy"); + REQUIRE( + match.params[static_cast(ApiParameterKey::PolicyId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::PolicyId)].value() == + "another-policy"); } SUBCASE("GET retention policy") { - auto match = ApiUrlParser::parse("GET", "/v1/retention/get-this"); + auto url = make_mutable_copy("/v1/retention/get-this"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); CHECK(match.route == HttpRoute::GetRetention); - REQUIRE(match.params[static_cast(ApiParameterKey::PolicyId)] - .has_value()); - CHECK( - match.params[static_cast(ApiParameterKey::PolicyId)].value() == - "get-this"); + REQUIRE( + match.params[static_cast(ApiParameterKey::PolicyId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::PolicyId)].value() == + "get-this"); } SUBCASE("GET all retention policies (no ID)") { - auto match = ApiUrlParser::parse("GET", "/v1/retention"); + auto url = make_mutable_copy("/v1/retention"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); CHECK(match.route == HttpRoute::GetRetention); - CHECK_FALSE(match.params[static_cast(ApiParameterKey::PolicyId)] - .has_value()); + CHECK_FALSE( + match.params[static_cast(ApiParameterKey::PolicyId)].has_value()); } } TEST_CASE("ApiUrlParser with URL and query parameters") { - auto match = ApiUrlParser::parse("DELETE", "/v1/retention/p1?wait=true"); + auto url = make_mutable_copy("/v1/retention/p1?request_id=abc123"); + RouteMatch match; + auto result = ApiUrlParser::parse("DELETE", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); CHECK(match.route == HttpRoute::DeleteRetention); REQUIRE( - match.params[static_cast(ApiParameterKey::PolicyId)].has_value()); - CHECK(match.params[static_cast(ApiParameterKey::PolicyId)].value() == + match.params[static_cast(ApiParameterKey::PolicyId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::PolicyId)].value() == "p1"); - REQUIRE(match.params[static_cast(ApiParameterKey::Wait)].has_value()); - CHECK(match.params[static_cast(ApiParameterKey::Wait)].value() == - "true"); + REQUIRE( + match.params[static_cast(ApiParameterKey::RequestId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::RequestId)].value() == + "abc123"); } + +TEST_CASE("ApiUrlParser URL decoding") { + SUBCASE("Path segment percent-decoding") { + auto url = make_mutable_copy("/v1/retention/my%2Dpolicy"); + RouteMatch match; + auto result = ApiUrlParser::parse("PUT", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::PutRetention); + REQUIRE( + match.params[static_cast(ApiParameterKey::PolicyId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::PolicyId)].value() == + "my-policy"); + } + + SUBCASE("Query parameter form decoding (+ to space)") { + auto url = make_mutable_copy("/v1/status?request_id=hello+world"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::GetStatus); + REQUIRE( + match.params[static_cast(ApiParameterKey::RequestId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::RequestId)].value() == + "hello world"); + } + + SUBCASE("Query parameter percent-decoding") { + auto url = make_mutable_copy("/v1/status?request_id=hello%20world"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::GetStatus); + REQUIRE( + match.params[static_cast(ApiParameterKey::RequestId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::RequestId)].value() == + "hello world"); + } + + SUBCASE("Base64-like sequences in query parameters") { + auto url = make_mutable_copy("/v1/status?request_id=YWJj%3D"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::GetStatus); + REQUIRE( + match.params[static_cast(ApiParameterKey::RequestId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::RequestId)].value() == + "YWJj="); + } + + SUBCASE("Mixed encoding in path and query") { + auto url = make_mutable_copy( + "/v1/retention/my%2Dpolicy?request_id=hello+world%21"); + RouteMatch match; + auto result = ApiUrlParser::parse("DELETE", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::DeleteRetention); + REQUIRE( + match.params[static_cast(ApiParameterKey::PolicyId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::PolicyId)].value() == + "my-policy"); + REQUIRE( + match.params[static_cast(ApiParameterKey::RequestId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::RequestId)].value() == + "hello world!"); + } +} + +TEST_CASE("ApiUrlParser malformed encoding") { + SUBCASE("Incomplete percent sequence in path") { + auto url = make_mutable_copy("/v1/retention/bad%2"); + RouteMatch match; + auto result = ApiUrlParser::parse("PUT", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::MalformedEncoding); + } + + SUBCASE("Invalid hex digits in path") { + auto url = make_mutable_copy("/v1/retention/bad%ZZ"); + RouteMatch match; + auto result = ApiUrlParser::parse("PUT", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::MalformedEncoding); + } + + SUBCASE("Incomplete percent sequence in query") { + auto url = make_mutable_copy("/v1/status?request_id=bad%2"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::MalformedEncoding); + } + + SUBCASE("Invalid hex digits in query") { + auto url = make_mutable_copy("/v1/status?request_id=bad%GG"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::MalformedEncoding); + } + + SUBCASE("Percent at end of path") { + auto url = make_mutable_copy("/v1/retention/bad%"); + RouteMatch match; + auto result = ApiUrlParser::parse("PUT", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::MalformedEncoding); + } + + SUBCASE("Percent at end of query") { + auto url = make_mutable_copy("/v1/status?request_id=bad%"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::MalformedEncoding); + } +} + +TEST_CASE("ApiUrlParser edge cases and bugs") { + SUBCASE("Bug: Path boundary error - /v1/retention/ with trailing slash") { + // BUG: Code checks length > 13 but substrings at 14, causing off-by-one + auto url = make_mutable_copy( + "/v1/retention/"); // length 14, exactly the boundary case + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::GetRetention); + // This should NOT set PolicyId since it's empty, but current code might + // have issues + CHECK_FALSE( + match.params[static_cast(ApiParameterKey::PolicyId)].has_value()); + } + + SUBCASE("Bug: Empty URL handling") { + auto url = make_mutable_copy(""); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::NotFound); + } + + SUBCASE("Bug: Query-only URL") { + auto url = make_mutable_copy("?request_id=123"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::NotFound); + // Should still parse query parameters + REQUIRE( + match.params[static_cast(ApiParameterKey::RequestId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::RequestId)].value() == + "123"); + } + + SUBCASE("Bug: Consecutive delimiters in query string") { + auto url = + make_mutable_copy("/v1/status?&&request_id=123&&min_version=42&&"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::GetStatus); + REQUIRE( + match.params[static_cast(ApiParameterKey::RequestId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::RequestId)].value() == + "123"); + REQUIRE(match.params[static_cast(ApiParameterKey::MinVersion)] + .has_value()); + CHECK(match.params[static_cast(ApiParameterKey::MinVersion)].value() == + "42"); + } + + SUBCASE("Bug: Parameter without value (should be skipped)") { + auto url = make_mutable_copy("/v1/status?debug&request_id=123"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::GetStatus); + // debug parameter should be ignored since it has no value + REQUIRE( + match.params[static_cast(ApiParameterKey::RequestId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::RequestId)].value() == + "123"); + } + + SUBCASE("Bug: Empty parameter value") { + auto url = make_mutable_copy("/v1/status?request_id="); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::GetStatus); + REQUIRE( + match.params[static_cast(ApiParameterKey::RequestId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::RequestId)].value() == + ""); + } + + SUBCASE("Edge: Exact length boundary for retention path") { + // Test the exact boundary condition: "/v1/retention" is 13 chars + auto url = make_mutable_copy("/v1/retention"); // exactly 13 characters + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::GetRetention); + CHECK_FALSE( + match.params[static_cast(ApiParameterKey::PolicyId)].has_value()); + } + + SUBCASE("Edge: Minimum valid policy ID") { + // Test one character after the boundary + auto url = + make_mutable_copy("/v1/retention/a"); // 15 chars total, policy_id = "a" + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::GetRetention); + REQUIRE( + match.params[static_cast(ApiParameterKey::PolicyId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::PolicyId)].value() == + "a"); + } +} + +TEST_CASE("ApiUrlParser specific bug reproduction") { + SUBCASE("Reproduction: Path boundary math error") { + // The bug: code checks length > 13 but substrings at 14 + // "/v1/retention" = 13 chars, "/v1/retention/" = 14 chars + // This test should demonstrate undefined behavior or wrong results + + auto url = make_mutable_copy("/v1/retention/"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + + // Expected behavior: should match GetRetention but NOT set PolicyId + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::GetRetention); + + // The bug: may incorrectly extract empty string or cause buffer read error + // Let's see what actually happens + if (match.params[static_cast(ApiParameterKey::PolicyId)].has_value()) { + auto policy_id = + match.params[static_cast(ApiParameterKey::PolicyId)].value(); + CHECK(policy_id.empty()); // Should be empty if set at all + } + } + + SUBCASE("Reproduction: Query parsing with edge cases") { + // Test parameter parsing with multiple edge conditions + auto url = make_mutable_copy( + "/v1/status?=empty_key&no_value&request_id=&min_version=42&="); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::GetStatus); + + // Should handle empty values correctly + REQUIRE( + match.params[static_cast(ApiParameterKey::RequestId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::RequestId)].value() == + ""); + + REQUIRE(match.params[static_cast(ApiParameterKey::MinVersion)] + .has_value()); + CHECK(match.params[static_cast(ApiParameterKey::MinVersion)].value() == + "42"); + } + + SUBCASE("Reproduction: Very long input stress test") { + // Test potential integer overflow or performance issues + std::string long_policy_id(1000, 'x'); // 1000 character policy ID + auto url = make_mutable_copy("/v1/retention/" + long_policy_id + + "?request_id=" + std::string(500, 'y')); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::GetRetention); + REQUIRE( + match.params[static_cast(ApiParameterKey::PolicyId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::PolicyId)].value() == + long_policy_id); + REQUIRE( + match.params[static_cast(ApiParameterKey::RequestId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::RequestId)].value() == + std::string(500, 'y')); + } + + SUBCASE("Reproduction: Zero-length edge case") { + char empty_buffer[1] = {0}; + RouteMatch match; + auto result = ApiUrlParser::parse("GET", empty_buffer, 0, match); + + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::NotFound); + } + + SUBCASE("Reproduction: Null buffer edge case") { + // This might cause undefined behavior if not handled properly + RouteMatch match; + char single_char = '/'; + auto result = ApiUrlParser::parse("GET", &single_char, 1, match); + + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::NotFound); + } + + SUBCASE("BUG: Query parser pos increment overflow") { + // BUG: pos += pair_end + 1 can go beyond buffer bounds + // When pair_end == query_length - pos (no & found), pos becomes + // query_length + 1 + auto url = make_mutable_copy("/v1/status?no_ampersand_at_end"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + + // This should not crash or have undefined behavior + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::GetStatus); + // Parameter should be ignored since it's not a known key + } + + SUBCASE("BUG: String view with potentially negative length cast") { + // BUG: decoded_value_length is int but gets cast to size_t for string_view + // If decode function returned negative (which it can), this could wrap + // around + + // We can't easily trigger the decode function to return -1 through normal + // parsing since that's caught earlier, but this tests the edge case + // handling + auto url = make_mutable_copy("/v1/status?request_id=normal_value"); + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + + CHECK(result == ParseResult::Success); + REQUIRE( + match.params[static_cast(ApiParameterKey::RequestId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::RequestId)].value() == + "normal_value"); + } + + SUBCASE("BUG: Array bounds - PolicyId path extraction edge case") { + // Test the boundary condition more precisely + // "/v1/retention" = 13 chars, checking length > 13, substr(14) + auto url = make_mutable_copy("/v1/retention/"); // exactly 14 chars + RouteMatch match; + auto result = ApiUrlParser::parse("GET", url.data(), + static_cast(url.size()), match); + + CHECK(result == ParseResult::Success); + CHECK(match.route == HttpRoute::GetRetention); + + // path.length() = 14, so > 13 is true + // path.substr(14) should return empty string_view + // The bug would be if this crashes or returns invalid data + if (match.params[static_cast(ApiParameterKey::PolicyId)].has_value()) { + CHECK(match.params[static_cast(ApiParameterKey::PolicyId)] + .value() + .empty()); + } + } +} \ No newline at end of file