Mirror
/
Crow
mirror of https://github.com/CrowCpp/Crow.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Addressed code reviews + small tweaks (unit test and paser.done())

This commit is contained in:
The-EDev 2022-07-20 23:03:42 +03:00 committed by Farook Al-Sammarraie
parent b986d1e38a
commit 25eefa1711
5 changed files with 73 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
include/crow/app.h
View File

@ -92,7 +92,7 @@ namespace crow
void handle_full(request& req, response& res)
{
auto found = handle_initial(req, res);
if (std::get<0>(*found))
if (found->rule_index)
handle(req, res, found);
}

22
include/crow/common.h
View File

@ -276,8 +276,26 @@ namespace crow
}
/// @endcond
using routing_search_result = std::tuple<uint16_t, std::vector<uint16_t>, routing_params>;
using routing_handle_result = std::tuple<uint16_t, std::vector<uint16_t>, routing_params, HTTPMethod>;
struct routing_handle_result
{
uint16_t rule_index;
std::vector<uint16_t> blueprint_indices;
routing_params r_params;
HTTPMethod method;
routing_handle_result() {}
routing_handle_result(uint16_t rule_index_, std::vector<uint16_t> blueprint_indices_, routing_params r_params_):
rule_index(rule_index_),
blueprint_indices(blueprint_indices_),
r_params(r_params_) {}
routing_handle_result(uint16_t rule_index_, std::vector<uint16_t> blueprint_indices_, routing_params r_params_, HTTPMethod method_):
rule_index(rule_index_),
blueprint_indices(blueprint_indices_),
r_params(r_params_),
method(method_) {}
};
} // namespace crow
// clang-format off

9
include/crow/http_connection.h
View File

@ -99,14 +99,17 @@ namespace crow
{
routing_handle_result_ = handler_->handle_initial(req_, res);
// if no route is found for the request method, return the response without parsing or processing anything further.
if (!std::get<0>(*routing_handle_result_))
if (!routing_handle_result_->rule_index)
{
parser_.done();
complete_request();
}
}
void handle_header()
{
// HTTP 1.1 Expect: 100-continue
if (req_.http_ver_major == 1 && req_.http_ver_minor == 1 && get_header_value(req_.headers, "expect") == "100-continue") // Using the parser because the request isn't made yet.
if (req_.http_ver_major == 1 && req_.http_ver_minor == 1 && get_header_value(req_.headers, "expect") == "100-continue")
{
buffers_.clear();
static std::string expect_100_continue = "HTTP/1.1 100 Continue\r\n\r\n";
@ -117,7 +120,7 @@ namespace crow
void handle()
{
// TODO(EDev): This should be looked into, it might be a good idea to add it to handle_url() and then restart the timer once everything passes
// TODO(EDev): cancel_deadline_timer should be looked into, it might be a good idea to add it to handle_url() and then restart the timer once everything passes
cancel_deadline_timer();
bool is_invalid_request = false;
add_keep_alive_ = false;

70
include/crow/routing.h
View File

@ -823,7 +823,7 @@ namespace crow
}
//Rule_index, Blueprint_index, routing_params
routing_search_result find(const std::string& req_url, const Node* node = nullptr, unsigned pos = 0, routing_params* params = nullptr, std::vector<uint16_t>* blueprints = nullptr) const
routing_handle_result find(const std::string& req_url, const Node* node = nullptr, unsigned pos = 0, routing_params* params = nullptr, std::vector<uint16_t>* blueprints = nullptr) const
{
//start params as an empty struct
routing_params empty;
@ -842,12 +842,12 @@ namespace crow
if (node == nullptr)
node = &head_;
auto update_found = [&found, &found_BP, &match_params](std::tuple<uint16_t, std::vector<uint16_t>, routing_params>& ret) {
found_BP = std::move(std::get<1>(ret));
if (std::get<0>(ret) && (!found || found > std::get<0>(ret)))
auto update_found = [&found, &found_BP, &match_params](routing_handle_result& ret) {
found_BP = std::move(ret.blueprint_indices);
if (ret.rule_index && (!found || found > ret.rule_index))
{
found = std::get<0>(ret);
match_params = std::move(std::get<2>(ret));
found = ret.rule_index;
match_params = std::move(ret.r_params);
}
};
@ -855,7 +855,7 @@ namespace crow
if (pos == req_url.size())
{
found_BP = std::move(*blueprints);
return std::tuple<uint16_t, std::vector<uint16_t>, routing_params>{node->rule_index, *blueprints, *params};
return routing_handle_result{node->rule_index, *blueprints, *params};
}
bool found_fragment = false;
@ -982,7 +982,7 @@ namespace crow
if (!found_fragment)
found_BP = std::move(*blueprints);
return std::tuple<uint16_t, std::vector<uint16_t>, routing_params>{found, found_BP, match_params}; //Called after all the recursions have been done
return routing_handle_result{found, found_BP, match_params}; //Called after all the recursions have been done
}
//This functions assumes any blueprint info passed is valid
@ -1399,13 +1399,13 @@ namespace crow
auto& per_method = per_methods_[static_cast<int>(req.method)];
auto& rules = per_method.rules;
unsigned rule_index = std::get<0>(per_method.trie.find(req.url));
unsigned rule_index = per_method.trie.find(req.url).rule_index;
if (!rule_index)
{
for (auto& per_method : per_methods_)
{
if (std::get<0>(per_method.trie.find(req.url)))
if (per_method.trie.find(req.url).rule_index)
{
CROW_LOG_DEBUG << "Cannot match method " << req.url << " " << method_name(req.method);
res = response(405);
@ -1506,14 +1506,14 @@ namespace crow
}
/// Is used to handle errors, you insert the error code, found route, request, and response. and it'll either call the appropriate catchall route (considering the blueprint system) and send you a status string (which is mainly used for debug messages), or just set the response code to the proper error code.
std::string get_error(unsigned short code, routing_search_result& found, const request& req, response& res)
std::string get_error(unsigned short code, routing_handle_result& found, const request& req, response& res)
{
res.code = code;
std::vector<Blueprint*> bps_found;
get_found_bp(std::get<1>(found), blueprints_, bps_found);
get_found_bp(found.blueprint_indices, blueprints_, bps_found);
for (int i = bps_found.size() - 1; i > 0; i--)
{
std::vector<uint16_t> bpi = std::get<1>(found);
std::vector<uint16_t> bpi = found.blueprint_indices;
if (bps_found[i]->catchall_rule().has_handler())
{
bps_found[i]->catchall_rule().handler_(req, res);
@ -1540,21 +1540,25 @@ namespace crow
{
HTTPMethod method_actual = req.method;
std::unique_ptr<routing_handle_result> found{new routing_handle_result}; // This is always returned to avoid a null pointer dereference.
routing_search_result search_result;
std::unique_ptr<routing_handle_result> found{
new routing_handle_result(
0,
std::vector<uint16_t>(),
routing_params(),
HTTPMethod::InternalMethodCount)}; // This is always returned to avoid a null pointer dereference.
// NOTE(EDev): This most likely will never run since the parser should handle this situation and close the connection before it gets here.
if (CROW_UNLIKELY(req.method >= HTTPMethod::InternalMethodCount))
return found;
else if (req.method == HTTPMethod::Head)
{
search_result = per_methods_[static_cast<int>(method_actual)].trie.find(req.url);
*found = per_methods_[static_cast<int>(method_actual)].trie.find(req.url);
// support HEAD requests using GET if not defined as method for the requested URL
if (!std::get<0>(search_result))
if (!found->rule_index)
{
method_actual = HTTPMethod::Get;
search_result = per_methods_[static_cast<int>(method_actual)].trie.find(req.url);
if (!std::get<0>(search_result)) // If a route is still not found, return a 404 without executing the rest of the HEAD specific code.
*found = per_methods_[static_cast<int>(method_actual)].trie.find(req.url);
if (!found->rule_index) // If a route is still not found, return a 404 without executing the rest of the HEAD specific code.
{
CROW_LOG_DEBUG << "Cannot match rules " << req.url;
res = response(404); //TODO(EDev): Should this redirect to catchall?
@ -1564,7 +1568,7 @@ namespace crow
}
res.skip_body = true;
*found = routing_handle_result(std::get<0>(search_result), std::get<1>(search_result), std::get<2>(search_result), method_actual);
found->method = method_actual;
return found;
}
else if (req.method == HTTPMethod::Options)
@ -1587,7 +1591,7 @@ namespace crow
res = response(204);
res.set_header("Allow", allow);
res.end();
*found = routing_handle_result(std::get<0>(search_result), std::get<1>(search_result), std::get<2>(search_result), method_actual);
found->method = method_actual;
return found;
}
else
@ -1595,7 +1599,7 @@ namespace crow
bool rules_matched = false;
for (int i = 0; i < static_cast<int>(HTTPMethod::InternalMethodCount); i++)
{
if (std::get<0>(per_methods_[i].trie.find(req.url)))
if (per_methods_[i].trie.find(req.url).rule_index)
{
rules_matched = true;
@ -1611,7 +1615,7 @@ namespace crow
res = response(204);
res.set_header("Allow", allow);
res.end();
*found = routing_handle_result(std::get<0>(search_result), std::get<1>(search_result), std::get<2>(search_result), method_actual);
found->method = method_actual;
return found;
}
else
@ -1625,15 +1629,15 @@ namespace crow
}
else // Every request that isn't a HEAD or OPTIONS request
{
search_result = per_methods_[static_cast<int>(method_actual)].trie.find(req.url);
// TODO(EDev): maybe ending the else here would allow the requests coming from above (after removing the return statement) to be checked on whether they actually point to a route
if (!std::get<0>(search_result))
*found = per_methods_[static_cast<int>(method_actual)].trie.find(req.url);
// TODO(EDev): maybe ending the else here would allow the requests coming from above (after removing the return statement) to be checked on whether they actually point to a route
if (!found->rule_index)
{
for (auto& per_method : per_methods_)
{
if (std::get<0>(per_method.trie.find(req.url))) //Route found, but in another method
if (per_method.trie.find(req.url).rule_index) //Route found, but in another method
{
const std::string error_message(get_error(405, search_result, req, res));
const std::string error_message(get_error(405, *found, req, res));
CROW_LOG_DEBUG << "Cannot match method " << req.url << " " << method_name(method_actual) << ". " << error_message;
res.end();
return found;
@ -1641,13 +1645,13 @@ namespace crow
}
//Route does not exist anywhere
const std::string error_message(get_error(404, search_result, req, res));
const std::string error_message(get_error(404, *found, req, res));
CROW_LOG_DEBUG << "Cannot match rules " << req.url << ". " << error_message;
res.end();
return found;
}
*found = routing_handle_result(std::get<0>(search_result), std::get<1>(search_result), std::get<2>(search_result), method_actual);
found->method = method_actual;
return found;
}
}
@ -1655,9 +1659,9 @@ namespace crow
template<typename App>
void handle(request& req, response& res, routing_handle_result found)
{
HTTPMethod method_actual = std::get<3>(found);
HTTPMethod method_actual = found.method;
auto& rules = per_methods_[static_cast<int>(method_actual)].rules;
unsigned rule_index = std::get<0>(found);
unsigned rule_index = found.rule_index;
if (rule_index >= rules.size())
throw std::runtime_error("Trie internal structure corrupted!");
@ -1686,7 +1690,7 @@ namespace crow
try
{
auto& rule = rules[rule_index];
handle_rule<App>(rule, req, res, std::get<2>(found));
handle_rule<App>(rule, req, res, found.r_params);
}
catch (std::exception& e)
{

13
tests/unittest.cpp
View File

@ -2478,10 +2478,15 @@ TEST_CASE("websocket")
SimpleApp app;
CROW_WEBSOCKET_ROUTE(app, "/ws").onopen([&](websocket::connection&) {
connected = true;
CROW_LOG_INFO << "Connected websocket and value is " << connected;
})
CROW_WEBSOCKET_ROUTE(app, "/ws")
.onaccept([&](const crow::request& req, void**) {
CROW_LOG_INFO << "Accepted websocket with URL " << req.url;
return true;
})
.onopen([&](websocket::connection&) {
connected = true;
CROW_LOG_INFO << "Connected websocket and value is " << connected;
})
.onmessage([&](websocket::connection& conn, const std::string& message, bool isbin) {
CROW_LOG_INFO << "Message is \"" << message << '\"';
if (!isbin && message == "PINGME")