diff --git a/fineftp-server/src/ftp_session.cpp b/fineftp-server/src/ftp_session.cpp index eda76dc..3be83a9 100755 --- a/fineftp-server/src/ftp_session.cpp +++ b/fineftp-server/src/ftp_session.cpp @@ -1289,6 +1289,12 @@ namespace fineftp { me->sendFtpMessage(FtpReplyCode::CLOSING_DATA_CONNECTION, "Done"); } + else if (file->data() == nullptr) + { + // Error that should never happen. If it does, it's a bug in the server. + // Usually, if the data is null, the file size should be 0. + me->sendFtpMessage(FtpReplyCode::TRANSFER_ABORTED, "Data transfer aborted: File data is null"); + } else { me->data_socket_weakptr_ = data_socket; diff --git a/fineftp-server/src/unix/file_man.cpp b/fineftp-server/src/unix/file_man.cpp index 4ae6d4c..08079e8 100644 --- a/fineftp-server/src/unix/file_man.cpp +++ b/fineftp-server/src/unix/file_man.cpp @@ -15,69 +15,73 @@ namespace fineftp { -namespace { - -std::mutex guard; -std::map> files; - -} // namespace - -ReadableFile::~ReadableFile() -{ - if (nullptr != data_) + namespace { - ::munmap(data_, size_); - } + std::mutex guard; + std::map> files; + } // namespace - const std::lock_guard lock{guard}; - if (!pth_.empty()) + ReadableFile::~ReadableFile() { - (void)files.erase(pth_); - } -} + if (nullptr != data_) + { + ::munmap(data_, size_); + } -std::shared_ptr ReadableFile::get(const std::string& pth) -{ - // See if we already have this file mapped - const std::lock_guard lock{guard}; - auto fit = files.find(pth); - if (files.end() != fit) - { - auto p = fit->second.lock(); - if (p) + const std::lock_guard lock{guard}; + if (!path_.empty()) { - return p; + (void)files.erase(path_); } } - - auto handle = ::open(pth.c_str(), O_RDONLY); - if (-1 == handle) - { - return {}; - } - struct stat st {}; - if (-1 == ::fstat(handle, &st)) + std::shared_ptr ReadableFile::get(const std::string& file_path) { - ::close(handle); - return {}; - } + // See if we already have this file mapped + const std::lock_guard lock{guard}; + auto existing_files_it = files.find(file_path); + if (files.end() != existing_files_it) + { + auto readable_file_ptr = existing_files_it->second.lock(); + if (readable_file_ptr) + { + return readable_file_ptr; + } + } - auto* map_start = ::mmap(nullptr, st.st_size, PROT_READ, MAP_SHARED, handle, 0); - if (MAP_FAILED == map_start) - { - ::close(handle); - return {}; - } + auto handle = ::open(file_path.c_str(), O_RDONLY); + if (-1 == handle) + { + return {}; + } - ::close(handle); + struct stat file_status {}; + if (-1 == ::fstat(handle, &file_status)) + { + ::close(handle); + return {}; + } - std::shared_ptr p{new ReadableFile{}}; - p->pth_ = pth; - p->size_ = st.st_size; - p->data_ = static_cast(map_start); - files[p->pth_] = p; - return p; -} + void* map_start = nullptr; + + if (file_status.st_size > 0) + { + // Only mmap file with a size > 0 + map_start = ::mmap(nullptr, file_status.st_size, PROT_READ, MAP_SHARED, handle, 0); + if (MAP_FAILED == map_start) + { + ::close(handle); + return {}; + } + } + ::close(handle); + + std::shared_ptr readable_file_ptr{new ReadableFile{}}; + readable_file_ptr->path_ = file_path; + readable_file_ptr->size_ = file_status.st_size; + readable_file_ptr->data_ = static_cast(map_start); + files[readable_file_ptr->path_] = readable_file_ptr; + return readable_file_ptr; + } } diff --git a/fineftp-server/src/unix/file_man.h b/fineftp-server/src/unix/file_man.h index f7bf93b..01a595f 100644 --- a/fineftp-server/src/unix/file_man.h +++ b/fineftp-server/src/unix/file_man.h @@ -28,10 +28,10 @@ class ReadableFile /// Retrieves the file at the specified path. /// - /// @param pth The path of the file. + /// @param file_path The path of the file. /// /// @param The requested file or nullptr if the file could not be retrieved. - static std::shared_ptr get(const std::string& pth); + static std::shared_ptr get(const std::string& file_path); /// Returns the size of the file. /// @@ -51,7 +51,7 @@ class ReadableFile private: ReadableFile() = default; - std::string pth_ = {}; + std::string path_ = {}; std::size_t size_ = {}; std::uint8_t* data_ = {}; }; @@ -118,7 +118,7 @@ inline const std::uint8_t* ReadableFile::data() const inline const std::string& ReadableFile::path() const { - return pth_; + return path_; } } diff --git a/fineftp-server/src/win32/file_man.cpp b/fineftp-server/src/win32/file_man.cpp index cded3b1..ce53c29 100644 --- a/fineftp-server/src/win32/file_man.cpp +++ b/fineftp-server/src/win32/file_man.cpp @@ -21,24 +21,26 @@ std::map> files; ReadableFile::~ReadableFile() { - if (INVALID_HANDLE_VALUE != handle_) - { + if (data_ != nullptr) ::UnmapViewOfFile(data_); + + if (map_handle_ != INVALID_HANDLE_VALUE) ::CloseHandle(map_handle_); + + if (handle_ != INVALID_HANDLE_VALUE) ::CloseHandle(handle_); - } const std::lock_guard lock{guard}; - if (!pth_.empty()) + if (!path_.empty()) { - (void)files.erase(pth_); + (void)files.erase(path_); } } -std::shared_ptr ReadableFile::get(const Str& pth) +std::shared_ptr ReadableFile::get(const Str& file_path) { std::basic_ostringstream os; - for (auto c : pth) + for (auto c : file_path) { if (c == '/') { @@ -50,62 +52,83 @@ std::shared_ptr ReadableFile::get(const Str& pth) } } - auto&& s = os.str(); + auto&& file_path_fixed_separators = os.str(); // See if we already have this file mapped const std::lock_guard lock{guard}; - auto fit = files.find(s); - if (files.end() != fit) + auto existing_files_it = files.find(file_path_fixed_separators); + if (files.end() != existing_files_it) { - auto p = fit->second.lock(); - if (p) + auto readable_file_ptr = existing_files_it->second.lock(); + if (readable_file_ptr) { - return p; + return readable_file_ptr; } } #if !defined(__GNUG__) - HANDLE handle = - ::CreateFileW(s.c_str(), GENERIC_READ, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); + HANDLE file_handle = + ::CreateFileW(file_path_fixed_separators.c_str(), GENERIC_READ, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); #else - auto handle = - ::CreateFileA(s.c_str(), GENERIC_READ, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); + auto file_handle = + ::CreateFileA(file_path_fixed_separators.c_str(), GENERIC_READ, FILE_SHARE_READ, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); #endif - if (INVALID_HANDLE_VALUE == handle) + if (INVALID_HANDLE_VALUE == file_handle) { return {}; } - LARGE_INTEGER sz; - if (0 == ::GetFileSizeEx(handle, &sz)) + // Get the file size by Using GetFileInformationByHandle + BY_HANDLE_FILE_INFORMATION file_info; + if (0 == ::GetFileInformationByHandle(file_handle, &file_info)) { - ::CloseHandle(handle); + ::CloseHandle(file_handle); return {}; } - - auto* map_handle = ::CreateFileMapping(handle, nullptr, PAGE_READONLY, sz.HighPart, sz.LowPart, nullptr); - if ((map_handle == INVALID_HANDLE_VALUE) || (map_handle == nullptr)) - { - ::CloseHandle(handle); - return {}; + LARGE_INTEGER file_size; + file_size.LowPart = file_info.nFileSizeLow; + file_size.HighPart = file_info.nFileSizeHigh; + + // Create new ReadableFile ptr + std::shared_ptr readable_file_ptr(new ReadableFile{}); + + if (file_size.QuadPart == 0) + { + // Handle zero-size files + readable_file_ptr->path_ = std::move(file_path_fixed_separators); + readable_file_ptr->size_ = file_size.QuadPart; + readable_file_ptr->data_ = static_cast(nullptr); + readable_file_ptr->handle_ = file_handle; + readable_file_ptr->map_handle_ = INVALID_HANDLE_VALUE; } - - auto* map_start = ::MapViewOfFile(map_handle, FILE_MAP_READ, 0, 0, sz.QuadPart); - if (nullptr == map_start) + else { - ::CloseHandle(map_handle); - ::CloseHandle(handle); - return {}; + // Handle non-zero-size files + auto* map_handle = ::CreateFileMapping(file_handle, nullptr, PAGE_READONLY, file_size.HighPart, file_size.LowPart, nullptr); + if ((map_handle == INVALID_HANDLE_VALUE) || (map_handle == nullptr)) + { + ::CloseHandle(file_handle); + return {}; + } + + auto* map_start = ::MapViewOfFile(map_handle, FILE_MAP_READ, 0, 0, file_size.QuadPart); + if (nullptr == map_start) + { + ::CloseHandle(map_handle); + ::CloseHandle(file_handle); + return {}; + } + + readable_file_ptr->path_ = std::move(file_path_fixed_separators); + readable_file_ptr->size_ = file_size.QuadPart; + readable_file_ptr->data_ = static_cast(map_start); + readable_file_ptr->handle_ = file_handle; + readable_file_ptr->map_handle_ = map_handle; } - std::shared_ptr p{new ReadableFile{}}; - p->pth_ = std::move(s); - p->size_ = sz.QuadPart; - p->data_ = static_cast(map_start); - p->handle_ = handle; - p->map_handle_ = map_handle; - files[p->pth_] = p; - return p; + // Add readable_file_ptr to the map and return it to the user + files[readable_file_ptr->path_] = readable_file_ptr; + return readable_file_ptr; } WriteableFile::WriteableFile(const std::string& filename, std::ios::openmode mode) diff --git a/fineftp-server/src/win32/file_man.h b/fineftp-server/src/win32/file_man.h index 7b8ad12..8fc12f3 100644 --- a/fineftp-server/src/win32/file_man.h +++ b/fineftp-server/src/win32/file_man.h @@ -33,10 +33,10 @@ class ReadableFile /// Retrieves the file at the specified path. /// - /// @param pth The path of the file. + /// @param file_path The path of the file. /// /// @param The requested file or nullptr if the file could not be retrieved. - static std::shared_ptr get(const Str& pth); + static std::shared_ptr get(const Str& file_path); /// Returns the size of the file. /// @@ -56,7 +56,7 @@ class ReadableFile private: ReadableFile() = default; - Str pth_ = {}; + Str path_ = {}; std::size_t size_ = {}; std::uint8_t* data_ = {}; HANDLE handle_ = INVALID_HANDLE_VALUE; @@ -105,7 +105,7 @@ inline const std::uint8_t* ReadableFile::data() const inline const ReadableFile::Str& ReadableFile::path() const { - return pth_; + return path_; } inline bool WriteableFile::good() const diff --git a/fineftp-server/version.cmake b/fineftp-server/version.cmake index 7c441ad..730153e 100644 --- a/fineftp-server/version.cmake +++ b/fineftp-server/version.cmake @@ -1,3 +1,3 @@ set(FINEFTP_SERVER_VERSION_MAJOR 1) set(FINEFTP_SERVER_VERSION_MINOR 4) -set(FINEFTP_SERVER_VERSION_PATCH 1) +set(FINEFTP_SERVER_VERSION_PATCH 2) diff --git a/tests/fineftp_test/src/fineftp_stresstest.cpp b/tests/fineftp_test/src/fineftp_stresstest.cpp index 471f105..75087a9 100644 --- a/tests/fineftp_test/src/fineftp_stresstest.cpp +++ b/tests/fineftp_test/src/fineftp_stresstest.cpp @@ -101,6 +101,85 @@ TEST(FineFTPTest, SimpleUploadDownload) { } #endif +#if 1 +TEST(FineFTPTest, EmptyFile) +{ + const auto test_working_dir = std::filesystem::current_path(); + const auto ftp_root_dir = test_working_dir / "ftp_root"; + const auto local_root_dir = test_working_dir / "local_root"; + + if (std::filesystem::exists(ftp_root_dir)) + std::filesystem::remove_all(ftp_root_dir); + + if (std::filesystem::exists(local_root_dir)) + std::filesystem::remove_all(local_root_dir); + + // Make sure that we start clean, so no old dir exists + ASSERT_FALSE(std::filesystem::exists(ftp_root_dir)); + ASSERT_FALSE(std::filesystem::exists(local_root_dir)); + + std::filesystem::create_directory(ftp_root_dir); + std::filesystem::create_directory(local_root_dir); + + // Make sure that we were able to create the dir + ASSERT_TRUE(std::filesystem::is_directory(ftp_root_dir)); + ASSERT_TRUE(std::filesystem::is_directory(local_root_dir)); + + fineftp::FtpServer server(2121); + server.start(1); + + server.addUserAnonymous(ftp_root_dir.string(), fineftp::Permission::All); + + // Create an empty file in the local root dir + auto local_file = local_root_dir / "empty_file.txt"; + std::ofstream ofs(local_file.string()); + ofs.close(); + + // Make sure that the file exists + ASSERT_TRUE(std::filesystem::exists(local_file)); + ASSERT_TRUE(std::filesystem::is_regular_file(local_file)); + + // Upload the file to the FTP server using curl + { + const std::string curl_command = "curl -S -s -T \"" + local_file.string() + "\" \"ftp://localhost:2121/\""; + const auto curl_result = std::system(curl_command.c_str()); + + // Make sure that the upload was successful + ASSERT_EQ(curl_result, 0); + + // Make sure that the file exists in the FTP root dir + auto ftp_file = ftp_root_dir / "empty_file.txt"; + ASSERT_TRUE(std::filesystem::exists(ftp_file)); + ASSERT_TRUE(std::filesystem::is_regular_file(ftp_file)); + + // Make sure that the file is empty + std::ifstream ifs(ftp_file.string()); + const std::string content((std::istreambuf_iterator(ifs)), (std::istreambuf_iterator())); + ASSERT_TRUE(content.empty()); + } + + // Download the file again + { + const std::string curl_command_download = "curl -S -s -o \"" + local_root_dir.string() + "/empty_file_download.txt\" \"ftp://localhost:2121/empty_file.txt\""; + const auto curl_result = std::system(curl_command_download.c_str()); + ASSERT_EQ(curl_result, 0); + + // Make sure that the files are identical + auto local_file_download = local_root_dir / "empty_file_download.txt"; + + ASSERT_TRUE(std::filesystem::exists(local_file_download)); + ASSERT_TRUE(std::filesystem::is_regular_file(local_file_download)); + + std::ifstream ifs(local_file_download.string()); + const std::string content((std::istreambuf_iterator(ifs)), (std::istreambuf_iterator())); + ASSERT_TRUE(content.empty()); + } + + + // Stop the server +} +#endif + #if 1 TEST(FineFTPTest, BigFilesMultipleClients) {