From 481d41333c5c3a4a6a0cc0b968d1b904bc9284ee Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Tue, 10 Sep 2024 13:29:50 +0100 Subject: [PATCH] feat: [#569] allow UDP clients to limit peers in response The UDP tracker announce response always include all peers available up to a maxium of 74 peers, ignoring the `num_want` param in the request described in: https://www.bittorrent.org/beps/bep_0015.html This change applies that limit only when is lower than then TORRENT_PEERS_LIMIT (74). --- src/core/mod.rs | 178 ++++++++++++++++++++--- src/servers/http/v1/services/announce.rs | 4 +- src/servers/http/v1/services/scrape.rs | 8 +- src/servers/udp/handlers.rs | 5 +- tests/servers/udp/contract.rs | 2 +- 5 files changed, 167 insertions(+), 30 deletions(-) diff --git a/src/core/mod.rs b/src/core/mod.rs index cbdd7bcb..1d2d856b 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -448,6 +448,7 @@ pub mod torrent; pub mod peer_tests; +use std::cmp::max; use std::collections::HashMap; use std::net::IpAddr; use std::panic::Location; @@ -520,6 +521,38 @@ pub struct AnnounceData { pub policy: AnnouncePolicy, } +/// How many peers the peer announcing wants in the announce response. +#[derive(Clone, Debug, PartialEq, Default)] +pub enum PeersWanted { + /// The peer wants as many peers as possible in the announce response. + #[default] + All, + /// The peer only wants a certain amount of peers in the announce response. + Only { amount: usize }, +} + +impl PeersWanted { + fn limit(&self) -> usize { + match self { + PeersWanted::All => TORRENT_PEERS_LIMIT, + PeersWanted::Only { amount } => *amount, + } + } +} + +impl From for PeersWanted { + fn from(value: i32) -> Self { + if value > 0 { + match value.try_into() { + Ok(peers_wanted) => Self::Only { amount: peers_wanted }, + Err(_) => Self::All, + } + } else { + Self::All + } + } +} + /// Structure that holds the data returned by the `scrape` request. #[derive(Debug, PartialEq, Default)] pub struct ScrapeData { @@ -639,7 +672,13 @@ impl Tracker { /// # Context: Tracker /// /// BEP 03: [The `BitTorrent` Protocol Specification](https://www.bittorrent.org/beps/bep_0003.html). - pub fn announce(&self, info_hash: &InfoHash, peer: &mut peer::Peer, remote_client_ip: &IpAddr) -> AnnounceData { + pub fn announce( + &self, + info_hash: &InfoHash, + peer: &mut peer::Peer, + remote_client_ip: &IpAddr, + peers_wanted: &PeersWanted, + ) -> AnnounceData { // code-review: maybe instead of mutating the peer we could just return // a tuple with the new peer and the announce data: (Peer, AnnounceData). // It could even be a different struct: `StoredPeer` or `PublicPeer`. @@ -661,7 +700,7 @@ impl Tracker { let stats = self.upsert_peer_and_get_stats(info_hash, peer); - let peers = self.get_peers_for(info_hash, peer); + let peers = self.get_peers_for(info_hash, peer, peers_wanted.limit()); AnnounceData { peers, @@ -713,16 +752,21 @@ impl Tracker { Ok(()) } - fn get_peers_for(&self, info_hash: &InfoHash, peer: &peer::Peer) -> Vec> { + /// # Context: Tracker + /// + /// Get torrent peers for a given torrent and client. + /// + /// It filters out the client making the request. + fn get_peers_for(&self, info_hash: &InfoHash, peer: &peer::Peer, limit: usize) -> Vec> { match self.torrents.get(info_hash) { None => vec![], - Some(entry) => entry.get_peers_for_client(&peer.peer_addr, Some(TORRENT_PEERS_LIMIT)), + Some(entry) => entry.get_peers_for_client(&peer.peer_addr, Some(max(limit, TORRENT_PEERS_LIMIT))), } } /// # Context: Tracker /// - /// Get all torrent peers for a given torrent + /// Get torrent peers for a given torrent. pub fn get_torrent_peers(&self, info_hash: &InfoHash) -> Vec> { match self.torrents.get(info_hash) { None => vec![], @@ -1199,6 +1243,7 @@ mod tests { use std::sync::Arc; use aquatic_udp_protocol::{AnnounceEvent, NumberOfBytes, PeerId}; + use torrust_tracker_configuration::TORRENT_PEERS_LIMIT; use torrust_tracker_primitives::info_hash::InfoHash; use torrust_tracker_primitives::DurationSinceUnixEpoch; use torrust_tracker_test_helpers::configuration; @@ -1328,7 +1373,7 @@ mod tests { } #[tokio::test] - async fn it_should_return_all_the_peers_for_a_given_torrent() { + async fn it_should_return_the_peers_for_a_given_torrent() { let tracker = public_tracker(); let info_hash = sample_info_hash(); @@ -1341,8 +1386,51 @@ mod tests { assert_eq!(peers, vec![Arc::new(peer)]); } + /// It generates a peer id from a number where the number is the last + /// part of the peer ID. For example, for `12` it returns + /// `-qB00000000000000012`. + fn numeric_peer_id(two_digits_value: i32) -> PeerId { + // Format idx as a string with leading zeros, ensuring it has exactly 2 digits + let idx_str = format!("{two_digits_value:02}"); + + // Create the base part of the peer ID. + let base = b"-qB00000000000000000"; + + // Concatenate the base with idx bytes, ensuring the total length is 20 bytes. + let mut peer_id_bytes = [0u8; 20]; + peer_id_bytes[..base.len()].copy_from_slice(base); + peer_id_bytes[base.len() - idx_str.len()..].copy_from_slice(idx_str.as_bytes()); + + PeerId(peer_id_bytes) + } + #[tokio::test] - async fn it_should_return_all_the_peers_for_a_given_torrent_excluding_a_given_peer() { + async fn it_should_return_74_peers_at_the_most_for_a_given_torrent() { + let tracker = public_tracker(); + + let info_hash = sample_info_hash(); + + for idx in 1..=75 { + let peer = Peer { + peer_id: numeric_peer_id(idx), + peer_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(126, 0, 0, idx.try_into().unwrap())), 8080), + updated: DurationSinceUnixEpoch::new(1_669_397_478_934, 0), + uploaded: NumberOfBytes::new(0), + downloaded: NumberOfBytes::new(0), + left: NumberOfBytes::new(0), // No bytes left to download + event: AnnounceEvent::Completed, + }; + + tracker.upsert_peer_and_get_stats(&info_hash, &peer); + } + + let peers = tracker.get_torrent_peers(&info_hash); + + assert_eq!(peers.len(), 74); + } + + #[tokio::test] + async fn it_should_return_the_peers_for_a_given_torrent_excluding_a_given_peer() { let tracker = public_tracker(); let info_hash = sample_info_hash(); @@ -1350,11 +1438,41 @@ mod tests { tracker.upsert_peer_and_get_stats(&info_hash, &peer); - let peers = tracker.get_peers_for(&info_hash, &peer); + let peers = tracker.get_peers_for(&info_hash, &peer, TORRENT_PEERS_LIMIT); assert_eq!(peers, vec![]); } + #[tokio::test] + async fn it_should_return_74_peers_at_the_most_for_a_given_torrent_when_it_filters_out_a_given_peer() { + let tracker = public_tracker(); + + let info_hash = sample_info_hash(); + + let excluded_peer = sample_peer(); + + tracker.upsert_peer_and_get_stats(&info_hash, &excluded_peer); + + // Add 74 peers + for idx in 2..=75 { + let peer = Peer { + peer_id: numeric_peer_id(idx), + peer_addr: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(126, 0, 0, idx.try_into().unwrap())), 8080), + updated: DurationSinceUnixEpoch::new(1_669_397_478_934, 0), + uploaded: NumberOfBytes::new(0), + downloaded: NumberOfBytes::new(0), + left: NumberOfBytes::new(0), // No bytes left to download + event: AnnounceEvent::Completed, + }; + + tracker.upsert_peer_and_get_stats(&info_hash, &peer); + } + + let peers = tracker.get_peers_for(&info_hash, &excluded_peer, TORRENT_PEERS_LIMIT); + + assert_eq!(peers.len(), 74); + } + #[tokio::test] async fn it_should_return_the_torrent_metrics() { let tracker = public_tracker(); @@ -1409,6 +1527,7 @@ mod tests { use crate::core::tests::the_tracker::{ peer_ip, public_tracker, sample_info_hash, sample_peer, sample_peer_1, sample_peer_2, }; + use crate::core::PeersWanted; mod should_assign_the_ip_to_the_peer { @@ -1514,7 +1633,7 @@ mod tests { let mut peer = sample_peer(); - let announce_data = tracker.announce(&sample_info_hash(), &mut peer, &peer_ip()); + let announce_data = tracker.announce(&sample_info_hash(), &mut peer, &peer_ip(), &PeersWanted::All); assert_eq!(announce_data.peers, vec![]); } @@ -1524,10 +1643,15 @@ mod tests { let tracker = public_tracker(); let mut previously_announced_peer = sample_peer_1(); - tracker.announce(&sample_info_hash(), &mut previously_announced_peer, &peer_ip()); + tracker.announce( + &sample_info_hash(), + &mut previously_announced_peer, + &peer_ip(), + &PeersWanted::All, + ); let mut peer = sample_peer_2(); - let announce_data = tracker.announce(&sample_info_hash(), &mut peer, &peer_ip()); + let announce_data = tracker.announce(&sample_info_hash(), &mut peer, &peer_ip(), &PeersWanted::All); assert_eq!(announce_data.peers, vec![Arc::new(previously_announced_peer)]); } @@ -1537,6 +1661,7 @@ mod tests { use crate::core::tests::the_tracker::{ completed_peer, leecher, peer_ip, public_tracker, sample_info_hash, seeder, started_peer, }; + use crate::core::PeersWanted; #[tokio::test] async fn when_the_peer_is_a_seeder() { @@ -1544,7 +1669,7 @@ mod tests { let mut peer = seeder(); - let announce_data = tracker.announce(&sample_info_hash(), &mut peer, &peer_ip()); + let announce_data = tracker.announce(&sample_info_hash(), &mut peer, &peer_ip(), &PeersWanted::All); assert_eq!(announce_data.stats.complete, 1); } @@ -1555,7 +1680,7 @@ mod tests { let mut peer = leecher(); - let announce_data = tracker.announce(&sample_info_hash(), &mut peer, &peer_ip()); + let announce_data = tracker.announce(&sample_info_hash(), &mut peer, &peer_ip(), &PeersWanted::All); assert_eq!(announce_data.stats.incomplete, 1); } @@ -1566,10 +1691,11 @@ mod tests { // We have to announce with "started" event because peer does not count if peer was not previously known let mut started_peer = started_peer(); - tracker.announce(&sample_info_hash(), &mut started_peer, &peer_ip()); + tracker.announce(&sample_info_hash(), &mut started_peer, &peer_ip(), &PeersWanted::All); let mut completed_peer = completed_peer(); - let announce_data = tracker.announce(&sample_info_hash(), &mut completed_peer, &peer_ip()); + let announce_data = + tracker.announce(&sample_info_hash(), &mut completed_peer, &peer_ip(), &PeersWanted::All); assert_eq!(announce_data.stats.downloaded, 1); } @@ -1583,7 +1709,7 @@ mod tests { use torrust_tracker_primitives::info_hash::InfoHash; use crate::core::tests::the_tracker::{complete_peer, incomplete_peer, public_tracker}; - use crate::core::{ScrapeData, SwarmMetadata}; + use crate::core::{PeersWanted, ScrapeData, SwarmMetadata}; #[tokio::test] async fn it_should_return_a_zeroed_swarm_metadata_for_the_requested_file_if_the_tracker_does_not_have_that_torrent( @@ -1609,11 +1735,21 @@ mod tests { // Announce a "complete" peer for the torrent let mut complete_peer = complete_peer(); - tracker.announce(&info_hash, &mut complete_peer, &IpAddr::V4(Ipv4Addr::new(126, 0, 0, 10))); + tracker.announce( + &info_hash, + &mut complete_peer, + &IpAddr::V4(Ipv4Addr::new(126, 0, 0, 10)), + &PeersWanted::All, + ); // Announce an "incomplete" peer for the torrent let mut incomplete_peer = incomplete_peer(); - tracker.announce(&info_hash, &mut incomplete_peer, &IpAddr::V4(Ipv4Addr::new(126, 0, 0, 11))); + tracker.announce( + &info_hash, + &mut incomplete_peer, + &IpAddr::V4(Ipv4Addr::new(126, 0, 0, 11)), + &PeersWanted::All, + ); // Scrape let scrape_data = tracker.scrape(&vec![info_hash]).await; @@ -1740,7 +1876,7 @@ mod tests { use crate::core::tests::the_tracker::{ complete_peer, incomplete_peer, peer_ip, sample_info_hash, whitelisted_tracker, }; - use crate::core::ScrapeData; + use crate::core::{PeersWanted, ScrapeData}; #[test] fn it_should_be_able_to_build_a_zeroed_scrape_data_for_a_list_of_info_hashes() { @@ -1761,11 +1897,11 @@ mod tests { let info_hash = "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::().unwrap(); let mut peer = incomplete_peer(); - tracker.announce(&info_hash, &mut peer, &peer_ip()); + tracker.announce(&info_hash, &mut peer, &peer_ip(), &PeersWanted::All); // Announce twice to force non zeroed swarm metadata let mut peer = complete_peer(); - tracker.announce(&info_hash, &mut peer, &peer_ip()); + tracker.announce(&info_hash, &mut peer, &peer_ip(), &PeersWanted::All); let scrape_data = tracker.scrape(&vec![info_hash]).await; diff --git a/src/servers/http/v1/services/announce.rs b/src/servers/http/v1/services/announce.rs index 6b7f8af5..a58df4e1 100644 --- a/src/servers/http/v1/services/announce.rs +++ b/src/servers/http/v1/services/announce.rs @@ -14,7 +14,7 @@ use std::sync::Arc; use torrust_tracker_primitives::info_hash::InfoHash; use torrust_tracker_primitives::peer; -use crate::core::{statistics, AnnounceData, Tracker}; +use crate::core::{statistics, AnnounceData, PeersWanted, Tracker}; /// The HTTP tracker `announce` service. /// @@ -30,7 +30,7 @@ pub async fn invoke(tracker: Arc, info_hash: InfoHash, peer: &mut peer: let original_peer_ip = peer.peer_addr.ip(); // The tracker could change the original peer ip - let announce_data = tracker.announce(&info_hash, peer, &original_peer_ip); + let announce_data = tracker.announce(&info_hash, peer, &original_peer_ip, &PeersWanted::All); match original_peer_ip { IpAddr::V4(_) => { diff --git a/src/servers/http/v1/services/scrape.rs b/src/servers/http/v1/services/scrape.rs index 42fe4b51..0d561c7b 100644 --- a/src/servers/http/v1/services/scrape.rs +++ b/src/servers/http/v1/services/scrape.rs @@ -103,7 +103,7 @@ mod tests { use torrust_tracker_primitives::swarm_metadata::SwarmMetadata; use torrust_tracker_test_helpers::configuration; - use crate::core::{statistics, ScrapeData, Tracker}; + use crate::core::{statistics, PeersWanted, ScrapeData, Tracker}; use crate::servers::http::v1::services::scrape::invoke; use crate::servers::http::v1::services::scrape::tests::{ public_tracker, sample_info_hash, sample_info_hashes, sample_peer, @@ -119,7 +119,7 @@ mod tests { // Announce a new peer to force scrape data to contain not zeroed data let mut peer = sample_peer(); let original_peer_ip = peer.ip(); - tracker.announce(&info_hash, &mut peer, &original_peer_ip); + tracker.announce(&info_hash, &mut peer, &original_peer_ip, &PeersWanted::All); let scrape_data = invoke(&tracker, &info_hashes, &original_peer_ip).await; @@ -194,7 +194,7 @@ mod tests { use mockall::predicate::eq; use torrust_tracker_test_helpers::configuration; - use crate::core::{statistics, ScrapeData, Tracker}; + use crate::core::{statistics, PeersWanted, ScrapeData, Tracker}; use crate::servers::http::v1::services::scrape::fake; use crate::servers::http::v1::services::scrape::tests::{ public_tracker, sample_info_hash, sample_info_hashes, sample_peer, @@ -210,7 +210,7 @@ mod tests { // Announce a new peer to force scrape data to contain not zeroed data let mut peer = sample_peer(); let original_peer_ip = peer.ip(); - tracker.announce(&info_hash, &mut peer, &original_peer_ip); + tracker.announce(&info_hash, &mut peer, &original_peer_ip, &PeersWanted::All); let scrape_data = fake(&tracker, &info_hashes, &original_peer_ip).await; diff --git a/src/servers/udp/handlers.rs b/src/servers/udp/handlers.rs index 373fb9c1..69a427e0 100644 --- a/src/servers/udp/handlers.rs +++ b/src/servers/udp/handlers.rs @@ -18,7 +18,7 @@ use zerocopy::network_endian::I32; use super::connection_cookie::{check, from_connection_id, into_connection_id, make}; use super::RawRequest; -use crate::core::{statistics, ScrapeData, Tracker}; +use crate::core::{statistics, PeersWanted, ScrapeData, Tracker}; use crate::servers::udp::error::Error; use crate::servers::udp::logging::{log_bad_request, log_error_response, log_request, log_response}; use crate::servers::udp::peer_builder; @@ -162,8 +162,9 @@ pub async fn handle_announce( })?; let mut peer = peer_builder::from_request(announce_request, &remote_client_ip); + let peers_wanted: PeersWanted = i32::from(announce_request.peers_wanted.0).into(); - let response = tracker.announce(&info_hash, &mut peer, &remote_client_ip); + let response = tracker.announce(&info_hash, &mut peer, &remote_client_ip, &peers_wanted); match remote_client_ip { IpAddr::V4(_) => { diff --git a/tests/servers/udp/contract.rs b/tests/servers/udp/contract.rs index 91f4c4e0..1f9b71b6 100644 --- a/tests/servers/udp/contract.rs +++ b/tests/servers/udp/contract.rs @@ -159,7 +159,7 @@ mod receiving_an_announce_request { Err(err) => panic!("{err}"), }; - println!("test response {response:?}"); + // println!("test response {response:?}"); assert!(is_ipv4_announce_response(&response)); }