Skip to content

Commit

Permalink
Merge torrust#866: Add timeout for time waiting for the first HTTP tr…
Browse files Browse the repository at this point in the history
…acker request

23d5e5e fix: [torrust#613] add timeout for time waiting for the first HTTP tracker request (Jose Celano)

Pull request description:

  This adds a timeout to the HTTP tracker for the time the server waits for the first request from the client after opening a new HTTP connection.

  It also adds a tower middleware for timeouts in requests.

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

  ```output
  telnet 127.0.0.1 7070
  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.
  ```

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

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

ACKs for top commit:
  josecelano:
    ACK 23d5e5e

Tree-SHA512: a348141c1c0cb8a0b5ffd6537fe3cc3793038afba0a4bae30ad806f8fd45d090cd5fd8fcd0a1fb63ccf37807cd2e9469fd7450f620d1356dde70d516a79635ca
  • Loading branch information
josecelano committed May 15, 2024
2 parents d0e66b7 + 23d5e5e commit 67e6cf1
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 4 deletions.
7 changes: 5 additions & 2 deletions src/servers/http/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use tokio::sync::oneshot::{Receiver, Sender};
use super::v1::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 @@ -60,13 +61,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(app.into_make_service_with_connect_info::<std::net::SocketAddr>())
.await
.expect("Axum server crashed."),
None => axum_server::from_tcp(socket)
None => custom_axum_server::from_tcp_with_timeouts(socket)
.handle(handle)
.acceptor(TimeoutAcceptor)
.serve(app.into_make_service_with_connect_info::<std::net::SocketAddr>())
.await
.expect("Axum server crashed."),
Expand Down
16 changes: 14 additions & 2 deletions src/servers/http/v1/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@ use std::net::SocketAddr;
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::Router;
use axum::{BoxError, Router};
use axum_client_ip::SecureClientIpSource;
use hyper::Request;
use hyper::{Request, StatusCode};
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 @@ -18,6 +21,8 @@ use tracing::{Level, Span};
use super::handlers::{announce, health_check, scrape};
use crate::core::Tracker;

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

/// It adds the routes to the router.
///
/// > **NOTICE**: it's added a layer to get the client IP from the connection
Expand Down Expand Up @@ -69,4 +74,11 @@ pub fn router(tracker: Arc<Tracker>, server_socket_addr: SocketAddr) -> 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)),
)
}

0 comments on commit 67e6cf1

Please sign in to comment.