Repository: seanmonstar/reqwest (language: rust)

Issue:
Panic in Client::head with automatic GZIP decompression
When using `reqwest::Client` to make a HEAD request with GZIP decompression enabled (which is the default), a panic happens in the library.

Code to reproduce:
```rust
use reqwest::Client;
fn main() {
    let url = "https://www.google.com/";
    let client = Client::new().unwrap();
    client.head(url).send();
}
```

Backtrace:
```
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { repr: Custom(Custom { kind: UnexpectedEof, error: StringError("failed to fill whole buffer") }) }', /checkout/src/libcore/result.rs:859
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:355
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:371
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:549
   5: std::panicking::begin_panic
             at /checkout/src/libstd/panicking.rs:511
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:495
   7: rust_begin_unwind
             at /checkout/src/libstd/panicking.rs:471
   8: core::panicking::panic_fmt
             at /checkout/src/libcore/panicking.rs:69
   9: core::result::unwrap_failed
             at /checkout/src/libcore/macros.rs:29
  10: <core::result::Result<T, E>>::unwrap
             at /checkout/src/libcore/result.rs:737
  11: reqwest::client::Decoder::from_hyper_response
             at /home/martin/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.5.1/src/client.rs:458
  12: reqwest::client::RequestBuilder::send
             at /home/martin/.cargo/registry/src/github.com-1ecc6299db9ec823/reqwest-0.5.1/src/client.rs:331
  13: main::bug_repro
             at ./src/main.rs:41
  14: main::main
             at ./src/main.rs:47
  15: std::panicking::try::do_call
             at /checkout/src/libstd/panicking.rs:454
  16: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
  17: std::rt::lang_start
             at /checkout/src/libstd/panicking.rs:433
             at /checkout/src/libstd/panic.rs:361
             at /checkout/src/libstd/rt.rs:57
  18: main
  19: __libc_start_main
  20: _start
```
Here's the debug log: [log.txt](https://github.com/seanmonstar/reqwest/files/979368/log.txt)
When I disable automatic decompression (`client.gzip(false)`), it works fine. 


Discussion / hints:

Fix patch:
diff --git a/src/response.rs b/src/response.rs
index 1b7ce03c9..a488c52bf 100644
--- a/src/response.rs
+++ b/src/response.rs
@@ -5,6 +5,7 @@ use hyper::header::{Headers, ContentEncoding, ContentLength, Encoding, TransferE
 use hyper::status::StatusCode;
 use hyper::version::HttpVersion;
 use hyper::Url;
+use libflate::gzip;
 use serde::Deserialize;
 use serde_json;
 
@@ -22,8 +23,8 @@ pub fn new(res: ::hyper::client::Response, gzip: bool) -> Response {
 
 impl fmt::Debug for Response {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        return match &self.inner {
-            &Decoder::PlainText(ref hyper_response) => {
+        match self.inner {
+            Decoder::PlainText(ref hyper_response) => {
                 f.debug_struct("Response")
                     .field("url", &hyper_response.url)
                     .field("status", &hyper_response.status)
@@ -31,12 +32,13 @@ impl fmt::Debug for Response {
                     .field("version", &hyper_response.version)
                     .finish()
             },
-            &Decoder::Gzip{ref url, ref status, ref version, ref headers, ..} => {
+            Decoder::Gzip{ ref head, .. } |
+            Decoder::Errored { ref head, .. } => {
                 f.debug_struct("Response")
-                    .field("url", &url)
-                    .field("status", &status)
-                    .field("headers", &headers)
-                    .field("version", &version)
+                    .field("url", &head.url)
+                    .field("status", &head.status)
+                    .field("headers", &head.headers)
+                    .field("version", &head.version)
                     .finish()
             }
         }
@@ -47,36 +49,40 @@ impl Response {
     /// Get the final `Url` of this response.
     #[inline]
     pub fn url(&self) -> &Url {
-        match &self.inner {
-            &Decoder::PlainText(ref hyper_response) => &hyper_response.url,
-            &Decoder::Gzip{ref url, ..} => url,
+        match self.inner {
+            Decoder::PlainText(ref hyper_response) => &hyper_response.url,
+            Decoder::Gzip{ ref head, .. } |
+            Decoder::Errored { ref head, .. } => &head.url,
         }
     }
 
     /// Get the `StatusCode`.
     #[inline]
     pub fn status(&self) -> &StatusCode {
-        match &self.inner {
-            &Decoder::PlainText(ref hyper_response) => &hyper_response.status,
-            &Decoder::Gzip{ref status, ..} => status
+        match self.inner {
+            Decoder::PlainText(ref hyper_response) => &hyper_response.status,
+            Decoder::Gzip{ ref head, .. } |
+            Decoder::Errored { ref head, .. } => &head.status,
         }
     }
 
     /// Get the `Headers`.
     #[inline]
     pub fn headers(&self) -> &Headers {
-        match &self.inner {
-            &Decoder::PlainText(ref hyper_response) => &hyper_response.headers,
-            &Decoder::Gzip{ref headers, ..} => headers
+        match self.inner {
+            Decoder::PlainText(ref hyper_response) => &hyper_response.headers,
+            Decoder::Gzip{ ref head, .. } |
+            Decoder::Errored { ref head, .. } => &head.headers,
         }
     }
 
     /// Get the `HttpVersion`.
     #[inline]
     pub fn version(&self) -> &HttpVersion {
-        match &self.inner {
-            &Decoder::PlainText(ref hyper_response) => &hyper_response.version,
-            &Decoder::Gzip{ref version, ..} => version
+        match self.inner {
+            Decoder::PlainText(ref hyper_response) => &hyper_response.version,
+            Decoder::Gzip{ ref head, .. } |
+            Decoder::Errored { ref head, .. } => &head.version,
         }
     }
 
@@ -92,11 +98,14 @@ enum Decoder {
     PlainText(::hyper::client::Response),
     /// A `Gzip` decoder will uncompress the gziped response content before returning it.
     Gzip {
-        decoder: ::libflate::gzip::Decoder<::hyper::client::Response>,
-        url: ::hyper::Url,
-        headers: ::hyper::header::Headers,
-        version: ::hyper::version::HttpVersion,
-        status: ::hyper::status::StatusCode,
+        decoder: gzip::Decoder<Peeked>,
+        head: Head,
+    },
+    /// An error occured reading the Gzip header, so return that error
+    /// when the user tries to read on the `Response`.
+    Errored {
+        err: Option<io::Error>,
+        head: Head,
     }
 }
 
@@ -120,10 +129,6 @@ impl Decoder {
                 encs.contains(&Encoding::Gzip)
             })
         };
-        if content_encoding_gzip {
-            res.headers.remove::<ContentEncoding>();
-            res.headers.remove::<ContentLength>();
-        }
         if is_gzip {
             if let Some(content_length) = res.headers.get::<ContentLength>() {
                 if content_length.0 == 0 {
@@ -132,33 +137,110 @@ impl Decoder {
                 }
             }
         }
+        if content_encoding_gzip {
+            res.headers.remove::<ContentEncoding>();
+            res.headers.remove::<ContentLength>();
+        }
         if is_gzip {
-            return Decoder::Gzip {
-                url: res.url.clone(),
-                status: res.status.clone(),
-                version: res.version.clone(),
+            new_gzip(res)
+        } else {
+            Decoder::PlainText(res)
+        }
+    }
+}
+
+fn new_gzip(mut res: ::hyper::client::Response) -> Decoder {
+    // libflate does a read_exact([0; 2]), so its impossible to tell
+    // if the stream was empty, or truly had an UnexpectedEof.
+    // Therefore, we need to peek a byte to make check for EOF first.
+    let mut peek = [0];
+    match res.read(&mut peek) {
+        Ok(0) => return Decoder::PlainText(res),
+        Ok(n) => {
+            debug_assert_eq!(n, 1);
+        },
+        Err(e) => return Decoder::Errored {
+            err: Some(e),
+            head: Head {
                 headers: res.headers.clone(),
-                decoder: ::libflate::gzip::Decoder::new(res).unwrap(),
-            };
+                status: res.status,
+                url: res.url.clone(),
+                version: res.version,
+            }
+        },
+    }
+
+    let head = Head {
+        headers: res.headers.clone(),
+        status: res.status,
+        url: res.url.clone(),
+        version: res.version,
+    };
+
+    let reader = Peeked {
+        peeked: Some(peek[0]),
+        inner: res,
+    };
+    match gzip::Decoder::new(reader) {
+        Ok(gzip) => Decoder::Gzip {
+            decoder: gzip,
+            head: head,
+        },
+        Err(e) => Decoder::Errored {
+            err: Some(e),
+            head: head,
+        }
+    }
+}
+
+struct Head {
+    headers: ::hyper::header::Headers,
+    url: ::hyper::Url,
+    version: ::hyper::version::HttpVersion,
+    status: ::hyper::status::StatusCode,
+}
+
+struct Peeked {
+    peeked: Option<u8>,
+    inner: ::hyper::client::Response,
+}
+
+impl Read for Peeked {
+    #[inline]
+    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
+        if buf.is_empty() {
+            return Ok(0);
+        }
+        if let Some(byte) = self.peeked.take() {
+            buf[0] = byte;
+            Ok(1)
         } else {
-            return Decoder::PlainText(res);
+            self.inner.read(buf)
         }
     }
 }
 
 impl Read for Decoder {
     fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
-        match self {
-            &mut Decoder::PlainText(ref mut hyper_response) => {
+        match *self {
+            Decoder::PlainText(ref mut hyper_response) => {
                 hyper_response.read(buf)
             },
-            &mut Decoder::Gzip{ref mut decoder, ..} => {
+            Decoder::Gzip{ref mut decoder, ..} => {
                 decoder.read(buf)
+            },
+            Decoder::Errored { ref mut err, .. } => {
+                Err(err.take().unwrap_or_else(previously_errored))
             }
         }
     }
 }
 
+#[inline]
+fn previously_errored() -> io::Error {
+    io::Error::new(io::ErrorKind::Other, "permanently errored")
+}
+
 /// Read the body of the Response.
 impl Read for Response {
     #[inline]


Test patch:
diff --git a/tests/client.rs b/tests/client.rs
index dc9c8289b..a36a3ed83 100644
--- a/tests/client.rs
+++ b/tests/client.rs
@@ -344,3 +344,62 @@ fn test_gzip_response() {
 
     assert_eq!(body, "test request");
 }
+
+#[test]
+fn test_gzip_empty_body() {
+    let server = server! {
+        request: b"\
+            HEAD /gzip HTTP/1.1\r\n\
+            Host: $HOST\r\n\
+            User-Agent: $USERAGENT\r\n\
+            Accept: */*\r\n\
+            Accept-Encoding: gzip\r\n\
+            \r\n\
+            ",
+        response: b"\
+            HTTP/1.1 200 OK\r\n\
+            Server: test-accept\r\n\
+            Content-Encoding: gzip\r\n\
+            Content-Length: 100\r\n\
+            \r\n"
+    };
+
+    let client = reqwest::Client::new().unwrap();
+    let mut res = client.head(&format!("http://{}/gzip", server.addr()))
+        .send()
+        .unwrap();
+
+    let mut body = ::std::string::String::new();
+    res.read_to_string(&mut body).unwrap();
+
+    assert_eq!(body, "");
+}
+
+#[test]
+fn test_gzip_invalid_body() {
+    let server = server! {
+        request: b"\
+            GET /gzip HTTP/1.1\r\n\
+            Host: $HOST\r\n\
+            User-Agent: $USERAGENT\r\n\
+            Accept: */*\r\n\
+            Accept-Encoding: gzip\r\n\
+            \r\n\
+            ",
+        response: b"\
+            HTTP/1.1 200 OK\r\n\
+            Server: test-accept\r\n\
+            Content-Encoding: gzip\r\n\
+            Content-Length: 100\r\n\
+            \r\n\
+            0"
+    };
+
+    let mut res = reqwest::get(&format!("http://{}/gzip", server.addr()))
+        .unwrap();
+    // this tests that the request.send() didn't error, but that the error
+    // is in reading the body
+
+    let mut body = ::std::string::String::new();
+    res.read_to_string(&mut body).unwrap_err();
+}