Skip to content

Commit

Permalink
Config::max_redirects_do_error()
Browse files Browse the repository at this point in the history
  • Loading branch information
algesten committed Jan 1, 2025
1 parent a2b050b commit 1e3f7bc
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 9 deletions.
45 changes: 43 additions & 2 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ pub struct Config {
proxy: Option<Proxy>,
no_delay: bool,
max_redirects: u32,
max_redirects_will_error: bool,
redirect_auth_headers: RedirectAuthHeaders,
user_agent: AutoHeaderValue,
accept: AutoHeaderValue,
Expand Down Expand Up @@ -187,6 +188,10 @@ impl Config {

Some(proxy.uri())
}

pub(crate) fn max_redirects_do_error(&self) -> bool {
self.max_redirects > 0 && self.max_redirects_will_error
}
}

impl Config {
Expand Down Expand Up @@ -238,13 +243,30 @@ impl Config {
self.no_delay
}

/// The max number of redirects to follow before giving up
/// The max number of redirects to follow before giving up.
///
/// Whe max redirects are reached, the behavior is controlled by the
/// `max_redirects_will_error` setting. Set to `true` (which
/// is the default) the result is a `TooManyRedirects` error. Set
/// to `false`, the response is returned as is.
///
/// If `max_redirects` is 0, no redirects are followed and the response
/// is always returned (never a `TooManyRedirects` error).
///
/// Defaults to 10
pub fn max_redirects(&self) -> u32 {
self.max_redirects
}

/// If we should error when max redirects are reached.
///
/// This has no meaning if `max_redirects` is 0.
///
/// Defaults to true
pub fn max_redirects_will_error(&self) -> bool {
self.max_redirects_will_error
}

/// How to handle `Authorization` headers when following redirects
///
/// * `Never` (the default) means the authorization header is never attached to a redirected call.
Expand Down Expand Up @@ -423,14 +445,32 @@ impl<Scope: private::ConfigScope> ConfigBuilder<Scope> {
self
}

/// The max number of redirects to follow before giving up
/// The max number of redirects to follow before giving up.
///
/// Whe max redirects are reached, the behavior is controlled by the
/// `max_redirects_will_error` setting. Set to `true` (which
/// is the default) the result is a `TooManyRedirects` error. Set
/// to `false`, the response is returned as is.
///
/// If `max_redirects` is 0, no redirects are followed and the response
/// is always returned (never a `TooManyRedirects` error).
///
/// Defaults to 10
pub fn max_redirects(mut self, v: u32) -> Self {
self.config().max_redirects = v;
self
}

/// If we should error when max redirects are reached.
///
/// This has no meaning if `max_redirects` is 0.
///
/// Defaults to true
pub fn max_redirects_will_error(mut self, v: bool) -> Self {
self.config().max_redirects_will_error = v;
self
}

/// How to handle `Authorization` headers when following redirects
///
/// * `Never` (the default) means the authorization header is never attached to a redirected call.
Expand Down Expand Up @@ -767,6 +807,7 @@ impl Default for Config {
proxy: Proxy::try_from_env(),
no_delay: true,
max_redirects: 10,
max_redirects_will_error: true,
redirect_auth_headers: RedirectAuthHeaders::Never,
user_agent: AutoHeaderValue::default(),
accept: AutoHeaderValue::default(),
Expand Down
11 changes: 11 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,16 @@ pub enum Error {
/// A send body (Such as `&str`) is larger than the `content-length` header.
BodyExceedsLimit(u64),

/// Too many redirects.
///
/// The error can be turned off by setting
/// [`max_redirects_will_error()`](crate::config::ConfigBuilder::max_redirects_will_error)
/// to false. When turned off, the last response will be returned instead of causing
/// an error, even if it is a redirect.
///
/// The number of redirects is limited to 10 by default.
TooManyRedirects,

/// Some error with TLS.
#[cfg(feature = "_tls")]
Tls(&'static str),
Expand Down Expand Up @@ -190,6 +200,7 @@ impl fmt::Display for Error {
Error::BodyExceedsLimit(v) => {
write!(f, "the response body is larger than request limit: {}", v)
}
Error::TooManyRedirects => write!(f, "too many redirects"),
#[cfg(feature = "_tls")]
Error::Tls(v) => write!(f, "{}", v),
#[cfg(feature = "_tls")]
Expand Down
32 changes: 32 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,38 @@ pub(crate) mod test {
assert_eq!(txt, "");
}

#[test]
fn redirect_max_with_error() {
init_test_log();
let agent: Agent = Config::builder().max_redirects(3).build().into();
let res = agent
.get(
"http://httpbin.org/redirect-to?url=%2Fredirect-to%3F\
url%3D%2Fredirect-to%3Furl%3D%252Fredirect-to%253Furl%253D",
)
.call();
let err = res.unwrap_err();
assert_eq!(err.to_string(), "too many redirects");
}

#[test]
fn redirect_max_without_error() {
init_test_log();
let agent: Agent = Config::builder()
.max_redirects(3)
.max_redirects_will_error(false)
.build()
.into();
let res = agent
.get(
"http://httpbin.org/redirect-to?url=%2Fredirect-to%3F\
url%3D%2Fredirect-to%3Furl%3D%252Fredirect-to%253Furl%253D",
)
.call()
.unwrap();
assert_eq!(res.status(), 302);
}

#[test]
fn redirect_follow() {
init_test_log();
Expand Down
22 changes: 17 additions & 5 deletions src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,18 @@ fn flow_run(
..Default::default()
};

if response.status().is_redirection() && redirect_count < config.max_redirects() {
let flow = handler.consume_redirect_body()?;

FlowResult::Redirect(flow, handler.timings)
if response.status().is_redirection() {
if redirect_count < config.max_redirects() {
let flow = handler.consume_redirect_body()?;

FlowResult::Redirect(flow, handler.timings)
} else {

Check failure on line 186 in src/run.rs

View workflow job for this annotation

GitHub Actions / Lint

this `else { if .. }` block can be collapsed

Check failure on line 186 in src/run.rs

View workflow job for this annotation

GitHub Actions / Lint

this `else { if .. }` block can be collapsed
if config.max_redirects_do_error() {
return Err(Error::TooManyRedirects);
} else {
FlowResult::Response(response, handler)
}
}
} else {
FlowResult::Response(response, handler)
}
Expand All @@ -192,7 +200,11 @@ fn flow_run(
if redirect_count < config.max_redirects() {
FlowResult::Redirect(flow, mem::take(timings))
} else {

Check failure on line 202 in src/run.rs

View workflow job for this annotation

GitHub Actions / Lint

this `else { if .. }` block can be collapsed

Check failure on line 202 in src/run.rs

View workflow job for this annotation

GitHub Actions / Lint

this `else { if .. }` block can be collapsed
FlowResult::Response(response, BodyHandler::default())
if config.max_redirects_do_error() {
return Err(Error::TooManyRedirects);
} else {
FlowResult::Response(response, BodyHandler::default())
}
}
}
RecvResponseResult::Cleanup(flow) => {
Expand Down
11 changes: 9 additions & 2 deletions src/unversioned/transport/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,16 +289,23 @@ fn setup_default_handlers(handlers: &mut Vec<TestHandler>) {
);

maybe_add(
TestHandler::new("/redirect-to", |_uri, _req, w| {
TestHandler::new("/redirect-to", |uri, _req, w| {
let location = uri.query().unwrap();
assert!(location.starts_with("url="));
let location = &location[4..];
let location = percent_encoding::percent_decode_str(location)
.decode_utf8()
.unwrap();
write!(
w,
"HTTP/1.1 302 FOUND\r\n\
Location: /get\r\n\
Location: {}\r\n\
Content-Length: 22\r\n\
Connection: close\r\n\
\r\n\
You've been redirected\
",
location
)
}),
handlers,
Expand Down

0 comments on commit 1e3f7bc

Please sign in to comment.