fix cache-control bugs & polish

Author: yoshuawuyts

src/cache/cache_control/cache_control.rs

@@ -7,6 +7,28 @@ use std::option;
 use std::slice;
 
 /// A Cache-Control header.
+///
+/// # Examples
+///
+/// ```
+/// # fn main() -> http_types::Result<()> {
+/// #
+/// use http_types::Response;
+/// use http_types::cache::{CacheControl, CacheDirective};
+/// let mut entries = CacheControl::new();
+/// entries.push(CacheDirective::Immutable);
+/// entries.push(CacheDirective::NoStore);
+///
+/// let mut res = Response::new(200);
+/// entries.apply(&mut res);
+///
+/// let entries = CacheControl::from_headers(res)?.unwrap();
+/// let mut entries = entries.iter();
+/// assert_eq!(entries.next().unwrap(), &CacheDirective::Immutable);
+/// assert_eq!(entries.next().unwrap(), &CacheDirective::NoStore);
+/// #
+/// # Ok(()) }
+/// ```
 pub struct CacheControl {
     entries: Vec<CacheDirective>,
 }
@@ -29,13 +51,12 @@ impl CacheControl {
             for part in value.as_str().trim().split(',') {
                 // Try and parse a directive from a str. If the directive is
                 // unkown we skip it.
-                let s = part.trim_start();
-                s.to_lowercase();
-                if let Some(entry) = CacheDirective::from_str(s)? {
+                if let Some(entry) = CacheDirective::from_str(part)? {
                     entries.push(entry);
                 }
             }
         }
+
         Ok(Some(Self { entries }))
     }
 

src/cache/cache_control/cache_directive.rs

@@ -4,6 +4,7 @@ use crate::Status;
 use std::time::Duration;
 
 /// An HTTP `Cache-Control` directive.
+#[non_exhaustive]
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum CacheDirective {
     /// The response body will not change over time.
@@ -76,6 +77,15 @@ impl CacheDirective {
     // sense.
     pub(crate) fn from_str(s: &str) -> crate::Result<Option<Self>> {
         use CacheDirective::*;
+
+        let s = s.trim();
+
+        // We're dealing with an empty string.
+        if s.is_empty() {
+            return Ok(None);
+        }
+
+        s.to_lowercase();
         let mut parts = s.split('=');
         let next = parts.next().unwrap().clone();
 
@@ -104,9 +114,10 @@ impl CacheDirective {
                 }
                 None => Some(MaxStale(None)),
             },
-            "min-fresh=<seconds>" => Some(MinFresh(get_dur()?)),
-            "max-age=<seconds>" => Some(MaxAge(get_dur()?)),
-            "s-maxage=<seconds>" => Some(SMaxAge(get_dur()?)),
+            "min-fresh" => Some(MinFresh(get_dur()?)),
+            "s-maxage" => Some(SMaxAge(get_dur()?)),
+            "stale-if-error" => Some(StaleIfError(get_dur()?)),
+            "stale-while-revalidate" => Some(StaleWhileRevalidate(get_dur()?)),
             _ => None,
         };
         Ok(res)

src/cache/cache_control/mod.rs

@@ -15,7 +15,7 @@ pub use cache_directive::CacheDirective;
 #[cfg(test)]
 mod test {
     use super::*;
-    use crate::headers::Headers;
+    use crate::headers::{Headers, CACHE_CONTROL};
 
     #[test]
     fn smoke() -> crate::Result<()> {
@@ -32,4 +32,23 @@ mod test {
         assert_eq!(entries.next().unwrap(), &CacheDirective::NoStore);
         Ok(())
     }
+
+    #[test]
+    fn ignore_unkonwn_directives() -> crate::Result<()> {
+        let mut headers = Headers::new();
+        headers.insert(CACHE_CONTROL, "barrel_roll");
+        let entries = CacheControl::from_headers(headers)?.unwrap();
+        let mut entries = entries.iter();
+        assert!(entries.next().is_none());
+        Ok(())
+    }
+
+    #[test]
+    fn bad_request_on_parse_error() -> crate::Result<()> {
+        let mut headers = Headers::new();
+        headers.insert(CACHE_CONTROL, "min-fresh=0.9"); // floats are not supported
+        let err = CacheControl::from_headers(headers).unwrap_err();
+        assert_eq!(err.status(), 400);
+        Ok(())
+    }
 }

src/lib.rs

@@ -96,7 +96,7 @@
 #![deny(missing_debug_implementations, nonstandard_style)]
 #![warn(missing_docs, unreachable_pub)]
 #![allow(clippy::new_without_default)]
-// #![cfg_attr(test, deny(warnings))]
+#![cfg_attr(test, deny(warnings))]
 #![cfg_attr(feature = "docs", feature(doc_cfg))]
 #![doc(html_favicon_url = "https://yoshuawuyts.com/assets/http-rs/favicon.ico")]
 #![doc(html_logo_url = "https://yoshuawuyts.com/assets/http-rs/logo-rounded.png")]