CookieStore cleanup/fixes

* Remove TODO
* Remove Cookie::set_ setters
* Do not expose SameSite enum, provide getters on Cookie instead
* Simplify Response::cookies signature (now ignores errors)

src/async_impl/client.rs

@@ -546,7 +546,6 @@ impl Client {
                     .collect::<Vec<_>>()
                     .join("; ");
                 if !header.is_empty() {
-                    // TODO: is it safe to unwrap here? Investigate if Cookie content is always valid.
                     headers.insert(::header::COOKIE, HeaderValue::from_bytes(header.as_bytes()).unwrap());
                 }
             }

src/async_impl/response.rs

@@ -71,10 +71,11 @@ impl Response {
     }
 
     /// Retrieve the cookies contained in the response.
-    pub fn cookies<'a>(&'a self) -> impl Iterator<
+    /// 
-        Item = Result<cookie::Cookie<'a>, cookie::CookieParseError>
+    /// Note that invalid 'Set-Cookie' headers will be ignored.
-    > + 'a {
+    pub fn cookies<'a>(&'a self) -> impl Iterator<Item = cookie::Cookie<'a>> + 'a {
         cookie::extract_response_cookies(&self.headers)
+            .filter_map(Result::ok)
     }
 
     /// Get the final `Url` of this `Response`.

src/cookie.rs

@@ -17,49 +17,6 @@ fn tm_to_systemtime(tm: ::time::Tm) -> SystemTime {
     }
 }
 
-/// Convert a SystemTime to time::Tm.
-/// Returns None if the conversion failed.
-fn systemtime_to_tm(time: SystemTime) -> Option<::time::Tm> {
-    let seconds = match time.duration_since(SystemTime::UNIX_EPOCH) {
-        Ok(duration) => duration.as_secs() as i64,
-        Err(_) => {
-            if let Ok(duration) = SystemTime::UNIX_EPOCH.duration_since(time) {
-                (duration.as_secs() as i64) * -1
-            } else {
-                return None;
-            }
-        }
-    };
-    Some(::time::at_utc(::time::Timespec::new(seconds, 0)))
-}
-
-/// Represents the 'SameSite' attribute of a cookie.
-#[derive(PartialEq, Eq, Clone, Copy, Debug)]
-pub enum SameSite {
-    /// Strict same-site policy.
-    Strict,
-    /// Lax same-site policy.
-    Lax,
-}
-
-impl SameSite {
-    fn from_inner(value: cookie_crate::SameSite) -> Option<Self> {
-        match value {
-            cookie_crate::SameSite::Strict => Some(SameSite::Strict),
-            cookie_crate::SameSite::Lax => Some(SameSite::Lax),
-            cookie_crate::SameSite::None => None,
-        }
-    }
-
-    fn to_inner(value: Option<Self>) -> cookie_crate::SameSite {
-        match value {
-            Some(SameSite::Strict) => cookie_crate::SameSite::Strict,
-            Some(SameSite::Lax) => cookie_crate::SameSite::Lax,
-            None => cookie_crate::SameSite::None,
-        }
-    }
-}
-
 /// Error representing a parse failure of a 'Set-Cookie' header.
 pub struct CookieParseError(cookie::ParseError);
 
@@ -115,49 +72,29 @@ impl<'a> Cookie<'a> {
         self.0.name()
     }
 
-    /// Set the cookie name.
-    pub fn set_name<P: Into<Cow<'static, str>>>(&mut self, name: P) {
-        self.0.set_name(name)
-    }
-
     /// The value of the cookie.
     pub fn value(&self) -> &str {
         self.0.value()
     }
 
-    /// Set the cookie value.
-    pub fn set_value<P: Into<Cow<'static, str>>>(&mut self, value: P) {
-        self.0.set_value(value)
-    }
-
     /// Returns true if the 'HttpOnly' directive is enabled.
     pub fn http_only(&self) -> bool {
         self.0.http_only().unwrap_or(false)
     }
 
-    /// Set the 'HttpOnly' directive.
-    pub fn set_http_only(&mut self, value: bool) {
-        self.0.set_http_only(value)
-    }
-
     /// Returns true if the 'Secure' directive is enabled.
     pub fn secure(&self) -> bool {
         self.0.secure().unwrap_or(false)
     }
 
-    /// Set the 'Secure' directive.
+    /// Returns true if  'SameSite' directive is 'Lax'.
-    pub fn set_secure(&mut self, value: bool) {
+    pub fn same_site_lax(&self) -> bool {
-        self.0.set_secure(value)
+        self.0.same_site() == Some(cookie_crate::SameSite::Lax)
     }
 
-    /// Returns the 'SameSite' directive if present.
+    /// Returns true if  'SameSite' directive is 'Strict'.
-    pub fn same_site(&self) -> Option<SameSite> {
+    pub fn same_site_strict(&self) -> bool {
-        self.0.same_site().and_then(SameSite::from_inner)
+        self.0.same_site() == Some(cookie_crate::SameSite::Strict)
-    }
-
-    /// Set the 'SameSite" directive.
-    pub fn set_same_site(&mut self, value: Option<SameSite>) {
-        self.0.set_same_site(SameSite::to_inner(value))
     }
 
     /// Returns the path directive of the cookie, if set.
@@ -165,21 +102,11 @@ impl<'a> Cookie<'a> {
         self.0.path()
     }
 
-    /// Set the cookie path.
-    pub fn set_path<P: Into<Cow<'static, str>>>(&mut self, path: P) {
-        self.0.set_path(path)
-    }
-
     /// Returns the domain directive of the cookie, if set.
     pub fn domain(&self) -> Option<&str> {
         self.0.domain()
     }
 
-    /// Set the cookie domain.
-    pub fn set_domain<P: Into<Cow<'static, str>>>(&mut self, domain: P) {
-        self.0.set_domain(domain)
-    }
-
     /// Get the Max-Age information.
     pub fn max_age(&self) -> Option<std::time::Duration> {
         self.0.max_age().map(|d| std::time::Duration::new(d.num_seconds() as u64, 0))
@@ -189,15 +116,6 @@ impl<'a> Cookie<'a> {
     pub fn expires(&self) -> Option<SystemTime> {
         self.0.expires().map(tm_to_systemtime)
     }
-
-    /// Set expiration time.
-    ///
-    /// Currently, providing a `None` value will have no effect.
-    pub fn set_expires(&mut self, value: Option<SystemTime>) {
-        if let Some(tm) = value.and_then(systemtime_to_tm) {
-            self.0.set_expires(tm);
-        }
-    }
 }
 
 pub(crate) fn extract_response_cookies<'a>(

src/response.rs

@@ -124,10 +124,11 @@ impl Response {
     }
 
     /// Retrieve the cookies contained in the response.
-    pub fn cookies<'a>(&'a self) -> impl Iterator<
+    /// 
-        Item = Result<cookie::Cookie<'a>, cookie::CookieParseError>
+    /// Note that invalid 'Set-Cookie' headers will be ignored.
-    > + 'a {
+    pub fn cookies<'a>(&'a self) -> impl Iterator< Item = cookie::Cookie<'a> > + 'a {
         cookie::extract_response_cookies(self.headers())
+            .filter_map(Result::ok)
     }
 
 

tests/cookie.rs

@@ -36,7 +36,7 @@ fn cookie_response_accessor() {
     let url = format!("http://{}/", server.addr());
     let res = rt.block_on(client.get(&url).send()).unwrap();
 
-    let cookies = res.cookies().map(|c| c.unwrap()).collect::<Vec<_>>();
+    let cookies = res.cookies().collect::<Vec<_>>();
 
     // key=val
     assert_eq!(cookies[0].name(), "key");
@@ -71,11 +71,11 @@ fn cookie_response_accessor() {
 
     // samesitelax
     assert_eq!(cookies[7].name(), "samesitelax");
-    assert_eq!(cookies[7].same_site().unwrap(), reqwest::cookie::SameSite::Lax);
+    assert!(cookies[7].same_site_lax());
 
     // samesitestrict
     assert_eq!(cookies[8].name(), "samesitestrict");
-    assert_eq!(cookies[8].same_site().unwrap(), reqwest::cookie::SameSite::Strict);
+    assert!(cookies[8].same_site_strict());
 }
 
 #[test]