fix handling of torrents with too large pieces

ChangeLog

@@ -1,3 +1,4 @@
+	* fix handling of torrents with too large pieces
 	* fixed division by zero in anti-leech choker
 	* fixed bug in torrent_info::swap
 

include/libtorrent/piece_picker.hpp

@@ -95,7 +95,11 @@ namespace libtorrent {
 			// the number of priority levels
 			priority_levels = 8,
 			// priority factor
-			prio_factor = 3
+			prio_factor = 3,
+			// max blocks per piece
+			// there are counters in downloading_piece that only have 15 bits to
+			// count blocks per piece, that's restricting this
+			max_blocks_per_piece = (1 << 15) - 1
 		};
 
 		struct block_info

include/libtorrent/torrent.hpp

@@ -1699,6 +1699,11 @@ namespace libtorrent {
 		// millionths of completeness)
 		std::uint32_t m_progress_ppm:20;
 
+		// set to true once init() completes successfully. This is important to
+		// track in case it fails and need to be retried if the client clears
+		// the torrent error
+		bool m_torrent_initialized:1;
+
 #if TORRENT_USE_ASSERTS
 		// set to true when torrent is start()ed. It may only be started once
 		bool m_was_started = false;

src/piece_picker.cpp

@@ -155,6 +155,10 @@ namespace libtorrent {
 #ifdef TORRENT_PICKER_LOG
 		std::cerr << "[" << this << "] " << "piece_picker::resize()" << std::endl;
 #endif
+
+		if (blocks_per_piece > max_blocks_per_piece)
+			throw system_error(errors::invalid_piece_size);
+
 		// allocate the piece_map to cover all pieces
 		// and make them invalid (as if we don't have a single piece)
 		m_piece_map.resize(total_num_pieces, piece_pos(0, 0));
@@ -191,9 +195,9 @@ namespace libtorrent {
 			m_reverse_cursor > piece_index_t(0) && (i->have() || i->filtered());
 			++i, --m_reverse_cursor);
 
-		m_blocks_per_piece = std::uint16_t(blocks_per_piece);
+		m_blocks_per_piece = aux::numeric_cast<std::uint16_t>(blocks_per_piece);
-		m_blocks_in_last_piece = std::uint16_t(blocks_in_last_piece);
+		m_blocks_in_last_piece = aux::numeric_cast<std::uint16_t>(blocks_in_last_piece);
-		if (m_blocks_in_last_piece == 0) m_blocks_in_last_piece = std::uint16_t(blocks_per_piece);
+		if (m_blocks_in_last_piece == 0) m_blocks_in_last_piece = aux::numeric_cast<std::uint16_t>(blocks_per_piece);
 
 		TORRENT_ASSERT(m_blocks_in_last_piece <= m_blocks_per_piece);
 	}

src/torrent.cpp

@@ -226,6 +226,7 @@ bool is_downloading_state(int const st)
 		, m_inactive(false)
 		, m_downloaded(0xffffff)
 		, m_progress_ppm(0)
+		, m_torrent_initialized(false)
 	{
 		// we cannot log in the constructor, because it relies on shared_from_this
 		// being initialized, which happens after the constructor returns.
@@ -1776,6 +1777,15 @@ bool is_downloading_state(int const st)
 			return;
 		}
 
+		int const blocks_per_piece
+			= (m_torrent_file->piece_length() + default_block_size - 1) / default_block_size;
+		if (blocks_per_piece > piece_picker::max_blocks_per_piece)
+		{
+			set_error(errors::invalid_piece_size, torrent_status::error_file_none);
+			pause();
+			return;
+		}
+
 		// --- MAPPED FILES ---
 		file_storage const& fs = m_torrent_file->files();
 		if (m_add_torrent_params)
@@ -1860,7 +1870,6 @@ bool is_downloading_state(int const st)
 				TORRENT_ASSERT((pr.start & (block_size() - 1)) == 0);
 
 				int block = block_size();
-				int blocks_per_piece = m_torrent_file->piece_length() / block;
 				piece_block pb(pr.piece, pr.start / block);
 				for (; pr.length >= block; pr.length -= block, ++pb.block_index)
 				{
@@ -1970,6 +1979,8 @@ bool is_downloading_state(int const st)
 
 		// this will remove the piece picker, if we're done with it
 		maybe_done_flushing();
+
+		m_torrent_initialized = true;
 	}
 
 	bt_peer_connection* torrent::find_introducer(tcp::endpoint const& ep) const
@@ -8183,7 +8194,7 @@ bool is_downloading_state(int const st)
 		}
 #endif
 		// if the error happened during initialization, try again now
-		if (!m_connections_initialized && valid_metadata()) init();
+		if (!m_torrent_initialized && valid_metadata()) init();
 		if (!checking_files && should_check_files())
 			start_checking();
 	}

test/test_torrent.cpp

@@ -166,11 +166,34 @@ void test_running_torrent(std::shared_ptr<torrent_info> info, std::int64_t file_
 	TEST_CHECK(h.get_file_priorities() == prio);
 }
 
+void test_large_piece_size(int const size)
+{
+	entry torrent;
+	entry& info = torrent["info"];
+	info["pieces"] = "aaaaaaaaaaaaaaaaaaaa";
+	info["name"] = "test";
+	info["piece length"] = size;
+	info["length"] = size;
+
+	std::vector<char> buf;
+	bencode(std::back_inserter(buf), torrent);
+	add_torrent_params atp;
+	atp.ti = std::make_shared<torrent_info>(buf, from_span);
+	atp.save_path = ".";
+
+	lt::session ses;
+	auto h = ses.add_torrent(atp);
+	TEST_CHECK(h.status().errc == error_code(lt::errors::invalid_piece_size));
+	h.clear_error();
+	TEST_CHECK(h.status().errc == error_code(lt::errors::invalid_piece_size));
+}
+
 } // anonymous namespace
 
 TORRENT_TEST(long_names)
 {
-	entry info;
+	entry torrent;
+	entry& info = torrent["info"];
 	info["pieces"] = "aaaaaaaaaaaaaaaaaaaa";
 	info["name"] = "slightly shorter name, it's kind of sad that people started "
 		"the trend of incorrectly encoding the regular name field and then adding "
@@ -180,14 +203,17 @@ TORRENT_TEST(long_names)
 		"read this that particular bug should have been fixed";
 	info["piece length"] = 16 * 1024;
 	info["length"] = 3245;
-	entry torrent;
-	torrent["info"] = info;
 
 	std::vector<char> buf;
 	bencode(std::back_inserter(buf), torrent);
-	error_code ec;
+	auto ti = std::make_shared<torrent_info>(buf, from_span);
-	auto ti = std::make_shared<torrent_info>(buf, std::ref(ec), from_span);
+}
-	TEST_CHECK(!ec);
+
+TORRENT_TEST(large_piece_size)
+{
+	test_large_piece_size(32768 * 16 * 1024);
+	test_large_piece_size(65536 * 16 * 1024);
+	test_large_piece_size(65537 * 16 * 1024);
 }
 
 TORRENT_TEST(total_wanted)
@@ -202,8 +228,7 @@ TORRENT_TEST(total_wanted)
 	lt::create_torrent t(fs, 1024);
 	std::vector<char> tmp;
 	bencode(std::back_inserter(tmp), t.generate());
-	error_code ec;
+	auto info = std::make_shared<torrent_info>(tmp, from_span);
-	auto info = std::make_shared<torrent_info>(tmp, std::ref(ec), from_span);
 
 	settings_pack pack = settings();
 	pack.set_int(settings_pack::alert_mask, alert::storage_notification);
@@ -243,9 +268,7 @@ TORRENT_TEST(added_peers)
 	lt::create_torrent t(fs, 1024);
 	std::vector<char> tmp;
 	bencode(std::back_inserter(tmp), t.generate());
-	error_code ec;
+	auto info = std::make_shared<torrent_info>(tmp, from_span);
-	auto info = std::make_shared<torrent_info>(
-		tmp, std::ref(ec), from_span);
 
 	settings_pack pack = settings();
 	pack.set_str(settings_pack::listen_interfaces, "0.0.0.0:48130");
@@ -253,11 +276,9 @@ TORRENT_TEST(added_peers)
 	lt::session ses(pack);
 
 	add_torrent_params p = parse_magnet_uri(
-		"magnet:?xt=urn:btih:abababababababababababababababababababab&x.pe=127.0.0.1:48081&x.pe=127.0.0.2:48082"
+		"magnet:?xt=urn:btih:abababababababababababababababababababab&x.pe=127.0.0.1:48081&x.pe=127.0.0.2:48082");
-		, ec);
 	p.ti = info;
 	p.save_path = ".";
-	TEST_CHECK(!ec);
 
 	torrent_handle h = ses.add_torrent(std::move(p));
 
@@ -324,8 +345,7 @@ TORRENT_TEST(torrent)
 		std::vector<char> tmp;
 		std::back_insert_iterator<std::vector<char>> out(tmp);
 		bencode(out, t.generate());
-		error_code ec;
+		auto info = std::make_shared<torrent_info>(tmp, from_span);
-		auto info = std::make_shared<torrent_info>(tmp, std::ref(ec), from_span);
 		test_running_torrent(info, 1024);
 	}
 }
@@ -367,13 +387,12 @@ TORRENT_TEST(duplicate_is_not_error)
 	std::vector<char> tmp;
 	std::back_insert_iterator<std::vector<char>> out(tmp);
 	bencode(out, t.generate());
-	error_code ec;
 
 	int called = 0;
 	plugin_creator creator(called);
 
 	add_torrent_params p;
-	p.ti = std::make_shared<torrent_info>(tmp, std::ref(ec), from_span);
+	p.ti = std::make_shared<torrent_info>(tmp, from_span);
 	p.flags &= ~torrent_flags::paused;
 	p.flags &= ~torrent_flags::auto_managed;
 	p.flags &= ~torrent_flags::duplicate_is_error;
@@ -394,13 +413,12 @@ TORRENT_TEST(duplicate_is_not_error)
 TORRENT_TEST(torrent_total_size_zero)
 {
 	file_storage fs;
-	error_code ec;
 
 	fs.add_file("test_torrent_dir2/tmp1", 0);
 	TEST_CHECK(fs.num_files() == 1);
 	TEST_CHECK(fs.total_size() == 0);
 
-	ec.clear();
+	error_code ec;
 	lt::create_torrent t1(fs);
 	set_piece_hashes(t1, ".", ec);
 	TEST_CHECK(ec);
@@ -426,8 +444,7 @@ TORRENT_TEST(rename_file)
 	std::vector<char> tmp;
 	std::back_insert_iterator<std::vector<char>> out(tmp);
 	bencode(out, t.generate());
-	error_code ec;
+	auto info = std::make_shared<torrent_info>(tmp, from_span);
-	auto info = std::make_shared<torrent_info>(tmp, std::ref(ec), from_span);
 
 	TEST_EQUAL(info->files().file_path(file_index_t(0)), combine_path("test3","tmp1"));
 
@@ -605,8 +622,7 @@ TORRENT_TEST(queue_paused)
 TORRENT_TEST(test_move_storage_no_metadata)
 {
 	lt::session ses(settings());
-	error_code ec;
+	add_torrent_params p = parse_magnet_uri("magnet:?xt=urn:btih:abababababababababababababababababababab");
-	add_torrent_params p = parse_magnet_uri("magnet:?xt=urn:btih:abababababababababababababababababababab", ec);
 	p.save_path = "save_path";
 	torrent_handle h = ses.add_torrent(p);
 
@@ -620,8 +636,7 @@ TORRENT_TEST(test_move_storage_no_metadata)
 TORRENT_TEST(test_have_piece_no_metadata)
 {
 	lt::session ses(settings());
-	error_code ec;
+	add_torrent_params p = parse_magnet_uri("magnet:?xt=urn:btih:abababababababababababababababababababab");
-	add_torrent_params p = parse_magnet_uri("magnet:?xt=urn:btih:abababababababababababababababababababab", ec);
 	p.save_path = "save_path";
 	torrent_handle h = ses.add_torrent(p);
 
@@ -650,8 +665,7 @@ TORRENT_TEST(test_have_piece_out_of_range)
 TORRENT_TEST(test_read_piece_no_metadata)
 {
 	lt::session ses(settings());
-	error_code ec;
+	add_torrent_params p = parse_magnet_uri("magnet:?xt=urn:btih:abababababababababababababababababababab");
-	add_torrent_params p = parse_magnet_uri("magnet:?xt=urn:btih:abababababababababababababababababababab", ec);
 	p.save_path = "save_path";
 	torrent_handle h = ses.add_torrent(p);