Skip to content

Commit 47aae95

Browse files
josecelanojosecelano
committed
Merge #1325: Refactor packages: normalize HTTP tracker core fn signatures for announce and scrape handlers
df4f782 refactor: [#1318] return error instead of response from http scrape handler (Jose Celano) 0c2be36 refactor: [#1318] return error instead of response from http announce handler (Jose Celano) Pull request description: Refactor packages: normalize HTTP tracker core fn signatures for announce and scrape handlers. ACKs for top commit: josecelano: ACK df4f782 Tree-SHA512: 20588c0792651f79faa7563e9bbe0af02a80d32924deed9e78c6f79cb525d89a1ce221ee21262fd95d52110ceaf9d76d97a8c1cd8145f9b0ea7807b5e3779b40
2 parents e437f63 + df4f782 commit 47aae95

File tree

11 files changed

+240
-30
lines changed

11 files changed

+240
-30
lines changed

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/axum-http-tracker-server/src/v1/handlers/announce.rs‎

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use std::sync::Arc;
77
use aquatic_udp_protocol::AnnounceEvent;
88
use axum::extract::State;
99
use axum::response::{IntoResponse, Response};
10+
use bittorrent_http_tracker_core::services::announce::HttpAnnounceError;
1011
use bittorrent_http_tracker_protocol::v1::requests::announce::{Announce, Compact, Event};
1112
use bittorrent_http_tracker_protocol::v1::responses::{self};
1213
use bittorrent_http_tracker_protocol::v1::services::peer_ip_resolver::ClientIpSources;
@@ -111,7 +112,12 @@ async fn handle(
111112
.await
112113
{
113114
Ok(announce_data) => announce_data,
114-
Err(error) => return (StatusCode::OK, error.write()).into_response(),
115+
Err(error) => {
116+
let error_response = responses::error::Error {
117+
failure_reason: error.to_string(),
118+
};
119+
return (StatusCode::OK, error_response.write()).into_response();
120+
}
115121
};
116122
build_response(announce_request, announce_data)
117123
}
@@ -126,7 +132,7 @@ async fn handle_announce(
126132
announce_request: &Announce,
127133
client_ip_sources: &ClientIpSources,
128134
maybe_key: Option<Key>,
129-
) -> Result<AnnounceData, responses::error::Error> {
135+
) -> Result<AnnounceData, HttpAnnounceError> {
130136
bittorrent_http_tracker_core::services::announce::handle_announce(
131137
&core_config.clone(),
132138
&announce_handler.clone(),
@@ -290,6 +296,7 @@ mod tests {
290296

291297
use std::str::FromStr;
292298

299+
use bittorrent_http_tracker_protocol::v1::responses;
293300
use bittorrent_tracker_core::authentication;
294301

295302
use super::{initialize_private_tracker, sample_announce_request, sample_client_ip_sources};
@@ -315,7 +322,14 @@ mod tests {
315322
.await
316323
.unwrap_err();
317324

318-
assert_error_response(&response, "Tracker authentication error: Missing authentication key");
325+
let error_response = responses::error::Error {
326+
failure_reason: response.to_string(),
327+
};
328+
329+
assert_error_response(
330+
&error_response,
331+
"Tracker core error: Tracker core authentication error: Missing authentication key",
332+
);
319333
}
320334

321335
#[tokio::test]
@@ -339,15 +353,21 @@ mod tests {
339353
.await
340354
.unwrap_err();
341355

356+
let error_response = responses::error::Error {
357+
failure_reason: response.to_string(),
358+
};
359+
342360
assert_error_response(
343-
&response,
344-
"Tracker authentication error: Failed to read key: YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ",
361+
&error_response,
362+
"Tracker core error: Tracker core authentication error: Failed to read key: YZSl4lMZupRuOpSRC3krIKR5BPB14nrJ",
345363
);
346364
}
347365
}
348366

349367
mod with_tracker_in_listed_mode {
350368

369+
use bittorrent_http_tracker_protocol::v1::responses;
370+
351371
use super::{initialize_listed_tracker, sample_announce_request, sample_client_ip_sources};
352372
use crate::v1::handlers::announce::handle_announce;
353373
use crate::v1::handlers::announce::tests::assert_error_response;
@@ -371,10 +391,14 @@ mod tests {
371391
.await
372392
.unwrap_err();
373393

394+
let error_response = responses::error::Error {
395+
failure_reason: response.to_string(),
396+
};
397+
374398
assert_error_response(
375-
&response,
399+
&error_response,
376400
&format!(
377-
"Tracker whitelist error: The torrent: {}, is not whitelisted",
401+
"Tracker core error: Tracker core whitelist error: The torrent: {}, is not whitelisted",
378402
announce_request.info_hash
379403
),
380404
);
@@ -383,6 +407,7 @@ mod tests {
383407

384408
mod with_tracker_on_reverse_proxy {
385409

410+
use bittorrent_http_tracker_protocol::v1::responses;
386411
use bittorrent_http_tracker_protocol::v1::services::peer_ip_resolver::ClientIpSources;
387412

388413
use super::{initialize_tracker_on_reverse_proxy, sample_announce_request};
@@ -411,15 +436,20 @@ mod tests {
411436
.await
412437
.unwrap_err();
413438

439+
let error_response = responses::error::Error {
440+
failure_reason: response.to_string(),
441+
};
442+
414443
assert_error_response(
415-
&response,
444+
&error_response,
416445
"Error resolving peer IP: missing or invalid the right most X-Forwarded-For IP",
417446
);
418447
}
419448
}
420449

421450
mod with_tracker_not_on_reverse_proxy {
422451

452+
use bittorrent_http_tracker_protocol::v1::responses;
423453
use bittorrent_http_tracker_protocol::v1::services::peer_ip_resolver::ClientIpSources;
424454

425455
use super::{initialize_tracker_not_on_reverse_proxy, sample_announce_request};
@@ -448,8 +478,12 @@ mod tests {
448478
.await
449479
.unwrap_err();
450480

481+
let error_response = responses::error::Error {
482+
failure_reason: response.to_string(),
483+
};
484+
451485
assert_error_response(
452-
&response,
486+
&error_response,
453487
"Error resolving peer IP: cannot get the client IP from the connection info",
454488
);
455489
}

‎packages/axum-http-tracker-server/src/v1/handlers/scrape.rs‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use std::sync::Arc;
66

77
use axum::extract::State;
88
use axum::response::{IntoResponse, Response};
9+
use bittorrent_http_tracker_core::services::scrape::HttpScrapeError;
910
use bittorrent_http_tracker_protocol::v1::requests::scrape::Scrape;
1011
use bittorrent_http_tracker_protocol::v1::responses;
1112
use bittorrent_http_tracker_protocol::v1::services::peer_ip_resolver::ClientIpSources;
@@ -101,7 +102,12 @@ async fn handle(
101102
.await
102103
{
103104
Ok(scrape_data) => scrape_data,
104-
Err(error) => return (StatusCode::OK, error.write()).into_response(),
105+
Err(error) => {
106+
let error_response = responses::error::Error {
107+
failure_reason: error.to_string(),
108+
};
109+
return (StatusCode::OK, error_response.write()).into_response();
110+
}
105111
};
106112

107113
build_response(scrape_data)
@@ -116,7 +122,7 @@ async fn handle_scrape(
116122
scrape_request: &Scrape,
117123
client_ip_sources: &ClientIpSources,
118124
maybe_key: Option<Key>,
119-
) -> Result<ScrapeData, responses::error::Error> {
125+
) -> Result<ScrapeData, HttpScrapeError> {
120126
bittorrent_http_tracker_core::services::scrape::handle_scrape(
121127
core_config,
122128
scrape_handler,
@@ -316,6 +322,7 @@ mod tests {
316322

317323
mod with_tracker_on_reverse_proxy {
318324

325+
use bittorrent_http_tracker_protocol::v1::responses;
319326
use bittorrent_http_tracker_protocol::v1::services::peer_ip_resolver::ClientIpSources;
320327

321328
use super::{initialize_tracker_on_reverse_proxy, sample_scrape_request};
@@ -343,15 +350,20 @@ mod tests {
343350
.await
344351
.unwrap_err();
345352

353+
let error_response = responses::error::Error {
354+
failure_reason: response.to_string(),
355+
};
356+
346357
assert_error_response(
347-
&response,
358+
&error_response,
348359
"Error resolving peer IP: missing or invalid the right most X-Forwarded-For IP",
349360
);
350361
}
351362
}
352363

353364
mod with_tracker_not_on_reverse_proxy {
354365

366+
use bittorrent_http_tracker_protocol::v1::responses;
355367
use bittorrent_http_tracker_protocol::v1::services::peer_ip_resolver::ClientIpSources;
356368

357369
use super::{initialize_tracker_not_on_reverse_proxy, sample_scrape_request};
@@ -379,8 +391,12 @@ mod tests {
379391
.await
380392
.unwrap_err();
381393

394+
let error_response = responses::error::Error {
395+
failure_reason: response.to_string(),
396+
};
397+
382398
assert_error_response(
383-
&response,
399+
&error_response,
384400
"Error resolving peer IP: cannot get the client IP from the connection info",
385401
);
386402
}

‎packages/axum-http-tracker-server/tests/server/asserts.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,3 +147,13 @@ pub async fn assert_authentication_error_response(response: Response) {
147147
Location::caller(),
148148
);
149149
}
150+
151+
pub async fn assert_tracker_core_authentication_error_response(response: Response) {
152+
assert_eq!(response.status(), 200);
153+
154+
assert_bencoded_error(
155+
&response.text().await.unwrap(),
156+
"Tracker core error: Tracker core authentication error",
157+
Location::caller(),
158+
);
159+
}

‎packages/axum-http-tracker-server/tests/server/v1/contract.rs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1448,7 +1448,9 @@ mod configured_as_private {
14481448
use torrust_axum_http_tracker_server::environment::Started;
14491449
use torrust_tracker_test_helpers::{configuration, logging};
14501450

1451-
use crate::server::asserts::{assert_authentication_error_response, assert_is_announce_response};
1451+
use crate::server::asserts::{
1452+
assert_authentication_error_response, assert_is_announce_response, assert_tracker_core_authentication_error_response,
1453+
};
14521454
use crate::server::client::Client;
14531455
use crate::server::requests::announce::QueryBuilder;
14541456

@@ -1487,7 +1489,7 @@ mod configured_as_private {
14871489
.announce(&QueryBuilder::default().with_info_hash(&info_hash).query())
14881490
.await;
14891491

1490-
assert_authentication_error_response(response).await;
1492+
assert_tracker_core_authentication_error_response(response).await;
14911493

14921494
env.stop().await;
14931495
}
@@ -1522,7 +1524,7 @@ mod configured_as_private {
15221524
.announce(&QueryBuilder::default().query())
15231525
.await;
15241526

1525-
assert_authentication_error_response(response).await;
1527+
assert_tracker_core_authentication_error_response(response).await;
15261528

15271529
env.stop().await;
15281530
}

‎packages/http-protocol/src/v1/services/peer_ip_resolver.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ pub struct ClientIpSources {
3636
}
3737

3838
/// The error that can occur when resolving the peer IP.
39-
#[derive(Error, Debug)]
39+
#[derive(Error, Debug, Clone)]
4040
pub enum PeerIpResolutionError {
4141
/// The peer IP cannot be obtained because the tracker is configured as a
4242
/// reverse proxy but the `X-Forwarded-For` HTTP header is missing or

‎packages/http-tracker-core/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ bittorrent-http-tracker-protocol = { version = "3.0.0-develop", path = "../http-
1919
bittorrent-primitives = "0.1.0"
2020
bittorrent-tracker-core = { version = "3.0.0-develop", path = "../tracker-core" }
2121
futures = "0"
22+
thiserror = "2"
2223
tokio = { version = "1", features = ["macros", "net", "rt-multi-thread", "signal", "sync"] }
2324
torrust-tracker-configuration = { version = "3.0.0-develop", path = "../configuration" }
2425
torrust-tracker-primitives = { version = "3.0.0-develop", path = "../primitives" }

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

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,67 @@ use std::panic::Location;
1212
use std::sync::Arc;
1313

1414
use bittorrent_http_tracker_protocol::v1::requests::announce::{peer_from_request, Announce};
15-
use bittorrent_http_tracker_protocol::v1::responses;
16-
use bittorrent_http_tracker_protocol::v1::services::peer_ip_resolver::{self, ClientIpSources};
15+
use bittorrent_http_tracker_protocol::v1::services::peer_ip_resolver::{self, ClientIpSources, PeerIpResolutionError};
1716
use bittorrent_tracker_core::announce_handler::{AnnounceHandler, PeersWanted};
1817
use bittorrent_tracker_core::authentication::service::AuthenticationService;
1918
use bittorrent_tracker_core::authentication::{self, Key};
19+
use bittorrent_tracker_core::error::{AnnounceError, TrackerCoreError, WhitelistError};
2020
use bittorrent_tracker_core::whitelist;
2121
use torrust_tracker_configuration::Core;
2222
use torrust_tracker_primitives::core::AnnounceData;
2323

2424
use crate::statistics;
2525

26+
/// Errors related to announce requests.
27+
#[derive(thiserror::Error, Debug, Clone)]
28+
pub enum HttpAnnounceError {
29+
#[error("Error resolving peer IP: {source}")]
30+
PeerIpResolutionError { source: PeerIpResolutionError },
31+
32+
#[error("Tracker core error: {source}")]
33+
TrackerCoreError { source: TrackerCoreError },
34+
}
35+
36+
impl From<PeerIpResolutionError> for HttpAnnounceError {
37+
fn from(peer_ip_resolution_error: PeerIpResolutionError) -> Self {
38+
Self::PeerIpResolutionError {
39+
source: peer_ip_resolution_error,
40+
}
41+
}
42+
}
43+
44+
impl From<TrackerCoreError> for HttpAnnounceError {
45+
fn from(tracker_core_error: TrackerCoreError) -> Self {
46+
Self::TrackerCoreError {
47+
source: tracker_core_error,
48+
}
49+
}
50+
}
51+
52+
impl From<AnnounceError> for HttpAnnounceError {
53+
fn from(announce_error: AnnounceError) -> Self {
54+
Self::TrackerCoreError {
55+
source: announce_error.into(),
56+
}
57+
}
58+
}
59+
60+
impl From<WhitelistError> for HttpAnnounceError {
61+
fn from(whitelist_error: WhitelistError) -> Self {
62+
Self::TrackerCoreError {
63+
source: whitelist_error.into(),
64+
}
65+
}
66+
}
67+
68+
impl From<authentication::key::Error> for HttpAnnounceError {
69+
fn from(whitelist_error: authentication::key::Error) -> Self {
70+
Self::TrackerCoreError {
71+
source: whitelist_error.into(),
72+
}
73+
}
74+
}
75+
2676
/// The HTTP tracker `announce` service.
2777
///
2878
/// The service sends an statistics event that increments:
@@ -50,7 +100,7 @@ pub async fn handle_announce(
50100
announce_request: &Announce,
51101
client_ip_sources: &ClientIpSources,
52102
maybe_key: Option<Key>,
53-
) -> Result<AnnounceData, responses::error::Error> {
103+
) -> Result<AnnounceData, HttpAnnounceError> {
54104
// Authentication
55105
if core_config.private {
56106
match maybe_key {
@@ -59,22 +109,23 @@ pub async fn handle_announce(
59109
Err(error) => return Err(error.into()),
60110
},
61111
None => {
62-
return Err(responses::error::Error::from(authentication::key::Error::MissingAuthKey {
112+
return Err(authentication::key::Error::MissingAuthKey {
63113
location: Location::caller(),
64-
}))
114+
}
115+
.into())
65116
}
66117
}
67118
}
68119

69120
// Authorization
70121
match whitelist_authorization.authorize(&announce_request.info_hash).await {
71122
Ok(()) => (),
72-
Err(error) => return Err(responses::error::Error::from(error)),
123+
Err(error) => return Err(error.into()),
73124
}
74125

75126
let peer_ip = match peer_ip_resolver::invoke(core_config.net.on_reverse_proxy, client_ip_sources) {
76127
Ok(peer_ip) => peer_ip,
77-
Err(error) => return Err(responses::error::Error::from(error)),
128+
Err(error) => return Err(error.into()),
78129
};
79130

80131
let mut peer = peer_from_request(announce_request, &peer_ip);

0 commit comments

Comments
 (0)