Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redirect history #939

Merged
merged 4 commits into from
Jan 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

# 3.0.0-rc5

* Feature `Config::save_redirect_history` (#939)
* `TlsConfig::unversioned_rustls_crypto_provider()` (#931)
* Feature `rustls-no-provider` to compile without ring (#931)
* Fix CONNECT proxy Host header (#936)
Expand Down
2 changes: 1 addition & 1 deletion src/body/lossy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl<R: io::Read> io::Read for LossyUtf8Reader<R> {
invalid_sequence,
..
} => {
let valid_len = valid_prefix.as_bytes().len();
let valid_len = valid_prefix.len();
let invalid_len = invalid_sequence.len();

// Switch out the problem input chars
Expand Down
30 changes: 30 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ pub struct Config {
max_redirects: u32,
max_redirects_will_error: bool,
redirect_auth_headers: RedirectAuthHeaders,
save_redirect_history: bool,
user_agent: AutoHeaderValue,
accept: AutoHeaderValue,
accept_encoding: AutoHeaderValue,
Expand Down Expand Up @@ -277,6 +278,19 @@ impl Config {
self.redirect_auth_headers
}

/// If we should record a history of every redirect location,
/// including the request and final locations.
///
/// Comes at the cost of allocating/retaining the `Uri` for
/// every redirect loop.
///
/// See [`ResponseExt::get_redirect_history()`][crate::ResponseExt::get_redirect_history].
///
/// Defaults to `false`.
pub fn save_redirect_history(&self) -> bool {
self.save_redirect_history
}

/// Value to use for the `User-Agent` header.
///
/// This can be overridden by setting a `user-agent` header on the request
Expand Down Expand Up @@ -482,6 +496,20 @@ impl<Scope: private::ConfigScope> ConfigBuilder<Scope> {
self
}

/// If we should record a history of every redirect location,
/// including the request and final locations.
///
/// Comes at the cost of allocating/retaining the `Uri` for
/// every redirect loop.
///
/// See [`ResponseExt::get_redirect_history()`][crate::ResponseExt::get_redirect_history].
///
/// Defaults to `false`.
pub fn save_redirect_history(mut self, v: bool) -> Self {
self.config().save_redirect_history = v;
self
}

/// Value to use for the `User-Agent` header.
///
/// This can be overridden by setting a `user-agent` header on the request
Expand Down Expand Up @@ -809,6 +837,7 @@ impl Default for Config {
max_redirects: 10,
max_redirects_will_error: true,
redirect_auth_headers: RedirectAuthHeaders::Never,
save_redirect_history: false,
user_agent: AutoHeaderValue::default(),
accept: AutoHeaderValue::default(),
accept_encoding: AutoHeaderValue::default(),
Expand Down Expand Up @@ -884,6 +913,7 @@ impl fmt::Debug for Config {
.field("no_delay", &self.no_delay)
.field("max_redirects", &self.max_redirects)
.field("redirect_auth_headers", &self.redirect_auth_headers)
.field("save_redirect_history", &self.save_redirect_history)
.field("user_agent", &self.user_agent)
.field("timeouts", &self.timeouts)
.field("max_response_header_size", &self.max_response_header_size)
Expand Down
59 changes: 59 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -847,6 +847,65 @@ pub(crate) mod test {
assert_eq!(response_uri.path(), "/get")
}

#[test]
fn redirect_history_none() {
init_test_log();
let res = get("http://httpbin.org/redirect-to?url=%2Fget")
.call()
.unwrap();
let redirect_history = res.get_redirect_history();
assert_eq!(redirect_history, None)
}

#[test]
fn redirect_history_some() {
init_test_log();
let agent: Agent = Config::builder()
.max_redirects(3)
.max_redirects_will_error(false)
.save_redirect_history(true)
.build()
.into();
let res = agent
.get("http://httpbin.org/redirect-to?url=%2Fget")
.call()
.unwrap();
let redirect_history = res.get_redirect_history();
assert_eq!(
redirect_history,
Some(
vec![
"http://httpbin.org/redirect-to?url=%2Fget".parse().unwrap(),
"http://httpbin.org/get".parse().unwrap()
]
.as_ref()
)
);
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();
let redirect_history = res.get_redirect_history();
assert_eq!(
redirect_history,
Some(vec![
"http://httpbin.org/redirect-to?url=%2Fredirect-to%3Furl%3D%2Fredirect-to%3Furl%3D%252Fredirect-to%253Furl%253D".parse().unwrap(),
"http://httpbin.org/redirect-to?url=/redirect-to?url=%2Fredirect-to%3Furl%3D".parse().unwrap(),
"http://httpbin.org/redirect-to?url=/redirect-to?url=".parse().unwrap(),
"http://httpbin.org/redirect-to?url=".parse().unwrap(),
].as_ref())
);
let res = agent.get("https://www.google.com/").call().unwrap();
let redirect_history = res.get_redirect_history();
assert_eq!(
redirect_history,
Some(vec!["https://www.google.com/".parse().unwrap()].as_ref())
);
}

#[test]
fn connect_https_invalid_name() {
let result = get("https://example.com{REQUEST_URI}/").call();
Expand Down
52 changes: 49 additions & 3 deletions src/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,52 @@ use crate::http;
#[derive(Debug, Clone)]
pub(crate) struct ResponseUri(pub http::Uri);

/// Extension trait for `http::Response<Body>` objects
#[derive(Debug, Clone)]
pub(crate) struct RedirectHistory(pub Vec<Uri>);

/// Extension trait for [`http::Response<Body>`].
///
/// Allows the user to access the `Uri` in http::Response
/// Adds additional convenience methods to the `Response` that are not available
/// in the plain http API.
pub trait ResponseExt {
/// The Uri we ended up at. This can differ from the request uri when we have followed redirects.
/// The Uri that ultimately this Response is about.
///
/// This can differ from the request uri when we have followed redirects.
///
/// ```
/// use ureq::ResponseExt;
///
/// let res = ureq::get("https://httpbin.org/redirect-to?url=%2Fget")
/// .call().unwrap();
///
/// assert_eq!(res.get_uri(), "https://httpbin.org/get");
/// ```
fn get_uri(&self) -> &Uri;

/// The full history of uris, including the request and final uri.
///
/// Returns `None` when [`Config::save_redirect_history`][crate::config::Config::save_redirect_history]
/// is `false`.
///
///
/// ```
/// # use ureq::http::Uri;
/// use ureq::ResponseExt;
///
/// let uri1: Uri = "https://httpbin.org/redirect-to?url=%2Fget".parse().unwrap();
/// let uri2: Uri = "https://httpbin.org/get".parse::<Uri>().unwrap();
///
/// let res = ureq::get(&uri1)
/// .config()
/// .save_redirect_history(true)
/// .build()
/// .call().unwrap();
///
/// let history = res.get_redirect_history().unwrap();
///
/// assert_eq!(history, &[uri1, uri2]);
/// ```
fn get_redirect_history(&self) -> Option<&[Uri]>;
}

impl ResponseExt for http::Response<Body> {
Expand All @@ -22,4 +62,10 @@ impl ResponseExt for http::Response<Body> {
.expect("uri to have been set")
.0
}

fn get_redirect_history(&self) -> Option<&[Uri]> {
self.extensions()
.get::<RedirectHistory>()
.map(|r| r.0.as_ref())
}
}
13 changes: 12 additions & 1 deletion src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::body::ResponseInfo;
use crate::config::{Config, RequestLevelConfig, DEFAULT_USER_AGENT};
use crate::http;
use crate::pool::Connection;
use crate::response::ResponseUri;
use crate::response::{RedirectHistory, ResponseUri};
use crate::timings::{CallTimings, CurrentTime};
use crate::transport::time::{Duration, Instant};
use crate::transport::ConnectionDetails;
Expand All @@ -41,6 +41,9 @@ pub(crate) fn run(
.map(Arc::new)
.unwrap_or_else(|| agent.config.clone());

let mut redirect_history: Option<Vec<Uri>> =
config.save_redirect_history().then_some(Vec::new());

let timeouts = config.timeouts();

let mut timings = CallTimings::new(timeouts, CurrentTime::default());
Expand All @@ -67,6 +70,7 @@ pub(crate) fn run(
flow,
&mut body,
redirect_count,
&mut redirect_history,
&mut timings,
)? {
// Follow redirect
Expand Down Expand Up @@ -112,6 +116,7 @@ fn flow_run(
mut flow: Flow<Prepare>,
body: &mut SendBody,
redirect_count: u32,
redirect_history: &mut Option<Vec<Uri>>,
timings: &mut CallTimings,
) -> Result<FlowResult, Error> {
let uri = flow.uri().clone();
Expand Down Expand Up @@ -166,6 +171,12 @@ fn flow_run(
jar.store_response_cookies(iter, &uri);
}

if let Some(history) = redirect_history.as_mut() {
history.push(uri.clone());
response
.extensions_mut()
.insert(RedirectHistory(history.clone()));
}
response.extensions_mut().insert(ResponseUri(uri));

let ret = match response_result {
Expand Down
Loading