use common header macro for CacheControl

actix-http/src/header/utils.rs

@@ -65,3 +65,18 @@ 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 parse_from_multiple_headers() {
+        let headers = vec![
+            HeaderValue::from_static("1, 2"),
+            HeaderValue::from_static("3,4"),
+        ];
+        let result = from_comma_delimited::<_, usize>(headers.iter()).unwrap();
+        assert_eq!(result, vec![1, 2, 3, 4]);
+    }
+}

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
@@ -118,8 +118,9 @@ crate::http::header::common_header! {
 
         #[test]
         fn test_fuzzing1() {
-            use actix_http::test::TestRequest;
+            let req = test::TestRequest::default()
-            let req = TestRequest::default().insert_header((crate::http::header::ACCEPT, "chunk#;e")).finish();
+                .insert_header((header::ACCEPT, "chunk#;e"))
+                .finish();
             let header = Accept::parse(&req);
             assert!(header.is_ok());
         }

src/http/header/accept_language.rs

@@ -60,6 +60,12 @@ common_header! {
             vec![b"da, en-gb;q=0.8, en;q=0.7"]
         );
 
+        common_header_test!(
+            empty_value,
+            vec![b""],
+            None
+        );
+
         common_header_test!(
             not_ordered_by_weight,
             vec![b"en-US, en; q=0.5, fr"],

src/http/header/cache_control.rs

@@ -1,89 +1,97 @@
-use std::fmt::{self, Write};
+use std::{fmt, str};
-use std::str::FromStr;
-
-use derive_more::{Deref, DerefMut};
-
-use super::{fmt_comma_delimited, from_comma_delimited, Header, IntoHeaderValue, Writer};
 
+use super::common_header;
 use crate::http::header;
 
-/// `Cache-Control` header, defined
+common_header! {
-/// in [RFC 7234 §5.2](https://datatracker.ietf.org/doc/html/rfc7234#section-5.2).
+    /// `Cache-Control` header, defined
-///
+    /// in [RFC 7234 §5.2](https://datatracker.ietf.org/doc/html/rfc7234#section-5.2).
-/// The `Cache-Control` header field is used to specify directives for
+    ///
-/// caches along the request/response chain.  Such cache directives are
+    /// The `Cache-Control` header field is used to specify directives for
-/// unidirectional in that the presence of a directive in a request does
+    /// caches along the request/response chain.  Such cache directives are
-/// not imply that the same directive is to be given in the response.
+    /// unidirectional in that the presence of a directive in a request does
-///
+    /// not imply that the same directive is to be given in the response.
-/// # ABNF
+    ///
-/// ```plain
+    /// # ABNF
-/// Cache-Control   = 1#cache-directive
+    /// ```text
-/// cache-directive = token [ "=" ( token / quoted-string ) ]
+    /// Cache-Control   = 1#cache-directive
-/// ```
+    /// cache-directive = token [ "=" ( token / quoted-string ) ]
-///
+    /// ```
-/// # Example Values
+    ///
-///
+    /// # Example Values
-/// * `no-cache`
+    ///
-/// * `private, community="UCI"`
+    /// * `no-cache`
-/// * `max-age=30`
+    /// * `private, community="UCI"`
-///
+    /// * `max-age=30`
-/// # Examples
+    ///
-/// ```
+    /// # Examples
-/// use actix_web::HttpResponse;
+    /// ```
-/// use actix_web::http::header::{CacheControl, CacheDirective};
+    /// use actix_web::HttpResponse;
-///
+    /// use actix_web::http::header::{CacheControl, CacheDirective};
-/// let mut builder = HttpResponse::Ok();
+    ///
-/// builder.insert_header(CacheControl(vec![CacheDirective::MaxAge(86400u32)]));
+    /// let mut builder = HttpResponse::Ok();
-/// ```
+    /// builder.insert_header(CacheControl(vec![CacheDirective::MaxAge(86400u32)]));
-///
+    /// ```
-/// ```
+    ///
-/// use actix_web::HttpResponse;
+    /// ```
-/// use actix_web::http::header::{CacheControl, CacheDirective};
+    /// use actix_web::HttpResponse;
-///
+    /// use actix_web::http::header::{CacheControl, CacheDirective};
-/// let mut builder = HttpResponse::Ok();
+    ///
-/// builder.insert_header(CacheControl(vec![
+    /// let mut builder = HttpResponse::Ok();
-///     CacheDirective::NoCache,
+    /// builder.insert_header(CacheControl(vec![
-///     CacheDirective::Private,
+    ///     CacheDirective::NoCache,
-///     CacheDirective::MaxAge(360u32),
+    ///     CacheDirective::Private,
-///     CacheDirective::Extension("foo".to_owned(), Some("bar".to_owned())),
+    ///     CacheDirective::MaxAge(360u32),
-/// ]));
+    ///     CacheDirective::Extension("foo".to_owned(), Some("bar".to_owned())),
-/// ```
+    /// ]));
-#[derive(Debug, Clone, PartialEq, Eq, Deref, DerefMut)]
+    /// ```
-pub struct CacheControl(pub Vec<CacheDirective>);
 
-// TODO: this could just be the crate::http::header::common_header! macro
+    (CacheControl, header::CACHE_CONTROL) => (CacheDirective)+
-impl Header for CacheControl {
+
-    fn name() -> header::HeaderName {
+    test_parse_and_format {
-        header::CACHE_CONTROL
+        common_header_test!(
+            multiple_headers,
+            vec![&b"no-cache"[..], &b"private"[..]],
+            Some(CacheControl(vec![
+                CacheDirective::NoCache,
+                CacheDirective::Private,
+            ]))
+        );
+
+        common_header_test!(
+            argument,
+            vec![b"max-age=100, private"],
+            Some(CacheControl(vec![
+                CacheDirective::MaxAge(100),
+                CacheDirective::Private,
+            ]))
+        );
+
+        common_header_test!(
+            extension,
+            vec![b"foo, bar=baz"],
+            Some(CacheControl(vec![
+                CacheDirective::Extension("foo".to_owned(), None),
+                CacheDirective::Extension("bar".to_owned(), Some("baz".to_owned())),
+            ]))
+        );
+
+        common_header_test!(bad_syntax, vec![b"foo="], None);
+
+        common_header_test!(empty_header, vec![b""], None);
+
+        #[test]
+        fn parse_quote_form() {
+            use actix_http::test::TestRequest;
+            let req = TestRequest::default()
+                .insert_header((header::CACHE_CONTROL, "max-age=\"200\""))
+                .finish();
+            let cache = Header::parse(&req);
+            assert_eq!(
+                cache.ok(),
+                Some(CacheControl(vec![CacheDirective::MaxAge(200)]))
+            )
         }
-
-    #[inline]
-    fn parse<T>(msg: &T) -> Result<Self, crate::error::ParseError>
-    where
-        T: crate::HttpMessage,
-    {
-        let directives = from_comma_delimited(msg.headers().get_all(&Self::name()))?;
-        if !directives.is_empty() {
-            Ok(CacheControl(directives))
-        } else {
-            Err(crate::error::ParseError::Header)
-        }
-    }
-}
-
-impl fmt::Display for CacheControl {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        fmt_comma_delimited(f, &self.0[..])
-    }
-}
-
-impl IntoHeaderValue for CacheControl {
-    type Error = header::InvalidHeaderValue;
-
-    fn try_into_value(self) -> Result<header::HeaderValue, Self::Error> {
-        let mut writer = Writer::new();
-        let _ = write!(&mut writer, "{}", self);
-        header::HeaderValue::from_maybe_shared(writer.take())
     }
 }
 
@@ -126,8 +134,8 @@ pub enum CacheDirective {
 impl fmt::Display for CacheDirective {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         use self::CacheDirective::*;
-        fmt::Display::fmt(
+
-            match *self {
+        let dir_str = match self {
             NoCache => "no-cache",
             NoStore => "no-store",
             NoTransform => "no-transform",
@@ -143,21 +151,23 @@ impl fmt::Display for CacheDirective {
             ProxyRevalidate => "proxy-revalidate",
             SMaxAge(secs) => return write!(f, "s-maxage={}", secs),
 
-                Extension(ref name, None) => &name[..],
+            Extension(name, None) => name.as_str(),
-                Extension(ref name, Some(ref arg)) => {
+            Extension(name, Some(arg)) => return write!(f, "{}={}", name, arg),
-                    return write!(f, "{}={}", name, arg);
+        };
-                }
+
-            },
+        f.write_str(dir_str)
-            f,
-        )
     }
 }
 
-impl FromStr for CacheDirective {
+impl str::FromStr for CacheDirective {
-    type Err = Option<<u32 as FromStr>::Err>;
+    type Err = Option<<u32 as str::FromStr>::Err>;
-    fn from_str(s: &str) -> Result<CacheDirective, Option<<u32 as FromStr>::Err>> {
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
         use self::CacheDirective::*;
+
         match s {
+            "" => Err(None),
+
             "no-cache" => Ok(NoCache),
             "no-store" => Ok(NoStore),
             "no-transform" => Ok(NoTransform),
@@ -166,7 +176,7 @@ impl FromStr for CacheDirective {
             "public" => Ok(Public),
             "private" => Ok(Private),
             "proxy-revalidate" => Ok(ProxyRevalidate),
-            "" => Err(None),
+
             _ => match s.find('=') {
                 Some(idx) if idx + 1 < s.len() => {
                     match (&s[..idx], (&s[idx + 1..]).trim_matches('"')) {
@@ -183,76 +193,3 @@ impl FromStr for CacheDirective {
         }
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use crate::http::header::Header;
-    use actix_http::test::TestRequest;
-
-    #[test]
-    fn test_parse_multiple_headers() {
-        let req = TestRequest::default()
-            .insert_header((header::CACHE_CONTROL, "no-cache, private"))
-            .finish();
-        let cache = Header::parse(&req);
-        assert_eq!(
-            cache.ok(),
-            Some(CacheControl(vec![
-                CacheDirective::NoCache,
-                CacheDirective::Private,
-            ]))
-        )
-    }
-
-    #[test]
-    fn test_parse_argument() {
-        let req = TestRequest::default()
-            .insert_header((header::CACHE_CONTROL, "max-age=100, private"))
-            .finish();
-        let cache = Header::parse(&req);
-        assert_eq!(
-            cache.ok(),
-            Some(CacheControl(vec![
-                CacheDirective::MaxAge(100),
-                CacheDirective::Private,
-            ]))
-        )
-    }
-
-    #[test]
-    fn test_parse_quote_form() {
-        let req = TestRequest::default()
-            .insert_header((header::CACHE_CONTROL, "max-age=\"200\""))
-            .finish();
-        let cache = Header::parse(&req);
-        assert_eq!(
-            cache.ok(),
-            Some(CacheControl(vec![CacheDirective::MaxAge(200)]))
-        )
-    }
-
-    #[test]
-    fn test_parse_extension() {
-        let req = TestRequest::default()
-            .insert_header((header::CACHE_CONTROL, "foo, bar=baz"))
-            .finish();
-        let cache = Header::parse(&req);
-        assert_eq!(
-            cache.ok(),
-            Some(CacheControl(vec![
-                CacheDirective::Extension("foo".to_owned(), None),
-                CacheDirective::Extension("bar".to_owned(), Some("baz".to_owned())),
-            ]))
-        )
-    }
-
-    #[test]
-    fn test_parse_bad_syntax() {
-        let req = TestRequest::default()
-            .insert_header((header::CACHE_CONTROL, "foo="))
-            .finish();
-        let cache: Result<CacheControl, _> = Header::parse(&req);
-        assert_eq!(cache.ok(), None)
-    }
-}

src/http/header/macros.rs

@@ -3,11 +3,14 @@ macro_rules! common_header_test_module {
         #[allow(unused_imports)]
         #[cfg(test)]
         mod $tm {
-            use std::str;
+            use ::core::str;
-            use actix_http::http::Method;
+
-            use mime::*;
+            use ::actix_http::{http::Method, test};
-            use $crate::http::header::*;
+            use ::mime::*;
+
+            use $crate::http::header::{self, *};
             use super::$id as HeaderField;
+
             $($tf)*
         }
     }
@@ -18,22 +21,22 @@ macro_rules! common_header_test {
     ($id:ident, $raw:expr) => {
         #[test]
         fn $id() {
-            use actix_http::test;
+            use ::actix_http::test;
 
             let raw = $raw;
-            let a: Vec<Vec<u8>> = raw.iter().map(|x| x.to_vec()).collect();
+            let headers = raw.iter().map(|x| x.to_vec()).collect::<Vec<_>>();
 
             let mut req = test::TestRequest::default();
 
-            for item in a {
+            for item in headers {
-                req = req.insert_header((HeaderField::name(), item)).take();
+                req = req.append_header((HeaderField::name(), item)).take();
             }
 
             let req = req.finish();
             let value = HeaderField::parse(&req);
 
             let result = format!("{}", value.unwrap());
-            let expected = String::from_utf8(raw[0].to_vec()).unwrap();
+            let expected = ::std::string::String::from_utf8(raw[0].to_vec()).unwrap();
 
             let result_cmp: Vec<String> = result
                 .to_ascii_lowercase()
@@ -55,14 +58,17 @@ macro_rules! common_header_test {
         fn $id() {
             use actix_http::test;
 
-            let a: Vec<Vec<u8>> = $raw.iter().map(|x| x.to_vec()).collect();
+            let headers = $raw.iter().map(|x| x.to_vec()).collect::<Vec<_>>();
             let mut req = test::TestRequest::default();
-            for item in a {
+
-                req.insert_header((HeaderField::name(), item));
+            for item in headers {
+                req.append_header((HeaderField::name(), item));
             }
+
             let req = req.finish();
             let val = HeaderField::parse(&req);
-            let typed: Option<HeaderField> = $typed;
+
+            let typed: ::core::option::Option<HeaderField> = $typed;
 
             // Test parsing
             assert_eq!(val.ok(), typed);
@@ -118,6 +124,7 @@ macro_rules! common_header {
         impl $crate::http::header::IntoHeaderValue for $id {
             type Error = $crate::http::header::InvalidHeaderValue;
 
+            #[inline]
             fn try_into_value(self) -> Result<$crate::http::header::HeaderValue, Self::Error> {
                 use ::core::fmt::Write;
                 let mut writer = $crate::http::header::Writer::new();
@@ -133,18 +140,24 @@ macro_rules! common_header {
         #[derive(Debug, Clone, PartialEq, Eq, ::derive_more::Deref, ::derive_more::DerefMut)]
         pub struct $id(pub Vec<$item>);
 
-
         impl $crate::http::header::Header for $id {
             #[inline]
             fn name() -> $crate::http::header::HeaderName {
                 $name
             }
+
             #[inline]
-            fn parse<T>(msg: &T) -> Result<Self, $crate::error::ParseError>
+            fn parse<T: $crate::HttpMessage>(msg: &T) -> Result<Self, $crate::error::ParseError>{
-                where T: $crate::HttpMessage
+                let headers = msg.headers().get_all(Self::name());
-            {
+                println!("{:?}", &headers);
-                $crate::http::header::from_comma_delimited(
+                $crate::http::header::from_comma_delimited(headers)
-                    msg.headers().get_all(Self::name())).map($id)
+                    .and_then(|items| {
+                        if items.is_empty() {
+                            Err($crate::error::ParseError::Header)
+                        } else {
+                            Ok($id(items))
+                        }
+                    })
             }
         }
 
@@ -158,6 +171,7 @@ macro_rules! common_header {
         impl $crate::http::header::IntoHeaderValue for $id {
             type Error = $crate::http::header::InvalidHeaderValue;
 
+            #[inline]
             fn try_into_value(self) -> Result<$crate::http::header::HeaderValue, Self::Error> {
                 use ::core::fmt::Write;
                 let mut writer = $crate::http::header::Writer::new();
@@ -196,6 +210,7 @@ macro_rules! common_header {
         impl $crate::http::header::IntoHeaderValue for $id {
             type Error = $crate::http::header::InvalidHeaderValue;
 
+            #[inline]
             fn try_into_value(self) -> Result<$crate::http::header::HeaderValue, Self::Error> {
                 self.0.try_into_value()
             }
@@ -250,6 +265,7 @@ macro_rules! common_header {
         impl $crate::http::header::IntoHeaderValue for $id {
             type Error = $crate::http::header::InvalidHeaderValue;
 
+            #[inline]
             fn try_into_value(self) -> Result<$crate::http::header::HeaderValue, Self::Error> {
                 use ::core::fmt::Write;
                 let mut writer = $crate::http::header::Writer::new();