From 283f73c2f001da7044f0383ed725b3d6709b548a Mon Sep 17 00:00:00 2001 From: Alexander Dahl Date: Thu, 7 Apr 2022 15:44:57 +0200 Subject: [PATCH 1/4] http_request: Remove uploaded files in destructor If the user needs those files, she should copy/move theme in the request handler. References: #264 Signed-off-by: Alexander Dahl --- src/http_request.cpp | 9 +++++++++ src/httpserver/http_request.hpp | 5 +++++ 2 files changed, 14 insertions(+) diff --git a/src/http_request.cpp b/src/http_request.cpp index 3d88a382..195dddbc 100644 --- a/src/http_request.cpp +++ b/src/http_request.cpp @@ -256,4 +256,13 @@ std::ostream &operator<< (std::ostream &os, const http_request &r) { return os; } +http_request::~http_request() { + for ( const auto &file_key : this->get_files() ) { + for ( const auto &files : file_key.second ) { + // C++17 has std::filesystem::remove() + remove(files.second.get_file_system_file_name().c_str()); + } + } +} + } // namespace httpserver diff --git a/src/httpserver/http_request.hpp b/src/httpserver/http_request.hpp index 739de326..560f76c5 100644 --- a/src/httpserver/http_request.hpp +++ b/src/httpserver/http_request.hpp @@ -46,6 +46,8 @@ struct MHD_Connection; namespace httpserver { +namespace details { struct modded_request; } + /** * Class representing an abstraction for an Http Request. It is used from classes using these apis to receive information through http protocol. **/ @@ -254,6 +256,8 @@ class http_request { http_request& operator=(const http_request& b) = default; http_request& operator=(http_request&& b) = default; + ~http_request(); + std::string path; std::string method; std::map args; @@ -356,6 +360,7 @@ class http_request { const std::map get_headerlike_values(enum MHD_ValueKind kind) const; friend class webserver; + friend struct details::modded_request; }; std::ostream &operator<< (std::ostream &os, const http_request &r); From a19cf96bd2a4d3b62098744bf18b0c6701a79aeb Mon Sep 17 00:00:00 2001 From: Alexander Dahl Date: Fri, 8 Apr 2022 12:27:06 +0200 Subject: [PATCH 2/4] test: integ: file_upload: Remove redundant file delete Not needed anymore. http_request destructor does cleanup now. --- test/integ/file_upload.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/test/integ/file_upload.cpp b/test/integ/file_upload.cpp index 00cbd9ab..07aad7f8 100644 --- a/test/integ/file_upload.cpp +++ b/test/integ/file_upload.cpp @@ -216,7 +216,6 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk) httpserver::http::http_utils::upload_filename_template; LT_CHECK_EQ(file->second.get_file_system_file_name().substr(0, file->second.get_file_system_file_name().size() - 6), expected_filename.substr(0, expected_filename.size() - 6)); - unlink(file->second.get_file_system_file_name().c_str()); ws->stop(); delete ws; @@ -301,7 +300,6 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk_additional_par httpserver::http::http_utils::upload_filename_template; LT_CHECK_EQ(file->second.get_file_system_file_name().substr(0, file->second.get_file_system_file_name().size() - 6), expected_filename.substr(0, expected_filename.size() - 6)); - unlink(file->second.get_file_system_file_name().c_str()); ws->stop(); delete ws; @@ -355,7 +353,6 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk_two_files) httpserver::http::http_utils::upload_filename_template; LT_CHECK_EQ(file->second.get_file_system_file_name().substr(0, file->second.get_file_system_file_name().size() - 6), expected_filename.substr(0, expected_filename.size() - 6)); - unlink(file->second.get_file_system_file_name().c_str()); file_key++; LT_CHECK_EQ(file_key->first, TEST_KEY_2); @@ -370,7 +367,6 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk_two_files) httpserver::http::http_utils::upload_filename_template; LT_CHECK_EQ(file->second.get_file_system_file_name().substr(0, file->second.get_file_system_file_name().size() - 6), expected_filename.substr(0, expected_filename.size() - 6)); - unlink(file->second.get_file_system_file_name().c_str()); ws->stop(); @@ -416,7 +412,6 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_disk_only) httpserver::http::http_utils::upload_filename_template; LT_CHECK_EQ(file->second.get_file_system_file_name().substr(0, file->second.get_file_system_file_name().size() - 6), expected_filename.substr(0, expected_filename.size() - 6)); - unlink(file->second.get_file_system_file_name().c_str()); ws->stop(); delete ws; From a74cc132c5cbcc51d1c45edff6c2aab3b1751f2c Mon Sep 17 00:00:00 2001 From: Alexander Dahl Date: Fri, 8 Apr 2022 12:55:20 +0200 Subject: [PATCH 3/4] test: integ: file_upload: Check uploaded files are deleted --- test/integ/file_upload.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/integ/file_upload.cpp b/test/integ/file_upload.cpp index 07aad7f8..5e21d942 100644 --- a/test/integ/file_upload.cpp +++ b/test/integ/file_upload.cpp @@ -59,6 +59,12 @@ static size_t TEST_CONTENT_SIZE_2 = 28; static const char* TEST_PARAM_KEY = "param_key"; static const char* TEST_PARAM_VALUE = "Value of test param"; +static bool file_exists(const string &path) { + struct stat sb; + + return (stat(path.c_str(), &sb) == 0); +} + static CURLcode send_file_to_webserver(bool add_second_file, bool append_parameters) { curl_global_init(CURL_GLOBAL_ALL); @@ -216,6 +222,7 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk) httpserver::http::http_utils::upload_filename_template; LT_CHECK_EQ(file->second.get_file_system_file_name().substr(0, file->second.get_file_system_file_name().size() - 6), expected_filename.substr(0, expected_filename.size() - 6)); + LT_CHECK_EQ(file_exists(file->second.get_file_system_file_name()), false); ws->stop(); delete ws; @@ -300,6 +307,7 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk_additional_par httpserver::http::http_utils::upload_filename_template; LT_CHECK_EQ(file->second.get_file_system_file_name().substr(0, file->second.get_file_system_file_name().size() - 6), expected_filename.substr(0, expected_filename.size() - 6)); + LT_CHECK_EQ(file_exists(file->second.get_file_system_file_name()), false); ws->stop(); delete ws; @@ -353,6 +361,7 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk_two_files) httpserver::http::http_utils::upload_filename_template; LT_CHECK_EQ(file->second.get_file_system_file_name().substr(0, file->second.get_file_system_file_name().size() - 6), expected_filename.substr(0, expected_filename.size() - 6)); + LT_CHECK_EQ(file_exists(file->second.get_file_system_file_name()), false); file_key++; LT_CHECK_EQ(file_key->first, TEST_KEY_2); @@ -367,6 +376,7 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk_two_files) httpserver::http::http_utils::upload_filename_template; LT_CHECK_EQ(file->second.get_file_system_file_name().substr(0, file->second.get_file_system_file_name().size() - 6), expected_filename.substr(0, expected_filename.size() - 6)); + LT_CHECK_EQ(file_exists(file->second.get_file_system_file_name()), false); ws->stop(); @@ -412,6 +422,7 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_disk_only) httpserver::http::http_utils::upload_filename_template; LT_CHECK_EQ(file->second.get_file_system_file_name().substr(0, file->second.get_file_system_file_name().size() - 6), expected_filename.substr(0, expected_filename.size() - 6)); + LT_CHECK_EQ(file_exists(file->second.get_file_system_file_name()), false); ws->stop(); delete ws; From 0c18064c7c7597bf8e30818f3a6ab38e3d8b4b9d Mon Sep 17 00:00:00 2001 From: Alexander Dahl Date: Fri, 8 Apr 2022 15:15:55 +0200 Subject: [PATCH 4/4] test: integ: file_upload: Stop webserver earlier The webserver runs in a different thread than the test, therefor deleting the uploaded files and checking if they are deleted might lead into a race condition, and thus into tests failing sometimes, but not always. Moving the webserver stop and destructor behind the curl action should be safe, every information needed for the test is copied to or in the resource handler. --- test/integ/file_upload.cpp | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/test/integ/file_upload.cpp b/test/integ/file_upload.cpp index 5e21d942..ca506d97 100644 --- a/test/integ/file_upload.cpp +++ b/test/integ/file_upload.cpp @@ -197,6 +197,9 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk) CURLcode res = send_file_to_webserver(false, false); LT_ASSERT_EQ(res, 0); + ws->stop(); + delete ws; + string actual_content = resource.get_content(); LT_CHECK_EQ(actual_content.find(FILENAME_IN_GET_CONTENT) != string::npos, true); LT_CHECK_EQ(actual_content.find(TEST_CONTENT) != string::npos, true); @@ -223,9 +226,6 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk) LT_CHECK_EQ(file->second.get_file_system_file_name().substr(0, file->second.get_file_system_file_name().size() - 6), expected_filename.substr(0, expected_filename.size() - 6)); LT_CHECK_EQ(file_exists(file->second.get_file_system_file_name()), false); - - ws->stop(); - delete ws; LT_END_AUTO_TEST(file_upload_memory_and_disk) LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk_via_put) @@ -277,6 +277,9 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk_additional_par CURLcode res = send_file_to_webserver(false, true); LT_ASSERT_EQ(res, 0); + ws->stop(); + delete ws; + string actual_content = resource.get_content(); LT_CHECK_EQ(actual_content.find(FILENAME_IN_GET_CONTENT) != string::npos, true); LT_CHECK_EQ(actual_content.find(TEST_CONTENT) != string::npos, true); @@ -308,9 +311,6 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk_additional_par LT_CHECK_EQ(file->second.get_file_system_file_name().substr(0, file->second.get_file_system_file_name().size() - 6), expected_filename.substr(0, expected_filename.size() - 6)); LT_CHECK_EQ(file_exists(file->second.get_file_system_file_name()), false); - - ws->stop(); - delete ws; LT_END_AUTO_TEST(file_upload_memory_and_disk_additional_params) LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk_two_files) @@ -331,6 +331,9 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk_two_files) CURLcode res = send_file_to_webserver(true, false); LT_ASSERT_EQ(res, 0); + ws->stop(); + delete ws; + string actual_content = resource.get_content(); LT_CHECK_EQ(actual_content.find(FILENAME_IN_GET_CONTENT) != string::npos, true); LT_CHECK_EQ(actual_content.find(TEST_CONTENT) != string::npos, true); @@ -377,10 +380,6 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_and_disk_two_files) LT_CHECK_EQ(file->second.get_file_system_file_name().substr(0, file->second.get_file_system_file_name().size() - 6), expected_filename.substr(0, expected_filename.size() - 6)); LT_CHECK_EQ(file_exists(file->second.get_file_system_file_name()), false); - - - ws->stop(); - delete ws; LT_END_AUTO_TEST(file_upload_memory_and_disk_two_files) LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_disk_only) @@ -401,6 +400,9 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_disk_only) CURLcode res = send_file_to_webserver(false, false); LT_ASSERT_EQ(res, 0); + ws->stop(); + delete ws; + string actual_content = resource.get_content(); LT_CHECK_EQ(actual_content.size(), 0); @@ -423,9 +425,6 @@ LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_disk_only) LT_CHECK_EQ(file->second.get_file_system_file_name().substr(0, file->second.get_file_system_file_name().size() - 6), expected_filename.substr(0, expected_filename.size() - 6)); LT_CHECK_EQ(file_exists(file->second.get_file_system_file_name()), false); - - ws->stop(); - delete ws; LT_END_AUTO_TEST(file_upload_disk_only) LT_BEGIN_AUTO_TEST(file_upload_suite, file_upload_memory_only_incl_content)