From b491da0ce48114d4f1573730645a6cdeb1af59ed Mon Sep 17 00:00:00 2001 From: Charles Kerr Date: Thu, 21 Oct 2021 21:40:55 -0500 Subject: [PATCH] refactor: add tr_peer_id_t (#2004) * refactor: move handshake_done args into a convenience struct * refactor: move peer_id from peerIo to tr_handshake tr_handshake is a short-term object and tr_peerIo is long-term, so this effectively narrows the scope of this field. * chore: remove unused field tr_peerIo.isEncrypted * refactor: add tr_peer_id_t type to hold peer ids. this is a 'using' alias to a std::array<> so that code passing peer-ids around doesn't have to memcmp / memcpy PEER_ID_LEN anymore. Also removes the now-unused PEER_ID_LEN macro. --- libtransmission/announcer-common.h | 3 +- libtransmission/announcer-http.cc | 2 +- libtransmission/announcer-udp.cc | 3 +- libtransmission/announcer.cc | 2 +- libtransmission/handshake.cc | 102 ++++++++++++-------------- libtransmission/handshake.h | 30 +++++--- libtransmission/peer-io.cc | 20 ----- libtransmission/peer-io.h | 22 ------ libtransmission/peer-mgr.cc | 45 +++++------- libtransmission/session.cc | 31 ++++---- libtransmission/session.h | 7 +- libtransmission/torrent.cc | 14 ++-- libtransmission/torrent.h | 5 +- libtransmission/tr-macros.h | 11 +++ tests/libtransmission/session-test.cc | 8 +- 15 files changed, 134 insertions(+), 171 deletions(-) diff --git a/libtransmission/announcer-common.h b/libtransmission/announcer-common.h index e79b86e91..aef761bd0 100644 --- a/libtransmission/announcer-common.h +++ b/libtransmission/announcer-common.h @@ -15,7 +15,6 @@ #include #include "transmission.h" /* SHA_DIGEST_LENGTH */ -#include "session.h" /* PEER_ID_LEN */ /*** **** SCRAPE @@ -158,7 +157,7 @@ struct tr_announce_request /* the torrent's peer id. * this changes when a torrent is stopped -> restarted. */ - char peer_id[PEER_ID_LEN]; + tr_peer_id_t peer_id; /* the torrent's info_hash */ uint8_t info_hash[SHA_DIGEST_LENGTH]; diff --git a/libtransmission/announcer-http.cc b/libtransmission/announcer-http.cc index b152238a0..394161f0d 100644 --- a/libtransmission/announcer-http.cc +++ b/libtransmission/announcer-http.cc @@ -65,7 +65,7 @@ static char* announce_url_new(tr_session const* session, tr_announce_request con req->url, strchr(req->url, '?') != nullptr ? '&' : '?', escaped_info_hash, - TR_ARG_TUPLE(PEER_ID_LEN, PEER_ID_LEN, req->peer_id), + TR_ARG_TUPLE(int(std::size(req->peer_id)), int(std::size(req->peer_id)), std::data(req->peer_id)), req->port, req->up, req->down, diff --git a/libtransmission/announcer-udp.cc b/libtransmission/announcer-udp.cc index c7c1a82c8..93854bb92 100644 --- a/libtransmission/announcer-udp.cc +++ b/libtransmission/announcer-udp.cc @@ -24,6 +24,7 @@ #include "peer-io.h" #include "peer-mgr.h" /* tr_peerMgrCompactToPex() */ #include "ptrarray.h" +#include "session.h" #include "tr-assert.h" #include "tr-udp.h" #include "utils.h" @@ -317,7 +318,7 @@ static struct tau_announce_request* tau_announce_request_new( evbuffer_add_hton_32(buf, TAU_ACTION_ANNOUNCE); evbuffer_add_hton_32(buf, transaction_id); evbuffer_add(buf, in->info_hash, SHA_DIGEST_LENGTH); - evbuffer_add(buf, in->peer_id, PEER_ID_LEN); + evbuffer_add(buf, std::data(in->peer_id), std::size(in->peer_id)); evbuffer_add_hton_64(buf, in->down); evbuffer_add_hton_64(buf, in->leftUntilComplete); evbuffer_add_hton_64(buf, in->up); diff --git a/libtransmission/announcer.cc b/libtransmission/announcer.cc index 5180ee8d6..7dbc04f93 100644 --- a/libtransmission/announcer.cc +++ b/libtransmission/announcer.cc @@ -935,7 +935,7 @@ static tr_announce_request* announce_request_new( req->url = tr_strdup(tier->currentTracker->announce); req->tracker_id_str = tr_strdup(tier->currentTracker->tracker_id_str); memcpy(req->info_hash, tor->info.hash, SHA_DIGEST_LENGTH); - memcpy(req->peer_id, tr_torrentGetPeerId(tor), PEER_ID_LEN); + req->peer_id = tr_torrentGetPeerId(tor); req->up = tier->byteCounts[TR_ANN_UP]; req->down = tier->byteCounts[TR_ANN_DOWN]; req->corrupt = tier->byteCounts[TR_ANN_CORRUPT]; diff --git a/libtransmission/handshake.cc b/libtransmission/handshake.cc index b8297997e..985414b86 100644 --- a/libtransmission/handshake.cc +++ b/libtransmission/handshake.cc @@ -6,6 +6,7 @@ * */ +#include #include #include /* strcmp(), strlen(), strncmp() */ @@ -105,7 +106,6 @@ enum handshake_state_t struct tr_handshake { bool haveReadAnythingFromPeer; - bool havePeerID; bool haveSentBitTorrentHandshake; tr_peerIo* io; tr_crypto* crypto; @@ -118,9 +118,12 @@ struct tr_handshake uint32_t crypto_select; uint32_t crypto_provide; uint8_t myReq1[SHA_DIGEST_LENGTH]; - handshakeDoneCB doneCB; - void* doneUserData; struct event* timeout_timer; + + std::optional peer_id; + + tr_handshake_done_func done_func; + void* done_func_user_data; }; /** @@ -173,19 +176,17 @@ static bool buildHandshakeMessage(tr_handshake* handshake, uint8_t* buf) { uint8_t const* const torrent_hash = tr_cryptoGetTorrentHash(handshake->crypto); tr_torrent* const tor = torrent_hash == nullptr ? nullptr : tr_torrentFindFromHash(handshake->session, torrent_hash); - unsigned char const* const peer_id = tor == nullptr ? nullptr : tr_torrentGetPeerId(tor); - bool const success = peer_id != nullptr; + bool const success = tor != nullptr; if (success) { uint8_t* walk = buf; - memcpy(walk, HANDSHAKE_NAME, HANDSHAKE_NAME_LEN); - walk += HANDSHAKE_NAME_LEN; + walk = std::copy_n(HANDSHAKE_NAME, HANDSHAKE_NAME_LEN, walk); + memset(walk, 0, HANDSHAKE_FLAGS_LEN); HANDSHAKE_SET_LTEP(walk); HANDSHAKE_SET_FASTEXT(walk); - /* Note that this doesn't depend on whether the torrent is private. * We don't accept DHT peers for a private torrent, * but we participate in the DHT regardless. */ @@ -193,13 +194,14 @@ static bool buildHandshakeMessage(tr_handshake* handshake, uint8_t* buf) { HANDSHAKE_SET_DHT(walk); } - walk += HANDSHAKE_FLAGS_LEN; - memcpy(walk, torrent_hash, SHA_DIGEST_LENGTH); - walk += SHA_DIGEST_LENGTH; - memcpy(walk, peer_id, PEER_ID_LEN); - TR_ASSERT(walk + PEER_ID_LEN - buf == HANDSHAKE_SIZE); + walk = std::copy_n(torrent_hash, SHA_DIGEST_LENGTH, walk); + + auto const& peer_id = tr_torrentGetPeerId(tor); + std::copy_n(std::data(peer_id), std::size(peer_id), walk); + + TR_ASSERT(walk + std::size(peer_id) - buf == HANDSHAKE_SIZE); } return success; @@ -220,8 +222,6 @@ static handshake_parse_err_t parseHandshake(tr_handshake* handshake, struct evbu uint8_t name[HANDSHAKE_NAME_LEN]; uint8_t reserved[HANDSHAKE_FLAGS_LEN]; uint8_t hash[SHA_DIGEST_LENGTH]; - tr_torrent* tor; - uint8_t peer_id[PEER_ID_LEN]; dbgmsg(handshake, "payload: need %d, got %zu", HANDSHAKE_SIZE, evbuffer_get_length(inbuf)); @@ -252,17 +252,16 @@ static handshake_parse_err_t parseHandshake(tr_handshake* handshake, struct evbu return HANDSHAKE_BAD_TORRENT; } - /* peer_id */ - tr_peerIoReadBytes(handshake->io, inbuf, peer_id, sizeof(peer_id)); - tr_peerIoSetPeersId(handshake->io, peer_id); + // peer_id + auto peer_id = tr_peer_id_t{}; + tr_peerIoReadBytes(handshake->io, inbuf, std::data(peer_id), std::size(peer_id)); + handshake->peer_id = peer_id; /* peer id */ - handshake->havePeerID = true; - dbgmsg(handshake, "peer-id is [%*.*s]", TR_ARG_TUPLE(PEER_ID_LEN, PEER_ID_LEN, peer_id)); + dbgmsg(handshake, "peer-id is [%*.*s]", TR_ARG_TUPLE(int(std::size(peer_id)), int(std::size(peer_id)), std::data(peer_id))); - tor = tr_torrentFindFromHash(handshake->session, hash); - - if (memcmp(peer_id, tr_torrentGetPeerId(tor), PEER_ID_LEN) == 0) + auto* const tor = tr_torrentFindFromHash(handshake->session, hash); + if (peer_id == tr_torrentGetPeerId(tor)) { dbgmsg(handshake, "streuth! we've connected to ourselves."); return HANDSHAKE_PEER_IS_SELF; @@ -694,26 +693,22 @@ static ReadState readHandshake(tr_handshake* handshake, struct evbuffer* inbuf) static ReadState readPeerId(tr_handshake* handshake, struct evbuffer* inbuf) { - bool connected_to_self; - char client[128]; - uint8_t peer_id[PEER_ID_LEN]; - tr_torrent* tor; - - if (evbuffer_get_length(inbuf) < PEER_ID_LEN) + // read the peer_id + auto peer_id = tr_peer_id_t{}; + if (evbuffer_get_length(inbuf) < std::size(peer_id)) { return READ_LATER; } + tr_peerIoReadBytes(handshake->io, inbuf, std::data(peer_id), std::size(peer_id)); + handshake->peer_id = peer_id; - /* peer id */ - tr_peerIoReadBytes(handshake->io, inbuf, peer_id, PEER_ID_LEN); - tr_peerIoSetPeersId(handshake->io, peer_id); - handshake->havePeerID = true; - tr_clientForId(client, sizeof(client), peer_id); + char client[128] = {}; + tr_clientForId(client, sizeof(client), std::data(peer_id)); dbgmsg(handshake, "peer-id is [%s] ... isIncoming is %d", client, tr_peerIoIsIncoming(handshake->io)); - /* if we've somehow connected to ourselves, don't keep the connection */ - tor = tr_torrentFindFromHash(handshake->session, tr_peerIoGetTorrentHash(handshake->io)); - connected_to_self = tor != nullptr && memcmp(peer_id, tr_torrentGetPeerId(tor), PEER_ID_LEN) == 0; + // if we've somehow connected to ourselves, don't keep the connection + auto* const tor = tr_torrentFindFromHash(handshake->session, tr_peerIoGetTorrentHash(handshake->io)); + bool const connected_to_self = peer_id == tr_torrentGetPeerId(tor); return tr_handshakeDone(handshake, !connected_to_self); } @@ -1089,15 +1084,14 @@ static ReadState canRead(tr_peerIo* io, void* vhandshake, size_t* piece) static bool fireDoneFunc(tr_handshake* handshake, bool isConnected) { - uint8_t const* peer_id = (isConnected && handshake->havePeerID) ? tr_peerIoGetPeersId(handshake->io) : nullptr; - bool const success = (*handshake->doneCB)( - handshake, - handshake->io, - handshake->haveReadAnythingFromPeer, - isConnected, - peer_id, - handshake->doneUserData); - + auto result = tr_handshake_result{}; + result.handshake = handshake; + result.io = handshake->io; + result.readAnythingFromPeer = handshake->haveReadAnythingFromPeer; + result.isConnected = isConnected; + result.userData = handshake->done_func_user_data; + result.peer_id = handshake->peer_id; + bool const success = (*handshake->done_func)(result); return success; } @@ -1114,15 +1108,11 @@ static void tr_handshakeFree(tr_handshake* handshake) static ReadState tr_handshakeDone(tr_handshake* handshake, bool isOK) { - bool success; - dbgmsg(handshake, "handshakeDone: %s", isOK ? "connected" : "aborting"); tr_peerIoSetIOFuncs(handshake->io, nullptr, nullptr, nullptr, nullptr); - success = fireDoneFunc(handshake, isOK); - + bool const success = fireDoneFunc(handshake, isOK); tr_handshakeFree(handshake); - return success ? READ_LATER : READ_ERR; } @@ -1200,7 +1190,11 @@ static void handshakeTimeout([[maybe_unused]] evutil_socket_t s, [[maybe_unused] tr_handshakeAbort(static_cast(handshake)); } -tr_handshake* tr_handshakeNew(tr_peerIo* io, tr_encryption_mode encryptionMode, handshakeDoneCB doneCB, void* doneUserData) +tr_handshake* tr_handshakeNew( + tr_peerIo* io, + tr_encryption_mode encryptionMode, + tr_handshake_done_func done_func, + void* done_func_user_data) { tr_handshake* handshake; tr_session* session = tr_peerIoGetSession(io); @@ -1209,8 +1203,8 @@ tr_handshake* tr_handshakeNew(tr_peerIo* io, tr_encryption_mode encryptionMode, handshake->io = io; handshake->crypto = tr_peerIoGetCrypto(io); handshake->encryptionMode = encryptionMode; - handshake->doneCB = doneCB; - handshake->doneUserData = doneUserData; + handshake->done_func = done_func; + handshake->done_func_user_data = done_func_user_data; handshake->session = session; handshake->timeout_timer = evtimer_new(session->event_base, handshakeTimeout, handshake); tr_timerAdd(handshake->timeout_timer, HANDSHAKE_TIMEOUT_SEC, 0); diff --git a/libtransmission/handshake.h b/libtransmission/handshake.h index 9c0b9fef3..a81da751d 100644 --- a/libtransmission/handshake.h +++ b/libtransmission/handshake.h @@ -12,6 +12,8 @@ #error only libtransmission should #include this header. #endif +#include + #include "transmission.h" #include "net.h" @@ -24,17 +26,25 @@ class tr_peerIo; freed when the handshake is completed. */ struct tr_handshake; -/* returns true on success, false on error */ -using handshakeDoneCB = bool (*)( - struct tr_handshake* handshake, - tr_peerIo* io, - bool readAnythingFromPeer, - bool isConnected, - uint8_t const* peerId, - void* userData); +struct tr_handshake_result +{ + struct tr_handshake* handshake; + tr_peerIo* io; + bool readAnythingFromPeer; + bool isConnected; + void* userData; + std::optional peer_id; +}; -/** @brief instantiate a new handshake */ -tr_handshake* tr_handshakeNew(tr_peerIo* io, tr_encryption_mode encryptionMode, handshakeDoneCB doneCB, void* doneUserData); +/* returns true on success, false on error */ +using tr_handshake_done_func = bool (*)(tr_handshake_result const& result); + +/** @brief create a new handshake */ +tr_handshake* tr_handshakeNew( + tr_peerIo* io, + tr_encryption_mode encryption_mode, + tr_handshake_done_func when_done, + void* when_done_user_data); tr_address const* tr_handshakeGetAddr(struct tr_handshake const* handshake, tr_port* port); diff --git a/libtransmission/peer-io.cc b/libtransmission/peer-io.cc index a529a116b..6baace38c 100644 --- a/libtransmission/peer-io.cc +++ b/libtransmission/peer-io.cc @@ -1041,26 +1041,6 @@ bool tr_peerIoHasTorrentHash(tr_peerIo const* io) *** **/ -void tr_peerIoSetPeersId(tr_peerIo* io, uint8_t const* peer_id) -{ - TR_ASSERT(tr_isPeerIo(io)); - - if (peer_id == nullptr) - { - memset(io->peerId, '\0', sizeof(io->peerId)); - io->peerIdIsSet = false; - } - else - { - memcpy(io->peerId, peer_id, sizeof(io->peerId)); - io->peerIdIsSet = true; - } -} - -/** -*** -**/ - static unsigned int getDesiredOutputBufferSize(tr_peerIo const* io, uint64_t now) { /* this is all kind of arbitrary, but what seems to work well is diff --git a/libtransmission/peer-io.h b/libtransmission/peer-io.h index 6a50fae6f..5fd9af316 100644 --- a/libtransmission/peer-io.h +++ b/libtransmission/peer-io.h @@ -16,8 +16,6 @@ *** **/ -#include - #include #include "transmission.h" @@ -116,9 +114,6 @@ public: // TODO: use std::shared_ptr instead of manual refcounting? int refCount = 1; - // TODO(ckerr): I think this can be moved to tr_handshake - uint8_t peerId[SHA_DIGEST_LENGTH] = {}; - short int pendingEvents = 0; tr_port const port; @@ -129,9 +124,6 @@ public: bool dhtSupported = false; bool extendedProtocolSupported = false; bool fastExtensionSupported = false; - bool isEncrypted = false; - // TODO(ckerr): I think this can be moved to tr_handshake - bool peerIdIsSet = false; bool utpSupported = false; }; @@ -246,20 +238,6 @@ static inline int tr_peerIoGetAge(tr_peerIo const* io) *** **/ -void tr_peerIoSetPeersId(tr_peerIo* io, uint8_t const* peer_id); - -constexpr uint8_t const* tr_peerIoGetPeersId(tr_peerIo const* io) -{ - TR_ASSERT(tr_isPeerIo(io)); - TR_ASSERT(io->peerIdIsSet); - - return io->peerId; -} - -/** -*** -**/ - void tr_peerIoSetIOFuncs(tr_peerIo* io, tr_can_read_cb readcb, tr_did_write_cb writecb, tr_net_error_cb errcb, void* user_data); void tr_peerIoClear(tr_peerIo* io); diff --git a/libtransmission/peer-mgr.cc b/libtransmission/peer-mgr.cc index d63ebb280..18461ed60 100644 --- a/libtransmission/peer-mgr.cc +++ b/libtransmission/peer-mgr.cc @@ -1931,28 +1931,22 @@ static void createBitTorrentPeer(tr_torrent* tor, tr_peerIo* io, struct peer_ato } /* FIXME: this is kind of a mess. */ -static bool myHandshakeDoneCB( - tr_handshake* handshake, - tr_peerIo* io, - bool readAnythingFromPeer, - bool isConnected, - uint8_t const* peer_id, - void* vmanager) +static bool on_handshake_done(tr_handshake_result const& result) { - TR_ASSERT(io != nullptr); + TR_ASSERT(result.io != nullptr); - bool ok = isConnected; + bool ok = result.isConnected; bool success = false; - auto* manager = static_cast(vmanager); - tr_swarm* s = tr_peerIoHasTorrentHash(io) ? getExistingSwarm(manager, tr_peerIoGetTorrentHash(io)) : nullptr; + auto* manager = static_cast(result.userData); + tr_swarm* s = tr_peerIoHasTorrentHash(result.io) ? getExistingSwarm(manager, tr_peerIoGetTorrentHash(result.io)) : nullptr; - if (tr_peerIoIsIncoming(io)) + if (tr_peerIoIsIncoming(result.io)) { - tr_ptrArrayRemoveSortedPointer(&manager->incomingHandshakes, handshake, handshakeCompare); + tr_ptrArrayRemoveSortedPointer(&manager->incomingHandshakes, result.handshake, handshakeCompare); } else if (s != nullptr) { - tr_ptrArrayRemoveSortedPointer(&s->outgoingHandshakes, handshake, handshakeCompare); + tr_ptrArrayRemoveSortedPointer(&s->outgoingHandshakes, result.handshake, handshakeCompare); } if (s != nullptr) @@ -1961,7 +1955,7 @@ static bool myHandshakeDoneCB( } auto port = tr_port{}; - tr_address const* const addr = tr_peerIoGetAddress(io, &port); + tr_address const* const addr = tr_peerIoGetAddress(result.io, &port); if (!ok || s == nullptr || !s->isRunning) { @@ -1973,7 +1967,7 @@ static bool myHandshakeDoneCB( { ++atom->numFails; - if (!readAnythingFromPeer) + if (!result.readAnythingFromPeer) { tordbg(s, "marking peer %s as unreachable... numFails is %d", tr_atomAddrStr(atom), (int)atom->numFails); atom->flags2 |= MyflagUnreachable; @@ -1989,7 +1983,7 @@ static bool myHandshakeDoneCB( atom->piece_data_time = 0; atom->lastConnectionAt = tr_time(); - if (!tr_peerIoIsIncoming(io)) + if (!tr_peerIoIsIncoming(result.io)) { atom->flags |= ADDED_F_CONNECTABLE; atom->flags2 &= ~MyflagUnreachable; @@ -1997,7 +1991,7 @@ static bool myHandshakeDoneCB( /* In principle, this flag specifies whether the peer groks uTP, not whether it's currently connected over uTP. */ - if (io->socket.type == TR_PEER_SOCKET_TYPE_UTP) + if (result.io->socket.type == TR_PEER_SOCKET_TYPE_UTP) { atom->flags |= ADDED_F_UTP_FLAGS; } @@ -2006,7 +2000,7 @@ static bool myHandshakeDoneCB( { tordbg(s, "banned peer %s tried to reconnect", tr_atomAddrStr(atom)); } - else if (tr_peerIoIsIncoming(io) && getPeerCount(s) >= getMaxPeerCount(s->tor)) + else if (tr_peerIoIsIncoming(result.io) && getPeerCount(s) >= getMaxPeerCount(s->tor)) { /* too many peers already */ } @@ -2021,14 +2015,15 @@ static bool myHandshakeDoneCB( else { auto client = tr_quark{ TR_KEY_NONE }; - if (peer_id != nullptr) + if (result.peer_id) { - char buf[128]; - client = tr_quark_new(tr_clientForId(buf, sizeof(buf), peer_id)); + char buf[128] = {}; + tr_clientForId(buf, sizeof(buf), std::data(*result.peer_id)); + client = tr_quark_new(buf); } /* this steals its refcount too, which is balanced by our unref in peerDelete() */ - tr_peerIo* stolen = tr_handshakeStealIO(handshake); + tr_peerIo* stolen = tr_handshakeStealIO(result.handshake); tr_peerIoSetParent(stolen, s->tor->bandwidth); createBitTorrentPeer(s->tor, stolen, atom, client); @@ -2089,7 +2084,7 @@ void tr_peerMgrAddIncoming(tr_peerMgr* manager, tr_address* addr, tr_port port, else /* we don't have a connection to them yet... */ { tr_peerIo* const io = tr_peerIoNewIncoming(session, session->bandwidth, addr, port, socket); - tr_handshake* const handshake = tr_handshakeNew(io, session->encryptionMode, myHandshakeDoneCB, manager); + tr_handshake* const handshake = tr_handshakeNew(io, session->encryptionMode, on_handshake_done, manager); tr_peerIoUnref(io); /* balanced by the implicit ref in tr_peerIoNewIncoming() */ @@ -4216,7 +4211,7 @@ static void initiateConnection(tr_peerMgr* mgr, tr_swarm* s, struct peer_atom* a } else { - tr_handshake* handshake = tr_handshakeNew(io, mgr->session->encryptionMode, myHandshakeDoneCB, mgr); + tr_handshake* handshake = tr_handshakeNew(io, mgr->session->encryptionMode, on_handshake_done, mgr); TR_ASSERT(tr_peerIoGetTorrentHash(io)); diff --git a/libtransmission/session.cc b/libtransmission/session.cc index 0cfcd2345..0aa395267 100644 --- a/libtransmission/session.cc +++ b/libtransmission/session.cc @@ -83,27 +83,30 @@ static tr_port getRandomPort(tr_session* s) characters, where x is the major version number, y is the minor version number, z is the maintenance number, and b designates beta (Azureus-style) */ -void tr_peerIdInit(uint8_t* buf) +tr_peer_id_t tr_peerIdInit() { - int total = 0; - // TODO: use a string_view - char const* pool = "0123456789abcdefghijklmnopqrstuvwxyz"; - int const base = 36; + auto peer_id = tr_peer_id_t{}; + auto* it = std::data(peer_id); - memcpy(buf, PEERID_PREFIX, 8); + // starts with -TRXXXX- + auto constexpr Prefix = std::string_view{ PEERID_PREFIX }; + auto const* const end = it + std::size(peer_id); + it = std::copy_n(std::data(Prefix), std::size(Prefix), it); - tr_rand_buffer(buf + 8, 11); - - for (int i = 8; i < 19; ++i) + // remainder is randomly-generated characters + auto constexpr Pool = std::string_view{ "0123456789abcdefghijklmnopqrstuvwxyz" }; + auto total = int{ 0 }; + tr_rand_buffer(it, end - it); + while (it + 1 < end) { - int const val = buf[i] % base; + int const val = *it % std::size(Pool); total += val; - buf[i] = pool[val]; + *it++ = Pool[val]; } + int const val = total % std::size(Pool) != 0 ? std::size(Pool) - total % std::size(Pool) : 0; + *it = Pool[val]; - int const val = total % base != 0 ? base - total % base : 0; - buf[19] = pool[val]; - buf[20] = '\0'; + return peer_id; } /*** diff --git a/libtransmission/session.h b/libtransmission/session.h index c85e6c4ac..6b26bae8c 100644 --- a/libtransmission/session.h +++ b/libtransmission/session.h @@ -45,12 +45,7 @@ enum tr_auto_switch_state_t TR_AUTO_SWITCH_OFF, }; -enum -{ - PEER_ID_LEN = 20 -}; - -void tr_peerIdInit(uint8_t* setme); +tr_peer_id_t tr_peerIdInit(); struct event_base; struct evdns_base; diff --git a/libtransmission/torrent.cc b/libtransmission/torrent.cc index 92b14cf44..6b1cea1ce 100644 --- a/libtransmission/torrent.cc +++ b/libtransmission/torrent.cc @@ -155,10 +155,10 @@ bool tr_torrentIsPieceTransferAllowed(tr_torrent const* tor, tr_direction direct **** ***/ -static constexpr void tr_torrentUnsetPeerId(tr_torrent* tor) +static void tr_torrentUnsetPeerId(tr_torrent* tor) { - /* triggers a rebuild next time tr_torrentGetPeerId() is called */ - *tor->peer_id = '\0'; + // triggers a rebuild next time tr_torrentGetPeerId() is called + tor->peer_id.reset(); } static int peerIdTTL(tr_torrent const* tor) @@ -167,18 +167,18 @@ static int peerIdTTL(tr_torrent const* tor) return ctime == 0 ? 0 : (int)difftime(ctime + tor->session->peer_id_ttl_hours * 3600, tr_time()); } -unsigned char const* tr_torrentGetPeerId(tr_torrent* tor) +tr_peer_id_t const& tr_torrentGetPeerId(tr_torrent* tor) { - bool const needs_new_peer_id = (*tor->peer_id == '\0') || // doesn't have one + bool const needs_new_peer_id = !tor->peer_id || // doesn't have one (!tr_torrentIsPrivate(tor) && (peerIdTTL(tor) <= 0)); // has one but it's expired if (needs_new_peer_id) { - tr_peerIdInit(tor->peer_id); + tor->peer_id = tr_peerIdInit(); tor->peer_id_creation_time = tr_time(); } - return tor->peer_id; + return *tor->peer_id; } /*** diff --git a/libtransmission/torrent.h b/libtransmission/torrent.h index dbe463ec8..c2eb3ed41 100644 --- a/libtransmission/torrent.h +++ b/libtransmission/torrent.h @@ -12,6 +12,7 @@ #error only libtransmission should #include this header. #endif +#include #include #include #include @@ -154,7 +155,7 @@ struct tr_torrent * peer_id that was registered by the peer. The peer_id from the tracker * and in the handshake are expected to match. */ - unsigned char peer_id[PEER_ID_LEN + 1]; + std::optional peer_id; time_t peer_id_creation_time; @@ -426,7 +427,7 @@ time_t tr_torrentGetFileMTime(tr_torrent const* tor, tr_file_index_t i); uint64_t tr_torrentGetCurrentSizeOnDisk(tr_torrent const* tor); -unsigned char const* tr_torrentGetPeerId(tr_torrent* tor); +tr_peer_id_t const& tr_torrentGetPeerId(tr_torrent* tor); static inline uint64_t tr_torrentGetLeftUntilDone(tr_torrent const* tor) { diff --git a/libtransmission/tr-macros.h b/libtransmission/tr-macros.h index 511daef9c..e85eaa0f3 100644 --- a/libtransmission/tr-macros.h +++ b/libtransmission/tr-macros.h @@ -8,6 +8,9 @@ #pragma once +#include +#include + /*** **** ***/ @@ -119,3 +122,11 @@ // Mostly to enforce better formatting #define TR_ARG_TUPLE(...) __VA_ARGS__ + +auto inline constexpr PEER_ID_LEN = size_t{ 20 }; + +// https://www.bittorrent.org/beps/bep_0003.html +// A string of length 20 which this downloader uses as its id. Each +// downloader generates its own id at random at the start of a new +// download. This value will also almost certainly have to be escaped. +using tr_peer_id_t = std::array; diff --git a/tests/libtransmission/session-test.cc b/tests/libtransmission/session-test.cc index 9c844f892..617441787 100644 --- a/tests/libtransmission/session-test.cc +++ b/tests/libtransmission/session-test.cc @@ -28,14 +28,10 @@ TEST(Session, peerId) for (int i = 0; i < 100000; ++i) { // get a new peer-id - auto buf = std::array{}; - tr_peerIdInit(buf.data()); - - // confirm that it has the right length - EXPECT_EQ(PEER_ID_LEN, strlen(reinterpret_cast(buf.data()))); + auto const buf = tr_peerIdInit(); // confirm that it begins with peer_id_prefix - auto const peer_id = std::string(reinterpret_cast(buf.data()), PEER_ID_LEN); + auto const peer_id = std::string_view(reinterpret_cast(buf.data()), PEER_ID_LEN); EXPECT_EQ(peer_id_prefix, peer_id.substr(0, peer_id_prefix.size())); // confirm that its total is evenly divisible by 36