Conversation
Also (likely) fixed problem with streaming without keeping connection alive Also fixed problem with multipart hanging on request Also updated TODO entries to indicate Writer
|
Compression tests apparently used parts of the parser that the rest of Crow wasn't using (and were as such removed), so the test needs to be adapted to work without the parser (cut out the response line and headers, leaving only the body). |
include/crow/common.h
Outdated
| // TODO(EDev): Adding C++20's [[likely]] and [[unlikely]] attributes might be useful | ||
| #if defined(__GNUG__) || defined(__clang__) | ||
| #define CROW_LIKELY(X) __builtin_expect(!!(X), 1) | ||
| #define CROW_UNLIKELY(X) __builtin_expect(!!(X), 0) | ||
| #else | ||
| #define CROW_LIKELY(X) (X) | ||
| #define CROW_UNLIKELY(X) (X) | ||
| #endif | ||
|
|
There was a problem hiding this comment.
Came from json.h and the parser (is used in both now)
| const char cr = '\r'; | ||
| const char lf = '\n'; | ||
| const std::string crlf("\r\n"); | ||
|
|
There was a problem hiding this comment.
came from multipart.h, connection.h, and the parser.
|
|
||
| COPY, | ||
| LOCK, | ||
| MKCOL, | ||
| MOVE, | ||
| PROPFIND, | ||
| PROPPATCH, | ||
| SEARCH, | ||
| UNLOCK, | ||
| BIND, | ||
| REBIND, | ||
| UNBIND, | ||
| ACL, | ||
|
|
||
| REPORT, | ||
| MKACTIVITY, | ||
| CHECKOUT, | ||
| MERGE, | ||
|
|
||
| MSEARCH, | ||
| NOTIFY, | ||
| SUBSCRIBE, | ||
| UNSUBSCRIBE, | ||
|
|
||
| MKCALENDAR, | ||
|
|
||
| LINK, | ||
| UNLINK, | ||
|
|
||
| SOURCE, |
| const char* method_strings[] = | ||
| { | ||
| "DELETE", | ||
| "GET", | ||
| "HEAD", | ||
| "POST", | ||
| "PUT", | ||
|
|
||
| "CONNECT", | ||
| "OPTIONS", | ||
| "TRACE", | ||
|
|
||
| "PATCH", | ||
| "PURGE", | ||
|
|
||
| "COPY", | ||
| "LOCK", | ||
| "MKCOL", | ||
| "MOVE", | ||
| "PROPFIND", | ||
| "PROPPATCH", | ||
| "SEARCH", | ||
| "UNLOCK", | ||
| "BIND", | ||
| "REBIND", | ||
| "UNBIND", | ||
| "ACL", | ||
|
|
||
| "REPORT", | ||
| "MKACTIVITY", | ||
| "CHECKOUT", | ||
| "MERGE", | ||
|
|
||
| "M-SEARCH", | ||
| "NOTIFY", | ||
| "SUBSCRIBE", | ||
| "UNSUBSCRIBE", | ||
|
|
||
| "MKCALENDAR", | ||
|
|
||
| "LINK", | ||
| "UNLINK", | ||
|
|
||
| "SOURCE"}; | ||
|
|
||
|
|
||
| inline std::string method_name(HTTPMethod method) | ||
| { | ||
| if (CROW_LIKELY(method < HTTPMethod::InternalMethodCount)) | ||
| { | ||
| return method_strings[(unsigned char)method]; | ||
| } | ||
| return "invalid"; | ||
| } | ||
|
|
There was a problem hiding this comment.
Sort of inspired by the parser
| if (parser_.check_version(1, 0)) | ||
| { | ||
| // HTTP/1.0 | ||
| if (req.headers.count("connection")) | ||
| { | ||
| if (boost::iequals(req.get_header_value("connection"), "Keep-Alive")) | ||
| add_keep_alive_ = true; | ||
| } | ||
| else | ||
| close_connection_ = true; | ||
| } | ||
| else if (parser_.check_version(1, 1)) | ||
| add_keep_alive_ = req.keep_alive; | ||
| close_connection_ = req.close_connection; | ||
|
|
||
| if (req.check_version(1, 1)) // HTTP/1.1 | ||
| { | ||
| // HTTP/1.1 | ||
| if (req.headers.count("connection")) | ||
| { | ||
| if (req.get_header_value("connection") == "close") | ||
| close_connection_ = true; | ||
| else if (boost::iequals(req.get_header_value("connection"), "Keep-Alive")) | ||
| add_keep_alive_ = true; | ||
| } |
There was a problem hiding this comment.
Already being done in the parser
| return empty; | ||
| } | ||
|
|
||
| struct DetachHelper; |
| bool compressed = true; ///< If compression is enabled and this is false, the individual response will not be compressed. | ||
| #endif | ||
| bool is_head_response = false; ///< Whether this is a response to a HEAD request. | ||
| bool skip_body = false; ///< Whether this is a response to a HEAD request. |
There was a problem hiding this comment.
body is skipped with upgrade requests as well
| //Total bytes received | ||
| unsigned int received = 0; | ||
| sendmsg = "GET /test\r\n\r\n"; | ||
| sendmsg = "GET /test HTTP/1.0\r\n\r\n"; |
There was a problem hiding this comment.
I did this to test the close connection problem
| std::string response_deflate; | ||
| std::string response_gzip; | ||
| std::string response_deflate_no_header; | ||
| std::string response_gzip_no_header; | ||
| std::string response_deflate_none; | ||
| std::string response_gzip_none; | ||
|
|
||
| auto on_body = [](http_parser* parser, const char* body, size_t body_length) -> int { | ||
| std::string* body_ptr = reinterpret_cast<std::string*>(parser->data); | ||
| *body_ptr = std::string(body, body + body_length); | ||
|
|
||
| return 0; | ||
| }; | ||
|
|
||
| http_parser_settings settings{}; | ||
| settings.on_body = on_body; | ||
|
|
||
| asio::io_service is; | ||
| { | ||
| // Compression | ||
| { | ||
| asio::ip::tcp::socket socket[2] = {asio::ip::tcp::socket(is), asio::ip::tcp::socket(is)}; | ||
| socket[0].connect(asio::ip::tcp::endpoint(asio::ip::address::from_string(LOCALHOST_ADDRESS), 45451)); | ||
| socket[1].connect(asio::ip::tcp::endpoint(asio::ip::address::from_string(LOCALHOST_ADDRESS), 45452)); | ||
|
|
||
| socket[0].send(asio::buffer(test_compress_msg)); | ||
| socket[1].send(asio::buffer(test_compress_msg)); | ||
|
|
||
| size_t bytes_deflate = socket[0].receive(asio::buffer(buf_deflate, 2048)); | ||
| size_t bytes_gzip = socket[1].receive(asio::buffer(buf_gzip, 2048)); | ||
| // Compression | ||
| { | ||
| asio::ip::tcp::socket socket[2] = {asio::ip::tcp::socket(is), asio::ip::tcp::socket(is)}; | ||
| socket[0].connect(asio::ip::tcp::endpoint(asio::ip::address::from_string(LOCALHOST_ADDRESS), 45451)); | ||
| socket[1].connect(asio::ip::tcp::endpoint(asio::ip::address::from_string(LOCALHOST_ADDRESS), 45452)); | ||
|
|
||
| http_parser parser[2] = {{}, {}}; | ||
| http_parser_init(&parser[0], HTTP_RESPONSE); | ||
| http_parser_init(&parser[1], HTTP_RESPONSE); | ||
| parser[0].data = reinterpret_cast<void*>(&response_deflate); | ||
| parser[1].data = reinterpret_cast<void*>(&response_gzip); | ||
| socket[0].send(asio::buffer(test_compress_msg)); | ||
| socket[1].send(asio::buffer(test_compress_msg)); | ||
|
|
||
| http_parser_execute(&parser[0], &settings, buf_deflate, bytes_deflate); | ||
| http_parser_execute(&parser[1], &settings, buf_gzip, bytes_gzip); | ||
| socket[0].receive(asio::buffer(buf_deflate, 2048)); | ||
| socket[1].receive(asio::buffer(buf_gzip, 2048)); | ||
|
|
||
| response_deflate = inflate_string(response_deflate); | ||
| response_gzip = inflate_string(response_gzip); | ||
| std::string response_deflate; | ||
| std::string response_gzip; | ||
|
|
||
| socket[0].close(); | ||
| socket[1].close(); | ||
| } | ||
| // No Header (thus no compression) | ||
| { | ||
| asio::ip::tcp::socket socket[2] = {asio::ip::tcp::socket(is), asio::ip::tcp::socket(is)}; | ||
| socket[0].connect(asio::ip::tcp::endpoint(asio::ip::address::from_string(LOCALHOST_ADDRESS), 45451)); | ||
| socket[1].connect(asio::ip::tcp::endpoint(asio::ip::address::from_string(LOCALHOST_ADDRESS), 45452)); | ||
|
|
||
| socket[0].send(asio::buffer(test_compress_no_header_msg)); | ||
| socket[1].send(asio::buffer(test_compress_no_header_msg)); | ||
| for (unsigned i{0}; i < 2048; i++) | ||
| { | ||
| if (buf_deflate[i] == 0) | ||
| { | ||
| bool end = true; | ||
| for (unsigned j = i; j < 2048; j++) | ||
| { | ||
| if (buf_deflate[j] != 0) | ||
| { | ||
| end = false; | ||
| break; | ||
| } | ||
| } | ||
| if (end) | ||
| { | ||
| response_deflate.push_back(0); | ||
| response_deflate.push_back(0); | ||
| break; | ||
| } | ||
| } | ||
| response_deflate.push_back(buf_deflate[i]); | ||
| } | ||
|
|
||
| size_t bytes_deflate = socket[0].receive(asio::buffer(buf_deflate, 2048)); | ||
| size_t bytes_gzip = socket[1].receive(asio::buffer(buf_gzip, 2048)); | ||
| for (unsigned i{0}; i < 2048; i++) | ||
| { | ||
| if (buf_gzip[i] == 0) | ||
| { | ||
| bool end = true; | ||
| for (unsigned j = i; j < 2048; j++) | ||
| { | ||
| if (buf_gzip[j] != 0) | ||
| { | ||
| end = false; | ||
| break; | ||
| } | ||
| } | ||
| if (end) | ||
| { | ||
| response_deflate.push_back(0); | ||
| response_deflate.push_back(0); | ||
| break; | ||
| } | ||
| } | ||
| response_gzip.push_back(buf_gzip[i]); | ||
| } | ||
|
|
||
| http_parser parser[2] = {{}, {}}; | ||
| http_parser_init(&parser[0], HTTP_RESPONSE); | ||
| http_parser_init(&parser[1], HTTP_RESPONSE); | ||
| parser[0].data = reinterpret_cast<void*>(&response_deflate_no_header); | ||
| parser[1].data = reinterpret_cast<void*>(&response_gzip_no_header); | ||
| response_deflate = inflate_string(response_deflate.substr(response_deflate.find("\r\n\r\n") + 4)); | ||
| response_gzip = inflate_string(response_gzip.substr(response_gzip.find("\r\n\r\n") + 4)); | ||
|
|
||
| http_parser_execute(&parser[0], &settings, buf_deflate, bytes_deflate); | ||
| http_parser_execute(&parser[1], &settings, buf_gzip, bytes_gzip); | ||
| socket[0].close(); | ||
| socket[1].close(); | ||
| CHECK(expected_string == response_deflate); | ||
| CHECK(expected_string == response_gzip); | ||
| } | ||
| // No Header (thus no compression) | ||
| { | ||
| asio::ip::tcp::socket socket[2] = {asio::ip::tcp::socket(is), asio::ip::tcp::socket(is)}; | ||
| socket[0].connect(asio::ip::tcp::endpoint(asio::ip::address::from_string(LOCALHOST_ADDRESS), 45451)); | ||
| socket[1].connect(asio::ip::tcp::endpoint(asio::ip::address::from_string(LOCALHOST_ADDRESS), 45452)); | ||
|
|
||
| socket[0].close(); | ||
| socket[1].close(); | ||
| } | ||
| // No compression | ||
| { | ||
| asio::ip::tcp::socket socket[2] = {asio::ip::tcp::socket(is), asio::ip::tcp::socket(is)}; | ||
| socket[0].connect(asio::ip::tcp::endpoint(asio::ip::address::from_string(LOCALHOST_ADDRESS), 45451)); | ||
| socket[1].connect(asio::ip::tcp::endpoint(asio::ip::address::from_string(LOCALHOST_ADDRESS), 45452)); | ||
| socket[0].send(asio::buffer(test_compress_no_header_msg)); | ||
| socket[1].send(asio::buffer(test_compress_no_header_msg)); | ||
|
|
||
| socket[0].send(asio::buffer(test_none_msg)); | ||
| socket[1].send(asio::buffer(test_none_msg)); | ||
| socket[0].receive(asio::buffer(buf_deflate, 2048)); | ||
| socket[1].receive(asio::buffer(buf_gzip, 2048)); | ||
|
|
||
| size_t bytes_deflate = socket[0].receive(asio::buffer(buf_deflate, 2048)); | ||
| size_t bytes_gzip = socket[1].receive(asio::buffer(buf_gzip, 2048)); | ||
| std::string response_deflate(buf_deflate); | ||
| std::string response_gzip(buf_gzip); | ||
| response_deflate = response_deflate.substr(98); | ||
| response_gzip = response_gzip.substr(98); | ||
|
|
||
| http_parser parser[2] = {{}, {}}; | ||
| http_parser_init(&parser[0], HTTP_RESPONSE); | ||
| http_parser_init(&parser[1], HTTP_RESPONSE); | ||
| parser[0].data = reinterpret_cast<void*>(&response_deflate_none); | ||
| parser[1].data = reinterpret_cast<void*>(&response_gzip_none); | ||
| socket[0].close(); | ||
| socket[1].close(); | ||
| CHECK(expected_string == response_deflate); | ||
| CHECK(expected_string == response_gzip); | ||
| } | ||
| // No compression | ||
| { | ||
| asio::ip::tcp::socket socket[2] = {asio::ip::tcp::socket(is), asio::ip::tcp::socket(is)}; | ||
| socket[0].connect(asio::ip::tcp::endpoint(asio::ip::address::from_string(LOCALHOST_ADDRESS), 45451)); | ||
| socket[1].connect(asio::ip::tcp::endpoint(asio::ip::address::from_string(LOCALHOST_ADDRESS), 45452)); | ||
|
|
||
| http_parser_execute(&parser[0], &settings, buf_deflate, bytes_deflate); | ||
| http_parser_execute(&parser[1], &settings, buf_gzip, bytes_gzip); | ||
| socket[0].send(asio::buffer(test_none_msg)); | ||
| socket[1].send(asio::buffer(test_none_msg)); | ||
|
|
||
| socket[0].close(); | ||
| socket[1].close(); | ||
| } | ||
| } | ||
| { | ||
| CHECK(expected_string == response_deflate); | ||
| CHECK(expected_string == response_gzip); | ||
| socket[0].receive(asio::buffer(buf_deflate, 2048)); | ||
| socket[1].receive(asio::buffer(buf_gzip, 2048)); | ||
|
|
||
| CHECK(expected_string == response_deflate_no_header); | ||
| CHECK(expected_string == response_gzip_no_header); | ||
| std::string response_deflate(buf_deflate); | ||
| std::string response_gzip(buf_gzip); | ||
| response_deflate = response_deflate.substr(98); | ||
| response_gzip = response_gzip.substr(98); | ||
|
|
||
| CHECK(expected_string == response_deflate_none); | ||
| CHECK(expected_string == response_gzip_none); | ||
| } | ||
| socket[0].close(); | ||
| socket[1].close(); | ||
| CHECK(expected_string == response_deflate); | ||
| CHECK(expected_string == response_gzip); | ||
| } | ||
| } | ||
|
|
||
| app_deflate.stop(); | ||
| app_gzip.stop(); | ||
| app_deflate.stop(); | ||
| app_gzip.stop(); |
There was a problem hiding this comment.
This needed to change because it used the parser's response code
| const std::uint16_t port = 12345; | ||
|
|
||
| std::thread runTest([&]() { | ||
| auto _ = async(launch::async, [&] { |
There was a problem hiding this comment.
I'm not 100% sure if it worked, but this change was made to fix a problem where the tests would block for some reason.
| inline int | ||
| http_parser_parse_url(const char *buf, size_t buflen, int is_connect, | ||
| struct http_parser_url *u) | ||
| { | ||
| enum state s; | ||
| const char *p; | ||
| enum http_parser_url_fields uf, old_uf; | ||
| int found_at = 0; | ||
|
|
||
| if (buflen == 0) { | ||
| return 1; | ||
| } | ||
|
|
||
| u->port = u->field_set = 0; | ||
| s = is_connect ? s_req_server_start : s_req_spaces_before_url; | ||
| old_uf = UF_MAX; | ||
|
|
||
| for (p = buf; p < buf + buflen; p++) { | ||
| s = parse_url_char(s, *p); | ||
|
|
||
| /* Figure out the next field that we're operating on */ | ||
| switch (s) { | ||
| case s_dead: | ||
| return 1; | ||
|
|
||
| /* Skip delimeters */ | ||
| case s_req_schema_slash: | ||
| case s_req_schema_slash_slash: | ||
| case s_req_server_start: | ||
| case s_req_query_string_start: | ||
| case s_req_fragment_start: | ||
| continue; | ||
|
|
||
| case s_req_schema: | ||
| uf = UF_SCHEMA; | ||
| break; | ||
|
|
||
| case s_req_server_with_at: | ||
| found_at = 1; | ||
| break; | ||
|
|
||
| /* fall through */ | ||
| case s_req_server: | ||
| uf = UF_HOST; | ||
| break; | ||
|
|
||
| case s_req_path: | ||
| uf = UF_PATH; | ||
| break; | ||
|
|
||
| case s_req_query_string: | ||
| uf = UF_QUERY; | ||
| break; | ||
|
|
||
| case s_req_fragment: | ||
| uf = UF_FRAGMENT; | ||
| break; | ||
|
|
||
| default: | ||
| assert(!"Unexpected state"); | ||
| return 1; | ||
| } | ||
|
|
||
| /* Nothing's changed; soldier on */ | ||
| if (uf == old_uf) { | ||
| u->field_data[uf].len++; | ||
| continue; | ||
| } | ||
|
|
||
| u->field_data[uf].off = (uint16_t)(p - buf); | ||
| u->field_data[uf].len = 1; | ||
|
|
||
| u->field_set |= (1 << uf); | ||
| old_uf = uf; | ||
| } | ||
|
|
||
| /* host must be present if there is a schema */ | ||
| /* parsing http:///toto will fail */ | ||
| if ((u->field_set & (1 << UF_SCHEMA)) && | ||
| (u->field_set & (1 << UF_HOST)) == 0) { | ||
| return 1; | ||
| } | ||
|
|
||
| if (u->field_set & (1 << UF_HOST)) { | ||
| if (http_parse_host(buf, u, found_at) != 0) { | ||
| return 1; | ||
| } | ||
| } | ||
|
|
||
| /* CONNECT requests can only contain "hostname:port" */ | ||
| if (is_connect && u->field_set != ((1 << UF_HOST)|(1 << UF_PORT))) { | ||
| return 1; | ||
| } | ||
|
|
||
| if (u->field_set & (1 << UF_PORT)) { | ||
| uint16_t off; | ||
| uint16_t len; | ||
| const char* p; | ||
| const char* end; | ||
| unsigned long v; | ||
|
|
||
| off = u->field_data[UF_PORT].off; | ||
| len = u->field_data[UF_PORT].len; | ||
| end = buf + off + len; | ||
|
|
||
| /* NOTE: The characters are already validated and are in the [0-9] range */ | ||
| assert((size_t)(off + len) <= buflen && "Port number overflow"); | ||
| v = 0; | ||
| for (p = buf + off; p < end; p++) { | ||
| v *= 10; | ||
| v += *p - '0'; | ||
|
|
||
| /* Ports have a max value of 2^16 */ | ||
| if (v > 0xffff) { | ||
| return 1; | ||
| } | ||
| } | ||
|
|
||
| u->port = static_cast<uint16_t>(v); | ||
| } | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
parsing the URL for requests is done in the framework
| enum http_parser_url_fields | ||
| { UF_SCHEMA = 0 | ||
| , UF_HOST = 1 | ||
| , UF_PORT = 2 | ||
| , UF_PATH = 3 | ||
| , UF_QUERY = 4 | ||
| , UF_FRAGMENT = 5 | ||
| , UF_USERINFO = 6 | ||
| , UF_MAX = 7 | ||
| }; |
Also moved builtin_expect to utility.h (for use in sanitizer function)
--- include/crow/parser.h (before formatting)
+++ include/crow/parser.h (after formatting)
@@ -154,13 +154,13 @@
// HTTP1.1 = always send keep_alive, HTTP1.0 = only send if header exists, HTTP?.? = never send
keep_alive = (http_major == 1 && http_minor == 0) ?
- ((flags & F_CONNECTION_KEEP_ALIVE) ? true : false) :
- ((http_major == 1 && http_minor == 1) ? true : false);
+ ((flags & F_CONNECTION_KEEP_ALIVE) ? true : false) :
+ ((http_major == 1 && http_minor == 1) ? true : false);
// HTTP1.1 = only close if close header exists, HTTP1.0 = always close unless keep_alive header exists, HTTP?.?= never close
close_connection = (http_major == 1 && http_minor == 0) ?
- ((flags & F_CONNECTION_KEEP_ALIVE) ? false : true) :
- ((http_major == 1 && http_minor == 1) ? ((flags & F_CONNECTION_CLOSE) ? true : false) : false);
+ ((flags & F_CONNECTION_KEEP_ALIVE) ? false : true) :
+ ((http_major == 1 && http_minor == 1) ? ((flags & F_CONNECTION_CLOSE) ? true : false) : false);
}
/// Take the parsed HTTP request data and convert it to a \ref crow.request
--- include/crow/utility.h (before formatting)
+++ include/crow/utility.h (after formatting)
@@ -702,8 +702,8 @@
data[i] = replacement;
}
else if ((c == '/') || (c == '\\'))
- {
- if (CROW_UNLIKELY( i == 0 )) //Prevent Unix Absolute Paths (Windows Absolute Paths are prevented with `(c == ':')`)
+ {
+ if (CROW_UNLIKELY(i == 0)) //Prevent Unix Absolute Paths (Windows Absolute Paths are prevented with `(c == ':')`)
{
data[i] = replacement;
}
@@ -711,9 +711,9 @@
{
checkForSpecialEntries = true;
}
- }
}
- }
+ }
+ }
} // namespace utility
} // namespace crow |
In a nutshell, This PR:
request.HttpClient(due to the boundary being wrapped with quotation marks (")).NOTEorTODO.More detailed explanation of the changes will be provided as review comments to make the code changes easy to digest.
Closes #306, #320, #331