From 61b69169c77cee9a67f041b5fe2dc73da8e61bed Mon Sep 17 00:00:00 2001 From: Alex Hultman Date: Sun, 19 Jan 2020 19:43:26 +0100 Subject: [PATCH] Quick-fix for HttpRouter priority flaw --- src/HttpRouter.h | 19 +++++++++----- tests/HttpRouter.cpp | 62 ++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 8 deletions(-) diff --git a/src/HttpRouter.h b/src/HttpRouter.h index db62b88..0353110 100644 --- a/src/HttpRouter.h +++ b/src/HttpRouter.h @@ -42,7 +42,7 @@ private: /* Handler ids are 32-bit */ static const uint32_t HANDLER_MASK = 0x0fffffff; - + /* Methods and their respective priority */ std::map priority; @@ -59,19 +59,26 @@ private: std::string name; std::vector> children; std::vector handlers; + bool isHighPriority; } root = {"rootNode"}; /* Advance from parent to child, adding child if necessary */ - Node *getNode(Node *parent, std::string child) { + Node *getNode(Node *parent, std::string child, bool isHighPriority) { for (std::unique_ptr &node : parent->children) { - if (node->name == child) { + if (node->name == child && node->isHighPriority == isHighPriority) { return node.get(); } } /* Insert sorted, but keep order if parent is root (we sort methods by priority elsewhere) */ std::unique_ptr newNode(new Node({child})); + newNode->isHighPriority = isHighPriority; return parent->children.emplace(std::upper_bound(parent->children.begin(), parent->children.end(), newNode, [parent, this](auto &a, auto &b) { + + if (a->isHighPriority != b->isHighPriority) { + return a->isHighPriority; + } + return b->name.length() && (parent != &root) && (b->name < a->name); }), std::move(newNode))->get(); } @@ -213,11 +220,11 @@ public: void add(std::vector methods, std::string pattern, fu2::unique_function &&handler, int priority = MEDIUM_PRIORITY) { for (std::string method : methods) { /* Lookup method */ - Node *node = getNode(&root, method); + Node *node = getNode(&root, method, false); /* Iterate over all segments */ setUrl(pattern); for (int i = 0; getUrlSegment(i).length() || i == 0; i++) { - node = getNode(node, std::string(getUrlSegment(i))); + node = getNode(node, std::string(getUrlSegment(i)), priority == HIGH_PRIORITY); } /* Insert handler in order sorted by priority (most significant 1 byte) */ node->handlers.insert(std::upper_bound(node->handlers.begin(), node->handlers.end(), (uint32_t) (priority | handlers.size())), (uint32_t) (priority | handlers.size())); @@ -230,4 +237,4 @@ public: } -#endif // UWS_HTTPROUTER_HPP +#endif // UWS_HTTPROUTER_HPP \ No newline at end of file diff --git a/tests/HttpRouter.cpp b/tests/HttpRouter.cpp index 629a0f7..61dbda8 100644 --- a/tests/HttpRouter.cpp +++ b/tests/HttpRouter.cpp @@ -119,7 +119,7 @@ void testUpgrade() { }, r.HIGH_PRIORITY); assert(r.route("get", "/something")); - assert(result == "GS"); + assert(result == "WWGS"); result.clear(); assert(r.route("get", "/") == false); @@ -150,7 +150,7 @@ void testBugReports() { }, r.MEDIUM_PRIORITY); r.route("get", "/ok"); - assert(result == "GSWWGW"); + assert(result == "WWGSGW"); } { @@ -172,6 +172,64 @@ void testBugReports() { r.route("get", "/"); assert(result == "WSGS"); } + + { + uWS::HttpRouter r; + std::string result; + + /* WS on /* */ + r.add({"get"}, "/*", [&result](auto *) { + result += "WW"; + return false; + }, r.HIGH_PRIORITY); + + /* GET on /static */ + r.add({"get"}, "/static", [&result](auto *) { + result += "GSL"; + return false; + }, r.MEDIUM_PRIORITY); + + /* ANY on /* */ + r.add(r.methods, "/*", [&result](auto *) { + result += "AW"; + return false; + }, r.LOW_PRIORITY); + + r.route("get", "/static"); + assert(result == "WWGSLAW"); + } + + { + uWS::HttpRouter r; + std::string result; + + /* WS on /* */ + r.add({"get"}, "/*", [&result](auto *) { + result += "WW"; + return false; + }, r.HIGH_PRIORITY); + + /* GET on / */ + r.add({"get"}, "/", [&result](auto *) { + result += "GSS"; + return false; + }, r.MEDIUM_PRIORITY); + + /* GET on /static */ + r.add({"get"}, "/static", [&result](auto *) { + result += "GSL"; + return false; + }, r.MEDIUM_PRIORITY); + + /* ANY on /* */ + r.add(r.methods, "/*", [&result](auto *) { + result += "AW"; + return false; + }, r.LOW_PRIORITY); + + r.route("get", "/static"); + assert(result == "WWGSLAW"); + } } void testParameters() {