improve checking and testing of max piece size

Makefile

@@ -1012,6 +1012,7 @@ TEST_TORRENTS = \
  invalid_pieces.torrent \
  invalid_symlink.torrent \
  large.torrent \
+ large_piece_size.torrent \
  long_name.torrent \
  many_pieces.torrent \
  missing_path_list.torrent \

include/libtorrent/file_storage.hpp

@@ -238,6 +238,22 @@ namespace aux {
  , std::int64_t((std::numeric_limits<int>::max)() / 2) * default_block_size);
  static constexpr std::int64_t max_file_offset = (std::int64_t(1) << 48) - 1;
 
+ // we use a signed 32 bit integer for piece indices internally, but
+ // frequently need headroom for intermediate calculations, so we limit
+ // the number of pieces 1 bit below the maximum
+ static constexpr std::int32_t max_num_pieces = (std::int32_t(1) << 30) - 1;
+
+ // limit the piece length at (2 ^ 30) to get a bit of headroom. We
+ // commonly compute the number of blocks per pieces by adding
+ // block_size - 1 before dividing by block_size. That would overflow with
+ // a piece size of 2 ^ 31. This limit is still an unreasonably large
+ // piece size anyway.
+ // The piece picker (currently) has a limit of no more than (2^15)-1
+ // blocks per piece, which is more restrictive, at a block size of 16
+ // kiB (0x4000).
+ static constexpr std::int32_t max_piece_size = ((1 << 15) - 1) * 0x4000;
+ // static constexpr std::int32_t max_piece_size = (std::int32_t(1) << 30) - 1;
+
  // returns true if the piece length has been initialized
  // on the file_storage. This is typically taken as a proxy
  // of whether the file_storage as a whole is initialized or

src/torrent_info.cpp

@@ -377,7 +377,7 @@ namespace {
  // calculations of the size of the merkle tree (which is all 'int'
  // indices)
  if (file_size < 0
- || (file_size / default_block_size) >= std::numeric_limits<int>::max() / 2
+ || (file_size / default_block_size) >= file_storage::max_num_pieces
  || file_size > file_storage::max_file_size)
  {
  ec = errors::torrent_invalid_length;
@@ -1162,12 +1162,7 @@ namespace {
 
  // extract piece length
  std::int64_t const piece_length = info.dict_find_int_value("piece length", -1);
- // limit the piece length at INT_MAX / 2 to get a bit of headroom. We
- // commonly compute the number of blocks per pieces by adding
- // block_size - 1 before dividing by block_size. That would overflow with
- // a piece size of INT_MAX. This limit is still an unreasonably large
- // piece size anyway.
- if (piece_length <= 0 || piece_length > std::numeric_limits<int>::max() / 2)
+ if (piece_length <= 0 || piece_length > file_storage::max_piece_size)
  {
  ec = errors::torrent_missing_piece_length;
  return false;
@@ -1319,8 +1314,7 @@ namespace {
  // we want this division to round upwards, that's why we have the
  // extra addition
 
- if (files.total_size() / files.piece_length() >=
- std::numeric_limits<int>::max())
+ if (files.total_size() / files.piece_length() > file_storage::max_num_pieces)
  {
  ec = errors::too_many_pieces_in_torrent;
  // mark the torrent as invalid

test/test_torrent.cpp

@@ -186,14 +186,16 @@ void test_large_piece_size(int const size)
 
  std::vector<char> buf;
  bencode(std::back_inserter(buf), torrent);
- add_torrent_params atp = load_torrent_buffer(buf);
- atp.save_path = ".";
-
- lt::session ses;
- auto h = ses.add_torrent(std::move(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));
+ try
+ {
+ add_torrent_params atp = load_torrent_buffer(buf);
+ // we expect this to fail with an exception
+ TEST_CHECK(false);
+ }
+ catch (lt::system_error const& e)
+ {
+ TEST_CHECK(e.code() == error_code(lt::errors::torrent_missing_piece_length));
+ }
 }
 
 } // anonymous namespace

test/test_torrent_info.cpp

@@ -398,6 +398,10 @@ static test_torrent_t const test_torrents[] =
  TEST_CHECK((ti->nodes() == std::vector<np>{np("127.0.0.1", 6881), np("192.168.1.1", 6881)}));
  }
  },
+ { "large_piece_size.torrent", [](torrent_info const* ti) {
+ TEST_EQUAL(ti->piece_length(), (32767 * 0x4000));
+ }
+ },
 };
 
 struct test_failing_torrent_t
@@ -980,7 +984,7 @@ void sanity_check(std::shared_ptr<torrent_info> const& ti)
  // for it, it's still no good.
  piece_picker pp(ti->total_size(), ti->piece_length());
 
- TEST_CHECK(ti->piece_length() < std::numeric_limits<int>::max() / 2);
+ TEST_CHECK(ti->piece_length() <= (std::numeric_limits<int>::max() / 2 + 1));
  TEST_EQUAL(ti->v1(), ti->info_hashes().has_v1());
  TEST_EQUAL(ti->v2(), ti->info_hashes().has_v2());
 }

test/test_torrents/large_piece_size.torrent

@@ -0,0 +1 @@
+d10:created by10:libtorrent13:creation datei1359599503e4:infod6:lengthi425e4:name4:temp12:piece lengthi536854528e6:pieces20:����&��JW�}�A4u,����ee