From ca2017030b471010a8301b186e0c7a9de5906ba6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Wed, 23 Jul 2025 11:47:30 +0200 Subject: [PATCH 1/5] refactor: Make `RequestConfig::timeout` optional This allows us to remove the default timeout on a per-request basis --- crates/matrix-sdk/src/client/mod.rs | 2 +- crates/matrix-sdk/src/config/request.rs | 6 ++++-- crates/matrix-sdk/src/http_client/native.rs | 15 ++++++++++----- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index d51c9e0728d..c1c5b448ac4 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -2312,7 +2312,7 @@ impl Client { }); let mut request_config = self.request_config(); if let Some(timeout) = sync_settings.timeout { - request_config.timeout += timeout; + request_config.timeout = Some(timeout); } let response = self.send(request).with_request_config(request_config).await?; diff --git a/crates/matrix-sdk/src/config/request.rs b/crates/matrix-sdk/src/config/request.rs index 34d60b5f80d..61c317300eb 100644 --- a/crates/matrix-sdk/src/config/request.rs +++ b/crates/matrix-sdk/src/config/request.rs @@ -44,6 +44,7 @@ use crate::http_client::DEFAULT_REQUEST_TIMEOUT; #[derive(Copy, Clone)] pub struct RequestConfig { pub(crate) timeout: Duration, + pub(crate) timeout: Option, pub(crate) retry_limit: Option, pub(crate) max_retry_time: Option, pub(crate) max_concurrent_requests: Option, @@ -82,6 +83,7 @@ impl Default for RequestConfig { fn default() -> Self { Self { timeout: DEFAULT_REQUEST_TIMEOUT, + timeout: Some(DEFAULT_REQUEST_TIMEOUT), retry_limit: Default::default(), max_retry_time: Default::default(), max_concurrent_requests: Default::default(), @@ -132,8 +134,8 @@ impl RequestConfig { /// Set the timeout duration for all HTTP requests. #[must_use] - pub fn timeout(mut self, timeout: Duration) -> Self { - self.timeout = timeout; + pub fn timeout(mut self, timeout: impl Into>) -> Self { + self.timeout = timeout.into(); self } diff --git a/crates/matrix-sdk/src/http_client/native.rs b/crates/matrix-sdk/src/http_client/native.rs index 3c483bda9f5..ac476968444 100644 --- a/crates/matrix-sdk/src/http_client/native.rs +++ b/crates/matrix-sdk/src/http_client/native.rs @@ -149,7 +149,8 @@ pub(crate) struct HttpSettings { pub(crate) disable_ssl_verification: bool, pub(crate) proxy: Option, pub(crate) user_agent: Option, - pub(crate) timeout: Duration, + pub(crate) timeout: Option, + pub(crate) read_timeout: Option, pub(crate) additional_root_certificates: Vec, pub(crate) disable_built_in_root_certificates: bool, } @@ -161,7 +162,8 @@ impl Default for HttpSettings { disable_ssl_verification: false, proxy: None, user_agent: None, - timeout: DEFAULT_REQUEST_TIMEOUT, + timeout: Some(DEFAULT_REQUEST_TIMEOUT), + read_timeout: None, additional_root_certificates: Default::default(), disable_built_in_root_certificates: false, } @@ -175,11 +177,14 @@ impl HttpSettings { let user_agent = self.user_agent.clone().unwrap_or_else(|| "matrix-rust-sdk".to_owned()); let mut http_client = reqwest::Client::builder() .user_agent(user_agent) - .timeout(self.timeout) // As recommended by BCP 195. // See: https://datatracker.ietf.org/doc/bcp195/ .min_tls_version(tls::Version::TLS_1_2); + if let Some(timeout) = self.timeout { + http_client = http_client.timeout(timeout); + } + if self.disable_ssl_verification { warn!("SSL verification disabled in the HTTP client!"); http_client = http_client.danger_accept_invalid_certs(true) @@ -213,7 +218,7 @@ impl HttpSettings { pub(super) async fn send_request( client: &reqwest::Client, request: &http::Request, - timeout: Duration, + timeout: Option, send_progress: SharedObservable, ) -> Result, HttpError> { use std::convert::Infallible; @@ -251,7 +256,7 @@ pub(super) async fn send_request( reqwest::Request::try_from(request)? }; - *request.timeout_mut() = Some(timeout); + *request.timeout_mut() = timeout; request }; From 9c412b6a2e4eadf67a1611f4a80809ea4c270da5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Wed, 23 Jul 2025 11:48:10 +0200 Subject: [PATCH 2/5] refactor: Add `RequestConfig::read_timeout` to cancel network connections that have been stale for longer than the specified timeout --- crates/matrix-sdk/src/config/request.rs | 26 +++++++++++++++++---- crates/matrix-sdk/src/http_client/native.rs | 4 ++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/crates/matrix-sdk/src/config/request.rs b/crates/matrix-sdk/src/config/request.rs index 61c317300eb..cefeff6fd72 100644 --- a/crates/matrix-sdk/src/config/request.rs +++ b/crates/matrix-sdk/src/config/request.rs @@ -43,8 +43,8 @@ use crate::http_client::DEFAULT_REQUEST_TIMEOUT; /// ``` #[derive(Copy, Clone)] pub struct RequestConfig { - pub(crate) timeout: Duration, pub(crate) timeout: Option, + pub(crate) read_timeout: Option, pub(crate) retry_limit: Option, pub(crate) max_retry_time: Option, pub(crate) max_concurrent_requests: Option, @@ -57,6 +57,7 @@ impl Debug for RequestConfig { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { let Self { timeout, + read_timeout, retry_limit, max_retry_time: retry_timeout, force_auth, @@ -66,6 +67,7 @@ impl Debug for RequestConfig { let mut res = fmt.debug_struct("RequestConfig"); res.field("timeout", timeout) + .maybe_field("read_timeout", read_timeout) .maybe_field("retry_limit", retry_limit) .maybe_field("max_retry_time", retry_timeout) .maybe_field("max_concurrent_requests", max_concurrent_requests) @@ -82,8 +84,8 @@ impl Debug for RequestConfig { impl Default for RequestConfig { fn default() -> Self { Self { - timeout: DEFAULT_REQUEST_TIMEOUT, timeout: Some(DEFAULT_REQUEST_TIMEOUT), + read_timeout: None, retry_limit: Default::default(), max_retry_time: Default::default(), max_concurrent_requests: Default::default(), @@ -139,6 +141,20 @@ impl RequestConfig { self } + /// Set the read timeout duration for all HTTP requests. + /// + /// The timeout applies to each read operation, and resets after a + /// successful read. This is more appropriate for detecting stalled + /// connections when the size isn’t known beforehand. + /// + /// **IMPORTANT**: note this value can only be applied when the HTTP client + /// is instantiated, it won't have any effect on a per-request basis. + #[must_use] + pub fn read_timeout(mut self, timeout: impl Into>) -> Self { + self.read_timeout = timeout.into(); + self + } + /// Set a time limit for how long a request should be retried. The default /// is that there isn't a limit, meaning requests are retried forever. /// @@ -182,12 +198,14 @@ mod tests { .force_auth() .max_retry_time(Duration::from_secs(32)) .retry_limit(4) - .timeout(Duration::from_secs(600)); + .timeout(Duration::from_secs(600)) + .read_timeout(Duration::from_secs(10)); assert!(cfg.force_auth); assert_eq!(cfg.retry_limit, Some(4)); assert_eq!(cfg.max_retry_time, Some(Duration::from_secs(32))); - assert_eq!(cfg.timeout, Duration::from_secs(600)); + assert_eq!(cfg.timeout, Some(Duration::from_secs(600))); + assert_eq!(cfg.read_timeout, Some(Duration::from_secs(10))); } #[test] diff --git a/crates/matrix-sdk/src/http_client/native.rs b/crates/matrix-sdk/src/http_client/native.rs index ac476968444..67ed8228d58 100644 --- a/crates/matrix-sdk/src/http_client/native.rs +++ b/crates/matrix-sdk/src/http_client/native.rs @@ -185,6 +185,10 @@ impl HttpSettings { http_client = http_client.timeout(timeout); } + if let Some(read_timeout) = self.read_timeout { + http_client = http_client.read_timeout(read_timeout); + } + if self.disable_ssl_verification { warn!("SSL verification disabled in the HTTP client!"); http_client = http_client.danger_accept_invalid_certs(true) From a5485769be4bbe11bfa475bd4346545a6a5fcef3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Wed, 23 Jul 2025 11:48:56 +0200 Subject: [PATCH 3/5] refactor: Remove the default timeout when downloading media, rely on `read_timeout` instead Users with poor network connectivity complained their downloads were cancelled for no good reason after 30s before (the default `timeout` value). --- crates/matrix-sdk/src/media.rs | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/crates/matrix-sdk/src/media.rs b/crates/matrix-sdk/src/media.rs index b96df15622b..5af0f621bb2 100644 --- a/crates/matrix-sdk/src/media.rs +++ b/crates/matrix-sdk/src/media.rs @@ -434,18 +434,24 @@ impl Media { } } + let request_config = self + .client + .request_config() + // Downloading a file should have no timeout as we don't know the network connectivity + // available for the user or the file size + .timeout(None); + // Use the authenticated endpoints when the server supports Matrix 1.11 or the // authenticated media stable feature. let (use_auth, request_config) = if self.client.server_versions().await?.contains(&MatrixVersion::V1_11) { - (true, None) + (true, request_config) } else if self.client.unstable_features().await?.contains(&FeatureFlag::Msc3916Stable) { // We need to force the use of the stable endpoint with the Matrix version // because Ruma does not handle stable features. - let request_config = self.client.request_config(); - (true, Some(request_config.force_matrix_version(MatrixVersion::V1_11))) + (true, request_config.force_matrix_version(MatrixVersion::V1_11)) } else { - (false, None) + (false, request_config) }; let content: Vec = match &request.source { From ce150466539bee5f60dd41b1b867be91b7ce9837 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Wed, 23 Jul 2025 11:49:16 +0200 Subject: [PATCH 4/5] refactor(ffi): Add the default `read_timeout` value to the FFI layer --- bindings/matrix-sdk-ffi/src/client_builder.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bindings/matrix-sdk-ffi/src/client_builder.rs b/bindings/matrix-sdk-ffi/src/client_builder.rs index 30ef4543f81..11618c1f2ad 100644 --- a/bindings/matrix-sdk-ffi/src/client_builder.rs +++ b/bindings/matrix-sdk-ffi/src/client_builder.rs @@ -133,6 +133,11 @@ pub struct ClientBuilder { threads_enabled: bool, } +/// The timeout applies to each read operation, and resets after a successful +/// read. This is more appropriate for detecting stalled connections when the +/// size isn’t known beforehand. +const DEFAULT_READ_TIMEOUT: Duration = Duration::from_secs(60); + #[matrix_sdk_ffi_macros::export] impl ClientBuilder { #[uniffi::constructor] @@ -539,6 +544,7 @@ impl ClientBuilder { if let Some(timeout) = config.timeout { updated_config = updated_config.timeout(Duration::from_millis(timeout)); } + updated_config = updated_config.read_timeout(DEFAULT_READ_TIMEOUT); if let Some(max_concurrent_requests) = config.max_concurrent_requests { if max_concurrent_requests > 0 { updated_config = updated_config.max_concurrent_requests(NonZeroUsize::new( From ac10d4a46b5b97684303e3e263f5a085040ddfcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Wed, 23 Jul 2025 12:13:54 +0200 Subject: [PATCH 5/5] fix: Restore the previous behaviour for calculating the timeout for syncs --- crates/matrix-sdk/CHANGELOG.md | 7 +++++++ crates/matrix-sdk/src/client/mod.rs | 4 +++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk/CHANGELOG.md b/crates/matrix-sdk/CHANGELOG.md index aa9ce0cd568..5d8e3834695 100644 --- a/crates/matrix-sdk/CHANGELOG.md +++ b/crates/matrix-sdk/CHANGELOG.md @@ -31,6 +31,13 @@ All notable changes to this project will be documented in this file. - [**breaking**] The MSRV has been bumped to Rust 1.88. ([#5431](https://github.com/matrix-org/matrix-rust-sdk/pull/5431)) +### Bugfix + +- All HTTP requests now have a default `read_timeout` of 60s, which means they'll disconnect if the connection stalls. + `RequestConfig::timeout` is now optional and can be disabled on a per-request basis. This will be done for + the requests used to download media, so they don't get cancelled after the default 30s timeout for no good reason. + ([#5437](https://github.com/matrix-org/matrix-rust-sdk/pull/5437)) + ## [0.13.0] - 2025-07-10 ### Security Fixes diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index c1c5b448ac4..e9a81164bd6 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -20,6 +20,7 @@ use std::{ future::{ready, Future}, pin::Pin, sync::{Arc, Mutex as StdMutex, RwLock as StdRwLock, Weak}, + time::Duration, }; use caches::ClientCaches; @@ -2312,7 +2313,8 @@ impl Client { }); let mut request_config = self.request_config(); if let Some(timeout) = sync_settings.timeout { - request_config.timeout = Some(timeout); + let base_timeout = request_config.timeout.unwrap_or(Duration::from_secs(30)); + request_config.timeout = Some(base_timeout + timeout); } let response = self.send(request).with_request_config(request_config).await?;