diff --git a/src/session/download_storer.cc b/src/session/download_storer.cc index 1464a906..53f6f553 100644 --- a/src/session/download_storer.cc +++ b/src/session/download_storer.cc @@ -2,6 +2,7 @@ #include "download_storer.h" +#include #include #include #include @@ -116,28 +117,36 @@ is_correct_format(const std::string& f) { void save_stream(const std::string& path, bool use_fsyncdisk, const std::stringstream& stream) { - std::fstream output(path.c_str(), std::ios::out | std::ios::trunc); + // Remove any leftover temporary file first so that O_EXCL only ever fails on + // an entry that appeared after the unlink, and O_NOFOLLOW keeps a symlink + // planted in the session directory from redirecting the write. + if (::unlink(path.c_str()) == -1 && errno != ENOENT) + throw torrent::storage_error("failed to remove stale file : " + path); // TODO: If we cannot open more files, wait for some to finish and try again. - if (!output.is_open()) - throw torrent::storage_error("failed to open file for writing : " + path); - - output << stream.rdbuf(); - - if (!output.good()) - throw torrent::storage_error("failed to write stream to file : " + path); - - // The data only reaches the kernel here, so this is where a full disk is seen. - output.close(); - - if (!output.good()) - throw torrent::storage_error("failed to flush stream to file : " + path); - - // Ensure that the new file is actually written to the disk - int fd = ::open(path.c_str(), O_WRONLY); + int fd = ::open(path.c_str(), O_WRONLY | O_CREAT | O_EXCL | O_NOFOLLOW, 0600); if (fd < 0) - throw torrent::storage_error("failed to open file descriptor for fsync : " + path); + throw torrent::storage_error("failed to open file for writing : " + path); + + const auto data = stream.view(); + std::size_t remaining = data.size(); + const char* cursor = data.data(); + + while (remaining != 0) { + ssize_t result = ::write(fd, cursor, remaining); + + if (result == -1) { + if (errno == EINTR) + continue; + + ::close(fd); + throw torrent::storage_error("failed to write stream to file : " + path); + } + + cursor += result; + remaining -= result; + } if (use_fsyncdisk) { #ifdef __APPLE__ @@ -152,6 +161,7 @@ save_stream(const std::string& path, bool use_fsyncdisk, const std::stringstream } } + // A full disk may only be seen when the descriptor is closed. if (::close(fd) == -1) throw torrent::storage_error("failed to close file descriptor : " + path); } diff --git a/src/utils/directory.cc b/src/utils/directory.cc index 6233a071..2a8296a6 100644 --- a/src/utils/directory.cc +++ b/src/utils/directory.cc @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -13,6 +14,24 @@ namespace utils { +namespace { + +uint8_t +entry_type_from_mode(mode_t mode) { + if (S_ISREG(mode)) + return DT_REG; + + if (S_ISDIR(mode)) + return DT_DIR; + + if (S_ISLNK(mode)) + return DT_LNK; + + return DT_UNKNOWN; +} + +} // namespace + // Keep this? bool Directory::is_valid() const { @@ -38,9 +57,6 @@ Directory::update(int flags) { return false; struct dirent* entry; -#ifdef __sun__ - struct stat s; -#endif while ((entry = readdir(d)) != NULL) { if ((flags & update_hide_dot) && entry->d_name[0] == '.') @@ -49,16 +65,22 @@ Directory::update(int flags) { iterator itr = base_type::insert(end(), value_type()); #ifdef __sun__ - stat(entry->d_name, &s); itr->s_fileno = entry->d_ino; itr->s_reclen = 0; - itr->s_type = s.st_mode; + itr->s_type = DT_UNKNOWN; #else itr->s_fileno = entry->d_fileno; itr->s_reclen = entry->d_reclen; itr->s_type = entry->d_type; #endif + if (itr->s_type == DT_UNKNOWN) { + struct stat st; + + if (fstatat(dirfd(d), entry->d_name, &st, AT_SYMLINK_NOFOLLOW) == 0) + itr->s_type = entry_type_from_mode(st.st_mode); + } + #ifdef DIRENT_NAMLEN_EXISTS_FOOBAR itr->s_name = std::string(entry->d_name, entry->d_name + entry->d_namlen); #else diff --git a/src/utils/directory.h b/src/utils/directory.h index 17602ab3..70612bfa 100644 --- a/src/utils/directory.h +++ b/src/utils/directory.h @@ -2,14 +2,14 @@ #define RTORRENT_UTILS_DIRECTORY_H #include +#include #include #include namespace utils { struct directory_entry { - // Fix. - bool is_file() const { return true; } + bool is_file() const { return s_type == DT_REG; } // The name and types should match POSIX. uint32_t s_fileno; diff --git a/test/Makefile.am b/test/Makefile.am index 0a6f3897..80d5e0a5 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -69,6 +69,8 @@ rtorrent_Test_Src_SOURCES = $(rtorrent_Test_Common) \ src/test_command_string.h \ src/test_command_throttle.cc \ src/test_command_throttle.h \ + src/test_session_storer.cc \ + src/test_session_storer.h \ src/test_setup.cc \ src/test_setup.h \ src/test_ui_download_list.cc \ diff --git a/test/src/test_session_storer.cc b/test/src/test_session_storer.cc new file mode 100644 index 00000000..1febdb4f --- /dev/null +++ b/test/src/test_session_storer.cc @@ -0,0 +1,152 @@ +#include "config.h" + +#include "test/src/test_session_storer.h" + +#include +#include +#include +#include +#include +#include + +#include "session/download_storer.h" +#include "utils/directory.h" + +CPPUNIT_TEST_SUITE_REGISTRATION(TestSessionStorer); + +namespace { + +const char* entry_name = "0123456789ABCDEF0123456789ABCDEF01234567.torrent"; +const char* link_name = "FEDCBA9876543210FEDCBA9876543210FEDCBA98.torrent"; + +void +write_file(const std::string& path, const std::string& content) { + std::ofstream file(path.c_str()); + + file << content; + file.close(); + + CPPUNIT_ASSERT(file.good()); +} + +std::string +read_file(const std::string& path) { + std::ifstream file(path.c_str()); + std::stringstream buffer; + + buffer << file.rdbuf(); + return buffer.str(); +} + +void +save_session_files(const std::string& path) { + std::stringstream torrent_stream("torrent-data"); + std::stringstream rtorrent_stream("rtorrent-data"); + std::stringstream libtorrent_stream("libtorrent-data"); + + session::DownloadStorer::save_and_move_streams(path, false, &torrent_stream, &rtorrent_stream, &libtorrent_stream); +} + +unsigned int +permissions_of(const std::string& path) { + struct stat st; + + CPPUNIT_ASSERT_EQUAL(0, ::stat(path.c_str(), &st)); + return st.st_mode & 07777; +} + +void +remove_directory(const std::string& path) { + DIR* d = ::opendir(path.c_str()); + + if (d == NULL) + return; + + struct dirent* entry; + + while ((entry = ::readdir(d)) != NULL) { + if (entry->d_name[0] == '.' && (entry->d_name[1] == '\0' || (entry->d_name[1] == '.' && entry->d_name[2] == '\0'))) + continue; + + ::unlink((path + "/" + entry->d_name).c_str()); + } + + ::closedir(d); + ::rmdir(path.c_str()); +} + +} // namespace + +void +TestSessionStorer::setUp() { + test_fixture::setUp(); + + char temp_dir[] = "/tmp/rtorrent_test_session_XXXXXX"; + + CPPUNIT_ASSERT(mkdtemp(temp_dir) != nullptr); + + m_temp_dir = temp_dir; + m_session_dir = m_temp_dir + "/session"; + + CPPUNIT_ASSERT_EQUAL(0, ::mkdir(m_session_dir.c_str(), 0755)); +} + +void +TestSessionStorer::tearDown() { + remove_directory(m_session_dir); + remove_directory(m_temp_dir); + + test_fixture::tearDown(); +} + +// A symlink planted where the next temporary session file will be written must +// not redirect the write to the file it points at. +void +TestSessionStorer::test_temp_file_symlink_is_not_followed() { + auto outside = m_temp_dir + "/outside.txt"; + auto path = m_session_dir + "/" + entry_name; + + write_file(outside, "original"); + CPPUNIT_ASSERT_EQUAL(0, ::symlink(outside.c_str(), (path + ".new").c_str())); + + save_session_files(path); + + CPPUNIT_ASSERT_EQUAL(std::string("original"), read_file(outside)); + CPPUNIT_ASSERT_EQUAL(std::string("torrent-data"), read_file(path)); +} + +// Session files carry tracker announce urls, so they must not be readable by +// other users regardless of the umask rtorrent was started with. +void +TestSessionStorer::test_saved_files_are_owner_only() { + auto path = m_session_dir + "/" + entry_name; + auto prev_umask = ::umask(0); + + save_session_files(path); + ::umask(prev_umask); + + CPPUNIT_ASSERT_EQUAL(0600u, permissions_of(path)); + CPPUNIT_ASSERT_EQUAL(0600u, permissions_of(path + ".rtorrent")); + CPPUNIT_ASSERT_EQUAL(0600u, permissions_of(path + ".libtorrent_resume")); +} + +// Entries listed for loading must report their real type so that a symlink in +// the session directory is skipped instead of loaded. +void +TestSessionStorer::test_symlinked_entry_is_not_a_file() { + write_file(m_temp_dir + "/outside.txt", "d0:e"); + write_file(m_session_dir + "/" + entry_name, "d0:e"); + + CPPUNIT_ASSERT_EQUAL(0, ::symlink((m_temp_dir + "/outside.txt").c_str(), (m_session_dir + "/" + link_name).c_str())); + + auto entries = session::DownloadStorer::get_formated_entries(m_session_dir + "/"); + + CPPUNIT_ASSERT_EQUAL(size_t{2}, size_t{entries.size()}); + + for (const auto& entry : entries) { + if (entry.s_name == entry_name) + CPPUNIT_ASSERT(entry.is_file()); + else + CPPUNIT_ASSERT(!entry.is_file()); + } +} diff --git a/test/src/test_session_storer.h b/test/src/test_session_storer.h new file mode 100644 index 00000000..34079950 --- /dev/null +++ b/test/src/test_session_storer.h @@ -0,0 +1,25 @@ +#include "test/helpers/test_fixture.h" + +#include + +class TestSessionStorer : public test_fixture { + CPPUNIT_TEST_SUITE(TestSessionStorer); + + CPPUNIT_TEST(test_temp_file_symlink_is_not_followed); + CPPUNIT_TEST(test_saved_files_are_owner_only); + CPPUNIT_TEST(test_symlinked_entry_is_not_a_file); + + CPPUNIT_TEST_SUITE_END(); + +public: + void setUp(); + void tearDown(); + + void test_temp_file_symlink_is_not_followed(); + void test_saved_files_are_owner_only(); + void test_symlinked_entry_is_not_a_file(); + +private: + std::string m_temp_dir; + std::string m_session_dir; +};