Fix removal of param routes

Author: uNetworkingAB

src/HttpRouter.h

@@ -217,7 +217,7 @@ struct HttpRouter {
                     std::string segment = std::string(getUrlSegment(i).first);
                     Node *next = nullptr;
                     for (std::unique_ptr<Node> &child : n->children) {
-                        if (child->name == segment && child->isHighPriority == (priority == HIGH_PRIORITY)) {
+                        if (((segment.length() && child->name.length() && segment[0] == ':' && child->name[0] == ':') || child->name == segment) && child->isHighPriority == (priority == HIGH_PRIORITY)) {
                             next = child.get();
                             break;
                         }
@@ -271,7 +271,10 @@ struct HttpRouter {
             }
         }
 
-        /* Always test any route last */
+        /* Always test any route last (this check should not be necessary if we always have at least one handler) */
+        if (root.children.empty()) [[unlikely]] {
+            return false;
+        }
         return executeHandlers(root.children.back().get(), 0, userData);
     }
 
@@ -356,11 +359,11 @@ struct HttpRouter {
     /* Removes ALL routes with the same handler as can be found with the given parameters.
      * Removing a wildcard is done by removing ONE OF the methods the wildcard would match with.
      * Example: If wildcard includes POST, GET, PUT, you can remove ALL THREE by removing GET. */
-    void remove(std::string method, std::string pattern, uint32_t priority) {
+    bool remove(std::string method, std::string pattern, uint32_t priority) {
         uint32_t handler = findHandler(method, pattern, priority);
         if (handler == UINT32_MAX) {
             /* Not found or already removed, do nothing */
-            return;
+            return false;
         }
 
         /* Cull the entire tree */
@@ -371,6 +374,8 @@ struct HttpRouter {
 
         /* Now remove the actual handler */
         handlers.erase(handlers.begin() + (handler & HANDLER_MASK));
+
+        return true;
     }
 };
 

tests/HttpRouter.cpp

@@ -182,11 +182,13 @@ void testBugReports() {
         r.route("GET", "/route");
         assert(result == "");
 
-        r.remove("GET", "/route/:id", r.MEDIUM_PRIORITY);
+        std::cout << "TRYING TO DELETE /route/:id" << std::endl;
+        bool found = r.remove("GET", "/route/:id", r.MEDIUM_PRIORITY);
+        std::cout << "Found:" << found << std::endl;
 
         /* The bug is really only this line, it does not remove parameter routes */
         r.route("GET", "/route/21");
-        std::cout << result << std::endl;
+        std::cout << "Found:" << found << ", " << result << std::endl;
         assert(result == "");
     }
     {

tests/Makefile

@@ -5,7 +5,7 @@ default:
 	./ChunkedEncoding
 	$(CXX) -std=c++17 -fsanitize=address TopicTree.cpp -o TopicTree
 	./TopicTree
-	$(CXX) -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG -std=c++17 -fsanitize=address HttpRouter.cpp -o HttpRouter
+	$(CXX) -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG -std=c++17 -g -fsanitize=address HttpRouter.cpp -o HttpRouter
 	./HttpRouter
 	$(CXX) -std=c++17 -fsanitize=address BloomFilter.cpp -o BloomFilter
 	./BloomFilter