From 0edd419618d13f5cf5e689cb9666f485ca7f1bcb Mon Sep 17 00:00:00 2001 From: Rob Ede Date: Wed, 1 Sep 2021 08:38:02 +0100 Subject: [PATCH] remove from bound on compress middleware --- CHANGES.md | 8 +++- actix-http/CHANGES.md | 4 +- .../src/header/shared/content_encoding.rs | 18 ++++---- actix-http/src/header/shared/quality_item.rs | 29 +++++++------ src/middleware/compress.rs | 41 ++++++++++--------- src/middleware/mod.rs | 2 +- 6 files changed, 54 insertions(+), 48 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index a08d8cfea..217ec4f78 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -4,13 +4,17 @@ ### Added * Re-export actix-service `ServiceFactory` in `dev` module. [#2325] -### Changes -* Fix quality parse error in Accept-Encoding HTTP header. [#2344] +### Changed * Minimum supported Rust version (MSRV) is now 1.51. +* Compress middleware will return 406 Not Acceptable when no content encoding is acceptable to the client. [#2344] + +### Fixed +* Fix quality parse error in Accept-Encoding header. [#2344] [#2325]: https://github.com/actix/actix-web/pull/2325 [#2344]: https://github.com/actix/actix-web/pull/2344 + ## 4.0.0-beta.8 - 2021-06-26 ### Added * Add `ServiceRequest::parts_mut`. [#2177] diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index 9a9a4527c..f4efef54a 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -1,13 +1,13 @@ # Changes ## Unreleased - 2021-xx-xx -### Changes +### Changed * Minimum supported Rust version (MSRV) is now 1.51. ### Fixed * Remove slice creation pointing to potential uninitialized data on h1 encoder. [#2364] * Remove `Into` bound on `Encoder` body types. [#2375] -* Fix quality parse error in Accept-Encoding HTTP header. [#2344] +* Fix quality parse error in Accept-Encoding header. [#2344] [#2364]: https://github.com/actix/actix-web/pull/2364 [#2375]: https://github.com/actix/actix-web/pull/2375 diff --git a/actix-http/src/header/shared/content_encoding.rs b/actix-http/src/header/shared/content_encoding.rs index bc2364d1f..375e8c2fa 100644 --- a/actix-http/src/header/shared/content_encoding.rs +++ b/actix-http/src/header/shared/content_encoding.rs @@ -51,7 +51,7 @@ impl ContentEncoding { matches!(self, ContentEncoding::Identity | ContentEncoding::Auto) } - /// Convert content encoding to string + /// Convert content encoding to string. #[inline] pub fn as_str(self) -> &'static str { match self { @@ -74,14 +74,6 @@ impl FromStr for ContentEncoding { type Err = ContentEncodingParseError; fn from_str(val: &str) -> Result { - Self::try_from(val) - } -} - -impl TryFrom<&str> for ContentEncoding { - type Error = ContentEncodingParseError; - - fn try_from(val: &str) -> Result { let val = val.trim(); if val.eq_ignore_ascii_case("br") { @@ -98,6 +90,14 @@ impl TryFrom<&str> for ContentEncoding { } } +impl TryFrom<&str> for ContentEncoding { + type Error = ContentEncodingParseError; + + fn try_from(val: &str) -> Result { + val.parse() + } +} + impl IntoHeaderValue for ContentEncoding { type Error = InvalidHeaderValue; diff --git a/actix-http/src/header/shared/quality_item.rs b/actix-http/src/header/shared/quality_item.rs index 240a0afa2..63fa02e7b 100644 --- a/actix-http/src/header/shared/quality_item.rs +++ b/actix-http/src/header/shared/quality_item.rs @@ -1,11 +1,14 @@ use std::{ cmp, convert::{TryFrom, TryInto}, - fmt, str, + fmt, + str::{self, FromStr}, }; use derive_more::{Display, Error}; +use crate::error::ParseError; + const MAX_QUALITY: u16 = 1000; const MAX_FLOAT_QUALITY: f32 = 1.0; @@ -113,12 +116,12 @@ impl fmt::Display for QualityItem { } } -impl str::FromStr for QualityItem { - type Err = crate::error::ParseError; +impl FromStr for QualityItem { + type Err = ParseError; - fn from_str(qitem_str: &str) -> Result, crate::error::ParseError> { + fn from_str(qitem_str: &str) -> Result { if !qitem_str.is_ascii() { - return Err(crate::error::ParseError::Header); + return Err(ParseError::Header); } // Set defaults used if parsing fails. @@ -139,7 +142,7 @@ impl str::FromStr for QualityItem { if parts[0].len() < 2 { // Can't possibly be an attribute since an attribute needs at least a name followed // by an equals sign. And bare identifiers are forbidden. - return Err(crate::error::ParseError::Header); + return Err(ParseError::Header); } let start = &parts[0][0..2]; @@ -148,25 +151,21 @@ impl str::FromStr for QualityItem { let q_val = &parts[0][2..]; if q_val.len() > 5 { // longer than 5 indicates an over-precise q-factor - return Err(crate::error::ParseError::Header); + return Err(ParseError::Header); } - let q_value = q_val - .parse::() - .map_err(|_| crate::error::ParseError::Header)?; + let q_value = q_val.parse::().map_err(|_| ParseError::Header)?; if (0f32..=1f32).contains(&q_value) { quality = q_value; raw_item = parts[1]; } else { - return Err(crate::error::ParseError::Header); + return Err(ParseError::Header); } } } - let item = raw_item - .parse::() - .map_err(|_| crate::error::ParseError::Header)?; + let item = raw_item.parse::().map_err(|_| ParseError::Header)?; // we already checked above that the quality is within range Ok(QualityItem::new(item, Quality::from_f32(quality))) @@ -224,7 +223,7 @@ mod tests { } } - impl str::FromStr for Encoding { + impl FromStr for Encoding { type Err = crate::error::ParseError; fn from_str(s: &str) -> Result { use Encoding::*; diff --git a/src/middleware/compress.rs b/src/middleware/compress.rs index ed0588562..0e61a8e7e 100644 --- a/src/middleware/compress.rs +++ b/src/middleware/compress.rs @@ -18,6 +18,7 @@ use actix_http::{ use actix_service::{Service, Transform}; use actix_utils::future::{ok, Either, Ready}; use futures_core::ready; +use once_cell::sync::Lazy; use pin_project::pin_project; use crate::{ @@ -57,7 +58,7 @@ impl Default for Compress { impl Transform for Compress where - B: MessageBody + From, + B: MessageBody, S: Service, Error = Error>, { type Response = ServiceResponse>>; @@ -79,7 +80,7 @@ pub struct CompressMiddleware { encoding: ContentEncoding, } -fn supported_algorithm_names() -> String { +static SUPPORTED_ALGORITHM_NAMES: Lazy = Lazy::new(|| { let mut encoding = vec![]; #[cfg(feature = "compress-brotli")] @@ -98,16 +99,16 @@ fn supported_algorithm_names() -> String { assert!( !encoding.is_empty(), - "encoding can not be empty unless __compress feature has been explicitly enabled" + "encoding can not be empty unless __compress feature has been explicitly enabled by itself" ); encoding.join(", ") -} +}); impl Service for CompressMiddleware where - B: MessageBody + From, S: Service, Error = Error>, + B: MessageBody, { type Response = ServiceResponse>>; type Error = Error; @@ -143,12 +144,12 @@ where Some(Err(_)) => { let res = HttpResponse::with_body( StatusCode::NOT_ACCEPTABLE, - supported_algorithm_names().into(), + SUPPORTED_ALGORITHM_NAMES.as_str(), ); let enc = ContentEncoding::Identity; Either::right(ok(req.into_response(res.map_body(move |head, body| { - Encoder::response(enc, head, ResponseBody::Body(body)) + Encoder::response(enc, head, ResponseBody::Other(body.into())) })))) } } @@ -195,6 +196,7 @@ where struct AcceptEncoding { encoding: ContentEncoding, + // TODO: use Quality or QualityItem quality: f64, } @@ -221,17 +223,17 @@ impl PartialOrd for AcceptEncoding { impl PartialEq for AcceptEncoding { fn eq(&self, other: &AcceptEncoding) -> bool { - self.quality == other.quality && self.encoding == other.encoding + self.encoding == other.encoding && self.quality == other.quality } } -/// Parse qfactor from HTTP header +/// Parse q-factor from quality strings. /// -/// If parse fail, then fallback to default value which is 1 -/// More details available here: https://developer.mozilla.org/en-US/docs/Glossary/Quality_values +/// 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.starts_with("q=") { + if part.trim().starts_with("q=") { return part[2..].parse().unwrap_or(1.0); } } @@ -241,9 +243,8 @@ fn parse_quality(parts: &[&str]) -> f64 { #[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 + /// This error occurs when client only support compressed response and server do not have any + /// algorithm that match client accepted algorithms. CompressionAlgorithmMismatch, } @@ -262,11 +263,12 @@ impl AcceptEncoding { if quality <= 0.0 || quality > 1.0 { return None; } + Some(AcceptEncoding { encoding, quality }) } - /// Parse a raw Accept-Encoding header value into an ordered list - /// then return the best match based on middleware configuration. + /// 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, @@ -285,8 +287,9 @@ impl AcceptEncoding { } } - // Special case if user cannot accept uncompressed data + // 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); } @@ -356,7 +359,7 @@ mod tests { #[test] fn test_parse_compression_required() { - // Check we fallback to identity if there is an unsuported compression algorithm + // Check we fallback to identity if there is an unsupported compression algorithm assert_parse_eq!("compress", ContentEncoding::Identity); // User do not want any compression diff --git a/src/middleware/mod.rs b/src/middleware/mod.rs index dfcd9fb4e..d19cb64e9 100644 --- a/src/middleware/mod.rs +++ b/src/middleware/mod.rs @@ -53,7 +53,7 @@ mod tests { #[cfg(feature = "__compress")] { let _ = App::new().wrap(Compress::default()).wrap(Logger::default()); - // let _ = App::new().wrap(Logger::default()).wrap(Compress::default()); + let _ = App::new().wrap(Logger::default()).wrap(Compress::default()); let _ = App::new().wrap(Compat::new(Compress::default())); let _ = App::new().wrap(Condition::new(true, Compat::new(Compress::default()))); }