diff --git a/Cargo.toml b/Cargo.toml index cc1ff435..38aa1799 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,6 +41,7 @@ log = "0.4" once_cell = "1" slab = "0.4" sluice = "0.5" +url = "2.2" waker-fn = "1" [dependencies.chrono] diff --git a/src/handler.rs b/src/handler.rs index 10e5c1f8..b9784e24 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -2,8 +2,7 @@ use crate::{ headers, - redirect::RedirectUri, - response::{EffectiveUri, LocalAddr, RemoteAddr}, + response::{LocalAddr, RemoteAddr}, Body, Error, Metrics, }; use crossbeam_utils::atomic::AtomicCell; @@ -11,12 +10,11 @@ use curl::easy::{InfoType, ReadError, SeekResult, WriteError}; use curl_sys::CURL; use flume::Sender; use futures_lite::{io::{AsyncRead, AsyncWrite}, pin}; -use http::{Response, Uri}; +use http::Response; use once_cell::sync::OnceCell; use sluice::pipe; use std::{ ascii, - convert::TryInto, ffi::CStr, fmt, future::Future, @@ -244,14 +242,6 @@ impl RequestHandler { headers.extend(self.response_headers.drain()); } - if let Some(uri) = self.get_effective_uri() { - builder = builder.extension(EffectiveUri(uri)); - } - - if let Some(uri) = self.get_redirect_uri() { - builder = builder.extension(RedirectUri(uri)); - } - if let Some(addr) = self.get_local_addr() { builder = builder.extension(LocalAddr(addr)); } @@ -290,36 +280,6 @@ impl RequestHandler { } } - fn get_effective_uri(&mut self) -> Option { - self.get_uri(curl_sys::CURLINFO_EFFECTIVE_URL) - } - - fn get_redirect_uri(&mut self) -> Option { - self.get_uri(curl_sys::CURLINFO_REDIRECT_URL) - } - - fn get_uri(&mut self, info: curl_sys::CURLINFO) -> Option { - if self.handle.is_null() { - return None; - } - - let mut ptr = ptr::null::(); - - unsafe { - if curl_sys::curl_easy_getinfo(self.handle, info, &mut ptr) - != curl_sys::CURLE_OK - { - return None; - } - } - - if ptr.is_null() { - return None; - } - - unsafe { CStr::from_ptr(ptr) }.to_bytes().try_into().ok() - } - fn get_primary_addr(&mut self) -> Option { let ip = self.get_primary_ip()?.parse().ok()?; let port = self.get_primary_port()?; diff --git a/src/redirect.rs b/src/redirect.rs index bf16f180..f50f08b8 100644 --- a/src/redirect.rs +++ b/src/redirect.rs @@ -3,19 +3,20 @@ use crate::{ handler::RequestBody, interceptor::{Context, Interceptor, InterceptorFuture}, request::RequestExt, - Body, - Error, + Body, Error, }; -use http::{Request, Uri}; +use http::{Request, Response, Uri}; +use std::convert::TryFrom; +use url::Url; /// How many redirects to follow by default if a limit is not specified. We /// don't actually allow infinite redirects as that could result in a dangerous /// infinite loop, so by default we actually limit redirects to a large amount. const DEFAULT_REDIRECT_LIMIT: u32 = 1024; -/// Extension containing the redirect target on the response determined by curl, -/// if any. -pub(crate) struct RedirectUri(pub(crate) Uri); +/// Extension containing the final "effective" URI that was visited, after +/// following any redirects. +pub(crate) struct EffectiveUri(pub(crate) Uri); /// Interceptor that implements automatic following of HTTP redirects. pub(crate) struct RedirectInterceptor; @@ -23,95 +24,149 @@ pub(crate) struct RedirectInterceptor; impl Interceptor for RedirectInterceptor { type Err = Error; - fn intercept<'a>(&'a self, request: Request, ctx: Context<'a>) -> InterceptorFuture<'a, Self::Err> { + fn intercept<'a>( + &'a self, + mut request: Request, + ctx: Context<'a>, + ) -> InterceptorFuture<'a, Self::Err> { Box::pin(async move { + // Store the effective URI to include in the response. + let mut effective_uri = request.uri().clone(); + // Get the redirect policy for this request. - let policy = request.extensions() + let policy = request + .extensions() .get::() .cloned() .unwrap_or_default(); // No redirect handling, just proceed normally. if policy == RedirectPolicy::None { - return ctx.send(request).await; + let mut response = ctx.send(request).await?; + response + .extensions_mut() + .insert(EffectiveUri(effective_uri)); + + return Ok(response); } - let auto_referer = request.extensions() + let auto_referer = request + .extensions() .get::() .is_some(); - // Make a copy of the request before sending it. - let mut request_builder = request.to_builder(); - - // Send the request to get the ball rolling. - let mut response = ctx.send(request).await?; - - let mut redirect_count: u32 = 0; let limit = match policy { RedirectPolicy::Limit(limit) => limit, _ => DEFAULT_REDIRECT_LIMIT, }; - // Check for redirects. If a redirect should happen, then curl will - // return a URI to redirect to, which the request handler will - // attach to the response as this extension. - while let Some(RedirectUri(redirect_uri)) = response.extensions_mut().remove::() { - // Sanity check. - if !response.status().is_redirection() { - break; - } + // Keep track of how many redirects we've done. + let mut redirect_count: u32 = 0; - // If we've reached the limit, return an error as requested. - if redirect_count >= limit { - return Err(Error::TooManyRedirects); + loop { + // Preserve a clone of the request before sending it. + let mut request_builder = request.to_builder(); + + // Send the request to get the ball rolling. + let mut response = ctx.send(request).await?; + + // Check for a redirect. + if let Some(location) = get_redirect_location(&effective_uri, &response) { + // If we've reached the limit, return an error as requested. + if redirect_count >= limit { + return Err(Error::TooManyRedirects); + } + + // Set referer header. + if auto_referer { + let referer = request_builder.uri_ref().unwrap().to_string(); + request_builder = request_builder.header(http::header::REFERER, referer); + } + + // Check if we should change the request method into a GET. HTTP + // specs don't really say one way or another when this should + // happen for most status codes, so we just mimic curl's + // behavior here since it is so common. + if response.status() == 301 || response.status() == 302 || response.status() == 303 + { + request_builder = request_builder.method(http::Method::GET); + } + + // Grab the request body back from the internal handler, as we + // might need to send it again (if possible...) + let mut request_body = response + .extensions_mut() + .remove::() + .map(|v| v.0) + .unwrap_or_default(); + + // Redirect handling is tricky when we are uploading something. + // If we can, reset the body stream to the beginning. This might + // work if the body to upload is an in-memory byte buffer, but + // for arbitrary streams we can't do this. + // + // There's not really a good way of handling this gracefully, so + // we just return an error so that the user knows about it. + if !request_body.reset() { + return Err(Error::RequestBodyError(Some(String::from( + "could not follow redirect because request body is not rewindable", + )))); + } + + // Update the request to point to the new URI. + effective_uri = location.clone(); + request = request_builder.uri(location).body(request_body)?; + redirect_count += 1; } - // Set referer header. - if auto_referer { - let referer = request_builder.uri_ref().unwrap().to_string(); - request_builder = request_builder.header(http::header::REFERER, referer); - } + // No more redirects; set the effective URI we finally settled on and return. + else { + response + .extensions_mut() + .insert(EffectiveUri(effective_uri)); - // Check if we should change the request method into a GET. HTTP - // specs don't really say one way or another when this should - // happen for most status codes, so we just mimic curl's - // behavior here since it is so common. - if response.status() == 301 || response.status() == 302 || response.status() == 303 { - request_builder = request_builder.method(http::Method::GET); + return Ok(response); } + } + }) + } +} - // Grab the request body back from the internal handler, as we - // might need to send it again (if possible...) - let mut request_body = response.extensions_mut() - .remove::() - .map(|v| v.0) - .unwrap_or_default(); - - // Redirect handling is tricky when we are uploading something. - // If we can, reset the body stream to the beginning. This might - // work if the body to upload is an in-memory byte buffer, but - // for arbitrary streams we can't do this. - // - // There's not really a good way of handling this gracefully, so - // we just return an error so that the user knows about it. - if !request_body.reset() { - return Err(Error::RequestBodyError(Some(String::from("could not follow redirect because request body is not rewindable")))); +fn get_redirect_location(request_uri: &Uri, response: &Response) -> Option { + if response.status().is_redirection() { + let location = response.headers().get(http::header::LOCATION)?; + + match location.to_str() { + Ok(location) => { + match resolve(request_uri, location) { + Ok(uri) => return Some(uri), + Err(e) => { + tracing::debug!("bad redirect location: {}", e); + } } + } + Err(e) => { + tracing::debug!("bad redirect location: {}", e); + } + } + } - let request = request_builder.uri(redirect_uri) - .body(request_body)?; + None +} - // Keep another clone of the request around again, in case we - // need to follow yet another redirect. - request_builder = request.to_builder(); +/// Resolve one URI in terms of another. +fn resolve(base: &Uri, target: &str) -> Result> { + // Optimistically check if this is an absolute URI. + match Url::parse(target) { + Ok(url) => Ok(Uri::try_from(url.as_str())?), - // Send the redirected request. - response = ctx.send(request).await?; + // Relative URI, resolve against the base. + Err(url::ParseError::RelativeUrlWithoutBase) => { + let base = Url::parse(base.to_string().as_str())?; - redirect_count += 1; - } + Ok(Uri::try_from(base.join(target)?.as_str())?) + } - Ok(response) - }) + Err(e) => Err(Box::new(e)), } } diff --git a/src/response.rs b/src/response.rs index 119f139d..a4992725 100644 --- a/src/response.rs +++ b/src/response.rs @@ -1,4 +1,4 @@ -use crate::Metrics; +use crate::{redirect::EffectiveUri, Metrics}; use futures_lite::io::AsyncRead; use http::{Response, Uri}; use std::{ @@ -226,8 +226,6 @@ impl ResponseExt for Response { } } -pub(crate) struct EffectiveUri(pub(crate) Uri); - pub(crate) struct LocalAddr(pub(crate) SocketAddr); pub(crate) struct RemoteAddr(pub(crate) SocketAddr); diff --git a/tests/redirects.rs b/tests/redirects.rs index 1af83aae..de218a85 100644 --- a/tests/redirects.rs +++ b/tests/redirects.rs @@ -144,6 +144,36 @@ fn redirect_also_sends_post(status: u16) { assert_eq!(m2.request().method, "POST"); } +// Issue #250 +#[test] +fn redirect_with_response_body() { + let m2 = mock! { + body: "OK", + }; + let location = m2.url(); + + let m1 = mock! { + status: 302, + headers { + "Location": location, + } + body: "REDIRECT", + }; + + let response = Request::post(m1.url()) + .redirect_policy(RedirectPolicy::Follow) + .body(()) + .unwrap() + .send() + .unwrap(); + + assert_eq!(response.status(), 200); + assert_eq!(response.effective_uri().unwrap().to_string(), m2.url()); + + assert_eq!(m1.request().method, "POST"); + assert_eq!(m2.request().method, "GET"); +} + // Issue #250 #[test] fn redirect_policy_from_client() {