Skip to content

Commit

Permalink
Merge branch 'fix-redirect'
Browse files Browse the repository at this point in the history
  • Loading branch information
Byron committed Aug 13, 2023
2 parents f584d76 + 7663a48 commit e83c38f
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 8 deletions.
2 changes: 1 addition & 1 deletion gix-transport/src/client/blocking_io/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -517,5 +517,5 @@ pub fn connect(url: gix_url::Url, desired_version: Protocol) -> Transport<Impl>
}

///
#[cfg(feature = "http-client-curl")]
#[cfg(any(feature = "http-client-curl", feature = "http-client-reqwest"))]
pub mod redirect;
66 changes: 59 additions & 7 deletions gix-transport/src/client/blocking_io/http/reqwest/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@ use std::{
convert::TryFrom,
io::{Read, Write},
str::FromStr,
sync::{atomic, Arc},
};

use gix_features::io::pipe;

use crate::client::{
http,
http::{reqwest::Remote, traits::PostBodyDataKind},
};
use crate::client::http::{self, options::FollowRedirects, redirect, reqwest::Remote, traits::PostBodyDataKind};

/// The error returned by the 'remote' helper, a purely internal construct to perform http requests.
#[derive(Debug, thiserror::Error)]
Expand All @@ -22,6 +20,8 @@ pub enum Error {
ReadPostBody(#[from] std::io::Error),
#[error("Request configuration failed")]
ConfigureRequest(#[from] Box<dyn std::error::Error + Send + Sync + 'static>),
#[error(transparent)]
Redirect(#[from] redirect::Error),
}

impl crate::IsSpuriousError for Error {
Expand All @@ -40,23 +40,57 @@ impl Default for Remote {
let (req_send, req_recv) = std::sync::mpsc::sync_channel(0);
let (res_send, res_recv) = std::sync::mpsc::sync_channel(0);
let handle = std::thread::spawn(move || -> Result<(), Error> {
let mut follow = None;
let mut redirected_base_url = None::<String>;
let allow_redirects = Arc::new(atomic::AtomicBool::new(false));

// We may error while configuring, which is expected as part of the internal protocol. The error will be
// received and the sender of the request might restart us.
let client = reqwest::blocking::ClientBuilder::new()
.connect_timeout(std::time::Duration::from_secs(20))
.http1_title_case_headers()
.redirect(reqwest::redirect::Policy::custom({
let allow_redirects = allow_redirects.clone();
move |attempt| {
if allow_redirects.load(atomic::Ordering::Relaxed) {
let curr_url = attempt.url();
let prev_urls = attempt.previous();

match prev_urls.first() {
Some(prev_url) if prev_url.host_str() != curr_url.host_str() => {
// git does not want to be redirected to a different host.
attempt.stop()
}
_ => {
// emulate default git behaviour which relies on curl default behaviour apparently.
const CURL_DEFAULT_REDIRS: usize = 50;
if prev_urls.len() >= CURL_DEFAULT_REDIRS {
attempt.error("too many redirects")
} else {
attempt.follow()
}
}
}
} else {
attempt.stop()
}
}
}))
.build()?;

for Request {
url,
base_url,
headers,
upload_body_kind,
config,
} in req_recv
{
let effective_url = redirect::swap_tails(redirected_base_url.as_deref(), &base_url, url.clone());
let mut req_builder = if upload_body_kind.is_some() {
client.post(url)
client.post(&effective_url)
} else {
client.get(url)
client.get(&effective_url)
}
.headers(headers);
let (post_body_tx, mut post_body_rx) = pipe::unidirectional(0);
Expand Down Expand Up @@ -91,6 +125,17 @@ impl Default for Remote {
}
}
}

let follow = follow.get_or_insert(config.follow_redirects);
allow_redirects.store(
matches!(follow, FollowRedirects::Initial | FollowRedirects::All),
atomic::Ordering::Relaxed,
);

if *follow == FollowRedirects::Initial {
*follow = FollowRedirects::None;
}

let mut res = match client
.execute(req)
.and_then(reqwest::blocking::Response::error_for_status)
Expand All @@ -116,6 +161,11 @@ impl Default for Remote {
}
};

let actual_url = res.url().as_str();
if actual_url != effective_url.as_str() {
redirected_base_url = redirect::base_url(actual_url, &base_url, url)?.into();
}

let send_headers = {
let headers = res.headers();
move || -> std::io::Result<()> {
Expand Down Expand Up @@ -172,7 +222,7 @@ impl Remote {
fn make_request(
&mut self,
url: &str,
_base_url: &str,
base_url: &str,
headers: impl IntoIterator<Item = impl AsRef<str>>,
upload_body_kind: Option<PostBodyDataKind>,
) -> Result<http::PostResponse<pipe::Reader, pipe::Reader, pipe::Writer>, http::Error> {
Expand All @@ -197,6 +247,7 @@ impl Remote {
.request
.send(Request {
url: url.to_owned(),
base_url: base_url.to_owned(),
headers: header_map,
upload_body_kind,
config: self.config.clone(),
Expand Down Expand Up @@ -259,6 +310,7 @@ impl http::Http for Remote {

pub(crate) struct Request {
pub url: String,
pub base_url: String,
pub headers: reqwest::header::HeaderMap,
pub upload_body_kind: Option<PostBodyDataKind>,
pub config: http::Options,
Expand Down

0 comments on commit e83c38f

Please sign in to comment.