From e5f3d88a4eb92d6618e55077623c4d1868ec3ad2 Mon Sep 17 00:00:00 2001 From: daxpedda Date: Sat, 7 Dec 2019 16:55:41 +0100 Subject: [PATCH] Switch brotli compressor to rust. (#1197) * Switch to a rustified version of brotli. * Some memory optimizations. * Make brotli not optional anymore. --- Cargo.toml | 9 ++---- actix-http/Cargo.toml | 7 ++--- actix-http/src/encoding/decoder.rs | 17 ++++------- actix-http/src/encoding/encoder.rs | 30 +++++++++--------- actix-http/src/encoding/mod.rs | 3 ++ awc/Cargo.toml | 9 ++---- awc/src/request.rs | 49 ++++++++++++------------------ awc/tests/test_client.rs | 3 +- src/lib.rs | 2 -- tests/test_server.rs | 6 ++-- 10 files changed, 53 insertions(+), 82 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 356e4941..1674502d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,7 +16,7 @@ exclude = [".gitignore", ".travis.yml", ".cargo/config", "appveyor.yml"] edition = "2018" [package.metadata.docs.rs] -features = ["openssl", "brotli", "flate2-zlib", "secure-cookies", "client"] +features = ["openssl", "flate2-zlib", "secure-cookies", "client"] [badges] travis-ci = { repository = "actix/actix-web", branch = "master" } @@ -43,14 +43,11 @@ members = [ ] [features] -default = ["brotli", "flate2-zlib", "client", "fail"] +default = ["flate2-zlib", "client", "fail"] # http client client = ["awc"] -# brotli encoding, requires c compiler -brotli = ["actix-http/brotli", "awc/brotli"] - # miniz-sys backend for flate2 crate flate2-zlib = ["actix-http/flate2-zlib", "awc/flate2-zlib"] @@ -111,7 +108,7 @@ actix-http-test = "1.0.0-alpha.3" rand = "0.7" env_logger = "0.6" serde_derive = "1.0" -brotli2 = "0.3.2" +brotli = "3.3.0" flate2 = "1.0.2" [profile.release] diff --git a/actix-http/Cargo.toml b/actix-http/Cargo.toml index 4d89e55f..0a8787e0 100644 --- a/actix-http/Cargo.toml +++ b/actix-http/Cargo.toml @@ -16,7 +16,7 @@ edition = "2018" workspace = ".." [package.metadata.docs.rs] -features = ["openssl", "rustls", "fail", "brotli", "flate2-zlib", "secure-cookies"] +features = ["openssl", "rustls", "fail", "flate2-zlib", "secure-cookies"] [lib] name = "actix_http" @@ -31,9 +31,6 @@ openssl = ["actix-tls/openssl", "actix-connect/openssl"] # rustls support rustls = ["actix-tls/rustls", "actix-connect/rustls"] -# brotli encoding, requires c compiler -brotli = ["brotli2"] - # miniz-sys backend for flate2 crate flate2-zlib = ["flate2/miniz-sys"] @@ -88,7 +85,7 @@ time = "0.1.42" ring = { version = "0.16.9", optional = true } # compression -brotli2 = { version="0.3.2", optional = true } +brotli = "3.3.0" flate2 = { version="1.0.7", optional = true, default-features = false } # optional deps diff --git a/actix-http/src/encoding/decoder.rs b/actix-http/src/encoding/decoder.rs index dca77483..90199b4f 100644 --- a/actix-http/src/encoding/decoder.rs +++ b/actix-http/src/encoding/decoder.rs @@ -4,8 +4,7 @@ use std::pin::Pin; use std::task::{Context, Poll}; use actix_threadpool::{run, CpuFuture}; -#[cfg(feature = "brotli")] -use brotli2::write::BrotliDecoder; +use brotli::DecompressorWriter; use bytes::Bytes; #[cfg(any(feature = "flate2-zlib", feature = "flate2-rust"))] use flate2::write::{GzDecoder, ZlibDecoder}; @@ -32,9 +31,8 @@ where #[inline] pub fn new(stream: S, encoding: ContentEncoding) -> Decoder { let decoder = match encoding { - #[cfg(feature = "brotli")] ContentEncoding::Br => Some(ContentDecoder::Br(Box::new( - BrotliDecoder::new(Writer::new()), + DecompressorWriter::new(Writer::new(), 0), ))), #[cfg(any(feature = "flate2-zlib", feature = "flate2-rust"))] ContentEncoding::Deflate => Some(ContentDecoder::Deflate(Box::new( @@ -144,18 +142,16 @@ enum ContentDecoder { Deflate(Box>), #[cfg(any(feature = "flate2-zlib", feature = "flate2-rust"))] Gzip(Box>), - #[cfg(feature = "brotli")] - Br(Box>), + Br(Box>), } impl ContentDecoder { #[allow(unreachable_patterns)] fn feed_eof(&mut self) -> io::Result> { match self { - #[cfg(feature = "brotli")] - ContentDecoder::Br(ref mut decoder) => match decoder.finish() { - Ok(mut writer) => { - let b = writer.take(); + ContentDecoder::Br(ref mut decoder) => match decoder.flush() { + Ok(()) => { + let b = decoder.get_mut().take(); if !b.is_empty() { Ok(Some(b)) } else { @@ -195,7 +191,6 @@ impl ContentDecoder { #[allow(unreachable_patterns)] fn feed_data(&mut self, data: Bytes) -> io::Result> { match self { - #[cfg(feature = "brotli")] ContentDecoder::Br(ref mut decoder) => match decoder.write_all(&data) { Ok(_) => { decoder.flush()?; diff --git a/actix-http/src/encoding/encoder.rs b/actix-http/src/encoding/encoder.rs index d1b64bfb..d0676278 100644 --- a/actix-http/src/encoding/encoder.rs +++ b/actix-http/src/encoding/encoder.rs @@ -5,8 +5,7 @@ use std::pin::Pin; use std::task::{Context, Poll}; use actix_threadpool::{run, CpuFuture}; -#[cfg(feature = "brotli")] -use brotli2::write::BrotliEncoder; +use brotli::CompressorWriter; use bytes::Bytes; #[cfg(any(feature = "flate2-zlib", feature = "flate2-rust"))] use flate2::write::{GzEncoder, ZlibEncoder}; @@ -177,8 +176,7 @@ enum ContentEncoder { Deflate(ZlibEncoder), #[cfg(any(feature = "flate2-zlib", feature = "flate2-rust"))] Gzip(GzEncoder), - #[cfg(feature = "brotli")] - Br(BrotliEncoder), + Br(CompressorWriter), } impl ContentEncoder { @@ -194,10 +192,12 @@ impl ContentEncoder { Writer::new(), flate2::Compression::fast(), ))), - #[cfg(feature = "brotli")] - ContentEncoding::Br => { - Some(ContentEncoder::Br(BrotliEncoder::new(Writer::new(), 3))) - } + ContentEncoding::Br => Some(ContentEncoder::Br(CompressorWriter::new( + Writer::new(), + 0, + 3, + 0, + ))), _ => None, } } @@ -205,8 +205,11 @@ impl ContentEncoder { #[inline] pub(crate) fn take(&mut self) -> Bytes { match *self { - #[cfg(feature = "brotli")] - ContentEncoder::Br(ref mut encoder) => encoder.get_mut().take(), + ContentEncoder::Br(ref mut encoder) => { + let mut encoder_new = CompressorWriter::new(Writer::new(), 0, 3, 0); + std::mem::swap(encoder, &mut encoder_new); + encoder_new.into_inner().freeze() + } #[cfg(any(feature = "flate2-zlib", feature = "flate2-rust"))] ContentEncoder::Deflate(ref mut encoder) => encoder.get_mut().take(), #[cfg(any(feature = "flate2-zlib", feature = "flate2-rust"))] @@ -216,11 +219,7 @@ impl ContentEncoder { fn finish(self) -> Result { match self { - #[cfg(feature = "brotli")] - ContentEncoder::Br(encoder) => match encoder.finish() { - Ok(writer) => Ok(writer.buf.freeze()), - Err(err) => Err(err), - }, + ContentEncoder::Br(encoder) => Ok(encoder.into_inner().buf.freeze()), #[cfg(any(feature = "flate2-zlib", feature = "flate2-rust"))] ContentEncoder::Gzip(encoder) => match encoder.finish() { Ok(writer) => Ok(writer.buf.freeze()), @@ -236,7 +235,6 @@ impl ContentEncoder { fn write(&mut self, data: &[u8]) -> Result<(), io::Error> { match *self { - #[cfg(feature = "brotli")] ContentEncoder::Br(ref mut encoder) => match encoder.write_all(data) { Ok(_) => Ok(()), Err(err) => { diff --git a/actix-http/src/encoding/mod.rs b/actix-http/src/encoding/mod.rs index 9f65f800..48cf8325 100644 --- a/actix-http/src/encoding/mod.rs +++ b/actix-http/src/encoding/mod.rs @@ -22,6 +22,9 @@ impl Writer { fn take(&mut self) -> Bytes { self.buf.split().freeze() } + fn freeze(self) -> Bytes { + self.buf.freeze() + } } impl io::Write for Writer { diff --git a/awc/Cargo.toml b/awc/Cargo.toml index c4f3b7bf..9bb72ca9 100644 --- a/awc/Cargo.toml +++ b/awc/Cargo.toml @@ -21,10 +21,10 @@ name = "awc" path = "src/lib.rs" [package.metadata.docs.rs] -features = ["openssl", "rustls", "brotli", "flate2-zlib"] +features = ["openssl", "rustls", "flate2-zlib"] [features] -default = ["brotli", "flate2-zlib"] +default = ["flate2-zlib"] # openssl openssl = ["open-ssl", "actix-http/openssl"] @@ -32,9 +32,6 @@ openssl = ["open-ssl", "actix-http/openssl"] # rustls rustls = ["rust-tls", "actix-http/rustls"] -# brotli encoding, requires c compiler -brotli = ["actix-http/brotli"] - # miniz-sys backend for flate2 crate flate2-zlib = ["actix-http/flate2-zlib"] @@ -69,7 +66,7 @@ actix-http-test = { version = "1.0.0-alpha.3", features=["openssl"] } actix-utils = "1.0.0-alpha.3" actix-server = { version = "1.0.0-alpha.3" } actix-tls = { version = "1.0.0-alpha.3", features=["openssl", "rustls"] } -brotli2 = { version="0.3.2" } +brotli = "3.3.0" flate2 = { version="1.0.2" } env_logger = "0.6" webpki = { version = "0.21" } diff --git a/awc/src/request.rs b/awc/src/request.rs index 5ca4973c..b9d728b7 100644 --- a/awc/src/request.rs +++ b/awc/src/request.rs @@ -23,13 +23,10 @@ use crate::frozen::FrozenClientRequest; use crate::sender::{PrepForSendingError, RequestSender, SendClientRequest}; use crate::ClientConfig; -#[cfg(any(feature = "brotli", feature = "flate2-zlib", feature = "flate2-rust"))] +#[cfg(any(feature = "flate2-zlib", feature = "flate2-rust"))] const HTTPS_ENCODING: &str = "br, gzip, deflate"; -#[cfg(all( - any(feature = "flate2-zlib", feature = "flate2-rust"), - not(feature = "brotli") -))] -const HTTPS_ENCODING: &str = "gzip, deflate"; +#[cfg(not(any(feature = "flate2-zlib", feature = "flate2-rust")))] +const HTTPS_ENCODING: &str = "br"; /// An HTTP Client request builder /// @@ -544,31 +541,23 @@ impl ClientRequest { let mut slf = self; - // enable br only for https - #[cfg(any( - feature = "brotli", - feature = "flate2-zlib", - feature = "flate2-rust" - ))] - { - if slf.response_decompress { - let https = slf - .head - .uri - .scheme() - .map(|s| s == &uri::Scheme::HTTPS) - .unwrap_or(true); + if slf.response_decompress { + let https = slf + .head + .uri + .scheme() + .map(|s| s == &uri::Scheme::HTTPS) + .unwrap_or(true); - if https { - slf = slf.set_header_if_none(header::ACCEPT_ENCODING, HTTPS_ENCODING) - } else { - #[cfg(any(feature = "flate2-zlib", feature = "flate2-rust"))] - { - slf = slf - .set_header_if_none(header::ACCEPT_ENCODING, "gzip, deflate") - } - }; - } + if https { + slf = slf.set_header_if_none(header::ACCEPT_ENCODING, HTTPS_ENCODING) + } else { + #[cfg(any(feature = "flate2-zlib", feature = "flate2-rust"))] + { + slf = + slf.set_header_if_none(header::ACCEPT_ENCODING, "gzip, deflate") + } + }; } Ok(slf) diff --git a/awc/tests/test_client.rs b/awc/tests/test_client.rs index 6bd39973..3d2ed235 100644 --- a/awc/tests/test_client.rs +++ b/awc/tests/test_client.rs @@ -4,7 +4,7 @@ use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; use std::time::Duration; -use brotli2::write::BrotliEncoder; +use brotli::write::BrotliEncoder; use bytes::Bytes; use flate2::read::GzDecoder; use flate2::write::GzEncoder; @@ -568,7 +568,6 @@ async fn test_client_brotli_encoding() { // assert_eq!(bytes, Bytes::from(data)); // } -// #[cfg(feature = "brotli")] // #[actix_rt::test] // async fn test_client_deflate_encoding() { // let srv = test::TestServer::start(|app| { diff --git a/src/lib.rs b/src/lib.rs index b7fd8d15..04149fbf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -72,8 +72,6 @@ //! * `rustls` - enables ssl support via `rustls` crate, supports `http/2` //! * `secure-cookies` - enables secure cookies support, includes `ring` crate as //! dependency -//! * `brotli` - enables `brotli` compression support, requires `c` -//! compiler (default enabled) //! * `flate2-zlib` - enables `gzip`, `deflate` compression support, requires //! `c` compiler (default enabled) //! * `flate2-rust` - experimental rust based implementation for diff --git a/tests/test_server.rs b/tests/test_server.rs index 7cfda04a..a8352695 100644 --- a/tests/test_server.rs +++ b/tests/test_server.rs @@ -6,7 +6,7 @@ use actix_http::http::header::{ }; use actix_http::{Error, HttpService, Response}; use actix_http_test::TestServer; -use brotli2::write::{BrotliDecoder, BrotliEncoder}; +use brotli::write::{BrotliDecoder, BrotliEncoder}; use bytes::Bytes; use flate2::read::GzDecoder; use flate2::write::{GzEncoder, ZlibDecoder, ZlibEncoder}; @@ -296,7 +296,6 @@ async fn test_body_chunked_implicit() { } #[actix_rt::test] -#[cfg(feature = "brotli")] async fn test_body_br_streaming() { let srv = TestServer::start(move || { HttpService::build() @@ -411,7 +410,6 @@ async fn test_body_deflate() { } #[actix_rt::test] -#[cfg(any(feature = "brotli"))] async fn test_body_brotli() { let srv = TestServer::start(move || { HttpService::build() @@ -717,7 +715,7 @@ async fn test_brotli_encoding_large() { assert_eq!(bytes, Bytes::from(data)); } -// #[cfg(all(feature = "brotli", feature = "ssl"))] +// #[cfg(feature = "ssl")] // #[actix_rt::test] // async fn test_brotli_encoding_large_ssl() { // use actix::{Actor, System};