Fix quality parse error in Accept-Encoding HTTP header (#2344)

actix-http/src/header/shared/content_encoding.rs

@@ -1,4 +1,4 @@
-use std::{convert::Infallible, str::FromStr};
+use std::{convert::TryFrom, error, fmt, str::FromStr};
 
 use http::header::InvalidHeaderValue;
 
@@ -8,6 +8,20 @@ use crate::{
     HttpMessage,
 };
 
+/// Error return when a content encoding is unknown.
+///
+/// Example: 'compress'
+#[derive(Debug)]
+pub struct ContentEncodingParseError;
+
+impl fmt::Display for ContentEncodingParseError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "Unsupported content encoding")
+    }
+}
+
+impl error::Error for ContentEncodingParseError {}
+
 /// Represents a supported content encoding.
 #[derive(Copy, Clone, PartialEq, Debug)]
 pub enum ContentEncoding {
@@ -37,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 {
@@ -48,18 +62,6 @@ impl ContentEncoding {
             ContentEncoding::Identity | ContentEncoding::Auto => "identity",
         }
     }
-
-    /// Default Q-factor (quality) value.
-    #[inline]
-    pub fn quality(self) -> f64 {
-        match self {
-            ContentEncoding::Br => 1.1,
-            ContentEncoding::Gzip => 1.0,
-            ContentEncoding::Deflate => 0.9,
-            ContentEncoding::Identity | ContentEncoding::Auto => 0.1,
-            ContentEncoding::Zstd => 0.0,
-        }
-    }
 }
 
 impl Default for ContentEncoding {
@@ -69,31 +71,33 @@ impl Default for ContentEncoding {
 }
 
 impl FromStr for ContentEncoding {
-    type Err = Infallible;
+    type Err = ContentEncodingParseError;
 
     fn from_str(val: &str) -> Result<Self, Self::Err> {
-        Ok(Self::from(val))
-    }
-}
-
-impl From<&str> for ContentEncoding {
-    fn from(val: &str) -> ContentEncoding {
         let val = val.trim();
 
         if val.eq_ignore_ascii_case("br") {
-            ContentEncoding::Br
+            Ok(ContentEncoding::Br)
         } else if val.eq_ignore_ascii_case("gzip") {
-            ContentEncoding::Gzip
+            Ok(ContentEncoding::Gzip)
         } else if val.eq_ignore_ascii_case("deflate") {
-            ContentEncoding::Deflate
+            Ok(ContentEncoding::Deflate)
         } else if val.eq_ignore_ascii_case("zstd") {
-            ContentEncoding::Zstd
+            Ok(ContentEncoding::Zstd)
         } else {
-            ContentEncoding::default()
+            Err(ContentEncodingParseError)
         }
     }
 }
 
+impl TryFrom<&str> for ContentEncoding {
+    type Error = ContentEncodingParseError;
+
+    fn try_from(val: &str) -> Result<Self, Self::Error> {
+        val.parse()
+    }
+}
+
 impl IntoHeaderValue for ContentEncoding {
     type Error = InvalidHeaderValue;
 

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<T: fmt::Display> fmt::Display for QualityItem<T> {
     }
 }
 
-impl<T: str::FromStr> str::FromStr for QualityItem<T> {
-    type Err = crate::error::ParseError;
+impl<T: FromStr> FromStr for QualityItem<T> {
+    type Err = ParseError;
 
-    fn from_str(qitem_str: &str) -> Result<QualityItem<T>, crate::error::ParseError> {
+    fn from_str(qitem_str: &str) -> Result<Self, Self::Err> {
         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<T: str::FromStr> str::FromStr for QualityItem<T> {
             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<T: str::FromStr> str::FromStr for QualityItem<T> {
                 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::<f32>()
-                    .map_err(|_| crate::error::ParseError::Header)?;
+                let q_value = q_val.parse::<f32>().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::<T>()
-            .map_err(|_| crate::error::ParseError::Header)?;
+        let item = raw_item.parse::<T>().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<Encoding, crate::error::ParseError> {
             use Encoding::*;