From 4799280ccd87d04602e50768e50bce2f804c4cae Mon Sep 17 00:00:00 2001
From: Stefan Reiter <s.reiter@proxmox.com>
Date: Mon, 21 Dec 2020 14:56:11 +0100
Subject: [PATCH] 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>
---
 src/client/http_client.rs | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/client/http_client.rs b/src/client/http_client.rs
index a9b9c06c..92df9572 100644
--- a/src/client/http_client.rs
+++ b/src/client/http_client.rs
@@ -18,6 +18,7 @@ use proxmox::{
     api::error::HttpError,
     sys::linux::tty,
     tools::fs::{file_get_json, replace_file, CreateOptions},
+    tools::future::TimeoutFutureExt,
 };
 
 use super::pipe_to_stream::PipeToSendStream;
@@ -29,6 +30,10 @@ use crate::tools::{
     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)]
 pub struct AuthInfo {
     pub auth_id: Authid,
@@ -557,7 +562,10 @@ impl HttpClient {
         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());
 
-        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();
         if !status.is_success() {
             HttpClient::api_response(resp)
@@ -624,7 +632,10 @@ impl HttpClient {
 
         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();
 
         if status != http::StatusCode::SWITCHING_PROTOCOLS {
@@ -705,7 +716,7 @@ impl HttpClient {
     ) -> Result<Value, Error> {
 
         client.request(req)
-            .map_err(Error::from)
+            .or_timeout_err(HTTP_TIMEOUT, format_err!("http request timed out"))
             .and_then(Self::api_response)
             .await
     }