Make "*" method actually catch all methods, tweak priority order a bit

This commit is contained in:
Alex Hultman
2023-12-19 03:18:44 +01:00
parent 85c6f0fb69
commit 97e8099bc2
3 changed files with 36 additions and 22 deletions
+1 -1
View File
@@ -426,7 +426,7 @@ public:
/* Todo: This is ugly, fix */ /* Todo: This is ugly, fix */
std::vector<std::string> methods; std::vector<std::string> methods;
if (method == "*") { if (method == "*") {
methods = httpContextData->currentRouter->upperCasedMethods; methods = {"*"};
} else { } else {
methods = {method}; methods = {method};
} }
+26 -12
View File
@@ -35,8 +35,7 @@ namespace uWS {
template <class USERDATA> template <class USERDATA>
struct HttpRouter { struct HttpRouter {
/* These are public for now */ static constexpr std::string_view ANY_METHOD_TOKEN = "*";
std::vector<std::string> upperCasedMethods = {"GET", "POST", "HEAD", "PUT", "DELETE", "CONNECT", "OPTIONS", "TRACE", "PATCH"};
static const uint32_t HIGH_PRIORITY = 0xd0000000, MEDIUM_PRIORITY = 0xe0000000, LOW_PRIORITY = 0xf0000000; static const uint32_t HIGH_PRIORITY = 0xd0000000, MEDIUM_PRIORITY = 0xe0000000, LOW_PRIORITY = 0xf0000000;
private: private:
@@ -46,9 +45,6 @@ private:
/* Handler ids are 32-bit */ /* Handler ids are 32-bit */
static const uint32_t HANDLER_MASK = 0x0fffffff; static const uint32_t HANDLER_MASK = 0x0fffffff;
/* Methods and their respective priority */
std::map<std::string, int> priority;
/* List of handlers */ /* List of handlers */
std::vector<MoveOnlyFunction<bool(HttpRouter *)>> handlers; std::vector<MoveOnlyFunction<bool(HttpRouter *)>> handlers;
@@ -245,10 +241,8 @@ private:
public: public:
HttpRouter() { HttpRouter() {
int p = 0; /* Always have ANY route */
for (std::string &method : upperCasedMethods) { getNode(&root, std::string(ANY_METHOD_TOKEN.data(), ANY_METHOD_TOKEN.length()), false);
priority[method] = p++;
}
} }
std::pair<int, std::string_view *> getParameters() { std::pair<int, std::string_view *> getParameters() {
@@ -269,12 +263,16 @@ public:
for (auto &p : root.children) { for (auto &p : root.children) {
if (p->name == method) { if (p->name == method) {
/* Then route the url */ /* Then route the url */
return executeHandlers(p.get(), 0, userData); if (executeHandlers(p.get(), 0, userData)) {
return true;
} else {
break;
}
} }
} }
/* We did not find any handler for this method and url */ /* Always test any route last */
return false; return executeHandlers(root.children.back().get(), 0, userData);
} }
/* Adds the corresponding entires in matching tree and handler list */ /* Adds the corresponding entires in matching tree and handler list */
@@ -299,6 +297,22 @@ public:
std::cerr << "Error: Internal routing error" << std::endl; std::cerr << "Error: Internal routing error" << std::endl;
std::abort(); std::abort();
} }
/* ANY method must be last, GET must be first */
std::sort(root.children.begin(), root.children.end(), [](const auto &a, const auto &b) {
/* Assuming the list of methods is unique, non-repeating */
if (a->name == "GET") {
return true;
} else if (b->name == "GET") {
return false;
} else if (a->name == ANY_METHOD_TOKEN) {
return false;
} else if (b->name == ANY_METHOD_TOKEN) {
return true;
} else {
return a->name < b->name;
}
});
} }
bool cullNode(Node *parent, Node *node, uint32_t handler) { bool cullNode(Node *parent, Node *node, uint32_t handler) {
+9 -9
View File
@@ -8,7 +8,7 @@ void testMethodPriority() {
uWS::HttpRouter<int> r; uWS::HttpRouter<int> r;
std::string result; std::string result;
r.add(r.upperCasedMethods, "/static/route", [&result](auto *) { r.add({"*"}, "/static/route", [&result](auto *) {
std::cout << "ANY static route" << std::endl; std::cout << "ANY static route" << std::endl;
result += "AS"; result += "AS";
return true; return true;
@@ -26,9 +26,9 @@ void testMethodPriority() {
return true; return true;
}); });
assert(r.route("nonsense", "/static/route") == false); assert(r.route("nonsense", "/static/route") == true);
assert(r.route("GET", "/static") == false); assert(r.route("GET", "/static") == false);
assert(result == ""); assert(result == "AS");
/* Should end up directly in ANY handler */ /* Should end up directly in ANY handler */
result.clear(); result.clear();
@@ -51,7 +51,7 @@ void testPatternPriority() {
uWS::HttpRouter<int> r; uWS::HttpRouter<int> r;
std::string result; std::string result;
r.add(r.upperCasedMethods, "/a/b/c", [&result](auto *) { r.add({"*"}, "/a/b/c", [&result](auto *) {
std::cout << "ANY static route" << std::endl; std::cout << "ANY static route" << std::endl;
result += "AS"; result += "AS";
return false; return false;
@@ -81,18 +81,18 @@ void testPatternPriority() {
return false; return false;
}); });
r.add(r.upperCasedMethods, "/a/:b/c", [&result](auto *) { r.add({"*"}, "/a/:b/c", [&result](auto *) {
std::cout << "ANY parameter route" << std::endl; std::cout << "ANY parameter route" << std::endl;
result += "AP"; result += "AP";
return false; return false;
}, r.LOW_PRIORITY); }, r.LOW_PRIORITY);
assert(r.route("POST", "/a/b/c") == false); assert(r.route("POST", "/a/b/c") == false);
assert(result == "ASPPAP"); assert(result == "PPASAP");
result.clear(); result.clear();
assert(r.route("GET", "/a/b/c") == false); assert(r.route("GET", "/a/b/c") == false);
assert(result == "GSASGPAPGW"); assert(result == "GSGPGWASAP");
} }
void testUpgrade() { void testUpgrade() {
@@ -224,7 +224,7 @@ void testBugReports() {
}, r.MEDIUM_PRIORITY); }, r.MEDIUM_PRIORITY);
/* ANY on /* */ /* ANY on /* */
r.add(r.upperCasedMethods, "/*", [&result](auto *) { r.add({"*"}, "/*", [&result](auto *) {
result += "AW"; result += "AW";
return false; return false;
}, r.LOW_PRIORITY); }, r.LOW_PRIORITY);
@@ -256,7 +256,7 @@ void testBugReports() {
}, r.MEDIUM_PRIORITY); }, r.MEDIUM_PRIORITY);
/* ANY on /* */ /* ANY on /* */
r.add(r.upperCasedMethods, "/*", [&result](auto *) { r.add({"*"}, "/*", [&result](auto *) {
result += "AW"; result += "AW";
return false; return false;
}, r.LOW_PRIORITY); }, r.LOW_PRIORITY);