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.

Sign up for GitHub

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

Fix redirect handling with response bodies #255

Merged
merged 1 commit into from
Nov 14, 2020
+155 −111
Merged

Fix redirect handling with response bodies #255

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ log = "0.4"
once_cell = "1"
slab = "0.4"
sluice = "0.5"
url = "2.2"
waker-fn = "1"

[dependencies.chrono]
Expand Down
44 changes: 2 additions & 42 deletions src/handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,19 @@

use crate::{
headers,
redirect::RedirectUri,
response::{EffectiveUri, LocalAddr, RemoteAddr},
response::{LocalAddr, RemoteAddr},
Body, Error, Metrics,
};
use crossbeam_utils::atomic::AtomicCell;
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,
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -290,36 +280,6 @@ impl RequestHandler {
}
}

fn get_effective_uri(&mut self) -> Option<Uri> {
self.get_uri(curl_sys::CURLINFO_EFFECTIVE_URL)
}

fn get_redirect_uri(&mut self) -> Option<Uri> {
self.get_uri(curl_sys::CURLINFO_REDIRECT_URL)
}

fn get_uri(&mut self, info: curl_sys::CURLINFO) -> Option<Uri> {
if self.handle.is_null() {
return None;
}

let mut ptr = ptr::null::<c_char>();

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<SocketAddr> {
let ip = self.get_primary_ip()?.parse().ok()?;
let port = self.get_primary_port()?;
Expand Down
187 changes: 121 additions & 66 deletions src/redirect.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,115 +3,170 @@ 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;

impl Interceptor for RedirectInterceptor {
type Err = Error;

fn intercept<'a>(&'a self, request: Request<Body>, ctx: Context<'a>) -> InterceptorFuture<'a, Self::Err> {
fn intercept<'a>(
&'a self,
mut request: Request<Body>,
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::<RedirectPolicy>()
.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::<crate::config::redirect::AutoReferer>()
.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::<RedirectUri>() {
// 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::<RequestBody>()
.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::<RequestBody>()
.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<T>(request_uri: &Uri, response: &Response<T>) -> Option<Uri> {
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<Uri, Box<dyn std::error::Error>> {
// 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)),
}
}
4 changes: 1 addition & 3 deletions src/response.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::Metrics;
use crate::{redirect::EffectiveUri, Metrics};
use futures_lite::io::AsyncRead;
use http::{Response, Uri};
use std::{
Expand Down Expand Up @@ -226,8 +226,6 @@ impl<T> ResponseExt<T> for Response<T> {
}
}

pub(crate) struct EffectiveUri(pub(crate) Uri);

pub(crate) struct LocalAddr(pub(crate) SocketAddr);

pub(crate) struct RemoteAddr(pub(crate) SocketAddr);
30 changes: 30 additions & 0 deletions tests/redirects.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down