Fix panicking when passed a file:// URL

Closes #347
seanmonstar · Sep 18, 2018 · 610cdd2 · 610cdd2
1 parent 68d012e
commit 610cdd2
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 20 deletions.
diff --git a/src/async_impl/client.rs b/src/async_impl/client.rs
@@ -291,10 +291,9 @@ impl Client {
     ///
     /// This method fails whenever supplied `Url` cannot be parsed.
     pub fn request<U: IntoUrl>(&self, method: Method, url: U) -> RequestBuilder {
-        let req = match url.into_url() {
-            Ok(url) => Ok(Request::new(method, url)),
-            Err(err) => Err(::error::from(err)),
-        };
+        let req = url
+            .into_url()
+            .map(move |url| Request::new(method, url));
         RequestBuilder::new(self.clone(), req)
     }
 

diff --git a/src/client.rs b/src/client.rs
@@ -339,10 +339,9 @@ impl Client {
     ///
     /// This method fails whenever supplied `Url` cannot be parsed.
     pub fn request<U: IntoUrl>(&self, method: Method, url: U) -> RequestBuilder {
-        let req = match url.into_url() {
-            Ok(url) => Ok(Request::new(method, url)),
-            Err(err) => Err(::error::from(err)),
-        };
+        let req = url
+            .into_url()
+            .map(move |url| Request::new(method, url));
         RequestBuilder::new(self.clone(), req)
     }
 

diff --git a/src/error.rs b/src/error.rs
@@ -119,6 +119,7 @@ impl Error {
             Kind::Io(ref e) => Some(e),
             Kind::UrlEncoded(ref e) => Some(e),
             Kind::Json(ref e) => Some(e),
+            Kind::UrlBadScheme |
             Kind::TooManyRedirects |
             Kind::RedirectLoop |
             Kind::ClientError(_) |
@@ -196,6 +197,7 @@ impl fmt::Display for Error {
             Kind::Hyper(ref e) => fmt::Display::fmt(e, f),
             Kind::Mime(ref e) => fmt::Display::fmt(e, f),
             Kind::Url(ref e) => fmt::Display::fmt(e, f),
+            Kind::UrlBadScheme => f.write_str("URL scheme is not allowed"),
             Kind::Tls(ref e) => fmt::Display::fmt(e, f),
             Kind::Io(ref e) => fmt::Display::fmt(e, f),
             Kind::UrlEncoded(ref e) => fmt::Display::fmt(e, f),
@@ -221,6 +223,7 @@ impl StdError for Error {
             Kind::Hyper(ref e) => e.description(),
             Kind::Mime(ref e) => e.description(),
             Kind::Url(ref e) => e.description(),
+            Kind::UrlBadScheme => "URL scheme is not allowed",
             Kind::Tls(ref e) => e.description(),
             Kind::Io(ref e) => e.description(),
             Kind::UrlEncoded(ref e) => e.description(),
@@ -242,6 +245,7 @@ impl StdError for Error {
             Kind::Io(ref e) => e.cause(),
             Kind::UrlEncoded(ref e) => e.cause(),
             Kind::Json(ref e) => e.cause(),
+            Kind::UrlBadScheme |
             Kind::TooManyRedirects |
             Kind::RedirectLoop |
             Kind::ClientError(_) |
@@ -258,6 +262,7 @@ pub(crate) enum Kind {
     Hyper(::hyper::Error),
     Mime(::mime::FromStrError),
     Url(::url::ParseError),
+    UrlBadScheme,
     Tls(::native_tls::Error),
     Io(io::Error),
     UrlEncoded(::serde_urlencoded::ser::Error),
@@ -438,6 +443,13 @@ pub(crate) fn server_error(url: Url, status: StatusCode) -> Error {
     }
 }
 
+pub(crate) fn url_bad_scheme(url: Url) -> Error {
+    Error {
+        kind: Kind::UrlBadScheme,
+        url: Some(url),
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

diff --git a/src/into_url.rs b/src/into_url.rs
@@ -1,4 +1,4 @@
-use url::{Url, ParseError};
+use url::Url;
 
 /// A trait to try to convert some type into a `Url`.
 ///
@@ -12,27 +12,47 @@ impl<T: PolyfillTryInto> IntoUrl for T {}
 // pub(crate)
 
 pub trait PolyfillTryInto {
-    fn into_url(self) -> Result<Url, ParseError>;
+    // Besides parsing as a valid `Url`, the `Url` must be a valid
+    // `http::Uri`, in that it makes sense to use in a network request.
+    fn into_url(self) -> ::Result<Url>;
 }
 
 impl PolyfillTryInto for Url {
-    fn into_url(self) -> Result<Url, ParseError> {
-        Ok(self)
+    fn into_url(self) -> ::Result<Url> {
+        if self.has_host() {
+            Ok(self)
+        } else {
+            Err(::error::url_bad_scheme(self))
+        }
     }
 }
 
 impl<'a> PolyfillTryInto for &'a str {
-    fn into_url(self) -> Result<Url, ParseError> {
-        Url::parse(self)
+    fn into_url(self) -> ::Result<Url> {
+        try_!(Url::parse(self))
+            .into_url()
     }
 }
 
 impl<'a> PolyfillTryInto for &'a String {
-    fn into_url(self) -> Result<Url, ParseError> {
-        Url::parse(self)
+    fn into_url(self) -> ::Result<Url> {
+        (&**self).into_url()
     }
 }
 
-pub fn to_uri(url: &Url) -> ::hyper::Uri {
+pub(crate) fn to_uri(url: &Url) -> ::hyper::Uri {
     url.as_str().parse().expect("a parsed Url should always be a valid Uri")
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn into_url_file_scheme() {
+        let err = "file:///etc/hosts"
+            .into_url()
+            .unwrap_err();
+        assert_eq!(err.to_string(), "file:///etc/hosts: URL scheme is not allowed");
+    }
+}
diff --git a/src/proxy.rs b/src/proxy.rs
@@ -49,7 +49,7 @@ impl Proxy {
     /// # fn main() {}
     /// ```
     pub fn http<U: IntoUrl>(url: U) -> ::Result<Proxy> {
-        let uri = ::into_url::to_uri(&try_!(url.into_url()));
+        let uri = ::into_url::to_uri(&url.into_url()?);
         Ok(Proxy::new(Intercept::Http(uri)))
     }
 
@@ -68,7 +68,7 @@ impl Proxy {
     /// # fn main() {}
     /// ```
     pub fn https<U: IntoUrl>(url: U) -> ::Result<Proxy> {
-        let uri = ::into_url::to_uri(&try_!(url.into_url()));
+        let uri = ::into_url::to_uri(&url.into_url()?);
         Ok(Proxy::new(Intercept::Https(uri)))
     }
 
@@ -87,7 +87,7 @@ impl Proxy {
     /// # fn main() {}
     /// ```
     pub fn all<U: IntoUrl>(url: U) -> ::Result<Proxy> {
-        let uri = ::into_url::to_uri(&try_!(url.into_url()));
+        let uri = ::into_url::to_uri(&url.into_url()?);
         Ok(Proxy::new(Intercept::All(uri)))
     }