Skip to content

Commit 657a5d0

Browse files
josecelanojosecelano
committed
refactor: [#1376] remove initial peer state from event. Only the peer IP may change
before adding the peer to the swarm, and the original remote client ip is already included in the `ConnectionCotext`.
1 parent 92f049e commit 657a5d0

4 files changed

Lines changed: 46 additions & 90 deletions

File tree

‎packages/http-tracker-core/src/event/mod.rs‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::net::{IpAddr, SocketAddr};
22

33
use torrust_tracker_metrics::label::{LabelSet, LabelValue};
44
use torrust_tracker_metrics::label_name;
5-
use torrust_tracker_primitives::peer::Peer;
5+
use torrust_tracker_primitives::peer::PeerAnnouncement;
66
use torrust_tracker_primitives::service_binding::ServiceBinding;
77

88
pub mod sender;
@@ -12,15 +12,7 @@ pub mod sender;
1212
pub enum Event {
1313
TcpAnnounce {
1414
connection: ConnectionContext,
15-
16-
/// The peer that is announcing itself to the tracker.
17-
announced_peer: Peer,
18-
19-
/// The peer that is added to the tracker.
20-
///
21-
/// It might not be the same as the `announced_peer` because the tracker
22-
/// can change the peer's IP address.
23-
added_peer: Peer,
15+
announcement: PeerAnnouncement,
2416
},
2517
TcpScrape {
2618
connection: ConnectionContext,
@@ -109,14 +101,15 @@ pub mod test {
109101

110102
use crate::event::{ConnectionContext, Event};
111103

104+
let remote_client_ip = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1));
105+
112106
let event1 = Event::TcpAnnounce {
113107
connection: ConnectionContext::new(
114-
IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)),
108+
remote_client_ip,
115109
Some(8080),
116110
ServiceBinding::new(Protocol::HTTP, SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 7070)).unwrap(),
117111
),
118-
announced_peer: Peer::default(),
119-
added_peer: Peer::default(),
112+
announcement: Peer::default(),
120113
};
121114

122115
let event2 = Event::TcpAnnounce {
@@ -125,8 +118,7 @@ pub mod test {
125118
Some(8080),
126119
ServiceBinding::new(Protocol::HTTP, SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 7070)).unwrap(),
127120
),
128-
announced_peer: Peer::default(),
129-
added_peer: Peer::default(),
121+
announcement: Peer::default(),
130122
};
131123

132124
let event1_clone = event1.clone();

‎packages/http-tracker-core/src/services/announce.rs‎

Lines changed: 31 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use bittorrent_tracker_core::error::{AnnounceError, TrackerCoreError, WhitelistE
2121
use bittorrent_tracker_core::whitelist;
2222
use torrust_tracker_configuration::Core;
2323
use torrust_tracker_primitives::core::AnnounceData;
24-
use torrust_tracker_primitives::peer::Peer;
24+
use torrust_tracker_primitives::peer::PeerAnnouncement;
2525
use torrust_tracker_primitives::service_binding::ServiceBinding;
2626

2727
use crate::event;
@@ -78,29 +78,19 @@ impl AnnounceService {
7878

7979
self.authorize(announce_request.info_hash).await?;
8080

81-
let (remote_client_ip, opt_remote_client_port) = self.resolve_remote_client_address(client_ip_sources)?;
81+
let (remote_client_ip, opt_remote_client_port) = self.resolve_remote_client_ip(client_ip_sources)?;
8282

8383
let mut peer = peer_from_request(announce_request, &remote_client_ip);
8484

85-
let announced_peer = peer;
86-
8785
let peers_wanted = Self::peers_wanted(announce_request);
8886

8987
let announce_data = self
9088
.announce_handler
9189
.announce(&announce_request.info_hash, &mut peer, &remote_client_ip, &peers_wanted)
9290
.await?;
9391

94-
let added_peer = peer;
95-
96-
self.send_event(
97-
remote_client_ip,
98-
opt_remote_client_port,
99-
server_service_binding.clone(),
100-
announced_peer,
101-
added_peer,
102-
)
103-
.await;
92+
self.send_event(remote_client_ip, opt_remote_client_port, server_service_binding.clone(), peer)
93+
.await;
10494

10595
Ok(announce_data)
10696
}
@@ -122,7 +112,7 @@ impl AnnounceService {
122112
}
123113

124114
/// Resolves the client's real IP address considering proxy headers
125-
fn resolve_remote_client_address(
115+
fn resolve_remote_client_ip(
126116
&self,
127117
client_ip_sources: &ClientIpSources,
128118
) -> Result<(IpAddr, Option<u16>), PeerIpResolutionError> {
@@ -152,17 +142,15 @@ impl AnnounceService {
152142

153143
async fn send_event(
154144
&self,
155-
peer_ip: IpAddr,
145+
remote_client_ip: IpAddr,
156146
opt_peer_ip_port: Option<u16>,
157147
server_service_binding: ServiceBinding,
158-
announced_peer: Peer,
159-
added_peer: Peer,
148+
announcement: PeerAnnouncement,
160149
) {
161150
if let Some(http_stats_event_sender) = self.opt_http_stats_event_sender.as_deref() {
162151
let event = Event::TcpAnnounce {
163-
connection: event::ConnectionContext::new(peer_ip, opt_peer_ip_port, server_service_binding),
164-
announced_peer,
165-
added_peer,
152+
connection: event::ConnectionContext::new(remote_client_ip, opt_peer_ip_port, server_service_binding),
153+
announcement,
166154
};
167155

168156
tracing::debug!("Sending TcpAnnounce event: {:?}", event);
@@ -389,6 +377,7 @@ mod tests {
389377
let server_socket_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 7070);
390378
let server_service_binding = ServiceBinding::new(Protocol::HTTP, server_socket_addr).unwrap();
391379
let peer = sample_peer_using_ipv4();
380+
let remote_client_ip = IpAddr::V4(Ipv4Addr::new(126, 0, 0, 1));
392381

393382
let server_service_binding_clone = server_service_binding.clone();
394383
let peer_copy = peer;
@@ -400,32 +389,25 @@ mod tests {
400389
let mut announced_peer = peer_copy;
401390
announced_peer.peer_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(126, 0, 0, 1)), 8080);
402391

403-
let mut added_peer = peer;
404-
added_peer.peer_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(126, 0, 0, 1)), 8080);
392+
let mut announcement = peer;
393+
announcement.peer_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(126, 0, 0, 1)), 8080);
405394

406395
let expected_event = Event::TcpAnnounce {
407-
connection: ConnectionContext::new(
408-
IpAddr::V4(Ipv4Addr::new(126, 0, 0, 1)),
409-
Some(8080),
410-
server_service_binding.clone(),
411-
),
412-
announced_peer,
413-
added_peer,
396+
connection: ConnectionContext::new(remote_client_ip, Some(8080), server_service_binding.clone()),
397+
announcement,
414398
};
415399

416400
match (event, expected_event) {
417401
(
418402
Event::TcpAnnounce {
419403
connection: a_conn,
420-
announced_peer: a1,
421-
added_peer: a2,
404+
announcement: a2,
422405
},
423406
Event::TcpAnnounce {
424407
connection: b_conn,
425-
announced_peer: b1,
426-
added_peer: b2,
408+
announcement: b2,
427409
},
428-
) => *a_conn == b_conn && a1.peer_addr == b1.peer_addr && a2.peer_addr == b2.peer_addr,
410+
) => *a_conn == b_conn && a2.peer_addr == b2.peer_addr,
429411
_ => false,
430412
}
431413
}))
@@ -477,6 +459,7 @@ mod tests {
477459
let server_socket_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 7070);
478460
let server_service_binding = ServiceBinding::new(Protocol::HTTP, server_socket_addr).unwrap();
479461
let peer = peer_with_the_ipv4_loopback_ip();
462+
let remote_client_ip = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1));
480463

481464
let server_service_binding_clone = server_service_binding.clone();
482465
let peer_copy = peer;
@@ -488,35 +471,28 @@ mod tests {
488471
let mut announced_peer = peer_copy;
489472
announced_peer.peer_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8080);
490473

491-
let mut added_peer = peer;
492-
added_peer.peer_addr = SocketAddr::new(
474+
let mut peer_announcement = peer;
475+
peer_announcement.peer_addr = SocketAddr::new(
493476
IpAddr::V6(Ipv6Addr::new(0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969)),
494477
8080,
495478
);
496479

497480
let expected_event = Event::TcpAnnounce {
498-
connection: ConnectionContext::new(
499-
IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)),
500-
Some(8080),
501-
server_service_binding.clone(),
502-
),
503-
announced_peer,
504-
added_peer,
481+
connection: ConnectionContext::new(remote_client_ip, Some(8080), server_service_binding.clone()),
482+
announcement: peer_announcement,
505483
};
506484

507485
match (event, expected_event) {
508486
(
509487
Event::TcpAnnounce {
510488
connection: a_conn,
511-
announced_peer: a1,
512-
added_peer: a2,
489+
announcement: a2,
513490
},
514491
Event::TcpAnnounce {
515492
connection: b_conn,
516-
announced_peer: b1,
517-
added_peer: b2,
493+
announcement: b2,
518494
},
519-
) => *a_conn == b_conn && a1.peer_addr == b1.peer_addr && a2.peer_addr == b2.peer_addr,
495+
) => *a_conn == b_conn && a2.peer_addr == b2.peer_addr,
520496
_ => false,
521497
}
522498
}))
@@ -553,42 +529,28 @@ mod tests {
553529
let server_socket_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 7070);
554530
let server_service_binding = ServiceBinding::new(Protocol::HTTP, server_socket_addr).unwrap();
555531
let peer = sample_peer_using_ipv6();
556-
557-
let peer_copy = peer;
532+
let remote_client_ip = IpAddr::V6(Ipv6Addr::new(0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969));
558533

559534
let mut http_stats_event_sender_mock = MockHttpStatsEventSender::new();
560535
http_stats_event_sender_mock
561536
.expect_send_event()
562537
.with(predicate::function(move |event| {
563-
let announced_peer = peer_copy;
564-
//announced_peer.peer_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(126, 0, 0, 1)), 8080);
565-
566-
let added_peer = peer;
567-
//added_peer.peer_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(126, 0, 0, 1)), 8080);
568-
569538
let expected_event = Event::TcpAnnounce {
570-
connection: ConnectionContext::new(
571-
IpAddr::V6(Ipv6Addr::new(0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969)),
572-
Some(8080),
573-
server_service_binding.clone(),
574-
),
575-
announced_peer,
576-
added_peer,
539+
connection: ConnectionContext::new(remote_client_ip, Some(8080), server_service_binding.clone()),
540+
announcement: peer,
577541
};
578542

579543
match (event, expected_event) {
580544
(
581545
Event::TcpAnnounce {
582546
connection: a_conn,
583-
announced_peer: a1,
584-
added_peer: a2,
547+
announcement: a2,
585548
},
586549
Event::TcpAnnounce {
587550
connection: b_conn,
588-
announced_peer: b1,
589-
added_peer: b2,
551+
announcement: b2,
590552
},
591-
) => *a_conn == b_conn && a1.peer_addr == b1.peer_addr && a2.peer_addr == b2.peer_addr,
553+
) => *a_conn == b_conn && a2.peer_addr == b2.peer_addr,
592554
_ => false,
593555
}
594556
}))

‎packages/http-tracker-core/src/statistics/event/handler.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,16 +86,16 @@ mod tests {
8686
async fn should_increase_the_tcp4_announces_counter_when_it_receives_a_tcp4_announce_event() {
8787
let stats_repository = Repository::new();
8888
let peer = sample_peer_using_ipv4();
89+
let remote_client_ip = IpAddr::V4(Ipv4Addr::new(127, 0, 0, 2));
8990

9091
handle_event(
9192
Event::TcpAnnounce {
9293
connection: ConnectionContext::new(
93-
IpAddr::V4(Ipv4Addr::new(127, 0, 0, 2)),
94+
remote_client_ip,
9495
Some(8080),
9596
ServiceBinding::new(Protocol::HTTP, SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 7070)).unwrap(),
9697
),
97-
announced_peer: peer,
98-
added_peer: peer,
98+
announcement: peer,
9999
},
100100
&stats_repository,
101101
CurrentClock::now(),
@@ -133,16 +133,16 @@ mod tests {
133133
async fn should_increase_the_tcp6_announces_counter_when_it_receives_a_tcp6_announce_event() {
134134
let stats_repository = Repository::new();
135135
let peer = sample_peer_using_ipv6();
136+
let remote_client_ip = IpAddr::V6(Ipv6Addr::new(0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969));
136137

137138
handle_event(
138139
Event::TcpAnnounce {
139140
connection: ConnectionContext::new(
140-
IpAddr::V6(Ipv6Addr::new(0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969)),
141+
remote_client_ip,
141142
Some(8080),
142143
ServiceBinding::new(Protocol::HTTP, SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 7070)).unwrap(),
143144
),
144-
announced_peer: peer,
145-
added_peer: peer,
145+
announcement: peer,
146146
},
147147
&stats_repository,
148148
CurrentClock::now(),

‎packages/primitives/src/peer.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ use zerocopy::FromBytes as _;
3232

3333
use crate::DurationSinceUnixEpoch;
3434

35+
pub type PeerAnnouncement = Peer;
36+
3537
/// Peer struct used by the core `Tracker`.
3638
///
3739
/// A sample peer:

0 commit comments

Comments
 (0)