Make --http-request-timeout optional by default

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## Unreleased
+
+### Fixed
+
+- Further fixes to HTTPS connections lock-ups.
+
+### Changed
+
+- **BREAKING**: Make `--http-request-timeout` optional by default.
+
 ## 0.3.2 (2025-01-12)
 
 ### Added

book/src/cli.md

@@ -243,7 +243,9 @@ Expose HTTP/SSH/TCP services through SSH port forwarding.
           [default: 5s]
 
       <b>--http-request-timeout</b> &lt;DURATION&gt;
-          Time until an outgoing HTTP request is automatically canceled
+          Time until an outgoing HTTP request is automatically canceled.
+
+          By default, outgoing requests are not terminated by Sandhole.
 
           [default: 30s]
 

docker-compose-example/compose.yml

@@ -29,5 +29,6 @@ services:
       --http-port=80
       --https-port=443
       --force-https
+      --http-request-timeout=60s
     network_mode: host
     restart: unless-stopped

src/config.rs

@@ -295,8 +295,10 @@ pub struct ApplicationConfig {
     pub authentication_request_timeout: Duration,
 
     /// Time until an outgoing HTTP request is automatically canceled.
-    #[arg(long, default_value = "30s", value_name = "DURATION")]
-    pub http_request_timeout: Duration,
+    ///
+    /// By default, outgoing requests are not terminated by Sandhole.
+    #[arg(long, value_name = "DURATION")]
+    pub http_request_timeout: Option<Duration>,
 
     /// How long until TCP connections (including Websockets and local forwardings) are automatically garbage-collected.
     ///
@@ -372,7 +374,7 @@ mod application_config_tests {
                 idle_connection_timeout: Duration::from_str("2s").unwrap(),
                 unproxied_connection_timeout: None,
                 authentication_request_timeout: Duration::from_str("5s").unwrap(),
-                http_request_timeout: Duration::from_str("30s").unwrap(),
+                http_request_timeout: None,
                 tcp_connection_timeout: None
             }
         )
@@ -419,7 +421,7 @@ mod application_config_tests {
             "--idle-connection-timeout=3s",
             "--unproxied-connection-timeout=4s",
             "--authentication-request-timeout=6s",
-            "--http-request-timeout=11s",
+            "--http-request-timeout=15s",
             "--tcp-connection-timeout=30s",
         ]);
         assert_eq!(
@@ -465,7 +467,7 @@ mod application_config_tests {
                 idle_connection_timeout: Duration::from_str("3s").unwrap(),
                 unproxied_connection_timeout: Some(Duration::from_str("4s").unwrap()),
                 authentication_request_timeout: Duration::from_str("6s").unwrap(),
-                http_request_timeout: Duration::from_str("11s").unwrap(),
+                http_request_timeout: Some(Duration::from_str("15s").unwrap()),
                 tcp_connection_timeout: Some(Duration::from_str("30s").unwrap())
             }
         )

src/http.rs

@@ -121,8 +121,8 @@ where
     pub(crate) protocol: Protocol,
     // Configuration on which type of channel to retrieve from the handler.
     pub(crate) proxy_type: ProxyType,
-    // Duration until an outgoing request is canceled.
-    pub(crate) http_request_timeout: Duration,
+    // Optional duration until an outgoing request is canceled.
+    pub(crate) http_request_timeout: Option<Duration>,
     // Optional duration until an established Websocket connection is canceled.
     pub(crate) websocket_timeout: Option<Duration>,
     // If set, disables sending HTTP logs to the handler.
@@ -283,13 +283,13 @@ where
                     warn!("Connection failed: {:?}", err);
                 }
             });
-            // Await for a response under the given duration.
-            let response = timeout(
-                proxy_data.http_request_timeout,
-                sender.send_request(request),
-            )
-            .await
-            .map_err(|_| ServerError::RequestTimeout)??;
+            let response = match proxy_data.http_request_timeout {
+                // Await for a response under the given duration.
+                Some(duration) => timeout(duration, sender.send_request(request))
+                    .await
+                    .map_err(|_| ServerError::RequestTimeout)??,
+                None => sender.send_request(request).await?,
+            };
             let elapsed_time = timer.elapsed();
             http_log(
                 HttpLog {
@@ -317,13 +317,13 @@ where
             let request_type = request_upgrade.to_str()?.to_string();
             // Retrieve the OnUpgrade from the incoming request
             let upgraded_request = hyper::upgrade::on(&mut request);
-            // Await for a response under the given duration.
-            let mut response = timeout(
-                proxy_data.http_request_timeout,
-                sender.send_request(request),
-            )
-            .await
-            .map_err(|_| ServerError::RequestTimeout)??;
+            let mut response = match proxy_data.http_request_timeout {
+                // Await for a response under the given duration.
+                Some(duration) => timeout(duration, sender.send_request(request))
+                    .await
+                    .map_err(|_| ServerError::RequestTimeout)??,
+                None => sender.send_request(request).await?,
+            };
             let elapsed_time = timer.elapsed();
             http_log(
                 HttpLog {
@@ -395,14 +395,15 @@ mod proxy_handler_tests {
     use hyper::{body::Incoming, service::service_fn, HeaderMap, Request, StatusCode};
     use hyper_util::rt::TokioIo;
     use std::{marker::PhantomData, sync::Arc, time::Duration};
-    use tokio::{io::DuplexStream, sync::mpsc};
+    use tokio::{io::DuplexStream, sync::mpsc, time::sleep};
     use tokio_tungstenite::client_async;
     use tower::Service;
 
     use crate::{
         config::LoadBalancing,
         connection_handler::{ConnectionHttpData, MockConnectionHandler},
         connections::ConnectionMap,
+        http::ServerError,
         quota::{DummyQuotaHandler, TokenHolder, UserIdentification},
         reactor::MockConnectionMapReactor,
         telemetry::Telemetry,
@@ -441,7 +442,7 @@ mod proxy_handler_tests {
                 }),
                 protocol: Protocol::Http { port: 80 },
                 proxy_type: ProxyType::Tunneling,
-                http_request_timeout: Duration::from_secs(5),
+                http_request_timeout: None,
                 websocket_timeout: None,
                 disable_http_logs: false,
                 _phantom_data: PhantomData,
@@ -483,7 +484,7 @@ mod proxy_handler_tests {
                 }),
                 protocol: Protocol::Http { port: 80 },
                 proxy_type: ProxyType::Tunneling,
-                http_request_timeout: Duration::from_secs(5),
+                http_request_timeout: None,
                 websocket_timeout: None,
                 disable_http_logs: false,
                 _phantom_data: PhantomData,
@@ -527,7 +528,7 @@ mod proxy_handler_tests {
                 }),
                 protocol: Protocol::Http { port: 80 },
                 proxy_type: ProxyType::Tunneling,
-                http_request_timeout: Duration::from_secs(5),
+                http_request_timeout: None,
                 websocket_timeout: None,
                 disable_http_logs: false,
                 _phantom_data: PhantomData,
@@ -592,7 +593,7 @@ mod proxy_handler_tests {
                 }),
                 protocol: Protocol::TlsRedirect { from: 80, to: 443 },
                 proxy_type: ProxyType::Tunneling,
-                http_request_timeout: Duration::from_secs(5),
+                http_request_timeout: None,
                 websocket_timeout: None,
                 disable_http_logs: false,
                 _phantom_data: PhantomData,
@@ -660,7 +661,7 @@ mod proxy_handler_tests {
                 }),
                 protocol: Protocol::TlsRedirect { from: 80, to: 8443 },
                 proxy_type: ProxyType::Tunneling,
-                http_request_timeout: Duration::from_secs(5),
+                http_request_timeout: None,
                 websocket_timeout: None,
                 disable_http_logs: false,
                 _phantom_data: PhantomData,
@@ -728,7 +729,7 @@ mod proxy_handler_tests {
                 }),
                 protocol: Protocol::Http { port: 80 },
                 proxy_type: ProxyType::Tunneling,
-                http_request_timeout: Duration::from_secs(5),
+                http_request_timeout: None,
                 websocket_timeout: None,
                 disable_http_logs: false,
                 _phantom_data: PhantomData,
@@ -796,7 +797,7 @@ mod proxy_handler_tests {
                 }),
                 protocol: Protocol::Http { port: 80 },
                 proxy_type: ProxyType::Tunneling,
-                http_request_timeout: Duration::from_secs(5),
+                http_request_timeout: None,
                 websocket_timeout: None,
                 disable_http_logs: false,
                 _phantom_data: PhantomData,
@@ -815,6 +816,101 @@ mod proxy_handler_tests {
         );
     }
 
+    #[tokio::test]
+    async fn returns_error_for_outgoing_request_timeout() {
+        let conn_manager: Arc<
+            ConnectionMap<
+                String,
+                Arc<MockConnectionHandler<DuplexStream>>,
+                MockConnectionMapReactor<String>,
+            >,
+        > = Arc::new(ConnectionMap::new(
+            LoadBalancing::Allow,
+            Arc::new(Box::new(DummyQuotaHandler)),
+            None,
+        ));
+        let (server, handler) = tokio::io::duplex(1024);
+        let (logging_tx, logging_rx) = mpsc::unbounded_channel::<Vec<u8>>();
+        let mut mock = MockConnectionHandler::new();
+        mock.expect_log_channel()
+            .once()
+            .return_once(move || Some(logging_tx));
+        mock.expect_tunneling_channel()
+            .once()
+            .return_once(move |_, _| Ok(handler));
+        mock.expect_http_data().once().return_once(move || {
+            Some(ConnectionHttpData {
+                redirect_http_to_https_port: None,
+                is_aliasing: false,
+            })
+        });
+        conn_manager
+            .insert(
+                "slow.handler".into(),
+                "127.0.0.1:12345".parse().unwrap(),
+                TokenHolder::User(UserIdentification::Username("a".into())),
+                Arc::new(mock),
+            )
+            .unwrap();
+        let request = Request::builder()
+            .method("GET")
+            .uri("/slow_endpoint")
+            .header("host", "slow.handler")
+            .body(Empty::<Bytes>::new())
+            .unwrap();
+        let router = axum::Router::new()
+            .route(
+                "/slow_endpoint",
+                axum::routing::get(|| async move {
+                    sleep(Duration::from_secs(1)).await;
+                    "Slow hello."
+                }),
+            )
+            .into_service();
+        let router_service = service_fn(move |req: Request<Incoming>| router.clone().call(req));
+        let jh = tokio::spawn(async move {
+            hyper_util::server::conn::auto::Builder::new(hyper_util::rt::TokioExecutor::new())
+                .serve_connection(TokioIo::new(server), router_service)
+                .await
+                .expect("Invalid request");
+        });
+        assert!(
+            logging_rx.is_empty(),
+            "shouldn't log before handling request"
+        );
+        let response = proxy_handler(
+            request,
+            "127.0.0.1:12345".parse().unwrap(),
+            None,
+            Arc::new(ProxyData {
+                conn_manager: Arc::clone(&conn_manager),
+                telemetry: Arc::new(Telemetry::new()),
+                domain_redirect: Arc::new(DomainRedirect {
+                    from: "main.domain".into(),
+                    to: "https://example.com".into(),
+                }),
+                protocol: Protocol::Https { port: 443 },
+                proxy_type: ProxyType::Tunneling,
+                http_request_timeout: Some(Duration::from_millis(500)),
+                websocket_timeout: None,
+                disable_http_logs: false,
+                _phantom_data: PhantomData,
+            }),
+        )
+        .await;
+        assert!(
+            logging_rx.is_empty(),
+            "shouldn't log if failed to proxy request"
+        );
+        assert!(response.is_err(), "should fail if timed out");
+        let error = response.unwrap_err();
+        assert!(matches!(
+            error.downcast().unwrap(),
+            ServerError::RequestTimeout
+        ));
+        jh.abort();
+    }
+
     #[tokio::test]
     async fn returns_response_for_existing_handler() {
         let conn_manager: Arc<
@@ -896,7 +992,7 @@ mod proxy_handler_tests {
                 }),
                 protocol: Protocol::Https { port: 443 },
                 proxy_type: ProxyType::Tunneling,
-                http_request_timeout: Duration::from_secs(5),
+                http_request_timeout: None,
                 websocket_timeout: None,
                 disable_http_logs: false,
                 _phantom_data: PhantomData,
@@ -994,7 +1090,7 @@ mod proxy_handler_tests {
                 }),
                 protocol: Protocol::Http { port: 80 },
                 proxy_type: ProxyType::Aliasing,
-                http_request_timeout: Duration::from_secs(5),
+                http_request_timeout: None,
                 websocket_timeout: None,
                 disable_http_logs: false,
                 _phantom_data: PhantomData,
@@ -1092,7 +1188,7 @@ mod proxy_handler_tests {
                 }),
                 protocol: Protocol::Https { port: 443 },
                 proxy_type: ProxyType::Tunneling,
-                http_request_timeout: Duration::from_secs(5),
+                http_request_timeout: None,
                 websocket_timeout: None,
                 disable_http_logs: false,
                 _phantom_data: PhantomData,
@@ -1184,7 +1280,7 @@ mod proxy_handler_tests {
                     }),
                     protocol: Protocol::Https { port: 443 },
                     proxy_type: ProxyType::Tunneling,
-                    http_request_timeout: Duration::from_secs(5),
+                    http_request_timeout: None,
                     websocket_timeout: None,
                     disable_http_logs: false,
                     _phantom_data: PhantomData,

src/lib.rs

@@ -492,7 +492,7 @@ pub async fn entrypoint(config: ApplicationConfig) -> anyhow::Result<()> {
         },
         // Always use aliasing channels instead of tunneling channels.
         proxy_type: ProxyType::Aliasing,
-        http_request_timeout: config.http_request_timeout.into(),
+        http_request_timeout: config.http_request_timeout.map(Into::into),
         websocket_timeout: config.tcp_connection_timeout.map(Into::into),
         disable_http_logs: config.disable_http_logs,
         _phantom_data: PhantomData,
@@ -562,7 +562,7 @@ pub async fn entrypoint(config: ApplicationConfig) -> anyhow::Result<()> {
             },
             // Always use tunneling channels.
             proxy_type: ProxyType::Tunneling,
-            http_request_timeout: config.http_request_timeout.into(),
+            http_request_timeout: config.http_request_timeout.map(Into::into),
             websocket_timeout: config.tcp_connection_timeout.map(Into::into),
             disable_http_logs: config.disable_http_logs,
             _phantom_data: PhantomData,
@@ -626,7 +626,7 @@ pub async fn entrypoint(config: ApplicationConfig) -> anyhow::Result<()> {
             },
             // Always use tunneling channels.
             proxy_type: ProxyType::Tunneling,
-            http_request_timeout: config.http_request_timeout.into(),
+            http_request_timeout: config.http_request_timeout.map(Into::into),
             websocket_timeout: config.tcp_connection_timeout.map(Into::into),
             disable_http_logs: config.disable_http_logs,
             _phantom_data: PhantomData,