Skip to content

Commit

Permalink
feat: [#569] allow UDP clients to limit peers in response
Browse files Browse the repository at this point in the history 
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).
  • Loading branch information
@josecelano
josecelano committed Sep 10, 2024
1 parent 1e437f7 commit 21f6b2e
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 28 deletions.
101 changes: 82 additions & 19 deletions src/core/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)]

Check warning on line 525 in src/core/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/core/mod.rs#L525 

Added line #L525 was not covered by tests
pub enum PeersWanted {
/// The peer wants as many peers as possible in the announce response.
#[default]
All,

Check warning on line 529 in src/core/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/core/mod.rs#L529 

Added line #L529 was not covered by tests
/// The peer only wants a certain amount of peers in the announce response.
Only { amount: usize },

Check warning on line 531 in src/core/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/core/mod.rs#L531 

Added line #L531 was not covered by tests
}

impl PeersWanted {
fn limit(&self) -> usize {
match self {
PeersWanted::All => TORRENT_PEERS_LIMIT,
PeersWanted::Only { amount } => *amount,
}
}
}

impl From<i32> 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,

Check warning on line 548 in src/core/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/core/mod.rs#L548 

Added line #L548 was not covered by tests
}
} else {
Self::All

Check warning on line 551 in src/core/mod.rs

View check run for this annotation

Codecov / codecov/patch

src/core/mod.rs#L551 

Added line #L551 was not covered by tests
}
}
}

/// Structure that holds the data returned by the `scrape` request.
#[derive(Debug, PartialEq, Default)]
pub struct ScrapeData {
Expand Down Expand Up @@ -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`.
Expand All @@ -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,
Expand Down Expand Up @@ -713,16 +752,21 @@ impl Tracker {
Ok(())
}

fn get_peers_for(&self, info_hash: &InfoHash, peer: &peer::Peer) -> Vec<Arc<peer::Peer>> {
/// # 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<Arc<peer::Peer>> {
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<Arc<peer::Peer>> {
match self.torrents.get(info_hash) {
None => vec![],
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -1350,7 +1395,7 @@ 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![]);
}
Expand Down Expand Up @@ -1409,6 +1454,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 {

Expand Down Expand Up @@ -1514,7 +1560,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![]);
}
Expand All @@ -1524,10 +1570,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)]);
}
Expand All @@ -1537,14 +1588,15 @@ 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() {
let tracker = public_tracker();

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);
}
Expand All @@ -1555,7 +1607,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);
}
Expand All @@ -1566,10 +1618,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);
}
Expand All @@ -1583,7 +1636,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(
Expand All @@ -1609,11 +1662,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;
Expand Down Expand Up @@ -1740,7 +1803,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() {
Expand All @@ -1761,11 +1824,11 @@ mod tests {
let info_hash = "3b245504cf5f11bbdbe1201cea6a6bf45aee1bc0".parse::<InfoHash>().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;

Expand Down
4 changes: 2 additions & 2 deletions src/servers/http/v1/services/announce.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -30,7 +30,7 @@ pub async fn invoke(tracker: Arc<Tracker>, 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(_) => {
Expand Down
8 changes: 4 additions & 4 deletions src/servers/http/v1/services/scrape.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;

Expand Down Expand Up @@ -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,
Expand All @@ -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;

Expand Down
5 changes: 3 additions & 2 deletions src/servers/udp/handlers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(_) => {
Expand Down
2 changes: 1 addition & 1 deletion tests/servers/udp/contract.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down

0 comments on commit 21f6b2e

Please sign in to comment.