From 5026659cb6c0dd26ed94b691662f1ec54c9356d4 Mon Sep 17 00:00:00 2001
From: arvidn <arvid@libtorrent.org>
Date: Sun, 1 Oct 2017 03:17:10 +0200
Subject: [PATCH] clean up and fix edge cases in update_path_index

---
 include/libtorrent/file_storage.hpp |  1 +
 src/file_storage.cpp                | 31 ++++++++++++++++-------------
 test/test_file_storage.cpp          | 27 +++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/include/libtorrent/file_storage.hpp b/include/libtorrent/file_storage.hpp
index 282bc844a..563ec63f3 100644
--- a/include/libtorrent/file_storage.hpp
+++ b/include/libtorrent/file_storage.hpp
@@ -509,6 +509,7 @@ namespace libtorrent {
 		// low-level function. returns a pointer to the internal storage for
 		// the filename. This string may not be 0-terminated!
 		// the ``file_name_len()`` function returns the length of the filename.
+		// prefer to use ``file_name()`` instead, which returns a ``string_view``.
 		char const* file_name_ptr(file_index_t index) const;
 		int file_name_len(file_index_t index) const;
 
diff --git a/src/file_storage.cpp b/src/file_storage.cpp
index f968b59a0..c258b0497 100644
--- a/src/file_storage.cpp
+++ b/src/file_storage.cpp
@@ -131,34 +131,37 @@ namespace {
 		// sorry about this messy string handling, but I did
 		// profile it, and it was expensive
 		char const* leaf = filename_cstr(path.c_str());
-		char const* branch_path = "";
-		int branch_len = 0;
+		string_view branch_path;
 		if (leaf > path.c_str())
 		{
 			// split the string into the leaf filename
 			// and the branch path
-			branch_path = path.c_str();
-			branch_len = int(leaf - path.c_str());
+			branch_path = path;
+			branch_path = branch_path.substr(0
+				, static_cast<std::size_t>(leaf - path.c_str()));
 
 			// trim trailing slashes
-			if (branch_len > 0 && branch_path[branch_len - 1] == TORRENT_SEPARATOR)
-				--branch_len;
+			while (!branch_path.empty() && branch_path.back() == TORRENT_SEPARATOR)
+			{
+				branch_path.remove_suffix(1);
+			}
 		}
-		if (branch_len <= 0)
+		if (branch_path.empty())
 		{
 			if (set_name) e.set_name(leaf);
 			e.path_index = -1;
 			return;
 		}
 
-		if (branch_len >= int(m_name.size())
-			&& std::memcmp(branch_path, m_name.c_str(), m_name.size()) == 0
+		if (branch_path.size() >= m_name.size()
+			&& branch_path.substr(0, m_name.size()) == m_name
 			&& branch_path[m_name.size()] == TORRENT_SEPARATOR)
 		{
-			int const offset = int(m_name.size())
-				+ (int(m_name.size()) == branch_len ? 0 : 1);
-			branch_path += offset;
-			branch_len -= offset;
+			branch_path.remove_prefix(m_name.size());
+			while (!branch_path.empty() && branch_path.front() == TORRENT_SEPARATOR)
+			{
+				branch_path.remove_prefix(1);
+			}
 			e.no_root_dir = false;
 		}
 		else
@@ -166,7 +169,7 @@ namespace {
 			e.no_root_dir = true;
 		}
 
-		e.path_index = get_or_add_path({branch_path, aux::numeric_cast<std::size_t>(branch_len)});
+		e.path_index = get_or_add_path(branch_path);
 		if (set_name) e.set_name(leaf);
 	}
 
diff --git a/test/test_file_storage.cpp b/test/test_file_storage.cpp
index e23ccd597..20911a0d0 100644
--- a/test/test_file_storage.cpp
+++ b/test/test_file_storage.cpp
@@ -183,6 +183,7 @@ TORRENT_TEST(pointer_offset)
 	// test filename_ptr and filename_len
 	TEST_EQUAL(st.file_name_ptr(file_index_t{0}), filename);
 	TEST_EQUAL(st.file_name_len(file_index_t{0}), 5);
+	TEST_EQUAL(st.file_name(file_index_t{0}), string_view(filename, 5));
 
 	TEST_EQUAL(st.file_path(file_index_t{0}, ""), combine_path("test-torrent-1", "test1"));
 	TEST_EQUAL(st.file_path(file_index_t{0}, "tmp"), combine_path("tmp"
@@ -202,6 +203,32 @@ TORRENT_TEST(pointer_offset)
 	TEST_EQUAL(st.file_name_len(file_index_t{0}), 5);
 }
 
+TORRENT_TEST(invalid_path1)
+{
+	file_storage st;
+#ifdef TORRENT_WINDOWS
+	st.add_file_borrow({}, R"(+\\\()", 10);
+#else
+	st.add_file_borrow({}, "+///(", 10);
+#endif
+
+	TEST_EQUAL(st.file_name(file_index_t{0}), "(");
+	TEST_EQUAL(st.file_path(file_index_t{0}, ""), combine_path("+", "("));
+}
+
+TORRENT_TEST(invalid_path2)
+{
+	file_storage st;
+#ifdef TORRENT_WINDOWS
+	st.add_file_borrow({}, R"(+\\\+\\()", 10);
+#else
+	st.add_file_borrow({}, "+///+//(", 10);
+#endif
+
+	TEST_EQUAL(st.file_name(file_index_t{0}), "(");
+	TEST_EQUAL(st.file_path(file_index_t{0}, ""), combine_path("+", combine_path("+", "(")));
+}
+
 TORRENT_TEST(map_file)
 {
 	// test map_file