diff --git a/src/client/http/service.rs b/src/client/http/service.rs index 995a5e821..d6fb8f902 100644 --- a/src/client/http/service.rs +++ b/src/client/http/service.rs @@ -4,7 +4,11 @@ use std::{ }; use futures_util::future::{self, Either, MapErr, Ready, TryFutureExt}; -use http::{HeaderMap, Request, Response, header::PROXY_AUTHORIZATION, uri::Scheme}; +use http::{ + HeaderMap, Request, Response, + header::{Entry, PROXY_AUTHORIZATION}, + uri::Scheme, +}; use tower::Service; use super::{Body, connect::Connector}; @@ -38,7 +42,7 @@ struct Config { } impl ClientService { - /// Creates a new [`ClientService`] with the provided HTTP client and configuration. + /// Creates a new [`ClientService`]. pub(super) fn new( client: HttpClient, headers: HeaderMap, @@ -132,33 +136,38 @@ impl Service> for ClientService { fn call(&mut self, mut req: Request) -> Self::Future { let scheme = req.uri().scheme(); - // Validate scheme (http/https), enforce https if required + // check if the request URI scheme is valid. if (scheme != Some(&Scheme::HTTP) && scheme != Some(&Scheme::HTTPS)) || (self.config.https_only && scheme != Some(&Scheme::HTTPS)) { - return Either::Right(future::err(crate::Error::url_bad_scheme2().into())); + return Either::Right(future::err(BoxError::from(crate::Error::url_bad_scheme()))); } - // Optionally insert default headers - let skip = self + // insert default headers in the request headers + // without overwriting already appended headers. + if self .config .skip_default_headers .fetch(req.extensions()) .copied() - == Some(true); - - if !skip { + != Some(true) + { let headers = req.headers_mut(); - for name in self.config.headers.keys() { - if !headers.contains_key(name) { - for value in self.config.headers.get_all(name) { - headers.append(name, value.clone()); + for (name, value) in &self.config.headers { + match headers.entry(name) { + // If the header already exists but has a different value, + // append the new value to the existing one. + Entry::Occupied(mut entry) => { + entry.append(value.clone()); + } + // If the header does not exist, insert it. + Entry::Vacant(entry) => { + entry.insert(value.clone()); } } } } - // Apply original headers and proxy headers self.config.orig_headers.store(req.extensions_mut()); self.ensure_proxy_headers(&mut req); diff --git a/src/client/layer/redirect/future.rs b/src/client/layer/redirect/future.rs index ac221095b..519379d50 100644 --- a/src/client/layer/redirect/future.rs +++ b/src/client/layer/redirect/future.rs @@ -40,7 +40,7 @@ pin_project! { body: BodyRepr, }, - NoRedirect { + Direct { #[pin] future: S::Future, }, @@ -144,7 +144,7 @@ where Action::Stop => Poll::Ready(Ok(res)), } } - ResponseFutureProj::NoRedirect { mut future } => { + ResponseFutureProj::Direct { mut future } => { let res = ready!(future.as_mut().poll(cx)?); Poll::Ready(Ok(res)) } diff --git a/src/client/layer/redirect/mod.rs b/src/client/layer/redirect/mod.rs index ee2c3fcaf..f6dc4fa2d 100644 --- a/src/client/layer/redirect/mod.rs +++ b/src/client/layer/redirect/mod.rs @@ -76,7 +76,7 @@ where let service = self.inner.clone(); let mut service = mem::replace(&mut self.inner, service); let mut policy = self.policy.clone(); - policy.load(&req); + policy.on_extensions(req.extensions()); if policy.allowed() { let mut body = BodyRepr::None; @@ -94,7 +94,7 @@ where policy, } } else { - ResponseFuture::NoRedirect { + ResponseFuture::Direct { future: service.call(req), } } diff --git a/src/client/layer/redirect/policy.rs b/src/client/layer/redirect/policy.rs index 5c04f993e..3556b5b0a 100644 --- a/src/client/layer/redirect/policy.rs +++ b/src/client/layer/redirect/policy.rs @@ -1,6 +1,6 @@ //! Tools for customizing the behavior of a [`FollowRedirect`][super::FollowRedirect] middleware. -use http::{Request, StatusCode, Uri}; +use http::{Extensions, Request, StatusCode, Uri}; /// Trait for the policy on handling redirection responses. pub trait Policy { @@ -21,11 +21,14 @@ pub trait Policy { /// Loads redirect policy configuration from the request's [`Extensions`]. /// - /// This is typically used to extract request-specific redirect settings (e.g., max redirect - /// count, HTTPS-only rules) that override global client configuration. + /// This method is called once at the beginning of request processing to extract + /// request-specific redirect settings that may override the policy's default behavior. + /// Examples include per-request maximum redirect limits, allowed/blocked domains, + /// or security policies. /// - /// This method is called before any redirection decisions are made. - fn load(&mut self, _request: &Request); + /// The default implementation does nothing, meaning the policy uses its default + /// configuration for all requests. + fn on_extensions(&mut self, _extensions: &Extensions); /// Returns whether redirection is currently permitted by this policy. /// @@ -63,8 +66,8 @@ where } #[inline(always)] - fn load(&mut self, request: &Request) { - (**self).load(request) + fn on_extensions(&mut self, extensions: &Extensions) { + (**self).on_extensions(extensions) } #[inline(always)] diff --git a/src/client/layer/timeout/future.rs b/src/client/layer/timeout/future.rs index dd9babc1c..eef248f3b 100644 --- a/src/client/layer/timeout/future.rs +++ b/src/client/layer/timeout/future.rs @@ -84,10 +84,8 @@ where fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { let total_timeout = self.total_timeout; let read_timeout = self.read_timeout; - let this = self.project(); - let res = ready!(this.inner.poll(cx))?; - Poll::Ready(Ok( - res.map(|body| TimeoutBody::new(total_timeout, read_timeout, body)) - )) + let res = ready!(self.project().inner.poll(cx))? + .map(|body| TimeoutBody::new(total_timeout, read_timeout, body)); + Poll::Ready(Ok(res)) } } diff --git a/src/client/layer/timeout/layer.rs b/src/client/layer/timeout/layer.rs index 1368bfc5a..fa698ca82 100644 --- a/src/client/layer/timeout/layer.rs +++ b/src/client/layer/timeout/layer.rs @@ -66,13 +66,10 @@ where fn call(&mut self, req: Request) -> Self::Future { let (total_timeout, read_timeout) = resolve_timeout_config(&self.timeout, req.extensions()); - let total_timeout = total_timeout.map(tokio::time::sleep); - let read_timeout = read_timeout.map(tokio::time::sleep); - ResponseFuture { response: self.inner.call(req), - total_timeout, - read_timeout, + total_timeout: total_timeout.map(tokio::time::sleep), + read_timeout: read_timeout.map(tokio::time::sleep), } } } diff --git a/src/client/ws/mod.rs b/src/client/ws/mod.rs index 7f86941f6..0277ecbff 100644 --- a/src/client/ws/mod.rs +++ b/src/client/ws/mod.rs @@ -306,15 +306,17 @@ impl WebSocketRequestBuilder { "ws" => Scheme::HTTP, "wss" => Scheme::HTTPS, _ => { - return Err(Error::url_bad_scheme(url.clone())); + return Err(Error::url_bad_scheme().with_url(url.clone())); } }; // Update the scheme url.set_scheme(new_scheme.as_str()) - .map_err(|_| Error::url_bad_scheme(url.clone()))?; + .map_err(|_| Error::url_bad_scheme().with_url(url.clone()))?; // Get the version of the request + // This is used to determine if we should use HTTP/1.1 or HTTP/2 + // for the websocket handshake. let version = request.version(); // Set the headers for the websocket handshake @@ -351,8 +353,10 @@ impl WebSocketRequestBuilder { .insert(Protocol::from_static("websocket")); None } - _ => { - return Err(Error::upgrade(format!("unsupported version: {version:?}"))); + unsupported => { + return Err(Error::upgrade(format!( + "unsupported version: {unsupported:?}" + ))); } }; diff --git a/src/error.rs b/src/error.rs index 676ab5082..88f0103bf 100644 --- a/src/error.rs +++ b/src/error.rs @@ -74,11 +74,7 @@ impl Error { Error::new(Kind::Status(status, reason), None::).with_url(url) } - pub(crate) fn url_bad_scheme(url: Url) -> Error { - Error::new(Kind::Builder, Some(BadScheme)).with_url(url) - } - - pub(crate) fn url_bad_scheme2() -> Error { + pub(crate) fn url_bad_scheme() -> Error { Error::new(Kind::Builder, Some(BadScheme)) } } @@ -340,14 +336,14 @@ impl StdError for Error { pub(crate) enum Kind { Builder, Request, + Tls, Redirect, Status(StatusCode, Option), Body, - Tls, Decode, + Upgrade, #[cfg(feature = "ws")] WebSocket, - Upgrade, } #[derive(Debug)] diff --git a/src/into_url.rs b/src/into_url.rs index 1e851ab0c..99810cbbb 100644 --- a/src/into_url.rs +++ b/src/into_url.rs @@ -27,7 +27,7 @@ impl IntoUrlSealed for Url { if self.has_host() { Ok(self) } else { - Err(Error::url_bad_scheme(self)) + Err(Error::url_bad_scheme().with_url(self)) } } @@ -41,7 +41,7 @@ impl IntoUrlSealed for &Url { if self.has_host() { Ok(self.clone()) } else { - Err(Error::url_bad_scheme(self.clone())) + Err(Error::url_bad_scheme().with_url(self.clone())) } } diff --git a/src/redirect.rs b/src/redirect.rs index 516fd5719..3ab2f1e53 100644 --- a/src/redirect.rs +++ b/src/redirect.rs @@ -6,7 +6,7 @@ use std::{error::Error as StdError, fmt, sync::Arc}; -use http::{HeaderMap, HeaderValue, StatusCode}; +use http::{Extensions, HeaderMap, HeaderValue, StatusCode, uri::Scheme}; use crate::{ Url, @@ -290,7 +290,7 @@ impl FollowRedirectPolicy { } fn make_referer(next: &Url, previous: &Url) -> Option { - if next.scheme() == "http" && previous.scheme() == "https" { + if Scheme::HTTP.eq(next.scheme()) && Scheme::HTTPS.eq(previous.scheme()) { return None; } @@ -314,18 +314,20 @@ impl policy::Policy for FollowRedirectPolicy { let policy = self .policy .as_ref() - .ok_or_else(|| Error::request("RequestRedirectPolicy not set in request config"))?; + .expect("FollowRedirectPolicy should always have a policy set"); // Check if the next URL is already in the list of URLs. match policy.check(attempt.status(), &next_url, &self.urls) { ActionKind::Follow => { - if next_url.scheme() != "http" && next_url.scheme() != "https" { - return Err(BoxError::from(Error::url_bad_scheme(next_url))); + // Validate the next URL's scheme. + if Scheme::HTTP.ne(next_url.scheme()) && Scheme::HTTPS.ne(next_url.scheme()) { + return Err(BoxError::from(Error::url_bad_scheme().with_url(next_url))); } - if self.https_only && next_url.scheme() != "https" { + // Validate HTTPS-only policy. + if self.https_only && Scheme::HTTPS.ne(next_url.scheme()) { return Err(BoxError::from(Error::redirect( - Error::url_bad_scheme(next_url.clone()), + Error::url_bad_scheme().with_url(next_url.clone()), next_url, ))); } @@ -351,8 +353,8 @@ impl policy::Policy for FollowRedirectPolicy { } #[inline(always)] - fn load(&mut self, req: &http::Request) { - self.policy.load(req.extensions()); + fn on_extensions(&mut self, extensions: &Extensions) { + self.policy.load(extensions); } #[inline(always)]