From 0a327687519d88f29f652a0c741332aac60b13b3 Mon Sep 17 00:00:00 2001 From: Sean McArthur Date: Fri, 5 May 2017 11:30:01 -0700 Subject: [PATCH] fix panic from Gzip reading an empty stream Closes #82 --- src/response.rs | 144 +++++++++++++++++++++++++++++++++++++----------- tests/client.rs | 59 ++++++++++++++++++++ 2 files changed, 171 insertions(+), 32 deletions(-) diff --git a/src/response.rs b/src/response.rs index d7226a728..5b2e938aa 100644 --- a/src/response.rs +++ b/src/response.rs @@ -4,6 +4,7 @@ use std::io::{self, Read}; use hyper::header::{Headers, ContentEncoding, ContentLength, Encoding, TransferEncoding}; use hyper::status::StatusCode; use hyper::version::HttpVersion; +use libflate::gzip; use serde::Deserialize; use serde_json; @@ -21,19 +22,20 @@ 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("status", &hyper_response.status) .field("headers", &hyper_response.headers) .field("version", &hyper_response.version) .finish() }, - &Decoder::Gzip{ref status, ref version, ref headers, ..} => { + Decoder::Gzip{ ref head, .. } | + Decoder::Errored { ref head, .. } => { f.debug_struct("Response") - .field("status", &status) - .field("headers", &headers) - .field("version", &version) + .field("status", &head.status) + .field("headers", &head.headers) + .field("version", &head.version) .finish() } } @@ -44,27 +46,30 @@ impl Response { /// 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, } } @@ -80,10 +85,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>, - headers: ::hyper::header::Headers, - version: ::hyper::version::HttpVersion, - status: ::hyper::status::StatusCode, + decoder: gzip::Decoder, + 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, + head: Head, } } @@ -107,10 +116,6 @@ impl Decoder { encs.contains(&Encoding::Gzip) }) }; - if content_encoding_gzip { - res.headers.remove::(); - res.headers.remove::(); - } if is_gzip { if let Some(content_length) = res.headers.get::() { if content_length.0 == 0 { @@ -119,32 +124,107 @@ impl Decoder { } } } + if content_encoding_gzip { + res.headers.remove::(); + res.headers.remove::(); + } if is_gzip { - return Decoder::Gzip { - 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, + version: res.version, + } + }, + } + + let head = Head { + headers: res.headers.clone(), + status: res.status, + 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, + version: ::hyper::version::HttpVersion, + status: ::hyper::status::StatusCode, +} + +struct Peeked { + peeked: Option, + inner: ::hyper::client::Response, +} + +impl Read for Peeked { + #[inline] + fn read(&mut self, buf: &mut [u8]) -> io::Result { + 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 { - 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] diff --git a/tests/client.rs b/tests/client.rs index 7af28a6c7..c02d48c99 100644 --- a/tests/client.rs +++ b/tests/client.rs @@ -331,3 +331,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(); +}