From f7758feab52325d500ab393d7e74ca6926475647 Mon Sep 17 00:00:00 2001 From: erik Date: Fri, 10 Jun 2022 12:45:43 +0200 Subject: [PATCH 1/3] Make stream_response test independent of header size The test was failing when built using my mingw64-11.2.0 toolset on the Windows platform, on the 1.0+3 release. --- tests/unittest.cpp | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/tests/unittest.cpp b/tests/unittest.cpp index 568a30e1a..b2cf95c2d 100644 --- a/tests/unittest.cpp +++ b/tests/unittest.cpp @@ -2240,7 +2240,7 @@ TEST_CASE("stream_response") app.validate(); // running the test on a separate thread to allow the client to sleep - std::thread runTest([&app, &key_response, key_response_size]() { + std::thread runTest([&app, &key_response, key_response_size, keyword_]() { auto _ = app.bindaddr(LOCALHOST_ADDRESS).port(45451).run_async(); app.wait_for_server_start(); asio::io_service is; @@ -2257,15 +2257,21 @@ TEST_CASE("stream_response") asio::ip::address::from_string(LOCALHOST_ADDRESS), 45451)); c.send(asio::buffer(sendmsg)); - //consuming the headers, since we don't need those for the test + // consuming the headers, since we don't need those for the test static char buf[2048]; size_t received_headers_bytes = 0; - // magic number is 102 (it's the size of the headers, which is how much this line below needs to read) - const size_t headers_bytes = 102; - while (received_headers_bytes < headers_bytes) - received_headers_bytes += c.receive(asio::buffer(buf, 102)); - received += received_headers_bytes - headers_bytes; //add any extra that might have been received to the proper received count + // Magic number is 102. It's the size of the headers, which is at + // least how much we need to read. Since the header size may change + // and break the test, we read twice as much as the header and + // search in the received data for the first occurrence of keyword_. + const size_t headers_bytes_and_some = 102 * 2; + while (received_headers_bytes < headers_bytes_and_some) + received_headers_bytes += c.receive(asio::buffer(buf + received_headers_bytes, + sizeof(buf) / sizeof(buf[0]) - received_headers_bytes)); + + const std::string::size_type header_end_pos = std::string(buf, received_headers_bytes).find(keyword_); + received += received_headers_bytes - header_end_pos; // add any extra that might have been received to the proper received count while (received < key_response_size) { From 3fe1ee062ae2677152d6fbcf34171159ad4835c0 Mon Sep 17 00:00:00 2001 From: erik Date: Fri, 10 Jun 2022 12:48:04 +0200 Subject: [PATCH 2/3] Fixed base64 test to not depend on terminating null-characters in binary strings std::strings with binary data should always be created with the constructor taking a char pointer and the length of the character data. --- tests/unittest.cpp | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/tests/unittest.cpp b/tests/unittest.cpp index b2cf95c2d..5bf838e04 100644 --- a/tests/unittest.cpp +++ b/tests/unittest.cpp @@ -2921,12 +2921,18 @@ TEST_CASE("blueprint") TEST_CASE("base64") { unsigned char sample_bin[] = {0x14, 0xfb, 0x9c, 0x03, 0xd9, 0x7e}; + std::string sample_bin_str(reinterpret_cast(sample_bin), + sizeof(sample_bin) / sizeof(sample_bin[0])); std::string sample_bin_enc = "FPucA9l+"; std::string sample_bin_enc_url = "FPucA9l-"; unsigned char sample_bin1[] = {0x14, 0xfb, 0x9c, 0x03, 0xd9}; + std::string sample_bin1_str(reinterpret_cast(sample_bin1), + sizeof(sample_bin1) / sizeof(sample_bin1[0])); std::string sample_bin1_enc = "FPucA9k="; std::string sample_bin1_enc_np = "FPucA9k"; unsigned char sample_bin2[] = {0x14, 0xfb, 0x9c, 0x03}; + std::string sample_bin2_str(reinterpret_cast(sample_bin2), + sizeof(sample_bin2) / sizeof(sample_bin2[0])); std::string sample_bin2_enc = "FPucAw=="; std::string sample_bin2_enc_np = "FPucAw"; std::string sample_text = "Crow Allows users to handle requests that may not come from the network. This is done by calling the handle(req, res) method and providing a request and response objects. Which causes crow to identify and run the appropriate handler, returning the resulting response."; @@ -2934,20 +2940,19 @@ TEST_CASE("base64") CHECK(crow::utility::base64encode(sample_text, sample_text.length()) == sample_base64); - CHECK(crow::utility::base64encode((unsigned char*)sample_bin, 6) == sample_bin_enc); - CHECK(crow::utility::base64encode_urlsafe((unsigned char*)sample_bin, 6) == sample_bin_enc_url); - CHECK(crow::utility::base64encode((unsigned char*)sample_bin1, 5) == sample_bin1_enc); - CHECK(crow::utility::base64encode((unsigned char*)sample_bin2, 4) == sample_bin2_enc); - + CHECK(crow::utility::base64encode(sample_bin, 6) == sample_bin_enc); + CHECK(crow::utility::base64encode_urlsafe(sample_bin, 6) == sample_bin_enc_url); + CHECK(crow::utility::base64encode(sample_bin1, 5) == sample_bin1_enc); + CHECK(crow::utility::base64encode(sample_bin2, 4) == sample_bin2_enc); CHECK(crow::utility::base64decode(sample_base64) == sample_text); CHECK(crow::utility::base64decode(sample_base64, sample_base64.length()) == sample_text); - CHECK(crow::utility::base64decode(sample_bin_enc, 8) == std::string(reinterpret_cast(sample_bin))); - CHECK(crow::utility::base64decode(sample_bin_enc_url, 8) == std::string(reinterpret_cast(sample_bin))); - CHECK(crow::utility::base64decode(sample_bin1_enc, 8) == std::string(reinterpret_cast(sample_bin1)).substr(0, 5)); - CHECK(crow::utility::base64decode(sample_bin1_enc_np, 7) == std::string(reinterpret_cast(sample_bin1)).substr(0, 5)); - CHECK(crow::utility::base64decode(sample_bin2_enc, 8) == std::string(reinterpret_cast(sample_bin2)).substr(0, 4)); - CHECK(crow::utility::base64decode(sample_bin2_enc_np, 6) == std::string(reinterpret_cast(sample_bin2)).substr(0, 4)); + CHECK(crow::utility::base64decode(sample_bin_enc, 8) == sample_bin_str); + CHECK(crow::utility::base64decode(sample_bin_enc_url, 8) == sample_bin_str); + CHECK(crow::utility::base64decode(sample_bin1_enc, 8) == sample_bin1_str); + CHECK(crow::utility::base64decode(sample_bin1_enc_np, 7) == sample_bin1_str); + CHECK(crow::utility::base64decode(sample_bin2_enc, 8) == sample_bin2_str); + CHECK(crow::utility::base64decode(sample_bin2_enc_np, 6) == sample_bin2_str); } // base64 TEST_CASE("sanitize_filename") From 3782044249b4daf1172a2d4f8a13e1cdc0f64070 Mon Sep 17 00:00:00 2001 From: erik Date: Mon, 13 Jun 2022 10:53:27 +0200 Subject: [PATCH 3/3] Fixed race condition: calling stop() immediately after async_run() and wait_for_server_start() could lead to deadlock This is the case in some short, simple unit tests, such as "get_port". --- include/crow/app.h | 31 +++++++++++++++++++------------ include/crow/http_server.h | 20 ++++++++++++++++++++ 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/include/crow/app.h b/include/crow/app.h index 73060a669..0657a893e 100644 --- a/include/crow/app.h +++ b/include/crow/app.h @@ -293,14 +293,6 @@ namespace crow } } - /// Notify anything using `wait_for_server_start()` to proceed - void notify_server_start() - { - std::unique_lock lock(start_mutex_); - server_started_ = true; - cv_started_.notify_all(); - } - /// Run the server void run() { @@ -492,10 +484,17 @@ namespace crow /// Wait until the server has properly started void wait_for_server_start() { - std::unique_lock lock(start_mutex_); - if (server_started_) - return; - cv_started_.wait(lock); + { + std::unique_lock lock(start_mutex_); + if (!server_started_) + cv_started_.wait(lock); + } + if (server_) + server_->wait_for_start(); +#ifdef CROW_ENABLE_SSL + else if (ssl_server_) + ssl_server_->wait_for_start(); +#endif } private: @@ -508,6 +507,14 @@ namespace crow black_magic::tuple_extract(fwd))...); } + /// Notify anything using `wait_for_server_start()` to proceed + void notify_server_start() + { + std::unique_lock lock(start_mutex_); + server_started_ = true; + cv_started_.notify_all(); + } + private: std::uint8_t timeout_{5}; diff --git a/include/crow/http_server.h b/include/crow/http_server.h index e674bf83c..faaf14f57 100644 --- a/include/crow/http_server.h +++ b/include/crow/http_server.h @@ -153,6 +153,7 @@ namespace crow std::thread( [this] { + notify_start(); io_service_.run(); CROW_LOG_INFO << "Exiting."; }) @@ -175,6 +176,14 @@ namespace crow io_service_.stop(); // Close main io_service } + /// Wait until the server has properly started + void wait_for_start() + { + std::unique_lock lock(start_mutex_); + if (!server_started_) + cv_started_.wait(lock); + } + void signal_clear() { signals_.clear(); @@ -236,6 +245,14 @@ namespace crow } } + /// Notify anything using `wait_for_start()` to proceed + void notify_start() + { + std::unique_lock lock(start_mutex_); + server_started_ = true; + cv_started_.notify_all(); + } + private: asio::io_service io_service_; std::vector> io_service_pool_; @@ -243,6 +260,9 @@ namespace crow std::vector> get_cached_date_str_pool_; tcp::acceptor acceptor_; bool shutting_down_ = false; + bool server_started_{false}; + std::condition_variable cv_started_; + std::mutex start_mutex_; boost::asio::signal_set signals_; boost::asio::deadline_timer tick_timer_;