re-instate accept-encoding typed header (#2482)

CHANGES.md

@@ -3,6 +3,7 @@
 ## Unreleased - 2021-xx-xx
 ### Added
 * Methods on `AcceptLanguage`: `ranked` and `preference`. [#2480]
+* `AcceptEncoding` typed header. [#2482]
 
 ### Changed
 * Rename `Accept::{mime_precedence => ranked}`. [#2480]
@@ -10,8 +11,10 @@
 
 ### Fixed
 * Accept wildcard `*` items in `AcceptLanguage`. [#2480]
+* Typed headers containing lists that require one or more items now enforce this minimum. [#2482]
 
 [#2480]: https://github.com/actix/actix-web/pull/2480
+[#2482]: https://github.com/actix/actix-web/pull/2482
 
 
 ## 4.0.0-beta.13 - 2021-11-30

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

@@ -27,7 +27,7 @@ const MAX_FLOAT_QUALITY: f32 = 1.0;
 ///
 /// [RFC 7231 §5.3.1](https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.1) gives more
 /// information on quality values in HTTP header fields.
-#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
 pub struct Quality(u16);
 
 impl Quality {

actix-http/src/header/utils.rs

@@ -83,6 +83,13 @@ mod tests {
         let res: Vec<usize> = from_comma_delimited(headers.iter()).unwrap();
         assert_eq!(res, vec![0; 0]);
 
+        let headers = vec![
+            HeaderValue::from_static("1, 2"),
+            HeaderValue::from_static("3,4"),
+        ];
+        let res: Vec<usize> = from_comma_delimited(headers.iter()).unwrap();
+        assert_eq!(res, vec![1, 2, 3, 4]);
+
         let headers = vec![
             HeaderValue::from_static(""),
             HeaderValue::from_static(","),

scripts/ci-test

@@ -14,3 +14,5 @@ cargo test --lib --tests -p=actix-test --all-features
 cargo test --lib --tests -p=actix-files
 cargo test --lib --tests -p=actix-multipart --all-features
 cargo test --lib --tests -p=actix-web-actors --all-features
+
+cargo test --workspace --doc

src/http/header/accept.rs

@@ -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_encoding.rs

@@ -1,8 +1,9 @@
-// TODO: reinstate module
+use actix_http::header::QualityItem;
 
-use header::{Encoding, QualityItem};
+use super::{common_header, Encoding};
+use crate::http::header;
 
-header! {
+common_header! {
     /// `Accept-Encoding` header, defined
     /// in [RFC 7231](https://datatracker.ietf.org/doc/html/rfc7231#section-5.3.4)
     ///
@@ -30,7 +31,7 @@ header! {
     /// use actix_web::HttpResponse;
     /// use actix_web::http::header::{AcceptEncoding, Encoding, qitem};
     ///
-    /// let mut builder = HttpResponse::new();
+    /// let mut builder = HttpResponse::Ok();
     /// builder.insert_header(
     ///     AcceptEncoding(vec![qitem(Encoding::Chunked)])
     /// );
@@ -39,7 +40,7 @@ header! {
     /// use actix_web::HttpResponse;
     /// use actix_web::http::header::{AcceptEncoding, Encoding, qitem};
     ///
-    /// let mut builder = HttpResponse::new();
+    /// let mut builder = HttpResponse::Ok();
     /// builder.insert_header(
     ///     AcceptEncoding(vec![
     ///         qitem(Encoding::Chunked),
@@ -52,7 +53,7 @@ header! {
     /// use actix_web::HttpResponse;
     /// use actix_web::http::header::{AcceptEncoding, Encoding, QualityItem, q, qitem};
     ///
-    /// let mut builder = HttpResponse::new();
+    /// let mut builder = HttpResponse::Ok();
     /// builder.insert_header(
     ///     AcceptEncoding(vec![
     ///         qitem(Encoding::Chunked),
@@ -65,14 +66,14 @@ header! {
 
     test_parse_and_format {
         // From the RFC
-        crate::http::header::common_header_test!(test1, vec![b"compress, gzip"]);
+        common_header_test!(test1, vec![b"compress, gzip"]);
-        crate::http::header::common_header_test!(test2, vec![b""], Some(AcceptEncoding(vec![])));
+        common_header_test!(test2, vec![b""], Some(AcceptEncoding(vec![])));
-        crate::http::header::common_header_test!(test3, vec![b"*"]);
+        common_header_test!(test3, vec![b"*"]);
 
         // Note: Removed quality 1 from gzip
-        crate::http::header::common_header_test!(test4, vec![b"compress;q=0.5, gzip"]);
+        common_header_test!(test4, vec![b"compress;q=0.5, gzip"]);
 
         // Note: Removed quality 1 from gzip
-        crate::http::header::common_header_test!(test5, vec![b"gzip, identity; q=0.5, *;q=0"]);
+        common_header_test!(test5, vec![b"gzip, identity; q=0.5, *;q=0"]);
     }
 }

src/http/header/accept_language.rs

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

src/http/header/cache_control.rs

@@ -1,92 +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`
+    /// * `no-cache`
-/// * `private, community="UCI"`
+    /// * `private, community="UCI"`
-/// * `max-age=30`
+    /// * `max-age=30`
-///
+    ///
-/// # Examples
+    /// # Examples
-/// ```
+    /// ```
-/// use actix_web::HttpResponse;
+    /// use actix_web::HttpResponse;
-/// use actix_web::http::header::{CacheControl, CacheDirective};
+    /// use actix_web::http::header::{CacheControl, CacheDirective};
-///
+    ///
-/// let mut builder = HttpResponse::Ok();
+    /// let mut builder = HttpResponse::Ok();
-/// builder.insert_header(CacheControl(vec![CacheDirective::MaxAge(86400u32)]));
+    /// builder.insert_header(CacheControl(vec![CacheDirective::MaxAge(86400u32)]));
-/// ```
+    /// ```
-///
+    ///
-/// ```
+    /// ```
-/// use actix_web::HttpResponse;
+    /// use actix_web::HttpResponse;
-/// use actix_web::http::header::{CacheControl, CacheDirective};
+    /// use actix_web::http::header::{CacheControl, CacheDirective};
-///
+    ///
-/// let mut builder = HttpResponse::Ok();
+    /// let mut builder = HttpResponse::Ok();
-/// builder.insert_header(CacheControl(vec![
+    /// builder.insert_header(CacheControl(vec![
-///     CacheDirective::NoCache,
+    ///     CacheDirective::NoCache,
-///     CacheDirective::Private,
+    ///     CacheDirective::Private,
-///     CacheDirective::MaxAge(360u32),
+    ///     CacheDirective::MaxAge(360u32),
-///     CacheDirective::Extension("foo".to_owned(), Some("bar".to_owned())),
+    ///     CacheDirective::Extension("foo".to_owned(), Some("bar".to_owned())),
-/// ]));
+    /// ]));
-/// ```
+    /// ```
-#[derive(Debug, Clone, PartialEq, Eq, Deref, DerefMut)]
+    (CacheControl, header::CACHE_CONTROL) => (CacheDirective)+
-pub struct CacheControl(pub Vec<CacheDirective>);
 
-// TODO: this could just be the crate::http::header::common_header! macro
+    test_parse_and_format {
-impl Header for CacheControl {
+        common_header_test!(no_headers, vec![b""; 0], None);
-    fn name() -> header::HeaderName {
+        common_header_test!(empty_header, vec![b""; 1], None);
-        header::CACHE_CONTROL
+        common_header_test!(bad_syntax, vec![b"foo="], None);
-    }
 
-    #[inline]
+        common_header_test!(
-    fn parse<T>(msg: &T) -> Result<Self, crate::error::ParseError>
+            multiple_headers,
-    where
+            vec![&b"no-cache"[..], &b"private"[..]],
-        T: crate::HttpMessage,
+            Some(CacheControl(vec![
-    {
+                CacheDirective::NoCache,
-        let directives = from_comma_delimited(msg.headers().get_all(&Self::name()))?;
+                CacheDirective::Private,
-        if !directives.is_empty() {
+            ]))
-            Ok(CacheControl(directives))
+        );
-        } else {
+
-            Err(crate::error::ParseError::Header)
+        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())),
+            ]))
+        );
+
+        #[test]
+        fn parse_quote_form() {
+            let req = test::TestRequest::default()
+                .insert_header((header::CACHE_CONTROL, "max-age=\"200\""))
+                .finish();
+
+            assert_eq!(
+                Header::parse(&req).ok(),
+                Some(CacheControl(vec![CacheDirective::MaxAge(200)]))
+            )
         }
     }
 }
 
-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())
-    }
-}
-
 /// `CacheControl` contains a list of these directives.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum CacheDirective {
@@ -126,38 +131,40 @@ 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 {
-                NoCache => "no-cache",
-                NoStore => "no-store",
-                NoTransform => "no-transform",
-                OnlyIfCached => "only-if-cached",
 
-                MaxAge(secs) => return write!(f, "max-age={}", secs),
+        let dir_str = match self {
-                MaxStale(secs) => return write!(f, "max-stale={}", secs),
+            NoCache => "no-cache",
-                MinFresh(secs) => return write!(f, "min-fresh={}", secs),
+            NoStore => "no-store",
+            NoTransform => "no-transform",
+            OnlyIfCached => "only-if-cached",
 
-                MustRevalidate => "must-revalidate",
+            MaxAge(secs) => return write!(f, "max-age={}", secs),
-                Public => "public",
+            MaxStale(secs) => return write!(f, "max-stale={}", secs),
-                Private => "private",
+            MinFresh(secs) => return write!(f, "min-fresh={}", secs),
-                ProxyRevalidate => "proxy-revalidate",
-                SMaxAge(secs) => return write!(f, "s-maxage={}", secs),
 
-                Extension(ref name, None) => &name[..],
+            MustRevalidate => "must-revalidate",
-                Extension(ref name, Some(ref arg)) => {
+            Public => "public",
-                    return write!(f, "{}={}", name, arg);
+            Private => "private",
-                }
+            ProxyRevalidate => "proxy-revalidate",
-            },
+            SMaxAge(secs) => return write!(f, "s-maxage={}", secs),
-            f,
+
-        )
+            Extension(name, None) => name.as_str(),
+            Extension(name, Some(arg)) => return write!(f, "{}={}", name, arg),
+        };
+
+        f.write_str(dir_str)
     }
 }
 
-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 +173,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 +190,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/encoding.rs

@@ -4,26 +4,33 @@ pub use self::Encoding::{
     Brotli, Chunked, Compress, Deflate, EncodingExt, Gzip, Identity, Trailers, Zstd,
 };
 
-/// A value to represent an encoding used in `Transfer-Encoding`
+/// A value to represent an encoding used in `Transfer-Encoding` or `Accept-Encoding` header.
-/// or `Accept-Encoding` header.
+#[derive(Debug, Clone, PartialEq, Eq)]
-#[derive(Clone, PartialEq, Debug)]
 pub enum Encoding {
     /// The `chunked` encoding.
     Chunked,
+
     /// The `br` encoding.
     Brotli,
+
     /// The `gzip` encoding.
     Gzip,
+
     /// The `deflate` encoding.
     Deflate,
+
     /// The `compress` encoding.
     Compress,
+
     /// The `identity` encoding.
     Identity,
+
     /// The `trailers` encoding.
     Trailers,
+
     /// The `zstd` encoding.
     Zstd,
+
     /// Some other encoding that is less common, can be any String.
     EncodingExt(String),
 }

src/http/header/macros.rs

@@ -4,11 +4,14 @@ macro_rules! common_header_test_module {
         mod $tm {
             #![allow(unused_imports)]
 
-            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)*
         }
     }
@@ -19,22 +22,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()
@@ -56,14 +59,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 exp: Option<HeaderField> = $exp;
+
+            let exp: ::core::option::Option<HeaderField> = $exp;
 
             // test parsing
             assert_eq!(val.ok(), exp);
@@ -122,6 +128,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();
@@ -142,10 +149,19 @@ macro_rules! common_header {
             fn name() -> $crate::http::header::HeaderName {
                 $name
             }
+
             #[inline]
-            fn parse<M: $crate::HttpMessage>(msg: &M) -> Result<Self, $crate::error::ParseError> {
+            fn parse<M: $crate::HttpMessage>(msg: &M) -> Result<Self, $crate::error::ParseError>{
-                $crate::http::header::from_comma_delimited(
+                let headers = msg.headers().get_all(Self::name());
-                    msg.headers().get_all(Self::name())).map($id)
+
+                $crate::http::header::from_comma_delimited(headers)
+                    .and_then(|items| {
+                        if items.is_empty() {
+                            Err($crate::error::ParseError::Header)
+                        } else {
+                            Ok($id(items))
+                        }
+                    })
             }
         }
 
@@ -159,6 +175,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();
@@ -197,6 +214,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()
             }
@@ -251,6 +269,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();

src/http/header/mod.rs

@@ -17,9 +17,9 @@ use bytes::{Bytes, BytesMut};
 // - header parsing utils
 pub use actix_http::http::header::*;
 
-mod accept_charset;
-// mod accept_encoding;
 mod accept;
+mod accept_charset;
+mod accept_encoding;
 mod accept_language;
 mod allow;
 mod cache_control;
@@ -46,9 +46,9 @@ mod preference;
 pub(crate) use macros::common_header_test;
 pub(crate) use macros::{common_header, common_header_test_module};
 
-pub use self::accept_charset::AcceptCharset;
-//pub use self::accept_encoding::AcceptEncoding;
 pub use self::accept::Accept;
+pub use self::accept_charset::AcceptCharset;
+pub use self::accept_encoding::AcceptEncoding;
 pub use self::accept_language::AcceptLanguage;
 pub use self::allow::Allow;
 pub use self::cache_control::{CacheControl, CacheDirective};