From 64261685d8d474ee378a9dbaf3c2ee07761be342 Mon Sep 17 00:00:00 2001 From: tearfur <46261767+tearfur@users.noreply.github.com> Date: Tue, 20 Jun 2023 09:26:39 +0800 Subject: [PATCH] fix: use both address + port together as a key for peer lookup (#5619) --- code_style.sh | 10 +++++-- libtransmission/handshake.cc | 6 +++-- libtransmission/handshake.h | 5 ++-- libtransmission/peer-mgr.cc | 36 ++++++++++++------------- libtransmission/peer-mgr.h | 4 +-- libtransmission/peer-msgs.cc | 8 +++--- tests/libtransmission/handshake-test.cc | 5 ++-- 7 files changed, 43 insertions(+), 31 deletions(-) diff --git a/code_style.sh b/code_style.sh index b3bddc1f8..8769ce79c 100755 --- a/code_style.sh +++ b/code_style.sh @@ -66,13 +66,19 @@ if ! find_cfiles -exec "${clang_format_exe}" $clang_format_args '{}' '+'; then fi # enforce east const -matches="$(find_cfiles -exec perl -ne 'print "west const:",$ARGV,":",$_ if /((?:^|[(<,;]|\bstatic\s+)\s*)\b(const)\b(?!\s+\w+\s*\[)/' '{}' '+')" +# look for 'const' +# - as the first token in the line +# - or preceded by 'static' +# - or following any of (<,; +# - but not if 'const' is followed by ` override` (const virtual function) +# - but not if 'const' is followed by ` = 0` (const virtual function) +matches="$(find_cfiles -exec perl -ne 'print "west const:",$ARGV,":",$_ if /((?:^|[(<,;]|\bstatic\s+)\s*)\b(const)\b(?!\s+((\w+\s*\[)|(override)|(\=\ 0)))/' '{}' '+')" if [ -n "$matches" ]; then echo "$matches" exitcode=1 fi if [ -n "$fix" ]; then - find_cfiles -exec perl -pi -e 's/((?:^|[(<,;]|\bstatic\s+)\s*)\b(const)\b(?!\s+\w+\s*\[)/\1>\2\2torrent_from_obfuscated(obfuscated_hash); info) { + auto const& [addr, port] = peer_io->socket_address(); bool const client_is_seed = info->is_done; - bool const peer_is_seed = mediator_->is_peer_known_seed(info->id, peer_io->address()); + bool const peer_is_seed = mediator_->is_peer_known_seed(info->id, addr, port); tr_logAddTraceHand(this, fmt::format("got INCOMING connection's encrypted handshake for torrent [{}]", info->id)); peer_io->set_torrent_hash(info->info_hash); @@ -788,7 +789,8 @@ void tr_handshake::on_error(tr_peerIo* io, tr_error const& error, void* vhandsha /* Don't mark a peer as non-µTP unless it's really a connect failure. */ if ((error.code == ETIMEDOUT || error.code == ECONNREFUSED) && info) { - handshake->mediator_->set_utp_failed(info_hash, io->address()); + auto const& [addr, port] = io->socket_address(); + handshake->mediator_->set_utp_failed(info_hash, addr, port); } if (handshake->mediator_->allows_tcp() && io->reconnect()) diff --git a/libtransmission/handshake.h b/libtransmission/handshake.h index c9b6582c4..de30aa167 100644 --- a/libtransmission/handshake.h +++ b/libtransmission/handshake.h @@ -61,14 +61,15 @@ public: [[nodiscard]] virtual libtransmission::TimerMaker& timer_maker() = 0; [[nodiscard]] virtual bool allows_dht() const = 0; [[nodiscard]] virtual bool allows_tcp() const = 0; - [[nodiscard]] virtual bool is_peer_known_seed(tr_torrent_id_t tor_id, tr_address const& addr) const = 0; + [[nodiscard]] virtual bool is_peer_known_seed(tr_torrent_id_t tor_id, tr_address const& addr, tr_port const& port) + const = 0; [[nodiscard]] virtual size_t pad(void* setme, size_t max_bytes) const = 0; [[nodiscard]] virtual DH::private_key_bigend_t private_key() const { return DH::randomPrivateKey(); } - virtual void set_utp_failed(tr_sha1_digest_t const& info_hash, tr_address const&) = 0; + virtual void set_utp_failed(tr_sha1_digest_t const& info_hash, tr_address const&, tr_port const&) = 0; }; tr_handshake(Mediator* mediator, std::shared_ptr peer_io, tr_encryption_mode mode_in, DoneFunc on_done); diff --git a/libtransmission/peer-mgr.cc b/libtransmission/peer-mgr.cc index a6b67e1f0..3e170ffac 100644 --- a/libtransmission/peer-mgr.cc +++ b/libtransmission/peer-mgr.cc @@ -107,15 +107,15 @@ public: return session_.allowsTCP(); } - void set_utp_failed(tr_sha1_digest_t const& info_hash, tr_address const& addr) override + void set_utp_failed(tr_sha1_digest_t const& info_hash, tr_address const& addr, tr_port const& port) override { if (auto* const tor = session_.torrents().get(info_hash); tor != nullptr) { - tr_peerMgrSetUtpFailed(tor, addr, true); + tr_peerMgrSetUtpFailed(tor, addr, port, true); } } - [[nodiscard]] bool is_peer_known_seed(tr_torrent_id_t tor_id, tr_address const& addr) const override; + [[nodiscard]] bool is_peer_known_seed(tr_torrent_id_t tor_id, tr_address const& addr, tr_port const& port) const override; [[nodiscard]] libtransmission::TimerMaker& timer_maker() override { @@ -473,36 +473,36 @@ public: pool_is_all_seeds_.reset(); } - [[nodiscard]] peer_atom* get_existing_atom(tr_address const& addr) noexcept + [[nodiscard]] peer_atom* get_existing_atom(tr_address const& addr, tr_port const& port) noexcept { auto const iter = std::find_if( std::begin(pool), std::end(pool), - [&addr](auto const& atom) { return atom.addr == addr; }); + [&addr, &port](auto const& atom) { return atom.addr == addr && atom.port == port; }); return iter != std::end(pool) ? &*iter : nullptr; } - [[nodiscard]] peer_atom const* get_existing_atom(tr_address const& addr) const noexcept + [[nodiscard]] peer_atom const* get_existing_atom(tr_address const& addr, tr_port const& port) const noexcept { auto const iter = std::find_if( std::begin(pool), std::end(pool), - [&addr](auto const& atom) { return atom.addr == addr; }); + [&addr, &port](auto const& atom) { return atom.addr == addr && atom.port == port; }); return iter != std::end(pool) ? &*iter : nullptr; } - [[nodiscard]] bool peer_is_a_seed(tr_address const& addr) const noexcept + [[nodiscard]] bool peer_is_a_seed(tr_address const& addr, tr_port const& port) const noexcept { - auto const* const atom = get_existing_atom(addr); + auto const* const atom = get_existing_atom(addr, port); return atom != nullptr && atom->isSeed(); } - peer_atom* ensure_atom_exists(tr_address const& addr, tr_port const port, uint8_t const flags, uint8_t const from) + peer_atom* ensure_atom_exists(tr_address const& addr, tr_port const& port, uint8_t const flags, uint8_t const from) { TR_ASSERT(addr.is_valid()); TR_ASSERT(from < TR_PEER_FROM__MAX); - peer_atom* atom = get_existing_atom(addr); + peer_atom* atom = get_existing_atom(addr, port); if (atom == nullptr) { @@ -808,17 +808,17 @@ void tr_peerMgrOnBlocklistChanged(tr_peerMgr* mgr) // --- -void tr_peerMgrSetUtpSupported(tr_torrent* tor, tr_address const& addr) +void tr_peerMgrSetUtpSupported(tr_torrent* tor, tr_address const& addr, tr_port const& port) { - if (auto* const atom = tor->swarm->get_existing_atom(addr); atom != nullptr) + if (auto* const atom = tor->swarm->get_existing_atom(addr, port); atom != nullptr) { atom->flags |= ADDED_F_UTP_FLAGS; } } -void tr_peerMgrSetUtpFailed(tr_torrent* tor, tr_address const& addr, bool failed) +void tr_peerMgrSetUtpFailed(tr_torrent* tor, tr_address const& addr, tr_port const& port, bool failed) { - if (auto* const atom = tor->swarm->get_existing_atom(addr); atom != nullptr) + if (auto* const atom = tor->swarm->get_existing_atom(addr, port); atom != nullptr) { atom->utp_failed = failed; } @@ -1024,7 +1024,7 @@ void create_bit_torrent_peer(tr_torrent* tor, std::shared_ptr io, str { if (s != nullptr) { - struct peer_atom* atom = s->get_existing_atom(addr); + struct peer_atom* atom = s->get_existing_atom(addr, port); if (atom != nullptr) { @@ -2493,8 +2493,8 @@ void tr_peerMgr::makeNewPeerConnections(size_t max) // --- -bool HandshakeMediator::is_peer_known_seed(tr_torrent_id_t tor_id, tr_address const& addr) const +bool HandshakeMediator::is_peer_known_seed(tr_torrent_id_t tor_id, tr_address const& addr, tr_port const& port) const { auto const* const tor = session_.torrents().get(tor_id); - return tor != nullptr && tor->swarm != nullptr && tor->swarm->peer_is_a_seed(addr); + return tor != nullptr && tor->swarm != nullptr && tor->swarm->peer_is_a_seed(addr, port); } diff --git a/libtransmission/peer-mgr.h b/libtransmission/peer-mgr.h index 4566e685b..77623cb5d 100644 --- a/libtransmission/peer-mgr.h +++ b/libtransmission/peer-mgr.h @@ -163,9 +163,9 @@ constexpr bool tr_isPex(tr_pex const* pex) void tr_peerMgrFree(tr_peerMgr* manager); -void tr_peerMgrSetUtpSupported(tr_torrent* tor, tr_address const& addr); +void tr_peerMgrSetUtpSupported(tr_torrent* tor, tr_address const& addr, tr_port const& port); -void tr_peerMgrSetUtpFailed(tr_torrent* tor, tr_address const& addr, bool failed); +void tr_peerMgrSetUtpFailed(tr_torrent* tor, tr_address const& addr, tr_port const& port, bool failed); [[nodiscard]] std::vector tr_peerMgrGetNextRequests(tr_torrent* torrent, tr_peer const* peer, size_t numwant); diff --git a/libtransmission/peer-msgs.cc b/libtransmission/peer-msgs.cc index 3e9397d2e..9125b39c7 100644 --- a/libtransmission/peer-msgs.cc +++ b/libtransmission/peer-msgs.cc @@ -336,8 +336,9 @@ public: if (io->supports_utp()) { - tr_peerMgrSetUtpSupported(torrent, io->address()); - tr_peerMgrSetUtpFailed(torrent, io->address(), false); + auto const& [addr, port] = socketAddress(); + tr_peerMgrSetUtpSupported(torrent, addr, port); + tr_peerMgrSetUtpFailed(torrent, addr, port, false); } if (io->supports_ltep()) @@ -1090,7 +1091,8 @@ void parseLtepHandshake(tr_peerMsgsImpl* msgs, MessageReader& payload) { /* Mysterious µTorrent extension that we don't grok. However, it implies support for µTP, so use it to indicate that. */ - tr_peerMgrSetUtpFailed(msgs->torrent, msgs->io->address(), false); + auto const& [addr, port] = msgs->socketAddress(); + tr_peerMgrSetUtpFailed(msgs->torrent, addr, port, false); } } diff --git a/tests/libtransmission/handshake-test.cc b/tests/libtransmission/handshake-test.cc index 4c63a5e96..68cab886c 100644 --- a/tests/libtransmission/handshake-test.cc +++ b/tests/libtransmission/handshake-test.cc @@ -81,7 +81,8 @@ public: return true; } - [[nodiscard]] bool is_peer_known_seed(tr_torrent_id_t /*tor_id*/, tr_address const& /*addr*/) const override + [[nodiscard]] bool is_peer_known_seed(tr_torrent_id_t /*tor_id*/, tr_address const& /*addr*/, tr_port const& /*port*/) + const override { return false; } @@ -99,7 +100,7 @@ public: return private_key_; } - void set_utp_failed(tr_sha1_digest_t const& /*info_hash*/, tr_address const& /*addr*/) override + void set_utp_failed(tr_sha1_digest_t const& /*info_hash*/, tr_address const& /*addr*/, tr_port const& /*port*/) override { }