Repository: seanmonstar/reqwest (language: rust)

Issue:
Reqwest panic due to invalid uri on redirects
The following code panics due to InvalidUri(InvalidUriChar) even though the URL is valid.

Opening the URL on the browser it turns out that the page redirect to "http://www.eishockey.at{request_uri}/" which may be actually invalid which may cause the panic.

```
#[macro_use]
extern crate error_chain;

error_chain! {
    foreign_links {
        Reqwest(reqwest::Error);
    }
}

fn main() -> Result<()> {
    // This panics
    let body = reqwest::get("http://eishockey.at")?.text()?;

    // This does not panic because the redirect does not happen
    //let body = reqwest::get("http://www.eishockey.at")?.text()?;

    println!("body = {:?}", body);

    Ok(())
}
```

The backtrace is as follows:
```
thread 'reqwest-internal-sync-runtime' panicked at 'a parsed Url should always be a valid Uri: InvalidUri(InvalidUriChar)', src/libcore/result.rs:997:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:39
   1: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:70
   2: std::panicking::default_hook::{{closure}}
             at src/libstd/sys_common/backtrace.rs:58
             at src/libstd/panicking.rs:200
   3: std::panicking::default_hook
             at src/libstd/panicking.rs:215
   4: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:478
   5: std::panicking::continue_panic_fmt
             at src/libstd/panicking.rs:385
   6: rust_begin_unwind
             at src/libstd/panicking.rs:312
   7: core::panicking::panic_fmt
             at src/libcore/panicking.rs:85
   8: core::result::unwrap_failed
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libcore/macros.rs:16
   9: <core::result::Result<T, E>>::expect
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libcore/result.rs:825
  10: reqwest::into_url::to_uri
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.9.12/src/into_url.rs:42
  11: <reqwest::async_impl::client::PendingRequest as futures::future::Future>::poll
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.9.12/src/async_impl/client.rs:736
  12: <reqwest::async_impl::client::Pending as futures::future::Future>::poll
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.9.12/src/async_impl/client.rs:661
  13: reqwest::client::ClientHandle::new::{{closure}}::{{closure}}::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.9.12/src/client.rs:562
  14: <futures::future::poll_fn::PollFn<F> as futures::future::Future>::poll
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/future/poll_fn.rs:43
  15: <alloc::boxed::Box<F> as futures::future::Future>::poll
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/future/mod.rs:113
  16: <futures::task_impl::Spawn<T>>::poll_future_notify::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/task_impl/mod.rs:326
  17: <futures::task_impl::Spawn<T>>::enter::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/task_impl/mod.rs:396
  18: futures::task_impl::std::set
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/task_impl/std/mod.rs:78
  19: <futures::task_impl::Spawn<T>>::enter
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/task_impl/mod.rs:396
  20: <futures::task_impl::Spawn<T>>::poll_fn_notify
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/task_impl/mod.rs:288
  21: <futures::task_impl::Spawn<T>>::poll_future_notify
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/task_impl/mod.rs:326
  22: <tokio_current_thread::scheduler::Scheduled<'a, U>>::tick
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-current-thread-0.1.6/src/scheduler.rs:351
  23: <tokio_current_thread::scheduler::Scheduler<U>>::tick::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-current-thread-0.1.6/src/scheduler.rs:330
  24: <tokio_current_thread::Borrow<'a, U>>::enter::{{closure}}::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-current-thread-0.1.6/src/lib.rs:788
  25: tokio_current_thread::CurrentRunner::set_spawn
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-current-thread-0.1.6/src/lib.rs:825
  26: <tokio_current_thread::Borrow<'a, U>>::enter::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-current-thread-0.1.6/src/lib.rs:788
  27: <std::thread::local::LocalKey<T>>::try_with
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libstd/thread/local.rs:299
  28: <std::thread::local::LocalKey<T>>::with
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libstd/thread/local.rs:245
  29: <tokio_current_thread::Borrow<'a, U>>::enter
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-current-thread-0.1.6/src/lib.rs:786
  30: <tokio_current_thread::scheduler::Scheduler<U>>::tick
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-current-thread-0.1.6/src/scheduler.rs:330
  31: <tokio_current_thread::Entered<'a, P>>::tick
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-current-thread-0.1.6/src/lib.rs:612
  32: <tokio_current_thread::Entered<'a, P>>::block_on
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-current-thread-0.1.6/src/lib.rs:502
  33: tokio::runtime::current_thread::runtime::Runtime::block_on::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-0.1.18/src/runtime/current_thread/runtime.rs:200
  34: tokio::runtime::current_thread::runtime::Runtime::enter::{{closure}}::{{closure}}::{{closure}}::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-0.1.18/src/runtime/current_thread/runtime.rs:241
  35: tokio_executor::global::with_default::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-executor-0.1.7/src/global.rs:209
  36: <std::thread::local::LocalKey<T>>::try_with
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libstd/thread/local.rs:299
  37: <std::thread::local::LocalKey<T>>::with
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libstd/thread/local.rs:245
  38: tokio_executor::global::with_default
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-executor-0.1.7/src/global.rs:178
  39: tokio::runtime::current_thread::runtime::Runtime::enter::{{closure}}::{{closure}}::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-0.1.18/src/runtime/current_thread/runtime.rs:239
  40: tokio_timer::timer::handle::with_default::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-timer-0.2.10/src/timer/handle.rs:94
  41: <std::thread::local::LocalKey<T>>::try_with
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libstd/thread/local.rs:299
  42: <std::thread::local::LocalKey<T>>::with
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libstd/thread/local.rs:245
  43: tokio_timer::timer::handle::with_default
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-timer-0.2.10/src/timer/handle.rs:81
  44: tokio::runtime::current_thread::runtime::Runtime::enter::{{closure}}::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-0.1.18/src/runtime/current_thread/runtime.rs:232
  45: tokio_timer::clock::clock::with_default::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-timer-0.2.10/src/clock/clock.rs:141
  46: <std::thread::local::LocalKey<T>>::try_with
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libstd/thread/local.rs:299
  47: <std::thread::local::LocalKey<T>>::with
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libstd/thread/local.rs:245
  48: tokio_timer::clock::clock::with_default
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-timer-0.2.10/src/clock/clock.rs:124
  49: tokio::runtime::current_thread::runtime::Runtime::enter::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-0.1.18/src/runtime/current_thread/runtime.rs:231
  50: tokio_reactor::with_default::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-reactor-0.1.9/src/lib.rs:237
  51: <std::thread::local::LocalKey<T>>::try_with
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libstd/thread/local.rs:299
  52: <std::thread::local::LocalKey<T>>::with
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libstd/thread/local.rs:245
  53: tokio_reactor::with_default
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-reactor-0.1.9/src/lib.rs:217
  54: tokio::runtime::current_thread::runtime::Runtime::enter
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-0.1.18/src/runtime/current_thread/runtime.rs:230
  55: tokio::runtime::current_thread::runtime::Runtime::block_on
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-0.1.18/src/runtime/current_thread/runtime.rs:198
  56: reqwest::client::ClientHandle::new::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.9.12/src/client.rs:581
thread 'main' panicked at 'event loop thread panicked', /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.9.12/src/client.rs:665:5
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:39
   1: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:70
   2: std::panicking::default_hook::{{closure}}
             at src/libstd/sys_common/backtrace.rs:58
             at src/libstd/panicking.rs:200
   3: std::panicking::default_hook
             at src/libstd/panicking.rs:215
   4: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:478
   5: std::panicking::begin_panic
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libstd/panicking.rs:412
   6: reqwest::client::event_loop_panicked
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.9.12/src/client.rs:665
   7: reqwest::client::ClientHandle::execute_request::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.9.12/src/client.rs:622
   8: <core::result::Result<T, E>>::map_err
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libcore/result.rs:520
   9: <futures::future::map_err::MapErr<A, F> as futures::future::Future>::poll
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/future/map_err.rs:34
  10: <futures::future::join::MaybeDone<A>>::poll
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/future/join.rs:153
  11: <futures::future::join::Join<A, B> as futures::future::Future>::poll
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/future/join.rs:75
  12: <futures::future::map::Map<A, F> as futures::future::Future>::poll
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/future/map.rs:30
  13: <futures::task_impl::Spawn<T>>::poll_future_notify::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/task_impl/mod.rs:326
  14: <futures::task_impl::Spawn<T>>::enter::{{closure}}
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/task_impl/mod.rs:396
  15: futures::task_impl::std::set
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/task_impl/std/mod.rs:78
  16: <futures::task_impl::Spawn<T>>::enter
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/task_impl/mod.rs:396
  17: <futures::task_impl::Spawn<T>>::poll_fn_notify
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/task_impl/mod.rs:288
  18: <futures::task_impl::Spawn<T>>::poll_future_notify
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/futures-0.1.25/src/task_impl/mod.rs:326
  19: reqwest::wait::timeout
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.9.12/src/wait.rs:23
  20: reqwest::client::ClientHandle::execute_request
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.9.12/src/client.rs:626
  21: reqwest::client::Client::execute
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.9.12/src/client.rs:477
  22: reqwest::request::RequestBuilder::send
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.9.12/src/request.rs:502
  23: reqwest::get
             at /home/abdullah/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.9.12/src/lib.rs:301
  24: reqwest_test::main
             at src/main.rs:12
  25: std::rt::lang_start::{{closure}}
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libstd/rt.rs:64
  26: std::panicking::try::do_call
             at src/libstd/rt.rs:49
             at src/libstd/panicking.rs:297
  27: __rust_maybe_catch_panic
             at src/libpanic_unwind/lib.rs:92
  28: std::rt::lang_start_internal
             at src/libstd/panicking.rs:276
             at src/libstd/panic.rs:388
             at src/libstd/rt.rs:48
  29: std::rt::lang_start
             at /rustc/2aa4c46cfdd726e97360c2734835aa3515e8c858/src/libstd/rt.rs:64
  30: main
  31: __libc_start_main
  32: _start

```


Discussion / hints:

Fix patch:
diff --git a/src/async_impl/client.rs b/src/async_impl/client.rs
index 07438cf37..3f1287566 100644
--- a/src/async_impl/client.rs
+++ b/src/async_impl/client.rs
@@ -30,7 +30,7 @@ use native_tls::TlsConnector;
 use super::request::{Request, RequestBuilder};
 use super::response::Response;
 use connect::Connector;
-use into_url::to_uri;
+use into_url::{expect_uri, try_uri};
 use redirect::{self, RedirectPolicy, remove_sensitive_headers};
 use {IntoUrl, Method, Proxy, StatusCode, Url};
 #[cfg(feature = "tls")]
@@ -520,7 +520,7 @@ impl Client {
             headers.insert(ACCEPT_ENCODING, HeaderValue::from_static("gzip"));
         }
 
-        let uri = to_uri(&url);
+        let uri = expect_uri(&url);
 
         let (reusable, body) = match body {
             Some(body) => {
@@ -709,6 +709,17 @@ impl Future for PendingRequest {
                             self.url.join(str::from_utf8(val.as_bytes()).ok()?).ok()
                         })();
 
+                        // Check that the `url` is also a valid `http::Uri`.
+                        //
+                        // If not, just log it and skip the redirect.
+                        let loc = loc.and_then(|url| {
+                            if try_uri(&url).is_some() {
+                                Some(url)
+                            } else {
+                                None
+                            }
+                        });
+
                         if loc.is_none() {
                             debug!("Location header had invalid URI: {:?}", val);
                         }
@@ -733,7 +744,7 @@ impl Future for PendingRequest {
 
                             remove_sensitive_headers(&mut self.headers, &self.url, &self.urls);
                             debug!("redirecting to {:?} '{}'", self.method, self.url);
-                            let uri = to_uri(&self.url);
+                            let uri = expect_uri(&self.url);
                             let body = match self.body {
                                 Some(Some(ref body)) => ::hyper::Body::from(body.clone()),
                                 _ => ::hyper::Body::empty(),
diff --git a/src/into_url.rs b/src/into_url.rs
index bc7ab2c04..97502ca9c 100644
--- a/src/into_url.rs
+++ b/src/into_url.rs
@@ -38,10 +38,14 @@ impl<'a> PolyfillTryInto for &'a String {
     }
 }
 
-pub(crate) fn to_uri(url: &Url) -> ::hyper::Uri {
+pub(crate) fn expect_uri(url: &Url) -> ::hyper::Uri {
     url.as_str().parse().expect("a parsed Url should always be a valid Uri")
 }
 
+pub(crate) fn try_uri(url: &Url) -> Option<::hyper::Uri> {
+    url.as_str().parse().ok()
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
diff --git a/src/proxy.rs b/src/proxy.rs
index 1845d903d..79261869b 100644
--- a/src/proxy.rs
+++ b/src/proxy.rs
@@ -56,7 +56,7 @@ impl Proxy {
     /// # fn main() {}
     /// ```
     pub fn http<U: IntoUrl>(url: U) -> ::Result<Proxy> {
-        let uri = ::into_url::to_uri(&url.into_url()?);
+        let uri = ::into_url::expect_uri(&url.into_url()?);
         Ok(Proxy::new(Intercept::Http(uri)))
     }
 
@@ -75,7 +75,7 @@ impl Proxy {
     /// # fn main() {}
     /// ```
     pub fn https<U: IntoUrl>(url: U) -> ::Result<Proxy> {
-        let uri = ::into_url::to_uri(&url.into_url()?);
+        let uri = ::into_url::expect_uri(&url.into_url()?);
         Ok(Proxy::new(Intercept::Https(uri)))
     }
 
@@ -94,7 +94,7 @@ impl Proxy {
     /// # fn main() {}
     /// ```
     pub fn all<U: IntoUrl>(url: U) -> ::Result<Proxy> {
-        let uri = ::into_url::to_uri(&url.into_url()?);
+        let uri = ::into_url::expect_uri(&url.into_url()?);
         Ok(Proxy::new(Intercept::All(uri)))
     }
 
@@ -200,7 +200,7 @@ impl Proxy {
                         .parse()
                         .expect("should be valid Url")
                 )
-                    .map(|u| into_url::to_uri(&u) )
+                    .map(|u| into_url::expect_uri(&u) )
             },
         }
     }


Test patch:
diff --git a/tests/redirect.rs b/tests/redirect.rs
index 593f62043..0eb2d7945 100644
--- a/tests/redirect.rs
+++ b/tests/redirect.rs
@@ -358,3 +358,32 @@ fn test_referer_is_not_set_if_disabled() {
         .send()
         .unwrap();
 }
+
+#[test]
+fn test_invalid_location_stops_redirect_gh484() {
+    let server = server! {
+        request: b"\
+            GET /yikes HTTP/1.1\r\n\
+            user-agent: $USERAGENT\r\n\
+            accept: */*\r\n\
+            accept-encoding: gzip\r\n\
+            host: $HOST\r\n\
+            \r\n\
+            ",
+        response: b"\
+            HTTP/1.1 302 Found\r\n\
+            Server: test-yikes\r\n\
+            Location: http://www.yikes{KABOOM}\r\n\
+            Content-Length: 0\r\n\
+            \r\n\
+            "
+    };
+
+    let url = format!("http://{}/yikes", server.addr());
+
+    let res = reqwest::get(&url).unwrap();
+
+    assert_eq!(res.url().as_str(), url);
+    assert_eq!(res.status(), reqwest::StatusCode::FOUND);
+    assert_eq!(res.headers().get(reqwest::header::SERVER).unwrap(), &"test-yikes");
+}