improve unit test coverage and fix handling of some invalid .torrent files and invalid utf-8 sequences

ChangeLog

@@ -14,6 +14,8 @@
 	* fix uTP edge case where udp socket buffer fills up
 	* fix nagle implementation in uTP
 
+	* improve handling of invalid utf-8 sequences in strings in torrent files
+	* handle more cases of broken .torrent files
 	* fix bug filename collision resolver
 	* fix bug in filename utf-8 verification
 	* make need_save_resume() a bit more robust

src/torrent_info.cpp

@@ -73,12 +73,6 @@ POSSIBILITY OF SUCH DAMAGE.
 namespace libtorrent
 {
 	
-	void convert_to_utf8(std::string& str, unsigned char chr)
-	{
-		str += 0xc0 | ((chr & 0xff) >> 6);
-		str += 0x80 | (chr & 0x3f);
-	}
-
 	bool valid_path_character(char c)
 	{
 #ifdef TORRENT_WINDOWS
@@ -103,7 +97,7 @@ namespace libtorrent
 			// valid ascii-character
 			if ((*i & 0x80) == 0)
 			{
-				// replace invalid characters with '.'
+				// replace invalid characters with '_'
 				if (!fix_paths || valid_path_character(*i))
 				{
 					tmp_path += *i;
@@ -118,7 +112,7 @@ namespace libtorrent
 			
 			if (end - i < 2)
 			{
-				convert_to_utf8(tmp_path, *i);
+				tmp_path += "_";
 				valid_encoding = false;
 				break;
 			}
@@ -135,7 +129,7 @@ namespace libtorrent
 
 			if (end - i < 3)
 			{
-				convert_to_utf8(tmp_path, *i);
+				tmp_path += "_";
 				valid_encoding = false;
 				break;
 			}
@@ -154,7 +148,7 @@ namespace libtorrent
 
 			if (end - i < 4)
 			{
-				convert_to_utf8(tmp_path, *i);
+				tmp_path += "_";
 				valid_encoding = false;
 				break;
 			}
@@ -173,7 +167,7 @@ namespace libtorrent
 				continue;
 			}
 
-			convert_to_utf8(tmp_path, *i);
+			tmp_path += "_";
 			valid_encoding = false;
 		}
 		// the encoding was not valid utf-8
@@ -184,7 +178,7 @@ namespace libtorrent
 		return valid_encoding;
 	}
 
-	// TODO: 1 we might save constructing a std::String if this would take a char const* instead
+	// TODO: 1 we might save constructing a std::string if this would take a char const* instead
 	bool valid_path_element(std::string const& element)
 	{
 		if (element.empty()
@@ -261,6 +255,8 @@ namespace libtorrent
 		if (length == 0 || length->type() != lazy_entry::int_t)
 			return false;
 		target.size = length->int_value();
+		if (target.size < 0)
+			return false;
 
 		size_type ts = dict.dict_find_int_value("mtime", -1);
 		if (ts > 0) *mtime = std::time_t(ts);
@@ -358,7 +354,6 @@ namespace libtorrent
 		// TODO: 1 this logic should be a separate step
 		// done once the torrent is loaded, and the original
 		// filenames should be preserved!
-		int cnt = 0;
 		std::set<std::string, string_less_no_case> files;
 
 		for (int i = 0, end(list.list_size()); i < end; ++i)
@@ -373,6 +368,7 @@ namespace libtorrent
 
 			// as long as this file already exists
 			// increase the counter
+			int cnt = 0;
 			while (!files.insert(e.path).second)
 			{
 				++cnt;
@@ -910,6 +906,11 @@ namespace libtorrent
 			e.path = name;
 			e.offset = 0;
 			e.size = info.dict_find_int_value("length", -1);
+			if (e.size < 0)
+			{
+				ec = errors::torrent_file_parse_failed;
+				return false;
+			}
 			e.mtime = info.dict_find_int_value("mtime", 0);
 			lazy_entry const* attr = info.dict_find_string("attr");
 			if (attr)

test/Makefile.am

@@ -53,7 +53,23 @@ EXTRA_DIST = Jamfile \
   test_torrents/empty_httpseed.torrent \
   test_torrents/long_name.torrent \
   test_torrents/whitespace_url.torrent \
-  test_torrents/duplicate_files.torrent
+  test_torrents/duplicate_files.torrent \
+  test_torrents/invalid_piece_len.torrent \
+  test_torrents/missing_piece_len.torrent \
+  test_torrents/negative_piece_len.torrent \
+  test_torrents/no_name.torrent \
+  test_torrents/invalid_name.torrent \
+  test_torrents/invalid_name2.torrent \
+  test_torrents/invalid_info.torrent \
+  test_torrents/string.torrent \
+  test_torrents/negative_size.torrent \
+  test_torrents/negative_file_size.torrent \
+  test_torrents/pad_file.torrent \
+  test_torrents/invalid_path_list.torrent \
+  test_torrents/missing_path_list.torrent \
+  test_torrents/invalid_pieces.torrent \
+  test_torrents/unaligned_pieces.torrent \
+  test_torrents/creation_date.torrent
 
 EXTRA_PROGRAMS = $(test_programs)
 

test/test_primitives.cpp

@@ -1314,14 +1314,14 @@ int test_main()
 	// valid 2-byte sequence
 	test = "filename\xc2\xa1";
 	TEST_CHECK(verify_encoding(test));
-	fprintf(stderr, "%s %x %x\n", test.c_str(), test[test.size()-2], test[test.size()-1]);
+	fprintf(stderr, "%s\n", test.c_str());
 	TEST_CHECK(test == "filename\xc2\xa1");
 
 	// truncated 2-byte sequence
 	test = "filename\xc2";
 	TEST_CHECK(!verify_encoding(test));
-	fprintf(stderr, "%s %x %x\n", test.c_str(), test[test.size()-2], test[test.size()-1]);
-	TEST_CHECK(test == "filename\xc3\x82");
+	fprintf(stderr, "%s\n", test.c_str());
+	TEST_CHECK(test == "filename_");
 
 	// valid 3-byte sequence
 	test = "filename\xe2\x9f\xb9";
@@ -1332,26 +1332,32 @@ int test_main()
 	// truncated 3-byte sequence
 	test = "filename\xe2\x9f";
 	TEST_CHECK(!verify_encoding(test));
-	fprintf(stderr, "%s %x %x\n", test.c_str(), test[test.size()-2], test[test.size()-1]);
-	TEST_CHECK(test == "filename\xc3\xa2");
+	fprintf(stderr, "%s\n", test.c_str());
+	TEST_CHECK(test == "filename_");
 
 	// truncated 3-byte sequence
 	test = "filename\xe2";
 	TEST_CHECK(!verify_encoding(test));
-	fprintf(stderr, "%s %x %x\n", test.c_str(), test[test.size()-2], test[test.size()-1]);
-	TEST_CHECK(test == "filename\xc3\xa2");
+	fprintf(stderr, "%s\n", test.c_str());
+	TEST_CHECK(test == "filename_");
 
 	// valid 4-byte sequence
 	test = "filename\xf0\x9f\x92\x88";
 	TEST_CHECK(verify_encoding(test));
-	fprintf(stderr, "%s %x %x\n", test.c_str(), test[test.size()-2], test[test.size()-1]);
+	fprintf(stderr, "%s\n", test.c_str());
 	TEST_CHECK(test == "filename\xf0\x9f\x92\x88");
 
 	// truncated 4-byte sequence
 	test = "filename\xf0\x9f\x92";
 	TEST_CHECK(!verify_encoding(test));
-	fprintf(stderr, "%s %x %x\n", test.c_str(), test[test.size()-2], test[test.size()-1]);
-	TEST_CHECK(test == "filename\xc3\xb0");
+	fprintf(stderr, "%s\n", test.c_str());
+	TEST_CHECK(test == "filename_");
+
+	// 5-byte utf-8 sequence (not allowed)
+	test = "filename\xf8\x9f\x9f\x9f\x9f""foobar";
+	TEST_CHECK(!verify_encoding(test));
+	fprintf(stderr, "%s\n", test.c_str());
+	TEST_CHECK(test == "filename_____foobar");
 
 	// trim_path_element
 

test/test_torrent_parse.cpp

@@ -58,30 +58,45 @@ test_torrent_t test_torrents[] =
 	{ "long_name.torrent" },
 	{ "whitespace_url.torrent" },
 	{ "duplicate_files.torrent" },
-//	{ "" },
+	{ "pad_file.torrent" },
+	{ "creation_date.torrent" },
+};
+
+struct test_failing_torrent_t
+{
+	char const* file;
+	error_code error; // the expected error
+};
+
+test_failing_torrent_t test_error_torrents[] =
+{
+	{ "missing_piece_len.torrent", errors::torrent_missing_piece_length },
+	{ "invalid_piece_len.torrent", errors::torrent_missing_piece_length },
+	{ "negative_piece_len.torrent", errors::torrent_missing_piece_length },
+	{ "no_name.torrent", errors::torrent_missing_name },
+	{ "invalid_name.torrent", errors::torrent_missing_name },
+	{ "invalid_name2.torrent", errors::torrent_invalid_name },
+	{ "invalid_info.torrent", errors::torrent_missing_info },
+	{ "string.torrent", errors::torrent_is_no_dict },
+	{ "negative_size.torrent", errors::torrent_file_parse_failed},
+	{ "negative_file_size.torrent", errors::torrent_file_parse_failed },
+	{ "invalid_path_list.torrent", errors::torrent_file_parse_failed },
+	{ "missing_path_list.torrent", errors::torrent_file_parse_failed },
+	{ "invalid_pieces.torrent", errors::torrent_missing_pieces },
+	{ "unaligned_pieces.torrent", errors::torrent_invalid_hashes },
 };
 
 // TODO: create a separate list of all torrents that should
 // fail to parse, and include the expected error code in that list
 
 // TODO: merkle torrents. specifically torrent_info::add_merkle_nodes and torrent with "root hash"
-// TODO: torrent where info-section is not a dict
-// TODO: torrent with "piece length" <= 0
-// TODO: torrent with no "name" nor "name.utf8"
-// TODO: torrent with "name" refering to an invalid path
 // TODO: torrent with 'p' (padfile) attribute
 // TODO: torrent with 'h' (hidden) attribute
 // TODO: torrent with 'x' (executable) attribute
 // TODO: torrent with 'l' (symlink) attribute
-// TODO: torrent with bitcomet style padfiles (name convention)
-// TODO: torrent with a negative file size
-// TODO: torrent with a negative total size
-// TODO: torrent with a pieces field that's not a string
-// TODO: torrent with a pieces field whose length is not divisible by 20
 // 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: torrent with web seed. make sure we append '/' for multifile torrents
-// TODO: test that creation date is parsed correctly
 
 int test_main()
 {
@@ -105,6 +120,16 @@ int test_main()
 			TEST_CHECK(ti->file_at(0).path == "temp/foo/bar.txt");
 			TEST_CHECK(ti->file_at(1).path == "temp/foo/bar.1.txt");
 		}
+		else if (std::string(test_torrents[i].file) == "pad_file.torrent")
+		{
+			TEST_EQUAL(ti->num_files(), 2);
+			TEST_CHECK(ti->file_at(0).pad_file == false);
+			TEST_CHECK(ti->file_at(1).pad_file == true);
+		}
+		else if (std::string(test_torrents[i].file) == "creation_date.torrent")
+		{
+			TEST_CHECK(*ti->creation_date() == 1234567);
+		}
 
 		int index = 0;
 		for (torrent_info::file_iterator i = ti->begin_files();
@@ -127,6 +152,16 @@ int test_main()
 		}
 
 	}
+
+	for (int i = 0; i < sizeof(test_error_torrents)/sizeof(test_error_torrents[0]); ++i)
+	{
+		error_code ec;
+		fprintf(stderr, "loading %s\n", test_error_torrents[i].file);
+		boost::intrusive_ptr<torrent_info> ti(new torrent_info(combine_path("test_torrents", test_error_torrents[i].file), ec));
+		fprintf(stderr, "E: %s\nexpected: %s\n", ec.message().c_str(), test_error_torrents[i].error.message().c_str());
+		TEST_EQUAL(ec, test_error_torrents[i].error);
+	}
+
 	return 0;
 }
 

test/test_torrents/creation_date.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1234567e4:infod5:filesld6:lengthi425e4:pathl3:foo7:bar.txteed6:lengthi425e4:pathl3:foo7:var.txteee4:name4:temp12:piece lengthi16384e6:pieces20:01234567890123456789ee

test/test_torrents/invalid_info.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:info5:filese

test/test_torrents/invalid_name.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:infod5:filesld6:lengthi425e4:pathl3:foo7:bar.txteed6:lengthi425e4:pathl3:foo7:var.txteee4:namei1348e12:piece lengthi16384e6:pieces20:‚ž¼Œ&¾ÇJW›}ÜA4u,·¼‘‡ee

test/test_torrents/invalid_name2.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:infod5:filesld6:lengthi425e4:pathl3:foo7:bar.txteed6:lengthi425e4:pathl3:foo7:var.txteee4:name2:..12:piece lengthi16384e6:pieces20:‚ž¼Œ&¾ÇJW›}ÜA4u,·¼‘‡ee

test/test_torrents/invalid_path_list.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:infod5:filesld6:lengthi425e4:pathli1242e3:foo7:bar.txteed6:lengthi425e4:pathl3:foo7:var.txteee4:name4:temp12:piece lengthi16384e6:pieces20:‚ž¼Œ&¾ÇJW›}ÜA4u,·¼‘‡ee

test/test_torrents/invalid_piece_len.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:infod5:filesld6:lengthi425e4:pathl3:foo7:bar.txteed6:lengthi425e4:pathl3:foo7:var.txteee4:name4:temp12:piece length5:163846:pieces20:‚ž¼Œ&¾ÇJW›}ÜA4u,·¼‘‡ee

test/test_torrents/invalid_pieces.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:infod5:filesld6:lengthi425e4:pathl3:foo7:bar.txteed6:lengthi425e4:pathl3:foo7:var.txteee4:name4:temp12:piece lengthi16384e6:piecesi-23eee

test/test_torrents/missing_path_list.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:infod5:filesld6:lengthi425eed6:lengthi425e4:pathl3:foo7:var.txteee4:name4:temp12:piece lengthi16384e6:pieces20:‚ž¼Œ&¾ÇJW›}ÜA4u,·¼‘‡ee

test/test_torrents/missing_piece_len.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:infod5:filesld6:lengthi425e4:pathl3:foo7:bar.txteed6:lengthi425e4:pathl3:foo7:var.txteee4:name4:temp6:pieces20:‚ž¼Œ&¾ÇJW›}ÜA4u,·¼‘‡ee

test/test_torrents/negative_file_size.torrent

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

test/test_torrents/negative_piece_len.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:infod5:filesld6:lengthi425e4:pathl3:foo7:bar.txteed6:lengthi425e4:pathl3:foo7:var.txteee4:name4:temp12:piece lengthi-16384e6:pieces20:‚ž¼Œ&¾ÇJW›}ÜA4u,·¼‘‡ee

test/test_torrents/negative_size.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:infod6:lengthi-425e4:name4:temp12:piece lengthi16384e6:pieces20:cdcdcdcdcdcdcdcdcdcdee

test/test_torrents/no_name.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:infod5:filesld6:lengthi425e4:pathl3:foo7:bar.txteed6:lengthi425e4:pathl3:foo7:var.txteee12:piece lengthi16384e6:pieces20:‚ž¼Œ&¾ÇJW›}ÜA4u,·¼‘‡ee

test/test_torrents/pad_file.torrent

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

test/test_torrents/string.torrent

@@ -0,0 +1 @@
+10:libtorrent

test/test_torrents/unaligned_pieces.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:infod5:filesld6:lengthi425e4:pathl3:foo7:bar.txteed6:lengthi425e4:pathl3:foo7:var.txteee4:name4:temp12:piece lengthi16384e6:pieces24:012345678901234567890123ee