Skip to content

Commit

Permalink
Merge #865: Add timeouts for Axum server in the Tracker API
Browse files Browse the repository at this point in the history
9e42a1a feat: [#612] tower middleware to apply timeouts to requests (Jose Celano)
112b76d fix: [#612] add timeout for time waiting for the first API requests (Jose Celano)
dadc216 chore(deps): add cargo dependencies needed for axum timeouts (Jose Celano)

Pull request description:

  Add timeouts for Axum server in the Tracker API:

  1. Timeout for the first request after opening the HTTP connection. [408 Request Timeout](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/408).
  2. HTTP1 header read timeout. Set a timeout for reading client request headers. If a client does not transmit the entire header within this time, the connection is closed.
  3. HTTP2 keep alive timeout: Sets a timeout for receiving an acknowledgement of the keep-alive ping. If the ping is not acknowledged within the timeout, the connection will be closed. Does nothing if `http2_keep_alive_interval` is disabled.
  4. HTTP2 keep alive interval: Sets an interval for HTTP2 Ping frames should be sent to keep a connection alive.
  5. [Tower middleware which applies a timeout to requests](https://docs.rs/tower-http/latest/tower_http/timeout/index.html).

  The current implementation for the first timeout does not send any message ([408 Request Timeout](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/408)). [It seems is also common practice](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/408).

  To test the first case, you can open a connection to the API without sending any request with:

  ```output
  telnet 127.0.0.1 1212
  Trying 127.0.0.1...
  Connected to 127.0.0.1.
  Escape character is '^]'.
  ```

  After 5 seconds you will see:

  ```output
  Connection closed by foreign host.
  ```

  For advance manual testing you can use: https://github.com/josecelano/axum-server-timeout

ACKs for top commit:
  josecelano:
    ACK 9e42a1a

Tree-SHA512: 08923b1f1d314a87c5c5fb3651219470fd48f6c6d9ce00f7dd802536b3eb9d44290f319ac8ef44a2f1a005bfab5d75abaf554c5b2241b1a86c6e2008b61894ab
  • Loading branch information
josecelano committed May 15, 2024
2 parents 8dc0520 + 9e42a1a commit d0e66b7
Show file tree
Hide file tree
Showing 7 changed files with 310 additions and 5 deletions.
5 changes: 5 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,17 @@ derive_more = "0"
fern = "0"
figment = "0.10.18"
futures = "0"
futures-util = "0.3.30"
hex-literal = "0"
http-body = "1.0.0"
hyper = "1"
hyper-util = { version = "0.1.3", features = ["http1", "http2", "tokio"] }
lazy_static = "1"
log = { version = "0", features = ["release_max_level_info"] }
multimap = "0"
parking_lot = "0.12.1"
percent-encoding = "2"
pin-project-lite = "0.2.14"
r2d2 = "0"
r2d2_mysql = "24"
r2d2_sqlite = { version = "0", features = ["bundled"] }
Expand All @@ -72,6 +76,7 @@ torrust-tracker-contrib-bencode = { version = "3.0.0-alpha.12-develop", path = "
torrust-tracker-located-error = { version = "3.0.0-alpha.12-develop", path = "packages/located-error" }
torrust-tracker-primitives = { version = "3.0.0-alpha.12-develop", path = "packages/primitives" }
torrust-tracker-torrent-repository = { version = "3.0.0-alpha.12-develop", path = "packages/torrent-repository" }
tower = { version = "0.4.13", features = ["timeout"] }
tower-http = { version = "0", features = ["compression-full", "cors", "propagate-header", "request-id", "trace"] }
trace = "0"
tracing = "0"
Expand Down
4 changes: 4 additions & 0 deletions cSpell.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"downloadedi",
"dtolnay",
"elif",
"Eray",
"filesd",
"flamegraph",
"Freebox",
Expand All @@ -73,6 +74,7 @@
"intervali",
"Joakim",
"kallsyms",
"Karatay",
"kcachegrind",
"kexec",
"keyout",
Expand Down Expand Up @@ -107,6 +109,7 @@
"Pando",
"peekable",
"peerlist",
"programatik",
"proot",
"proto",
"Quickstart",
Expand Down Expand Up @@ -137,6 +140,7 @@
"sharktorrent",
"SHLVL",
"skiplist",
"slowloris",
"socketaddr",
"sqllite",
"subsec",
Expand Down
16 changes: 14 additions & 2 deletions src/servers/apis/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,15 @@
use std::sync::Arc;
use std::time::Duration;

use axum::error_handling::HandleErrorLayer;
use axum::http::HeaderName;
use axum::response::Response;
use axum::routing::get;
use axum::{middleware, Router};
use hyper::Request;
use axum::{middleware, BoxError, Router};
use hyper::{Request, StatusCode};
use torrust_tracker_configuration::AccessTokens;
use tower::timeout::TimeoutLayer;
use tower::ServiceBuilder;
use tower_http::compression::CompressionLayer;
use tower_http::propagate_header::PropagateHeaderLayer;
use tower_http::request_id::{MakeRequestUuid, SetRequestIdLayer};
Expand All @@ -25,6 +28,8 @@ use super::v1::context::health_check::handlers::health_check_handler;
use super::v1::middlewares::auth::State;
use crate::core::Tracker;

const TIMEOUT: Duration = Duration::from_secs(5);

/// Add all API routes to the router.
#[allow(clippy::needless_pass_by_value)]
pub fn router(tracker: Arc<Tracker>, access_tokens: Arc<AccessTokens>) -> Router {
Expand Down Expand Up @@ -73,4 +78,11 @@ pub fn router(tracker: Arc<Tracker>, access_tokens: Arc<AccessTokens>) -> Router
}),
)
.layer(SetRequestIdLayer::x_request_id(MakeRequestUuid))
.layer(
ServiceBuilder::new()
// this middleware goes above `TimeoutLayer` because it will receive
// errors returned by `TimeoutLayer`
.layer(HandleErrorLayer::new(|_: BoxError| async { StatusCode::REQUEST_TIMEOUT }))
.layer(TimeoutLayer::new(TIMEOUT)),
)
}
9 changes: 6 additions & 3 deletions src/servers/apis/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use torrust_tracker_configuration::AccessTokens;
use super::routes::router;
use crate::bootstrap::jobs::Started;
use crate::core::Tracker;
use crate::servers::custom_axum_server::{self, TimeoutAcceptor};
use crate::servers::registar::{ServiceHealthCheckJob, ServiceRegistration, ServiceRegistrationForm};
use crate::servers::signals::{graceful_shutdown, Halted};

Expand Down Expand Up @@ -177,7 +178,7 @@ impl ApiServer<Running> {
/// Or if there request returns an error code.
#[must_use]
pub fn check_fn(binding: &SocketAddr) -> ServiceHealthCheckJob {
let url = format!("http://{binding}/api/health_check");
let url = format!("http://{binding}/api/health_check"); // DevSkim: ignore DS137138

let info = format!("checking api health check at: {url}");

Expand Down Expand Up @@ -234,13 +235,15 @@ impl Launcher {

let running = Box::pin(async {
match tls {
Some(tls) => axum_server::from_tcp_rustls(socket, tls)
Some(tls) => custom_axum_server::from_tcp_rustls_with_timeouts(socket, tls)
.handle(handle)
.acceptor(TimeoutAcceptor)
.serve(router.into_make_service_with_connect_info::<std::net::SocketAddr>())
.await
.expect("Axum server for tracker API crashed."),
None => axum_server::from_tcp(socket)
None => custom_axum_server::from_tcp_with_timeouts(socket)
.handle(handle)
.acceptor(TimeoutAcceptor)
.serve(router.into_make_service_with_connect_info::<std::net::SocketAddr>())
.await
.expect("Axum server for tracker API crashed."),
Expand Down
Loading

0 comments on commit d0e66b7

Please sign in to comment.