Repository: sagebind/isahc (rust)

Issue:
Cookies aren't sent when following redirects
Cookies and `RedirectPolicy` don't play nicely together. Since we currently allow curl to handle redirects for us internally, our middlewares are not executed for subsequent requests in a redirect chain, and so no cookies are sent that should have been.

To fix this, we're going to need to re-implement redirect following outside of curl (which is honestly a good thing, because curl makes it very hard to keep track of what redirects it is following). And to do _that_, we're going to need #42 so that interceptors can initiate new requests. The interceptor API doesn't need to be stable, we just need the functionality internally available.

Pull request description:
Replace curl redirector with redirect interceptor
Implement redirect following ourselves using an interceptor instead of using curl's redirect implementation. This ensures that redirect handling happens at a higher layer than other interceptors (such as the cookie interceptor) but also opens the door for custom redirect logic.

Fixes #232.

Patch:
diff --git a/Cargo.toml b/Cargo.toml
index d8cf28b..ea04ba7 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -38,7 +38,7 @@ curl = "0.4.34"
 curl-sys = "0.4.37"
 futures-channel = "0.3"
 futures-io = "0.3"
-http = "0.2"
+http = "0.2.1"
 lazy_static = "1"
 log = "0.4"
 slab = "0.4"
diff --git a/src/client.rs b/src/client.rs
index c9714d1..9b54be1 100644
--- a/src/client.rs
+++ b/src/client.rs
@@ -98,7 +98,13 @@ impl HttpClientBuilder {
         Self {
             agent_builder: AgentBuilder::default(),
             defaults,
-            interceptors: Vec::new(),
+            interceptors: vec![
+                // Add redirect support. Note that this is _always_ the first,
+                // and thus the outermost, interceptor. Also note that this does
+                // not enable redirect following, it just implements support for
+                // it, if a request asks for it.
+                InterceptorObj::new(crate::redirect::RedirectInterceptor),
+            ],
             default_headers: HeaderMap::new(),
             error: None,
 
@@ -890,71 +896,18 @@ impl HttpClient {
         );
 
         let ctx = interceptor::Context {
-            invoker: Arc::new(move |mut request| {
-                Box::pin(
-                    async move {
-                        // We are checking here if header already contains the key, simply ignore it.
-                        // In case the key wasn't present in parts.headers ensure that
-                        // we have all the headers from default headers.
-                        for name in self.default_headers.keys() {
-                            if !request.headers().contains_key(name) {
-                                for v in self.default_headers.get_all(name).iter() {
-                                    request.headers_mut().append(name, v.clone());
-                                }
-                            }
-                        }
-
-                        // Set default user agent if not specified.
-                        request
-                            .headers_mut()
-                            .entry(http::header::USER_AGENT)
-                            .or_insert(USER_AGENT.parse().unwrap());
-
-                        // Create and configure a curl easy handle to fulfil the request.
-                        let (easy, future) = self.create_easy_handle(request)?;
-
-                        // Send the request to the agent to be executed.
-                        self.agent.submit_request(easy)?;
-
-                        // Await for the response headers.
-                        let response = future.await?;
-
-                        // If a Content-Length header is present, include that information in
-                        // the body as well.
-                        let content_length = response
-                            .headers()
-                            .get(http::header::CONTENT_LENGTH)
-                            .and_then(|v| v.to_str().ok())
-                            .and_then(|v| v.parse().ok());
-
-                        // Convert the reader into an opaque Body.
-                        Ok(response.map(|reader| {
-                            let body = ResponseBody {
-                                inner: reader,
-                                // Extend the lifetime of the agent by including a reference
-                                // to its handle in the response body.
-                                _agent: self.agent.clone(),
-                            };
-
-                            if let Some(len) = content_length {
-                                Body::from_reader_sized(body, len)
-                            } else {
-                                Body::from_reader(body)
-                            }
-                        }))
-                    }
-                    .instrument(span.clone()),
-                )
-            }),
+            invoker: Arc::new(self),
             interceptors: &self.interceptors,
         };
 
-        ctx.send(request).await
+        ctx.send(request)
+            .instrument(span)
+            .await
     }
 
     fn create_easy_handle(
         &self,
-        request: Request<Body>,
+        mut request: Request<Body>,
     ) -> Result<
         (
             curl::easy::Easy2<RequestHandler>,
@@ -963,7 +916,8 @@ impl HttpClient {
         Error,
     > {
         // Prepare the request plumbing.
-        let (mut parts, body) = request.into_parts();
+        // let (mut parts, body) = request.into_parts();
+        let body = std::mem::take(request.body_mut());
         let has_body = !body.is_empty();
         let body_length = body.len();
         let (handler, future) = RequestHandler::new(body);
@@ -988,7 +942,7 @@ impl HttpClient {
 
         set_opts!(
             &mut easy,
-            parts.extensions,
+            request.extensions(),
             self.defaults,
             [
                 Timeout,
@@ -997,8 +951,6 @@ impl HttpClient {
                 TcpNoDelay,
                 NetworkInterface,
                 Dialer,
-                RedirectPolicy,
-                redirect::AutoReferer,
                 AutomaticDecompression,
                 Authentication,
                 Credentials,
@@ -1025,7 +977,7 @@ impl HttpClient {
         // Set the HTTP method to use. Curl ties in behavior with the request
         // method, so we need to configure this carefully.
         #[allow(indirect_structural_match)]
-        match (&parts.method, has_body) {
+        match (request.method(), has_body) {
             // Normal GET request.
             (&http::Method::GET, false) => {
                 easy.get(true)?;
@@ -1049,7 +1001,7 @@ impl HttpClient {
             }
         }
 
-        easy.url(&parts.uri.to_string())?;
+        easy.url(&request.uri().to_string())?;
 
         // If the request has a body, then we either need to tell curl how large
         // the body is if we know it, or tell curl to use chunked encoding. If
@@ -1057,15 +1009,15 @@ impl HttpClient {
         if has_body {
             // Use length given in Content-Length header, or the size defined by
             // the body itself.
-            let body_length = parts
-                .headers
+            let body_length = request
+                .headers()
                 .get("Content-Length")
                 .and_then(|value| value.to_str().ok())
                 .and_then(|value| value.parse().ok())
                 .or(body_length);
 
             if let Some(len) = body_length {
-                if parts.method == http::Method::POST {
+                if request.method() == http::Method::POST {
                     easy.post_field_size(len)?;
                 } else {
                     easy.in_filesize(len)?;
@@ -1074,7 +1026,7 @@ impl HttpClient {
                 // Set the Transfer-Encoding header to instruct curl to use
                 // chunked encoding. Replaces any existing values that may be
                 // incorrect.
-                parts.headers.insert(
+                request.headers_mut().insert(
                     "Transfer-Encoding",
                     http::header::HeaderValue::from_static("chunked"),
                 );
@@ -1084,14 +1036,14 @@ impl HttpClient {
         // Generate a header list for curl.
         let mut headers = curl::easy::List::new();
 
-        let title_case = parts
-            .extensions
+        let title_case = request
+            .extensions()
             .get::<TitleCaseHeaders>()
             .or_else(|| self.defaults.get())
             .map(|v| v.0)
             .unwrap_or(false);
 
-        for (name, value) in parts.headers.iter() {
+        for (name, value) in request.headers().iter() {
             headers.append(&headers::to_curl_string(name, value, title_case))?;
         }
 
@@ -1101,6 +1053,64 @@ impl HttpClient {
     }
 }
 
+impl crate::interceptor::Invoke for &HttpClient {
+    fn invoke<'a>(&'a self, mut request: Request<Body>) -> crate::interceptor::InterceptorFuture<'a, Error> {
+        Box::pin(
+            async move {
+                // We are checking here if header already contains the key, simply ignore it.
+                // In case the key wasn't present in parts.headers ensure that
+                // we have all the headers from default headers.
+                for name in self.default_headers.keys() {
+                    if !request.headers().contains_key(name) {
+                        for v in self.default_headers.get_all(name).iter() {
+                            request.headers_mut().append(name, v.clone());
+                        }
+                    }
+                }
+
+                // Set default user agent if not specified.
+                request
+                    .headers_mut()
+                    .entry(http::header::USER_AGENT)
+                    .or_insert(USER_AGENT.parse().unwrap());
+
+                // Create and configure a curl easy handle to fulfil the request.
+                let (easy, future) = self.create_easy_handle(request)?;
+
+                // Send the request to the agent to be executed.
+                self.agent.submit_request(easy)?;
+
+                // Await for the response headers.
+                let response = future.await?;
+
+                // If a Content-Length header is present, include that information in
+                // the body as well.
+                let content_length = response
+                    .headers()
+                    .get(http::header::CONTENT_LENGTH)
+                    .and_then(|v| v.to_str().ok())
+                    .and_then(|v| v.parse().ok());
+
+                // Convert the reader into an opaque Body.
+                Ok(response.map(|reader| {
+                    let body = ResponseBody {
+                        inner: reader,
+                        // Extend the lifetime of the agent by including a reference
+                        // to its handle in the response body.
+                        _agent: self.agent.clone(),
+                    };
+
+                    if let Some(len) = content_length {
+                        Body::from_reader_sized(body, len)
+                    } else {
+                        Body::from_reader(body)
+                    }
+                }))
+            }
+        )
+    }
+}
+
 impl fmt::Debug for HttpClient {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("HttpClient").finish()
diff --git a/src/config/dial.rs b/src/config/dial.rs
index ac30003..b79019b 100644
--- a/src/config/dial.rs
+++ b/src/config/dial.rs
@@ -46,10 +46,10 @@ impl std::error::Error for DialerParseError {}
 /// let unix_socket = "unix:/path/to/my.sock".parse::<Dialer>()?;
 /// # Ok::<(), isahc::config::DialerParseError>(())
 /// ```
-#[derive(Debug)]
+#[derive(Clone, Debug)]
 pub struct Dialer(Inner);
 
-#[derive(Debug, Eq, PartialEq)]
+#[derive(Clone, Debug, Eq, PartialEq)]
 enum Inner {
     Default,
 
diff --git a/src/config/redirect.rs b/src/config/redirect.rs
index e07a223..bf95120 100644
--- a/src/config/redirect.rs
+++ b/src/config/redirect.rs
@@ -1,6 +1,3 @@
-use super::SetOpt;
-use curl::easy::Easy2;
-
 /// Describes a policy for handling server redirects.
 ///
 /// The default is to not follow redirects.
@@ -11,8 +8,10 @@ pub enum RedirectPolicy {
     ///
     /// This is the default policy.
     None,
+
     /// Follow all redirects automatically.
     Follow,
+
     /// Follow redirects automatically up to a maximum number of redirects.
     Limit(u32),
 }
@@ -23,30 +22,5 @@ impl Default for RedirectPolicy {
     }
 }
 
-impl SetOpt for RedirectPolicy {
-    fn set_opt<H>(&self, easy: &mut Easy2<H>) -> Result<(), curl::Error> {
-        match self {
-            RedirectPolicy::Follow => {
-                easy.follow_location(true)?;
-            }
-            RedirectPolicy::Limit(max) => {
-                easy.follow_location(true)?;
-                easy.max_redirections(*max)?;
-            }
-            RedirectPolicy::None => {
-                easy.follow_location(false)?;
-            }
-        }
-
-        Ok(())
-    }
-}
-
 #[derive(Clone, Copy, Debug)]
 pub(crate) struct AutoReferer;
-
-impl SetOpt for AutoReferer {
-    fn set_opt<H>(&self, easy: &mut Easy2<H>) -> Result<(), curl::Error> {
-        easy.autoreferer(true)
-    }
-}
diff --git a/src/handler.rs b/src/handler.rs
index 78bf4cd..458c2bc 100644
--- a/src/handler.rs
+++ b/src/handler.rs
@@ -2,6 +2,7 @@
 
 use crate::{
     headers,
+    redirect::RedirectUri,
     response::{EffectiveUri, LocalAddr, RemoteAddr},
     Body, Error, Metrics,
 };
@@ -20,6 +21,7 @@ use std::{
     fmt,
     future::Future,
     io,
+    mem,
     net::SocketAddr,
     os::raw::{c_char, c_long},
     pin::Pin,
@@ -28,6 +30,8 @@ use std::{
     task::{Context, Poll, Waker},
 };
 
+pub(crate) struct RequestBody(pub(crate) Body);
+
 /// Manages the state of a single request/response life cycle.
 ///
 /// During the lifetime of a handler, it will receive callbacks from curl about
@@ -240,6 +244,10 @@ impl RequestHandler {
                 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));
             }
@@ -248,6 +256,10 @@ impl RequestHandler {
                 builder = builder.extension(RemoteAddr(addr));
             }
 
+            // Keep the request body around in case interceptors need access to
+            // it. Otherwise we're just going to drop it later.
+            builder = builder.extension(RequestBody(mem::take(&mut self.request_body)));
+
             // Include metrics in response, but only if it was created. If
             // metrics are disabled then it won't have been created.
             if let Some(metrics) = self.metrics.clone() {
@@ -280,6 +292,14 @@ 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;
         }
@@ -287,7 +307,7 @@ impl RequestHandler {
         let mut ptr = ptr::null::<c_char>();
 
         unsafe {
-            if curl_sys::curl_easy_getinfo(self.handle, curl_sys::CURLINFO_EFFECTIVE_URL, &mut ptr)
+            if curl_sys::curl_easy_getinfo(self.handle, info, &mut ptr)
                 != curl_sys::CURLE_OK
             {
                 return None;
diff --git a/src/interceptor/context.rs b/src/interceptor/context.rs
index eee6400..3eb3174 100644
--- a/src/interceptor/context.rs
+++ b/src/interceptor/context.rs
@@ -8,11 +8,11 @@ use std::{
 
 /// Execution context for an interceptor.
 pub struct Context<'a> {
-    pub(crate) invoker: Arc<dyn (Fn(Request<Body>) -> InterceptorFuture<'a, Error>) + Send + Sync + 'a>,
+    pub(crate) invoker: Arc<dyn Invoke + Send + Sync + 'a>,
     pub(crate) interceptors: &'a [InterceptorObj],
 }
 
-impl Context<'_> {
+impl<'a> Context<'a> {
     /// Send a request asynchronously, executing the next interceptor in the
     /// chain, if any.
     pub async fn send(&self, request: Request<Body>) -> Result<Response<Body>, Error> {
@@ -35,7 +35,7 @@ impl Context<'_> {
                 },
             }
         } else {
-            (self.invoker)(request).await
+            self.invoker.invoke(request).await
         }
     }
 }
@@ -45,3 +45,7 @@ impl fmt::Debug for Context<'_> {
         f.debug_struct("Context").finish()
     }
 }
+
+pub(crate) trait Invoke {
+    fn invoke<'a>(&'a self, request: Request<Body>) -> InterceptorFuture<'a, Error>;
+}
diff --git a/src/interceptor/mod.rs b/src/interceptor/mod.rs
index b7b0f83..0f563c9 100644
--- a/src/interceptor/mod.rs
+++ b/src/interceptor/mod.rs
@@ -39,7 +39,10 @@ mod context;
 mod obj;
 
 pub use self::context::Context;
-pub(crate) use self::obj::InterceptorObj;
+pub(crate) use self::{
+    context::Invoke,
+    obj::InterceptorObj,
+};
 
 /// Defines an inline interceptor using a closure-like syntax.
 ///
diff --git a/src/lib.rs b/src/lib.rs
index 1294b2e..31d38ba 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -234,6 +234,7 @@ mod error;
 mod handler;
 mod headers;
 mod metrics;
+mod redirect;
 mod request;
 mod response;
 mod task;
diff --git a/src/redirect.rs b/src/redirect.rs
new file mode 100644
index 0000000..bf16f18
--- /dev/null
+++ b/src/redirect.rs
@@ -0,0 +1,117 @@
+use crate::{
+    config::RedirectPolicy,
+    handler::RequestBody,
+    interceptor::{Context, Interceptor, InterceptorFuture},
+    request::RequestExt,
+    Body,
+    Error,
+};
+use http::{Request, Uri};
+
+/// 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);
+
+/// 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> {
+        Box::pin(async move {
+            // Get the redirect policy for this request.
+            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 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;
+                }
+
+                // 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"))));
+                }
+
+                let request = request_builder.uri(redirect_uri)
+                    .body(request_body)?;
+
+                // Keep another clone of the request around again, in case we
+                // need to follow yet another redirect.
+                request_builder = request.to_builder();
+
+                // Send the redirected request.
+                response = ctx.send(request).await?;
+
+                redirect_count += 1;
+            }
+
+            Ok(response)
+        })
+    }
+}
diff --git a/src/request.rs b/src/request.rs
index fcf3dd4..71da3bb 100644
--- a/src/request.rs
+++ b/src/request.rs
@@ -7,6 +7,12 @@ use http::{Request, Response};
 
 /// Extension methods on an HTTP request.
 pub trait RequestExt<T> {
+    /// Create a new request builder with the method, URI, and headers cloned
+    /// from this request.
+    ///
+    /// Note that third-party extensions are not cloned.
+    fn to_builder(&self) -> http::request::Builder;
+
     /// Send the HTTP request synchronously using the default client.
     ///
     /// This is a convenience method that is equivalent to
@@ -40,6 +46,64 @@ pub trait RequestExt<T> {
 }
 
 impl<T> RequestExt<T> for Request<T> {
+    fn to_builder(&self) -> http::request::Builder {
+        let mut builder = Request::builder()
+            .method(self.method().clone())
+            .uri(self.uri().clone())
+            .version(self.version());
+
+        *builder.headers_mut().unwrap() = self.headers().clone();
+
+        // Clone known extensions.
+        macro_rules! try_clone_extension {
+            ($extensions:expr, $builder:expr, [$($ty:ty,)*]) => {{
+                let extensions = $extensions;
+                $(
+                    if let Some(extension) = extensions.get::<$ty>() {
+                        $builder = $builder.extension(extension.clone());
+                    }
+                )*
+            }}
+        }
+
+        try_clone_extension!(
+            self.extensions(),
+            builder,
+            [
+                crate::config::Timeout,
+                crate::config::ConnectTimeout,
+                crate::config::TcpKeepAlive,
+                crate::config::TcpNoDelay,
+                crate::config::NetworkInterface,
+                crate::config::Dialer,
+                crate::config::RedirectPolicy,
+                crate::config::redirect::AutoReferer,
+                crate::config::AutomaticDecompression,
+                crate::auth::Authentication,
+                crate::auth::Credentials,
+                crate::config::MaxAgeConn,
+                crate::config::MaxUploadSpeed,
+                crate::config::MaxDownloadSpeed,
+                crate::config::VersionNegotiation,
+                crate::config::proxy::Proxy<Option<http::Uri>>,
+                crate::config::proxy::Blacklist,
+                crate::config::proxy::Proxy<crate::auth::Authentication>,
+                crate::config::proxy::Proxy<crate::auth::Credentials>,
+                crate::config::DnsCache,
+                crate::config::dns::ResolveMap,
+                crate::config::dns::Servers,
+                crate::config::ssl::Ciphers,
+                crate::config::ClientCertificate,
+                crate::config::CaCertificate,
+                crate::config::SslOption,
+                crate::config::CloseConnection,
+                crate::config::EnableMetrics,
+            ]
+        );
+
+        builder
+    }
+
     fn send(self) -> Result<Response<Body>, Error>
     where
         T: Into<Body>,


Test patch:
diff --git a/tests/redirects.rs b/tests/redirects.rs
index 23542c8..da7c6ab 100644
--- a/tests/redirects.rs
+++ b/tests/redirects.rs
@@ -2,6 +2,7 @@ use isahc::{
     config::RedirectPolicy,
     prelude::*,
 };
+use test_case::test_case;
 use testserver::mock;
 
 #[test]
@@ -88,13 +89,15 @@ fn headers_are_reset_every_redirect() {
     assert!(!m2.requests().is_empty());
 }
 
-#[test]
-fn _303_redirect_changes_post_to_get() {
+#[test_case(301)]
+#[test_case(302)]
+#[test_case(303)]
+fn redirect_changes_post_to_get(status: u16) {
     let m2 = mock!();
     let location = m2.url();
 
     let m1 = mock! {
-        status: 303,
+        status: status,
         headers {
             "Location": location,
         }
@@ -114,6 +117,58 @@ fn _303_redirect_changes_post_to_get() {
     assert_eq!(m2.request().method, "GET");
 }
 
+#[test_case(307)]
+#[test_case(308)]
+fn redirect_also_sends_post(status: u16) {
+    let m2 = mock!();
+    let location = m2.url();
+
+    let m1 = mock! {
+        status: status,
+        headers {
+            "Location": location,
+        }
+    };
+
+    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, "POST");
+}
+
+#[test]
+fn redirect_non_rewindable_body_returns_error() {
+    let m2 = mock!();
+    let location = m2.url();
+
+    let m1 = mock! {
+        status: 307,
+        headers {
+            "Location": location,
+        }
+    };
+
+    // Create a streaming body of unknown size.
+    let upload_stream = Body::from_reader(Body::from_bytes(b"hello world"));
+
+    let result = Request::post(m1.url())
+        .redirect_policy(RedirectPolicy::Follow)
+        .body(upload_stream)
+        .unwrap()
+        .send();
+
+    assert!(matches!(result, Err(isahc::Error::RequestBodyError(_))));
+    assert_eq!(m1.request().method, "POST");
+}
+
 #[test]
 fn redirect_limit_is_respected() {
     let m = mock! {