diff --git a/src/client.rs b/src/client.rs index 4abe25d8..3fb2d551 100644 --- a/src/client.rs +++ b/src/client.rs @@ -114,9 +114,7 @@ type MaybeDecompressionBody = tower_http::decompression::DecompressionBody type ClientService = Timeout< ConfigService< - MaybeDecompression< - Retry, FollowRedirectPolicy>>, - >, + MaybeDecompression>>>, >, >; diff --git a/src/client/layer/redirect.rs b/src/client/layer/redirect.rs index 3bf2d646..3e8e0ce2 100644 --- a/src/client/layer/redirect.rs +++ b/src/client/layer/redirect.rs @@ -10,11 +10,12 @@ use std::{ use futures_util::future::Either; use http::{Request, Response}; -use http_body::Body; +use http_body::Body as HttpBody; use tower::{BoxError, Layer, Service}; use self::future::ResponseFuture; -pub use self::policy::{Action, Attempt, Policy}; +pub use self::policy::{Action, Attempt}; +use crate::{client::body::Body, redirect::FollowRedirectPolicy}; enum BodyRepr { Some(B), @@ -22,31 +23,25 @@ enum BodyRepr { None, } -impl BodyRepr -where - B: Body + Default, -{ - fn take(&mut self) -> Option { +impl BodyRepr { + fn take(&mut self) -> Option { match mem::replace(self, BodyRepr::None) { BodyRepr::Some(body) => Some(body), BodyRepr::Empty => { *self = BodyRepr::Empty; - Some(B::default()) + Some(Body::default()) } BodyRepr::None => None, } } - fn try_clone_from(&mut self, body: &B, policy: &P) - where - P: Policy, - { + fn try_clone_from(&mut self, body: &Body) { match self { BodyRepr::Some(_) | BodyRepr::Empty => {} BodyRepr::None => { if body.size_hint().exact() == Some(0) { - *self = BodyRepr::Some(B::default()); - } else if let Some(cloned) = policy.clone_body(body) { + *self = BodyRepr::Some(Body::default()); + } else if let Some(cloned) = body.try_clone() { *self = BodyRepr::Some(cloned); } } @@ -55,25 +50,24 @@ where } /// [`Layer`] for retrying requests with a [`Service`] to follow redirection responses. -#[derive(Clone, Copy, Default)] -pub struct FollowRedirectLayer

{ - policy: P, +#[derive(Clone)] +pub struct FollowRedirectLayer { + policy: FollowRedirectPolicy, } -impl

FollowRedirectLayer

{ - /// Create a new [`FollowRedirectLayer`] with the given redirection [`Policy`]. +impl FollowRedirectLayer { + /// Create a new [`FollowRedirectLayer`] with the given redirection policy. #[inline(always)] - pub fn with_policy(policy: P) -> Self { + pub(crate) fn with_policy(policy: FollowRedirectPolicy) -> Self { FollowRedirectLayer { policy } } } -impl Layer for FollowRedirectLayer

+impl Layer for FollowRedirectLayer where S: Clone, - P: Clone, { - type Service = FollowRedirect; + type Service = FollowRedirect; #[inline(always)] fn layer(&self, inner: S) -> Self::Service { @@ -82,63 +76,57 @@ where } /// Middleware that retries requests with a [`Service`] to follow redirection responses. -#[derive(Clone, Copy)] -pub struct FollowRedirect { +#[derive(Clone)] +pub struct FollowRedirect { inner: S, - policy: P, + policy: FollowRedirectPolicy, } -impl FollowRedirect -where - P: Clone, -{ - /// Create a new [`FollowRedirect`] with the given redirection [`Policy`]. +impl FollowRedirect { + /// Create a new [`FollowRedirect`] with the given redirection policy. #[inline(always)] - pub fn with_policy(inner: S, policy: P) -> Self { + fn with_policy(inner: S, policy: FollowRedirectPolicy) -> Self { FollowRedirect { inner, policy } } } -impl Service> for FollowRedirect +impl Service> for FollowRedirect where - S: Service, Response = Response> + Clone, + S: Service, Response = Response> + Clone, S::Error: From, - P: Policy + Clone, - ReqBody: Body + Default, { type Response = Response; type Error = S::Error; - type Future = ResponseFuture; + type Future = ResponseFuture; #[inline(always)] fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { self.inner.poll_ready(cx) } - fn call(&mut self, mut req: Request) -> Self::Future { - if self.policy.follow_redirects(&mut req) { - let service = self.inner.clone(); - let mut service = mem::replace(&mut self.inner, service); - let mut policy = self.policy.clone(); - - let mut body_repr = BodyRepr::None; - body_repr.try_clone_from(req.body(), &policy); - policy.on_request(&mut req); - - let (parts, body) = req.into_parts(); - let req = Request::from_parts(parts.clone(), body); - ResponseFuture::Redirect { - future: Either::Left(service.call(req)), - pending_future: None, - service, - policy, - parts, - body_repr, - } - } else { - ResponseFuture::Direct { + fn call(&mut self, mut req: Request) -> Self::Future { + let Some(mut policy) = self.policy.for_request(&mut req) else { + return ResponseFuture::Direct { future: self.inner.call(req), - } + }; + }; + + let service = self.inner.clone(); + let mut service = mem::replace(&mut self.inner, service); + + let mut body_repr = BodyRepr::None; + body_repr.try_clone_from(req.body()); + policy.on_request(&mut req); + + let (parts, body) = req.into_parts(); + let req = Request::from_parts(parts.clone(), body); + ResponseFuture::Redirect { + future: Either::Left(service.call(req)), + pending_future: None, + service, + policy, + parts, + body_repr, } } } diff --git a/src/client/layer/redirect/future.rs b/src/client/layer/redirect/future.rs index 59543885..402b6d56 100644 --- a/src/client/layer/redirect/future.rs +++ b/src/client/layer/redirect/future.rs @@ -11,40 +11,39 @@ use http::{ header::{CONTENT_ENCODING, CONTENT_LENGTH, CONTENT_TYPE, LOCATION, TRANSFER_ENCODING}, request::Parts, }; -use http_body::Body; use pin_project_lite::pin_project; use tower::{BoxError, Service, util::Oneshot}; use url::Url; use super::{ BodyRepr, - policy::{Action, Attempt, Policy}, + policy::{Action, Attempt}, }; -use crate::{Error, ext::RequestUri, into_uri::IntoUriSealed}; +use crate::{Body, ext::RequestUri, into_uri::IntoUriSealed, redirect::FollowRedirectPolicy}; /// Pending future state for handling redirects. -pub struct Pending { +pub struct Pending { future: Pin + Send>>, location: Uri, - body: ReqBody, + body: Body, res: Response, } pin_project! { /// Response future for [`FollowRedirect`]. #[project = ResponseFutureProj] - pub enum ResponseFuture + pub enum ResponseFuture where - S: Service>, + S: Service>, { Redirect { #[pin] - future: Either>>, - pending_future: Option>, + future: Either>>, + pending_future: Option>, service: S, - policy: P, + policy: FollowRedirectPolicy, parts: Parts, - body_repr: BodyRepr, + body_repr: BodyRepr, }, Direct { @@ -54,14 +53,12 @@ pin_project! { } } -impl Future for ResponseFuture +impl Future for ResponseFuture where - S: Service, Response = Response> + Clone, + S: Service, Response = Response> + Clone, S::Error: From, - P: Policy, - ReqBody: Body + Default, { - type Output = Result, S::Error>; + type Output = Result, S::Error>; fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll { match self.project() { @@ -215,41 +212,36 @@ fn drop_payload_headers(headers: &mut HeaderMap) { } } -type RedirectFuturePin<'a, S, ReqBody> = - Pin<&'a mut Either<>>::Future, Oneshot>>>; +type RedirectFuturePin<'a, S> = + Pin<&'a mut Either<>>::Future, Oneshot>>>; -struct RedirectAction<'a, S, ReqBody, ResBody, P> +struct RedirectAction<'a, S, B> where - S: Service, Response = Response> + Clone, - P: Policy, + S: Service, Response = Response> + Clone, { action: Action, - future: &'a mut RedirectFuturePin<'a, S, ReqBody>, + future: &'a mut RedirectFuturePin<'a, S>, service: &'a S, - policy: &'a mut P, + policy: &'a mut FollowRedirectPolicy, parts: &'a mut Parts, - body: ReqBody, - body_repr: &'a mut BodyRepr, - res: Response, + body: Body, + body_repr: &'a mut BodyRepr, + res: Response, location: Uri, } -fn handle_action( +fn handle_action( cx: &mut Context<'_>, - redirect: RedirectAction<'_, S, ReqBody, ResBody, P>, -) -> Poll, S::Error>> + redirect: RedirectAction<'_, S, B>, +) -> Poll, S::Error>> where - S: Service, Response = Response> + Clone, + S: Service, Response = Response> + Clone, S::Error: From, - P: Policy, - ReqBody: Body + Default, { match redirect.action { Action::Follow => { redirect.parts.uri = redirect.location; - redirect - .body_repr - .try_clone_from(&redirect.body, redirect.policy); + redirect.body_repr.try_clone_from(&redirect.body); let mut req = Request::from_parts(redirect.parts.clone(), redirect.body); redirect.policy.on_request(&mut req); @@ -261,13 +253,7 @@ where Poll::Pending } Action::Stop => Poll::Ready(Ok(redirect.res)), - Action::Pending(_) => Poll::Ready(Err(S::Error::from( - Error::redirect( - "Nested pending Action is not supported in redirect policy", - redirect.parts.uri.clone(), - ) - .into(), - ))), Action::Error(err) => Poll::Ready(Err(err.into())), + Action::Pending(_) => unreachable!(), } } diff --git a/src/client/layer/redirect/policy.rs b/src/client/layer/redirect/policy.rs index 741beb8a..c0badb65 100644 --- a/src/client/layer/redirect/policy.rs +++ b/src/client/layer/redirect/policy.rs @@ -2,34 +2,10 @@ use std::{fmt, pin::Pin}; -use http::{HeaderMap, Request, Response, StatusCode, Uri}; +use http::{HeaderMap, StatusCode, Uri}; use crate::error::BoxError; -/// Trait for the policy on handling redirection responses. -pub trait Policy { - /// Invoked when the service received a response with a redirection status code (`3xx`). - /// - /// This method returns an [`Action`] which indicates whether the service should follow - /// the redirection. - fn redirect(&mut self, attempt: Attempt<'_>) -> Result; - - /// Returns whether redirection is currently permitted by this policy. - /// - /// This method is called to determine whether the client should follow redirects at all. - /// It allows policies to enable or disable redirection behavior based on the [`Request`]. - fn follow_redirects(&mut self, _request: &mut Request) -> bool; - - /// Invoked right before the service makes a [`Request`]. - fn on_request(&mut self, _request: &mut Request); - - /// Invoked right after the service received a [`Response`]. - fn on_response(&mut self, _response: &mut Response); - - /// Try to clone a request body before the service makes a redirected request. - fn clone_body(&self, _body: &B) -> Option; -} - /// A type that holds information on a redirection attempt. pub struct Attempt<'a> { pub(crate) status: StatusCode, @@ -38,7 +14,7 @@ pub struct Attempt<'a> { pub(crate) previous: &'a Uri, } -/// A value returned by [`Policy::redirect`] which indicates the action +/// A value which indicates the action /// [`FollowRedirect`][super::FollowRedirect] should take for a redirection response. pub enum Action { /// Follow the redirection. diff --git a/src/redirect.rs b/src/redirect.rs index 6362b712..7a0fb678 100644 --- a/src/redirect.rs +++ b/src/redirect.rs @@ -11,7 +11,7 @@ use futures_util::FutureExt; use http::{HeaderMap, HeaderName, HeaderValue, StatusCode, Uri}; use crate::{ - client::{body::Body, layer::redirect}, + client::layer::redirect, config::RequestConfig, error::{BoxError, Error}, ext::UriExt, @@ -52,9 +52,7 @@ pub struct Attempt<'a, const PENDING: bool = true> { /// An action to perform when a redirect status code is found. #[derive(Debug)] -pub struct Action { - inner: redirect::Action, -} +pub struct Action(redirect::Action); /// Redirect history information for a response. #[derive(Debug, Clone)] @@ -221,7 +219,7 @@ impl Policy { uri: Cow::Borrowed(next), previous: Cow::Borrowed(previous), }) - .inner + .0 } } @@ -241,9 +239,7 @@ impl Attempt<'_, PENDING> { /// Returns an action meaning wreq should follow the next URI. #[inline] pub fn follow(self) -> Action { - Action { - inner: redirect::Action::Follow, - } + Action(redirect::Action::Follow) } /// Returns an action meaning wreq should not follow the next URI. @@ -251,9 +247,7 @@ impl Attempt<'_, PENDING> { /// The 30x response will be returned as the `Ok` result. #[inline] pub fn stop(self) -> Action { - Action { - inner: redirect::Action::Stop, - } + Action(redirect::Action::Stop) } /// Returns an [`Action`] failing the redirect with an error. @@ -261,9 +255,7 @@ impl Attempt<'_, PENDING> { /// The [`Error`] will be returned for the result of the sent request. #[inline] pub fn error>(self, error: E) -> Action { - Action { - inner: redirect::Action::Error(error.into()), - } + Action(redirect::Action::Error(error.into())) } } @@ -300,10 +292,10 @@ impl Attempt<'_, true> { uri: Cow::Owned(self.uri.into_owned()), previous: Cow::Owned(self.previous.into_owned()), }; - let pending = Box::pin(func(attempt).map(|action| action.inner)); - Action { - inner: redirect::Action::Pending(pending), - } + + Action(redirect::Action::Pending(Box::pin( + func(attempt).map(|action| action.0), + ))) } } @@ -356,7 +348,7 @@ impl StdError for TooManyRedirects {} impl FollowRedirectPolicy { /// Creates a new redirect policy handler with the given [`Policy`]. pub fn new(policy: Policy) -> Self { - Self { + FollowRedirectPolicy { policy: RequestConfig::new(Some(policy)), referer: false, uris: Vec::new(), @@ -380,13 +372,14 @@ impl FollowRedirectPolicy { } } -impl redirect::Policy for FollowRedirectPolicy { - fn redirect(&mut self, attempt: redirect::Attempt<'_>) -> Result { +impl FollowRedirectPolicy { + pub(crate) fn redirect( + &mut self, + attempt: redirect::Attempt<'_>, + ) -> Result { // Parse the next URI from the attempt. let previous_uri = attempt.previous; let next_uri = attempt.location; - - // Push the previous URI to the list of URLs. self.uris.push(previous_uri.clone()); // Get policy from config @@ -394,49 +387,51 @@ impl redirect::Policy for FollowRedirectPolicy { .policy .as_ref() .expect("[BUG] FollowRedirectPolicy should always have a policy set"); + let action = policy.check(attempt.status, attempt.headers, next_uri, &self.uris); - // Check if the next URI is already in the list of URLs. - match policy.check(attempt.status, attempt.headers, next_uri, &self.uris) { - redirect::Action::Follow => { - // Validate the redirect URI scheme - if !(next_uri.is_http() || next_uri.is_https()) { - return Err(Error::uri_bad_scheme(next_uri.clone()).into()); - } + // Handle errors from the policy immediately + if let redirect::Action::Error(err) = action { + return Err(Error::redirect(err, previous_uri.clone()).into()); + } - // Check HTTPS-only policy - if self.https_only && !next_uri.is_https() { - return Err(Error::redirect( - Error::uri_bad_scheme(next_uri.clone()), - next_uri.clone(), - ) - .into()); - } + // Handle follow redirect action + if matches!(&action, redirect::Action::Follow) { + // Validate the redirect URI scheme + if !(next_uri.is_http() || next_uri.is_https()) { + return Err(Error::uri_bad_scheme(next_uri.clone()).into()); + } - // Record redirect history - if !matches!(policy.inner, PolicyKind::None) { - self.history.get_or_insert_default().push(HistoryEntry { - status: attempt.status, - uri: attempt.location.clone(), - previous: attempt.previous.clone(), - headers: attempt.headers.clone(), - }); - } + // Check HTTPS-only policy + if self.https_only && !next_uri.is_https() { + return Err(Error::redirect( + Error::uri_bad_scheme(next_uri.clone()), + next_uri.clone(), + ) + .into()); + } - Ok(redirect::Action::Follow) + // Record redirect history + if !matches!(policy.inner, PolicyKind::None) { + self.history.get_or_insert_default().push(HistoryEntry { + status: attempt.status, + uri: attempt.location.clone(), + previous: attempt.previous.clone(), + headers: attempt.headers.clone(), + }); } - redirect::Action::Stop => Ok(redirect::Action::Stop), - redirect::Action::Pending(task) => Ok(redirect::Action::Pending(task)), - redirect::Action::Error(err) => Err(Error::redirect(err, previous_uri.clone()).into()), } + + Ok(action) } - fn follow_redirects(&mut self, request: &mut http::Request) -> bool { + pub(crate) fn for_request(&mut self, request: &mut http::Request) -> Option { self.policy .load(request.extensions_mut()) .is_some_and(|policy| !matches!(policy.inner, PolicyKind::None)) + .then(|| self.clone()) } - fn on_request(&mut self, req: &mut http::Request) { + pub(crate) fn on_request(&mut self, req: &mut http::Request) { let next_url = req.uri().clone(); remove_sensitive_headers(req.headers_mut(), &next_url, &self.uris); if self.referer { @@ -448,16 +443,11 @@ impl redirect::Policy for FollowRedirectPolicy { } } - fn on_response(&mut self, response: &mut http::Response) { + pub(crate) fn on_response(&mut self, response: &mut http::Response) { if let Some(history) = self.history.take() { response.extensions_mut().insert(History(history)); } } - - #[inline] - fn clone_body(&self, body: &Body) -> Option { - body.try_clone() - } } fn make_referer(next: Uri, previous: &Uri) -> Option {