make pad-file and symlink support conform to BEP47 (#992)

make pad-file and symlink support conform to BEP47

ChangeLog

@@ -1,5 +1,6 @@
 1.1.1 release
 
+	* make pad-file and symlink support conform to BEP47
 	* fix piece picker bug that could result in division by zero
 	* fix value of current_tracker when all tracker failed
 	* deprecate lt_trackers extension

include/libtorrent/bdecode.hpp

@@ -109,7 +109,7 @@ namespace bdecode_errors
 	{
 		// Not an error
 		no_error = 0,
-		// expected string in bencoded string
+		// expected digit in bencoded string
 		expected_digit,
 		// expected colon in bencoded string
 		expected_colon,

src/create_torrent.cpp

@@ -548,7 +548,7 @@ namespace libtorrent
 		{
 			if (m_include_mtime) info["mtime"] = m_files.mtime(0);
 			info["length"] = m_files.file_size(0);
-			int flags = m_files.file_flags(0);
+			int const flags = m_files.file_flags(0);
 			if (flags & (file_storage::flag_pad_file
 				| file_storage::flag_hidden
 				| file_storage::flag_executable

src/file_storage.cpp

@@ -47,8 +47,10 @@ POSSIBILITY OF SUCH DAMAGE.
 
 #if defined(TORRENT_WINDOWS) || defined(TORRENT_OS2)
 #define TORRENT_SEPARATOR '\\'
+#define TORRENT_SEPARATOR_STR "\\"
 #else
 #define TORRENT_SEPARATOR '/'
+#define TORRENT_SEPARATOR_STR "/"
 #endif
 
 namespace libtorrent
@@ -538,10 +540,10 @@ namespace libtorrent
 			, symlink_path);
 	}
 
-	void file_storage::add_file_borrow(char const* filename, int filename_len
+	void file_storage::add_file_borrow(char const* filename, int const filename_len
-		, std::string const& path, boost::int64_t file_size
+		, std::string const& path, boost::int64_t const file_size
-		, boost::uint32_t file_flags, char const* filehash
+		, boost::uint32_t const file_flags, char const* filehash
-		, boost::int64_t mtime, std::string const& symlink_path)
+		, boost::int64_t const mtime, std::string const& symlink_path)
 	{
 		TORRENT_ASSERT_PRECOND(file_size >= 0);
 		if (!has_parent_path(path))
@@ -576,10 +578,10 @@ namespace libtorrent
 
 		e.size = file_size;
 		e.offset = m_total_size;
-		e.pad_file = file_flags & file_storage::flag_pad_file;
+		e.pad_file = (file_flags & file_storage::flag_pad_file) != 0;
-		e.hidden_attribute = file_flags & file_storage::flag_hidden;
+		e.hidden_attribute = (file_flags & file_storage::flag_hidden) != 0;
-		e.executable_attribute = file_flags & file_storage::flag_executable;
+		e.executable_attribute = (file_flags & file_storage::flag_executable) != 0;
-		e.symlink_attribute = file_flags & file_storage::flag_symlink;
+		e.symlink_attribute = (file_flags & file_storage::flag_symlink) != 0;
 
 		if (filehash)
 		{
@@ -965,8 +967,8 @@ namespace libtorrent
 
 				if (best_match != i)
 				{
-					int index = best_match - m_files.begin();
+					int const index = best_match - m_files.begin();
-					int cur_index = i - m_files.begin();
+					int const cur_index = i - m_files.begin();
 					reorder_file(index, cur_index);
 					i = m_files.begin() + cur_index;
 				}
@@ -979,7 +981,7 @@ namespace libtorrent
 				// not piece-aligned and the file size exceeds the
 				// limit, and it's not a padding file itself.
 				// so add a padding file in front of it
-				int pad_size = alignment - (off % alignment);
+				int const pad_size = alignment - (off % alignment);
 
 				// find the largest file that fits in pad_size
 				std::vector<internal_file_entry>::iterator best_match = m_files.end();
@@ -1064,7 +1066,8 @@ namespace libtorrent
 		e.size = size;
 		e.offset = offset;
 		char name[30];
-		snprintf(name, sizeof(name), ".____padding_file/%d", pad_file_counter);
+		snprintf(name, sizeof(name), ".pad" TORRENT_SEPARATOR_STR "%d"
+			, pad_file_counter);
 		std::string path = combine_path(m_name, name);
 		e.set_name(path.c_str());
 		e.pad_file = true;

src/torrent_info.cpp

@@ -161,7 +161,8 @@ namespace libtorrent
 		return valid_encoding;
 	}
 
-	void sanitize_append_path_element(std::string& path, char const* element, int element_len)
+	void sanitize_append_path_element(std::string& path
+		, char const* element, int element_len)
 	{
 		if (element_len == 1 && element[0] == '.') return;
 
@@ -390,6 +391,46 @@ namespace libtorrent
 
 	namespace {
 
+	boost::uint32_t get_file_attributes(bdecode_node const& dict)
+	{
+		boost::uint32_t file_flags = 0;
+		bdecode_node attr = dict.dict_find_string("attr");
+		if (attr)
+		{
+			for (int i = 0; i < attr.string_length(); ++i)
+			{
+				switch (attr.string_ptr()[i])
+				{
+					case 'l': file_flags |= file_storage::flag_symlink; break;
+					case 'x': file_flags |= file_storage::flag_executable; break;
+					case 'h': file_flags |= file_storage::flag_hidden; break;
+					case 'p': file_flags |= file_storage::flag_pad_file; break;
+				}
+			}
+		}
+		return file_flags;
+	}
+
+	// iterates an array of strings and returns the sum of the lengths of all
+	// strings + one additional character per entry (to account for the presumed
+	// forward- or backslash to seaprate directory entries)
+	int path_length(bdecode_node const& p, error_code& ec)
+	{
+		int ret = 0;
+		int const len = p.list_size();
+		for (int i = 0; i < len; ++i)
+		{
+			bdecode_node e = p.list_at(i);
+			if (e.type() != bdecode_node::string_t)
+			{
+				ec = errors::torrent_invalid_name;
+				return -1;
+			}
+			ret += e.string_length();
+		}
+		return ret + len;
+	}
+
 	// 'top_level' is extracting the file for a single-file torrent. The
 	// distinction is that the filename is found in "name" rather than
 	// "path"
@@ -397,17 +438,24 @@ namespace libtorrent
 	// torrent, in which case it's empty.
 	bool extract_single_file(bdecode_node const& dict, file_storage& files
 		, std::string const& root_dir, ptrdiff_t info_ptr_diff, bool top_level
-		, error_code& ec)
+		, int& pad_file_cnt, error_code& ec)
 	{
 		if (dict.type() != bdecode_node::dict_t) return false;
-		boost::int64_t file_size = dict.dict_find_int_value("length", -1);
+
+		boost::uint32_t file_flags = get_file_attributes(dict);
+
+		// symlinks have an implied "size" of zero. i.e. they use up 0 bytes of
+		// the torrent payload space
+		boost::int64_t const file_size = (file_flags & file_storage::flag_symlink)
+			? 0
+			: dict.dict_find_int_value("length", -1);
 		if (file_size < 0 )
 		{
 			ec = errors::torrent_invalid_length;
 			return false;
 		}
 
-		boost::int64_t mtime = dict.dict_find_int_value("mtime", 0);
+		boost::int64_t const mtime = dict.dict_find_int_value("mtime", 0);
 
 		std::string path = root_dir;
 		std::string path_element;
@@ -434,23 +482,11 @@ namespace libtorrent
 		{
 			bdecode_node p = dict.dict_find_list("path.utf-8");
 			if (!p) p = dict.dict_find_list("path");
-			if (!p || p.list_size() == 0)
-			{
-				ec = errors::torrent_missing_name;
-				return false;
-			}
 
-			int preallocate = path.size();
+			if (p && p.list_size() > 0)
-			for (int i = 0, end(p.list_size()); i < end; ++i)
 			{
-				bdecode_node e = p.list_at(i);
+				int const preallocate = path.size() + path_length(p, ec);
-				if (e.type() != bdecode_node::string_t)
+				if (ec) return false;
-				{
-					ec = errors::torrent_missing_name;
-					return false;
-				}
-				preallocate += e.string_length() + 1;
-			}
 				path.reserve(preallocate);
 
 				for (int i = 0, end(p.list_size()); i < end; ++i)
@@ -464,41 +500,45 @@ namespace libtorrent
 					sanitize_append_path_element(path, e.string_ptr(), e.string_length());
 				}
 			}
+			else if (file_flags & file_storage::flag_pad_file)
+			{
+				// pad files don't need a path element, we'll just store them
+				// under the .pad directory
+				char cnt[10];
+				snprintf(cnt, sizeof(cnt), "%d", pad_file_cnt);
+				path = combine_path(".pad", cnt);
+				++pad_file_cnt;
+			}
+			else
+			{
+				ec = errors::torrent_missing_name;
+				return false;
+			}
+		}
 
 		// bitcomet pad file
-		boost::uint32_t file_flags = 0;
 		if (path.find("_____padding_file_") != std::string::npos)
 			file_flags = file_storage::flag_pad_file;
 
-		bdecode_node attr = dict.dict_find_string("attr");
-		if (attr)
-		{
-			for (int i = 0; i < attr.string_length(); ++i)
-			{
-				switch (attr.string_ptr()[i])
-				{
-					case 'l': file_flags |= file_storage::flag_symlink; file_size = 0; break;
-					case 'x': file_flags |= file_storage::flag_executable; break;
-					case 'h': file_flags |= file_storage::flag_hidden; break;
-					case 'p': file_flags |= file_storage::flag_pad_file; break;
-				}
-			}
-		}
-
 		bdecode_node fh = dict.dict_find_string("sha1");
 		char const* filehash = NULL;
 		if (fh && fh.string_length() == 20)
 			filehash = fh.string_ptr() + info_ptr_diff;
 
 		std::string symlink_path;
-		bdecode_node s_p = dict.dict_find("symlink path");
+		if (file_flags & file_storage::flag_symlink)
-		if (s_p && s_p.type() == bdecode_node::list_t
-			&& (file_flags & file_storage::flag_symlink))
 		{
+			if (bdecode_node s_p = dict.dict_find_list("symlink path"))
+			{
+				int const preallocate = path_length(s_p, ec);
+				if (ec) return false;
+				symlink_path.reserve(preallocate);
 				for (int i = 0, end(s_p.list_size()); i < end; ++i)
 				{
-				std::string pe = s_p.list_at(i).string_value();
+					bdecode_node const& n = s_p.list_at(i);
-				symlink_path = combine_path(symlink_path, pe);
+					sanitize_append_path_element(symlink_path, n.string_ptr()
+						, n.string_length());
+				}
 			}
 		}
 		else
@@ -591,10 +631,12 @@ namespace libtorrent
 		}
 		target.reserve(list.list_size());
 
+		// this is the counter used to name pad files
+		int pad_file_cnt = 0;
 		for (int i = 0, end(list.list_size()); i < end; ++i)
 		{
 			if (!extract_single_file(list.list_at(i), target, root_dir
-				, info_ptr_diff, false, ec))
+				, info_ptr_diff, false, pad_file_cnt, ec))
 				return false;
 		}
 		return true;
@@ -1226,7 +1268,9 @@ namespace libtorrent
 		{
 			// if there's no list of files, there has to be a length
 			// field.
-			if (!extract_single_file(info, files, "", info_ptr_diff, true, ec))
+			// this is the counter used to name pad files
+			int pad_file_cnt = 0;
+			if (!extract_single_file(info, files, "", info_ptr_diff, true, pad_file_cnt, ec))
 				return false;
 
 			m_multifile = false;
@@ -1287,7 +1331,7 @@ namespace libtorrent
 			m_merkle_tree[0].assign(root_hash.string_ptr());
 		}
 
-		m_private = info.dict_find_int_value("private", 0);
+		m_private = info.dict_find_int_value("private", 0) != 0;
 
 #ifndef TORRENT_DISABLE_MUTABLE_TORRENTS
 		bdecode_node similar = info.dict_find_list("similar");

test/Makefile.am

@@ -75,6 +75,7 @@ EXTRA_DIST = Jamfile \
   test_torrents/invalid_pieces.torrent \
   test_torrents/invalid_root_hash.torrent \
   test_torrents/invalid_root_hash2.torrent \
+  test_torrents/invalid_symlink.torrent \
   test_torrents/long_name.torrent \
   test_torrents/missing_path_list.torrent \
   test_torrents/missing_piece_len.torrent \
@@ -84,6 +85,7 @@ EXTRA_DIST = Jamfile \
   test_torrents/no_creation_date.torrent \
   test_torrents/no_name.torrent \
   test_torrents/pad_file.torrent \
+  test_torrents/pad_file_no_path.torrent \
   test_torrents/parent_path.torrent \
   test_torrents/root_hash.torrent \
   test_torrents/sample.torrent \
@@ -93,6 +95,7 @@ EXTRA_DIST = Jamfile \
   test_torrents/slash_path3.torrent \
   test_torrents/string.torrent \
   test_torrents/symlink1.torrent \
+  test_torrents/symlink_zero_size.torrent \
   test_torrents/unaligned_pieces.torrent \
   test_torrents/unordered.torrent \
   test_torrents/url_list.torrent \

test/test_torrent_info.cpp

@@ -132,6 +132,8 @@ static test_torrent_t test_torrents[] =
 	{ "invalid_name3.torrent" },
 	{ "symlink1.torrent" },
 	{ "unordered.torrent" },
+	{ "symlink_zero_size.torrent" },
+	{ "pad_file_no_path.torrent" },
 };
 
 struct test_failing_torrent_t
@@ -151,13 +153,14 @@ test_failing_torrent_t test_error_torrents[] =
 	{ "string.torrent", errors::torrent_is_no_dict },
 	{ "negative_size.torrent", errors::torrent_invalid_length },
 	{ "negative_file_size.torrent", errors::torrent_invalid_length },
-	{ "invalid_path_list.torrent", errors::torrent_missing_name},
+	{ "invalid_path_list.torrent", errors::torrent_invalid_name},
 	{ "missing_path_list.torrent", errors::torrent_missing_name },
 	{ "invalid_pieces.torrent", errors::torrent_missing_pieces },
 	{ "unaligned_pieces.torrent", errors::torrent_invalid_hashes },
 	{ "invalid_root_hash.torrent", errors::torrent_invalid_hashes },
 	{ "invalid_root_hash2.torrent", errors::torrent_missing_pieces },
 	{ "invalid_file_size.torrent", errors::torrent_invalid_length },
+	{ "invalid_symlink.torrent", errors::torrent_invalid_name },
 };
 
 // TODO: test remap_files
@@ -168,7 +171,6 @@ test_failing_torrent_t test_error_torrents[] =
 // TODO: torrent with 'l' (symlink) attribute
 // TODO: creating a merkle torrent (torrent_info::build_merkle_list)
 // TODO: torrent with multiple trackers in multiple tiers, making sure we shuffle them (how do you test shuffling?, load it multiple times and make sure it's in different order at least once)
-// TODO: torrents with a missing name
 // TODO: torrents with a zero-length name
 // TODO: torrents with a merkle tree and add_merkle_nodes
 // TODO: torrent with a non-dictionary info-section
@@ -718,6 +720,16 @@ TORRENT_TEST(parse_torrents)
 			TEST_EQUAL(ti->num_files(), 1);
 			TEST_EQUAL(ti->files().file_path(0), "temp....abc");
 		}
+		else if (std::string(test_torrents[i].file) == "symlink_zero_size.torrent")
+		{
+			TEST_EQUAL(ti->num_files(), 2);
+			TEST_EQUAL(ti->files().symlink(1), combine_path("foo", "bar"));
+		}
+		else if (std::string(test_torrents[i].file) == "pad_file_no_path.torrent")
+		{
+			TEST_EQUAL(ti->num_files(), 2);
+			TEST_EQUAL(ti->files().file_path(1), combine_path(".pad", "0"));
+		}
 
 		file_storage const& fs = ti->files();
 		for (int i = 0; i < fs.num_files(); ++i)

test/test_torrents/invalid_symlink.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:infod5:filesld6:lengthi0e4:pathl1:a1:b3:bareed4:attr1:l6:lengthi425e4:pathl1:a1:b3:foo12:symlink pathl3:foo3:bari4eeeee4:name4:temp12:piece lengthi16384e6:pieces20:‚ž¼Œ&¾ÇJW›}ÜA4u,·¼‘‡ee

test/test_torrents/pad_file_no_path.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:infod5:filesld4:pathl3:foo7:bar.txte6:lengthi45eed4:attr1:p6:lengthi2124eee4:name4:temp12:piece lengthi16384e6:pieces20:‚ž¼Œ&¾ÇJW›}ÜA4u,·¼‘‡ee

test/test_torrents/symlink1.torrent

@@ -1 +1 @@
-d10:created by10:libtorrent13:creation datei1359599503e4:infod5:filesld6:lengthi425e4:pathl1:a1:b3:bareed4:attr1:l6:lengthi425e4:pathl1:a1:b3:fooeee4:name4:temp12:piece lengthi16384e6:pieces20:‚ž¼Œ&¾ÇJW›}ÜA4u,·¼‘‡ee
+d10:created by10:libtorrent13:creation datei1359599503e4:infod5:filesld6:lengthi425e4:pathl1:a1:b3:bareed4:attr1:l6:lengthi425e4:pathl1:a1:b3:fooe12:symlink pathl3:foo3:bareee4:name4:temp12:piece lengthi16384e6:pieces20:‚ž¼Œ&¾ÇJW›}ÜA4u,·¼‘‡ee

test/test_torrents/symlink_zero_size.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:infod5:filesld6:lengthi425e4:pathl1:a1:b3:bareed4:attr1:l4:pathl1:a1:b3:fooe12:symlink pathl3:foo3:bareee4:name4:temp12:piece lengthi16384e6:pieces20:aaaaaaaaaaaaaaaaaaaaee