diff --git a/actix-http/src/header/utils.rs b/actix-http/src/header/utils.rs index f4fec9335..c6f77848a 100644 --- a/actix-http/src/header/utils.rs +++ b/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]); + } +} diff --git a/src/http/header/accept.rs b/src/http/header/accept.rs index 4c3d9da0a..ba67e3e35 100644 --- a/src/http/header/accept.rs +++ b/src/http/header/accept.rs @@ -77,7 +77,7 @@ crate::http::header::common_header! { /// ]) /// ); /// ``` - (Accept, header::ACCEPT) => (QualityItem)+ + (Accept, header::ACCEPT) => (QualityItem)* 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 = TestRequest::default().insert_header((crate::http::header::ACCEPT, "chunk#;e")).finish(); + let req = test::TestRequest::default() + .insert_header((header::ACCEPT, "chunk#;e")) + .finish(); let header = Accept::parse(&req); assert!(header.is_ok()); } diff --git a/src/http/header/accept_language.rs b/src/http/header/accept_language.rs index 6f47b306d..e4b8e2449 100644 --- a/src/http/header/accept_language.rs +++ b/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"], diff --git a/src/http/header/cache_control.rs b/src/http/header/cache_control.rs index 27cf30ce4..8c348aa0d 100644 --- a/src/http/header/cache_control.rs +++ b/src/http/header/cache_control.rs @@ -1,92 +1,100 @@ -use std::fmt::{self, Write}; -use std::str::FromStr; - -use derive_more::{Deref, DerefMut}; - -use super::{fmt_comma_delimited, from_comma_delimited, Header, IntoHeaderValue, Writer}; +use std::{fmt, str}; +use super::common_header; use crate::http::header; -/// `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 -/// 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 -/// Cache-Control = 1#cache-directive -/// cache-directive = token [ "=" ( token / quoted-string ) ] -/// ``` -/// -/// # Example Values -/// -/// * `no-cache` -/// * `private, community="UCI"` -/// * `max-age=30` -/// -/// # Examples -/// ``` -/// use actix_web::HttpResponse; -/// use actix_web::http::header::{CacheControl, CacheDirective}; -/// -/// let mut builder = HttpResponse::Ok(); -/// builder.insert_header(CacheControl(vec![CacheDirective::MaxAge(86400u32)])); -/// ``` -/// -/// ``` -/// use actix_web::HttpResponse; -/// use actix_web::http::header::{CacheControl, CacheDirective}; -/// -/// let mut builder = HttpResponse::Ok(); -/// builder.insert_header(CacheControl(vec![ -/// CacheDirective::NoCache, -/// CacheDirective::Private, -/// CacheDirective::MaxAge(360u32), -/// CacheDirective::Extension("foo".to_owned(), Some("bar".to_owned())), -/// ])); -/// ``` -#[derive(Debug, Clone, PartialEq, Eq, Deref, DerefMut)] -pub struct CacheControl(pub Vec); +common_header! { + /// `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 + /// 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 + /// ```text + /// Cache-Control = 1#cache-directive + /// cache-directive = token [ "=" ( token / quoted-string ) ] + /// ``` + /// + /// # Example Values + /// + /// * `no-cache` + /// * `private, community="UCI"` + /// * `max-age=30` + /// + /// # Examples + /// ``` + /// use actix_web::HttpResponse; + /// use actix_web::http::header::{CacheControl, CacheDirective}; + /// + /// let mut builder = HttpResponse::Ok(); + /// builder.insert_header(CacheControl(vec![CacheDirective::MaxAge(86400u32)])); + /// ``` + /// + /// ``` + /// use actix_web::HttpResponse; + /// use actix_web::http::header::{CacheControl, CacheDirective}; + /// + /// let mut builder = HttpResponse::Ok(); + /// builder.insert_header(CacheControl(vec![ + /// CacheDirective::NoCache, + /// CacheDirective::Private, + /// CacheDirective::MaxAge(360u32), + /// CacheDirective::Extension("foo".to_owned(), Some("bar".to_owned())), + /// ])); + /// ``` -// TODO: this could just be the crate::http::header::common_header! macro -impl Header for CacheControl { - fn name() -> header::HeaderName { - header::CACHE_CONTROL - } + (CacheControl, header::CACHE_CONTROL) => (CacheDirective)+ - #[inline] - fn parse(msg: &T) -> Result - 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) + test_parse_and_format { + 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)])) + ) } } } -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 { - 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 +134,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), - MaxStale(secs) => return write!(f, "max-stale={}", secs), - MinFresh(secs) => return write!(f, "min-fresh={}", secs), + let dir_str = match self { + NoCache => "no-cache", + NoStore => "no-store", + NoTransform => "no-transform", + OnlyIfCached => "only-if-cached", - MustRevalidate => "must-revalidate", - Public => "public", - Private => "private", - ProxyRevalidate => "proxy-revalidate", - SMaxAge(secs) => return write!(f, "s-maxage={}", secs), + MaxAge(secs) => return write!(f, "max-age={}", secs), + MaxStale(secs) => return write!(f, "max-stale={}", secs), + MinFresh(secs) => return write!(f, "min-fresh={}", secs), - Extension(ref name, None) => &name[..], - Extension(ref name, Some(ref arg)) => { - return write!(f, "{}={}", name, arg); - } - }, - f, - ) + MustRevalidate => "must-revalidate", + Public => "public", + Private => "private", + ProxyRevalidate => "proxy-revalidate", + SMaxAge(secs) => return write!(f, "s-maxage={}", secs), + + Extension(name, None) => name.as_str(), + Extension(name, Some(arg)) => return write!(f, "{}={}", name, arg), + }; + + f.write_str(dir_str) } } -impl FromStr for CacheDirective { - type Err = Option<::Err>; - fn from_str(s: &str) -> Result::Err>> { +impl str::FromStr for CacheDirective { + type Err = Option<::Err>; + + fn from_str(s: &str) -> Result { 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 = Header::parse(&req); - assert_eq!(cache.ok(), None) - } -} diff --git a/src/http/header/macros.rs b/src/http/header/macros.rs index dd36e7e7e..02004c013 100644 --- a/src/http/header/macros.rs +++ b/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 actix_http::http::Method; - use mime::*; - use $crate::http::header::*; + use ::core::str; + + use ::actix_http::{http::Method, test}; + 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> = raw.iter().map(|x| x.to_vec()).collect(); + let headers = raw.iter().map(|x| x.to_vec()).collect::>(); let mut req = test::TestRequest::default(); - for item in a { - req = req.insert_header((HeaderField::name(), item)).take(); + for item in headers { + 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 = result .to_ascii_lowercase() @@ -55,14 +58,17 @@ macro_rules! common_header_test { fn $id() { use actix_http::test; - let a: Vec> = $raw.iter().map(|x| x.to_vec()).collect(); + let headers = $raw.iter().map(|x| x.to_vec()).collect::>(); 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 = $typed; + + let typed: ::core::option::Option = $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(msg: &T) -> Result - where T: $crate::HttpMessage - { - $crate::http::header::from_comma_delimited( - msg.headers().get_all(Self::name())).map($id) + fn parse(msg: &T) -> Result{ + let headers = msg.headers().get_all(Self::name()); + println!("{:?}", &headers); + $crate::http::header::from_comma_delimited(headers) + .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();