diff --git a/actix-http/CHANGES.md b/actix-http/CHANGES.md index cafaa5e09..a25f44e36 100644 --- a/actix-http/CHANGES.md +++ b/actix-http/CHANGES.md @@ -4,6 +4,7 @@ ### Added * HttpResponse builders for 1xx status codes. [#1768] * `Accept::mime_precedence` and `Accept::mime_preference`. [#1793] +* `TryFrom` and `TryFrom` for `http::header::Quality`. [#1797] ### Fixed * Started dropping `transfer-encoding: chunked` and `Content-Length` for 1XX and 204 responses. [#1767] @@ -14,6 +15,7 @@ [#1767]: https://github.com/actix/actix-web/pull/1767 [#1768]: https://github.com/actix/actix-web/pull/1768 [#1793]: https://github.com/actix/actix-web/pull/1793 +[#1797]: https://github.com/actix/actix-web/pull/1797 ## 2.1.0 - 2020-10-30 diff --git a/actix-http/src/header/mod.rs b/actix-http/src/header/mod.rs index 46fb31a62..0f87516eb 100644 --- a/actix-http/src/header/mod.rs +++ b/actix-http/src/header/mod.rs @@ -370,9 +370,7 @@ impl fmt::Display for ExtendedValue { } /// Percent encode a sequence of bytes with a character set defined in -/// [https://tools.ietf.org/html/rfc5987#section-3.2][url] -/// -/// [url]: https://tools.ietf.org/html/rfc5987#section-3.2 +/// pub fn http_percent_encode(f: &mut fmt::Formatter<'_>, bytes: &[u8]) -> fmt::Result { let encoded = percent_encoding::percent_encode(bytes, HTTP_VALUE); fmt::Display::fmt(&encoded, f) diff --git a/actix-http/src/header/shared/charset.rs b/actix-http/src/header/shared/charset.rs index 00e7309d4..36bdbf7e2 100644 --- a/actix-http/src/header/shared/charset.rs +++ b/actix-http/src/header/shared/charset.rs @@ -7,9 +7,7 @@ use self::Charset::*; /// /// The string representation is normalized to upper case. /// -/// See [http://www.iana.org/assignments/character-sets/character-sets.xhtml][url]. -/// -/// [url]: http://www.iana.org/assignments/character-sets/character-sets.xhtml +/// See . #[derive(Clone, Debug, PartialEq)] #[allow(non_camel_case_types)] pub enum Charset { diff --git a/actix-http/src/header/shared/quality_item.rs b/actix-http/src/header/shared/quality_item.rs index f4331a18e..01a3b988a 100644 --- a/actix-http/src/header/shared/quality_item.rs +++ b/actix-http/src/header/shared/quality_item.rs @@ -1,10 +1,17 @@ -use std::{cmp, fmt, str}; +use std::{ + cmp, + convert::{TryFrom, TryInto}, + fmt, str, +}; -use self::internal::IntoQuality; +use derive_more::{Display, Error}; + +const MAX_QUALITY: u16 = 1000; +const MAX_FLOAT_QUALITY: f32 = 1.0; /// Represents a quality used in quality values. /// -/// Can be created with the `q` function. +/// Can be created with the [`q`] function. /// /// # Implementation notes /// @@ -21,9 +28,51 @@ use self::internal::IntoQuality; #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct Quality(u16); +impl Quality { + /// # Panics + /// Panics in debug mode when value is not in the range 0.0 <= n <= 1.0. + fn from_f32(value: f32) -> Self { + // 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), + "q value must be between 0.0 and 1.0" + ); + + Quality((value * MAX_QUALITY as f32) as u16) + } +} + impl Default for Quality { fn default() -> Quality { - Quality(1000) + Quality(MAX_QUALITY) + } +} + +#[derive(Debug, Clone, Display, Error)] +pub struct QualityOutOfBounds; + +impl TryFrom for Quality { + type Error = QualityOutOfBounds; + + fn try_from(value: u16) -> Result { + if (0..=MAX_QUALITY).contains(&value) { + Ok(Quality(value)) + } else { + Err(QualityOutOfBounds) + } + } +} + +impl TryFrom for Quality { + type Error = QualityOutOfBounds; + + fn try_from(value: f32) -> Result { + if (0.0..=MAX_FLOAT_QUALITY).contains(&value) { + Ok(Quality::from_f32(value)) + } else { + Err(QualityOutOfBounds) + } } } @@ -55,8 +104,9 @@ impl cmp::PartialOrd for QualityItem { impl fmt::Display for QualityItem { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fmt::Display::fmt(&self.item, f)?; + match self.quality.0 { - 1000 => Ok(()), + MAX_QUALITY => Ok(()), 0 => f.write_str("; q=0"), x => write!(f, "; q=0.{}", format!("{:03}", x).trim_end_matches('0')), } @@ -66,56 +116,61 @@ impl fmt::Display for QualityItem { impl str::FromStr for QualityItem { type Err = crate::error::ParseError; - fn from_str(s: &str) -> Result, crate::error::ParseError> { - if !s.is_ascii() { + fn from_str(qitem_str: &str) -> Result, crate::error::ParseError> { + if !qitem_str.is_ascii() { return Err(crate::error::ParseError::Header); } + // Set defaults used if parsing fails. - let mut raw_item = s; + let mut raw_item = qitem_str; let mut quality = 1f32; - let parts: Vec<&str> = s.rsplitn(2, ';').map(|x| x.trim()).collect(); + let parts: Vec<_> = qitem_str.rsplitn(2, ';').map(str::trim).collect(); + if parts.len() == 2 { + // example for item with q-factor: + // + // gzip; q=0.65 + // ^^^^^^ parts[0] + // ^^ start + // ^^^^ q_val + // ^^^^ parts[1] + 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); } + let start = &parts[0][0..2]; + if start == "q=" || start == "Q=" { - let q_part = &parts[0][2..parts[0].len()]; - if q_part.len() > 5 { + 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); } - match q_part.parse::() { - Ok(q_value) => { - if 0f32 <= q_value && q_value <= 1f32 { - quality = q_value; - raw_item = parts[1]; - } else { - return Err(crate::error::ParseError::Header); - } - } - Err(_) => return Err(crate::error::ParseError::Header), + + let q_value = q_val + .parse::() + .map_err(|_| crate::error::ParseError::Header)?; + + if (0f32..=1f32).contains(&q_value) { + quality = q_value; + raw_item = parts[1]; + } else { + return Err(crate::error::ParseError::Header); } } } - match raw_item.parse::() { - // we already checked above that the quality is within range - Ok(item) => Ok(QualityItem::new(item, from_f32(quality))), - Err(_) => Err(crate::error::ParseError::Header), - } - } -} -#[inline] -fn from_f32(f: f32) -> Quality { - // this function is only used internally. A check that `f` is within range - // should be done before calling this method. Just in case, this - // debug_assert should catch if we were forgetful - debug_assert!( - f >= 0f32 && f <= 1f32, - "q value must be between 0.0 and 1.0" - ); - Quality((f * 1000f32) as u16) + let item = raw_item + .parse::() + .map_err(|_| crate::error::ParseError::Header)?; + + // we already checked above that the quality is within range + Ok(QualityItem::new(item, Quality::from_f32(quality))) + } } /// Convenience function to wrap a value in a `QualityItem` @@ -127,44 +182,13 @@ pub fn qitem(item: T) -> QualityItem { /// Convenience function to create a `Quality` from a float or integer. /// /// Implemented for `u16` and `f32`. Panics if value is out of range. -pub fn q(val: T) -> Quality { - val.into_quality() -} - -mod internal { - use super::Quality; - - // TryFrom is probably better, but it's not stable. For now, we want to - // keep the functionality of the `q` function, while allowing it to be - // generic over `f32` and `u16`. - // - // `q` would panic before, so keep that behavior. `TryFrom` can be - // introduced later for a non-panicking conversion. - - pub trait IntoQuality: Sealed + Sized { - fn into_quality(self) -> Quality; - } - - impl IntoQuality for f32 { - fn into_quality(self) -> Quality { - assert!( - self >= 0f32 && self <= 1f32, - "float must be between 0.0 and 1.0" - ); - super::from_f32(self) - } - } - - impl IntoQuality for u16 { - fn into_quality(self) -> Quality { - assert!(self <= 1000, "u16 must be between 0 and 1000"); - Quality(self) - } - } - - pub trait Sealed {} - impl Sealed for u16 {} - impl Sealed for f32 {} +pub fn q(val: T) -> Quality +where + T: TryInto, + T::Error: fmt::Debug, +{ + // TODO: on next breaking change, handle unwrap differently + val.try_into().unwrap() } #[cfg(test)] @@ -270,15 +294,13 @@ mod tests { } #[test] - #[should_panic] // FIXME - 32-bit msvc unwinding broken - #[cfg_attr(all(target_arch = "x86", target_env = "msvc"), ignore)] + #[should_panic] fn test_quality_invalid() { q(-1.0); } #[test] - #[should_panic] // FIXME - 32-bit msvc unwinding broken - #[cfg_attr(all(target_arch = "x86", target_env = "msvc"), ignore)] + #[should_panic] fn test_quality_invalid2() { q(2.0); }