revert handshake round-trip optimization (#797)
revert handshake round-trip optimization. it prevents taking advantage of FAST extensions since the bitfield is sent before receiving the other peer's handshake
This commit is contained in:
parent
c8f7d89652
commit
5eaf713d1f
|
@ -1,5 +1,6 @@
|
|||
1.1.1 release
|
||||
|
||||
* fix issue where FAST extension messages were not used during handshake
|
||||
* fixed crash on invalid input in http_parser
|
||||
* upgraded to libtommath 1.0
|
||||
* fixed parsing of IPv6 endpoint with invalid port character separator
|
||||
|
|
|
@ -223,7 +223,7 @@ namespace libtorrent
|
|||
void write_dont_have(int index) TORRENT_OVERRIDE;
|
||||
void write_piece(peer_request const& r, disk_buffer_holder& buffer) TORRENT_OVERRIDE;
|
||||
void write_keepalive() TORRENT_OVERRIDE;
|
||||
void write_handshake(bool plain_handshake = false);
|
||||
void write_handshake();
|
||||
#ifndef TORRENT_DISABLE_EXTENSIONS
|
||||
void write_extensions();
|
||||
void write_upload_only();
|
||||
|
|
|
@ -765,7 +765,7 @@ namespace libtorrent
|
|||
}
|
||||
}
|
||||
|
||||
void bt_peer_connection::write_handshake(bool plain_handshake)
|
||||
void bt_peer_connection::write_handshake()
|
||||
{
|
||||
INVARIANT_CHECK;
|
||||
|
||||
|
@ -853,33 +853,6 @@ namespace libtorrent
|
|||
, "ih: %s", to_hex(ih.to_string()).c_str());
|
||||
#endif
|
||||
send_buffer(handshake, sizeof(handshake));
|
||||
|
||||
// for encrypted peers, just send a plain handshake. We
|
||||
// don't know at this point if the rest should be
|
||||
// obfuscated or not, we have to wait for the other end's
|
||||
// response first.
|
||||
if (plain_handshake) return;
|
||||
|
||||
// we don't know how many pieces there are until we
|
||||
// have the metadata
|
||||
if (t->ready_for_connections())
|
||||
{
|
||||
write_bitfield();
|
||||
#ifndef TORRENT_DISABLE_DHT
|
||||
if (m_supports_dht_port && m_ses.has_dht())
|
||||
write_dht_port(m_ses.external_udp_port());
|
||||
#endif
|
||||
|
||||
// if we don't have any pieces, don't do any preemptive
|
||||
// unchoking at all.
|
||||
if (t->num_have() > 0)
|
||||
{
|
||||
// if the peer is ignoring unchoke slots, or if we have enough
|
||||
// unused slots, unchoke this peer right away, to save a round-trip
|
||||
// in case it's interested.
|
||||
maybe_unchoke_this_peer();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
boost::optional<piece_block_progress> bt_peer_connection::downloading_piece_progress() const
|
||||
|
@ -2113,6 +2086,10 @@ namespace libtorrent
|
|||
{
|
||||
INVARIANT_CHECK;
|
||||
|
||||
// if we have not received the other peer's extension bits yet, how do we
|
||||
// know whether to send a have-all or have-none?
|
||||
TORRENT_ASSERT(m_state >= read_peer_id);
|
||||
|
||||
boost::shared_ptr<torrent> t = associated_torrent().lock();
|
||||
TORRENT_ASSERT(t);
|
||||
TORRENT_ASSERT(m_sent_handshake);
|
||||
|
@ -2698,7 +2675,7 @@ namespace libtorrent
|
|||
// always rc4 if sent here. m_rc4_encrypted is flagged
|
||||
// again according to peer selection.
|
||||
switch_send_crypto(m_rc4);
|
||||
write_handshake(true);
|
||||
write_handshake();
|
||||
switch_send_crypto(boost::shared_ptr<crypto_plugin>());
|
||||
|
||||
// vc,crypto_select,len(pad),pad, encrypt(handshake)
|
||||
|
@ -3134,25 +3111,6 @@ namespace libtorrent
|
|||
bytes_transferred = 0;
|
||||
TORRENT_ASSERT(m_encrypted);
|
||||
|
||||
if (is_outgoing() && t->ready_for_connections())
|
||||
{
|
||||
write_bitfield();
|
||||
#ifndef TORRENT_DISABLE_DHT
|
||||
if (m_supports_dht_port && m_ses.has_dht())
|
||||
write_dht_port(m_ses.external_udp_port());
|
||||
#endif
|
||||
|
||||
// if we don't have any pieces, don't do any preemptive
|
||||
// unchoking at all.
|
||||
if (t->num_have() > 0)
|
||||
{
|
||||
// if the peer is ignoring unchoke slots, or if we have enough
|
||||
// unused slots, unchoke this peer right away, to save a round-trip
|
||||
// in case it's interested.
|
||||
maybe_unchoke_this_peer();
|
||||
}
|
||||
}
|
||||
|
||||
// decrypt remaining received bytes
|
||||
if (m_rc4_encrypted)
|
||||
{
|
||||
|
@ -3480,6 +3438,28 @@ namespace libtorrent
|
|||
}
|
||||
#endif
|
||||
|
||||
// complete the handshake
|
||||
// we don't know how many pieces there are until we
|
||||
// have the metadata
|
||||
if (t->ready_for_connections())
|
||||
{
|
||||
write_bitfield();
|
||||
#ifndef TORRENT_DISABLE_DHT
|
||||
if (m_supports_dht_port && m_ses.has_dht())
|
||||
write_dht_port(m_ses.external_udp_port());
|
||||
#endif
|
||||
|
||||
// if we don't have any pieces, don't do any preemptive
|
||||
// unchoking at all.
|
||||
if (t->num_have() > 0)
|
||||
{
|
||||
// if the peer is ignoring unchoke slots, or if we have enough
|
||||
// unused slots, unchoke this peer right away, to save a round-trip
|
||||
// in case it's interested.
|
||||
maybe_unchoke_this_peer();
|
||||
}
|
||||
}
|
||||
|
||||
m_state = read_packet_size;
|
||||
m_recv_buffer.reset(5);
|
||||
|
||||
|
|
|
@ -580,10 +580,10 @@ namespace libtorrent
|
|||
return;
|
||||
}
|
||||
|
||||
int num_allowed_pieces = m_settings.get_int(settings_pack::allowed_fast_set_size);
|
||||
int const num_allowed_pieces = m_settings.get_int(settings_pack::allowed_fast_set_size);
|
||||
if (num_allowed_pieces == 0) return;
|
||||
|
||||
int num_pieces = t->torrent_file().num_pieces();
|
||||
int const num_pieces = t->torrent_file().num_pieces();
|
||||
|
||||
if (num_allowed_pieces >= num_pieces)
|
||||
{
|
||||
|
|
|
@ -273,14 +273,20 @@ void do_handshake(tcp::socket& s, sha1_hash const& ih, char* buffer)
|
|||
// check for fast extension support
|
||||
TEST_CHECK(extensions[7] & 0x4);
|
||||
|
||||
#ifndef TORRENT_DISABLE_EXTENSIONS
|
||||
// check for extension protocol support
|
||||
TEST_CHECK(extensions[5] & 0x10);
|
||||
bool const lt_extension_protocol = (extensions[5] & 0x10) != 0;
|
||||
#ifndef TORRENT_DISABLE_EXTENSIONS
|
||||
TEST_CHECK(lt_extension_protocol == true);
|
||||
#else
|
||||
TEST_CHECK(lt_extension_protocol == false);
|
||||
#endif
|
||||
|
||||
#ifndef TORRENT_DISABLE_DHT
|
||||
// check for DHT support
|
||||
TEST_CHECK(extensions[7] & 0x1);
|
||||
bool const dht_support = (extensions[7] & 0x1) != 0;
|
||||
#ifndef TORRENT_DISABLE_DHT
|
||||
TEST_CHECK(dht_support == true);
|
||||
#else
|
||||
TEST_CHECK(dht_support == false);
|
||||
#endif
|
||||
|
||||
TEST_CHECK(std::memcmp(buffer + 28, ih.begin(), 20) == 0);
|
||||
|
@ -402,7 +408,8 @@ entry read_ut_metadata_msg(tcp::socket& s, char* recv_buffer, int size)
|
|||
}
|
||||
|
||||
boost::shared_ptr<torrent_info> setup_peer(tcp::socket& s, sha1_hash& ih
|
||||
, boost::shared_ptr<lt::session>& ses, torrent_handle* th = NULL)
|
||||
, boost::shared_ptr<lt::session>& ses, bool incoming = true
|
||||
, int flags = 0, torrent_handle* th = NULL)
|
||||
{
|
||||
boost::shared_ptr<torrent_info> t = ::create_torrent();
|
||||
ih = t->info_hash();
|
||||
|
@ -413,12 +420,17 @@ boost::shared_ptr<torrent_info> setup_peer(tcp::socket& s, sha1_hash& ih
|
|||
sett.set_bool(settings_pack::enable_natpmp, false);
|
||||
sett.set_bool(settings_pack::enable_lsd, false);
|
||||
sett.set_bool(settings_pack::enable_dht, false);
|
||||
sett.set_int(settings_pack::in_enc_policy, settings_pack::pe_disabled);
|
||||
sett.set_int(settings_pack::out_enc_policy, settings_pack::pe_disabled);
|
||||
sett.set_bool(settings_pack::enable_outgoing_utp, false);
|
||||
sett.set_bool(settings_pack::enable_incoming_utp, false);
|
||||
ses.reset(new lt::session(sett, lt::session::add_default_plugins));
|
||||
|
||||
error_code ec;
|
||||
add_torrent_params p;
|
||||
p.flags &= ~add_torrent_params::flag_paused;
|
||||
p.flags &= ~add_torrent_params::flag_auto_managed;
|
||||
p.flags |= flags;
|
||||
p.ti = t;
|
||||
p.save_path = "./tmp1_fast";
|
||||
|
||||
|
@ -429,10 +441,29 @@ boost::shared_ptr<torrent_info> setup_peer(tcp::socket& s, sha1_hash& ih
|
|||
if (th) *th = ret;
|
||||
|
||||
// wait for the torrent to be ready
|
||||
if ((flags & add_torrent_params::flag_seed_mode) == 0)
|
||||
{
|
||||
wait_for_downloading(*ses, "ses");
|
||||
}
|
||||
|
||||
if (incoming)
|
||||
{
|
||||
s.connect(tcp::endpoint(address::from_string("127.0.0.1", ec), ses->listen_port()), ec);
|
||||
if (ec) TEST_ERROR(ec.message());
|
||||
}
|
||||
else
|
||||
{
|
||||
tcp::acceptor l(s.get_io_service());
|
||||
l.open(tcp::v4());
|
||||
l.bind(tcp::endpoint(address_v4::from_string("127.0.0.1")
|
||||
, 3000 + rand() % 60000));
|
||||
l.listen();
|
||||
tcp::endpoint addr = l.local_endpoint();
|
||||
|
||||
ret.connect_peer(addr);
|
||||
print_session_log(*ses);
|
||||
l.accept(s);
|
||||
}
|
||||
|
||||
print_session_log(*ses);
|
||||
|
||||
|
@ -706,7 +737,7 @@ TORRENT_TEST(dont_have)
|
|||
boost::shared_ptr<lt::session> ses;
|
||||
io_service ios;
|
||||
tcp::socket s(ios);
|
||||
boost::shared_ptr<torrent_info> ti = setup_peer(s, ih, ses, &th);
|
||||
boost::shared_ptr<torrent_info> ti = setup_peer(s, ih, ses, true, 0, &th);
|
||||
|
||||
char recv_buffer[1000];
|
||||
do_handshake(s, ih, recv_buffer);
|
||||
|
@ -875,6 +906,55 @@ TORRENT_TEST(invalid_request)
|
|||
send_request(s, req);
|
||||
}
|
||||
|
||||
void have_all_test(bool const incoming)
|
||||
{
|
||||
sha1_hash ih;
|
||||
boost::shared_ptr<lt::session> ses;
|
||||
io_service ios;
|
||||
tcp::socket s(ios);
|
||||
setup_peer(s, ih, ses, incoming, add_torrent_params::flag_seed_mode);
|
||||
|
||||
char recv_buffer[1000];
|
||||
do_handshake(s, ih, recv_buffer);
|
||||
print_session_log(*ses);
|
||||
|
||||
// expect to receive a have-all (not a bitfield)
|
||||
// since we advertised support for FAST extensions
|
||||
for (;;)
|
||||
{
|
||||
int const len = read_message(s, recv_buffer, sizeof(recv_buffer));
|
||||
if (len == -1)
|
||||
{
|
||||
TEST_ERROR("failed to receive have-all despite advertising support for FAST");
|
||||
break;
|
||||
}
|
||||
print_message(recv_buffer, len);
|
||||
int const msg = recv_buffer[0];
|
||||
if (msg == 0xe) // have-all
|
||||
{
|
||||
// success!
|
||||
break;
|
||||
}
|
||||
if (msg == 5) // bitfield
|
||||
{
|
||||
TEST_ERROR("received bitfield from seed despite advertising support for FAST");
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
TORRENT_TEST(outgoing_have_all)
|
||||
{
|
||||
std::cerr << "\n === test outgoing have-all ===\n" << std::endl;
|
||||
have_all_test(true);
|
||||
}
|
||||
|
||||
TORRENT_TEST(incoming_have_all)
|
||||
{
|
||||
std::cerr << "\n === test outgoing have-all ===\n" << std::endl;
|
||||
have_all_test(false);
|
||||
}
|
||||
|
||||
#endif // TORRENT_DISABLE_EXTENSIONS
|
||||
|
||||
// TODO: test sending invalid requests (out of bound piece index, offsets and
|
||||
|
|
Loading…
Reference in New Issue