http_client: add timeouts for critical connects

Use timeout futures for sections that might hang in certain error
conditions. This is mostly intended to be used as a safeguard, not a
first line of defense - i.e. best-effort avoidance of total hangs.

Not every future used for the HttpClient/H2Client is changed, only those
where a quick response is to be expected. For example, the response
reading futures are left alone, so data transfer is never capped with
timeout, only the initial server connect.

It is also used for upgrading to H2 connections, as that can take a long
time on overloaded servers.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
This commit is contained in:
Stefan Reiter 2020-12-21 14:56:11 +01:00 committed by Dietmar Maurer
parent cb4865466e
commit 4799280ccd

View File

@ -18,6 +18,7 @@ use proxmox::{
api::error::HttpError, api::error::HttpError,
sys::linux::tty, sys::linux::tty,
tools::fs::{file_get_json, replace_file, CreateOptions}, tools::fs::{file_get_json, replace_file, CreateOptions},
tools::future::TimeoutFutureExt,
}; };
use super::pipe_to_stream::PipeToSendStream; use super::pipe_to_stream::PipeToSendStream;
@ -29,6 +30,10 @@ use crate::tools::{
http::HttpsConnector, http::HttpsConnector,
}; };
/// Timeout used for several HTTP operations that are expected to finish quickly but may block in
/// certain error conditions.
const HTTP_TIMEOUT: Duration = Duration::from_secs(20);
#[derive(Clone)] #[derive(Clone)]
pub struct AuthInfo { pub struct AuthInfo {
pub auth_id: Authid, pub auth_id: Authid,
@ -557,7 +562,10 @@ impl HttpClient {
let enc_ticket = format!("PBSAuthCookie={}", percent_encode(auth.ticket.as_bytes(), DEFAULT_ENCODE_SET)); let enc_ticket = format!("PBSAuthCookie={}", percent_encode(auth.ticket.as_bytes(), DEFAULT_ENCODE_SET));
req.headers_mut().insert("Cookie", HeaderValue::from_str(&enc_ticket).unwrap()); req.headers_mut().insert("Cookie", HeaderValue::from_str(&enc_ticket).unwrap());
let resp = client.request(req).await?; let resp = client
.request(req)
.or_timeout_err(HTTP_TIMEOUT, format_err!("http download request timed out"))
.await?;
let status = resp.status(); let status = resp.status();
if !status.is_success() { if !status.is_success() {
HttpClient::api_response(resp) HttpClient::api_response(resp)
@ -624,7 +632,10 @@ impl HttpClient {
req.headers_mut().insert("UPGRADE", HeaderValue::from_str(&protocol_name).unwrap()); req.headers_mut().insert("UPGRADE", HeaderValue::from_str(&protocol_name).unwrap());
let resp = client.request(req).await?; let resp = client
.request(req)
.or_timeout_err(HTTP_TIMEOUT, format_err!("http upgrade request timed out"))
.await?;
let status = resp.status(); let status = resp.status();
if status != http::StatusCode::SWITCHING_PROTOCOLS { if status != http::StatusCode::SWITCHING_PROTOCOLS {
@ -705,7 +716,7 @@ impl HttpClient {
) -> Result<Value, Error> { ) -> Result<Value, Error> {
client.request(req) client.request(req)
.map_err(Error::from) .or_timeout_err(HTTP_TIMEOUT, format_err!("http request timed out"))
.and_then(Self::api_response) .and_then(Self::api_response)
.await .await
} }