Fix HttpService route precedence so `Any("*")` no longer shadows specific handlers by Copilot · Pull Request #832 · ithewei/libhv

Copilot · 2026-05-18T07:08:46Z

HttpService::GetRoute(HttpRequest*, ...) iterated pathHandlers via std::unordered_map, so match order was non-deterministic. In practice, a catch-all Any("*") could win before concrete routes (e.g. /ping), breaking expected router behavior.

Routing precedence in HttpService::GetRoute(HttpRequest*, ...)
- Added explicit exact-path lookup first (pathHandlers.find(path)), with normal method resolution.
- Refactored pattern matching into deterministic phases:
  - RESTful patterns (/:id, /{id}) first
  - wildcard patterns (*) after
- Added stable ordering among pattern candidates (longer path first, then lexical tie-break) to avoid unordered-map iteration effects.
Routing matcher extraction
- Extracted route pattern matching into a helper (match_route_path) to keep wildcard + RESTful matching behavior consistent while making precedence explicit.
Focused regression test
- Added unittest/http_service_route_test.cpp and wired it into unittest/CMakeLists.txt.
- Test asserts:
  - specific route /ping is selected over Any("*")
  - unknown path still falls through to catch-all

HttpService service;
service.Any("*", fallback_handler);
service.GET("/ping", specific_handler);

// /ping resolves to specific handler, not 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>

ithewei · 2026-05-20T11:24:57Z

+        }
+        return lhs->first < rhs->first;
+    };
+    std::sort(restful_iters.begin(), restful_iters.end(), route_compare);


GetRoute的调用频率很高，每次都筛选出restful和wildcard再排序，影响性能，能在AddRoute时就自动分类好，来提高GetRoute效率吗

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR makes HttpService::GetRoute deterministic and precedence-aware so catch-all routes like Any("*") no longer unpredictably shadow more specific handlers, and adds a regression test to lock the behavior.

Changes:

Added exact-path lookup before pattern matching, with deterministic RESTful-then-wildcard matching order.
Introduced stable ordering of pattern candidates (longer-first, then lexical) to avoid unordered_map iteration effects.
Added a focused regression test executable and integrated it into the unittest CMake targets.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
`http/server/HttpService.cpp`	Deterministic route selection with explicit precedence and extracted path-matching helper.
`unittest/http_service_route_test.cpp`	Regression test ensuring `/ping` beats `Any("*")` and unknown paths still hit the catch-all.
`unittest/CMakeLists.txt`	Wires the new route precedence regression test into the unittest build.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    std::vector<http_path_handlers::const_iterator> restful_iters;
+    std::vector<http_path_handlers::const_iterator> wildcard_iters;
+    for (auto it = pathHandlers.begin(); it != pathHandlers.end(); ++it) {
+        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);


+    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;
+    }


+    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;
+    }


@@ -0,0 +1,43 @@
+#include <stdio.h>


+    service.Any("*", fallback_handler);
+    service.GET("/ping", specific_handler);
+
+    http_handler* handler = NULL;


Initial plan

260919e

Copilot AI assigned Copilot and ithewei May 18, 2026

Copilot started work on behalf of ithewei May 18, 2026 07:08 View session

Copilot AI linked an issue May 18, 2026 that may be closed by this pull request

HttpService::GetRoute first-match-wins over std::unordered_map breaks ANY("*") + specific routes #831

Open

Copilot AI and others added 2 commits May 18, 2026 07:12

Fix HttpService route precedence over wildcard catch-all

f9a7759

Agent-Logs-Url: https://github.com/ithewei/libhv/sessions/1d649b64-5e89-4b6a-9049-5853eec30954 Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com>

Address review nits in route candidate iteration

0d88b52

Agent-Logs-Url: https://github.com/ithewei/libhv/sessions/1d649b64-5e89-4b6a-9049-5853eec30954 Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com>

Copilot AI changed the title ~~[WIP] Fix HttpService route matching for specific handlers~~ Fix HttpService route precedence so Any("*") no longer shadows specific handlers May 18, 2026

Copilot AI requested a review from ithewei May 18, 2026 07:16

Copilot finished work on behalf of ithewei May 18, 2026 07:16

ithewei reviewed May 20, 2026

View reviewed changes

ithewei marked this pull request as ready for review May 20, 2026 11:25

Copilot AI review requested due to automatic review settings May 20, 2026 11:25

Copilot AI reviewed May 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix HttpService route precedence so `Any("*")` no longer shadows specific handlers#832

Fix HttpService route precedence so `Any("*")` no longer shadows specific handlers#832
Copilot wants to merge 3 commits into
masterfrom
copilot/fix-httpservice-getroute-issue

Copilot AI commented May 18, 2026 •

edited

Loading

Uh oh!

ithewei May 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Copilot AI commented May 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ithewei May 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Copilot AI commented May 18, 2026 •

edited

Loading