History

Merge branch 'fix-redirect'

Sebastian Thiel committed 3 years ago

Commit e83c38f2 parents f584d76 7663a482 files · +60 −8Browse files

Unified · Split

2 files
  1. gix-transport/src/client/blocking_io/http/mod.rs +1 −1
  2. gix-transport/src/client/blocking_io/http/reqwest/remote.rs +59 −7
gix-transport/src/client/blocking_io/http/mod.rs+1−1
@@ -517,5 +517,5 @@ pub fn connect(url: gix_url::Url, desired_version: Protocol) -> Transport<Impl> · 516 unmodified lines
517}517}
518 518
519///519///
520#[cfg(feature = "http-client-curl")]520#[cfg(any(feature = "http-client-curl", feature = "http-client-reqwest"))]
521pub mod redirect;521pub mod redirect;
gix-transport/src/client/blocking_io/http/reqwest/remote.rs+59−7
@@ -3,14 +3,12 @@ use std::{ · 2 unmodified lines
3 convert::TryFrom,3 convert::TryFrom,
4 io::{Read, Write},4 io::{Read, Write},
5 str::FromStr,5 str::FromStr,
6 sync::{atomic, Arc},
6};7};
7 8
8use gix_features::io::pipe;9use gix_features::io::pipe;
9 10
10use crate::client::{11use crate::client::http::{self, options::FollowRedirects, redirect, reqwest::Remote, traits::PostBodyDataKind};
11 http,
12 http::{reqwest::Remote, traits::PostBodyDataKind},
13};
14 12
15/// The error returned by the 'remote' helper, a purely internal construct to perform http requests.13/// The error returned by the 'remote' helper, a purely internal construct to perform http requests.
16#[derive(Debug, thiserror::Error)]14#[derive(Debug, thiserror::Error)]
@@ -22,6 +20,8 @@ pub enum Error { · 5 unmodified lines
22 ReadPostBody(#[from] std::io::Error),20 ReadPostBody(#[from] std::io::Error),
23 #[error("Request configuration failed")]21 #[error("Request configuration failed")]
24 ConfigureRequest(#[from] Box<dyn std::error::Error + Send + Sync + 'static>),22 ConfigureRequest(#[from] Box<dyn std::error::Error + Send + Sync + 'static>),
23 #[error(transparent)]
24 Redirect(#[from] redirect::Error),
25}25}
26 26
27impl crate::IsSpuriousError for Error {27impl crate::IsSpuriousError for Error {
@@ -40,23 +40,57 @@ impl Default for Remote { · 12 unmodified lines
40 let (req_send, req_recv) = std::sync::mpsc::sync_channel(0);40 let (req_send, req_recv) = std::sync::mpsc::sync_channel(0);
41 let (res_send, res_recv) = std::sync::mpsc::sync_channel(0);41 let (res_send, res_recv) = std::sync::mpsc::sync_channel(0);
42 let handle = std::thread::spawn(move || -> Result<(), Error> {42 let handle = std::thread::spawn(move || -> Result<(), Error> {
43 let mut follow = None;
44 let mut redirected_base_url = None::<String>;
45 let allow_redirects = Arc::new(atomic::AtomicBool::new(false));
46
43 // We may error while configuring, which is expected as part of the internal protocol. The error will be47 // We may error while configuring, which is expected as part of the internal protocol. The error will be
44 // received and the sender of the request might restart us.48 // received and the sender of the request might restart us.
45 let client = reqwest::blocking::ClientBuilder::new()49 let client = reqwest::blocking::ClientBuilder::new()
46 .connect_timeout(std::time::Duration::from_secs(20))50 .connect_timeout(std::time::Duration::from_secs(20))
47 .http1_title_case_headers()51 .http1_title_case_headers()
52 .redirect(reqwest::redirect::Policy::custom({
53 let allow_redirects = allow_redirects.clone();
54 move |attempt| {
55 if allow_redirects.load(atomic::Ordering::Relaxed) {
56 let curr_url = attempt.url();
57 let prev_urls = attempt.previous();
58
59 match prev_urls.first() {
60 Some(prev_url) if prev_url.host_str() != curr_url.host_str() => {
61 // git does not want to be redirected to a different host.
62 attempt.stop()
63 }
64 _ => {
65 // emulate default git behaviour which relies on curl default behaviour apparently.
66 const CURL_DEFAULT_REDIRS: usize = 50;
67 if prev_urls.len() >= CURL_DEFAULT_REDIRS {
68 attempt.error("too many redirects")
69 } else {
70 attempt.follow()
71 }
72 }
73 }
74 } else {
75 attempt.stop()
76 }
77 }
78 }))
48 .build()?;79 .build()?;
80
49 for Request {81 for Request {
50 url,82 url,
83 base_url,
51 headers,84 headers,
52 upload_body_kind,85 upload_body_kind,
53 config,86 config,
54 } in req_recv87 } in req_recv
55 {88 {
89 let effective_url = redirect::swap_tails(redirected_base_url.as_deref(), &base_url, url.clone());
56 let mut req_builder = if upload_body_kind.is_some() {90 let mut req_builder = if upload_body_kind.is_some() {
57 client.post(url)91 client.post(&effective_url)
58 } else {92 } else {
59 client.get(url)93 client.get(&effective_url)
60 }94 }
61 .headers(headers);95 .headers(headers);
62 let (post_body_tx, mut post_body_rx) = pipe::unidirectional(0);96 let (post_body_tx, mut post_body_rx) = pipe::unidirectional(0);
@@ -91,6 +125,17 @@ impl Default for Remote { · 28 unmodified lines
91 }125 }
92 }126 }
93 }127 }
128
129 let follow = follow.get_or_insert(config.follow_redirects);
130 allow_redirects.store(
131 matches!(follow, FollowRedirects::Initial | FollowRedirects::All),
132 atomic::Ordering::Relaxed,
133 );
134
135 if *follow == FollowRedirects::Initial {
136 *follow = FollowRedirects::None;
137 }
138
94 let mut res = match client139 let mut res = match client
95 .execute(req)140 .execute(req)
96 .and_then(reqwest::blocking::Response::error_for_status)141 .and_then(reqwest::blocking::Response::error_for_status)
@@ -116,6 +161,11 @@ impl Default for Remote { · 19 unmodified lines
116 }161 }
117 };162 };
118 163
164 let actual_url = res.url().as_str();
165 if actual_url != effective_url.as_str() {
166 redirected_base_url = redirect::base_url(actual_url, &base_url, url)?.into();
167 }
168
119 let send_headers = {169 let send_headers = {
120 let headers = res.headers();170 let headers = res.headers();
121 move || -> std::io::Result<()> {171 move || -> std::io::Result<()> {
@@ -172,7 +222,7 @@ impl Remote { · 50 unmodified lines
172 fn make_request(222 fn make_request(
173 &mut self,223 &mut self,
174 url: &str,224 url: &str,
175 _base_url: &str,225 base_url: &str,
176 headers: impl IntoIterator<Item = impl AsRef<str>>,226 headers: impl IntoIterator<Item = impl AsRef<str>>,
177 upload_body_kind: Option<PostBodyDataKind>,227 upload_body_kind: Option<PostBodyDataKind>,
178 ) -> Result<http::PostResponse<pipe::Reader, pipe::Reader, pipe::Writer>, http::Error> {228 ) -> Result<http::PostResponse<pipe::Reader, pipe::Reader, pipe::Writer>, http::Error> {
@@ -197,6 +247,7 @@ impl Remote { · 18 unmodified lines
197 .request247 .request
198 .send(Request {248 .send(Request {
199 url: url.to_owned(),249 url: url.to_owned(),
250 base_url: base_url.to_owned(),
200 headers: header_map,251 headers: header_map,
201 upload_body_kind,252 upload_body_kind,
202 config: self.config.clone(),253 config: self.config.clone(),
@@ -259,6 +310,7 @@ impl http::Http for Remote { · 56 unmodified lines
259 310
260pub(crate) struct Request {311pub(crate) struct Request {
261 pub url: String,312 pub url: String,
313 pub base_url: String,
262 pub headers: reqwest::header::HeaderMap,314 pub headers: reqwest::header::HeaderMap,
263 pub upload_body_kind: Option<PostBodyDataKind>,315 pub upload_body_kind: Option<PostBodyDataKind>,
264 pub config: http::Options,316 pub config: http::Options,