From e890307091059c5598d348365ca4e8c41b64731d Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 3 Jan 2022 13:17:57 +0000 Subject: [PATCH 1/6] Fix AcceptEncoding header (#2501) --- .github/workflows/bench.yml | 2 - CHANGES.md | 16 + actix-files/src/named.rs | 6 +- actix-http/CHANGES.md | 21 + actix-http/src/encoding/decoder.rs | 12 +- actix-http/src/encoding/encoder.rs | 31 +- actix-http/src/error.rs | 1 + actix-http/src/header/map.rs | 7 + actix-http/src/header/mod.rs | 7 - .../src/header/shared/content_encoding.rs | 51 +- actix-http/src/header/shared/quality.rs | 23 +- actix-http/src/header/shared/quality_item.rs | 14 +- awc/tests/test_client.rs | 14 +- scripts/ci-test | 20 +- src/dev.rs | 126 +++-- src/http/header/accept.rs | 65 +-- src/http/header/accept_charset.rs | 13 +- src/http/header/accept_encoding.rs | 414 ++++++++++++-- src/http/header/accept_language.rs | 64 +-- src/http/header/content_disposition.rs | 1 - src/http/header/encoding.rs | 78 ++- src/middleware/compress.rs | 328 +++-------- src/types/mod.rs | 15 +- src/types/payload.rs | 1 + src/web.rs | 5 +- tests/compression.rs | 313 +++++++++++ tests/fixtures/lorem.txt | 5 + tests/fixtures/lorem.txt.br | Bin 0 -> 905 bytes tests/fixtures/lorem.txt.gz | Bin 0 -> 920 bytes tests/fixtures/lorem.txt.xz | Bin 0 -> 1020 bytes tests/fixtures/lorem.txt.zst | Bin 0 -> 898 bytes tests/test_server.rs | 520 ++++++------------ tests/test_utils.rs | 76 +++ 33 files changed, 1360 insertions(+), 889 deletions(-) create mode 100644 tests/compression.rs create mode 100644 tests/fixtures/lorem.txt create mode 100644 tests/fixtures/lorem.txt.br create mode 100644 tests/fixtures/lorem.txt.gz create mode 100644 tests/fixtures/lorem.txt.xz create mode 100644 tests/fixtures/lorem.txt.zst create mode 100644 tests/test_utils.rs diff --git a/.github/workflows/bench.yml b/.github/workflows/bench.yml index 828d62561..a4b54ca7a 100644 --- a/.github/workflows/bench.yml +++ b/.github/workflows/bench.yml @@ -1,8 +1,6 @@ name: Benchmark on: - pull_request: - types: [opened, synchronize, reopened] push: branches: - master diff --git a/CHANGES.md b/CHANGES.md index 28dd6698b..423ea9fdc 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,6 +1,22 @@ # Changes ## Unreleased - 2021-xx-xx +### Added +- `impl Hash` for `http::header::Encoding`. [#2501] +- `AcceptEncoding::negotiate()`. [#2501] + +### Changed +- `AcceptEncoding::preference` now returns `Option>`. [#2501] +- Rename methods `BodyEncoding::{encoding => encode_with, get_encoding => preferred_encoding}`. [#2501] +- `http::header::Encoding` now only represents `Content-Encoding` types. [#2501] + +### Fixed +- Auto-negotiation of content encoding is more fault-tolerant when using the `Compress` middleware. [#2501] + +### Removed +- `Compress::new`; restricting compression algorithm is done through feature flags. [#2501] + +[#2501]: https://github.com/actix/actix-web/pull/2501 ## 4.0.0-beta.18 - 2021-12-29 diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs index 810988f0c..04e453580 100644 --- a/actix-files/src/named.rs +++ b/actix-files/src/named.rs @@ -420,7 +420,7 @@ impl NamedFile { } if let Some(current_encoding) = self.encoding { - res.encoding(current_encoding); + res.encode_with(current_encoding); } let reader = chunked::new_chunked_read(self.md.len(), 0, self.file); @@ -494,7 +494,7 @@ impl NamedFile { // default compressing if let Some(current_encoding) = self.encoding { - res.encoding(current_encoding); + res.encode_with(current_encoding); } if let Some(lm) = last_modified { @@ -517,7 +517,7 @@ impl NamedFile { length = ranges[0].length; offset = ranges[0].start; - res.encoding(ContentEncoding::Identity); + res.encode_with(ContentEncoding::Identity); res.insert_header(( header::CONTENT_RANGE, format!("bytes {}-{}/{}", offset, offset + length - 1, self.md.len()), diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 6911d9969..cbdccd93c 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,8 +1,29 @@ # Changes ## Unreleased - 2021-xx-xx +### Added +- `impl Eq` for `header::ContentEncoding`. [#2501] +- `impl Copy` for `QualityItem` where `T: Copy`. [#2501] +- `Quality::ZERO` equivalent to `q=0`. [#2501] +- `QualityItem::zero` that uses `Quality::ZERO`. [#2501] +- `ContentEncoding::to_header_value()`. [#2501] + +### Changed +- `Quality::MIN` is now the smallest non-zero value. [#2501] +- `QualityItem::min` semantics changed with `QualityItem::MIN`. [#2501] +- Rename `ContentEncoding::{Br => Brotli}`. [#2501] - Minimum supported Rust version (MSRV) is now 1.54. +### Fixed +- `ContentEncoding::Identity` can now be parsed from a string. [#2501] +- A `Vary` header is now correctly sent along with compressed content. [#2501] + +### Removed +- `ContentEncoding::Auto` variant. [#2501] +- `ContentEncoding::is_compression()`. [#2501] + +[#2501]: https://github.com/actix/actix-web/pull/2501 + ## 3.0.0-beta.17 - 2021-12-27 ### Changes diff --git a/actix-http/src/encoding/decoder.rs b/actix-http/src/encoding/decoder.rs index 0f519637a..da4b56c6a 100644 --- a/actix-http/src/encoding/decoder.rs +++ b/actix-http/src/encoding/decoder.rs @@ -47,9 +47,9 @@ where pub fn new(stream: S, encoding: ContentEncoding) -> Decoder { let decoder = match encoding { #[cfg(feature = "compress-brotli")] - ContentEncoding::Br => Some(ContentDecoder::Br(Box::new(BrotliDecoder::new( - Writer::new(), - )))), + ContentEncoding::Brotli => Some(ContentDecoder::Brotli(Box::new( + BrotliDecoder::new(Writer::new()), + ))), #[cfg(feature = "compress-gzip")] ContentEncoding::Deflate => Some(ContentDecoder::Deflate(Box::new( ZlibDecoder::new(Writer::new()), @@ -165,7 +165,7 @@ enum ContentDecoder { #[cfg(feature = "compress-gzip")] Gzip(Box>), #[cfg(feature = "compress-brotli")] - Br(Box>), + Brotli(Box>), // We need explicit 'static lifetime here because ZstdDecoder need lifetime // argument, and we use `spawn_blocking` in `Decoder::poll_next` that require `FnOnce() -> R + Send + 'static` #[cfg(feature = "compress-zstd")] @@ -176,7 +176,7 @@ impl ContentDecoder { fn feed_eof(&mut self) -> io::Result> { match self { #[cfg(feature = "compress-brotli")] - ContentDecoder::Br(ref mut decoder) => match decoder.flush() { + ContentDecoder::Brotli(ref mut decoder) => match decoder.flush() { Ok(()) => { let b = decoder.get_mut().take(); @@ -234,7 +234,7 @@ impl ContentDecoder { fn feed_data(&mut self, data: Bytes) -> io::Result> { match self { #[cfg(feature = "compress-brotli")] - ContentDecoder::Br(ref mut decoder) => match decoder.write_all(&data) { + ContentDecoder::Brotli(ref mut decoder) => match decoder.write_all(&data) { Ok(_) => { decoder.flush()?; let b = decoder.get_mut().take(); diff --git a/actix-http/src/encoding/encoder.rs b/actix-http/src/encoding/encoder.rs index b565bb2b5..f40e0e579 100644 --- a/actix-http/src/encoding/encoder.rs +++ b/actix-http/src/encoding/encoder.rs @@ -56,11 +56,10 @@ impl Encoder { } pub fn response(encoding: ContentEncoding, head: &mut ResponseHead, body: B) -> Self { - let can_encode = !(head.headers().contains_key(&CONTENT_ENCODING) + let should_encode = !(head.headers().contains_key(&CONTENT_ENCODING) || head.status == StatusCode::SWITCHING_PROTOCOLS || head.status == StatusCode::NO_CONTENT - || encoding == ContentEncoding::Identity - || encoding == ContentEncoding::Auto); + || encoding == ContentEncoding::Identity); // no need to compress an empty body if matches!(body.size(), BodySize::None) { @@ -72,8 +71,8 @@ impl Encoder { Err(body) => EncoderBody::Stream { body }, }; - if can_encode { - // Modify response body only if encoder is set + if should_encode { + // wrap body only if encoder is feature-enabled if let Some(enc) = ContentEncoder::encoder(encoding) { update_head(encoding, head); @@ -252,10 +251,10 @@ where } fn update_head(encoding: ContentEncoding, head: &mut ResponseHead) { - head.headers_mut().insert( - header::CONTENT_ENCODING, - HeaderValue::from_static(encoding.as_str()), - ); + head.headers_mut() + .insert(header::CONTENT_ENCODING, encoding.to_header_value()); + head.headers_mut() + .insert(header::VARY, HeaderValue::from_static("accept-encoding")); head.no_chunking(false); } @@ -268,7 +267,7 @@ enum ContentEncoder { Gzip(GzEncoder), #[cfg(feature = "compress-brotli")] - Br(BrotliEncoder), + Brotli(BrotliEncoder), // Wwe need explicit 'static lifetime here because ZstdEncoder needs a lifetime argument and we // use `spawn_blocking` in `Encoder::poll_next` that requires `FnOnce() -> R + Send + 'static`. @@ -292,8 +291,8 @@ impl ContentEncoder { ))), #[cfg(feature = "compress-brotli")] - ContentEncoding::Br => { - Some(ContentEncoder::Br(BrotliEncoder::new(Writer::new(), 3))) + ContentEncoding::Brotli => { + Some(ContentEncoder::Brotli(BrotliEncoder::new(Writer::new(), 3))) } #[cfg(feature = "compress-zstd")] @@ -302,7 +301,7 @@ impl ContentEncoder { Some(ContentEncoder::Zstd(encoder)) } - _ => None, + ContentEncoding::Identity => None, } } @@ -310,7 +309,7 @@ impl ContentEncoder { pub(crate) fn take(&mut self) -> Bytes { match *self { #[cfg(feature = "compress-brotli")] - ContentEncoder::Br(ref mut encoder) => encoder.get_mut().take(), + ContentEncoder::Brotli(ref mut encoder) => encoder.get_mut().take(), #[cfg(feature = "compress-gzip")] ContentEncoder::Deflate(ref mut encoder) => encoder.get_mut().take(), @@ -326,7 +325,7 @@ impl ContentEncoder { fn finish(self) -> Result { match self { #[cfg(feature = "compress-brotli")] - ContentEncoder::Br(encoder) => match encoder.finish() { + ContentEncoder::Brotli(encoder) => match encoder.finish() { Ok(writer) => Ok(writer.buf.freeze()), Err(err) => Err(err), }, @@ -354,7 +353,7 @@ impl ContentEncoder { fn write(&mut self, data: &[u8]) -> Result<(), io::Error> { match *self { #[cfg(feature = "compress-brotli")] - ContentEncoder::Br(ref mut encoder) => match encoder.write_all(data) { + ContentEncoder::Brotli(ref mut encoder) => match encoder.write_all(data) { Ok(_) => Ok(()), Err(err) => { trace!("Error decoding br encoding: {}", err); diff --git a/actix-http/src/error.rs b/actix-http/src/error.rs index 3d2a918f4..cdf495c45 100644 --- a/actix-http/src/error.rs +++ b/actix-http/src/error.rs @@ -250,6 +250,7 @@ impl From for Response { /// A set of errors that can occur running blocking tasks in thread pool. #[derive(Debug, Display, Error)] #[display(fmt = "Blocking thread pool is gone")] +// TODO: non-exhaustive pub struct BlockingError; /// A set of errors that can occur during payload parsing. diff --git a/actix-http/src/header/map.rs b/actix-http/src/header/map.rs index 478867ed0..5cf4ba2fa 100644 --- a/actix-http/src/header/map.rs +++ b/actix-http/src/header/map.rs @@ -605,6 +605,13 @@ impl<'a> IntoIterator for &'a HeaderMap { } } +/// Convert `http::HeaderMap` to our `HeaderMap`. +impl From for HeaderMap { + fn from(mut map: http::HeaderMap) -> HeaderMap { + HeaderMap::from_drain(map.drain()) + } +} + /// Iterator over removed, owned values with the same associated name. /// /// Returned from methods that remove or replace items. See [`HeaderMap::insert`] diff --git a/actix-http/src/header/mod.rs b/actix-http/src/header/mod.rs index dd4f06106..4e9140db4 100644 --- a/actix-http/src/header/mod.rs +++ b/actix-http/src/header/mod.rs @@ -57,13 +57,6 @@ pub trait Header: TryIntoHeaderValue { fn parse(msg: &M) -> Result; } -/// Convert `http::HeaderMap` to our `HeaderMap`. -impl From for HeaderMap { - fn from(mut map: http::HeaderMap) -> HeaderMap { - HeaderMap::from_drain(map.drain()) - } -} - /// This encode set is used for HTTP header values and is defined at /// . pub(crate) const HTTP_VALUE: &AsciiSet = &CONTROLS diff --git a/actix-http/src/header/shared/content_encoding.rs b/actix-http/src/header/shared/content_encoding.rs index 68511a8ee..ce011f107 100644 --- a/actix-http/src/header/shared/content_encoding.rs +++ b/actix-http/src/header/shared/content_encoding.rs @@ -20,14 +20,16 @@ pub struct ContentEncodingParseError; /// See [IANA HTTP Content Coding Registry]. /// /// [IANA HTTP Content Coding Registry]: https://www.iana.org/assignments/http-parameters/http-parameters.xhtml -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] #[non_exhaustive] pub enum ContentEncoding { - /// Automatically select encoding based on encoding negotiation. - Auto, + /// Indicates the no-op identity encoding. + /// + /// I.e., no compression or modification. + Identity, /// A format using the Brotli algorithm. - Br, + Brotli, /// A format using the zlib structure with deflate algorithm. Deflate, @@ -37,27 +39,30 @@ pub enum ContentEncoding { /// Zstd algorithm. Zstd, - - /// Indicates the identity function (i.e. no compression, nor modification). - Identity, } impl ContentEncoding { - /// Is the content compressed? - #[inline] - pub const fn is_compression(self) -> bool { - matches!(self, ContentEncoding::Identity | ContentEncoding::Auto) - } - /// Convert content encoding to string. #[inline] pub const fn as_str(self) -> &'static str { match self { - ContentEncoding::Br => "br", + ContentEncoding::Brotli => "br", ContentEncoding::Gzip => "gzip", ContentEncoding::Deflate => "deflate", ContentEncoding::Zstd => "zstd", - ContentEncoding::Identity | ContentEncoding::Auto => "identity", + ContentEncoding::Identity => "identity", + } + } + + /// Convert content encoding to header value. + #[inline] + pub const fn to_header_value(self) -> HeaderValue { + match self { + ContentEncoding::Brotli => HeaderValue::from_static("br"), + ContentEncoding::Gzip => HeaderValue::from_static("gzip"), + ContentEncoding::Deflate => HeaderValue::from_static("deflate"), + ContentEncoding::Zstd => HeaderValue::from_static("zstd"), + ContentEncoding::Identity => HeaderValue::from_static("identity"), } } } @@ -71,16 +76,18 @@ impl Default for ContentEncoding { impl FromStr for ContentEncoding { type Err = ContentEncodingParseError; - fn from_str(val: &str) -> Result { - let val = val.trim(); + fn from_str(enc: &str) -> Result { + let enc = enc.trim(); - if val.eq_ignore_ascii_case("br") { - Ok(ContentEncoding::Br) - } else if val.eq_ignore_ascii_case("gzip") { + if enc.eq_ignore_ascii_case("br") { + Ok(ContentEncoding::Brotli) + } else if enc.eq_ignore_ascii_case("gzip") { Ok(ContentEncoding::Gzip) - } else if val.eq_ignore_ascii_case("deflate") { + } else if enc.eq_ignore_ascii_case("deflate") { Ok(ContentEncoding::Deflate) - } else if val.eq_ignore_ascii_case("zstd") { + } else if enc.eq_ignore_ascii_case("identity") { + Ok(ContentEncoding::Identity) + } else if enc.eq_ignore_ascii_case("zstd") { Ok(ContentEncoding::Zstd) } else { Err(ContentEncodingParseError) diff --git a/actix-http/src/header/shared/quality.rs b/actix-http/src/header/shared/quality.rs index c2f08edc2..c80dd0a8e 100644 --- a/actix-http/src/header/shared/quality.rs +++ b/actix-http/src/header/shared/quality.rs @@ -27,7 +27,8 @@ const MAX_QUALITY_FLOAT: f32 = 1.0; /// /// assert_eq!(q(0.42).to_string(), "0.42"); /// assert_eq!(q(1.0).to_string(), "1"); -/// assert_eq!(Quality::MIN.to_string(), "0"); +/// assert_eq!(Quality::MIN.to_string(), "0.001"); +/// assert_eq!(Quality::ZERO.to_string(), "0"); /// ``` /// /// [RFC 7231 §5.3.1]: https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.1 @@ -38,8 +39,11 @@ impl Quality { /// The maximum quality value, equivalent to `q=1.0`. pub const MAX: Quality = Quality(MAX_QUALITY_INT); - /// The minimum quality value, equivalent to `q=0.0`. - pub const MIN: Quality = Quality(0); + /// The minimum, non-zero quality value, equivalent to `q=0.001`. + pub const MIN: Quality = Quality(1); + + /// The zero quality value, equivalent to `q=0.0`. + pub const ZERO: Quality = Quality(0); /// Converts a float in the range 0.0–1.0 to a `Quality`. /// @@ -51,7 +55,7 @@ impl Quality { // Check that `value` is within range should be done before calling this method. // Just in case, this debug_assert should catch if we were forgetful. debug_assert!( - (0.0f32..=1.0f32).contains(&value), + (0.0..=MAX_QUALITY_FLOAT).contains(&value), "q value must be between 0.0 and 1.0" ); @@ -154,10 +158,13 @@ impl TryFrom for Quality { /// let q1 = q(1.0); /// assert_eq!(q1, Quality::MAX); /// -/// let q2 = q(0.0); +/// let q2 = q(0.001); /// assert_eq!(q2, Quality::MIN); /// -/// let q3 = q(0.42); +/// let q3 = q(0.0); +/// assert_eq!(q3, Quality::ZERO); +/// +/// let q4 = q(0.42); /// ``` /// /// An out-of-range `f32` quality will panic. @@ -185,6 +192,10 @@ mod tests { #[test] fn display_output() { + assert_eq!(Quality::ZERO.to_string(), "0"); + assert_eq!(Quality::MIN.to_string(), "0.001"); + assert_eq!(Quality::MAX.to_string(), "1"); + assert_eq!(q(0.0).to_string(), "0"); assert_eq!(q(1.0).to_string(), "1"); assert_eq!(q(0.001).to_string(), "0.001"); diff --git a/actix-http/src/header/shared/quality_item.rs b/actix-http/src/header/shared/quality_item.rs index c9eee7d9d..71a3bdd53 100644 --- a/actix-http/src/header/shared/quality_item.rs +++ b/actix-http/src/header/shared/quality_item.rs @@ -31,7 +31,7 @@ use super::Quality; /// let q_item_fallback: QualityItem = "abc;q=0.1".parse().unwrap(); /// assert!(q_item > q_item_fallback); /// ``` -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct QualityItem { /// The wrapped contents of the field. pub item: T, @@ -53,10 +53,15 @@ impl QualityItem { Self::new(item, Quality::MAX) } - /// Constructs a new `QualityItem` from an item, using the minimum q-value. + /// Constructs a new `QualityItem` from an item, using the minimum, non-zero q-value. pub fn min(item: T) -> Self { Self::new(item, Quality::MIN) } + + /// Constructs a new `QualityItem` from an item, using zero q-value of zero. + pub fn zero(item: T) -> Self { + Self::new(item, Quality::ZERO) + } } impl PartialOrd for QualityItem { @@ -73,7 +78,10 @@ impl fmt::Display for QualityItem { // q-factor value is implied for max value Quality::MAX => Ok(()), - Quality::MIN => f.write_str("; q=0"), + // fast path for zero + Quality::ZERO => f.write_str("; q=0"), + + // quality formatting is already using itoa q => write!(f, "; q={}", q), } } diff --git a/awc/tests/test_client.rs b/awc/tests/test_client.rs index c453a768d..6eb6800d3 100644 --- a/awc/tests/test_client.rs +++ b/awc/tests/test_client.rs @@ -473,7 +473,7 @@ async fn test_no_decompress() { .wrap(actix_web::middleware::Compress::default()) .service(web::resource("/").route(web::to(|| { let mut res = HttpResponse::Ok().body(STR); - res.encoding(header::ContentEncoding::Gzip); + res.encode_with(header::ContentEncoding::Gzip); res }))) }); @@ -644,7 +644,9 @@ async fn test_client_brotli_encoding_large_random() { async fn test_client_deflate_encoding() { let srv = actix_test::start(|| { App::new().default_service(web::to(|body: Bytes| { - HttpResponse::Ok().encoding(ContentEncoding::Br).body(body) + HttpResponse::Ok() + .encode_with(ContentEncoding::Brotli) + .body(body) })) }); @@ -667,7 +669,9 @@ async fn test_client_deflate_encoding_large_random() { let srv = actix_test::start(|| { App::new().default_service(web::to(|body: Bytes| { - HttpResponse::Ok().encoding(ContentEncoding::Br).body(body) + HttpResponse::Ok() + .encode_with(ContentEncoding::Brotli) + .body(body) })) }); @@ -685,7 +689,7 @@ async fn test_client_streaming_explicit() { let srv = actix_test::start(|| { App::new().default_service(web::to(|body: web::Payload| { HttpResponse::Ok() - .encoding(ContentEncoding::Identity) + .encode_with(ContentEncoding::Identity) .streaming(body) })) }); @@ -710,7 +714,7 @@ async fn test_body_streaming_implicit() { }); HttpResponse::Ok() - .encoding(ContentEncoding::Gzip) + .encode_with(ContentEncoding::Gzip) .streaming(Box::pin(body)) })) }); diff --git a/scripts/ci-test b/scripts/ci-test index 3ab229665..567012d33 100755 --- a/scripts/ci-test +++ b/scripts/ci-test @@ -12,16 +12,16 @@ save_exit_code() { [ "$CMD_EXIT" = "0" ] || EXIT=$CMD_EXIT } -save_exit_code cargo test --lib --tests -p=actix-router --all-features -save_exit_code cargo test --lib --tests -p=actix-http --all-features -save_exit_code cargo test --lib --tests -p=actix-web --features=rustls,openssl -- --skip=test_reading_deflate_encoding_large_random_rustls -save_exit_code cargo test --lib --tests -p=actix-web-codegen --all-features -save_exit_code cargo test --lib --tests -p=awc --all-features -save_exit_code cargo test --lib --tests -p=actix-http-test --all-features -save_exit_code cargo test --lib --tests -p=actix-test --all-features -save_exit_code cargo test --lib --tests -p=actix-files -save_exit_code cargo test --lib --tests -p=actix-multipart --all-features -save_exit_code cargo test --lib --tests -p=actix-web-actors --all-features +save_exit_code cargo test --lib --tests -p=actix-router --all-features -- --nocapture +save_exit_code cargo test --lib --tests -p=actix-http --all-features -- --nocapture +save_exit_code cargo test --lib --tests -p=actix-web --features=rustls,openssl -- --nocapture --skip=test_reading_deflate_encoding_large_random_rustls +save_exit_code cargo test --lib --tests -p=actix-web-codegen --all-features -- --nocapture +save_exit_code cargo test --lib --tests -p=awc --all-features -- --nocapture +save_exit_code cargo test --lib --tests -p=actix-http-test --all-features -- --nocapture +save_exit_code cargo test --lib --tests -p=actix-test --all-features -- --nocapture +save_exit_code cargo test --lib --tests -p=actix-files -- --nocapture +save_exit_code cargo test --lib --tests -p=actix-multipart --all-features -- --nocapture +save_exit_code cargo test --lib --tests -p=actix-web-actors --all-features -- --nocapture save_exit_code cargo test --workspace --doc diff --git a/src/dev.rs b/src/dev.rs index bb1385bde..b15d15747 100644 --- a/src/dev.rs +++ b/src/dev.rs @@ -20,11 +20,9 @@ pub use crate::info::{ConnectionInfo, PeerAddr}; pub use crate::rmap::ResourceMap; pub use crate::service::{HttpServiceFactory, ServiceRequest, ServiceResponse, WebService}; -pub use crate::types::form::UrlEncoded; -pub use crate::types::json::JsonBody; -pub use crate::types::readlines::Readlines; +pub use crate::types::{JsonBody, Readlines, UrlEncoded}; -use crate::http::header::ContentEncoding; +use crate::{http::header::ContentEncoding, HttpMessage as _}; use actix_router::Patterns; @@ -47,59 +45,109 @@ pub(crate) fn ensure_leading_slash(mut patterns: Patterns) -> Patterns { patterns } -/// Helper trait that allows to set specific encoding for response. +/// Helper trait for managing response encoding. +/// +/// Use `pre_encoded_with` to flag response as already encoded. For example, when serving a Gzip +/// compressed file from disk. pub trait BodyEncoding { /// Get content encoding - fn get_encoding(&self) -> Option; + fn preferred_encoding(&self) -> Option; - /// Set content encoding + /// Set content encoding to use. /// - /// Must be used with [`crate::middleware::Compress`] to take effect. - fn encoding(&mut self, encoding: ContentEncoding) -> &mut Self; + /// Must be used with [`Compress`] to take effect. + /// + /// [`Compress`]: crate::middleware::Compress + fn encode_with(&mut self, encoding: ContentEncoding) -> &mut Self; + + // /// Flags that a file already is encoded so that [`Compress`] does not modify it. + // /// + // /// Effectively a shortcut for `compress_with("identity")` + // /// plus `insert_header(ContentEncoding, encoding)`. + // /// + // /// [`Compress`]: crate::middleware::Compress + // fn pre_encoded_with(&mut self, encoding: ContentEncoding) -> &mut Self; } -impl BodyEncoding for actix_http::ResponseBuilder { - fn get_encoding(&self) -> Option { - self.extensions().get::().map(|enc| enc.0) - } +struct CompressWith(ContentEncoding); - fn encoding(&mut self, encoding: ContentEncoding) -> &mut Self { - self.extensions_mut().insert(Enc(encoding)); - self - } -} - -struct Enc(ContentEncoding); - -impl BodyEncoding for actix_http::Response { - fn get_encoding(&self) -> Option { - self.extensions().get::().map(|enc| enc.0) - } - - fn encoding(&mut self, encoding: ContentEncoding) -> &mut Self { - self.extensions_mut().insert(Enc(encoding)); - self - } -} +// TODO: add or delete this +// struct PreCompressed(ContentEncoding); impl BodyEncoding for crate::HttpResponseBuilder { - fn get_encoding(&self) -> Option { - self.extensions().get::().map(|enc| enc.0) + fn preferred_encoding(&self) -> Option { + self.extensions().get::().map(|enc| enc.0) } - fn encoding(&mut self, encoding: ContentEncoding) -> &mut Self { - self.extensions_mut().insert(Enc(encoding)); + fn encode_with(&mut self, encoding: ContentEncoding) -> &mut Self { + self.extensions_mut().insert(CompressWith(encoding)); self } + + // fn pre_encoded_with(&mut self, encoding: ContentEncoding) -> &mut Self { + // self.extensions_mut().insert(PreCompressed(encoding)); + // self + // } } impl BodyEncoding for crate::HttpResponse { - fn get_encoding(&self) -> Option { - self.extensions().get::().map(|enc| enc.0) + fn preferred_encoding(&self) -> Option { + self.extensions().get::().map(|enc| enc.0) } - fn encoding(&mut self, encoding: ContentEncoding) -> &mut Self { - self.extensions_mut().insert(Enc(encoding)); + fn encode_with(&mut self, encoding: ContentEncoding) -> &mut Self { + self.extensions_mut().insert(CompressWith(encoding)); + self + } + + // fn pre_encoded_with(&mut self, encoding: ContentEncoding) -> &mut Self { + // self.extensions_mut().insert(PreCompressed(encoding)); + // self + // } +} + +impl BodyEncoding for ServiceResponse { + fn preferred_encoding(&self) -> Option { + self.request() + .extensions() + .get::() + .map(|enc| enc.0) + } + + fn encode_with(&mut self, encoding: ContentEncoding) -> &mut Self { + self.request() + .extensions_mut() + .insert(CompressWith(encoding)); + self + } + + // fn pre_encoded_with(&mut self, encoding: ContentEncoding) -> &mut Self { + // self.request() + // .extensions_mut() + // .insert(PreCompressed(encoding)); + // self + // } +} + +// TODO: remove these impls ? +impl BodyEncoding for actix_http::ResponseBuilder { + fn preferred_encoding(&self) -> Option { + self.extensions().get::().map(|enc| enc.0) + } + + fn encode_with(&mut self, encoding: ContentEncoding) -> &mut Self { + self.extensions_mut().insert(CompressWith(encoding)); + self + } +} + +impl BodyEncoding for actix_http::Response { + fn preferred_encoding(&self) -> Option { + self.extensions().get::().map(|enc| enc.0) + } + + fn encode_with(&mut self, encoding: ContentEncoding) -> &mut Self { + self.extensions_mut().insert(CompressWith(encoding)); self } } diff --git a/src/http/header/accept.rs b/src/http/header/accept.rs index c61e6ab49..368a05bb2 100644 --- a/src/http/header/accept.rs +++ b/src/http/header/accept.rs @@ -147,6 +147,39 @@ impl Accept { Accept(vec![QualityItem::max(mime::TEXT_HTML)]) } + // TODO: method for getting best content encoding based on q-factors, available from server side + // and if none are acceptable return None + + /// Extracts the most preferable mime type, accounting for [q-factor weighting]. + /// + /// If no q-factors are provided, the first mime type is chosen. Note that items without + /// q-factors are given the maximum preference value. + /// + /// As per the spec, will return [`mime::STAR_STAR`] (indicating no preference) if the contained + /// list is empty. + /// + /// [q-factor weighting]: https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.2 + pub fn preference(&self) -> Mime { + use actix_http::header::Quality; + + let mut max_item = None; + let mut max_pref = Quality::ZERO; + + // uses manual max lookup loop since we want the first occurrence in the case of same + // preference but `Iterator::max_by_key` would give us the last occurrence + + for pref in &self.0 { + // only change if strictly greater + // equal items, even while unsorted, still have higher preference if they appear first + if pref.quality > max_pref { + max_pref = pref.quality; + max_item = Some(pref.item.clone()); + } + } + + max_item.unwrap_or(mime::STAR_STAR) + } + /// Returns a sorted list of mime types from highest to lowest preference, accounting for /// [q-factor weighting] and specificity. /// @@ -196,36 +229,6 @@ impl Accept { types.into_iter().map(|qitem| qitem.item).collect() } - - /// Extracts the most preferable mime type, accounting for [q-factor weighting]. - /// - /// If no q-factors are provided, the first mime type is chosen. Note that items without - /// q-factors are given the maximum preference value. - /// - /// As per the spec, will return [`mime::STAR_STAR`] (indicating no preference) if the contained - /// list is empty. - /// - /// [q-factor weighting]: https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.2 - pub fn preference(&self) -> Mime { - use actix_http::header::Quality; - - let mut max_item = None; - let mut max_pref = Quality::MIN; - - // uses manual max lookup loop since we want the first occurrence in the case of same - // preference but `Iterator::max_by_key` would give us the last occurrence - - for pref in &self.0 { - // only change if strictly greater - // equal items, even while unsorted, still have higher preference if they appear first - if pref.quality > max_pref { - max_pref = pref.quality; - max_item = Some(pref.item.clone()); - } - } - - max_item.unwrap_or(mime::STAR_STAR) - } } #[cfg(test)] @@ -239,7 +242,7 @@ mod tests { assert!(test.ranked().is_empty()); let test = Accept(vec![QualityItem::max(mime::APPLICATION_JSON)]); - assert_eq!(test.ranked(), vec!(mime::APPLICATION_JSON)); + assert_eq!(test.ranked(), vec![mime::APPLICATION_JSON]); let test = Accept(vec![ QualityItem::max(mime::TEXT_HTML), diff --git a/src/http/header/accept_charset.rs b/src/http/header/accept_charset.rs index c8b918c91..c7f7e1a68 100644 --- a/src/http/header/accept_charset.rs +++ b/src/http/header/accept_charset.rs @@ -1,8 +1,7 @@ -use super::{Charset, QualityItem, ACCEPT_CHARSET}; +use super::{common_header, Charset, QualityItem, ACCEPT_CHARSET}; -crate::http::header::common_header! { - /// `Accept-Charset` header, defined in - /// [RFC 7231 §5.3.3](https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.3) +common_header! { + /// `Accept-Charset` header, defined in [RFC 7231 §5.3.3]. /// /// The `Accept-Charset` header field can be sent by a user agent to /// indicate what charsets are acceptable in textual response content. @@ -52,10 +51,12 @@ crate::http::header::common_header! { /// AcceptCharset(vec![QualityItem::max(Charset::Ext("utf-8".to_owned()))]) /// ); /// ``` - (AcceptCharset, ACCEPT_CHARSET) => (QualityItem)+ + /// + /// [RFC 7231 §5.3.3]: https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.3 + (AcceptCharset, ACCEPT_CHARSET) => (QualityItem)* test_parse_and_format { // Test case from RFC - crate::http::header::common_header_test!(test1, vec![b"iso-8859-5, unicode-1-1;q=0.8"]); + common_header_test!(test1, vec![b"iso-8859-5, unicode-1-1;q=0.8"]); } } diff --git a/src/http/header/accept_encoding.rs b/src/http/header/accept_encoding.rs index 828a0533c..8c35179b6 100644 --- a/src/http/header/accept_encoding.rs +++ b/src/http/header/accept_encoding.rs @@ -1,17 +1,15 @@ -use actix_http::header::QualityItem; +use std::collections::HashSet; -use super::{common_header, Encoding}; +use super::{common_header, ContentEncoding, Encoding, Preference, Quality, QualityItem}; use crate::http::header; common_header! { /// `Accept-Encoding` header, defined /// in [RFC 7231](https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.4) /// - /// The `Accept-Encoding` header field can be used by user agents to - /// indicate what response content-codings are - /// acceptable in the response. An `identity` token is used as a synonym - /// for "no encoding" in order to communicate when no encoding is - /// preferred. + /// The `Accept-Encoding` header field can be used by user agents to indicate what response + /// content-codings are acceptable in the response. An `identity` token is used as a synonym + /// for "no encoding" in order to communicate when no encoding is preferred. /// /// # ABNF /// ```plain @@ -29,11 +27,11 @@ common_header! { /// # Examples /// ``` /// use actix_web::HttpResponse; - /// use actix_web::http::header::{AcceptEncoding, Encoding, QualityItem}; + /// use actix_web::http::header::{AcceptEncoding, Encoding, Preference, QualityItem}; /// /// let mut builder = HttpResponse::Ok(); /// builder.insert_header( - /// AcceptEncoding(vec![QualityItem::max(Encoding::Chunked)]) + /// AcceptEncoding(vec![QualityItem::max(Preference::Specific(Encoding::gzip()))]) /// ); /// ``` /// @@ -44,40 +42,388 @@ common_header! { /// let mut builder = HttpResponse::Ok(); /// builder.insert_header( /// AcceptEncoding(vec![ - /// QualityItem::max(Encoding::Chunked), - /// QualityItem::max(Encoding::Gzip), - /// QualityItem::max(Encoding::Deflate), + /// "gzip".parse().unwrap(), + /// "br".parse().unwrap(), /// ]) /// ); /// ``` - /// - /// ``` - /// use actix_web::HttpResponse; - /// use actix_web::http::header::{AcceptEncoding, Encoding, QualityItem, q}; - /// - /// let mut builder = HttpResponse::Ok(); - /// builder.insert_header( - /// AcceptEncoding(vec![ - /// QualityItem::max(Encoding::Chunked), - /// QualityItem::new(Encoding::Gzip, q(0.60)), - /// QualityItem::min(Encoding::EncodingExt("*".to_owned())), - /// ]) - /// ); - /// ``` - (AcceptEncoding, header::ACCEPT_ENCODING) => (QualityItem)* + (AcceptEncoding, header::ACCEPT_ENCODING) => (QualityItem>)* test_parse_and_format { - // From the RFC - common_header_test!(test1, vec![b"compress, gzip"]); - common_header_test!(test2, vec![b""], Some(AcceptEncoding(vec![]))); - common_header_test!(test3, vec![b"*"]); + common_header_test!(no_headers, vec![b""; 0], Some(AcceptEncoding(vec![]))); + common_header_test!(empty_header, vec![b""; 1], Some(AcceptEncoding(vec![]))); + + common_header_test!( + order_of_appearance, + vec![b"br, gzip"], + Some(AcceptEncoding(vec![ + QualityItem::max(Preference::Specific(Encoding::brotli())), + QualityItem::max(Preference::Specific(Encoding::gzip())), + ])) + ); + + common_header_test!(any, vec![b"*"], Some(AcceptEncoding(vec![ + QualityItem::max(Preference::Any), + ]))); // Note: Removed quality 1 from gzip - common_header_test!(test4, vec![b"compress;q=0.5, gzip"]); + common_header_test!(implicit_quality, vec![b"gzip, identity; q=0.5, *;q=0"]); // Note: Removed quality 1 from gzip - common_header_test!(test5, vec![b"gzip, identity; q=0.5, *;q=0"]); + common_header_test!(implicit_quality_out_of_order, vec![b"compress;q=0.5, gzip"]); + + common_header_test!( + only_gzip_no_identity, + vec![b"gzip, *; q=0"], + Some(AcceptEncoding(vec![ + QualityItem::max(Preference::Specific(Encoding::gzip())), + QualityItem::zero(Preference::Any), + ])) + ); } } -// TODO: shortcut for EncodingExt(*) = Any +impl AcceptEncoding { + /// Selects the most acceptable encoding according to client preference and supported types. + /// + /// The "identity" encoding is not assumed and should be included in the `supported` iterator + /// if a non-encoded representation can be selected. + /// + /// If `None` is returned, this indicates that none of the supported encodings are acceptable to + /// the client. The caller should generate a 406 Not Acceptable response (unencoded) that + /// includes the server's supported encodings in the body plus a [`Vary`] header. + /// + /// [`Vary`]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Vary + pub fn negotiate<'a>( + &self, + supported: impl Iterator, + ) -> Option { + // 1. If no Accept-Encoding field is in the request, any content-coding is considered + // acceptable by the user agent. + + let supported_set = supported.collect::>(); + + if supported_set.is_empty() { + return None; + } + + if self.0.is_empty() { + // though it is not recommended to encode in this case, return identity encoding + return Some(Encoding::identity()); + } + + // 2. If the representation has no content-coding, then it is acceptable by default unless + // specifically excluded by the Accept-Encoding field stating either "identity;q=0" or + // "*;q=0" without a more specific entry for "identity". + + let acceptable_items = self.ranked_items().collect::>(); + + let identity_acceptable = is_identity_acceptable(&acceptable_items); + let identity_supported = supported_set.contains(&Encoding::identity()); + + if identity_acceptable && identity_supported && supported_set.len() == 1 { + return Some(Encoding::identity()); + } + + // 3. If the representation's content-coding is one of the content-codings listed in the + // Accept-Encoding field, then it is acceptable unless it is accompanied by a qvalue of 0. + + // 4. If multiple content-codings are acceptable, then the acceptable content-coding with + // the highest non-zero qvalue is preferred. + + let matched = acceptable_items + .into_iter() + .filter(|q| q.quality > Quality::ZERO) + // search relies on item list being in descending order of quality + .find(|q| { + let enc = &q.item; + matches!(enc, Preference::Specific(enc) if supported_set.contains(enc)) + }) + .map(|q| q.item); + + match matched { + Some(Preference::Specific(enc)) => Some(enc), + + _ if identity_acceptable => Some(Encoding::identity()), + + _ => None, + } + } + + /// Extracts the most preferable encoding, accounting for [q-factor weighting]. + /// + /// If no q-factors are provided, the first encoding is chosen. Note that items without + /// q-factors are given the maximum preference value. + /// + /// As per the spec, returns [`Preference::Any`] if acceptable list is empty. Though, if this is + /// returned, it is recommended to use an un-encoded representation. + /// + /// If `None` is returned, it means that the client has signalled that no representations + /// are acceptable. This should never occur for a well behaved user-agent. + /// + /// [q-factor weighting]: https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.2 + pub fn preference(&self) -> Option> { + // empty header indicates no preference + if self.0.is_empty() { + return Some(Preference::Any); + } + + let mut max_item = None; + let mut max_pref = Quality::ZERO; + + // uses manual max lookup loop since we want the first occurrence in the case of same + // preference but `Iterator::max_by_key` would give us the last occurrence + + for pref in &self.0 { + // only change if strictly greater + // equal items, even while unsorted, still have higher preference if they appear first + if pref.quality > max_pref { + max_pref = pref.quality; + max_item = Some(pref.item.clone()); + } + } + + // Return max_item if any items were above 0 quality... + max_item.or_else(|| { + // ...or else check for "*" or "identity". We can elide quality checks since + // entering this block means all items had "q=0". + match self.0.iter().find(|pref| { + matches!( + pref.item, + Preference::Any + | Preference::Specific(Encoding::Known(ContentEncoding::Identity)) + ) + }) { + // "identity" or "*" found so no representation is acceptable + Some(_) => None, + + // implicit "identity" is acceptable + None => Some(Preference::Specific(Encoding::identity())), + } + }) + } + + /// Returns a sorted list of encodings from highest to lowest precedence, accounting + /// for [q-factor weighting]. + /// + /// [q-factor weighting]: https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.2 + pub fn ranked(&self) -> Vec> { + self.ranked_items().map(|q| q.item).collect() + } + + fn ranked_items(&self) -> impl Iterator>> { + if self.0.is_empty() { + return vec![].into_iter(); + } + + let mut types = self.0.clone(); + + // use stable sort so items with equal q-factor retain listed order + types.sort_by(|a, b| { + // sort by q-factor descending + b.quality.cmp(&a.quality) + }); + + types.into_iter() + } +} + +/// Returns true if "identity" is an acceptable encoding. +/// +/// Internal algorithm relies on item list being in descending order of quality. +fn is_identity_acceptable(items: &'_ [QualityItem>]) -> bool { + if items.is_empty() { + return true; + } + + // Loop algorithm depends on items being sorted in descending order of quality. As such, it + // is sufficient to return (q > 0) when reaching either an "identity" or "*" item. + for q in items { + match (q.quality, &q.item) { + // occurrence of "identity;q=n"; return true if quality is non-zero + (q, Preference::Specific(Encoding::Known(ContentEncoding::Identity))) => { + return q > Quality::ZERO + } + + // occurrence of "*;q=n"; return true if quality is non-zero + (q, Preference::Any) => return q > Quality::ZERO, + + _ => {} + } + } + + // implicit acceptable identity + true +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::http::header::*; + + macro_rules! accept_encoding { + () => { AcceptEncoding(vec![]) }; + ($($q:expr),+ $(,)?) => { AcceptEncoding(vec![$($q.parse().unwrap()),+]) }; + } + + /// Parses an encoding string. + fn enc(enc: &str) -> Preference { + enc.parse().unwrap() + } + + #[test] + fn detect_identity_acceptable() { + macro_rules! accept_encoding_ranked { + () => { accept_encoding!().ranked_items().collect::>() }; + ($($q:expr),+ $(,)?) => { accept_encoding!($($q),+).ranked_items().collect::>() }; + } + + let test = accept_encoding_ranked!(); + assert!(is_identity_acceptable(&test)); + let test = accept_encoding_ranked!("gzip"); + assert!(is_identity_acceptable(&test)); + let test = accept_encoding_ranked!("gzip", "br"); + assert!(is_identity_acceptable(&test)); + let test = accept_encoding_ranked!("gzip", "*;q=0.1"); + assert!(is_identity_acceptable(&test)); + let test = accept_encoding_ranked!("gzip", "identity;q=0.1"); + assert!(is_identity_acceptable(&test)); + let test = accept_encoding_ranked!("gzip", "identity;q=0.1", "*;q=0"); + assert!(is_identity_acceptable(&test)); + let test = accept_encoding_ranked!("gzip", "*;q=0", "identity;q=0.1"); + assert!(is_identity_acceptable(&test)); + + let test = accept_encoding_ranked!("gzip", "*;q=0"); + assert!(!is_identity_acceptable(&test)); + let test = accept_encoding_ranked!("gzip", "identity;q=0"); + assert!(!is_identity_acceptable(&test)); + let test = accept_encoding_ranked!("gzip", "identity;q=0", "*;q=0"); + assert!(!is_identity_acceptable(&test)); + let test = accept_encoding_ranked!("gzip", "*;q=0", "identity;q=0"); + assert!(!is_identity_acceptable(&test)); + } + + #[test] + fn encoding_negotiation() { + // no preference + let test = accept_encoding!(); + assert_eq!(test.negotiate([].iter()), None); + + let test = accept_encoding!(); + assert_eq!( + test.negotiate([Encoding::identity()].iter()), + Some(Encoding::identity()), + ); + + let test = accept_encoding!("identity;q=0"); + assert_eq!(test.negotiate([Encoding::identity()].iter()), None); + + let test = accept_encoding!("*;q=0"); + assert_eq!(test.negotiate([Encoding::identity()].iter()), None); + + let test = accept_encoding!(); + assert_eq!( + test.negotiate([Encoding::gzip(), Encoding::identity()].iter()), + Some(Encoding::identity()), + ); + + let test = accept_encoding!("gzip"); + assert_eq!( + test.negotiate([Encoding::gzip(), Encoding::identity()].iter()), + Some(Encoding::gzip()), + ); + assert_eq!( + test.negotiate([Encoding::brotli(), Encoding::identity()].iter()), + Some(Encoding::identity()), + ); + assert_eq!( + test.negotiate([Encoding::brotli(), Encoding::gzip(), Encoding::identity()].iter()), + Some(Encoding::gzip()), + ); + + let test = accept_encoding!("gzip", "identity;q=0"); + assert_eq!( + test.negotiate([Encoding::gzip(), Encoding::identity()].iter()), + Some(Encoding::gzip()), + ); + assert_eq!( + test.negotiate([Encoding::brotli(), Encoding::identity()].iter()), + None + ); + + let test = accept_encoding!("gzip", "*;q=0"); + assert_eq!( + test.negotiate([Encoding::gzip(), Encoding::identity()].iter()), + Some(Encoding::gzip()), + ); + assert_eq!( + test.negotiate([Encoding::brotli(), Encoding::identity()].iter()), + None + ); + + let test = accept_encoding!("gzip", "deflate", "br"); + assert_eq!( + test.negotiate([Encoding::gzip(), Encoding::identity()].iter()), + Some(Encoding::gzip()), + ); + assert_eq!( + test.negotiate([Encoding::brotli(), Encoding::identity()].iter()), + Some(Encoding::brotli()) + ); + assert_eq!( + test.negotiate([Encoding::deflate(), Encoding::identity()].iter()), + Some(Encoding::deflate()) + ); + assert_eq!( + test.negotiate( + [Encoding::gzip(), Encoding::deflate(), Encoding::identity()].iter() + ), + Some(Encoding::gzip()) + ); + assert_eq!( + test.negotiate([Encoding::gzip(), Encoding::brotli(), Encoding::identity()].iter()), + Some(Encoding::gzip()) + ); + assert_eq!( + test.negotiate([Encoding::brotli(), Encoding::gzip(), Encoding::identity()].iter()), + Some(Encoding::gzip()) + ); + } + + #[test] + fn ranking_precedence() { + let test = accept_encoding!(); + assert!(test.ranked().is_empty()); + + let test = accept_encoding!("gzip"); + assert_eq!(test.ranked(), vec![enc("gzip")]); + + let test = accept_encoding!("gzip;q=0.900", "*;q=0.700", "br;q=1.0"); + assert_eq!(test.ranked(), vec![enc("br"), enc("gzip"), enc("*")]); + + let test = accept_encoding!("br", "gzip", "*"); + assert_eq!(test.ranked(), vec![enc("br"), enc("gzip"), enc("*")]); + } + + #[test] + fn preference_selection() { + assert_eq!(accept_encoding!().preference(), Some(Preference::Any)); + + assert_eq!(accept_encoding!("identity;q=0").preference(), None); + assert_eq!(accept_encoding!("*;q=0").preference(), None); + assert_eq!(accept_encoding!("compress;q=0", "*;q=0").preference(), None); + assert_eq!(accept_encoding!("identity;q=0", "*;q=0").preference(), None); + + let test = accept_encoding!("*;q=0.5"); + assert_eq!(test.preference().unwrap(), enc("*")); + + let test = accept_encoding!("br;q=0"); + assert_eq!(test.preference().unwrap(), enc("identity")); + + let test = accept_encoding!("br;q=0.900", "gzip;q=1.0", "*;q=0.500"); + assert_eq!(test.preference().unwrap(), enc("gzip")); + + let test = accept_encoding!("br", "gzip", "*"); + assert_eq!(test.preference().unwrap(), enc("br")); + } +} diff --git a/src/http/header/accept_language.rs b/src/http/header/accept_language.rs index 011257b87..9943e121f 100644 --- a/src/http/header/accept_language.rs +++ b/src/http/header/accept_language.rs @@ -37,7 +37,7 @@ common_header! { /// let mut builder = HttpResponse::Ok(); /// builder.insert_header( /// AcceptLanguage(vec![ - /// QualityItem::max("en-US".parse().unwrap()) + /// "en-US".parse().unwrap(), /// ]) /// ); /// ``` @@ -49,9 +49,9 @@ common_header! { /// let mut builder = HttpResponse::Ok(); /// builder.insert_header( /// AcceptLanguage(vec![ - /// QualityItem::max("da".parse().unwrap()), - /// QualityItem::new("en-GB".parse().unwrap(), q(0.8)), - /// QualityItem::new("en".parse().unwrap(), q(0.7)), + /// "da".parse().unwrap(), + /// "en-GB;q=0.8".parse().unwrap(), + /// "en;q=0.7".parse().unwrap(), /// ]) /// ); /// ``` @@ -93,6 +93,33 @@ common_header! { } impl AcceptLanguage { + /// Extracts the most preferable language, accounting for [q-factor weighting]. + /// + /// If no q-factors are provided, the first language is chosen. Note that items without + /// q-factors are given the maximum preference value. + /// + /// As per the spec, returns [`Preference::Any`] if contained list is empty. + /// + /// [q-factor weighting]: https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.2 + pub fn preference(&self) -> Preference { + let mut max_item = None; + let mut max_pref = Quality::ZERO; + + // uses manual max lookup loop since we want the first occurrence in the case of same + // preference but `Iterator::max_by_key` would give us the last occurrence + + for pref in &self.0 { + // only change if strictly greater + // equal items, even while unsorted, still have higher preference if they appear first + if pref.quality > max_pref { + max_pref = pref.quality; + max_item = Some(pref.item.clone()); + } + } + + max_item.unwrap_or(Preference::Any) + } + /// Returns a sorted list of languages from highest to lowest precedence, accounting /// for [q-factor weighting]. /// @@ -112,33 +139,6 @@ impl AcceptLanguage { types.into_iter().map(|qitem| qitem.item).collect() } - - /// Extracts the most preferable language, accounting for [q-factor weighting]. - /// - /// If no q-factors are provided, the first language is chosen. Note that items without - /// q-factors are given the maximum preference value. - /// - /// As per the spec, returns [`Preference::Any`] if contained list is empty. - /// - /// [q-factor weighting]: https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.2 - pub fn preference(&self) -> Preference { - let mut max_item = None; - let mut max_pref = Quality::MIN; - - // uses manual max lookup loop since we want the first occurrence in the case of same - // preference but `Iterator::max_by_key` would give us the last occurrence - - for pref in &self.0 { - // only change if strictly greater - // equal items, even while unsorted, still have higher preference if they appear first - if pref.quality > max_pref { - max_pref = pref.quality; - max_item = Some(pref.item.clone()); - } - } - - max_item.unwrap_or(Preference::Any) - } } #[cfg(test)] @@ -152,7 +152,7 @@ mod tests { assert!(test.ranked().is_empty()); let test = AcceptLanguage(vec![QualityItem::max("fr-CH".parse().unwrap())]); - assert_eq!(test.ranked(), vec!("fr-CH".parse().unwrap())); + assert_eq!(test.ranked(), vec!["fr-CH".parse().unwrap()]); let test = AcceptLanguage(vec![ QualityItem::new("fr".parse().unwrap(), q(0.900)), diff --git a/src/http/header/content_disposition.rs b/src/http/header/content_disposition.rs index 26a9d8e76..8b7101aa1 100644 --- a/src/http/header/content_disposition.rs +++ b/src/http/header/content_disposition.rs @@ -301,7 +301,6 @@ impl DispositionParam { /// change to match local file system conventions if applicable, and do not use directory path /// information that may be present. /// See [RFC 2183 §2.3](https://datatracker.ietf.org/doc/html/rfc2183#section-2.3). -// TODO: think about using private fields and smallvec #[derive(Clone, Debug, PartialEq)] pub struct ContentDisposition { /// The disposition type diff --git a/src/http/header/encoding.rs b/src/http/header/encoding.rs index a61edda67..ab11f50be 100644 --- a/src/http/header/encoding.rs +++ b/src/http/header/encoding.rs @@ -1,69 +1,55 @@ use std::{fmt, str}; -pub use self::Encoding::{ - Brotli, Chunked, Compress, Deflate, EncodingExt, Gzip, Identity, Trailers, Zstd, -}; +use actix_http::ContentEncoding; -/// A value to represent an encoding used in `Transfer-Encoding` or `Accept-Encoding` header. -#[derive(Debug, Clone, PartialEq, Eq)] +/// A value to represent an encoding used in the `Accept-Encoding` and `Content-Encoding` header. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub enum Encoding { - /// The `chunked` encoding. - Chunked, + /// A supported content encoding. See [`ContentEncoding`] for variants. + Known(ContentEncoding), - /// The `br` encoding. - Brotli, + /// Some other encoding that is less common, can be any string. + Unknown(String), +} - /// The `gzip` encoding. - Gzip, +impl Encoding { + pub const fn identity() -> Self { + Self::Known(ContentEncoding::Identity) + } - /// The `deflate` encoding. - Deflate, + pub const fn brotli() -> Self { + Self::Known(ContentEncoding::Brotli) + } - /// The `compress` encoding. - Compress, + pub const fn deflate() -> Self { + Self::Known(ContentEncoding::Deflate) + } - /// The `identity` encoding. - Identity, + pub const fn gzip() -> Self { + Self::Known(ContentEncoding::Gzip) + } - /// The `trailers` encoding. - Trailers, - - /// The `zstd` encoding. - Zstd, - - /// Some other encoding that is less common, can be any String. - EncodingExt(String), + pub const fn zstd() -> Self { + Self::Known(ContentEncoding::Zstd) + } } impl fmt::Display for Encoding { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str(match *self { - Chunked => "chunked", - Brotli => "br", - Gzip => "gzip", - Deflate => "deflate", - Compress => "compress", - Identity => "identity", - Trailers => "trailers", - Zstd => "zstd", - EncodingExt(ref s) => s.as_ref(), + f.write_str(match self { + Encoding::Known(enc) => enc.as_str(), + Encoding::Unknown(enc) => enc.as_str(), }) } } impl str::FromStr for Encoding { type Err = crate::error::ParseError; - fn from_str(s: &str) -> Result { - match s { - "chunked" => Ok(Chunked), - "br" => Ok(Brotli), - "deflate" => Ok(Deflate), - "gzip" => Ok(Gzip), - "compress" => Ok(Compress), - "identity" => Ok(Identity), - "trailers" => Ok(Trailers), - "zstd" => Ok(Zstd), - _ => Ok(EncodingExt(s.to_owned())), + + fn from_str(enc: &str) -> Result { + match enc.parse::() { + Ok(enc) => Ok(Self::Known(enc)), + Err(_) => Ok(Self::Unknown(enc.to_owned())), } } } diff --git a/src/middleware/compress.rs b/src/middleware/compress.rs index d3cdf5763..3a0d5630d 100644 --- a/src/middleware/compress.rs +++ b/src/middleware/compress.rs @@ -1,20 +1,13 @@ //! For middleware documentation, see [`Compress`]. use std::{ - cmp, - convert::TryFrom as _, future::Future, marker::PhantomData, pin::Pin, task::{Context, Poll}, }; -use actix_http::{ - body::{EitherBody, MessageBody}, - encoding::Encoder, - header::{ContentEncoding, ACCEPT_ENCODING}, - StatusCode, -}; +use actix_http::encoding::Encoder; use actix_service::{Service, Transform}; use actix_utils::future::{ok, Either, Ready}; use futures_core::ready; @@ -22,9 +15,14 @@ use once_cell::sync::Lazy; use pin_project_lite::pin_project; use crate::{ - dev::BodyEncoding, + body::{EitherBody, MessageBody}, + dev::BodyEncoding as _, + http::{ + header::{self, AcceptEncoding, Encoding, HeaderValue}, + StatusCode, + }, service::{ServiceRequest, ServiceResponse}, - Error, HttpResponse, + Error, HttpMessage, HttpResponse, }; /// Middleware for compressing response payloads. @@ -40,21 +38,9 @@ use crate::{ /// .wrap(middleware::Compress::default()) /// .default_service(web::to(|| HttpResponse::NotFound())); /// ``` -#[derive(Debug, Clone)] -pub struct Compress(ContentEncoding); - -impl Compress { - /// Create new `Compress` middleware with the specified encoding. - pub fn new(encoding: ContentEncoding) -> Self { - Compress(encoding) - } -} - -impl Default for Compress { - fn default() -> Self { - Compress::new(ContentEncoding::Auto) - } -} +#[derive(Debug, Clone, Default)] +#[non_exhaustive] +pub struct Compress; impl Transform for Compress where @@ -68,44 +54,14 @@ where type Future = Ready>; fn new_transform(&self, service: S) -> Self::Future { - ok(CompressMiddleware { - service, - encoding: self.0, - }) + ok(CompressMiddleware { service }) } } pub struct CompressMiddleware { service: S, - encoding: ContentEncoding, } -static SUPPORTED_ALGORITHM_NAMES: Lazy = Lazy::new(|| { - #[allow(unused_mut)] // only unused when no compress features enabled - let mut encoding: Vec<&str> = vec![]; - - #[cfg(feature = "compress-brotli")] - { - encoding.push("br"); - } - - #[cfg(feature = "compress-gzip")] - { - encoding.push("gzip"); - encoding.push("deflate"); - } - - #[cfg(feature = "compress-zstd")] - encoding.push("zstd"); - - assert!( - !encoding.is_empty(), - "encoding can not be empty unless __compress feature has been explicitly enabled by itself" - ); - - encoding.join(", ") -}); - impl Service for CompressMiddleware where S: Service, Error = Error>, @@ -121,39 +77,43 @@ where #[allow(clippy::borrow_interior_mutable_const)] fn call(&self, req: ServiceRequest) -> Self::Future { // negotiate content-encoding - let encoding_result = req - .headers() - .get(&ACCEPT_ENCODING) - .and_then(|val| val.to_str().ok()) - .map(|enc| AcceptEncoding::try_parse(enc, self.encoding)); + let accept_encoding = req.get_header::(); - match encoding_result { - // Missing header => fallback to identity - None => Either::left(CompressResponse { - encoding: ContentEncoding::Identity, - fut: self.service.call(req), - _phantom: PhantomData, - }), + let accept_encoding = match accept_encoding { + // missing header; fallback to identity + None => { + return Either::left(CompressResponse { + encoding: Encoding::identity(), + fut: self.service.call(req), + _phantom: PhantomData, + }) + } - // Valid encoding - Some(Ok(encoding)) => Either::left(CompressResponse { - encoding, - fut: self.service.call(req), - _phantom: PhantomData, - }), + // valid accept-encoding header + Some(accept_encoding) => accept_encoding, + }; - // There is an HTTP header but we cannot match what client as asked for - Some(Err(_)) => { - let res = HttpResponse::with_body( + match accept_encoding.negotiate(SUPPORTED_ENCODINGS.iter()) { + None => { + let mut res = HttpResponse::with_body( StatusCode::NOT_ACCEPTABLE, - SUPPORTED_ALGORITHM_NAMES.clone(), + SUPPORTED_ENCODINGS_STRING.as_str(), ); + res.headers_mut() + .insert(header::VARY, HeaderValue::from_static("Accept-Encoding")); + Either::right(ok(req .into_response(res) .map_into_boxed_body() .map_into_right_body())) } + + Some(encoding) => Either::left(CompressResponse { + fut: self.service.call(req), + encoding, + _phantom: PhantomData, + }), } } } @@ -165,7 +125,7 @@ pin_project! { { #[pin] fut: S::Future, - encoding: ContentEncoding, + encoding: Encoding, _phantom: PhantomData, } } @@ -182,10 +142,15 @@ where match ready!(this.fut.poll(cx)) { Ok(resp) => { - let enc = if let Some(enc) = resp.response().get_encoding() { + let enc = if let Some(enc) = resp.response().preferred_encoding() { enc } else { - *this.encoding + match this.encoding { + Encoding::Known(enc) => *enc, + Encoding::Unknown(enc) => { + unimplemented!("encoding {} should not be here", enc); + } + } }; Poll::Ready(Ok(resp.map_body(move |head, body| { @@ -198,178 +163,57 @@ where } } -struct AcceptEncoding { - encoding: ContentEncoding, - // TODO: use Quality or QualityItem - quality: f64, -} +static SUPPORTED_ENCODINGS_STRING: Lazy = Lazy::new(|| { + #[allow(unused_mut)] // only unused when no compress features enabled + let mut encoding: Vec<&str> = vec![]; -impl Eq for AcceptEncoding {} - -impl Ord for AcceptEncoding { - #[allow(clippy::comparison_chain)] - fn cmp(&self, other: &AcceptEncoding) -> cmp::Ordering { - if self.quality > other.quality { - cmp::Ordering::Less - } else if self.quality < other.quality { - cmp::Ordering::Greater - } else { - cmp::Ordering::Equal - } - } -} - -impl PartialOrd for AcceptEncoding { - fn partial_cmp(&self, other: &AcceptEncoding) -> Option { - Some(self.cmp(other)) - } -} - -impl PartialEq for AcceptEncoding { - fn eq(&self, other: &AcceptEncoding) -> bool { - self.encoding == other.encoding && self.quality == other.quality - } -} - -/// Parse q-factor from quality strings. -/// -/// If parse fail, then fallback to default value which is 1. -/// More details available here: -fn parse_quality(parts: &[&str]) -> f64 { - for part in parts { - if part.trim().starts_with("q=") { - return part[2..].parse().unwrap_or(1.0); - } + #[cfg(feature = "compress-brotli")] + { + encoding.push("br"); } - 1.0 -} - -#[derive(Debug, PartialEq, Eq)] -enum AcceptEncodingError { - /// This error occurs when client only support compressed response and server do not have any - /// algorithm that match client accepted algorithms. - CompressionAlgorithmMismatch, -} - -impl AcceptEncoding { - fn new(tag: &str) -> Option { - let parts: Vec<&str> = tag.split(';').collect(); - let encoding = match parts.len() { - 0 => return None, - _ => match ContentEncoding::try_from(parts[0]) { - Err(_) => return None, - Ok(x) => x, - }, - }; - - let quality = parse_quality(&parts[1..]); - if quality <= 0.0 || quality > 1.0 { - return None; - } - - Some(AcceptEncoding { encoding, quality }) + #[cfg(feature = "compress-gzip")] + { + encoding.push("gzip"); + encoding.push("deflate"); } - /// Parse a raw Accept-Encoding header value into an ordered list then return the best match - /// based on middleware configuration. - pub fn try_parse( - raw: &str, - encoding: ContentEncoding, - ) -> Result { - let mut encodings = raw - .replace(' ', "") - .split(',') - .filter_map(AcceptEncoding::new) - .collect::>(); - - encodings.sort(); - - for enc in encodings { - if encoding == ContentEncoding::Auto || encoding == enc.encoding { - return Ok(enc.encoding); - } - } - - // Special case if user cannot accept uncompressed data. - // See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Accept-Encoding - // TODO: account for whitespace - if raw.contains("*;q=0") || raw.contains("identity;q=0") { - return Err(AcceptEncodingError::CompressionAlgorithmMismatch); - } - - Ok(ContentEncoding::Identity) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - macro_rules! assert_parse_eq { - ($raw:expr, $result:expr) => { - assert_eq!( - AcceptEncoding::try_parse($raw, ContentEncoding::Auto), - Ok($result) - ); - }; + #[cfg(feature = "compress-zstd")] + { + encoding.push("zstd"); } - macro_rules! assert_parse_fail { - ($raw:expr) => { - assert!(AcceptEncoding::try_parse($raw, ContentEncoding::Auto).is_err()); - }; + assert!( + !encoding.is_empty(), + "encoding can not be empty unless __compress feature has been explicitly enabled by itself" + ); + + encoding.join(", ") +}); + +static SUPPORTED_ENCODINGS: Lazy> = Lazy::new(|| { + let mut encodings = vec![Encoding::identity()]; + + #[cfg(feature = "compress-brotli")] + { + encodings.push(Encoding::brotli()); } - #[test] - fn test_parse_encoding() { - // Test simple case - assert_parse_eq!("br", ContentEncoding::Br); - assert_parse_eq!("gzip", ContentEncoding::Gzip); - assert_parse_eq!("deflate", ContentEncoding::Deflate); - assert_parse_eq!("zstd", ContentEncoding::Zstd); - - // Test space, trim, missing values - assert_parse_eq!("br,,,,", ContentEncoding::Br); - assert_parse_eq!("gzip , br, zstd", ContentEncoding::Gzip); - - // Test float number parsing - assert_parse_eq!("br;q=1 ,", ContentEncoding::Br); - assert_parse_eq!("br;q=1.0 , br", ContentEncoding::Br); - - // Test wildcard - assert_parse_eq!("*", ContentEncoding::Identity); - assert_parse_eq!("*;q=1.0", ContentEncoding::Identity); + #[cfg(feature = "compress-gzip")] + { + encodings.push(Encoding::gzip()); + encodings.push(Encoding::deflate()); } - #[test] - fn test_parse_encoding_qfactor_ordering() { - assert_parse_eq!("gzip, br, zstd", ContentEncoding::Gzip); - assert_parse_eq!("zstd, br, gzip", ContentEncoding::Zstd); - - assert_parse_eq!("gzip;q=0.4, br;q=0.6", ContentEncoding::Br); - assert_parse_eq!("gzip;q=0.8, br;q=0.4", ContentEncoding::Gzip); + #[cfg(feature = "compress-zstd")] + { + encodings.push(Encoding::zstd()); } - #[test] - fn test_parse_encoding_qfactor_invalid() { - // Out of range - assert_parse_eq!("gzip;q=-5.0", ContentEncoding::Identity); - assert_parse_eq!("gzip;q=5.0", ContentEncoding::Identity); + assert!( + !encodings.is_empty(), + "encodings can not be empty unless __compress feature has been explicitly enabled by itself" + ); - // Disabled - assert_parse_eq!("gzip;q=0", ContentEncoding::Identity); - } - - #[test] - fn test_parse_compression_required() { - // Check we fallback to identity if there is an unsupported compression algorithm - assert_parse_eq!("compress", ContentEncoding::Identity); - - // User do not want any compression - assert_parse_fail!("compress, identity;q=0"); - assert_parse_fail!("compress, identity;q=0.0"); - assert_parse_fail!("compress, *;q=0"); - assert_parse_fail!("compress, *;q=0.0"); - } -} + encodings +}); diff --git a/src/types/mod.rs b/src/types/mod.rs index 461d771eb..bab7c3bc0 100644 --- a/src/types/mod.rs +++ b/src/types/mod.rs @@ -1,19 +1,18 @@ //! Common extractors and responders. -// TODO: review visibility mod either; -pub(crate) mod form; +mod form; mod header; -pub(crate) mod json; +mod json; mod path; -pub(crate) mod payload; +mod payload; mod query; -pub(crate) mod readlines; +mod readlines; -pub use self::either::{Either, EitherExtractError}; -pub use self::form::{Form, FormConfig}; +pub use self::either::Either; +pub use self::form::{Form, FormConfig, UrlEncoded}; pub use self::header::Header; -pub use self::json::{Json, JsonConfig}; +pub use self::json::{Json, JsonBody, JsonConfig}; pub use self::path::{Path, PathConfig}; pub use self::payload::{Payload, PayloadConfig}; pub use self::query::{Query, QueryConfig}; diff --git a/src/types/payload.rs b/src/types/payload.rs index 73987def5..d2ab29639 100644 --- a/src/types/payload.rs +++ b/src/types/payload.rs @@ -248,6 +248,7 @@ impl PayloadConfig { } } } + Ok(()) } diff --git a/src/web.rs b/src/web.rs index e4339352b..4858600af 100644 --- a/src/web.rs +++ b/src/web.rs @@ -2,13 +2,12 @@ use std::future::Future; -use actix_http::Method; use actix_router::IntoPatterns; pub use bytes::{Buf, BufMut, Bytes, BytesMut}; use crate::{ - error::BlockingError, extract::FromRequest, handler::Handler, resource::Resource, - route::Route, scope::Scope, service::WebService, Responder, + error::BlockingError, http::Method, service::WebService, FromRequest, Handler, Resource, + Responder, Route, Scope, }; pub use crate::config::ServiceConfig; diff --git a/tests/compression.rs b/tests/compression.rs new file mode 100644 index 000000000..c0bf10e4c --- /dev/null +++ b/tests/compression.rs @@ -0,0 +1,313 @@ +use actix_http::ContentEncoding; +use actix_web::{ + dev::BodyEncoding as _, + http::{header, StatusCode}, + middleware::Compress, + web, App, HttpResponse, +}; +use bytes::Bytes; + +mod test_utils; +use test_utils::{brotli, gzip, zstd}; + +static LOREM: &[u8] = include_bytes!("fixtures/lorem.txt"); +static LOREM_GZIP: &[u8] = include_bytes!("fixtures/lorem.txt.gz"); +static LOREM_BR: &[u8] = include_bytes!("fixtures/lorem.txt.br"); +static LOREM_ZSTD: &[u8] = include_bytes!("fixtures/lorem.txt.zst"); +static LOREM_XZ: &[u8] = include_bytes!("fixtures/lorem.txt.xz"); + +macro_rules! test_server { + () => { + actix_test::start(|| { + App::new() + .wrap(Compress::default()) + .route("/static", web::to(|| HttpResponse::Ok().body(LOREM))) + .route( + "/static-gzip", + web::to(|| { + HttpResponse::Ok() + // signal to compressor that content should not be altered + .encode_with(ContentEncoding::Identity) + // signal to client that content is encoded + .insert_header(ContentEncoding::Gzip) + .body(LOREM_GZIP) + }), + ) + .route( + "/static-br", + web::to(|| { + HttpResponse::Ok() + // signal to compressor that content should not be altered + .encode_with(ContentEncoding::Identity) + // signal to client that content is encoded + .insert_header(ContentEncoding::Brotli) + .body(LOREM_BR) + }), + ) + .route( + "/static-zstd", + web::to(|| { + HttpResponse::Ok() + // signal to compressor that content should not be altered + .encode_with(ContentEncoding::Identity) + // signal to client that content is encoded + .insert_header(ContentEncoding::Zstd) + .body(LOREM_ZSTD) + }), + ) + .route( + "/static-xz", + web::to(|| { + HttpResponse::Ok() + // signal to compressor that content should not be altered + .encode_with(ContentEncoding::Identity) + // signal to client that content is encoded as 7zip + .insert_header((header::CONTENT_ENCODING, "xz")) + .body(LOREM_XZ) + }), + ) + .route( + "/echo", + web::to(|body: Bytes| HttpResponse::Ok().body(body)), + ) + }) + }; +} + +#[actix_rt::test] +async fn negotiate_encoding_identity() { + let srv = test_server!(); + + let req = srv + .post("/static") + .insert_header((header::ACCEPT_ENCODING, "identity")) + .send(); + + let mut res = req.await.unwrap(); + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers().get(header::CONTENT_ENCODING), None); + + let bytes = res.body().await.unwrap(); + assert_eq!(bytes, Bytes::from_static(LOREM)); + + srv.stop().await; +} + +#[actix_rt::test] +async fn negotiate_encoding_gzip() { + let srv = test_server!(); + + let req = srv + .post("/static") + .insert_header((header::ACCEPT_ENCODING, "gzip,br,zstd")) + .send(); + + let mut res = req.await.unwrap(); + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers().get(header::CONTENT_ENCODING).unwrap(), "gzip"); + + let bytes = res.body().await.unwrap(); + assert_eq!(bytes, Bytes::from_static(LOREM)); + + let mut res = srv + .post("/static") + .no_decompress() + .insert_header((header::ACCEPT_ENCODING, "gzip,br,zstd")) + .send() + .await + .unwrap(); + let bytes = res.body().await.unwrap(); + assert_eq!(gzip::decode(bytes), LOREM); + + srv.stop().await; +} + +#[actix_rt::test] +async fn negotiate_encoding_br() { + let srv = test_server!(); + + let req = srv + .post("/static") + .insert_header((header::ACCEPT_ENCODING, "br,zstd,gzip")) + .send(); + + let mut res = req.await.unwrap(); + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers().get(header::CONTENT_ENCODING).unwrap(), "br"); + + let bytes = res.body().await.unwrap(); + assert_eq!(bytes, Bytes::from_static(LOREM)); + + let mut res = srv + .post("/static") + .no_decompress() + .insert_header((header::ACCEPT_ENCODING, "br,zstd,gzip")) + .send() + .await + .unwrap(); + let bytes = res.body().await.unwrap(); + assert_eq!(brotli::decode(bytes), LOREM); + + srv.stop().await; +} + +#[actix_rt::test] +async fn negotiate_encoding_zstd() { + let srv = test_server!(); + + let req = srv + .post("/static") + .insert_header((header::ACCEPT_ENCODING, "zstd,gzip,br")) + .send(); + + let mut res = req.await.unwrap(); + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers().get(header::CONTENT_ENCODING).unwrap(), "zstd"); + + let bytes = res.body().await.unwrap(); + assert_eq!(bytes, Bytes::from_static(LOREM)); + + let mut res = srv + .post("/static") + .no_decompress() + .insert_header((header::ACCEPT_ENCODING, "zstd,gzip,br")) + .send() + .await + .unwrap(); + let bytes = res.body().await.unwrap(); + assert_eq!(zstd::decode(bytes), LOREM); + + srv.stop().await; +} + +#[cfg(all( + feature = "compress-brotli", + feature = "compress-gzip", + feature = "compress-zstd", +))] +#[actix_rt::test] +async fn client_encoding_prefers_brotli() { + let srv = test_server!(); + + let req = srv.post("/static").send(); + + let mut res = req.await.unwrap(); + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers().get(header::CONTENT_ENCODING).unwrap(), "br"); + + let bytes = res.body().await.unwrap(); + assert_eq!(bytes, Bytes::from_static(LOREM)); + + srv.stop().await; +} + +#[actix_rt::test] +async fn gzip_no_decompress() { + let srv = test_server!(); + + let req = srv + .post("/static-gzip") + // don't decompress response body + .no_decompress() + // signal that we want a compressed body + .insert_header((header::ACCEPT_ENCODING, "gzip,br,zstd")) + .send(); + + let mut res = req.await.unwrap(); + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers().get(header::CONTENT_ENCODING).unwrap(), "gzip"); + + let bytes = res.body().await.unwrap(); + assert_eq!(bytes, Bytes::from_static(LOREM_GZIP)); + + srv.stop().await; +} + +#[actix_rt::test] +async fn manual_custom_coding() { + let srv = test_server!(); + + let req = srv + .post("/static-xz") + // don't decompress response body + .no_decompress() + // signal that we want a compressed body + .insert_header((header::ACCEPT_ENCODING, "xz")) + .send(); + + let mut res = req.await.unwrap(); + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers().get(header::CONTENT_ENCODING).unwrap(), "xz"); + + let bytes = res.body().await.unwrap(); + assert_eq!(bytes, Bytes::from_static(LOREM_XZ)); + + srv.stop().await; +} + +#[actix_rt::test] +async fn deny_identity_coding() { + let srv = test_server!(); + + let req = srv + .post("/static") + // signal that we want a compressed body + .insert_header((header::ACCEPT_ENCODING, "br, identity;q=0")) + .send(); + + let mut res = req.await.unwrap(); + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers().get(header::CONTENT_ENCODING).unwrap(), "br"); + + let bytes = res.body().await.unwrap(); + assert_eq!(bytes, Bytes::from_static(LOREM)); + + srv.stop().await; +} + +#[actix_rt::test] +async fn deny_identity_coding_no_decompress() { + let srv = test_server!(); + + let req = srv + .post("/static-br") + // don't decompress response body + .no_decompress() + // signal that we want a compressed body + .insert_header((header::ACCEPT_ENCODING, "br, identity;q=0")) + .send(); + + let mut res = req.await.unwrap(); + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers().get(header::CONTENT_ENCODING).unwrap(), "br"); + + let bytes = res.body().await.unwrap(); + assert_eq!(bytes, Bytes::from_static(LOREM_BR)); + + srv.stop().await; +} + +// TODO: fix test +// currently fails because negotiation doesn't consider unknown encoding types +#[ignore] +#[actix_rt::test] +async fn deny_identity_for_manual_coding() { + let srv = test_server!(); + + let req = srv + .post("/static-xz") + // don't decompress response body + .no_decompress() + // signal that we want a compressed body + .insert_header((header::ACCEPT_ENCODING, "xz, identity;q=0")) + .send(); + + let mut res = req.await.unwrap(); + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers().get(header::CONTENT_ENCODING).unwrap(), "xz"); + + let bytes = res.body().await.unwrap(); + assert_eq!(bytes, Bytes::from_static(LOREM_XZ)); + + srv.stop().await; +} diff --git a/tests/fixtures/lorem.txt b/tests/fixtures/lorem.txt new file mode 100644 index 000000000..a2abb6432 --- /dev/null +++ b/tests/fixtures/lorem.txt @@ -0,0 +1,5 @@ +Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin interdum tincidunt lacus, sed tempor lorem consectetur et. Pellentesque et egestas sem, at cursus massa. Nunc feugiat elit sit amet ipsum commodo luctus. Proin auctor dignissim pharetra. Integer iaculis quam a tellus auctor, vitae auctor nisl viverra. Nullam consequat maximus porttitor. Pellentesque tortor enim, molestie at varius non, tempor non nibh. Suspendisse tempus erat lorem, vel faucibus magna blandit vel. Sed pellentesque ligula augue, vitae fermentum eros blandit et. Cras dignissim in massa ut varius. Vestibulum commodo nunc sit amet pellentesque dignissim. + +Donec imperdiet blandit lobortis. Suspendisse fringilla nunc quis venenatis tempor. Nunc tempor sed erat sed convallis. Pellentesque aliquet elit lectus, quis vulputate arcu pharetra sed. Etiam laoreet aliquet arcu cursus vehicula. Maecenas odio odio, elementum faucibus sollicitudin vitae, pellentesque ac purus. Donec venenatis faucibus lorem, et finibus lacus tincidunt vitae. Quisque laoreet metus sapien, vitae euismod mauris lobortis malesuada. Integer sit amet elementum turpis. Maecenas ex mauris, dapibus eu placerat vitae, rutrum convallis enim. Nulla vitae orci ultricies, sagittis turpis et, lacinia dui. Praesent egestas urna turpis, sit amet feugiat mauris tristique eu. Quisque id tempor libero. Donec ullamcorper dapibus lorem, vel consequat risus congue a. + +Nullam dignissim ut lectus vitae tempor. Pellentesque ut odio fringilla, volutpat mi et, vulputate tellus. Fusce eget diam non odio tincidunt viverra eu vel augue. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia curae; Nullam sed eleifend purus, vitae aliquam orci. Cras fringilla justo eget tempus bibendum. Phasellus venenatis, odio nec pulvinar commodo, quam neque lacinia turpis, ut rutrum tortor massa eu nulla. Vivamus tincidunt ut lectus a gravida. Donec varius mi quis enim interdum ultrices. Sed aliquam consectetur nisi vitae viverra. Praesent nec ligula egestas, porta lectus sed, consectetur augue. diff --git a/tests/fixtures/lorem.txt.br b/tests/fixtures/lorem.txt.br new file mode 100644 index 0000000000000000000000000000000000000000..e12d7510ee184cbd234d803327ee4f17285eb5be GIT binary patch literal 905 zcmV;419tquKtKQ>rfUAzr>O?L;iCEFef5mQ_{~D193IB{^CrMb%JNqgxdcY!GuiB4Q*LyOU-gf=gDZa z13H*He)yu{FUh~(Ya(fly(}@LRcr|DP|4_WjB%ofX${h=V=S!raXd?*2|YAY0-fSXL{(T_o2SrYjaJguL#0Wb-#9r9^!2279s?dde4r3#dO|l zAR!C397=Hx7Vnw;c6+l-8#qsO%e29-FxGu}GCGic7{M6FkRfW%I!(a!rY6Z~9@wI9 z2oa@W{93j0GWsel0$CSFK#%7!*mXl1je=fu zgqr9{2?nPbFE{x`+m?SY8Pq(J&PZ%Kpw+ZCn%e^J>I)FDgiUSvsa~QNs`HAnlOZE* z*#T?+e!IDeQ(C^rh?)bxWA0WfDP_{wvEf>y@+>05J~$GqwXNLouA*rtdJ>((>@^Z8 z;)s|W!^63YS;Rt@wUuz*6{mADoWV#a_^1;z6BVhnI^*?G>WWr(4vJ6)L|-|nsCDg7 zKGZtT&3{>46`8SNNzM%B{;SAU68!O%JMU_}=8hjP5{7P4$!_&BVh7F|7fsx@vfCt3 zHPwakCy=CG$xQgT`rwEb>_WGTyksiP!FXD*BR&fT%2ms+PC5-!+)bvTXX@Ue0nr3H fZVdLv<|E@cj{%nl-80*ql$VUo3+=PJV4S`IH*m~F literal 0 HcmV?d00001 diff --git a/tests/fixtures/lorem.txt.gz b/tests/fixtures/lorem.txt.gz new file mode 100644 index 0000000000000000000000000000000000000000..d3a0664a320f93c575f0b77417152154140cefc5 GIT binary patch literal 920 zcmV;J184jniwFn>|IlFq18i?{Wo<5Wcys_=RZEWSHVoX)DR=;*FOX$EK!7Aj@PO>u zHrvBMmOT12-tkk^&)sh`)3zn5SXC_j5uzNVXDSD6!G%akGw~sLp*8pf zM^yYBXYUe8f!ArGufhj0Fgu51+s1{Snps8})Npw7lo%@{=eYNU%^6U?>w&%KIcLR& zADYA)t0G>};Sn7ARk2o|92NU~@XO!~zj56C^G4rFI>m2*FZCl9O2nqhDgcV32k_~= zlC^uLyJM`+_6gge!}*HB>0TV8rM<}L?;-KP0?;Tiq*=gZ`Z+?Ai$8E$6`?X<-{@a! z<5H;vU67TO%`^Ff8AA1%sxvmUffwSXRL{9nclH#H~KXz)OQRSuz3`z z3%hln@~Q9^tnw$z3Ydw)rlI~>aGZ3`r+g^@-m1B5pt&luymQAZoh2IQjk&A)S)aooMYewv;trisdpSpj@3$Ct~xM%+&ZohSEb@|1Y2TZ!@K5cf_fxbdZ8)^ zVA_gW8n7gwnkiEv2BocDF6<*+(BJ;xDQ zOS=HPw3%q$83qB>-IYDVTLyw5Z}7p6KKXc?vP(jyJ@Qj+So+kqxQ9kHWz62!MLPIu u$vv8Xy*%L&?cB_}X!_gg_k{0OU+R6t0}Y7&;byP;e*6!TUtO9B2mk=cqQn#c literal 0 HcmV?d00001 diff --git a/tests/fixtures/lorem.txt.xz b/tests/fixtures/lorem.txt.xz new file mode 100644 index 0000000000000000000000000000000000000000..bb45fe7535a6be83dfd723f7cef89a30bf91a8fe GIT binary patch literal 1020 zcmVvH<%0_2e@^`p>hS)*d z1vSlsrF;^l1>2ClFBH>pa+auyeS}tVjcOv8tDZf>RqxED!`*h!Yk|yv>94Zxt%Y>l zx!)*UoMfrNhCmDJ5z-pE^?*Khxa-Rkh5F9&sE&}>fzuKP9f!n_^287?_V%J<`&(N` z%EaV)8FkZ3m^+%RdW2gqzqwI}Q-@s)e7;FLTr8-qXl)82u5Hyax3CUJMg#$f!?{6e zGBqGhAn>3a8r!A2pCP9*NKi{NZ<;hPB&2^lHx8=1J!PW_M*Kw&UoK6hjjm#={;IW1 zr~sJjK%Y_b^=_4(;5Q+kFY@-ALsfu$YNagy9ARpbW;`4-`UsFa@D#G!9mcK&*wutK z3(km&0Jk^-G~}Be%LNxwy6^K`PpRgUE_x+3D6HC*EJDO;B7(0)U)%=bHQZYEY+6;I zB820Pp)Gz4ddfA|<_FHC0EGP$@bE~f)cpQg3{!iop(5NgPt{jUZEVbSg=rNEK>H}D zIp)Mv^j~cD=QMWp7piqRNER2Bp zzI+&u&w$(0wMCe36A*NKf{(uiZKzT{EclGTf3oq|ZGa*e<}tpst}3p&gT%#->^?DE zY~BJL`vq0E>H7W#OrO8WGSj6f+3!~V&YqHqviV3a-(<1gz28HGOyfzUQtT%k?T!noG7SCE zTqagZa=8zm#8ZuT_|B+W7qkPXZ`)VqUHNQAqNuXZq6U zjz!qrdmNv9e3Ys{EG(b|wF%cc*de0X=h*ESnMga0K7|S26hB_NHz>u?T5o(>T|4oC>mUaFFC`5PY5P z_Q{~AG~^+!`Xv0uZzrP@}x z^QzK5oQB4H^|Oz4>rnhswCs$vpjX0(ljl5ZvbFb4-n9oiNN1WPP9RSwLoT>X@CbSC zYIKgaCpfoE=H=AYtEu0ptuzi<&! literal 0 HcmV?d00001 diff --git a/tests/fixtures/lorem.txt.zst b/tests/fixtures/lorem.txt.zst new file mode 100644 index 0000000000000000000000000000000000000000..13bea6d7a26e41c8c2bc848dc4d7ec0e764eed2b GIT binary patch literal 898 zcmV-|1AY7`wJ-f-2?wPc07kQU9biq;2GAngv5Dk9#kP_V3OgqJdn{wnx>gdr1Uzp5 za{zGwhv^wOQK*LHfEYdVcib40maSYZdz?%{C>5u!*md62)c`18UO%Dl-oP2XmDbE&q9GrXhy4N2yJ0|Y`M-JKZuR#&DBx3 z?ohmJEqpAop@rsL%e*;5av7X(Q2cCMI_DlMv1{5%$OifuCdH&z#e_H)J))4c7NX8U zwB(6T{;c$gjofogT}HJK&YvxHqXjRR(4sDuI*8YhGBzSCkKn4QO*@60$1P-&P=4*~ zNU5Rkf)n?J)o>9P#KzsMzWkh&cFab)NzOVeRtQei*>VR<)C^M{qq-6}u?5SfrHh<3 za<$m`sr)&WTq?#guXT2oIKL2e&W}4T@`_23j@nHCWjuci;iEE73J({m)Kl{s77jsvW4i|gl#EjF~xgg8yjcjwU&F}+$dsvE17+yW<#=$@pz zW!ob+k7x*{lF$_YLR|BxMjh^1T78Y{cSFs2)gP5M(q4{w)&YR5sNs=ONX^UyNmEeB zbPx~$Aw)T+2OvtBvu)A=;|kqt*(622pA>V)D3A4`#+|OGKDq$i0EBx>XFM<+AkHCQ zSQhEV{!7ayBqNf8M8{>!6p+w=Z0H4=knDrU7AUsv-Wrtv^HmIHe6+fBg8^QWEpC*q z>mdf7%Ui3~|M=8+V!@|p5L%I!3-nqR!t4s<#78)j&8*R0LQb0`F-H$v5r12S8pCKJ zb=NYN=ieAP(;iKfU7^K{%L;VVG>SVVsDN}iH+>r4GfZ<58IAnmXiOxHfkcqEzBbtG znUus>V3JezR^va-&N4fpk!3iLiBGHAUbhFiH}$g*-f`DfVIj#KccO`v) -> Vec { + let mut encoder = GzEncoder::new(Vec::new(), Compression::fast()); + encoder.write_all(bytes.as_ref()).unwrap(); + encoder.finish().unwrap() + } + + pub fn decode(bytes: impl AsRef<[u8]>) -> Vec { + let mut decoder = GzDecoder::new(bytes.as_ref()); + let mut buf = Vec::new(); + decoder.read_to_end(&mut buf).unwrap(); + buf + } +} + +pub mod deflate { + use super::*; + use flate2::{read::ZlibDecoder, write::ZlibEncoder, Compression}; + + pub fn encode(bytes: impl AsRef<[u8]>) -> Vec { + let mut encoder = ZlibEncoder::new(Vec::new(), Compression::fast()); + encoder.write_all(bytes.as_ref()).unwrap(); + encoder.finish().unwrap() + } + + pub fn decode(bytes: impl AsRef<[u8]>) -> Vec { + let mut decoder = ZlibDecoder::new(bytes.as_ref()); + let mut buf = Vec::new(); + decoder.read_to_end(&mut buf).unwrap(); + buf + } +} + +pub mod brotli { + use super::*; + use ::brotli2::{read::BrotliDecoder, write::BrotliEncoder}; + + pub fn encode(bytes: impl AsRef<[u8]>) -> Vec { + let mut encoder = BrotliEncoder::new(Vec::new(), 3); + encoder.write_all(bytes.as_ref()).unwrap(); + encoder.finish().unwrap() + } + + pub fn decode(bytes: impl AsRef<[u8]>) -> Vec { + let mut decoder = BrotliDecoder::new(bytes.as_ref()); + let mut buf = Vec::new(); + decoder.read_to_end(&mut buf).unwrap(); + buf + } +} + +pub mod zstd { + use super::*; + use ::zstd::stream::{read::Decoder, write::Encoder}; + + pub fn encode(bytes: impl AsRef<[u8]>) -> Vec { + let mut encoder = Encoder::new(Vec::new(), 3).unwrap(); + encoder.write_all(bytes.as_ref()).unwrap(); + encoder.finish().unwrap() + } + + pub fn decode(bytes: impl AsRef<[u8]>) -> Vec { + let mut decoder = Decoder::new(bytes.as_ref()).unwrap(); + let mut buf = Vec::new(); + decoder.read_to_end(&mut buf).unwrap(); + buf + } +} From 25fe1bbaa5b3cc2c8e4629fb0866036cf4c778d4 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 3 Jan 2022 14:05:08 +0000 Subject: [PATCH 2/6] add double compress layer test --- Cargo.toml | 1 + actix-http/src/encoding/encoder.rs | 12 +++--- src/middleware/compress.rs | 60 ++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7abcba5a6..0e966cf46 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -109,6 +109,7 @@ actix-test = { version = "0.1.0-beta.10", features = ["openssl", "rustls"] } awc = { version = "3.0.0-beta.17", features = ["openssl"] } brotli2 = "0.3.2" +const-str = "0.3" criterion = { version = "0.3", features = ["html_reports"] } env_logger = "0.9" flate2 = "1.0.13" diff --git a/actix-http/src/encoding/encoder.rs b/actix-http/src/encoding/encoder.rs index f40e0e579..6f6fc09d8 100644 --- a/actix-http/src/encoding/encoder.rs +++ b/actix-http/src/encoding/encoder.rs @@ -56,16 +56,16 @@ impl Encoder { } pub fn response(encoding: ContentEncoding, head: &mut ResponseHead, body: B) -> Self { - let should_encode = !(head.headers().contains_key(&CONTENT_ENCODING) - || head.status == StatusCode::SWITCHING_PROTOCOLS - || head.status == StatusCode::NO_CONTENT - || encoding == ContentEncoding::Identity); - // no need to compress an empty body if matches!(body.size(), BodySize::None) { return Self::none(); } + let should_encode = !(head.headers().contains_key(&CONTENT_ENCODING) + || head.status == StatusCode::SWITCHING_PROTOCOLS + || head.status == StatusCode::NO_CONTENT + || encoding == ContentEncoding::Identity); + let body = match body.try_into_bytes() { Ok(body) => EncoderBody::Full { body }, Err(body) => EncoderBody::Stream { body }, @@ -301,7 +301,7 @@ impl ContentEncoder { Some(ContentEncoder::Zstd(encoder)) } - ContentEncoding::Identity => None, + _ => None, } } diff --git a/src/middleware/compress.rs b/src/middleware/compress.rs index 3a0d5630d..056381847 100644 --- a/src/middleware/compress.rs +++ b/src/middleware/compress.rs @@ -217,3 +217,63 @@ static SUPPORTED_ENCODINGS: Lazy> = Lazy::new(|| { encodings }); + +// move cfg(feature) to prevents_double_compressing if more tests are added +#[cfg(feature = "compress-gzip")] +#[cfg(test)] +mod tests { + use super::*; + use crate::{middleware::DefaultHeaders, test, web, App}; + + pub fn gzip_decode(bytes: impl AsRef<[u8]>) -> Vec { + use std::io::Read as _; + let mut decoder = flate2::read::GzDecoder::new(bytes.as_ref()); + let mut buf = Vec::new(); + decoder.read_to_end(&mut buf).unwrap(); + buf + } + + #[actix_rt::test] + async fn prevents_double_compressing() { + const D: &str = "hello world "; + const DATA: &str = const_str::repeat!(D, 100); + + let app = test::init_service({ + App::new() + .wrap(Compress::default()) + .route( + "/single", + web::get().to(move || HttpResponse::Ok().body(DATA)), + ) + .service( + web::resource("/double") + .wrap(Compress::default()) + .wrap(DefaultHeaders::new().add(("x-double", "true"))) + .route(web::get().to(move || HttpResponse::Ok().body(DATA))), + ) + }) + .await; + + let req = test::TestRequest::default() + .uri("/single") + .insert_header((header::ACCEPT_ENCODING, "gzip")) + .to_request(); + let res = test::call_service(&app, req).await; + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers().get("x-double"), None); + assert_eq!(res.headers().get(header::CONTENT_ENCODING).unwrap(), "gzip"); + let bytes = test::read_body(res).await; + assert_eq!(gzip_decode(bytes), DATA.as_bytes()); + + let req = test::TestRequest::default() + .uri("/double") + .insert_header((header::ACCEPT_ENCODING, "gzip")) + .to_request(); + let res = test::call_service(&app, req).await; + assert_eq!(res.status(), StatusCode::OK); + assert_eq!(res.headers().get("x-double").unwrap(), "true"); + assert_eq!(res.headers().get(header::CONTENT_ENCODING).unwrap(), "gzip"); + let bytes = test::read_body(res).await; + assert_eq!(gzip_decode(bytes), DATA.as_bytes()); + } +} From 68cd853aa22aa8f9ffa204e15db2828ae6ca5049 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 3 Jan 2022 14:59:01 +0000 Subject: [PATCH 3/6] improve docs for Compress --- Cargo.toml | 11 ++++---- actix-http/src/encoding/encoder.rs | 5 ++-- src/dev.rs | 31 ---------------------- src/middleware/compress.rs | 42 +++++++++++++++++++++++++++--- 4 files changed, 47 insertions(+), 42 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 0e966cf46..3118f3b03 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,15 +28,15 @@ path = "src/lib.rs" resolver = "2" members = [ ".", - "awc", - "actix-http", "actix-files", + "actix-http-test", + "actix-http", "actix-multipart", + "actix-router", + "actix-test", "actix-web-actors", "actix-web-codegen", - "actix-http-test", - "actix-test", - "actix-router", + "awc", ] [features] @@ -105,6 +105,7 @@ time = { version = "0.3", default-features = false, features = ["formatting"] } url = "2.1" [dev-dependencies] +actix-files = "0.6.0-beta.12" actix-test = { version = "0.1.0-beta.10", features = ["openssl", "rustls"] } awc = { version = "3.0.0-beta.17", features = ["openssl"] } diff --git a/actix-http/src/encoding/encoder.rs b/actix-http/src/encoding/encoder.rs index 6f6fc09d8..2848ad697 100644 --- a/actix-http/src/encoding/encoder.rs +++ b/actix-http/src/encoding/encoder.rs @@ -73,7 +73,7 @@ impl Encoder { if should_encode { // wrap body only if encoder is feature-enabled - if let Some(enc) = ContentEncoder::encoder(encoding) { + if let Some(enc) = ContentEncoder::select(encoding) { update_head(encoding, head); return Encoder { @@ -168,6 +168,7 @@ where cx: &mut Context<'_>, ) -> Poll>> { let mut this = self.project(); + loop { if *this.eof { return Poll::Ready(None); @@ -276,7 +277,7 @@ enum ContentEncoder { } impl ContentEncoder { - fn encoder(encoding: ContentEncoding) -> Option { + fn select(encoding: ContentEncoding) -> Option { match encoding { #[cfg(feature = "compress-gzip")] ContentEncoding::Deflate => Some(ContentEncoder::Deflate(ZlibEncoder::new( diff --git a/src/dev.rs b/src/dev.rs index b15d15747..333d1acf8 100644 --- a/src/dev.rs +++ b/src/dev.rs @@ -46,9 +46,6 @@ pub(crate) fn ensure_leading_slash(mut patterns: Patterns) -> Patterns { } /// Helper trait for managing response encoding. -/// -/// Use `pre_encoded_with` to flag response as already encoded. For example, when serving a Gzip -/// compressed file from disk. pub trait BodyEncoding { /// Get content encoding fn preferred_encoding(&self) -> Option; @@ -59,21 +56,10 @@ pub trait BodyEncoding { /// /// [`Compress`]: crate::middleware::Compress fn encode_with(&mut self, encoding: ContentEncoding) -> &mut Self; - - // /// Flags that a file already is encoded so that [`Compress`] does not modify it. - // /// - // /// Effectively a shortcut for `compress_with("identity")` - // /// plus `insert_header(ContentEncoding, encoding)`. - // /// - // /// [`Compress`]: crate::middleware::Compress - // fn pre_encoded_with(&mut self, encoding: ContentEncoding) -> &mut Self; } struct CompressWith(ContentEncoding); -// TODO: add or delete this -// struct PreCompressed(ContentEncoding); - impl BodyEncoding for crate::HttpResponseBuilder { fn preferred_encoding(&self) -> Option { self.extensions().get::().map(|enc| enc.0) @@ -83,11 +69,6 @@ impl BodyEncoding for crate::HttpResponseBuilder { self.extensions_mut().insert(CompressWith(encoding)); self } - - // fn pre_encoded_with(&mut self, encoding: ContentEncoding) -> &mut Self { - // self.extensions_mut().insert(PreCompressed(encoding)); - // self - // } } impl BodyEncoding for crate::HttpResponse { @@ -99,11 +80,6 @@ impl BodyEncoding for crate::HttpResponse { self.extensions_mut().insert(CompressWith(encoding)); self } - - // fn pre_encoded_with(&mut self, encoding: ContentEncoding) -> &mut Self { - // self.extensions_mut().insert(PreCompressed(encoding)); - // self - // } } impl BodyEncoding for ServiceResponse { @@ -120,13 +96,6 @@ impl BodyEncoding for ServiceResponse { .insert(CompressWith(encoding)); self } - - // fn pre_encoded_with(&mut self, encoding: ContentEncoding) -> &mut Self { - // self.request() - // .extensions_mut() - // .insert(PreCompressed(encoding)); - // self - // } } // TODO: remove these impls ? diff --git a/src/middleware/compress.rs b/src/middleware/compress.rs index 056381847..058f3d43e 100644 --- a/src/middleware/compress.rs +++ b/src/middleware/compress.rs @@ -27,17 +27,51 @@ use crate::{ /// Middleware for compressing response payloads. /// -/// Use `BodyEncoding` trait for overriding response compression. To disable compression set -/// encoding to `ContentEncoding::Identity`. +/// # Encoding Negotiation +/// `Compress` will read the `Accept-Encoding` header to negotiate which compression codec to use. +/// Payloads are not compressed if the header is not sent. The `compress-*` [feature flags] are also +/// considered in this selection process. +/// +/// # Pre-compressed Payload +/// If you are serving some data is already using a compressed representation (e.g., a gzip +/// compressed HTML file from disk) you can signal this to `Compress` by setting an appropriate +/// `Content-Encoding` header. In addition to preventing double compressing the payload, this header +/// is required by the spec when using compressed representations and will inform the client that +/// the content should be uncompressed. +/// +/// However, it is not advised to unconditionally serve encoded representations of content because +/// the client may not support it. The [`AcceptEncoding`] typed header has some utilities to help +/// perform manual encoding negotiation, if required. When negotiating content encoding, it is also +/// required by the spec to send a `Vary: Accept-Encoding` header. +/// +/// A (naïve) example serving an pre-compressed Gzip file is included below. /// /// # Examples +/// To enable automatic payload compression just include `Compress` as a top-level middleware: /// ``` -/// use actix_web::{web, middleware, App, HttpResponse}; +/// use actix_web::{middleware, web, App, HttpResponse}; /// /// let app = App::new() /// .wrap(middleware::Compress::default()) -/// .default_service(web::to(|| HttpResponse::NotFound())); +/// .default_service(web::to(|| HttpResponse::Ok().body("hello world"))); /// ``` +/// +/// Pre-compressed Gzip file being served from disk with correct headers added to bypass middleware: +/// ```no_run +/// use actix_web::{middleware, http::header, web, App, HttpResponse, Responder}; +/// +/// async fn index_handler() -> actix_web::Result { +/// Ok(actix_files::NamedFile::open("./assets/index.html.gz")? +/// .customize() +/// .insert_header(header::ContentEncoding::Gzip)) +/// } +/// +/// let app = App::new() +/// .wrap(middleware::Compress::default()) +/// .default_service(web::to(index_handler)); +/// ``` +/// +/// [feature flags]: ../index.html#crate-features #[derive(Debug, Clone, Default)] #[non_exhaustive] pub struct Compress; From 19a46e3925fb1b6f443a5923972870cf4fe9dd9a Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 3 Jan 2022 15:35:47 +0000 Subject: [PATCH 4/6] fix doc test --- src/middleware/compress.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/middleware/compress.rs b/src/middleware/compress.rs index 058f3d43e..bdd193693 100644 --- a/src/middleware/compress.rs +++ b/src/middleware/compress.rs @@ -61,7 +61,7 @@ use crate::{ /// use actix_web::{middleware, http::header, web, App, HttpResponse, Responder}; /// /// async fn index_handler() -> actix_web::Result { -/// Ok(actix_files::NamedFile::open("./assets/index.html.gz")? +/// Ok(actix_files::NamedFile::open_async("./assets/index.html.gz").await? /// .customize() /// .insert_header(header::ContentEncoding::Gzip)) /// } From 0bc4ae9158d51e06e4845bc2e0ba987a5e3aae2e Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Mon, 3 Jan 2022 18:46:04 +0000 Subject: [PATCH 5/6] remove `BodyEncoding` trait (#2565) --- .github/workflows/clippy-fmt.yml | 11 +- CHANGES.md | 2 + actix-files/src/lib.rs | 3 +- actix-files/src/named.rs | 67 +++--- actix-http/CHANGES.md | 2 + actix-http/src/header/map.rs | 2 +- actix-http/src/header/mod.rs | 4 +- .../src/header/shared/content_encoding.rs | 1 + awc/Cargo.toml | 2 + awc/tests/test_client.rs | 151 ++++-------- tests/test_utils.rs => awc/tests/utils.rs | 0 src/dev.rs | 78 ------- src/http/header/entity.rs | 84 ++++--- src/http/header/etag.rs | 20 +- src/http/header/if_match.rs | 9 +- src/http/header/if_none_match.rs | 4 +- src/middleware/compress.rs | 13 +- tests/compression.rs | 14 +- tests/test_server.rs | 215 ++++++++---------- tests/utils.rs | 76 +++++++ 20 files changed, 355 insertions(+), 403 deletions(-) rename tests/test_utils.rs => awc/tests/utils.rs (100%) create mode 100644 tests/utils.rs diff --git a/.github/workflows/clippy-fmt.yml b/.github/workflows/clippy-fmt.yml index 957256d32..9fcb0a561 100644 --- a/.github/workflows/clippy-fmt.yml +++ b/.github/workflows/clippy-fmt.yml @@ -14,6 +14,7 @@ jobs: uses: actions-rs/toolchain@v1 with: toolchain: stable + profile: minimal components: rustfmt - name: Check with rustfmt uses: actions-rs/cargo@v1 @@ -30,10 +31,18 @@ jobs: uses: actions-rs/toolchain@v1 with: toolchain: stable + profile: minimal components: clippy override: true + + - name: Generate Cargo.lock + uses: actions-rs/cargo@v1 + with: { command: generate-lockfile } + - name: Cache Dependencies + uses: Swatinem/rust-cache@v1.2.0 + - name: Check with Clippy uses: actions-rs/clippy-check@v1 with: token: ${{ secrets.GITHUB_TOKEN }} - args: --workspace --all-features --tests + args: --workspace --tests --examples --all-features diff --git a/CHANGES.md b/CHANGES.md index 423ea9fdc..9e5acce5d 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -15,8 +15,10 @@ ### Removed - `Compress::new`; restricting compression algorithm is done through feature flags. [#2501] +- `BodyEncoding` trait; signalling content encoding is now only done via the `Content-Encoding` header. [#2565] [#2501]: https://github.com/actix/actix-web/pull/2501 +[#2565]: https://github.com/actix/actix-web/pull/2565 ## 4.0.0-beta.18 - 2021-12-29 diff --git a/actix-files/src/lib.rs b/actix-files/src/lib.rs index 6408e02da..8ed7d44e0 100644 --- a/actix-files/src/lib.rs +++ b/actix-files/src/lib.rs @@ -597,7 +597,8 @@ mod tests { .to_request(); let res = test::call_service(&srv, request).await; assert_eq!(res.status(), StatusCode::OK); - assert!(!res.headers().contains_key(header::CONTENT_ENCODING)); + assert!(res.headers().contains_key(header::CONTENT_ENCODING)); + assert!(!test::read_body(res).await.is_empty()); } #[actix_rt::test] diff --git a/actix-files/src/named.rs b/actix-files/src/named.rs index 04e453580..019730dc6 100644 --- a/actix-files/src/named.rs +++ b/actix-files/src/named.rs @@ -9,14 +9,11 @@ use std::{ use actix_service::{Service, ServiceFactory}; use actix_web::{ body::{self, BoxBody, SizedStream}, - dev::{ - AppService, BodyEncoding, HttpServiceFactory, ResourceDef, ServiceRequest, - ServiceResponse, - }, + dev::{AppService, HttpServiceFactory, ResourceDef, ServiceRequest, ServiceResponse}, http::{ header::{ self, Charset, ContentDisposition, ContentEncoding, DispositionParam, - DispositionType, ExtendedValue, + DispositionType, ExtendedValue, HeaderValue, }, StatusCode, }, @@ -224,7 +221,6 @@ impl NamedFile { }) } - #[cfg(not(feature = "experimental-io-uring"))] /// Attempts to open a file in read-only mode. /// /// # Examples @@ -232,6 +228,7 @@ impl NamedFile { /// use actix_files::NamedFile; /// let file = NamedFile::open("foo.txt"); /// ``` + #[cfg(not(feature = "experimental-io-uring"))] pub fn open>(path: P) -> io::Result { let file = File::open(&path)?; Self::from_file(file, path) @@ -295,23 +292,21 @@ impl NamedFile { self } - /// Set the MIME Content-Type for serving this file. By default - /// the Content-Type is inferred from the filename extension. + /// Set the MIME Content-Type for serving this file. By default the Content-Type is inferred + /// from the filename extension. #[inline] pub fn set_content_type(mut self, mime_type: mime::Mime) -> Self { self.content_type = mime_type; self } - /// Set the Content-Disposition for serving this file. This allows - /// changing the inline/attachment disposition as well as the filename - /// sent to the peer. + /// Set the Content-Disposition for serving this file. This allows changing the + /// `inline/attachment` disposition as well as the filename sent to the peer. /// /// By default the disposition is `inline` for `text/*`, `image/*`, `video/*` and - /// `application/{javascript, json, wasm}` mime types, and `attachment` otherwise, - /// and the filename is taken from the path provided in the `open` method - /// after converting it to UTF-8 using. - /// [`std::ffi::OsStr::to_string_lossy`] + /// `application/{javascript, json, wasm}` mime types, and `attachment` otherwise, and the + /// filename is taken from the path provided in the `open` method after converting it to UTF-8 + /// (using `to_string_lossy`). #[inline] pub fn set_content_disposition(mut self, cd: header::ContentDisposition) -> Self { self.content_disposition = cd; @@ -337,7 +332,7 @@ impl NamedFile { self } - /// Specifies whether to use ETag or not. + /// Specifies whether to return `ETag` header in response. /// /// Default is true. #[inline] @@ -346,7 +341,7 @@ impl NamedFile { self } - /// Specifies whether to use Last-Modified or not. + /// Specifies whether to return `Last-Modified` header in response. /// /// Default is true. #[inline] @@ -364,7 +359,7 @@ impl NamedFile { self } - /// Creates a etag in a format is similar to Apache's. + /// Creates an `ETag` in a format is similar to Apache's. pub(crate) fn etag(&self) -> Option { self.modified.as_ref().map(|mtime| { let ino = { @@ -386,7 +381,7 @@ impl NamedFile { .duration_since(UNIX_EPOCH) .expect("modification time must be after epoch"); - header::EntityTag::strong(format!( + header::EntityTag::new_strong(format!( "{:x}:{:x}:{:x}:{:x}", ino, self.md.len(), @@ -405,12 +400,13 @@ impl NamedFile { if self.status_code != StatusCode::OK { let mut res = HttpResponse::build(self.status_code); - if self.flags.contains(Flags::PREFER_UTF8) { - let ct = equiv_utf8_text(self.content_type.clone()); - res.insert_header((header::CONTENT_TYPE, ct.to_string())); + let ct = if self.flags.contains(Flags::PREFER_UTF8) { + equiv_utf8_text(self.content_type.clone()) } else { - res.insert_header((header::CONTENT_TYPE, self.content_type.to_string())); - } + self.content_type + }; + + res.insert_header((header::CONTENT_TYPE, ct.to_string())); if self.flags.contains(Flags::CONTENT_DISPOSITION) { res.insert_header(( @@ -420,7 +416,7 @@ impl NamedFile { } if let Some(current_encoding) = self.encoding { - res.encode_with(current_encoding); + res.insert_header((header::CONTENT_ENCODING, current_encoding.as_str())); } let reader = chunked::new_chunked_read(self.md.len(), 0, self.file); @@ -478,12 +474,13 @@ impl NamedFile { let mut res = HttpResponse::build(self.status_code); - if self.flags.contains(Flags::PREFER_UTF8) { - let ct = equiv_utf8_text(self.content_type.clone()); - res.insert_header((header::CONTENT_TYPE, ct.to_string())); + let ct = if self.flags.contains(Flags::PREFER_UTF8) { + equiv_utf8_text(self.content_type.clone()) } else { - res.insert_header((header::CONTENT_TYPE, self.content_type.to_string())); - } + self.content_type + }; + + res.insert_header((header::CONTENT_TYPE, ct.to_string())); if self.flags.contains(Flags::CONTENT_DISPOSITION) { res.insert_header(( @@ -492,9 +489,8 @@ impl NamedFile { )); } - // default compressing if let Some(current_encoding) = self.encoding { - res.encode_with(current_encoding); + res.insert_header((header::CONTENT_ENCODING, current_encoding.as_str())); } if let Some(lm) = last_modified { @@ -517,7 +513,12 @@ impl NamedFile { length = ranges[0].length; offset = ranges[0].start; - res.encode_with(ContentEncoding::Identity); + // don't allow compression middleware to modify partial content + res.insert_header(( + header::CONTENT_ENCODING, + HeaderValue::from_static("identity"), + )); + res.insert_header(( header::CONTENT_RANGE, format!("bytes {}-{}/{}", offset, offset + length - 1, self.md.len()), diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index cbdccd93c..621b42450 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -13,6 +13,7 @@ - `QualityItem::min` semantics changed with `QualityItem::MIN`. [#2501] - Rename `ContentEncoding::{Br => Brotli}`. [#2501] - Minimum supported Rust version (MSRV) is now 1.54. +- Rename `header::EntityTag::{weak => new_weak, strong => new_strong}`. [#2565] ### Fixed - `ContentEncoding::Identity` can now be parsed from a string. [#2501] @@ -23,6 +24,7 @@ - `ContentEncoding::is_compression()`. [#2501] [#2501]: https://github.com/actix/actix-web/pull/2501 +[#2565]: https://github.com/actix/actix-web/pull/2565 ## 3.0.0-beta.17 - 2021-12-27 diff --git a/actix-http/src/header/map.rs b/actix-http/src/header/map.rs index 5cf4ba2fa..33fb262c4 100644 --- a/actix-http/src/header/map.rs +++ b/actix-http/src/header/map.rs @@ -6,7 +6,7 @@ use ahash::AHashMap; use http::header::{HeaderName, HeaderValue}; use smallvec::{smallvec, SmallVec}; -use crate::header::AsHeaderName; +use super::AsHeaderName; /// A multi-map of HTTP headers. /// diff --git a/actix-http/src/header/mod.rs b/actix-http/src/header/mod.rs index 4e9140db4..5f352fc12 100644 --- a/actix-http/src/header/mod.rs +++ b/actix-http/src/header/mod.rs @@ -50,10 +50,10 @@ pub use self::utils::{ /// An interface for types that already represent a valid header. pub trait Header: TryIntoHeaderValue { - /// Returns the name of the header field + /// Returns the name of the header field. fn name() -> HeaderName; - /// Parse a header + /// Parse the header from a HTTP message. fn parse(msg: &M) -> Result; } diff --git a/actix-http/src/header/shared/content_encoding.rs b/actix-http/src/header/shared/content_encoding.rs index ce011f107..bd25de704 100644 --- a/actix-http/src/header/shared/content_encoding.rs +++ b/actix-http/src/header/shared/content_encoding.rs @@ -68,6 +68,7 @@ impl ContentEncoding { } impl Default for ContentEncoding { + #[inline] fn default() -> Self { Self::Identity } diff --git a/awc/Cargo.toml b/awc/Cargo.toml index 9c1f56f64..0650b5508 100644 --- a/awc/Cargo.toml +++ b/awc/Cargo.toml @@ -102,12 +102,14 @@ actix-utils = "3.0.0" actix-web = { version = "4.0.0-beta.18", features = ["openssl"] } brotli2 = "0.3.2" +const-str = "0.3" env_logger = "0.9" flate2 = "1.0.13" futures-util = { version = "0.3.7", default-features = false } static_assertions = "1.1" rcgen = "0.8" rustls-pemfile = "0.2" +zstd = "0.9" [[example]] name = "client" diff --git a/awc/tests/test_client.rs b/awc/tests/test_client.rs index 6eb6800d3..c1378157b 100644 --- a/awc/tests/test_client.rs +++ b/awc/tests/test_client.rs @@ -1,5 +1,6 @@ use std::{ collections::HashMap, + convert::Infallible, io::{Read, Write}, net::{IpAddr, Ipv4Addr}, sync::{ @@ -15,43 +16,16 @@ use cookie::Cookie; use futures_util::stream; use rand::Rng; -#[cfg(feature = "compress-brotli")] -use brotli2::write::BrotliEncoder; - -#[cfg(feature = "compress-gzip")] -use flate2::{read::GzDecoder, write::GzEncoder, Compression}; - -use actix_http::{ContentEncoding, HttpService, StatusCode}; +use actix_http::{HttpService, StatusCode}; use actix_http_test::test_server; use actix_service::{fn_service, map_config, ServiceFactoryExt as _}; -use actix_web::{ - dev::{AppConfig, BodyEncoding}, - http::header, - web, App, Error, HttpRequest, HttpResponse, -}; +use actix_web::{dev::AppConfig, http::header, web, App, Error, HttpRequest, HttpResponse}; use awc::error::{JsonPayloadError, PayloadError, SendRequestError}; -const STR: &str = "Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World"; +mod utils; + +const S: &str = "Hello World "; +const STR: &str = const_str::repeat!(S, 100); #[actix_rt::test] async fn test_simple() { @@ -471,15 +445,12 @@ async fn test_no_decompress() { let srv = actix_test::start(|| { App::new() .wrap(actix_web::middleware::Compress::default()) - .service(web::resource("/").route(web::to(|| { - let mut res = HttpResponse::Ok().body(STR); - res.encode_with(header::ContentEncoding::Gzip); - res - }))) + .service(web::resource("/").route(web::to(|| HttpResponse::Ok().body(STR)))) }); let mut res = awc::Client::new() .get(srv.url("/")) + .insert_header((header::ACCEPT_ENCODING, "gzip")) .no_decompress() .send() .await @@ -488,15 +459,12 @@ async fn test_no_decompress() { // read response let bytes = res.body().await.unwrap(); - - 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_static(STR.as_ref())); + assert_eq!(utils::gzip::decode(bytes), STR.as_bytes()); // POST let mut res = awc::Client::new() .post(srv.url("/")) + .insert_header((header::ACCEPT_ENCODING, "gzip")) .no_decompress() .send() .await @@ -504,10 +472,7 @@ async fn test_no_decompress() { assert!(res.status().is_success()); let bytes = res.body().await.unwrap(); - 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_static(STR.as_ref())); + assert_eq!(utils::gzip::decode(bytes), STR.as_bytes()); } #[cfg(feature = "compress-gzip")] @@ -515,13 +480,9 @@ async fn test_no_decompress() { async fn test_client_gzip_encoding() { let srv = actix_test::start(|| { App::new().service(web::resource("/").route(web::to(|| { - let mut e = GzEncoder::new(Vec::new(), Compression::default()); - e.write_all(STR.as_ref()).unwrap(); - let data = e.finish().unwrap(); - HttpResponse::Ok() - .insert_header(("content-encoding", "gzip")) - .body(data) + .insert_header(header::ContentEncoding::Gzip) + .body(utils::gzip::encode(STR)) }))) }); @@ -531,7 +492,7 @@ async fn test_client_gzip_encoding() { // read response let bytes = response.body().await.unwrap(); - assert_eq!(bytes, Bytes::from_static(STR.as_ref())); + assert_eq!(bytes, STR); } #[cfg(feature = "compress-gzip")] @@ -539,13 +500,9 @@ async fn test_client_gzip_encoding() { async fn test_client_gzip_encoding_large() { let srv = actix_test::start(|| { App::new().service(web::resource("/").route(web::to(|| { - let mut e = GzEncoder::new(Vec::new(), Compression::default()); - e.write_all(STR.repeat(10).as_ref()).unwrap(); - let data = e.finish().unwrap(); - HttpResponse::Ok() - .insert_header(("content-encoding", "gzip")) - .body(data) + .insert_header(header::ContentEncoding::Gzip) + .body(utils::gzip::encode(STR.repeat(10))) }))) }); @@ -555,7 +512,7 @@ async fn test_client_gzip_encoding_large() { // read response let bytes = response.body().await.unwrap(); - assert_eq!(bytes, Bytes::from(STR.repeat(10))); + assert_eq!(bytes, STR.repeat(10)); } #[cfg(feature = "compress-gzip")] @@ -569,12 +526,9 @@ async fn test_client_gzip_encoding_large_random() { let srv = actix_test::start(|| { App::new().service(web::resource("/").route(web::to(|data: Bytes| { - let mut e = GzEncoder::new(Vec::new(), Compression::default()); - e.write_all(&data).unwrap(); - let data = e.finish().unwrap(); HttpResponse::Ok() - .insert_header(("content-encoding", "gzip")) - .body(data) + .insert_header(header::ContentEncoding::Gzip) + .body(utils::gzip::encode(data)) }))) }); @@ -584,7 +538,7 @@ async fn test_client_gzip_encoding_large_random() { // read response let bytes = response.body().await.unwrap(); - assert_eq!(bytes, Bytes::from(data)); + assert_eq!(bytes, data); } #[cfg(feature = "compress-brotli")] @@ -592,12 +546,9 @@ async fn test_client_gzip_encoding_large_random() { async fn test_client_brotli_encoding() { let srv = actix_test::start(|| { App::new().service(web::resource("/").route(web::to(|data: Bytes| { - let mut e = BrotliEncoder::new(Vec::new(), 5); - e.write_all(&data).unwrap(); - let data = e.finish().unwrap(); HttpResponse::Ok() .insert_header(("content-encoding", "br")) - .body(data) + .body(utils::brotli::encode(data)) }))) }); @@ -621,12 +572,9 @@ async fn test_client_brotli_encoding_large_random() { let srv = actix_test::start(|| { App::new().service(web::resource("/").route(web::to(|data: Bytes| { - let mut e = BrotliEncoder::new(Vec::new(), 5); - e.write_all(&data).unwrap(); - let data = e.finish().unwrap(); HttpResponse::Ok() - .insert_header(("content-encoding", "br")) - .body(data) + .insert_header(header::ContentEncoding::Brotli) + .body(utils::brotli::encode(&data)) }))) }); @@ -636,27 +584,25 @@ async fn test_client_brotli_encoding_large_random() { // read response let bytes = response.body().await.unwrap(); - assert_eq!(bytes.len(), data.len()); - assert_eq!(bytes, Bytes::from(data)); + assert_eq!(bytes, data); } #[actix_rt::test] async fn test_client_deflate_encoding() { let srv = actix_test::start(|| { - App::new().default_service(web::to(|body: Bytes| { - HttpResponse::Ok() - .encode_with(ContentEncoding::Brotli) - .body(body) - })) + App::new().default_service(web::to(|body: Bytes| HttpResponse::Ok().body(body))) }); - let req = srv.post("/").send_body(STR); + let req = srv + .post("/") + .insert_header((header::ACCEPT_ENCODING, "gzip")) + .send_body(STR); let mut res = req.await.unwrap(); assert_eq!(res.status(), StatusCode::OK); let bytes = res.body().await.unwrap(); - assert_eq!(bytes, Bytes::from_static(STR.as_ref())); + assert_eq!(bytes, STR); } #[actix_rt::test] @@ -668,14 +614,13 @@ async fn test_client_deflate_encoding_large_random() { .collect::(); let srv = actix_test::start(|| { - App::new().default_service(web::to(|body: Bytes| { - HttpResponse::Ok() - .encode_with(ContentEncoding::Brotli) - .body(body) - })) + App::new().default_service(web::to(|body: Bytes| HttpResponse::Ok().body(body))) }); - let req = srv.post("/").send_body(data.clone()); + let req = srv + .post("/") + .insert_header((header::ACCEPT_ENCODING, "br")) + .send_body(data.clone()); let mut res = req.await.unwrap(); let bytes = res.body().await.unwrap(); @@ -688,15 +633,16 @@ async fn test_client_deflate_encoding_large_random() { async fn test_client_streaming_explicit() { let srv = actix_test::start(|| { App::new().default_service(web::to(|body: web::Payload| { - HttpResponse::Ok() - .encode_with(ContentEncoding::Identity) - .streaming(body) + HttpResponse::Ok().streaming(body) })) }); let body = stream::once(async { Ok::<_, actix_http::Error>(Bytes::from_static(STR.as_bytes())) }); - let req = srv.post("/").send_stream(Box::pin(body)); + let req = srv + .post("/") + .insert_header((header::ACCEPT_ENCODING, "identity")) + .send_stream(Box::pin(body)); let mut res = req.await.unwrap(); assert!(res.status().is_success()); @@ -709,17 +655,16 @@ async fn test_client_streaming_explicit() { async fn test_body_streaming_implicit() { let srv = actix_test::start(|| { App::new().default_service(web::to(|| { - let body = stream::once(async { - Ok::<_, actix_http::Error>(Bytes::from_static(STR.as_bytes())) - }); - - HttpResponse::Ok() - .encode_with(ContentEncoding::Gzip) - .streaming(Box::pin(body)) + let body = + stream::once(async { Ok::<_, Infallible>(Bytes::from_static(STR.as_bytes())) }); + HttpResponse::Ok().streaming(body) })) }); - let req = srv.get("/").send(); + let req = srv + .get("/") + .insert_header((header::ACCEPT_ENCODING, "gzip")) + .send(); let mut res = req.await.unwrap(); assert!(res.status().is_success()); diff --git a/tests/test_utils.rs b/awc/tests/utils.rs similarity index 100% rename from tests/test_utils.rs rename to awc/tests/utils.rs diff --git a/src/dev.rs b/src/dev.rs index 333d1acf8..def545ec7 100644 --- a/src/dev.rs +++ b/src/dev.rs @@ -22,8 +22,6 @@ pub use crate::service::{HttpServiceFactory, ServiceRequest, ServiceResponse, We pub use crate::types::{JsonBody, Readlines, UrlEncoded}; -use crate::{http::header::ContentEncoding, HttpMessage as _}; - use actix_router::Patterns; pub(crate) fn ensure_leading_slash(mut patterns: Patterns) -> Patterns { @@ -44,79 +42,3 @@ pub(crate) fn ensure_leading_slash(mut patterns: Patterns) -> Patterns { patterns } - -/// Helper trait for managing response encoding. -pub trait BodyEncoding { - /// Get content encoding - fn preferred_encoding(&self) -> Option; - - /// Set content encoding to use. - /// - /// Must be used with [`Compress`] to take effect. - /// - /// [`Compress`]: crate::middleware::Compress - fn encode_with(&mut self, encoding: ContentEncoding) -> &mut Self; -} - -struct CompressWith(ContentEncoding); - -impl BodyEncoding for crate::HttpResponseBuilder { - fn preferred_encoding(&self) -> Option { - self.extensions().get::().map(|enc| enc.0) - } - - fn encode_with(&mut self, encoding: ContentEncoding) -> &mut Self { - self.extensions_mut().insert(CompressWith(encoding)); - self - } -} - -impl BodyEncoding for crate::HttpResponse { - fn preferred_encoding(&self) -> Option { - self.extensions().get::().map(|enc| enc.0) - } - - fn encode_with(&mut self, encoding: ContentEncoding) -> &mut Self { - self.extensions_mut().insert(CompressWith(encoding)); - self - } -} - -impl BodyEncoding for ServiceResponse { - fn preferred_encoding(&self) -> Option { - self.request() - .extensions() - .get::() - .map(|enc| enc.0) - } - - fn encode_with(&mut self, encoding: ContentEncoding) -> &mut Self { - self.request() - .extensions_mut() - .insert(CompressWith(encoding)); - self - } -} - -// TODO: remove these impls ? -impl BodyEncoding for actix_http::ResponseBuilder { - fn preferred_encoding(&self) -> Option { - self.extensions().get::().map(|enc| enc.0) - } - - fn encode_with(&mut self, encoding: ContentEncoding) -> &mut Self { - self.extensions_mut().insert(CompressWith(encoding)); - self - } -} - -impl BodyEncoding for actix_http::Response { - fn preferred_encoding(&self) -> Option { - self.extensions().get::().map(|enc| enc.0) - } - - fn encode_with(&mut self, encoding: ContentEncoding) -> &mut Self { - self.extensions_mut().insert(CompressWith(encoding)); - self - } -} diff --git a/src/http/header/entity.rs b/src/http/header/entity.rs index 76fe39f23..0eaa12b5d 100644 --- a/src/http/header/entity.rs +++ b/src/http/header/entity.rs @@ -17,8 +17,7 @@ fn check_slice_validity(slice: &str) -> bool { slice.bytes().all(entity_validate_char) } -/// An entity tag, defined -/// in [RFC 7232 §2.3](https://datatracker.ietf.org/doc/html/rfc7232#section-2.3) +/// An entity tag, defined in [RFC 7232 §2.3]. /// /// An entity tag consists of a string enclosed by two literal double quotes. /// Preceding the first double quote is an optional weakness indicator, @@ -48,16 +47,20 @@ fn check_slice_validity(slice: &str) -> bool { /// | `W/"1"` | `W/"2"` | no match | no match | /// | `W/"1"` | `"1"` | no match | match | /// | `"1"` | `"1"` | match | match | -#[derive(Clone, Debug, Eq, PartialEq)] +/// +/// [RFC 7232 §2.3](https://datatracker.ietf.org/doc/html/rfc7232#section-2.3) +#[derive(Debug, Clone, PartialEq, Eq)] pub struct EntityTag { /// Weakness indicator for the tag pub weak: bool, + /// The opaque string in between the DQUOTEs tag: String, } impl EntityTag { - /// Constructs a new EntityTag. + /// Constructs a new `EntityTag`. + /// /// # Panics /// If the tag contains invalid characters. pub fn new(weak: bool, tag: String) -> EntityTag { @@ -66,51 +69,64 @@ impl EntityTag { } /// Constructs a new weak EntityTag. + /// /// # Panics /// If the tag contains invalid characters. - pub fn weak(tag: String) -> EntityTag { + pub fn new_weak(tag: String) -> EntityTag { EntityTag::new(true, tag) } + #[deprecated(since = "3.0.0", note = "Renamed to `new_weak`.")] + pub fn weak(tag: String) -> EntityTag { + Self::new_weak(tag) + } + /// Constructs a new strong EntityTag. + /// /// # Panics /// If the tag contains invalid characters. - pub fn strong(tag: String) -> EntityTag { + pub fn new_strong(tag: String) -> EntityTag { EntityTag::new(false, tag) } - /// Get the tag. + #[deprecated(since = "3.0.0", note = "Renamed to `new_strong`.")] + pub fn strong(tag: String) -> EntityTag { + Self::new_strong(tag) + } + + /// Returns tag. pub fn tag(&self) -> &str { self.tag.as_ref() } - /// Set the tag. + /// Sets tag. + /// /// # Panics /// If the tag contains invalid characters. - pub fn set_tag(&mut self, tag: String) { + pub fn set_tag(&mut self, tag: impl Into) { + let tag = tag.into(); assert!(check_slice_validity(&tag), "Invalid tag: {:?}", tag); self.tag = tag } - /// For strong comparison two entity-tags are equivalent if both are not - /// weak and their opaque-tags match character-by-character. + /// For strong comparison two entity-tags are equivalent if both are not weak and their + /// opaque-tags match character-by-character. pub fn strong_eq(&self, other: &EntityTag) -> bool { !self.weak && !other.weak && self.tag == other.tag } - /// For weak comparison two entity-tags are equivalent if their - /// opaque-tags match character-by-character, regardless of either or - /// both being tagged as "weak". + /// For weak comparison two entity-tags are equivalent if their opaque-tags match + /// character-by-character, regardless of either or both being tagged as "weak". pub fn weak_eq(&self, other: &EntityTag) -> bool { self.tag == other.tag } - /// The inverse of `EntityTag.strong_eq()`. + /// Returns the inverse of `strong_eq()`. pub fn strong_ne(&self, other: &EntityTag) -> bool { !self.strong_eq(other) } - /// The inverse of `EntityTag.weak_eq()`. + /// Returns inverse of `weak_eq()`. pub fn weak_ne(&self, other: &EntityTag) -> bool { !self.weak_eq(other) } @@ -178,23 +194,23 @@ mod tests { // Expected success assert_eq!( "\"foobar\"".parse::().unwrap(), - EntityTag::strong("foobar".to_owned()) + EntityTag::new_strong("foobar".to_owned()) ); assert_eq!( "\"\"".parse::().unwrap(), - EntityTag::strong("".to_owned()) + EntityTag::new_strong("".to_owned()) ); assert_eq!( "W/\"weaktag\"".parse::().unwrap(), - EntityTag::weak("weaktag".to_owned()) + EntityTag::new_weak("weaktag".to_owned()) ); assert_eq!( "W/\"\x65\x62\"".parse::().unwrap(), - EntityTag::weak("\x65\x62".to_owned()) + EntityTag::new_weak("\x65\x62".to_owned()) ); assert_eq!( "W/\"\"".parse::().unwrap(), - EntityTag::weak("".to_owned()) + EntityTag::new_weak("".to_owned()) ); } @@ -214,19 +230,19 @@ mod tests { #[test] fn test_etag_fmt() { assert_eq!( - format!("{}", EntityTag::strong("foobar".to_owned())), + format!("{}", EntityTag::new_strong("foobar".to_owned())), "\"foobar\"" ); - assert_eq!(format!("{}", EntityTag::strong("".to_owned())), "\"\""); + assert_eq!(format!("{}", EntityTag::new_strong("".to_owned())), "\"\""); assert_eq!( - format!("{}", EntityTag::weak("weak-etag".to_owned())), + format!("{}", EntityTag::new_weak("weak-etag".to_owned())), "W/\"weak-etag\"" ); assert_eq!( - format!("{}", EntityTag::weak("\u{0065}".to_owned())), + format!("{}", EntityTag::new_weak("\u{0065}".to_owned())), "W/\"\x65\"" ); - assert_eq!(format!("{}", EntityTag::weak("".to_owned())), "W/\"\""); + assert_eq!(format!("{}", EntityTag::new_weak("".to_owned())), "W/\"\""); } #[test] @@ -237,29 +253,29 @@ mod tests { // | `W/"1"` | `W/"2"` | no match | no match | // | `W/"1"` | `"1"` | no match | match | // | `"1"` | `"1"` | match | match | - let mut etag1 = EntityTag::weak("1".to_owned()); - let mut etag2 = EntityTag::weak("1".to_owned()); + let mut etag1 = EntityTag::new_weak("1".to_owned()); + let mut etag2 = EntityTag::new_weak("1".to_owned()); assert!(!etag1.strong_eq(&etag2)); assert!(etag1.weak_eq(&etag2)); assert!(etag1.strong_ne(&etag2)); assert!(!etag1.weak_ne(&etag2)); - etag1 = EntityTag::weak("1".to_owned()); - etag2 = EntityTag::weak("2".to_owned()); + etag1 = EntityTag::new_weak("1".to_owned()); + etag2 = EntityTag::new_weak("2".to_owned()); assert!(!etag1.strong_eq(&etag2)); assert!(!etag1.weak_eq(&etag2)); assert!(etag1.strong_ne(&etag2)); assert!(etag1.weak_ne(&etag2)); - etag1 = EntityTag::weak("1".to_owned()); - etag2 = EntityTag::strong("1".to_owned()); + etag1 = EntityTag::new_weak("1".to_owned()); + etag2 = EntityTag::new_strong("1".to_owned()); assert!(!etag1.strong_eq(&etag2)); assert!(etag1.weak_eq(&etag2)); assert!(etag1.strong_ne(&etag2)); assert!(!etag1.weak_ne(&etag2)); - etag1 = EntityTag::strong("1".to_owned()); - etag2 = EntityTag::strong("1".to_owned()); + etag1 = EntityTag::new_strong("1".to_owned()); + etag2 = EntityTag::new_strong("1".to_owned()); assert!(etag1.strong_eq(&etag2)); assert!(etag1.weak_eq(&etag2)); assert!(!etag1.strong_ne(&etag2)); diff --git a/src/http/header/etag.rs b/src/http/header/etag.rs index 4724c917e..78f5447b3 100644 --- a/src/http/header/etag.rs +++ b/src/http/header/etag.rs @@ -31,7 +31,7 @@ crate::http::header::common_header! { /// /// let mut builder = HttpResponse::Ok(); /// builder.insert_header( - /// ETag(EntityTag::new(false, "xyzzy".to_owned())) + /// ETag(EntityTag::new_strong("xyzzy".to_owned())) /// ); /// ``` /// @@ -41,7 +41,7 @@ crate::http::header::common_header! { /// /// let mut builder = HttpResponse::Ok(); /// builder.insert_header( - /// ETag(EntityTag::new(true, "xyzzy".to_owned())) + /// ETag(EntityTag::new_weak("xyzzy".to_owned())) /// ); /// ``` (ETag, ETAG) => [EntityTag] @@ -50,29 +50,29 @@ crate::http::header::common_header! { // From the RFC crate::http::header::common_header_test!(test1, vec![b"\"xyzzy\""], - Some(ETag(EntityTag::new(false, "xyzzy".to_owned())))); + Some(ETag(EntityTag::new_strong("xyzzy".to_owned())))); crate::http::header::common_header_test!(test2, vec![b"W/\"xyzzy\""], - Some(ETag(EntityTag::new(true, "xyzzy".to_owned())))); + Some(ETag(EntityTag::new_weak("xyzzy".to_owned())))); crate::http::header::common_header_test!(test3, vec![b"\"\""], - Some(ETag(EntityTag::new(false, "".to_owned())))); + Some(ETag(EntityTag::new_strong("".to_owned())))); // Own tests crate::http::header::common_header_test!(test4, vec![b"\"foobar\""], - Some(ETag(EntityTag::new(false, "foobar".to_owned())))); + Some(ETag(EntityTag::new_strong("foobar".to_owned())))); crate::http::header::common_header_test!(test5, vec![b"\"\""], - Some(ETag(EntityTag::new(false, "".to_owned())))); + Some(ETag(EntityTag::new_strong("".to_owned())))); crate::http::header::common_header_test!(test6, vec![b"W/\"weak-etag\""], - Some(ETag(EntityTag::new(true, "weak-etag".to_owned())))); + Some(ETag(EntityTag::new_weak("weak-etag".to_owned())))); crate::http::header::common_header_test!(test7, vec![b"W/\"\x65\x62\""], - Some(ETag(EntityTag::new(true, "\u{0065}\u{0062}".to_owned())))); + Some(ETag(EntityTag::new_weak("\u{0065}\u{0062}".to_owned())))); crate::http::header::common_header_test!(test8, vec![b"W/\"\""], - Some(ETag(EntityTag::new(true, "".to_owned())))); + Some(ETag(EntityTag::new_weak("".to_owned())))); crate::http::header::common_header_test!(test9, vec![b"no-dquotes"], None::); diff --git a/src/http/header/if_match.rs b/src/http/header/if_match.rs index a565b9125..e299d30fe 100644 --- a/src/http/header/if_match.rs +++ b/src/http/header/if_match.rs @@ -54,14 +54,15 @@ common_header! { test1, vec![b"\"xyzzy\""], Some(HeaderField::Items( - vec![EntityTag::new(false, "xyzzy".to_owned())]))); + vec![EntityTag::new_strong("xyzzy".to_owned())]))); + crate::http::header::common_header_test!( test2, vec![b"\"xyzzy\", \"r2d2xxxx\", \"c3piozzzz\""], Some(HeaderField::Items( - vec![EntityTag::new(false, "xyzzy".to_owned()), - EntityTag::new(false, "r2d2xxxx".to_owned()), - EntityTag::new(false, "c3piozzzz".to_owned())]))); + vec![EntityTag::new_strong("xyzzy".to_owned()), + EntityTag::new_strong("r2d2xxxx".to_owned()), + EntityTag::new_strong("c3piozzzz".to_owned())]))); crate::http::header::common_header_test!(test3, vec![b"*"], Some(IfMatch::Any)); } } diff --git a/src/http/header/if_none_match.rs b/src/http/header/if_none_match.rs index fb1895fc8..863be70cf 100644 --- a/src/http/header/if_none_match.rs +++ b/src/http/header/if_none_match.rs @@ -82,8 +82,8 @@ mod tests { if_none_match = Header::parse(&req); let mut entities: Vec = Vec::new(); - let foobar_etag = EntityTag::new(false, "foobar".to_owned()); - let weak_etag = EntityTag::new(true, "weak-etag".to_owned()); + let foobar_etag = EntityTag::new_strong("foobar".to_owned()); + let weak_etag = EntityTag::new_weak("weak-etag".to_owned()); entities.push(foobar_etag); entities.push(weak_etag); assert_eq!(if_none_match.ok(), Some(IfNoneMatch::Items(entities))); diff --git a/src/middleware/compress.rs b/src/middleware/compress.rs index bdd193693..16af4c2cd 100644 --- a/src/middleware/compress.rs +++ b/src/middleware/compress.rs @@ -16,7 +16,6 @@ use pin_project_lite::pin_project; use crate::{ body::{EitherBody, MessageBody}, - dev::BodyEncoding as _, http::{ header::{self, AcceptEncoding, Encoding, HeaderValue}, StatusCode, @@ -176,14 +175,10 @@ where match ready!(this.fut.poll(cx)) { Ok(resp) => { - let enc = if let Some(enc) = resp.response().preferred_encoding() { - enc - } else { - match this.encoding { - Encoding::Known(enc) => *enc, - Encoding::Unknown(enc) => { - unimplemented!("encoding {} should not be here", enc); - } + let enc = match this.encoding { + Encoding::Known(enc) => *enc, + Encoding::Unknown(enc) => { + unimplemented!("encoding {} should not be here", enc); } }; diff --git a/tests/compression.rs b/tests/compression.rs index c0bf10e4c..88c462f60 100644 --- a/tests/compression.rs +++ b/tests/compression.rs @@ -1,14 +1,12 @@ use actix_http::ContentEncoding; use actix_web::{ - dev::BodyEncoding as _, http::{header, StatusCode}, middleware::Compress, web, App, HttpResponse, }; use bytes::Bytes; -mod test_utils; -use test_utils::{brotli, gzip, zstd}; +mod utils; static LOREM: &[u8] = include_bytes!("fixtures/lorem.txt"); static LOREM_GZIP: &[u8] = include_bytes!("fixtures/lorem.txt.gz"); @@ -27,7 +25,6 @@ macro_rules! test_server { web::to(|| { HttpResponse::Ok() // signal to compressor that content should not be altered - .encode_with(ContentEncoding::Identity) // signal to client that content is encoded .insert_header(ContentEncoding::Gzip) .body(LOREM_GZIP) @@ -38,7 +35,6 @@ macro_rules! test_server { web::to(|| { HttpResponse::Ok() // signal to compressor that content should not be altered - .encode_with(ContentEncoding::Identity) // signal to client that content is encoded .insert_header(ContentEncoding::Brotli) .body(LOREM_BR) @@ -49,7 +45,6 @@ macro_rules! test_server { web::to(|| { HttpResponse::Ok() // signal to compressor that content should not be altered - .encode_with(ContentEncoding::Identity) // signal to client that content is encoded .insert_header(ContentEncoding::Zstd) .body(LOREM_ZSTD) @@ -60,7 +55,6 @@ macro_rules! test_server { web::to(|| { HttpResponse::Ok() // signal to compressor that content should not be altered - .encode_with(ContentEncoding::Identity) // signal to client that content is encoded as 7zip .insert_header((header::CONTENT_ENCODING, "xz")) .body(LOREM_XZ) @@ -117,7 +111,7 @@ async fn negotiate_encoding_gzip() { .await .unwrap(); let bytes = res.body().await.unwrap(); - assert_eq!(gzip::decode(bytes), LOREM); + assert_eq!(utils::gzip::decode(bytes), LOREM); srv.stop().await; } @@ -146,7 +140,7 @@ async fn negotiate_encoding_br() { .await .unwrap(); let bytes = res.body().await.unwrap(); - assert_eq!(brotli::decode(bytes), LOREM); + assert_eq!(utils::brotli::decode(bytes), LOREM); srv.stop().await; } @@ -175,7 +169,7 @@ async fn negotiate_encoding_zstd() { .await .unwrap(); let bytes = res.body().await.unwrap(); - assert_eq!(zstd::decode(bytes), LOREM); + assert_eq!(utils::zstd::decode(bytes), LOREM); srv.stop().await; } diff --git a/tests/test_server.rs b/tests/test_server.rs index e8d514e64..987e51a65 100644 --- a/tests/test_server.rs +++ b/tests/test_server.rs @@ -12,11 +12,7 @@ use std::{ use actix_web::{ cookie::{Cookie, CookieBuilder}, - dev::BodyEncoding, - http::{ - header::{self, ContentEncoding, ACCEPT_ENCODING, CONTENT_ENCODING, TRANSFER_ENCODING}, - StatusCode, - }, + http::{header, StatusCode}, middleware::{Compress, NormalizePath, TrailingSlash}, web, App, Error, HttpResponse, }; @@ -31,30 +27,10 @@ use openssl::{ x509::X509, }; -mod test_utils; -use test_utils::{brotli, deflate, gzip, zstd}; +mod utils; -const STR: &str = "Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World \ - Hello World Hello World Hello World Hello World Hello World"; +const S: &str = "Hello World "; +const STR: &str = const_str::repeat!(S, 100); #[cfg(feature = "openssl")] fn openssl_config() -> SslAcceptor { @@ -129,51 +105,52 @@ async fn test_body() { srv.stop().await; } -#[actix_rt::test] -async fn test_body_encoding_override() { - let srv = actix_test::start_with(actix_test::config().h1(), || { - App::new() - .wrap(Compress::default()) - .service(web::resource("/").route(web::to(|| { - HttpResponse::Ok() - .encode_with(ContentEncoding::Deflate) - .body(STR) - }))) - .service(web::resource("/raw").route(web::to(|| { - let mut res = HttpResponse::with_body(actix_web::http::StatusCode::OK, STR); - res.encode_with(ContentEncoding::Deflate); - res.map_into_boxed_body() - }))) - }); +// enforcing an encoding per-response is removed +// #[actix_rt::test] +// async fn test_body_encoding_override() { +// let srv = actix_test::start_with(actix_test::config().h1(), || { +// App::new() +// .wrap(Compress::default()) +// .service(web::resource("/").route(web::to(|| { +// HttpResponse::Ok() +// .encode_with(ContentEncoding::Deflate) +// .body(STR) +// }))) +// .service(web::resource("/raw").route(web::to(|| { +// let mut res = HttpResponse::with_body(actix_web::http::StatusCode::OK, STR); +// res.encode_with(ContentEncoding::Deflate); +// res.map_into_boxed_body() +// }))) +// }); - // Builder - let mut res = srv - .get("/") - .no_decompress() - .append_header((ACCEPT_ENCODING, "deflate")) - .send() - .await - .unwrap(); - assert_eq!(res.status(), StatusCode::OK); +// // Builder +// let mut res = srv +// .get("/") +// .no_decompress() +// .append_header((ACCEPT_ENCODING, "deflate")) +// .send() +// .await +// .unwrap(); +// assert_eq!(res.status(), StatusCode::OK); - let bytes = res.body().await.unwrap(); - assert_eq!(deflate::decode(bytes), STR.as_bytes()); +// let bytes = res.body().await.unwrap(); +// assert_eq!(utils::deflate::decode(bytes), STR.as_bytes()); - // Raw Response - let mut res = srv - .request(actix_web::http::Method::GET, srv.url("/raw")) - .no_decompress() - .append_header((ACCEPT_ENCODING, "deflate")) - .send() - .await - .unwrap(); - assert_eq!(res.status(), StatusCode::OK); +// // Raw Response +// let mut res = srv +// .request(actix_web::http::Method::GET, srv.url("/raw")) +// .no_decompress() +// .append_header((ACCEPT_ENCODING, "deflate")) +// .send() +// .await +// .unwrap(); +// assert_eq!(res.status(), StatusCode::OK); - let bytes = res.body().await.unwrap(); - assert_eq!(deflate::decode(bytes), STR.as_bytes()); +// let bytes = res.body().await.unwrap(); +// assert_eq!(utils::deflate::decode(bytes), STR.as_bytes()); - srv.stop().await; -} +// srv.stop().await; +// } #[actix_rt::test] async fn body_gzip_large() { @@ -191,14 +168,14 @@ async fn body_gzip_large() { let mut res = srv .get("/") .no_decompress() - .append_header((ACCEPT_ENCODING, "gzip")) + .append_header((header::ACCEPT_ENCODING, "gzip")) .send() .await .unwrap(); assert_eq!(res.status(), StatusCode::OK); let bytes = res.body().await.unwrap(); - assert_eq!(gzip::decode(bytes), data.as_bytes()); + assert_eq!(utils::gzip::decode(bytes), data.as_bytes()); srv.stop().await; } @@ -222,14 +199,14 @@ async fn test_body_gzip_large_random() { let mut res = srv .get("/") .no_decompress() - .append_header((ACCEPT_ENCODING, "gzip")) + .append_header((header::ACCEPT_ENCODING, "gzip")) .send() .await .unwrap(); assert_eq!(res.status(), StatusCode::OK); let bytes = res.body().await.unwrap(); - assert_eq!(gzip::decode(bytes), data.as_bytes()); + assert_eq!(utils::gzip::decode(bytes), data.as_bytes()); srv.stop().await; } @@ -248,15 +225,18 @@ async fn test_body_chunked_implicit() { let mut res = srv .get("/") .no_decompress() - .append_header((ACCEPT_ENCODING, "gzip")) + .append_header((header::ACCEPT_ENCODING, "gzip")) .send() .await .unwrap(); assert_eq!(res.status(), StatusCode::OK); - assert_eq!(res.headers().get(TRANSFER_ENCODING).unwrap(), "chunked"); + assert_eq!( + res.headers().get(header::TRANSFER_ENCODING).unwrap(), + "chunked" + ); let bytes = res.body().await.unwrap(); - assert_eq!(gzip::decode(bytes), STR.as_bytes()); + assert_eq!(utils::gzip::decode(bytes), STR.as_bytes()); srv.stop().await; } @@ -274,7 +254,7 @@ async fn test_body_br_streaming() { let mut res = srv .get("/") - .append_header((ACCEPT_ENCODING, "br")) + .append_header((header::ACCEPT_ENCODING, "br")) .no_decompress() .send() .await @@ -282,7 +262,7 @@ async fn test_body_br_streaming() { assert_eq!(res.status(), StatusCode::OK); let bytes = res.body().await.unwrap(); - assert_eq!(brotli::decode(bytes), STR.as_bytes()); + assert_eq!(utils::brotli::decode(bytes), STR.as_bytes()); srv.stop().await; } @@ -319,7 +299,7 @@ async fn test_no_chunking() { let mut res = srv.get("/").send().await.unwrap(); assert_eq!(res.status(), StatusCode::OK); - assert!(!res.headers().contains_key(TRANSFER_ENCODING)); + assert!(!res.headers().contains_key(header::TRANSFER_ENCODING)); let bytes = res.body().await.unwrap(); assert_eq!(bytes, Bytes::from_static(STR.as_ref())); @@ -337,7 +317,7 @@ async fn test_body_deflate() { let mut res = srv .get("/") - .append_header((ACCEPT_ENCODING, "deflate")) + .append_header((header::ACCEPT_ENCODING, "deflate")) .no_decompress() .send() .await @@ -345,7 +325,7 @@ async fn test_body_deflate() { assert_eq!(res.status(), StatusCode::OK); let bytes = res.body().await.unwrap(); - assert_eq!(deflate::decode(bytes), STR.as_bytes()); + assert_eq!(utils::deflate::decode(bytes), STR.as_bytes()); srv.stop().await; } @@ -360,7 +340,7 @@ async fn test_body_brotli() { let mut res = srv .get("/") - .append_header((ACCEPT_ENCODING, "br")) + .append_header((header::ACCEPT_ENCODING, "br")) .no_decompress() .send() .await @@ -368,7 +348,7 @@ async fn test_body_brotli() { assert_eq!(res.status(), StatusCode::OK); let bytes = res.body().await.unwrap(); - assert_eq!(brotli::decode(bytes), STR.as_bytes()); + assert_eq!(utils::brotli::decode(bytes), STR.as_bytes()); srv.stop().await; } @@ -383,7 +363,7 @@ async fn test_body_zstd() { let mut res = srv .get("/") - .append_header((ACCEPT_ENCODING, "zstd")) + .append_header((header::ACCEPT_ENCODING, "zstd")) .no_decompress() .send() .await @@ -391,7 +371,7 @@ async fn test_body_zstd() { assert_eq!(res.status(), StatusCode::OK); let bytes = res.body().await.unwrap(); - assert_eq!(zstd::decode(bytes), STR.as_bytes()); + assert_eq!(utils::zstd::decode(bytes), STR.as_bytes()); srv.stop().await; } @@ -409,7 +389,7 @@ async fn test_body_zstd_streaming() { let mut res = srv .get("/") - .append_header((ACCEPT_ENCODING, "zstd")) + .append_header((header::ACCEPT_ENCODING, "zstd")) .no_decompress() .send() .await @@ -417,7 +397,7 @@ async fn test_body_zstd_streaming() { assert_eq!(res.status(), StatusCode::OK); let bytes = res.body().await.unwrap(); - assert_eq!(zstd::decode(bytes), STR.as_bytes()); + assert_eq!(utils::zstd::decode(bytes), STR.as_bytes()); srv.stop().await; } @@ -432,8 +412,8 @@ async fn test_zstd_encoding() { let request = srv .post("/") - .append_header((CONTENT_ENCODING, "zstd")) - .send_body(zstd::encode(STR)); + .append_header((header::CONTENT_ENCODING, "zstd")) + .send_body(utils::zstd::encode(STR)); let mut res = request.await.unwrap(); assert_eq!(res.status(), StatusCode::OK); @@ -463,8 +443,8 @@ async fn test_zstd_encoding_large() { let request = srv .post("/") - .append_header((CONTENT_ENCODING, "zstd")) - .send_body(zstd::encode(&data)); + .append_header((header::CONTENT_ENCODING, "zstd")) + .send_body(utils::zstd::encode(&data)); let mut res = request.await.unwrap(); assert_eq!(res.status(), StatusCode::OK); @@ -484,8 +464,8 @@ async fn test_encoding() { let request = srv .post("/") - .insert_header((CONTENT_ENCODING, "gzip")) - .send_body(gzip::encode(STR)); + .insert_header((header::CONTENT_ENCODING, "gzip")) + .send_body(utils::gzip::encode(STR)); let mut res = request.await.unwrap(); assert_eq!(res.status(), StatusCode::OK); @@ -505,8 +485,8 @@ async fn test_gzip_encoding() { let request = srv .post("/") - .append_header((CONTENT_ENCODING, "gzip")) - .send_body(gzip::encode(STR)); + .append_header((header::CONTENT_ENCODING, "gzip")) + .send_body(utils::gzip::encode(STR)); let mut res = request.await.unwrap(); assert_eq!(res.status(), StatusCode::OK); @@ -527,8 +507,8 @@ async fn test_gzip_encoding_large() { let req = srv .post("/") - .append_header((CONTENT_ENCODING, "gzip")) - .send_body(gzip::encode(&data)); + .append_header((header::CONTENT_ENCODING, "gzip")) + .send_body(utils::gzip::encode(&data)); let mut res = req.await.unwrap(); assert_eq!(res.status(), StatusCode::OK); @@ -554,8 +534,8 @@ async fn test_reading_gzip_encoding_large_random() { let request = srv .post("/") - .append_header((CONTENT_ENCODING, "gzip")) - .send_body(gzip::encode(&data)); + .append_header((header::CONTENT_ENCODING, "gzip")) + .send_body(utils::gzip::encode(&data)); let mut res = request.await.unwrap(); assert_eq!(res.status(), StatusCode::OK); @@ -575,8 +555,8 @@ async fn test_reading_deflate_encoding() { let request = srv .post("/") - .append_header((CONTENT_ENCODING, "deflate")) - .send_body(deflate::encode(STR)); + .append_header((header::CONTENT_ENCODING, "deflate")) + .send_body(utils::deflate::encode(STR)); let mut res = request.await.unwrap(); assert_eq!(res.status(), StatusCode::OK); @@ -597,8 +577,8 @@ async fn test_reading_deflate_encoding_large() { let request = srv .post("/") - .append_header((CONTENT_ENCODING, "deflate")) - .send_body(deflate::encode(&data)); + .append_header((header::CONTENT_ENCODING, "deflate")) + .send_body(utils::deflate::encode(&data)); let mut res = request.await.unwrap(); assert_eq!(res.status(), StatusCode::OK); @@ -624,8 +604,8 @@ async fn test_reading_deflate_encoding_large_random() { let request = srv .post("/") - .append_header((CONTENT_ENCODING, "deflate")) - .send_body(deflate::encode(&data)); + .append_header((header::CONTENT_ENCODING, "deflate")) + .send_body(utils::deflate::encode(&data)); let mut res = request.await.unwrap(); assert_eq!(res.status(), StatusCode::OK); @@ -646,8 +626,8 @@ async fn test_brotli_encoding() { let request = srv .post("/") - .append_header((CONTENT_ENCODING, "br")) - .send_body(brotli::encode(STR)); + .append_header((header::CONTENT_ENCODING, "br")) + .send_body(utils::brotli::encode(STR)); let mut res = request.await.unwrap(); assert_eq!(res.status(), StatusCode::OK); @@ -677,8 +657,8 @@ async fn test_brotli_encoding_large() { let request = srv .post("/") - .append_header((CONTENT_ENCODING, "br")) - .send_body(brotli::encode(&data)); + .append_header((header::CONTENT_ENCODING, "br")) + .send_body(utils::brotli::encode(&data)); let mut res = request.await.unwrap(); assert_eq!(res.status(), StatusCode::OK); @@ -697,8 +677,9 @@ async fn test_brotli_encoding_large_openssl() { let srv = actix_test::start_with(actix_test::config().openssl(openssl_config()), move || { App::new().service(web::resource("/").route(web::to(|bytes: Bytes| { + // echo decompressed request body back in response HttpResponse::Ok() - .encode_with(ContentEncoding::Identity) + .insert_header(header::ContentEncoding::Identity) .body(bytes) }))) }); @@ -706,7 +687,7 @@ async fn test_brotli_encoding_large_openssl() { let mut res = srv .post("/") .append_header((header::CONTENT_ENCODING, "br")) - .send_body(brotli::encode(&data)) + .send_body(utils::brotli::encode(&data)) .await .unwrap(); assert_eq!(res.status(), StatusCode::OK); @@ -758,16 +739,20 @@ mod plus_rustls { let srv = actix_test::start_with(actix_test::config().rustls(tls_config()), || { App::new().service(web::resource("/").route(web::to(|bytes: Bytes| { + // echo decompressed request body back in response HttpResponse::Ok() - .encode_with(ContentEncoding::Identity) + .insert_header(header::ContentEncoding::Identity) .body(bytes) }))) }); let req = srv .post("/") - .insert_header((actix_web::http::header::CONTENT_ENCODING, "deflate")) - .send_stream(TestBody::new(Bytes::from(deflate::encode(&data)), 1024)); + .insert_header((header::CONTENT_ENCODING, "deflate")) + .send_stream(TestBody::new( + Bytes::from(utils::deflate::encode(&data)), + 1024, + )); let mut res = req.await.unwrap(); assert_eq!(res.status(), StatusCode::OK); @@ -931,14 +916,14 @@ async fn test_accept_encoding_no_match() { let mut res = srv .get("/") - .insert_header((ACCEPT_ENCODING, "xz, identity;q=0")) + .insert_header((header::ACCEPT_ENCODING, "xz, identity;q=0")) .no_decompress() .send() .await .unwrap(); assert_eq!(res.status(), StatusCode::NOT_ACCEPTABLE); - assert_eq!(res.headers().get(CONTENT_ENCODING), None); + assert_eq!(res.headers().get(header::CONTENT_ENCODING), None); let bytes = res.body().await.unwrap(); // body should contain the supported encodings diff --git a/tests/utils.rs b/tests/utils.rs new file mode 100644 index 000000000..9a3743d8b --- /dev/null +++ b/tests/utils.rs @@ -0,0 +1,76 @@ +// compiling some tests will trigger unused function warnings even though other tests use them +#![allow(dead_code)] + +use std::io::{Read as _, Write as _}; + +pub mod gzip { + use super::*; + use flate2::{read::GzDecoder, write::GzEncoder, Compression}; + + pub fn encode(bytes: impl AsRef<[u8]>) -> Vec { + let mut encoder = GzEncoder::new(Vec::new(), Compression::fast()); + encoder.write_all(bytes.as_ref()).unwrap(); + encoder.finish().unwrap() + } + + pub fn decode(bytes: impl AsRef<[u8]>) -> Vec { + let mut decoder = GzDecoder::new(bytes.as_ref()); + let mut buf = Vec::new(); + decoder.read_to_end(&mut buf).unwrap(); + buf + } +} + +pub mod deflate { + use super::*; + use flate2::{read::ZlibDecoder, write::ZlibEncoder, Compression}; + + pub fn encode(bytes: impl AsRef<[u8]>) -> Vec { + let mut encoder = ZlibEncoder::new(Vec::new(), Compression::fast()); + encoder.write_all(bytes.as_ref()).unwrap(); + encoder.finish().unwrap() + } + + pub fn decode(bytes: impl AsRef<[u8]>) -> Vec { + let mut decoder = ZlibDecoder::new(bytes.as_ref()); + let mut buf = Vec::new(); + decoder.read_to_end(&mut buf).unwrap(); + buf + } +} + +pub mod brotli { + use super::*; + use ::brotli2::{read::BrotliDecoder, write::BrotliEncoder}; + + pub fn encode(bytes: impl AsRef<[u8]>) -> Vec { + let mut encoder = BrotliEncoder::new(Vec::new(), 3); + encoder.write_all(bytes.as_ref()).unwrap(); + encoder.finish().unwrap() + } + + pub fn decode(bytes: impl AsRef<[u8]>) -> Vec { + let mut decoder = BrotliDecoder::new(bytes.as_ref()); + let mut buf = Vec::new(); + decoder.read_to_end(&mut buf).unwrap(); + buf + } +} + +pub mod zstd { + use super::*; + use ::zstd::stream::{read::Decoder, write::Encoder}; + + pub fn encode(bytes: impl AsRef<[u8]>) -> Vec { + let mut encoder = Encoder::new(Vec::new(), 3).unwrap(); + encoder.write_all(bytes.as_ref()).unwrap(); + encoder.finish().unwrap() + } + + pub fn decode(bytes: impl AsRef<[u8]>) -> Vec { + let mut decoder = Decoder::new(bytes.as_ref()).unwrap(); + let mut buf = Vec::new(); + decoder.read_to_end(&mut buf).unwrap(); + buf + } +} From c7639bc3be5b1779d573dc1d2c0d0396b4968554 Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Tue, 4 Jan 2022 03:48:12 +0000 Subject: [PATCH 6/6] document quoter --- actix-files/src/service.rs | 8 +-- actix-router/src/url.rs | 141 +++++++++++++++++++++++++------------ 2 files changed, 100 insertions(+), 49 deletions(-) diff --git a/actix-files/src/service.rs b/actix-files/src/service.rs index 057dbe5a3..152e1855e 100644 --- a/actix-files/src/service.rs +++ b/actix-files/src/service.rs @@ -114,7 +114,7 @@ impl Service for FilesService { Box::pin(async move { if !is_method_valid { return Ok(req.into_response( - actix_web::HttpResponse::MethodNotAllowed() + HttpResponse::MethodNotAllowed() .insert_header(header::ContentType(mime::TEXT_PLAIN_UTF_8)) .body("Request did not meet this resource's requirements."), )); @@ -123,7 +123,7 @@ impl Service for FilesService { let real_path = match PathBufWrap::parse_path(req.match_info().path(), this.hidden_files) { Ok(item) => item, - Err(e) => return Ok(req.error_response(e)), + Err(err) => return Ok(req.error_response(err)), }; if let Some(filter) = &this.path_filter { @@ -131,9 +131,7 @@ impl Service for FilesService { if let Some(ref default) = this.default { return default.call(req).await; } else { - return Ok( - req.into_response(actix_web::HttpResponse::NotFound().finish()) - ); + return Ok(req.into_response(HttpResponse::NotFound().finish())); } } } diff --git a/actix-router/src/url.rs b/actix-router/src/url.rs index 10193dde8..fee8eaaf3 100644 --- a/actix-router/src/url.rs +++ b/actix-router/src/url.rs @@ -26,16 +26,6 @@ const ALLOWED: &[u8] = b"abcdefghijklmnopqrstuvwxyz const QS: &[u8] = b"+&=;b"; -#[inline] -fn bit_at(array: &[u8], ch: u8) -> bool { - array[(ch >> 3) as usize] & (1 << (ch & 7)) != 0 -} - -#[inline] -fn set_bit(array: &mut [u8], ch: u8) { - array[(ch >> 3) as usize] |= 1 << (ch & 7) -} - thread_local! { static DEFAULT_QUOTER: Quoter = Quoter::new(b"@:", b"%/+"); } @@ -96,7 +86,10 @@ impl ResourcePath for Url { /// A quoter pub struct Quoter { + /// Simple bit-map of safe values in the 0-127 ASCII range. safe_table: [u8; 16], + + /// Simple bit-map of protected values in the 0-127 ASCII range. protected_table: [u8; 16], } @@ -108,28 +101,32 @@ impl Quoter { }; // prepare safe table - for i in 0..128 { - if ALLOWED.contains(&i) { - set_bit(&mut quoter.safe_table, i); + for ch in 0..128 { + if ALLOWED.contains(&ch) { + set_bit(&mut quoter.safe_table, ch); } - if QS.contains(&i) { - set_bit(&mut quoter.safe_table, i); + + if QS.contains(&ch) { + set_bit(&mut quoter.safe_table, ch); } } - for ch in safe { - set_bit(&mut quoter.safe_table, *ch) + for &ch in safe { + set_bit(&mut quoter.safe_table, ch) } // prepare protected table - for ch in protected { - set_bit(&mut quoter.safe_table, *ch); - set_bit(&mut quoter.protected_table, *ch); + for &ch in protected { + set_bit(&mut quoter.safe_table, ch); + set_bit(&mut quoter.protected_table, ch); } quoter } + /// Re-quotes... ? + /// + /// Returns `None` when no modification to the original string was required. pub fn requote(&self, val: &[u8]) -> Option { let mut has_pct = 0; let mut pct = [b'%', 0, 0]; @@ -137,17 +134,19 @@ impl Quoter { let mut cloned: Option> = None; let len = val.len(); + while idx < len { let ch = val[idx]; if has_pct != 0 { pct[has_pct] = val[idx]; has_pct += 1; + if has_pct == 3 { has_pct = 0; let buf = cloned.as_mut().unwrap(); - if let Some(ch) = restore_ch(pct[1], pct[2]) { + if let Some(ch) = hex_pair_to_char(pct[1], pct[2]) { if ch < 128 { if bit_at(&self.protected_table, ch) { buf.extend_from_slice(&pct); @@ -161,6 +160,7 @@ impl Quoter { continue; } } + buf.push(ch); } else { buf.extend_from_slice(&pct[..]); @@ -168,6 +168,7 @@ impl Quoter { } } else if ch == b'%' { has_pct = 1; + if cloned.is_none() { let mut c = Vec::with_capacity(len); c.extend_from_slice(&val[..idx]); @@ -176,6 +177,7 @@ impl Quoter { } else if let Some(ref mut cloned) = cloned { cloned.push(ch) } + idx += 1; } @@ -183,22 +185,52 @@ impl Quoter { } } -#[inline] -fn from_hex(v: u8) -> Option { - if (b'0'..=b'9').contains(&v) { - Some(v - 0x30) // ord('0') == 0x30 - } else if (b'A'..=b'F').contains(&v) { - Some(v - 0x41 + 10) // ord('A') == 0x41 - } else if (b'a'..=b'f').contains(&v) { - Some(v - 0x61 + 10) // ord('a') == 0x61 - } else { - None +/// Converts an ASCII character in the hex-encoded set (`0-9`, `A-F`, `a-f`) to its integer +/// representation from `0x0`–`0xF`. +/// +/// - `0x30 ('0') => 0x0` +/// - `0x39 ('9') => 0x9` +/// - `0x41 ('a') => 0xA` +/// - `0x61 ('A') => 0xA` +/// - `0x46 ('f') => 0xF` +/// - `0x66 ('F') => 0xF` +fn from_ascii_hex(v: u8) -> Option { + match v { + b'0'..=b'9' => Some(v - 0x30), // ord('0') == 0x30 + b'A'..=b'F' => Some(v - 0x41 + 10), // ord('A') == 0x41 + b'a'..=b'f' => Some(v - 0x61 + 10), // ord('a') == 0x61 + _ => None, } } -#[inline] -fn restore_ch(d1: u8, d2: u8) -> Option { - from_hex(d1).and_then(|d1| from_hex(d2).map(move |d2| d1 << 4 | d2)) +/// Decode a ASCII hex-encoded pair to an integer. +/// +/// Returns `None` if either portion of the decoded pair does not evaluate to a valid hex value. +/// +/// - `0x33 ('3'), 0x30 ('0') => 0x30 ('0')` +/// - `0x34 ('4'), 0x31 ('1') => 0x41 ('A')` +/// - `0x36 ('6'), 0x31 ('1') => 0x61 ('a')` +fn hex_pair_to_char(d1: u8, d2: u8) -> Option { + let (d_high, d_low) = (from_ascii_hex(d1)?, from_ascii_hex(d2)?); + + // left shift high nibble by 4 bits + Some(d_high << 4 | d_low) +} + +/// Sets bit in given bit-map to 1=true. +/// +/// # Panics +/// Panics if `ch` index is out of bounds. +fn set_bit(array: &mut [u8], ch: u8) { + array[(ch >> 3) as usize] |= 0b1 << (ch & 0b111) +} + +/// Returns true if bit to true in given bit-map. +/// +/// # Panics +/// Panics if `ch` index is out of bounds. +fn bit_at(array: &[u8], ch: u8) -> bool { + array[(ch >> 3) as usize] & (0b1 << (ch & 0b111)) != 0 } #[cfg(test)] @@ -229,6 +261,16 @@ mod tests { let path = match_url(re, "/user/2345/test"); assert_eq!(path.get("id").unwrap(), "2345"); + } + + #[test] + fn protected_chars() { + let re = "/user/{id}/test"; + + let encoded = percent_encode(PROTECTED); + let path = match_url(re, format!("/user/{}/test", encoded)); + // characters in captured segment remain unencoded + assert_eq!(path.get("id").unwrap(), &encoded); // "%25" should never be decoded into '%' to guarantee the output is a valid // percent-encoded format @@ -239,13 +281,6 @@ mod tests { assert_eq!(path.get("id").unwrap(), "qwe%25rty"); } - #[test] - fn protected_chars() { - let encoded = percent_encode(PROTECTED); - let path = match_url("/user/{id}/test", format!("/user/{}/test", encoded)); - assert_eq!(path.get("id").unwrap(), &encoded); - } - #[test] fn non_protected_ascii() { let non_protected_ascii = ('\u{0}'..='\u{7F}') @@ -281,9 +316,9 @@ mod tests { for i in 0..256 { let c = i as u8; if hex.contains(&c) { - assert!(from_hex(c).is_some()) + assert!(from_ascii_hex(c).is_some()) } else { - assert!(from_hex(c).is_none()) + assert!(from_ascii_hex(c).is_none()) } } @@ -291,7 +326,25 @@ mod tests { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 10, 11, 12, 13, 14, 15, ]; for i in 0..hex.len() { - assert_eq!(from_hex(hex[i]).unwrap(), expected[i]); + assert_eq!(from_ascii_hex(hex[i]).unwrap(), expected[i]); } } + + #[test] + fn custom_quoter() { + let q = Quoter::new(b"", b"+"); + assert_eq!(q.requote(b"/a%25c").unwrap(), "/a%c"); + assert_eq!(q.requote(b"/a%2Bc").unwrap(), "/a%2Bc"); + + let q = Quoter::new(b"%+", b"/"); + assert_eq!(q.requote(b"/a%25b%2Bc").unwrap(), "/a%b+c"); + assert_eq!(q.requote(b"/a%2fb").unwrap(), "/a%2fb"); + assert_eq!(q.requote(b"/a%2Fb").unwrap(), "/a%2Fb"); + } + + #[test] + fn quoter_no_modification() { + let q = Quoter::new(b"", b""); + assert_eq!(q.requote(b"/abc/../efg"), None); + } }