Fix handling of requests with an encoded body with a length > 8192 #93

CHANGES.md

@@ -1,5 +1,9 @@
 # Changes
 
+## 0.4.4 (2018-03-xx)
+
+* Fix handling of requests with an encoded body with a length > 8192 #93
+
 ## 0.4.3 (2018-03-03)
 
 * Fix request body read bug

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "actix-web"
-version = "0.4.3"
+version = "0.4.4"
 authors = ["Nikolay Kim <fafhrd91@gmail.com>"]
 description = "Actix web is a small, pragmatic, extremely fast, web framework for Rust."
 readme = "README.md"

src/server/encoding.rs

@@ -11,7 +11,7 @@ use flate2::Compression;
 use flate2::read::GzDecoder;
 use flate2::write::{GzEncoder, DeflateDecoder, DeflateEncoder};
 use brotli2::write::{BrotliDecoder, BrotliEncoder};
-use bytes::{Bytes, BytesMut, BufMut, Writer};
+use bytes::{Bytes, BytesMut, BufMut};
 
 use headers::ContentEncoding;
 use body::{Body, Binary};
@@ -128,177 +128,6 @@ impl PayloadWriter for PayloadType {
     }
 }
 
-pub(crate) enum Decoder {
-    Deflate(Box<DeflateDecoder<Writer<BytesMut>>>),
-    Gzip(Option<Box<GzDecoder<Wrapper>>>),
-    Br(Box<BrotliDecoder<Writer<BytesMut>>>),
-    Identity,
-}
-
-// should go after write::GzDecoder get implemented
-#[derive(Debug)]
-pub(crate) struct Wrapper {
-    pub buf: BytesMut,
-    pub eof: bool,
-}
-
-impl io::Read for Wrapper {
-    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
-        let len = cmp::min(buf.len(), self.buf.len());
-        buf[..len].copy_from_slice(&self.buf[..len]);
-        self.buf.split_to(len);
-        if len == 0 {
-            if self.eof {
-                Ok(0)
-            } else {
-                Err(io::Error::new(io::ErrorKind::WouldBlock, ""))
-            }
-        } else {
-            Ok(len)
-        }
-    }
-}
-
-impl io::Write for Wrapper {
-    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
-        self.buf.extend_from_slice(buf);
-        Ok(buf.len())
-    }
-    fn flush(&mut self) -> io::Result<()> {
-        Ok(())
-    }
-}
-
-/// Payload stream with decompression support
-pub(crate) struct PayloadStream {
-    decoder: Decoder,
-    dst: BytesMut,
-}
-
-impl PayloadStream {
-    pub fn new(enc: ContentEncoding) -> PayloadStream {
-        let dec = match enc {
-            ContentEncoding::Br => Decoder::Br(
-                Box::new(BrotliDecoder::new(BytesMut::with_capacity(8192).writer()))),
-            ContentEncoding::Deflate => Decoder::Deflate(
-                Box::new(DeflateDecoder::new(BytesMut::with_capacity(8192).writer()))),
-            ContentEncoding::Gzip => Decoder::Gzip(None),
-            _ => Decoder::Identity,
-        };
-        PayloadStream{ decoder: dec, dst: BytesMut::new() }
-    }
-}
-
-impl PayloadStream {
-
-    pub fn feed_eof(&mut self) -> io::Result<Option<Bytes>> {
-        match self.decoder {
-            Decoder::Br(ref mut decoder) => {
-                match decoder.finish() {
-                    Ok(mut writer) => {
-                        let b = writer.get_mut().take().freeze();
-                        if !b.is_empty() {
-                            Ok(Some(b))
-                        } else {
-                            Ok(None)
-                        }
-                    },
-                    Err(err) => Err(err),
-                }
-            },
-            Decoder::Gzip(ref mut decoder) => {
-                if let Some(ref mut decoder) = *decoder {
-                    decoder.as_mut().get_mut().eof = true;
-
-                    loop {
-                        self.dst.reserve(8192);
-                        match decoder.read(unsafe{self.dst.bytes_mut()}) {
-                            Ok(n) =>  {
-                                if n == 0 {
-                                    return Ok(Some(self.dst.take().freeze()))
-                                } else {
-                                    unsafe{self.dst.set_len(n)};
-                                }
-                            }
-                            Err(err) => return Err(err),
-                        }
-                    }
-                } else {
-                    Ok(None)
-                }
-            },
-            Decoder::Deflate(ref mut decoder) => {
-                match decoder.try_finish() {
-                    Ok(_) => {
-                        let b = decoder.get_mut().get_mut().take().freeze();
-                        if !b.is_empty() {
-                            Ok(Some(b))
-                        } else {
-                            Ok(None)
-                        }
-                    },
-                    Err(err) => Err(err),
-                }
-            },
-            Decoder::Identity => Ok(None),
-        }
-    }
-
-    pub fn feed_data(&mut self, data: Bytes) -> io::Result<Option<Bytes>> {
-        match self.decoder {
-            Decoder::Br(ref mut decoder) => {
-                match decoder.write(&data).and_then(|_| decoder.flush()) {
-                    Ok(_) => {
-                        let b = decoder.get_mut().get_mut().take().freeze();
-                        if !b.is_empty() {
-                            Ok(Some(b))
-                        } else {
-                            Ok(None)
-                        }
-                    },
-                    Err(err) => Err(err)
-                }
-            },
-            Decoder::Gzip(ref mut decoder) => {
-                if decoder.is_none() {
-                    *decoder = Some(
-                        Box::new(GzDecoder::new(
-                            Wrapper{buf: BytesMut::from(data), eof: false})));
-                } else {
-                    let _ = decoder.as_mut().unwrap().write(&data);
-                }
-
-                loop {
-                    self.dst.reserve(8192);
-                    match decoder.as_mut().as_mut().unwrap().read(unsafe{self.dst.bytes_mut()}) {
-                        Ok(n) =>  {
-                            if n == 0 {
-                                return Ok(Some(self.dst.split_to(n).freeze()));
-                            } else {
-                                unsafe{self.dst.set_len(n)};
-                            }
-                        }
-                        Err(e) => return Err(e),
-                    }
-                }
-            },
-            Decoder::Deflate(ref mut decoder) => {
-                match decoder.write(&data).and_then(|_| decoder.flush()) {
-                    Ok(_) => {
-                        let b = decoder.get_mut().get_mut().take().freeze();
-                        if !b.is_empty() {
-                            Ok(Some(b))
-                        } else {
-                            Ok(None)
-                        }
-                    },
-                    Err(e) => Err(e),
-                }
-            },
-            Decoder::Identity => Ok(Some(data)),
-        }
-    }
-}
 
 /// Payload wrapper with content decompression support
 pub(crate) struct EncodedPayload {
@@ -357,6 +186,203 @@ impl PayloadWriter for EncodedPayload {
     }
 }
 
+pub(crate) enum Decoder {
+    Deflate(Box<DeflateDecoder<Writer>>),
+    Gzip(Option<Box<GzDecoder<Wrapper>>>),
+    Br(Box<BrotliDecoder<Writer>>),
+    Identity,
+}
+
+// should go after write::GzDecoder get implemented
+#[derive(Debug)]
+pub(crate) struct Wrapper {
+    pub buf: BytesMut,
+    pub eof: bool,
+}
+
+impl io::Read for Wrapper {
+    fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
+        let len = cmp::min(buf.len(), self.buf.len());
+        buf[..len].copy_from_slice(&self.buf[..len]);
+        self.buf.split_to(len);
+        if len == 0 {
+            if self.eof {
+                Ok(0)
+            } else {
+                Err(io::Error::new(io::ErrorKind::WouldBlock, ""))
+            }
+        } else {
+            Ok(len)
+        }
+    }
+}
+
+impl io::Write for Wrapper {
+    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
+        self.buf.extend_from_slice(buf);
+        Ok(buf.len())
+    }
+    fn flush(&mut self) -> io::Result<()> {
+        Ok(())
+    }
+}
+
+pub(crate) struct Writer {
+    buf: BytesMut,
+}
+
+impl Writer {
+    fn new() -> Writer {
+        Writer{buf: BytesMut::with_capacity(8192)}
+    }
+    fn take(&mut self) -> Bytes {
+        self.buf.take().freeze()
+    }
+}
+
+impl io::Write for Writer {
+    fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
+        self.buf.extend_from_slice(buf);
+        Ok(buf.len())
+    }
+    fn flush(&mut self) -> io::Result<()> {
+        Ok(())
+    }
+}
+
+/// Payload stream with decompression support
+pub(crate) struct PayloadStream {
+    decoder: Decoder,
+    dst: BytesMut,
+}
+
+impl PayloadStream {
+    pub fn new(enc: ContentEncoding) -> PayloadStream {
+        let dec = match enc {
+            ContentEncoding::Br => Decoder::Br(
+                Box::new(BrotliDecoder::new(Writer::new()))),
+            ContentEncoding::Deflate => Decoder::Deflate(
+                Box::new(DeflateDecoder::new(Writer::new()))),
+            ContentEncoding::Gzip => Decoder::Gzip(None),
+            _ => Decoder::Identity,
+        };
+        PayloadStream{ decoder: dec, dst: BytesMut::new() }
+    }
+}
+
+impl PayloadStream {
+
+    pub fn feed_eof(&mut self) -> io::Result<Option<Bytes>> {
+        match self.decoder {
+            Decoder::Br(ref mut decoder) => {
+                match decoder.finish() {
+                    Ok(mut writer) => {
+                        let b = writer.take();
+                        if !b.is_empty() {
+                            Ok(Some(b))
+                        } else {
+                            Ok(None)
+                        }
+                    },
+                    Err(e) => Err(e),
+                }
+            },
+            Decoder::Gzip(ref mut decoder) => {
+                if let Some(ref mut decoder) = *decoder {
+                    decoder.as_mut().get_mut().eof = true;
+
+                    loop {
+                        self.dst.reserve(8192);
+                        match decoder.read(unsafe{self.dst.bytes_mut()}) {
+                            Ok(n) =>  {
+                                if n == 0 {
+                                    return Ok(Some(self.dst.take().freeze()))
+                                } else {
+                                    unsafe{self.dst.advance_mut(n)};
+                                }
+                            }
+                            Err(e) => return Err(e),
+                        }
+                    }
+                } else {
+                    Ok(None)
+                }
+            },
+            Decoder::Deflate(ref mut decoder) => {
+                match decoder.try_finish() {
+                    Ok(_) => {
+                        let b = decoder.get_mut().take();
+                        if !b.is_empty() {
+                            Ok(Some(b))
+                        } else {
+                            Ok(None)
+                        }
+                    },
+                    Err(e) => Err(e),
+                }
+            },
+            Decoder::Identity => Ok(None),
+        }
+    }
+
+    pub fn feed_data(&mut self, data: Bytes) -> io::Result<Option<Bytes>> {
+        match self.decoder {
+            Decoder::Br(ref mut decoder) => {
+                match decoder.write(&data).and_then(|_| decoder.flush()) {
+                    Ok(_) => {
+                        let b = decoder.get_mut().take();
+                        if !b.is_empty() {
+                            Ok(Some(b))
+                        } else {
+                            Ok(None)
+                        }
+                    },
+                    Err(e) => Err(e)
+                }
+            },
+            Decoder::Gzip(ref mut decoder) => {
+                if decoder.is_none() {
+                    *decoder = Some(
+                        Box::new(GzDecoder::new(
+                            Wrapper{buf: BytesMut::from(data), eof: false})));
+                } else {
+                    let _ = decoder.as_mut().unwrap().write(&data);
+                }
+
+                loop {
+                    self.dst.reserve(8192);
+                    match decoder.as_mut().as_mut().unwrap().read(unsafe{self.dst.bytes_mut()}) {
+                        Ok(n) =>  {
+                            if n == 0 {
+                                return Ok(Some(self.dst.take().freeze()));
+                            } else {
+                                unsafe{self.dst.advance_mut(n)};
+                            }
+                        }
+                        Err(e) => {
+                            return Err(e)
+                        }
+                    }
+                }
+            },
+            Decoder::Deflate(ref mut decoder) => {
+                match decoder.write(&data).and_then(|_| decoder.flush()) {
+                    Ok(_) => {
+                        let b = decoder.get_mut().take();
+                        if !b.is_empty() {
+                            Ok(Some(b))
+                        } else {
+                            Ok(None)
+                        }
+                    },
+                    Err(e) => Err(e),
+                }
+            },
+            Decoder::Identity => Ok(Some(data)),
+        }
+    }
+}
+
 pub(crate) enum ContentEncoder {
     Deflate(DeflateEncoder<TransferEncoding>),
     Gzip(GzEncoder<TransferEncoding>),

tests/test_client.rs

@@ -116,6 +116,32 @@ fn test_client_gzip_encoding() {
     assert_eq!(bytes, Bytes::from_static(STR.as_ref()));
 }
 
+#[test]
+fn test_client_gzip_encoding_large() {
+    let data = STR.to_owned() + STR + STR + STR + STR + STR + STR + STR + STR + STR;
+
+    let mut srv = test::TestServer::new(|app| app.handler(|req: HttpRequest| {
+        req.body()
+            .and_then(|bytes: Bytes| {
+                Ok(httpcodes::HTTPOk
+                   .build()
+                   .content_encoding(headers::ContentEncoding::Deflate)
+                   .body(bytes))
+            }).responder()}
+    ));
+
+    // client request
+    let request = srv.post()
+        .content_encoding(headers::ContentEncoding::Gzip)
+        .body(data.clone()).unwrap();
+    let response = srv.execute(request.send()).unwrap();
+    assert!(response.status().is_success());
+
+    // read response
+    let bytes = srv.execute(response.body()).unwrap();
+    assert_eq!(bytes, Bytes::from(data));
+}
+
 #[test]
 fn test_client_brotli_encoding() {
     let mut srv = test::TestServer::new(|app| app.handler(|req: HttpRequest| {

tests/test_server.rs

@@ -136,27 +136,32 @@ fn test_simple() {
 
 #[test]
 fn test_headers() {
+    let data = STR.to_owned() + STR + STR + STR + STR + STR + STR + STR + STR + STR;
+    let srv_data = Arc::new(data.clone());
     let mut srv = test::TestServer::new(
-        |app| app.handler(|_| {
+        move |app| {
-            let mut builder = httpcodes::HTTPOk.build();
+            let data = srv_data.clone();
-            for idx in 0..90 {
+            app.handler(move |_| {
-                builder.header(
+                let mut builder = httpcodes::HTTPOk.build();
-                    format!("X-TEST-{}", idx).as_str(),
+                for idx in 0..90 {
-                    "TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
+                    builder.header(
-                     TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
+                        format!("X-TEST-{}", idx).as_str(),
-                     TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
+                        "TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
-                     TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
+                         TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
-                     TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
+                         TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
-                     TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
+                         TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
-                     TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
+                         TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
-                     TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
+                         TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
-                     TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
+                         TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
-                     TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
+                         TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
-                     TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
+                         TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
-                     TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
+                         TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
-                     TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST ");
+                         TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
-            }
+                         TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST \
-            builder.body(STR)}));
+                         TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST TEST ");
+                }
+                builder.body(data.as_ref())})
+        });
 
     let request = srv.get().finish().unwrap();
     let response = srv.execute(request.send()).unwrap();
@@ -164,7 +169,7 @@ fn test_headers() {
 
     // read response
     let bytes = srv.execute(response.body()).unwrap();
-    assert_eq!(bytes, Bytes::from_static(STR.as_ref()));
+    assert_eq!(bytes, Bytes::from(data));
 }
 
 #[test]
@@ -203,6 +208,33 @@ fn test_body_gzip() {
     assert_eq!(Bytes::from(dec), Bytes::from_static(STR.as_ref()));
 }
 
+#[test]
+fn test_body_gzip_large() {
+    let data = STR.to_owned() + STR + STR + STR + STR + STR + STR + STR + STR + STR;
+    let srv_data = Arc::new(data.clone());
+
+    let mut srv = test::TestServer::new(
+        move |app| {
+            let data = srv_data.clone();
+            app.handler(
+                move |_| httpcodes::HTTPOk.build()
+                    .content_encoding(headers::ContentEncoding::Gzip)
+                    .body(data.as_ref()))});
+
+    let request = srv.get().disable_decompress().finish().unwrap();
+    let response = srv.execute(request.send()).unwrap();
+    assert!(response.status().is_success());
+
+    // read response
+    let bytes = srv.execute(response.body()).unwrap();
+
+    // decode
+    let mut e = GzDecoder::new(&bytes[..]);
+    let mut dec = Vec::new();
+    e.read_to_end(&mut dec).unwrap();
+    assert_eq!(Bytes::from(dec), Bytes::from(data));
+}
+
 #[test]
 fn test_body_chunked_implicit() {
     let mut srv = test::TestServer::new(
@@ -430,6 +462,35 @@ fn test_gzip_encoding() {
     assert_eq!(bytes, Bytes::from_static(STR.as_ref()));
 }
 
+#[test]
+fn test_gzip_encoding_large() {
+    let data = STR.to_owned() + STR + STR + STR + STR + STR + STR + STR + STR + STR;
+    let mut srv = test::TestServer::new(|app| app.handler(|req: HttpRequest| {
+        req.body()
+            .and_then(|bytes: Bytes| {
+                Ok(httpcodes::HTTPOk
+                   .build()
+                   .content_encoding(headers::ContentEncoding::Identity)
+                   .body(bytes))
+            }).responder()}
+    ));
+
+    // client request
+    let mut e = GzEncoder::new(Vec::new(), Compression::default());
+    e.write_all(data.as_ref()).unwrap();
+    let enc = e.finish().unwrap();
+
+    let request = srv.post()
+        .header(header::CONTENT_ENCODING, "gzip")
+        .body(enc.clone()).unwrap();
+    let response = srv.execute(request.send()).unwrap();
+    assert!(response.status().is_success());
+
+    // read response
+    let bytes = srv.execute(response.body()).unwrap();
+    assert_eq!(bytes, Bytes::from(data));
+}
+
 #[test]
 fn test_deflate_encoding() {
     let mut srv = test::TestServer::new(|app| app.handler(|req: HttpRequest| {
@@ -458,6 +519,35 @@ fn test_deflate_encoding() {
     assert_eq!(bytes, Bytes::from_static(STR.as_ref()));
 }
 
+#[test]
+fn test_deflate_encoding_large() {
+    let data = STR.to_owned() + STR + STR + STR + STR + STR + STR + STR + STR + STR + STR;
+    let mut srv = test::TestServer::new(|app| app.handler(|req: HttpRequest| {
+        req.body()
+            .and_then(|bytes: Bytes| {
+                Ok(httpcodes::HTTPOk
+                   .build()
+                   .content_encoding(headers::ContentEncoding::Identity)
+                   .body(bytes))
+            }).responder()}
+    ));
+
+    let mut e = DeflateEncoder::new(Vec::new(), Compression::default());
+    e.write_all(data.as_ref()).unwrap();
+    let enc = e.finish().unwrap();
+
+    // client request
+    let request = srv.post()
+        .header(header::CONTENT_ENCODING, "deflate")
+        .body(enc).unwrap();
+    let response = srv.execute(request.send()).unwrap();
+    assert!(response.status().is_success());
+
+    // read response
+    let bytes = srv.execute(response.body()).unwrap();
+    assert_eq!(bytes, Bytes::from(data));
+}
+
 #[test]
 fn test_brotli_encoding() {
     let mut srv = test::TestServer::new(|app| app.handler(|req: HttpRequest| {
@@ -486,6 +576,35 @@ fn test_brotli_encoding() {
     assert_eq!(bytes, Bytes::from_static(STR.as_ref()));
 }
 
+#[test]
+fn test_brotli_encoding_large() {
+    let data = STR.to_owned() + STR + STR + STR + STR + STR + STR + STR + STR + STR + STR;
+    let mut srv = test::TestServer::new(|app| app.handler(|req: HttpRequest| {
+        req.body()
+            .and_then(|bytes: Bytes| {
+                Ok(httpcodes::HTTPOk
+                   .build()
+                   .content_encoding(headers::ContentEncoding::Identity)
+                   .body(bytes))
+            }).responder()}
+    ));
+
+    let mut e = BrotliEncoder::new(Vec::new(), 5);
+    e.write_all(data.as_ref()).unwrap();
+    let enc = e.finish().unwrap();
+
+    // client request
+    let request = srv.post()
+        .header(header::CONTENT_ENCODING, "br")
+        .body(enc).unwrap();
+    let response = srv.execute(request.send()).unwrap();
+    assert!(response.status().is_success());
+
+    // read response
+    let bytes = srv.execute(response.body()).unwrap();
+    assert_eq!(bytes, Bytes::from(data));
+}
+
 #[test]
 fn test_h2() {
     let srv = test::TestServer::new(|app| app.handler(|_|{