From f8d1c1a8af122d8955b4b08fa564723badbb3f77 Mon Sep 17 00:00:00 2001 From: "Aode (Lion)" Date: Mon, 24 Jan 2022 18:42:15 -0600 Subject: [PATCH 1/3] Re-use a basic request in all possible cases --- src/appservice_server.rs | 6 +----- src/database/globals.rs | 40 ++++++++++++++++++++++++++++++---------- src/database/pusher.rs | 6 +----- src/server_server.rs | 23 ++++++++++++----------- 4 files changed, 44 insertions(+), 31 deletions(-) diff --git a/src/appservice_server.rs b/src/appservice_server.rs index ed886d6c..a5d795f6 100644 --- a/src/appservice_server.rs +++ b/src/appservice_server.rs @@ -46,11 +46,7 @@ where *reqwest_request.timeout_mut() = Some(Duration::from_secs(30)); let url = reqwest_request.url().clone(); - let mut response = globals - .reqwest_client()? - .build()? - .execute(reqwest_request) - .await?; + let mut response = globals.reqwest_client().execute(reqwest_request).await?; // reqwest::Response -> http::Response conversion let status = response.status(); diff --git a/src/database/globals.rs b/src/database/globals.rs index 098d8197..da91c1fb 100644 --- a/src/database/globals.rs +++ b/src/database/globals.rs @@ -39,6 +39,7 @@ pub struct Globals { keypair: Arc, dns_resolver: TokioAsyncResolver, jwt_decoding_key: Option>, + basic_client: reqwest::Client, pub(super) server_signingkeys: Arc, pub bad_event_ratelimiter: Arc, RateLimitState>>>, pub bad_signature_ratelimiter: Arc, RateLimitState>>>, @@ -132,6 +133,8 @@ impl Globals { .as_ref() .map(|secret| jsonwebtoken::DecodingKey::from_secret(secret.as_bytes()).into_static()); + let basic_client = reqwest_client_builder(&config, None)?.build()?; + let s = Self { globals, config, @@ -141,6 +144,7 @@ impl Globals { })?, actual_destination_cache: Arc::new(RwLock::new(WellKnownMap::new())), tls_name_override, + basic_client, server_signingkeys, jwt_decoding_key, bad_event_ratelimiter: Arc::new(RwLock::new(HashMap::new())), @@ -163,17 +167,15 @@ impl Globals { &self.keypair } - /// Returns a reqwest client which can be used to send requests. - pub fn reqwest_client(&self) -> Result { - let mut reqwest_client_builder = reqwest::Client::builder() - .connect_timeout(Duration::from_secs(30)) - .timeout(Duration::from_secs(60 * 3)) - .pool_max_idle_per_host(1); - if let Some(proxy) = self.config.proxy.to_proxy()? { - reqwest_client_builder = reqwest_client_builder.proxy(proxy); - } + /// Returns a reqwest client which can be used to send requests + pub fn reqwest_client(&self) -> reqwest::Client { + // can't return &Client or else we'll hold a lock around the DB across an await + self.basic_client.clone() + } - Ok(reqwest_client_builder) + /// Returns a reqwest client builder which can be customized and used to send requests. + pub fn reqwest_client_builder(&self) -> Result { + reqwest_client_builder(&self.config, Some(1)) } #[tracing::instrument(skip(self))] @@ -340,3 +342,21 @@ impl Globals { r } } + +fn reqwest_client_builder( + config: &Config, + max_idle: Option, +) -> Result { + let mut reqwest_client_builder = reqwest::Client::builder() + .connect_timeout(Duration::from_secs(30)) + .timeout(Duration::from_secs(60 * 3)); + + if let Some(max_idle) = max_idle { + reqwest_client_builder = reqwest_client_builder.pool_max_idle_per_host(max_idle); + } + if let Some(proxy) = config.proxy.to_proxy()? { + reqwest_client_builder = reqwest_client_builder.proxy(proxy); + } + + Ok(reqwest_client_builder) +} diff --git a/src/database/pusher.rs b/src/database/pusher.rs index 97ca85d8..d63db1d7 100644 --- a/src/database/pusher.rs +++ b/src/database/pusher.rs @@ -115,11 +115,7 @@ where //*reqwest_request.timeout_mut() = Some(Duration::from_secs(5)); let url = reqwest_request.url().clone(); - let response = globals - .reqwest_client()? - .build()? - .execute(reqwest_request) - .await; + let response = globals.reqwest_client().execute(reqwest_request).await; match response { Ok(mut response) => { diff --git a/src/server_server.rs b/src/server_server.rs index 9129951b..205355f9 100644 --- a/src/server_server.rs +++ b/src/server_server.rs @@ -237,21 +237,25 @@ where let url = reqwest_request.url().clone(); - let mut client = globals.reqwest_client()?; - if let Some((override_name, port)) = globals + let client = if let Some((override_name, port)) = globals .tls_name_override .read() .unwrap() .get(&actual_destination.hostname()) { - client = client.resolve( - &actual_destination.hostname(), - SocketAddr::new(override_name[0], *port), - ); + globals + .reqwest_client_builder()? + .resolve( + &actual_destination.hostname(), + SocketAddr::new(override_name[0], *port), + ) + .build()? // port will be ignored - } + } else { + globals.reqwest_client() + }; - let response = client.build()?.execute(reqwest_request).await; + let response = client.execute(reqwest_request).await; match response { Ok(mut response) => { @@ -492,9 +496,6 @@ async fn request_well_known( let body: serde_json::Value = serde_json::from_str( &globals .reqwest_client() - .ok()? - .build() - .ok()? .get(&format!( "https://{}/.well-known/matrix/server", destination From 1059f35fdcf4942fd253748121c883ea38b427a7 Mon Sep 17 00:00:00 2001 From: "Aode (lion)" Date: Thu, 27 Jan 2022 10:19:28 -0600 Subject: [PATCH 2/3] use pre-constructed client for well-known requests also --- Cargo.lock | 3 +-- Cargo.toml | 2 +- src/database/globals.rs | 30 ++++++++++++++++++------------ src/server_server.rs | 20 +------------------- 4 files changed, 21 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 794a0257..21c27700 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1950,8 +1950,7 @@ dependencies = [ [[package]] name = "reqwest" version = "0.11.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "87f242f1488a539a79bac6dbe7c8609ae43b7914b7736210f239a37cccb32525" +source = "git+https://github.com/niuhuan/reqwest?branch=dns-resolver-fn#57b7cf4feb921573dfafad7d34b9ac6e44ead0bd" dependencies = [ "base64 0.13.0", "bytes", diff --git a/Cargo.toml b/Cargo.toml index 9ba1ac05..974b4ce8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,7 +48,7 @@ rand = "0.8.4" # Used to hash passwords rust-argon2 = "0.8.3" # Used to send requests -reqwest = { version = "0.11.4", default-features = false, features = ["rustls-tls", "socks"] } +reqwest = { version = "0.11.4", default-features = false, features = ["rustls-tls", "socks"], git = "https://github.com/niuhuan/reqwest", branch = "dns-resolver-fn" } # Used for conduit::Error type thiserror = "1.0.28" # Used to generate thumbnails for images diff --git a/src/database/globals.rs b/src/database/globals.rs index da91c1fb..3278b7f6 100644 --- a/src/database/globals.rs +++ b/src/database/globals.rs @@ -10,7 +10,7 @@ use std::{ collections::{BTreeMap, HashMap}, fs, future::Future, - net::IpAddr, + net::{IpAddr, SocketAddr}, path::PathBuf, sync::{Arc, Mutex, RwLock}, time::{Duration, Instant}, @@ -39,6 +39,7 @@ pub struct Globals { keypair: Arc, dns_resolver: TokioAsyncResolver, jwt_decoding_key: Option>, + well_known_client: reqwest::Client, basic_client: reqwest::Client, pub(super) server_signingkeys: Arc, pub bad_event_ratelimiter: Arc, RateLimitState>>>, @@ -133,7 +134,16 @@ impl Globals { .as_ref() .map(|secret| jsonwebtoken::DecodingKey::from_secret(secret.as_bytes()).into_static()); - let basic_client = reqwest_client_builder(&config, None)?.build()?; + let basic_client = reqwest_client_builder(&config)?.build()?; + let name_override = Arc::clone(&tls_name_override); + let well_known_client = reqwest_client_builder(&config)? + .resolve_fn(move |domain| { + let read_guard = name_override.read().unwrap(); + let (override_name, port) = read_guard.get(&domain)?; + let first_name = override_name.get(0)?; + Some(SocketAddr::new(*first_name, *port)) + }) + .build()?; let s = Self { globals, @@ -144,6 +154,7 @@ impl Globals { })?, actual_destination_cache: Arc::new(RwLock::new(WellKnownMap::new())), tls_name_override, + well_known_client, basic_client, server_signingkeys, jwt_decoding_key, @@ -173,9 +184,10 @@ impl Globals { self.basic_client.clone() } - /// Returns a reqwest client builder which can be customized and used to send requests. - pub fn reqwest_client_builder(&self) -> Result { - reqwest_client_builder(&self.config, Some(1)) + /// Returns a client used for resolving .well-knowns + pub fn well_known_client(&self) -> reqwest::Client { + // can't return &Client or else we'll hold a lock around the DB across an await + self.well_known_client.clone() } #[tracing::instrument(skip(self))] @@ -343,17 +355,11 @@ impl Globals { } } -fn reqwest_client_builder( - config: &Config, - max_idle: Option, -) -> Result { +fn reqwest_client_builder(config: &Config) -> Result { let mut reqwest_client_builder = reqwest::Client::builder() .connect_timeout(Duration::from_secs(30)) .timeout(Duration::from_secs(60 * 3)); - if let Some(max_idle) = max_idle { - reqwest_client_builder = reqwest_client_builder.pool_max_idle_per_host(max_idle); - } if let Some(proxy) = config.proxy.to_proxy()? { reqwest_client_builder = reqwest_client_builder.proxy(proxy); } diff --git a/src/server_server.rs b/src/server_server.rs index 205355f9..978eb67f 100644 --- a/src/server_server.rs +++ b/src/server_server.rs @@ -237,25 +237,7 @@ where let url = reqwest_request.url().clone(); - let client = if let Some((override_name, port)) = globals - .tls_name_override - .read() - .unwrap() - .get(&actual_destination.hostname()) - { - globals - .reqwest_client_builder()? - .resolve( - &actual_destination.hostname(), - SocketAddr::new(override_name[0], *port), - ) - .build()? - // port will be ignored - } else { - globals.reqwest_client() - }; - - let response = client.execute(reqwest_request).await; + let response = globals.well_known_client().execute(reqwest_request).await; match response { Ok(mut response) => { From b39ddf7be9150b8baa6cecabed7730d2ab610a72 Mon Sep 17 00:00:00 2001 From: "Aode (lion)" Date: Fri, 28 Jan 2022 12:42:47 -0600 Subject: [PATCH 3/3] Rename reqwest clients, mention cheap client clones in comment --- src/appservice_server.rs | 2 +- src/database/globals.rs | 24 ++++++++++++------------ src/database/pusher.rs | 2 +- src/server_server.rs | 4 ++-- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/appservice_server.rs b/src/appservice_server.rs index a5d795f6..e78fb344 100644 --- a/src/appservice_server.rs +++ b/src/appservice_server.rs @@ -46,7 +46,7 @@ where *reqwest_request.timeout_mut() = Some(Duration::from_secs(30)); let url = reqwest_request.url().clone(); - let mut response = globals.reqwest_client().execute(reqwest_request).await?; + let mut response = globals.default_client().execute(reqwest_request).await?; // reqwest::Response -> http::Response conversion let status = response.status(); diff --git a/src/database/globals.rs b/src/database/globals.rs index 3278b7f6..decd84c3 100644 --- a/src/database/globals.rs +++ b/src/database/globals.rs @@ -39,8 +39,8 @@ pub struct Globals { keypair: Arc, dns_resolver: TokioAsyncResolver, jwt_decoding_key: Option>, - well_known_client: reqwest::Client, - basic_client: reqwest::Client, + federation_client: reqwest::Client, + default_client: reqwest::Client, pub(super) server_signingkeys: Arc, pub bad_event_ratelimiter: Arc, RateLimitState>>>, pub bad_signature_ratelimiter: Arc, RateLimitState>>>, @@ -134,9 +134,9 @@ impl Globals { .as_ref() .map(|secret| jsonwebtoken::DecodingKey::from_secret(secret.as_bytes()).into_static()); - let basic_client = reqwest_client_builder(&config)?.build()?; + let default_client = reqwest_client_builder(&config)?.build()?; let name_override = Arc::clone(&tls_name_override); - let well_known_client = reqwest_client_builder(&config)? + let federation_client = reqwest_client_builder(&config)? .resolve_fn(move |domain| { let read_guard = name_override.read().unwrap(); let (override_name, port) = read_guard.get(&domain)?; @@ -154,8 +154,8 @@ impl Globals { })?, actual_destination_cache: Arc::new(RwLock::new(WellKnownMap::new())), tls_name_override, - well_known_client, - basic_client, + federation_client, + default_client, server_signingkeys, jwt_decoding_key, bad_event_ratelimiter: Arc::new(RwLock::new(HashMap::new())), @@ -179,15 +179,15 @@ impl Globals { } /// Returns a reqwest client which can be used to send requests - pub fn reqwest_client(&self) -> reqwest::Client { - // can't return &Client or else we'll hold a lock around the DB across an await - self.basic_client.clone() + pub fn default_client(&self) -> reqwest::Client { + // Client is cheap to clone (Arc wrapper) and avoids lifetime issues + self.default_client.clone() } /// Returns a client used for resolving .well-knowns - pub fn well_known_client(&self) -> reqwest::Client { - // can't return &Client or else we'll hold a lock around the DB across an await - self.well_known_client.clone() + pub fn federation_client(&self) -> reqwest::Client { + // Client is cheap to clone (Arc wrapper) and avoids lifetime issues + self.federation_client.clone() } #[tracing::instrument(skip(self))] diff --git a/src/database/pusher.rs b/src/database/pusher.rs index d63db1d7..bbe85a8d 100644 --- a/src/database/pusher.rs +++ b/src/database/pusher.rs @@ -115,7 +115,7 @@ where //*reqwest_request.timeout_mut() = Some(Duration::from_secs(5)); let url = reqwest_request.url().clone(); - let response = globals.reqwest_client().execute(reqwest_request).await; + let response = globals.default_client().execute(reqwest_request).await; match response { Ok(mut response) => { diff --git a/src/server_server.rs b/src/server_server.rs index 978eb67f..c5e0b1a9 100644 --- a/src/server_server.rs +++ b/src/server_server.rs @@ -237,7 +237,7 @@ where let url = reqwest_request.url().clone(); - let response = globals.well_known_client().execute(reqwest_request).await; + let response = globals.federation_client().execute(reqwest_request).await; match response { Ok(mut response) => { @@ -477,7 +477,7 @@ async fn request_well_known( ) -> Option { let body: serde_json::Value = serde_json::from_str( &globals - .reqwest_client() + .default_client() .get(&format!( "https://{}/.well-known/matrix/server", destination