From 260919eb862896299f99f640136c468b1b3cbb5c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 18 May 2026 07:08:45 +0000 Subject: [PATCH 1/3] Initial plan From f9a7759fb43c9583f9dd5b6149210c3e4d67e139 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 18 May 2026 07:12:34 +0000 Subject: [PATCH 2/3] Fix HttpService route precedence over wildcard catch-all Agent-Logs-Url: https://github.com/ithewei/libhv/sessions/1d649b64-5e89-4b6a-9049-5853eec30954 Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com> --- http/server/HttpService.cpp | 134 ++++++++++++++++++--------- unittest/CMakeLists.txt | 5 + unittest/http_service_route_test.cpp | 43 +++++++++ 3 files changed, 140 insertions(+), 42 deletions(-) create mode 100644 unittest/http_service_route_test.cpp diff --git a/http/server/HttpService.cpp b/http/server/HttpService.cpp index acab0fa08..9630e381d 100644 --- a/http/server/HttpService.cpp +++ b/http/server/HttpService.cpp @@ -2,9 +2,50 @@ #include "HttpMiddleware.h" #include "hbase.h" // import hv_strendswith +#include +#include namespace hv { +static bool is_restful_route(const std::string& path) { + return path.find("/:") != std::string::npos || + path.find("/{") != std::string::npos; +} + +static bool match_route_path(const std::string& key_path, const std::string& path, std::map& params) { + const char *kp = key_path.c_str(), *ks, *vp = path.c_str(), *vs; + bool match = false; + while (*kp && *vp) { + if (kp[0] == '*') { + // wildcard * + match = hv_strendswith(vp, kp+1); + break; + } else if (*kp != *vp) { + match = false; + break; + } else if (kp[0] == '/' && (kp[1] == ':' || kp[1] == '{')) { + // RESTful /:field/ + // RESTful /{field}/ + kp += 2; + ks = kp; + while (*kp && *kp != '/') {++kp;} + vp += 1; + vs = vp; + while (*vp && *vp != '/') {++vp;} + int klen = kp - ks; + if (*(ks-1) == '{' && *(kp-1) == '}') { + --klen; + } + params[std::string(ks, klen)] = std::string(vs, vp-vs); + continue; + } else { + ++kp; + ++vp; + } + } + return match ? match : (*kp == '\0' && *vp == '\0'); +} + void HttpService::AddRoute(const char* path, http_method method, const http_handler& handler) { std::shared_ptr method_handlers = NULL; auto iter = pathHandlers.find(path); @@ -67,64 +108,73 @@ int HttpService::GetRoute(HttpRequest* req, http_handler** handler) { while (*e && *e != '?') ++e; std::string path = std::string(s, e); - const char *kp, *ks, *vp, *vs; - bool match; - for (auto iter = pathHandlers.begin(); iter != pathHandlers.end(); ++iter) { - kp = iter->first.c_str(); - vp = path.c_str(); - match = false; - std::map params; - - while (*kp && *vp) { - if (kp[0] == '*') { - // wildcard * - match = hv_strendswith(vp, kp+1); - break; - } else if (*kp != *vp) { - match = false; - break; - } else if (kp[0] == '/' && (kp[1] == ':' || kp[1] == '{')) { - // RESTful /:field/ - // RESTful /{field}/ - kp += 2; - ks = kp; - while (*kp && *kp != '/') {++kp;} - vp += 1; - vs = vp; - while (*vp && *vp != '/') {++vp;} - int klen = kp - ks; - if (*(ks-1) == '{' && *(kp-1) == '}') { - --klen; - } - params[std::string(ks, klen)] = std::string(vs, vp-vs); - continue; - } else { - ++kp; - ++vp; + auto iter = pathHandlers.find(path); + if (iter != pathHandlers.end()) { + auto method_handlers = iter->second; + for (auto mh = method_handlers->begin(); mh != method_handlers->end(); ++mh) { + if (mh->method == req->method) { + if (handler) *handler = &mh->handler; + return 0; } } + if (handler) *handler = NULL; + return HTTP_STATUS_METHOD_NOT_ALLOWED; + } - match = match ? match : (*kp == '\0' && *vp == '\0'); + std::vector restful_iters; + std::vector wildcard_iters; + restful_iters.reserve(pathHandlers.size()); + wildcard_iters.reserve(pathHandlers.size()); + for (auto it = pathHandlers.begin(); it != pathHandlers.end(); ++it) { + if (it->first == path) continue; + if (it->first.find('*') != std::string::npos) { + wildcard_iters.push_back(it); + } else if (is_restful_route(it->first)) { + restful_iters.push_back(it); + } + } + auto route_compare = [](const http_path_handlers::const_iterator& lhs, const http_path_handlers::const_iterator& rhs) { + if (lhs->first.size() != rhs->first.size()) { + return lhs->first.size() > rhs->first.size(); + } + return lhs->first < rhs->first; + }; + std::sort(restful_iters.begin(), restful_iters.end(), route_compare); + std::sort(wildcard_iters.begin(), wildcard_iters.end(), route_compare); + for (auto route_iter : restful_iters) { + std::map params; + bool match = match_route_path(route_iter->first, path, params); if (match) { - auto method_handlers = iter->second; - for (auto iter = method_handlers->begin(); iter != method_handlers->end(); ++iter) { - if (iter->method == req->method) { + auto method_handlers = route_iter->second; + for (auto mh = method_handlers->begin(); mh != method_handlers->end(); ++mh) { + if (mh->method == req->method) { for (auto& param : params) { // RESTful /:field/ => req->query_params[field] req->query_params[param.first] = param.second; } - if (handler) *handler = &iter->handler; + if (handler) *handler = &mh->handler; return 0; } } + } + } - if (params.size() == 0) { - if (handler) *handler = NULL; - return HTTP_STATUS_METHOD_NOT_ALLOWED; + for (auto route_iter : wildcard_iters) { + std::map params; + bool match = match_route_path(route_iter->first, path, params); + if (!match) continue; + auto method_handlers = route_iter->second; + for (auto mh = method_handlers->begin(); mh != method_handlers->end(); ++mh) { + if (mh->method == req->method) { + if (handler) *handler = &mh->handler; + return 0; } } + if (handler) *handler = NULL; + return HTTP_STATUS_METHOD_NOT_ALLOWED; } + if (handler) *handler = NULL; return HTTP_STATUS_NOT_FOUND; } diff --git a/unittest/CMakeLists.txt b/unittest/CMakeLists.txt index 2eec34790..b25f56fbe 100644 --- a/unittest/CMakeLists.txt +++ b/unittest/CMakeLists.txt @@ -72,6 +72,10 @@ add_executable(objectpool_test objectpool_test.cpp) target_include_directories(objectpool_test PRIVATE .. ../base ../cpputil) target_link_libraries(objectpool_test -lpthread) +add_executable(http_service_route_test http_service_route_test.cpp) +target_include_directories(http_service_route_test PRIVATE ..) +target_link_libraries(http_service_route_test hv_static) + # ------protocol------ add_executable(nslookup nslookup_test.c ../protocol/dns.c ../base/hsocket.c ../base/htime.c) target_include_directories(nslookup PRIVATE .. ../base ../protocol) @@ -111,6 +115,7 @@ add_custom_target(unittest DEPENDS synchronized_test threadpool_test objectpool_test + http_service_route_test nslookup ping ftp diff --git a/unittest/http_service_route_test.cpp b/unittest/http_service_route_test.cpp new file mode 100644 index 000000000..56e94c04e --- /dev/null +++ b/unittest/http_service_route_test.cpp @@ -0,0 +1,43 @@ +#include + +#include "hv/HttpService.h" + +using namespace hv; + +static int specific_handler(HttpRequest* req, HttpResponse* resp) { + (void)req; + (void)resp; + return 200; +} + +static int fallback_handler(HttpRequest* req, HttpResponse* resp) { + (void)req; + (void)resp; + return 418; +} + +int main() { + HttpService service; + service.Any("*", fallback_handler); + service.GET("/ping", specific_handler); + + http_handler* handler = NULL; + HttpResponse resp; + HttpRequest req; + + req.path = "/ping"; + req.method = HTTP_GET; + if (service.GetRoute(&req, &handler) != 0 || handler == NULL || handler->sync_handler(&req, &resp) != 200) { + fprintf(stderr, "specific route should win over '*' catch-all\n"); + return -1; + } + + req.path = "/not-found"; + req.method = HTTP_GET; + if (service.GetRoute(&req, &handler) != 0 || handler == NULL || handler->sync_handler(&req, &resp) != 418) { + fprintf(stderr, "catch-all should match unknown path\n"); + return -2; + } + + return 0; +} From 0d88b524f9c2b6ba23542f32329df20886fb6881 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 18 May 2026 07:15:44 +0000 Subject: [PATCH 3/3] Address review nits in route candidate iteration Agent-Logs-Url: https://github.com/ithewei/libhv/sessions/1d649b64-5e89-4b6a-9049-5853eec30954 Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com> --- http/server/HttpService.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/http/server/HttpService.cpp b/http/server/HttpService.cpp index 9630e381d..7a86781c7 100644 --- a/http/server/HttpService.cpp +++ b/http/server/HttpService.cpp @@ -123,10 +123,7 @@ int HttpService::GetRoute(HttpRequest* req, http_handler** handler) { std::vector restful_iters; std::vector wildcard_iters; - restful_iters.reserve(pathHandlers.size()); - wildcard_iters.reserve(pathHandlers.size()); for (auto it = pathHandlers.begin(); it != pathHandlers.end(); ++it) { - if (it->first == path) continue; if (it->first.find('*') != std::string::npos) { wildcard_iters.push_back(it); } else if (is_restful_route(it->first)) {