From e6b38acd0bb880836cc1ec61cbec17fe27b4a810 Mon Sep 17 00:00:00 2001 From: Martin Algesten Date: Wed, 12 Feb 2025 21:21:36 +0100 Subject: [PATCH] Do not pass state in Config --- CHANGELOG.md | 1 + src/agent.rs | 7 +------ src/config.rs | 9 --------- src/request.rs | 10 ++++++---- src/run.rs | 19 +++++++++++++------ src/tls/native_tls.rs | 12 +++++++----- src/tls/rustls.rs | 9 ++++----- src/unversioned/transport/mod.rs | 7 ++++++- 8 files changed, 38 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3360d2e..72a7bb2c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Unreleased + * Stop passing internal state in Config (#996) * Support request level TlsConfig (#996) # 3.0.5 diff --git a/src/agent.rs b/src/agent.rs index fd6ddd94..84aa1bb6 100644 --- a/src/agent.rs +++ b/src/agent.rs @@ -245,12 +245,7 @@ impl Agent { } pub(crate) fn new_request_level_config(&self) -> RequestLevelConfig { - let mut config = self.config.as_ref().clone(); - - // Set flag indicating this is request level. - config.request_level = true; - - RequestLevelConfig(config) + RequestLevelConfig(self.config.as_ref().clone()) } /// Make a GET request using this agent. diff --git a/src/config.rs b/src/config.rs index e01e76ca..415ff448 100644 --- a/src/config.rs +++ b/src/config.rs @@ -159,13 +159,6 @@ pub struct Config { // Chain built for middleware. pub(crate) middleware: MiddlewareChain, - - // Techically not config, but here to pass as argument from - // RequestBuilder::force_send_body() to run() - pub(crate) force_send_body: bool, - - // If this config instance is request level. - pub(crate) request_level: bool, } impl Config { @@ -852,8 +845,6 @@ impl Default for Config { max_idle_connections_per_host: 3, max_idle_age: Duration::from_secs(15), middleware: MiddlewareChain::default(), - force_send_body: false, - request_level: false, } } } diff --git a/src/request.rs b/src/request.rs index 26c66e9d..62f81c90 100644 --- a/src/request.rs +++ b/src/request.rs @@ -402,10 +402,9 @@ impl RequestBuilder { /// # Ok::<_, ureq::Error>(()) /// ``` pub fn force_send_body(mut self) -> RequestBuilder { - // This is how we communicate to run() that we want to disable - // the method-body-compliance check. - let config = self.request_level_config(); - config.force_send_body = true; + if let Some(exts) = self.extensions_mut() { + exts.insert(ForceSendBody); + } RequestBuilder { agent: self.agent, @@ -417,6 +416,9 @@ impl RequestBuilder { } } +#[derive(Debug, Clone)] +pub(crate) struct ForceSendBody; + impl RequestBuilder { pub(crate) fn new(agent: Agent, method: Method, uri: T) -> Self where diff --git a/src/run.rs b/src/run.rs index e4bf2439..4271c301 100644 --- a/src/run.rs +++ b/src/run.rs @@ -13,6 +13,7 @@ use crate::body::ResponseInfo; use crate::config::{Config, RequestLevelConfig, DEFAULT_USER_AGENT}; use crate::http; use crate::pool::Connection; +use crate::request::ForceSendBody; use crate::response::{RedirectHistory, ResponseUri}; use crate::timings::{CallTimings, CurrentTime}; use crate::transport::time::{Duration, Instant}; @@ -33,12 +34,13 @@ pub(crate) fn run( let mut redirect_count = 0; // Configuration on the request level overrides the agent level. - let config = request + let (config, request_level) = request .extensions_mut() .remove::() - .map(|rl| rl.0) - .map(Arc::new) - .unwrap_or_else(|| agent.config.clone()); + .map(|rl| (Arc::new(rl.0), true)) + .unwrap_or_else(|| (agent.config.clone(), false)); + + let force_send_body = request.extensions_mut().remove::().is_some(); let mut redirect_history: Option> = config.save_redirect_history().then_some(Vec::new()); @@ -49,7 +51,7 @@ pub(crate) fn run( let mut flow = Flow::new(request)?; - if config.force_send_body { + if force_send_body { flow.send_body_despite_method(); } @@ -66,6 +68,7 @@ pub(crate) fn run( match flow_run( agent, &config, + request_level, flow, &mut body, redirect_count, @@ -109,9 +112,11 @@ pub(crate) fn run( Ok(response) } +#[allow(clippy::too_many_arguments)] fn flow_run( agent: &Agent, config: &Config, + request_level: bool, mut flow: Flow, body: &mut SendBody, redirect_count: u32, @@ -127,7 +132,7 @@ fn flow_run( add_headers(&mut flow, agent, config, body, &uri)?; - let mut connection = connect(agent, config, &uri, timings)?; + let mut connection = connect(agent, config, request_level, &uri, timings)?; let mut flow = flow.proceed(); @@ -336,6 +341,7 @@ fn add_headers( fn connect( agent: &Agent, config: &Config, + request_level: bool, wanted_uri: &Uri, timings: &mut CallTimings, ) -> Result { @@ -363,6 +369,7 @@ fn connect( addrs, resolver: &*agent.resolver, config, + request_level, now: timings.now(), timeout: timings.next_timeout(Timeout::Connect), proxied, diff --git a/src/tls/native_tls.rs b/src/tls/native_tls.rs index 77aabc93..ebf1ebc9 100644 --- a/src/tls/native_tls.rs +++ b/src/tls/native_tls.rs @@ -3,7 +3,6 @@ use std::fmt; use std::io::{Read, Write}; use std::sync::{Arc, OnceLock}; -use crate::config::Config; use crate::tls::{RootCerts, TlsProvider}; use crate::{transport::*, Error}; use der::pem::LineEnding; @@ -52,7 +51,7 @@ impl Connector for NativeTlsConnector { trace!("Try wrap TLS"); - let connector = self.get_cached_native_tls_connector(details.config)?; + let connector = self.get_cached_native_tls_connector(details)?; let domain = details .uri @@ -78,10 +77,13 @@ impl Connector for NativeTlsConnector { } impl NativeTlsConnector { - fn get_cached_native_tls_connector(&self, config: &Config) -> Result, Error> { - let tls_config = config.tls_config(); + fn get_cached_native_tls_connector( + &self, + details: &ConnectionDetails, + ) -> Result, Error> { + let tls_config = details.config.tls_config(); - let connector = if config.request_level { + let connector = if details.request_level { // If the TlsConfig is request level, it is not allowed to // initialize the self.config OnceLock, but it should // reuse the cached value if it is the same TlsConfig diff --git a/src/tls/rustls.rs b/src/tls/rustls.rs index b8521621..61151307 100644 --- a/src/tls/rustls.rs +++ b/src/tls/rustls.rs @@ -9,7 +9,6 @@ use rustls::{ClientConfig, ClientConnection, RootCertStore, StreamOwned, ALL_VER use rustls_pki_types::{CertificateDer, PrivateKeyDer, PrivatePkcs1KeyDer, PrivatePkcs8KeyDer}; use rustls_pki_types::{PrivateSec1KeyDer, ServerName}; -use crate::config::Config; use crate::tls::cert::KeyKind; use crate::tls::{RootCerts, TlsProvider}; use crate::transport::{Buffers, ConnectionDetails, Connector, LazyBuffers}; @@ -57,7 +56,7 @@ impl Connector for RustlsConnector { trace!("Try wrap in TLS"); - let config = self.get_cached_config(details.config); + let config = self.get_cached_config(details); let name_borrowed: ServerName<'_> = details .uri @@ -92,10 +91,10 @@ impl Connector for RustlsConnector { } impl RustlsConnector { - fn get_cached_config(&self, config: &Config) -> Arc { - let tls_config = config.tls_config(); + fn get_cached_config(&self, details: &ConnectionDetails) -> Arc { + let tls_config = details.config.tls_config(); - if config.request_level { + if details.request_level { // If the TlsConfig is request level, it is not allowed to // initialize the self.config OnceLock, but it should // reuse the cached value if it is the same TlsConfig diff --git a/src/unversioned/transport/mod.rs b/src/unversioned/transport/mod.rs index 96930cce..c0a42f28 100644 --- a/src/unversioned/transport/mod.rs +++ b/src/unversioned/transport/mod.rs @@ -183,9 +183,14 @@ pub struct ConnectionDetails<'a> { /// For CONNECT proxy, this is the address of the proxy server. pub addrs: ResolvedSocketAddrs, - /// The Agent configuration. + /// The configuration. + /// + /// Agent or Request level. pub config: &'a Config, + /// Whether the config is request level. + pub request_level: bool, + /// The resolver configured on [`Agent`](crate::Agent). /// /// Typically the IP address of the host in the uri is already resolved to the `addr`