rename AnyOrSome to Preference

actix-http/src/header/mod.rs

@@ -55,7 +55,7 @@ pub trait Header: IntoHeaderValue {
     fn name() -> HeaderName;
 
     /// Parse a header
-    fn parse<T: HttpMessage>(msg: &T) -> Result<Self, ParseError>;
+    fn parse<M: HttpMessage>(msg: &M) -> Result<Self, ParseError>;
 }
 
 /// Convert `http::HeaderMap` to our `HeaderMap`.

actix-http/src/header/utils.rs

@@ -12,7 +12,8 @@ where
     I: Iterator<Item = &'a HeaderValue> + 'a,
     T: FromStr,
 {
-    let mut result = Vec::new();
+    let size_guess = all.size_hint().1.unwrap_or(2);
+    let mut result = Vec::with_capacity(size_guess);
 
     for h in all {
         let s = h.to_str().map_err(|_| ParseError::Header)?;
@@ -26,6 +27,7 @@ where
                 .filter_map(|x| x.trim().parse().ok()),
         )
     }
+
     Ok(result)
 }
 
@@ -34,10 +36,12 @@ where
 pub fn from_one_raw_str<T: FromStr>(val: Option<&HeaderValue>) -> Result<T, ParseError> {
     if let Some(line) = val {
         let line = line.to_str().map_err(|_| ParseError::Header)?;
+
         if !line.is_empty() {
             return T::from_str(line).or(Err(ParseError::Header));
         }
     }
+
     Err(ParseError::Header)
 }
 
@@ -48,13 +52,16 @@ where
     T: fmt::Display,
 {
     let mut iter = parts.iter();
+
     if let Some(part) = iter.next() {
         fmt::Display::fmt(part, f)?;
     }
+
     for part in iter {
         f.write_str(", ")?;
         fmt::Display::fmt(part, f)?;
     }
+
     Ok(())
 }
 
@@ -65,3 +72,25 @@ pub fn http_percent_encode(f: &mut fmt::Formatter<'_>, bytes: &[u8]) -> fmt::Res
     let encoded = percent_encoding::percent_encode(bytes, HTTP_VALUE);
     fmt::Display::fmt(&encoded, f)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn comma_delimited_parsing() {
+        let headers = vec![];
+        let res: Vec<usize> = from_comma_delimited(headers.iter()).unwrap();
+        assert_eq!(res, vec![0; 0]);
+
+        let headers = vec![
+            HeaderValue::from_static(""),
+            HeaderValue::from_static(","),
+            HeaderValue::from_static("  "),
+            HeaderValue::from_static("1    ,"),
+            HeaderValue::from_static(""),
+        ];
+        let res: Vec<usize> = from_comma_delimited(headers.iter()).unwrap();
+        assert_eq!(res, vec![1]);
+    }
+}

src/http/header/accept.rs

@@ -77,7 +77,7 @@ crate::http::header::common_header! {
     ///     ])
     /// );
     /// ```
-    (Accept, header::ACCEPT) => (QualityItem<Mime>)+
+    (Accept, header::ACCEPT) => (QualityItem<Mime>)*
 
     test_parse_and_format {
         // Tests from the RFC
@@ -207,12 +207,29 @@ impl Accept {
     /// If no q-factors are provided, the first mime type is chosen. Note that items without
     /// q-factors are given the maximum preference value.
     ///
-    /// Returns `None` if contained list is empty.
+    /// 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) -> Option<Mime> {
+    pub fn preference(&self) -> Mime {
-        // PERF: creating a sorted list is not necessary
+        use actix_http::header::q;
-        self.ranked().into_iter().next()
+
+        let mut max_item = None;
+        let mut max_pref = q(0);
+
+        // 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)
     }
 }
 
@@ -264,7 +281,7 @@ mod tests {
             QualityItem::new("application/xml".parse().unwrap(), q(0.9)),
             QualityItem::new(mime::STAR_STAR, q(0.8)),
         ]);
-        assert_eq!(test.preference(), Some(mime::TEXT_HTML));
+        assert_eq!(test.preference(), mime::TEXT_HTML);
 
         let test = Accept(vec![
             QualityItem::new("video/*".parse().unwrap(), q(0.8)),
@@ -273,6 +290,6 @@ mod tests {
             qitem(mime::IMAGE_SVG),
             QualityItem::new(mime::IMAGE_STAR, q(0.8)),
         ]);
-        assert_eq!(test.preference(), Some(mime::IMAGE_PNG));
+        assert_eq!(test.preference(), mime::IMAGE_PNG);
     }
 }

src/http/header/accept_language.rs

@@ -1,6 +1,6 @@
 use language_tags::LanguageTag;
 
-use super::{common_header, AnyOrSome, QualityItem};
+use super::{common_header, Preference, QualityItem};
 use crate::http::header;
 
 common_header! {
@@ -55,9 +55,13 @@ common_header! {
     ///     ])
     /// );
     /// ```
-    (AcceptLanguage, header::ACCEPT_LANGUAGE) => (QualityItem<AnyOrSome<LanguageTag>>)+
+    (AcceptLanguage, header::ACCEPT_LANGUAGE) => (QualityItem<Preference<LanguageTag>>)*
 
     parse_and_fmt_tests {
+        common_header_test!(no_headers, vec![b""; 0], Some(AcceptLanguage(vec![])));
+
+        common_header_test!(empty_header, vec![b""; 1], Some(AcceptLanguage(vec![])));
+
         common_header_test!(
             example_from_rfc,
             vec![b"da, en-gb;q=0.8, en;q=0.7"]
@@ -92,7 +96,7 @@ impl AcceptLanguage {
     /// for [q-factor weighting].
     ///
     /// [q-factor weighting]: https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.2
-    pub fn ranked(&self) -> Vec<AnyOrSome<LanguageTag>> {
+    pub fn ranked(&self) -> Vec<Preference<LanguageTag>> {
         if self.0.is_empty() {
             return vec![];
         }
@@ -113,12 +117,28 @@ impl AcceptLanguage {
     /// 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 [`AnyOrSome::Any`] if contained list is empty.
+    /// 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) -> AnyOrSome<LanguageTag> {
+    pub fn preference(&self) -> Preference<LanguageTag> {
-        // PERF: creating a sorted list is not necessary
+        use actix_http::header::q;
-        self.ranked().into_iter().next().unwrap_or(AnyOrSome::Any)
+
+        let mut max_item = None;
+        let mut max_pref = q(0);
+
+        // 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)
     }
 }
 
@@ -181,7 +201,10 @@ mod tests {
             QualityItem::new("*".parse().unwrap(), q(500)),
             QualityItem::new("de".parse().unwrap(), q(700)),
         ]);
-        assert_eq!(test.preference(), AnyOrSome::Item("fr-CH".parse().unwrap()));
+        assert_eq!(
+            test.preference(),
+            Preference::Specific("fr-CH".parse().unwrap())
+        );
 
         let test = AcceptLanguage(vec![
             qitem("fr".parse().unwrap()),
@@ -190,9 +213,12 @@ mod tests {
             qitem("*".parse().unwrap()),
             qitem("de".parse().unwrap()),
         ]);
-        assert_eq!(test.preference(), AnyOrSome::Item("fr".parse().unwrap()));
+        assert_eq!(
+            test.preference(),
+            Preference::Specific("fr".parse().unwrap())
+        );
 
         let test = AcceptLanguage(vec![]);
-        assert_eq!(test.preference(), AnyOrSome::Any);
+        assert_eq!(test.preference(), Preference::Any);
     }
 }

src/http/header/any_or_some.rs

@@ -1,70 +0,0 @@
-use std::{
-    fmt::{self, Write as _},
-    str,
-};
-
-/// A wrapper for types used in header values where wildcard (`*`) items are allowed but the
-/// underlying type does not support them.
-///
-/// For example, we use the `language-tags` crate for the [`AcceptLanguage`](super::AcceptLanguage)
-/// typed header but it does parse `*` successfully. On the other hand, the `mime` crate, used for
-/// [`Accept`](super::Accept), has first-party support for wildcard items so this wrapper is not
-/// used in those header types.
-#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Hash)]
-pub enum AnyOrSome<T> {
-    /// A wildcard value.
-    Any,
-
-    /// A valid `T`.
-    Item(T),
-}
-
-impl<T> AnyOrSome<T> {
-    /// Returns true if item is wildcard (`*`) variant.
-    pub fn is_any(&self) -> bool {
-        matches!(self, Self::Any)
-    }
-
-    /// Returns true if item is a valid item (`T`) variant.
-    pub fn is_item(&self) -> bool {
-        matches!(self, Self::Item(_))
-    }
-
-    /// Returns reference to value in `Item` variant, if it is set.
-    pub fn item(&self) -> Option<&T> {
-        match self {
-            AnyOrSome::Item(ref item) => Some(item),
-            AnyOrSome::Any => None,
-        }
-    }
-
-    /// Consumes the container, returning the value in the `Item` variant, if it is set.
-    pub fn into_item(self) -> Option<T> {
-        match self {
-            AnyOrSome::Item(item) => Some(item),
-            AnyOrSome::Any => None,
-        }
-    }
-}
-
-impl<T: fmt::Display> fmt::Display for AnyOrSome<T> {
-    #[inline]
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            AnyOrSome::Any => f.write_char('*'),
-            AnyOrSome::Item(item) => fmt::Display::fmt(item, f),
-        }
-    }
-}
-
-impl<T: str::FromStr> str::FromStr for AnyOrSome<T> {
-    type Err = T::Err;
-
-    #[inline]
-    fn from_str(s: &str) -> Result<Self, Self::Err> {
-        match s.trim() {
-            "*" => Ok(Self::Any),
-            other => other.parse().map(AnyOrSome::Item),
-        }
-    }
-}

src/http/header/macros.rs

@@ -19,16 +19,18 @@ macro_rules! common_header_deref {
     };
 }
 
+/// Sets up a test module with some useful imports for use with [`common_header_test!`].
 macro_rules! common_header_test_module {
     ($id:ident, $tm:ident{$($tf:item)*}) => {
-        #[allow(unused_imports)]
         #[cfg(test)]
         mod $tm {
+            #![allow(unused_imports)]
+
             use std::str;
             use actix_http::http::Method;
             use mime::*;
             use $crate::http::header::*;
-            use super::$id as HeaderField;
+            use super::{$id as HeaderField, *};
             $($tf)*
         }
     }
@@ -71,7 +73,7 @@ macro_rules! common_header_test {
         }
     };
 
-    ($id:ident, $raw:expr, $typed:expr) => {
+    ($id:ident, $raw:expr, $exp:expr) => {
         #[test]
         fn $id() {
             use actix_http::test;
@@ -83,28 +85,35 @@ macro_rules! common_header_test {
             }
             let req = req.finish();
             let val = HeaderField::parse(&req);
-            let typed: Option<HeaderField> = $typed;
+            let exp: Option<HeaderField> = $exp;
 
-            // Test parsing
+            println!("req: {:?}", &req);
-            assert_eq!(val.ok(), typed);
+            println!("val: {:?}", &val);
+            println!("exp: {:?}", &exp);
 
-            // Test formatting
+            // test parsing
-            if typed.is_some() {
+            assert_eq!(val.ok(), exp);
+
+            // test formatting
+            if let Some(exp) = exp {
                 let raw = &($raw)[..];
                 let mut iter = raw.iter().map(|b| str::from_utf8(&b[..]).unwrap());
                 let mut joined = String::new();
-                joined.push_str(iter.next().unwrap());
+                if let Some(s) = iter.next() {
+                    joined.push_str(s);
                     for s in iter {
                         joined.push_str(", ");
                         joined.push_str(s);
                     }
-                assert_eq!(format!("{}", typed.unwrap()), joined);
+                }
+                assert_eq!(format!("{}", exp), joined);
             }
         }
     };
 }
 
 macro_rules! common_header {
+    // TODO: these docs are wrong, there's no $n or $nn
     // $a:meta: Attributes associated with the header item (usually docs)
     // $id:ident: Identifier of the header
     // $n:expr: Lowercase name of the header
@@ -123,7 +132,7 @@ macro_rules! common_header {
             }
 
             #[inline]
-            fn parse<T: $crate::HttpMessage>(msg: &T) -> Result<Self, $crate::error::ParseError> {
+            fn parse<M: $crate::HttpMessage>(msg: &M) -> Result<Self, $crate::error::ParseError> {
                 let headers = msg.headers().get_all(Self::name());
                 $crate::http::header::from_comma_delimited(headers).map($id)
             }
@@ -162,9 +171,7 @@ macro_rules! common_header {
                 $name
             }
             #[inline]
-            fn parse<T>(msg: &T) -> Result<Self, $crate::error::ParseError>
+            fn parse<M: $crate::HttpMessage>(msg: &M) -> Result<Self, $crate::error::ParseError> {
-                where T: $crate::HttpMessage
-            {
                 $crate::http::header::from_comma_delimited(
                     msg.headers().get_all(Self::name())).map($id)
             }
@@ -204,7 +211,7 @@ macro_rules! common_header {
             }
 
             #[inline]
-            fn parse<T: $crate::HttpMessage>(msg: &T) -> Result<Self, $crate::error::ParseError> {
+            fn parse<M: $crate::HttpMessage>(msg: &M) -> Result<Self, $crate::error::ParseError> {
                 let header = msg.headers().get(Self::name());
                 $crate::http::header::from_one_raw_str(header).map($id)
             }
@@ -244,7 +251,7 @@ macro_rules! common_header {
             }
 
             #[inline]
-            fn parse<T: $crate::HttpMessage>(msg: &T) -> Result<Self, $crate::error::ParseError> {
+            fn parse<M: $crate::HttpMessage>(msg: &M) -> Result<Self, $crate::error::ParseError> {
                 let is_any = msg
                     .headers()
                     .get(Self::name())

src/http/header/mod.rs

@@ -22,7 +22,6 @@ mod accept_charset;
 mod accept;
 mod accept_language;
 mod allow;
-mod any_or_some;
 mod cache_control;
 mod content_disposition;
 mod content_language;
@@ -40,6 +39,7 @@ mod if_range;
 mod if_unmodified_since;
 mod last_modified;
 mod macros;
+mod preference;
 // mod range;
 
 #[cfg(test)]
@@ -51,7 +51,6 @@ pub use self::accept_charset::AcceptCharset;
 pub use self::accept::Accept;
 pub use self::accept_language::AcceptLanguage;
 pub use self::allow::Allow;
-pub use self::any_or_some::AnyOrSome;
 pub use self::cache_control::{CacheControl, CacheDirective};
 pub use self::content_disposition::{ContentDisposition, DispositionParam, DispositionType};
 pub use self::content_language::ContentLanguage;
@@ -68,6 +67,7 @@ pub use self::if_none_match::IfNoneMatch;
 pub use self::if_range::IfRange;
 pub use self::if_unmodified_since::IfUnmodifiedSince;
 pub use self::last_modified::LastModified;
+pub use self::preference::Preference;
 //pub use self::range::{Range, ByteRangeSpec};
 
 /// Format writer ([`fmt::Write`]) for a [`BytesMut`].

src/http/header/preference.rs

@@ -0,0 +1,70 @@
+use std::{
+    fmt::{self, Write as _},
+    str,
+};
+
+/// A wrapper for types used in header values where wildcard (`*`) items are allowed but the
+/// underlying type does not support them.
+///
+/// For example, we use the `language-tags` crate for the [`AcceptLanguage`](super::AcceptLanguage)
+/// typed header but it does not parse `*` successfully. On the other hand, the `mime` crate, used
+/// for [`Accept`](super::Accept), has first-party support for wildcard items so this wrapper is not
+/// used in those header types.
+#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Hash)]
+pub enum Preference<T> {
+    /// A wildcard value.
+    Any,
+
+    /// A valid `T`.
+    Specific(T),
+}
+
+impl<T> Preference<T> {
+    /// Returns true if the preference is the any/wildcard (`*`) value.
+    pub fn is_any(&self) -> bool {
+        matches!(self, Self::Any)
+    }
+
+    /// Returns true if variant is a specific item (`T`) variant.
+    pub fn is_specific(&self) -> bool {
+        matches!(self, Self::Specific(_))
+    }
+
+    /// Returns reference to value in `Specific` variant, if it is set.
+    pub fn item(&self) -> Option<&T> {
+        match self {
+            Preference::Specific(ref item) => Some(item),
+            Preference::Any => None,
+        }
+    }
+
+    /// Consumes the container, returning the value in the `Specific` variant, if it is set.
+    pub fn into_item(self) -> Option<T> {
+        match self {
+            Preference::Specific(item) => Some(item),
+            Preference::Any => None,
+        }
+    }
+}
+
+impl<T: fmt::Display> fmt::Display for Preference<T> {
+    #[inline]
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Preference::Any => f.write_char('*'),
+            Preference::Specific(item) => fmt::Display::fmt(item, f),
+        }
+    }
+}
+
+impl<T: str::FromStr> str::FromStr for Preference<T> {
+    type Err = T::Err;
+
+    #[inline]
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        match s.trim() {
+            "*" => Ok(Self::Any),
+            other => other.parse().map(Preference::Specific),
+        }
+    }
+}