Skip to content

Commit

Permalink
Refactor Redirect API (#741)
Browse files Browse the repository at this point in the history
Changed the redirect types to be from the `redirect` module:

- `reqwest::RedirectPolicy` is now `reqwest::redirect::Policy`
- `reqwest::RedirectAttempt` is now `reqwest::redirect::Attempt`
- `reqwest::RedirectAction` is now `reqwest::redirect::Action`

Changed behavior of default policy to no longer check for redirect loops
(loops should still be caught eventually by the maximum limit).

Removed the `too_many_redirects` and `loop_detected` methods from
`Action`.

Added `error` to `Action` that can be passed any error type.

Closes #717
  • Loading branch information
seanmonstar committed Dec 16, 2019
1 parent 382f1c0 commit ce43f80
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 144 deletions.
22 changes: 10 additions & 12 deletions src/async_impl/client.rs
Expand Up @@ -29,7 +29,7 @@ use crate::connect::Connector;
#[cfg(feature = "cookies")]
use crate::cookie;
use crate::into_url::{expect_uri, try_uri};
use crate::redirect::{self, remove_sensitive_headers, RedirectPolicy};
use crate::redirect::{self, remove_sensitive_headers};
#[cfg(feature = "tls")]
use crate::tls::TlsBackend;
#[cfg(feature = "tls")]
Expand Down Expand Up @@ -70,7 +70,7 @@ struct Config {
identity: Option<Identity>,
proxies: Vec<Proxy>,
auto_sys_proxy: bool,
redirect_policy: RedirectPolicy,
redirect_policy: redirect::Policy,
referer: bool,
timeout: Option<Duration>,
#[cfg(feature = "tls")]
Expand Down Expand Up @@ -114,7 +114,7 @@ impl ClientBuilder {
max_idle_per_host: std::usize::MAX,
proxies: Vec::new(),
auto_sys_proxy: true,
redirect_policy: RedirectPolicy::default(),
redirect_policy: redirect::Policy::default(),
referer: true,
timeout: None,
#[cfg(feature = "tls")]
Expand Down Expand Up @@ -372,7 +372,7 @@ impl ClientBuilder {
/// Set a `RedirectPolicy` for this client.
///
/// Default will follow redirects up to a maximum of 10.
pub fn redirect(mut self, policy: RedirectPolicy) -> ClientBuilder {
pub fn redirect(mut self, policy: redirect::Policy) -> ClientBuilder {
self.config.redirect_policy = policy;
self
}
Expand Down Expand Up @@ -915,7 +915,7 @@ struct ClientRef {
gzip: bool,
headers: HeaderMap,
hyper: HyperClient,
redirect_policy: RedirectPolicy,
redirect_policy: redirect::Policy,
referer: bool,
request_timeout: Option<Duration>,
proxies: Arc<Vec<Proxy>>,
Expand Down Expand Up @@ -1124,7 +1124,7 @@ impl Future for PendingRequest {
.check(res.status(), &loc, &self.urls);

match action {
redirect::Action::Follow => {
redirect::ActionKind::Follow => {
self.url = loc;

let mut headers =
Expand Down Expand Up @@ -1159,14 +1159,12 @@ impl Future for PendingRequest {
*self.as_mut().in_flight().get_mut() = self.client.hyper.request(req);
continue;
}
redirect::Action::Stop => {
redirect::ActionKind::Stop => {
debug!("redirect_policy disallowed redirection to '{}'", loc);
}
redirect::Action::LoopDetected => {
return Poll::Ready(Err(crate::error::loop_detected(self.url.clone())));
}
redirect::Action::TooManyRedirects => {
return Poll::Ready(Err(crate::error::too_many_redirects(
redirect::ActionKind::Error(err) => {
return Poll::Ready(Err(crate::error::redirect(
err,
self.url.clone(),
)));
}
Expand Down
8 changes: 4 additions & 4 deletions src/blocking/client.rs
Expand Up @@ -13,7 +13,7 @@ use log::{error, trace};
use super::request::{Request, RequestBuilder};
use super::response::Response;
use super::wait;
use crate::{async_impl, header, IntoUrl, Method, Proxy, RedirectPolicy};
use crate::{async_impl, header, IntoUrl, Method, Proxy, redirect};
#[cfg(feature = "tls")]
use crate::{Certificate, Identity};

Expand Down Expand Up @@ -176,10 +176,10 @@ impl ClientBuilder {

// Redirect options

/// Set a `RedirectPolicy` for this client.
/// Set a `redirect::Policy` for this client.
///
/// Default will follow redirects up to a maximum of 10.
pub fn redirect(self, policy: RedirectPolicy) -> ClientBuilder {
pub fn redirect(self, policy: redirect::Policy) -> ClientBuilder {
self.with_inner(move |inner| inner.redirect(policy))
}

Expand Down Expand Up @@ -541,7 +541,7 @@ impl Client {
/// # Errors
///
/// This method fails if there was an error while sending request,
/// redirect loop was detected or redirect limit was exhausted.
/// or redirect limit was exhausted.
pub fn execute(&self, request: Request) -> crate::Result<Response> {
self.inner.execute_request(request)
}
Expand Down
8 changes: 2 additions & 6 deletions src/error.rs
Expand Up @@ -212,12 +212,8 @@ pub(crate) fn request<E: Into<BoxError>>(e: E) -> Error {
Error::new(Kind::Request, Some(e))
}

pub(crate) fn loop_detected(url: Url) -> Error {
Error::new(Kind::Redirect, Some("infinite redirect loop detected")).with_url(url)
}

pub(crate) fn too_many_redirects(url: Url) -> Error {
Error::new(Kind::Redirect, Some("too many redirects")).with_url(url)
pub(crate) fn redirect<E: Into<BoxError>>(e: E, url: Url) -> Error {
Error::new(Kind::Redirect, Some(e)).with_url(url)
}

pub(crate) fn status_code(url: Url, status: StatusCode) -> Error {
Expand Down
20 changes: 5 additions & 15 deletions src/lib.rs
Expand Up @@ -119,9 +119,9 @@
//!
//! ## Redirect Policies
//!
//! By default, a `Client` will automatically handle HTTP redirects, detecting
//! loops, and having a maximum redirect chain of 10 hops. To customize this
//! behavior, a [`RedirectPolicy`][redirect] can used with a `ClientBuilder`.
//! By default, a `Client` will automatically handle HTTP redirects, having a
//! maximum redirect chain of 10 hops. To customize this behavior, a
//! [`redirect::Policy`][redirect] can be used with a `ClientBuilder`.
//!
//! ## Cookies
//!
Expand Down Expand Up @@ -175,7 +175,7 @@
//! [get]: ./fn.get.html
//! [builder]: ./struct.RequestBuilder.html
//! [serde]: http://serde.rs
//! [redirect]: ./struct.RedirectPolicy.html
//! [redirect]: crate::redirect
//! [Proxy]: ./struct.Proxy.html
//! [cargo-features]: https://doc.rust-lang.org/stable/cargo/reference/manifest.html#the-features-section

Expand Down Expand Up @@ -237,7 +237,6 @@ pub use self::into_url::IntoUrl;
/// - native TLS backend cannot be initialized
/// - supplied `Url` cannot be parsed
/// - there was an error while sending request
/// - redirect loop was detected
/// - redirect limit was exhausted
pub async fn get<T: IntoUrl>(url: T) -> crate::Result<Response> {
Client::builder().build()?.get(url).send().await
Expand Down Expand Up @@ -279,7 +278,6 @@ if_hyper! {
multipart, Body, Client, ClientBuilder, Request, RequestBuilder, Response, ResponseBuilderExt,
};
pub use self::proxy::Proxy;
pub use self::redirect::{RedirectAction, RedirectAttempt, RedirectPolicy};
#[cfg(feature = "tls")]
pub use self::tls::{Certificate, Identity};

Expand All @@ -293,17 +291,9 @@ if_hyper! {
//#[cfg(feature = "trust-dns")]
//mod dns;
mod proxy;
mod redirect;
pub mod redirect;
#[cfg(feature = "tls")]
mod tls;

#[doc(hidden)]
#[deprecated(note = "types moved to top of crate")]
pub mod r#async {
pub use crate::async_impl::{
multipart, Body, Client, ClientBuilder, Request, RequestBuilder, Response,
};
}
}

if_wasm! {
Expand Down

0 comments on commit ce43f80

Please sign in to comment.