Merge pull request #83 from seanmonstar/gzip-empty

fix panic from Gzip reading an empty stream

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 {
+        match self.inner {
-            &Decoder::PlainText(ref hyper_response) => {
+            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("url", &head.url)
-                    .field("status", &status)
+                    .field("status", &head.status)
-                    .field("headers", &headers)
+                    .field("headers", &head.headers)
-                    .field("version", &version)
+                    .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 {
+        match self.inner {
-            &Decoder::PlainText(ref hyper_response) => &hyper_response.url,
+            Decoder::PlainText(ref hyper_response) => &hyper_response.url,
-            &Decoder::Gzip{ref url, ..} => url,
+            Decoder::Gzip{ ref head, .. } |
+            Decoder::Errored { ref head, .. } => &head.url,
         }
     }
 
     /// Get the `StatusCode`.
     #[inline]
     pub fn status(&self) -> &StatusCode {
-        match &self.inner {
+        match self.inner {
-            &Decoder::PlainText(ref hyper_response) => &hyper_response.status,
+            Decoder::PlainText(ref hyper_response) => &hyper_response.status,
-            &Decoder::Gzip{ref status, ..} => status
+            Decoder::Gzip{ ref head, .. } |
+            Decoder::Errored { ref head, .. } => &head.status,
         }
     }
 
     /// Get the `Headers`.
     #[inline]
     pub fn headers(&self) -> &Headers {
-        match &self.inner {
+        match self.inner {
-            &Decoder::PlainText(ref hyper_response) => &hyper_response.headers,
+            Decoder::PlainText(ref hyper_response) => &hyper_response.headers,
-            &Decoder::Gzip{ref headers, ..} => headers
+            Decoder::Gzip{ ref head, .. } |
+            Decoder::Errored { ref head, .. } => &head.headers,
         }
     }
 
     /// Get the `HttpVersion`.
     #[inline]
     pub fn version(&self) -> &HttpVersion {
-        match &self.inner {
+        match self.inner {
-            &Decoder::PlainText(ref hyper_response) => &hyper_response.version,
+            Decoder::PlainText(ref hyper_response) => &hyper_response.version,
-            &Decoder::Gzip{ref version, ..} => 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>,
+        decoder: gzip::Decoder<Peeked>,
-        url: ::hyper::Url,
+        head: Head,
-        headers: ::hyper::header::Headers,
+    },
-        version: ::hyper::version::HttpVersion,
+    /// An error occured reading the Gzip header, so return that error
-        status: ::hyper::status::StatusCode,
+    /// 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 {
+            new_gzip(res)
-                url: res.url.clone(),
-                status: res.status.clone(),
-                version: res.version.clone(),
-                headers: res.headers.clone(),
-                decoder: ::libflate::gzip::Decoder::new(res).unwrap(),
-            };
         } else {
-            return Decoder::PlainText(res);
+            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(),
+                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 {
+            self.inner.read(buf)
         }
     }
 }
 
 impl Read for Decoder {
     fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
-        match self {
+        match *self {
-            &mut Decoder::PlainText(ref mut hyper_response) => {
+            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]

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();
+}